git: b84f41b4e82d - main - tcp: properly reset sackhint values when SACK recovery is done
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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);
}