diff --git a/cmd/tailscale/cli/netcheck.go b/cmd/tailscale/cli/netcheck.go index a61505087..dd7d6a9af 100644 --- a/cmd/tailscale/cli/netcheck.go +++ b/cmd/tailscale/cli/netcheck.go @@ -53,7 +53,7 @@ func runNetcheck(ctx context.Context, args []string) error { return err } c := &netcheck.Client{ - PortMapper: portmapper.NewClient(logf, netMon, nil, nil), + PortMapper: portmapper.NewClient(logf, netMon, nil, nil, nil), UseDNSCache: false, // always resolve, don't cache } if netcheckArgs.verbose { diff --git a/cmd/tailscaled/tailscaled.go b/cmd/tailscaled/tailscaled.go index 9fbbe4869..15611928d 100644 --- a/cmd/tailscaled/tailscaled.go +++ b/cmd/tailscaled/tailscaled.go @@ -607,6 +607,7 @@ func tryEngine(logf logger.Logf, sys *tsd.System, name string) (onlyNetstack boo NetMon: sys.NetMon.Get(), Dialer: sys.Dialer.Get(), SetSubsystem: sys.Set, + ControlKnobs: sys.ControlKnobs(), } onlyNetstack = name == "userspace-networking" diff --git a/cmd/tsconnect/wasm/wasm_js.go b/cmd/tsconnect/wasm/wasm_js.go index 6066612fc..b49b43883 100644 --- a/cmd/tsconnect/wasm/wasm_js.go +++ b/cmd/tsconnect/wasm/wasm_js.go @@ -102,6 +102,7 @@ func newIPN(jsConfig js.Value) map[string]any { eng, err := wgengine.NewUserspaceEngine(logf, wgengine.Config{ Dialer: dialer, SetSubsystem: sys.Set, + ControlKnobs: sys.ControlKnobs(), }) if err != nil { log.Fatal(err) diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index dfd8efb12..30a81c5de 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -25,7 +25,6 @@ import ( "slices" "strings" "sync" - "sync/atomic" "time" "go4.org/mem" @@ -44,7 +43,6 @@ import ( "tailscale.com/net/tsdial" "tailscale.com/net/tshttpproxy" "tailscale.com/smallzstd" - "tailscale.com/syncs" "tailscale.com/tailcfg" "tailscale.com/tka" "tailscale.com/tstime" @@ -66,7 +64,8 @@ type Direct struct { httpc *http.Client // HTTP client used to talk to tailcontrol dialer *tsdial.Dialer dnsCache *dnscache.Resolver - serverURL string // URL of the tailcontrol server + controlKnobs *controlknobs.Knobs // always non-nil + serverURL string // URL of the tailcontrol server clock tstime.Clock lastPrintMap time.Time logf logger.Logf @@ -129,6 +128,7 @@ type Options struct { OnControlTime func(time.Time) // optional func to notify callers of new time from control Dialer *tsdial.Dialer // non-nil C2NHandler http.Handler // or nil + ControlKnobs *controlknobs.Knobs // or nil to ignore // Observer is called when there's a change in status to report // from the control client. @@ -202,6 +202,9 @@ func NewDirect(opts Options) (*Direct, error) { if opts.GetMachinePrivateKey == nil { return nil, errors.New("controlclient.New: no GetMachinePrivateKey specified") } + if opts.ControlKnobs == nil { + opts.ControlKnobs = &controlknobs.Knobs{} + } opts.ServerURL = strings.TrimRight(opts.ServerURL, "/") serverURL, err := url.Parse(opts.ServerURL) if err != nil { @@ -249,6 +252,7 @@ func NewDirect(opts Options) (*Direct, error) { c := &Direct{ httpc: httpc, + controlKnobs: opts.ControlKnobs, getMachinePrivKey: opts.GetMachinePrivateKey, serverURL: opts.ServerURL, clock: opts.Clock, @@ -946,7 +950,7 @@ func (c *Direct) sendMapRequest(ctx context.Context, isStreaming bool, nu Netmap var mapResIdx int // 0 for first message, then 1+ for deltas - sess := newMapSession(persist.PrivateNodeKey(), nu) + sess := newMapSession(persist.PrivateNodeKey(), nu, c.controlKnobs) defer sess.Close() sess.cancel = cancel sess.logf = c.logf @@ -1287,38 +1291,11 @@ func initDevKnob() devKnobs { var clock tstime.Clock = tstime.StdClock{} -// config from control. -var ( - controlDisableDRPO atomic.Bool - controlKeepFullWGConfig atomic.Bool - controlRandomizeClientPort atomic.Bool - controlOneCGNAT syncs.AtomicValue[opt.Bool] -) - -// DisableDRPO reports whether control says to disable the -// DERP route optimization (Issue 150). -func DisableDRPO() bool { - return controlDisableDRPO.Load() -} - -// KeepFullWGConfig reports whether control says we should disable the lazy -// wireguard programming and instead give it the full netmap always. -func KeepFullWGConfig() bool { - return controlKeepFullWGConfig.Load() -} - -// RandomizeClientPort reports whether control says we should randomize -// the client port. -func RandomizeClientPort() bool { - return controlRandomizeClientPort.Load() -} - -// ControlOneCGNATSetting returns control's OneCGNAT setting, if any. -func ControlOneCGNATSetting() opt.Bool { - return controlOneCGNAT.Load() -} - -func setControlKnobsFromNodeAttrs(selfNodeAttrs []string) { +func (ms *mapSession) setControlKnobsFromNodeAttrs(selfNodeAttrs []string) { + k := ms.controlKnobs + if k == nil { + return + } var ( keepFullWG bool disableDRPO bool @@ -1342,11 +1319,11 @@ func setControlKnobsFromNodeAttrs(selfNodeAttrs []string) { oneCGNAT.Set(false) } } - controlKeepFullWGConfig.Store(keepFullWG) - controlDisableDRPO.Store(disableDRPO) - controlknobs.SetDisableUPnP(disableUPnP) - controlRandomizeClientPort.Store(randomizeClientPort) - controlOneCGNAT.Store(oneCGNAT) + k.KeepFullWGConfig.Store(keepFullWG) + k.DisableDRPO.Store(disableDRPO) + k.DisableUPnP.Store(disableUPnP) + k.RandomizeClientPort.Store(randomizeClientPort) + k.OneCGNAT.Store(oneCGNAT) } // ipForwardingBroken reports whether the system's IP forwarding is disabled diff --git a/control/controlclient/map.go b/control/controlclient/map.go index 176f89a53..55eb20b04 100644 --- a/control/controlclient/map.go +++ b/control/controlclient/map.go @@ -15,6 +15,7 @@ import ( "strconv" "sync" + "tailscale.com/control/controlknobs" "tailscale.com/envknob" "tailscale.com/tailcfg" "tailscale.com/tstime" @@ -38,7 +39,8 @@ import ( // one MapRequest). type mapSession struct { // Immutable fields. - nu NetmapUpdater // called on changes (in addition to the optional hooks below) + nu NetmapUpdater // called on changes (in addition to the optional hooks below) + controlKnobs *controlknobs.Knobs // or nil privateNodeKey key.NodePrivate publicNodeKey key.NodePublic logf logger.Logf @@ -94,9 +96,10 @@ type mapSession struct { // Modify its optional fields on the returned value before use. // // It must have its Close method called to release resources. -func newMapSession(privateNodeKey key.NodePrivate, nu NetmapUpdater) *mapSession { +func newMapSession(privateNodeKey key.NodePrivate, nu NetmapUpdater, controlKnobs *controlknobs.Knobs) *mapSession { ms := &mapSession{ nu: nu, + controlKnobs: controlKnobs, privateNodeKey: privateNodeKey, publicNodeKey: privateNodeKey.Public(), lastDNSConfig: new(tailcfg.DNSConfig), @@ -184,7 +187,7 @@ func (ms *mapSession) HandleNonKeepAliveMapResponse(ctx context.Context, resp *t if DevKnob.StripCaps() { resp.Node.Capabilities = nil } - setControlKnobsFromNodeAttrs(resp.Node.Capabilities) + ms.setControlKnobsFromNodeAttrs(resp.Node.Capabilities) } // Call Node.InitDisplayNames on any changed nodes. diff --git a/control/controlclient/map_test.go b/control/controlclient/map_test.go index 1cfd511ff..bddec375f 100644 --- a/control/controlclient/map_test.go +++ b/control/controlclient/map_test.go @@ -16,6 +16,7 @@ import ( "github.com/google/go-cmp/cmp" "go4.org/mem" + "tailscale.com/control/controlknobs" "tailscale.com/tailcfg" "tailscale.com/tstest" "tailscale.com/tstime" @@ -392,7 +393,7 @@ func formatNodes(nodes []*tailcfg.Node) string { } func newTestMapSession(t testing.TB, nu NetmapUpdater) *mapSession { - ms := newMapSession(key.NewNode(), nu) + ms := newMapSession(key.NewNode(), nu, new(controlknobs.Knobs)) t.Cleanup(ms.Close) ms.logf = t.Logf return ms diff --git a/control/controlknobs/controlknobs.go b/control/controlknobs/controlknobs.go index 65492b39e..ead726e90 100644 --- a/control/controlknobs/controlknobs.go +++ b/control/controlknobs/controlknobs.go @@ -8,22 +8,30 @@ package controlknobs import ( "sync/atomic" - "tailscale.com/envknob" + "tailscale.com/syncs" + "tailscale.com/types/opt" ) -// disableUPnP indicates whether to attempt UPnP mapping. -var disableUPnPControl atomic.Bool +// Knobs is the set of knobs that the control plane's coordination server can +// adjust at runtime. +type Knobs struct { + // DisableUPnP indicates whether to attempt UPnP mapping. + DisableUPnP atomic.Bool -var disableUPnpEnv = envknob.RegisterBool("TS_DISABLE_UPNP") + // DisableDRPO is whether control says to disable the + // DERP route optimization (Issue 150). + DisableDRPO atomic.Bool -// DisableUPnP reports the last reported value from control -// whether UPnP portmapping should be disabled. -func DisableUPnP() bool { - return disableUPnPControl.Load() || disableUPnpEnv() -} + // KeepFullWGConfig is whether we should disable the lazy wireguard + // programming and instead give WireGuard the full netmap always, even for + // idle peers. + KeepFullWGConfig atomic.Bool + + // RandomizeClientPort is whether control says we should randomize + // the client port. + RandomizeClientPort atomic.Bool -// SetDisableUPnP sets whether control says that UPnP should be -// disabled. -func SetDisableUPnP(v bool) { - disableUPnPControl.Store(v) + // OneCGNAT is whether the the node should make one big CGNAT route + // in the OS rather than one /32 per peer. + OneCGNAT syncs.AtomicValue[opt.Bool] } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index d3024af81..57d1e66aa 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -34,6 +34,7 @@ import ( "gvisor.dev/gvisor/pkg/tcpip" "tailscale.com/client/tailscale/apitype" "tailscale.com/control/controlclient" + "tailscale.com/control/controlknobs" "tailscale.com/doctor" "tailscale.com/doctor/permissions" "tailscale.com/doctor/routetable" @@ -3084,7 +3085,7 @@ func (b *LocalBackend) authReconfig() { return } - oneCGNATRoute := shouldUseOneCGNATRoute(b.logf, version.OS()) + oneCGNATRoute := shouldUseOneCGNATRoute(b.logf, b.sys.ControlKnobs(), version.OS()) rcfg := b.routerConfig(cfg, prefs, oneCGNATRoute) dcfg := dnsConfigForNetmap(nm, prefs, b.logf, version.OS()) @@ -3102,11 +3103,13 @@ func (b *LocalBackend) authReconfig() { // // The versionOS is a Tailscale-style version ("iOS", "macOS") and not // a runtime.GOOS. -func shouldUseOneCGNATRoute(logf logger.Logf, versionOS string) bool { - // Explicit enabling or disabling always take precedence. - if v, ok := controlclient.ControlOneCGNATSetting().Get(); ok { - logf("[v1] shouldUseOneCGNATRoute: explicit=%v", v) - return v +func shouldUseOneCGNATRoute(logf logger.Logf, controlKnobs *controlknobs.Knobs, versionOS string) bool { + if controlKnobs != nil { + // Explicit enabling or disabling always take precedence. + if v, ok := controlKnobs.OneCGNAT.Load().Get(); ok { + logf("[v1] shouldUseOneCGNATRoute: explicit=%v", v) + return v + } } // Also prefer to do this on the Mac, so that we don't need to constantly @@ -4663,6 +4666,11 @@ func (b *LocalBackend) DebugReSTUN() error { return nil } +// ControlKnobs returns the node's control knobs. +func (b *LocalBackend) ControlKnobs() *controlknobs.Knobs { + return b.sys.ControlKnobs() +} + func (b *LocalBackend) magicConn() (*magicsock.Conn, error) { mc, ok := b.sys.MagicSock.GetOK() if !ok { diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index 678e91458..82bb28797 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -694,7 +694,7 @@ func (h *Handler) serveDebugPortmap(w http.ResponseWriter, r *http.Request) { done := make(chan bool, 1) var c *portmapper.Client - c = portmapper.NewClient(logger.WithPrefix(logf, "portmapper: "), h.netMon, debugKnobs, func() { + c = portmapper.NewClient(logger.WithPrefix(logf, "portmapper: "), h.netMon, debugKnobs, h.b.ControlKnobs(), func() { logf("portmapping changed.") logf("have mapping: %v", c.HaveMapping()) diff --git a/net/portmapper/igd_test.go b/net/portmapper/igd_test.go index 57c0d7aa6..3551a7fa8 100644 --- a/net/portmapper/igd_test.go +++ b/net/portmapper/igd_test.go @@ -14,6 +14,7 @@ import ( "sync/atomic" "testing" + "tailscale.com/control/controlknobs" "tailscale.com/net/netaddr" "tailscale.com/types/logger" ) @@ -249,7 +250,7 @@ func (d *TestIGD) handlePCPQuery(pkt []byte, src netip.AddrPort) { func newTestClient(t *testing.T, igd *TestIGD) *Client { var c *Client - c = NewClient(t.Logf, nil, nil, func() { + c = NewClient(t.Logf, nil, nil, new(controlknobs.Knobs), func() { t.Logf("port map changed") t.Logf("have mapping: %v", c.HaveMapping()) }) diff --git a/net/portmapper/portmapper.go b/net/portmapper/portmapper.go index d934e202b..ee16bfe61 100644 --- a/net/portmapper/portmapper.go +++ b/net/portmapper/portmapper.go @@ -18,6 +18,7 @@ import ( "time" "go4.org/mem" + "tailscale.com/control/controlknobs" "tailscale.com/net/interfaces" "tailscale.com/net/netaddr" "tailscale.com/net/neterror" @@ -66,6 +67,7 @@ const trustServiceStillAvailableDuration = 10 * time.Minute type Client struct { logf logger.Logf netMon *netmon.Monitor // optional; nil means interfaces will be looked up on-demand + controlKnobs *controlknobs.Knobs ipAndGateway func() (gw, ip netip.Addr, ok bool) onChange func() // or nil debug DebugKnobs @@ -166,15 +168,19 @@ func (m *pmpMapping) Release(ctx context.Context) { // The debug argument allows configuring the behaviour of the portmapper for // debugging; if nil, a sensible set of defaults will be used. // -// The optional onChange argument specifies a func to run in a new -// goroutine whenever the port mapping status has changed. If nil, -// it doesn't make a callback. -func NewClient(logf logger.Logf, netMon *netmon.Monitor, debug *DebugKnobs, onChange func()) *Client { +// The controlKnobs, if non-nil, specifies the control knobs from the control +// plane that might disable portmapping. +// +// The optional onChange argument specifies a func to run in a new goroutine +// whenever the port mapping status has changed. If nil, it doesn't make a +// callback. +func NewClient(logf logger.Logf, netMon *netmon.Monitor, debug *DebugKnobs, controlKnobs *controlknobs.Knobs, onChange func()) *Client { ret := &Client{ logf: logf, netMon: netMon, ipAndGateway: interfaces.LikelyHomeRouterIP, onChange: onChange, + controlKnobs: controlKnobs, } if debug != nil { ret.debug = *debug diff --git a/net/portmapper/portmapper_test.go b/net/portmapper/portmapper_test.go index 15f2849fc..83a2afe66 100644 --- a/net/portmapper/portmapper_test.go +++ b/net/portmapper/portmapper_test.go @@ -10,13 +10,15 @@ import ( "strconv" "testing" "time" + + "tailscale.com/control/controlknobs" ) func TestCreateOrGetMapping(t *testing.T) { if v, _ := strconv.ParseBool(os.Getenv("HIT_NETWORK")); !v { t.Skip("skipping test without HIT_NETWORK=1") } - c := NewClient(t.Logf, nil, nil, nil) + c := NewClient(t.Logf, nil, nil, new(controlknobs.Knobs), nil) defer c.Close() c.SetLocalPort(1234) for i := 0; i < 2; i++ { @@ -32,7 +34,7 @@ func TestClientProbe(t *testing.T) { if v, _ := strconv.ParseBool(os.Getenv("HIT_NETWORK")); !v { t.Skip("skipping test without HIT_NETWORK=1") } - c := NewClient(t.Logf, nil, nil, nil) + c := NewClient(t.Logf, nil, nil, new(controlknobs.Knobs), nil) defer c.Close() for i := 0; i < 3; i++ { if i > 0 { @@ -47,7 +49,7 @@ func TestClientProbeThenMap(t *testing.T) { if v, _ := strconv.ParseBool(os.Getenv("HIT_NETWORK")); !v { t.Skip("skipping test without HIT_NETWORK=1") } - c := NewClient(t.Logf, nil, nil, nil) + c := NewClient(t.Logf, nil, nil, new(controlknobs.Knobs), nil) defer c.Close() c.SetLocalPort(1234) res, err := c.Probe(context.Background()) diff --git a/net/portmapper/upnp.go b/net/portmapper/upnp.go index b85eb488d..54c1c2f1a 100644 --- a/net/portmapper/upnp.go +++ b/net/portmapper/upnp.go @@ -24,7 +24,7 @@ import ( "github.com/tailscale/goupnp" "github.com/tailscale/goupnp/dcps/internetgateway2" - "tailscale.com/control/controlknobs" + "tailscale.com/envknob" "tailscale.com/net/netns" "tailscale.com/types/logger" ) @@ -192,7 +192,7 @@ func addAnyPortMapping( // The provided ctx is not retained in the returned upnpClient, but // its associated HTTP client is (if set via goupnp.WithHTTPClient). func getUPnPClient(ctx context.Context, logf logger.Logf, debug DebugKnobs, gw netip.Addr, meta uPnPDiscoResponse) (client upnpClient, err error) { - if controlknobs.DisableUPnP() || debug.DisableUPnP { + if debug.DisableUPnP { return nil, nil } @@ -270,6 +270,10 @@ func (c *Client) upnpHTTPClientLocked() *http.Client { return c.uPnPHTTPClient } +var ( + disableUPnpEnv = envknob.RegisterBool("TS_DISABLE_UPNP") +) + // getUPnPPortMapping attempts to create a port-mapping over the UPnP protocol. On success, // it will return the externally exposed IP and port. Otherwise, it will return a zeroed IP and // port and an error. @@ -279,7 +283,7 @@ func (c *Client) getUPnPPortMapping( internal netip.AddrPort, prevPort uint16, ) (external netip.AddrPort, ok bool) { - if controlknobs.DisableUPnP() || c.debug.DisableUPnP { + if disableUPnpEnv() || c.debug.DisableUPnP || (c.controlKnobs != nil && c.controlKnobs.DisableUPnP.Load()) { return netip.AddrPort{}, false } diff --git a/tsd/tsd.go b/tsd/tsd.go index ffa245bc9..f0f624f3a 100644 --- a/tsd/tsd.go +++ b/tsd/tsd.go @@ -21,6 +21,7 @@ import ( "fmt" "reflect" + "tailscale.com/control/controlknobs" "tailscale.com/ipn" "tailscale.com/net/dns" "tailscale.com/net/netmon" @@ -42,6 +43,7 @@ type System struct { Router SubSystem[router.Router] Tun SubSystem[*tstun.Wrapper] StateStore SubSystem[ipn.StateStore] + controlKnobs controlknobs.Knobs } // Set is a convenience method to set a subsystem value. @@ -85,6 +87,11 @@ func (s *System) IsNetstack() bool { return name == tstun.FakeTUNName } +// ControlKnobs returns the control knobs for this node. +func (s *System) ControlKnobs() *controlknobs.Knobs { + return &s.controlKnobs +} + // SubSystem represents some subsystem of the Tailscale node daemon. // // A subsystem can be set to a value, and then later retrieved. A subsystem diff --git a/tsnet/tsnet.go b/tsnet/tsnet.go index 94ae47911..fb92b7139 100644 --- a/tsnet/tsnet.go +++ b/tsnet/tsnet.go @@ -501,6 +501,7 @@ func (s *Server) start() (reterr error) { NetMon: s.netMon, Dialer: s.dialer, SetSubsystem: sys.Set, + ControlKnobs: sys.ControlKnobs(), }) if err != nil { return err diff --git a/wgengine/magicsock/derp.go b/wgengine/magicsock/derp.go index 8a0328cc9..fa23d1d55 100644 --- a/wgengine/magicsock/derp.go +++ b/wgengine/magicsock/derp.go @@ -18,7 +18,6 @@ import ( "time" "github.com/tailscale/wireguard-go/conn" - "tailscale.com/control/controlclient" "tailscale.com/derp" "tailscale.com/derp/derphttp" "tailscale.com/health" @@ -38,11 +37,11 @@ import ( // // By default it's enabled, unless an environment variable // or control says to disable it. -func useDerpRoute() bool { +func (c *Conn) useDerpRoute() bool { if b, ok := debugUseDerpRoute().Get(); ok { return b } - return !controlclient.DisableDRPO() + return c.controlKnobs == nil || !c.controlKnobs.DisableDRPO.Load() } // derpRoute is a route entry for a public key, saying that a certain @@ -294,7 +293,7 @@ func (c *Conn) derpWriteChanOfAddr(addr netip.AddrPort, peer key.NodePublic) cha // perhaps peer's home is Frankfurt, but they dialed our home DERP // node in SF to reach us, so we can reply to them using our // SF connection rather than dialing Frankfurt. (Issue 150) - if !peer.IsZero() && useDerpRoute() { + if !peer.IsZero() && c.useDerpRoute() { if r, ok := c.derpRoute[peer]; ok { if ad, ok := c.activeDerp[r.derpID]; ok && ad.c == r.dc { c.setPeerLastDerpLocked(peer, r.derpID, regionID) diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index b82196c9a..f9244e0fb 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -25,6 +25,7 @@ import ( "golang.org/x/net/ipv4" "golang.org/x/net/ipv6" + "tailscale.com/control/controlknobs" "tailscale.com/disco" "tailscale.com/envknob" "tailscale.com/health" @@ -87,6 +88,7 @@ type Conn struct { testOnlyPacketListener nettype.PacketListener noteRecvActivity func(key.NodePublic) // or nil, see Options.NoteRecvActivity netMon *netmon.Monitor // or nil + controlKnobs *controlknobs.Knobs // or nil // ================================================================ // No locking required to access these fields, either because @@ -340,6 +342,10 @@ type Options struct { // NetMon is the network monitor to use. // With one, the portmapper won't be used. NetMon *netmon.Monitor + + // ControlKnobs are the set of control knobs to use. + // If nil, they're ignored and not updated. + ControlKnobs *controlknobs.Knobs } func (o *Options) logf() logger.Logf { @@ -400,13 +406,14 @@ func newConn() *Conn { func NewConn(opts Options) (*Conn, error) { c := newConn() c.port.Store(uint32(opts.Port)) + c.controlKnobs = opts.ControlKnobs c.logf = opts.logf() c.epFunc = opts.endpointsFunc() c.derpActiveFunc = opts.derpActiveFunc() c.idleFunc = opts.IdleFunc c.testOnlyPacketListener = opts.TestOnlyPacketListener c.noteRecvActivity = opts.NoteRecvActivity - c.portMapper = portmapper.NewClient(logger.WithPrefix(c.logf, "portmapper: "), opts.NetMon, nil, c.onPortMapChanged) + c.portMapper = portmapper.NewClient(logger.WithPrefix(c.logf, "portmapper: "), opts.NetMon, nil, opts.ControlKnobs, c.onPortMapChanged) if opts.NetMon != nil { c.portMapper.SetGatewayLookupFunc(opts.NetMon.GatewayAndSelfIP) } diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 967c6ea3d..62d40d336 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -19,7 +19,7 @@ import ( "github.com/tailscale/wireguard-go/device" "github.com/tailscale/wireguard-go/tun" - "tailscale.com/control/controlclient" + "tailscale.com/control/controlknobs" "tailscale.com/envknob" "tailscale.com/health" "tailscale.com/ipn/ipnstate" @@ -95,9 +95,10 @@ type userspaceEngine struct { dns *dns.Manager magicConn *magicsock.Conn netMon *netmon.Monitor - netMonOwned bool // whether we created netMon (and thus need to close it) - netMonUnregister func() // unsubscribes from changes; used regardless of netMonOwned - birdClient BIRDClient // or nil + netMonOwned bool // whether we created netMon (and thus need to close it) + netMonUnregister func() // unsubscribes from changes; used regardless of netMonOwned + birdClient BIRDClient // or nil + controlKnobs *controlknobs.Knobs // or nil testMaybeReconfigHook func() // for tests; if non-nil, fires if maybeReconfigWireguardLocked called @@ -183,6 +184,11 @@ type Config struct { // If nil, a new Dialer is created Dialer *tsdial.Dialer + // ControlKnobs is the set of control plane-provied knobs + // to use. + // If nil, defaults are used. + ControlKnobs *controlknobs.Knobs + // ListenPort is the port on which the engine will listen. // If zero, a port is automatically selected. ListenPort uint16 @@ -220,6 +226,8 @@ func NewFakeUserspaceEngine(logf logger.Logf, opts ...any) (Engine, error) { conf.ListenPort = uint16(v) case func(any): conf.SetSubsystem = v + case *controlknobs.Knobs: + conf.ControlKnobs = v default: return nil, fmt.Errorf("unknown option type %T", v) } @@ -271,6 +279,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) router: conf.Router, confListenPort: conf.ListenPort, birdClient: conf.BIRDClient, + controlKnobs: conf.ControlKnobs, } if e.birdClient != nil { @@ -326,6 +335,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) IdleFunc: e.tundev.IdleDuration, NoteRecvActivity: e.noteRecvActivity, NetMon: e.netMon, + ControlKnobs: conf.ControlKnobs, } var err error @@ -493,12 +503,12 @@ var debugTrimWireguard = envknob.RegisterOptBool("TS_DEBUG_TRIM_WIREGUARD") // That's sad too. Or we get rid of these knobs (lazy wireguard config has been // stable!) but I'm worried that a future regression would be easier to debug // with these knobs in place. -func forceFullWireguardConfig(numPeers int) bool { +func (e *userspaceEngine) forceFullWireguardConfig(numPeers int) bool { // Did the user explicitly enable trimming via the environment variable knob? if b, ok := debugTrimWireguard().Get(); ok { return !b } - return controlclient.KeepFullWGConfig() + return e.controlKnobs != nil && e.controlKnobs.KeepFullWGConfig.Load() } // isTrimmablePeer reports whether p is a peer that we can trim out of the @@ -508,8 +518,8 @@ func forceFullWireguardConfig(numPeers int) bool { // only non-subnet AllowedIPs (an IPv4 /32 or IPv6 /128), which is the // common case for most peers. Subnet router nodes will just always be // created in the wireguard-go config. -func isTrimmablePeer(p *wgcfg.Peer, numPeers int) bool { - if forceFullWireguardConfig(numPeers) { +func (e *userspaceEngine) isTrimmablePeer(p *wgcfg.Peer, numPeers int) bool { + if e.forceFullWireguardConfig(numPeers) { return false } @@ -625,7 +635,7 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked(discoChanged map[key.Node for i := range full.Peers { p := &full.Peers[i] nk := p.PublicKey - if !isTrimmablePeer(p, len(full.Peers)) { + if !e.isTrimmablePeer(p, len(full.Peers)) { min.Peers = append(min.Peers, *p) if discoChanged[nk] { needRemoveStep = true @@ -766,8 +776,6 @@ func hasOverlap(aips, rips views.Slice[netip.Prefix]) bool { return false } -var randomizeClientPort = controlclient.RandomizeClientPort - func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, dnsCfg *dns.Config) error { if routerCfg == nil { panic("routerCfg must not be nil") @@ -794,7 +802,7 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, e.mu.Unlock() listenPort := e.confListenPort - if randomizeClientPort() { + if e.controlKnobs != nil && e.controlKnobs.RandomizeClientPort.Load() { listenPort = 0 } diff --git a/wgengine/userspace_test.go b/wgengine/userspace_test.go index 2afbf7a93..c59bc8253 100644 --- a/wgengine/userspace_test.go +++ b/wgengine/userspace_test.go @@ -11,6 +11,7 @@ import ( "go4.org/mem" "tailscale.com/cmd/testwrapper/flakytest" + "tailscale.com/control/controlknobs" "tailscale.com/net/dns" "tailscale.com/net/netaddr" "tailscale.com/net/tstun" @@ -152,15 +153,16 @@ func TestUserspaceEngineReconfig(t *testing.T) { } func TestUserspaceEnginePortReconfig(t *testing.T) { - tstest.Replace(t, &randomizeClientPort, func() bool { return false }) - flakytest.Mark(t, "https://github.com/tailscale/tailscale/issues/2855") const defaultPort = 49983 + + var knobs controlknobs.Knobs + // Keep making a wgengine until we find an unused port var ue *userspaceEngine for i := 0; i < 100; i++ { attempt := uint16(defaultPort + i) - e, err := NewFakeUserspaceEngine(t.Logf, attempt) + e, err := NewFakeUserspaceEngine(t.Logf, attempt, &knobs) if err != nil { t.Fatal(err) } @@ -199,7 +201,7 @@ func TestUserspaceEnginePortReconfig(t *testing.T) { t.Errorf("no debug setting changed local port to %d from %d", got, startingPort) } - randomizeClientPort = func() bool { return true } + knobs.RandomizeClientPort.Store(true) if err := ue.Reconfig(cfg, routerCfg, &dns.Config{}); err != nil { t.Fatal(err) } @@ -208,7 +210,7 @@ func TestUserspaceEnginePortReconfig(t *testing.T) { } lastPort := ue.magicConn.LocalPort() - randomizeClientPort = func() bool { return false } + knobs.RandomizeClientPort.Store(false) if err := ue.Reconfig(cfg, routerCfg, &dns.Config{}); err != nil { t.Fatal(err) }