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

[Gleb Smirnoff]

The branch main has been updated by glebius:

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

commit b84f41b4e82df373f8e682d45791b6ab636cd94e
Author:     Gleb Smirnoff <gleb...@freebsd.org>
AuthorDate: 2025-01-13 18:13:45 +0000
Commit:     Gleb Smirnoff <gleb...@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);
 }