ipn/ipnlocal: consider all DERP regions for exit node recommendations
When recommending an exit node, suggestExitNodeLocked ranks candidates by the latency to their home DERP region, taken from the most recent netcheck report. But netcheck alternates between full reports, which probe every region, and incremental reports, which only re-probe the home region and a handful of the fastest regions. When the most recent report is incremental, the suggestion fell back to a random for exit nodes that are far away. Now we rank candidates against the best recent latency, tracked by the `netcheck.Client` - the same data that is used to pick the preferred DERP. It uses a history of measurements which includes a full netcheck report, so should cover all DERP regions. Updates tailscale/corp#17516 Signed-off-by: Anton Tolchanov <anton@tailscale.com>
This commit is contained in:
committed by
Anton Tolchanov
parent
6a275c01db
commit
f442cda999
+33
-20
@@ -58,7 +58,6 @@ import (
|
||||
"tailscale.com/net/dnscache"
|
||||
"tailscale.com/net/dnsfallback"
|
||||
"tailscale.com/net/ipset"
|
||||
"tailscale.com/net/netcheck"
|
||||
"tailscale.com/net/netkernelconf"
|
||||
"tailscale.com/net/netmon"
|
||||
"tailscale.com/net/netns"
|
||||
@@ -8643,10 +8642,19 @@ func (b *LocalBackend) suggestExitNodeLocked() (response apitype.ExitNodeSuggest
|
||||
if !buildfeatures.HasUseExitNode {
|
||||
return response, feature.ErrUnavailable
|
||||
}
|
||||
lastReport := b.MagicConn().GetLastNetcheckReport(b.ctx)
|
||||
mc := b.MagicConn()
|
||||
var preferredDERP int
|
||||
if lastReport := mc.GetLastNetcheckReport(b.ctx); lastReport != nil {
|
||||
preferredDERP = lastReport.PreferredDERP
|
||||
}
|
||||
// Use netcheck's recent per-region latency rather than only the latest report:
|
||||
// the latest report may be an incremental netcheck that didn't re-probe the
|
||||
// regions where far-away candidate exit nodes live, which would otherwise force
|
||||
// a random choice.
|
||||
regionLatency := mc.GetDERPRegionLatency()
|
||||
prevSuggestion := b.lastSuggestedExitNode
|
||||
|
||||
res, err := suggestExitNode(lastReport, b.currentNode(), prevSuggestion, randomRegion, randomNode, b.getAllowedSuggestions())
|
||||
res, err := suggestExitNode(preferredDERP, regionLatency, b.currentNode(), prevSuggestion, randomRegion, randomNode, b.getAllowedSuggestions())
|
||||
if err != nil {
|
||||
return res, err
|
||||
}
|
||||
@@ -8718,7 +8726,11 @@ func fillAllowedSuggestions(polc policyclient.Client) (set.Set[tailcfg.StableNod
|
||||
|
||||
// suggestExitNode returns a suggestion for reasonably good exit node based on
|
||||
// the current netmap and the previous suggestion.
|
||||
func suggestExitNode(report *netcheck.Report, nb *nodeBackend, prevSuggestion tailcfg.StableNodeID, selectRegion selectRegionFunc, selectNode selectNodeFunc, allowList set.Set[tailcfg.StableNodeID]) (res apitype.ExitNodeSuggestionResponse, err error) {
|
||||
//
|
||||
// preferredDERP is this device's home DERP region (0 if unknown) and
|
||||
// regionLatency is the best recent latency to each DERP region; both come from
|
||||
// netcheck and are only used by the DERP-based algorithm.
|
||||
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) {
|
||||
switch {
|
||||
case nb.SelfHasCap(tailcfg.NodeAttrTrafficSteering):
|
||||
// The traffic-steering feature flag is enabled on this tailnet.
|
||||
@@ -8727,7 +8739,7 @@ func suggestExitNode(report *netcheck.Report, nb *nodeBackend, prevSuggestion ta
|
||||
// The control plane will always strip the `traffic-steering`
|
||||
// node attribute if it isn’t enabled for this tailnet, even if
|
||||
// it is set in the policy file: tailscale/corp#34401
|
||||
res, err = suggestExitNodeUsingDERP(report, nb, prevSuggestion, selectRegion, selectNode, allowList)
|
||||
res, err = suggestExitNodeUsingDERP(preferredDERP, regionLatency, nb, prevSuggestion, selectRegion, selectNode, allowList)
|
||||
}
|
||||
if err != nil {
|
||||
nb.logf("netmap: suggested exit node: %v", err)
|
||||
@@ -8742,22 +8754,23 @@ func suggestExitNode(report *netcheck.Report, nb *nodeBackend, prevSuggestion ta
|
||||
// before traffic steering was implemented. This handles the plain failover
|
||||
// case, in addition to the optional Regional Routing.
|
||||
//
|
||||
// It computes a suggestion based on the current netmap and last netcheck
|
||||
// report. If there are multiple equally good options, one is selected at
|
||||
// random, so the result is not stable. To be eligible for consideration, the
|
||||
// peer must have NodeAttrSuggestExitNode in its CapMap.
|
||||
// It computes a suggestion based on the current netmap, this device's preferred
|
||||
// DERP region, and the recent measured latency to each DERP region (regionLatency).
|
||||
// If there are multiple equally good options, one is selected at random, so the
|
||||
// result is not stable. To be eligible for consideration, the peer must have
|
||||
// NodeAttrSuggestExitNode in its CapMap.
|
||||
//
|
||||
// Currently, peers with a DERP home are preferred over those without (typically
|
||||
// this means Mullvad). Peers are selected based on having a DERP home that is
|
||||
// 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(report *netcheck.Report, nb *nodeBackend, prevSuggestion tailcfg.StableNodeID, selectRegion selectRegionFunc, selectNode selectNodeFunc, allowList set.Set[tailcfg.StableNodeID]) (res apitype.ExitNodeSuggestionResponse, err error) {
|
||||
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 report == nil || report.PreferredDERP == 0 || netMap == nil || netMap.DERPMap == nil {
|
||||
if preferredRegionID == 0 || netMap == nil || netMap.DERPMap == nil {
|
||||
return res, ErrNoPreferredDERP
|
||||
}
|
||||
// Use [nodeBackend.AppendMatchingPeers] instead of the netmap directly,
|
||||
@@ -8788,7 +8801,7 @@ func suggestExitNodeUsingDERP(report *netcheck.Report, nb *nodeBackend, prevSugg
|
||||
}
|
||||
|
||||
candidatesByRegion := make(map[int][]tailcfg.NodeView, len(netMap.DERPMap.Regions))
|
||||
preferredDERP, ok := netMap.DERPMap.Regions[report.PreferredDERP]
|
||||
preferredDERP, ok := netMap.DERPMap.Regions[preferredRegionID]
|
||||
if !ok {
|
||||
return res, ErrNoPreferredDERP
|
||||
}
|
||||
@@ -8824,10 +8837,10 @@ func suggestExitNodeUsingDERP(report *netcheck.Report, nb *nodeBackend, prevSugg
|
||||
}
|
||||
distances = append(distances, nodeDistance{nv: c, distance: distance})
|
||||
}
|
||||
// First, try to select an exit node that has the closest DERP home, based on lastReport's DERP latency.
|
||||
// First, try to select an exit node that has the closest DERP home, based on recent DERP latency.
|
||||
// If there are no latency values, it returns an arbitrary region
|
||||
if len(candidatesByRegion) > 0 {
|
||||
minRegion := minLatencyDERPRegion(slicesx.MapKeys(candidatesByRegion), report)
|
||||
minRegion := minLatencyDERPRegion(slicesx.MapKeys(candidatesByRegion), regionLatency)
|
||||
if minRegion == 0 {
|
||||
minRegion = selectRegion(views.SliceOf(slicesx.MapKeys(candidatesByRegion)))
|
||||
}
|
||||
@@ -9016,16 +9029,16 @@ func randomNode(nodes views.Slice[tailcfg.NodeView], prefer tailcfg.StableNodeID
|
||||
return nodes.At(rand.IntN(nodes.Len()))
|
||||
}
|
||||
|
||||
// minLatencyDERPRegion returns the region with the lowest latency value given the last netcheck report.
|
||||
// If there are no latency values, it returns 0.
|
||||
func minLatencyDERPRegion(regions []int, report *netcheck.Report) int {
|
||||
// minLatencyDERPRegion returns the region with the lowest latency value given
|
||||
// the per-region latency map. If there are no latency values, it returns 0.
|
||||
func minLatencyDERPRegion(regions []int, regionLatency map[int]time.Duration) int {
|
||||
min := slices.MinFunc(regions, func(i, j int) int {
|
||||
const largeDuration time.Duration = math.MaxInt64
|
||||
iLatency, ok := report.RegionLatency[i]
|
||||
iLatency, ok := regionLatency[i]
|
||||
if !ok {
|
||||
iLatency = largeDuration
|
||||
}
|
||||
jLatency, ok := report.RegionLatency[j]
|
||||
jLatency, ok := regionLatency[j]
|
||||
if !ok {
|
||||
jLatency = largeDuration
|
||||
}
|
||||
@@ -9034,7 +9047,7 @@ func minLatencyDERPRegion(regions []int, report *netcheck.Report) int {
|
||||
}
|
||||
return cmp.Compare(i, j)
|
||||
})
|
||||
latency, ok := report.RegionLatency[min]
|
||||
latency, ok := regionLatency[min]
|
||||
if !ok || latency == 0 {
|
||||
return 0
|
||||
} else {
|
||||
|
||||
+113
-26
@@ -1421,9 +1421,11 @@ func TestConfigureExitNode(t *testing.T) {
|
||||
lb := newTestLocalBackendWithSys(t, sys)
|
||||
lb.SetPrefsForTest(tt.prefs.Clone())
|
||||
|
||||
// Then set the netcheck report and netmap, if any.
|
||||
// Then set the netcheck report and netmap, if any. Clone the shared
|
||||
// report because AddNetcheckReportForTest mutates it and subtests run
|
||||
// in parallel.
|
||||
if tt.report != nil {
|
||||
lb.MagicConn().SetLastNetcheckReportForTest(t.Context(), tt.report)
|
||||
lb.MagicConn().AddNetcheckReportForTest(clientNetmap.DERPMap, tt.report, time.Now())
|
||||
}
|
||||
if tt.netMap != nil {
|
||||
lb.SetControlClientStatus(lb.cc, controlclient.Status{NetMap: tt.netMap})
|
||||
@@ -1681,7 +1683,7 @@ func TestExitNodeNotifyOrder(t *testing.T) {
|
||||
clientNetmap := buildNetmapWithPeers(selfNode, exitNode1, exitNode2)
|
||||
|
||||
lb := newTestLocalBackend(t)
|
||||
lb.sys.MagicSock.Get().SetLastNetcheckReportForTest(lb.ctx, report)
|
||||
lb.sys.MagicSock.Get().AddNetcheckReportForTest(clientNetmap.DERPMap, report, time.Now())
|
||||
lb.SetPrefsForTest(&ipn.Prefs{
|
||||
ControlURL: controlURL,
|
||||
AutoExitNode: ipn.AnyExitNode,
|
||||
@@ -4090,7 +4092,7 @@ func TestUpdateNetmapDeltaAutoExitNode(t *testing.T) {
|
||||
b := newTestLocalBackendWithSys(t, sys)
|
||||
b.currentNode().SetNetMap(tt.netmap)
|
||||
b.lastSuggestedExitNode = tt.lastSuggestedExitNode
|
||||
b.sys.MagicSock.Get().SetLastNetcheckReportForTest(b.ctx, tt.report)
|
||||
b.sys.MagicSock.Get().AddNetcheckReportForTest(derpMap, tt.report, time.Now())
|
||||
b.SetPrefsForTest(b.pm.CurrentPrefs().AsStruct())
|
||||
|
||||
allDone := make(chan bool, 1)
|
||||
@@ -4224,14 +4226,14 @@ func TestAutoExitNodeSetNetInfoCallback(t *testing.T) {
|
||||
t.Errorf("got initial exit node %v, want %v", eid, peer1.StableID())
|
||||
}
|
||||
b.refreshAutoExitNode = true
|
||||
b.sys.MagicSock.Get().SetLastNetcheckReportForTest(b.ctx, &netcheck.Report{
|
||||
b.sys.MagicSock.Get().AddNetcheckReportForTest(defaultDERPMap, &netcheck.Report{
|
||||
RegionLatency: map[int]time.Duration{
|
||||
1: 10 * time.Millisecond,
|
||||
2: 5 * time.Millisecond,
|
||||
3: 30 * time.Millisecond,
|
||||
},
|
||||
PreferredDERP: 2,
|
||||
})
|
||||
}, time.Now())
|
||||
b.setNetInfo(&ni)
|
||||
if eid := b.Prefs().ExitNodeID(); eid != peer2.StableID() {
|
||||
t.Errorf("got final exit node %v, want %v", eid, peer2.StableID())
|
||||
@@ -4288,7 +4290,7 @@ func TestSetControlClientStatusAutoExitNode(t *testing.T) {
|
||||
// Peer 2 should be the initial exit node, as it's better than peer 1
|
||||
// in terms of latency and DERP region.
|
||||
b.lastSuggestedExitNode = peer2.StableID()
|
||||
b.sys.MagicSock.Get().SetLastNetcheckReportForTest(b.ctx, report)
|
||||
b.sys.MagicSock.Get().AddNetcheckReportForTest(derpMap, report, time.Now())
|
||||
b.SetPrefsForTest(b.pm.CurrentPrefs().AsStruct())
|
||||
offlinePeer2 := makePeer(2, withCap(26), withSuggest(), withExitRoutes(), withOnline(false), withNodeKey())
|
||||
updatedNetmap := &netmap.NetworkMap{
|
||||
@@ -6058,7 +6060,14 @@ func TestSuggestExitNode(t *testing.T) {
|
||||
defer nb.shutdown(errShutdown)
|
||||
nb.SetNetMap(tt.netMap)
|
||||
|
||||
got, err := suggestExitNode(tt.lastReport, nb, tt.lastSuggestion, selectRegion, selectNode, allowList)
|
||||
var preferredDERP int
|
||||
var regionLatency map[int]time.Duration
|
||||
if tt.lastReport != nil {
|
||||
preferredDERP = tt.lastReport.PreferredDERP
|
||||
regionLatency = tt.lastReport.RegionLatency
|
||||
}
|
||||
|
||||
got, err := suggestExitNode(preferredDERP, regionLatency, nb, tt.lastSuggestion, selectRegion, selectNode, allowList)
|
||||
if got.Name != tt.wantName {
|
||||
t.Errorf("name=%v, want %v", got.Name, tt.wantName)
|
||||
}
|
||||
@@ -6078,6 +6087,89 @@ func TestSuggestExitNode(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestSuggestExitNodeUsesRecentDERPLatency exercises DERP latency-based exit node
|
||||
// suggestion when the most recent netcheck report is incremental.
|
||||
func TestSuggestExitNodeUsesRecentDERPLatency(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
derpMap := &tailcfg.DERPMap{
|
||||
Regions: map[int]*tailcfg.DERPRegion{
|
||||
1: {Nodes: []*tailcfg.DERPNode{{Name: "1a", RegionID: 1}}},
|
||||
2: {Nodes: []*tailcfg.DERPNode{{Name: "2a", RegionID: 2}}},
|
||||
3: {Nodes: []*tailcfg.DERPNode{{Name: "3a", RegionID: 3}}},
|
||||
4: {Nodes: []*tailcfg.DERPNode{{Name: "4a", RegionID: 4}}},
|
||||
5: {Nodes: []*tailcfg.DERPNode{{Name: "5a", RegionID: 5}}},
|
||||
},
|
||||
}
|
||||
|
||||
// Two candidate exit nodes, each homed in a far region (4 and 5) that the most
|
||||
// recent (incremental) netcheck won't re-probe.
|
||||
exitRegion4 := makePeer(4, withDERP(4), withExitRoutes(), withSuggest())
|
||||
exitRegion5 := makePeer(5, withDERP(5), withExitRoutes(), withSuggest())
|
||||
|
||||
selfNode := tailcfg.Node{
|
||||
Addresses: []netip.Prefix{netip.MustParsePrefix("100.64.1.1/32")},
|
||||
}
|
||||
netMap := &netmap.NetworkMap{
|
||||
SelfNode: selfNode.View(),
|
||||
DERPMap: derpMap,
|
||||
Peers: []tailcfg.NodeView{exitRegion4, exitRegion5},
|
||||
}
|
||||
|
||||
// A full netcheck measured every region: region 4 (100ms) is closer than
|
||||
// region 5 (200ms).
|
||||
fullReport := &netcheck.Report{
|
||||
PreferredDERP: 1,
|
||||
RegionLatency: map[int]time.Duration{
|
||||
1: 10 * time.Millisecond,
|
||||
2: 20 * time.Millisecond,
|
||||
3: 30 * time.Millisecond,
|
||||
4: 100 * time.Millisecond,
|
||||
5: 200 * time.Millisecond,
|
||||
},
|
||||
}
|
||||
// A later incremental netcheck only re-probed the home and fastest regions, so
|
||||
// it has no latency for regions 4 or 5.
|
||||
incrementalReport := &netcheck.Report{
|
||||
RegionLatency: map[int]time.Duration{
|
||||
1: 10 * time.Millisecond,
|
||||
2: 20 * time.Millisecond,
|
||||
3: 30 * time.Millisecond,
|
||||
},
|
||||
}
|
||||
|
||||
b := newTestLocalBackend(t)
|
||||
b.currentNode().SetNetMap(netMap)
|
||||
mc := b.sys.MagicSock.Get()
|
||||
|
||||
// Without any netcheck reports, SuggestExitNode returns an error because no
|
||||
// preferred DERP can be determined.
|
||||
if _, err := b.SuggestExitNode(); !errors.Is(err, ErrNoPreferredDERP) {
|
||||
t.Fatalf("SuggestExitNode() error = %v, want ErrNoPreferredDERP", err)
|
||||
}
|
||||
|
||||
// Seed the full report, then the incremental one a minute later, so both
|
||||
// remain in netcheck's recent history.
|
||||
now := time.Now()
|
||||
mc.AddNetcheckReportForTest(derpMap, fullReport, now.Add(-time.Minute))
|
||||
mc.AddNetcheckReportForTest(derpMap, incrementalReport, now)
|
||||
|
||||
// suggestExitNodeLocked falls back to a random region when it cannot order the
|
||||
// candidates by latency, so query repeatedly to make sure it always returns the closer region-4 node.
|
||||
const iterations = 64
|
||||
got := make(map[tailcfg.StableNodeID]int)
|
||||
for range iterations {
|
||||
res, err := b.SuggestExitNode()
|
||||
if err != nil {
|
||||
t.Fatalf("SuggestExitNode() error = %v", err)
|
||||
}
|
||||
got[res.ID]++
|
||||
}
|
||||
if len(got) != 1 || got[exitRegion4.StableID()] != iterations {
|
||||
t.Errorf("expected all %v suggestions to be for %v, got %+v", iterations, exitRegion4.StableID(), got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSuggestExitNodePickWeighted(t *testing.T) {
|
||||
location10 := tailcfg.Location{
|
||||
Priority: 10,
|
||||
@@ -6565,46 +6657,41 @@ func TestSuggestExitNodeTrafficSteering(t *testing.T) {
|
||||
|
||||
func TestMinLatencyDERPregion(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
regions []int
|
||||
report *netcheck.Report
|
||||
wantRegion int
|
||||
name string
|
||||
regions []int
|
||||
regionLatency map[int]time.Duration
|
||||
wantRegion int
|
||||
}{
|
||||
{
|
||||
name: "regions-no-latency",
|
||||
regions: []int{1, 2, 3},
|
||||
wantRegion: 0,
|
||||
report: &netcheck.Report{},
|
||||
},
|
||||
{
|
||||
name: "regions-different-latency",
|
||||
regions: []int{1, 2, 3},
|
||||
wantRegion: 2,
|
||||
report: &netcheck.Report{
|
||||
RegionLatency: map[int]time.Duration{
|
||||
1: 10 * time.Millisecond,
|
||||
2: 5 * time.Millisecond,
|
||||
3: 30 * time.Millisecond,
|
||||
},
|
||||
regionLatency: map[int]time.Duration{
|
||||
1: 10 * time.Millisecond,
|
||||
2: 5 * time.Millisecond,
|
||||
3: 30 * time.Millisecond,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "regions-same-latency",
|
||||
regions: []int{1, 2, 3},
|
||||
wantRegion: 1,
|
||||
report: &netcheck.Report{
|
||||
RegionLatency: map[int]time.Duration{
|
||||
1: 10 * time.Millisecond,
|
||||
2: 10 * time.Millisecond,
|
||||
3: 10 * time.Millisecond,
|
||||
},
|
||||
regionLatency: map[int]time.Duration{
|
||||
1: 10 * time.Millisecond,
|
||||
2: 10 * time.Millisecond,
|
||||
3: 10 * time.Millisecond,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
got := minLatencyDERPRegion(tt.regions, tt.report)
|
||||
got := minLatencyDERPRegion(tt.regions, tt.regionLatency)
|
||||
if got != tt.wantRegion {
|
||||
t.Errorf("got region %v want region %v", got, tt.wantRegion)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user