From b994aba45aad4da33abdce7f72a080ea563cdd19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Claus=20Lensb=C3=B8l?= Date: Wed, 10 Jun 2026 17:40:20 -0400 Subject: [PATCH] wgengine/magicsock,tstest/natlab/vmtest: only send callMeMaybe with endpoints (#20088) (#20089) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 9be21088f42f5a8beca72480cfc3ea56ed1a7343 changed sending disco pings so a callMeMaybe would be not be gated by endpoints existing if the node was running off of a cached netmap. This commit partly reverts that change, but keeps in a few bug fixes in that commit and the tests that was introduced and now skipped. The behaviour prior to 9be21088f42f5a8beca72480cfc3ea56ed1a7343 is retained. Updates #20085 (cherry picked from commit 2690d58e47e0f25385e27569ec0db40931932828) Signed-off-by: Claus Lensbøl --- tstest/natlab/vmtest/vmtest_test.go | 14 ++++++++++++++ wgengine/magicsock/endpoint.go | 9 +-------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/tstest/natlab/vmtest/vmtest_test.go b/tstest/natlab/vmtest/vmtest_test.go index ec6bfd86b..00c1ff58f 100644 --- a/tstest/natlab/vmtest/vmtest_test.go +++ b/tstest/natlab/vmtest/vmtest_test.go @@ -14,6 +14,7 @@ import ( "time" "tailscale.com/client/local" + "tailscale.com/cmd/testwrapper/flakytest" "tailscale.com/ipn" "tailscale.com/net/udprelay/status" "tailscale.com/tailcfg" @@ -1064,9 +1065,17 @@ func TestCachedNetmapAfterRestart(t *testing.T) { // // The test has two modes, pinging from the offline node to the online node, // and pinging from the online node to the offline node. +// +// TODO(cmol): The online -> offline test is flakey for now. The TSMP disco +// key exchange currently relies on disco ping messages being sent. These will +// however not be sent if there is not endpoints on the online node which is +// possible in the event where a node has registered but not finished STUN. func TestDirectConnectionWithCachedNetmapOnOneNode(t *testing.T) { for _, testPingFrom := range []string{"offline", "online"} { t.Run(fmt.Sprintf("ping_from_%s", testPingFrom), func(t *testing.T) { + if testPingFrom == "online" { + flakytest.Mark(t, "https://github.com/tailscale/tailscale/issues/19843") + } env := vmtest.New(t) aNet := env.AddNetwork("1.0.0.1", "192.168.1.1/24", vnet.EasyNAT) @@ -1151,7 +1160,12 @@ func TestDirectConnectionWithCachedNetmapOnOneNode(t *testing.T) { // WireGuard tunnel after both restarted while the control server is // unreachable. After restart one node must use only its on-disk cached // netmaps to re-connect and ping the other node. +// TODO(cmol): The test is flakey for now. The TSMP disco key exchange +// currently relies on disco ping messages being sent. These will however not +// be sent if there is not endpoints on the online node which is possible in +// the event where a node has registered but not finished STUN. func TestDirectConnectionWithCachedNetmapOnTwoNodes(t *testing.T) { + flakytest.Mark(t, "https://github.com/tailscale/tailscale/issues/19843") env := vmtest.New(t) aNet := env.AddNetwork("1.0.0.1", "192.168.1.1/24", vnet.EasyNAT) diff --git a/wgengine/magicsock/endpoint.go b/wgengine/magicsock/endpoint.go index 515b15dc5..fd8fabbe1 100644 --- a/wgengine/magicsock/endpoint.go +++ b/wgengine/magicsock/endpoint.go @@ -1388,19 +1388,12 @@ func (de *endpoint) sendDiscoPingsLocked(now mono.Time, sendCallMeMaybe bool) { de.startDiscoPingLocked(epAddr{ap: ep}, now, pingDiscovery, 0, nil) } derpAddr := de.derpAddr - if sendCallMeMaybe && derpAddr.IsValid() && (sentAny || de.c.usingCachedNetmap.Load()) { + if sentAny && sendCallMeMaybe && derpAddr.IsValid() { // Have our magicsock.Conn figure out its STUN endpoint (if // it doesn't know already) and then send a CallMeMaybe // message to our peer via DERP informing them that we've // sent so our firewall ports are probably open and now // would be a good time for them to connect. - // - // When working off of a cached netmap, send out a CallMeMaybe - // even if we don't know about any peer endpoints. - // Since we cannot rely on control to transfer endpoints for us, - // this makes establishing direct connections more reliable - // as the peer will respond with its own message and initiate - // the connection. go de.c.enqueueCallMeMaybe(derpAddr, de) } }