envknob: support changing envknobs post-init

Updates #5114

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

wgengine/magicsock/debugknobs.go

@@ -11,28 +11,30 @@ import (
 	"tailscale.com/envknob"
 )
 
+const linkDebug = true
+
 // Various debugging and experimental tweakables, set by environment
 // variable.
 var (
 	// debugDisco prints verbose logs of active discovery events as
 	// they happen.
-	debugDisco = envknob.Bool("TS_DEBUG_DISCO")
+	debugDisco = envknob.RegisterBool("TS_DEBUG_DISCO")
 	// debugOmitLocalAddresses removes all local interface addresses
 	// from magicsock's discovered local endpoints. Used in some tests.
-	debugOmitLocalAddresses = envknob.Bool("TS_DEBUG_OMIT_LOCAL_ADDRS")
+	debugOmitLocalAddresses = envknob.RegisterBool("TS_DEBUG_OMIT_LOCAL_ADDRS")
 	// debugUseDerpRoute temporarily (2020-03-22) controls whether DERP
 	// reverse routing is enabled (Issue 150).
-	debugUseDerpRoute = envknob.OptBool("TS_DEBUG_ENABLE_DERP_ROUTE")
+	debugUseDerpRoute = envknob.RegisterOptBool("TS_DEBUG_ENABLE_DERP_ROUTE")
 	// logDerpVerbose logs all received DERP packets, including their
 	// full payload.
-	logDerpVerbose = envknob.Bool("TS_DEBUG_DERP")
+	logDerpVerbose = envknob.RegisterBool("TS_DEBUG_DERP")
 	// debugReSTUNStopOnIdle unconditionally enables the "shut down
 	// STUN if magicsock is idle" behavior that normally only triggers
 	// on mobile devices, lowers the shutdown interval, and logs more
 	// verbosely about idle measurements.
-	debugReSTUNStopOnIdle = envknob.Bool("TS_DEBUG_RESTUN_STOP_ON_IDLE")
+	debugReSTUNStopOnIdle = envknob.RegisterBool("TS_DEBUG_RESTUN_STOP_ON_IDLE")
 	// debugAlwaysDERP disables the use of UDP, forcing all peer communication over DERP.
-	debugAlwaysDERP = envknob.Bool("TS_DEBUG_ALWAYS_USE_DERP")
+	debugAlwaysDERP = envknob.RegisterBool("TS_DEBUG_ALWAYS_USE_DERP")
 )
 
 // inTest reports whether the running program is a test that set the

wgengine/magicsock/debugknobs_stubs.go

@@ -10,15 +10,15 @@ package magicsock
 import "tailscale.com/types/opt"
 
 // All knobs are disabled on iOS and Wasm.
-// Further, they're const, so the toolchain can produce smaller binaries.
-const (
-	debugDisco                       = false
-	debugOmitLocalAddresses          = false
-	debugUseDerpRouteEnv             = ""
-	debugUseDerpRoute       opt.Bool = ""
-	logDerpVerbose                   = false
-	debugReSTUNStopOnIdle            = false
-	debugAlwaysDERP                  = false
-)
+//
+// They're inlinable and the linker can deadcode that's guarded by them to make
+// smaller binaries.
+func debugDisco() bool              { return false }
+func debugOmitLocalAddresses() bool { return false }
+func logDerpVerbose() bool          { return false }
+func debugReSTUNStopOnIdle() bool   { return false }
+func debugAlwaysDERP() bool         { return false }
+func debugUseDerpRouteEnv() string  { return "" }
+func debugUseDerpRoute() opt.Bool   { return "" }
 
 func inTest() bool { return false }

wgengine/magicsock/magicsock.go

@@ -74,7 +74,7 @@ const (
 // useDerpRoute reports whether magicsock should enable the DERP
 // return path optimization (Issue 150).
 func useDerpRoute() bool {
-	if b, ok := debugUseDerpRoute.Get(); ok {
+	if b, ok := debugUseDerpRoute().Get(); ok {
 		return b
 	}
 	ob := controlclient.DERPRouteFlag()
@@ -638,18 +638,18 @@ func (c *Conn) updateEndpoints(why string) {
 				// etc)
 				d := tstime.RandomDurationBetween(20*time.Second, 26*time.Second)
 				if t := c.periodicReSTUNTimer; t != nil {
-					if debugReSTUNStopOnIdle {
+					if debugReSTUNStopOnIdle() {
 						c.logf("resetting existing periodicSTUN to run in %v", d)
 					}
 					t.Reset(d)
 				} else {
-					if debugReSTUNStopOnIdle {
+					if debugReSTUNStopOnIdle() {
 						c.logf("scheduling periodicSTUN to run in %v", d)
 					}
 					c.periodicReSTUNTimer = time.AfterFunc(d, c.doPeriodicSTUN)
 				}
 			} else {
-				if debugReSTUNStopOnIdle {
+				if debugReSTUNStopOnIdle() {
 					c.logf("periodic STUN idle")
 				}
 				c.stopPeriodicReSTUNTimerLocked()
@@ -1074,7 +1074,7 @@ func (c *Conn) determineEndpoints(ctx context.Context) ([]tailcfg.Endpoint, erro
 		return
 	}
 	addAddr := func(ipp netip.AddrPort, et tailcfg.EndpointType) {
-		if !ipp.IsValid() || (debugOmitLocalAddresses && et == tailcfg.EndpointLocal) {
+		if !ipp.IsValid() || (debugOmitLocalAddresses() && et == tailcfg.EndpointLocal) {
 			return
 		}
 		if _, ok := already[ipp]; !ok {
@@ -1575,7 +1575,7 @@ func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr netip.AddrPort, d
 			pkt = m
 			res.n = len(m.Data)
 			res.src = m.Source
-			if logDerpVerbose {
+			if logDerpVerbose() {
 				c.logf("magicsock: got derp-%v packet: %q", regionID, m.Data)
 			}
 			// If this is a new sender we hadn't seen before, remember it and
@@ -1826,7 +1826,7 @@ func (c *Conn) sendDiscoMessage(dst netip.AddrPort, dstKey key.NodePublic, dstDi
 	pkt = append(pkt, box...)
 	sent, err = c.sendAddr(dst, dstKey, pkt)
 	if sent {
-		if logLevel == discoLog || (logLevel == discoVerboseLog && debugDisco) {
+		if logLevel == discoLog || (logLevel == discoVerboseLog && debugDisco()) {
 			node := "?"
 			if !dstKey.IsZero() {
 				node = dstKey.ShortString()
@@ -1890,7 +1890,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netip.AddrPort, derpNodeSrc ke
 	if c.closed {
 		return
 	}
-	if debugDisco {
+	if debugDisco() {
 		c.logf("magicsock: disco: got disco-looking frame from %v", sender.ShortString())
 	}
 	if c.privateKey.IsZero() {
@@ -1899,7 +1899,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netip.AddrPort, derpNodeSrc ke
 		return
 	}
 	if c.discoPrivate.IsZero() {
-		if debugDisco {
+		if debugDisco() {
 			c.logf("magicsock: disco: ignoring disco-looking frame, no local key")
 		}
 		return
@@ -1907,7 +1907,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netip.AddrPort, derpNodeSrc ke
 
 	if !c.peerMap.anyEndpointForDiscoKey(sender) {
 		metricRecvDiscoBadPeer.Add(1)
-		if debugDisco {
+		if debugDisco() {
 			c.logf("magicsock: disco: ignoring disco-looking frame, don't know endpoint for %v", sender.ShortString())
 		}
 		return
@@ -1931,7 +1931,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netip.AddrPort, derpNodeSrc ke
 		// Don't log in normal case. Pass on to wireguard, in case
 		// it's actually a wireguard packet (super unlikely,
 		// but).
-		if debugDisco {
+		if debugDisco() {
 			c.logf("magicsock: disco: failed to open naclbox from %v (wrong rcpt?)", sender)
 		}
 		metricRecvDiscoBadKey.Add(1)
@@ -1939,7 +1939,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netip.AddrPort, derpNodeSrc ke
 	}
 
 	dm, err := disco.Parse(payload)
-	if debugDisco {
+	if debugDisco() {
 		c.logf("magicsock: disco: disco.Parse = %T, %v", dm, err)
 	}
 	if err != nil {
@@ -2094,7 +2094,7 @@ func (c *Conn) handlePingLocked(dm *disco.Ping, src netip.AddrPort, di *discoInf
 		return
 	}
 
-	if !likelyHeartBeat || debugDisco {
+	if !likelyHeartBeat || debugDisco() {
 		pingNodeSrcStr := dstKey.ShortString()
 		if numNodes > 1 {
 			pingNodeSrcStr = "[one-of-multi]"
@@ -2381,7 +2381,7 @@ func (c *Conn) SetNetworkMap(nm *netmap.NetworkMap) {
 		}
 		ep.wgEndpoint = n.Key.UntypedHexString()
 		ep.initFakeUDPAddr()
-		if debugDisco { // rather than making a new knob
+		if debugDisco() { // rather than making a new knob
 			c.logf("magicsock: created endpoint key=%s: disco=%s; %v", n.Key.ShortString(), n.DiscoKey.ShortString(), logger.ArgWriter(func(w *bufio.Writer) {
 				const derpPrefix = "127.3.3.40:"
 				if strings.HasPrefix(n.DERP, derpPrefix) {
@@ -2736,7 +2736,7 @@ func (c *Conn) goroutinesRunningLocked() bool {
 }
 
 func maxIdleBeforeSTUNShutdown() time.Duration {
-	if debugReSTUNStopOnIdle {
+	if debugReSTUNStopOnIdle() {
 		return 45 * time.Second
 	}
 	return sessionActiveTimeout
@@ -2753,7 +2753,7 @@ func (c *Conn) shouldDoPeriodicReSTUNLocked() bool {
 	}
 	if f := c.idleFunc; f != nil {
 		idleFor := f()
-		if debugReSTUNStopOnIdle {
+		if debugReSTUNStopOnIdle() {
 			c.logf("magicsock: periodicReSTUN: idle for %v", idleFor.Round(time.Second))
 		}
 		if idleFor > maxIdleBeforeSTUNShutdown() {
@@ -2834,7 +2834,7 @@ func (c *Conn) bindSocket(ruc *RebindingUDPConn, network string, curPortFate cur
 		return nil
 	}
 
-	if debugAlwaysDERP {
+	if debugAlwaysDERP() {
 		c.logf("disabled %v per TS_DEBUG_ALWAYS_USE_DERP", network)
 		ruc.setConnLocked(newBlockForeverConn())
 		return nil
@@ -3626,7 +3626,7 @@ func (de *endpoint) pingTimeout(txid stun.TxID) {
 	if !ok {
 		return
 	}
-	if debugDisco || !de.bestAddr.IsValid() || mono.Now().After(de.trustBestAddrUntil) {
+	if debugDisco() || !de.bestAddr.IsValid() || mono.Now().After(de.trustBestAddrUntil) {
 		de.c.logf("[v1] magicsock: disco: timeout waiting for pong %x from %v (%v, %v)", txid[:6], sp.to, de.publicKey.ShortString(), de.discoShort)
 	}
 	de.removeSentPingLocked(txid, sp)

wgengine/magicsock/magicsock_linux.go

@@ -28,7 +28,7 @@ const (
 )
 
 // Enable/disable using raw sockets to receive disco traffic.
-var debugDisableRawDisco = envknob.Bool("TS_DEBUG_DISABLE_RAW_DISCO")
+var debugDisableRawDisco = envknob.RegisterBool("TS_DEBUG_DISABLE_RAW_DISCO")
 
 // These are our BPF filters that we use for testing packets.
 var (
@@ -125,7 +125,7 @@ var (
 // and BPF filter.
 // https://github.com/tailscale/tailscale/issues/3824
 func (c *Conn) listenRawDisco(family string) (io.Closer, error) {
-	if debugDisableRawDisco {
+	if debugDisableRawDisco() {
 		return nil, errors.New("raw disco listening disabled by debug flag")
 	}
 

wgengine/monitor/monitor_linux.go

@@ -20,7 +20,7 @@ import (
 	"tailscale.com/types/logger"
 )
 
-var debugNetlinkMessages = envknob.Bool("TS_DEBUG_NETLINK")
+var debugNetlinkMessages = envknob.RegisterBool("TS_DEBUG_NETLINK")
 
 // unspecifiedMessage is a minimal message implementation that should not
 // be ignored. In general, OS-specific implementations should use better
@@ -96,7 +96,7 @@ func (c *nlConn) Receive() (message, error) {
 
 		nip := netaddrIP(rmsg.Attributes.Address)
 
-		if debugNetlinkMessages {
+		if debugNetlinkMessages() {
 			typ := "RTM_NEWADDR"
 			if msg.Header.Type == unix.RTM_DELADDR {
 				typ = "RTM_DELADDR"
@@ -125,7 +125,7 @@ func (c *nlConn) Receive() (message, error) {
 			}
 
 			if addrs[nip] {
-				if debugNetlinkMessages {
+				if debugNetlinkMessages() {
 					c.logf("ignored duplicate RTM_NEWADDR for %s", nip)
 				}
 				return ignoreMessage{}, nil
@@ -147,7 +147,7 @@ func (c *nlConn) Receive() (message, error) {
 			Addr:    nip,
 			Delete:  msg.Header.Type == unix.RTM_DELADDR,
 		}
-		if debugNetlinkMessages {
+		if debugNetlinkMessages() {
 			c.logf("%+v", nam)
 		}
 		return nam, nil
@@ -169,7 +169,7 @@ func (c *nlConn) Receive() (message, error) {
 			(rmsg.Attributes.Table == 255 || rmsg.Attributes.Table == 254) &&
 			(dst.Addr().IsMulticast() || dst.Addr().IsLinkLocalUnicast()) {
 
-			if debugNetlinkMessages {
+			if debugNetlinkMessages() {
 				c.logf("%s ignored", typeStr)
 			}
 
@@ -202,7 +202,7 @@ func (c *nlConn) Receive() (message, error) {
 			Dst:     dst,
 			Gateway: gw,
 		}
-		if debugNetlinkMessages {
+		if debugNetlinkMessages() {
 			c.logf("%+v", nrm)
 		}
 		return nrm, nil
@@ -225,7 +225,7 @@ func (c *nlConn) Receive() (message, error) {
 			table:    rmsg.Table,
 			priority: rmsg.Attributes.Priority,
 		}
-		if debugNetlinkMessages {
+		if debugNetlinkMessages() {
 			c.logf("%+v", rdm)
 		}
 		return rdm, nil

wgengine/netstack/netstack.go

@@ -55,7 +55,7 @@ import (
 
 const debugPackets = false
 
-var debugNetstack = envknob.Bool("TS_DEBUG_NETSTACK")
+var debugNetstack = envknob.RegisterBool("TS_DEBUG_NETSTACK")
 
 var (
 	magicDNSIP   = tsaddr.TailscaleServiceIP()
@@ -638,7 +638,7 @@ func (ns *Impl) userPing(dstIP netip.Addr, pingResPkt []byte) {
 		}
 		return
 	}
-	if debugNetstack {
+	if debugNetstack() {
 		ns.logf("exec pinged %v in %v", dstIP, time.Since(t0))
 	}
 	if err := ns.tundev.InjectOutbound(pingResPkt); err != nil {
@@ -718,7 +718,7 @@ func netaddrIPFromNetstackIP(s tcpip.Address) netip.Addr {
 
 func (ns *Impl) acceptTCP(r *tcp.ForwarderRequest) {
 	reqDetails := r.ID()
-	if debugNetstack {
+	if debugNetstack() {
 		ns.logf("[v2] TCP ForwarderRequest: %s", stringifyTEI(reqDetails))
 	}
 	clientRemoteIP := netaddrIPFromNetstackIP(reqDetails.RemoteAddress)
@@ -849,7 +849,7 @@ func (ns *Impl) acceptTCP(r *tcp.ForwarderRequest) {
 
 func (ns *Impl) forwardTCP(getClient func() *gonet.TCPConn, clientRemoteIP netip.Addr, wq *waiter.Queue, dialAddr netip.AddrPort) (handled bool) {
 	dialAddrStr := dialAddr.String()
-	if debugNetstack {
+	if debugNetstack() {
 		ns.logf("[v2] netstack: forwarding incoming connection to %s", dialAddrStr)
 	}
 
@@ -866,7 +866,7 @@ func (ns *Impl) forwardTCP(getClient func() *gonet.TCPConn, clientRemoteIP netip
 	go func() {
 		select {
 		case <-notifyCh:
-			if debugNetstack {
+			if debugNetstack() {
 				ns.logf("[v2] netstack: forwardTCP notifyCh fired; canceling context for %s", dialAddrStr)
 			}
 		case <-done:
@@ -919,7 +919,7 @@ func (ns *Impl) forwardTCP(getClient func() *gonet.TCPConn, clientRemoteIP netip
 
 func (ns *Impl) acceptUDP(r *udp.ForwarderRequest) {
 	sess := r.ID()
-	if debugNetstack {
+	if debugNetstack() {
 		ns.logf("[v2] UDP ForwarderRequest: %v", stringifyTEI(sess))
 	}
 	var wq waiter.Queue
@@ -995,7 +995,7 @@ func (ns *Impl) handleMagicDNSUDP(srcAddr netip.AddrPort, c *gonet.UDPConn) {
 // proxy to it directly.
 func (ns *Impl) forwardUDP(client *gonet.UDPConn, wq *waiter.Queue, clientAddr, dstAddr netip.AddrPort) {
 	port, srcPort := dstAddr.Port(), clientAddr.Port()
-	if debugNetstack {
+	if debugNetstack() {
 		ns.logf("[v2] netstack: forwarding incoming UDP connection on port %v", port)
 	}
 
@@ -1071,7 +1071,7 @@ func (ns *Impl) forwardUDP(client *gonet.UDPConn, wq *waiter.Queue, clientAddr,
 }
 
 func startPacketCopy(ctx context.Context, cancel context.CancelFunc, dst net.PacketConn, dstAddr net.Addr, src net.PacketConn, logf logger.Logf, extend func()) {
-	if debugNetstack {
+	if debugNetstack() {
 		logf("[v2] netstack: startPacketCopy to %v (%T) from %T", dstAddr, dst, src)
 	}
 	go func() {
@@ -1096,7 +1096,7 @@ func startPacketCopy(ctx context.Context, cancel context.CancelFunc, dst net.Pac
 					}
 					return
 				}
-				if debugNetstack {
+				if debugNetstack() {
 					logf("[v2] wrote UDP packet %s -> %s", srcAddr, dstAddr)
 				}
 				extend()

wgengine/router/router_linux.go

@@ -316,7 +316,7 @@ func useAmbientCaps() bool {
 	return distro.DSMVersion() >= 7
 }
 
-var forceIPCommand = envknob.Bool("TS_DEBUG_USE_IP_COMMAND")
+var forceIPCommand = envknob.RegisterBool("TS_DEBUG_USE_IP_COMMAND")
 
 // useIPCommand reports whether r should use the "ip" command (or its
 // fake commandRunner for tests) instead of netlink.
@@ -324,7 +324,7 @@ func (r *linuxRouter) useIPCommand() bool {
 	if r.cmd == nil {
 		panic("invalid init")
 	}
-	if forceIPCommand {
+	if forceIPCommand() {
 		return true
 	}
 	// In the future we might need to fall back to using the "ip"

wgengine/userspace.go

@@ -534,7 +534,7 @@ func (e *userspaceEngine) pollResolver() {
 	}
 }
 
-var debugTrimWireguard = envknob.OptBool("TS_DEBUG_TRIM_WIREGUARD")
+var debugTrimWireguard = envknob.RegisterOptBool("TS_DEBUG_TRIM_WIREGUARD")
 
 // forceFullWireguardConfig reports whether we should give wireguard our full
 // network map, even for inactive peers.
@@ -550,7 +550,7 @@ var debugTrimWireguard = envknob.OptBool("TS_DEBUG_TRIM_WIREGUARD")
 // with these knobs in place.
 func forceFullWireguardConfig(numPeers int) bool {
 	// Did the user explicitly enable trimmming via the environment variable knob?
-	if b, ok := debugTrimWireguard.Get(); ok {
+	if b, ok := debugTrimWireguard().Get(); ok {
 		return !b
 	}
 	if opt := controlclient.TrimWGConfig(); opt != "" {