wgengine/magicsock: fix buggy fast path in Conn.SetNetworkMap

Spotted by Maisem.

Fixes #6680

Change-Id: I5fdc01de8b006a1c43a2a4848f69397f54b4453a
Co-authored-By: Maisem Ali <maisem@tailscale.com>
Co-authored-By: Andrew Dunham <andrew@tailscale.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
pull/7058/head
Brad Fitzpatrick 2 years ago committed by Brad Fitzpatrick
parent 30380403d0
commit d8feeeee4c

@ -2499,15 +2499,25 @@ func (c *Conn) SetNetworkMap(nm *netmap.NetworkMap) {
return return
} }
if c.netMap != nil && nodesEqual(c.netMap.Peers, nm.Peers) { priorNetmap := c.netMap
return var priorDebug *tailcfg.Debug
if priorNetmap != nil {
priorDebug = priorNetmap.Debug
} }
debugChanged := !reflect.DeepEqual(priorDebug, nm.Debug)
metricNumPeers.Set(int64(len(nm.Peers))) metricNumPeers.Set(int64(len(nm.Peers)))
c.logf("[v1] magicsock: got updated network map; %d peers", len(nm.Peers)) // Update c.netMap regardless, before the following early return.
c.netMap = nm c.netMap = nm
if priorNetmap != nil && nodesEqual(priorNetmap.Peers, nm.Peers) && !debugChanged {
// The rest of this function is all adjusting state for peers that have
// changed. But if the set of peers is equal and the debug flags (for
// silent disco) haven't changed, no need to do anything else.
return
}
c.logf("[v1] magicsock: got updated network map; %d peers", len(nm.Peers))
heartbeatDisabled := debugEnableSilentDisco() || (c.netMap != nil && c.netMap.Debug != nil && c.netMap.Debug.EnableSilentDisco) heartbeatDisabled := debugEnableSilentDisco() || (c.netMap != nil && c.netMap.Debug != nil && c.netMap.Debug.EnableSilentDisco)
// Try a pass of just upserting nodes and creating missing // Try a pass of just upserting nodes and creating missing

@ -1836,3 +1836,21 @@ func TestRebindingUDPConn(t *testing.T) {
c.setConnLocked(realConn.(nettype.PacketConn), "udp4") c.setConnLocked(realConn.(nettype.PacketConn), "udp4")
c.setConnLocked(newBlockForeverConn(), "") c.setConnLocked(newBlockForeverConn(), "")
} }
// https://github.com/tailscale/tailscale/issues/6680: don't ignore
// SetNetworkMap calls when there are no peers. (A too aggressive fast path was
// previously bailing out early, thinking there were no changes since all zero
// peers didn't change, but the netmap has non-peer info in it too we shouldn't discard)
func TestSetNetworkMapWithNoPeers(t *testing.T) {
var c Conn
c.logf = logger.Discard
for i := 1; i <= 3; i++ {
nm := &netmap.NetworkMap{}
c.SetNetworkMap(nm)
t.Logf("ptr %d: %p", i, nm)
if c.netMap != nm {
t.Fatalf("call %d: didn't store netmap", i)
}
}
}

Loading…
Cancel
Save