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 <bradfitz@tailscale.com>

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) {

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) {

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