ipn: fix netmap change tracking and dns map generation (#609)

Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>

control/controlclient/direct.go

@@ -4,6 +4,8 @@
 
 package controlclient
 
+//go:generate go run tailscale.com/cmd/cloner -type=Persist -output=direct_clone.go
+
 import (
 	"bytes"
 	"context"
@@ -628,6 +630,7 @@ func (c *Direct) PollNetMap(ctx context.Context, maxPolls int, cb func(*NetworkM
 			NodeKey:      tailcfg.NodeKey(persist.PrivateNodeKey.Public()),
 			PrivateKey:   persist.PrivateNodeKey,
 			Expiry:       resp.Node.KeyExpiry,
+			Name:         resp.Node.Name,
 			Addresses:    resp.Node.Addresses,
 			Peers:        resp.Peers,
 			LocalPort:    localPort,

control/controlclient/direct_clone.go

@@ -0,0 +1,20 @@
+// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Code generated by tailscale.com/cmd/cloner -type Persist; DO NOT EDIT.
+
+package controlclient
+
+import ()
+
+// Clone makes a deep copy of Persist.
+// The result aliases no memory with the original.
+func (src *Persist) Clone() *Persist {
+	if src == nil {
+		return nil
+	}
+	dst := new(Persist)
+	*dst = *src
+	return dst
+}

control/controlclient/netmap.go

@@ -26,6 +26,8 @@ type NetworkMap struct {
 	NodeKey    tailcfg.NodeKey
 	PrivateKey wgcfg.PrivateKey
 	Expiry     time.Time
+	// Name is the DNS name assigned to this node.
+	Name          string
 	Addresses     []wgcfg.CIDR
 	LocalPort     uint16 // used for debugging
 	MachineStatus tailcfg.MachineStatus

internal/deepprint/deepprint.go

@@ -18,13 +18,23 @@ import (
 	"reflect"
 )
 
-func Hash(v interface{}) string {
+func Hash(v ...interface{}) string {
 	h := sha256.New()
 	Print(h, v)
 	return fmt.Sprintf("%x", h.Sum(nil))
 }
 
-func Print(w io.Writer, v interface{}) {
+// UpdateHash sets last to the hash of v and reports whether its value changed.
+func UpdateHash(last *string, v ...interface{}) (changed bool) {
+	sig := Hash(v)
+	if *last != sig {
+		*last = sig
+		return true
+	}
+	return false
+}
+
+func Print(w io.Writer, v ...interface{}) {
 	print(w, reflect.ValueOf(v), make(map[uintptr]bool))
 }
 

ipn/local.go

@@ -16,6 +16,7 @@ import (
 	"golang.org/x/oauth2"
 	"inet.af/netaddr"
 	"tailscale.com/control/controlclient"
+	"tailscale.com/internal/deepprint"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/ipn/policy"
 	"tailscale.com/portlist"
@@ -51,12 +52,10 @@ type LocalBackend struct {
 	backendLogID    string
 	portpoll        *portlist.Poller // may be nil
 	portpollOnce    sync.Once
+	serverURL       string // tailcontrol URL
 	newDecompressor func() (controlclient.Decompressor, error)
 
-	// TODO: these fields are accessed unsafely by concurrent
+	filterHash string
-	// goroutines. They need to be protected.
-	serverURL       string // tailcontrol URL
-	lastFilterPrint time.Time
 
 	// The mutex protects the following elements.
 	mu       sync.Mutex
@@ -186,21 +185,56 @@ func (b *LocalBackend) SetDecompressor(fn func() (controlclient.Decompressor, er
 // setClientStatus is the callback invoked by the control client whenever it posts a new status.
 // Among other things, this is where we update the netmap, packet filters, DNS and DERP maps.
 func (b *LocalBackend) setClientStatus(st controlclient.Status) {
+	// The following do not depend on any data for which we need to lock b.
+	if st.Err != "" {
+		// TODO(crawshaw): display in the UI.
+		b.logf("Received error: %v", st.Err)
+		return
+	}
 	if st.LoginFinished != nil {
 		// Auth completed, unblock the engine
 		b.blockEngineUpdates(false)
 		b.authReconfig()
 		b.send(Notify{LoginFinished: &empty.Message{}})
 	}
-	if st.Persist != nil {
-		persist := *st.Persist // copy
 
+	prefsChanged := false
+
+	// Lock b once and do only the things that require locking.
 	b.mu.Lock()
-		b.prefs.Persist = &persist
+
-		prefs := b.prefs.Clone()
+	prefs := b.prefs
 	stateKey := b.stateKey
+	netMap := b.netMap
+	interact := b.interact
+
+	if st.Persist != nil {
+		if b.prefs.Persist.Equals(st.Persist) {
+			prefsChanged = true
+			b.prefs.Persist = st.Persist.Clone()
+		}
+	}
+	if st.NetMap != nil {
+		b.netMap = st.NetMap
+	}
+	if st.URL != "" {
+		b.authURL = st.URL
+	}
+	if b.state == NeedsLogin {
+		if !b.prefs.WantRunning {
+			prefsChanged = true
+		}
+		b.prefs.WantRunning = true
+	}
+	// Prefs will be written out; this is not safe unless locked or cloned.
+	if prefsChanged {
+		prefs = b.prefs.Clone()
+	}
+
 	b.mu.Unlock()
 
+	// Now complete the lock-free parts of what we started while locked.
+	if prefsChanged {
 		if stateKey != "" {
 			if err := b.store.WriteState(stateKey, prefs.ToBytes()); err != nil {
 				b.logf("Failed to save new controlclient state: %v", err)
@@ -209,63 +243,41 @@ func (b *LocalBackend) setClientStatus(st controlclient.Status) {
 		b.send(Notify{Prefs: prefs})
 	}
 	if st.NetMap != nil {
-		// Netmap is unchanged only when the diff is empty.
+		if netMap != nil {
-		changed := true
+			diff := st.NetMap.ConciseDiffFrom(netMap)
-		b.mu.Lock()
-		if b.netMap != nil {
-			diff := st.NetMap.ConciseDiffFrom(b.netMap)
 			if strings.TrimSpace(diff) == "" {
-				changed = false
 				b.logf("netmap diff: (none)")
 			} else {
 				b.logf("netmap diff:\n%v", diff)
 			}
 		}
-		disableDERP := b.prefs != nil && b.prefs.DisableDERP
-		b.netMap = st.NetMap
-		b.mu.Unlock()
 
-		b.send(Notify{NetMap: st.NetMap})
+		b.updateFilter(st.NetMap, prefs)
-		// There is nothing to update if the map hasn't changed.
-		if changed {
-			b.updateFilter(st.NetMap)
-			b.updateDNSMap(st.NetMap)
 		b.e.SetNetworkMap(st.NetMap)
+
+		if !dnsMapsEqual(st.NetMap, netMap) {
+			b.updateDNSMap(st.NetMap)
 		}
+
+		disableDERP := prefs != nil && prefs.DisableDERP
 		if disableDERP {
 			b.e.SetDERPMap(nil)
 		} else {
 			b.e.SetDERPMap(st.NetMap.DERPMap)
 		}
+
+		b.send(Notify{NetMap: st.NetMap})
 	}
 	if st.URL != "" {
 		b.logf("Received auth URL: %.20v...", st.URL)
-
-		b.mu.Lock()
-		interact := b.interact
-		b.authURL = st.URL
-		b.mu.Unlock()
-
 		if interact > 0 {
 			b.popBrowserAuthNow()
 		}
 	}
-	if st.Err != "" {
-		// TODO(crawshaw): display in the UI.
-		b.logf("Received error: %v", st.Err)
-		return
-	}
-	if st.NetMap != nil {
-		b.mu.Lock()
-		if b.state == NeedsLogin {
-			b.prefs.WantRunning = true
-		}
-		prefs := b.prefs
-		b.mu.Unlock()
-
-		b.SetPrefs(prefs)
-	}
 	b.stateMachine()
+	// This is currently (2020-07-28) necessary; conditionally disabling it is fragile!
+	// This is where netmap information gets propagated to router and magicsock.
+	b.authReconfig()
 }
 
 // setWgengineStatus is the callback by the wireguard engine whenever it posts a new status.
@@ -360,7 +372,7 @@ func (b *LocalBackend) Start(opts Options) error {
 	persist := b.prefs.Persist
 	b.mu.Unlock()
 
-	b.updateFilter(nil)
+	b.updateFilter(nil, nil)
 
 	var discoPublic tailcfg.DiscoKey
 	if controlclient.Debug.Disco {
@@ -424,20 +436,30 @@ func (b *LocalBackend) Start(opts Options) error {
 
 // updateFilter updates the packet filter in wgengine based on the
 // given netMap and user preferences.
-func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap) {
+func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap, prefs *Prefs) {
 	if netMap == nil {
 		// Not configured yet, block everything
 		b.logf("netmap packet filter: (not ready yet)")
 		b.e.SetFilter(filter.NewAllowNone(b.logf))
 		return
 	}
+	packetFilter := netMap.PacketFilter
+
+	var advRoutes []wgcfg.CIDR
+	if prefs != nil {
+		advRoutes = prefs.AdvertiseRoutes
+	}
+	// Be conservative while not ready.
+	shieldsUp := prefs == nil || prefs.ShieldsUp
+
+	changed := deepprint.UpdateHash(&b.filterHash, packetFilter, advRoutes, shieldsUp)
+	if !changed {
+		return
+	}
 
-	b.mu.Lock()
-	advRoutes := b.prefs.AdvertiseRoutes
-	b.mu.Unlock()
 	localNets := wgCIDRsToFilter(netMap.Addresses, advRoutes)
 
-	if b.shieldsAreUp() {
+	if shieldsUp {
 		// Shields up, block everything
 		b.logf("netmap packet filter: (shields up)")
 		var prevFilter *filter.Filter // don't reuse old filter state
@@ -445,44 +467,81 @@ func (b *LocalBackend) updateFilter(netMap *controlclient.NetworkMap) {
 		return
 	}
 
-	// TODO(apenwarr): don't replace filter at all if unchanged.
+	b.logf("netmap packet filter: %v", packetFilter)
-	// TODO(apenwarr): print a diff instead of full filter.
+	b.e.SetFilter(filter.New(packetFilter, localNets, b.e.GetFilter(), b.logf))
-	now := time.Now()
+}
-	if now.Sub(b.lastFilterPrint) > 1*time.Minute {
+
-		b.logf("netmap packet filter: %v", netMap.PacketFilter)
+// dnsCIDRsEqual determines whether two CIDR lists are equal
-		b.lastFilterPrint = now
+// for DNS map construction purposes (that is, only the first entry counts).
-	} else {
+func dnsCIDRsEqual(newAddr, oldAddr []wgcfg.CIDR) bool {
-		b.logf("netmap packet filter: (length %d)", len(netMap.PacketFilter))
+	if len(newAddr) != len(oldAddr) {
+		return false
+	}
+	if len(newAddr) == 0 || newAddr[0] == oldAddr[0] {
+		return true
+	}
+	return false
+}
+
+// dnsMapsEqual determines whether the new and the old network map
+// induce the same DNS map. It does so without allocating memory,
+// at the expense of giving false negatives if peers are reordered.
+func dnsMapsEqual(new, old *controlclient.NetworkMap) bool {
+	if (old == nil) != (new == nil) {
+		return false
+	}
+	if old == nil && new == nil {
+		return true
+	}
+
+	if len(new.Peers) != len(old.Peers) {
+		return false
+	}
+
+	if new.Name != old.Name {
+		return false
+	}
+	if !dnsCIDRsEqual(new.Addresses, old.Addresses) {
+		return false
 	}
-	b.e.SetFilter(filter.New(netMap.PacketFilter, localNets, b.e.GetFilter(), b.logf))
+
+	for i, newPeer := range new.Peers {
+		oldPeer := old.Peers[i]
+		if newPeer.Name != oldPeer.Name {
+			return false
+		}
+		if !dnsCIDRsEqual(newPeer.Addresses, oldPeer.Addresses) {
+			return false
+		}
+	}
+
+	return true
 }
 
 // updateDNSMap updates the domain map in the DNS resolver in wgengine
 // based on the given netMap and user preferences.
 func (b *LocalBackend) updateDNSMap(netMap *controlclient.NetworkMap) {
 	if netMap == nil {
+		b.logf("dns map: (not ready)")
 		return
 	}
 
-	domainToIP := make(map[string]netaddr.IP)
+	nameToIP := make(map[string]netaddr.IP)
-	set := func(hostname string, addrs []wgcfg.CIDR) {
+	set := func(name string, addrs []wgcfg.CIDR) {
 		if len(addrs) == 0 {
 			return
 		}
-		domain := hostname
+		nameToIP[name] = netaddr.IPFrom16(addrs[0].IP.Addr)
-		// Like PeerStatus.SimpleHostName()
-		domain = strings.TrimSuffix(domain, ".local")
-		domain = strings.TrimSuffix(domain, ".localdomain")
-		domain = domain + ".b.tailscale.net"
-		domainToIP[domain] = netaddr.IPFrom16(addrs[0].IP.Addr)
 	}
 
 	for _, peer := range netMap.Peers {
-		set(peer.Hostinfo.Hostname, peer.Addresses)
+		set(peer.Name, peer.Addresses)
 	}
-	set(netMap.Hostinfo.Hostname, netMap.Addresses)
+	set(netMap.Name, netMap.Addresses)
 
-	b.e.SetDNSMap(tsdns.NewMap(domainToIP))
+	dnsMap := tsdns.NewMap(nameToIP)
+	// map diff will be logged in tsdns.Resolver.SetMap.
+	b.e.SetDNSMap(dnsMap)
 }
 
 // readPoller is a goroutine that receives service lists from
@@ -721,37 +780,45 @@ func (b *LocalBackend) SetPrefs(new *Prefs) {
 	}
 
 	b.mu.Lock()
+
+	netMap := b.netMap
+	stateKey := b.stateKey
+
 	old := b.prefs
 	new.Persist = old.Persist // caller isn't allowed to override this
 	b.prefs = new
-	if b.stateKey != "" {
+	// We do this to avoid holding the lock while doing everything else.
-		if err := b.store.WriteState(b.stateKey, b.prefs.ToBytes()); err != nil {
+	new = b.prefs.Clone()
-			b.logf("Failed to save new controlclient state: %v", err)
+
-		}
-	}
 	oldHi := b.hostinfo
 	newHi := oldHi.Clone()
 	newHi.RoutableIPs = append([]wgcfg.CIDR(nil), b.prefs.AdvertiseRoutes...)
 	applyPrefsToHostinfo(newHi, new)
 	b.hostinfo = newHi
 	hostInfoChanged := !oldHi.Equal(newHi)
+
 	b.mu.Unlock()
 
+	if stateKey != "" {
+		if err := b.store.WriteState(stateKey, new.ToBytes()); err != nil {
+			b.logf("Failed to save new controlclient state: %v", err)
+		}
+	}
+
 	b.logf("SetPrefs: %v", new.Pretty())
 
 	if old.ShieldsUp != new.ShieldsUp || hostInfoChanged {
 		b.doSetHostinfoFilterServices(newHi)
 	}
 
-	b.updateFilter(b.netMap)
+	b.updateFilter(netMap, new)
-	// TODO(dmytro): when Prefs gain an EnableTailscaleDNS toggle, updateDNSMap here.
 
 	turnDERPOff := new.DisableDERP && !old.DisableDERP
 	turnDERPOn := !new.DisableDERP && old.DisableDERP
 	if turnDERPOff {
 		b.e.SetDERPMap(nil)
-	} else if turnDERPOn && b.netMap != nil {
+	} else if turnDERPOn && netMap != nil {
-		b.e.SetDERPMap(b.netMap.DERPMap)
+		b.e.SetDERPMap(netMap.DERPMap)
 	}
 
 	if old.WantRunning != new.WantRunning {

wgengine/tsdns/map.go

@@ -0,0 +1,118 @@
+// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package tsdns
+
+import (
+	"fmt"
+	"sort"
+	"strings"
+
+	"inet.af/netaddr"
+)
+
+// Map is all the data Resolver needs to resolve DNS queries within the Tailscale network.
+type Map struct {
+	// nameToIP is a mapping of Tailscale domain names to their IP addresses.
+	// For example, monitoring.tailscale.us -> 100.64.0.1.
+	nameToIP map[string]netaddr.IP
+	// names are the keys of nameToIP in sorted order.
+	names []string
+}
+
+// NewMap returns a new Map with name to address mapping given by nameToIP.
+func NewMap(nameToIP map[string]netaddr.IP) *Map {
+	names := make([]string, 0, len(nameToIP))
+	for name := range nameToIP {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+
+	return &Map{
+		nameToIP: nameToIP,
+		names:    names,
+	}
+}
+
+func printSingleNameIP(buf *strings.Builder, name string, ip netaddr.IP) {
+	// Output width is exactly 80 columns.
+	fmt.Fprintf(buf, "%s\t%s\n", name, ip)
+}
+
+func (m *Map) Pretty() string {
+	buf := new(strings.Builder)
+	for _, name := range m.names {
+		printSingleNameIP(buf, name, m.nameToIP[name])
+	}
+	return buf.String()
+}
+
+func (m *Map) PrettyDiffFrom(old *Map) string {
+	var (
+		oldNameToIP map[string]netaddr.IP
+		newNameToIP map[string]netaddr.IP
+		oldNames    []string
+		newNames    []string
+	)
+	if old != nil {
+		oldNameToIP = old.nameToIP
+		oldNames = old.names
+	}
+	if m != nil {
+		newNameToIP = m.nameToIP
+		newNames = m.names
+	}
+
+	buf := new(strings.Builder)
+
+	for len(oldNames) > 0 && len(newNames) > 0 {
+		var name string
+
+		newName, oldName := newNames[0], oldNames[0]
+		switch {
+		case oldName < newName:
+			name = oldName
+			oldNames = oldNames[1:]
+		case oldName > newName:
+			name = newName
+			newNames = newNames[1:]
+		case oldNames[0] == newNames[0]:
+			name = oldNames[0]
+			oldNames = oldNames[1:]
+			newNames = newNames[1:]
+		}
+
+		ipOld, inOld := oldNameToIP[name]
+		ipNew, inNew := newNameToIP[name]
+		switch {
+		case !inOld:
+			buf.WriteByte('+')
+			printSingleNameIP(buf, name, ipNew)
+		case !inNew:
+			buf.WriteByte('-')
+			printSingleNameIP(buf, name, ipOld)
+		case ipOld != ipNew:
+			buf.WriteByte('-')
+			printSingleNameIP(buf, name, ipOld)
+			buf.WriteByte('+')
+			printSingleNameIP(buf, name, ipNew)
+		}
+	}
+
+	for _, name := range oldNames {
+		if _, ok := newNameToIP[name]; !ok {
+			buf.WriteByte('-')
+			printSingleNameIP(buf, name, oldNameToIP[name])
+		}
+	}
+
+	for _, name := range newNames {
+		if _, ok := oldNameToIP[name]; !ok {
+			buf.WriteByte('+')
+			printSingleNameIP(buf, name, newNameToIP[name])
+		}
+	}
+
+	return buf.String()
+}

wgengine/tsdns/map_test.go

@@ -0,0 +1,138 @@
+// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package tsdns
+
+import (
+	"testing"
+
+	"inet.af/netaddr"
+)
+
+func TestPretty(t *testing.T) {
+	tests := []struct {
+		name string
+		dmap *Map
+		want string
+	}{
+		{"empty", NewMap(nil), ""},
+		{
+			"single",
+			NewMap(map[string]netaddr.IP{
+				"hello.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+			}),
+			"hello.ipn.dev\t100.101.102.103\n",
+		},
+		{
+			"multiple",
+			NewMap(map[string]netaddr.IP{
+				"test1.domain":     netaddr.IPv4(100, 101, 102, 103),
+				"test2.sub.domain": netaddr.IPv4(100, 99, 9, 1),
+			}),
+			"test1.domain\t100.101.102.103\ntest2.sub.domain\t100.99.9.1\n",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.dmap.Pretty()
+			if tt.want != got {
+				t.Errorf("want %v; got %v", tt.want, got)
+			}
+		})
+	}
+}
+
+func TestPrettyDiffFrom(t *testing.T) {
+	tests := []struct {
+		name string
+		map1 *Map
+		map2 *Map
+		want string
+	}{
+		{
+			"from_empty",
+			nil,
+			NewMap(map[string]netaddr.IP{
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+			}),
+			"+test1.ipn.dev\t100.101.102.103\n+test2.ipn.dev\t100.103.102.101\n",
+		},
+		{
+			"equal",
+			NewMap(map[string]netaddr.IP{
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+			}),
+			NewMap(map[string]netaddr.IP{
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+			}),
+			"",
+		},
+		{
+			"changed_ip",
+			NewMap(map[string]netaddr.IP{
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+			}),
+			NewMap(map[string]netaddr.IP{
+				"test2.ipn.dev": netaddr.IPv4(100, 104, 102, 101),
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+			}),
+			"-test2.ipn.dev\t100.103.102.101\n+test2.ipn.dev\t100.104.102.101\n",
+		},
+		{
+			"new_domain",
+			NewMap(map[string]netaddr.IP{
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+			}),
+			NewMap(map[string]netaddr.IP{
+				"test3.ipn.dev": netaddr.IPv4(100, 105, 106, 107),
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+			}),
+			"+test3.ipn.dev\t100.105.106.107\n",
+		},
+		{
+			"gone_domain",
+			NewMap(map[string]netaddr.IP{
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+			}),
+			NewMap(map[string]netaddr.IP{
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+			}),
+			"-test2.ipn.dev\t100.103.102.101\n",
+		},
+		{
+			"mixed",
+			NewMap(map[string]netaddr.IP{
+				"test1.ipn.dev": netaddr.IPv4(100, 101, 102, 103),
+				"test4.ipn.dev": netaddr.IPv4(100, 107, 106, 105),
+				"test5.ipn.dev": netaddr.IPv4(100, 64, 1, 1),
+				"test2.ipn.dev": netaddr.IPv4(100, 103, 102, 101),
+			}),
+			NewMap(map[string]netaddr.IP{
+				"test2.ipn.dev": netaddr.IPv4(100, 104, 102, 101),
+				"test1.ipn.dev": netaddr.IPv4(100, 100, 101, 102),
+				"test3.ipn.dev": netaddr.IPv4(100, 64, 1, 1),
+			}),
+			"-test1.ipn.dev\t100.101.102.103\n+test1.ipn.dev\t100.100.101.102\n" +
+				"-test2.ipn.dev\t100.103.102.101\n+test2.ipn.dev\t100.104.102.101\n" +
+				"+test3.ipn.dev\t100.64.1.1\n-test4.ipn.dev\t100.107.106.105\n-test5.ipn.dev\t100.64.1.1\n",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.map2.PrettyDiffFrom(tt.map1)
+			if tt.want != got {
+				t.Errorf("want %v; got %v", tt.want, got)
+			}
+		})
+	}
+}

wgengine/tsdns/tsdns.go

@@ -45,18 +45,6 @@ var (
 	errNotQuery       = errors.New("not a DNS query")
 )
 
-// Map is all the data Resolver needs to resolve DNS queries within the Tailscale network.
-type Map struct {
-	// domainToIP is a mapping of Tailscale domains to their IP addresses.
-	// For example, monitoring.tailscale.us -> 100.64.0.1.
-	domainToIP map[string]netaddr.IP
-}
-
-// NewMap returns a new Map with domain to address mapping given by domainToIP.
-func NewMap(domainToIP map[string]netaddr.IP) *Map {
-	return &Map{domainToIP: domainToIP}
-}
-
 // Packet represents a DNS payload together with the address of its origin.
 type Packet struct {
 	// Payload is the application layer DNS payload.
@@ -142,8 +130,10 @@ func (r *Resolver) Close() {
 // SetMap sets the resolver's DNS map, taking ownership of it.
 func (r *Resolver) SetMap(m *Map) {
 	r.mu.Lock()
+	oldMap := r.dnsMap
 	r.dnsMap = m
 	r.mu.Unlock()
+	r.logf("map diff:\n%s", m.PrettyDiffFrom(oldMap))
 }
 
 // SetUpstreamNameservers sets the addresses of the resolver's
@@ -189,7 +179,7 @@ func (r *Resolver) Resolve(domain string) (netaddr.IP, dns.RCode, error) {
 		r.mu.RUnlock()
 		return netaddr.IP{}, dns.RCodeServerFailure, errMapNotSet
 	}
-	addr, found := r.dnsMap.domainToIP[domain]
+	addr, found := r.dnsMap.nameToIP[domain]
 	r.mu.RUnlock()
 
 	if !found {

wgengine/tsdns/tsdns_test.go

@@ -23,7 +23,7 @@ var testipv6 = netaddr.IPv6Raw([16]byte{
 })
 
 var dnsMap = &Map{
-	domainToIP: map[string]netaddr.IP{
+	nameToIP: map[string]netaddr.IP{
 		"test1.ipn.dev": testipv4,
 		"test2.ipn.dev": testipv6,
 	},

wgengine/userspace.go

@@ -560,15 +560,6 @@ func (e *userspaceEngine) pinger(peerKey wgcfg.Key, ips []wgcfg.IP) {
 	p.run(ctx, peerKey, ips, srcIP)
 }
 
-func updateSig(last *string, v interface{}) (changed bool) {
-	sig := deepprint.Hash(v)
-	if *last != sig {
-		*last = sig
-		return true
-	}
-	return false
-}
-
 // isTrimmablePeer reports whether p is a peer that we can trim out of the
 // network map.
 //
@@ -693,7 +684,7 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked() error {
 		}
 	}
 
-	if !updateSig(&e.lastEngineSigTrim, min) {
+	if !deepprint.UpdateHash(&e.lastEngineSigTrim, min) {
 		// No changes
 		return nil
 	}
@@ -795,8 +786,8 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config)
 		routerCfg.Domains = append([]string{magicDNSDomain}, routerCfg.Domains...)
 	}
 
-	engineChanged := updateSig(&e.lastEngineSigFull, cfg)
+	engineChanged := deepprint.UpdateHash(&e.lastEngineSigFull, cfg)
-	routerChanged := updateSig(&e.lastRouterSig, routerCfg)
+	routerChanged := deepprint.UpdateHash(&e.lastRouterSig, routerCfg)
 	if !engineChanged && !routerChanged {
 		return ErrNoChanges
 	}