From nobody Fri Oct 29 07:19:22 2021 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 6F01E18321F6; Fri, 29 Oct 2021 07:19:22 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HgYhV2X2cz3QGw; Fri, 29 Oct 2021 07:19:22 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 367AB12E54; Fri, 29 Oct 2021 07:19:22 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 19T7JMBK069052; Fri, 29 Oct 2021 07:19:22 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 19T7JM26069051; Fri, 29 Oct 2021 07:19:22 GMT (envelope-from git) Date: Fri, 29 Oct 2021 07:19:22 GMT Message-Id: <202110290719.19T7JM26069051@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Randall Stewart Subject: git: aeda85278255 - main - tcp: Rack at times can miscalculate the RTT from what it thinks is a persists probe respone. List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rrs X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: aeda8527825525cfd75bfbcae7bc895cee17f04b Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by rrs: URL: https://cgit.FreeBSD.org/src/commit/?id=aeda8527825525cfd75bfbcae7bc895cee17f04b commit aeda8527825525cfd75bfbcae7bc895cee17f04b Author: Randall Stewart AuthorDate: 2021-10-29 07:17:43 +0000 Commit: Randall Stewart 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