Repository navigation
lib: cgen: fix the state map BTF for chains with several rules - #626
Open
prashantpcm94 wants to merge 1 commit into
Open
prashantpcm94 wants to merge 1 commit into
prashantpcm94 wants to merge 1 commit into
Conversation
The state map's single value holds one struct bf_rule_state per rule of the chain, but its BTF describes the value as a single bf_rule_state. The kernel rejects a BTF value type whose size doesn't match the map's value size, so any chain with 2 rules or more that uses "log ... every" fails to load with -EINVAL. Describe the value as an array of bf_rule_state, one per rule.
Contributor
Author
|
@claude review |
|
Claude review failed — see workflow run for details. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A chain with 2 rules or more fails to load as soon as one of its rules uses
log ... every <rate>:The state map has a single value holding one
struct bf_rule_stateper rule (n_rules * sizeof(struct bf_rule_state)), but its BTF describes the value as a singlebf_rule_state. The kernel rejects a BTF value type whose size doesn't match the map's value size, so only single-rule chains could uselog ... every. This describes the value as an array ofbf_rule_state, one per rule.Related issue
None.
Testing
CMake release build (
-DNO_DOCS=1) on CentOS Stream 9, kernel 6.16.1, gcc 11.5, clang 22.1.3, libbpf 1.6.3, thenmake -C $BUILD test_binandctest(withoutfuzzing.parser.infinite, which runs until stopped):check.iwyudidn't run becauseiwyu_tool.pyisn't installed here (it only checkscgen/program.c).check.astgrepandcheck.astgrep.testpass (in a second build withast-greponPATH).e2e.rules.log: a newchain setof a 2-rule XDP chain whose second rule useslog every 1s. It fails on main with the error above and passes with this PR.log ... everyfail on main and load with this PR;bpftool map dump name bf_smapshows onelast_log_tsper rule;log internet every 1s, 12 ICMP packets (5 pings, then 1 more after 1.2 s) give 2 log entries for the second rule, and the first TCP packet (first rule) and the first ICMP packet (second rule), sent a few milliseconds apart, are both logged: each rule has its own state.Notes for the reviewer
u32type ID is kept in a localintbecausebtf__add_array()takesinttype IDs andkey_type_idis auint32_t(clang-tidy flags the narrowing).AI disclosure
Claude Code found the issue, drafted the change, the test, the commit message and this description under my direction, and ran the tests listed under Testing. I reviewed the change and the results.
Checklist
make -C $BUILD test_bin testpasses (check.iwyunot run, see Testing) and the code follows the style guidecomponent: subcomponent: short description