From 22ff402da93f1487f9de72a413014500a39003cf Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 29 Apr 2026 19:36:30 +0000 Subject: [PATCH] wgengine/magicsock: restore SetDERPMap signature, add SetDERPMapWithoutReSTUN Commit 78627c132f changed the signature of magicsock.Conn.SetDERPMap to take an additional bool doReStun parameter. Avoid both the boolean parameter and the API signature change by restoring SetDERPMap to its original single-argument form and adding a new SetDERPMapWithoutReSTUN method for the cache-loading caller that wants to skip the post-set ReSTUN. Updates #19490 Change-Id: I97d9e82156bfc546ccf59756d1ea52f039b5de06 Signed-off-by: Brad Fitzpatrick --- ipn/ipnlocal/local.go | 6 +++--- wgengine/bench/wg.go | 4 ++-- wgengine/magicsock/derp.go | 21 +++++++++++++++++---- wgengine/magicsock/derp_test.go | 12 ++++++------ wgengine/magicsock/magicsock_test.go | 6 +++--- 5 files changed, 31 insertions(+), 18 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index dbe381cf8..b97c43d59 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -1867,7 +1867,7 @@ func (b *LocalBackend) setControlClientStatusLocked(c controlclient.Client, st c } b.e.SetNetworkMap(st.NetMap) - b.MagicConn().SetDERPMap(st.NetMap.DERPMap, false) + b.MagicConn().SetDERPMapWithoutReSTUN(st.NetMap.DERPMap) if c == nil && st.NetMap.Cached && st.NetMap.SelfNode.Valid() { // Loading from a cached netmap (c == nil means no live control // client). Pre-seed the home DERP from the cached self node so @@ -3445,7 +3445,7 @@ func (b *LocalBackend) DebugForceNetmapUpdate() { nm := b.currentNode().NetMap() b.e.SetNetworkMap(nm) if nm != nil { - b.MagicConn().SetDERPMap(nm.DERPMap, true) + b.MagicConn().SetDERPMap(nm.DERPMap) } b.setNetMapLocked(nm) } @@ -4903,7 +4903,7 @@ func (b *LocalBackend) setPrefsLocked(newp *ipn.Prefs) ipn.PrefsView { } if netMap != nil { - b.MagicConn().SetDERPMap(netMap.DERPMap, true) + b.MagicConn().SetDERPMap(netMap.DERPMap) } if !oldp.WantRunning() && newp.WantRunning && cc != nil { diff --git a/wgengine/bench/wg.go b/wgengine/bench/wg.go index c7decdd91..7b35a089a 100644 --- a/wgengine/bench/wg.go +++ b/wgengine/bench/wg.go @@ -156,8 +156,8 @@ func setupWGTest(b *testing.B, logf logger.Logf, traf *TrafficGen, a1, a2 netip. }) // Not using DERP in this test (for now?). - s1.MagicSock.Get().SetDERPMap(&tailcfg.DERPMap{}, true) - s2.MagicSock.Get().SetDERPMap(&tailcfg.DERPMap{}, true) + s1.MagicSock.Get().SetDERPMap(&tailcfg.DERPMap{}) + s2.MagicSock.Get().SetDERPMap(&tailcfg.DERPMap{}) wait.Wait() } diff --git a/wgengine/magicsock/derp.go b/wgengine/magicsock/derp.go index 30dca2f41..72c75db5a 100644 --- a/wgengine/magicsock/derp.go +++ b/wgengine/magicsock/derp.go @@ -793,10 +793,23 @@ func (c *Conn) SetOnlyTCP443(v bool) { // SetDERPMap controls which (if any) DERP servers are used. // A nil value means to disable DERP; it's disabled by default. // -// If doReStun is false, the post setting ReSTUN is not performed. -// This flag is used for setting the map from a cache to make it possible to -// also set the homeDERP from cache. -func (c *Conn) SetDERPMap(dm *tailcfg.DERPMap, doReStun bool) { +// SetDERPMap triggers a ReSTUN after updating the map. Callers that want to +// set the map without triggering a ReSTUN should use [Conn.SetDERPMapWithoutReSTUN] +// instead. +func (c *Conn) SetDERPMap(dm *tailcfg.DERPMap) { + c.setDERPMap(dm, true) +} + +// SetDERPMapWithoutReSTUN is like [Conn.SetDERPMap] but does not trigger a +// ReSTUN after updating the map. +// +// It is used for setting the map from a cache, so the homeDERP can be set +// from cache before any STUN happens. +func (c *Conn) SetDERPMapWithoutReSTUN(dm *tailcfg.DERPMap) { + c.setDERPMap(dm, false) +} + +func (c *Conn) setDERPMap(dm *tailcfg.DERPMap, doReStun bool) { c.mu.Lock() defer c.mu.Unlock() diff --git a/wgengine/magicsock/derp_test.go b/wgengine/magicsock/derp_test.go index 14b4244f8..c79882d54 100644 --- a/wgengine/magicsock/derp_test.go +++ b/wgengine/magicsock/derp_test.go @@ -116,15 +116,15 @@ func TestSetDERPMapDoReStun(t *testing.T) { // spawning updateEndpoints. c.everHadKey = true - // Should not trigger a ReSTUN. - c.SetDERPMap(derpMap1, false) + // SetDERPMapWithoutReSTUN should not trigger a ReSTUN. + c.SetDERPMapWithoutReSTUN(derpMap1) if reSTUNCalls != 0 { - t.Errorf("SetDERPMap(dm, doReStun=false): got %d ReSTUN calls, want 0", reSTUNCalls) + t.Errorf("SetDERPMapWithoutReSTUN: got %d ReSTUN calls, want 0", reSTUNCalls) } - // doReStun=true: should trigger a ReSTUN. - c.SetDERPMap(derpMap2, true) + // SetDERPMap should trigger a ReSTUN. + c.SetDERPMap(derpMap2) if reSTUNCalls != 1 { - t.Errorf("SetDERPMap(dm, doReStun=true): got %d ReSTUN calls, want 1", reSTUNCalls) + t.Errorf("SetDERPMap: got %d ReSTUN calls, want 1", reSTUNCalls) } } diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index ccc7ba621..df9e57351 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -203,7 +203,7 @@ func newMagicStackWithKey(t testing.TB, logf logger.Logf, ln nettype.PacketListe if err != nil { t.Fatalf("constructing magicsock: %v", err) } - conn.SetDERPMap(derpMap, true) + conn.SetDERPMap(derpMap) if err := conn.SetPrivateKey(privateKey); err != nil { t.Fatalf("setting private key in magicsock: %v", err) } @@ -435,7 +435,7 @@ func TestNewConn(t *testing.T) { t.Fatal("LocalPort returned 0") } - conn.SetDERPMap(stuntest.DERPMapOf(stunAddr.String()), true) + conn.SetDERPMap(stuntest.DERPMapOf(stunAddr.String())) conn.SetPrivateKey(key.NewNode()) go func() { @@ -567,7 +567,7 @@ func TestDERPActiveFuncCalledAfterConnect(t *testing.T) { } defer conn.Close() - conn.SetDERPMap(derpMap, true) + conn.SetDERPMap(derpMap) if err := conn.SetPrivateKey(key.NewNode()); err != nil { t.Fatal(err) }