From 7f174e84e627dc04bc91352e9897f22cf4724191 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 26 Mar 2021 09:09:12 -0700 Subject: [PATCH] net/interfaces: remove mutating methods, add EqualFiltered instead Now callers (wgengine/monitor) don't need to mutate the state to remove boring interfaces before calling State.Equal. Instead, the methods to remove boring interfaces from the State are removed, as is the reflect-using Equal method itself, and in their place is a new EqualFiltered method that takes a func predicate to match interfaces to compare. And then the FilterInteresting predicate is added for use with EqualFiltered to do the job that that wgengine/monitor previously wanted. Now wgengine/monitor can keep the full interface state around, including the "boring" interfaces, which we'll need for peerapi on macOS/iOS to bind to the interface index of the utunN device. Signed-off-by: Brad Fitzpatrick --- net/interfaces/interfaces.go | 105 +++++++++++++++++++----------- net/interfaces/interfaces_test.go | 12 ++-- wgengine/monitor/monitor.go | 9 +-- 3 files changed, 77 insertions(+), 49 deletions(-) diff --git a/net/interfaces/interfaces.go b/net/interfaces/interfaces.go index 625164dfd..f2988af34 100644 --- a/net/interfaces/interfaces.go +++ b/net/interfaces/interfaces.go @@ -6,10 +6,10 @@ package interfaces import ( + "bytes" "fmt" "net" "net/http" - "reflect" "runtime" "sort" "strings" @@ -190,6 +190,9 @@ func ForeachInterface(fn func(Interface, []netaddr.IPPrefix)) error { } } } + sort.Slice(pfxs, func(i, j int) bool { + return pfxs[i].IP.Less(pfxs[j].IP) + }) fn(Interface{iface}, pfxs) } return nil @@ -286,10 +289,71 @@ func (s *State) String() string { return sb.String() } -func (s *State) Equal(s2 *State) bool { - return reflect.DeepEqual(s, s2) +// EqualFiltered reports whether s and s2 are equal, +// considering only interfaces in s for which filter returns true. +func (s *State) EqualFiltered(s2 *State, filter func(i Interface, ips []netaddr.IPPrefix) bool) bool { + if s == nil && s2 == nil { + return true + } + if s == nil || s2 == nil { + return false + } + if s.HaveV6Global != s2.HaveV6Global || + s.HaveV4 != s2.HaveV4 || + s.IsExpensive != s2.IsExpensive || + s.DefaultRouteInterface != s2.DefaultRouteInterface || + s.HTTPProxy != s2.HTTPProxy || + s.PAC != s2.PAC { + return false + } + for iname, i := range s.Interface { + ips := s.InterfaceIPs[iname] + if !filter(i, ips) { + continue + } + i2, ok := s2.Interface[iname] + if !ok { + return false + } + ips2, ok := s2.InterfaceIPs[iname] + if !ok { + return false + } + if !interfacesEqual(i, i2) || !prefixesEqual(ips, ips2) { + return false + } + } + return true } +func interfacesEqual(a, b Interface) bool { + return a.Index == b.Index && + a.MTU == b.MTU && + a.Name == b.Name && + a.Flags == b.Flags && + bytes.Equal([]byte(a.HardwareAddr), []byte(b.HardwareAddr)) +} + +func prefixesEqual(a, b []netaddr.IPPrefix) bool { + if len(a) != len(b) { + return false + } + for i, v := range a { + if b[i] != v { + return false + } + } + return true +} + +// FilterInteresting reports whether i is an interesting non-Tailscale interface. +func FilterInteresting(i Interface, ips []netaddr.IPPrefix) bool { + return !isTailscaleInterface(i.Name, ips) && anyInterestingIP(ips) +} + +// FilterAll always returns true, to use EqualFiltered against all interfaces. +func FilterAll(i Interface, ips []netaddr.IPPrefix) bool { return true } + func (s *State) HasPAC() bool { return s != nil && s.PAC != "" } // AnyInterfaceUp reports whether any interface seems like it has Internet access. @@ -297,41 +361,6 @@ func (s *State) AnyInterfaceUp() bool { return s != nil && (s.HaveV4 || s.HaveV6Global) } -// RemoveUninterestingInterfacesAndAddresses removes uninteresting IPs -// from InterfaceIPs, also removing from both the InterfaceIPs and -// InterfaceUp map any interfaces that don't have any interesting IPs. -func (s *State) RemoveUninterestingInterfacesAndAddresses() { - for ifName := range s.Interface { - ips := s.InterfaceIPs[ifName] - keep := ips[:0] - for _, pfx := range ips { - if isInterestingIP(pfx.IP) { - keep = append(keep, pfx) - } - } - if len(keep) == 0 { - delete(s.Interface, ifName) - delete(s.InterfaceIPs, ifName) - continue - } - if len(keep) < len(ips) { - s.InterfaceIPs[ifName] = keep - } - } -} - -// RemoveTailscaleInterfaces modifes s to remove any interfaces that -// are owned by this process. (TODO: make this true; currently it -// uses some heuristics) -func (s *State) RemoveTailscaleInterfaces() { - for name, pfxs := range s.InterfaceIPs { - if isTailscaleInterface(name, pfxs) { - delete(s.InterfaceIPs, name) - delete(s.Interface, name) - } - } -} - func hasTailscaleIP(pfxs []netaddr.IPPrefix) bool { for _, pfx := range pfxs { if tsaddr.IsTailscaleIP(pfx.IP) { diff --git a/net/interfaces/interfaces_test.go b/net/interfaces/interfaces_test.go index 88948b579..ab0ee3734 100644 --- a/net/interfaces/interfaces_test.go +++ b/net/interfaces/interfaces_test.go @@ -5,6 +5,7 @@ package interfaces import ( + "encoding/json" "testing" ) @@ -13,7 +14,11 @@ func TestGetState(t *testing.T) { if err != nil { t.Fatal(err) } - t.Logf("Got: %#v", st) + j, err := json.MarshalIndent(st, "", "\t") + if err != nil { + t.Errorf("JSON: %v", err) + } + t.Logf("Got: %s", j) t.Logf("As string: %s", st) st2, err := GetState() @@ -21,14 +26,13 @@ func TestGetState(t *testing.T) { t.Fatal(err) } - if !st.Equal(st2) { + if !st.EqualFiltered(st2, FilterAll) { // let's assume nobody was changing the system network interfaces between // the two GetState calls. t.Fatal("two States back-to-back were not equal") } - st.RemoveTailscaleInterfaces() - t.Logf("As string without Tailscale:\n\t%s", st) + t.Logf("As string:\n\t%s", st) } func TestLikelyHomeRouterIP(t *testing.T) { diff --git a/wgengine/monitor/monitor.go b/wgengine/monitor/monitor.go index 254df7cb6..3d88a9a8b 100644 --- a/wgengine/monitor/monitor.go +++ b/wgengine/monitor/monitor.go @@ -101,12 +101,7 @@ func (m *Mon) InterfaceState() *interfaces.State { } func (m *Mon) interfaceStateUncached() (*interfaces.State, error) { - s, err := interfaces.GetState() - if s != nil { - s.RemoveTailscaleInterfaces() - s.RemoveUninterestingInterfacesAndAddresses() - } - return s, err + return interfaces.GetState() } // GatewayAndSelfIP returns the current network's default gateway, and @@ -233,7 +228,7 @@ func (m *Mon) debounce() { } else { m.mu.Lock() oldState := m.ifState - changed := !curState.Equal(oldState) + changed := !curState.EqualFiltered(oldState, interfaces.FilterInteresting) if changed { m.gwValid = false m.ifState = curState