Date: Fri, 29 Oct 2021 07:19:22 GMT From: Randall Stewart <rrs@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: aeda85278255 - main - tcp: Rack at times can miscalculate the RTT from what it thinks is a persists probe respone. Message-ID: <202110290719.19T7JM26069051@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by rrs: URL: https://cgit.FreeBSD.org/src/commit/?id=aeda8527825525cfd75bfbcae7bc895cee17f04b commit aeda8527825525cfd75bfbcae7bc895cee17f04b Author: Randall Stewart <rrs@FreeBSD.org> AuthorDate: 2021-10-29 07:17:43 +0000 Commit: Randall Stewart <rrs@FreeBSD.org> CommitDate: 2021-10-29 07:17:43 +0000 tcp: Rack at times can miscalculate the RTT from what it thinks is a persists probe respone. Turns out that if a peer sends in a window update right after rack fires off a persists probe, we can mis-interpret the window update and calculate a bogus RTT (very short). We still process the window update and send the data but we incorrectly generate an RTT. We should be only doing the RTT stuff if the rwnd is still small and has not changed. Reviewed by: Michael Tuexen Sponsored by: Netflix Inc. Differential Revision: https://reviews.freebsd.org/D32717 --- sys/netinet/tcp_stacks/rack.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/sys/netinet/tcp_stacks/rack.c b/sys/netinet/tcp_stacks/rack.c index a92e43205f09..04252511ad18 100644 --- a/sys/netinet/tcp_stacks/rack.c +++ b/sys/netinet/tcp_stacks/rack.c @@ -5363,8 +5363,6 @@ rack_get_persists_timer_val(struct tcpcb *tp, struct tcp_rack *rack) t = (tp->t_srtt + (tp->t_rttvar << 2)); RACK_TCPT_RANGESET(tt, t * tcp_backoff[tp->t_rxtshift], rack_persist_min, rack_persist_max, rack->r_ctl.timer_slop); - if (tp->t_rxtshift < TCP_MAXRXTSHIFT) - tp->t_rxtshift++; rack->r_ctl.rc_hpts_flags |= PACE_TMR_PERSIT; ret_val = (uint32_t)tt; return (ret_val); @@ -14448,11 +14446,20 @@ rack_do_segment_nounlock(struct mbuf *m, struct tcphdr *th, struct socket *so, * at least use timestamps if available to validate). */ rack->forced_ack = 0; - us_rtt = us_cts - rack->r_ctl.forced_ack_ts; - if (us_rtt == 0) - us_rtt = 1; - rack_apply_updated_usrtt(rack, us_rtt, us_cts); - tcp_rack_xmit_timer(rack, us_rtt, 0, us_rtt, 3, NULL, 1); + if (tiwin == tp->snd_wnd) { + /* + * Only apply the RTT update if this is + * a response to our window probe. And that + * means the rwnd sent must match the current + * snd_wnd. If it does not, then we got a + * window update ack instead. + */ + us_rtt = us_cts - rack->r_ctl.forced_ack_ts; + if (us_rtt == 0) + us_rtt = 1; + rack_apply_updated_usrtt(rack, us_rtt, us_cts); + tcp_rack_xmit_timer(rack, us_rtt, 0, us_rtt, 3, NULL, 1); + } } /* * This is the one exception case where we set the rack state
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202110290719.19T7JM26069051>