perf(ebpf): use fused get_value() API for shard lookups, removing raw…#76
perf(ebpf): use fused get_value() API for shard lookups, removing raw…#76
Conversation
… bpf_map_lookup_elem workaround Replace the two-step shard_lookup (typed outer get() + raw inner bpf_map_lookup_elem) with the fused ArrayOfMaps::get_value() API from Brskt's updated hashmapofmaps-new branch. The fused method performs both outer and inner bpf_map_lookup_elem calls without intermediate struct indirection (MapDef::as_ptr() on the inner map), which was causing the BPF verifier's state exploration to explode in the DWARF binary search loop. This also updates the userspace ArrayOfMaps usage to pass typed Array<MapData, UnwindEntryPod> references directly to set(), matching the updated aya API. Measured on kernel 6.14, 16 CPUs: Version profile_cpu verification ────────────────────────────────── ───────────────────────── Raw bpf_map_lookup_elem (pre-#73) ~345ms Typed two-step get()+Array::get() 10.65s Workaround: typed outer, raw inner ~345ms Fused get_value() API (this commit) ~433ms
📝 WalkthroughWalkthroughReplaces two-step outer+inner eBPF map lookup with a fused typed get_value() call in the eBPF program and updates userspace to insert inner arrays by reference into ArrayOfMaps instead of passing raw file descriptors. Changes
Sequence Diagram(s)sequenceDiagram
participant Userspace
participant KernelMaps
participant eBPF
rect rgba(180,220,255,0.5)
Userspace->>KernelMaps: create inner Array<MapData,UnwindEntryPod>
Userspace->>KernelMaps: insert inner array reference into ArrayOfMaps (outer)
end
rect rgba(200,255,200,0.5)
eBPF->>KernelMaps: UNWIND_SHARDS.get_value(shard_id, &idx)
KernelMaps-->>eBPF: UnwindEntry (direct)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@profile-bee-ebpf/src/lib.rs`:
- Around line 1089-1096: Update the "Critical Pitfalls" section in AGENTS.md to
reflect the current implementation: replace the statement that "shard_lookup()
uses an 8-way static match — dynamic array indexing fails verification" with a
note that shard_lookup() now uses the fused get_value() API
(UNWIND_SHARDS.get_value(shard_id as u32, &idx)) which performs both outer and
inner bpf_map_lookup_elem calls and avoids verifier state explosion; mention the
refactor (hashmapofmaps-new / Brskt) and that the fused approach is the
supported pattern instead of static match-based indexing.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Line 270: Update the "Key Dependencies" table entry that currently lists "aya
/ aya-ebpf" to explicitly require the Brskt fork and branch (Brskt/aya with the
hashmapofmaps-new branch) so readers install the correct aya variant; locate the
Key Dependencies table in AGENTS.md (the table starting near the previously
noted line) and change the aya entry to mention "Brskt/aya (hashmapofmaps-new
branch required)" or equivalent clarifying text and optionally add a brief note
referencing the shard_lookup() usage of ArrayOfMaps::get_value() to justify the
requirement.
| - All loops MUST be bounded (`for _ in 0..CONST`). No `while`, no dynamic bounds. | ||
| - Map accesses must be bounds-checked. The verifier tracks these statically. | ||
| - `shard_lookup()` uses an 8-way static `match` — dynamic array indexing fails verification. | ||
| - `shard_lookup()` uses `ArrayOfMaps::get_value()` (from the Brskt/aya `hashmapofmaps-new` branch) which fuses the outer and inner `bpf_map_lookup_elem` into a single call without intermediate struct indirection. Using the two-step approach (`get()` then `Array::get()`) causes verifier state explosion (~10s vs ~400ms). The fused `get_value()` is the required pattern. |
There was a problem hiding this comment.
Update the Key Dependencies table to note the required aya fork.
Line 270 correctly documents the Brskt/aya hashmapofmaps-new branch requirement, but the Key Dependencies table at line 314 still lists plain aya / aya-ebpf without qualification. A contributor following only that table would install mainline aya and get a compilation error with no hint of why.
📝 Proposed fix to Key Dependencies table (line 314)
-| `aya` / `aya-ebpf` | eBPF loading, map access, program attachment |
+| `aya` / `aya-ebpf` (fork: `Brskt/aya hashmapofmaps-new` branch) | eBPF loading, map access, program attachment — requires the forked branch for `ArrayOfMaps::get_value()` |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` at line 270, Update the "Key Dependencies" table entry that
currently lists "aya / aya-ebpf" to explicitly require the Brskt fork and branch
(Brskt/aya with the hashmapofmaps-new branch) so readers install the correct aya
variant; locate the Key Dependencies table in AGENTS.md (the table starting near
the previously noted line) and change the aya entry to mention "Brskt/aya
(hashmapofmaps-new branch required)" or equivalent clarifying text and
optionally add a brief note referencing the shard_lookup() usage of
ArrayOfMaps::get_value() to justify the requirement.
… bpf_map_lookup_elem workaround
Replace the two-step shard_lookup (typed outer get() + raw inner bpf_map_lookup_elem) with the fused ArrayOfMaps::get_value() API from Brskt's updated hashmapofmaps-new branch. The fused method performs both outer and inner bpf_map_lookup_elem calls without intermediate struct indirection (MapDef::as_ptr() on the inner map), which was causing the BPF verifier's state exploration to explode in the DWARF binary search loop.
This also updates the userspace ArrayOfMaps usage to pass typed Array<MapData, UnwindEntryPod> references directly to set(), matching the updated aya API.
Measured on kernel 6.14, 16 CPUs:
Here's a cleaned-up version of the table:
Summary by CodeRabbit
Refactor
Documentation