Skip to content

lib: cgen: batch-load map elements in a single syscall #542

Merged
qdeslandes merged 2 commits into
facebook:mainfrom
pzmarzly:push-lknlunzwztvv
May 15, 2026
Merged

lib: cgen: batch-load map elements in a single syscall #542
qdeslandes merged 2 commits into
facebook:mainfrom
pzmarzly:push-lknlunzwztvv

Conversation

@pzmarzly
Copy link
Copy Markdown
Contributor

This will once again make us load entire BPF map in one syscall, after #520 changed them to load with 2n syscalls. We'll compute the desired shape on userspace side, and use bf_bpf_map_update_batch to load it into kernelspace.

Benchmark

Before #520 (I git revert-ed it and let AI fix merge conflicts):

-----------------------------------------------------------------------------------
Benchmark                                         Time             CPU   Iterations
-----------------------------------------------------------------------------------
chain_set__ip4_saddr__x_elem_set/8             6.64 ms         6.44 ms          121 load chain, ip4.saddr, 8 elements set
chain_set__ip4_saddr__x_elem_set/65536         34.2 ms         33.6 ms           21 load chain, ip4.saddr, 65536 elements set
chain_set__ip4_saddr__x_elem_set/1048576        714 ms          708 ms            1 load chain, ip4.saddr, 1048576 elements set

Trunk:

-----------------------------------------------------------------------------------
Benchmark                                         Time             CPU   Iterations
-----------------------------------------------------------------------------------
chain_set__ip4_saddr__x_elem_set/8             8.26 ms         8.01 ms           75 load chain, ip4.saddr, 8 elements set
chain_set__ip4_saddr__x_elem_set/65536          107 ms          106 ms            7 load chain, ip4.saddr, 65536 elements set
chain_set__ip4_saddr__x_elem_set/1048576       1782 ms         1770 ms            1 load chain, ip4.saddr, 1048576 elements set

With this PR:

-----------------------------------------------------------------------------------
Benchmark                                         Time             CPU   Iterations
-----------------------------------------------------------------------------------
chain_set__ip4_saddr__x_elem_set/8             6.48 ms         6.30 ms          121 load chain, ip4.saddr, 8 elements set
chain_set__ip4_saddr__x_elem_set/65536         35.1 ms         34.6 ms           19 load chain, ip4.saddr, 65536 elements set
chain_set__ip4_saddr__x_elem_set/1048576        816 ms          810 ms            1 load chain, ip4.saddr, 1048576 elements set

@pzmarzly pzmarzly requested a review from qdeslandes as a code owner May 13, 2026 17:37
@meta-cla meta-cla Bot added the cla signed label May 13, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 13, 2026

Claude review of PR #542 (df4fdda)

Suggestions

  • Deduplicate hash/equal callbackssrc/libbpfilter/cgen/program.c:906_bf_dedup_hash and _bf_dedup_equal are identical to _bf_set_elem_hash/_bf_set_elem_equal in set.c; consider sharing them via a private header
  • Pre-reserve dedup hashset capacitysrc/libbpfilter/cgen/program.c:962 — Computing an upper-bound and calling bf_hashset_reserve before the dedup loop would avoid rehashing for large sets

Workflow run

Comment thread src/libbpfilter/cgen/program.c
Comment thread src/libbpfilter/cgen/program.c
@pzmarzly pzmarzly force-pushed the push-lknlunzwztvv branch from df4fdda to 27ce007 Compare May 13, 2026 17:56
Copy link
Copy Markdown
Contributor

@qdeslandes qdeslandes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great addition, a few nits, but otherwise LGTM!

For reference, the benchmark CI job shares the report as artifact, here's the results before (main) and after (PR):

Name Before After
load chain, ip4.saddr, 8 elements set 9.17ms 9.83ms
load chain, ip4.saddr, 65536 elements set 159.79ms 69.99ms
load chain, ip4.saddr, 1048576 elements set 2913.68ms 1584.11ms

That's a very good addition!

Comment thread src/libbpfilter/cgen/program.c
Comment thread src/libbpfilter/cgen/program.c
Comment thread src/libbpfilter/cgen/program.c
@pzmarzly pzmarzly force-pushed the push-lknlunzwztvv branch 2 times, most recently from 20b6fa7 to 19ed706 Compare May 13, 2026 21:19
@pzmarzly pzmarzly requested a review from qdeslandes May 13, 2026 21:19
Copy link
Copy Markdown
Contributor

@qdeslandes qdeslandes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks for fixing this! :D

@qdeslandes
Copy link
Copy Markdown
Contributor

@pzmarzly I miss clicked and GitHub created a merge commit :( Could you remove it so I can merge the branch? Thanks!

pzmarzly added 2 commits May 15, 2026 11:36
Build keys and bitmask values in user space and submit them all at once
with one bf_bpf_map_update_batch() call. A bf_hashset is used to
deduplicate entries.
@pzmarzly pzmarzly force-pushed the push-lknlunzwztvv branch from 2588252 to c892fa4 Compare May 15, 2026 11:36
@pzmarzly
Copy link
Copy Markdown
Contributor Author

pzmarzly commented May 15, 2026

No problem. Rebased. :)

@qdeslandes qdeslandes merged commit 7a9e06a into facebook:main May 15, 2026
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants