netstack: reduce MSS from SYN to account tcp options

See: https://tools.ietf.org/html/rfc6691#section-2
PiperOrigin-RevId: 239305632
Change-Id: Ie8eb912a43332e6490045dc95570709c5b81855e

pkg/sentry/fs/proc/README.md

@@ -11,7 +11,6 @@ inconsistency, please file a bug.
 
 The following files are implemented:
 
-
 | File /proc/                 | Content                                               |
 | :------------------------   | :---------------------------------------------------- |
 | [cpuinfo](#cpuinfo)         | Info about the CPU                                    |
@@ -23,7 +22,6 @@ The following files are implemented:
 | [uptime](#uptime)           | Wall clock since boot, combined idle time of all cpus |
 | [version](#version)         | Kernel version                                        |
 
-
 ### cpuinfo
 
 ```bash

pkg/tcpip/transport/tcp/endpoint.go

@@ -1596,6 +1596,16 @@ func (e *endpoint) maybeEnableSACKPermitted(synOpts *header.TCPSynOptions) {
 	}
 }
 
+// maxOptionSize return the maximum size of TCP options.
+func (e *endpoint) maxOptionSize() (size int) {
+	var maxSackBlocks [header.TCPMaxSACKBlocks]header.SACKBlock
+	options := e.makeOptions(maxSackBlocks[:])
+	size = len(options)
+	putOptions(options)
+
+	return size
+}
+
 // completeState makes a full copy of the endpoint and returns it. This is used
 // before invoking the probe. The state returned may not be fully consistent if
 // there are intervening syscalls when the state is being copied.

pkg/tcpip/transport/tcp/snd.go

@@ -172,6 +172,11 @@ type fastRecovery struct {
 }
 
 func newSender(ep *endpoint, iss, irs seqnum.Value, sndWnd seqnum.Size, mss uint16, sndWndScale int) *sender {
+	// The sender MUST reduce the TCP data length to account for any IP or
+	// TCP options that it is including in the packets that it sends.
+	// See: https://tools.ietf.org/html/rfc6691#section-2
+	maxPayloadSize := int(mss) - ep.maxOptionSize()
+
 	s := &sender{
 		ep:               ep,
 		sndCwnd:          InitialCwnd,
@@ -183,7 +188,7 @@ func newSender(ep *endpoint, iss, irs seqnum.Value, sndWnd seqnum.Size, mss uint
 		rto:              1 * time.Second,
 		rttMeasureSeqNum: iss + 1,
 		lastSendTime:     time.Now(),
-		maxPayloadSize:   int(mss),
+		maxPayloadSize:   maxPayloadSize,
 		maxSentAck:       irs + 1,
 		fr: fastRecovery{
 			// See: https://tools.ietf.org/html/rfc6582#section-3.2 Step 1.
@@ -226,11 +231,7 @@ func (s *sender) initCongestionControl(congestionControlName CongestionControlOp
 func (s *sender) updateMaxPayloadSize(mtu, count int) {
 	m := mtu - header.TCPMinimumSize
 
-	// Calculate the maximum option size.
-	var maxSackBlocks [header.TCPMaxSACKBlocks]header.SACKBlock
-	options := s.ep.makeOptions(maxSackBlocks[:])
-	m -= len(options)
-	putOptions(options)
+	m -= s.ep.maxOptionSize()
 
 	// We don't adjust up for now.
 	if m >= s.maxPayloadSize {

runsc/test/README.md

@@ -12,13 +12,11 @@ they may need extra setup in the test machine and extra configuration to run.
 
 The following setup steps are required in order to run these tests:
 
-
      `./runsc/test/install.sh [--runtime <name>]`
 
 The tests expect the runtime name to be provided in the `RUNSC_RUNTIME`
 environment variable (default: `runsc-test`). To run the tests execute:
 
-
 ```
 bazel test --test_env=RUNSC_RUNTIME=runsc-test \
   //runsc/test/image:image_test \

runsc/test/root/crictl_test.go

@@ -36,6 +36,7 @@ import (
 
 // Tests for crictl have to be run as root (rather than in a user namespace)
 // because crictl creates named network namespaces in /var/run/netns/.
+
 func TestCrictlSanity(t *testing.T) {
 	// Setup containerd and crictl.
 	crictl, cleanup, err := setup(t)
@@ -58,6 +59,7 @@ func TestCrictlSanity(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
 func TestMountPaths(t *testing.T) {
 	// Setup containerd and crictl.
 	crictl, cleanup, err := setup(t)
@@ -80,6 +82,7 @@ func TestMountPaths(t *testing.T) {
 		t.Fatal(err)
 	}
 }
+
 func TestMountOverSymlinks(t *testing.T) {
 	// Setup containerd and crictl.
 	crictl, cleanup, err := setup(t)

test/syscalls/linux/exec.cc

@@ -58,6 +58,7 @@ std::string WorkloadPath(absl::string_view binary) {
   if (test_src) {
     full_path = JoinPath(test_src, "__main__/test/syscalls/linux", binary);
   }
+
   TEST_CHECK(full_path.empty() == false);
   return full_path;
 }

test/syscalls/linux/preadv.cc

@@ -37,6 +37,7 @@ namespace gvisor {
 namespace testing {
 
 namespace {
+
 TEST(PreadvTest, MMConcurrencyStress) {
   // Fill a one-page file with zeroes (the contents don't really matter).
   const auto f = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileWith(

test/syscalls/linux/proc.cc

@@ -1258,6 +1258,7 @@ TEST(ProcPidSymlink, SubprocessRunning) {
   EXPECT_THAT(ReadlinkWhileRunning("ns/user", buf, sizeof(buf)),
               SyscallSucceedsWithValue(sizeof(buf)));
 }
+
 // FIXME: Inconsistent behavior between gVisor and linux
 // on proc files.
 TEST(ProcPidSymlink, SubprocessZombied) {
@@ -1362,6 +1363,7 @@ TEST(ProcPidFile, SubprocessRunning) {
 // Test whether /proc/PID/ files can be read for a zombie process.
 TEST(ProcPidFile, SubprocessZombie) {
   char buf[1];
+
   // 4.17: Succeeds and returns 1
   // gVisor: Succeds and returns 0
   EXPECT_THAT(ReadWhileZombied("auxv", buf, sizeof(buf)), SyscallSucceeds());

test/syscalls/linux/sigaltstack.cc

@@ -101,6 +101,7 @@ TEST(SigaltstackTest, ResetByExecve) {
   if (test_src) {
     full_path = JoinPath(test_src, "../../linux/sigaltstack_check");
   }
+
   ASSERT_FALSE(full_path.empty());
 
   pid_t child_pid = -1;

test/syscalls/linux/time.cc

@@ -61,6 +61,7 @@ TEST(TimeTest, VsyscallTime_InvalidAddressSIGSEGV) {
   EXPECT_EXIT(vsyscall_time(reinterpret_cast<time_t*>(0x1)),
               ::testing::KilledBySignal(SIGSEGV), "");
 }
+
 int vsyscall_gettimeofday(struct timeval* tv, struct timezone* tz) {
   constexpr uint64_t kVsyscallGettimeofdayEntry = 0xffffffffff600000;
   return reinterpret_cast<int (*)(struct timeval*, struct timezone*)>(

test/util/temp_path.cc

@@ -75,6 +75,7 @@ std::string NewTempRelPath() { return NextTempBasename(); }
 std::string GetAbsoluteTestTmpdir() {
   char* env_tmpdir = getenv("TEST_TMPDIR");
   std::string tmp_dir = env_tmpdir != nullptr ? std::string(env_tmpdir) : "/tmp";
+
   return MakeAbsolute(tmp_dir, "").ValueOrDie();
 }
 

test/util/test_util.cc

@@ -26,6 +26,7 @@
 
 #include <ctime>
 #include <vector>
+
 #include "absl/base/attributes.h"
 #include "absl/strings/numbers.h"
 #include "absl/strings/str_cat.h"
@@ -234,6 +235,7 @@ bool Equivalent(uint64_t current, uint64_t target, double tolerance) {
   auto abs_diff = target > current ? target - current : current - target;
   return abs_diff <= static_cast<uint64_t>(tolerance * target);
 }
+
 void TestInit(int* argc, char*** argv) {
   ::testing::InitGoogleTest(argc, *argv);
   ::gflags::ParseCommandLineFlags(argc, argv, true);

test/util/test_util.h

@@ -184,6 +184,7 @@
 #include <thread>  // NOLINT: using std::thread::hardware_concurrency().
 #include <utility>
 #include <vector>
+
 #include <gflags/gflags.h>
 #include <glog/logging.h>
 #include "gmock/gmock.h"