RACK: Fix re-transmitting the segment twice when entering recovery.

TestRACKWithDuplicateACK is flaky as the reorder window can expire before
receiving three duplicate ACKs which will result in sending the first
unacknowledged segment twice: when reorder timer expired and again after
receiving the third duplicate ACK.

This CL will fix this behavior and will not resend the segment again if it was
already re-transmittted when reorder timer expired.

Update the TestRACKWithDuplicateACK to test that the first segment is
considered as lost and is re-transmitted.

PiperOrigin-RevId: 356855168

2 files changed, 8 insertions, 5 deletions

diff --git a/pkg/tcpip/transport/tcp/snd.go b/pkg/tcpip/transport/tcp/snd.go
index d6365b93d..37f064aaf 100644
--- a/pkg/tcpip/transport/tcp/snd.go
+++ b/pkg/tcpip/transport/tcp/snd.go
@@ -1041,6 +1041,7 @@ func (s *sender) enterRecovery() {
 	// the 3 duplicate ACKs and are now not in flight.
 	s.sndCwnd = s.sndSsthresh + 3
 	s.sackedOut = 0
+	s.dupAckCount = 0
 	s.fr.first = s.sndUna
 	s.fr.last = s.sndNxt - 1
 	s.fr.maxCwnd = s.sndCwnd + s.outstanding
@@ -1174,7 +1175,6 @@ func (s *sender) detectLoss(seg *segment) (fastRetransmit bool) {
 	}
 	s.cc.HandleLossDetected()
 	s.enterRecovery()
-	s.dupAckCount = 0
 	return true
 }
 
@@ -1521,10 +1521,11 @@ func (s *sender) handleRcvdSegment(rcvdSeg *segment) {
 			// the lost segments.
 			s.cc.HandleLossDetected()
 			s.enterRecovery()
+			fastRetransmit = true
 		}
 
 		if s.fr.active {
-			s.rc.DoRecovery(nil, true)
+			s.rc.DoRecovery(nil, fastRetransmit)
 		}
 	}
 
diff --git a/pkg/tcpip/transport/tcp/tcp_rack_test.go b/pkg/tcpip/transport/tcp/tcp_rack_test.go
index 6da981d80..b397bb7ff 100644
--- a/pkg/tcpip/transport/tcp/tcp_rack_test.go
+++ b/pkg/tcpip/transport/tcp/tcp_rack_test.go
@@ -617,17 +617,19 @@ func TestRACKWithDuplicateACK(t *testing.T) {
 	const numPackets = 4
 	data := sendAndReceive(t, c, numPackets)
 
-	// Send three duplicate ACKs to trigger fast recovery.
+	// Send three duplicate ACKs to trigger fast recovery. The first
+	// segment is considered as lost and will be retransmitted after
+	// receiving the duplicate ACKs.
 	seq := seqnum.Value(context.TestInitialSequenceNumber).Add(1)
 	start := c.IRS.Add(1 + seqnum.Size(maxPayload))
 	end := start.Add(seqnum.Size(maxPayload))
 	for i := 0; i < 3; i++ {
-		c.SendAckWithSACK(seq, maxPayload, []header.SACKBlock{{start, end}})
+		c.SendAckWithSACK(seq, 0, []header.SACKBlock{{start, end}})
 		end = end.Add(seqnum.Size(maxPayload))
 	}
 
 	// Receive the retransmitted packet.
-	c.ReceiveAndCheckPacketWithOptions(data, maxPayload, maxPayload, tsOptionSize)
+	c.ReceiveAndCheckPacketWithOptions(data, 0, maxPayload, tsOptionSize)
 
 	metricPollFn := func() error {
 		tcpStats := c.Stack().Stats().TCP