net/netcheck: extract Compare function for region latencies (#20658)

There are two places in the code where we need to compare the
latencies of a netcheck report. In both cases, the comparison wasn’t
well tested.

This PR extracts that logic into a Compare method of the new
RegionLatency type. This type wraps the map of latency measurements
keyed by region ID.

Updates #cleanup

Change-Id: I7f248988973007c2f452283c1b82f84b03068f77

Signed-off-by: Simon Law <sfllaw@tailscale.com>
This commit is contained in:
Simon Law
2026-07-29 16:51:55 -04:00
committed by GitHub
parent d4dce80927
commit bb498a8393
11 changed files with 162 additions and 41 deletions

View File

@@ -797,7 +797,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/
tailscale.com/net/ipset from tailscale.com/ipn/ipnlocal+
tailscale.com/net/memnet from tailscale.com/tsnet
tailscale.com/net/netaddr from tailscale.com/ipn+
tailscale.com/net/netcheck from tailscale.com/wgengine/magicsock
tailscale.com/net/netcheck from tailscale.com/wgengine/magicsock+
tailscale.com/net/neterror from tailscale.com/net/dns/resolver+
tailscale.com/net/netkernelconf from tailscale.com/ipn/ipnlocal
tailscale.com/net/netknob from tailscale.com/logpolicy+

View File

@@ -10,10 +10,11 @@
"fmt"
"io"
"log"
"maps"
"math"
"net/http"
"net/netip"
"sort"
"slices"
"strings"
"time"
@@ -214,21 +215,7 @@ func printNetCheckReport(dm *tailcfg.DERPMap, report *netcheck.Report) error {
printf("\t* Nearest DERP: [none]\n")
}
printf("\t* DERP latency:\n")
var rids []int
for rid := range dm.Regions {
rids = append(rids, rid)
}
sort.Slice(rids, func(i, j int) bool {
l1, ok1 := report.RegionLatency[rids[i]]
l2, ok2 := report.RegionLatency[rids[j]]
if ok1 != ok2 {
return ok1 // defined things sort first
}
if !ok1 {
return rids[i] < rids[j]
}
return l1 < l2
})
rids := slices.SortedFunc(maps.Keys(dm.Regions), report.RegionLatency.Compare)
for _, rid := range rids {
d, ok := report.RegionLatency[rid]
var latency string

View File

@@ -267,6 +267,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
tailscale.com/tsweb from tailscale.com/util/eventbus+
tailscale.com/tsweb/varz from tailscale.com/util/usermetric+
tailscale.com/types/appctype from tailscale.com/client/local+
tailscale.com/types/bools from tailscale.com/net/netcheck
tailscale.com/types/dnstype from tailscale.com/tailcfg+
tailscale.com/types/empty from tailscale.com/ipn
tailscale.com/types/ipproto from tailscale.com/ipn+

View File

@@ -97,7 +97,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
tailscale.com/net/flowtrack from tailscale.com/wgengine/filter
tailscale.com/net/ipset from tailscale.com/ipn/ipnlocal+
tailscale.com/net/netaddr from tailscale.com/ipn+
tailscale.com/net/netcheck from tailscale.com/wgengine/magicsock
tailscale.com/net/netcheck from tailscale.com/wgengine/magicsock+
tailscale.com/net/neterror from tailscale.com/net/batching+
tailscale.com/net/netkernelconf from tailscale.com/ipn/ipnlocal
tailscale.com/net/netknob from tailscale.com/logpolicy+
@@ -135,6 +135,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
tailscale.com/tstime/mono from tailscale.com/net/tstun+
tailscale.com/tstime/rate from tailscale.com/wgengine/filter
tailscale.com/types/appctype from tailscale.com/ipn/ipnlocal+
tailscale.com/types/bools from tailscale.com/net/netcheck
tailscale.com/types/dnstype from tailscale.com/client/tailscale/apitype+
tailscale.com/types/empty from tailscale.com/ipn+
tailscale.com/types/events from tailscale.com/control/controlclient+

View File

@@ -154,6 +154,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
tailscale.com/tstime/mono from tailscale.com/net/tstun+
tailscale.com/tstime/rate from tailscale.com/wgengine/filter
tailscale.com/types/appctype from tailscale.com/ipn/ipnlocal+
tailscale.com/types/bools from tailscale.com/net/netcheck
tailscale.com/types/dnstype from tailscale.com/client/tailscale/apitype+
tailscale.com/types/empty from tailscale.com/ipn+
tailscale.com/types/events from tailscale.com/control/controlclient+

View File

@@ -431,7 +431,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
tailscale.com/tsweb from tailscale.com/util/eventbus+
tailscale.com/tsweb/varz from tailscale.com/cmd/tailscaled+
tailscale.com/types/appctype from tailscale.com/ipn/ipnlocal+
tailscale.com/types/bools from tailscale.com/wgengine/netlog
tailscale.com/types/bools from tailscale.com/wgengine/netlog+
tailscale.com/types/dnstype from tailscale.com/ipn/ipnlocal+
tailscale.com/types/empty from tailscale.com/ipn+
tailscale.com/types/events from tailscale.com/control/controlclient+

View File

@@ -194,7 +194,7 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar
tailscale.com/net/ipset from tailscale.com/ipn/ipnlocal+
tailscale.com/net/memnet from tailscale.com/tsnet
tailscale.com/net/netaddr from tailscale.com/ipn+
tailscale.com/net/netcheck from tailscale.com/wgengine/magicsock
tailscale.com/net/netcheck from tailscale.com/wgengine/magicsock+
tailscale.com/net/neterror from tailscale.com/net/dns/resolver+
tailscale.com/net/netkernelconf from tailscale.com/ipn/ipnlocal
tailscale.com/net/netknob from tailscale.com/logpolicy+

View File

@@ -58,6 +58,7 @@
"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"
@@ -9059,22 +9060,8 @@ func randomNode(nodes views.Slice[tailcfg.NodeView], prefer tailcfg.StableNodeID
// 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 := regionLatency[i]
if !ok {
iLatency = largeDuration
}
jLatency, ok := regionLatency[j]
if !ok {
jLatency = largeDuration
}
if c := cmp.Compare(iLatency, jLatency); c != 0 {
return c
}
return cmp.Compare(i, j)
})
func minLatencyDERPRegion(regions []int, regionLatency netcheck.RegionLatency) int {
min := slices.MinFunc(regions, regionLatency.Compare)
latency, ok := regionLatency[min]
if !ok || latency == 0 {
return 0

View File

@@ -39,6 +39,7 @@
"tailscale.com/net/stun"
"tailscale.com/syncs"
"tailscale.com/tailcfg"
"tailscale.com/types/bools"
"tailscale.com/types/logger"
"tailscale.com/types/nettype"
"tailscale.com/types/opt"
@@ -111,10 +112,10 @@ type Report struct {
// Empty means not checked.
PCP opt.Bool
PreferredDERP int // or 0 for unknown
RegionLatency map[int]time.Duration // keyed by DERP Region ID
RegionV4Latency map[int]time.Duration // keyed by DERP Region ID
RegionV6Latency map[int]time.Duration // keyed by DERP Region ID
PreferredDERP int // or 0 for unknown
RegionLatency RegionLatency // keyed by DERP Region ID
RegionV4Latency RegionLatency // keyed by DERP Region ID
RegionV6Latency RegionLatency // keyed by DERP Region ID
GlobalV4Counters map[netip.AddrPort]int // number of times the endpoint was observed
GlobalV6Counters map[netip.AddrPort]int // number of times the endpoint was observed
@@ -184,6 +185,34 @@ func (r *Report) Clone() *Report {
return &r2
}
// RegionLatency is a map of DERP region IDs associated with their measured latencies.
type RegionLatency map[int]time.Duration
// Compare compares the latency of the regions i and j. It returns
//
// -1 if region i is contained in lat, but region j is missing
// +1 if region j is contained in lat, but region i is missing
//
// Otherwise, it returns
//
// -1 if the latency of region i is less than the latency of region j
// +1 if the latency of region i is greater than the latency of region j
//
// If both latency values are equal, then it breaks ties by region ID and returns
//
// -1 if i is less than j
// 0 if i equals j
// +1 if i is greater than j
func (lat RegionLatency) Compare(i, j int) int {
iLat, iOK := lat[i]
jLat, jOK := lat[j]
return cmp.Or(
bools.Compare(!iOK, !jOK), // defined things sort first
cmp.Compare(iLat, jLat), // sort by latency
cmp.Compare(i, j), // break ties by region ID
)
}
// Client generates Reports describing the result of both passive and active
// network configuration probing. It provides two different modes of report, a
// full report (see MakeNextReportFull) and a more lightweight incremental

View File

@@ -1101,3 +1101,118 @@ func TestNoUDPNilGetReportOpts(t *testing.T) {
t.Fatal("unexpected working UDP")
}
}
func TestRegionLatencyCompare(t *testing.T) {
for _, tc := range []struct {
name string
lat RegionLatency
i int
j int
want int
}{
{
name: "no-lat-ids-equal",
lat: RegionLatency{},
i: 1,
j: 1,
want: 0,
},
{
name: "no-lat-ids-increasing",
lat: RegionLatency{},
i: 1,
j: 2,
want: -1,
},
{
name: "no-lat-ids-decreasing",
lat: RegionLatency{},
i: 2,
j: 1,
want: +1,
},
{
name: "i-lat-only-ids-increasing",
lat: RegionLatency{1: 10},
i: 1,
j: 2,
want: -1,
},
{
name: "i-lat-only-ids-decreasing",
lat: RegionLatency{2: 20},
i: 2,
j: 1,
want: -1,
},
{
name: "j-lat-only-ids-increasing",
lat: RegionLatency{2: 20},
i: 1,
j: 2,
want: +1,
},
{
name: "j-lat-only-ids-decreasing",
lat: RegionLatency{1: 10},
i: 2,
j: 1,
want: +1,
},
{
name: "i-lower-lat-ids-increasing",
lat: RegionLatency{1: 10, 2: 20},
i: 1,
j: 2,
want: -1,
},
{
name: "i-lower-lat-ids-decreasing",
lat: RegionLatency{2: 20, 1: 100},
i: 2,
j: 1,
want: -1,
},
{
name: "j-lower-lat-ids-increasing",
lat: RegionLatency{1: 100, 2: 20},
i: 1,
j: 2,
want: +1,
},
{
name: "j-lower-lat-ids-decreasing",
lat: RegionLatency{2: 20, 1: 10},
i: 2,
j: 1,
want: +1,
},
{
name: "equal-lat-ids-increasing",
lat: RegionLatency{1: 10, 2: 10},
i: 1,
j: 2,
want: -1,
},
{
name: "equal-lat-ids-decreasing",
lat: RegionLatency{2: 20, 1: 20},
i: 2,
j: 1,
want: +1,
},
{
name: "equal-lat-ids-equal",
lat: RegionLatency{1: 10},
i: 1,
j: 1,
want: 0,
},
} {
t.Run(tc.name, func(t *testing.T) {
if got := tc.lat.Compare(tc.i, tc.j); got != tc.want {
t.Errorf("got %d, want %d", got, tc.want)
}
})
}
}

View File

@@ -190,7 +190,7 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware)
tailscale.com/net/ipset from tailscale.com/ipn/ipnlocal+
tailscale.com/net/memnet from tailscale.com/tsnet
tailscale.com/net/netaddr from tailscale.com/ipn+
tailscale.com/net/netcheck from tailscale.com/wgengine/magicsock
tailscale.com/net/netcheck from tailscale.com/wgengine/magicsock+
tailscale.com/net/neterror from tailscale.com/net/dns/resolver+
tailscale.com/net/netkernelconf from tailscale.com/ipn/ipnlocal
tailscale.com/net/netknob from tailscale.com/logpolicy+