From e84873692706002db42db5ff07179470c6aa50e6 Mon Sep 17 00:00:00 2001 From: Jordan Whited Date: Mon, 13 Nov 2023 10:05:04 -0800 Subject: [PATCH] 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 Co-authored-by: Brad Fitzpatrick --- control/controlknobs/controlknobs.go | 7 +++++++ ipn/ipnlocal/local.go | 2 ++ tailcfg/tailcfg.go | 4 ++++ wgengine/magicsock/endpoint.go | 7 +++++++ wgengine/magicsock/magicsock.go | 25 ++++++++++++++++++++++++- 5 files changed, 44 insertions(+), 1 deletion(-) diff --git a/control/controlknobs/controlknobs.go b/control/controlknobs/controlknobs.go index e64bc8011..c0ee040b7 100644 --- a/control/controlknobs/controlknobs.go +++ b/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(), } } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index cc2131a66..99dde77f2 100644 --- a/ipn/ipnlocal/local.go +++ b/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. diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 113d94b61..2f10d0e47 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -2123,6 +2123,10 @@ type Oauth2Token struct { // 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. diff --git a/wgengine/magicsock/endpoint.go b/wgengine/magicsock/endpoint.go index 79c129a29..9d052c59e 100644 --- a/wgengine/magicsock/endpoint.go +++ b/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. // diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 5f1288e0e..2bf807973 100644 --- a/wgengine/magicsock/magicsock.go +++ b/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,10 +1799,31 @@ 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. //