VERSION.txt: this is v1.58.1

Signed-off-by: kari-ts <kari@tailscale.com>

VERSION.txt

@@ -1 +1 @@
-1.57.0
+1.58.1

appc/appconnector.go

@@ -10,6 +10,7 @@
 package appc
 
 import (
+	"context"
 	"net/netip"
 	"slices"
 	"strings"
@@ -20,17 +21,18 @@ import (
 	"tailscale.com/types/logger"
 	"tailscale.com/types/views"
 	"tailscale.com/util/dnsname"
+	"tailscale.com/util/execqueue"
 )
 
 // RouteAdvertiser is an interface that allows the AppConnector to advertise
 // newly discovered routes that need to be served through the AppConnector.
 type RouteAdvertiser interface {
-	// AdvertiseRoute adds a new route advertisement if the route is not already
-	// being advertised.
-	AdvertiseRoute(netip.Prefix) error
+	// AdvertiseRoute adds one or more route advertisements skipping any that
+	// are already advertised.
+	AdvertiseRoute(...netip.Prefix) error
 
-	// UnadvertiseRoute removes a route advertisement.
-	UnadvertiseRoute(netip.Prefix) error
+	// UnadvertiseRoute removes any matching route advertisements.
+	UnadvertiseRoute(...netip.Prefix) error
 }
 
 // AppConnector is an implementation of an AppConnector that performs
@@ -58,6 +60,9 @@ type AppConnector struct {
 
 	// wildcards is the list of domain strings that match subdomains.
 	wildcards []string
+
+	// queue provides ordering for update operations
+	queue execqueue.ExecQueue
 }
 
 // NewAppConnector creates a new AppConnector.
@@ -68,11 +73,33 @@ func NewAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser) *AppConn
 	}
 }
 
-// UpdateDomains replaces the current set of configured domains with the
-// supplied set of domains. Domains must not contain a trailing dot, and should
-// be lower case. If the domain contains a leading '*' label it matches all
-// subdomains of a domain.
+// UpdateDomainsAndRoutes starts an asynchronous update of the configuration
+// given the new domains and routes.
+func (e *AppConnector) UpdateDomainsAndRoutes(domains []string, routes []netip.Prefix) {
+	e.queue.Add(func() {
+		// Add the new routes first.
+		e.updateRoutes(routes)
+		e.updateDomains(domains)
+	})
+}
+
+// UpdateDomains asynchronously replaces the current set of configured domains
+// with the supplied set of domains. Domains must not contain a trailing dot,
+// and should be lower case. If the domain contains a leading '*' label it
+// matches all subdomains of a domain.
 func (e *AppConnector) UpdateDomains(domains []string) {
+	e.queue.Add(func() {
+		e.updateDomains(domains)
+	})
+}
+
+// Wait waits for the currently scheduled asynchronous configuration changes to
+// complete.
+func (e *AppConnector) Wait(ctx context.Context) {
+	e.queue.Wait(ctx)
+}
+
+func (e *AppConnector) updateDomains(domains []string) {
 	e.mu.Lock()
 	defer e.mu.Unlock()
 
@@ -104,11 +131,11 @@ func (e *AppConnector) UpdateDomains(domains []string) {
 	e.logf("handling domains: %v and wildcards: %v", xmaps.Keys(e.domains), e.wildcards)
 }
 
-// UpdateRoutes merges the supplied routes into the currently configured routes. The routes supplied
+// updateRoutes merges the supplied routes into the currently configured routes. The routes supplied
 // by control for UpdateRoutes are supplemental to the routes discovered by DNS resolution, but are
 // also more often whole ranges. UpdateRoutes will remove any single address routes that are now
 // covered by new ranges.
-func (e *AppConnector) UpdateRoutes(routes []netip.Prefix) {
+func (e *AppConnector) updateRoutes(routes []netip.Prefix) {
 	e.mu.Lock()
 	defer e.mu.Unlock()
 
@@ -117,26 +144,30 @@ func (e *AppConnector) UpdateRoutes(routes []netip.Prefix) {
 		return
 	}
 
+	if err := e.routeAdvertiser.AdvertiseRoute(routes...); err != nil {
+		e.logf("failed to advertise routes: %v: %v", routes, err)
+		return
+	}
+
+	var toRemove []netip.Prefix
+
 nextRoute:
 	for _, r := range routes {
-		if err := e.routeAdvertiser.AdvertiseRoute(r); err != nil {
-			e.logf("failed to advertise route: %v: %v", r, err)
-			continue
-		}
-
 		for _, addr := range e.domains {
 			for _, a := range addr {
-				if r.Contains(a) {
+				if r.Contains(a) && netip.PrefixFrom(a, a.BitLen()) != r {
 					pfx := netip.PrefixFrom(a, a.BitLen())
-					if err := e.routeAdvertiser.UnadvertiseRoute(pfx); err != nil {
-						e.logf("failed to unadvertise route: %v: %v", pfx, err)
-					}
+					toRemove = append(toRemove, pfx)
 					continue nextRoute
 				}
 			}
 		}
 	}
 
+	if err := e.routeAdvertiser.UnadvertiseRoute(toRemove...); err != nil {
+		e.logf("failed to unadvertise routes: %v: %v", toRemove, err)
+	}
+
 	e.controlRoutes = routes
 }
 

appc/appconnector_test.go

@@ -4,6 +4,7 @@
 package appc
 
 import (
+	"context"
 	"net/netip"
 	"reflect"
 	"slices"
@@ -11,13 +12,17 @@ import (
 
 	xmaps "golang.org/x/exp/maps"
 	"golang.org/x/net/dns/dnsmessage"
+	"tailscale.com/appc/appctest"
 	"tailscale.com/util/mak"
 	"tailscale.com/util/must"
 )
 
 func TestUpdateDomains(t *testing.T) {
+	ctx := context.Background()
 	a := NewAppConnector(t.Logf, nil)
 	a.UpdateDomains([]string{"example.com"})
+
+	a.Wait(ctx)
 	if got, want := a.Domains().AsSlice(), []string{"example.com"}; !slices.Equal(got, want) {
 		t.Errorf("got %v; want %v", got, want)
 	}
@@ -25,6 +30,7 @@ func TestUpdateDomains(t *testing.T) {
 	addr := netip.MustParseAddr("192.0.0.8")
 	a.domains["example.com"] = append(a.domains["example.com"], addr)
 	a.UpdateDomains([]string{"example.com"})
+	a.Wait(ctx)
 
 	if got, want := a.domains["example.com"], []netip.Addr{addr}; !slices.Equal(got, want) {
 		t.Errorf("got %v; want %v", got, want)
@@ -32,39 +38,66 @@ func TestUpdateDomains(t *testing.T) {
 
 	// domains are explicitly downcased on set.
 	a.UpdateDomains([]string{"UP.EXAMPLE.COM"})
+	a.Wait(ctx)
 	if got, want := xmaps.Keys(a.domains), []string{"up.example.com"}; !slices.Equal(got, want) {
 		t.Errorf("got %v; want %v", got, want)
 	}
 }
 
 func TestUpdateRoutes(t *testing.T) {
-	rc := &routeCollector{}
+	ctx := context.Background()
+	rc := &appctest.RouteCollector{}
 	a := NewAppConnector(t.Logf, rc)
-	routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")}
-	a.UpdateRoutes(routes)
+	a.updateDomains([]string{"*.example.com"})
 
-	if !slices.EqualFunc(routes, rc.routes, prefixEqual) {
-		t.Fatalf("got %v, want %v", rc.routes, routes)
+	// This route should be collapsed into the range
+	a.ObserveDNSResponse(dnsResponse("a.example.com.", "192.0.2.1"))
+	a.Wait(ctx)
+
+	if !slices.Equal(rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}) {
+		t.Fatalf("got %v, want %v", rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")})
+	}
+
+	// This route should not be collapsed or removed
+	a.ObserveDNSResponse(dnsResponse("b.example.com.", "192.0.0.1"))
+	a.Wait(ctx)
+
+	routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24"), netip.MustParsePrefix("192.0.0.1/32")}
+	a.updateRoutes(routes)
+
+	slices.SortFunc(rc.Routes(), prefixCompare)
+	rc.SetRoutes(slices.Compact(rc.Routes()))
+	slices.SortFunc(routes, prefixCompare)
+
+	// Ensure that the non-matching /32 is preserved, even though it's in the domains table.
+	if !slices.EqualFunc(routes, rc.Routes(), prefixEqual) {
+		t.Errorf("added routes: got %v, want %v", rc.Routes(), routes)
+	}
+
+	// Ensure that the contained /32 is removed, replaced by the /24.
+	wantRemoved := []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}
+	if !slices.EqualFunc(rc.RemovedRoutes(), wantRemoved, prefixEqual) {
+		t.Fatalf("unexpected removed routes: %v", rc.RemovedRoutes())
 	}
 }
 
 func TestUpdateRoutesUnadvertisesContainedRoutes(t *testing.T) {
-	rc := &routeCollector{}
+	rc := &appctest.RouteCollector{}
 	a := NewAppConnector(t.Logf, rc)
 	mak.Set(&a.domains, "example.com", []netip.Addr{netip.MustParseAddr("192.0.2.1")})
-	rc.routes = []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}
+	rc.SetRoutes([]netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")})
 	routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")}
-	a.UpdateRoutes(routes)
+	a.updateRoutes(routes)
 
-	if !slices.EqualFunc(routes, rc.routes, prefixEqual) {
-		t.Fatalf("got %v, want %v", rc.routes, routes)
+	if !slices.EqualFunc(routes, rc.Routes(), prefixEqual) {
+		t.Fatalf("got %v, want %v", rc.Routes(), routes)
 	}
 }
 
 func TestDomainRoutes(t *testing.T) {
-	rc := &routeCollector{}
+	rc := &appctest.RouteCollector{}
 	a := NewAppConnector(t.Logf, rc)
-	a.UpdateDomains([]string{"example.com"})
+	a.updateDomains([]string{"example.com"})
 	a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8"))
 
 	want := map[string][]netip.Addr{
@@ -77,43 +110,43 @@ func TestDomainRoutes(t *testing.T) {
 }
 
 func TestObserveDNSResponse(t *testing.T) {
-	rc := &routeCollector{}
+	rc := &appctest.RouteCollector{}
 	a := NewAppConnector(t.Logf, rc)
 
 	// a has no domains configured, so it should not advertise any routes
 	a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8"))
-	if got, want := rc.routes, ([]netip.Prefix)(nil); !slices.Equal(got, want) {
+	if got, want := rc.Routes(), ([]netip.Prefix)(nil); !slices.Equal(got, want) {
 		t.Errorf("got %v; want %v", got, want)
 	}
 
 	wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")}
 
-	a.UpdateDomains([]string{"example.com"})
+	a.updateDomains([]string{"example.com"})
 	a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8"))
-	if got, want := rc.routes, wantRoutes; !slices.Equal(got, want) {
+	if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) {
 		t.Errorf("got %v; want %v", got, want)
 	}
 
 	wantRoutes = append(wantRoutes, netip.MustParsePrefix("2001:db8::1/128"))
 
 	a.ObserveDNSResponse(dnsResponse("example.com.", "2001:db8::1"))
-	if got, want := rc.routes, wantRoutes; !slices.Equal(got, want) {
+	if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) {
 		t.Errorf("got %v; want %v", got, want)
 	}
 
 	// don't re-advertise routes that have already been advertised
 	a.ObserveDNSResponse(dnsResponse("example.com.", "2001:db8::1"))
-	if !slices.Equal(rc.routes, wantRoutes) {
-		t.Errorf("rc.routes: got %v; want %v", rc.routes, wantRoutes)
+	if !slices.Equal(rc.Routes(), wantRoutes) {
+		t.Errorf("rc.Routes(): got %v; want %v", rc.Routes(), wantRoutes)
 	}
 
 	// don't advertise addresses that are already in a control provided route
 	pfx := netip.MustParsePrefix("192.0.2.0/24")
-	a.UpdateRoutes([]netip.Prefix{pfx})
+	a.updateRoutes([]netip.Prefix{pfx})
 	wantRoutes = append(wantRoutes, pfx)
 	a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.2.1"))
-	if !slices.Equal(rc.routes, wantRoutes) {
-		t.Errorf("rc.routes: got %v; want %v", rc.routes, wantRoutes)
+	if !slices.Equal(rc.Routes(), wantRoutes) {
+		t.Errorf("rc.Routes(): got %v; want %v", rc.Routes(), wantRoutes)
 	}
 	if !slices.Contains(a.domains["example.com"], netip.MustParseAddr("192.0.2.1")) {
 		t.Errorf("missing %v from %v", "192.0.2.1", a.domains["exmaple.com"])
@@ -121,19 +154,19 @@ func TestObserveDNSResponse(t *testing.T) {
 }
 
 func TestWildcardDomains(t *testing.T) {
-	rc := &routeCollector{}
+	rc := &appctest.RouteCollector{}
 	a := NewAppConnector(t.Logf, rc)
 
-	a.UpdateDomains([]string{"*.example.com"})
+	a.updateDomains([]string{"*.example.com"})
 	a.ObserveDNSResponse(dnsResponse("foo.example.com.", "192.0.0.8"))
-	if got, want := rc.routes, []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")}; !slices.Equal(got, want) {
+	if got, want := rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")}; !slices.Equal(got, want) {
 		t.Errorf("routes: got %v; want %v", got, want)
 	}
 	if got, want := a.wildcards, []string{"example.com"}; !slices.Equal(got, want) {
 		t.Errorf("wildcards: got %v; want %v", got, want)
 	}
 
-	a.UpdateDomains([]string{"*.example.com", "example.com"})
+	a.updateDomains([]string{"*.example.com", "example.com"})
 	if _, ok := a.domains["foo.example.com"]; !ok {
 		t.Errorf("expected foo.example.com to be preserved in domains due to wildcard")
 	}
@@ -142,7 +175,7 @@ func TestWildcardDomains(t *testing.T) {
 	}
 
 	// There was an early regression where the wildcard domain was added repeatedly, this guards against that.
-	a.UpdateDomains([]string{"*.example.com", "example.com"})
+	a.updateDomains([]string{"*.example.com", "example.com"})
 	if len(a.wildcards) != 1 {
 		t.Errorf("expected only one wildcard domain, got %v", a.wildcards)
 	}
@@ -185,30 +218,13 @@ func dnsResponse(domain, address string) []byte {
 	return must.Get(b.Finish())
 }
 
-// routeCollector is a test helper that collects the list of routes advertised
-type routeCollector struct {
-	routes []netip.Prefix
-}
-
-// routeCollector implements RouteAdvertiser
-var _ RouteAdvertiser = (*routeCollector)(nil)
-
-func (rc *routeCollector) AdvertiseRoute(pfx netip.Prefix) error {
-	rc.routes = append(rc.routes, pfx)
-	return nil
-}
-
-func (rc *routeCollector) UnadvertiseRoute(pfx netip.Prefix) error {
-	routes := rc.routes
-	rc.routes = rc.routes[:0]
-	for _, r := range routes {
-		if r != pfx {
-			rc.routes = append(rc.routes, r)
-		}
-	}
-	return nil
-}
-
 func prefixEqual(a, b netip.Prefix) bool {
-	return a.Addr().Compare(b.Addr()) == 0 && a.Bits() == b.Bits()
+	return a == b
+}
+
+func prefixCompare(a, b netip.Prefix) int {
+	if a.Addr().Compare(b.Addr()) == 0 {
+		return a.Bits() - b.Bits()
+	}
+	return a.Addr().Compare(b.Addr())
 }

appc/appctest/appctest.go

@@ -0,0 +1,49 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+package appctest
+
+import (
+	"net/netip"
+	"slices"
+)
+
+// RouteCollector is a test helper that collects the list of routes advertised
+type RouteCollector struct {
+	routes        []netip.Prefix
+	removedRoutes []netip.Prefix
+}
+
+func (rc *RouteCollector) AdvertiseRoute(pfx ...netip.Prefix) error {
+	rc.routes = append(rc.routes, pfx...)
+	return nil
+}
+
+func (rc *RouteCollector) UnadvertiseRoute(toRemove ...netip.Prefix) error {
+	routes := rc.routes
+	rc.routes = rc.routes[:0]
+	for _, r := range routes {
+		if !slices.Contains(toRemove, r) {
+			rc.routes = append(rc.routes, r)
+		} else {
+			rc.removedRoutes = append(rc.removedRoutes, r)
+		}
+	}
+	return nil
+}
+
+// RemovedRoutes returns the list of routes that were removed.
+func (rc *RouteCollector) RemovedRoutes() []netip.Prefix {
+	return rc.removedRoutes
+}
+
+// Routes returns the ordered list of routes that were added, including
+// possible duplicates.
+func (rc *RouteCollector) Routes() []netip.Prefix {
+	return rc.routes
+}
+
+func (rc *RouteCollector) SetRoutes(routes []netip.Prefix) error {
+	rc.routes = routes
+	return nil
+}

cmd/tailscaled/depaware.txt

@@ -348,6 +348,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
      💣 tailscale.com/util/deephash                                  from tailscale.com/ipn/ipnlocal+
    L 💣 tailscale.com/util/dirwalk                                   from tailscale.com/metrics+
         tailscale.com/util/dnsname                                   from tailscale.com/hostinfo+
+        tailscale.com/util/execqueue                                 from tailscale.com/control/controlclient+
         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

control/controlclient/auto.go

@@ -22,6 +22,7 @@ import (
 	"tailscale.com/types/netmap"
 	"tailscale.com/types/persist"
 	"tailscale.com/types/structs"
+	"tailscale.com/util/execqueue"
 )
 
 type LoginGoal struct {
@@ -118,7 +119,7 @@ type Auto struct {
 	closed        bool
 	updateCh      chan struct{} // readable when we should inform the server of a change
 	observer      Observer      // called to update Client status; always non-nil
-	observerQueue execQueue
+	observerQueue execqueue.ExecQueue
 
 	unregisterHealthWatch func()
 
@@ -675,7 +676,7 @@ func (c *Auto) Shutdown() {
 	direct := c.direct
 	if !closed {
 		c.closed = true
-		c.observerQueue.shutdown()
+		c.observerQueue.Shutdown()
 		c.cancelAuthCtxLocked()
 		c.cancelMapCtxLocked()
 		for _, w := range c.unpauseWaiters {
@@ -696,7 +697,7 @@ func (c *Auto) Shutdown() {
 		}
 		ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 		defer cancel()
-		c.observerQueue.wait(ctx)
+		c.observerQueue.Wait(ctx)
 		c.logf("Client.Shutdown done.")
 	}
 }
@@ -737,95 +738,3 @@ func (c *Auto) DoNoiseRequest(req *http.Request) (*http.Response, error) {
 func (c *Auto) GetSingleUseNoiseRoundTripper(ctx context.Context) (http.RoundTripper, *tailcfg.EarlyNoise, error) {
 	return c.direct.GetSingleUseNoiseRoundTripper(ctx)
 }
-
-type execQueue struct {
-	mu         sync.Mutex
-	closed     bool
-	inFlight   bool          // whether a goroutine is running q.run
-	doneWaiter chan struct{} // non-nil if waiter is waiting, then closed
-	queue      []func()
-}
-
-func (q *execQueue) Add(f func()) {
-	q.mu.Lock()
-	defer q.mu.Unlock()
-	if q.closed {
-		return
-	}
-	if q.inFlight {
-		q.queue = append(q.queue, f)
-	} else {
-		q.inFlight = true
-		go q.run(f)
-	}
-}
-
-// RunSync waits for the queue to be drained and then synchronously runs f.
-// It returns an error if the queue is closed before f is run or ctx expires.
-func (q *execQueue) RunSync(ctx context.Context, f func()) error {
-	for {
-		if err := q.wait(ctx); err != nil {
-			return err
-		}
-		q.mu.Lock()
-		if q.inFlight {
-			q.mu.Unlock()
-			continue
-		}
-		defer q.mu.Unlock()
-		if q.closed {
-			return errors.New("closed")
-		}
-		f()
-		return nil
-	}
-}
-
-func (q *execQueue) run(f func()) {
-	f()
-
-	q.mu.Lock()
-	for len(q.queue) > 0 && !q.closed {
-		f := q.queue[0]
-		q.queue[0] = nil
-		q.queue = q.queue[1:]
-		q.mu.Unlock()
-		f()
-		q.mu.Lock()
-	}
-	q.inFlight = false
-	q.queue = nil
-	if q.doneWaiter != nil {
-		close(q.doneWaiter)
-		q.doneWaiter = nil
-	}
-	q.mu.Unlock()
-}
-
-func (q *execQueue) shutdown() {
-	q.mu.Lock()
-	defer q.mu.Unlock()
-	q.closed = true
-}
-
-// wait waits for the queue to be empty.
-func (q *execQueue) wait(ctx context.Context) error {
-	q.mu.Lock()
-	waitCh := q.doneWaiter
-	if q.inFlight && waitCh == nil {
-		waitCh = make(chan struct{})
-		q.doneWaiter = waitCh
-	}
-	q.mu.Unlock()
-
-	if waitCh == nil {
-		return nil
-	}
-
-	select {
-	case <-waitCh:
-		return nil
-	case <-ctx.Done():
-		return ctx.Err()
-	}
-}

ipn/ipnlocal/local.go

@@ -3460,8 +3460,7 @@ func (b *LocalBackend) reconfigAppConnectorLocked(nm *netmap.NetworkMap, prefs i
 	slices.SortFunc(routes, func(i, j netip.Prefix) int { return i.Addr().Compare(j.Addr()) })
 	domains = slices.Compact(domains)
 	routes = slices.Compact(routes)
-	b.appConnector.UpdateRoutes(routes)
-	b.appConnector.UpdateDomains(domains)
+	b.appConnector.UpdateDomainsAndRoutes(domains, routes)
 }
 
 // authReconfig pushes a new configuration into wgengine, if engine
@@ -5791,45 +5790,73 @@ var ErrDisallowedAutoRoute = errors.New("route is not allowed")
 // AdvertiseRoute implements the appc.RouteAdvertiser interface. It sets a new
 // route advertisement if one is not already present in the existing routes.
 // If the route is disallowed, ErrDisallowedAutoRoute is returned.
-func (b *LocalBackend) AdvertiseRoute(ipp netip.Prefix) error {
-	if !allowedAutoRoute(ipp) {
-		return ErrDisallowedAutoRoute
+func (b *LocalBackend) AdvertiseRoute(ipps ...netip.Prefix) error {
+	finalRoutes := b.Prefs().AdvertiseRoutes().AsSlice()
+	newRoutes := false
+
+	for _, ipp := range ipps {
+		if !allowedAutoRoute(ipp) {
+			continue
+		}
+		if slices.Contains(finalRoutes, ipp) {
+			continue
+		}
+
+		// If the new prefix is already contained by existing routes, skip it.
+		if coveredRouteRange(finalRoutes, ipp) {
+			continue
+		}
+
+		finalRoutes = append(finalRoutes, ipp)
+		newRoutes = true
 	}
-	currentRoutes := b.Prefs().AdvertiseRoutes()
-	if currentRoutes.ContainsFunc(func(r netip.Prefix) bool {
-		// TODO(raggi): add support for subset checks and avoid subset route creations.
-		return ipp.IsSingleIP() && r.Contains(ipp.Addr()) || r == ipp
-	}) {
+
+	if !newRoutes {
 		return nil
 	}
-	routes := append(currentRoutes.AsSlice(), ipp)
+
 	_, err := b.EditPrefs(&ipn.MaskedPrefs{
 		Prefs: ipn.Prefs{
-			AdvertiseRoutes: routes,
+			AdvertiseRoutes: finalRoutes,
 		},
 		AdvertiseRoutesSet: true,
 	})
 	return err
 }
 
+// coveredRouteRange checks if a route is already included in a slice of
+// prefixes.
+func coveredRouteRange(finalRoutes []netip.Prefix, ipp netip.Prefix) bool {
+	for _, r := range finalRoutes {
+		if ipp.IsSingleIP() {
+			if r.Contains(ipp.Addr()) {
+				return true
+			}
+		} else {
+			if r.Contains(ipp.Addr()) && r.Contains(netipx.PrefixLastIP(ipp)) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // UnadvertiseRoute implements the appc.RouteAdvertiser interface. It removes
 // a route advertisement if one is present in the existing routes.
-func (b *LocalBackend) UnadvertiseRoute(ipp netip.Prefix) error {
+func (b *LocalBackend) UnadvertiseRoute(toRemove ...netip.Prefix) error {
 	currentRoutes := b.Prefs().AdvertiseRoutes().AsSlice()
-	if !slices.Contains(currentRoutes, ipp) {
-		return nil
-	}
+	finalRoutes := currentRoutes[:0]
 
-	newRoutes := currentRoutes[:0]
-	for _, r := range currentRoutes {
-		if r != ipp {
-			newRoutes = append(newRoutes, r)
+	for _, ipp := range currentRoutes {
+		if slices.Contains(toRemove, ipp) {
+			continue
 		}
+		finalRoutes = append(finalRoutes, ipp)
 	}
 
 	_, err := b.EditPrefs(&ipn.MaskedPrefs{
 		Prefs: ipn.Prefs{
-			AdvertiseRoutes: newRoutes,
+			AdvertiseRoutes: finalRoutes,
 		},
 		AdvertiseRoutesSet: true,
 	})

ipn/ipnlocal/local_test.go

@@ -18,6 +18,7 @@ import (
 	"go4.org/netipx"
 	"golang.org/x/net/dns/dnsmessage"
 	"tailscale.com/appc"
+	"tailscale.com/appc/appctest"
 	"tailscale.com/control/controlclient"
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/store/mem"
@@ -1204,14 +1205,52 @@ func TestObserveDNSResponse(t *testing.T) {
 	// ensure no error when no app connector is configured
 	b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8"))
 
-	rc := &routeCollector{}
+	rc := &appctest.RouteCollector{}
 	b.appConnector = appc.NewAppConnector(t.Logf, rc)
 	b.appConnector.UpdateDomains([]string{"example.com"})
+	b.appConnector.Wait(context.Background())
 
 	b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8"))
+	b.appConnector.Wait(context.Background())
 	wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")}
-	if !slices.Equal(rc.routes, wantRoutes) {
-		t.Fatalf("got routes %v, want %v", rc.routes, wantRoutes)
+	if !slices.Equal(rc.Routes(), wantRoutes) {
+		t.Fatalf("got routes %v, want %v", rc.Routes(), wantRoutes)
+	}
+}
+
+func TestCoveredRouteRange(t *testing.T) {
+	tests := []struct {
+		existingRoute netip.Prefix
+		newRoute      netip.Prefix
+		want          bool
+	}{
+		{
+			existingRoute: netip.MustParsePrefix("192.0.0.1/32"),
+			newRoute:      netip.MustParsePrefix("192.0.0.1/32"),
+			want:          true,
+		},
+		{
+			existingRoute: netip.MustParsePrefix("192.0.0.1/32"),
+			newRoute:      netip.MustParsePrefix("192.0.0.2/32"),
+			want:          false,
+		},
+		{
+			existingRoute: netip.MustParsePrefix("192.0.0.0/24"),
+			newRoute:      netip.MustParsePrefix("192.0.0.1/32"),
+			want:          true,
+		},
+		{
+			existingRoute: netip.MustParsePrefix("192.0.0.0/16"),
+			newRoute:      netip.MustParsePrefix("192.0.0.0/24"),
+			want:          true,
+		},
+	}
+
+	for _, tt := range tests {
+		got := coveredRouteRange([]netip.Prefix{tt.existingRoute}, tt.newRoute)
+		if got != tt.want {
+			t.Errorf("coveredRouteRange(%v, %v) = %v, want %v", tt.existingRoute, tt.newRoute, got, tt.want)
+		}
 	}
 }
 
@@ -1250,6 +1289,7 @@ func TestReconfigureAppConnector(t *testing.T) {
 	}).View()
 
 	b.reconfigAppConnectorLocked(b.netMap, b.pm.prefs)
+	b.appConnector.Wait(context.Background())
 
 	want := []string{"example.com"}
 	if !slices.Equal(b.appConnector.Domains().AsSlice(), want) {
@@ -1349,27 +1389,6 @@ func dnsResponse(domain, address string) []byte {
 	return must.Get(b.Finish())
 }
 
-// routeCollector is a test helper that collects the list of routes advertised
-type routeCollector struct {
-	routes []netip.Prefix
-}
-
-func (rc *routeCollector) AdvertiseRoute(pfx netip.Prefix) error {
-	rc.routes = append(rc.routes, pfx)
-	return nil
-}
-
-func (rc *routeCollector) UnadvertiseRoute(pfx netip.Prefix) error {
-	routes := rc.routes
-	rc.routes = rc.routes[:0]
-	for _, r := range routes {
-		if r != pfx {
-			rc.routes = append(rc.routes, r)
-		}
-	}
-	return nil
-}
-
 type errorSyspolicyHandler struct {
 	t         *testing.T
 	err       error

ipn/ipnlocal/peerapi_test.go

@@ -23,6 +23,7 @@ import (
 	"go4.org/netipx"
 	"golang.org/x/net/dns/dnsmessage"
 	"tailscale.com/appc"
+	"tailscale.com/appc/appctest"
 	"tailscale.com/client/tailscale/apitype"
 	"tailscale.com/ipn"
 	"tailscale.com/ipn/store/mem"
@@ -685,10 +686,11 @@ func TestPeerAPIReplyToDNSQueries(t *testing.T) {
 }
 
 func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) {
+	ctx := context.Background()
 	var h peerAPIHandler
 	h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345")
 
-	rc := &routeCollector{}
+	rc := &appctest.RouteCollector{}
 	eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0)
 	pm := must.Get(newProfileManager(new(mem.Store), t.Logf))
 	h.ps = &peerAPIServer{
@@ -700,6 +702,7 @@ func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) {
 		},
 	}
 	h.ps.b.appConnector.UpdateDomains([]string{"example.com"})
+	h.ps.b.appConnector.Wait(ctx)
 
 	h.ps.resolver = &fakeResolver{}
 	f := filter.NewAllowAllForTest(logger.Discard)
@@ -717,10 +720,11 @@ func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) {
 	if w.Code != http.StatusOK {
 		t.Errorf("unexpected status code: %v", w.Code)
 	}
+	h.ps.b.appConnector.Wait(ctx)
 
 	wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")}
-	if !slices.Equal(rc.routes, wantRoutes) {
-		t.Errorf("got %v; want %v", rc.routes, wantRoutes)
+	if !slices.Equal(rc.Routes(), wantRoutes) {
+		t.Errorf("got %v; want %v", rc.Routes(), wantRoutes)
 	}
 }
 

util/execqueue/execqueue.go

@@ -0,0 +1,104 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+// Package execqueue implements an ordered asynchronous queue for executing functions.
+package execqueue
+
+import (
+	"context"
+	"errors"
+	"sync"
+)
+
+type ExecQueue struct {
+	mu         sync.Mutex
+	closed     bool
+	inFlight   bool          // whether a goroutine is running q.run
+	doneWaiter chan struct{} // non-nil if waiter is waiting, then closed
+	queue      []func()
+}
+
+func (q *ExecQueue) Add(f func()) {
+	q.mu.Lock()
+	defer q.mu.Unlock()
+	if q.closed {
+		return
+	}
+	if q.inFlight {
+		q.queue = append(q.queue, f)
+	} else {
+		q.inFlight = true
+		go q.run(f)
+	}
+}
+
+// RunSync waits for the queue to be drained and then synchronously runs f.
+// It returns an error if the queue is closed before f is run or ctx expires.
+func (q *ExecQueue) RunSync(ctx context.Context, f func()) error {
+	for {
+		if err := q.Wait(ctx); err != nil {
+			return err
+		}
+		q.mu.Lock()
+		if q.inFlight {
+			q.mu.Unlock()
+			continue
+		}
+		defer q.mu.Unlock()
+		if q.closed {
+			return errors.New("closed")
+		}
+		f()
+		return nil
+	}
+}
+
+func (q *ExecQueue) run(f func()) {
+	f()
+
+	q.mu.Lock()
+	for len(q.queue) > 0 && !q.closed {
+		f := q.queue[0]
+		q.queue[0] = nil
+		q.queue = q.queue[1:]
+		q.mu.Unlock()
+		f()
+		q.mu.Lock()
+	}
+	q.inFlight = false
+	q.queue = nil
+	if q.doneWaiter != nil {
+		close(q.doneWaiter)
+		q.doneWaiter = nil
+	}
+	q.mu.Unlock()
+}
+
+// Shutdown asynchronously signals the queue to stop.
+func (q *ExecQueue) Shutdown() {
+	q.mu.Lock()
+	defer q.mu.Unlock()
+	q.closed = true
+}
+
+// Wait waits for the queue to be empty.
+func (q *ExecQueue) Wait(ctx context.Context) error {
+	q.mu.Lock()
+	waitCh := q.doneWaiter
+	if q.inFlight && waitCh == nil {
+		waitCh = make(chan struct{})
+		q.doneWaiter = waitCh
+	}
+	q.mu.Unlock()
+
+	if waitCh == nil {
+		return nil
+	}
+
+	select {
+	case <-waitCh:
+		return nil
+	case <-ctx.Done():
+		return ctx.Err()
+	}
+}

util/execqueue/execqueue_test.go

@@ -0,0 +1,22 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+package execqueue
+
+import (
+	"context"
+	"sync/atomic"
+	"testing"
+)
+
+func TestExecQueue(t *testing.T) {
+	ctx := context.Background()
+	var n atomic.Int32
+	q := &ExecQueue{}
+	defer q.Shutdown()
+	q.Add(func() { n.Add(1) })
+	q.Wait(ctx)
+	if got := n.Load(); got != 1 {
+		t.Errorf("n=%d; want 1", got)
+	}
+}