From 6a709216b9465e716a938de4fc7adb1d4309156f Mon Sep 17 00:00:00 2001 From: Mike O'Driscoll Date: Fri, 5 Jun 2026 13:36:00 -0400 Subject: [PATCH] ipn/ipnlocal,wgengine/magicsock: re-report NetInfo to new control client (#20025) magicsock de-duplicates NetInfo callbacks against c.netInfoLast, a cache that lives on the long-lived magicsock.Conn. That cache survives a control client swap (interactive login or profile switch), where only the control client (and its own per-client NetInfo dedup) is replaced. As a result, the first netcheck after the swap produces a structurally-identical NetInfo (same PreferredDERP, same NAT shape), magicsock suppresses it as unchanged, and the new control session never learns our home DERP. Peers can't reach the node over DERP until some unrelated NetInfo field happens to change. Add Conn.ResetNetInfoLast to clear the dedup cache, and call it from LocalBackend.setControlClientLocked whenever a control client is installed, so the next netcheck re-reports the current NetInfo to the new client. netInfoLast is only a dedup/optimization cache (all readers nil-guard, and it is recomputed by every netcheck), so clearing it can only add a delivery, never lose or misroute one; it is scoped to control-client lifecycle events, not steady-state operation. Updates #17887 Fixes #20024 Signed-off-by: Mike O'Driscoll --- ipn/ipnlocal/local.go | 6 +++ wgengine/magicsock/magicsock.go | 20 ++++++++++ wgengine/magicsock/magicsock_test.go | 56 ++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 7b2b00b50..20a22e29e 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -6696,6 +6696,12 @@ func (b *LocalBackend) setControlClientLocked(cc controlclient.Client) { b.cc = cc b.ccAuto, _ = cc.(*controlclient.Auto) b.ignoreControlClientUpdates.Store(cc == nil) + + // magicsock de-dupes NetInfo against a cache that outlives the control + // client; clear it on (re)install so the next netcheck re-reports our + // NetInfo (notably PreferredDERP, our home DERP) to the new client rather + // than suppressing it as unchanged. + b.MagicConn().ResetNetInfoLast() } // resetControlClientLocked sets b.cc to nil and returns the old value. If the diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index f4f5f94f4..92138cc4e 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -1104,6 +1104,26 @@ func (c *Conn) callNetInfoCallbackLocked(ni *tailcfg.NetInfo) { } } +// ResetNetInfoLast clears the cached NetInfo used to de-duplicate NetInfo +// callbacks, so that the next NetInfo is delivered to the registered callback +// (see [Conn.SetNetInfoCallback]) even if it's structurally identical to the +// previously delivered one. +// +// It must be called whenever the downstream consumer of NetInfo updates is +// replaced, notably when [ipnlocal.LocalBackend] installs a new control client +// after an interactive login or a profile switch. +// +// TODO(tailscale/tailscale#17887): remove once NetInfo updates move to the +// eventbus, where a newly-installed consumer can fetch current state on +// subscribe instead of magicsock exposing this de-dup-cache reset hook. +// +// c.mu must NOT be held. +func (c *Conn) ResetNetInfoLast() { + c.mu.Lock() + defer c.mu.Unlock() + c.netInfoLast = nil +} + // addValidDiscoPathForTest makes addr a validated disco address for // discoKey. It's used in tests to enable receiving of packets from // addr without having to spin up the entire active discovery diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index 322252f2b..394a26210 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -489,6 +489,62 @@ collectEndpoints: } } +// TestResetNetInfoLast verifies that ResetNetInfoLast clears the NetInfo +// de-duplication cache, so the next NetInfo is delivered to the callback even +// when it's structurally unchanged (which callNetInfoCallback would normally +// suppress). This is what lets a node re-advertise its home DERP +// (NetInfo.PreferredDERP) to a freshly-installed control client after an +// interactive login or profile switch; without it, peers can't reach the node +// over DERP until some unrelated NetInfo field changes. +func TestResetNetInfoLast(t *testing.T) { + tstest.PanicOnLog() + + c := newConn(t.Logf) + + got := make(chan *tailcfg.NetInfo, 8) + c.SetNetInfoCallback(func(ni *tailcfg.NetInfo) { + got <- ni + }) + + wantCall := func(why string, wantDERP int) { + t.Helper() + select { + case ni := <-got: + if ni.PreferredDERP != wantDERP { + t.Fatalf("%s: got PreferredDERP=%d, want %d", why, ni.PreferredDERP, wantDERP) + } + case <-time.After(5 * time.Second): + t.Fatalf("%s: timed out waiting for NetInfo callback", why) + } + } + wantNoCall := func(why string) { + t.Helper() + select { + case ni := <-got: + t.Fatalf("%s: unexpected NetInfo callback: %+v", why, ni) + case <-time.After(200 * time.Millisecond): + } + } + + ni := &tailcfg.NetInfo{PreferredDERP: 7, WorkingUDP: "true", LinkType: "wired"} + + // First delivery fires the callback. + c.callNetInfoCallback(ni.Clone()) + wantCall("initial", 7) + + // An identical NetInfo is de-duplicated: no callback. This is exactly the + // behavior that, on a control-client swap, drops the home DERP. + c.callNetInfoCallback(ni.Clone()) + wantNoCall("duplicate suppressed") + + // Simulate a new control client being installed: clearing the de-dup cache + // must let the next (otherwise-identical) NetInfo through, modeling the + // post-login netcheck that re-reports derp-7 to the new session. + c.ResetNetInfoLast() + c.callNetInfoCallback(ni.Clone()) + wantCall("after ResetNetInfoLast", 7) +} + func TestPickDERPFallback(t *testing.T) { tstest.PanicOnLog() tstest.ResourceCheck(t)