diff --git a/control/controlclient/map.go b/control/controlclient/map.go index 2f69d57ac..176f89a53 100644 --- a/control/controlclient/map.go +++ b/control/controlclient/map.go @@ -5,9 +5,15 @@ import ( "context" + "encoding/json" "fmt" + "net" "net/netip" + "reflect" + "slices" "sort" + "strconv" + "sync" "tailscale.com/envknob" "tailscale.com/tailcfg" @@ -17,6 +23,7 @@ "tailscale.com/types/netmap" "tailscale.com/types/ptr" "tailscale.com/types/views" + "tailscale.com/util/clientmetric" "tailscale.com/util/cmpx" "tailscale.com/wgengine/filter" ) @@ -183,6 +190,8 @@ func (ms *mapSession) HandleNonKeepAliveMapResponse(ctx context.Context, resp *t // Call Node.InitDisplayNames on any changed nodes. initDisplayNames(cmpx.Or(resp.Node.View(), ms.lastNode), resp) + ms.patchifyPeersChanged(resp) + ms.updateStateFromResponse(resp) nm := ms.netmap() @@ -278,6 +287,22 @@ func (ms *mapSession) updateStateFromResponse(resp *tailcfg.MapResponse) { } } +var ( + patchDERPRegion = clientmetric.NewCounter("controlclient_patch_derp") + patchEndpoints = clientmetric.NewCounter("controlclient_patch_endpoints") + patchCap = clientmetric.NewCounter("controlclient_patch_capver") + patchKey = clientmetric.NewCounter("controlclient_patch_key") + patchDiscoKey = clientmetric.NewCounter("controlclient_patch_discokey") + patchOnline = clientmetric.NewCounter("controlclient_patch_online") + patchLastSeen = clientmetric.NewCounter("controlclient_patch_lastseen") + patchKeyExpiry = clientmetric.NewCounter("controlclient_patch_keyexpiry") + patchCapabilities = clientmetric.NewCounter("controlclient_patch_capabilities") + patchKeySignature = clientmetric.NewCounter("controlclient_patch_keysig") + + patchifiedPeer = clientmetric.NewCounter("controlclient_patchified_peer") + patchifiedPeerEqual = clientmetric.NewCounter("controlclient_patchified_peer_equal") +) + // updatePeersStateFromResponseres updates ms.peers and ms.sortedPeers from res. It takes ownership of res. func (ms *mapSession) updatePeersStateFromResponse(resp *tailcfg.MapResponse) (stats updateStats) { defer func() { @@ -362,33 +387,43 @@ func (ms *mapSession) updatePeersStateFromResponse(resp *tailcfg.MapResponse) (s mut := vp.AsStruct() if pc.DERPRegion != 0 { mut.DERP = fmt.Sprintf("%s:%v", tailcfg.DerpMagicIP, pc.DERPRegion) + patchDERPRegion.Add(1) } if pc.Cap != 0 { mut.Cap = pc.Cap + patchCap.Add(1) } if pc.Endpoints != nil { mut.Endpoints = pc.Endpoints + patchEndpoints.Add(1) } if pc.Key != nil { mut.Key = *pc.Key + patchKey.Add(1) } if pc.DiscoKey != nil { mut.DiscoKey = *pc.DiscoKey + patchDiscoKey.Add(1) } if v := pc.Online; v != nil { mut.Online = ptr.To(*v) + patchOnline.Add(1) } if v := pc.LastSeen; v != nil { mut.LastSeen = ptr.To(*v) + patchLastSeen.Add(1) } if v := pc.KeyExpiry; v != nil { mut.KeyExpiry = *v + patchKeyExpiry.Add(1) } if v := pc.Capabilities; v != nil { mut.Capabilities = *v + patchCapabilities.Add(1) } if v := pc.KeySignature; v != nil { mut.KeySignature = v + patchKeySignature.Add(1) } *vp = mut.View() } @@ -428,6 +463,217 @@ func (ms *mapSession) addUserProfile(nm *netmap.NetworkMap, userID tailcfg.UserI } } +var debugPatchifyPeer = envknob.RegisterBool("TS_DEBUG_PATCHIFY_PEER") + +// patchifyPeersChanged mutates resp to promote PeersChanged entries to PeersChangedPatch +// when possible. +func (ms *mapSession) patchifyPeersChanged(resp *tailcfg.MapResponse) { + filtered := resp.PeersChanged[:0] + for _, n := range resp.PeersChanged { + if p, ok := ms.patchifyPeer(n); ok { + patchifiedPeer.Add(1) + if debugPatchifyPeer() { + patchj, _ := json.Marshal(p) + ms.logf("debug: patchifyPeer[ID=%v]: %s", n.ID, patchj) + } + if p != nil { + resp.PeersChangedPatch = append(resp.PeersChangedPatch, p) + } else { + patchifiedPeerEqual.Add(1) + } + } else { + filtered = append(filtered, n) + } + } + resp.PeersChanged = filtered + if len(resp.PeersChanged) == 0 { + resp.PeersChanged = nil + } +} + +var nodeFields = sync.OnceValue(getNodeFields) + +// getNodeFields returns the fails of tailcfg.Node. +func getNodeFields() []string { + rt := reflect.TypeOf((*tailcfg.Node)(nil)).Elem() + ret := make([]string, rt.NumField()) + for i := 0; i < rt.NumField(); i++ { + ret[i] = rt.Field(i).Name + } + return ret +} + +// patchifyPeer returns a *tailcfg.PeerChange of the session's existing copy of +// the n.ID Node to n. +// +// It returns ok=false if a patch can't be made, (V, ok) on a delta, or (nil, +// true) if all the fields were identical (a zero change). +func (ms *mapSession) patchifyPeer(n *tailcfg.Node) (_ *tailcfg.PeerChange, ok bool) { + was, ok := ms.peers[n.ID] + if !ok { + return nil, false + } + return peerChangeDiff(*was, n) +} + +// peerChangeDiff returns the difference from 'was' to 'n', if possible. +// +// It returns (nil, true) if the fields were identical. +func peerChangeDiff(was tailcfg.NodeView, n *tailcfg.Node) (_ *tailcfg.PeerChange, ok bool) { + var ret *tailcfg.PeerChange + pc := func() *tailcfg.PeerChange { + if ret == nil { + ret = new(tailcfg.PeerChange) + } + return ret + } + for _, field := range nodeFields() { + switch field { + default: + // The whole point of using reflect in this function is to panic + // here in tests if we forget to handle a new field. + panic("unhandled field: " + field) + case "computedHostIfDifferent", "ComputedName", "ComputedNameWithHost": + // Caller's responsibility to have populated these. + continue + case "DataPlaneAuditLogID": + // Not sent for peers. + case "ID": + if was.ID() != n.ID { + return nil, false + } + case "StableID": + if was.StableID() != n.StableID { + return nil, false + } + case "Name": + if was.Name() != n.Name { + return nil, false + } + case "User": + if was.User() != n.User { + return nil, false + } + case "Sharer": + if was.Sharer() != n.Sharer { + return nil, false + } + case "Key": + if was.Key() != n.Key { + pc().Key = ptr.To(n.Key) + } + case "KeyExpiry": + if !was.KeyExpiry().Equal(n.KeyExpiry) { + pc().KeyExpiry = ptr.To(n.KeyExpiry) + } + case "KeySignature": + if !was.KeySignature().Equal(n.KeySignature) { + pc().KeySignature = slices.Clone(n.KeySignature) + } + case "Machine": + if was.Machine() != n.Machine { + return nil, false + } + case "DiscoKey": + if was.DiscoKey() != n.DiscoKey { + pc().DiscoKey = ptr.To(n.DiscoKey) + } + case "Addresses": + if !views.SliceEqual(was.Addresses(), views.SliceOf(n.Addresses)) { + return nil, false + } + case "AllowedIPs": + if !views.SliceEqual(was.AllowedIPs(), views.SliceOf(n.AllowedIPs)) { + return nil, false + } + case "Endpoints": + if !views.SliceEqual(was.Endpoints(), views.SliceOf(n.Endpoints)) { + pc().Endpoints = slices.Clone(n.Endpoints) + } + case "DERP": + if was.DERP() != n.DERP { + ip, portStr, err := net.SplitHostPort(n.DERP) + if err != nil || ip != "127.3.3.40" { + return nil, false + } + port, err := strconv.Atoi(portStr) + if err != nil || port < 1 || port > 65535 { + return nil, false + } + pc().DERPRegion = port + } + case "Hostinfo": + if !was.Hostinfo().Valid() && !n.Hostinfo.Valid() { + continue + } + if !was.Hostinfo().Valid() || !n.Hostinfo.Valid() { + return nil, false + } + if !was.Hostinfo().Equal(n.Hostinfo) { + return nil, false + } + case "Created": + if !was.Created().Equal(n.Created) { + return nil, false + } + case "Cap": + if was.Cap() != n.Cap { + pc().Cap = n.Cap + } + case "Tags": + if !views.SliceEqual(was.Tags(), views.SliceOf(n.Tags)) { + return nil, false + } + case "PrimaryRoutes": + if !views.SliceEqual(was.PrimaryRoutes(), views.SliceOf(n.PrimaryRoutes)) { + return nil, false + } + case "Online": + wasOnline := was.Online() + if n.Online != nil && wasOnline != nil && *n.Online != *wasOnline { + pc().Online = ptr.To(*n.Online) + } + case "LastSeen": + wasSeen := was.LastSeen() + if n.LastSeen != nil && wasSeen != nil && !wasSeen.Equal(*n.LastSeen) { + pc().LastSeen = ptr.To(*n.LastSeen) + } + case "MachineAuthorized": + if was.MachineAuthorized() != n.MachineAuthorized { + return nil, false + } + case "Capabilities": + if !views.SliceEqual(was.Capabilities(), views.SliceOf(n.Capabilities)) { + pc().Capabilities = ptr.To(n.Capabilities) + } + case "UnsignedPeerAPIOnly": + if was.UnsignedPeerAPIOnly() != n.UnsignedPeerAPIOnly { + return nil, false + } + case "IsWireGuardOnly": + if was.IsWireGuardOnly() != n.IsWireGuardOnly { + return nil, false + } + case "Expired": + if was.Expired() != n.Expired { + return nil, false + } + case "SelfNodeV4MasqAddrForThisPeer": + va, vb := was.SelfNodeV4MasqAddrForThisPeer(), n.SelfNodeV4MasqAddrForThisPeer + if va == nil && vb == nil { + continue + } + if va == nil || vb == nil || *va != *vb { + return nil, false + } + } + } + if ret != nil { + ret.NodeID = n.ID + } + return ret, true +} + // netmap returns a fully populated NetworkMap from the last state seen from // a call to updateStateFromResponse, filling in omitted // information from prior MapResponse values. diff --git a/control/controlclient/map_test.go b/control/controlclient/map_test.go index 231f9d7ca..1b54aecb7 100644 --- a/control/controlclient/map_test.go +++ b/control/controlclient/map_test.go @@ -4,6 +4,7 @@ package controlclient import ( + "bytes" "context" "encoding/json" "fmt" @@ -14,6 +15,7 @@ "testing" "time" + "github.com/google/go-cmp/cmp" "go4.org/mem" "tailscale.com/tailcfg" "tailscale.com/tstest" @@ -642,6 +644,132 @@ type step struct { } } +func TestPeerChangeDiff(t *testing.T) { + tests := []struct { + name string + a, b *tailcfg.Node + want *tailcfg.PeerChange // nil means want ok=false, unless wantEqual is set + wantEqual bool // means test wants (nil, true) + }{ + { + name: "eq", + a: &tailcfg.Node{ID: 1}, + b: &tailcfg.Node{ID: 1}, + wantEqual: true, + }, + { + name: "patch-derp", + a: &tailcfg.Node{ID: 1, DERP: "127.3.3.40:1"}, + b: &tailcfg.Node{ID: 1, DERP: "127.3.3.40:2"}, + want: &tailcfg.PeerChange{NodeID: 1, DERPRegion: 2}, + }, + { + name: "patch-endpoints", + a: &tailcfg.Node{ID: 1, Endpoints: []string{"10.0.0.1:1"}}, + b: &tailcfg.Node{ID: 1, Endpoints: []string{"10.0.0.2:2"}}, + want: &tailcfg.PeerChange{NodeID: 1, Endpoints: []string{"10.0.0.2:2"}}, + }, + { + name: "patch-cap", + a: &tailcfg.Node{ID: 1, Cap: 1}, + b: &tailcfg.Node{ID: 1, Cap: 2}, + want: &tailcfg.PeerChange{NodeID: 1, Cap: 2}, + }, + { + name: "patch-lastseen", + a: &tailcfg.Node{ID: 1, LastSeen: ptr.To(time.Unix(1, 0))}, + b: &tailcfg.Node{ID: 1, LastSeen: ptr.To(time.Unix(2, 0))}, + want: &tailcfg.PeerChange{NodeID: 1, LastSeen: ptr.To(time.Unix(2, 0))}, + }, + { + name: "patch-capabilities-to-nonempty", + a: &tailcfg.Node{ID: 1, Capabilities: []string{"foo"}}, + b: &tailcfg.Node{ID: 1, Capabilities: []string{"bar"}}, + want: &tailcfg.PeerChange{NodeID: 1, Capabilities: ptr.To([]string{"bar"})}, + }, + { + name: "patch-capabilities-to-empty", + a: &tailcfg.Node{ID: 1, Capabilities: []string{"foo"}}, + b: &tailcfg.Node{ID: 1}, + want: &tailcfg.PeerChange{NodeID: 1, Capabilities: ptr.To([]string(nil))}, + }, + { + name: "patch-online-to-true", + a: &tailcfg.Node{ID: 1, Online: ptr.To(false)}, + b: &tailcfg.Node{ID: 1, Online: ptr.To(true)}, + want: &tailcfg.PeerChange{NodeID: 1, Online: ptr.To(true)}, + }, + { + name: "patch-online-to-false", + a: &tailcfg.Node{ID: 1, Online: ptr.To(true)}, + b: &tailcfg.Node{ID: 1, Online: ptr.To(false)}, + want: &tailcfg.PeerChange{NodeID: 1, Online: ptr.To(false)}, + }, + { + name: "mix-patchable-and-not", + a: &tailcfg.Node{ID: 1, Cap: 1}, + b: &tailcfg.Node{ID: 1, Cap: 2, StableID: "foo"}, + want: nil, + }, + { + name: "miss-change-stableid", + a: &tailcfg.Node{ID: 1}, + b: &tailcfg.Node{ID: 1, StableID: "diff"}, + want: nil, + }, + { + name: "miss-change-id", + a: &tailcfg.Node{ID: 1}, + b: &tailcfg.Node{ID: 2}, + want: nil, + }, + { + name: "miss-change-name", + a: &tailcfg.Node{ID: 1, Name: "foo"}, + b: &tailcfg.Node{ID: 1, Name: "bar"}, + want: nil, + }, + { + name: "miss-change-user", + a: &tailcfg.Node{ID: 1, User: 1}, + b: &tailcfg.Node{ID: 1, User: 2}, + want: nil, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pc, ok := peerChangeDiff(tt.a.View(), tt.b) + if tt.wantEqual { + if !ok || pc != nil { + t.Errorf("got (%p, %v); want (nil, true); pc=%v", pc, ok, must.Get(json.Marshal(pc))) + } + return + } + if (pc != nil) != ok { + t.Fatalf("inconsistent ok=%v, pc=%p", ok, pc) + } + gotj := must.Get(json.Marshal(pc)) + wantj := must.Get(json.Marshal(tt.want)) + if !bytes.Equal(gotj, wantj) { + t.Errorf("mismatch\n got: %s\nwant: %s\n", gotj, wantj) + } + }) + } +} + +func TestPeerChangeDiffAllocs(t *testing.T) { + a := &tailcfg.Node{ID: 1} + b := &tailcfg.Node{ID: 1} + n := testing.AllocsPerRun(10000, func() { + diff, ok := peerChangeDiff(a.View(), b) + if !ok || diff != nil { + t.Fatalf("unexpected result: (%s, %v)", must.Get(json.Marshal(diff)), ok) + } + }) + if n != 0 { + t.Errorf("allocs = %v; want 0", int(n)) + } +} + type countingNetmapUpdater struct { full atomic.Int64 } @@ -650,6 +778,80 @@ func (nu *countingNetmapUpdater) UpdateFullNetmap(nm *netmap.NetworkMap) { nu.full.Add(1) } +// tests (*mapSession).patchifyPeersChanged; smaller tests are in TestPeerChangeDiff +func TestPatchifyPeersChanged(t *testing.T) { + hi := (&tailcfg.Hostinfo{}).View() + tests := []struct { + name string + mr0 *tailcfg.MapResponse // initial + mr1 *tailcfg.MapResponse // incremental + want *tailcfg.MapResponse // what the incremental one should've been mutated to + }{ + { + name: "change_one_endpoint", + mr0: &tailcfg.MapResponse{ + Node: &tailcfg.Node{Name: "foo.bar.ts.net."}, + Peers: []*tailcfg.Node{ + {ID: 1, Hostinfo: hi}, + }, + }, + mr1: &tailcfg.MapResponse{ + PeersChanged: []*tailcfg.Node{ + {ID: 1, Endpoints: []string{"10.0.0.1:1111"}, Hostinfo: hi}, + }, + }, + want: &tailcfg.MapResponse{ + PeersChanged: nil, + PeersChangedPatch: []*tailcfg.PeerChange{ + {NodeID: 1, Endpoints: []string{"10.0.0.1:1111"}}, + }, + }, + }, + { + name: "change_some", + mr0: &tailcfg.MapResponse{ + Node: &tailcfg.Node{Name: "foo.bar.ts.net."}, + Peers: []*tailcfg.Node{ + {ID: 1, DERP: "127.3.3.40:1", Hostinfo: hi}, + {ID: 2, DERP: "127.3.3.40:2", Hostinfo: hi}, + {ID: 3, DERP: "127.3.3.40:3", Hostinfo: hi}, + }, + }, + mr1: &tailcfg.MapResponse{ + PeersChanged: []*tailcfg.Node{ + {ID: 1, DERP: "127.3.3.40:11", Hostinfo: hi}, + {ID: 2, StableID: "other-change", Hostinfo: hi}, + {ID: 3, DERP: "127.3.3.40:33", Hostinfo: hi}, + {ID: 4, DERP: "127.3.3.40:4", Hostinfo: hi}, + }, + }, + want: &tailcfg.MapResponse{ + PeersChanged: []*tailcfg.Node{ + {ID: 2, StableID: "other-change", Hostinfo: hi}, + {ID: 4, DERP: "127.3.3.40:4", Hostinfo: hi}, + }, + PeersChangedPatch: []*tailcfg.PeerChange{ + {NodeID: 1, DERPRegion: 11}, + {NodeID: 3, DERPRegion: 33}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + nu := &countingNetmapUpdater{} + ms := newTestMapSession(t, nu) + ms.updateStateFromResponse(tt.mr0) + mr1 := new(tailcfg.MapResponse) + must.Do(json.Unmarshal(must.Get(json.Marshal(tt.mr1)), mr1)) + ms.patchifyPeersChanged(mr1) + if diff := cmp.Diff(tt.want, mr1); diff != "" { + t.Errorf("wrong result (-want +got):\n%s", diff) + } + }) + } +} + func BenchmarkMapSessionDelta(b *testing.B) { for _, size := range []int{10, 100, 1_000, 10_000} { b.Run(fmt.Sprintf("size_%d", size), func(b *testing.B) { @@ -700,5 +902,4 @@ func BenchmarkMapSessionDelta(b *testing.B) { } }) } - }