diff --git a/wgengine/router/callback.go b/wgengine/router/callback.go index b66bec0c8..dd5676c80 100644 --- a/wgengine/router/callback.go +++ b/wgengine/router/callback.go @@ -13,7 +13,7 @@ // CallbackRouter is an implementation of both Router and dns.OSConfigurator. // When either network or DNS settings are changed, SetBoth is called with both configs. // Mainly used as a shim for OSes that want to set both network and -// DNS configuration simultaneously (iOS, android). +// DNS configuration simultaneously (Mac, iOS, Android). type CallbackRouter struct { SetBoth func(rcfg *Config, dcfg *dns.OSConfig) error SplitDNS bool @@ -39,6 +39,9 @@ func (r *CallbackRouter) Up() error { func (r *CallbackRouter) Set(rcfg *Config) error { r.mu.Lock() defer r.mu.Unlock() + if r.rcfg.Equal(rcfg) { + return nil + } r.rcfg = rcfg return r.SetBoth(r.rcfg, r.dcfg) } @@ -47,6 +50,9 @@ func (r *CallbackRouter) Set(rcfg *Config) error { func (r *CallbackRouter) SetDNS(dcfg dns.OSConfig) error { r.mu.Lock() defer r.mu.Unlock() + if r.dcfg != nil && r.dcfg.Equal(dcfg) { + return nil + } r.dcfg = &dcfg return r.SetBoth(r.rcfg, r.dcfg) } diff --git a/wgengine/router/router.go b/wgengine/router/router.go index 0a162dc28..b71ae839e 100644 --- a/wgengine/router/router.go +++ b/wgengine/router/router.go @@ -7,6 +7,8 @@ package router import ( + "reflect" + "golang.zx2c4.com/wireguard/tun" "inet.af/netaddr" "tailscale.com/types/logger" @@ -72,6 +74,16 @@ type Config struct { NetfilterMode preftype.NetfilterMode // how much to manage netfilter rules } +func (a *Config) Equal(b *Config) bool { + if a == nil && b == nil { + return true + } + if (a == nil) != (b == nil) { + return false + } + return reflect.DeepEqual(a, b) +} + // shutdownConfig is a routing configuration that removes all router // state from the OS. It's the config used when callers pass in a nil // Config. diff --git a/wgengine/router/router_test.go b/wgengine/router/router_test.go index e493f1c48..fe068602c 100644 --- a/wgengine/router/router_test.go +++ b/wgengine/router/router_test.go @@ -2,12 +2,15 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -//go:build linux || windows -// +build linux windows - package router -import "inet.af/netaddr" +import ( + "reflect" + "testing" + + "inet.af/netaddr" + "tailscale.com/types/preftype" +) func mustCIDRs(ss ...string) []netaddr.IPPrefix { var ret []netaddr.IPPrefix @@ -16,3 +19,127 @@ func mustCIDRs(ss ...string) []netaddr.IPPrefix { } return ret } + +func TestConfigEqual(t *testing.T) { + testedFields := []string{ + "LocalAddrs", "Routes", "LocalRoutes", "SubnetRoutes", + "SNATSubnetRoutes", "NetfilterMode", + } + configType := reflect.TypeOf(Config{}) + configFields := []string{} + for i := 0; i < configType.NumField(); i++ { + configFields = append(configFields, configType.Field(i).Name) + } + if !reflect.DeepEqual(configFields, testedFields) { + t.Errorf("Config.Equal check might be out of sync\nfields: %q\nhandled: %q\n", + configFields, testedFields) + } + + nets := func(strs ...string) (ns []netaddr.IPPrefix) { + for _, s := range strs { + n, err := netaddr.ParseIPPrefix(s) + if err != nil { + panic(err) + } + ns = append(ns, n) + } + return ns + } + tests := []struct { + a, b *Config + want bool + }{ + { + nil, + nil, + true, + }, + { + &Config{}, + nil, + false, + }, + { + nil, + &Config{}, + false, + }, + { + &Config{}, + &Config{}, + true, + }, + + { + &Config{LocalAddrs: nets("100.1.27.82/32")}, + &Config{LocalAddrs: nets("100.2.19.82/32")}, + false, + }, + { + &Config{LocalAddrs: nets("100.1.27.82/32")}, + &Config{LocalAddrs: nets("100.1.27.82/32")}, + true, + }, + + { + &Config{Routes: nets("100.1.27.0/24")}, + &Config{Routes: nets("100.2.19.0/24")}, + false, + }, + { + &Config{Routes: nets("100.2.19.0/24")}, + &Config{Routes: nets("100.2.19.0/24")}, + true, + }, + + { + &Config{LocalRoutes: nets("100.1.27.0/24")}, + &Config{LocalRoutes: nets("100.2.19.0/24")}, + false, + }, + { + &Config{LocalRoutes: nets("100.1.27.0/24")}, + &Config{LocalRoutes: nets("100.1.27.0/24")}, + true, + }, + + { + &Config{SubnetRoutes: nets("100.1.27.0/24")}, + &Config{SubnetRoutes: nets("100.2.19.0/24")}, + false, + }, + { + &Config{SubnetRoutes: nets("100.1.27.0/24")}, + &Config{SubnetRoutes: nets("100.1.27.0/24")}, + true, + }, + + { + &Config{SNATSubnetRoutes: false}, + &Config{SNATSubnetRoutes: true}, + false, + }, + { + &Config{SNATSubnetRoutes: false}, + &Config{SNATSubnetRoutes: false}, + true, + }, + + { + &Config{NetfilterMode: preftype.NetfilterOff}, + &Config{NetfilterMode: preftype.NetfilterNoDivert}, + false, + }, + { + &Config{NetfilterMode: preftype.NetfilterNoDivert}, + &Config{NetfilterMode: preftype.NetfilterNoDivert}, + true, + }, + } + for i, tt := range tests { + got := tt.a.Equal(tt.b) + if got != tt.want { + t.Errorf("%d. Equal = %v; want %v", i, got, tt.want) + } + } +}