Skip to content

lib: cgen: replace counters map with per-CPU array#547

Merged
qdeslandes merged 1 commit into
facebook:mainfrom
qdeslandes:percpu
May 27, 2026
Merged

lib: cgen: replace counters map with per-CPU array#547
qdeslandes merged 1 commit into
facebook:mainfrom
qdeslandes:percpu

Conversation

@qdeslandes

Copy link
Copy Markdown
Contributor

Switch BF_MAP_TYPE_COUNTERS from BPF_MAP_TYPE_ARRAY to
BPF_MAP_TYPE_PERCPU_ARRAY. Each CPU updates its own counter slot
without contention, which eliminates the need for atomic operations
in the BPF fast path (bpf_map_lookup_elem on a per-CPU map returns
a direct pointer to the current CPU's value).

Changes

lib:

  • Add BF_BPF_MAP_TYPE_PERCPU_ARRAY to bf_bpf_map_type
  • Switch the counters map to BF_BPF_MAP_TYPE_PERCPU_ARRAY
  • bf_handle_get_counter(): allocate a per-CPU buffer, do one lookup,
    then sum all CPU slots into the returned bf_counter
  • Add bf_handle_set_counter(): builds a per-CPU buffer with the value
    in CPU 0's slot and zeroes elsewhere, then calls
    bf_bpf_map_update_elem() with the full buffer
  • _bf_cgen_transfer_counters(): replace bf_map_set_elem() with
    bf_handle_set_counter() — the old call passed a 16-byte buffer to
    a syscall that reads num_cpus × 16 bytes, writing garbage from the
    stack into the new map

tests:

  • Update bpftool map dump jq queries in 8 e2e tests: per-CPU dumps
    use values (array per CPU) instead of value, so counter reads
    change from .[N].value.count to
    .[N].values | map(.value.count) | add

@meta-cla meta-cla Bot added the cla signed label May 23, 2026
@qdeslandes

Copy link
Copy Markdown
Contributor Author

@claude review

@github-actions

github-actions Bot commented May 23, 2026

Copy link
Copy Markdown

Claude review of PR #547 (f0720d2)

Suggestions

  • Use backticks instead of @p in Doxygensrc/libbpfilter/cgen/handle.h:150 — ast-grep doxygen-prefer-backticks rule (severity: error) flags @p counter and @p counter_idx; style guide mandates backticks
  • Repeated per-CPU buffer allocation in loopsrc/libbpfilter/cgen/handle.c:402bf_handle_set_counter allocates/frees a per-CPU buffer on each call; in _bf_cgen_transfer_counters this happens N+2 times per chain update

CLAUDE.md improvements

  • The style guide's @p prohibition could be referenced in CLAUDE.md's Comments section for visibility (currently only in doc/developers/style.rst)

Workflow run

Comment thread src/libbpfilter/cgen/handle.h Outdated
Comment thread src/libbpfilter/cgen/handle.c
Switch BF_MAP_TYPE_COUNTERS from BPF_MAP_TYPE_ARRAY to
BPF_MAP_TYPE_PERCPU_ARRAY so each CPU updates its own counter slot
without atomic contention.

The userspace read path in bf_handle_get_counter() now allocates a
per-CPU buffer and sums all CPU slots into the returned bf_counter.
A matching bf_handle_set_counter() does the reverse for counter
preservation: it places the aggregated value in CPU 0's slot and
zeroes the rest, replacing the old bf_map_set_elem() call that was
passing a single-value buffer to the syscall (which reads
num_cpus * value_size bytes and dragged garbage off the stack).

E2E tests that read the counter map via bpftool are updated to use
the per-CPU dump format: .[N].values | map(.value.count) | add
instead of .[N].value.count.
@qdeslandes qdeslandes enabled auto-merge (rebase) May 27, 2026 09:47
@qdeslandes qdeslandes merged commit 1908cfa into facebook:main May 27, 2026
37 of 38 checks passed
@qdeslandes qdeslandes deleted the percpu branch May 27, 2026 09:56
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.

1 participant