Browse Source

util/checkchange: stop using deephash everywhere

Saves 45 KB from the min build, no longer pulling in deephash or
util/hashx, both with unsafe code.

It can actually be more efficient to not use deephash, as you don't
have to walk all bytes of all fields recursively to answer that two
things are not equal. Instead, you can just return false at the first
difference you see. And then with views (as we use ~everywhere
nowadays), the cloning the old value isn't expensive, as it's just a
pointer under the hood.

Updates #12614

Change-Id: I7b08616b8a09b3ade454bb5e0ac5672086fe8aec
Signed-off-by: Brad Fitzpatrick <[email protected]>
Brad Fitzpatrick 5 months ago
parent
commit
316afe7d02

+ 2 - 1
cmd/k8s-operator/depaware.txt

@@ -825,12 +825,13 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/
         tailscale.com/types/tkatype                                  from tailscale.com/client/local+
         tailscale.com/types/views                                    from tailscale.com/appc+
         tailscale.com/util/backoff                                   from tailscale.com/cmd/k8s-operator+
+        tailscale.com/util/checkchange                               from tailscale.com/ipn/ipnlocal+
         tailscale.com/util/cibuild                                   from tailscale.com/health
         tailscale.com/util/clientmetric                              from tailscale.com/cmd/k8s-operator+
         tailscale.com/util/cloudenv                                  from tailscale.com/hostinfo+
   LW    tailscale.com/util/cmpver                                    from tailscale.com/net/dns+
         tailscale.com/util/ctxkey                                    from tailscale.com/client/tailscale/apitype+
-     💣 tailscale.com/util/deephash                                  from tailscale.com/ipn/ipnlocal+
+     💣 tailscale.com/util/deephash                                  from tailscale.com/util/syspolicy/setting
    L 💣 tailscale.com/util/dirwalk                                   from tailscale.com/metrics
         tailscale.com/util/dnsname                                   from tailscale.com/appc+
         tailscale.com/util/eventbus                                  from tailscale.com/tsd+

+ 1 - 2
cmd/tailscaled/depaware-min.txt

@@ -144,17 +144,16 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/types/tkatype                                  from tailscale.com/control/controlclient+
         tailscale.com/types/views                                    from tailscale.com/appc+
         tailscale.com/util/backoff                                   from tailscale.com/control/controlclient+
+        tailscale.com/util/checkchange                               from tailscale.com/ipn/ipnlocal+
         tailscale.com/util/cibuild                                   from tailscale.com/health
         tailscale.com/util/clientmetric                              from tailscale.com/appc+
         tailscale.com/util/cloudenv                                  from tailscale.com/hostinfo+
         tailscale.com/util/ctxkey                                    from tailscale.com/client/tailscale/apitype+
-     💣 tailscale.com/util/deephash                                  from tailscale.com/ipn/ipnlocal+
         tailscale.com/util/dnsname                                   from tailscale.com/appc+
         tailscale.com/util/eventbus                                  from tailscale.com/control/controlclient+
         tailscale.com/util/execqueue                                 from tailscale.com/appc+
         tailscale.com/util/goroutines                                from tailscale.com/ipn/ipnlocal
         tailscale.com/util/groupmember                               from tailscale.com/ipn/ipnauth
-     💣 tailscale.com/util/hashx                                     from tailscale.com/util/deephash
         tailscale.com/util/httpm                                     from tailscale.com/ipn/ipnlocal+
         tailscale.com/util/lineiter                                  from tailscale.com/hostinfo+
         tailscale.com/util/mak                                       from tailscale.com/control/controlclient+

+ 1 - 2
cmd/tailscaled/depaware-minbox.txt

@@ -170,18 +170,17 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/types/tkatype                                  from tailscale.com/control/controlclient+
         tailscale.com/types/views                                    from tailscale.com/appc+
         tailscale.com/util/backoff                                   from tailscale.com/control/controlclient+
+        tailscale.com/util/checkchange                               from tailscale.com/ipn/ipnlocal+
         tailscale.com/util/cibuild                                   from tailscale.com/health
         tailscale.com/util/clientmetric                              from tailscale.com/appc+
         tailscale.com/util/cloudenv                                  from tailscale.com/hostinfo+
         tailscale.com/util/cmpver                                    from tailscale.com/clientupdate
         tailscale.com/util/ctxkey                                    from tailscale.com/client/tailscale/apitype+
-     💣 tailscale.com/util/deephash                                  from tailscale.com/ipn/ipnlocal+
         tailscale.com/util/dnsname                                   from tailscale.com/appc+
         tailscale.com/util/eventbus                                  from tailscale.com/client/local+
         tailscale.com/util/execqueue                                 from tailscale.com/appc+
         tailscale.com/util/goroutines                                from tailscale.com/ipn/ipnlocal
         tailscale.com/util/groupmember                               from tailscale.com/ipn/ipnauth
-     💣 tailscale.com/util/hashx                                     from tailscale.com/util/deephash
         tailscale.com/util/httpm                                     from tailscale.com/ipn/ipnlocal+
         tailscale.com/util/lineiter                                  from tailscale.com/hostinfo+
         tailscale.com/util/mak                                       from tailscale.com/control/controlclient+

+ 2 - 1
cmd/tailscaled/depaware.txt

@@ -412,12 +412,13 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/types/tkatype                                  from tailscale.com/tka+
         tailscale.com/types/views                                    from tailscale.com/ipn/ipnlocal+
         tailscale.com/util/backoff                                   from tailscale.com/cmd/tailscaled+
+        tailscale.com/util/checkchange                               from tailscale.com/ipn/ipnlocal+
         tailscale.com/util/cibuild                                   from tailscale.com/health
         tailscale.com/util/clientmetric                              from tailscale.com/control/controlclient+
         tailscale.com/util/cloudenv                                  from tailscale.com/net/dns/resolver+
         tailscale.com/util/cmpver                                    from tailscale.com/net/dns+
         tailscale.com/util/ctxkey                                    from tailscale.com/ipn/ipnlocal+
-     💣 tailscale.com/util/deephash                                  from tailscale.com/ipn/ipnlocal+
+     💣 tailscale.com/util/deephash                                  from tailscale.com/util/syspolicy/setting
    L 💣 tailscale.com/util/dirwalk                                   from tailscale.com/metrics+
         tailscale.com/util/dnsname                                   from tailscale.com/appc+
         tailscale.com/util/eventbus                                  from tailscale.com/tsd+

+ 4 - 0
cmd/tailscaled/deps_test.go

@@ -244,6 +244,8 @@ func TestMinTailscaledNoCLI(t *testing.T) {
 		"internal/socks",
 		"github.com/tailscale/peercred",
 		"tailscale.com/types/netlogtype",
+		"deephash",
+		"util/hashx",
 	}
 	deptest.DepChecker{
 		GOOS:   "linux",
@@ -268,6 +270,8 @@ func TestMinTailscaledWithCLI(t *testing.T) {
 		"tailscale.com/metrics",
 		"tailscale.com/tsweb/varz",
 		"dirwalk",
+		"deephash",
+		"util/hashx",
 	}
 	deptest.DepChecker{
 		GOOS:   "linux",

+ 2 - 1
cmd/tsidp/depaware.txt

@@ -252,12 +252,13 @@ tailscale.com/cmd/tsidp dependencies: (generated by github.com/tailscale/depawar
         tailscale.com/types/tkatype                                  from tailscale.com/client/local+
         tailscale.com/types/views                                    from tailscale.com/appc+
         tailscale.com/util/backoff                                   from tailscale.com/control/controlclient+
+        tailscale.com/util/checkchange                               from tailscale.com/ipn/ipnlocal+
         tailscale.com/util/cibuild                                   from tailscale.com/health
         tailscale.com/util/clientmetric                              from tailscale.com/appc+
         tailscale.com/util/cloudenv                                  from tailscale.com/hostinfo+
   LW    tailscale.com/util/cmpver                                    from tailscale.com/net/dns+
         tailscale.com/util/ctxkey                                    from tailscale.com/client/tailscale/apitype+
-     💣 tailscale.com/util/deephash                                  from tailscale.com/ipn/ipnlocal+
+     💣 tailscale.com/util/deephash                                  from tailscale.com/util/syspolicy/setting
    L 💣 tailscale.com/util/dirwalk                                   from tailscale.com/metrics
         tailscale.com/util/dnsname                                   from tailscale.com/appc+
         tailscale.com/util/eventbus                                  from tailscale.com/client/local+

+ 52 - 22
ipn/ipnlocal/local.go

@@ -83,8 +83,8 @@ import (
 	"tailscale.com/types/preftype"
 	"tailscale.com/types/ptr"
 	"tailscale.com/types/views"
+	"tailscale.com/util/checkchange"
 	"tailscale.com/util/clientmetric"
-	"tailscale.com/util/deephash"
 	"tailscale.com/util/dnsname"
 	"tailscale.com/util/eventbus"
 	"tailscale.com/util/goroutines"
@@ -262,13 +262,13 @@ type LocalBackend struct {
 	// of [LocalBackend]'s own state that is not tied to the node context.
 	currentNodeAtomic atomic.Pointer[nodeBackend]
 
-	conf           *conffile.Config   // latest parsed config, or nil if not in declarative mode
-	pm             *profileManager    // mu guards access
-	filterHash     deephash.Sum       // TODO(nickkhyl): move to nodeBackend
-	httpTestClient *http.Client       // for controlclient. nil by default, used by tests.
-	ccGen          clientGen          // function for producing controlclient; lazily populated
-	sshServer      SSHServer          // or nil, initialized lazily.
-	appConnector   *appc.AppConnector // or nil, initialized when configured.
+	conf             *conffile.Config // latest parsed config, or nil if not in declarative mode
+	pm               *profileManager  // mu guards access
+	lastFilterInputs *filterInputs
+	httpTestClient   *http.Client       // for controlclient. nil by default, used by tests.
+	ccGen            clientGen          // function for producing controlclient; lazily populated
+	sshServer        SSHServer          // or nil, initialized lazily.
+	appConnector     *appc.AppConnector // or nil, initialized when configured.
 	// notifyCancel cancels notifications to the current SetNotifyCallback.
 	notifyCancel   context.CancelFunc
 	cc             controlclient.Client // TODO(nickkhyl): move to nodeBackend
@@ -2626,6 +2626,36 @@ var invalidPacketFilterWarnable = health.Register(&health.Warnable{
 	Text:     health.StaticMessage("The coordination server sent an invalid packet filter permitting traffic to unlocked nodes; rejecting all packets for safety"),
 })
 
+// filterInputs holds the inputs to the packet filter.
+//
+// Any field changes or additions here should be accompanied by a change to
+// [filterInputs.Equal] and [filterInputs.Clone] if necessary. (e.g. non-view
+// and non-value fields)
+type filterInputs struct {
+	HaveNetmap  bool
+	Addrs       views.Slice[netip.Prefix]
+	FilterMatch views.Slice[filter.Match]
+	LocalNets   views.Slice[netipx.IPRange]
+	LogNets     views.Slice[netipx.IPRange]
+	ShieldsUp   bool
+	SSHPolicy   tailcfg.SSHPolicyView
+}
+
+func (fi *filterInputs) Equal(o *filterInputs) bool {
+	if fi == nil || o == nil {
+		return fi == o
+	}
+	return reflect.DeepEqual(fi, o)
+}
+
+func (fi *filterInputs) Clone() *filterInputs {
+	if fi == nil {
+		return nil
+	}
+	v := *fi // all fields are shallow copyable
+	return &v
+}
+
 // updateFilterLocked updates the packet filter in wgengine based on the
 // given netMap and user preferences.
 //
@@ -2722,20 +2752,20 @@ func (b *LocalBackend) updateFilterLocked(prefs ipn.PrefsView) {
 	}
 	localNets, _ := localNetsB.IPSet()
 	logNets, _ := logNetsB.IPSet()
-	var sshPol tailcfg.SSHPolicy
-	if haveNetmap && netMap.SSHPolicy != nil {
-		sshPol = *netMap.SSHPolicy
-	}
-
-	changed := deephash.Update(&b.filterHash, &struct {
-		HaveNetmap  bool
-		Addrs       views.Slice[netip.Prefix]
-		FilterMatch []filter.Match
-		LocalNets   []netipx.IPRange
-		LogNets     []netipx.IPRange
-		ShieldsUp   bool
-		SSHPolicy   tailcfg.SSHPolicy
-	}{haveNetmap, addrs, packetFilter, localNets.Ranges(), logNets.Ranges(), shieldsUp, sshPol})
+	var sshPol tailcfg.SSHPolicyView
+	if buildfeatures.HasSSH && haveNetmap && netMap.SSHPolicy != nil {
+		sshPol = netMap.SSHPolicy.View()
+	}
+
+	changed := checkchange.Update(&b.lastFilterInputs, &filterInputs{
+		HaveNetmap:  haveNetmap,
+		Addrs:       addrs,
+		FilterMatch: views.SliceOf(packetFilter),
+		LocalNets:   views.SliceOf(localNets.Ranges()),
+		LogNets:     views.SliceOf(logNets.Ranges()),
+		ShieldsUp:   shieldsUp,
+		SSHPolicy:   sshPol,
+	})
 	if !changed {
 		return
 	}

+ 21 - 0
net/dns/config.go

@@ -8,6 +8,7 @@ import (
 	"bufio"
 	"fmt"
 	"net/netip"
+	"reflect"
 	"slices"
 	"sort"
 
@@ -188,3 +189,23 @@ func sameResolverNames(a, b []*dnstype.Resolver) bool {
 	}
 	return true
 }
+
+func (c *Config) Clone() *Config {
+	if c == nil {
+		return nil
+	}
+	return &Config{
+		DefaultResolvers: slices.Clone(c.DefaultResolvers),
+		Routes:           make(map[dnsname.FQDN][]*dnstype.Resolver, len(c.Routes)),
+		SearchDomains:    slices.Clone(c.SearchDomains),
+		Hosts:            make(map[dnsname.FQDN][]netip.Addr, len(c.Hosts)),
+		OnlyIPv6:         c.OnlyIPv6,
+	}
+}
+
+func (c *Config) Equal(o *Config) bool {
+	if c == nil || o == nil {
+		return c == o
+	}
+	return reflect.DeepEqual(c, o)
+}

+ 1 - 1
tailcfg/tailcfg.go

@@ -5,7 +5,7 @@
 // the node and the coordination server.
 package tailcfg
 
-//go:generate go run tailscale.com/cmd/viewer --type=User,Node,Hostinfo,NetInfo,Login,DNSConfig,RegisterResponse,RegisterResponseAuth,RegisterRequest,DERPHomeParams,DERPRegion,DERPMap,DERPNode,SSHRule,SSHAction,SSHPrincipal,ControlDialPlan,Location,UserProfile,VIPService --clonefunc
+//go:generate go run tailscale.com/cmd/viewer --type=User,Node,Hostinfo,NetInfo,Login,DNSConfig,RegisterResponse,RegisterResponseAuth,RegisterRequest,DERPHomeParams,DERPRegion,DERPMap,DERPNode,SSHRule,SSHAction,SSHPrincipal,ControlDialPlan,Location,UserProfile,VIPService,SSHPolicy --clonefunc
 
 import (
 	"bytes"

+ 36 - 1
tailcfg/tailcfg_clone.go

@@ -651,9 +651,35 @@ var _VIPServiceCloneNeedsRegeneration = VIPService(struct {
 	Active bool
 }{})
 
+// Clone makes a deep copy of SSHPolicy.
+// The result aliases no memory with the original.
+func (src *SSHPolicy) Clone() *SSHPolicy {
+	if src == nil {
+		return nil
+	}
+	dst := new(SSHPolicy)
+	*dst = *src
+	if src.Rules != nil {
+		dst.Rules = make([]*SSHRule, len(src.Rules))
+		for i := range dst.Rules {
+			if src.Rules[i] == nil {
+				dst.Rules[i] = nil
+			} else {
+				dst.Rules[i] = src.Rules[i].Clone()
+			}
+		}
+	}
+	return dst
+}
+
+// A compilation failure here means this code must be regenerated, with the command at the top of this file.
+var _SSHPolicyCloneNeedsRegeneration = SSHPolicy(struct {
+	Rules []*SSHRule
+}{})
+
 // 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,RegisterResponseAuth,RegisterRequest,DERPHomeParams,DERPRegion,DERPMap,DERPNode,SSHRule,SSHAction,SSHPrincipal,ControlDialPlan,Location,UserProfile,VIPService.
+// where T is one of User,Node,Hostinfo,NetInfo,Login,DNSConfig,RegisterResponse,RegisterResponseAuth,RegisterRequest,DERPHomeParams,DERPRegion,DERPMap,DERPNode,SSHRule,SSHAction,SSHPrincipal,ControlDialPlan,Location,UserProfile,VIPService,SSHPolicy.
 func Clone(dst, src any) bool {
 	switch src := src.(type) {
 	case *User:
@@ -836,6 +862,15 @@ func Clone(dst, src any) bool {
 			*dst = src.Clone()
 			return true
 		}
+	case *SSHPolicy:
+		switch dst := dst.(type) {
+		case *SSHPolicy:
+			*dst = *src.Clone()
+			return true
+		case **SSHPolicy:
+			*dst = src.Clone()
+			return true
+		}
 	}
 	return false
 }

+ 92 - 1
tailcfg/tailcfg_view.go

@@ -21,7 +21,7 @@ import (
 	"tailscale.com/types/views"
 )
 
-//go:generate go run tailscale.com/cmd/cloner  -clonefunc=true -type=User,Node,Hostinfo,NetInfo,Login,DNSConfig,RegisterResponse,RegisterResponseAuth,RegisterRequest,DERPHomeParams,DERPRegion,DERPMap,DERPNode,SSHRule,SSHAction,SSHPrincipal,ControlDialPlan,Location,UserProfile,VIPService
+//go:generate go run tailscale.com/cmd/cloner  -clonefunc=true -type=User,Node,Hostinfo,NetInfo,Login,DNSConfig,RegisterResponse,RegisterResponseAuth,RegisterRequest,DERPHomeParams,DERPRegion,DERPMap,DERPNode,SSHRule,SSHAction,SSHPrincipal,ControlDialPlan,Location,UserProfile,VIPService,SSHPolicy
 
 // View returns a read-only view of User.
 func (p *User) View() UserView {
@@ -2604,3 +2604,94 @@ var _VIPServiceViewNeedsRegeneration = VIPService(struct {
 	Ports  []ProtoPortRange
 	Active bool
 }{})
+
+// View returns a read-only view of SSHPolicy.
+func (p *SSHPolicy) View() SSHPolicyView {
+	return SSHPolicyView{ж: p}
+}
+
+// SSHPolicyView provides a read-only view over SSHPolicy.
+//
+// Its methods should only be called if `Valid()` returns true.
+type SSHPolicyView 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.
+	ж *SSHPolicy
+}
+
+// Valid reports whether v's underlying value is non-nil.
+func (v SSHPolicyView) Valid() bool { return v.ж != nil }
+
+// AsStruct returns a clone of the underlying value which aliases no memory with
+// the original.
+func (v SSHPolicyView) AsStruct() *SSHPolicy {
+	if v.ж == nil {
+		return nil
+	}
+	return v.ж.Clone()
+}
+
+// MarshalJSON implements [jsonv1.Marshaler].
+func (v SSHPolicyView) MarshalJSON() ([]byte, error) {
+	return jsonv1.Marshal(v.ж)
+}
+
+// MarshalJSONTo implements [jsonv2.MarshalerTo].
+func (v SSHPolicyView) MarshalJSONTo(enc *jsontext.Encoder) error {
+	return jsonv2.MarshalEncode(enc, v.ж)
+}
+
+// UnmarshalJSON implements [jsonv1.Unmarshaler].
+func (v *SSHPolicyView) UnmarshalJSON(b []byte) error {
+	if v.ж != nil {
+		return errors.New("already initialized")
+	}
+	if len(b) == 0 {
+		return nil
+	}
+	var x SSHPolicy
+	if err := jsonv1.Unmarshal(b, &x); err != nil {
+		return err
+	}
+	v.ж = &x
+	return nil
+}
+
+// UnmarshalJSONFrom implements [jsonv2.UnmarshalerFrom].
+func (v *SSHPolicyView) UnmarshalJSONFrom(dec *jsontext.Decoder) error {
+	if v.ж != nil {
+		return errors.New("already initialized")
+	}
+	var x SSHPolicy
+	if err := jsonv2.UnmarshalDecode(dec, &x); err != nil {
+		return err
+	}
+	v.ж = &x
+	return nil
+}
+
+// Rules are the rules to process for an incoming SSH connection. The first
+// matching rule takes its action and stops processing further rules.
+//
+// When an incoming connection first starts, all rules are evaluated in
+// "none" auth mode, where the client hasn't even been asked to send a
+// public key. All SSHRule.Principals requiring a public key won't match. If
+// a rule matches on the first pass and its Action is reject, the
+// authentication fails with that action's rejection message, if any.
+//
+// If the first pass rule evaluation matches nothing without matching an
+// Action with Reject set, the rules are considered to see whether public
+// keys might still result in a match. If not, "none" auth is terminated
+// before proceeding to public key mode. If so, the client is asked to try
+// public key authentication and the rules are evaluated again for each of
+// the client's present keys.
+func (v SSHPolicyView) Rules() views.SliceView[*SSHRule, SSHRuleView] {
+	return views.SliceOfViews[*SSHRule, SSHRuleView](v.ж.Rules)
+}
+
+// A compilation failure here means this code must be regenerated, with the command at the top of this file.
+var _SSHPolicyViewNeedsRegeneration = SSHPolicy(struct {
+	Rules []*SSHRule
+}{})

+ 2 - 1
tsnet/depaware.txt

@@ -247,12 +247,13 @@ tailscale.com/tsnet dependencies: (generated by github.com/tailscale/depaware)
         tailscale.com/types/tkatype                                  from tailscale.com/client/local+
         tailscale.com/types/views                                    from tailscale.com/appc+
         tailscale.com/util/backoff                                   from tailscale.com/control/controlclient+
+        tailscale.com/util/checkchange                               from tailscale.com/ipn/ipnlocal+
         tailscale.com/util/cibuild                                   from tailscale.com/health
         tailscale.com/util/clientmetric                              from tailscale.com/appc+
         tailscale.com/util/cloudenv                                  from tailscale.com/hostinfo+
   LW    tailscale.com/util/cmpver                                    from tailscale.com/net/dns+
         tailscale.com/util/ctxkey                                    from tailscale.com/client/tailscale/apitype+
-     💣 tailscale.com/util/deephash                                  from tailscale.com/ipn/ipnlocal+
+     💣 tailscale.com/util/deephash                                  from tailscale.com/util/syspolicy/setting
   LA 💣 tailscale.com/util/dirwalk                                   from tailscale.com/metrics
         tailscale.com/util/dnsname                                   from tailscale.com/appc+
         tailscale.com/util/eventbus                                  from tailscale.com/client/local+

+ 25 - 0
util/checkchange/checkchange.go

@@ -0,0 +1,25 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+// Package checkchange defines a utility for determining whether a value
+// has changed since the last time it was checked.
+package checkchange
+
+// EqualCloner is an interface for types that can be compared for equality
+// and can be cloned.
+type EqualCloner[T any] interface {
+	Equal(T) bool
+	Clone() T
+}
+
+// Update sets *old to a clone of new if they are not equal, returning whether
+// they were different.
+//
+// It only modifies *old if they are different. old must be non-nil.
+func Update[T EqualCloner[T]](old *T, new T) (changed bool) {
+	if new.Equal(*old) {
+		return false
+	}
+	*old = new.Clone()
+	return true
+}

+ 13 - 0
wgengine/router/router.go

@@ -11,6 +11,7 @@ import (
 	"net/netip"
 	"reflect"
 	"runtime"
+	"slices"
 
 	"github.com/tailscale/wireguard-go/tun"
 	"tailscale.com/feature"
@@ -146,3 +147,15 @@ func (a *Config) Equal(b *Config) bool {
 	}
 	return reflect.DeepEqual(a, b)
 }
+
+func (c *Config) Clone() *Config {
+	if c == nil {
+		return nil
+	}
+	c2 := *c
+	c2.LocalAddrs = slices.Clone(c.LocalAddrs)
+	c2.Routes = slices.Clone(c.Routes)
+	c2.LocalRoutes = slices.Clone(c.LocalRoutes)
+	c2.SubnetRoutes = slices.Clone(c.SubnetRoutes)
+	return &c2
+}

+ 39 - 17
wgengine/userspace.go

@@ -10,8 +10,10 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"maps"
 	"math"
 	"net/netip"
+	"reflect"
 	"runtime"
 	"slices"
 	"strings"
@@ -45,8 +47,8 @@ import (
 	"tailscale.com/types/logger"
 	"tailscale.com/types/netmap"
 	"tailscale.com/types/views"
+	"tailscale.com/util/checkchange"
 	"tailscale.com/util/clientmetric"
-	"tailscale.com/util/deephash"
 	"tailscale.com/util/eventbus"
 	"tailscale.com/util/mak"
 	"tailscale.com/util/set"
@@ -128,9 +130,9 @@ type userspaceEngine struct {
 	wgLock              sync.Mutex // serializes all wgdev operations; see lock order comment below
 	lastCfgFull         wgcfg.Config
 	lastNMinPeers       int
-	lastRouterSig       deephash.Sum // of router.Config
-	lastEngineSigFull   deephash.Sum // of full wireguard config
-	lastEngineSigTrim   deephash.Sum // of trimmed wireguard config
+	lastRouter          *router.Config
+	lastEngineFull      *wgcfg.Config // of full wireguard config, not trimmed
+	lastEngineInputs    *maybeReconfigInputs
 	lastDNSConfig       *dns.Config
 	lastIsSubnetRouter  bool // was the node a primary subnet router in the last run.
 	recvActivityAt      map[key.NodePublic]mono.Time
@@ -725,6 +727,29 @@ func (e *userspaceEngine) isActiveSinceLocked(nk key.NodePublic, ip netip.Addr,
 	return timePtr.LoadAtomic().After(t)
 }
 
+// maybeReconfigInputs holds the inputs to the maybeReconfigWireguardLocked
+// function. If these things don't change between calls, there's nothing to do.
+type maybeReconfigInputs struct {
+	WGConfig     *wgcfg.Config
+	TrimmedNodes map[key.NodePublic]bool
+	TrackNodes   views.Slice[key.NodePublic]
+	TrackIPs     views.Slice[netip.Addr]
+}
+
+func (i *maybeReconfigInputs) Equal(o *maybeReconfigInputs) bool {
+	return reflect.DeepEqual(i, o)
+}
+
+func (i *maybeReconfigInputs) Clone() *maybeReconfigInputs {
+	if i == nil {
+		return nil
+	}
+	v := *i
+	v.WGConfig = i.WGConfig.Clone()
+	v.TrimmedNodes = maps.Clone(i.TrimmedNodes)
+	return &v
+}
+
 // discoChanged are the set of peers whose disco keys have changed, implying they've restarted.
 // If a peer is in this set and was previously in the live wireguard config,
 // it needs to be first removed and then re-added to flush out its wireguard session key.
@@ -803,12 +828,12 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked(discoChanged map[key.Node
 	}
 	e.lastNMinPeers = len(min.Peers)
 
-	if changed := deephash.Update(&e.lastEngineSigTrim, &struct {
-		WGConfig     *wgcfg.Config
-		TrimmedNodes map[key.NodePublic]bool
-		TrackNodes   []key.NodePublic
-		TrackIPs     []netip.Addr
-	}{&min, e.trimmedNodes, trackNodes, trackIPs}); !changed {
+	if changed := checkchange.Update(&e.lastEngineInputs, &maybeReconfigInputs{
+		WGConfig:     &min,
+		TrimmedNodes: e.trimmedNodes,
+		TrackNodes:   views.SliceOf(trackNodes),
+		TrackIPs:     views.SliceOf(trackIPs),
+	}); !changed {
 		return nil
 	}
 
@@ -937,7 +962,6 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config,
 	e.wgLock.Lock()
 	defer e.wgLock.Unlock()
 	e.tundev.SetWGConfig(cfg)
-	e.lastDNSConfig = dnsCfg
 
 	peerSet := make(set.Set[key.NodePublic], len(cfg.Peers))
 	e.mu.Lock()
@@ -965,14 +989,12 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config,
 	}
 	isSubnetRouterChanged := isSubnetRouter != e.lastIsSubnetRouter
 
-	engineChanged := deephash.Update(&e.lastEngineSigFull, cfg)
-	routerChanged := deephash.Update(&e.lastRouterSig, &struct {
-		RouterConfig *router.Config
-		DNSConfig    *dns.Config
-	}{routerCfg, dnsCfg})
+	engineChanged := checkchange.Update(&e.lastEngineFull, cfg)
+	dnsChanged := checkchange.Update(&e.lastDNSConfig, dnsCfg)
+	routerChanged := checkchange.Update(&e.lastRouter, routerCfg)
 	listenPortChanged := listenPort != e.magicConn.LocalPort()
 	peerMTUChanged := peerMTUEnable != e.magicConn.PeerMTUEnabled()
-	if !engineChanged && !routerChanged && !listenPortChanged && !isSubnetRouterChanged && !peerMTUChanged {
+	if !engineChanged && !routerChanged && !dnsChanged && !listenPortChanged && !isSubnetRouterChanged && !peerMTUChanged {
 		return ErrNoChanges
 	}
 	newLogIDs := cfg.NetworkLogging

+ 33 - 0
wgengine/wgcfg/config.go

@@ -6,6 +6,7 @@ package wgcfg
 
 import (
 	"net/netip"
+	"slices"
 
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
@@ -35,6 +36,20 @@ type Config struct {
 	}
 }
 
+func (c *Config) Equal(o *Config) bool {
+	if c == nil || o == nil {
+		return c == o
+	}
+	return c.Name == o.Name &&
+		c.NodeID == o.NodeID &&
+		c.PrivateKey.Equal(o.PrivateKey) &&
+		c.MTU == o.MTU &&
+		c.NetworkLogging == o.NetworkLogging &&
+		slices.Equal(c.Addresses, o.Addresses) &&
+		slices.Equal(c.DNS, o.DNS) &&
+		slices.EqualFunc(c.Peers, o.Peers, Peer.Equal)
+}
+
 type Peer struct {
 	PublicKey           key.NodePublic
 	DiscoKey            key.DiscoPublic // present only so we can handle restarts within wgengine, not passed to WireGuard
@@ -50,6 +65,24 @@ type Peer struct {
 	WGEndpoint key.NodePublic
 }
 
+func addrPtrEq(a, b *netip.Addr) bool {
+	if a == nil || b == nil {
+		return a == b
+	}
+	return *a == *b
+}
+
+func (p Peer) Equal(o Peer) bool {
+	return p.PublicKey == o.PublicKey &&
+		p.DiscoKey == o.DiscoKey &&
+		slices.Equal(p.AllowedIPs, o.AllowedIPs) &&
+		p.IsJailed == o.IsJailed &&
+		p.PersistentKeepalive == o.PersistentKeepalive &&
+		addrPtrEq(p.V4MasqAddr, o.V4MasqAddr) &&
+		addrPtrEq(p.V6MasqAddr, o.V6MasqAddr) &&
+		p.WGEndpoint == o.WGEndpoint
+}
+
 // PeerWithKey returns the Peer with key k and reports whether it was found.
 func (config Config) PeerWithKey(k key.NodePublic) (Peer, bool) {
 	for _, p := range config.Peers {

+ 41 - 0
wgengine/wgcfg/config_test.go

@@ -0,0 +1,41 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+package wgcfg
+
+import (
+	"reflect"
+	"testing"
+)
+
+// Tests that [Config.Equal] tests all fields of [Config], even ones
+// that might get added in the future.
+func TestConfigEqual(t *testing.T) {
+	rt := reflect.TypeFor[Config]()
+	for i := range rt.NumField() {
+		sf := rt.Field(i)
+		switch sf.Name {
+		case "Name", "NodeID", "PrivateKey", "MTU", "Addresses", "DNS", "Peers",
+			"NetworkLogging":
+			// These are compared in [Config.Equal].
+		default:
+			t.Errorf("Have you added field %q to Config.Equal? Do so if not, and then update TestConfigEqual", sf.Name)
+		}
+	}
+}
+
+// Tests that [Peer.Equal] tests all fields of [Peer], even ones
+// that might get added in the future.
+func TestPeerEqual(t *testing.T) {
+	rt := reflect.TypeFor[Peer]()
+	for i := range rt.NumField() {
+		sf := rt.Field(i)
+		switch sf.Name {
+		case "PublicKey", "DiscoKey", "AllowedIPs", "IsJailed",
+			"PersistentKeepalive", "V4MasqAddr", "V6MasqAddr", "WGEndpoint":
+			// These are compared in [Peer.Equal].
+		default:
+			t.Errorf("Have you added field %q to Peer.Equal? Do so if not, and then update TestPeerEqual", sf.Name)
+		}
+	}
+}