From fe009c134e5b6a0fe91a002122ebfb821e772895 Mon Sep 17 00:00:00 2001
From: Andrew Dunham <andrew@du.nham.ca>
Date: Tue, 30 Apr 2024 15:09:12 -0400
Subject: [PATCH] ipn/ipnlocal: reset the dialPlan only when the URL is
 unchanged

Also, reset it in a few more places (e.g. logout, new blank profiles,
etc.) to avoid a few more cases where a pre-existing dialPlan can cause
a new Headscale server take 10+ seconds to connect.

Updates #11938

Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I3095173a5a3d9720507afe4452548491e9e45a3e
---
 ipn/ipnlocal/local.go | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index 83b9426e7..2605db961 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -4594,6 +4594,7 @@ func (b *LocalBackend) ResetForClientDisconnect() {
 	b.authURLSticky = ""
 	b.authURLTime = time.Time{}
 	b.activeLogin = ""
+	b.resetDialPlan()
 	b.setAtomicValuesFromPrefsLocked(ipn.PrefsView{})
 	b.enterStateLockedOnEntry(ipn.Stopped, unlock)
 }
@@ -4675,7 +4676,7 @@ func (b *LocalBackend) Logout(ctx context.Context) error {
 	// b.mu is now unlocked, after editPrefsLockedOnEntry.
 
 	// Clear any previous dial plan(s), if set.
-	b.dialPlan.Store(nil)
+	b.resetDialPlan()
 
 	if cc == nil {
 		// Double Logout can happen via repeated IPN
@@ -5878,13 +5879,17 @@ func (b *LocalBackend) SwitchProfile(profile ipn.ProfileID) error {
 	unlock := b.lockAndGetUnlock()
 	defer unlock()
 
+	oldControlURL := b.pm.CurrentPrefs().ControlURLOrDefault()
 	if err := b.pm.SwitchProfile(profile); err != nil {
 		return err
 	}
 
-	// TODO: Check if serverUrl in the switched profile is actually changed;
-	// if not, then no need to reset dial plan
-	b.dialPlan.Store(nil)
+	// As an optimization, only reset the dialPlan if the control URL
+	// changed; we treat an empty URL as "unknown" and always reset.
+	newControlURL := b.pm.CurrentPrefs().ControlURLOrDefault()
+	if oldControlURL != newControlURL || oldControlURL == "" || newControlURL == "" {
+		b.resetDialPlan()
+	}
 
 	return b.resetForProfileChangeLockedOnEntry(unlock)
 }
@@ -5936,6 +5941,17 @@ func (b *LocalBackend) initTKALocked() error {
 	return nil
 }
 
+// resetDialPlan resets the dialPlan for this LocalBackend. It will log if
+// anything is reset.
+//
+// It is safe to call this concurrently, with or without b.mu held.
+func (b *LocalBackend) resetDialPlan() {
+	old := b.dialPlan.Swap(nil)
+	if old != nil {
+		b.logf("resetDialPlan: did reset")
+	}
+}
+
 // resetForProfileChangeLockedOnEntry resets the backend for a profile change.
 //
 // b.mu must held on entry. It is released on exit.
@@ -5994,6 +6010,11 @@ func (b *LocalBackend) NewProfile() error {
 	defer unlock()
 
 	b.pm.NewProfile()
+
+	// The new profile doesn't yet have a ControlURL because it hasn't been
+	// set. Conservatively reset the dialPlan.
+	b.resetDialPlan()
+
 	return b.resetForProfileChangeLockedOnEntry(unlock)
 }
 
@@ -6022,6 +6043,7 @@ func (b *LocalBackend) ResetAuth() error {
 	if err := b.pm.DeleteAllProfiles(); err != nil {
 		return err
 	}
+	b.resetDialPlan() // always reset if we're removing everything
 	return b.resetForProfileChangeLockedOnEntry(unlock)
 }