From owner-dev-commits-src-branches@freebsd.org Sun Feb 28 08:09:08 2021 Return-Path: Delivered-To: dev-commits-src-branches@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 3075D563AE4; Sun, 28 Feb 2021 08:09:08 +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 4DpGJ40t5Gz3H87; Sun, 28 Feb 2021 08:09:08 +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 0645F52E1; Sun, 28 Feb 2021 08:09:08 +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 11S897Nb067861; Sun, 28 Feb 2021 08:09:07 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 11S897B7067860; Sun, 28 Feb 2021 08:09:07 GMT (envelope-from git) Date: Sun, 28 Feb 2021 08:09:07 GMT Message-Id: <202102280809.11S897B7067860@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Richard Scheffenegger Subject: git: ffbf1b809ef5 - stable/13 - Address two incorrect calculations and enhance readability of PRR code MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rscheff X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: ffbf1b809ef5080afa130c11fa4afce9fef7e7fe Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-branches@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commits to the stable branches of the FreeBSD src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 28 Feb 2021 08:09:08 -0000 The branch stable/13 has been updated by rscheff: URL: https://cgit.FreeBSD.org/src/commit/?id=ffbf1b809ef5080afa130c11fa4afce9fef7e7fe commit ffbf1b809ef5080afa130c11fa4afce9fef7e7fe Author: Richard Scheffenegger AuthorDate: 2021-02-25 16:59:45 +0000 Commit: Richard Scheffenegger CommitDate: 2021-02-28 08:07:54 +0000 Address two incorrect calculations and enhance readability of PRR code - address second instance of cwnd potentially becoming zero - fix sublte bug due to implicit int to uint typecase in max() - fix bug due to typo in hand-coded CEILING() function by using howmany() macro - use int instead of long, and add a missing long typecast - replace if conditionals with easier to read imax/imin (as in pseudocode) Reviewed By: #transport, kbowling MFC after: 3 days Sponsored by: NetApp, Inc. Differential Revision: https://reviews.freebsd.org/D28813 (cherry picked from commit 48396dc77922c68377ecac0ead2f8b0b5453c451) --- sys/netinet/tcp_input.c | 60 ++++++++++++++++++++----------------------------- 1 file changed, 24 insertions(+), 36 deletions(-) diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c index aaa9465c00e9..96c594a4c7cd 100644 --- a/sys/netinet/tcp_input.c +++ b/sys/netinet/tcp_input.c @@ -2577,8 +2577,8 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, struct socket *so, if (V_tcp_do_prr && IN_FASTRECOVERY(tp->t_flags) && (tp->t_flags & TF_SACK_PERMIT)) { - long snd_cnt = 0, limit = 0; - long del_data = 0, pipe = 0; + int snd_cnt = 0, limit = 0; + int del_data = 0, pipe = 0; /* * In a duplicate ACK del_data is only the * diff_in_sack. If no SACK is used del_data @@ -2595,39 +2595,29 @@ tcp_do_segment(struct mbuf *m, struct tcphdr *th, struct socket *so, if (pipe > tp->snd_ssthresh) { if (tp->sackhint.recover_fs == 0) tp->sackhint.recover_fs = - max(1, tp->snd_nxt - tp->snd_una); - snd_cnt = (tp->sackhint.prr_delivered * - tp->snd_ssthresh / - tp->sackhint.recover_fs) + - 1 - tp->sackhint.sack_bytes_rexmit; + imax(1, tp->snd_nxt - tp->snd_una); + snd_cnt = howmany((long)tp->sackhint.prr_delivered * + tp->snd_ssthresh, tp->sackhint.recover_fs) - + tp->sackhint.sack_bytes_rexmit; } else { if (V_tcp_do_prr_conservative) limit = tp->sackhint.prr_delivered - tp->sackhint.sack_bytes_rexmit; else - if ((tp->sackhint.prr_delivered - - tp->sackhint.sack_bytes_rexmit) > - del_data) - limit = tp->sackhint.prr_delivered - - tp->sackhint.sack_bytes_rexmit + - maxseg; - else - limit = del_data + maxseg; - if ((tp->snd_ssthresh - pipe) < limit) - snd_cnt = tp->snd_ssthresh - pipe; - else - snd_cnt = limit; + limit = imax(tp->sackhint.prr_delivered - + tp->sackhint.sack_bytes_rexmit, + del_data) + maxseg; + snd_cnt = imin(tp->snd_ssthresh - pipe, limit); } - snd_cnt = max((snd_cnt / maxseg), 0); + snd_cnt = imax(snd_cnt, 0) / maxseg; /* * Send snd_cnt new data into the network in * response to this ACK. If there is a going * to be a SACK retransmission, adjust snd_cwnd * accordingly. */ - tp->snd_cwnd = tp->snd_nxt - tp->snd_recover + - tp->sackhint.sack_bytes_rexmit + - (snd_cnt * maxseg); + tp->snd_cwnd = imax(maxseg, tp->snd_nxt - tp->snd_recover + + tp->sackhint.sack_bytes_rexmit + (snd_cnt * maxseg)); } else if ((tp->t_flags & TF_SACK_PERMIT) && IN_FASTRECOVERY(tp->t_flags)) { int awnd; @@ -3930,7 +3920,7 @@ tcp_mssopt(struct in_conninfo *inc) void tcp_prr_partialack(struct tcpcb *tp, struct tcphdr *th) { - long snd_cnt = 0, limit = 0, del_data = 0, pipe = 0; + int snd_cnt = 0, limit = 0, del_data = 0, pipe = 0; int maxseg = tcp_maxseg(tp); INP_WLOCK_ASSERT(tp->t_inpcb); @@ -3956,29 +3946,27 @@ tcp_prr_partialack(struct tcpcb *tp, struct tcphdr *th) if (pipe > tp->snd_ssthresh) { if (tp->sackhint.recover_fs == 0) tp->sackhint.recover_fs = - max(1, tp->snd_nxt - tp->snd_una); - snd_cnt = (tp->sackhint.prr_delivered * tp->snd_ssthresh / - tp->sackhint.recover_fs) - tp->sackhint.sack_bytes_rexmit; + imax(1, tp->snd_nxt - tp->snd_una); + snd_cnt = howmany((long)tp->sackhint.prr_delivered * + tp->snd_ssthresh, tp->sackhint.recover_fs) - + tp->sackhint.sack_bytes_rexmit; } else { if (V_tcp_do_prr_conservative) limit = tp->sackhint.prr_delivered - tp->sackhint.sack_bytes_rexmit; else - if ((tp->sackhint.prr_delivered - - tp->sackhint.sack_bytes_rexmit) > del_data) - limit = tp->sackhint.prr_delivered - - tp->sackhint.sack_bytes_rexmit + maxseg; - else - limit = del_data + maxseg; - snd_cnt = min((tp->snd_ssthresh - pipe), limit); + limit = imax(tp->sackhint.prr_delivered - + tp->sackhint.sack_bytes_rexmit, + del_data) + maxseg; + snd_cnt = imin((tp->snd_ssthresh - pipe), limit); } - snd_cnt = max((snd_cnt / maxseg), 0); + snd_cnt = imax(snd_cnt, 0) / maxseg; /* * Send snd_cnt new data into the network in response to this ack. * If there is going to be a SACK retransmission, adjust snd_cwnd * accordingly. */ - tp->snd_cwnd = max(maxseg, (int64_t)tp->snd_nxt - tp->snd_recover + + tp->snd_cwnd = imax(maxseg, tp->snd_nxt - tp->snd_recover + tp->sackhint.sack_bytes_rexmit + (snd_cnt * maxseg)); tp->t_flags |= TF_ACKNOW; (void) tcp_output(tp);