control/controlknobs, all: add plumbed Knobs type, not global variables

Previously two tsnet nodes in the same process couldn't have disjoint
sets of controlknob settings from control as both would overwrite each
other's global variables.

This plumbs a new controlknobs.Knobs type around everywhere and hangs
the knobs sent by control on that instead.

Updates #9351

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

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)

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)
 	}

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
 	}
 

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)
 	}