felix/collector: evaluate pending policy off the main loop on a bounded worker pool - #13903
Draft
dimitri-nicolo wants to merge 7 commits into
Draft
dimitri-nicolo wants to merge 7 commits into
dimitri-nicolo wants to merge 7 commits into
Conversation
…benchmarks Lift the two synthetic policy sets out of the checker benchmarks into a package that renders the same set three ways: a policystore.PolicyStore and endpoint for engine and collector benchmarks, the ToDataplane updates that load it through ProcessUpdate, and Calico resources (Tier, GlobalNetworkPolicy, GlobalNetworkSet) for applying it to a cluster. A number measured at the engine, in the collector and on a node then describes the same policy set. The fixture also carries an oracle: Fixture.Expect computes the trace the engine must report for a flow from the generator's own model of each rule, independently of the matching code, and MatchingFlow / DeniedFlow / Sampler produce flows aimed at chosen depths of the walk or following a flow model (miss fraction, repeat fraction). Later changes to the engine (compiled policies, a verdict cache, evaluation off the collector's main loop) are gated on differential tests built on these. Composite applies both measured shapes to one endpoint: 19,992 ingress rules, 18,662 egress rules, 7,571 IP sets. BenchmarkEvaluateComposite measures it in the three cases that matter for the collector; the two existing benchmarks keep their cases and their numbers. hack/cmd/policyscale renders a preset as YAML, or prints sampled flows with their expected verdicts, for node-level runs. Tracking: CORE-13316.
…yscale oracle Run Evaluate over a corpus drawn from each policyscale preset (the denied flow, flows aimed at every 257th rule, sampled flows following a flow model, and replays as UDP, with an invalid protocol and with nil addresses) and require the trace to equal the one the fixture's oracle computes from its own model of the rules, in both policy scopes. The harness takes two evaluators, so a second implementation of the walk (compiled policies, a verdict cache, evaluation on another goroutine) is checked against Evaluate the same way before it is switched on. The named cases cover what a generated corpus cannot reach: nil addresses against positive and negated references, named and negated named ports, IP+port sets, negated and missing sets, protocols out of range, by name, by number and negated, staged policies in both scopes, HTTP criteria against an L4 flow, Log rules, Pass into the next tier and into profiles, and a policy missing from the store. runNamedCases takes an evaluator for the same reason. Tracking: CORE-13316.
…neration
A verdict is a function of the endpoint's applicable rules, the store's
IP sets and the flow. For a flow with no L7 attributes (every flow the
collector evaluates) the criteria in match() reduce to protocol, source
and destination address, destination port and, only where a rule looks
at it, source port; identity and HTTP criteria match such a flow
whatever the rule says. So a store can remember the verdict for that
key and answer a flow that repeats an earlier flow's endpoints on a new
source port without walking the policy set, which on a large policy
set is the difference between a millisecond and a map lookup.
PolicyStore gains a Generation that ProcessUpdate moves on every update
but InSync, and an optional VerdictCache bound to it: entries are valid
for one generation and the cache starts over when the store moves on,
so a cached verdict never outlives the policies, IP sets or endpoints it
was computed from. Whether the source port is part of the key is decided
once per endpoint, scope, direction and generation by walking the
endpoint's applicable rules. The cache is bounded and starts over when
full. Evaluate consults it when the store has one; failed evaluations
are not cached; flows carrying identity or HTTP attributes (Dikastes
requests) and flows with a nil address bypass it.
The collector gets the cache through a PolicyStoreManager option so that
every store the manager creates, including on reconnect, carries one,
and reports it as felix_collector_policy_eval_cache_{hits,misses,resets,
evictions}_total. FlowLogsPolicyEvaluationCacheSize sizes it (default
65536; 0 disables). It is a config-file / environment-variable parameter
with no FelixConfiguration field yet, so that it picks to release
branches without an API change; the field can follow on master.
On the composite reference set (18,662 egress rules) with the sampler's
flow model, per evaluation: 1.00 ms uncached; 0.72 ms cached with no
deliberate repeats (11% hit ratio from coincidental ones), 0.31 ms at a
50% repeat rate (62% hits), 0.08 ms at 90% (92% hits).
Tests: the differential corpus and the named cases through a cached
store, twice, against an uncached one; invalidation on IP set delta,
policy update and policy removal through ProcessUpdate; source-port
keying; L7 and nil-address bypass; generation, capacity and option
behaviour in policystore; collector wiring.
Tracking: CORE-13316.
…uation
BenchmarkCollectorPolicyEval drives a real collector, wired for
evaluation only (a lookups cache that knows one local workload, a policy
store loaded with the policyscale composite set and that workload's
endpoint, no readers, no reporters, main loop not running), with
conntrack updates for flows drawn from the sampler's model, and
measures the two paths on the collector's main goroutine: handleCtInfo
for a new tuple, which creates the Data and evaluates it inline, and one
full re-evaluation sweep over a fixed live population. Each reports
ns/flow, the flow rate one goroutine could sustain at that cost, and the
verdict cache's hit ratio when caching.
This is the K2 level of the benchmark definition in
felix/design/flow-logs-policy-evaluation.md: the number that says
whether the collector, not just the engine, meets the per-node target.
On one laptop core, 200 new flows per case:
NewFlow/Egress/Uncached 1.24 ms per flow ~800 flows/s
NewFlow/Ingress/Uncached 0.82 ms per flow ~1,200 flows/s
NewFlow/Egress/Cached 0.38 ms per flow ~2,600 flows/s (59% hits)
NewFlow/Ingress/Cached 0.09 ms per flow ~10,700 flows/s (93% hits)
Sweep/Uncached 1.04 ms per live flow
Sweep/Cached 0.48 us per live flow (a sweep over an
unchanged store is all hits)
Tracking: CORE-13316.
The design doc for the user-mode policy evaluation the flow-log collector runs for pending (staged) policy verdicts: the two evaluation paths and the invariants they keep, the performance target and the three-level benchmark (engine, collector, node) that defines it, the correctness gates every new evaluator runs through, the verdict cache, the planned off-main-loop evaluation and sweep invalidation, the backport-first constraints, and the failure modes a reviewer should look for. Indexed from felix/DESIGN.md; the collector as a whole stays with the flow-logs-collector sub-design. Tracking: CORE-13316.
A Semaphore block that runs the engine (K1) and collector (K2) benchmarks of the pending-policy evaluation on the policyscale reference set whenever app-policy, felix/collector or hack/perf change, and in the scheduled builds, and publishes one hack/perf document per case to the Lens trend store through send-perf-results, as the nftables dataplane benchmark does. Trend tracking, not a per-PR gate. hack/perf/perfdoc is the producer side of the hack/perf contract for Go benchmarks: a Recorder measures the timed loop (wall ns, bytes and allocs per op) and writes <dir>/<family>/<name>.json when the artifacts directory is set. Both benchmarks use it under POLICY_EVAL_PERF_ARTIFACTS_DIR; make bench-policy-eval in app-policy and felix runs them in go-build with the variable set. The benchmark_data_policy_eval index template pins the field types. Tracking: CORE-13316.
…ed worker pool
Pending-policy evaluation leaves the collector's main goroutine. Where a
flow needs evaluation (a new flow, or a live flow reached by the
re-evaluation sweep) the main loop builds a request holding copies of
what the evaluation needs (the tuple, the endpoint data it resolved, a
sequence number) and hands it to a bounded pool of workers. A worker
evaluates under the policy store's read lock, exactly as the main loop
used to, and sends the traces back. The main loop applies a result only
if the Data is still the current entry for its tuple, the result is the
flow's latest request, and the endpoints have not moved since; anything
else is counted as stale and dropped. epStats and every Data field stay
owned by the main goroutine.
New flows and sweeps use separate queues and the workers serve new flows
first. A new flow whose queue is full is evaluated on the main loop as
before, and counted; a sweep whose queue is full pauses until a result
frees a slot. Nothing is dropped, and a flow with an evaluation in
flight is not re-queued.
FlowLogsPolicyEvaluationWorkers (default 0: inline, the previous path)
and FlowLogsPolicyEvaluationBacklog (4096 per queue) configure it; both
are config-file / environment-variable parameters so that the change
picks to release branches without an API change. Metrics:
felix_collector_policy_eval_{latency_seconds,backlog,inline_total,
deferred_total,stale_results_total,workers}.
On the composite reference set, on a 12-thread laptop with 10 workers,
the main loop's cost per new flow drops from 1.6 ms to about 1 us and
throughput becomes worker-bound: 4,700 new egress flows/s with the
interpreted engine and no cache hits, 9,900 at a 50% repeat rate; with
the compiled engine (projectcalico#13267) 14,200 and 27,000.
Tests, under -race: off-loop application of traces; stale results
(superseded, flow gone, endpoints moved) dropped and counted; a full
new-flow queue falls back inline; a full recalc queue pauses and
resumes the sweep; in-flight flows are not re-queued; the keep-last-
trace-on-failure test adapted to the new split; and 500 composite-set
flows through a pooled and an inline collector give identical pending
traces. The design doc's off-loop section moves from planned to what
shipped.
Tracking: CORE-13316.
dimitri-nicolo
added
release-note-required
Contributor
CI triage — CalicoRecommendation: Dig deeper before merging Likely caused by this PR
workflow_id: 42f08ff7-3c63-40b4-b806-a6e9dbadc402 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Seventh of the stack for CORE-13316, stacked on #13901; review the last commit only. This is the change that takes the collector to the per-node target; the engine work (#13897 cache, #13898 compile) decides how many workers that takes.
What it does. Pending-policy evaluation leaves the collector's main goroutine. Where a flow needs evaluation (a new flow, or a live flow reached by the re-evaluation sweep) the main loop builds a request holding copies of what the evaluation needs — the tuple, the endpoint data it just resolved, a sequence number — and hands it to a bounded pool of workers. A worker evaluates under the policy store's read lock, exactly as the main loop used to, and sends the traces back. The main loop applies a result only if the
Datais still the current entry for its tuple, the result is the flow's latest request, and the endpoints have not moved since; anything else is counted as stale and dropped.epStatsand everyDatafield stay owned by the main goroutine: no worker touches either.Shedding, without dropping. New flows and sweeps use separate queues and the workers serve new flows first. A new flow whose queue is full is evaluated on the main loop as before, and counted. A sweep whose queue is full pauses: the flow goes back on the snapshot and the main loop masks the batch trigger until a result arrives. Nothing is dropped, no sweep floods the workers, and a flow with an evaluation already in flight is not re-queued.
Configuration.
FlowLogsPolicyEvaluationWorkers(config file / env var; default 0 = inline, exactly the previous path) andFlowLogsPolicyEvaluationBacklog(4096 per queue). Opt-in first, per the epic's backport rule; the default flips to a CPU-derived value after the node-level run and customer validation. With 0 workers the code path is the old one and the collector suite runs unchanged.Metrics.
felix_collector_policy_eval_latency_seconds{reason},_backlog{queue},_inline_total,_deferred_total,_stale_results_total,_workers.Numbers —
BenchmarkCollectorPolicyEval, composite reference set (18,662 egress / 19,992 ingress rules), 12-thread laptop, 10 workers,-benchtime 200x(sweep3x). Main loop is what the collector goroutine spends per new flow; throughput is the rate at which flows actually received their verdict.This branch (interpreted engine, cache from #13897):
With #13898's compiled engine merged on top (branch
dimitri-pmreq954-allinon the fork, a measurement branch, not for merge):So on this machine the reference set clears 10,000 new flows/s in the worst direction with no cache hits once the compiled engine is in, and with the interpreted engine at a realistic repeat rate. The main loop is no longer where the cost is: 1–1.6 µs per flow against a 100 µs budget. What remains is CPU: at 10k evaluations/s of 0.35 ms each the workers burn about 3.5 cores, which is the price of a 20k-rule set, and why the cache and the compiled engine matter as much as the pool.
Correctness. Unit tests under
-race: off-loop evaluation applies the right traces; stale results (superseded, flow gone, endpoints moved) are dropped and counted; a full new-flow queue falls back inline; a full recalc queue pauses the sweep and resumes; in-flight flows are not re-queued; the legacy keep-last-trace-on-failure test adapted to the new split.TestPolicyEvalWorkersMatchInlineruns 500 composite-set flows through a pooled and an inline collector and requires identical pending traces on every flow. The design doc section moves from "planned" to what shipped.Not in this PR. Sweep invalidation (re-evaluating only flows whose inputs changed) is still the design doc's planned section; with the cache an unchanged store's sweep is nearly free and with workers an uncached sweep runs at 19k flows/s, so it is now an optimisation rather than a blocker. The node-level (K3) run has not happened. Both directions of a flow between two local workloads are two evaluations, as before.
Testing
Release note:
AI assistance: Written with Claude Code (Claude Fable 5.1); the author reviewed the diff and ran the tests and benchmarks above.
By opening this PR you take responsibility for every line in it, and you agree to explain the change yourself during review rather than routing review comments back through an agent. See AI_POLICY.md.
🤖 Generated with Claude Code