fix(ebpf): use raw bpf_map_lookup_elem for inner shard lookup to fix …#74
fix(ebpf): use raw bpf_map_lookup_elem for inner shard lookup to fix …#74
Conversation
…26x verifier slowdown The typed Array::get() API from Brskt's aya-ebpf fork, when used for the inner map lookup in shard_lookup(), generates BPF bytecode that causes the kernel verifier's state exploration to explode. Although the typed and raw code paths produce identical branch counts (199 conditional jumps in profile_cpu), the typed path inserts extra intermediate instructions between the outer and inner bpf_map_lookup_elem calls — notably binary search midpoint calculations that the compiler reorders into the gap. This different code structure causes the verifier to track exponentially more register states at join points inside the DWARF binary search loop. Measured on kernel 6.14, 16 CPUs: Version profile_cpu.load() Total ──────────────────────────── ────────────────── ───── Pre-merge (raw lookups) ~345ms ~408ms Post-merge (typed Array::get()) 10.65s 10.7s This fix (typed outer, raw inner) ~345ms ~403ms The fix keeps the Brskt typed ArrayOfMaps<Array<UnwindEntry>> for map definitions and the outer map lookup, but reverts the inner map lookup to raw bpf_map_lookup_elem. This preserves the cleaner map definition API while keeping the BPF bytecode verifier-friendly. All 16 e2e tests pass, 51 unit tests pass, fmt and clippy clean.
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughReplaced typed inner-map Array lookups in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
profile-bee-ebpf/src/lib.rs (1)
1095-1108: Ensure the prebuilt eBPF binary is rebuilt and committed.The
shard_lookupchange alters the compiled BPF bytecode. The prebuilt object atprofile-bee/ebpf-bin/profile-bee.bpf.ois embedded at compile time and must stay in sync — users runningcargo installon stable Rust depend on it.Run
cargo xtask build-ebpf --releaseand commit the updated binary.Based on learnings: "Prebuilt eBPF binaries are embedded at compile time in
profile-bee/ebpf-bin/profile-bee.bpf.o. Keep in sync withprofile-bee-ebpf/changes viacargo xtask build-ebpf --release."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@profile-bee-ebpf/src/lib.rs` around lines 1095 - 1108, The change to shard_lookup modifies the compiled BPF bytecode so you must rebuild the prebuilt embedded eBPF object and commit the updated artifact; run the build task (cargo xtask build-ebpf --release) in the ebpf crate to produce a new eBPF object that matches the updated shard_lookup and then replace the committed prebuilt eBPF object in the repo with that new build and commit it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@profile-bee-ebpf/src/lib.rs`:
- Around line 1095-1108: The change to shard_lookup modifies the compiled BPF
bytecode so you must rebuild the prebuilt embedded eBPF object and commit the
updated artifact; run the build task (cargo xtask build-ebpf --release) in the
ebpf crate to produce a new eBPF object that matches the updated shard_lookup
and then replace the committed prebuilt eBPF object in the repo with that new
build and commit it.
The upstream aya dependency (hashmapofmaps-new branch) moved the fd() method behind the InnerMap trait. Import it in both call sites to fix the CI build.
1 participant
…26x verifier slowdown
The typed Array::get() API from Brskt's aya-ebpf PR (aya-rs/aya#1446), when used for the inner map lookup in shard_lookup(), generates BPF bytecode that causes the kernel verifier's state exploration to explode. Although the typed and raw code paths produce identical branch counts (199 conditional jumps in profile_cpu), the typed path inserts extra intermediate instructions between the outer and inner bpf_map_lookup_elem calls — notably binary search midpoint calculations that the compiler reorders into the gap. This different code structure causes the verifier to track exponentially more register states at join points inside the DWARF binary search loop.
Measured on kernel 6.14, 16 CPUs:
profile_cpu.load()(ms)The fix keeps the Brskt typed ArrayOfMaps<Array> for map definitions and the outer map lookup, but reverts the inner map lookup to raw bpf_map_lookup_elem. This preserves the cleaner map definition API while keeping the BPF bytecode verifier-friendly.
All 16 e2e tests pass, 51 unit tests pass, fmt and clippy clean.
Summary by CodeRabbit