wgengine, internal/deepprint: replace UAPI usage as hash func; add deepprint

The new deepprint package just walks a Go data structure and writes to
an io.Writer. It's not pretty like go-spew, etc.

We then use it to replace the use of UAPI (which we have a TODO to
remove) to generate signatures of data structures to detect whether
anything changed (without retaining the old copy).

This was necessary because the UAPI conversion ends up trying to do
DNS lookups which an upcoming change depends on not happening.

internal/deepprint/deepprint.go

@@ -0,0 +1,89 @@
+// Package deepprint walks a Go value recursively, in a predictable
+// order, without looping, and prints each value out to a given
+// Writer, which is assumed to be a hash.Hash, as this package doesn't
+// format things nicely.
+//
+// This is intended as a lighter version of go-spew, etc. We don't need its
+// features when our writer is just a hash.
+package deepprint
+
+import (
+	"crypto/sha256"
+	"fmt"
+	"io"
+	"reflect"
+)
+
+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{}) {
+	print(w, reflect.ValueOf(v), make(map[uintptr]bool))
+}
+
+func print(w io.Writer, v reflect.Value, visited map[uintptr]bool) {
+	if !v.IsValid() {
+		return
+	}
+	switch v.Kind() {
+	default:
+		panic(fmt.Sprintf("unhandled kind %v for type %v", v.Kind(), v.Type()))
+	case reflect.Ptr:
+		ptr := v.Pointer()
+		if visited[ptr] {
+			return
+		}
+		visited[ptr] = true
+		print(w, v.Elem(), visited)
+		return
+	case reflect.Struct:
+		fmt.Fprintf(w, "struct{\n")
+		t := v.Type()
+		for i, n := 0, v.NumField(); i < n; i++ {
+			sf := t.Field(i)
+			fmt.Fprintf(w, "%s: ", sf.Name)
+			print(w, v.Field(i), visited)
+			fmt.Fprintf(w, "\n")
+		}
+	case reflect.Slice, reflect.Array:
+		if v.Type().Elem().Kind() == reflect.Uint8 && v.CanInterface() {
+			fmt.Fprintf(w, "%q", v.Interface())
+			return
+		}
+		fmt.Fprintf(w, "[%d]{\n", v.Len())
+		for i, ln := 0, v.Len(); i < ln; i++ {
+			fmt.Fprintf(w, " [%d]: ", i)
+			print(w, v.Index(i), visited)
+			fmt.Fprintf(w, "\n")
+		}
+		fmt.Fprintf(w, "}\n")
+	case reflect.Interface:
+		print(w, v.Elem(), visited)
+	case reflect.Map:
+		sm := newSortedMap(v)
+		fmt.Fprintf(w, "map[%d]{\n", len(sm.Key))
+		for i, k := range sm.Key {
+			print(w, k, visited)
+			fmt.Fprintf(w, ": ")
+			print(w, sm.Value[i], visited)
+			fmt.Fprintf(w, "\n")
+		}
+		fmt.Fprintf(w, "}\n")
+
+	case reflect.String:
+		fmt.Fprintf(w, "%s", v.String())
+	case reflect.Bool:
+		fmt.Fprintf(w, "%v", v.Bool())
+	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
+		fmt.Fprintf(w, "%v", v.Int())
+	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
+		fmt.Fprintf(w, "%v", v.Uint())
+	case reflect.Float32, reflect.Float64:
+		fmt.Fprintf(w, "%v", v.Float())
+	case reflect.Complex64, reflect.Complex128:
+		fmt.Fprintf(w, "%v", v.Complex())
+	}
+}

internal/deepprint/deepprint_test.go

@@ -0,0 +1,63 @@
+package deepprint
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/tailscale/wireguard-go/wgcfg"
+	"inet.af/netaddr"
+	"tailscale.com/wgengine/router"
+)
+
+func TestDeepPrint(t *testing.T) {
+	// v contains the types of values we care about for our current callers.
+	// Mostly we're just testing that we don't panic on handled types.
+	v := getVal()
+
+	var buf bytes.Buffer
+	Print(&buf, v)
+	t.Logf("Got: %s", buf.Bytes())
+
+	hash1 := Hash(v)
+	t.Logf("hash: %v", hash1)
+	for i := 0; i < 20; i++ {
+		hash2 := Hash(getVal())
+		if hash1 != hash2 {
+			t.Error("second hash didn't match")
+		}
+	}
+}
+
+func getVal() []interface{} {
+	return []interface{}{
+		&wgcfg.Config{
+			Name:       "foo",
+			Addresses:  []wgcfg.CIDR{{Mask: 5, IP: wgcfg.IP{Addr: [16]byte{3: 3}}}},
+			ListenPort: 5,
+			Peers: []wgcfg.Peer{
+				{
+					Endpoints: []wgcfg.Endpoint{
+						{
+							Host: "foo",
+							Port: 5,
+						},
+					},
+				},
+			},
+		},
+		&router.Config{
+			DNS: []netaddr.IP{netaddr.IPv4(8, 8, 8, 8)},
+		},
+		map[string]string{
+			"key1": "val1",
+			"key2": "val2",
+			"key3": "val3",
+			"key4": "val4",
+			"key5": "val5",
+			"key6": "val6",
+			"key7": "val7",
+			"key8": "val8",
+			"key9": "val9",
+		},
+	}
+}

internal/deepprint/fmtsort.go

@@ -0,0 +1,218 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// This is a copy of Go's src/internal/fmtsort/sort.go
+
+package deepprint
+
+import (
+	"reflect"
+	"sort"
+)
+
+// Note: Throughout this package we avoid calling reflect.Value.Interface as
+// it is not always legal to do so and it's easier to avoid the issue than to face it.
+
+// sortedMap represents a map's keys and values. The keys and values are
+// aligned in index order: Value[i] is the value in the map corresponding to Key[i].
+type sortedMap struct {
+	Key   []reflect.Value
+	Value []reflect.Value
+}
+
+func (o *sortedMap) Len() int           { return len(o.Key) }
+func (o *sortedMap) Less(i, j int) bool { return compare(o.Key[i], o.Key[j]) < 0 }
+func (o *sortedMap) Swap(i, j int) {
+	o.Key[i], o.Key[j] = o.Key[j], o.Key[i]
+	o.Value[i], o.Value[j] = o.Value[j], o.Value[i]
+}
+
+// Sort accepts a map and returns a sortedMap that has the same keys and
+// values but in a stable sorted order according to the keys, modulo issues
+// raised by unorderable key values such as NaNs.
+//
+// The ordering rules are more general than with Go's < operator:
+//
+//  - when applicable, nil compares low
+//  - ints, floats, and strings order by <
+//  - NaN compares less than non-NaN floats
+//  - bool compares false before true
+//  - complex compares real, then imag
+//  - pointers compare by machine address
+//  - channel values compare by machine address
+//  - structs compare each field in turn
+//  - arrays compare each element in turn.
+//    Otherwise identical arrays compare by length.
+//  - interface values compare first by reflect.Type describing the concrete type
+//    and then by concrete value as described in the previous rules.
+//
+func newSortedMap(mapValue reflect.Value) *sortedMap {
+	if mapValue.Type().Kind() != reflect.Map {
+		return nil
+	}
+	// Note: this code is arranged to not panic even in the presence
+	// of a concurrent map update. The runtime is responsible for
+	// yelling loudly if that happens. See issue 33275.
+	n := mapValue.Len()
+	key := make([]reflect.Value, 0, n)
+	value := make([]reflect.Value, 0, n)
+	iter := mapValue.MapRange()
+	for iter.Next() {
+		key = append(key, iter.Key())
+		value = append(value, iter.Value())
+	}
+	sorted := &sortedMap{
+		Key:   key,
+		Value: value,
+	}
+	sort.Stable(sorted)
+	return sorted
+}
+
+// compare compares two values of the same type. It returns -1, 0, 1
+// according to whether a > b (1), a == b (0), or a < b (-1).
+// If the types differ, it returns -1.
+// See the comment on Sort for the comparison rules.
+func compare(aVal, bVal reflect.Value) int {
+	aType, bType := aVal.Type(), bVal.Type()
+	if aType != bType {
+		return -1 // No good answer possible, but don't return 0: they're not equal.
+	}
+	switch aVal.Kind() {
+	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
+		a, b := aVal.Int(), bVal.Int()
+		switch {
+		case a < b:
+			return -1
+		case a > b:
+			return 1
+		default:
+			return 0
+		}
+	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
+		a, b := aVal.Uint(), bVal.Uint()
+		switch {
+		case a < b:
+			return -1
+		case a > b:
+			return 1
+		default:
+			return 0
+		}
+	case reflect.String:
+		a, b := aVal.String(), bVal.String()
+		switch {
+		case a < b:
+			return -1
+		case a > b:
+			return 1
+		default:
+			return 0
+		}
+	case reflect.Float32, reflect.Float64:
+		return floatCompare(aVal.Float(), bVal.Float())
+	case reflect.Complex64, reflect.Complex128:
+		a, b := aVal.Complex(), bVal.Complex()
+		if c := floatCompare(real(a), real(b)); c != 0 {
+			return c
+		}
+		return floatCompare(imag(a), imag(b))
+	case reflect.Bool:
+		a, b := aVal.Bool(), bVal.Bool()
+		switch {
+		case a == b:
+			return 0
+		case a:
+			return 1
+		default:
+			return -1
+		}
+	case reflect.Ptr:
+		a, b := aVal.Pointer(), bVal.Pointer()
+		switch {
+		case a < b:
+			return -1
+		case a > b:
+			return 1
+		default:
+			return 0
+		}
+	case reflect.Chan:
+		if c, ok := nilCompare(aVal, bVal); ok {
+			return c
+		}
+		ap, bp := aVal.Pointer(), bVal.Pointer()
+		switch {
+		case ap < bp:
+			return -1
+		case ap > bp:
+			return 1
+		default:
+			return 0
+		}
+	case reflect.Struct:
+		for i := 0; i < aVal.NumField(); i++ {
+			if c := compare(aVal.Field(i), bVal.Field(i)); c != 0 {
+				return c
+			}
+		}
+		return 0
+	case reflect.Array:
+		for i := 0; i < aVal.Len(); i++ {
+			if c := compare(aVal.Index(i), bVal.Index(i)); c != 0 {
+				return c
+			}
+		}
+		return 0
+	case reflect.Interface:
+		if c, ok := nilCompare(aVal, bVal); ok {
+			return c
+		}
+		c := compare(reflect.ValueOf(aVal.Elem().Type()), reflect.ValueOf(bVal.Elem().Type()))
+		if c != 0 {
+			return c
+		}
+		return compare(aVal.Elem(), bVal.Elem())
+	default:
+		// Certain types cannot appear as keys (maps, funcs, slices), but be explicit.
+		panic("bad type in compare: " + aType.String())
+	}
+}
+
+// nilCompare checks whether either value is nil. If not, the boolean is false.
+// If either value is nil, the boolean is true and the integer is the comparison
+// value. The comparison is defined to be 0 if both are nil, otherwise the one
+// nil value compares low. Both arguments must represent a chan, func,
+// interface, map, pointer, or slice.
+func nilCompare(aVal, bVal reflect.Value) (int, bool) {
+	if aVal.IsNil() {
+		if bVal.IsNil() {
+			return 0, true
+		}
+		return -1, true
+	}
+	if bVal.IsNil() {
+		return 1, true
+	}
+	return 0, false
+}
+
+// floatCompare compares two floating-point values. NaNs compare low.
+func floatCompare(a, b float64) int {
+	switch {
+	case isNaN(a):
+		return -1 // No good answer if b is a NaN so don't bother checking.
+	case isNaN(b):
+		return 1
+	case a < b:
+		return -1
+	case a > b:
+		return 1
+	}
+	return 0
+}
+
+func isNaN(a float64) bool {
+	return a != a
+}

wgengine/userspace.go

@@ -26,6 +26,7 @@
 	"github.com/tailscale/wireguard-go/wgcfg"
 	"go4.org/mem"
 	"tailscale.com/control/controlclient"
+	"tailscale.com/internal/deepprint"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/net/interfaces"
 	"tailscale.com/tailcfg"
@@ -488,17 +489,13 @@ func (e *userspaceEngine) pinger(peerKey wgcfg.Key, ips []wgcfg.IP) {
 	p.run(ctx, peerKey, ips, srcIP)
 }
 
-// configSignatures returns string signatures for the given configs.
+func updateSig(last *string, v interface{}) (changed bool) {
-// The exact strings should be considered opaque; the only guarantee is that
+	sig := deepprint.Hash(v)
-// if cfg or routerCfg change, so do their signatures.
+	if *last != sig {
-func configSignatures(cfg *wgcfg.Config, routerCfg *router.Config) (configSig, engineSig string, err error) {
+		*last = sig
-	// TODO(apenwarr): get rid of uapi stuff for in-process comms
+		return true
-	uapi, err := cfg.ToUAPI()
-	if err != nil {
-		return "", "", err
 	}
-
+	return false
-	return uapi, fmt.Sprintf("%v", routerCfg), nil
 }
 
 func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config) error {
@@ -529,17 +526,11 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config)
 	}
 	e.mu.Unlock()
 
-	ec, rc, err := configSignatures(cfg, routerCfg)
+	engineChanged := updateSig(&e.lastEngineSig, cfg)
-	if err != nil {
+	routerChanged := updateSig(&e.lastRouterSig, routerCfg)
-		return err
-	}
-	engineChanged := ec != e.lastEngineSig
-	routerChanged := rc != e.lastRouterSig
 	if !engineChanged && !routerChanged {
 		return ErrNoChanges
 	}
-	e.lastEngineSig = ec
-	e.lastRouterSig = rc
 	e.lastCfg = cfg.Copy()
 
 	if engineChanged {