quic: use an enum for sentPacket state

neild · gopherbot · commit 3f563d3b0dee · 2025-04-09T16:02:25.000-07:00
The sentPacket type tracks the state of a packet sent to the peer. Packets can be in progress, acknowledged, or lost. Track this state with an enum rather than a set of bools, to avoid the possibility of nonsensical states such as a packet being both acknowledged and lost. This also simplifies a following change to add an "unsent" state for intentionally skipped packet numbers. Change-Id: I87c8fc399c72337c033ab7ec5ec8db2c56c732f9 Reviewed-on: https://go-review.googlesource.com/c/net/+/664297 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Jonathan Amsterdam <jba@google.com> Auto-Submit: Damien Neil <dneil@google.com>
diff --git a/quic/loss.go b/quic/loss.go
@@ -246,20 +246,20 @@ func (c *lossState) receiveAckRange(now time.Time, space numberSpace, rangeIndex
 		// If the latest packet in the ACK frame is newly-acked,
 		// record the RTT in c.ackFrameRTT.
 		sent := c.spaces[space].num(end - 1)
-		if !sent.acked {
+		if sent.state == sentPacketSent {
 			c.ackFrameRTT = max(0, now.Sub(sent.time))
 		}
 	}
 	for pnum := start; pnum < end; pnum++ {
 		sent := c.spaces[space].num(pnum)
-		if sent.acked || sent.lost {
+		if sent.state != sentPacketSent {
 			continue
 		}
 		// This is a newly-acknowledged packet.
 		if pnum > c.spaces[space].maxAcked {
 			c.spaces[space].maxAcked = pnum
 		}
-		sent.acked = true
+		sent.state = sentPacketAcked
 		c.cc.packetAcked(now, sent)
 		ackf(space, sent, packetAcked)
 		if sent.ackEliciting {
@@ -315,12 +315,12 @@ func (c *lossState) receiveAckEnd(now time.Time, log *slog.Logger, space numberS
 func (c *lossState) discardPackets(space numberSpace, log *slog.Logger, lossf func(numberSpace, *sentPacket, packetFate)) {
 	for i := 0; i < c.spaces[space].size; i++ {
 		sent := c.spaces[space].nth(i)
-		if sent.acked || sent.lost {
+		if sent.state != sentPacketSent {
 			// This should not be possible, since we only discard packets
 			// in spaces which have never received an ack, but check anyway.
 			continue
 		}
-		sent.lost = true
+		sent.state = sentPacketLost
 		c.cc.packetDiscarded(sent)
 		lossf(numberSpace(space), sent, packetLost)
 	}
@@ -335,7 +335,7 @@ func (c *lossState) discardKeys(now time.Time, log *slog.Logger, space numberSpa
 	// https://www.rfc-editor.org/rfc/rfc9002.html#section-6.4
 	for i := 0; i < c.spaces[space].size; i++ {
 		sent := c.spaces[space].nth(i)
-		if sent.acked || sent.lost {
+		if sent.state != sentPacketSent {
 			continue
 		}
 		c.cc.packetDiscarded(sent)
@@ -362,7 +362,7 @@ func (c *lossState) detectLoss(now time.Time, lossf func(numberSpace, *sentPacke
 	for space := numberSpace(0); space < numberSpaceCount; space++ {
 		for i := 0; i < c.spaces[space].size; i++ {
 			sent := c.spaces[space].nth(i)
-			if sent.lost || sent.acked {
+			if sent.state != sentPacketSent {
 				continue
 			}
 			// RFC 9002 Section 6.1 states that a packet is only declared lost if it
@@ -378,13 +378,13 @@ func (c *lossState) detectLoss(now time.Time, lossf func(numberSpace, *sentPacke
 			case sent.num <= c.spaces[space].maxAcked && !sent.time.After(lossTime):
 				// Time threshold
 				// https://www.rfc-editor.org/rfc/rfc9002.html#section-6.1.2
-				sent.lost = true
+				sent.state = sentPacketLost
 				lossf(space, sent, packetLost)
 				if sent.inFlight {
 					c.cc.packetLost(now, space, sent, &c.rtt)
 				}
 			}
-			if !sent.lost {
+			if sent.state != sentPacketLost {
 				break
 			}
 		}
diff --git a/quic/sent_packet.go b/quic/sent_packet.go
@@ -19,10 +19,9 @@ type sentPacket struct {
 	time  time.Time // time sent
 	ptype packetType
 
+	state        sentPacketState
 	ackEliciting bool // https://www.rfc-editor.org/rfc/rfc9002.html#section-2-3.4.1
 	inFlight     bool // https://www.rfc-editor.org/rfc/rfc9002.html#section-2-3.6.1
-	acked        bool // ack has been received
-	lost         bool // packet is presumed lost
 
 	// Frames sent in the packet.
 	//
@@ -36,6 +35,14 @@ type sentPacket struct {
 	n int // read offset into b
 }
 
+type sentPacketState uint8
+
+const (
+	sentPacketSent  = sentPacketState(iota) // sent but neither acked nor lost
+	sentPacketAcked                         // acked
+	sentPacketLost                          // declared lost
+)
+
 var sentPool = sync.Pool{
 	New: func() any {
 		return &sentPacket{}
diff --git a/quic/sent_packet_list.go b/quic/sent_packet_list.go
@@ -67,7 +67,7 @@ func (s *sentPacketList) num(num packetNumber) *sentPacket {
 func (s *sentPacketList) clean() {
 	for s.size > 0 {
 		sent := s.p[s.off]
-		if !sent.acked && !sent.lost {
+		if sent.state == sentPacketSent {
 			return
 		}
 		sent.recycle()
diff --git a/quic/sent_packet_list_test.go b/quic/sent_packet_list_test.go
@@ -26,7 +26,7 @@ func TestSentPacketListSlidingWindow(t *testing.T) {
 		if got := list.nth(0); got != sent {
 			t.Fatalf("list.nth(0) != list.num(%v)", prev)
 		}
-		sent.acked = true
+		sent.state = sentPacketAcked
 		list.clean()
 		if got := list.num(prev); got != nil {
 			t.Fatalf("list.num(%v) = packet %v, expected it to be discarded", prev, got.num)
@@ -82,7 +82,7 @@ func TestSentPacketListCleanAll(t *testing.T) {
 	}
 	// Mark all the packets as acked.
 	for i := packetNumber(0); i < count; i++ {
-		list.num(i).acked = true
+		list.num(i).state = sentPacketAcked
 	}
 	list.clean()
 	if got, want := list.size, 0; got != want {
