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 <mikeo@tailscale.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user