diff options
author | Zeling Feng <zeling@google.com> | 2021-03-22 00:04:17 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2021-03-22 00:06:18 -0700 |
commit | cbac2d9f97031bdc18cb301e95db7c052dccc1ee (patch) | |
tree | bf3270a0e5c7fcef8065185869391d46013578e2 | |
parent | 7fac7e32f3a866134bcee499dfc64459946dfe9d (diff) |
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
-rw-r--r-- | test/packetimpact/runner/defs.bzl | 10 | ||||
-rw-r--r-- | test/packetimpact/tests/BUILD | 22 | ||||
-rw-r--r-- | test/packetimpact/tests/tcp_outside_the_window_closing_test.go | 86 | ||||
-rw-r--r-- | test/packetimpact/tests/tcp_outside_the_window_test.go | 72 | ||||
-rw-r--r-- | test/packetimpact/tests/tcp_unacc_seq_ack_closing_test.go | 94 | ||||
-rw-r--r-- | test/packetimpact/tests/tcp_unacc_seq_ack_test.go | 63 |
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) + } + }) + } +} |