From 0bef51ec76f19ff2f1eec1d80fdcdc1469eb35c1 Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Fri, 29 Aug 2025 11:39:41 +0800
Subject: [PATCH] ackhandler: account for skipped packets in packet threshold
 calculation (#5316)

---
 internal/ackhandler/sent_packet_handler.go    |  2 +-
 internal/ackhandler/sent_packet_history.go    | 21 +++++++++++++++
 .../ackhandler/sent_packet_history_test.go    | 26 +++++++++++++++++++
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/internal/ackhandler/sent_packet_handler.go b/internal/ackhandler/sent_packet_handler.go
index 5e4623a87..b4a113e31 100644
--- a/internal/ackhandler/sent_packet_handler.go
+++ b/internal/ackhandler/sent_packet_handler.go
@@ -710,7 +710,7 @@ func (h *sentPacketHandler) detectLostPackets(now time.Time, encLevel protocol.E
 					h.tracer.LostPacket(p.EncryptionLevel, pn, logging.PacketLossTimeThreshold)
 				}
 			}
-		} else if pnSpace.largestAcked >= pn+packetThreshold {
+		} else if pnSpace.history.Difference(pnSpace.largestAcked, pn) >= packetThreshold {
 			packetLost = true
 			if !p.isPathProbePacket {
 				if h.logger.Debug() {
diff --git a/internal/ackhandler/sent_packet_history.go b/internal/ackhandler/sent_packet_history.go
index c92c642c9..3dc1de40b 100644
--- a/internal/ackhandler/sent_packet_history.go
+++ b/internal/ackhandler/sent_packet_history.go
@@ -254,3 +254,24 @@ func (h *sentPacketHistory) DeclareLost(pn protocol.PacketNumber) {
 		h.cleanupStart()
 	}
 }
+
+// Difference returns the difference between two packet numbers a and b (a - b),
+// taking into account any skipped packet numbers between them.
+//
+// Note that old skipped packets are garbage collected at some point,
+// so this function is not guaranteed to return the correct result after a while.
+func (h *sentPacketHistory) Difference(a, b protocol.PacketNumber) protocol.PacketNumber {
+	diff := a - b
+	if len(h.skippedPackets) == 0 {
+		return diff
+	}
+	if a < h.skippedPackets[0] || b > h.skippedPackets[len(h.skippedPackets)-1] {
+		return diff
+	}
+	for _, p := range h.skippedPackets {
+		if p > b && p < a {
+			diff--
+		}
+	}
+	return diff
+}
diff --git a/internal/ackhandler/sent_packet_history_test.go b/internal/ackhandler/sent_packet_history_test.go
index 8c08923ad..62b3a3240 100644
--- a/internal/ackhandler/sent_packet_history_test.go
+++ b/internal/ackhandler/sent_packet_history_test.go
@@ -296,3 +296,29 @@ func TestSentPacketHistoryPathProbes(t *testing.T) {
 	require.Equal(t, protocol.InvalidPacketNumber, pn)
 	require.Nil(t, p)
 }
+
+func TestSentPacketHistoryDifference(t *testing.T) {
+	hist := newSentPacketHistory(true)
+	hist.SentNonAckElicitingPacket(0)
+	hist.SentAckElicitingPacket(1, &packet{})
+	hist.SentAckElicitingPacket(2, &packet{})
+	hist.SentAckElicitingPacket(3, &packet{})
+	hist.SkippedPacket(4)
+	hist.SkippedPacket(5)
+	hist.SentAckElicitingPacket(6, &packet{})
+	hist.SentNonAckElicitingPacket(7)
+	hist.SkippedPacket(8)
+	hist.SentAckElicitingPacket(9, &packet{})
+
+	require.Zero(t, hist.Difference(1, 1))
+	require.Zero(t, hist.Difference(2, 2))
+	require.Zero(t, hist.Difference(7, 7))
+
+	require.Equal(t, protocol.PacketNumber(1), hist.Difference(2, 1))
+	require.Equal(t, protocol.PacketNumber(2), hist.Difference(3, 1))
+	require.Equal(t, protocol.PacketNumber(3), hist.Difference(4, 1))
+	require.Equal(t, protocol.PacketNumber(3), hist.Difference(6, 1)) // 4 and 5 were skipped
+	require.Equal(t, protocol.PacketNumber(4), hist.Difference(7, 1)) // 4 and 5 were skipped
+	require.Equal(t, protocol.PacketNumber(3), hist.Difference(7, 2)) // 4 and 5 were skipped
+	require.Equal(t, protocol.PacketNumber(5), hist.Difference(9, 1)) // 4, 5 and 8 were skipped
+}