From ff04d019e3d20adf0f5ef3146fa28d3b83a4819a Mon Sep 17 00:00:00 2001 From: Nayana Bidari Date: Wed, 10 Feb 2021 16:33:31 -0800 Subject: 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 --- pkg/tcpip/transport/tcp/snd.go | 5 +++-- pkg/tcpip/transport/tcp/tcp_rack_test.go | 8 +++++--- 2 files changed, 8 insertions(+), 5 deletions(-) (limited to 'pkg/tcpip') 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 -- cgit v1.2.3