git: 48396dc77922 - main - Address two incorrect calculations and enhance readability of PRR code

[Richard Scheffenegger]

The branch main has been updated by rscheff:

URL: https://cgit.FreeBSD.org/src/commit/?id=48396dc77922c68377ecac0ead2f8b0b5453c451

commit 48396dc77922c68377ecac0ead2f8b0b5453c451
Author:     Richard Scheffenegger <rscheff at FreeBSD.org>
AuthorDate: 2021-02-25 16:59:45 +0000
Commit:     Richard Scheffenegger <rscheff at 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);