From 03f17c7d902945489f44bbd4e0c6e15695098b52 Mon Sep 17 00:00:00 2001 From: Mithun Iyer Date: Fri, 4 Jun 2021 23:37:01 -0700 Subject: Honor data and FIN from the ACK completing handshake If the ACK completing the handshake has FIN or data, requeue the segment for further processing by the newly established endpoint. Otherwise, the segments would have to be retransmitted by the peer to be processed by the established endpoint. Doing this, keeps the behavior in parity with Linux. This also addresses a test flake with TCPNonBlockingConnectClose where the ACK (completing the handshake) and multiple retransmitted FINACKs from the peer could be dropped by the listener, when using syncookies and the accept queue is full. The handshake could eventually get completed with a retransmitted FINACK, without actual processing of FIN. This can cause the poll with POLLRDHUP on the accepted socket to sometimes time out before the next FINACK retransmission. PiperOrigin-RevId: 377651695 --- test/packetimpact/testbench/dut.go | 4 ++-- test/packetimpact/tests/BUILD | 1 + test/packetimpact/tests/tcp_syncookie_test.go | 18 +++++++++++++++++- 3 files changed, 20 insertions(+), 3 deletions(-) (limited to 'test') diff --git a/test/packetimpact/testbench/dut.go b/test/packetimpact/testbench/dut.go index 269e163bb..0cac0bf1b 100644 --- a/test/packetimpact/testbench/dut.go +++ b/test/packetimpact/testbench/dut.go @@ -498,8 +498,8 @@ func (dut *DUT) PollOne(t *testing.T, fd int32, events int16, timeout time.Durat if readyFd := pfds[0].Fd; readyFd != fd { t.Fatalf("Poll returned an fd %d that was not requested (%d)", readyFd, fd) } - if got, want := pfds[0].Revents, int16(events); got&want == 0 { - t.Fatalf("Poll returned no events in our interest, got: %#b, want: %#b", got, want) + if got, want := pfds[0].Revents, int16(events); got&want != want { + t.Fatalf("Poll returned events does not include all of the interested events, got: %#b, want: %#b", got, want) } } diff --git a/test/packetimpact/tests/BUILD b/test/packetimpact/tests/BUILD index 37b59b1d9..4cff0cf4c 100644 --- a/test/packetimpact/tests/BUILD +++ b/test/packetimpact/tests/BUILD @@ -384,6 +384,7 @@ packetimpact_testbench( deps = [ "//pkg/tcpip/header", "//test/packetimpact/testbench", + "@com_github_google_go_cmp//cmp:go_default_library", "@org_golang_x_sys//unix:go_default_library", ], ) diff --git a/test/packetimpact/tests/tcp_syncookie_test.go b/test/packetimpact/tests/tcp_syncookie_test.go index 1a016bd1a..6be09996b 100644 --- a/test/packetimpact/tests/tcp_syncookie_test.go +++ b/test/packetimpact/tests/tcp_syncookie_test.go @@ -21,6 +21,7 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" "golang.org/x/sys/unix" "gvisor.dev/gvisor/pkg/tcpip/header" "gvisor.dev/gvisor/test/packetimpact/testbench" @@ -114,7 +115,10 @@ func TestTCPSynCookie(t *testing.T) { t.Fatalf("dut.Poll(...) = %d, want = %d", got, want) } - c.conn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(test.flags)}) + sampleData := []byte("Sample Data") + samplePayload := &testbench.Payload{Bytes: sampleData} + + c.conn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(test.flags)}, samplePayload) pfds = dut.Poll(t, []unix.PollFd{{Fd: listenFD, Events: unix.POLLIN}}, time.Second) want := 0 if test.accept { @@ -126,6 +130,18 @@ func TestTCPSynCookie(t *testing.T) { // Accept the connection to enable poll on any subsequent connection. if test.accept { fd, _ := dut.Accept(t, listenFD) + if test.flags.Contains(header.TCPFlagFin) { + if dut.Uname.IsLinux() { + dut.PollOne(t, fd, unix.POLLIN|unix.POLLRDHUP, time.Second) + } else { + // TODO(gvisor.dev/issue/6015): Notify POLLIN|POLLRDHUP on incoming FIN. + dut.PollOne(t, fd, unix.POLLIN, time.Second) + } + } + got := dut.Recv(t, fd, int32(len(sampleData)), 0) + if diff := cmp.Diff(got, sampleData); diff != "" { + t.Fatalf("dut.Recv: data mismatch (-want +got):\n%s", diff) + } dut.Close(t, fd) } }) -- cgit v1.2.3