diff options
30 files changed, 331 insertions, 114 deletions
diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go index 4a32821bd..7f2181216 100644 --- a/pkg/sentry/fsimpl/gofer/filesystem.go +++ b/pkg/sentry/fsimpl/gofer/filesystem.go @@ -21,6 +21,8 @@ import ( "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/fspath" "gvisor.dev/gvisor/pkg/p9" + "gvisor.dev/gvisor/pkg/sentry/fsimpl/host" + "gvisor.dev/gvisor/pkg/sentry/kernel" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/sentry/kernel/pipe" "gvisor.dev/gvisor/pkg/sentry/socket/unix/transport" @@ -835,6 +837,9 @@ func (d *dentry) openLocked(ctx context.Context, rp *vfs.ResolvingPath, opts *vf if d.isSynthetic() { return nil, syserror.ENXIO } + if d.fs.iopts.OpenSocketsByConnecting { + return d.connectSocketLocked(ctx, opts) + } case linux.S_IFIFO: if d.isSynthetic() { return d.pipe.Open(ctx, mnt, &d.vfsd, opts.Flags) @@ -843,10 +848,28 @@ func (d *dentry) openLocked(ctx context.Context, rp *vfs.ResolvingPath, opts *vf return d.openSpecialFileLocked(ctx, mnt, opts) } +func (d *dentry) connectSocketLocked(ctx context.Context, opts *vfs.OpenOptions) (*vfs.FileDescription, error) { + if opts.Flags&linux.O_DIRECT != 0 { + return nil, syserror.EINVAL + } + fdObj, err := d.file.connect(ctx, p9.AnonymousSocket) + if err != nil { + return nil, err + } + fd, err := host.NewFD(ctx, kernel.KernelFromContext(ctx).HostMount(), fdObj.FD(), &host.NewFDOptions{ + HaveFlags: true, + Flags: opts.Flags, + }) + if err != nil { + fdObj.Close() + return nil, err + } + fdObj.Release() + return fd, nil +} + func (d *dentry) openSpecialFileLocked(ctx context.Context, mnt *vfs.Mount, opts *vfs.OpenOptions) (*vfs.FileDescription, error) { ats := vfs.AccessTypesForOpenFlags(opts) - // Treat as a special file. This is done for non-synthetic pipes as well as - // regular files when d.fs.opts.regularFilesUseSpecialFileFD is true. if opts.Flags&linux.O_DIRECT != 0 { return nil, syserror.EINVAL } @@ -854,10 +877,15 @@ func (d *dentry) openSpecialFileLocked(ctx context.Context, mnt *vfs.Mount, opts if err != nil { return nil, err } + seekable := d.fileType() == linux.S_IFREG fd := &specialFileFD{ - handle: h, + handle: h, + seekable: seekable, } - if err := fd.vfsfd.Init(fd, opts.Flags, mnt, &d.vfsd, &vfs.FileDescriptionOptions{}); err != nil { + if err := fd.vfsfd.Init(fd, opts.Flags, mnt, &d.vfsd, &vfs.FileDescriptionOptions{ + DenyPRead: !seekable, + DenyPWrite: !seekable, + }); err != nil { h.close(ctx) return nil, err } @@ -888,7 +916,11 @@ func (d *dentry) createAndOpenChildLocked(ctx context.Context, rp *vfs.Resolving } creds := rp.Credentials() name := rp.Component() - fdobj, openFile, createQID, _, err := dirfile.create(ctx, name, (p9.OpenFlags)(opts.Flags), (p9.FileMode)(opts.Mode), (p9.UID)(creds.EffectiveKUID), (p9.GID)(creds.EffectiveKGID)) + // Filter file creation flags and O_LARGEFILE out; the create RPC already + // has the semantics of O_CREAT|O_EXCL, while some servers will choke on + // O_LARGEFILE. + createFlags := p9.OpenFlags(opts.Flags &^ (linux.O_CREAT | linux.O_EXCL | linux.O_NOCTTY | linux.O_TRUNC | linux.O_LARGEFILE)) + fdobj, openFile, createQID, _, err := dirfile.create(ctx, name, createFlags, (p9.FileMode)(opts.Mode), (p9.UID)(creds.EffectiveKUID), (p9.GID)(creds.EffectiveKGID)) if err != nil { dirfile.close(ctx) return nil, err @@ -896,7 +928,7 @@ func (d *dentry) createAndOpenChildLocked(ctx context.Context, rp *vfs.Resolving // Then we need to walk to the file we just created to get a non-open fid // representing it, and to get its metadata. This must use d.file since, as // explained above, dirfile was invalidated by dirfile.Create(). - walkQID, nonOpenFile, attrMask, attr, err := d.file.walkGetAttrOne(ctx, name) + _, nonOpenFile, attrMask, attr, err := d.file.walkGetAttrOne(ctx, name) if err != nil { openFile.close(ctx) if fdobj != nil { @@ -904,17 +936,6 @@ func (d *dentry) createAndOpenChildLocked(ctx context.Context, rp *vfs.Resolving } return nil, err } - // Sanity-check that we walked to the file we created. - if createQID.Path != walkQID.Path { - // Probably due to concurrent remote filesystem mutation? - ctx.Warningf("gofer.dentry.createAndOpenChildLocked: created file has QID %v before walk, QID %v after (interop=%v)", createQID, walkQID, d.fs.opts.interop) - nonOpenFile.close(ctx) - openFile.close(ctx) - if fdobj != nil { - fdobj.Close() - } - return nil, syserror.EAGAIN - } // Construct the new dentry. child, err := d.fs.newDentry(ctx, nonOpenFile, createQID, attrMask, &attr) @@ -960,16 +981,21 @@ func (d *dentry) createAndOpenChildLocked(ctx context.Context, rp *vfs.Resolving } childVFSFD = &fd.vfsfd } else { + seekable := child.fileType() == linux.S_IFREG fd := &specialFileFD{ handle: handle{ file: openFile, fd: -1, }, + seekable: seekable, } if fdobj != nil { fd.handle.fd = int32(fdobj.Release()) } - if err := fd.vfsfd.Init(fd, opts.Flags, mnt, &child.vfsd, &vfs.FileDescriptionOptions{}); err != nil { + if err := fd.vfsfd.Init(fd, opts.Flags, mnt, &child.vfsd, &vfs.FileDescriptionOptions{ + DenyPRead: !seekable, + DenyPWrite: !seekable, + }); err != nil { fd.handle.close(ctx) return nil, err } diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index e68e37ebc..1da8d5d82 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/pkg/sentry/fsimpl/gofer/gofer.go @@ -221,6 +221,10 @@ type InternalFilesystemOptions struct { // which servers can handle only a single client and report failure if that // client disconnects. LeakConnection bool + + // If OpenSocketsByConnecting is true, silently translate attempts to open + // files identifying as sockets to connect RPCs. + OpenSocketsByConnecting bool } // Name implements vfs.FilesystemType.Name. diff --git a/pkg/sentry/fsimpl/gofer/special_file.go b/pkg/sentry/fsimpl/gofer/special_file.go index 507e0e276..a464e6a94 100644 --- a/pkg/sentry/fsimpl/gofer/special_file.go +++ b/pkg/sentry/fsimpl/gofer/special_file.go @@ -33,13 +33,14 @@ import ( type specialFileFD struct { fileDescription - // handle is immutable. + // handle is used for file I/O. handle is immutable. handle handle - // off is the file offset. off is protected by mu. (POSIX 2.9.7 only - // requires operations using the file offset to be atomic for regular files - // and symlinks; however, since specialFileFD may be used for regular - // files, we apply this atomicity unconditionally.) + // seekable is true if this file description represents a file for which + // file offset is significant, i.e. a regular file. seekable is immutable. + seekable bool + + // If seekable is true, off is the file offset. off is protected by mu. mu sync.Mutex off int64 } @@ -63,7 +64,7 @@ func (fd *specialFileFD) OnClose(ctx context.Context) error { // PRead implements vfs.FileDescriptionImpl.PRead. func (fd *specialFileFD) PRead(ctx context.Context, dst usermem.IOSequence, offset int64, opts vfs.ReadOptions) (int64, error) { - if offset < 0 { + if fd.seekable && offset < 0 { return 0, syserror.EINVAL } if opts.Flags != 0 { @@ -91,6 +92,10 @@ func (fd *specialFileFD) PRead(ctx context.Context, dst usermem.IOSequence, offs // Read implements vfs.FileDescriptionImpl.Read. func (fd *specialFileFD) Read(ctx context.Context, dst usermem.IOSequence, opts vfs.ReadOptions) (int64, error) { + if !fd.seekable { + return fd.PRead(ctx, dst, -1, opts) + } + fd.mu.Lock() n, err := fd.PRead(ctx, dst, fd.off, opts) fd.off += n @@ -100,14 +105,14 @@ func (fd *specialFileFD) Read(ctx context.Context, dst usermem.IOSequence, opts // PWrite implements vfs.FileDescriptionImpl.PWrite. func (fd *specialFileFD) PWrite(ctx context.Context, src usermem.IOSequence, offset int64, opts vfs.WriteOptions) (int64, error) { - if offset < 0 { + if fd.seekable && offset < 0 { return 0, syserror.EINVAL } if opts.Flags != 0 { return 0, syserror.EOPNOTSUPP } - if fd.dentry().fileType() == linux.S_IFREG { + if fd.seekable { limit, err := vfs.CheckLimit(ctx, offset, src.NumBytes()) if err != nil { return 0, err @@ -130,6 +135,10 @@ func (fd *specialFileFD) PWrite(ctx context.Context, src usermem.IOSequence, off // Write implements vfs.FileDescriptionImpl.Write. func (fd *specialFileFD) Write(ctx context.Context, src usermem.IOSequence, opts vfs.WriteOptions) (int64, error) { + if !fd.seekable { + return fd.PWrite(ctx, src, -1, opts) + } + fd.mu.Lock() n, err := fd.PWrite(ctx, src, fd.off, opts) fd.off += n @@ -139,6 +148,9 @@ func (fd *specialFileFD) Write(ctx context.Context, src usermem.IOSequence, opts // Seek implements vfs.FileDescriptionImpl.Seek. func (fd *specialFileFD) Seek(ctx context.Context, offset int64, whence int32) (int64, error) { + if !fd.seekable { + return 0, syserror.ESPIPE + } fd.mu.Lock() defer fd.mu.Unlock() switch whence { diff --git a/pkg/sentry/fsimpl/host/host.go b/pkg/sentry/fsimpl/host/host.go index 36bceeaa4..8caf55a1b 100644 --- a/pkg/sentry/fsimpl/host/host.go +++ b/pkg/sentry/fsimpl/host/host.go @@ -40,8 +40,20 @@ import ( "gvisor.dev/gvisor/pkg/waiter" ) -// ImportFD sets up and returns a vfs.FileDescription from a donated fd. -func ImportFD(ctx context.Context, mnt *vfs.Mount, hostFD int, isTTY bool) (*vfs.FileDescription, error) { +// NewFDOptions contains options to NewFD. +type NewFDOptions struct { + // If IsTTY is true, the file descriptor is a TTY. + IsTTY bool + + // If HaveFlags is true, use Flags for the new file description. Otherwise, + // the new file description will inherit flags from hostFD. + HaveFlags bool + Flags uint32 +} + +// NewFD returns a vfs.FileDescription representing the given host file +// descriptor. mnt must be Kernel.HostMount(). +func NewFD(ctx context.Context, mnt *vfs.Mount, hostFD int, opts *NewFDOptions) (*vfs.FileDescription, error) { fs, ok := mnt.Filesystem().Impl().(*filesystem) if !ok { return nil, fmt.Errorf("can't import host FDs into filesystems of type %T", mnt.Filesystem().Impl()) @@ -53,10 +65,14 @@ func ImportFD(ctx context.Context, mnt *vfs.Mount, hostFD int, isTTY bool) (*vfs return nil, err } - // Get flags for the imported FD. - flags, err := unix.FcntlInt(uintptr(hostFD), syscall.F_GETFL, 0) - if err != nil { - return nil, err + flags := opts.Flags + if !opts.HaveFlags { + // Get flags for the imported FD. + flagsInt, err := unix.FcntlInt(uintptr(hostFD), syscall.F_GETFL, 0) + if err != nil { + return nil, err + } + flags = uint32(flagsInt) } fileMode := linux.FileMode(s.Mode) @@ -65,13 +81,13 @@ func ImportFD(ctx context.Context, mnt *vfs.Mount, hostFD int, isTTY bool) (*vfs // Determine if hostFD is seekable. If not, this syscall will return ESPIPE // (see fs/read_write.c:llseek), e.g. for pipes, sockets, and some character // devices. - _, err = unix.Seek(hostFD, 0, linux.SEEK_CUR) + _, err := unix.Seek(hostFD, 0, linux.SEEK_CUR) seekable := err != syserror.ESPIPE i := &inode{ hostFD: hostFD, seekable: seekable, - isTTY: isTTY, + isTTY: opts.IsTTY, canMap: canMap(uint32(fileType)), wouldBlock: wouldBlock(uint32(fileType)), ino: fs.NextIno(), @@ -101,7 +117,14 @@ func ImportFD(ctx context.Context, mnt *vfs.Mount, hostFD int, isTTY bool) (*vfs // i.open will take a reference on d. defer d.DecRef() - return i.open(ctx, d.VFSDentry(), mnt, uint32(flags)) + return i.open(ctx, d.VFSDentry(), mnt, flags) +} + +// ImportFD sets up and returns a vfs.FileDescription from a donated fd. +func ImportFD(ctx context.Context, mnt *vfs.Mount, hostFD int, isTTY bool) (*vfs.FileDescription, error) { + return NewFD(ctx, mnt, hostFD, &NewFDOptions{ + IsTTY: isTTY, + }) } // filesystemType implements vfs.FilesystemType. diff --git a/pkg/tcpip/buffer/view.go b/pkg/tcpip/buffer/view.go index f01217c91..9a3c5d6c3 100644 --- a/pkg/tcpip/buffer/view.go +++ b/pkg/tcpip/buffer/view.go @@ -59,6 +59,9 @@ func (v *View) Reader() bytes.Reader { // ToVectorisedView returns a VectorisedView containing the receiver. func (v View) ToVectorisedView() VectorisedView { + if len(v) == 0 { + return VectorisedView{} + } return NewVectorisedView(len(v), []View{v}) } @@ -229,6 +232,9 @@ func (vv *VectorisedView) Append(vv2 VectorisedView) { // AppendView appends the given view into this vectorised view. func (vv *VectorisedView) AppendView(v View) { + if len(v) == 0 { + return + } vv.views = append(vv.views, v) vv.size += len(v) } diff --git a/pkg/tcpip/buffer/view_test.go b/pkg/tcpip/buffer/view_test.go index c56795c7b..726e54de9 100644 --- a/pkg/tcpip/buffer/view_test.go +++ b/pkg/tcpip/buffer/view_test.go @@ -483,3 +483,39 @@ func TestPullUp(t *testing.T) { } } } + +func TestToVectorisedView(t *testing.T) { + testCases := []struct { + in View + want VectorisedView + }{ + {nil, VectorisedView{}}, + {View{}, VectorisedView{}}, + {View{'a'}, VectorisedView{size: 1, views: []View{{'a'}}}}, + } + for _, tc := range testCases { + if got, want := tc.in.ToVectorisedView(), tc.want; !reflect.DeepEqual(got, want) { + t.Errorf("(%v).ToVectorisedView failed got: %+v, want: %+v", tc.in, got, want) + } + } +} + +func TestAppendView(t *testing.T) { + testCases := []struct { + vv VectorisedView + in View + want VectorisedView + }{ + {VectorisedView{}, nil, VectorisedView{}}, + {VectorisedView{}, View{}, VectorisedView{}}, + {VectorisedView{[]View{{'a', 'b', 'c', 'd'}}, 4}, nil, VectorisedView{[]View{{'a', 'b', 'c', 'd'}}, 4}}, + {VectorisedView{[]View{{'a', 'b', 'c', 'd'}}, 4}, View{}, VectorisedView{[]View{{'a', 'b', 'c', 'd'}}, 4}}, + {VectorisedView{[]View{{'a', 'b', 'c', 'd'}}, 4}, View{'e'}, VectorisedView{[]View{{'a', 'b', 'c', 'd'}, {'e'}}, 5}}, + } + for _, tc := range testCases { + tc.vv.AppendView(tc.in) + if got, want := tc.vv, tc.want; !reflect.DeepEqual(got, want) { + t.Errorf("(%v).ToVectorisedView failed got: %+v, want: %+v", tc.in, got, want) + } + } +} diff --git a/pkg/tcpip/link/qdisc/fifo/endpoint.go b/pkg/tcpip/link/qdisc/fifo/endpoint.go index be9fec3b3..54432194d 100644 --- a/pkg/tcpip/link/qdisc/fifo/endpoint.go +++ b/pkg/tcpip/link/qdisc/fifo/endpoint.go @@ -163,7 +163,7 @@ func (e *endpoint) WritePacket(r *stack.Route, gso *stack.GSO, protocol tcpip.Ne // WritePackets implements stack.LinkEndpoint.WritePackets. // -// Being a batch API each packet in pkts should have the following fields +// Being a batch API, each packet in pkts should have the following fields // populated: // - pkt.EgressRoute // - pkt.GSOOptions diff --git a/runsc/main.go b/runsc/main.go index 0216e9481..0625a06e0 100644 --- a/runsc/main.go +++ b/runsc/main.go @@ -72,7 +72,7 @@ var ( network = flag.String("network", "sandbox", "specifies which network to use: sandbox (default), host, none. Using network inside the sandbox is more secure because it's isolated from the host network.") hardwareGSO = flag.Bool("gso", true, "enable hardware segmentation offload if it is supported by a network device.") softwareGSO = flag.Bool("software-gso", true, "enable software segmentation offload when hardware ofload can't be enabled.") - qDisc = flag.String("qdisc", "none", "specifies which queueing discipline to apply by default to the non loopback nics used by the sandbox.") + qDisc = flag.String("qdisc", "fifo", "specifies which queueing discipline to apply by default to the non loopback nics used by the sandbox.") fileAccess = flag.String("file-access", "exclusive", "specifies which filesystem to use for the root mount: exclusive (default), shared. Volume mounts are always shared.") fsGoferHostUDS = flag.Bool("fsgofer-host-uds", false, "allow the gofer to mount Unix Domain Sockets.") overlay = flag.Bool("overlay", false, "wrap filesystem mounts with writable overlay. All modifications are stored in memory inside the sandbox.") diff --git a/test/packetimpact/testbench/BUILD b/test/packetimpact/testbench/BUILD index 3ceceb9d7..fed51006f 100644 --- a/test/packetimpact/testbench/BUILD +++ b/test/packetimpact/testbench/BUILD @@ -13,6 +13,7 @@ go_library( "dut_client.go", "layers.go", "rawsockets.go", + "testbench.go", ], deps = [ "//pkg/tcpip", diff --git a/test/packetimpact/testbench/connections.go b/test/packetimpact/testbench/connections.go index 28c841612..463fd0556 100644 --- a/test/packetimpact/testbench/connections.go +++ b/test/packetimpact/testbench/connections.go @@ -17,7 +17,6 @@ package testbench import ( - "flag" "fmt" "math/rand" "net" @@ -32,13 +31,6 @@ import ( "gvisor.dev/gvisor/pkg/tcpip/seqnum" ) -var localIPv4 = flag.String("local_ipv4", "", "local IPv4 address for test packets") -var remoteIPv4 = flag.String("remote_ipv4", "", "remote IPv4 address for test packets") -var localIPv6 = flag.String("local_ipv6", "", "local IPv6 address for test packets") -var remoteIPv6 = flag.String("remote_ipv6", "", "remote IPv6 address for test packets") -var localMAC = flag.String("local_mac", "", "local mac address for test packets") -var remoteMAC = flag.String("remote_mac", "", "remote mac address for test packets") - func portFromSockaddr(sa unix.Sockaddr) (uint16, error) { switch sa := sa.(type) { case *unix.SockaddrInet4: @@ -64,11 +56,11 @@ func pickPort(domain, typ int) (fd int, sa unix.Sockaddr, err error) { switch domain { case unix.AF_INET: var sa4 unix.SockaddrInet4 - copy(sa4.Addr[:], net.ParseIP(*localIPv4).To4()) + copy(sa4.Addr[:], net.ParseIP(LocalIPv4).To4()) sa = &sa4 case unix.AF_INET6: var sa6 unix.SockaddrInet6 - copy(sa6.Addr[:], net.ParseIP(*localIPv6).To16()) + copy(sa6.Addr[:], net.ParseIP(LocalIPv6).To16()) sa = &sa6 default: return -1, nil, fmt.Errorf("invalid domain %d, it should be one of unix.AF_INET or unix.AF_INET6", domain) @@ -120,12 +112,12 @@ var _ layerState = (*etherState)(nil) // newEtherState creates a new etherState. func newEtherState(out, in Ether) (*etherState, error) { - lMAC, err := tcpip.ParseMACAddress(*localMAC) + lMAC, err := tcpip.ParseMACAddress(LocalMAC) if err != nil { return nil, err } - rMAC, err := tcpip.ParseMACAddress(*remoteMAC) + rMAC, err := tcpip.ParseMACAddress(RemoteMAC) if err != nil { return nil, err } @@ -172,8 +164,8 @@ var _ layerState = (*ipv4State)(nil) // newIPv4State creates a new ipv4State. func newIPv4State(out, in IPv4) (*ipv4State, error) { - lIP := tcpip.Address(net.ParseIP(*localIPv4).To4()) - rIP := tcpip.Address(net.ParseIP(*remoteIPv4).To4()) + lIP := tcpip.Address(net.ParseIP(LocalIPv4).To4()) + rIP := tcpip.Address(net.ParseIP(RemoteIPv4).To4()) s := ipv4State{ out: IPv4{SrcAddr: &lIP, DstAddr: &rIP}, in: IPv4{SrcAddr: &rIP, DstAddr: &lIP}, @@ -217,8 +209,8 @@ var _ layerState = (*ipv6State)(nil) // newIPv6State creates a new ipv6State. func newIPv6State(out, in IPv6) (*ipv6State, error) { - lIP := tcpip.Address(net.ParseIP(*localIPv6).To16()) - rIP := tcpip.Address(net.ParseIP(*remoteIPv6).To16()) + lIP := tcpip.Address(net.ParseIP(LocalIPv6).To16()) + rIP := tcpip.Address(net.ParseIP(RemoteIPv6).To16()) s := ipv6State{ out: IPv6{SrcAddr: &lIP, DstAddr: &rIP}, in: IPv6{SrcAddr: &rIP, DstAddr: &lIP}, diff --git a/test/packetimpact/testbench/dut.go b/test/packetimpact/testbench/dut.go index f68d9d62b..a78b7d7ee 100644 --- a/test/packetimpact/testbench/dut.go +++ b/test/packetimpact/testbench/dut.go @@ -16,12 +16,10 @@ package testbench import ( "context" - "flag" "net" "strconv" "syscall" "testing" - "time" pb "gvisor.dev/gvisor/test/packetimpact/proto/posix_server_go_proto" @@ -30,29 +28,21 @@ import ( "google.golang.org/grpc/keepalive" ) -var ( - posixServerIP = flag.String("posix_server_ip", "", "ip address to listen to for UDP commands") - posixServerPort = flag.Int("posix_server_port", 40000, "port to listen to for UDP commands") - rpcTimeout = flag.Duration("rpc_timeout", 100*time.Millisecond, "gRPC timeout") - rpcKeepalive = flag.Duration("rpc_keepalive", 10*time.Second, "gRPC keepalive") -) - // DUT communicates with the DUT to force it to make POSIX calls. type DUT struct { t *testing.T conn *grpc.ClientConn - posixServer PosixClient + posixServer POSIXClient } // NewDUT creates a new connection with the DUT over gRPC. func NewDUT(t *testing.T) DUT { - flag.Parse() - posixServerAddress := *posixServerIP + ":" + strconv.Itoa(*posixServerPort) - conn, err := grpc.Dial(posixServerAddress, grpc.WithInsecure(), grpc.WithKeepaliveParams(keepalive.ClientParameters{Timeout: *rpcKeepalive})) + posixServerAddress := POSIXServerIP + ":" + strconv.Itoa(POSIXServerPort) + conn, err := grpc.Dial(posixServerAddress, grpc.WithInsecure(), grpc.WithKeepaliveParams(keepalive.ClientParameters{Timeout: RPCKeepalive})) if err != nil { t.Fatalf("failed to grpc.Dial(%s): %s", posixServerAddress, err) } - posixServer := NewPosixClient(conn) + posixServer := NewPOSIXClient(conn) return DUT{ t: t, conn: conn, @@ -149,12 +139,12 @@ func (dut *DUT) CreateBoundSocket(typ, proto int32, addr net.IP) (int32, uint16) // CreateListener makes a new TCP connection. If it fails, the test ends. func (dut *DUT) CreateListener(typ, proto, backlog int32) (int32, uint16) { - fd, remotePort := dut.CreateBoundSocket(typ, proto, net.ParseIP(*remoteIPv4)) + fd, remotePort := dut.CreateBoundSocket(typ, proto, net.ParseIP(RemoteIPv4)) dut.Listen(fd, backlog) return fd, remotePort } -// All the functions that make gRPC calls to the Posix service are below, sorted +// All the functions that make gRPC calls to the POSIX service are below, sorted // alphabetically. // Accept calls accept on the DUT and causes a fatal test failure if it doesn't @@ -162,7 +152,7 @@ func (dut *DUT) CreateListener(typ, proto, backlog int32) (int32, uint16) { // AcceptWithErrno. func (dut *DUT) Accept(sockfd int32) (int32, unix.Sockaddr) { dut.t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), *rpcTimeout) + ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) defer cancel() fd, sa, err := dut.AcceptWithErrno(ctx, sockfd) if fd < 0 { @@ -189,7 +179,7 @@ func (dut *DUT) AcceptWithErrno(ctx context.Context, sockfd int32) (int32, unix. // needed, use BindWithErrno. func (dut *DUT) Bind(fd int32, sa unix.Sockaddr) { dut.t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), *rpcTimeout) + ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) defer cancel() ret, err := dut.BindWithErrno(ctx, fd, sa) if ret != 0 { @@ -216,7 +206,7 @@ func (dut *DUT) BindWithErrno(ctx context.Context, fd int32, sa unix.Sockaddr) ( // CloseWithErrno. func (dut *DUT) Close(fd int32) { dut.t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), *rpcTimeout) + ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) defer cancel() ret, err := dut.CloseWithErrno(ctx, fd) if ret != 0 { @@ -242,7 +232,7 @@ func (dut *DUT) CloseWithErrno(ctx context.Context, fd int32) (int32, error) { // needed, use ConnectWithErrno. func (dut *DUT) Connect(fd int32, sa unix.Sockaddr) { dut.t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), *rpcTimeout) + ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) defer cancel() ret, err := dut.ConnectWithErrno(ctx, fd, sa) if ret != 0 { @@ -269,7 +259,7 @@ func (dut *DUT) ConnectWithErrno(ctx context.Context, fd int32, sa unix.Sockaddr // needed, use GetSockNameWithErrno. func (dut *DUT) GetSockName(sockfd int32) unix.Sockaddr { dut.t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), *rpcTimeout) + ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) defer cancel() ret, sa, err := dut.GetSockNameWithErrno(ctx, sockfd) if ret != 0 { @@ -318,7 +308,7 @@ func (dut *DUT) getSockOpt(ctx context.Context, sockfd, level, optname, optlen i // more specific GetSockOptXxx function. func (dut *DUT) GetSockOpt(sockfd, level, optname, optlen int32) []byte { dut.t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), *rpcTimeout) + ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) defer cancel() ret, optval, err := dut.GetSockOptWithErrno(ctx, sockfd, level, optname, optlen) if ret != 0 { @@ -345,7 +335,7 @@ func (dut *DUT) GetSockOptWithErrno(ctx context.Context, sockfd, level, optname, // is needed, use GetSockOptIntWithErrno. func (dut *DUT) GetSockOptInt(sockfd, level, optname int32) int32 { dut.t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), *rpcTimeout) + ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) defer cancel() ret, intval, err := dut.GetSockOptIntWithErrno(ctx, sockfd, level, optname) if ret != 0 { @@ -370,7 +360,7 @@ func (dut *DUT) GetSockOptIntWithErrno(ctx context.Context, sockfd, level, optna // needed, use GetSockOptTimevalWithErrno. func (dut *DUT) GetSockOptTimeval(sockfd, level, optname int32) unix.Timeval { dut.t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), *rpcTimeout) + ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) defer cancel() ret, timeval, err := dut.GetSockOptTimevalWithErrno(ctx, sockfd, level, optname) if ret != 0 { @@ -399,7 +389,7 @@ func (dut *DUT) GetSockOptTimevalWithErrno(ctx context.Context, sockfd, level, o // ListenWithErrno. func (dut *DUT) Listen(sockfd, backlog int32) { dut.t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), *rpcTimeout) + ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) defer cancel() ret, err := dut.ListenWithErrno(ctx, sockfd, backlog) if ret != 0 { @@ -426,7 +416,7 @@ func (dut *DUT) ListenWithErrno(ctx context.Context, sockfd, backlog int32) (int // SendWithErrno. func (dut *DUT) Send(sockfd int32, buf []byte, flags int32) int32 { dut.t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), *rpcTimeout) + ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) defer cancel() ret, err := dut.SendWithErrno(ctx, sockfd, buf, flags) if ret == -1 { @@ -455,7 +445,7 @@ func (dut *DUT) SendWithErrno(ctx context.Context, sockfd int32, buf []byte, fla // SendToWithErrno. func (dut *DUT) SendTo(sockfd int32, buf []byte, flags int32, destAddr unix.Sockaddr) int32 { dut.t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), *rpcTimeout) + ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) defer cancel() ret, err := dut.SendToWithErrno(ctx, sockfd, buf, flags, destAddr) if ret == -1 { @@ -502,7 +492,7 @@ func (dut *DUT) setSockOpt(ctx context.Context, sockfd, level, optname int32, op // more specific SetSockOptXxx function. func (dut *DUT) SetSockOpt(sockfd, level, optname int32, optval []byte) { dut.t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), *rpcTimeout) + ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) defer cancel() ret, err := dut.SetSockOptWithErrno(ctx, sockfd, level, optname, optval) if ret != 0 { @@ -523,7 +513,7 @@ func (dut *DUT) SetSockOptWithErrno(ctx context.Context, sockfd, level, optname // is needed, use SetSockOptIntWithErrno. func (dut *DUT) SetSockOptInt(sockfd, level, optname, optval int32) { dut.t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), *rpcTimeout) + ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) defer cancel() ret, err := dut.SetSockOptIntWithErrno(ctx, sockfd, level, optname, optval) if ret != 0 { @@ -542,7 +532,7 @@ func (dut *DUT) SetSockOptIntWithErrno(ctx context.Context, sockfd, level, optna // needed, use SetSockOptTimevalWithErrno. func (dut *DUT) SetSockOptTimeval(sockfd, level, optname int32, tv *unix.Timeval) { dut.t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), *rpcTimeout) + ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) defer cancel() ret, err := dut.SetSockOptTimevalWithErrno(ctx, sockfd, level, optname, tv) if ret != 0 { @@ -593,7 +583,7 @@ func (dut *DUT) SocketWithErrno(domain, typ, proto int32) (int32, error) { // RecvWithErrno. func (dut *DUT) Recv(sockfd, len, flags int32) []byte { dut.t.Helper() - ctx, cancel := context.WithTimeout(context.Background(), *rpcTimeout) + ctx, cancel := context.WithTimeout(context.Background(), RPCTimeout) defer cancel() ret, buf, err := dut.RecvWithErrno(ctx, sockfd, len, flags) if ret == -1 { diff --git a/test/packetimpact/testbench/dut_client.go b/test/packetimpact/testbench/dut_client.go index b130a33a2..d0e68c5da 100644 --- a/test/packetimpact/testbench/dut_client.go +++ b/test/packetimpact/testbench/dut_client.go @@ -20,9 +20,9 @@ import ( ) // PosixClient is a gRPC client for the Posix service. -type PosixClient pb.PosixClient +type POSIXClient pb.PosixClient -// NewPosixClient makes a new gRPC client for the Posix service. -func NewPosixClient(c grpc.ClientConnInterface) PosixClient { +// NewPOSIXClient makes a new gRPC client for the POSIX service. +func NewPOSIXClient(c grpc.ClientConnInterface) POSIXClient { return pb.NewPosixClient(c) } diff --git a/test/packetimpact/testbench/rawsockets.go b/test/packetimpact/testbench/rawsockets.go index a9ad72b63..4665f60b2 100644 --- a/test/packetimpact/testbench/rawsockets.go +++ b/test/packetimpact/testbench/rawsockets.go @@ -27,8 +27,6 @@ import ( "gvisor.dev/gvisor/pkg/usermem" ) -var device = flag.String("device", "", "local device for test packets") - // Sniffer can sniff raw packets on the wire. type Sniffer struct { t *testing.T @@ -139,7 +137,7 @@ type Injector struct { // NewInjector creates a new injector on *device. func NewInjector(t *testing.T) (Injector, error) { flag.Parse() - ifInfo, err := net.InterfaceByName(*device) + ifInfo, err := net.InterfaceByName(Device) if err != nil { return Injector{}, err } diff --git a/test/packetimpact/testbench/testbench.go b/test/packetimpact/testbench/testbench.go new file mode 100644 index 000000000..a1242b189 --- /dev/null +++ b/test/packetimpact/testbench/testbench.go @@ -0,0 +1,63 @@ +// 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 testbench + +import ( + "flag" + "time" +) + +var ( + // Device is the local device on the test network. + Device = "" + // LocalIPv4 is the local IPv4 address on the test network. + LocalIPv4 = "" + // LocalIPv6 is the local IPv6 address on the test network. + LocalIPv6 = "" + // LocalMAC is the local MAC address on the test network. + LocalMAC = "" + // POSIXServerIP is the POSIX server's IP address on the control network. + POSIXServerIP = "" + // POSIXServerPort is the UDP port the POSIX server is bound to on the + // control network. + POSIXServerPort = 40000 + // RemoteIPv4 is the DUT's IPv4 address on the test network. + RemoteIPv4 = "" + // RemoteIPv6 is the DUT's IPv6 address on the test network. + RemoteIPv6 = "" + // RemoteMAC is the DUT's MAC address on the test network. + RemoteMAC = "" + // RPCKeepalive is the gRPC keepalive. + RPCKeepalive = 10 * time.Second + // RPCTimeout is the gRPC timeout. + RPCTimeout = 100 * time.Millisecond +) + +// RegisterFlags defines flags and associates them with the package-level +// exported variables above. It should be called by tests in their init +// functions. +func RegisterFlags(fs *flag.FlagSet) { + fs.StringVar(&POSIXServerIP, "posix_server_ip", POSIXServerIP, "ip address to listen to for UDP commands") + fs.IntVar(&POSIXServerPort, "posix_server_port", POSIXServerPort, "port to listen to for UDP commands") + fs.DurationVar(&RPCTimeout, "rpc_timeout", RPCTimeout, "gRPC timeout") + fs.DurationVar(&RPCKeepalive, "rpc_keepalive", RPCKeepalive, "gRPC keepalive") + fs.StringVar(&LocalIPv4, "local_ipv4", LocalIPv4, "local IPv4 address for test packets") + fs.StringVar(&RemoteIPv4, "remote_ipv4", RemoteIPv4, "remote IPv4 address for test packets") + fs.StringVar(&LocalIPv6, "local_ipv6", LocalIPv6, "local IPv6 address for test packets") + fs.StringVar(&RemoteIPv6, "remote_ipv6", RemoteIPv6, "remote IPv6 address for test packets") + fs.StringVar(&LocalMAC, "local_mac", LocalMAC, "local mac address for test packets") + fs.StringVar(&RemoteMAC, "remote_mac", RemoteMAC, "remote mac address for test packets") + fs.StringVar(&Device, "device", Device, "local device for test packets") +} diff --git a/test/packetimpact/tests/fin_wait2_timeout_test.go b/test/packetimpact/tests/fin_wait2_timeout_test.go index 99dc77f9a..c26ab78d9 100644 --- a/test/packetimpact/tests/fin_wait2_timeout_test.go +++ b/test/packetimpact/tests/fin_wait2_timeout_test.go @@ -15,6 +15,7 @@ package fin_wait2_timeout_test import ( + "flag" "testing" "time" @@ -23,6 +24,10 @@ import ( tb "gvisor.dev/gvisor/test/packetimpact/testbench" ) +func init() { + tb.RegisterFlags(flag.CommandLine) +} + func TestFinWait2Timeout(t *testing.T) { for _, tt := range []struct { description string diff --git a/test/packetimpact/tests/icmpv6_param_problem_test.go b/test/packetimpact/tests/icmpv6_param_problem_test.go index b48e55df4..bb1fc26fc 100644 --- a/test/packetimpact/tests/icmpv6_param_problem_test.go +++ b/test/packetimpact/tests/icmpv6_param_problem_test.go @@ -16,6 +16,7 @@ package icmpv6_param_problem_test import ( "encoding/binary" + "flag" "testing" "time" @@ -23,6 +24,10 @@ import ( tb "gvisor.dev/gvisor/test/packetimpact/testbench" ) +func init() { + tb.RegisterFlags(flag.CommandLine) +} + // TestICMPv6ParamProblemTest sends a packet with a bad next header. The DUT // should respond with an ICMPv6 Parameter Problem message. func TestICMPv6ParamProblemTest(t *testing.T) { diff --git a/test/packetimpact/tests/tcp_close_wait_ack_test.go b/test/packetimpact/tests/tcp_close_wait_ack_test.go index 153ce285b..70a22a2db 100644 --- a/test/packetimpact/tests/tcp_close_wait_ack_test.go +++ b/test/packetimpact/tests/tcp_close_wait_ack_test.go @@ -15,6 +15,7 @@ package tcp_close_wait_ack_test import ( + "flag" "fmt" "testing" "time" @@ -25,6 +26,10 @@ import ( tb "gvisor.dev/gvisor/test/packetimpact/testbench" ) +func init() { + tb.RegisterFlags(flag.CommandLine) +} + func TestCloseWaitAck(t *testing.T) { for _, tt := range []struct { description string diff --git a/test/packetimpact/tests/tcp_noaccept_close_rst_test.go b/test/packetimpact/tests/tcp_noaccept_close_rst_test.go index 7ebdd1950..2c1ec27d3 100644 --- a/test/packetimpact/tests/tcp_noaccept_close_rst_test.go +++ b/test/packetimpact/tests/tcp_noaccept_close_rst_test.go @@ -15,6 +15,7 @@ package tcp_noaccept_close_rst_test import ( + "flag" "testing" "time" @@ -23,6 +24,10 @@ import ( tb "gvisor.dev/gvisor/test/packetimpact/testbench" ) +func init() { + tb.RegisterFlags(flag.CommandLine) +} + func TestTcpNoAcceptCloseReset(t *testing.T) { dut := tb.NewDUT(t) defer dut.TearDown() diff --git a/test/packetimpact/tests/tcp_outside_the_window_test.go b/test/packetimpact/tests/tcp_outside_the_window_test.go index db3d3273b..351df193e 100644 --- a/test/packetimpact/tests/tcp_outside_the_window_test.go +++ b/test/packetimpact/tests/tcp_outside_the_window_test.go @@ -15,6 +15,7 @@ package tcp_outside_the_window_test import ( + "flag" "fmt" "testing" "time" @@ -25,6 +26,10 @@ import ( tb "gvisor.dev/gvisor/test/packetimpact/testbench" ) +func init() { + tb.RegisterFlags(flag.CommandLine) +} + // TestTCPOutsideTheWindows tests the behavior of the DUT when packets arrive // that are inside or outside the TCP window. Packets that are outside the // window should force an extra ACK, as described in RFC793 page 69: diff --git a/test/packetimpact/tests/tcp_should_piggyback_test.go b/test/packetimpact/tests/tcp_should_piggyback_test.go index b0be6ba23..0240dc2f9 100644 --- a/test/packetimpact/tests/tcp_should_piggyback_test.go +++ b/test/packetimpact/tests/tcp_should_piggyback_test.go @@ -15,6 +15,7 @@ package tcp_should_piggyback_test import ( + "flag" "testing" "time" @@ -23,6 +24,10 @@ import ( tb "gvisor.dev/gvisor/test/packetimpact/testbench" ) +func init() { + tb.RegisterFlags(flag.CommandLine) +} + func TestPiggyback(t *testing.T) { dut := tb.NewDUT(t) defer dut.TearDown() diff --git a/test/packetimpact/tests/tcp_user_timeout_test.go b/test/packetimpact/tests/tcp_user_timeout_test.go index 3cf82badb..ce31917ee 100644 --- a/test/packetimpact/tests/tcp_user_timeout_test.go +++ b/test/packetimpact/tests/tcp_user_timeout_test.go @@ -15,6 +15,7 @@ package tcp_user_timeout_test import ( + "flag" "fmt" "testing" "time" @@ -24,6 +25,10 @@ import ( tb "gvisor.dev/gvisor/test/packetimpact/testbench" ) +func init() { + tb.RegisterFlags(flag.CommandLine) +} + func sendPayload(conn *tb.TCPIPv4, dut *tb.DUT, fd int32) error { sampleData := make([]byte, 100) for i := range sampleData { diff --git a/test/packetimpact/tests/tcp_window_shrink_test.go b/test/packetimpact/tests/tcp_window_shrink_test.go index c9354074e..58ec1d740 100644 --- a/test/packetimpact/tests/tcp_window_shrink_test.go +++ b/test/packetimpact/tests/tcp_window_shrink_test.go @@ -15,6 +15,7 @@ package tcp_window_shrink_test import ( + "flag" "testing" "time" @@ -23,6 +24,10 @@ import ( tb "gvisor.dev/gvisor/test/packetimpact/testbench" ) +func init() { + tb.RegisterFlags(flag.CommandLine) +} + func TestWindowShrink(t *testing.T) { dut := tb.NewDUT(t) defer dut.TearDown() diff --git a/test/packetimpact/tests/tcp_zero_window_probe_retransmit_test.go b/test/packetimpact/tests/tcp_zero_window_probe_retransmit_test.go index 864e5a634..dd43a24db 100644 --- a/test/packetimpact/tests/tcp_zero_window_probe_retransmit_test.go +++ b/test/packetimpact/tests/tcp_zero_window_probe_retransmit_test.go @@ -15,6 +15,7 @@ package tcp_zero_window_probe_retransmit_test import ( + "flag" "testing" "time" @@ -23,6 +24,10 @@ import ( tb "gvisor.dev/gvisor/test/packetimpact/testbench" ) +func init() { + tb.RegisterFlags(flag.CommandLine) +} + // TestZeroWindowProbeRetransmit tests retransmits of zero window probes // to be sent at exponentially inreasing time intervals. func TestZeroWindowProbeRetransmit(t *testing.T) { diff --git a/test/packetimpact/tests/tcp_zero_window_probe_test.go b/test/packetimpact/tests/tcp_zero_window_probe_test.go index 4fa3d0cd4..6c453505d 100644 --- a/test/packetimpact/tests/tcp_zero_window_probe_test.go +++ b/test/packetimpact/tests/tcp_zero_window_probe_test.go @@ -15,6 +15,7 @@ package tcp_zero_window_probe_test import ( + "flag" "testing" "time" @@ -23,6 +24,10 @@ import ( tb "gvisor.dev/gvisor/test/packetimpact/testbench" ) +func init() { + tb.RegisterFlags(flag.CommandLine) +} + // TestZeroWindowProbe tests few cases of zero window probing over the // same connection. func TestZeroWindowProbe(t *testing.T) { diff --git a/test/packetimpact/tests/tcp_zero_window_probe_usertimeout_test.go b/test/packetimpact/tests/tcp_zero_window_probe_usertimeout_test.go index 7d81c276c..193427fb9 100644 --- a/test/packetimpact/tests/tcp_zero_window_probe_usertimeout_test.go +++ b/test/packetimpact/tests/tcp_zero_window_probe_usertimeout_test.go @@ -15,6 +15,7 @@ package tcp_zero_window_probe_usertimeout_test import ( + "flag" "testing" "time" @@ -23,6 +24,10 @@ import ( tb "gvisor.dev/gvisor/test/packetimpact/testbench" ) +func init() { + tb.RegisterFlags(flag.CommandLine) +} + // TestZeroWindowProbeUserTimeout sanity tests user timeout when we are // retransmitting zero window probes. func TestZeroWindowProbeUserTimeout(t *testing.T) { diff --git a/test/packetimpact/tests/udp_icmp_error_propagation_test.go b/test/packetimpact/tests/udp_icmp_error_propagation_test.go index 30dcb336e..ca4df2ab0 100644 --- a/test/packetimpact/tests/udp_icmp_error_propagation_test.go +++ b/test/packetimpact/tests/udp_icmp_error_propagation_test.go @@ -16,8 +16,10 @@ package udp_icmp_error_propagation_test import ( "context" + "flag" "fmt" "net" + "sync" "syscall" "testing" "time" @@ -27,6 +29,10 @@ import ( tb "gvisor.dev/gvisor/test/packetimpact/testbench" ) +func init() { + tb.RegisterFlags(flag.CommandLine) +} + type connectionMode bool func (c connectionMode) String() string { @@ -300,19 +306,22 @@ func TestICMPErrorDuringUDPRecv(t *testing.T) { t.Fatalf("did not receive message from DUT: %s", err) } - c := make(chan error) - go func(c chan error) { + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + if wantErrno != syscall.Errno(0) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() ret, _, err := dut.RecvWithErrno(ctx, remoteFD, 100, 0) if ret != -1 { - c <- fmt.Errorf("recv during ICMP error succeeded unexpectedly, expected (%[1]d) %[1]v", wantErrno) + t.Errorf("recv during ICMP error succeeded unexpectedly, expected (%[1]d) %[1]v", wantErrno) return } if err != wantErrno { - c <- fmt.Errorf("recv during ICMP error resulted in error (%[1]d) %[1]v, expected (%[2]d) %[2]v", err, wantErrno) + t.Errorf("recv during ICMP error resulted in error (%[1]d) %[1]v, expected (%[2]d) %[2]v", err, wantErrno) return } } @@ -321,23 +330,20 @@ func TestICMPErrorDuringUDPRecv(t *testing.T) { defer cancel() if ret, _, err := dut.RecvWithErrno(ctx, remoteFD, 100, 0); ret == -1 { - c <- fmt.Errorf("recv after ICMP error failed with (%[1]d) %[1]", err) - return + t.Errorf("recv after ICMP error failed with (%[1]d) %[1]", err) } - c <- nil - }(c) + }() + + go func() { + defer wg.Done() - cleanChan := make(chan error) - go func(c chan error) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() if ret, _, err := dut.RecvWithErrno(ctx, cleanFD, 100, 0); ret == -1 { - c <- fmt.Errorf("recv on clean socket failed with (%[1]d) %[1]", err) - return + t.Errorf("recv on clean socket failed with (%[1]d) %[1]", err) } - c <- nil - }(cleanChan) + }() // TODO(b/155684889) This sleep is to allow time for the DUT to // actually call recv since we want the ICMP error to arrive during the @@ -351,14 +357,7 @@ func TestICMPErrorDuringUDPRecv(t *testing.T) { conn.Send(tb.UDP{DstPort: &cleanPort}) conn.Send(tb.UDP{}) - - err, errClean := <-c, <-cleanChan - if errClean != nil { - t.Error(err) - } - if err != nil { - t.Fatal(err) - } + wg.Wait() }) } } diff --git a/test/packetimpact/tests/udp_recv_multicast_test.go b/test/packetimpact/tests/udp_recv_multicast_test.go index 61fd17050..0bae18ba3 100644 --- a/test/packetimpact/tests/udp_recv_multicast_test.go +++ b/test/packetimpact/tests/udp_recv_multicast_test.go @@ -15,6 +15,7 @@ package udp_recv_multicast_test import ( + "flag" "net" "testing" @@ -23,6 +24,10 @@ import ( tb "gvisor.dev/gvisor/test/packetimpact/testbench" ) +func init() { + tb.RegisterFlags(flag.CommandLine) +} + func TestUDPRecvMulticast(t *testing.T) { dut := tb.NewDUT(t) defer dut.TearDown() diff --git a/test/packetimpact/tests/udp_send_recv_dgram_test.go b/test/packetimpact/tests/udp_send_recv_dgram_test.go index 1c682831c..350875a6f 100644 --- a/test/packetimpact/tests/udp_send_recv_dgram_test.go +++ b/test/packetimpact/tests/udp_send_recv_dgram_test.go @@ -15,6 +15,7 @@ package udp_send_recv_dgram_test import ( + "flag" "math/rand" "net" "testing" @@ -24,6 +25,10 @@ import ( tb "gvisor.dev/gvisor/test/packetimpact/testbench" ) +func init() { + tb.RegisterFlags(flag.CommandLine) +} + func generateRandomPayload(t *testing.T, n int) string { t.Helper() buf := make([]byte, n) diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 837e56042..adf259bba 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -2620,6 +2620,7 @@ cc_binary( ":socket_bind_to_device_util", ":socket_test_util", "//test/util:capability_util", + "@com_google_absl//absl/container:node_hash_map", gtest, "//test/util:test_main", "//test/util:test_util", diff --git a/test/syscalls/linux/socket_bind_to_device_sequence.cc b/test/syscalls/linux/socket_bind_to_device_sequence.cc index 637d1151a..1967329ee 100644 --- a/test/syscalls/linux/socket_bind_to_device_sequence.cc +++ b/test/syscalls/linux/socket_bind_to_device_sequence.cc @@ -33,6 +33,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "absl/container/node_hash_map.h" #include "test/syscalls/linux/ip_socket_test_util.h" #include "test/syscalls/linux/socket_bind_to_device_util.h" #include "test/syscalls/linux/socket_test_util.h" @@ -192,8 +193,8 @@ class BindToDeviceSequenceTest : public ::testing::TestWithParam<SocketKind> { in_port_t port_ = 0; // sockets_to_close_ is a map from action index to the socket that was // created. - std::unordered_map<int, - std::unique_ptr<gvisor::testing::FileDescriptor>> + absl::node_hash_map<int, + std::unique_ptr<gvisor::testing::FileDescriptor>> sockets_to_close_; int next_socket_id_ = 0; }; |