Add support for rate limiting out of window ACKs.

Netstack today will send dupACK's with no rate limit for incoming out of
window segments. This can result in ACK loops for example if a TCP socket
connects to itself (actually permitted by TCP). Where the ACK sent in
response to packets being out of order itself gets considered as an out
of window segment resulting in another ACK being generated.

PiperOrigin-RevId: 355206877

pkg/tcpip/stack/stack.go

@@ -458,6 +458,18 @@ type Stack struct {
 	// receiveBufferSize holds the min/default/max receive buffer sizes for
 	// endpoints other than TCP.
 	receiveBufferSize ReceiveBufferSizeOption
+
+	// tcpInvalidRateLimit is the maximal rate for sending duplicate
+	// acknowledgements in response to incoming TCP packets that are for an existing
+	// connection but that are invalid due to any of the following reasons:
+	//
+	//   a) out-of-window sequence number.
+	//   b) out-of-window acknowledgement number.
+	//   c) PAWS check failure (when implemented).
+	//
+	// This is required to prevent potential ACK loops.
+	// Setting this to 0 will disable all rate limiting.
+	tcpInvalidRateLimit time.Duration
 }
 
 // UniqueID is an abstract generator of unique identifiers.
@@ -668,6 +680,7 @@ func New(opts Options) *Stack {
 			Default: DefaultBufferSize,
 			Max:     DefaultMaxBufferSize,
 		},
+		tcpInvalidRateLimit: defaultTCPInvalidRateLimit,
 	}
 
 	// Add specified network protocols.

pkg/tcpip/stack/stack_options.go

@@ -15,6 +15,8 @@
 package stack
 
 import (
+	"time"
+
 	"gvisor.dev/gvisor/pkg/tcpip"
 )
 
@@ -29,6 +31,10 @@ const (
 	// DefaultMaxBufferSize is the default maximum permitted size of a
 	// send/receive buffer.
 	DefaultMaxBufferSize = 4 << 20 // 4 MiB
+
+	// defaultTCPInvalidRateLimit is the default value for
+	// stack.TCPInvalidRateLimit.
+	defaultTCPInvalidRateLimit = 500 * time.Millisecond
 )
 
 // ReceiveBufferSizeOption is used by stack.(Stack*).Option/SetOption to
@@ -39,6 +45,10 @@ type ReceiveBufferSizeOption struct {
 	Max     int
 }
 
+// TCPInvalidRateLimitOption is used by stack.(Stack*).Option/SetOption to get/set
+// stack.tcpInvalidRateLimit.
+type TCPInvalidRateLimitOption time.Duration
+
 // SetOption allows setting stack wide options.
 func (s *Stack) SetOption(option interface{}) tcpip.Error {
 	switch v := option.(type) {
@@ -74,6 +84,15 @@ func (s *Stack) SetOption(option interface{}) tcpip.Error {
 		s.mu.Unlock()
 		return nil
 
+	case TCPInvalidRateLimitOption:
+		if v < 0 {
+			return &tcpip.ErrInvalidOptionValue{}
+		}
+		s.mu.Lock()
+		s.tcpInvalidRateLimit = time.Duration(v)
+		s.mu.Unlock()
+		return nil
+
 	default:
 		return &tcpip.ErrUnknownProtocolOption{}
 	}
@@ -94,6 +113,12 @@ func (s *Stack) Option(option interface{}) tcpip.Error {
 		s.mu.RUnlock()
 		return nil
 
+	case *TCPInvalidRateLimitOption:
+		s.mu.RLock()
+		*v = TCPInvalidRateLimitOption(s.tcpInvalidRateLimit)
+		s.mu.RUnlock()
+		return nil
+
 	default:
 		return &tcpip.ErrUnknownProtocolOption{}
 	}

pkg/tcpip/transport/tcp/connect.go

@@ -333,7 +333,9 @@ func (h *handshake) synRcvdState(s *segment) tcpip.Error {
 	// number and "After sending the acknowledgment, drop the unacceptable
 	// segment and return."
 	if !s.sequenceNumber.InWindow(h.ackNum, h.rcvWnd) {
-		h.ep.sendRaw(buffer.VectorisedView{}, header.TCPFlagAck, h.iss+1, h.ackNum, h.rcvWnd)
+		if h.ep.allowOutOfWindowAck() {
+			h.ep.sendRaw(buffer.VectorisedView{}, header.TCPFlagAck, h.iss+1, h.ackNum, h.rcvWnd)
+		}
 		return nil
 	}
 
@@ -1185,8 +1187,7 @@ func (e *endpoint) handleSegment(s *segment) (cont bool, err tcpip.Error) {
 		// endpoint MUST terminate its connection.  The local TCP endpoint
 		// should then rely on SYN retransmission from the remote end to
 		// re-establish the connection.
-
-		e.snd.sendAck()
+		e.snd.maybeSendOutOfWindowAck(s)
 	} else if s.flagIsSet(header.TCPFlagAck) {
 		// Patch the window size in the segment according to the
 		// send window scale.

pkg/tcpip/transport/tcp/endpoint.go

@@ -688,6 +688,10 @@ type endpoint struct {
 
 	// ops is used to get socket level options.
 	ops tcpip.SocketOptions
+
+	// lastOutOfWindowAckTime is the time at which the an ACK was sent in response
+	// to an out of window segment being received by this endpoint.
+	lastOutOfWindowAckTime time.Time `state:".(unixTime)"`
 }
 
 // UniqueID implements stack.TransportEndpoint.UniqueID.
@@ -3125,3 +3129,19 @@ func GetTCPSendBufferLimits(s tcpip.StackHandler) tcpip.SendBufferSizeOption {
 		Max:     ss.Max,
 	}
 }
+
+// allowOutOfWindowAck returns true if an out-of-window ACK can be sent now.
+func (e *endpoint) allowOutOfWindowAck() bool {
+	var limit stack.TCPInvalidRateLimitOption
+	if err := e.stack.Option(&limit); err != nil {
+		panic(fmt.Sprintf("e.stack.Option(%+v) failed with error: %s", limit, err))
+	}
+
+	now := time.Now()
+	if now.Sub(e.lastOutOfWindowAckTime) < time.Duration(limit) {
+		return false
+	}
+
+	e.lastOutOfWindowAckTime = now
+	return true
+}

pkg/tcpip/transport/tcp/endpoint_state.go

@@ -308,6 +308,16 @@ func (e *endpoint) loadRecentTSTime(unix unixTime) {
 	e.recentTSTime = time.Unix(unix.second, unix.nano)
 }
 
+// saveLastOutOfWindowAckTime is invoked by stateify.
+func (e *endpoint) saveLastOutOfWindowAckTime() unixTime {
+	return unixTime{e.lastOutOfWindowAckTime.Unix(), e.lastOutOfWindowAckTime.UnixNano()}
+}
+
+// loadLastOutOfWindowAckTime is invoked by stateify.
+func (e *endpoint) loadLastOutOfWindowAckTime(unix unixTime) {
+	e.lastOutOfWindowAckTime = time.Unix(unix.second, unix.nano)
+}
+
 // saveMeasureTime is invoked by stateify.
 func (r *rcvBufAutoTuneParams) saveMeasureTime() unixTime {
 	return unixTime{r.measureTime.Unix(), r.measureTime.UnixNano()}

pkg/tcpip/transport/tcp/rcv.go

@@ -385,7 +385,7 @@ func (r *receiver) handleRcvdSegmentClosing(s *segment, state EndpointState, clo
 		// fails, we ignore the packet:
 		// https://github.com/torvalds/linux/blob/v5.8/net/ipv4/tcp_input.c#L5591
 		if r.ep.snd.sndNxt.LessThan(s.ackNumber) {
-			r.ep.snd.sendAck()
+			r.ep.snd.maybeSendOutOfWindowAck(s)
 			return true, nil
 		}
 
@@ -454,7 +454,7 @@ func (r *receiver) handleRcvdSegment(s *segment) (drop bool, err tcpip.Error) {
 	// send an ACK and stop further processing of the segment.
 	// This is according to RFC 793, page 68.
 	if !r.acceptable(segSeq, segLen) {
-		r.ep.snd.sendAck()
+		r.ep.snd.maybeSendOutOfWindowAck(s)
 		return true, nil
 	}
 

pkg/tcpip/transport/tcp/snd.go

@@ -1548,3 +1548,13 @@ func (s *sender) sendSegmentFromView(data buffer.VectorisedView, flags byte, seq
 
 	return s.ep.sendRaw(data, flags, seq, rcvNxt, rcvWnd)
 }
+
+// maybeSendOutOfWindowAck sends an ACK if we are not being rate limited
+// currently.
+func (s *sender) maybeSendOutOfWindowAck(seg *segment) {
+	// Data packets are unlikely to be part of an ACK loop. So always send
+	// an ACK for a packet w/ data.
+	if seg.payloadSize() > 0 || s.ep.allowOutOfWindowAck() {
+		s.sendAck()
+	}
+}

pkg/tcpip/transport/tcp/tcp_test.go

@@ -6302,6 +6302,13 @@ func TestReceiveBufferAutoTuning(t *testing.T) {
 
 	// Enable Auto-tuning.
 	stk := c.Stack()
+	// Disable out of window rate limiting for this test by setting it to 0 as we
+	// use out of window ACKs to measure the advertised window.
+	var tcpInvalidRateLimit stack.TCPInvalidRateLimitOption
+	if err := stk.SetOption(tcpInvalidRateLimit); err != nil {
+		t.Fatalf("e.stack.SetOption(%#v) = %s", tcpInvalidRateLimit, err)
+	}
+
 	const receiveBufferSize = 80 << 10 // 80KB.
 	const maxReceiveBufferSize = receiveBufferSize * 10
 	{

test/packetimpact/tests/tcp_outside_the_window_test.go

@@ -84,6 +84,24 @@ func TestTCPOutsideTheWindow(t *testing.T) {
 			if tt.expectACK && err != nil {
 				t.Fatalf("expected an ACK packet within %s but got none: %s", timeout, err)
 			}
+			// Data packets w/o SYN bits are always acked by Linux. Netstack ACK's data packets
+			// always right now. So only send a second segment and test for no ACK for packets
+			// with no data.
+			if tt.expectACK && tt.payload == nil {
+				// Sending another out-of-window segment immediately should not trigger
+				// an ACK if less than 500ms(default rate limit for out-of-window ACKs)
+				// has passed since the last ACK was sent.
+				t.Logf("sending another segment")
+				conn.Send(t, testbench.TCP{
+					Flags:  testbench.Uint8(tt.tcpFlags),
+					SeqNum: testbench.Uint32(uint32(conn.LocalSeqNum(t).Add(windowSize))),
+				}, tt.payload...)
+				timeout := 3 * time.Second
+				gotACK, err := conn.Expect(t, testbench.TCP{Flags: testbench.Uint8(header.TCPFlagAck), AckNum: localSeqNum}, timeout)
+				if err == nil {
+					t.Fatalf("expected no ACK packet but got one: %s", gotACK)
+				}
+			}
 			if !tt.expectACK && gotACK != nil {
 				t.Fatalf("expected no ACK packet within %s but got one: %s", timeout, gotACK)
 			}

test/syscalls/linux/tcp_socket.cc

@@ -1168,6 +1168,42 @@ TEST_P(SimpleTcpSocketTest, SelfConnectSendRecv_NoRandomSave) {
   EXPECT_EQ(read_bytes, kBufSz);
 }
 
+TEST_P(SimpleTcpSocketTest, SelfConnectSend_NoRandomSave) {
+  // Initialize address to the loopback one.
+  sockaddr_storage addr =
+      ASSERT_NO_ERRNO_AND_VALUE(InetLoopbackAddr(GetParam()));
+  socklen_t addrlen = sizeof(addr);
+
+  const FileDescriptor s =
+      ASSERT_NO_ERRNO_AND_VALUE(Socket(GetParam(), SOCK_STREAM, IPPROTO_TCP));
+
+  constexpr int max_seg = 256;
+  ASSERT_THAT(
+      setsockopt(s.get(), SOL_TCP, TCP_MAXSEG, &max_seg, sizeof(max_seg)),
+      SyscallSucceeds());
+
+  ASSERT_THAT(bind(s.get(), reinterpret_cast<struct sockaddr*>(&addr), addrlen),
+              SyscallSucceeds());
+  // Get the bound port.
+  ASSERT_THAT(
+      getsockname(s.get(), reinterpret_cast<struct sockaddr*>(&addr), &addrlen),
+      SyscallSucceeds());
+  ASSERT_THAT(RetryEINTR(connect)(
+                  s.get(), reinterpret_cast<struct sockaddr*>(&addr), addrlen),
+              SyscallSucceeds());
+
+  std::vector<char> writebuf(512 << 10);  // 512 KiB.
+
+  // Try to send the whole thing.
+  int n;
+  ASSERT_THAT(n = SendFd(s.get(), writebuf.data(), writebuf.size(), 0),
+              SyscallSucceeds());
+
+  // We should have written the whole thing.
+  EXPECT_EQ(n, writebuf.size());
+  EXPECT_THAT(shutdown(s.get(), SHUT_WR), SyscallSucceedsWithValue(0));
+}
+
 TEST_P(SimpleTcpSocketTest, NonBlockingConnect) {
   const FileDescriptor listener =
       ASSERT_NO_ERRNO_AND_VALUE(Socket(GetParam(), SOCK_STREAM, IPPROTO_TCP));