From 87ee0f4e982cbb252d03d31beec251dad9c8ba1c Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 26 Sep 2025 13:05:20 -0700 Subject: [PATCH] ipn/ipnlocal: move last unconditional gvisor import, complete ts_omit_netstack support Fixes #17283 Change-Id: Ia84d269683e4a68d7d10562561204934eeaf53bb Signed-off-by: Brad Fitzpatrick --- cmd/tailscaled/depaware-minbox.txt | 17 +---- cmd/tailscaled/deps_test.go | 13 ++++ .../feature_netstack_disabled.go | 2 +- .../buildfeatures/feature_netstack_enabled.go | 2 +- feature/featuretags/featuretags.go | 2 +- ipn/ipnlocal/local.go | 60 --------------- ipn/ipnlocal/netstack.go | 74 +++++++++++++++++++ 7 files changed, 91 insertions(+), 79 deletions(-) create mode 100644 ipn/ipnlocal/netstack.go diff --git a/cmd/tailscaled/depaware-minbox.txt b/cmd/tailscaled/depaware-minbox.txt index 3a7469c0f..144871c9b 100644 --- a/cmd/tailscaled/depaware-minbox.txt +++ b/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+ diff --git a/cmd/tailscaled/deps_test.go b/cmd/tailscaled/deps_test.go index 92c6a872c..a41a08f9d 100644 --- a/cmd/tailscaled/deps_test.go +++ b/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", diff --git a/feature/buildfeatures/feature_netstack_disabled.go b/feature/buildfeatures/feature_netstack_disabled.go index 7369645a0..acb6e8e76 100644 --- a/feature/buildfeatures/feature_netstack_disabled.go +++ b/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 diff --git a/feature/buildfeatures/feature_netstack_enabled.go b/feature/buildfeatures/feature_netstack_enabled.go index a7e57098b..04f671185 100644 --- a/feature/buildfeatures/feature_netstack_enabled.go +++ b/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 diff --git a/feature/featuretags/featuretags.go b/feature/featuretags/featuretags.go index 1db377277..25426c973 100644 --- a/feature/featuretags/featuretags.go +++ b/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", diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 62a3a2131..4b8032e9c 100644 --- a/ipn/ipnlocal/local.go +++ b/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() { diff --git a/ipn/ipnlocal/netstack.go b/ipn/ipnlocal/netstack.go new file mode 100644 index 000000000..f7ffd0305 --- /dev/null +++ b/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 +}