Skip to content

policy: Replace versioned with part.Map#42992

Merged
jrajahalme merged 4 commits intocilium:mainfrom
jrajahalme:policy-selectorcache-statedb
Dec 9, 2025
Merged

policy: Replace versioned with part.Map#42992
jrajahalme merged 4 commits intocilium:mainfrom
jrajahalme:policy-selectorcache-statedb

Conversation

@jrajahalme
Copy link
Copy Markdown
Member

@jrajahalme jrajahalme commented Nov 26, 2025

Replace versioned.Value with a part.Map as the solution for providing a transactional view on the selector cache for any given SelectorPolicy. Avoid most of the part.Map index update overhead by reusing the same transaction for the policy update. Addition of new selectors to the selector cache are pooled into a single write transaction. Whenever the changes need to be observable via CachedSelector.GetSelections[At](), the updater must call the new SelectorCache.Commit() function.

Some interfaces that called for a version handle were always used with the special latest version handle. StateDB does not have a similar concept, so we have removed the version handle from the following interface functions:

  • CachedSelector.GetSelections()
    • a new GetSelectionsAt() is added for explicit version access
  • CachedSelector.Selects()
  • DNSProxier.GetRules()

CachedSelector.Selects() and DNSProxier.GetRules() were always called with the static "latest version" handle, and most CachedSelector.GetSelections() calls in test code did the same. Implementations of these now internally get a read transaction on the latest version and access the selectors with that.

The main benefit of this change is the removal of pkg/container/versioned, that had an arcane API. This commit is the minimal change to achieve removal of pkg/container/versioned, with room for improvements like:

  • Consider using statedb, using statedb indexes for namespace indexing
  • Add CIDR indexing for (non-namespaced) CIDR selectors
  • Explore using part.Set for storing the selections instead of a read-only slice. This would likely save some on updates, but would cost more on iteration.

@jrajahalme jrajahalme added sig/policy Impacts whether traffic is allowed or denied based on user-defined policies. dont-merge/blocked Another PR must be merged before this one. labels Nov 26, 2025
@jrajahalme jrajahalme requested review from a team as code owners November 26, 2025 15:03
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 26, 2025
@jrajahalme jrajahalme requested review from a team as code owners November 26, 2025 15:03
@jrajahalme jrajahalme marked this pull request as draft November 26, 2025 15:03
@jrajahalme jrajahalme requested review from aanm, dylandreimerink and rgo3 and removed request for Artyop, aanm, bimmlerd, dylandreimerink, rgo3 and sayboras November 26, 2025 15:03
@jrajahalme
Copy link
Copy Markdown
Member Author

/test

@jrajahalme jrajahalme force-pushed the policy-selectorcache-statedb branch from b9caba5 to f4e4bc3 Compare December 3, 2025 12:03
Bump the number of rules in TestRegenerateCIDRDenyPolicyRules to be the
same as in BenchmarkRegenerateCIDRDenyPolicyRules and assert the number
of mapstate entries is 117515 in both instead of just dumping the number
on stdout.

The specific number works as the rule generation used is deterministic.

This helps catch regressions that may otherwise be left unnoticed.

Signed-off-by: Jarno Rajahalme <[email protected]>
Some CachedSelector intereface functions that called for a version handle
were always used with the special latest version handle. Simplify by
removing the version handle from the following interface functions:

- CachedSelector.GetSelections()
  - a new GetSelectionsAt() is added for explicit version access
- CachedSelector.Selects()
- DNSProxier.GetRules()

CachedSelector.Selects() and DNSProxier.GetRules() were always called
with the static "latest version" handle, and most
CachedSelector.GetSelections() calls in test code did the
same. Implementations of these now internally get a version handle on
the latest version and access the selectors with that.

Rename local variable 'idSel' as 'sel' for readability.

Signed-off-by: Jarno Rajahalme <[email protected]>
Move test package from pkg/proxy to pkg/envoy as that is the only place
it is used from.

Signed-off-by: Jarno Rajahalme <[email protected]>
@jrajahalme jrajahalme force-pushed the policy-selectorcache-statedb branch from 600eb65 to 6d5f84a Compare December 8, 2025 11:00
@jrajahalme jrajahalme requested review from joamaki and squeed December 8, 2025 11:03
@jrajahalme jrajahalme force-pushed the policy-selectorcache-statedb branch from 6d5f84a to a17b033 Compare December 8, 2025 11:38
@jrajahalme
Copy link
Copy Markdown
Member Author

Fixed lint issues

@jrajahalme
Copy link
Copy Markdown
Member Author

/test

Replace versioned.Value with a part.Map as the solution for providing a
transactional view on the selector cache for any given
SelectorPolicy. Avoid most of the part.Map index overhead by reusing the
same transaction for the policy update. Addition of new selectors to the
selector cache are pooled into a single write transaction. Whenever the
changes need to be observable via CachedSelector.GetSelections[At](), the
updater must call the new SelectorCache.Commit() function.

Pooling of changes into a single transaction makes this generally a bit
faster (~14%) even though part.Map uses more memory (+40%) for updates.

The main benefit of this change is the removal of
pkg/container/versioned, that had an arcane API. This commit is the
minimal change to achieve removal of pkg/container/versioned, with room
for improvements like:

- Consider using statedb, using statedb indexes for namespace indexing
- Add CIDR indexing for (non-namespaced) CIDR selectors
- Explore using part.Set for storing the selections instead of a
  read-only slice. This would likely save some on updates, but would cost
  more on iteration.

Signed-off-by: Jarno Rajahalme <[email protected]>
@jrajahalme jrajahalme force-pushed the policy-selectorcache-statedb branch from a17b033 to ace908f Compare December 8, 2025 14:46
@jrajahalme
Copy link
Copy Markdown
Member Author

/test

@jrajahalme jrajahalme requested a review from squeed December 8, 2025 14:51
@joamaki
Copy link
Copy Markdown
Contributor

joamaki commented Dec 9, 2025

Fuzzers broken?

+ /src/cilium/test/fuzzing/oss-fuzz-build.sh
Could not find the function: func FuzzMatchpatternValidate(f *testing.F)
# github.com/cilium/cilium/pkg/policy
pkg/policy/l4_filter_test_fuzz.go:508:15: undefined: OriginForTest

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Dec 9, 2025
@jrajahalme jrajahalme added this pull request to the merge queue Dec 9, 2025
Merged via the queue into cilium:main with commit 4c4c7c9 Dec 9, 2025
77 of 78 checks passed
@jrajahalme jrajahalme deleted the policy-selectorcache-statedb branch December 9, 2025 15:29
jrajahalme added a commit to jrajahalme/cilium that referenced this pull request Dec 10, 2025
Perform sanity checks on the cached selector selections only after the
corresponding snapshot is delivered via commit.

Fixes: cilium#42992
Signed-off-by: Jarno Rajahalme <[email protected]>
jrajahalme added a commit to jrajahalme/cilium that referenced this pull request Dec 11, 2025
Perform sanity checks on the cached selector selections only after the
corresponding snapshot is delivered via commit.

Fixes: cilium#42992
Signed-off-by: Jarno Rajahalme <[email protected]>
jrajahalme added a commit to jrajahalme/cilium that referenced this pull request Dec 11, 2025
Perform sanity checks on the cached selector selections only after the
corresponding snapshot is delivered via commit.

Fixes: cilium#42992
Signed-off-by: Jarno Rajahalme <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact. sig/policy Impacts whether traffic is allowed or denied based on user-defined policies.

Projects

No open projects
Status: Released

Development

Successfully merging this pull request may close these issues.

7 participants