policyutil: match DstPorts as ranges when reducing filters

Every node re-parsed every entry into an IPSet on every policy change;
under churn that was most of the batcher's CPU.

hscontrol/policy/policyutil/reduce.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/juanfont/headscale/hscontrol/types"
 	"github.com/juanfont/headscale/hscontrol/util"
+	"go4.org/netipx"
 	"tailscale.com/tailcfg"
 )
 
@@ -18,6 +19,7 @@ import (
 // both cases.
 func ReduceFilterRules(node types.NodeView, rules []tailcfg.FilterRule) []tailcfg.FilterRule {
 	ret := []tailcfg.FilterRule{}
+	nodeIPs := node.IPs()
 	subnetRoutes := node.SubnetRoutes()
 	exitRoutes := node.ExitRoutes()
 
@@ -37,32 +39,7 @@ func ReduceFilterRules(node types.NodeView, rules []tailcfg.FilterRule) []tailcf
 		var dests []tailcfg.NetPortRange
 
 		for _, dest := range rule.DstPorts {
-			expanded, err := util.ParseIPSet(dest.IP, nil)
-			// Fail closed: unparseable dests are dropped.
-			if err != nil {
-				continue
-			}
-
-			if node.InIPSet(expanded) {
-				dests = append(dests, dest)
-				continue
-			}
-
-			// If the node has approved subnet routes, preserve
-			// filter rules targeting those routes.
-			// [types.NodeView.SubnetRoutes] returns only approved,
-			// non-exit routes — matching Tailscale SaaS behavior,
-			// which does not generate filter rules for
-			// advertised-but-unapproved routes.
-			if slices.ContainsFunc(subnetRoutes, expanded.OverlapsPrefix) {
-				dests = append(dests, dest)
-				continue
-			}
-
-			// Approved exit routes count like subnet routes. They
-			// contain every destination, so Tailscale SaaS sends an
-			// exit node every rule, not only internet ones.
-			if slices.ContainsFunc(exitRoutes, expanded.OverlapsPrefix) {
+			if destReachesNode(dest.IP, nodeIPs, subnetRoutes, exitRoutes) {
 				dests = append(dests, dest)
 			}
 		}
@@ -79,6 +56,33 @@ func ReduceFilterRules(node types.NodeView, rules []tailcfg.FilterRule) []tailcf
 	return ret
 }
 
+// destReachesNode reports whether a DstPorts entry targets the node: one of
+// its addresses, or a route it serves. Every node reduces every entry on
+// every policy change, so the entry is parsed as one range rather than an
+// IPSet. Unparseable entries fail closed.
+func destReachesNode(dst string, nodeIPs []netip.Addr, subnetRoutes, exitRoutes []netip.Prefix) bool {
+	// Spans both families, so it is the one form that is not a range.
+	if dst == "*" {
+		return len(nodeIPs) > 0 || len(subnetRoutes) > 0 || len(exitRoutes) > 0
+	}
+
+	r, err := util.ParseIPRange(dst)
+	if err != nil {
+		return false
+	}
+
+	overlaps := func(p netip.Prefix) bool { return r.Overlaps(netipx.RangeOfPrefix(p)) }
+
+	// [types.NodeView.SubnetRoutes] holds only approved, non-exit routes:
+	// Tailscale SaaS generates no rules for advertised-but-unapproved
+	// routes. Approved exit routes count like subnet routes. They contain
+	// every destination, so SaaS sends an exit node every rule, not only
+	// internet ones.
+	return slices.ContainsFunc(nodeIPs, r.Contains) ||
+		slices.ContainsFunc(subnetRoutes, overlaps) ||
+		slices.ContainsFunc(exitRoutes, overlaps)
+}
+
 // reduceCapGrantRule filters a [tailcfg.CapGrant] rule to only include
 // [tailcfg.CapGrant] entries whose Dsts match the given node's IPs. When a
 // broad prefix (e.g. 100.64.0.0/10 from dst:*) contains a node's IP, it is

hscontrol/policy/policyutil/reduce_test.go

@@ -1339,3 +1339,78 @@ func TestReduceFilterRulesCapGrant(t *testing.T) {
 		})
 	}
 }
+
+// TestReduceFilterRulesDestForms reduces one rule holding every DstPorts form
+// for a node at 100.64.0.1 routing 10.0.0.0/24, and pins which entries reach
+// it. Malformed entries fail closed.
+func TestReduceFilterRulesDestForms(t *testing.T) {
+	node := &types.Node{
+		IPv4: ap("100.64.0.1"),
+		IPv6: ap("fd7a:115c:a1e0::1"),
+		Hostinfo: &tailcfg.Hostinfo{
+			RoutableIPs: []netip.Prefix{p("10.0.0.0/24")},
+		},
+		ApprovedRoutes: []netip.Prefix{p("10.0.0.0/24")},
+	}
+
+	dsts := map[string]bool{
+		"*":                         true,
+		"100.64.0.1":                true,
+		"fd7a:115c:a1e0::1":         true,
+		"100.64.0.2":                false,
+		"100.64.0.0/10":             true,
+		"100.128.0.0/10":            false,
+		"10.0.0.128/25":             true,
+		"10.0.1.0/24":               false,
+		"100.64.0.0-100.64.0.9":     true,
+		"100.64.0.2-100.64.0.9":     false,
+		"10.0.0.200-10.0.1.5":       true,
+		"100.64.0.1/10":             false, // host bits set
+		"100.64.0.9-100.64.0.1":     false, // reversed range
+		"100.64.0.1-fd7a:115c::1":   false, // mixed families
+		"not-an-ip":                 false,
+		"100.64.0.1-100.64.0.2-100": false,
+	}
+
+	rule := tailcfg.FilterRule{SrcIPs: []string{"*"}}
+	for dst := range dsts {
+		rule.DstPorts = append(rule.DstPorts, tailcfg.NetPortRange{IP: dst, Ports: tailcfg.PortRangeAny})
+	}
+
+	got := policyutil.ReduceFilterRules(node.View(), []tailcfg.FilterRule{rule})
+	require.Len(t, got, 1)
+
+	kept := make(map[string]bool)
+	for _, dp := range got[0].DstPorts {
+		kept[dp.IP] = true
+	}
+
+	for dst, want := range dsts {
+		require.Equal(t, want, kept[dst], "DstPorts %q", dst)
+	}
+}
+
+// TestReduceFilterRulesAllocsIndependentOfDests reduces the global filter as
+// every node does on every policy change. Its DstPorts are plain strings, so
+// anything allocated per entry, such as building an IPSet to test one
+// address, is paid nodes × entries times per change.
+func TestReduceFilterRulesAllocsIndependentOfDests(t *testing.T) {
+	node := (&types.Node{IPv4: ap("100.64.0.1"), IPv6: ap("fd7a:115c:a1e0::1")}).View()
+
+	allocs := func(n int) int {
+		rule := tailcfg.FilterRule{SrcIPs: []string{"*"}}
+		for i := range n {
+			rule.DstPorts = append(rule.DstPorts, tailcfg.NetPortRange{
+				IP:    fmt.Sprintf("100.64.%d.%d", i/250, i%250+2),
+				Ports: tailcfg.PortRangeAny,
+			})
+		}
+
+		rules := []tailcfg.FilterRule{rule}
+
+		return int(testing.AllocsPerRun(10, func() { policyutil.ReduceFilterRules(node, rules) }))
+	}
+
+	require.Equal(t, allocs(10), allocs(1000),
+		"reducing a filter must not allocate per DstPorts entry")
+}

hscontrol/util/addr.go

@@ -1,6 +1,7 @@
 package util
 
 import (
+	"errors"
 	"fmt"
 	"iter"
 	"net/netip"
@@ -85,6 +86,45 @@ func ParseIPSet(arg string, bits *int) (*netipx.IPSet, error) {
 	return ipSet.IPSet()
 }
 
+// ParseIPRange errors.
+var (
+	ErrPrefixHostBits   = errors.New("prefix contains non-network bits set")
+	ErrInvalidIPAddress = errors.New("invalid IP address")
+)
+
+// ParseIPRange parses what [ParseIPSet] accepts, apart from "*", as the single
+// range it denotes, without building a set.
+func ParseIPRange(arg string) (netipx.IPRange, error) {
+	if strings.Contains(arg, "/") {
+		pfx, err := netip.ParsePrefix(arg)
+		if err != nil {
+			return netipx.IPRange{}, err
+		}
+
+		if pfx != pfx.Masked() {
+			return netipx.IPRange{}, fmt.Errorf("%w: %v", ErrPrefixHostBits, pfx)
+		}
+
+		return netipx.RangeOfPrefix(pfx), nil
+	}
+
+	if strings.Count(arg, "-") == 1 {
+		return netipx.ParseIPRange(arg)
+	}
+
+	ip, err := netip.ParseAddr(arg)
+	if err != nil {
+		return netipx.IPRange{}, fmt.Errorf("%w: %q", ErrInvalidIPAddress, arg)
+	}
+
+	r := netipx.IPRangeFrom(ip, ip)
+	if !r.IsValid() {
+		return netipx.IPRange{}, fmt.Errorf("%w: %q", ErrInvalidIPAddress, arg)
+	}
+
+	return r, nil
+}
+
 func GetIPPrefixEndpoints(na netip.Prefix) (netip.Addr, netip.Addr) {
 	ipRange := netipx.RangeOfPrefix(na)
 

hscontrol/util/addr_test.go

@@ -119,3 +119,42 @@ func Test_parseIPSet(t *testing.T) {
 		})
 	}
 }
+
+// TestParseIPRangeMatchesParseIPSet checks ParseIPRange accepts exactly what
+// ParseIPSet does, "*" aside, and denotes the same addresses.
+func TestParseIPRangeMatchesParseIPSet(t *testing.T) {
+	for _, arg := range []string{
+		"10.0.0.1",
+		"2001:db8::2",
+		"192.168.0.0/16",
+		"2001:db8:abcd:1234::/64",
+		"192.168.0.0-192.168.255.255",
+		"10.0.0.5-10.0.0.9",
+		"fd7a::1-fd7a::ff",
+		"10.0.0.1/8",
+		"10.0.0.9-10.0.0.1",
+		"10.0.0.1-fd7a::1",
+		"10.0.0.1-10.0.0.2-10.0.0.3",
+		"fe80::1%eth0",
+		"10.0.0.0/33",
+		"not-an-ip",
+		"",
+	} {
+		set, setErr := ParseIPSet(arg, nil)
+		r, rangeErr := ParseIPRange(arg)
+
+		if (setErr != nil) != (rangeErr != nil) {
+			t.Errorf("%q: ParseIPSet error %v, ParseIPRange error %v", arg, setErr, rangeErr)
+
+			continue
+		}
+
+		if setErr != nil {
+			continue
+		}
+
+		if diff := cmp.Diff(set.Ranges(), []netipx.IPRange{r}, cmp.Comparer(func(a, b netipx.IPRange) bool { return a == b })); diff != "" {
+			t.Errorf("%q: (-ParseIPSet +ParseIPRange):\n%s", arg, diff)
+		}
+	}
+}