wgengine{,/netstack}: remove AddNetworkMapCallback from Engine interface

It had exactly one user: netstack. Just have LocalBackend notify
netstack when here's a new netmap instead, simplifying the bloated
Engine interface that has grown a bunch of non-Engine-y things.
(plenty of rando stuff remains after this, but it's a start)

Updates #cleanup

Change-Id: I45e10ab48119e962fc4967a95167656e35b141d8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

wgengine/netstack/netstack.go

@@ -44,6 +44,7 @@ import (
 	"tailscale.com/net/tsdial"
 	"tailscale.com/net/tstun"
 	"tailscale.com/syncs"
+	"tailscale.com/tailcfg"
 	"tailscale.com/types/ipproto"
 	"tailscale.com/types/logger"
 	"tailscale.com/types/netmap"
@@ -253,7 +254,6 @@ func (ns *Impl) Start(lb *ipnlocal.LocalBackend) error {
 		panic("nil LocalBackend")
 	}
 	ns.lb = lb
-	ns.e.AddNetworkMapCallback(ns.updateIPs)
 	// size = 0 means use default buffer size
 	const tcpReceiveBufferSize = 0
 	const maxInFlightConnectionAttempts = 1024
@@ -310,8 +310,19 @@ func ipPrefixToAddressWithPrefix(ipp netip.Prefix) tcpip.AddressWithPrefix {
 
 var v4broadcast = netaddr.IPv4(255, 255, 255, 255)
 
-func (ns *Impl) updateIPs(nm *netmap.NetworkMap) {
-	ns.atomicIsLocalIPFunc.Store(tsaddr.NewContainsIPFunc(nm.Addresses))
+// UpdateNetstackIPs updates the set of local IPs that netstack should handle
+// from nm.
+//
+// TODO(bradfitz): don't pass the whole netmap here; just pass the two
+// address slice views.
+func (ns *Impl) UpdateNetstackIPs(nm *netmap.NetworkMap) {
+	var selfNode tailcfg.NodeView
+	if nm != nil {
+		ns.atomicIsLocalIPFunc.Store(tsaddr.NewContainsIPFunc(nm.Addresses))
+		selfNode = nm.SelfNode
+	} else {
+		ns.atomicIsLocalIPFunc.Store(tsaddr.NewContainsIPFunc(nil))
+	}
 
 	oldIPs := make(map[tcpip.AddressWithPrefix]bool)
 	for _, protocolAddr := range ns.ipstack.AllAddresses()[nicID] {
@@ -328,14 +339,14 @@ func (ns *Impl) updateIPs(nm *netmap.NetworkMap) {
 	newIPs := make(map[tcpip.AddressWithPrefix]bool)
 
 	isAddr := map[netip.Prefix]bool{}
-	if nm.SelfNode.Valid() {
-		for i := range nm.SelfNode.Addresses().LenIter() {
-			ipp := nm.SelfNode.Addresses().At(i)
+	if selfNode.Valid() {
+		for i := range selfNode.Addresses().LenIter() {
+			ipp := selfNode.Addresses().At(i)
 			isAddr[ipp] = true
 			newIPs[ipPrefixToAddressWithPrefix(ipp)] = true
 		}
-		for i := range nm.SelfNode.AllowedIPs().LenIter() {
-			ipp := nm.SelfNode.AllowedIPs().At(i)
+		for i := range selfNode.AllowedIPs().LenIter() {
+			ipp := selfNode.AllowedIPs().At(i)
 			if !isAddr[ipp] && ns.ProcessSubnets {
 				newIPs[ipPrefixToAddressWithPrefix(ipp)] = true
 			}

wgengine/userspace.go

@@ -126,15 +126,14 @@ type userspaceEngine struct {
 	statusBufioReader   *bufio.Reader // reusable for UAPI
 	lastStatusPollTime  mono.Time     // last time we polled the engine status
 
-	mu                  sync.Mutex         // guards following; see lock order comment below
-	netMap              *netmap.NetworkMap // or nil
-	closing             bool               // Close was called (even if we're still closing)
-	statusCallback      StatusCallback
-	peerSequence        []key.NodePublic
-	endpoints           []tailcfg.Endpoint
-	pendOpen            map[flowtrack.Tuple]*pendingOpenFlow // see pendopen.go
-	networkMapCallbacks set.HandleSet[NetworkMapCallback]
-	tsIPByIPPort        map[netip.AddrPort]netip.Addr // allows registration of IP:ports as belonging to a certain Tailscale IP for whois lookups
+	mu             sync.Mutex         // guards following; see lock order comment below
+	netMap         *netmap.NetworkMap // or nil
+	closing        bool               // Close was called (even if we're still closing)
+	statusCallback StatusCallback
+	peerSequence   []key.NodePublic
+	endpoints      []tailcfg.Endpoint
+	pendOpen       map[flowtrack.Tuple]*pendingOpenFlow // see pendopen.go
+	tsIPByIPPort   map[netip.AddrPort]netip.Addr        // allows registration of IP:ports as belonging to a certain Tailscale IP for whois lookups
 
 	// pongCallback is the map of response handlers waiting for disco or TSMP
 	// pong callbacks. The map key is a random slice of bytes.
@@ -1158,20 +1157,6 @@ func (e *userspaceEngine) linkChange(delta *netmon.ChangeDelta) {
 	e.magicConn.ReSTUN(why)
 }
 
-func (e *userspaceEngine) AddNetworkMapCallback(cb NetworkMapCallback) func() {
-	e.mu.Lock()
-	defer e.mu.Unlock()
-	if e.networkMapCallbacks == nil {
-		e.networkMapCallbacks = make(set.HandleSet[NetworkMapCallback])
-	}
-	h := e.networkMapCallbacks.Add(cb)
-	return func() {
-		e.mu.Lock()
-		defer e.mu.Unlock()
-		delete(e.networkMapCallbacks, h)
-	}
-}
-
 func (e *userspaceEngine) SetNetInfoCallback(cb NetInfoCallback) {
 	e.magicConn.SetNetInfoCallback(cb)
 }
@@ -1184,14 +1169,7 @@ func (e *userspaceEngine) SetNetworkMap(nm *netmap.NetworkMap) {
 	e.magicConn.SetNetworkMap(nm)
 	e.mu.Lock()
 	e.netMap = nm
-	callbacks := make([]NetworkMapCallback, 0, 4)
-	for _, fn := range e.networkMapCallbacks {
-		callbacks = append(callbacks, fn)
-	}
 	e.mu.Unlock()
-	for _, fn := range callbacks {
-		fn(nm)
-	}
 }
 
 func (e *userspaceEngine) DiscoPublicKey() key.DiscoPublic {

wgengine/watchdog.go

@@ -149,11 +149,6 @@ func (e *watchdogEngine) SetDERPMap(m *tailcfg.DERPMap) {
 func (e *watchdogEngine) SetNetworkMap(nm *netmap.NetworkMap) {
 	e.watchdog("SetNetworkMap", func() { e.wrap.SetNetworkMap(nm) })
 }
-func (e *watchdogEngine) AddNetworkMapCallback(callback NetworkMapCallback) func() {
-	var fn func()
-	e.watchdog("AddNetworkMapCallback", func() { fn = e.wrap.AddNetworkMapCallback(callback) })
-	return func() { e.watchdog("RemoveNetworkMapCallback", fn) }
-}
 func (e *watchdogEngine) DiscoPublicKey() (k key.DiscoPublic) {
 	e.watchdog("DiscoPublicKey", func() { k = e.wrap.DiscoPublicKey() })
 	return k

wgengine/wgengine.go

@@ -125,12 +125,6 @@ type Engine interface {
 	// The network map should only be read from.
 	SetNetworkMap(*netmap.NetworkMap)
 
-	// AddNetworkMapCallback adds a function to a list of callbacks
-	// that are called when the network map updates. It returns a
-	// function that when called would remove the function from the
-	// list of callbacks.
-	AddNetworkMapCallback(NetworkMapCallback) (removeCallback func())
-
 	// SetNetInfoCallback sets the function to call when a
 	// new NetInfo summary is available.
 	SetNetInfoCallback(NetInfoCallback)