-
Notifications
You must be signed in to change notification settings - Fork 3.2k
bpf/complexity-tests: bpf_xdp coverage improvements #38561
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Moving to draft for now, apparently more config options should be enabled in the test, as it still fails in the E2E test while passing ci-verifier: https://github.com/cilium/cilium/actions/runs/14115207803/job/39543635184 |
c6627c9
to
ce252f3
Compare
ce252f3
to
9b9aa5b
Compare
/test |
9b9aa5b
to
6a7d8d6
Compare
/test |
1 similar comment
/test |
Thanks for catching that. I think at some point when working on #29711, I renamed Might be worth to have some sanity check in CI to verify if defines in those complexity tests actually affect anything. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nonblocking: maybe reverse the order of 3rd and 2nd patch, so as to ensure git-bisectibility?
Overall LGTM
3b3ebec
to
6382ddf
Compare
/test |
ci-e2e-upgrade test may fail on kernel 5.15 due to verifier complexity limit, while ci-verifier passes in all tests. Increase the coverage of the latter by adding that specific scenario and its extended version. Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
After enabling coverage for EVENTS_MAP_RATE_LIMIT (next commit), clang sometimes uninlines ratelimit_check_and_take (marked as inline, but not __always_inline), which produces this verification error in bpf_host and bpf_lxc on kernel 5.4: Error: program tail_ipv4_to_endpoint: load program: invalid argument: tail_calls are not allowed in programs with bpf-to-bpf calls Mark ratelimit_check_and_take as __always_inline to suppress this behavior. Fixes: 93f1619 ("bpf: Add ratelimiting feature and use it for ICMPv6 responses") Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
The correct define name is EVENTS_MAP_RATE_LIMIT, not EVENTS_MAP_RATE_LIMIT_PER_SECOND. Fix the typo to add coverage for the rate limiting feature. Fixes: b7a26eb ("Add rate limiting on BPF events map") Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
6382ddf
to
cc17ebd
Compare
/test |
ci-e2e-upgrade test may fail on kernel 5.15 due to verifier complexity limit, while ci-verifier passes in all tests. Increase the coverage of the latter by adding that specific scenario and its extended version.
Link to that specific occurrence: https://github.com/cilium/cilium/actions/runs/14091707307/job/39498542748?pr=38110
Fix a typo EVENTS_MAP_RATE_LIMIT -> EVENTS_MAP_RATE_LIMIT_PER_SECOND that led to missing coverage for rate limiting on BPF events map (Cc: @siwiutki, #29711).