From ddfdc9827b6cecc8d2c9d8c9e9afb80d85597c44 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Tue, 22 Jun 2021 14:10:05 -0700 Subject: Remove timeouts These aren't useful and create opportunities for flakes. PiperOrigin-RevId: 380889223 --- test/packetimpact/testbench/dut.go | 83 +++++-------------- test/packetimpact/testbench/testbench.go | 3 - .../tests/generic_dgram_socket_send_recv_test.go | 9 +- .../tests/tcp_connect_icmp_error_test.go | 35 ++++---- test/packetimpact/tests/tcp_linger_test.go | 82 +++++++++--------- .../tests/tcp_network_unreachable_test.go | 8 +- .../tests/tcp_queue_send_recv_in_syn_sent_test.go | 96 +++++++++++----------- .../tests/udp_icmp_error_propagation_test.go | 19 +---- 8 files changed, 133 insertions(+), 202 deletions(-) diff --git a/test/packetimpact/testbench/dut.go b/test/packetimpact/testbench/dut.go index 0cac0bf1b..7e89ba2b3 100644 --- a/test/packetimpact/testbench/dut.go +++ b/test/packetimpact/testbench/dut.go @@ -180,9 +180,7 @@ func (dut *DUT) CreateListener(t *testing.T, typ, proto, backlog int32) (int32, func (dut *DUT) Accept(t *testing.T, sockfd int32) (int32, unix.Sockaddr) { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - fd, sa, err := dut.AcceptWithErrno(ctx, t, sockfd) + fd, sa, err := dut.AcceptWithErrno(context.Background(), t, sockfd) if fd < 0 { t.Fatalf("failed to accept: %s", err) } @@ -209,9 +207,7 @@ func (dut *DUT) AcceptWithErrno(ctx context.Context, t *testing.T, sockfd int32) func (dut *DUT) Bind(t *testing.T, fd int32, sa unix.Sockaddr) { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, err := dut.BindWithErrno(ctx, t, fd, sa) + ret, err := dut.BindWithErrno(context.Background(), t, fd, sa) if ret != 0 { t.Fatalf("failed to bind socket: %s", err) } @@ -238,9 +234,7 @@ func (dut *DUT) BindWithErrno(ctx context.Context, t *testing.T, fd int32, sa un func (dut *DUT) Close(t *testing.T, fd int32) { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, err := dut.CloseWithErrno(ctx, t, fd) + ret, err := dut.CloseWithErrno(context.Background(), t, fd) if ret != 0 { t.Fatalf("failed to close: %s", err) } @@ -266,9 +260,7 @@ func (dut *DUT) CloseWithErrno(ctx context.Context, t *testing.T, fd int32) (int func (dut *DUT) Connect(t *testing.T, fd int32, sa unix.Sockaddr) { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, err := dut.ConnectWithErrno(ctx, t, fd, sa) + ret, err := dut.ConnectWithErrno(context.Background(), t, fd, sa) // Ignore 'operation in progress' error that can be returned when the socket // is non-blocking. if err != unix.EINPROGRESS && ret != 0 { @@ -297,9 +289,7 @@ func (dut *DUT) ConnectWithErrno(ctx context.Context, t *testing.T, fd int32, sa func (dut *DUT) GetSockName(t *testing.T, sockfd int32) unix.Sockaddr { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, sa, err := dut.GetSockNameWithErrno(ctx, t, sockfd) + ret, sa, err := dut.GetSockNameWithErrno(context.Background(), t, sockfd) if ret != 0 { t.Fatalf("failed to getsockname: %s", err) } @@ -349,9 +339,7 @@ func (dut *DUT) getSockOpt(ctx context.Context, t *testing.T, sockfd, level, opt func (dut *DUT) GetSockOpt(t *testing.T, sockfd, level, optname, optlen int32) []byte { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, optval, err := dut.GetSockOptWithErrno(ctx, t, sockfd, level, optname, optlen) + ret, optval, err := dut.GetSockOptWithErrno(context.Background(), t, sockfd, level, optname, optlen) if ret != 0 { t.Fatalf("failed to GetSockOpt: %s", err) } @@ -378,9 +366,7 @@ func (dut *DUT) GetSockOptWithErrno(ctx context.Context, t *testing.T, sockfd, l func (dut *DUT) GetSockOptInt(t *testing.T, sockfd, level, optname int32) int32 { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, intval, err := dut.GetSockOptIntWithErrno(ctx, t, sockfd, level, optname) + ret, intval, err := dut.GetSockOptIntWithErrno(context.Background(), t, sockfd, level, optname) if ret != 0 { t.Fatalf("failed to GetSockOptInt: %s", err) } @@ -405,9 +391,7 @@ func (dut *DUT) GetSockOptIntWithErrno(ctx context.Context, t *testing.T, sockfd func (dut *DUT) GetSockOptTimeval(t *testing.T, sockfd, level, optname int32) unix.Timeval { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, timeval, err := dut.GetSockOptTimevalWithErrno(ctx, t, sockfd, level, optname) + ret, timeval, err := dut.GetSockOptTimevalWithErrno(context.Background(), t, sockfd, level, optname) if ret != 0 { t.Fatalf("failed to GetSockOptTimeval: %s", err) } @@ -434,9 +418,7 @@ func (dut *DUT) GetSockOptTimevalWithErrno(ctx context.Context, t *testing.T, so func (dut *DUT) GetSockOptTCPInfo(t *testing.T, sockfd int32) linux.TCPInfo { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, info, err := dut.GetSockOptTCPInfoWithErrno(ctx, t, sockfd) + ret, info, err := dut.GetSockOptTCPInfoWithErrno(context.Background(), t, sockfd) if ret != 0 || err != unix.Errno(0) { t.Fatalf("failed to GetSockOptTCPInfo: %s", err) } @@ -463,9 +445,7 @@ func (dut *DUT) GetSockOptTCPInfoWithErrno(ctx context.Context, t *testing.T, so func (dut *DUT) Listen(t *testing.T, sockfd, backlog int32) { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, err := dut.ListenWithErrno(ctx, t, sockfd, backlog) + ret, err := dut.ListenWithErrno(context.Background(), t, sockfd, backlog) if ret != 0 { t.Fatalf("failed to listen: %s", err) } @@ -510,13 +490,7 @@ func (dut *DUT) PollOne(t *testing.T, fd int32, events int16, timeout time.Durat func (dut *DUT) Poll(t *testing.T, pfds []unix.PollFd, timeout time.Duration) []unix.PollFd { t.Helper() - ctx := context.Background() - var cancel context.CancelFunc - if timeout >= 0 { - ctx, cancel = context.WithTimeout(ctx, timeout+RPCTimeout) - defer cancel() - } - ret, result, err := dut.PollWithErrno(ctx, t, pfds, timeout) + ret, result, err := dut.PollWithErrno(context.Background(), t, pfds, timeout) if ret < 0 { t.Fatalf("failed to poll: %s", err) } @@ -559,9 +533,7 @@ func (dut *DUT) PollWithErrno(ctx context.Context, t *testing.T, pfds []unix.Pol func (dut *DUT) Send(t *testing.T, sockfd int32, buf []byte, flags int32) int32 { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, err := dut.SendWithErrno(ctx, t, sockfd, buf, flags) + ret, err := dut.SendWithErrno(context.Background(), t, sockfd, buf, flags) if ret == -1 { t.Fatalf("failed to send: %s", err) } @@ -590,9 +562,7 @@ func (dut *DUT) SendWithErrno(ctx context.Context, t *testing.T, sockfd int32, b func (dut *DUT) SendTo(t *testing.T, sockfd int32, buf []byte, flags int32, destAddr unix.Sockaddr) int32 { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, err := dut.SendToWithErrno(ctx, t, sockfd, buf, flags, destAddr) + ret, err := dut.SendToWithErrno(context.Background(), t, sockfd, buf, flags, destAddr) if ret == -1 { t.Fatalf("failed to sendto: %s", err) } @@ -625,10 +595,8 @@ func (dut *DUT) SetNonBlocking(t *testing.T, fd int32, nonblocking bool) { Fd: fd, Nonblocking: nonblocking, } - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - resp, err := dut.posixServer.SetNonblocking(ctx, req) + resp, err := dut.posixServer.SetNonblocking(context.Background(), req) if err != nil { t.Fatalf("failed to call SetNonblocking: %s", err) } @@ -661,9 +629,7 @@ func (dut *DUT) setSockOpt(ctx context.Context, t *testing.T, sockfd, level, opt func (dut *DUT) SetSockOpt(t *testing.T, sockfd, level, optname int32, optval []byte) { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, err := dut.SetSockOptWithErrno(ctx, t, sockfd, level, optname, optval) + ret, err := dut.SetSockOptWithErrno(context.Background(), t, sockfd, level, optname, optval) if ret != 0 { t.Fatalf("failed to SetSockOpt: %s", err) } @@ -684,9 +650,7 @@ func (dut *DUT) SetSockOptWithErrno(ctx context.Context, t *testing.T, sockfd, l func (dut *DUT) SetSockOptInt(t *testing.T, sockfd, level, optname, optval int32) { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, err := dut.SetSockOptIntWithErrno(ctx, t, sockfd, level, optname, optval) + ret, err := dut.SetSockOptIntWithErrno(context.Background(), t, sockfd, level, optname, optval) if ret != 0 { t.Fatalf("failed to SetSockOptInt: %s", err) } @@ -705,9 +669,7 @@ func (dut *DUT) SetSockOptIntWithErrno(ctx context.Context, t *testing.T, sockfd func (dut *DUT) SetSockOptTimeval(t *testing.T, sockfd, level, optname int32, tv *unix.Timeval) { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, err := dut.SetSockOptTimevalWithErrno(ctx, t, sockfd, level, optname, tv) + ret, err := dut.SetSockOptTimevalWithErrno(context.Background(), t, sockfd, level, optname, tv) if ret != 0 { t.Fatalf("failed to SetSockOptTimeval: %s", err) } @@ -746,8 +708,7 @@ func (dut *DUT) SocketWithErrno(t *testing.T, domain, typ, proto int32) (int32, Type: typ, Protocol: proto, } - ctx := context.Background() - resp, err := dut.posixServer.Socket(ctx, req) + resp, err := dut.posixServer.Socket(context.Background(), req) if err != nil { t.Fatalf("failed to call Socket: %s", err) } @@ -760,9 +721,7 @@ func (dut *DUT) SocketWithErrno(t *testing.T, domain, typ, proto int32) (int32, func (dut *DUT) Recv(t *testing.T, sockfd, len, flags int32) []byte { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, buf, err := dut.RecvWithErrno(ctx, t, sockfd, len, flags) + ret, buf, err := dut.RecvWithErrno(context.Background(), t, sockfd, len, flags) if ret == -1 { t.Fatalf("failed to recv: %s", err) } @@ -805,9 +764,7 @@ func (dut *DUT) SetSockLingerOption(t *testing.T, sockfd int32, timeout time.Dur func (dut *DUT) Shutdown(t *testing.T, fd, how int32) { t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) - defer cancel() - ret, err := dut.ShutdownWithErrno(ctx, t, fd, how) + ret, err := dut.ShutdownWithErrno(context.Background(), t, fd, how) if ret != 0 { t.Fatalf("failed to shutdown(%d, %d): %s", fd, how, err) } diff --git a/test/packetimpact/testbench/testbench.go b/test/packetimpact/testbench/testbench.go index caa389780..38ae9c1d7 100644 --- a/test/packetimpact/testbench/testbench.go +++ b/test/packetimpact/testbench/testbench.go @@ -31,8 +31,6 @@ var ( Native = false // RPCKeepalive is the gRPC keepalive. RPCKeepalive = 10 * time.Second - // RPCTimeout is the gRPC timeout. - RPCTimeout = 100 * time.Millisecond // dutInfosJSON is the json string that describes information about all the // duts available to use. @@ -124,7 +122,6 @@ func (n *DUTTestNet) SubnetBroadcast() net.IP { // functions. func registerFlags(fs *flag.FlagSet) { fs.BoolVar(&Native, "native", Native, "whether the test is running natively") - fs.DurationVar(&RPCTimeout, "rpc_timeout", RPCTimeout, "gRPC timeout") fs.DurationVar(&RPCKeepalive, "rpc_keepalive", RPCKeepalive, "gRPC keepalive") fs.StringVar(&dutInfosJSON, "dut_infos_json", dutInfosJSON, "json that describes the DUTs") } diff --git a/test/packetimpact/tests/generic_dgram_socket_send_recv_test.go b/test/packetimpact/tests/generic_dgram_socket_send_recv_test.go index 00e0f7690..a9ffafc74 100644 --- a/test/packetimpact/tests/generic_dgram_socket_send_recv_test.go +++ b/test/packetimpact/tests/generic_dgram_socket_send_recv_test.go @@ -48,7 +48,6 @@ func maxUDPPayloadSize(addr net.IP) int { func init() { testbench.Initialize(flag.CommandLine) - testbench.RPCTimeout = 500 * time.Millisecond } func expectedEthLayer(t *testing.T, dut testbench.DUT, socketFD int32, sendTo net.IP) testbench.Layer { @@ -437,9 +436,7 @@ func (test *icmpV6Test) Send(t *testing.T, dut testbench.DUT, bindTo, sendTo net copy(destSockaddr.Addr[:], sendTo.To16()) // Tell the DUT to send a packet out the ICMPv6 socket. - ctx, cancel := context.WithTimeout(context.Background(), testbench.RPCTimeout) - defer cancel() - gotRet, gotErrno := dut.SendToWithErrno(ctx, t, env.socketFD, bytes, 0, &destSockaddr) + gotRet, gotErrno := dut.SendToWithErrno(context.Background(), t, env.socketFD, bytes, 0, &destSockaddr) if gotErrno != wantErrno { t.Fatalf("got dut.SendToWithErrno(_, _, %d, _, _, %s) = (_, %s), want = (_, %s)", env.socketFD, sendTo, gotErrno, wantErrno) @@ -677,9 +674,7 @@ func (test *udpTest) Send(t *testing.T, dut testbench.DUT, bindTo, sendTo net.IP } // Tell the DUT to send a packet out the UDP socket. - ctx, cancel := context.WithTimeout(context.Background(), testbench.RPCTimeout) - defer cancel() - gotRet, gotErrno := dut.SendToWithErrno(ctx, t, env.socketFD, payload, 0, destSockaddr) + gotRet, gotErrno := dut.SendToWithErrno(context.Background(), t, env.socketFD, payload, 0, destSockaddr) if gotErrno != wantErrno { t.Fatalf("got dut.SendToWithErrno(_, _, %d, _, _, %s) = (_, %s), want = (_, %s)", env.socketFD, sendTo, gotErrno, wantErrno) diff --git a/test/packetimpact/tests/tcp_connect_icmp_error_test.go b/test/packetimpact/tests/tcp_connect_icmp_error_test.go index 3b4c4cd63..15d603328 100644 --- a/test/packetimpact/tests/tcp_connect_icmp_error_test.go +++ b/test/packetimpact/tests/tcp_connect_icmp_error_test.go @@ -15,9 +15,7 @@ package tcp_connect_icmp_error_test import ( - "context" "flag" - "sync" "testing" "time" @@ -66,35 +64,38 @@ func TestTCPConnectICMPError(t *testing.T) { t.Fatalf("expected SYN, %s", err) } - done := make(chan bool) - defer close(done) - var wg sync.WaitGroup - defer wg.Wait() - wg.Add(1) - var block sync.WaitGroup - block.Add(1) + // Continuously try to read the ICMP error in an attempt to trigger a race + // condition. + start := make(chan struct{}) + done := make(chan struct{}) go func() { - defer wg.Done() - _, cancel := context.WithTimeout(context.Background(), time.Second*3) - defer cancel() + defer close(done) - block.Done() + close(start) for { select { case <-done: return default: - if errno := dut.GetSockOptInt(t, clientFD, unix.SOL_SOCKET, unix.SO_ERROR); errno != 0 { - return - } } + const want = unix.EHOSTUNREACH + switch got := unix.Errno(dut.GetSockOptInt(t, clientFD, unix.SOL_SOCKET, unix.SO_ERROR)); got { + case unix.Errno(0): + continue + case want: + return + default: + t.Fatalf("got SO_ERROR = %s, want %s", got, want) + } + } }() - block.Wait() + <-start sendICMPError(t, &conn, tcp) dut.PollOne(t, clientFD, unix.POLLHUP, time.Second) + <-done conn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck)}) // The DUT should reply with RST to our ACK as the state should have diff --git a/test/packetimpact/tests/tcp_linger_test.go b/test/packetimpact/tests/tcp_linger_test.go index 88942904d..46b5ca5d8 100644 --- a/test/packetimpact/tests/tcp_linger_test.go +++ b/test/packetimpact/tests/tcp_linger_test.go @@ -98,20 +98,19 @@ func TestTCPLingerNonZeroTimeout(t *testing.T) { dut.SetSockLingerOption(t, acceptFD, lingerDuration, tt.lingerOn) - // Increase timeout as Close will take longer time to - // return when SO_LINGER is set with non-zero timeout. - timeout := lingerDuration + 1*time.Second - ctx, cancel := context.WithTimeout(context.Background(), timeout) - defer cancel() start := time.Now() - dut.CloseWithErrno(ctx, t, acceptFD) - end := time.Now() - diff := end.Sub(start) - - if tt.lingerOn && diff < lingerDuration { - t.Errorf("expected close to return after %v seconds, but returned sooner", lingerDuration) - } else if !tt.lingerOn && diff > 1*time.Second { - t.Errorf("expected close to return within a second, but returned later") + dut.CloseWithErrno(context.Background(), t, acceptFD) + elapsed := time.Since(start) + + expectedMaximum := time.Second + if tt.lingerOn { + expectedMaximum += lingerDuration + if elapsed < lingerDuration { + t.Errorf("expected close to take at least %s, but took %s", lingerDuration, elapsed) + } + } + if elapsed >= expectedMaximum { + t.Errorf("expected close to take at most %s, but took %s", expectedMaximum, elapsed) } if _, err := conn.Expect(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagFin | header.TCPFlagAck)}, time.Second); err != nil { @@ -144,20 +143,19 @@ func TestTCPLingerSendNonZeroTimeout(t *testing.T) { sampleData := []byte("Sample Data") dut.Send(t, acceptFD, sampleData, 0) - // Increase timeout as Close will take longer time to - // return when SO_LINGER is set with non-zero timeout. - timeout := lingerDuration + 1*time.Second - ctx, cancel := context.WithTimeout(context.Background(), timeout) - defer cancel() start := time.Now() - dut.CloseWithErrno(ctx, t, acceptFD) - end := time.Now() - diff := end.Sub(start) - - if tt.lingerOn && diff < lingerDuration { - t.Errorf("expected close to return after %v seconds, but returned sooner", lingerDuration) - } else if !tt.lingerOn && diff > 1*time.Second { - t.Errorf("expected close to return within a second, but returned later") + dut.CloseWithErrno(context.Background(), t, acceptFD) + elapsed := time.Since(start) + + expectedMaximum := time.Second + if tt.lingerOn { + expectedMaximum += lingerDuration + if elapsed < lingerDuration { + t.Errorf("expected close to take at least %s, but took %s", lingerDuration, elapsed) + } + } + if elapsed >= expectedMaximum { + t.Errorf("expected close to take at most %s, but took %s", expectedMaximum, elapsed) } samplePayload := &testbench.Payload{Bytes: sampleData} @@ -221,20 +219,19 @@ func TestTCPLingerShutdownSendNonZeroTimeout(t *testing.T) { dut.Shutdown(t, acceptFD, unix.SHUT_RDWR) - // Increase timeout as Close will take longer time to - // return when SO_LINGER is set with non-zero timeout. - timeout := lingerDuration + 1*time.Second - ctx, cancel := context.WithTimeout(context.Background(), timeout) - defer cancel() start := time.Now() - dut.CloseWithErrno(ctx, t, acceptFD) - end := time.Now() - diff := end.Sub(start) - - if tt.lingerOn && diff < lingerDuration { - t.Errorf("expected close to return after %v seconds, but returned sooner", lingerDuration) - } else if !tt.lingerOn && diff > 1*time.Second { - t.Errorf("expected close to return within a second, but returned later") + dut.CloseWithErrno(context.Background(), t, acceptFD) + elapsed := time.Since(start) + + expectedMaximum := time.Second + if tt.lingerOn { + expectedMaximum += lingerDuration + if elapsed < lingerDuration { + t.Errorf("expected close to take at least %s, but took %s", lingerDuration, elapsed) + } + } + if elapsed >= expectedMaximum { + t.Errorf("expected close to take at most %s, but took %s", expectedMaximum, elapsed) } samplePayload := &testbench.Payload{Bytes: sampleData} @@ -259,9 +256,10 @@ func TestTCPLingerNonEstablished(t *testing.T) { // and return immediately. start := time.Now() dut.CloseWithErrno(context.Background(), t, newFD) - diff := time.Since(start) + elapsed := time.Since(start) - if diff > lingerDuration { - t.Errorf("expected close to return within %s, but returned after %s", lingerDuration, diff) + expectedMaximum := time.Second + if elapsed >= time.Second { + t.Errorf("expected close to take at most %s, but took %s", expectedMaximum, elapsed) } } diff --git a/test/packetimpact/tests/tcp_network_unreachable_test.go b/test/packetimpact/tests/tcp_network_unreachable_test.go index 60a2dbf3d..e92e6aa9b 100644 --- a/test/packetimpact/tests/tcp_network_unreachable_test.go +++ b/test/packetimpact/tests/tcp_network_unreachable_test.go @@ -41,11 +41,9 @@ func TestTCPSynSentUnreachable(t *testing.T) { defer conn.Close(t) // Bring the DUT to SYN-SENT state with a non-blocking connect. - ctx, cancel := context.WithTimeout(context.Background(), testbench.RPCTimeout) - defer cancel() sa := unix.SockaddrInet4{Port: int(port)} copy(sa.Addr[:], dut.Net.LocalIPv4) - if _, err := dut.ConnectWithErrno(ctx, t, clientFD, &sa); err != unix.EINPROGRESS { + if _, err := dut.ConnectWithErrno(context.Background(), t, clientFD, &sa); err != unix.EINPROGRESS { t.Errorf("got connect() = %v, want EINPROGRESS", err) } @@ -86,14 +84,12 @@ func TestTCPSynSentUnreachable6(t *testing.T) { defer conn.Close(t) // Bring the DUT to SYN-SENT state with a non-blocking connect. - ctx, cancel := context.WithTimeout(context.Background(), testbench.RPCTimeout) - defer cancel() sa := unix.SockaddrInet6{ Port: int(conn.SrcPort()), ZoneId: dut.Net.RemoteDevID, } copy(sa.Addr[:], dut.Net.LocalIPv6) - if _, err := dut.ConnectWithErrno(ctx, t, clientFD, &sa); err != unix.EINPROGRESS { + if _, err := dut.ConnectWithErrno(context.Background(), t, clientFD, &sa); err != unix.EINPROGRESS { t.Errorf("got connect() = %v, want EINPROGRESS", err) } diff --git a/test/packetimpact/tests/tcp_queue_send_recv_in_syn_sent_test.go b/test/packetimpact/tests/tcp_queue_send_recv_in_syn_sent_test.go index 1c8b72ebe..974c15384 100644 --- a/test/packetimpact/tests/tcp_queue_send_recv_in_syn_sent_test.go +++ b/test/packetimpact/tests/tcp_queue_send_recv_in_syn_sent_test.go @@ -20,7 +20,6 @@ import ( "encoding/hex" "errors" "flag" - "sync" "testing" "time" @@ -54,37 +53,39 @@ func TestQueueSendInSynSentHandshake(t *testing.T) { // Test blocking send. dut.SetNonBlocking(t, socket, false) - var wg sync.WaitGroup - defer wg.Wait() - wg.Add(1) - var block sync.WaitGroup - block.Add(1) + start := make(chan struct{}) + done := make(chan struct{}) go func() { - defer wg.Done() - ctx, cancel := context.WithTimeout(context.Background(), time.Second*3) - defer cancel() + defer close(done) - block.Done() + close(start) // Issue SEND call in SYN-SENT, this should be queued for // process until the connection is established. - n, err := dut.SendWithErrno(ctx, t, socket, sampleData, 0) - if n == -1 { + if _, err := dut.SendWithErrno(context.Background(), t, socket, sampleData, 0); err != unix.Errno(0) { t.Errorf("failed to send on DUT: %s", err) - return } }() // Wait for the goroutine to be scheduled and before it // blocks on endpoint send/receive. - block.Wait() + <-start // The following sleep is used to prevent the connection // from being established before we are blocked: there is // still a small time window between we sending the RPC // request and the system actually being blocked. time.Sleep(100 * time.Millisecond) + select { + case <-done: + t.Fatal("expected send to be blocked in SYN-SENT") + default: + } + // Bring the connection to Established. conn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagSyn | header.TCPFlagAck)}) + + <-done + // Expect the data from the DUT's enqueued send request. // // On Linux, this can be piggybacked with the ACK completing the @@ -126,21 +127,16 @@ func TestQueueRecvInSynSentHandshake(t *testing.T) { // Test blocking read. dut.SetNonBlocking(t, socket, false) - var wg sync.WaitGroup - defer wg.Wait() - wg.Add(1) - var block sync.WaitGroup - block.Add(1) + start := make(chan struct{}) + done := make(chan struct{}) go func() { - defer wg.Done() - ctx, cancel := context.WithTimeout(context.Background(), time.Second*3) - defer cancel() + defer close(done) - block.Done() + close(start) // Issue RECEIVE call in SYN-SENT, this should be queued for // process until the connection is established. - n, buff, err := dut.RecvWithErrno(ctx, t, socket, int32(len(sampleData)), 0) - if n == -1 { + n, buff, err := dut.RecvWithErrno(context.Background(), t, socket, int32(len(sampleData)), 0) + if err != unix.Errno(0) { t.Errorf("failed to recv on DUT: %s", err) return } @@ -151,7 +147,8 @@ func TestQueueRecvInSynSentHandshake(t *testing.T) { // Wait for the goroutine to be scheduled and before it // blocks on endpoint send/receive. - block.Wait() + <-start + // The following sleep is used to prevent the connection // from being established before we are blocked: there is // still a small time window between we sending the RPC @@ -169,6 +166,8 @@ func TestQueueRecvInSynSentHandshake(t *testing.T) { if _, err := conn.Expect(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagAck)}, time.Second); err != nil { t.Fatalf("expected an ACK from DUT, but got none: %s", err) } + + <-done } // TestQueueSendInSynSentRST tests send behavior when the TCP state @@ -192,20 +191,15 @@ func TestQueueSendInSynSentRST(t *testing.T) { // Test blocking send. dut.SetNonBlocking(t, socket, false) - var wg sync.WaitGroup - defer wg.Wait() - wg.Add(1) - var block sync.WaitGroup - block.Add(1) + start := make(chan struct{}) + done := make(chan struct{}) go func() { - defer wg.Done() - ctx, cancel := context.WithTimeout(context.Background(), time.Second*3) - defer cancel() + defer close(done) - block.Done() + close(start) // Issue SEND call in SYN-SENT, this should be queued for // process until the connection is established. - n, err := dut.SendWithErrno(ctx, t, socket, sampleData, 0) + n, err := dut.SendWithErrno(context.Background(), t, socket, sampleData, 0) if err != unix.ECONNREFUSED { t.Errorf("expected error %s, got %s", unix.ECONNREFUSED, err) } @@ -216,14 +210,23 @@ func TestQueueSendInSynSentRST(t *testing.T) { // Wait for the goroutine to be scheduled and before it // blocks on endpoint send/receive. - block.Wait() + <-start + // The following sleep is used to prevent the connection // from being established before we are blocked: there is // still a small time window between we sending the RPC // request and the system actually being blocked. time.Sleep(100 * time.Millisecond) + select { + case <-done: + t.Fatal("expected send to be blocked in SYN-SENT") + default: + } + conn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagRst | header.TCPFlagAck)}) + + <-done } // TestQueueRecvInSynSentRST tests recv behavior when the TCP state @@ -251,20 +254,15 @@ func TestQueueRecvInSynSentRST(t *testing.T) { // Test blocking read. dut.SetNonBlocking(t, socket, false) - var wg sync.WaitGroup - defer wg.Wait() - wg.Add(1) - var block sync.WaitGroup - block.Add(1) + start := make(chan struct{}) + done := make(chan struct{}) go func() { - defer wg.Done() - ctx, cancel := context.WithTimeout(context.Background(), time.Second*3) - defer cancel() + defer close(done) - block.Done() + close(start) // Issue RECEIVE call in SYN-SENT, this should be queued for // process until the connection is established. - n, _, err := dut.RecvWithErrno(ctx, t, socket, int32(len(sampleData)), 0) + n, _, err := dut.RecvWithErrno(context.Background(), t, socket, int32(len(sampleData)), 0) if err != unix.ECONNREFUSED { t.Errorf("expected error %s, got %s", unix.ECONNREFUSED, err) } @@ -275,7 +273,8 @@ func TestQueueRecvInSynSentRST(t *testing.T) { // Wait for the goroutine to be scheduled and before it // blocks on endpoint send/receive. - block.Wait() + <-start + // The following sleep is used to prevent the connection // from being established before we are blocked: there is // still a small time window between we sending the RPC @@ -283,4 +282,5 @@ func TestQueueRecvInSynSentRST(t *testing.T) { time.Sleep(100 * time.Millisecond) conn.Send(t, testbench.TCP{Flags: testbench.TCPFlags(header.TCPFlagRst | header.TCPFlagAck)}) + <-done } diff --git a/test/packetimpact/tests/udp_icmp_error_propagation_test.go b/test/packetimpact/tests/udp_icmp_error_propagation_test.go index 087aeb66e..bb33ca4b3 100644 --- a/test/packetimpact/tests/udp_icmp_error_propagation_test.go +++ b/test/packetimpact/tests/udp_icmp_error_propagation_test.go @@ -141,8 +141,6 @@ func testRecv(ctx context.Context, t *testing.T, d testData) { d.conn.Send(t, testbench.UDP{}) if d.wantErrno != unix.Errno(0) { - ctx, cancel := context.WithTimeout(ctx, time.Second) - defer cancel() ret, _, err := d.dut.RecvWithErrno(ctx, t, d.remoteFD, 100, 0) if ret != -1 { t.Fatalf("recv after ICMP error succeeded unexpectedly, expected (%[1]d) %[1]v", d.wantErrno) @@ -167,8 +165,6 @@ func testSendTo(ctx context.Context, t *testing.T, d testData) { } if d.wantErrno != unix.Errno(0) { - ctx, cancel := context.WithTimeout(ctx, time.Second) - defer cancel() ret, err := d.dut.SendToWithErrno(ctx, t, d.remoteFD, nil, 0, d.conn.LocalAddr(t)) if ret != -1 { @@ -315,10 +311,7 @@ func TestICMPErrorDuringUDPRecv(t *testing.T) { defer wg.Done() if wantErrno != unix.Errno(0) { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - - ret, _, err := dut.RecvWithErrno(ctx, t, remoteFD, 100, 0) + ret, _, err := dut.RecvWithErrno(context.Background(), t, remoteFD, 100, 0) if ret != -1 { t.Errorf("recv during ICMP error succeeded unexpectedly, expected (%[1]d) %[1]v", wantErrno) return @@ -329,10 +322,7 @@ func TestICMPErrorDuringUDPRecv(t *testing.T) { } } - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - - if ret, _, err := dut.RecvWithErrno(ctx, t, remoteFD, 100, 0); ret == -1 { + if ret, _, err := dut.RecvWithErrno(context.Background(), t, remoteFD, 100, 0); ret == -1 { t.Errorf("recv after ICMP error failed with (%[1]d) %[1]", err) } }() @@ -340,10 +330,7 @@ func TestICMPErrorDuringUDPRecv(t *testing.T) { go func() { defer wg.Done() - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - - if ret, _, err := dut.RecvWithErrno(ctx, t, cleanFD, 100, 0); ret == -1 { + if ret, _, err := dut.RecvWithErrno(context.Background(), t, cleanFD, 100, 0); ret == -1 { t.Errorf("recv on clean socket failed with (%[1]d) %[1]", err) } }() -- cgit v1.2.3