Make the live gates deterministic on a busy host - #17
Conversation
The gate required a complete footer, which requires zero recursion misses. Helper tracking attaches kprobes to the map helpers the tracer itself calls, so any concurrent BPF hash activity anywhere on the host re-enters the tracer and trips the kernel recursion guard. The counter therefore measures what else is running: zero on an idle machine, reliably thousands on a workstation. The gate was red on any developer host and silently assumed an idle one. The misses are genuine missed observations and the footer is right to report them, so the assertion is what needs narrowing, not the behaviour. Accept an incomplete footer only when recursion misses explain it, and hold every other counter at zero. This tightens the gate rather than loosening it. Enrichment and output failures were never asserted at all and now are, and an incomplete footer that recursion misses do not explain is now rejected where before only the single `complete` flag was consulted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 48 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe live gate now requires zero userspace enrichment and output failures. It accepts incomplete capture footers only when kernel recursion misses explain the discrepancy. The validation guide documents these rules and the ChangesLive gate validation
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The gate captured consume_skb unfiltered, so on a busy host system-wide teardown filled the 1024-event budget before the consume_skb -> dst_release -> kmem_cache_free triple it asserts on could appear. It also required a complete footer, and kmem_cache_free sits on the teardown path the tracer itself walks, so concurrent allocator traffic re-enters the tracer and trips the kernel recursion guard. Both failures track host load rather than this code, and the gate was flaky in both directions because of it: measured across interleaved runs it failed on an unmodified binary and passed on a modified one in the same session, which is worse than a gate that is simply red. Scope the capture to icmp, the traffic the gate itself generates. Under four concurrent iperf3 streams that yields 30 events and 20 stack associations on every run, against 1024 capped events and a 15-to-30 spread unfiltered. The pairing assertion becomes deterministic instead of merely likely, and kernel read failures drop from hundreds to zero because the capture no longer sees SKBs it has no business reading. Scope the footer the same way as the BPF-helper gate: recursion misses are the only accepted incompleteness, every other counter stays at zero. Read, enrichment and output failures were never asserted here at all and now are. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
copyleftdev
changed the title
1 participant
Two live gates fail on
mainat b378c01 on any host with concurrent activity. Found while running the full suite against unrelated work; independent of it, so this is its own PR offmain.Result: 14/14 gates pass, previously 12/14 with one of the two flaky in both directions.
live-bpf-helper— asserted a condition only an idle host satisfiesThe gate required
complete == true, which requireskernel_recursion_misses == 0.Helper tracking kprobes the map helpers the tracer itself calls (
__htab_map_lookup_elem,htab_map_update_elem). Every BPF program on the host doing a hash-map operation fires those probes, and when the tracer is already executing on that CPU the kernel recursion guard skips the nested invocation and increments the counter.So the counter measures what else is running. Six runs of the unmodified binary on a 64-core workstation with ~32 loaded BPF programs:
Zero on an idle machine, thousands on a workstation, 2× swing between back-to-back runs of the same binary.
live-stack-lifetime— flaky in both directionsSame recursion-guard problem (it kprobes
kmem_cache_free, on the teardown path the tracer walks), plus a second one: it capturedconsume_skbunfiltered, so system-wide teardown filled the 1024-event budget before theconsume_skb → dst_release → kmem_cache_freetriple it asserts on could appear.Every run hit the cap. Interleaved across two binaries:
A gate that fails on an unmodified binary and passes on a modified one in the same session is worse than one that is simply red — it actively misleads. I drew a wrong conclusion from it myself before running it interleaved.
Fixes
Scope the capture, don't widen the assertion.
live-stack-lifetimenow filters toicmp, the traffic it generates. Under four concurrent iperf3 streams:icmpKernel read failures also drop from hundreds to zero, because the capture no longer reads SKBs that were never its business.
Scope the footer to counters the gate controls. Both gates now accept an incomplete footer only when
kernel_recursion_missesexplains it, and hold every other counter at zero. Those misses are genuine missed observations and the footer is right to report them — suppressing them in the product to green a test would trade a true reliability signal for a tick.This tightens both gates
Verified by mutating real captured traces one field at a time, for each gate:
kernel_reserve_failures = 1kernel_read_failures = 1userspace_decode_failures = 1userspace_enrichment_failures = 1output_failures = 1New coverage:
userspace_enrichment_failuresandoutput_failureswere asserted by neither gate before, andkernel_read_failuresby neitherlive-stack-lifetimenor the old helper footer. The last row is strictly stronger than the old check, which consulted only thecompleteflag and could not distinguish why a footer was incomplete.Checks run
make check— exit 0.live-stack-lifetime— 5 consecutive passes under four concurrent iperf3 streams, 3 more idle.live-bpf-helper— 3 consecutive passes; previously failed every run.Note
docs/validation.mdgains the general rules rather than just these two instances: scope a gate's capture to its own traffic so its assertion is deterministic, and when a gate traces machinery skbx itself uses, name the counter it cannot control and say why — instead of demanding a clean footer it cannot guarantee, or dropping the check.🤖 Generated with Claude Code