wgengine: add Engine.ProbeLocks, drop PeerForIP lock-probe overload

The watchdog (ipn/ipnlocal/watchdog.go) was abusing PeerForIP with an
invalid netip.Addr as a way to acquire and release the engine's
internal locks for deadlock detection. This does the TODO to break it out
into its own method like all the other similarly named methods.

Splitting this out as a prerequisite for a follow-up rewrite of
PeerForIP itself; not having to preserve the lock-probe overload in
the new implementation keeps that follow-up smaller.

Updates #12542
Updates #cleanup

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Change-Id: I25cbffd11aeb65600d9128845404c4918ef88ead

ipn/ipnlocal/state_test.go

@@ -1985,8 +1985,9 @@ func (e *mockEngine) InstallCaptureHook(packet.CaptureCallback) {}
 func (e *mockEngine) SetPeerByIPPacketFunc(func(netip.Addr) (_ key.NodePublic, ok bool)) {}
 func (e *mockEngine) SetPeerSessionStateFunc(func(key.NodePublic, wgengine.PeerWireGuardState)) {
 }
-func (e *mockEngine) SetNetLogNodeSource(netlog.NodeSource) {}
+func (e *mockEngine) SetNetLogNodeSource(netlog.NodeSource)                            {}
 func (e *mockEngine) SetWGPeerLookup(func(wgString string) (tsString string, ok bool)) {}
+func (e *mockEngine) ProbeLocks()                                                      {}
 
 func (e *mockEngine) Close() {
 	e.mu.Lock()

ipn/ipnlocal/watchdog.go

@@ -5,7 +5,6 @@ package ipnlocal
 
 import (
 	"log"
-	"net/netip"
 	"runtime"
 	"time"
 
@@ -117,7 +116,7 @@ func (b *LocalBackend) probeLocks() {
 		dm.ProbeLocks()
 	}
 	if e, ok := sys.Engine.GetOK(); ok && e != nil {
-		e.PeerForIP(netip.Addr{}) // acquires e.mu and e.wgLock
+		e.ProbeLocks()
 	}
 	if nm, ok := sys.NetMon.GetOK(); ok && nm != nil {
 		nm.ProbeLocks()

wgengine/userspace.go

@@ -1557,6 +1557,15 @@ func (e *userspaceEngine) setICMPEchoResponseCallback(idSeq uint32, cb func()) {
 	}
 }
 
+// ProbeLocks implements [Engine.ProbeLocks].
+func (e *userspaceEngine) ProbeLocks() {
+	e.mu.Lock()
+	e.mu.Unlock()
+
+	e.wgLock.Lock()
+	e.wgLock.Unlock()
+}
+
 // PeerForIP returns the Node in the wireguard config
 // that's responsible for handling the given IP address.
 //
@@ -1570,16 +1579,6 @@ func (e *userspaceEngine) PeerForIP(ip netip.Addr) (ret PeerForIP, ok bool) {
 	nm := e.netMap
 	e.mu.Unlock()
 
-	if !ip.IsValid() {
-		// Treat invalid IPs as just a mutex probe to detect deadlocks.
-		// TODO(bradfitz): extend the Engine interface to have an explicit method for
-		// this purpose, instead of overloading PeerForIP with this special case.
-		// But I'd rather do that at the beginning of a dev cycle.
-		e.wgLock.Lock()
-		defer e.wgLock.Unlock()
-		return ret, false
-	}
-
 	if nm == nil {
 		return ret, false
 	}

wgengine/wgengine.go

@@ -197,4 +197,9 @@ type Engine interface {
 	// ipnlocal.LocalBackend.onPeerWireGuardState, installed early in
 	// LocalBackend construction.
 	SetPeerSessionStateFunc(func(key.NodePublic, PeerWireGuardState))
+
+	// ProbeLocks acquires and releases the engine's internal locks so
+	// that [ipnlocal.LocalBackend]'s watchdog can detect deadlocks in
+	// the engine. It is otherwise a no-op.
+	ProbeLocks()
 }