diff --git a/derp/derp_client.go b/derp/derp_client.go index 2889d81ab..6da93521b 100644 --- a/derp/derp_client.go +++ b/derp/derp_client.go @@ -10,12 +10,14 @@ import ( "errors" "fmt" "io" + "net" "net/netip" "sync" "time" "go4.org/mem" "golang.org/x/time/rate" + "tailscale.com/net/tcpinfo" "tailscale.com/syncs" "tailscale.com/types/key" "tailscale.com/types/logger" @@ -621,3 +623,16 @@ func (c *Client) LocalAddr() (netip.AddrPort, error) { } return netip.ParseAddrPort(a.String()) } + +// RTT returns the current TCP round-trip time (RTT) between the current node +// and the DERP server, or an error if this is unimplemented on the current +// platform or the value cannot be retrieved. +func (c *Client) RTT() (time.Duration, error) { + // If the underlying value isn't a net.Conn, it's something that we + // don't support; abort now. + nc, ok := c.nc.(net.Conn) + if !ok { + return 0, tcpinfo.ErrNotTCP + } + return tcpinfo.RTT(nc) +} diff --git a/derp/derphttp/derphttp_client.go b/derp/derphttp/derphttp_client.go index 07317fcbf..cb5d4acae 100644 --- a/derp/derphttp/derphttp_client.go +++ b/derp/derphttp/derphttp_client.go @@ -851,7 +851,7 @@ func (c *Client) SendPing(data [8]byte) error { return ErrClientClosed } if client == nil { - return errors.New("client not connected") + return ErrClientNotConnected } return client.SendPing(data) } @@ -866,11 +866,26 @@ func (c *Client) LocalAddr() (netip.AddrPort, error) { return netip.AddrPort{}, ErrClientClosed } if client == nil { - return netip.AddrPort{}, errors.New("client not connected") + return netip.AddrPort{}, ErrClientNotConnected } return client.LocalAddr() } +// RTT reports the TCP RTT for the currently-active DERP connection, without +// any implicit connect or reconnect. +func (c *Client) RTT() (time.Duration, error) { + c.mu.Lock() + closed, client := c.closed, c.client + c.mu.Unlock() + if closed { + return 0, ErrClientClosed + } + if client == nil { + return 0, ErrClientNotConnected + } + return client.RTT() +} + func (c *Client) ForwardPacket(from, to key.NodePublic, b []byte) error { client, _, err := c.connect(context.TODO(), "derphttp.Client.ForwardPacket") if err != nil { @@ -1045,6 +1060,7 @@ func (c *Client) closeForReconnect(brokenClient *derp.Client) { } var ErrClientClosed = errors.New("derphttp.Client closed") +var ErrClientNotConnected = errors.New("client not connected") func parseMetaCert(certs []*x509.Certificate) (serverPub key.NodePublic, serverProtoVersion int) { for _, cert := range certs { diff --git a/net/netcheck/netcheck.go b/net/netcheck/netcheck.go index 676c5e695..7430ff288 100644 --- a/net/netcheck/netcheck.go +++ b/net/netcheck/netcheck.go @@ -790,10 +790,20 @@ func (c *Client) udpBindAddr() string { return ":0" } +// GetReportOptions contains options that can be passed to GetReport. +type GetReportOptions struct { + // DERPRegionLatencyBounds contains upper bounds for the latency to a + // given DERP region, typically determined by having an existing open + // connection to that region. This is used to bound the latency + // determined for a region when selecting a PreferredDERP ("home DERP") + // region. + DERPRegionLatencyBounds map[int]time.Duration +} + // GetReport gets a report. // // It may not be called concurrently with itself. -func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (_ *Report, reterr error) { +func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap, opts *GetReportOptions) (_ *Report, reterr error) { defer func() { if reterr != nil { metricNumGetReportError.Add(1) @@ -870,7 +880,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (_ *Report, if err := c.runHTTPOnlyChecks(ctx, last, rs, dm); err != nil { return nil, err } - return c.finishAndStoreReport(rs, dm), nil + return c.finishAndStoreReport(rs, dm, opts), nil } var ifState *interfaces.State @@ -1104,15 +1114,15 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap) (_ *Report, // Wait for captive portal check before finishing the report. <-captivePortalDone - return c.finishAndStoreReport(rs, dm), nil + return c.finishAndStoreReport(rs, dm, opts), nil } -func (c *Client) finishAndStoreReport(rs *reportState, dm *tailcfg.DERPMap) *Report { +func (c *Client) finishAndStoreReport(rs *reportState, dm *tailcfg.DERPMap, opts *GetReportOptions) *Report { rs.mu.Lock() report := rs.report.Clone() rs.mu.Unlock() - c.addReportHistoryAndSetPreferredDERP(report, dm.View()) + c.addReportHistoryAndSetPreferredDERP(report, dm.View(), opts) c.logConciseReport(report, dm) return report @@ -1457,7 +1467,7 @@ const ( // addReportHistoryAndSetPreferredDERP adds r to the set of recent Reports // and mutates r.PreferredDERP to contain the best recent one. -func (c *Client) addReportHistoryAndSetPreferredDERP(r *Report, dm tailcfg.DERPMapView) { +func (c *Client) addReportHistoryAndSetPreferredDERP(r *Report, dm tailcfg.DERPMapView, opts *GetReportOptions) { c.mu.Lock() defer c.mu.Unlock() @@ -1489,6 +1499,20 @@ func (c *Client) addReportHistoryAndSetPreferredDERP(r *Report, dm tailcfg.DERPM } } + // Bound each region's latency by the TCP RTT, if we have that option. + if opts != nil { + for regionID, bound := range opts.DERPRegionLatencyBounds { + curr, ok := bestRecent[regionID] + if !ok { + continue + } + + if curr > bound { + bestRecent[regionID] = bound + } + } + } + // Scale each region's best latency by any provided scores from the // DERPMap, for use in comparison below. var scores views.Map[int, float64] diff --git a/net/netcheck/netcheck_test.go b/net/netcheck/netcheck_test.go index 8ded621e6..3f46d4210 100644 --- a/net/netcheck/netcheck_test.go +++ b/net/netcheck/netcheck_test.go @@ -166,7 +166,7 @@ func TestBasic(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) defer cancel() - r, err := c.GetReport(ctx, stuntest.DERPMapOf(stunAddr.String())) + r, err := c.GetReport(ctx, stuntest.DERPMapOf(stunAddr.String()), nil) if err != nil { t.Fatal(err) } @@ -205,7 +205,7 @@ func TestWorksWhenUDPBlocked(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond) defer cancel() - r, err := c.GetReport(ctx, dm) + r, err := c.GetReport(ctx, dm, nil) if err != nil { t.Fatal(err) } @@ -401,7 +401,7 @@ func TestAddReportHistoryAndSetPreferredDERP(t *testing.T) { dm := &tailcfg.DERPMap{HomeParams: tt.homeParams} for _, s := range tt.steps { fakeTime = fakeTime.Add(s.after) - c.addReportHistoryAndSetPreferredDERP(s.r, dm.View()) + c.addReportHistoryAndSetPreferredDERP(s.r, dm.View(), nil) } lastReport := tt.steps[len(tt.steps)-1].r if got, want := len(c.prev), tt.wantPrevLen; got != want { @@ -862,7 +862,7 @@ func TestNoCaptivePortalWhenUDP(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) defer cancel() - r, err := c.GetReport(ctx, stuntest.DERPMapOf(stunAddr.String())) + r, err := c.GetReport(ctx, stuntest.DERPMapOf(stunAddr.String()), nil) if err != nil { t.Fatal(err) } diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 9277fce00..972deac22 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -841,19 +841,40 @@ func (c *Conn) setNetInfoHavePortMap() { func (c *Conn) updateNetInfo(ctx context.Context) (*netcheck.Report, error) { c.mu.Lock() dm := c.derpMap + + // We want to get the active RTTs for all open DERP connections, since + // we can use this to bound the latency to a given DERP region. + // However, we don't want to hold the mutex while making a bunch of + // syscalls; grab all clients here, and then actually fetch them below. + openDERPs := make(map[int]*derphttp.Client, len(c.activeDerp)) + for regionID, ad := range c.activeDerp { + openDERPs[regionID] = ad.c + } + c.mu.Unlock() if dm == nil || c.networkDown() { return new(netcheck.Report), nil } + derpBounds := make(map[int]time.Duration, len(openDERPs)) + for regionID, dclient := range openDERPs { + if dur, err := dclient.RTT(); err == nil { + derpBounds[regionID] = dur + } else { + c.dlogf("[v1] magicsock: error fetching RTT for region %d: %v", regionID, err) + } + } + ctx, cancel := context.WithTimeout(ctx, 2*time.Second) defer cancel() c.stunReceiveFunc.Store(c.netChecker.ReceiveSTUNPacket) defer c.ignoreSTUNPackets() - report, err := c.netChecker.GetReport(ctx, dm) + report, err := c.netChecker.GetReport(ctx, dm, &netcheck.GetReportOptions{ + DERPRegionLatencyBounds: derpBounds, + }) if err != nil { return nil, err }