diff --git a/cmd/tailscaled/netstack.go b/cmd/tailscaled/netstack.go index d896f384f..4e180d36b 100644 --- a/cmd/tailscaled/netstack.go +++ b/cmd/tailscaled/netstack.go @@ -44,11 +44,6 @@ func newNetstack(logf logger.Logf, sys *tsd.System, onlyNetstack bool) (tsd.Nets dialer := sys.Dialer.Get() // must be set by caller already if onlyNetstack { - e := sys.Engine.Get() - dialer.UseNetstackForIP = func(ip netip.Addr) bool { - _, ok := e.PeerForIP(ip) - return ok - } dialer.NetstackDialTCP = func(ctx context.Context, dst netip.AddrPort) (net.Conn, error) { // Note: don't just return ns.DialContextTCP or we'll return // *gonet.TCPConn(nil) instead of a nil interface which trips up diff --git a/cmd/tailscaled/tailscaled.go b/cmd/tailscaled/tailscaled.go index f1f278062..325b66182 100644 --- a/cmd/tailscaled/tailscaled.go +++ b/cmd/tailscaled/tailscaled.go @@ -18,6 +18,7 @@ "log" "net" "net/http" + "net/netip" "os" "os/signal" "path/filepath" @@ -694,6 +695,12 @@ func getLocalBackend(ctx context.Context, logf logger.Logf, logID logid.PublicID if err != nil { return nil, fmt.Errorf("ipnlocal.NewLocalBackend: %w", err) } + if onlyNetstack { + dialer.UseNetstackForIP = func(ip netip.Addr) bool { + _, ok := lb.PeerForIP(ip) + return ok + } + } lb.SetVarRoot(opts.VarRoot) if logPol != nil { lb.SetLogFlusher(logPol.Logtail.StartFlush) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index bf9823cd3..020bb0e84 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -634,7 +634,7 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo e.SetPeerByIPPacketFunc(b.lookupPeerByIP) e.SetPeerConfigFunc(b.peerAllowedIPs) - e.SetPeerForIPFunc(b.peerForIP) + e.SetPeerForIPFunc(b.PeerForIP) e.SetPeerSessionStateFunc(b.onPeerWireGuardState) e.SetNetLogSource(netLogNodeSource{b}) e.SetWGPeerLookup(b.lookupPeerWireGuardString) @@ -8492,7 +8492,7 @@ func (b *LocalBackend) ResetAuth() error { } func (b *LocalBackend) GetPeerEndpointChanges(ctx context.Context, ip netip.Addr) ([]magicsock.EndpointChange, error) { - pip, ok := b.e.PeerForIP(ip) + pip, ok := b.PeerForIP(ip) if !ok { return nil, fmt.Errorf("no matching peer") } diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 41165c68c..df2d71f11 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -9488,14 +9488,13 @@ func TestEnginePeerForIPAdjustsForPrefs(t *testing.T) { nm := buildNetmapWithPeers(selfNode, exitA, exitB, subnetBig, subnetSmall) - var eng wgengine.Engine var curLB *LocalBackend var curT *testing.T // active subtest, for test helpers wantPeer := func(ip string, n tailcfg.NodeView) { t := curT t.Helper() - pip, ok := eng.PeerForIP(netip.MustParseAddr(ip)) + pip, ok := curLB.PeerForIP(netip.MustParseAddr(ip)) if !ok { t.Fatalf("PeerForIP(%s): ok=false, want true", ip) } @@ -9509,7 +9508,7 @@ func TestEnginePeerForIPAdjustsForPrefs(t *testing.T) { wantNotPeer := func(ip string) { t := curT t.Helper() - if _, ok := eng.PeerForIP(netip.MustParseAddr(ip)); ok { + if _, ok := curLB.PeerForIP(netip.MustParseAddr(ip)); ok { t.Fatalf("PeerForIP(%s): ok=true, want false", ip) } } @@ -9534,7 +9533,7 @@ func TestEnginePeerForIPAdjustsForPrefs(t *testing.T) { wantSelf := func(ip string) { t := curT t.Helper() - pip, ok := eng.PeerForIP(netip.MustParseAddr(ip)) + pip, ok := curLB.PeerForIP(netip.MustParseAddr(ip)) if !ok { t.Fatalf("PeerForIP(%s): ok=false, want true", ip) } @@ -9663,7 +9662,6 @@ func TestEnginePeerForIPAdjustsForPrefs(t *testing.T) { LoggedIn: true, }) - eng = lb.sys.Engine.Get() curLB = lb curT = t tt.check() diff --git a/ipn/ipnlocal/resolve.go b/ipn/ipnlocal/resolve.go index e3f6780ff..fb28c622f 100644 --- a/ipn/ipnlocal/resolve.go +++ b/ipn/ipnlocal/resolve.go @@ -118,13 +118,15 @@ func addrFamilyMatch(ip netip.Addr, network string) bool { return true } -// peerForIP returns which peer is responsible for a given IP address. +// PeerForIP returns which peer is responsible for a given IP address. // Despite the name, it can also return the self node (with IsSelf set). // It handles both Tailscale IPs (returning the owning peer or self) and // non-Tailscale addresses like subnet-routed IPs or exit-node global // internet IPs (returning whichever peer would route that traffic). -// It is installed as the [wgengine.Engine.SetPeerForIPFunc] callback. -func (b *LocalBackend) peerForIP(ip netip.Addr) (_ wgengine.PeerForIP, ok bool) { +// It is installed as the [wgengine.Engine.SetPeerForIPFunc] callback, +// serving the engine's internal cold-path lookups (Ping, TSMP, pendopen +// diagnostics). +func (b *LocalBackend) PeerForIP(ip netip.Addr) (_ wgengine.PeerForIP, ok bool) { nb := b.currentNode() if tsaddr.IsTailscaleIP(ip) { diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index 3ae8e7aa5..b40e18853 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -1949,10 +1949,6 @@ func (e *mockEngine) DNSConfig() *dns.Config { return e.dnsCfg } -func (e *mockEngine) PeerForIP(netip.Addr) (_ wgengine.PeerForIP, ok bool) { - return wgengine.PeerForIP{}, false -} - func (e *mockEngine) GetFilter() *filter.Filter { e.mu.Lock() defer e.mu.Unlock() diff --git a/tsnet/tsnet.go b/tsnet/tsnet.go index 50e75417d..5ca840c5b 100644 --- a/tsnet/tsnet.go +++ b/tsnet/tsnet.go @@ -886,7 +886,8 @@ func (s *Server) start() (reterr error) { ns.GetUDPHandlerForFlow = s.getUDPHandlerForFlow s.netstack = ns s.dialer.UseNetstackForIP = func(ip netip.Addr) bool { - _, ok := eng.PeerForIP(ip) + // s.lb is assigned below, before any dials can happen. + _, ok := s.lb.PeerForIP(ip) return ok } s.dialer.NetstackDialTCP = func(ctx context.Context, dst netip.AddrPort) (net.Conn, error) { diff --git a/wgengine/pendopen.go b/wgengine/pendopen.go index 4c830bf68..52055b174 100644 --- a/wgengine/pendopen.go +++ b/wgengine/pendopen.go @@ -138,14 +138,14 @@ func (e *userspaceEngine) isOSNetworkProbe(dst netip.AddrPort) bool { // iOS had log spam like: // open-conn-track: timeout opening (100.115.73.60:52501 => 17.125.252.5:443); no associated peer node if runtime.GOOS == "ios" && dst.Port() == 443 && appleIPRange.Contains(dst.Addr()) { - if _, ok := e.PeerForIP(dst.Addr()); !ok { + if _, ok := e.peerForIP(dst.Addr()); !ok { return true } } // NetworkManager; https://github.com/tailscale/tailscale/issues/13687 // open-conn-track: timeout opening (TCP 100.96.229.119:42798 => 185.125.190.49:80); no associated peer node if runtime.GOOS == "linux" && dst.Port() == 80 && canonicalIPs()(dst.Addr()) { - if _, ok := e.PeerForIP(dst.Addr()); !ok { + if _, ok := e.peerForIP(dst.Addr()); !ok { return true } } @@ -199,7 +199,7 @@ func (e *userspaceEngine) onOpenTimeout(flow flowtrack.Tuple) { } // Diagnose why it might've timed out. - pip, ok := e.PeerForIP(flow.DstAddr()) + pip, ok := e.peerForIP(flow.DstAddr()) if !ok { e.logf("open-conn-track: timeout opening %v; no associated peer node", flow) return diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 79a755c43..009e6fd8f 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -103,10 +103,10 @@ type userspaceEngine struct { wgLock sync.Mutex // serializes all wgdev operations; see lock order comment below - // peerForIP, if non-nil, is the callback installed via - // [userspaceEngine.SetPeerForIPFunc]. PeerForIP delegates to it + // peerForIPFn, if non-nil, is the callback installed via + // [userspaceEngine.SetPeerForIPFunc]. peerForIP delegates to it // for the cold-path control lookups (Ping, TSMP, pendopen, etc). - peerForIP atomic.Pointer[func(netip.Addr) (_ PeerForIP, ok bool)] + peerForIPFn atomic.Pointer[func(netip.Addr) (_ PeerForIP, ok bool)] // peerConfigFn, if non-nil, is the live per-peer allowed-IPs // source installed via [userspaceEngine.SetPeerConfigFunc]. When @@ -605,7 +605,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) pkt := packet.TSMPDiscoKeyAdvertisement{ Key: update.Key, } - peer, ok := e.PeerForIP(update.Src) + peer, ok := e.peerForIP(update.Src) if !ok { e.logf("wgengine: no peer found for %v", update.Src) return @@ -1388,7 +1388,7 @@ func (e *userspaceEngine) UpdateStatus(sb *ipnstate.StatusBuilder) { func (e *userspaceEngine) Ping(ip netip.Addr, pingType tailcfg.PingType, size int, cb func(*ipnstate.PingResult)) { res := &ipnstate.PingResult{IP: ip.String()} - pip, ok := e.PeerForIP(ip) + pip, ok := e.peerForIP(ip) if !ok { e.logf("ping(%v): no matching peer", ip) res.Err = "no matching peer" @@ -1585,24 +1585,24 @@ func (e *userspaceEngine) ProbeLocks() { e.wgLock.Unlock() } -// SetPeerForIPFunc installs the callback used by [userspaceEngine.PeerForIP]. +// SetPeerForIPFunc installs the callback used by [userspaceEngine.peerForIP]. // See [Engine.SetPeerForIPFunc]. func (e *userspaceEngine) SetPeerForIPFunc(fn func(netip.Addr) (PeerForIP, bool)) { if fn == nil { - e.peerForIP.Store(nil) + e.peerForIPFn.Store(nil) return } - e.peerForIP.Store(&fn) + e.peerForIPFn.Store(&fn) } -// PeerForIP returns the node responsible for handling the given IP. -// It delegates to the callback installed via [SetPeerForIPFunc]; engines -// without an installed callback return (zero, false). -func (e *userspaceEngine) PeerForIP(ip netip.Addr) (ret PeerForIP, ok bool) { +// peerForIP returns the node responsible for handling the given IP. +// It delegates to the callback installed via [Engine.SetPeerForIPFunc]; +// engines without an installed callback return (zero, false). +func (e *userspaceEngine) peerForIP(ip netip.Addr) (ret PeerForIP, ok bool) { if !ip.IsValid() { return ret, false } - if fn := e.peerForIP.Load(); fn != nil { + if fn := e.peerForIPFn.Load(); fn != nil { return (*fn)(ip) } return ret, false diff --git a/wgengine/wgengine.go b/wgengine/wgengine.go index 6b68ccfe4..25c1de5cc 100644 --- a/wgengine/wgengine.go +++ b/wgengine/wgengine.go @@ -98,36 +98,18 @@ type Engine interface { // Unlike Reconfig, it does not return ErrNoChanges. ResetAndStop() (*Status, error) - // PeerForIP returns the node to which the provided IP routes, - // if any. If none is found, (zero, false) is returned. + // SetPeerForIPFunc installs the IP-to-node lookup used by the + // engine's internal cold paths (Ping, TSMP, pendopen diagnostics). + // It parallels [Engine.SetPeerByIPPacketFunc] but returns richer + // data (a full NodeView, the matched route prefix, and the IsSelf + // flag). // - // Despite the name, it can return the self node (with - // PeerForIP.IsSelf set). It handles Tailscale IPs, subnet-routed - // IPs, and exit-node global internet IPs, returning whichever - // node would handle that traffic. - // - // This is the cold path used by Ping, TSMP, pendopen diagnostics, - // and debug endpoints. It uses the same underlying data structures - // as the wireguard-go outbound packet path - // ([Engine.SetPeerByIPPacketFunc]), but is slower because it - // returns richer data (a full NodeView, the matched route prefix, - // and the IsSelf flag) requiring extra lookups. - // - // In production, the lookup is implemented by LocalBackend and - // plumbed in via [Engine.SetPeerForIPFunc]; the engine itself holds - // no peer-lookup state on this path. - PeerForIP(netip.Addr) (_ PeerForIP, ok bool) - - // SetPeerForIPFunc installs a callback used by [Engine.PeerForIP]. - // It parallels [Engine.SetPeerByIPPacketFunc] but serves the - // cold-path control lookups (Ping, TSMP, pendopen diagnostics, - // [tsdial.Dialer.UseNetstackForIP], debug endpoints). - // - // If fn is nil, PeerForIP returns (zero, false) for every IP. + // If fn is nil, those lookups fail for every IP. // // LocalBackend installs a func backed by the live nodeBackend for // exact-match and self addresses, with the RouteManager's outbound - // table supplying the subnet-route / exit-node fallback. + // table supplying the subnet-route / exit-node fallback; the engine + // itself holds no peer-lookup state on this path. SetPeerForIPFunc(fn func(netip.Addr) (_ PeerForIP, ok bool)) // GetFilter returns the current packet filter, if any.