control/controlknobs,wgengine/magicsock: implement SilentDisco toggle (#10195)

This change exposes SilentDisco as a control knob, and plumbs it down to
magicsock.endpoint. No changes are being made to magicsock.endpoint
disco behavior, yet.

Updates #540

Signed-off-by: Jordan Whited <jordan@tailscale.com>
Co-authored-by: Brad Fitzpatrick <bradfitz@tailscale.com>

control/controlknobs/controlknobs.go

@@ -52,6 +52,10 @@ type Knobs struct {
 	// DisableDNSForwarderTCPRetries is whether the DNS forwarder should
 	// skip retrying truncated queries over TCP.
 	DisableDNSForwarderTCPRetries atomic.Bool
+
+	// SilentDisco is whether the node should suppress disco heartbeats to its
+	// peers.
+	SilentDisco atomic.Bool
 }
 
 // UpdateFromNodeAttributes updates k (if non-nil) based on the provided self
@@ -74,6 +78,7 @@ func (k *Knobs) UpdateFromNodeAttributes(selfNodeAttrs []tailcfg.NodeCapability,
 		forceBackgroundSTUN           = has(tailcfg.NodeAttrDebugForceBackgroundSTUN)
 		peerMTUEnable                 = has(tailcfg.NodeAttrPeerMTUEnable)
 		dnsForwarderDisableTCPRetries = has(tailcfg.NodeAttrDNSForwarderDisableTCPRetries)
+		silentDisco                   = has(tailcfg.NodeAttrSilentDisco)
 	)
 
 	if has(tailcfg.NodeAttrOneCGNATEnable) {
@@ -91,6 +96,7 @@ func (k *Knobs) UpdateFromNodeAttributes(selfNodeAttrs []tailcfg.NodeCapability,
 	k.DisableDeltaUpdates.Store(disableDeltaUpdates)
 	k.PeerMTUEnable.Store(peerMTUEnable)
 	k.DisableDNSForwarderTCPRetries.Store(dnsForwarderDisableTCPRetries)
+	k.SilentDisco.Store(silentDisco)
 }
 
 // AsDebugJSON returns k as something that can be marshalled with json.Marshal
@@ -109,5 +115,6 @@ func (k *Knobs) AsDebugJSON() map[string]any {
 		"DisableDeltaUpdates":           k.DisableDeltaUpdates.Load(),
 		"PeerMTUEnable":                 k.PeerMTUEnable.Load(),
 		"DisableDNSForwarderTCPRetries": k.DisableDNSForwarderTCPRetries.Load(),
+		"SilentDisco":                   k.SilentDisco.Load(),
 	}
 }

ipn/ipnlocal/local.go

@@ -4372,6 +4372,8 @@ func (b *LocalBackend) setNetMapLocked(nm *netmap.NetworkMap) {
 	}
 	b.capFileSharing = fs
 
+	b.magicConn().SetSilentDisco(b.ControlKnobs().SilentDisco.Load())
+
 	b.setDebugLogsByCapabilityLocked(nm)
 
 	// See the netns package for documentation on what this capability does.

tailcfg/tailcfg.go

@@ -2123,6 +2123,10 @@ const (
 	// fixed port.
 	NodeAttrRandomizeClientPort NodeCapability = "randomize-client-port"
 
+	// NodeAttrSilentDisco makes the client suppress disco heartbeats to its
+	// peers.
+	NodeAttrSilentDisco NodeCapability = "silent-disco"
+
 	// NodeAttrOneCGNATEnable makes the client prefer one big CGNAT /10 route
 	// rather than a /32 per peer. At most one of this or
 	// NodeAttrOneCGNATDisable may be set; if neither are, it's automatic.

wgengine/magicsock/endpoint.go

@@ -440,6 +440,13 @@ func (de *endpoint) heartbeat() {
 	de.heartBeatTimer = time.AfterFunc(heartbeatInterval, de.heartbeat)
 }
 
+// setHeartbeatDisabled sets heartbeatDisabled to the provided value.
+func (de *endpoint) setHeartbeatDisabled(v bool) {
+	de.mu.Lock()
+	defer de.mu.Unlock()
+	de.heartbeatDisabled = v
+}
+
 // wantFullPingLocked reports whether we should ping to all our peers looking for
 // a better path.
 //

wgengine/magicsock/magicsock.go

@@ -139,6 +139,8 @@ type Conn struct {
 	// logging.
 	noV4, noV6 atomic.Bool
 
+	silentDiscoOn atomic.Bool // whether silent disco is enabled
+
 	// noV4Send is whether IPv4 UDP is known to be unable to transmit
 	// at all. This could happen if the socket is in an invalid state
 	// (as can happen on darwin after a network link status change).
@@ -1797,9 +1799,30 @@ type debugFlags struct {
 }
 
 func (c *Conn) debugFlagsLocked() (f debugFlags) {
-	f.heartbeatDisabled = debugEnableSilentDisco() // TODO(bradfitz): controlknobs too, later
+	f.heartbeatDisabled = debugEnableSilentDisco() || c.silentDiscoOn.Load()
+	return
+}
+
+// SetSilentDisco toggles silent disco based on v.
+func (c *Conn) SetSilentDisco(v bool) {
+	old := c.silentDiscoOn.Swap(v)
+	if old == v {
 		return
 	}
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	c.peerMap.forEachEndpoint(func(ep *endpoint) {
+		ep.setHeartbeatDisabled(v)
+	})
+}
+
+// SilentDisco returns true if silent disco is enabled, otherwise false.
+func (c *Conn) SilentDisco() bool {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	flags := c.debugFlagsLocked()
+	return flags.heartbeatDisabled
+}
 
 // SetNetworkMap is called when the control client gets a new network
 // map from the control server. It must always be non-nil.