ipn/ipnlocal: move last unconditional gvisor import, complete ts_omit_netstack support

Fixes #17283

Change-Id: Ia84d269683e4a68d7d10562561204934eeaf53bb
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

cmd/tailscaled/depaware-minbox.txt

@@ -53,21 +53,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         github.com/vishvananda/netns                                 from github.com/tailscale/netlink+
      💣 go4.org/mem                                                  from tailscale.com/control/controlbase+
         go4.org/netipx                                               from tailscale.com/ipn/ipnlocal+
-        gvisor.dev/gvisor/pkg/atomicbitops                           from gvisor.dev/gvisor/pkg/buffer+
-        gvisor.dev/gvisor/pkg/bits                                   from gvisor.dev/gvisor/pkg/buffer
-     💣 gvisor.dev/gvisor/pkg/buffer                                 from gvisor.dev/gvisor/pkg/tcpip
-        gvisor.dev/gvisor/pkg/context                                from gvisor.dev/gvisor/pkg/refs
-     💣 gvisor.dev/gvisor/pkg/gohacks                                from gvisor.dev/gvisor/pkg/state/wire+
-        gvisor.dev/gvisor/pkg/linewriter                             from gvisor.dev/gvisor/pkg/log
-        gvisor.dev/gvisor/pkg/log                                    from gvisor.dev/gvisor/pkg/context+
-        gvisor.dev/gvisor/pkg/rand                                   from gvisor.dev/gvisor/pkg/tcpip
-        gvisor.dev/gvisor/pkg/refs                                   from gvisor.dev/gvisor/pkg/buffer
-     💣 gvisor.dev/gvisor/pkg/state                                  from gvisor.dev/gvisor/pkg/atomicbitops+
-        gvisor.dev/gvisor/pkg/state/wire                             from gvisor.dev/gvisor/pkg/state
-     💣 gvisor.dev/gvisor/pkg/sync                                   from gvisor.dev/gvisor/pkg/atomicbitops+
-        gvisor.dev/gvisor/pkg/tcpip                                  from tailscale.com/ipn/ipnlocal
-     💣 gvisor.dev/gvisor/pkg/tcpip/checksum                         from gvisor.dev/gvisor/pkg/buffer
-        gvisor.dev/gvisor/pkg/waiter                                 from gvisor.dev/gvisor/pkg/context+
         tailscale.com                                                from tailscale.com/version
         tailscale.com/appc                                           from tailscale.com/ipn/ipnlocal
         tailscale.com/atomicfile                                     from tailscale.com/ipn+
@@ -283,7 +268,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         golang.org/x/text/transform                                  from golang.org/x/text/secure/bidirule+
         golang.org/x/text/unicode/bidi                               from golang.org/x/net/idna+
         golang.org/x/text/unicode/norm                               from golang.org/x/net/idna
-        golang.org/x/time/rate                                       from gvisor.dev/gvisor/pkg/log+
+        golang.org/x/time/rate                                       from tailscale.com/derp+
         archive/tar                                                  from tailscale.com/clientupdate
         bufio                                                        from compress/flate+
         bytes                                                        from archive/tar+

cmd/tailscaled/deps_test.go

@@ -186,6 +186,19 @@ func TestOmitDBus(t *testing.T) {
 	}.Check(t)
 }
 
+func TestNetstack(t *testing.T) {
+	deptest.DepChecker{
+		GOOS:   "linux",
+		GOARCH: "amd64",
+		Tags:   "ts_omit_gro,ts_omit_netstack,ts_omit_outboundproxy,ts_omit_serve,ts_omit_ssh,ts_omit_webclient,ts_omit_tap",
+		OnDep: func(dep string) {
+			if strings.Contains(dep, "gvisor") {
+				t.Errorf("unexpected gvisor dep: %q", dep)
+			}
+		},
+	}.Check(t)
+}
+
 func TestOmitPortlist(t *testing.T) {
 	deptest.DepChecker{
 		GOOS:   "linux",

feature/buildfeatures/feature_netstack_disabled.go

@@ -7,7 +7,7 @@
 
 package buildfeatures
 
-// HasNetstack is whether the binary was built with support for modular feature "gVisor netstack (userspace networking) support (TODO; not yet omittable)".
+// HasNetstack is whether the binary was built with support for modular feature "gVisor netstack (userspace networking) support".
 // Specifically, it's whether the binary was NOT built with the "ts_omit_netstack" build tag.
 // It's a const so it can be used for dead code elimination.
 const HasNetstack = false

feature/buildfeatures/feature_netstack_enabled.go

@@ -7,7 +7,7 @@
 
 package buildfeatures
 
-// HasNetstack is whether the binary was built with support for modular feature "gVisor netstack (userspace networking) support (TODO; not yet omittable)".
+// HasNetstack is whether the binary was built with support for modular feature "gVisor netstack (userspace networking) support".
 // Specifically, it's whether the binary was NOT built with the "ts_omit_netstack" build tag.
 // It's a const so it can be used for dead code elimination.
 const HasNetstack = true

feature/featuretags/featuretags.go

@@ -121,7 +121,7 @@ var Features = map[FeatureTag]FeatureMeta{
 	},
 	"portlist":   {"PortList", "Optionally advertise listening service ports", nil},
 	"portmapper": {"PortMapper", "NAT-PMP/PCP/UPnP port mapping support", nil},
-	"netstack":   {"Netstack", "gVisor netstack (userspace networking) support (TODO; not yet omittable)", nil},
+	"netstack":   {"Netstack", "gVisor netstack (userspace networking) support", nil},
 	"networkmanager": {
 		Sym:  "NetworkManager",
 		Desc: "Linux NetworkManager integration",

ipn/ipnlocal/local.go

@@ -38,7 +38,6 @@ import (
 	"go4.org/mem"
 	"go4.org/netipx"
 	"golang.org/x/net/dns/dnsmessage"
-	"gvisor.dev/gvisor/pkg/tcpip"
 	"tailscale.com/appc"
 	"tailscale.com/client/tailscale/apitype"
 	"tailscale.com/clientupdate"
@@ -4643,65 +4642,6 @@ var (
 	hookServeClearVIPServicesTCPPortsInterceptedLocked      feature.Hook[func(*LocalBackend)]
 )
 
-// TCPHandlerForDst returns a TCP handler for connections to dst, or nil if
-// no handler is needed. It also returns a list of TCP socket options to
-// apply to the socket before calling the handler.
-// TCPHandlerForDst is called both for connections to our node's local IP
-// as well as to the service IP (quad 100).
-func (b *LocalBackend) TCPHandlerForDst(src, dst netip.AddrPort) (handler func(c net.Conn) error, opts []tcpip.SettableSocketOption) {
-	// First handle internal connections to the service IP
-	hittingServiceIP := dst.Addr() == magicDNSIP || dst.Addr() == magicDNSIPv6
-	if hittingServiceIP {
-		switch dst.Port() {
-		case 80:
-			// TODO(mpminardi): do we want to show an error message if the web client
-			// has been disabled instead of the more "basic" web UI?
-			if b.ShouldRunWebClient() {
-				return b.handleWebClientConn, opts
-			}
-			return b.HandleQuad100Port80Conn, opts
-		case DriveLocalPort:
-			return b.handleDriveConn, opts
-		}
-	}
-
-	if f, ok := hookServeTCPHandlerForVIPService.GetOk(); ok {
-		if handler := f(b, dst, src); handler != nil {
-			return handler, opts
-		}
-	}
-	// Then handle external connections to the local IP.
-	if !b.isLocalIP(dst.Addr()) {
-		return nil, nil
-	}
-	if dst.Port() == 22 && b.ShouldRunSSH() {
-		// Use a higher keepalive idle time for SSH connections, as they are
-		// typically long lived and idle connections are more likely to be
-		// intentional. Ideally we would turn this off entirely, but we can't
-		// tell the difference between a long lived connection that is idle
-		// vs a connection that is dead because the peer has gone away.
-		// We pick 72h as that is typically sufficient for a long weekend.
-		opts = append(opts, ptr.To(tcpip.KeepaliveIdleOption(72*time.Hour)))
-		return b.handleSSHConn, opts
-	}
-	// TODO(will,sonia): allow customizing web client port ?
-	if dst.Port() == webClientPort && b.ShouldExposeRemoteWebClient() {
-		return b.handleWebClientConn, opts
-	}
-	if port, ok := b.GetPeerAPIPort(dst.Addr()); ok && dst.Port() == port {
-		return func(c net.Conn) error {
-			b.handlePeerAPIConn(src, dst, c)
-			return nil
-		}, opts
-	}
-	if f, ok := hookTCPHandlerForServe.GetOk(); ok {
-		if handler := f(b, dst.Port(), src, nil); handler != nil {
-			return handler, opts
-		}
-	}
-	return nil, nil
-}
-
 func (b *LocalBackend) handleDriveConn(conn net.Conn) error {
 	fs, ok := b.sys.DriveForLocal.GetOK()
 	if !ok || !b.DriveAccessEnabled() {

ipn/ipnlocal/netstack.go

@@ -0,0 +1,74 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build !ts_omit_netstack
+
+package ipnlocal
+
+import (
+	"net"
+	"net/netip"
+	"time"
+
+	"gvisor.dev/gvisor/pkg/tcpip"
+	"tailscale.com/types/ptr"
+)
+
+// TCPHandlerForDst returns a TCP handler for connections to dst, or nil if
+// no handler is needed. It also returns a list of TCP socket options to
+// apply to the socket before calling the handler.
+// TCPHandlerForDst is called both for connections to our node's local IP
+// as well as to the service IP (quad 100).
+func (b *LocalBackend) TCPHandlerForDst(src, dst netip.AddrPort) (handler func(c net.Conn) error, opts []tcpip.SettableSocketOption) {
+	// First handle internal connections to the service IP
+	hittingServiceIP := dst.Addr() == magicDNSIP || dst.Addr() == magicDNSIPv6
+	if hittingServiceIP {
+		switch dst.Port() {
+		case 80:
+			// TODO(mpminardi): do we want to show an error message if the web client
+			// has been disabled instead of the more "basic" web UI?
+			if b.ShouldRunWebClient() {
+				return b.handleWebClientConn, opts
+			}
+			return b.HandleQuad100Port80Conn, opts
+		case DriveLocalPort:
+			return b.handleDriveConn, opts
+		}
+	}
+
+	if f, ok := hookServeTCPHandlerForVIPService.GetOk(); ok {
+		if handler := f(b, dst, src); handler != nil {
+			return handler, opts
+		}
+	}
+	// Then handle external connections to the local IP.
+	if !b.isLocalIP(dst.Addr()) {
+		return nil, nil
+	}
+	if dst.Port() == 22 && b.ShouldRunSSH() {
+		// Use a higher keepalive idle time for SSH connections, as they are
+		// typically long lived and idle connections are more likely to be
+		// intentional. Ideally we would turn this off entirely, but we can't
+		// tell the difference between a long lived connection that is idle
+		// vs a connection that is dead because the peer has gone away.
+		// We pick 72h as that is typically sufficient for a long weekend.
+		opts = append(opts, ptr.To(tcpip.KeepaliveIdleOption(72*time.Hour)))
+		return b.handleSSHConn, opts
+	}
+	// TODO(will,sonia): allow customizing web client port ?
+	if dst.Port() == webClientPort && b.ShouldExposeRemoteWebClient() {
+		return b.handleWebClientConn, opts
+	}
+	if port, ok := b.GetPeerAPIPort(dst.Addr()); ok && dst.Port() == port {
+		return func(c net.Conn) error {
+			b.handlePeerAPIConn(src, dst, c)
+			return nil
+		}, opts
+	}
+	if f, ok := hookTCPHandlerForServe.GetOk(); ok {
+		if handler := f(b, dst.Port(), src, nil); handler != nil {
+			return handler, opts
+		}
+	}
+	return nil, nil
+}