From e9e209673e613836bf1a949af40f4af0062bbdcb Mon Sep 17 00:00:00 2001
From: Anton Tolchanov <anton@tailscale.com>
Date: Wed, 17 Jun 2026 08:47:45 +0000
Subject: [PATCH] net/netcheck: ensure recent history has a full report

suggestExitNodeLocked now ranks exit node candidates using the per-region
latency tracked by the netcheck Client (RecentRegionLatency), which merges
the reports retained in c.prev. That history is only useful for far-away
regions if it contains a full netcheck report, since incremental reports
only re-probe the home region and a handful of the fastest ones.

The full-report cadence in GetReport and the c.prev retention window were
two independent 5-min constants - the way we schedule netchecks ensured
that the history always contaned a full report, but it was not a strong
contract and we did not have any checks around this.

Now full report interval and retention window are driven by the same
var, and a test confirms that the history contains a full report.

Updates tailscale/corp#17516

Signed-off-by: Anton Tolchanov <anton@tailscale.com>
---
 net/netcheck/netcheck.go      |  8 ++++-
 net/netcheck/netcheck_test.go | 55 +++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/net/netcheck/netcheck.go b/net/netcheck/netcheck.go
index b33fd1458..5b6fb119f 100644
--- a/net/netcheck/netcheck.go
+++ b/net/netcheck/netcheck.go
@@ -853,7 +853,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap, opts *GetRe
 	}
 
 	doFull := false
-	if c.nextFull || now.Sub(c.lastFull) > 5*time.Minute {
+	if c.nextFull || now.Sub(c.lastFull) > fullReportInterval {
 		doFull = true
 	}
 	// If the last report had a captive portal and reported no UDP access,
@@ -1329,6 +1329,12 @@ func (c *Client) timeNow() time.Time {
 }
 
 const (
+	// fullReportInterval is the maximum time between full netcheck reports.
+	// Once this long has elapsed since the last full report, the next GetReport
+	// re-probes every DERP region rather than only the home and fastest regions
+	// (see GetReport). It also informs retention window for report history
+	// (c.prev).
+	fullReportInterval = 5 * time.Minute
 	// preferredDERPAbsoluteDiff specifies the minimum absolute difference
 	// in latencies between two DERP regions that would cause a node to
 	// switch its PreferredDERP ("home DERP"). This ensures that if a node
diff --git a/net/netcheck/netcheck_test.go b/net/netcheck/netcheck_test.go
index 0fd3460fa..123cbaca5 100644
--- a/net/netcheck/netcheck_test.go
+++ b/net/netcheck/netcheck_test.go
@@ -481,6 +481,61 @@ func TestAddReportHistoryAndSetPreferredDERP(t *testing.T) {
 	}
 }
 
+// TestRecentReportsRetainFullNetcheck confirms that the recent-report history
+// (c.prev) always retains at least one full netcheck report, so
+// RecentRegionLatency covers every DERP region even when the most recent
+// reports are incremental.
+func TestRecentReportsRetainFullNetcheck(t *testing.T) {
+	dm := &tailcfg.DERPMap{
+		Regions: map[int]*tailcfg.DERPRegion{
+			1: {RegionID: 1},
+			2: {RegionID: 2},
+			3: {RegionID: 3},
+		},
+	}
+	allRegions := []int{1, 2, 3}
+	incrementalRegions := []int{1, 2} // home + fastest; never includes region 3
+
+	var now time.Time
+	c := &Client{TimeNow: func() time.Time { return now }}
+
+	mkReport := func(regions []int) *Report {
+		r := &Report{RegionLatency: map[int]time.Duration{}}
+		for _, rid := range regions {
+			r.RegionLatency[rid] = 10 * time.Millisecond
+		}
+		return r
+	}
+
+	// Run one netcheck per minute for an hour, spanning many full-report
+	// intervals.
+	const tick = time.Minute
+	start := time.Unix(1700000000, 0)
+	var lastFull time.Time // zero => first report is full, as in GetReport
+	for i := 0; i < 60; i++ {
+		now = start.Add(time.Duration(i) * tick)
+
+		// Mirror GetReport's full-vs-incremental decision.
+		doFull := now.Sub(lastFull) > fullReportInterval
+		regions := incrementalRegions
+		if doFull {
+			regions = allRegions
+			lastFull = now
+		}
+		c.addReportHistoryAndSetPreferredDERP(&reportState{c: c, start: now}, mkReport(regions), dm.View())
+
+		// Recent latency must always cover every region, which is only
+		// possible while a full report remains in c.prev.
+		got := c.RecentRegionLatency()
+		for _, rid := range allRegions {
+			if _, ok := got[rid]; !ok {
+				t.Fatalf("after report %d at +%s (full=%v): region %d missing from RecentRegionLatency %v; no full report retained in c.prev",
+					i, now.Sub(start), doFull, rid, got)
+			}
+		}
+	}
+}
+
 func TestMakeProbePlan(t *testing.T) {
 	// basicMap has 5 regions. each region has a number of nodes
 	// equal to the region number (1 has 1a, 2 has 2a and 2b, etc.)