diff --git a/control/controlclient/netmap.go b/control/controlclient/netmap.go index 140107c2e..37e9a171e 100644 --- a/control/controlclient/netmap.go +++ b/control/controlclient/netmap.go @@ -30,7 +30,7 @@ type NetworkMap struct { Addresses []wgcfg.CIDR LocalPort uint16 // used for debugging MachineStatus tailcfg.MachineStatus - Peers []*tailcfg.Node + Peers []*tailcfg.Node // sorted by Node.ID DNS []wgcfg.IP DNSDomains []string Hostinfo tailcfg.Hostinfo @@ -56,6 +56,13 @@ type NetworkMap struct { } func (n *NetworkMap) Equal(n2 *NetworkMap) bool { + if n == nil && n2 == nil { + return true + } + if n == nil || n2 == nil { + return false + } + // TODO(crawshaw): this is crude, but is an easy way to avoid bugs. b, err := json.Marshal(n) if err != nil { @@ -74,6 +81,19 @@ func (nm NetworkMap) String() string { func (nm *NetworkMap) Concise() string { buf := new(strings.Builder) + + nm.printConciseHeader(buf) + for _, p := range nm.Peers { + printPeerConcise(buf, p) + } + return buf.String() +} + +// printConciseHeader prints a concise header line representing nm to buf. +// +// If this function is changed to access different fields of nm, keep +// in equalConciseHeader in sync. +func (nm *NetworkMap) printConciseHeader(buf *strings.Builder) { fmt.Fprintf(buf, "netmap: self: %v auth=%v", nm.NodeKey.ShortString(), nm.MachineStatus) if nm.LocalPort != 0 { @@ -85,72 +105,116 @@ func (nm *NetworkMap) Concise() string { } fmt.Fprintf(buf, " %v", nm.Addresses) buf.WriteByte('\n') - for _, p := range nm.Peers { - aip := make([]string, len(p.AllowedIPs)) - for i, a := range p.AllowedIPs { - s := strings.TrimSuffix(fmt.Sprint(a), "/32") - aip[i] = s - } +} - ep := make([]string, len(p.Endpoints)) - for i, e := range p.Endpoints { - // Align vertically on the ':' between IP and port - colon := strings.IndexByte(e, ':') - spaces := 0 - for colon > 0 && len(e)+spaces-colon < 6 { - spaces++ - colon-- - } - ep[i] = fmt.Sprintf("%21v", e+strings.Repeat(" ", spaces)) +// equalConciseHeader reports whether a and b are equal for the fields +// used by printConciseHeader. +func (a *NetworkMap) equalConciseHeader(b *NetworkMap) bool { + if a.NodeKey != b.NodeKey || + a.MachineStatus != b.MachineStatus || + a.LocalPort != b.LocalPort || + len(a.Addresses) != len(b.Addresses) { + return false + } + for i, a := range a.Addresses { + if b.Addresses[i] != a { + return false } + } + return (a.Debug == nil && b.Debug == nil) || reflect.DeepEqual(a.Debug, b.Debug) +} + +// printPeerConcise appends to buf a line repsenting the peer p. +// +// If this function is changed to access different fields of p, keep +// in nodeConciseEqual in sync. +func printPeerConcise(buf *strings.Builder, p *tailcfg.Node) { + aip := make([]string, len(p.AllowedIPs)) + for i, a := range p.AllowedIPs { + s := strings.TrimSuffix(fmt.Sprint(a), "/32") + aip[i] = s + } - derp := p.DERP - const derpPrefix = "127.3.3.40:" - if strings.HasPrefix(derp, derpPrefix) { - derp = "D" + derp[len(derpPrefix):] + ep := make([]string, len(p.Endpoints)) + for i, e := range p.Endpoints { + // Align vertically on the ':' between IP and port + colon := strings.IndexByte(e, ':') + spaces := 0 + for colon > 0 && len(e)+spaces-colon < 6 { + spaces++ + colon-- } + ep[i] = fmt.Sprintf("%21v", e+strings.Repeat(" ", spaces)) + } - // Most of the time, aip is just one element, so format the - // table to look good in that case. This will also make multi- - // subnet nodes stand out visually. - fmt.Fprintf(buf, " %v %-2v %-15v : %v\n", - p.Key.ShortString(), derp, - strings.Join(aip, " "), - strings.Join(ep, " ")) + derp := p.DERP + const derpPrefix = "127.3.3.40:" + if strings.HasPrefix(derp, derpPrefix) { + derp = "D" + derp[len(derpPrefix):] } - return buf.String() + + // Most of the time, aip is just one element, so format the + // table to look good in that case. This will also make multi- + // subnet nodes stand out visually. + fmt.Fprintf(buf, " %v %-2v %-15v : %v\n", + p.Key.ShortString(), derp, + strings.Join(aip, " "), + strings.Join(ep, " ")) } -func (b *NetworkMap) ConciseDiffFrom(a *NetworkMap) string { - if reflect.DeepEqual(a, b) { - // Fast path that only does one allocation. - return "" - } - out := []string{} - ra := strings.Split(a.Concise(), "\n") - rb := strings.Split(b.Concise(), "\n") +// nodeConciseEqual reports whether a and b are equal for the fields accessed by printPeerConcise. +func nodeConciseEqual(a, b *tailcfg.Node) bool { + return a.Key == b.Key && + a.DERP == b.DERP && + eqCIDRsIgnoreNil(a.AllowedIPs, b.AllowedIPs) && + eqStringsIgnoreNil(a.Endpoints, b.Endpoints) +} - ma := map[string]struct{}{} - for _, s := range ra { - ma[s] = struct{}{} - } +func (b *NetworkMap) ConciseDiffFrom(a *NetworkMap) string { + var diff strings.Builder - mb := map[string]struct{}{} - for _, s := range rb { - mb[s] = struct{}{} + // See if header (non-peers, "bare") part of the network map changed. + // If so, print its diff lines first. + if !a.equalConciseHeader(b) { + diff.WriteByte('-') + a.printConciseHeader(&diff) + diff.WriteByte('+') + b.printConciseHeader(&diff) } - for _, s := range ra { - if _, ok := mb[s]; !ok { - out = append(out, "-"+s) + aps, bps := a.Peers, b.Peers + for len(aps) > 0 && len(bps) > 0 { + pa, pb := aps[0], bps[0] + switch { + case pa.ID == pb.ID: + if !nodeConciseEqual(pa, pb) { + diff.WriteByte('-') + printPeerConcise(&diff, pa) + diff.WriteByte('+') + printPeerConcise(&diff, pb) + } + aps, bps = aps[1:], bps[1:] + case pa.ID > pb.ID: + // New peer in b. + diff.WriteByte('+') + printPeerConcise(&diff, pb) + bps = bps[1:] + case pb.ID > pa.ID: + // Deleted peer in b. + diff.WriteByte('-') + printPeerConcise(&diff, pa) + aps = aps[1:] } } - for _, s := range rb { - if _, ok := ma[s]; !ok { - out = append(out, "+"+s) - } + for _, pa := range aps { + diff.WriteByte('-') + printPeerConcise(&diff, pa) } - return strings.Join(out, "\n") + for _, pb := range bps { + diff.WriteByte('+') + printPeerConcise(&diff, pb) + } + return diff.String() } func (nm *NetworkMap) JSON() string { @@ -271,3 +335,31 @@ func appendEndpoint(peer *wgcfg.Peer, epStr string) error { peer.Endpoints = append(peer.Endpoints, wgcfg.Endpoint{Host: host, Port: uint16(port16)}) return nil } + +// eqStringsIgnoreNil reports whether a and b have the same length and +// contents, but ignore whether a or b are nil. +func eqStringsIgnoreNil(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i, v := range a { + if v != b[i] { + return false + } + } + return true +} + +// eqCIDRsIgnoreNil reports whether a and b have the same length and +// contents, but ignore whether a or b are nil. +func eqCIDRsIgnoreNil(a, b []wgcfg.CIDR) bool { + if len(a) != len(b) { + return false + } + for i, v := range a { + if v != b[i] { + return false + } + } + return true +} diff --git a/control/controlclient/netmap_test.go b/control/controlclient/netmap_test.go index d9c23b93c..33cfdd201 100644 --- a/control/controlclient/netmap_test.go +++ b/control/controlclient/netmap_test.go @@ -10,13 +10,14 @@ import ( "tailscale.com/tailcfg" ) -func TestNetworkMapConcise(t *testing.T) { - nodekey := func(b byte) (ret tailcfg.NodeKey) { - for i := range ret { - ret[i] = b - } - return +func testNodeKey(b byte) (ret tailcfg.NodeKey) { + for i := range ret { + ret[i] = b } + return +} + +func TestNetworkMapConcise(t *testing.T) { for _, tt := range []struct { name string nm *NetworkMap @@ -25,15 +26,15 @@ func TestNetworkMapConcise(t *testing.T) { { name: "basic", nm: &NetworkMap{ - NodeKey: nodekey(1), + NodeKey: testNodeKey(1), Peers: []*tailcfg.Node{ { - Key: nodekey(2), + Key: testNodeKey(2), DERP: "127.3.3.40:2", Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, }, { - Key: nodekey(3), + Key: testNodeKey(3), DERP: "127.3.3.40:4", Endpoints: []string{"10.2.0.100:12", "10.1.0.100:12345"}, }, @@ -44,7 +45,7 @@ func TestNetworkMapConcise(t *testing.T) { { name: "debug_non_nil", nm: &NetworkMap{ - NodeKey: nodekey(1), + NodeKey: testNodeKey(1), Debug: &tailcfg.Debug{}, }, want: "netmap: self: [AQEBA] auth=machine-unknown debug={} []\n", @@ -52,7 +53,7 @@ func TestNetworkMapConcise(t *testing.T) { { name: "debug_values", nm: &NetworkMap{ - NodeKey: nodekey(1), + NodeKey: testNodeKey(1), Debug: &tailcfg.Debug{LogHeapPprof: true}, }, want: "netmap: self: [AQEBA] auth=machine-unknown debug={\"LogHeapPprof\":true} []\n", @@ -70,3 +71,147 @@ func TestNetworkMapConcise(t *testing.T) { }) } } + +func TestConciseDiffFrom(t *testing.T) { + for _, tt := range []struct { + name string + a, b *NetworkMap + want string + }{ + { + name: "no_change", + a: &NetworkMap{ + NodeKey: testNodeKey(1), + Peers: []*tailcfg.Node{ + { + Key: testNodeKey(2), + DERP: "127.3.3.40:2", + Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, + }, + }, + }, + b: &NetworkMap{ + NodeKey: testNodeKey(1), + Peers: []*tailcfg.Node{ + { + Key: testNodeKey(2), + DERP: "127.3.3.40:2", + Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, + }, + }, + }, + want: "", + }, + { + name: "header_change", + a: &NetworkMap{ + NodeKey: testNodeKey(1), + Peers: []*tailcfg.Node{ + { + Key: testNodeKey(2), + DERP: "127.3.3.40:2", + Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, + }, + }, + }, + b: &NetworkMap{ + NodeKey: testNodeKey(2), + Peers: []*tailcfg.Node{ + { + Key: testNodeKey(2), + DERP: "127.3.3.40:2", + Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, + }, + }, + }, + want: "-netmap: self: [AQEBA] auth=machine-unknown []\n+netmap: self: [AgICA] auth=machine-unknown []\n", + }, + { + name: "peer_add", + a: &NetworkMap{ + NodeKey: testNodeKey(1), + Peers: []*tailcfg.Node{ + { + ID: 2, + Key: testNodeKey(2), + DERP: "127.3.3.40:2", + Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, + }, + }, + }, + b: &NetworkMap{ + NodeKey: testNodeKey(1), + Peers: []*tailcfg.Node{ + { + ID: 1, + Key: testNodeKey(1), + DERP: "127.3.3.40:1", + Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, + }, + { + ID: 2, + Key: testNodeKey(2), + DERP: "127.3.3.40:2", + Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, + }, + { + ID: 3, + Key: testNodeKey(3), + DERP: "127.3.3.40:3", + Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, + }, + }, + }, + want: "+ [AQEBA] D1 : 192.168.0.100:12 192.168.0.100:12354\n+ [AwMDA] D3 : 192.168.0.100:12 192.168.0.100:12354\n", + }, + { + name: "peer_remove", + a: &NetworkMap{ + NodeKey: testNodeKey(1), + Peers: []*tailcfg.Node{ + { + ID: 1, + Key: testNodeKey(1), + DERP: "127.3.3.40:1", + Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, + }, + { + ID: 2, + Key: testNodeKey(2), + DERP: "127.3.3.40:2", + Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, + }, + { + ID: 3, + Key: testNodeKey(3), + DERP: "127.3.3.40:3", + Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, + }, + }, + }, + b: &NetworkMap{ + NodeKey: testNodeKey(1), + Peers: []*tailcfg.Node{ + { + ID: 2, + Key: testNodeKey(2), + DERP: "127.3.3.40:2", + Endpoints: []string{"192.168.0.100:12", "192.168.0.100:12354"}, + }, + }, + }, + want: "- [AQEBA] D1 : 192.168.0.100:12 192.168.0.100:12354\n- [AwMDA] D3 : 192.168.0.100:12 192.168.0.100:12354\n", + }, + } { + t.Run(tt.name, func(t *testing.T) { + var got string + n := int(testing.AllocsPerRun(50, func() { + got = tt.b.ConciseDiffFrom(tt.a) + })) + t.Logf("Allocs = %d", n) + if got != tt.want { + t.Errorf("Wrong output\n Got: %q\nWant: %q\n## Got (unescaped):\n%s\n## Want (unescaped):\n%s\n", got, tt.want, got, tt.want) + } + }) + } +}