git: b84f41b4e82d - main - tcp: properly reset sackhint values when SACK recovery is done

From: Gleb Smirnoff <glebius_at_FreeBSD.org>
Date: Mon, 13 Jan 2025 18:15:21 UTC
The branch main has been updated by glebius:

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

commit b84f41b4e82df373f8e682d45791b6ab636cd94e
Author:     Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2025-01-13 18:13:45 +0000
Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2025-01-13 18:13:45 +0000

    tcp: properly reset sackhint values when SACK recovery is done
    
    When the SACK scoreboard collapses, properly clear all the counters.
    The counters are used in tcp_compute_pipe(), which can be called
    anytime later after the SACK recovery.  The returned result can be
    totally bogus, including both too large and too small values.
    
    PR:                     283649
    Reviewed by:            rscheff
    Differential Revision:  https://reviews.freebsd.org/D48236
---
 sys/netinet/tcp_sack.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/sys/netinet/tcp_sack.c b/sys/netinet/tcp_sack.c
index f642acd4c46a..90d789f0e224 100644
--- a/sys/netinet/tcp_sack.c
+++ b/sys/netinet/tcp_sack.c
@@ -653,8 +653,6 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
 		 * scoreboard).
 		 */
 		tp->snd_fack = SEQ_MAX(tp->snd_una, th_ack);
-		tp->sackhint.sacked_bytes = 0;	/* reset */
-		tp->sackhint.hole_bytes = 0;
 	}
 	/*
 	 * In the while-loop below, incoming SACK blocks (sack_blocks[]) and
@@ -870,12 +868,26 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
 		}
 	}
 
-	KASSERT(!(TAILQ_EMPTY(&tp->snd_holes) && (tp->sackhint.hole_bytes != 0)),
-	    ("SACK scoreboard empty, but accounting non-zero\n"));
-
+	KASSERT(delivered_data >= 0, ("delivered_data < 0"));
 	KASSERT(notlost_bytes <= tp->sackhint.hole_bytes,
 	    ("SACK: more bytes marked notlost than in scoreboard holes"));
 
+	if (TAILQ_EMPTY(&tp->snd_holes)) {
+		KASSERT(tp->sackhint.hole_bytes == 0,
+		    ("SACK scoreboard empty, but accounting non-zero\n"));
+		tp->sackhint.sack_bytes_rexmit = 0;
+		tp->sackhint.sacked_bytes = 0;
+		tp->sackhint.lost_bytes = 0;
+	} else {
+		KASSERT(tp->sackhint.hole_bytes > 0,
+		    ("SACK scoreboard not empty, but has no bytes\n"));
+		tp->sackhint.delivered_data = delivered_data;
+		tp->sackhint.sacked_bytes += delivered_data - left_edge_delta;
+		KASSERT((tp->sackhint.sacked_bytes >= 0), ("sacked_bytes < 0"));
+		tp->sackhint.lost_bytes = tp->sackhint.hole_bytes -
+		    notlost_bytes;
+	}
+
 	if (!(to->to_flags & TOF_SACK))
 		/*
 		 * If this ACK did not contain any
@@ -886,11 +898,6 @@ tcp_sack_doack(struct tcpcb *tp, struct tcpopt *to, tcp_seq th_ack)
 		 * for RFC6675 rescue retransmission.
 		 */
 		sack_changed = SACK_NOCHANGE;
-	tp->sackhint.delivered_data = delivered_data;
-	tp->sackhint.sacked_bytes += delivered_data - left_edge_delta;
-	tp->sackhint.lost_bytes = tp->sackhint.hole_bytes - notlost_bytes;
-	KASSERT((delivered_data >= 0), ("delivered_data < 0"));
-	KASSERT((tp->sackhint.sacked_bytes >= 0), ("sacked_bytes < 0"));
 	return (sack_changed);
 }