Date: Thu, 25 Feb 2021 17:34:20 GMT From: Richard Scheffenegger <rscheff@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 48396dc77922 - main - Address two incorrect calculations and enhance readability of PRR code Message-ID: <202102251734.11PHYKkH014729@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by rscheff: URL: https://cgit.FreeBSD.org/src/commit/?id=48396dc77922c68377ecac0ead2f8b0b5453c451 commit 48396dc77922c68377ecac0ead2f8b0b5453c451 Author: Richard Scheffenegger <rscheff@FreeBSD.org> AuthorDate: 2021-02-25 16:59:45 +0000 Commit: Richard Scheffenegger <rscheff@FreeBSD.org> CommitDate: 2021-02-25 17:32:04 +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 --- 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 59a5a2d6bf34..ca4a4c833dc2 100644 --- a/sys/netinet/tcp_input.c +++ b/sys/netinet/tcp_input.c @@ -2570,8 +2570,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 @@ -2588,39 +2588,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; @@ -3948,7 +3938,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); @@ -3974,29 +3964,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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202102251734.11PHYKkH014729>