From 932260511e4f9c3561800239f3cab38124df9d8d Mon Sep 17 00:00:00 2001 From: Simon Law Date: Fri, 24 Apr 2026 10:25:10 -0700 Subject: [PATCH] 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 --- cmd/derper/depaware.txt | 1 + cmd/k8s-operator/depaware.txt | 3 +- cmd/tailscale/depaware.txt | 1 + cmd/tailscaled/depaware-min.txt | 1 + cmd/tailscaled/depaware-minbox.txt | 1 + cmd/tailscaled/depaware.txt | 1 + cmd/tailscaled/deps_test.go | 2 +- cmd/tsidp/depaware.txt | 3 +- feature/routecheck/routecheck.go | 15 +++ ipn/ipnlocal/local.go | 35 +++--- ipn/ipnlocal/local_test.go | 4 +- ipn/ipnlocal/node_backend.go | 77 ++++++++---- ipn/ipnlocal/node_backend_test.go | 148 ++++++++++++++++++------ ipn/ipnlocal/routecheck_disabled.go | 12 ++ ipn/ipnlocal/routecheck_enabled.go | 15 +++ net/routecheck/peernode/doc.go | 5 + net/routecheck/peernode/reachability.go | 39 +++++++ net/routecheck/report.go | 14 +++ tsnet/depaware.txt | 3 +- 19 files changed, 306 insertions(+), 74 deletions(-) create mode 100644 ipn/ipnlocal/routecheck_disabled.go create mode 100644 ipn/ipnlocal/routecheck_enabled.go create mode 100644 net/routecheck/peernode/doc.go create mode 100644 net/routecheck/peernode/reachability.go diff --git a/cmd/derper/depaware.txt b/cmd/derper/depaware.txt index 72136901f..9155da0ad 100644 --- a/cmd/derper/depaware.txt +++ b/cmd/derper/depaware.txt @@ -108,6 +108,7 @@ tailscale.com/cmd/derper dependencies: (generated by github.com/tailscale/depawa tailscale.com/net/netutil from tailscale.com/client/local tailscale.com/net/netx from tailscale.com/net/dnscache+ tailscale.com/net/routecheck from tailscale.com/client/local + tailscale.com/net/routecheck/peernode from tailscale.com/net/routecheck tailscale.com/net/sockstats from tailscale.com/derp/derphttp tailscale.com/net/stun from tailscale.com/net/stunserver tailscale.com/net/stunserver from tailscale.com/cmd/derper diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt index fc0ebc42c..1177b46f1 100644 --- a/cmd/k8s-operator/depaware.txt +++ b/cmd/k8s-operator/depaware.txt @@ -807,7 +807,8 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ tailscale.com/net/portmapper from tailscale.com/feature/portmapper tailscale.com/net/portmapper/portmappertype from tailscale.com/net/netcheck+ tailscale.com/net/proxymux from tailscale.com/tsnet - tailscale.com/net/routecheck from tailscale.com/client/local + tailscale.com/net/routecheck from tailscale.com/client/local+ + tailscale.com/net/routecheck/peernode from tailscale.com/ipn/ipnlocal+ 💣 tailscale.com/net/sockopts from tailscale.com/wgengine/magicsock tailscale.com/net/socks5 from tailscale.com/tsnet tailscale.com/net/sockstats from tailscale.com/control/controlclient+ diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 91cbd70b7..0a61b4564 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -236,6 +236,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep tailscale.com/net/portmapper from tailscale.com/feature/portmapper tailscale.com/net/portmapper/portmappertype from tailscale.com/net/netcheck+ tailscale.com/net/routecheck from tailscale.com/client/local+ + tailscale.com/net/routecheck/peernode from tailscale.com/net/routecheck tailscale.com/net/sockstats from tailscale.com/control/controlhttp+ tailscale.com/net/stun from tailscale.com/net/netcheck tailscale.com/net/tlsdial from tailscale.com/cmd/tailscale/cli+ diff --git a/cmd/tailscaled/depaware-min.txt b/cmd/tailscaled/depaware-min.txt index 20ebac51b..83a007b6d 100644 --- a/cmd/tailscaled/depaware-min.txt +++ b/cmd/tailscaled/depaware-min.txt @@ -107,6 +107,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/net/packet/checksum from tailscale.com/net/tstun tailscale.com/net/ping from tailscale.com/net/netcheck+ tailscale.com/net/portmapper/portmappertype from tailscale.com/net/netcheck+ + tailscale.com/net/routecheck/peernode from tailscale.com/ipn/ipnlocal tailscale.com/net/sockopts from tailscale.com/wgengine/magicsock tailscale.com/net/sockstats from tailscale.com/control/controlclient+ tailscale.com/net/stun from tailscale.com/net/netcheck+ diff --git a/cmd/tailscaled/depaware-minbox.txt b/cmd/tailscaled/depaware-minbox.txt index e90289673..2fd6523f3 100644 --- a/cmd/tailscaled/depaware-minbox.txt +++ b/cmd/tailscaled/depaware-minbox.txt @@ -124,6 +124,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/net/packet/checksum from tailscale.com/net/tstun tailscale.com/net/ping from tailscale.com/net/netcheck+ tailscale.com/net/portmapper/portmappertype from tailscale.com/net/netcheck+ + tailscale.com/net/routecheck/peernode from tailscale.com/ipn/ipnlocal tailscale.com/net/sockopts from tailscale.com/wgengine/magicsock tailscale.com/net/sockstats from tailscale.com/control/controlclient+ tailscale.com/net/stun from tailscale.com/net/netcheck+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 92e3406b6..d124e7366 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -380,6 +380,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/net/portmapper/portmappertype from tailscale.com/feature/portmapper+ tailscale.com/net/proxymux from tailscale.com/cmd/tailscaled tailscale.com/net/routecheck from tailscale.com/feature/routecheck+ + tailscale.com/net/routecheck/peernode from tailscale.com/ipn/ipnlocal+ tailscale.com/net/routetable from tailscale.com/doctor/routetable 💣 tailscale.com/net/sockopts from tailscale.com/wgengine/magicsock+ tailscale.com/net/socks5 from tailscale.com/cmd/tailscaled diff --git a/cmd/tailscaled/deps_test.go b/cmd/tailscaled/deps_test.go index e91509765..9f17ea5d6 100644 --- a/cmd/tailscaled/deps_test.go +++ b/cmd/tailscaled/deps_test.go @@ -208,7 +208,7 @@ func TestOmitRouteCheck(t *testing.T) { GOARCH: "amd64", Tags: "ts_omit_routecheck,ts_include_cli", OnDep: func(dep string) { - if strings.Contains(dep, "routecheck") { + if strings.Contains(dep, "routecheck") && !strings.HasSuffix(dep, "/peernode") { t.Errorf("unexpected dep: %q", dep) } }, diff --git a/cmd/tsidp/depaware.txt b/cmd/tsidp/depaware.txt index ad421937d..fa2b87575 100644 --- a/cmd/tsidp/depaware.txt +++ b/cmd/tsidp/depaware.txt @@ -206,7 +206,8 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar tailscale.com/net/portmapper from tailscale.com/feature/portmapper tailscale.com/net/portmapper/portmappertype from tailscale.com/net/netcheck+ tailscale.com/net/proxymux from tailscale.com/tsnet - tailscale.com/net/routecheck from tailscale.com/client/local + tailscale.com/net/routecheck from tailscale.com/client/local+ + tailscale.com/net/routecheck/peernode from tailscale.com/ipn/ipnlocal+ 💣 tailscale.com/net/sockopts from tailscale.com/wgengine/magicsock tailscale.com/net/socks5 from tailscale.com/tsnet tailscale.com/net/sockstats from tailscale.com/control/controlclient+ diff --git a/feature/routecheck/routecheck.go b/feature/routecheck/routecheck.go index 5b0d6a92a..e4682c0d5 100644 --- a/feature/routecheck/routecheck.go +++ b/feature/routecheck/routecheck.go @@ -19,6 +19,7 @@ import ( "sync" "tailscale.com/ipn/ipnext" + "tailscale.com/ipn/ipnlocal" "tailscale.com/net/routecheck" "tailscale.com/tailcfg" "tailscale.com/types/logger" @@ -36,6 +37,8 @@ func init() { backend: b, }, nil }) + + ipnlocal.HookRouteCheckReport.Set(routeCheckReport) } // Extension implements the [ipnext.Extension] interface. @@ -162,3 +165,15 @@ func (e *Extension) reconcileLoop() { } } } + +func routeCheckReport(b *ipnlocal.LocalBackend) ipnlocal.RouteCheckReport { + c := ClientFor(b) + if c == nil { + return nil + } + r := c.Report() + if r == nil { + return nil + } + return r +} diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index cbae9c843..7af449283 100644 --- a/ipn/ipnlocal/local.go +++ b/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()) { diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 34e36fab3..7d8560503 100644 --- a/ipn/ipnlocal/local_test.go +++ b/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) } diff --git a/ipn/ipnlocal/node_backend.go b/ipn/ipnlocal/node_backend.go index 69593d2fd..e703f28ed 100644 --- a/ipn/ipnlocal/node_backend.go +++ b/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 { diff --git a/ipn/ipnlocal/node_backend_test.go b/ipn/ipnlocal/node_backend_test.go index ca61624b8..657aadb14 100644 --- a/ipn/ipnlocal/node_backend_test.go +++ b/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) +} diff --git a/ipn/ipnlocal/routecheck_disabled.go b/ipn/ipnlocal/routecheck_disabled.go new file mode 100644 index 000000000..3a4194651 --- /dev/null +++ b/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 +} diff --git a/ipn/ipnlocal/routecheck_enabled.go b/ipn/ipnlocal/routecheck_enabled.go new file mode 100644 index 000000000..3687ba60f --- /dev/null +++ b/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) +} diff --git a/net/routecheck/peernode/doc.go b/net/routecheck/peernode/doc.go new file mode 100644 index 000000000..fab5d5c6f --- /dev/null +++ b/net/routecheck/peernode/doc.go @@ -0,0 +1,5 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +// Package peernode contains constants that describe a peer node in the routecheck report. +package peernode diff --git a/net/routecheck/peernode/reachability.go b/net/routecheck/peernode/reachability.go new file mode 100644 index 000000000..04052c0e0 --- /dev/null +++ b/net/routecheck/peernode/reachability.go @@ -0,0 +1,39 @@ +// Copyright (c) Tailscale Inc & contributors +// SPDX-License-Identifier: BSD-3-Clause + +package peernode + +import "fmt" + +// Reachability reports whether a peer node is reachable. +type Reachability int + +const ( + // Unknown is a peer node whose reachability is unknown. + // This peer is new and we have yet to probe it. + Unknown Reachability = iota + // Reachable is a peer node that is reachable from this node. + Reachable + // Unreachable is a peer node that is unreachable from this node. + // This peer is not responding to probes. + Unreachable +) + +// String implements [fmt.Stringer]. +func (r Reachability) String() string { + switch r { + case Unknown: + return "Unknown" + case Reachable: + return "Reachable" + case Unreachable: + return "Unreachable" + default: + panic(fmt.Sprintf("unknown %#v", r)) + } +} + +// IsReachable reports if r is [Reachable]. +func (r Reachability) IsReachable() bool { + return r == Reachable +} diff --git a/net/routecheck/report.go b/net/routecheck/report.go index 4d4f859cb..afbe126c3 100644 --- a/net/routecheck/report.go +++ b/net/routecheck/report.go @@ -15,6 +15,7 @@ import ( "github.com/go-json-experiment/json/jsontext" jsonv1 "github.com/go-json-experiment/json/v1" + "tailscale.com/net/routecheck/peernode" "tailscale.com/tailcfg" "tailscale.com/util/clientmetric" "tailscale.com/util/mak" @@ -46,6 +47,19 @@ type Report struct { LastProbed map[tailcfg.NodeID]time.Time `json:"-"` // not marshaled } +// IsReachable reports whether a peer is reachable by the current node. +func (rp Report) IsReachable(id tailcfg.NodeID) peernode.Reachability { + // TODO(sfllaw): We should actually track all routers and consider the + // absence of a router in the report as it being recently added for + // consideration, so it is unknown. Then we should positively track + // whether a node was reachable or not. + _, k := rp.Reachable[id] + if k { + return peernode.Reachable + } + return peernode.Unknown +} + // RoutablePrefixes returns a map of routable network prefixes associated with // each prefix’s routers that were reachable by the current host, // at the time the report was finished. diff --git a/tsnet/depaware.txt b/tsnet/depaware.txt index 13af95a81..ee4c7b748 100644 --- a/tsnet/depaware.txt +++ b/tsnet/depaware.txt @@ -202,7 +202,8 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware) tailscale.com/net/portmapper from tailscale.com/feature/portmapper tailscale.com/net/portmapper/portmappertype from tailscale.com/net/netcheck+ tailscale.com/net/proxymux from tailscale.com/tsnet - tailscale.com/net/routecheck from tailscale.com/client/local + tailscale.com/net/routecheck from tailscale.com/client/local+ + tailscale.com/net/routecheck/peernode from tailscale.com/ipn/ipnlocal+ 💣 tailscale.com/net/sockopts from tailscale.com/wgengine/magicsock tailscale.com/net/socks5 from tailscale.com/tsnet tailscale.com/net/sockstats from tailscale.com/control/controlclient+