Ver Fonte

net/netcheck, tailcfg: add DERPHomeParams and use it

This allows providing additional information to the client about how to
select a home DERP region, such as preferring a given DERP region over
all others.

Updates #8603

Signed-off-by: Andrew Dunham <[email protected]>
Change-Id: I7c4a270f31d8585112fab5408799ffba5b75266f
Andrew Dunham há 2 anos atrás
pai
commit
7aba0b0d78

+ 21 - 2
control/controlclient/map.go

@@ -90,9 +90,28 @@ func (ms *mapSession) netmapForResponse(resp *tailcfg.MapResponse) *netmap.Netwo
 		ms.lastUserProfile[up.ID] = up
 	}
 
-	if resp.DERPMap != nil {
+	if dm := resp.DERPMap; dm != nil {
 		ms.vlogf("netmap: new map contains DERP map")
-		ms.lastDERPMap = resp.DERPMap
+
+		// Zero-valued fields in a DERPMap mean that we're not changing
+		// anything and are using the previous value(s).
+		if ldm := ms.lastDERPMap; ldm != nil {
+			if dm.Regions == nil {
+				dm.Regions = ldm.Regions
+				dm.OmitDefaultRegions = ldm.OmitDefaultRegions
+			}
+			if dm.HomeParams == nil {
+				dm.HomeParams = ldm.HomeParams
+			} else if oldhh := ldm.HomeParams; oldhh != nil {
+				// Propagate sub-fields of HomeParams
+				hh := dm.HomeParams
+				if hh.RegionScore == nil {
+					hh.RegionScore = oldhh.RegionScore
+				}
+			}
+		}
+
+		ms.lastDERPMap = dm
 	}
 
 	if pf := resp.PacketFilter; pf != nil {

+ 105 - 0
control/controlclient/map_test.go

@@ -619,3 +619,108 @@ func TestCopyDebugOptBools(t *testing.T) {
 		}
 	}
 }
+
+func TestDeltaDERPMap(t *testing.T) {
+	regions1 := map[int]*tailcfg.DERPRegion{
+		1: {
+			RegionID: 1,
+			Nodes: []*tailcfg.DERPNode{{
+				Name:     "derp1a",
+				RegionID: 1,
+				HostName: "derp1a" + tailcfg.DotInvalid,
+				IPv4:     "169.254.169.254",
+				IPv6:     "none",
+			}},
+		},
+	}
+
+	// As above, but with a changed IPv4 addr
+	regions2 := map[int]*tailcfg.DERPRegion{1: regions1[1].Clone()}
+	regions2[1].Nodes[0].IPv4 = "127.0.0.1"
+
+	type step struct {
+		got  *tailcfg.DERPMap
+		want *tailcfg.DERPMap
+	}
+	tests := []struct {
+		name  string
+		steps []step
+	}{
+		{
+			name: "nothing-to-nothing",
+			steps: []step{
+				{nil, nil},
+				{nil, nil},
+			},
+		},
+		{
+			name: "regions-sticky",
+			steps: []step{
+				{&tailcfg.DERPMap{Regions: regions1}, &tailcfg.DERPMap{Regions: regions1}},
+				{&tailcfg.DERPMap{}, &tailcfg.DERPMap{Regions: regions1}},
+			},
+		},
+		{
+			name: "regions-change",
+			steps: []step{
+				{&tailcfg.DERPMap{Regions: regions1}, &tailcfg.DERPMap{Regions: regions1}},
+				{&tailcfg.DERPMap{Regions: regions2}, &tailcfg.DERPMap{Regions: regions2}},
+			},
+		},
+		{
+			name: "home-params",
+			steps: []step{
+				// Send a DERP map
+				{&tailcfg.DERPMap{Regions: regions1}, &tailcfg.DERPMap{Regions: regions1}},
+				// Send home params, want to still have the same regions
+				{
+					&tailcfg.DERPMap{HomeParams: &tailcfg.DERPHomeParams{
+						RegionScore: map[int]float64{1: 0.5},
+					}},
+					&tailcfg.DERPMap{Regions: regions1, HomeParams: &tailcfg.DERPHomeParams{
+						RegionScore: map[int]float64{1: 0.5},
+					}},
+				},
+			},
+		},
+		{
+			name: "home-params-sub-fields",
+			steps: []step{
+				// Send a DERP map with home params
+				{
+					&tailcfg.DERPMap{Regions: regions1, HomeParams: &tailcfg.DERPHomeParams{
+						RegionScore: map[int]float64{1: 0.5},
+					}},
+					&tailcfg.DERPMap{Regions: regions1, HomeParams: &tailcfg.DERPHomeParams{
+						RegionScore: map[int]float64{1: 0.5},
+					}},
+				},
+				// Sending a struct with a 'HomeParams' field but nil RegionScore doesn't change home params...
+				{
+					&tailcfg.DERPMap{HomeParams: &tailcfg.DERPHomeParams{RegionScore: nil}},
+					&tailcfg.DERPMap{Regions: regions1, HomeParams: &tailcfg.DERPHomeParams{
+						RegionScore: map[int]float64{1: 0.5},
+					}},
+				},
+				// ... but sending one with a non-nil and empty RegionScore field zeroes that out.
+				{
+					&tailcfg.DERPMap{HomeParams: &tailcfg.DERPHomeParams{RegionScore: map[int]float64{}}},
+					&tailcfg.DERPMap{Regions: regions1, HomeParams: &tailcfg.DERPHomeParams{
+						RegionScore: map[int]float64{},
+					}},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ms := newTestMapSession(t)
+			for stepi, s := range tt.steps {
+				nm := ms.netmapForResponse(&tailcfg.MapResponse{DERPMap: s.got})
+				if !reflect.DeepEqual(nm.DERPMap, s.want) {
+					t.Errorf("unexpected result at step index %v; got: %s", stepi, must.Get(json.Marshal(nm.DERPMap)))
+				}
+			}
+		})
+	}
+}

+ 32 - 11
net/netcheck/netcheck.go

@@ -41,6 +41,7 @@ import (
 	"tailscale.com/types/nettype"
 	"tailscale.com/types/opt"
 	"tailscale.com/types/ptr"
+	"tailscale.com/types/views"
 	"tailscale.com/util/clientmetric"
 	"tailscale.com/util/cmpx"
 	"tailscale.com/util/mak"
@@ -1110,7 +1111,7 @@ func (c *Client) finishAndStoreReport(rs *reportState, dm *tailcfg.DERPMap) *Rep
 	report := rs.report.Clone()
 	rs.mu.Unlock()
 
-	c.addReportHistoryAndSetPreferredDERP(report)
+	c.addReportHistoryAndSetPreferredDERP(report, dm.View())
 	c.logConciseReport(report, dm)
 
 	return report
@@ -1444,7 +1445,7 @@ func (c *Client) timeNow() time.Time {
 
 // 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) {
+func (c *Client) addReportHistoryAndSetPreferredDERP(r *Report, dm tailcfg.DERPMapView) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
@@ -1476,11 +1477,33 @@ func (c *Client) addReportHistoryAndSetPreferredDERP(r *Report) {
 		}
 	}
 
+	// Scale each region's best latency by any provided scores from the
+	// DERPMap, for use in comparison below.
+	var scores views.Map[int, float64]
+	if hp := dm.HomeParams(); hp.Valid() {
+		scores = hp.RegionScore()
+	}
+	for regionID, d := range bestRecent {
+		if score := scores.Get(regionID); score > 0 {
+			bestRecent[regionID] = time.Duration(float64(d) * score)
+		}
+	}
+
 	// Then, pick which currently-alive DERP server from the
 	// current report has the best latency over the past maxAge.
-	var bestAny time.Duration
-	var oldRegionCurLatency time.Duration
+	var (
+		bestAny             time.Duration // global minimum
+		oldRegionCurLatency time.Duration // latency of old PreferredDERP
+	)
 	for regionID, d := range r.RegionLatency {
+		// Scale this report's latency by any scores provided by the
+		// server; we did this for the bestRecent map above, but we
+		// don't mutate the actual reports in-place (in case scores
+		// change), so we need to do it here as well.
+		if score := scores.Get(regionID); score > 0 {
+			d = time.Duration(float64(d) * score)
+		}
+
 		if regionID == prevDERP {
 			oldRegionCurLatency = d
 		}
@@ -1491,13 +1514,11 @@ func (c *Client) addReportHistoryAndSetPreferredDERP(r *Report) {
 		}
 	}
 
-	// If we're changing our preferred DERP but the old one's still
-	// accessible and the new one's not much better, just stick with
-	// where we are.
-	if prevDERP != 0 &&
-		r.PreferredDERP != prevDERP &&
-		oldRegionCurLatency != 0 &&
-		bestAny > oldRegionCurLatency/3*2 {
+	// If we're changing our preferred DERP, the old one's still
+	// accessible, and the new one's not much better, just stick
+	// with where we are.
+	changingPreferred := prevDERP != 0 && r.PreferredDERP != prevDERP
+	if changingPreferred && oldRegionCurLatency != 0 && bestAny > oldRegionCurLatency/3*2 {
 		r.PreferredDERP = prevDERP
 	}
 }

+ 49 - 1
net/netcheck/netcheck_test.go

@@ -264,6 +264,7 @@ func TestAddReportHistoryAndSetPreferredDERP(t *testing.T) {
 	tests := []struct {
 		name        string
 		steps       []step
+		homeParams  *tailcfg.DERPHomeParams
 		wantDERP    int // want PreferredDERP on final step
 		wantPrevLen int // wanted len(c.prev)
 	}{
@@ -335,6 +336,52 @@ func TestAddReportHistoryAndSetPreferredDERP(t *testing.T) {
 			wantPrevLen: 2,
 			wantDERP:    2, // 2 got fast enough
 		},
+		{
+			name: "derp_home_params",
+			homeParams: &tailcfg.DERPHomeParams{
+				RegionScore: map[int]float64{
+					1: 2.0 / 3, // 66%
+				},
+			},
+			steps: []step{
+				// We only use a single step here to avoid
+				// conflating DERP selection as a result of
+				// weight hints with the "stickiness" check
+				// that tries to not change the home DERP
+				// between steps.
+				{1 * time.Second, report("d1", 10, "d2", 8)},
+			},
+			wantPrevLen: 1,
+			wantDERP:    1, // 2 was faster, but not by 50%+
+		},
+		{
+			name: "derp_home_params_high_latency",
+			homeParams: &tailcfg.DERPHomeParams{
+				RegionScore: map[int]float64{
+					1: 2.0 / 3, // 66%
+				},
+			},
+			steps: []step{
+				// See derp_home_params for why this is a single step.
+				{1 * time.Second, report("d1", 100, "d2", 10)},
+			},
+			wantPrevLen: 1,
+			wantDERP:    2, // 2 was faster by more than 50%
+		},
+		{
+			name: "derp_home_params_invalid",
+			homeParams: &tailcfg.DERPHomeParams{
+				RegionScore: map[int]float64{
+					1: 0.0,
+					2: -1.0,
+				},
+			},
+			steps: []step{
+				{1 * time.Second, report("d1", 4, "d2", 5)},
+			},
+			wantPrevLen: 1,
+			wantDERP:    1,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -342,9 +389,10 @@ func TestAddReportHistoryAndSetPreferredDERP(t *testing.T) {
 			c := &Client{
 				TimeNow: func() time.Time { return fakeTime },
 			}
+			dm := &tailcfg.DERPMap{HomeParams: tt.homeParams}
 			for _, s := range tt.steps {
 				fakeTime = fakeTime.Add(s.after)
-				c.addReportHistoryAndSetPreferredDERP(s.r)
+				c.addReportHistoryAndSetPreferredDERP(s.r, dm.View())
 			}
 			lastReport := tt.steps[len(tt.steps)-1].r
 			if got, want := len(c.prev), tt.wantPrevLen; got != want {

+ 26 - 0
tailcfg/derpmap.go

@@ -7,6 +7,11 @@ import "sort"
 
 // DERPMap describes the set of DERP packet relay servers that are available.
 type DERPMap struct {
+	// HomeParams, if non-nil, is a change in home parameters.
+	//
+	// The rest of the DEPRMap fields, if zero, means unchanged.
+	HomeParams *DERPHomeParams `json:",omitempty"`
+
 	// Regions is the set of geographic regions running DERP node(s).
 	//
 	// It's keyed by the DERPRegion.RegionID.
@@ -16,6 +21,8 @@ type DERPMap struct {
 
 	// OmitDefaultRegions specifies to not use Tailscale's DERP servers, and only use those
 	// specified in this DERPMap. If there are none set outside of the defaults, this is a noop.
+	//
+	// This field is only meaningful if the Regions map is non-nil (indicating a change).
 	OmitDefaultRegions bool `json:"omitDefaultRegions,omitempty"`
 }
 
@@ -29,6 +36,25 @@ func (m *DERPMap) RegionIDs() []int {
 	return ret
 }
 
+// DERPHomeParams contains parameters from the server related to selecting a
+// DERP home region (sometimes referred to as the "preferred DERP").
+type DERPHomeParams struct {
+	// RegionScore scales latencies of DERP regions by a given scaling
+	// factor when determining which region to use as the home
+	// ("preferred") DERP. Scores in the range (0, 1) will cause this
+	// region to be proportionally more preferred, and scores in the range
+	// (1, ∞) will penalize a region.
+	//
+	// If a region is not present in this map, it is treated as having a
+	// score of 1.0.
+	//
+	// Scores should not be 0 or negative; such scores will be ignored.
+	//
+	// A nil map means no change from the previous value (if any); an empty
+	// non-nil map can be sent to reset all scores back to 1.0.
+	RegionScore map[int]float64 `json:",omitempty"`
+}
+
 // DERPRegion is a geographic region running DERP relay node(s).
 //
 // Client nodes discover which region they're closest to, advertise

+ 3 - 2
tailcfg/tailcfg.go

@@ -3,7 +3,7 @@
 
 package tailcfg
 
-//go:generate go run tailscale.com/cmd/viewer --type=User,Node,Hostinfo,NetInfo,Login,DNSConfig,RegisterResponse,DERPRegion,DERPMap,DERPNode,SSHRule,SSHAction,SSHPrincipal,ControlDialPlan,Location --clonefunc
+//go:generate go run tailscale.com/cmd/viewer --type=User,Node,Hostinfo,NetInfo,Login,DNSConfig,RegisterResponse,DERPHomeParams,DERPRegion,DERPMap,DERPNode,SSHRule,SSHAction,SSHPrincipal,ControlDialPlan,Location --clonefunc
 
 import (
 	"bytes"
@@ -101,7 +101,8 @@ type CapabilityVersion int
 //   - 62: 2023-05-05: Client can notify control over noise for SSHEventNotificationRequest recording failure events
 //   - 63: 2023-06-08: Client understands SSHAction.AllowRemotePortForwarding.
 //   - 64: 2023-07-11: Client understands s/CapabilityTailnetLockAlpha/CapabilityTailnetLock
-const CurrentCapabilityVersion CapabilityVersion = 64
+//   - 65: 2023-07-12: Client understands DERPMap.HomeParams + incremental DERPMap updates with params
+const CurrentCapabilityVersion CapabilityVersion = 65
 
 type StableID string
 

+ 34 - 1
tailcfg/tailcfg_clone.go

@@ -286,6 +286,28 @@ var _RegisterResponseCloneNeedsRegeneration = RegisterResponse(struct {
 	Error             string
 }{})
 
+// Clone makes a deep copy of DERPHomeParams.
+// The result aliases no memory with the original.
+func (src *DERPHomeParams) Clone() *DERPHomeParams {
+	if src == nil {
+		return nil
+	}
+	dst := new(DERPHomeParams)
+	*dst = *src
+	if dst.RegionScore != nil {
+		dst.RegionScore = map[int]float64{}
+		for k, v := range src.RegionScore {
+			dst.RegionScore[k] = v
+		}
+	}
+	return dst
+}
+
+// A compilation failure here means this code must be regenerated, with the command at the top of this file.
+var _DERPHomeParamsCloneNeedsRegeneration = DERPHomeParams(struct {
+	RegionScore map[int]float64
+}{})
+
 // Clone makes a deep copy of DERPRegion.
 // The result aliases no memory with the original.
 func (src *DERPRegion) Clone() *DERPRegion {
@@ -318,6 +340,7 @@ func (src *DERPMap) Clone() *DERPMap {
 	}
 	dst := new(DERPMap)
 	*dst = *src
+	dst.HomeParams = src.HomeParams.Clone()
 	if dst.Regions != nil {
 		dst.Regions = map[int]*DERPRegion{}
 		for k, v := range src.Regions {
@@ -329,6 +352,7 @@ func (src *DERPMap) Clone() *DERPMap {
 
 // A compilation failure here means this code must be regenerated, with the command at the top of this file.
 var _DERPMapCloneNeedsRegeneration = DERPMap(struct {
+	HomeParams         *DERPHomeParams
 	Regions            map[int]*DERPRegion
 	OmitDefaultRegions bool
 }{})
@@ -484,7 +508,7 @@ var _LocationCloneNeedsRegeneration = Location(struct {
 
 // Clone duplicates src into dst and reports whether it succeeded.
 // To succeed, <src, dst> must be of types <*T, *T> or <*T, **T>,
-// where T is one of User,Node,Hostinfo,NetInfo,Login,DNSConfig,RegisterResponse,DERPRegion,DERPMap,DERPNode,SSHRule,SSHAction,SSHPrincipal,ControlDialPlan,Location.
+// where T is one of User,Node,Hostinfo,NetInfo,Login,DNSConfig,RegisterResponse,DERPHomeParams,DERPRegion,DERPMap,DERPNode,SSHRule,SSHAction,SSHPrincipal,ControlDialPlan,Location.
 func Clone(dst, src any) bool {
 	switch src := src.(type) {
 	case *User:
@@ -550,6 +574,15 @@ func Clone(dst, src any) bool {
 			*dst = src.Clone()
 			return true
 		}
+	case *DERPHomeParams:
+		switch dst := dst.(type) {
+		case *DERPHomeParams:
+			*dst = *src.Clone()
+			return true
+		case **DERPHomeParams:
+			*dst = src.Clone()
+			return true
+		}
 	case *DERPRegion:
 		switch dst := dst.(type) {
 		case *DERPRegion:

+ 58 - 1
tailcfg/tailcfg_view.go

@@ -20,7 +20,7 @@ import (
 	"tailscale.com/types/views"
 )
 
-//go:generate go run tailscale.com/cmd/cloner  -clonefunc=true -type=User,Node,Hostinfo,NetInfo,Login,DNSConfig,RegisterResponse,DERPRegion,DERPMap,DERPNode,SSHRule,SSHAction,SSHPrincipal,ControlDialPlan,Location
+//go:generate go run tailscale.com/cmd/cloner  -clonefunc=true -type=User,Node,Hostinfo,NetInfo,Login,DNSConfig,RegisterResponse,DERPHomeParams,DERPRegion,DERPMap,DERPNode,SSHRule,SSHAction,SSHPrincipal,ControlDialPlan,Location
 
 // View returns a readonly view of User.
 func (p *User) View() UserView {
@@ -633,6 +633,60 @@ var _RegisterResponseViewNeedsRegeneration = RegisterResponse(struct {
 	Error             string
 }{})
 
+// View returns a readonly view of DERPHomeParams.
+func (p *DERPHomeParams) View() DERPHomeParamsView {
+	return DERPHomeParamsView{ж: p}
+}
+
+// DERPHomeParamsView provides a read-only view over DERPHomeParams.
+//
+// Its methods should only be called if `Valid()` returns true.
+type DERPHomeParamsView struct {
+	// ж is the underlying mutable value, named with a hard-to-type
+	// character that looks pointy like a pointer.
+	// It is named distinctively to make you think of how dangerous it is to escape
+	// to callers. You must not let callers be able to mutate it.
+	ж *DERPHomeParams
+}
+
+// Valid reports whether underlying value is non-nil.
+func (v DERPHomeParamsView) Valid() bool { return v.ж != nil }
+
+// AsStruct returns a clone of the underlying value which aliases no memory with
+// the original.
+func (v DERPHomeParamsView) AsStruct() *DERPHomeParams {
+	if v.ж == nil {
+		return nil
+	}
+	return v.ж.Clone()
+}
+
+func (v DERPHomeParamsView) MarshalJSON() ([]byte, error) { return json.Marshal(v.ж) }
+
+func (v *DERPHomeParamsView) UnmarshalJSON(b []byte) error {
+	if v.ж != nil {
+		return errors.New("already initialized")
+	}
+	if len(b) == 0 {
+		return nil
+	}
+	var x DERPHomeParams
+	if err := json.Unmarshal(b, &x); err != nil {
+		return err
+	}
+	v.ж = &x
+	return nil
+}
+
+func (v DERPHomeParamsView) RegionScore() views.Map[int, float64] {
+	return views.MapOf(v.ж.RegionScore)
+}
+
+// A compilation failure here means this code must be regenerated, with the command at the top of this file.
+var _DERPHomeParamsViewNeedsRegeneration = DERPHomeParams(struct {
+	RegionScore map[int]float64
+}{})
+
 // View returns a readonly view of DERPRegion.
 func (p *DERPRegion) View() DERPRegionView {
 	return DERPRegionView{ж: p}
@@ -740,6 +794,8 @@ func (v *DERPMapView) UnmarshalJSON(b []byte) error {
 	return nil
 }
 
+func (v DERPMapView) HomeParams() DERPHomeParamsView { return v.ж.HomeParams.View() }
+
 func (v DERPMapView) Regions() views.MapFn[int, *DERPRegion, DERPRegionView] {
 	return views.MapFnOf(v.ж.Regions, func(t *DERPRegion) DERPRegionView {
 		return t.View()
@@ -749,6 +805,7 @@ func (v DERPMapView) OmitDefaultRegions() bool { return v.ж.OmitDefaultRegions
 
 // A compilation failure here means this code must be regenerated, with the command at the top of this file.
 var _DERPMapViewNeedsRegeneration = DERPMap(struct {
+	HomeParams         *DERPHomeParams
 	Regions            map[int]*DERPRegion
 	OmitDefaultRegions bool
 }{})