From owner-svn-src-head@freebsd.org Fri Jun 12 19:56:20 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 310FD343C4F; Fri, 12 Jun 2020 19:56:20 +0000 (UTC) (envelope-from rrs@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 "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 49kBLX0Y11z3yx5; Fri, 12 Jun 2020 19:56:20 +0000 (UTC) (envelope-from rrs@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 0E10BF054; Fri, 12 Jun 2020 19:56:20 +0000 (UTC) (envelope-from rrs@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 05CJuJje017611; Fri, 12 Jun 2020 19:56:19 GMT (envelope-from rrs@FreeBSD.org) Received: (from rrs@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 05CJuJL2017608; Fri, 12 Jun 2020 19:56:19 GMT (envelope-from rrs@FreeBSD.org) Message-Id: <202006121956.05CJuJL2017608@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: rrs set sender to rrs@FreeBSD.org using -f From: Randall Stewart Date: Fri, 12 Jun 2020 19:56:19 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r362113 - in head/sys/netinet: . tcp_stacks X-SVN-Group: head X-SVN-Commit-Author: rrs X-SVN-Commit-Paths: in head/sys/netinet: . tcp_stacks X-SVN-Commit-Revision: 362113 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 12 Jun 2020 19:56:20 -0000 Author: rrs Date: Fri Jun 12 19:56:19 2020 New Revision: 362113 URL: https://svnweb.freebsd.org/changeset/base/362113 Log: So it turns out with the right window scaling you can get the code in all stacks to always want to do a window update, even when no data can be sent. Now in cases where you are not pacing thats probably ok, you just send an extra window update or two. However with bbr (and rack if its paced) every time the pacer goes off its going to send a "window update". Also in testing bbr I have found that if we are not responding to data right away we end up staying in startup but incorrectly holding a pacing gain of 192 (a loss). This is because the idle window code does not restict itself to only work with PROBE_BW. In all other states you dont want it doing a PROBE_BW state change. Sponsored by: Netflix Inc. Differential Revision: https://reviews.freebsd.org/D25247 Modified: head/sys/netinet/tcp_output.c head/sys/netinet/tcp_stacks/bbr.c head/sys/netinet/tcp_stacks/rack.c Modified: head/sys/netinet/tcp_output.c ============================================================================== --- head/sys/netinet/tcp_output.c Fri Jun 12 18:41:12 2020 (r362112) +++ head/sys/netinet/tcp_output.c Fri Jun 12 19:56:19 2020 (r362113) @@ -655,7 +655,10 @@ after_sack_rexmit: adv = recwin; if (SEQ_GT(tp->rcv_adv, tp->rcv_nxt)) { oldwin = (tp->rcv_adv - tp->rcv_nxt); - adv -= oldwin; + if (adv > oldwin) + adv -= oldwin; + else + adv = 0; } else oldwin = 0; Modified: head/sys/netinet/tcp_stacks/bbr.c ============================================================================== --- head/sys/netinet/tcp_stacks/bbr.c Fri Jun 12 18:41:12 2020 (r362112) +++ head/sys/netinet/tcp_stacks/bbr.c Fri Jun 12 19:56:19 2020 (r362113) @@ -8078,7 +8078,7 @@ bbr_restart_after_idle(struct tcp_bbr *bbr, uint32_t c bbr->r_ctl.rc_bbr_hptsi_gain = bbr->r_ctl.rc_startup_pg; bbr->r_ctl.rc_bbr_cwnd_gain = bbr->r_ctl.rc_startup_pg; bbr_log_type_statechange(bbr, cts, __LINE__); - } else { + } else if (bbr->rc_bbr_state == BBR_STATE_PROBE_BW) { bbr_substate_change(bbr, cts, __LINE__, 1); } } @@ -12000,21 +12000,27 @@ bbr_window_update_needed(struct tcpcb *tp, struct sock * "adv" is the amount we could increase the window, taking into * account that we are limited by TCP_MAXWIN << tp->rcv_scale. */ - uint32_t adv; + int32_t adv; int32_t oldwin; - adv = min(recwin, TCP_MAXWIN << tp->rcv_scale); + adv = recwin; if (SEQ_GT(tp->rcv_adv, tp->rcv_nxt)) { oldwin = (tp->rcv_adv - tp->rcv_nxt); - adv -= oldwin; + if (adv > oldwin) + adv -= oldwin; + else { + /* We can't increase the window */ + adv = 0; + } } else oldwin = 0; /* - * If the new window size ends up being the same as the old size - * when it is scaled, then don't force a window update. + * If the new window size ends up being the same as or less + * than the old size when it is scaled, then don't force + * a window update. */ - if (oldwin >> tp->rcv_scale == (adv + oldwin) >> tp->rcv_scale) + if (oldwin >> tp->rcv_scale >= (adv + oldwin) >> tp->rcv_scale) return (0); if (adv >= (2 * maxseg) && Modified: head/sys/netinet/tcp_stacks/rack.c ============================================================================== --- head/sys/netinet/tcp_stacks/rack.c Fri Jun 12 18:41:12 2020 (r362112) +++ head/sys/netinet/tcp_stacks/rack.c Fri Jun 12 19:56:19 2020 (r362113) @@ -12845,18 +12845,24 @@ again: int32_t adv; int oldwin; - adv = min(recwin, (long)TCP_MAXWIN << tp->rcv_scale); + adv = recwin; if (SEQ_GT(tp->rcv_adv, tp->rcv_nxt)) { oldwin = (tp->rcv_adv - tp->rcv_nxt); - adv -= oldwin; + if (adv > oldwin) + adv -= oldwin; + else { + /* We can't increase the window */ + adv = 0; + } } else oldwin = 0; /* - * If the new window size ends up being the same as the old - * size when it is scaled, then don't force a window update. + * If the new window size ends up being the same as or less + * than the old size when it is scaled, then don't force + * a window update. */ - if (oldwin >> tp->rcv_scale == (adv + oldwin) >> tp->rcv_scale) + if (oldwin >> tp->rcv_scale >= (adv + oldwin) >> tp->rcv_scale) goto dontupdate; if (adv >= (int32_t)(2 * segsiz) &&