From d6211abfb2bfae9f869c994ea6f61c24ab528fa7 Mon Sep 17 00:00:00 2001 From: James Tucker Date: Wed, 10 Jun 2026 23:22:00 +0000 Subject: [PATCH] net/udprelay,feature/relayserver,client/local: make endpoint lifetimes configurable The bind and steady state endpoint lifetimes were hardcoded to 30s and 5m, which makes tests reproducing relay-session-reap scenarios, e.g. in-process magicsock e2e tests and natlab VM tests for #20082 (black hole following relay session reap), take 5+ minutes of wall time. Add Server.SetLifetimes, which validates the new values and pokes the endpoint GC loop to reset its ticker so they take effect immediately, and a SetLifetimesForTest wrapper for brevity in in-process tests. For tests that run tailscaled as a separate process, e.g. natlab VM tests, expose it at runtime via a PermitWrite-gated LocalAPI debug endpoint, debug-peer-relay-server-lifetimes, mirroring the existing debug-peer-relay-sessions plumbing through the relayserver extension, with a corresponding client/local method. A LocalAPI hook is preferred over env knobs as it is per-instance and parallel-test-safe, and works after the relay server is started via the RelayServerPort pref. Production behavior is unchanged unless the hook is used. Updates #20082 Change-Id: I827c54b222a268cbdb90c41febfe95ae3f2295f2 Signed-off-by: James Tucker --- client/local/local.go | 20 ++++++ feature/relayserver/relayserver.go | 66 +++++++++++++++++ feature/relayserver/relayserver_test.go | 61 +++++++++++++++- net/udprelay/server.go | 96 +++++++++++++++++++------ net/udprelay/server_test.go | 81 +++++++++++++++++++++ 5 files changed, 302 insertions(+), 22 deletions(-) diff --git a/client/local/local.go b/client/local/local.go index 50050eb1b..62140f8de 100644 --- a/client/local/local.go +++ b/client/local/local.go @@ -1334,6 +1334,26 @@ func (lc *Client) DebugPeerRelaySessions(ctx context.Context) (*status.ServerSta return decodeJSON[*status.ServerStatus](body) } +// DebugSetPeerRelayServerLifetimes overrides the endpoint bind and steady +// state lifetimes of this node's running peer relay server. It returns an +// error if the relay server is not running, e.g. if no RelayServerPort pref +// is set, or if either lifetime is non-positive. +// +// It is meant for tests reproducing relay-session-reap scenarios, e.g. +// tailscale/tailscale#20082, in seconds rather than the minutes implied by +// the production default lifetimes. +func (lc *Client) DebugSetPeerRelayServerLifetimes(ctx context.Context, bind, steadyState time.Duration) error { + v := url.Values{ + "bind-lifetime": {bind.String()}, + "steady-state-lifetime": {steadyState.String()}, + } + body, err := lc.send(ctx, "POST", "/localapi/v0/debug-peer-relay-server-lifetimes?"+v.Encode(), 200, nil) + if err != nil { + return fmt.Errorf("error %w: %s", err, body) + } + return nil +} + // StreamDebugCapture streams a pcap-formatted packet capture. // // The provided context does not determine the lifetime of the diff --git a/feature/relayserver/relayserver.go b/feature/relayserver/relayserver.go index 2de5654c7..b0997604b 100644 --- a/feature/relayserver/relayserver.go +++ b/feature/relayserver/relayserver.go @@ -7,9 +7,11 @@ import ( "encoding/json" + "errors" "fmt" "net/http" "net/netip" + "time" "tailscale.com/disco" "tailscale.com/feature" @@ -36,6 +38,7 @@ func init() { feature.Register(featureName) ipnext.RegisterExtension(featureName, newExtension) localapi.Register("debug-peer-relay-sessions", servePeerRelayDebugSessions) + localapi.Register("debug-peer-relay-server-lifetimes", servePeerRelayDebugSetLifetimes) } // servePeerRelayDebugSessions is an HTTP handler for the Local API that @@ -63,6 +66,50 @@ func servePeerRelayDebugSessions(h *localapi.Handler, w http.ResponseWriter, r * w.Write(j) } +// servePeerRelayDebugSetLifetimes is an HTTP handler for the Local API that +// overrides the endpoint bind and steady state lifetimes of the running peer +// relay server, parsed as [time.Duration] from the "bind-lifetime" and +// "steady-state-lifetime" query parameters. It returns an HTTP 403/405/400 +// with error text as the body if access or validation fails, e.g. if the +// relay server is not running. +// +// It exists to enable tests, e.g. natlab VM tests, to reproduce +// relay-session-reap scenarios (tailscale/tailscale#20082) in seconds rather +// than the minutes implied by the production default lifetimes. It must be +// called after the relay server has been enabled via the RelayServerPort +// pref. +func servePeerRelayDebugSetLifetimes(h *localapi.Handler, w http.ResponseWriter, r *http.Request) { + if !h.PermitWrite { + http.Error(w, "debug access denied", http.StatusForbidden) + return + } + if r.Method != "POST" { + http.Error(w, "POST required", http.StatusMethodNotAllowed) + return + } + bind, err := time.ParseDuration(r.FormValue("bind-lifetime")) + if err != nil { + http.Error(w, fmt.Sprintf("invalid bind-lifetime: %v", err), http.StatusBadRequest) + return + } + steadyState, err := time.ParseDuration(r.FormValue("steady-state-lifetime")) + if err != nil { + http.Error(w, fmt.Sprintf("invalid steady-state-lifetime: %v", err), http.StatusBadRequest) + return + } + + var e *extension + if ok := h.LocalBackend().FindMatchingExtension(&e); !ok { + http.Error(w, "peer relay server extension unavailable", http.StatusInternalServerError) + return + } + + if err := e.setServerLifetimes(bind, steadyState); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } +} + // newExtension is an [ipnext.NewExtensionFn] that creates a new relay server // extension. It is registered with [ipnext.RegisterExtension] if the package is // imported. @@ -87,6 +134,7 @@ type relayServer interface { GetSessions() []status.ServerSession SetDERPMapView(tailcfg.DERPMapView) SetStaticAddrPorts(addrPorts views.Slice[netip.AddrPort]) + SetLifetimes(bind, steadyState time.Duration) error } // extension is an [ipnext.Extension] managing the relay server on platforms @@ -250,6 +298,24 @@ func (e *extension) Shutdown() error { return nil } +// errServerNotRunning is returned by [extension.setServerLifetimes] when the +// relay server has not been started, e.g. before the RelayServerPort pref is +// set, or while [tailcfg.NodeAttrDisableRelayServer] is present. +var errServerNotRunning = errors.New("peer relay server is not running; set the RelayServerPort pref to start it") + +// setServerLifetimes overrides the endpoint lifetimes of the running relay +// server, see [udprelay.Server.SetLifetimes]. It returns +// [errServerNotRunning] if the relay server is not running, or an error if +// the values are invalid. +func (e *extension) setServerLifetimes(bind, steadyState time.Duration) error { + e.mu.Lock() + defer e.mu.Unlock() + if e.rs == nil { + return errServerNotRunning + } + return e.rs.SetLifetimes(bind, steadyState) +} + // serverStatus gathers and returns current peer relay server status information // for this Tailscale node, and status of each peer relay session this node is // relaying (if any). diff --git a/feature/relayserver/relayserver_test.go b/feature/relayserver/relayserver_test.go index 97f4eb874..828a6dbd3 100644 --- a/feature/relayserver/relayserver_test.go +++ b/feature/relayserver/relayserver_test.go @@ -9,6 +9,7 @@ "reflect" "slices" "testing" + "time" "tailscale.com/ipn" "tailscale.com/net/udprelay/endpoint" @@ -244,8 +245,10 @@ func mockRelayServerNotZeroVal() *mockRelayServer { } type mockRelayServer struct { - set bool - addrPorts views.Slice[netip.AddrPort] + set bool + addrPorts views.Slice[netip.AddrPort] + bindLifetime time.Duration + steadyStateLifetime time.Duration } func (m *mockRelayServer) Close() error { return nil } @@ -257,6 +260,14 @@ func (m *mockRelayServer) SetDERPMapView(tailcfg.DERPMapView) { return } func (m *mockRelayServer) SetStaticAddrPorts(aps views.Slice[netip.AddrPort]) { m.addrPorts = aps } +func (m *mockRelayServer) SetLifetimes(bind, steadyState time.Duration) error { + if bind <= 0 || steadyState <= 0 { + return errors.New("lifetimes must be positive") + } + m.bindLifetime = bind + m.steadyStateLifetime = steadyState + return nil +} type mockSafeBackend struct { sys *tsd.System @@ -266,6 +277,52 @@ func (m mockSafeBackend) Sys() *tsd.System { return m.sys } func (mockSafeBackend) Clock() tstime.Clock { return nil } func (mockSafeBackend) TailscaleVarRoot() string { return "" } +// Test_extension_setServerLifetimes verifies [extension.setServerLifetimes], +// the plumbing behind the debug-peer-relay-server-lifetimes LocalAPI +// endpoint, returns a clear error when the relay server is not running, and +// otherwise passes the lifetimes through to the relay server. +func Test_extension_setServerLifetimes(t *testing.T) { + t.Parallel() + tests := []struct { + name string + rs relayServer + bind time.Duration + steadyState time.Duration + wantErr error + }{ + { + name: "not-running", + rs: nil, + bind: time.Second, + steadyState: time.Second, + wantErr: errServerNotRunning, + }, + { + name: "running", + rs: &mockRelayServer{}, + bind: time.Second, + steadyState: 2 * time.Second, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + e := &extension{rs: tt.rs} + err := e.setServerLifetimes(tt.bind, tt.steadyState) + if !errors.Is(err, tt.wantErr) { + t.Fatalf("setServerLifetimes(%v, %v) = %v; want %v", tt.bind, tt.steadyState, err, tt.wantErr) + } + if err != nil { + return + } + mock := tt.rs.(*mockRelayServer) + if mock.bindLifetime != tt.bind || mock.steadyStateLifetime != tt.steadyState { + t.Fatalf("relay server lifetimes = %v/%v; want %v/%v", mock.bindLifetime, mock.steadyStateLifetime, tt.bind, tt.steadyState) + } + }) + } +} + func Test_extension_handleRelayServerLifetimeLocked(t *testing.T) { tests := []struct { name string diff --git a/net/udprelay/server.go b/net/udprelay/server.go index 9052873b0..d96c3c225 100644 --- a/net/udprelay/server.go +++ b/net/udprelay/server.go @@ -67,26 +67,27 @@ // Server implements an experimental UDP relay server. type Server struct { // The following fields are initialized once and never mutated. - logf logger.Logf - disco key.DiscoPrivate - discoPublic key.DiscoPublic + logf logger.Logf + disco key.DiscoPrivate + discoPublic key.DiscoPublic + lifetimesChanged chan struct{} // buffered (cap 1), pokes endpointGCLoop on [Server.SetLifetimes] + bus *eventbus.Bus + uc4 []batching.Conn // length is always nonzero + uc4Port uint16 // always nonzero + uc6 []batching.Conn // length may be zero if udp6 bind fails + uc6Port uint16 // zero if len(uc6) is zero, otherwise nonzero + closeOnce sync.Once + wg sync.WaitGroup + closeCh chan struct{} + netChecker *netcheck.Client + metrics *metrics + netMon *netmon.Monitor + cloudInfo *cloudinfo.CloudInfo // used to query cloud metadata services + controlKnobs *controlknobs.Knobs // or nil + + mu sync.Mutex // guards the following fields bindLifetime time.Duration steadyStateLifetime time.Duration - bus *eventbus.Bus - uc4 []batching.Conn // length is always nonzero - uc4Port uint16 // always nonzero - uc6 []batching.Conn // length may be zero if udp6 bind fails - uc6Port uint16 // zero if len(uc6) is zero, otherwise nonzero - closeOnce sync.Once - wg sync.WaitGroup - closeCh chan struct{} - netChecker *netcheck.Client - metrics *metrics - netMon *netmon.Monitor - cloudInfo *cloudinfo.CloudInfo // used to query cloud metadata services - controlKnobs *controlknobs.Knobs // or nil - - mu sync.Mutex // guards the following fields macSecrets views.Slice[[blake2s.Size]byte] // [0] is most recent, max 2 elements macSecretRotatedAt mono.Time derpMap *tailcfg.DERPMap @@ -385,6 +386,7 @@ func NewServer(logf logger.Logf, port uint16, onlyStaticAddrPorts bool, metrics disco: key.NewDisco(), bindLifetime: defaultBindLifetime, steadyStateLifetime: defaultSteadyStateLifetime, + lifetimesChanged: make(chan struct{}, 1), closeCh: make(chan struct{}), onlyStaticAddrPorts: onlyStaticAddrPorts, serverEndpointByDisco: make(map[key.SortedPairOfDiscoPublic]*serverEndpoint), @@ -775,14 +777,68 @@ func (s *Server) endpointGC(bindLifetime, steadyStateLifetime time.Duration) { } } +// getLifetimes returns the current bind and steady state endpoint lifetimes. +func (s *Server) getLifetimes() (bind, steadyState time.Duration) { + s.mu.Lock() + defer s.mu.Unlock() + return s.bindLifetime, s.steadyStateLifetime +} + +// SetLifetimes overrides the lifetimes used to expire endpoints in +// [Server.endpointGC]: bind bounds the time a newly-allocated endpoint has to +// complete a handshake for both clients, and steadyState bounds the time a +// bound endpoint may remain idle (no packets seen from a client) before it is +// reaped. It returns an error if either value is non-positive, otherwise the +// new values take effect immediately. +// +// It exists to enable tests to reproduce relay-session-reap scenarios, e.g. +// tailscale/tailscale#20082, in seconds rather than the minutes implied by +// the production defaults. Tests exercising tailscaled as a separate process +// can reach it at runtime via the debug-peer-relay-server-lifetimes LocalAPI +// endpoint registered in the feature/relayserver package; in-process tests +// should prefer [Server.SetLifetimesForTest]. +// +// Endpoints allocated before the call were advertised to clients with the +// previous lifetime values via [endpoint.ServerEndpoint], but are expired +// using the new values. +func (s *Server) SetLifetimes(bind, steadyState time.Duration) error { + if bind <= 0 || steadyState <= 0 { + return fmt.Errorf("lifetimes must be positive; got bind=%v steadyState=%v", bind, steadyState) + } + s.mu.Lock() + s.bindLifetime = bind + s.steadyStateLifetime = steadyState + s.mu.Unlock() + // Poke endpointGCLoop to reload the lifetimes and reset its GC ticker, + // which ticks at the bind lifetime interval. + select { + case s.lifetimesChanged <- struct{}{}: + default: // a reload is already pending + } + return nil +} + +// SetLifetimesForTest is like [Server.SetLifetimes], but panics on invalid +// values instead of returning an error, for brevity in its only intended +// callers: in-process tests. +func (s *Server) SetLifetimesForTest(bind, steadyState time.Duration) { + if err := s.SetLifetimes(bind, steadyState); err != nil { + panic("SetLifetimesForTest: " + err.Error()) + } +} + func (s *Server) endpointGCLoop() { defer s.wg.Done() - ticker := time.NewTicker(s.bindLifetime) + bind, steadyState := s.getLifetimes() + ticker := time.NewTicker(bind) defer ticker.Stop() for { select { case <-ticker.C: - s.endpointGC(s.bindLifetime, s.steadyStateLifetime) + s.endpointGC(bind, steadyState) + case <-s.lifetimesChanged: + bind, steadyState = s.getLifetimes() + ticker.Reset(bind) case <-s.closeCh: return } diff --git a/net/udprelay/server_test.go b/net/udprelay/server_test.go index 6ba52db7b..80d424029 100644 --- a/net/udprelay/server_test.go +++ b/net/udprelay/server_test.go @@ -474,6 +474,87 @@ func TestServer_maybeRotateMACSecretLocked(t *testing.T) { qt.Assert(t, s.macSecrets.At(0), qt.Not(qt.Equals), s.macSecrets.At(1)) } +// TestServer_SetLifetimes verifies [Server.SetLifetimes] validates its +// arguments and mutates the lifetimes used by endpoint GC. +func TestServer_SetLifetimes(t *testing.T) { + t.Parallel() + s := &Server{ + bindLifetime: defaultBindLifetime, + steadyStateLifetime: defaultSteadyStateLifetime, + lifetimesChanged: make(chan struct{}, 1), + } + for _, invalid := range [][2]time.Duration{ + {0, time.Second}, + {time.Second, 0}, + {-time.Second, time.Second}, + {time.Second, -time.Second}, + } { + if err := s.SetLifetimes(invalid[0], invalid[1]); err == nil { + t.Errorf("SetLifetimes(%v, %v) = nil; want error", invalid[0], invalid[1]) + } + } + if bind, steadyState := s.getLifetimes(); bind != defaultBindLifetime || steadyState != defaultSteadyStateLifetime { + t.Fatalf("got lifetimes %v/%v after invalid SetLifetimes calls; want unchanged defaults %v/%v", bind, steadyState, defaultBindLifetime, defaultSteadyStateLifetime) + } + if err := s.SetLifetimes(time.Second, 2*time.Second); err != nil { + t.Fatalf("SetLifetimes(1s, 2s) = %v; want nil", err) + } + if bind, steadyState := s.getLifetimes(); bind != time.Second || steadyState != 2*time.Second { + t.Fatalf("got lifetimes %v/%v; want 1s/2s", bind, steadyState) + } + select { + case <-s.lifetimesChanged: + default: + t.Fatal("SetLifetimes did not poke lifetimesChanged") + } +} + +// TestServer_SetLifetimesForTest verifies [Server.SetLifetimesForTest] +// shortens the endpoint lifetimes of an already-running [Server], and that +// the GC loop picks up the new values, i.e. an unbound endpoint is reaped +// within the new bind lifetime instead of [defaultBindLifetime]. +func TestServer_SetLifetimesForTest(t *testing.T) { + deregisterMetrics() + server, err := NewServer(t.Logf, 0, true, new(usermetric.Registry), nil) + if err != nil { + t.Fatal(err) + } + defer server.Close() + if bind, steadyState := server.getLifetimes(); bind != defaultBindLifetime || steadyState != defaultSteadyStateLifetime { + t.Fatalf("got lifetimes %v/%v; want defaults %v/%v", bind, steadyState, defaultBindLifetime, defaultSteadyStateLifetime) + } + server.SetStaticAddrPorts(views.SliceOf([]netip.AddrPort{netip.MustParseAddrPort("127.0.0.1:1")})) + server.SetLifetimesForTest(10*time.Millisecond, 10*time.Millisecond) + + ep, err := server.AllocateEndpoint(key.NewDisco().Public(), key.NewDisco().Public()) + if err != nil { + t.Fatal(err) + } + if ep.BindLifetime.Duration != 10*time.Millisecond { + t.Errorf("endpoint BindLifetime = %v; want %v", ep.BindLifetime.Duration, 10*time.Millisecond) + } + if ep.SteadyStateLifetime.Duration != 10*time.Millisecond { + t.Errorf("endpoint SteadyStateLifetime = %v; want %v", ep.SteadyStateLifetime.Duration, 10*time.Millisecond) + } + + // The endpoint is never bound, so the GC loop must reap it once it is + // older than the 10ms bind lifetime. With the default 30s lifetime this + // would not happen before the deadline below. + deadline := time.Now().Add(5 * time.Second) + for { + server.mu.Lock() + remaining := len(server.serverEndpointByDisco) + server.mu.Unlock() + if remaining == 0 { + break + } + if time.Now().After(deadline) { + t.Fatalf("endpoint was not reaped within %v of allocation", 5*time.Second) + } + time.Sleep(5 * time.Millisecond) + } +} + func TestServer_endpointGC(t *testing.T) { for _, tc := range []struct { name string