diff --git a/net/netcheck/netcheck.go b/net/netcheck/netcheck.go index 862743540..3181119f2 100644 --- a/net/netcheck/netcheck.go +++ b/net/netcheck/netcheck.go @@ -1388,10 +1388,11 @@ const ( // node is near region 1 @ 4ms and region 2 @ 5ms, region 1 getting // 5ms slower would cause a flap). preferredDERPAbsoluteDiff = 10 * time.Millisecond - // preferredDERPFrameTime is the time which, if a DERP frame has been + // PreferredDERPFrameTime is the time which, if a DERP frame has been // received within that period, we treat that region as being present // even without receiving a STUN response. - preferredDERPFrameTime = 2 * time.Second + // Note: must remain higher than the derp package frameReceiveRecordRate + PreferredDERPFrameTime = 8 * time.Second ) // addReportHistoryAndSetPreferredDERP adds r to the set of recent Reports @@ -1480,7 +1481,7 @@ func (c *Client) addReportHistoryAndSetPreferredDERP(rs *reportState, r *Report, now := c.timeNow() heardFromOldRegionRecently = lastHeard.After(rs.start) - heardFromOldRegionRecently = heardFromOldRegionRecently || lastHeard.After(now.Add(-preferredDERPFrameTime)) + heardFromOldRegionRecently = heardFromOldRegionRecently || lastHeard.After(now.Add(-PreferredDERPFrameTime)) } } diff --git a/net/netcheck/netcheck_test.go b/net/netcheck/netcheck_test.go index 80aa5a657..aaff4e86b 100644 --- a/net/netcheck/netcheck_test.go +++ b/net/netcheck/netcheck_test.go @@ -410,7 +410,7 @@ func TestAddReportHistoryAndSetPreferredDERP(t *testing.T) { }, opts: &GetReportOpts{ GetLastDERPActivity: mkLDAFunc(map[int]time.Time{ - 1: startTime.Add(2*time.Second + preferredDERPFrameTime/2), // within active window of step (3) + 1: startTime.Add(2*time.Second + PreferredDERPFrameTime/2), // within active window of step (3) }), }, wantPrevLen: 3, @@ -425,7 +425,7 @@ func TestAddReportHistoryAndSetPreferredDERP(t *testing.T) { }, opts: &GetReportOpts{ GetLastDERPActivity: mkLDAFunc(map[int]time.Time{ - 1: startTime.Add(4*time.Second - preferredDERPFrameTime - 1), // not within active window of (3) + 1: startTime.Add(4*time.Second - PreferredDERPFrameTime - 1), // not within active window of (3) }), }, wantPrevLen: 3, diff --git a/wgengine/magicsock/derp.go b/wgengine/magicsock/derp.go index 73147e128..32611546f 100644 --- a/wgengine/magicsock/derp.go +++ b/wgengine/magicsock/derp.go @@ -35,6 +35,12 @@ import ( "tailscale.com/util/testenv" ) +// frameReceiveRecordRate is the minimum time between updates to last frame +// received times. +// Note: this is relevant to other parts of the system, such as netcheck +// preferredDERPFrameTime, so update with care. +const frameReceiveRecordRate = 5 * time.Second + // useDerpRoute reports whether magicsock should enable the DERP // return path optimization (Issue 150). // @@ -569,7 +575,7 @@ func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr netip.AddrPort, d bo.BackOff(ctx, nil) // reset now := time.Now() - if lastPacketTime.IsZero() || now.Sub(lastPacketTime) > 5*time.Second { + if lastPacketTime.IsZero() || now.Sub(lastPacketTime) > frameReceiveRecordRate { health.NoteDERPRegionReceivedFrame(regionID) lastPacketTime = now } diff --git a/wgengine/magicsock/derp_test.go b/wgengine/magicsock/derp_test.go new file mode 100644 index 000000000..ffb230789 --- /dev/null +++ b/wgengine/magicsock/derp_test.go @@ -0,0 +1,16 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package magicsock + +import ( + "testing" + + "tailscale.com/net/netcheck" +) + +func CheckDERPHeuristicTimes(t *testing.T) { + if netcheck.PreferredDERPFrameTime <= frameReceiveRecordRate { + t.Errorf("PreferredDERPFrameTime too low; should be at least frameReceiveRecordRate") + } +}