Packetimpact test for ACK to OTW Seq segments behavior in CLOSING

TCP, in CLOSING state, MUST send an ACK with next expected SEQ number after
receiving any segment with OTW SEQ number and remain in the same state.
While I am here, I also changed shutdown to behave the same as other calls
in posix_server.

PiperOrigin-RevId: 362976955

11 files changed, 368 insertions, 44 deletions

diff --git a/test/packetimpact/dut/posix_server.cc b/test/packetimpact/dut/posix_server.cc
index 0d93b806e..ea83bbe72 100644
--- a/test/packetimpact/dut/posix_server.cc
+++ b/test/packetimpact/dut/posix_server.cc
@@ -395,7 +395,8 @@ class PosixImpl final : public posix_server::Posix::Service {
   ::grpc::Status Shutdown(grpc::ServerContext *context,
                           const ::posix_server::ShutdownRequest *request,
                           ::posix_server::ShutdownResponse *response) override {
-    if (shutdown(request->fd(), request->how()) < 0) {
+    response->set_ret(shutdown(request->fd(), request->how()));
+    if (response->ret() < 0) {
       response->set_errno_(errno);
     }
     return ::grpc::Status::OK;
diff --git a/test/packetimpact/proto/posix_server.proto b/test/packetimpact/proto/posix_server.proto
index 521f03465..175a65336 100644
--- a/test/packetimpact/proto/posix_server.proto
+++ b/test/packetimpact/proto/posix_server.proto
@@ -214,7 +214,8 @@ message ShutdownRequest {
 }
 
 message ShutdownResponse {
-  int32 errno_ = 1;  // "errno" may fail to compile in c++.
+  int32 ret = 1;
+  int32 errno_ = 2;  // "errno" may fail to compile in c++.
 }
 
 message RecvRequest {
diff --git a/test/packetimpact/runner/defs.bzl b/test/packetimpact/runner/defs.bzl
index 8ce5edf2b..567f64c41 100644
--- a/test/packetimpact/runner/defs.bzl
+++ b/test/packetimpact/runner/defs.bzl
@@ -203,6 +203,11 @@ 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(
@@ -212,6 +217,11 @@ 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,
@@ -277,6 +287,11 @@ ALL_TESTS = [
     PacketimpactTestInfo(
         name = "tcp_info",
     ),
+    PacketimpactTestInfo(
+        name = "tcp_fin_retransmission",
+        # TODO(b/181625316): Fix netstack then remove the line below.
+        expect_netstack_failure = True,
+    ),
 ]
 
 def validate_all_tests():
diff --git a/test/packetimpact/testbench/connections.go b/test/packetimpact/testbench/connections.go
index bafc45d3b..8ad9040ff 100644
--- a/test/packetimpact/testbench/connections.go
+++ b/test/packetimpact/testbench/connections.go
@@ -823,6 +823,27 @@ func (conn *TCPIPv4) LocalAddr(t *testing.T) *unix.SockaddrInet4 {
 	return sa
 }
 
+// GenerateOTWSeqSegment generates a segment with
+// seqnum = RCV.NXT + RCV.WND + seqNumOffset, the generated segment is only
+// acceptable when seqNumOffset is 0, otherwise an ACK is expected from the
+// receiver.
+func GenerateOTWSeqSegment(t *testing.T, conn *TCPIPv4, seqNumOffset seqnum.Size, windowSize seqnum.Size) TCP {
+	t.Helper()
+	lastAcceptable := conn.LocalSeqNum(t).Add(windowSize)
+	otwSeq := uint32(lastAcceptable.Add(seqNumOffset))
+	return TCP{SeqNum: Uint32(otwSeq), Flags: TCPFlags(header.TCPFlagAck)}
+}
+
+// GenerateUnaccACKSegment generates a segment with
+// acknum = SND.NXT + seqNumOffset, the generated segment is only acceptable
+// when seqNumOffset is 0, otherwise an ACK is expected from the receiver.
+func GenerateUnaccACKSegment(t *testing.T, conn *TCPIPv4, seqNumOffset seqnum.Size, windowSize seqnum.Size) TCP {
+	t.Helper()
+	lastAcceptable := conn.RemoteSeqNum(t)
+	unaccAck := uint32(lastAcceptable.Add(seqNumOffset))
+	return TCP{AckNum: Uint32(unaccAck), Flags: TCPFlags(header.TCPFlagAck)}
+}
+
 // IPv4Conn maintains the state for all the layers in a IPv4 connection.
 type IPv4Conn struct {
 	Connection
diff --git a/test/packetimpact/testbench/dut.go b/test/packetimpact/testbench/dut.go
index f4c83ab96..eabdc8cb3 100644
--- a/test/packetimpact/testbench/dut.go
+++ b/test/packetimpact/testbench/dut.go
@@ -773,16 +773,19 @@ func (dut *DUT) SetSockLingerOption(t *testing.T, sockfd int32, timeout time.Dur
 // Shutdown calls shutdown on the DUT and causes a fatal test failure if it
 // doesn't succeed. If more control over the timeout or error handling is
 // needed, use ShutdownWithErrno.
-func (dut *DUT) Shutdown(t *testing.T, fd, how int32) error {
+func (dut *DUT) Shutdown(t *testing.T, fd, how int32) {
 	t.Helper()
 
 	ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout)
 	defer cancel()
-	return dut.ShutdownWithErrno(ctx, t, fd, how)
+	ret, err := dut.ShutdownWithErrno(ctx, t, fd, how)
+	if ret != 0 {
+		t.Fatalf("failed to shutdown(%d, %d): %s", fd, how, err)
+	}
 }
 
 // ShutdownWithErrno calls shutdown on the DUT.
-func (dut *DUT) ShutdownWithErrno(ctx context.Context, t *testing.T, fd, how int32) error {
+func (dut *DUT) ShutdownWithErrno(ctx context.Context, t *testing.T, fd, how int32) (int32, error) {
 	t.Helper()
 
 	req := &pb.ShutdownRequest{
@@ -793,5 +796,8 @@ func (dut *DUT) ShutdownWithErrno(ctx context.Context, t *testing.T, fd, how int
 	if err != nil {
 		t.Fatalf("failed to call Shutdown: %s", err)
 	}
-	return unix.Errno(resp.GetErrno_())
+	if resp.GetErrno_() == 0 {
+		return resp.GetRet(), nil
+	}
+	return resp.GetRet(), unix.Errno(resp.GetErrno_())
 }
diff --git a/test/packetimpact/tests/BUILD b/test/packetimpact/tests/BUILD
index 301cf4980..d5cb0ae06 100644
--- a/test/packetimpact/tests/BUILD
+++ b/test/packetimpact/tests/BUILD
@@ -124,6 +124,17 @@ 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 = [
@@ -155,6 +166,17 @@ 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 = [
@@ -375,6 +397,16 @@ packetimpact_testbench(
     ],
 )
 
+packetimpact_testbench(
+    name = "tcp_fin_retransmission",
+    srcs = ["tcp_fin_retransmission_test.go"],
+    deps = [
+        "//pkg/tcpip/header",
+        "//test/packetimpact/testbench",
+        "@org_golang_x_sys//unix:go_default_library",
+    ],
+)
+
 validate_all_tests()
 
 [packetimpact_go_test(
diff --git a/test/packetimpact/tests/tcp_fin_retransmission_test.go b/test/packetimpact/tests/tcp_fin_retransmission_test.go
new file mode 100644
index 000000000..500f7a783
--- /dev/null
+++ b/test/packetimpact/tests/tcp_fin_retransmission_test.go
@@ -0,0 +1,87 @@
+// 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_fin_retransmission_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.Initialize(flag.CommandLine)
+}
+
+// TestTCPClosingFinRetransmission tests that TCP implementation should retransmit
+// FIN segment in CLOSING state.
+func TestTCPClosingFinRetransmission(t *testing.T) {
+	for _, tt := range []struct {
+		description string
+		flags       header.TCPFlags
+	}{
+		{"CLOSING", header.TCPFlagAck | header.TCPFlagFin},
+		{"FIN_WAIT_1", header.TCPFlagAck},
+	} {
+		t.Run(tt.description, 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)
+
+			// Give a chance for the dut to estimate RTO with RTT from the DATA-ACK.
+			// TODO(gvisor.dev/issue/2685) Estimate RTO during handshake, after which
+			// we can skip the next block of code.
+			sampleData := []byte("Sample Data")
+			if got, want := dut.Send(t, acceptFD, sampleData, 0), len(sampleData); int(got) != want {
+				t.Fatalf("got dut.Send(t, %d, %s, 0) = %d, want %d", acceptFD, sampleData, got, want)
+			}
+			if _, err := conn.ExpectData(t, &testbench.TCP{}, &testbench.Payload{Bytes: sampleData}, time.Second); err != nil {
+				t.Fatalf("expected payload was not received: %s", err)
+			}
+			conn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck)})
+
+			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 we can test for retransmission.
+			seqNumForTheirFIN := testbench.Uint32(uint32(*conn.RemoteSeqNum(t)) - 1)
+			conn.Send(t, testbench.TCP{AckNum: seqNumForTheirFIN, Flags: testbench.TCPFlags(tt.flags)})
+
+			if tt.flags&header.TCPFlagFin != 0 {
+				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)
+				}
+			}
+
+			if _, err := conn.Expect(t, testbench.TCP{
+				SeqNum: seqNumForTheirFIN,
+				Flags:  testbench.TCPFlags(header.TCPFlagFin | header.TCPFlagAck),
+			}, time.Second); err != nil {
+				t.Errorf("expected retransmission of FIN from the DUT: %s", err)
+			}
+		})
+	}
+}
diff --git a/test/packetimpact/tests/tcp_outside_the_window_closing_test.go b/test/packetimpact/tests/tcp_outside_the_window_closing_test.go
new file mode 100644
index 000000000..1097746c7
--- /dev/null
+++ b/test/packetimpact/tests/tcp_outside_the_window_closing_test.go
@@ -0,0 +1,86 @@
+// 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 e442268be..7cd7ff703 100644
--- a/test/packetimpact/tests/tcp_outside_the_window_test.go
+++ b/test/packetimpact/tests/tcp_outside_the_window_test.go
@@ -79,7 +79,7 @@ func TestTCPOutsideTheWindow(t *testing.T) {
 				Flags:  testbench.TCPFlags(tt.tcpFlags),
 				SeqNum: testbench.Uint32(uint32(conn.LocalSeqNum(t).Add(windowSize))),
 			}, tt.payload...)
-			timeout := 3 * time.Second
+			timeout := time.Second
 			gotACK, err := conn.Expect(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck), AckNum: localSeqNum}, timeout)
 			if tt.expectACK && err != nil {
 				t.Fatalf("expected an ACK packet within %s but got none: %s", timeout, err)
diff --git a/test/packetimpact/tests/tcp_unacc_seq_ack_closing_test.go b/test/packetimpact/tests/tcp_unacc_seq_ack_closing_test.go
new file mode 100644
index 000000000..a208210ac
--- /dev/null
+++ b/test/packetimpact/tests/tcp_unacc_seq_ack_closing_test.go
@@ -0,0 +1,94 @@
+// 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 92b54b72e..ce0a26171 100644
--- a/test/packetimpact/tests/tcp_unacc_seq_ack_test.go
+++ b/test/packetimpact/tests/tcp_unacc_seq_ack_test.go
@@ -38,12 +38,12 @@ func TestEstablishedUnaccSeqAck(t *testing.T) {
 		expectAck      bool
 		restoreSeq     bool
 	}{
-		{description: "OTWSeq", makeTestingTCP: generateOTWSeqSegment, seqNumOffset: 0, expectAck: true, restoreSeq: true},
-		{description: "OTWSeq", makeTestingTCP: generateOTWSeqSegment, seqNumOffset: 1, expectAck: true, restoreSeq: true},
-		{description: "OTWSeq", makeTestingTCP: generateOTWSeqSegment, seqNumOffset: 2, expectAck: true, restoreSeq: true},
-		{description: "UnaccAck", makeTestingTCP: generateUnaccACKSegment, seqNumOffset: 0, expectAck: true, restoreSeq: false},
-		{description: "UnaccAck", makeTestingTCP: generateUnaccACKSegment, seqNumOffset: 1, expectAck: false, restoreSeq: true},
-		{description: "UnaccAck", makeTestingTCP: generateUnaccACKSegment, seqNumOffset: 2, expectAck: false, restoreSeq: true},
+		{description: "OTWSeq", makeTestingTCP: testbench.GenerateOTWSeqSegment, seqNumOffset: 0, expectAck: true, restoreSeq: true},
+		{description: "OTWSeq", makeTestingTCP: testbench.GenerateOTWSeqSegment, seqNumOffset: 1, expectAck: true, restoreSeq: true},
+		{description: "OTWSeq", makeTestingTCP: testbench.GenerateOTWSeqSegment, seqNumOffset: 2, expectAck: true, restoreSeq: true},
+		{description: "UnaccAck", makeTestingTCP: testbench.GenerateUnaccACKSegment, seqNumOffset: 0, expectAck: true, restoreSeq: false},
+		{description: "UnaccAck", makeTestingTCP: testbench.GenerateUnaccACKSegment, seqNumOffset: 1, expectAck: false, restoreSeq: true},
+		{description: "UnaccAck", makeTestingTCP: testbench.GenerateUnaccACKSegment, seqNumOffset: 2, expectAck: false, restoreSeq: true},
 	} {
 		t.Run(fmt.Sprintf("%s:offset=%d", tt.description, tt.seqNumOffset), func(t *testing.T) {
 			dut := testbench.NewDUT(t)
@@ -91,12 +91,12 @@ func TestPassiveCloseUnaccSeqAck(t *testing.T) {
 		seqNumOffset   seqnum.Size
 		expectAck      bool
 	}{
-		{description: "OTWSeq", makeTestingTCP: generateOTWSeqSegment, seqNumOffset: 0, expectAck: false},
-		{description: "OTWSeq", makeTestingTCP: generateOTWSeqSegment, seqNumOffset: 1, expectAck: true},
-		{description: "OTWSeq", makeTestingTCP: generateOTWSeqSegment, seqNumOffset: 2, expectAck: true},
-		{description: "UnaccAck", makeTestingTCP: generateUnaccACKSegment, seqNumOffset: 0, expectAck: false},
-		{description: "UnaccAck", makeTestingTCP: generateUnaccACKSegment, seqNumOffset: 1, expectAck: true},
-		{description: "UnaccAck", makeTestingTCP: generateUnaccACKSegment, seqNumOffset: 2, expectAck: true},
+		{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)
@@ -152,12 +152,12 @@ func TestActiveCloseUnaccpSeqAck(t *testing.T) {
 		seqNumOffset   seqnum.Size
 		restoreSeq     bool
 	}{
-		{description: "OTWSeq", makeTestingTCP: generateOTWSeqSegment, seqNumOffset: 0, restoreSeq: true},
-		{description: "OTWSeq", makeTestingTCP: generateOTWSeqSegment, seqNumOffset: 1, restoreSeq: true},
-		{description: "OTWSeq", makeTestingTCP: generateOTWSeqSegment, seqNumOffset: 2, restoreSeq: true},
-		{description: "UnaccAck", makeTestingTCP: generateUnaccACKSegment, seqNumOffset: 0, restoreSeq: false},
-		{description: "UnaccAck", makeTestingTCP: generateUnaccACKSegment, seqNumOffset: 1, restoreSeq: true},
-		{description: "UnaccAck", makeTestingTCP: generateUnaccACKSegment, seqNumOffset: 2, restoreSeq: true},
+		{description: "OTWSeq", makeTestingTCP: testbench.GenerateOTWSeqSegment, seqNumOffset: 0, restoreSeq: true},
+		{description: "OTWSeq", makeTestingTCP: testbench.GenerateOTWSeqSegment, seqNumOffset: 1, restoreSeq: true},
+		{description: "OTWSeq", makeTestingTCP: testbench.GenerateOTWSeqSegment, seqNumOffset: 2, restoreSeq: true},
+		{description: "UnaccAck", makeTestingTCP: testbench.GenerateUnaccACKSegment, seqNumOffset: 0, restoreSeq: false},
+		{description: "UnaccAck", makeTestingTCP: testbench.GenerateUnaccACKSegment, seqNumOffset: 1, restoreSeq: true},
+		{description: "UnaccAck", makeTestingTCP: testbench.GenerateUnaccACKSegment, seqNumOffset: 2, restoreSeq: true},
 	} {
 		t.Run(fmt.Sprintf("%s:offset=%d", tt.description, tt.seqNumOffset), func(t *testing.T) {
 			dut := testbench.NewDUT(t)
@@ -209,22 +209,3 @@ func TestActiveCloseUnaccpSeqAck(t *testing.T) {
 		})
 	}
 }
-
-// generateOTWSeqSegment generates an segment with
-// seqnum = RCV.NXT + RCV.WND + seqNumOffset, the generated segment is only
-// acceptable when seqNumOffset is 0, otherwise an ACK is expected from the
-// receiver.
-func generateOTWSeqSegment(t *testing.T, conn *testbench.TCPIPv4, seqNumOffset seqnum.Size, windowSize seqnum.Size) testbench.TCP {
-	lastAcceptable := conn.LocalSeqNum(t).Add(windowSize)
-	otwSeq := uint32(lastAcceptable.Add(seqNumOffset))
-	return testbench.TCP{SeqNum: testbench.Uint32(otwSeq), Flags: testbench.TCPFlags(header.TCPFlagAck)}
-}
-
-// generateUnaccACKSegment generates an segment with
-// acknum = SND.NXT + seqNumOffset, the generated segment is only acceptable
-// when seqNumOffset is 0, otherwise an ACK is expected from the receiver.
-func generateUnaccACKSegment(t *testing.T, conn *testbench.TCPIPv4, seqNumOffset seqnum.Size, windowSize seqnum.Size) testbench.TCP {
-	lastAcceptable := conn.RemoteSeqNum(t)
-	unaccAck := uint32(lastAcceptable.Add(seqNumOffset))
-	return testbench.TCP{AckNum: testbench.Uint32(unaccAck), Flags: testbench.TCPFlags(header.TCPFlagAck)}
-}