Browse Source

wgengine/magicsock: add client flag and envknob to disable heartbeat (#5638)

Baby steps towards turning off heartbeat pings entirely as per #540.
This doesn't change any current magicsock functionality and requires additional
changes to send/disco paths before the flag can be turned on.

Updates #540

Change-Id: Idc9a72748e74145b068d67e6dd4a4ffe3932efd0
Signed-off-by: Jenny Zhang <[email protected]>

Signed-off-by: Jenny Zhang <[email protected]>
phirework 3 years ago
parent
commit
5c42990c2f

+ 4 - 0
tailcfg/tailcfg.go

@@ -1456,6 +1456,10 @@ type Debug struct {
 	// re-enabled for the lifetime of the process.
 	DisableLogTail bool `json:",omitempty"`
 
+	// EnableSilentDisco disables the use of heartBeatTimer in magicsock and attempts to
+	// handle disco silently. See issue #540 for details.
+	EnableSilentDisco bool `json:",omitempty"`
+
 	// Exit optionally specifies that the client should os.Exit
 	// with this code.
 	Exit *int `json:",omitempty"`

+ 3 - 0
wgengine/magicsock/debugknobs.go

@@ -35,6 +35,9 @@ var (
 	debugReSTUNStopOnIdle = envknob.RegisterBool("TS_DEBUG_RESTUN_STOP_ON_IDLE")
 	// debugAlwaysDERP disables the use of UDP, forcing all peer communication over DERP.
 	debugAlwaysDERP = envknob.RegisterBool("TS_DEBUG_ALWAYS_USE_DERP")
+	// debugEnableSilentDisco disables the use of heartbeatTimer on the endpoint struct
+	// and attempts to handle disco silently. See issue #540 for details.
+	debugEnableSilentDisco = envknob.RegisterBool("TS_DEBUG_ENABLE_SILENT_DISCO")
 )
 
 // inTest reports whether the running program is a test that set the

+ 1 - 0
wgengine/magicsock/debugknobs_stubs.go

@@ -18,6 +18,7 @@ func debugOmitLocalAddresses() bool { return false }
 func logDerpVerbose() bool          { return false }
 func debugReSTUNStopOnIdle() bool   { return false }
 func debugAlwaysDERP() bool         { return false }
+func debugEnableSilentDisco() bool  { return false }
 func debugUseDerpRouteEnv() string  { return "" }
 func debugUseDerpRoute() opt.Bool   { return "" }
 

+ 16 - 5
wgengine/magicsock/magicsock.go

@@ -2356,6 +2356,8 @@ func (c *Conn) SetNetworkMap(nm *netmap.NetworkMap) {
 	}
 	c.netMap = nm
 
+	heartbeatDisabled := debugEnableSilentDisco() || (c.netMap != nil && c.netMap.Debug != nil && c.netMap.Debug.EnableSilentDisco)
+
 	// Try a pass of just upserting nodes and creating missing
 	// endpoints. If the set of nodes is the same, this is an
 	// efficient alloc-free update. If the set of nodes is different,
@@ -2364,16 +2366,18 @@ func (c *Conn) SetNetworkMap(nm *netmap.NetworkMap) {
 	for _, n := range nm.Peers {
 		if ep, ok := c.peerMap.endpointForNodeKey(n.Key); ok {
 			oldDiscoKey := ep.discoKey
+			ep.heartbeatDisabled = heartbeatDisabled
 			ep.updateFromNode(n)
 			c.peerMap.upsertEndpoint(ep, oldDiscoKey) // maybe update discokey mappings in peerMap
 			continue
 		}
 
 		ep := &endpoint{
-			c:             c,
-			publicKey:     n.Key,
-			sentPing:      map[stun.TxID]sentPing{},
-			endpointState: map[netip.AddrPort]*endpointState{},
+			c:                 c,
+			publicKey:         n.Key,
+			sentPing:          map[stun.TxID]sentPing{},
+			endpointState:     map[netip.AddrPort]*endpointState{},
+			heartbeatDisabled: heartbeatDisabled,
 		}
 		if !n.DiscoKey.IsZero() {
 			ep.discoKey = n.DiscoKey
@@ -3307,6 +3311,8 @@ type endpoint struct {
 	isCallMeMaybeEP    map[netip.AddrPort]bool
 
 	pendingCLIPings []pendingCLIPing // any outstanding "tailscale ping" commands running
+
+	heartbeatDisabled bool // heartBeatTimer disabled for silent disco. See issue #540.
 }
 
 type pendingCLIPing struct {
@@ -3502,6 +3508,11 @@ func (de *endpoint) heartbeat() {
 
 	de.heartBeatTimer = nil
 
+	if de.heartbeatDisabled {
+		// If control override to disable heartBeatTimer set, return early.
+		return
+	}
+
 	if !de.canP2P() {
 		// Cannot form p2p connections, no heartbeating necessary.
 		return
@@ -3560,7 +3571,7 @@ func (de *endpoint) wantFullPingLocked(now mono.Time) bool {
 
 func (de *endpoint) noteActiveLocked() {
 	de.lastSend = mono.Now()
-	if de.heartBeatTimer == nil && de.canP2P() {
+	if de.heartBeatTimer == nil && de.canP2P() && !de.heartbeatDisabled {
 		de.heartBeatTimer = time.AfterFunc(heartbeatInterval, de.heartbeat)
 	}
 }