Fix and merge tcp_{outside_the_window,tcp_unacc_seq_ack}_closing

The tests were not using the correct windowSize so the testing segments were
actually within the window for seqNumOffset=0 tests. The issue is already fixed
by #5674.

PiperOrigin-RevId: 364252630

6 files changed, 135 insertions, 212 deletions

diff --git a/test/packetimpact/runner/defs.bzl b/test/packetimpact/runner/defs.bzl
index 803a87a07..34e83ec49 100644
--- a/test/packetimpact/runner/defs.bzl
+++ b/test/packetimpact/runner/defs.bzl
@@ -203,11 +203,6 @@ ALL_TESTS = [
         name = "tcp_outside_the_window",
     ),
     PacketimpactTestInfo(
-        name = "tcp_outside_the_window_closing",
-        # TODO(b/181625316): Fix netstack then merge into tcp_outside_the_window.
-        expect_netstack_failure = True,
-    ),
-    PacketimpactTestInfo(
         name = "tcp_noaccept_close_rst",
     ),
     PacketimpactTestInfo(
@@ -217,11 +212,6 @@ ALL_TESTS = [
         name = "tcp_unacc_seq_ack",
     ),
     PacketimpactTestInfo(
-        name = "tcp_unacc_seq_ack_closing",
-        # TODO(b/181625316): Fix netstack then merge into tcp_unacc_seq_ack.
-        expect_netstack_failure = True,
-    ),
-    PacketimpactTestInfo(
         name = "tcp_paws_mechanism",
         # TODO(b/156682000): Fix netstack then remove the line below.
         expect_netstack_failure = True,
diff --git a/test/packetimpact/tests/BUILD b/test/packetimpact/tests/BUILD
index d5cb0ae06..c0deb33e5 100644
--- a/test/packetimpact/tests/BUILD
+++ b/test/packetimpact/tests/BUILD
@@ -124,17 +124,6 @@ packetimpact_testbench(
 )
 
 packetimpact_testbench(
-    name = "tcp_outside_the_window_closing",
-    srcs = ["tcp_outside_the_window_closing_test.go"],
-    deps = [
-        "//pkg/tcpip/header",
-        "//pkg/tcpip/seqnum",
-        "//test/packetimpact/testbench",
-        "@org_golang_x_sys//unix:go_default_library",
-    ],
-)
-
-packetimpact_testbench(
     name = "tcp_noaccept_close_rst",
     srcs = ["tcp_noaccept_close_rst_test.go"],
     deps = [
@@ -166,17 +155,6 @@ packetimpact_testbench(
 )
 
 packetimpact_testbench(
-    name = "tcp_unacc_seq_ack_closing",
-    srcs = ["tcp_unacc_seq_ack_closing_test.go"],
-    deps = [
-        "//pkg/tcpip/header",
-        "//pkg/tcpip/seqnum",
-        "//test/packetimpact/testbench",
-        "@org_golang_x_sys//unix:go_default_library",
-    ],
-)
-
-packetimpact_testbench(
     name = "tcp_paws_mechanism",
     srcs = ["tcp_paws_mechanism_test.go"],
     deps = [
diff --git a/test/packetimpact/tests/tcp_outside_the_window_closing_test.go b/test/packetimpact/tests/tcp_outside_the_window_closing_test.go
deleted file mode 100644
index 1097746c7..000000000
--- a/test/packetimpact/tests/tcp_outside_the_window_closing_test.go
+++ /dev/null
@@ -1,86 +0,0 @@
-// Copyright 2021 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_outside_the_window_closing_test
-
-import (
-	"flag"
-	"fmt"
-	"testing"
-	"time"
-
-	"golang.org/x/sys/unix"
-	"gvisor.dev/gvisor/pkg/tcpip/header"
-	"gvisor.dev/gvisor/pkg/tcpip/seqnum"
-	"gvisor.dev/gvisor/test/packetimpact/testbench"
-)
-
-func init() {
-	testbench.Initialize(flag.CommandLine)
-}
-
-// TestAckOTWSeqInClosing tests that the DUT should send an ACK with
-// the right ACK number when receiving a packet with OTW Seq number
-// in CLOSING state. https://tools.ietf.org/html/rfc793#page-69
-func TestAckOTWSeqInClosing(t *testing.T) {
-	for seqNumOffset := seqnum.Size(0); seqNumOffset < 3; seqNumOffset++ {
-		for _, tt := range []struct {
-			description string
-			flags       header.TCPFlags
-			payloads    testbench.Layers
-		}{
-			{"SYN", header.TCPFlagSyn, nil},
-			{"SYNACK", header.TCPFlagSyn | header.TCPFlagAck, nil},
-			{"ACK", header.TCPFlagAck, nil},
-			{"FINACK", header.TCPFlagFin | header.TCPFlagAck, nil},
-			{"Data", header.TCPFlagAck, []testbench.Layer{&testbench.Payload{Bytes: []byte("abc123")}}},
-		} {
-			t.Run(fmt.Sprintf("%s%d", tt.description, seqNumOffset), func(t *testing.T) {
-				dut := testbench.NewDUT(t)
-				listenFD, remotePort := dut.CreateListener(t, unix.SOCK_STREAM, unix.IPPROTO_TCP, 1)
-				defer dut.Close(t, listenFD)
-				conn := dut.Net.NewTCPIPv4(t, testbench.TCP{DstPort: &remotePort}, testbench.TCP{SrcPort: &remotePort})
-				defer conn.Close(t)
-				conn.Connect(t)
-				acceptFD, _ := dut.Accept(t, listenFD)
-				defer dut.Close(t, acceptFD)
-
-				dut.Shutdown(t, acceptFD, unix.SHUT_WR)
-
-				if _, err := conn.Expect(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagFin | header.TCPFlagAck)}, time.Second); err != nil {
-					t.Fatalf("expected FINACK from DUT, but got none: %s", err)
-				}
-
-				// Do not ack the FIN from DUT so that the TCP state on DUT is CLOSING instead of CLOSED.
-				seqNumForTheirFIN := testbench.Uint32(uint32(*conn.RemoteSeqNum(t)) - 1)
-				conn.Send(t, testbench.TCP{AckNum: seqNumForTheirFIN, Flags: testbench.TCPFlags(header.TCPFlagFin | header.TCPFlagAck)})
-
-				if _, err := conn.Expect(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck)}, time.Second); err != nil {
-					t.Errorf("expected an ACK to our FIN, but got none: %s", err)
-				}
-
-				windowSize := seqnum.Size(*conn.SynAck(t).WindowSize) + seqNumOffset
-				conn.SendFrameStateless(t, conn.CreateFrame(t, testbench.Layers{&testbench.TCP{
-					SeqNum: testbench.Uint32(uint32(conn.LocalSeqNum(t).Add(windowSize))),
-					AckNum: seqNumForTheirFIN,
-					Flags:  testbench.TCPFlags(tt.flags),
-				}}, tt.payloads...))
-
-				if _, err := conn.Expect(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck)}, time.Second); err != nil {
-					t.Errorf("expected an ACK but got none: %s", err)
-				}
-			})
-		}
-	}
-}
diff --git a/test/packetimpact/tests/tcp_outside_the_window_test.go b/test/packetimpact/tests/tcp_outside_the_window_test.go
index 7cd7ff703..0523887d9 100644
--- a/test/packetimpact/tests/tcp_outside_the_window_test.go
+++ b/test/packetimpact/tests/tcp_outside_the_window_test.go
@@ -108,3 +108,75 @@ func TestTCPOutsideTheWindow(t *testing.T) {
 		})
 	}
 }
+
+// TestAckOTWSeqInClosing tests that the DUT should send an ACK with
+// the right ACK number when receiving a packet with OTW Seq number
+// in CLOSING state. https://tools.ietf.org/html/rfc793#page-69
+func TestAckOTWSeqInClosing(t *testing.T) {
+	for _, tt := range []struct {
+		description  string
+		flags        header.TCPFlags
+		payloads     testbench.Layers
+		seqNumOffset seqnum.Size
+		expectACK    bool
+	}{
+		{"SYN", header.TCPFlagSyn, nil, 0, true},
+		{"SYNACK", header.TCPFlagSyn | header.TCPFlagAck, nil, 0, true},
+		{"ACK", header.TCPFlagAck, nil, 0, false},
+		{"FINACK", header.TCPFlagFin | header.TCPFlagAck, nil, 0, false},
+		{"Data", header.TCPFlagAck, []testbench.Layer{&testbench.Payload{Bytes: []byte("Sample Data")}}, 0, false},
+
+		{"SYN", header.TCPFlagSyn, nil, 1, true},
+		{"SYNACK", header.TCPFlagSyn | header.TCPFlagAck, nil, 1, true},
+		{"ACK", header.TCPFlagAck, nil, 1, true},
+		{"FINACK", header.TCPFlagFin | header.TCPFlagAck, nil, 1, true},
+		{"Data", header.TCPFlagAck, []testbench.Layer{&testbench.Payload{Bytes: []byte("Sample Data")}}, 1, true},
+
+		{"SYN", header.TCPFlagSyn, nil, 2, true},
+		{"SYNACK", header.TCPFlagSyn | header.TCPFlagAck, nil, 2, true},
+		{"ACK", header.TCPFlagAck, nil, 2, true},
+		{"FINACK", header.TCPFlagFin | header.TCPFlagAck, nil, 2, true},
+		{"Data", header.TCPFlagAck, []testbench.Layer{&testbench.Payload{Bytes: []byte("Sample Data")}}, 2, true},
+	} {
+		t.Run(fmt.Sprintf("%s%d", tt.description, tt.seqNumOffset), func(t *testing.T) {
+			dut := testbench.NewDUT(t)
+			listenFD, remotePort := dut.CreateListener(t, unix.SOCK_STREAM, unix.IPPROTO_TCP, 1)
+			defer dut.Close(t, listenFD)
+			conn := dut.Net.NewTCPIPv4(t, testbench.TCP{DstPort: &remotePort}, testbench.TCP{SrcPort: &remotePort})
+			defer conn.Close(t)
+			conn.Connect(t)
+			acceptFD, _ := dut.Accept(t, listenFD)
+			defer dut.Close(t, acceptFD)
+
+			dut.Shutdown(t, acceptFD, unix.SHUT_WR)
+
+			if _, err := conn.Expect(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagFin | header.TCPFlagAck)}, time.Second); err != nil {
+				t.Fatalf("expected FINACK from DUT, but got none: %s", err)
+			}
+
+			// Do not ack the FIN from DUT so that the TCP state on DUT is CLOSING instead of CLOSED.
+			seqNumForTheirFIN := testbench.Uint32(uint32(*conn.RemoteSeqNum(t)) - 1)
+			conn.Send(t, testbench.TCP{AckNum: seqNumForTheirFIN, Flags: testbench.TCPFlags(header.TCPFlagFin | header.TCPFlagAck)})
+
+			gotTCP, err := conn.Expect(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck)}, time.Second)
+			if err != nil {
+				t.Fatalf("expected an ACK to our FIN, but got none: %s", err)
+			}
+
+			windowSize := seqnum.Size(*gotTCP.WindowSize) + tt.seqNumOffset
+			conn.SendFrameStateless(t, conn.CreateFrame(t, testbench.Layers{&testbench.TCP{
+				SeqNum: testbench.Uint32(uint32(conn.LocalSeqNum(t).Add(windowSize))),
+				AckNum: seqNumForTheirFIN,
+				Flags:  testbench.TCPFlags(tt.flags),
+			}}, tt.payloads...))
+
+			gotACK, err := conn.Expect(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck)}, time.Second)
+			if tt.expectACK && err != nil {
+				t.Errorf("expected an ACK but got none: %s", err)
+			}
+			if !tt.expectACK && gotACK != nil {
+				t.Errorf("expected no ACK but got one: %s", gotACK)
+			}
+		})
+	}
+}
diff --git a/test/packetimpact/tests/tcp_unacc_seq_ack_closing_test.go b/test/packetimpact/tests/tcp_unacc_seq_ack_closing_test.go
deleted file mode 100644
index a208210ac..000000000
--- a/test/packetimpact/tests/tcp_unacc_seq_ack_closing_test.go
+++ /dev/null
@@ -1,94 +0,0 @@
-// Copyright 2021 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_unacc_seq_ack_closing_test
-
-import (
-	"flag"
-	"fmt"
-	"testing"
-	"time"
-
-	"golang.org/x/sys/unix"
-	"gvisor.dev/gvisor/pkg/tcpip/header"
-	"gvisor.dev/gvisor/pkg/tcpip/seqnum"
-	"gvisor.dev/gvisor/test/packetimpact/testbench"
-)
-
-func init() {
-	testbench.Initialize(flag.CommandLine)
-}
-
-func TestSimultaneousCloseUnaccSeqAck(t *testing.T) {
-	for _, tt := range []struct {
-		description    string
-		makeTestingTCP func(t *testing.T, conn *testbench.TCPIPv4, seqNumOffset, windowSize seqnum.Size) testbench.TCP
-		seqNumOffset   seqnum.Size
-		expectAck      bool
-	}{
-		{description: "OTWSeq", makeTestingTCP: testbench.GenerateOTWSeqSegment, seqNumOffset: 0, expectAck: true},
-		{description: "OTWSeq", makeTestingTCP: testbench.GenerateOTWSeqSegment, seqNumOffset: 1, expectAck: true},
-		{description: "OTWSeq", makeTestingTCP: testbench.GenerateOTWSeqSegment, seqNumOffset: 2, expectAck: true},
-		{description: "UnaccAck", makeTestingTCP: testbench.GenerateUnaccACKSegment, seqNumOffset: 0, expectAck: false},
-		{description: "UnaccAck", makeTestingTCP: testbench.GenerateUnaccACKSegment, seqNumOffset: 1, expectAck: true},
-		{description: "UnaccAck", makeTestingTCP: testbench.GenerateUnaccACKSegment, seqNumOffset: 2, expectAck: true},
-	} {
-		t.Run(fmt.Sprintf("%s:offset=%d", tt.description, tt.seqNumOffset), func(t *testing.T) {
-			dut := testbench.NewDUT(t)
-			listenFD, remotePort := dut.CreateListener(t, unix.SOCK_STREAM, unix.IPPROTO_TCP, 1 /*backlog*/)
-			defer dut.Close(t, listenFD)
-			conn := dut.Net.NewTCPIPv4(t, testbench.TCP{DstPort: &remotePort}, testbench.TCP{SrcPort: &remotePort})
-			defer conn.Close(t)
-
-			conn.Connect(t)
-			acceptFD, _ := dut.Accept(t, listenFD)
-
-			// Trigger active close.
-			dut.Shutdown(t, acceptFD, unix.SHUT_WR)
-
-			gotTCP, err := conn.Expect(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagFin | header.TCPFlagAck)}, time.Second)
-			if err != nil {
-				t.Fatalf("expected a FIN: %s", err)
-			}
-			// Do not ack the FIN from DUT so that we get to CLOSING.
-			seqNumForTheirFIN := testbench.Uint32(uint32(*conn.RemoteSeqNum(t)) - 1)
-			conn.Send(t, testbench.TCP{AckNum: seqNumForTheirFIN, Flags: testbench.TCPFlags(header.TCPFlagFin | header.TCPFlagAck)})
-
-			if _, err := conn.Expect(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck)}, time.Second); err != nil {
-				t.Errorf("expected an ACK to our FIN, but got none: %s", err)
-			}
-
-			sampleData := []byte("Sample Data")
-			samplePayload := &testbench.Payload{Bytes: sampleData}
-
-			origSeq := uint32(*conn.LocalSeqNum(t))
-			// Send a segment with OTW Seq / unacc ACK.
-			tcp := tt.makeTestingTCP(t, &conn, tt.seqNumOffset, seqnum.Size(*gotTCP.WindowSize))
-			if tt.description == "OTWSeq" {
-				// If we generate an OTW Seq segment, make sure we don't acknowledge their FIN so that
-				// we stay in CLOSING.
-				tcp.AckNum = seqNumForTheirFIN
-			}
-			conn.Send(t, tcp, samplePayload)
-
-			got, err := conn.Expect(t, testbench.TCP{AckNum: testbench.Uint32(origSeq), Flags: testbench.TCPFlags(header.TCPFlagAck)}, time.Second)
-			if tt.expectAck && err != nil {
-				t.Errorf("expected an ack in CLOSING state, but got none: %s", err)
-			}
-			if !tt.expectAck && got != nil {
-				t.Errorf("expected no ack in CLOSING state, but got one: %s", got)
-			}
-		})
-	}
-}
diff --git a/test/packetimpact/tests/tcp_unacc_seq_ack_test.go b/test/packetimpact/tests/tcp_unacc_seq_ack_test.go
index ce0a26171..389bfc629 100644
--- a/test/packetimpact/tests/tcp_unacc_seq_ack_test.go
+++ b/test/packetimpact/tests/tcp_unacc_seq_ack_test.go
@@ -209,3 +209,66 @@ func TestActiveCloseUnaccpSeqAck(t *testing.T) {
 		})
 	}
 }
+
+func TestSimultaneousCloseUnaccSeqAck(t *testing.T) {
+	for _, tt := range []struct {
+		description    string
+		makeTestingTCP func(t *testing.T, conn *testbench.TCPIPv4, seqNumOffset, windowSize seqnum.Size) testbench.TCP
+		seqNumOffset   seqnum.Size
+		expectAck      bool
+	}{
+		{description: "OTWSeq", makeTestingTCP: testbench.GenerateOTWSeqSegment, seqNumOffset: 0, expectAck: false},
+		{description: "OTWSeq", makeTestingTCP: testbench.GenerateOTWSeqSegment, seqNumOffset: 1, expectAck: true},
+		{description: "OTWSeq", makeTestingTCP: testbench.GenerateOTWSeqSegment, seqNumOffset: 2, expectAck: true},
+		{description: "UnaccAck", makeTestingTCP: testbench.GenerateUnaccACKSegment, seqNumOffset: 0, expectAck: false},
+		{description: "UnaccAck", makeTestingTCP: testbench.GenerateUnaccACKSegment, seqNumOffset: 1, expectAck: true},
+		{description: "UnaccAck", makeTestingTCP: testbench.GenerateUnaccACKSegment, seqNumOffset: 2, expectAck: true},
+	} {
+		t.Run(fmt.Sprintf("%s:offset=%d", tt.description, tt.seqNumOffset), func(t *testing.T) {
+			dut := testbench.NewDUT(t)
+			listenFD, remotePort := dut.CreateListener(t, unix.SOCK_STREAM, unix.IPPROTO_TCP, 1 /*backlog*/)
+			defer dut.Close(t, listenFD)
+			conn := dut.Net.NewTCPIPv4(t, testbench.TCP{DstPort: &remotePort}, testbench.TCP{SrcPort: &remotePort})
+			defer conn.Close(t)
+
+			conn.Connect(t)
+			acceptFD, _ := dut.Accept(t, listenFD)
+
+			// Trigger active close.
+			dut.Shutdown(t, acceptFD, unix.SHUT_WR)
+
+			if _, err := conn.Expect(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagFin | header.TCPFlagAck)}, time.Second); err != nil {
+				t.Fatalf("expected a FIN: %s", err)
+			}
+			// Do not ack the FIN from DUT so that we get to CLOSING.
+			seqNumForTheirFIN := testbench.Uint32(uint32(*conn.RemoteSeqNum(t)) - 1)
+			conn.Send(t, testbench.TCP{AckNum: seqNumForTheirFIN, Flags: testbench.TCPFlags(header.TCPFlagFin | header.TCPFlagAck)})
+
+			gotTCP, err := conn.Expect(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck)}, time.Second)
+			if err != nil {
+				t.Errorf("expected an ACK to our FIN, but got none: %s", err)
+			}
+
+			sampleData := []byte("Sample Data")
+			samplePayload := &testbench.Payload{Bytes: sampleData}
+
+			origSeq := uint32(*conn.LocalSeqNum(t))
+			// Send a segment with OTW Seq / unacc ACK.
+			tcp := tt.makeTestingTCP(t, &conn, tt.seqNumOffset, seqnum.Size(*gotTCP.WindowSize))
+			if tt.description == "OTWSeq" {
+				// If we generate an OTW Seq segment, make sure we don't acknowledge their FIN so that
+				// we stay in CLOSING.
+				tcp.AckNum = seqNumForTheirFIN
+			}
+			conn.Send(t, tcp, samplePayload)
+
+			got, err := conn.Expect(t, testbench.TCP{AckNum: testbench.Uint32(origSeq), Flags: testbench.TCPFlags(header.TCPFlagAck)}, time.Second)
+			if tt.expectAck && err != nil {
+				t.Errorf("expected an ack in CLOSING state, but got none: %s", err)
+			}
+			if !tt.expectAck && got != nil {
+				t.Errorf("expected no ack in CLOSING state, but got one: %s", got)
+			}
+		})
+	}
+}