ipn/ipnlocal: use routecheck reports to make exit node suggestions

Now that the routecheck subsystem is continuously collecting
reachability reports in the background, we can add a hook to
LocalBackend for fetching its report. That allows
suggestExitNodeUsingTrafficSteering to consult that report when
disqualifying candidates, instead of blocking on an immediate probe.

Exit node suggestions will only consult the report when the
`client-side-reachability` and `client-side-reachability-routecheck`
node attributes are both set on the current node.

Updates #17366
Updates tailscale/corp#33033

Signed-off-by: Simon Law <sfllaw@tailscale.com>

ipn/ipnlocal/local.go

@@ -2530,6 +2530,9 @@ func (b *LocalBackend) UpdateNetmapDelta(muts []netmap.NodeMutation) (handled bo
 		}
 	}
 
+	// TODO(sfllaw): If [buildfeatures.HasRouteCheck] is enabled,
+	// a probe may be triggered for an updated routecheck reachability report,
+	// which should influence which exit nodes are considered valid to suggest.
 	if cn.NetMap() != nil && mutationsAreWorthyOfRecalculatingSuggestedExitNode(muts, cn, b.lastSuggestedExitNode) {
 		// Recompute the suggested exit node
 		b.suggestExitNodeLocked()
@@ -8604,6 +8607,9 @@ func allowedAutoRoute(ipp netip.Prefix) bool {
 	return true
 }
 
+// HookRouteCheckReport is the hook that returns the latest routecheck Report for this LocalBackend.
+var HookRouteCheckReport feature.Hook[func(*LocalBackend) RouteCheckReport]
+
 var ErrNoPreferredDERP = errors.New("no preferred DERP, try again later")
 
 // suggestExitNodeLocked computes a suggestion based on the current netmap and
@@ -8611,11 +8617,15 @@ var ErrNoPreferredDERP = errors.New("no preferred DERP, try again later")
 // be selected at random, so the result is not stable. To be eligible for
 // consideration, the peer must have [tailcfg.NodeAttrSuggestExitNode] in its CapMap.
 //
+// When the client has [tailcfg.NodeAttrClientSideReachabilityRouteCheck] in its CapMap,
+// it will use [HookRouteCheckReport] to get the latest cached routecheck report.
+//
 // b.mu must be held.
 func (b *LocalBackend) suggestExitNodeLocked() (response apitype.ExitNodeSuggestionResponse, err error) {
 	if !buildfeatures.HasUseExitNode {
 		return response, feature.ErrUnavailable
 	}
+
 	mc := b.MagicConn()
 	var preferredDERP int
 	if lastReport := mc.GetLastNetcheckReport(b.ctx); lastReport != nil {
@@ -8628,7 +8638,12 @@ func (b *LocalBackend) suggestExitNodeLocked() (response apitype.ExitNodeSuggest
 	regionLatency := mc.GetDERPRegionLatency()
 	prevSuggestion := b.lastSuggestedExitNode
 
-	res, err := suggestExitNode(preferredDERP, regionLatency, b.currentNode(), prevSuggestion, randomRegion, randomNode, b.getAllowedSuggestions())
+	var rp RouteCheckReport
+	if hook := HookRouteCheckReport.GetOrNil(); hook != nil {
+		rp = hook(b)
+	}
+
+	res, err := suggestExitNode(preferredDERP, regionLatency, rp, b.currentNode(), prevSuggestion, randomRegion, randomNode, b.getAllowedSuggestions())
 	if err != nil {
 		return res, err
 	}
@@ -8710,11 +8725,11 @@ func fillAllowedSuggestions(polc policyclient.Client) (set.Set[tailcfg.StableNod
 // netcheck and are only used by the DERP-based algorithm.
 //
 // Errors are always logged. Suggestions are logged if they defer from prevSuggestion.
-func suggestExitNode(preferredDERP int, regionLatency map[int]time.Duration, nb *nodeBackend, prevSuggestion tailcfg.StableNodeID, selectRegion selectRegionFunc, selectNode selectNodeFunc, allowList set.Set[tailcfg.StableNodeID]) (res apitype.ExitNodeSuggestionResponse, err error) {
+func suggestExitNode(preferredDERP int, regionLatency map[int]time.Duration, rp RouteCheckReport, nb *nodeBackend, prevSuggestion tailcfg.StableNodeID, selectRegion selectRegionFunc, selectNode selectNodeFunc, allowList set.Set[tailcfg.StableNodeID]) (res apitype.ExitNodeSuggestionResponse, err error) {
 	switch {
 	case nb.SelfHasCap(tailcfg.NodeAttrTrafficSteering):
 		// The traffic-steering feature flag is enabled on this tailnet.
-		res, err = suggestExitNodeUsingTrafficSteering(nb, allowList)
+		res, err = suggestExitNodeUsingTrafficSteering(rp, nb, allowList)
 	default:
 		// The control plane will always strip the `traffic-steering`
 		// node attribute if it isn’t enabled for this tailnet, even if
@@ -8747,10 +8762,6 @@ func suggestExitNode(preferredDERP int, regionLatency map[int]time.Duration, nb
 // the lowest latency to this device. For peers without a DERP home, we look for
 // geographic proximity to this device's DERP home.
 func suggestExitNodeUsingDERP(preferredRegionID int, regionLatency map[int]time.Duration, nb *nodeBackend, prevSuggestion tailcfg.StableNodeID, selectRegion selectRegionFunc, selectNode selectNodeFunc, allowList set.Set[tailcfg.StableNodeID]) (res apitype.ExitNodeSuggestionResponse, err error) {
-	// TODO(sfllaw): Context needs to be plumbed down here to support
-	// reachability testing.
-	ctx := context.TODO()
-
 	netMap := nb.NetMap()
 	if preferredRegionID == 0 || netMap == nil || netMap.DERPMap == nil {
 		return res, ErrNoPreferredDERP
@@ -8759,7 +8770,7 @@ func suggestExitNodeUsingDERP(preferredRegionID int, regionLatency map[int]time.
 	// since the netmap doesn't include delta updates (e.g., home DERP or Online
 	// status changes) from the control plane since the last full update.
 	candidates := nb.AppendMatchingPeers(nil, func(peer tailcfg.NodeView) bool {
-		if !peer.Valid() || !nb.PeerIsReachable(ctx, peer) {
+		if !peer.Valid() || !nb.PeerIsReachable(nil, peer) {
 			return false
 		}
 		if allowList != nil && !allowList.Contains(peer.StableID()) {
@@ -8887,11 +8898,7 @@ var ErrNoNetMap = errors.New("no network map, try again later")
 // pick one of the best exit nodes. These priorities are provided by Control in
 // the node’s [tailcfg.Location]. To be eligible for consideration, the node
 // must have NodeAttrSuggestExitNode in its CapMap.
-func suggestExitNodeUsingTrafficSteering(nb *nodeBackend, allowed set.Set[tailcfg.StableNodeID]) (apitype.ExitNodeSuggestionResponse, error) {
-	// TODO(sfllaw): Context needs to be plumbed down here to support
-	// reachability testing.
-	ctx := context.TODO()
-
+func suggestExitNodeUsingTrafficSteering(rp RouteCheckReport, nb *nodeBackend, allowed set.Set[tailcfg.StableNodeID]) (apitype.ExitNodeSuggestionResponse, error) {
 	nm := nb.NetMap()
 	if nm == nil {
 		return apitype.ExitNodeSuggestionResponse{}, ErrNoNetMap
@@ -8910,7 +8917,7 @@ func suggestExitNodeUsingTrafficSteering(nb *nodeBackend, allowed set.Set[tailcf
 		if !p.Valid() {
 			return false
 		}
-		if !nb.PeerIsReachable(ctx, p) {
+		if !nb.PeerIsReachable(rp, p) {
 			return false
 		}
 		if allowed != nil && !allowed.Contains(p.StableID()) {

ipn/ipnlocal/local_test.go

@@ -6062,7 +6062,7 @@ func TestSuggestExitNode(t *testing.T) {
 				regionLatency = tt.lastReport.RegionLatency
 			}
 
-			got, err := suggestExitNode(preferredDERP, regionLatency, nb, tt.lastSuggestion, selectRegion, selectNode, allowList)
+			got, err := suggestExitNode(preferredDERP, regionLatency, nil, nb, tt.lastSuggestion, selectRegion, selectNode, allowList)
 			if got.Name != tt.wantName {
 				t.Errorf("name=%v, want %v", got.Name, tt.wantName)
 			}
@@ -6620,7 +6620,7 @@ func TestSuggestExitNodeTrafficSteering(t *testing.T) {
 			defer nb.shutdown(errShutdown)
 			nb.SetNetMap(tt.netMap)
 
-			got, err := suggestExitNodeUsingTrafficSteering(nb, allowList)
+			got, err := suggestExitNodeUsingTrafficSteering(nil, nb, allowList)
 			if tt.wantErr == nil && err != nil {
 				t.Fatalf("err=%v, want nil", err)
 			}

ipn/ipnlocal/node_backend.go

@@ -18,6 +18,7 @@ import (
 	"tailscale.com/feature/buildfeatures"
 	"tailscale.com/ipn"
 	"tailscale.com/net/dns"
+	"tailscale.com/net/routecheck/peernode"
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/syncs"
 	"tailscale.com/tailcfg"
@@ -448,10 +449,37 @@ func (nb *nodeBackend) PeerAPIBase(p tailcfg.NodeView) string {
 	return peerAPIBase(nm, p)
 }
 
-// PeerIsReachable reports whether the current node can reach p. If the ctx is
-// done, this function may return a result based on stale reachability data.
-func (nb *nodeBackend) PeerIsReachable(ctx context.Context, p tailcfg.NodeView) bool {
-	if !nb.SelfHasCap(tailcfg.NodeAttrClientSideReachability) {
+// RouteCheckReport is an interface that reports whether a peer is reachable by the current node.
+type RouteCheckReport interface {
+	// IsReachable reports whether a peer is reachable by the current node.
+	IsReachable(tailcfg.NodeID) peernode.Reachability
+}
+
+// PeerIsReachable reports whether the current node can reach p.
+// This function may return a result based on stale reachability data,
+// either from the control plane or because the latest routecheck report is old.
+// If rp is nil, then this will report whether p is connected to the control plane
+// according to [tailcfg.NodeView.Online].
+//
+// The latest routecheck report will be considered if the current node has both
+// [tailcfg.NodeAttrClientSideReachability] and [tailcfg.NodeAttrClientSideReachabilityRouteCheck]
+// in its CapMap.
+func (nb *nodeBackend) PeerIsReachable(rp RouteCheckReport, p tailcfg.NodeView) bool {
+	nb.mu.Lock()
+	nm := nb.netMap
+	nb.mu.Unlock()
+
+	if nm == nil || !p.Valid() {
+		// If there is no netmap, then how did we get a NodeView?
+		// Assuming that p came from the control plane,
+		// report whether it was connected to tailcontrol.
+		return p.Valid() && p.Online().Get()
+	}
+
+	self := nm.SelfNode
+	useRouteCheck := isRouteCheckEnabled(self)
+
+	if !useRouteCheck && !self.HasCap(tailcfg.NodeAttrClientSideReachability) {
 		// Legacy behavior is to always trust the control plane, which
 		// isn’t always correct because the peer could be slow to check
 		// in so that control marks it as offline.
@@ -459,27 +487,34 @@ func (nb *nodeBackend) PeerIsReachable(ctx context.Context, p tailcfg.NodeView)
 		return p.Online().Get()
 	}
 
-	nb.mu.Lock()
-	nm := nb.netMap
-	nb.mu.Unlock()
-
-	if self := nm.SelfNode; self.Valid() && self.ID() == p.ID() {
+	if self.Valid() && self.ID() == p.ID() {
 		// This node can always reach itself.
 		return true
 	}
-	return nb.peerIsReachable(ctx, p)
-}
 
-func (nb *nodeBackend) peerIsReachable(ctx context.Context, p tailcfg.NodeView) bool {
-	// TODO(sfllaw): The following does not actually test for client-side
-	// reachability. This would require a mechanism that tracks whether the
-	// current node can actually reach this peer, either because they are
-	// already communicating or because they can ping each other.
-	//
-	// Instead, it makes the client ignore p.Online completely.
-	//
-	// See tailscale/corp#32686.
-	return true
+	if !useRouteCheck && !self.HasCap(tailcfg.NodeAttrClientSideReachabilityRouteCheck) {
+		// TODO(sfllaw): The following does not actually test for client-side
+		// reachability. This would require a mechanism that tracks whether the
+		// current node can actually reach this peer, either because they are
+		// already communicating or because they can ping each other.
+		//
+		// Instead, it makes the client ignore p.Online completely.
+		//
+		// See tailscale/corp#32686.
+		return true
+	}
+
+	if rp == nil {
+		// The routecheck report hasn’t been collected yet,
+		// so fall back and report whether it was connected to tailcontrol.
+		return p.Online().Get()
+	}
+	r := rp.IsReachable(p.ID())
+	if r == peernode.Unknown {
+		// Reachability is unknown, because the node is a new router, so fall back.
+		return p.Online().Get()
+	}
+	return r.IsReachable()
 }
 
 func nodeIP(n tailcfg.NodeView, pred func(netip.Addr) bool) netip.Addr {

ipn/ipnlocal/node_backend_test.go

@@ -6,13 +6,18 @@ package ipnlocal
 import (
 	"context"
 	"errors"
+	"maps"
+	"slices"
 	"testing"
 	"time"
 
+	"tailscale.com/net/routecheck/peernode"
 	"tailscale.com/tailcfg"
 	"tailscale.com/tstest"
 	"tailscale.com/types/netmap"
 	"tailscale.com/util/eventbus"
+	"tailscale.com/util/mak"
+	"tailscale.com/util/set"
 )
 
 func TestNodeBackendReadiness(t *testing.T) {
@@ -134,58 +139,135 @@ func TestNodeBackendReachability(t *testing.T) {
 		//
 		// When disabled, the client relies on the control plane sending
 		// an accurate peer.Online flag. When enabled, the client
-		// ignores peer.Online and determines whether it can reach the
-		// peer node.
+		// ignores peer.Online and is forced to return true.
 		cap bool
+		// rchk sets [tailcfg.NodeAttrClientSideReachabilityRouteCheck]
+		// on the self node.
+		//
+		// When enabled with [tailcfg.NodeAttrClientSideReachability]
+		// above, the client ignores peer.Online and determines whether
+		// it can reach the peer node using [routecheck] reports.
+		rchk bool
 
-		peer tailcfg.Node
-		want bool
+		online bool
+		pong   peernode.Reachability
+		want   bool
 	}{
 		{
-			name: "disabled/offline",
-			cap:  false,
-			peer: tailcfg.Node{
-				Online: new(false),
-			},
-			want: false,
+			name:   "disabled/offline",
+			cap:    false,
+			online: false,
+			want:   false,
 		},
 		{
-			name: "disabled/online",
-			cap:  false,
-			peer: tailcfg.Node{
-				Online: new(true),
-			},
-			want: true,
+			name:   "disabled/online",
+			cap:    false,
+			online: true,
+			want:   true,
 		},
 		{
-			name: "enabled/offline",
-			cap:  true,
-			peer: tailcfg.Node{
-				Online: new(false),
-			},
-			want: true,
+			name:   "forced/offline",
+			cap:    true,
+			rchk:   false,
+			online: false,
+			want:   true,
 		},
 		{
-			name: "enabled/online",
-			cap:  true,
-			peer: tailcfg.Node{
-				Online: new(true),
-			},
-			want: true,
+			name:   "forced/online",
+			cap:    true,
+			rchk:   false,
+			online: true,
+			want:   true,
+		},
+		{
+			name:   "routecheck/offline/needs-probe",
+			cap:    true,
+			rchk:   true,
+			online: false,
+			pong:   peernode.Unknown,
+			want:   false,
+		},
+		{
+			name:   "routecheck/offline/unreachable",
+			cap:    true,
+			rchk:   true,
+			online: false,
+			pong:   peernode.Unreachable,
+			want:   false,
+		},
+		{
+			name:   "routecheck/offline/reachable",
+			cap:    true,
+			rchk:   true,
+			online: false,
+			pong:   peernode.Reachable,
+			want:   true,
+		},
+		{
+			name:   "routecheck/online/needs-probe",
+			cap:    true,
+			rchk:   true,
+			online: true,
+			pong:   peernode.Unknown,
+			want:   true,
+		},
+		{
+			name:   "routecheck/online/unreachable",
+			cap:    true,
+			rchk:   true,
+			online: true,
+			pong:   peernode.Unreachable,
+			want:   false,
+		},
+		{
+			name:   "routecheck/online/reachable",
+			cap:    true,
+			rchk:   true,
+			online: true,
+			pong:   peernode.Reachable,
+			want:   true,
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			nb := newNodeBackend(t.Context(), tstest.WhileTestRunningLogger(t), eventbus.New())
-			nb.netMap = &netmap.NetworkMap{}
+			self := &tailcfg.Node{
+				ID:       1,
+				StableID: "stable1",
+				Name:     "self",
+			}
 			if tc.cap {
-				nb.netMap.AllCaps.Make()
-				nb.netMap.AllCaps.Add(tailcfg.NodeAttrClientSideReachability)
+				mak.Set(&self.CapMap, tailcfg.NodeAttrClientSideReachability, nil)
+			}
+			if tc.rchk {
+				mak.Set(&self.CapMap, tailcfg.NodeAttrClientSideReachabilityRouteCheck, nil)
 			}
 
-			got := nb.PeerIsReachable(t.Context(), tc.peer.View())
+			peer := &tailcfg.Node{
+				ID:       2,
+				StableID: "stable2",
+				Name:     "peer",
+				Online:   &tc.online,
+			}
+
+			nb := newNodeBackend(t.Context(), tstest.WhileTestRunningLogger(t), eventbus.New())
+			nb.netMap = &netmap.NetworkMap{
+				SelfNode: self.View(),
+				Peers:    []tailcfg.NodeView{peer.View()},
+				// HACK: AllCaps is usually populated by Control
+				AllCaps: set.SetOf(slices.Collect(maps.Keys(self.CapMap))),
+			}
+
+			got := nb.PeerIsReachable(routecheckReport(tc.pong), peer.View())
 			if got != tc.want {
 				t.Errorf("got %v, want %v", got, tc.want)
 			}
 		})
 	}
 }
+
+type routecheckReport peernode.Reachability
+
+var _ RouteCheckReport = *new(routecheckReport)
+
+func (rp routecheckReport) IsReachable(_ tailcfg.NodeID) peernode.Reachability {
+	return peernode.Reachability(rp)
+}

ipn/ipnlocal/routecheck_disabled.go

@@ -0,0 +1,12 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build ts_omit_routecheck
+
+package ipnlocal
+
+import "tailscale.com/tailcfg"
+
+func isRouteCheckEnabled(self tailcfg.NodeView) bool {
+	return false
+}

ipn/ipnlocal/routecheck_enabled.go

@@ -0,0 +1,15 @@
+// Copyright (c) Tailscale Inc & contributors
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build !ts_omit_routecheck
+
+package ipnlocal
+
+import (
+	"tailscale.com/net/routecheck"
+	"tailscale.com/tailcfg"
+)
+
+func isRouteCheckEnabled(self tailcfg.NodeView) bool {
+	return routecheck.IsEnabled(self)
+}