From 526daf0841fead18e1a9d04a6eb2b4334abcd151 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 23 Oct 2026 01:49:06 +0000 Subject: [PATCH] policy/v2: resolve an ACL's sources once, not once per destination An ACL compiles to one grant per destination, and each resolved and merged the same sources again. A wide CI-style ACL took 3s and 674MB per compile; 88ms and 23MB now. --- hscontrol/policy/v2/compiled.go | 81 ++++++++++++++++++++++++--- hscontrol/policy/v2/filter_test.go | 4 +- hscontrol/policy/v2/policy_test.go | 89 ++++++++++++++++++++++++++++++ 3 files changed, 164 insertions(+), 10 deletions(-) diff --git a/hscontrol/policy/v2/compiled.go b/hscontrol/policy/v2/compiled.go index 32623c30e..514edd1b0 100644 --- a/hscontrol/policy/v2/compiled.go +++ b/hscontrol/policy/v2/compiled.go @@ -244,9 +244,10 @@ func (pol *Policy) compileGrants( } compiled := make([]compiledGrant, 0, len(grants)) + srcs := sourceCache{} for _, grant := range grants { - cg, err := pol.compileOneGrant(grant, users, nodes) + cg, err := pol.compileOneGrant(grant, users, nodes, srcs) if err != nil { log.Trace().Err(err).Msg("compiling grant") @@ -271,6 +272,7 @@ func (pol *Policy) compileOneGrant( grant Grant, users types.Users, nodes views.Slice[types.NodeView], + srcs sourceCache, ) (*compiledGrant, error) { // Via grants: resolve sources, store deferred data. if len(grant.Via) > 0 { @@ -291,9 +293,9 @@ func (pol *Policy) compileOneGrant( // Resolve sources per-alias, tracking non-wildcard sources // separately so we can preserve their IPs alongside the // wildcard CGNAT ranges (matching Tailscale SaaS behavior). - resolvedSrcs, nonWildcardSrcs, err := resolveSources( - pol, grant.Sources, users, nodes, - ) + sources := srcs.get(pol, grant.Sources, users, nodes) + + resolvedSrcs, err := sources.all, sources.err if err != nil { return nil, err } @@ -309,10 +311,7 @@ func (pol *Policy) compileOneGrant( hasWildcard := sourcesHaveWildcard(grant.Sources) hasDangerAll := sourcesHaveDangerAll(grant.Sources) - srcIPStrings := buildSrcIPStrings( - resolvedSrcs, nonWildcardSrcs, - hasWildcard, hasDangerAll, nodes, - ) + srcIPStrings := sources.strings(hasWildcard, hasDangerAll, nodes) cg := &compiledGrant{ srcIPStrings: srcIPStrings, @@ -348,6 +347,12 @@ func mergeResolvedSrcs(resolvedSrcs []ResolvedAddresses) (resolved, error) { var b netipx.IPSetBuilder for _, ips := range resolvedSrcs { + if r, ok := ips.(resolved); ok { + b.AddSet(&r.ips) + + continue + } + for _, pref := range ips.Prefixes() { b.AddPrefix(pref) } @@ -356,6 +361,66 @@ func mergeResolvedSrcs(resolvedSrcs []ResolvedAddresses) (resolved, error) { return newResolved(&b) } +// sourceCache holds each source list's resolution for one compile: an ACL +// compiles to one grant per destination, all sharing its sources, and +// resolving and merging them is most of a wide ACL's compile. +type sourceCache map[sourcesKey]*grantSources + +// sourcesKey names a source list by its backing array, which the grants +// of one ACL share. +type sourcesKey struct { + first *Alias + n int +} + +// grantSources is a source list resolved per alias, all and without +// wildcards, and its SrcIPs once built. +type grantSources struct { + all, nonWild []ResolvedAddresses + err error + + built bool + strs []string +} + +// get resolves sources, once per source list; c may be nil. +func (c sourceCache) get( + pol *Policy, + sources Aliases, + users types.Users, + nodes views.Slice[types.NodeView], +) *grantSources { + key := sourcesKey{n: len(sources)} + if len(sources) > 0 { + key.first = &sources[0] + } + + if s, ok := c[key]; ok { + return s + } + + s := &grantSources{} + s.all, s.nonWild, s.err = resolveSources(pol, sources, users, nodes) + + if c != nil { + c[key] = s + } + + return s +} + +// strings returns the SrcIPs of the sources. Both flags follow from the +// sources, so every grant sharing them asks the same. The slice is shared +// and clipped, so a caller appending to it copies. +func (s *grantSources) strings(hasWildcard, hasDangerAll bool, nodes views.Slice[types.NodeView]) []string { + if !s.built { + s.strs = slices.Clip(buildSrcIPStrings(s.all, s.nonWild, hasWildcard, hasDangerAll, nodes)) + s.built = true + } + + return s.strs +} + // compileOneViaGrant resolves sources for a via grant and stores the // deferred per-node data. The actual via-node matching and route // intersection happens in [compileViaForNode]. diff --git a/hscontrol/policy/v2/filter_test.go b/hscontrol/policy/v2/filter_test.go index fcdb7e66e..37441c22b 100644 --- a/hscontrol/policy/v2/filter_test.go +++ b/hscontrol/policy/v2/filter_test.go @@ -3956,7 +3956,7 @@ func TestCompileViaGrant(t *testing.T) { nodeView := tt.node.View() nodesSlice := tt.nodes.ViewSlice() - cg, err := tt.pol.compileOneGrant(tt.grant, users, nodesSlice) + cg, err := tt.pol.compileOneGrant(tt.grant, users, nodesSlice, nil) if tt.wantErr != nil { require.ErrorIs(t, err, tt.wantErr) @@ -4129,7 +4129,7 @@ func TestCompileGrantWithAutogroupSelf_GrantPaths(t *testing.T) { userIdx := buildUserNodeIndex(nodesSlice) cg, err := tt.pol.compileOneGrant( - tt.grant, users, nodesSlice, + tt.grant, users, nodesSlice, nil, ) if tt.wantErr != nil { diff --git a/hscontrol/policy/v2/policy_test.go b/hscontrol/policy/v2/policy_test.go index 65692d37b..0f01bb16e 100644 --- a/hscontrol/policy/v2/policy_test.go +++ b/hscontrol/policy/v2/policy_test.go @@ -1,8 +1,10 @@ package v2 import ( + "encoding/json" "fmt" "maps" + "math/rand/v2" "net/netip" "slices" "testing" @@ -4202,3 +4204,90 @@ func BenchmarkNodeCapMapsConstruction(b *testing.B) { } } } + +// BenchmarkCompileWideACL compiles a CI-shaped policy: one rule per CI tag +// to its own service, and one wide rule from every CI tag to the shared +// services, which compiles to one grant per destination. new compiles from +// scratch; set-nodes recompiles after a batch of tagged joins. +func BenchmarkCompileWideACL(b *testing.B) { + const ( + ciTags = 300 + common = 100 + ci = 5000 + joins = 50 + ) + + r := rand.New(rand.NewPCG(1, 2)) //nolint:gosec // reproducible benchmark input + + owners := map[string][]string{} + tag := func(kind string, i int) string { + t := fmt.Sprintf("tag:%s%d", kind, i) + owners[t] = []string{} + + return t + } + + acls := make([]map[string]any, 0, ciTags+1) + wide := make([]string, 0, ciTags) + dsts := make([]string, 0, common) + + for i := range ciTags { + acls = append(acls, map[string]any{"action": "accept", "src": []string{tag("ci-app", i)}, "dst": []string{tag("svc-app", i) + ":80"}}) + wide = append(wide, tag("ci-app", i)) + } + + for i := range common { + dsts = append(dsts, tag("svc-common", i)+":80") + } + + acls = append(acls, map[string]any{"action": "accept", "src": wide, "dst": dsts}) + pol := must(json.Marshal(map[string]any{"tagOwners": owners, "acls": acls})) + + node := func(i int, t string) *types.Node { + v4 := netip.AddrFrom4([4]byte{100, 64 + byte(i>>16), byte(i >> 8), byte(i)}) //nolint:gosec + v6 := netip.AddrFrom16([16]byte{0xfd, 0x7a, 0x11, 0x5c, 0xa1, 0xe0, 13: byte(i >> 16), 14: byte(i >> 8), 15: byte(i)}) //nolint:gosec + + return &types.Node{ID: types.NodeID(i + 1), Hostname: fmt.Sprintf("n%d", i), IPv4: &v4, IPv6: &v6, Tags: []string{t}} //nolint:gosec + } + + before := make(types.Nodes, 0, ciTags+common+ci) + for i := range ciTags { + before = append(before, node(len(before), fmt.Sprintf("tag:svc-app%d", i))) + } + + for i := range common { + before = append(before, node(len(before), fmt.Sprintf("tag:svc-common%d", i))) + } + + for range ci { + before = append(before, node(len(before), fmt.Sprintf("tag:ci-app%d", r.IntN(ciTags)))) + } + + after := slices.Clone(before) + for range joins { + after = append(after, node(len(after), fmt.Sprintf("tag:ci-app%d", r.IntN(ciTags)))) + } + + b.Run("new", func(b *testing.B) { + for b.Loop() { + _, err := NewPolicyManager(pol, nil, after.ViewSlice()) + require.NoError(b, err) + } + }) + + b.Run("set-nodes", func(b *testing.B) { + pm, err := NewPolicyManager(pol, nil, before.ViewSlice()) + require.NoError(b, err) + + sets := []types.Nodes{after, before} + + i := 0 + for b.Loop() { + changed, err := pm.SetNodes(sets[i%2].ViewSlice()) + require.NoError(b, err) + require.True(b, changed) + + i++ + } + }) +}