4 files changed, 90 insertions, 128 deletions
diff --git a/test/packetimpact/tests/BUILD b/test/packetimpact/tests/BUILD
index 1c9f59df5..85749c559 100644
--- a/test/packetimpact/tests/BUILD
+++ b/test/packetimpact/tests/BUILD
@@ -183,8 +183,6 @@ packetimpact_go_test(
 packetimpact_go_test(
     name = "tcp_queue_receive_in_syn_sent",
     srcs = ["tcp_queue_receive_in_syn_sent_test.go"],
-    # TODO(b/157658105): Fix netstack then remove the line below.
-    expect_netstack_failure = True,
     deps = [
         "//pkg/tcpip/header",
         "//test/packetimpact/testbench",
@@ -213,16 +211,6 @@ packetimpact_go_test(
 )
 
 packetimpact_go_test(
-    name = "tcp_splitseg_mss",
-    srcs = ["tcp_splitseg_mss_test.go"],
-    deps = [
-        "//pkg/tcpip/header",
-        "//test/packetimpact/testbench",
-        "@org_golang_x_sys//unix:go_default_library",
-    ],
-)
-
-packetimpact_go_test(
     name = "tcp_cork_mss",
     srcs = ["tcp_cork_mss_test.go"],
     deps = [
diff --git a/test/packetimpact/tests/tcp_queue_receive_in_syn_sent_test.go b/test/packetimpact/tests/tcp_queue_receive_in_syn_sent_test.go
index b640d8673..8fbec893b 100644
--- a/test/packetimpact/tests/tcp_queue_receive_in_syn_sent_test.go
+++ b/test/packetimpact/tests/tcp_queue_receive_in_syn_sent_test.go
@@ -35,53 +35,98 @@ func init() {
 	testbench.RegisterFlags(flag.CommandLine)
 }
 
+// TestQueueReceiveInSynSent tests receive behavior when the TCP state
+// is SYN-SENT.
+// It tests for 2 variants where the receive is blocked and:
+// (1) we complete handshake and send sample data.
+// (2) we send a TCP RST.
 func TestQueueReceiveInSynSent(t *testing.T) {
-	dut := testbench.NewDUT(t)
-	defer dut.TearDown()
+	for _, tt := range []struct {
+		description string
+		reset       bool
+	}{
+		{description: "Send DATA", reset: false},
+		{description: "Send RST", reset: true},
+	} {
+		t.Run(tt.description, func(t *testing.T) {
+			dut := testbench.NewDUT(t)
+			defer dut.TearDown()
 
-	socket, remotePort := dut.CreateBoundSocket(unix.SOCK_STREAM, unix.IPPROTO_TCP, net.ParseIP(testbench.RemoteIPv4))
-	conn := testbench.NewTCPIPv4(t, testbench.TCP{DstPort: &remotePort}, testbench.TCP{SrcPort: &remotePort})
-	defer conn.Close()
+			socket, remotePort := dut.CreateBoundSocket(unix.SOCK_STREAM, unix.IPPROTO_TCP, net.ParseIP(testbench.RemoteIPv4))
+			conn := testbench.NewTCPIPv4(t, testbench.TCP{DstPort: &remotePort}, testbench.TCP{SrcPort: &remotePort})
+			defer conn.Close()
 
-	sampleData := []byte("Sample Data")
+			sampleData := []byte("Sample Data")
 
-	dut.SetNonBlocking(socket, true)
-	if _, err := dut.ConnectWithErrno(context.Background(), socket, conn.LocalAddr()); !errors.Is(err, syscall.EINPROGRESS) {
-		t.Fatalf("failed to bring DUT to SYN-SENT, got: %s, want EINPROGRESS", err)
-	}
-	if _, err := conn.Expect(testbench.TCP{Flags: testbench.Uint8(header.TCPFlagSyn)}, time.Second); err != nil {
-		t.Fatalf("expected a SYN from DUT, but got none: %s", err)
-	}
+			dut.SetNonBlocking(socket, true)
+			if _, err := dut.ConnectWithErrno(context.Background(), socket, conn.LocalAddr()); !errors.Is(err, syscall.EINPROGRESS) {
+				t.Fatalf("failed to bring DUT to SYN-SENT, got: %s, want EINPROGRESS", err)
+			}
+			if _, err := conn.Expect(testbench.TCP{Flags: testbench.Uint8(header.TCPFlagSyn)}, time.Second); err != nil {
+				t.Fatalf("expected a SYN from DUT, but got none: %s", err)
+			}
 
-	// Issue RECEIVE call in SYN-SENT, this should be queued for process until the connection
-	// is established.
-	dut.SetNonBlocking(socket, false)
-	var wg sync.WaitGroup
-	defer wg.Wait()
-	wg.Add(1)
-	go func() {
-		defer wg.Done()
-		ctx, cancel := context.WithTimeout(context.Background(), time.Second*3)
-		defer cancel()
-		n, buff, err := dut.RecvWithErrno(ctx, socket, int32(len(sampleData)), 0)
-		if n == -1 {
-			t.Fatalf("failed to recv on DUT: %s", err)
-		}
-		if got := buff[:n]; !bytes.Equal(got, sampleData) {
-			t.Fatalf("received data don't match, got:\n%s, want:\n%s", hex.Dump(got), hex.Dump(sampleData))
-		}
-	}()
-
-	// The following sleep is used to prevent the connection from being established while the
-	// RPC is in flight.
-	time.Sleep(time.Second)
-
-	// Bring the connection to Established.
-	conn.Send(testbench.TCP{Flags: testbench.Uint8(header.TCPFlagSyn | header.TCPFlagAck)})
-	if _, err := conn.Expect(testbench.TCP{Flags: testbench.Uint8(header.TCPFlagAck)}, time.Second); err != nil {
-		t.Fatalf("expected an ACK from DUT, but got none: %s", err)
-	}
+			if _, _, err := dut.RecvWithErrno(context.Background(), socket, int32(len(sampleData)), 0); err != syscall.Errno(unix.EWOULDBLOCK) {
+				t.Fatalf("expected error %s, got %s", syscall.Errno(unix.EWOULDBLOCK), err)
+			}
+
+			// Test blocking read.
+			dut.SetNonBlocking(socket, false)
+
+			var wg sync.WaitGroup
+			defer wg.Wait()
+			wg.Add(1)
+			var block sync.WaitGroup
+			block.Add(1)
+			go func() {
+				defer wg.Done()
+				ctx, cancel := context.WithTimeout(context.Background(), time.Second*3)
+				defer cancel()
 
-	// Send sample data to DUT.
-	conn.Send(testbench.TCP{Flags: testbench.Uint8(header.TCPFlagAck)}, &testbench.Payload{Bytes: sampleData})
+				block.Done()
+				// Issue RECEIVE call in SYN-SENT, this should be queued for
+				// process until the connection is established.
+				n, buff, err := dut.RecvWithErrno(ctx, socket, int32(len(sampleData)), 0)
+				if tt.reset {
+					if err != syscall.Errno(unix.ECONNREFUSED) {
+						t.Errorf("expected error %s, got %s", syscall.Errno(unix.ECONNREFUSED), err)
+					}
+					if n != -1 {
+						t.Errorf("expected return value %d, got %d", -1, n)
+					}
+					return
+				}
+				if n == -1 {
+					t.Errorf("failed to recv on DUT: %s", err)
+				}
+				if got := buff[:n]; !bytes.Equal(got, sampleData) {
+					t.Errorf("received data doesn't match, got:\n%s, want:\n%s", hex.Dump(got), hex.Dump(sampleData))
+				}
+			}()
+
+			// Wait for the goroutine to be scheduled and before it
+			// blocks on endpoint receive.
+			block.Wait()
+			// The following sleep is used to prevent the connection
+			// from being established before we are blocked on Recv.
+			time.Sleep(100 * time.Millisecond)
+
+			if tt.reset {
+				conn.Send(testbench.TCP{Flags: testbench.Uint8(header.TCPFlagRst | header.TCPFlagAck)})
+				return
+			}
+
+			// Bring the connection to Established.
+			conn.Send(testbench.TCP{Flags: testbench.Uint8(header.TCPFlagSyn | header.TCPFlagAck)})
+			if _, err := conn.Expect(testbench.TCP{Flags: testbench.Uint8(header.TCPFlagAck)}, time.Second); err != nil {
+				t.Fatalf("expected an ACK from DUT, but got none: %s", err)
+			}
+
+			// Send sample payload and expect an ACK.
+			conn.Send(testbench.TCP{Flags: testbench.Uint8(header.TCPFlagAck)}, &testbench.Payload{Bytes: sampleData})
+			if _, err := conn.Expect(testbench.TCP{Flags: testbench.Uint8(header.TCPFlagAck)}, time.Second); err != nil {
+				t.Fatalf("expected an ACK from DUT, but got none: %s", err)
+			}
+		})
+	}
 }
diff --git a/test/packetimpact/tests/tcp_splitseg_mss_test.go b/test/packetimpact/tests/tcp_splitseg_mss_test.go
deleted file mode 100644
index 9350d0988..000000000
--- a/test/packetimpact/tests/tcp_splitseg_mss_test.go
+++ /dev/null
@@ -1,71 +0,0 @@
-// Copyright 2020 The gVisor Authors.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package tcp_splitseg_mss_test
-
-import (
-	"flag"
-	"testing"
-	"time"
-
-	"golang.org/x/sys/unix"
-	"gvisor.dev/gvisor/pkg/tcpip/header"
-	"gvisor.dev/gvisor/test/packetimpact/testbench"
-)
-
-func init() {
-	testbench.RegisterFlags(flag.CommandLine)
-}
-
-// TestTCPSplitSegMSS lets the dut try to send segments larger than MSS.
-// It tests if the transmitted segments are capped at MSS and are split.
-func TestTCPSplitSegMSS(t *testing.T) {
-	dut := testbench.NewDUT(t)
-	defer dut.TearDown()
-	listenFD, remotePort := dut.CreateListener(unix.SOCK_STREAM, unix.IPPROTO_TCP, 1)
-	defer dut.Close(listenFD)
-	conn := testbench.NewTCPIPv4(t, testbench.TCP{DstPort: &remotePort}, testbench.TCP{SrcPort: &remotePort})
-	defer conn.Close()
-
-	const mss = uint32(header.TCPDefaultMSS)
-	options := make([]byte, header.TCPOptionMSSLength)
-	header.EncodeMSSOption(mss, options)
-	conn.ConnectWithOptions(options)
-
-	acceptFD, _ := dut.Accept(listenFD)
-	defer dut.Close(acceptFD)
-
-	// Let the dut send a segment larger than MSS.
-	largeData := make([]byte, mss+1)
-	for i := 0; i < 2; i++ {
-		dut.Send(acceptFD, largeData, 0)
-		if i == 0 {
-			// On Linux, the initial segment goes out beyond MSS and the segment
-			// split occurs on retransmission. Call ExpectData to wait to
-			// receive the split segment.
-			if _, err := conn.ExpectData(&testbench.TCP{Flags: testbench.Uint8(header.TCPFlagAck)}, &testbench.Payload{Bytes: largeData[:mss]}, time.Second); err != nil {
-				t.Fatalf("expected payload was not received: %s", err)
-			}
-		} else {
-			if _, err := conn.ExpectNextData(&testbench.TCP{Flags: testbench.Uint8(header.TCPFlagAck)}, &testbench.Payload{Bytes: largeData[:mss]}, time.Second); err != nil {
-				t.Fatalf("expected payload was not received: %s", err)
-			}
-		}
-		conn.Send(testbench.TCP{Flags: testbench.Uint8(header.TCPFlagAck)})
-		if _, err := conn.ExpectNextData(&testbench.TCP{Flags: testbench.Uint8(header.TCPFlagAck | header.TCPFlagPsh)}, &testbench.Payload{Bytes: largeData[mss:]}, time.Second); err != nil {
-			t.Fatalf("expected payload was not received: %s", err)
-		}
-		conn.Send(testbench.TCP{Flags: testbench.Uint8(header.TCPFlagAck)})
-	}
-}
diff --git a/test/packetimpact/tests/tcp_zero_window_probe_retransmit_test.go b/test/packetimpact/tests/tcp_zero_window_probe_retransmit_test.go
index 5ab193181..8c89d57c9 100644
--- a/test/packetimpact/tests/tcp_zero_window_probe_retransmit_test.go
+++ b/test/packetimpact/tests/tcp_zero_window_probe_retransmit_test.go
@@ -88,8 +88,8 @@ func TestZeroWindowProbeRetransmit(t *testing.T) {
 			continue
 		}
 		// Check if the probes came at exponentially increasing intervals.
-		if p := time.Since(start); p < current-startProbeDuration {
-			t.Fatalf("zero probe came sooner interval %d probe %d\n", p, i)
+		if got, want := time.Since(start), current-startProbeDuration; got < want {
+			t.Errorf("got zero probe %d after %s, want >= %s", i, got, want)
 		}
 		// Acknowledge the zero-window probes from the dut.
 		conn.Send(testbench.TCP{AckNum: ackProbe, Flags: testbench.Uint8(header.TCPFlagAck), WindowSize: testbench.Uint16(0)})