policy/v2: merge rules sharing a source slice without rejoining it

The rules of one ACL now share one SrcIPs slice; key it once instead of
joining it again for every rule, globally and per node.

hscontrol/policy/v2/filter.go

@@ -617,7 +617,15 @@ func mergeFilterRules(rules []tailcfg.FilterRule) []tailcfg.FilterRule {
 		return rules
 	}
 
+	// sharedSrcIPs identifies a SrcIPs slice by its backing array.
+	type sharedSrcIPs struct {
+		first   *string
+		n       int
+		ipProto string
+	}
+
 	keyToIdx := make(map[string]int)
+	sharedToIdx := make(map[sharedSrcIPs]int)
 	result := make([]tailcfg.FilterRule, 0, len(rules))
 
 	for _, rule := range rules {
@@ -629,14 +637,35 @@ func mergeFilterRules(rules []tailcfg.FilterRule) []tailcfg.FilterRule {
 			continue
 		}
 
-		key := filterRuleKey(rule)
+		// An ACL compiles to one rule per destination, all with the same
+		// SrcIPs slice. Build the key of a slice once, or merging them is
+		// len(src) × len(dst) work.
+		shared := sharedSrcIPs{
+			n:       len(rule.SrcIPs),
+			ipProto: fmt.Sprint(rule.IPProto),
+		}
+		if len(rule.SrcIPs) > 0 {
+			shared.first = &rule.SrcIPs[0]
+		}
 
-		if idx, exists := keyToIdx[key]; exists {
+		idx, exists := sharedToIdx[shared]
+		if !exists {
+			key := filterRuleKey(rule)
+
+			idx, exists = keyToIdx[key]
+			if !exists {
+				idx = len(result)
+				keyToIdx[key] = idx
+			}
+
+			sharedToIdx[shared] = idx
+		}
+
+		if exists {
 			// Merge: append DstPorts to existing rule
 			result[idx].DstPorts = append(result[idx].DstPorts, rule.DstPorts...)
 		} else {
 			// New unique combination
-			keyToIdx[key] = len(result)
 			result = append(result, tailcfg.FilterRule{
 				SrcIPs:   rule.SrcIPs,
 				DstPorts: slices.Clone(rule.DstPorts),

hscontrol/policy/v2/filter_test.go

@@ -2,7 +2,9 @@ package v2
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/netip"
+	"runtime"
 	"slices"
 	"strings"
 	"testing"
@@ -4283,3 +4285,94 @@ func TestACLSourcesResolvedOnce(t *testing.T) {
 	require.Len(t, rules, 1)
 	require.Len(t, rules[0].DstPorts, 3)
 }
+
+// TestMergeFilterRulesSharedSrcIPs pins that recognising a shared SrcIPs
+// slice merges exactly the rules that comparing its content would.
+func TestMergeFilterRulesSharedSrcIPs(t *testing.T) {
+	t.Parallel()
+
+	shared := []string{"100.64.0.1", "100.64.0.2"}
+	rule := func(srcIPs []string, proto int, port uint16) tailcfg.FilterRule {
+		return tailcfg.FilterRule{
+			SrcIPs: srcIPs,
+			DstPorts: []tailcfg.NetPortRange{
+				{IP: "100.64.0.3", Ports: tailcfg.PortRange{First: port, Last: port}},
+			},
+			IPProto: []int{proto},
+		}
+	}
+
+	got := mergeFilterRules([]tailcfg.FilterRule{
+		rule(shared, ProtocolTCP, 1),
+		rule(shared, ProtocolUDP, 2),
+		rule(shared[:1], ProtocolTCP, 3),
+		rule(shared, ProtocolTCP, 4),
+		rule(slices.Clone(shared), ProtocolTCP, 5),
+		rule(shared, ProtocolUDP, 6),
+	})
+
+	want := []tailcfg.FilterRule{
+		rule(shared, ProtocolTCP, 1),
+		rule(shared, ProtocolUDP, 2),
+		rule(shared[:1], ProtocolTCP, 3),
+	}
+	want[0].DstPorts = append(
+		want[0].DstPorts, rule(nil, 0, 4).DstPorts[0], rule(nil, 0, 5).DstPorts[0],
+	)
+	want[1].DstPorts = append(want[1].DstPorts, rule(nil, 0, 6).DstPorts[0])
+
+	if diff := cmp.Diff(want, got); diff != "" {
+		t.Errorf("mergeFilterRules() mismatch (-want +got):\n%s", diff)
+	}
+}
+
+// TestMergeFilterRulesKeysSharedSrcIPsOnce bounds what merging the rules of
+// one wide ACL allocates: their shared SrcIPs are keyed once, not once per
+// rule. It reads the heap counters, so it does not run in parallel.
+func TestMergeFilterRulesKeysSharedSrcIPsOnce(t *testing.T) {
+	const numRules = 100
+
+	rulesSharing := func(numSrcs int) []tailcfg.FilterRule {
+		srcIPs := make([]string, numSrcs)
+		for i := range srcIPs {
+			srcIPs[i] = fmt.Sprintf("100.64.%d.%d", i/256, i%256)
+		}
+
+		rules := make([]tailcfg.FilterRule, numRules)
+		for i := range rules {
+			rules[i] = tailcfg.FilterRule{
+				SrcIPs: srcIPs,
+				DstPorts: []tailcfg.NetPortRange{
+					{IP: "100.100.0.1", Ports: tailcfg.PortRangeAny},
+				},
+				IPProto: []int{ProtocolTCP},
+			}
+		}
+
+		return rules
+	}
+
+	bytesPerMerge := func(rules []tailcfg.FilterRule) int64 {
+		const runs = 10
+
+		var before, after runtime.MemStats
+
+		runtime.ReadMemStats(&before)
+
+		for range runs {
+			mergeFilterRules(rules)
+		}
+
+		runtime.ReadMemStats(&after)
+
+		return int64(after.TotalAlloc-before.TotalAlloc) / runs //nolint:gosec
+	}
+
+	wide := rulesSharing(1024)
+	keyBytes := int64(len(filterRuleKey(wide[0])))
+
+	// Only the key, built once, may grow with the sources.
+	extra := bytesPerMerge(wide) - bytesPerMerge(rulesSharing(1))
+	assert.Less(t, extra, 8*keyBytes,
+		"merging %d rules that share %d sources", numRules, len(wide[0].SrcIPs))
+}