From 7f9ebc0a83f82922787f4e8336b9f626d895a08c Mon Sep 17 00:00:00 2001 From: James Tucker Date: Wed, 4 Dec 2024 12:02:59 -0800 Subject: [PATCH] cmd/tailscale,net/netcheck: add debug feature to force preferred DERP This provides an interface for a user to force a preferred DERP outcome for all future netchecks that will take precedence unless the forced region is unreachable. The option does not persist and will be lost when the daemon restarts. Updates tailscale/corp#18997 Updates tailscale/corp#24755 Signed-off-by: James Tucker --- client/tailscale/localclient.go | 11 +++++++ cmd/tailscale/cli/debug.go | 25 +++++++++++++++ ipn/ipnlocal/local.go | 6 ++++ ipn/localapi/localapi.go | 7 +++++ net/netcheck/netcheck.go | 28 +++++++++++++++++ net/netcheck/netcheck_test.go | 56 ++++++++++++++++++++++++++++++++- wgengine/magicsock/magicsock.go | 8 +++++ 7 files changed, 140 insertions(+), 1 deletion(-) diff --git a/client/tailscale/localclient.go b/client/tailscale/localclient.go index 5eb668176..34c094a63 100644 --- a/client/tailscale/localclient.go +++ b/client/tailscale/localclient.go @@ -493,6 +493,17 @@ func (lc *LocalClient) DebugAction(ctx context.Context, action string) error { return nil } +// DebugActionBody invokes a debug action with a body parameter, such as +// "debug-force-prefer-derp". +// These are development tools and subject to change or removal over time. +func (lc *LocalClient) DebugActionBody(ctx context.Context, action string, rbody io.Reader) error { + body, err := lc.send(ctx, "POST", "/localapi/v0/debug?action="+url.QueryEscape(action), 200, rbody) + if err != nil { + return fmt.Errorf("error %w: %s", err, body) + } + return nil +} + // DebugResultJSON invokes a debug action and returns its result as something JSON-able. // These are development tools and subject to change or removal over time. func (lc *LocalClient) DebugResultJSON(ctx context.Context, action string) (any, error) { diff --git a/cmd/tailscale/cli/debug.go b/cmd/tailscale/cli/debug.go index 78bd708e5..04b343e76 100644 --- a/cmd/tailscale/cli/debug.go +++ b/cmd/tailscale/cli/debug.go @@ -175,6 +175,12 @@ Exec: localAPIAction("pick-new-derp"), ShortHelp: "Switch to some other random DERP home region for a short time", }, + { + Name: "force-prefer-derp", + ShortUsage: "tailscale debug force-prefer-derp", + Exec: forcePreferDERP, + ShortHelp: "Prefer the given region ID if reachable (until restart, or 0 to clear)", + }, { Name: "force-netmap-update", ShortUsage: "tailscale debug force-netmap-update", @@ -577,6 +583,25 @@ func runDERPMap(ctx context.Context, args []string) error { return nil } +func forcePreferDERP(ctx context.Context, args []string) error { + var n int + if len(args) != 1 { + return errors.New("expected exactly one integer argument") + } + n, err := strconv.Atoi(args[0]) + if err != nil { + return fmt.Errorf("expected exactly one integer argument: %w", err) + } + b, err := json.Marshal(n) + if err != nil { + return fmt.Errorf("failed to marshal DERP region: %w", err) + } + if err := localClient.DebugActionBody(ctx, "force-prefer-derp", bytes.NewReader(b)); err != nil { + return fmt.Errorf("failed to force preferred DERP: %w", err) + } + return nil +} + func localAPIAction(action string) func(context.Context, []string) error { return func(ctx context.Context, args []string) error { if len(args) > 0 { diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 278614c0b..f456d4984 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -2920,6 +2920,12 @@ func (b *LocalBackend) DebugPickNewDERP() error { return b.sys.MagicSock.Get().DebugPickNewDERP() } +// DebugForcePreferDERP forwards to netcheck.DebugForcePreferDERP. +// See its docs. +func (b *LocalBackend) DebugForcePreferDERP(n int) { + b.sys.MagicSock.Get().DebugForcePreferDERP(n) +} + // send delivers n to the connected frontend and any API watchers from // LocalBackend.WatchNotifications (via the LocalAPI). // diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index ea931b028..c14a4bdf2 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -634,6 +634,13 @@ func (h *Handler) serveDebug(w http.ResponseWriter, r *http.Request) { } case "pick-new-derp": err = h.b.DebugPickNewDERP() + case "force-prefer-derp": + var n int + err = json.NewDecoder(r.Body).Decode(&n) + if err != nil { + break + } + h.b.DebugForcePreferDERP(n) case "": err = fmt.Errorf("missing parameter 'action'") default: diff --git a/net/netcheck/netcheck.go b/net/netcheck/netcheck.go index 0bb930568..d8f5e1d49 100644 --- a/net/netcheck/netcheck.go +++ b/net/netcheck/netcheck.go @@ -236,6 +236,10 @@ type Client struct { // If false, the default net.Resolver will be used, with no caching. UseDNSCache bool + // if non-zero, force this DERP region to be preferred in all reports where + // the DERP is found to be reachable. + ForcePreferredDERP int + // For tests testEnoughRegions int testCaptivePortalDelay time.Duration @@ -780,6 +784,12 @@ func (o *GetReportOpts) getLastDERPActivity(region int) time.Time { return o.GetLastDERPActivity(region) } +func (c *Client) SetForcePreferredDERP(region int) { + c.mu.Lock() + defer c.mu.Unlock() + c.ForcePreferredDERP = region +} + // GetReport gets a report. The 'opts' argument is optional and can be nil. // Callers are discouraged from passing a ctx with an arbitrary deadline as this // may cause GetReport to return prematurely before all reporting methods have @@ -1277,6 +1287,9 @@ func (c *Client) logConciseReport(r *Report, dm *tailcfg.DERPMap) { if r.CaptivePortal != "" { fmt.Fprintf(w, " captiveportal=%v", r.CaptivePortal) } + if c.ForcePreferredDERP != 0 { + fmt.Fprintf(w, " force=%v", c.ForcePreferredDERP) + } fmt.Fprintf(w, " derp=%v", r.PreferredDERP) if r.PreferredDERP != 0 { fmt.Fprintf(w, " derpdist=") @@ -1435,6 +1448,21 @@ func (c *Client) addReportHistoryAndSetPreferredDERP(rs *reportState, r *Report, // which undoes any region change we made above. r.PreferredDERP = prevDERP } + if c.ForcePreferredDERP != 0 { + // If the forced DERP region probed successfully, or has recent traffic, + // use it. + _, haveLatencySample := r.RegionLatency[c.ForcePreferredDERP] + var recentActivity bool + if lastHeard := rs.opts.getLastDERPActivity(c.ForcePreferredDERP); !lastHeard.IsZero() { + now := c.timeNow() + recentActivity = lastHeard.After(rs.start) + recentActivity = recentActivity || lastHeard.After(now.Add(-PreferredDERPFrameTime)) + } + + if haveLatencySample || recentActivity { + r.PreferredDERP = c.ForcePreferredDERP + } + } } func updateLatency(m map[int]time.Duration, regionID int, d time.Duration) { diff --git a/net/netcheck/netcheck_test.go b/net/netcheck/netcheck_test.go index 23891efcc..88c19623d 100644 --- a/net/netcheck/netcheck_test.go +++ b/net/netcheck/netcheck_test.go @@ -201,6 +201,7 @@ type step struct { steps []step homeParams *tailcfg.DERPHomeParams opts *GetReportOpts + forcedDERP int // if non-zero, force this DERP to be the preferred one wantDERP int // want PreferredDERP on final step wantPrevLen int // wanted len(c.prev) }{ @@ -366,12 +367,65 @@ type step struct { wantPrevLen: 2, wantDERP: 1, // diff is 11ms, but d2 is greater than 2/3s of d1 }, + { + name: "forced_two", + steps: []step{ + {time.Second, report("d1", 2, "d2", 3)}, + {2 * time.Second, report("d1", 4, "d2", 3)}, + }, + forcedDERP: 2, + wantPrevLen: 2, + wantDERP: 2, + }, + { + name: "forced_two_unavailable", + steps: []step{ + {time.Second, report("d1", 2, "d2", 1)}, + {2 * time.Second, report("d1", 4)}, + }, + forcedDERP: 2, + wantPrevLen: 2, + wantDERP: 1, + }, + { + name: "forced_two_no_probe_recent_activity", + steps: []step{ + {time.Second, report("d1", 2)}, + {2 * time.Second, report("d1", 4)}, + }, + opts: &GetReportOpts{ + GetLastDERPActivity: mkLDAFunc(map[int]time.Time{ + 1: startTime, + 2: startTime.Add(time.Second), + }), + }, + forcedDERP: 2, + wantPrevLen: 2, + wantDERP: 2, + }, + { + name: "forced_two_no_probe_no_recent_activity", + steps: []step{ + {time.Second, report("d1", 2)}, + {PreferredDERPFrameTime + time.Second, report("d1", 4)}, + }, + opts: &GetReportOpts{ + GetLastDERPActivity: mkLDAFunc(map[int]time.Time{ + 1: startTime, + 2: startTime, + }), + }, + forcedDERP: 2, + wantPrevLen: 2, + wantDERP: 1, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { fakeTime := startTime c := &Client{ - TimeNow: func() time.Time { return fakeTime }, + TimeNow: func() time.Time { return fakeTime }, + ForcePreferredDERP: tt.forcedDERP, } dm := &tailcfg.DERPMap{HomeParams: tt.homeParams} rs := &reportState{ diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 805716e61..bff905caa 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -3013,6 +3013,14 @@ func (c *Conn) DebugPickNewDERP() error { return errors.New("too few regions") } +func (c *Conn) DebugForcePreferDERP(n int) { + c.mu.Lock() + defer c.mu.Unlock() + + c.logf("magicsock: [debug] force preferred DERP set to: %d", n) + c.netChecker.SetForcePreferredDERP(n) +} + // portableTrySetSocketBuffer sets SO_SNDBUF and SO_RECVBUF on pconn to socketBufferSize, // logging an error if it occurs. func portableTrySetSocketBuffer(pconn nettype.PacketConn, logf logger.Logf) {