fix(modern_bpf): bpf_loop the sendmmsg/recvmmsg iovec data store - #3021
Open
deepskyblue86 wants to merge 1 commit into
Open
fix(modern_bpf): bpf_loop the sendmmsg/recvmmsg iovec data store#3021deepskyblue86 wants to merge 1 commit into
deepskyblue86 wants to merge 1 commit into
Conversation
poiana
added
area/drivers
On stricter verifiers sendmmsg_x/recvmmsg_x exceed the 1M processed-instruction limit and fail to load with E2BIG. handle_exit() runs as a bpf_loop callback, and the bounded for(j < MAX_IOVCNT) loop in the native iovec data store (parameter 4) is re-explored on every SCC fixed-point pass of that callback, multiplying the processed-instruction count. Replace the inner for-loop with a bpf_loop() helper so the verifier checks the per-iteration body once, collapsing that multiplication and keeping the program within the limit. The loop callback re-fetches the per-cpu auxmap (a map-value pointer cannot cross the bpf_loop context) and masks the index for bounds. handle_exit() is shared with the legacy single-message programs sendmmsg_old_x/recvmmsg_old_x, which are loaded only when the bpf_loop helper is NOT available and therefore must not contain a bpf_loop call. The store is chosen by a compile-time use_bpf_loop constant threaded from each program (true for the bpf_loop programs, false for the legacy ones), NOT by a CO-RE feature probe: some kernels expose BPF_FUNC_loop in their BTF enum without implementing the helper, so bpf_core_enum_value_exists() would wrongly route the legacy program through bpf_loop and it would fail to load with "invalid func unknown#181". The selector is a tiny __always_inline that, because use_bpf_loop is constant, collapses to a single call to one of two __noinline stores: the bpf_loop store for the _x programs and the for-loop store for the _old_x programs. The __noinline boundary is required on its own -- inlining the bpf_loop store into the callback re-introduces the SCC re-exploration and the E2BIG. The legacy single-message path is not subject to the SCC blowup, so its for-loop store is correct. The ia32 (compat) mmsg path keeps the bounded for-loop. The 3-frame 512-byte stack budget (program -> callback -> noinline callee) is preserved. Signed-off-by: Angelo Puglisi <angelopuglisi86@gmail.com>
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deepskyblue86 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
deepskyblue86
force-pushed
the
fix/mmsg-iovec-bpf_loop
branch
from
June 10, 2026 17:06
0c1f3c6 to
e2f6faf
Compare
Perf diff from master - unit testsHeap diff from master - unit testsHeap diff from master - scap fileBenchmarks diff from master |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3021 +/- ##
=======================================
Coverage 75.58% 75.58%
=======================================
Files 299 299
Lines 33123 33123
Branches 5138 5140 +2
=======================================
Hits 25037 25037
Misses 8086 8086
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
X64 kernel testing matrix
ARM64 kernel testing matrix
|
terror96
reviewed
Jun 30, 2026
| static long iovec_data_loop_callback(uint32_t index, void *ctx) { | ||
| iovec_data_loop_ctx_t *c = (iovec_data_loop_ctx_t *)ctx; | ||
|
|
||
| if(c->total_size_to_read > c->len_to_read) { |
Contributor
There was a problem hiding this comment.
Just double checking: if c->total_size_to_read == c->len_to_read, do we still want to proceed? i.e. should we check for >= ?
terror96
reviewed
Jun 30, 2026
| .truncated = false, | ||
| }; | ||
|
|
||
| bpf_loop(MAX_IOVCNT, iovec_data_loop_callback, &ctx, 0); |
Contributor
There was a problem hiding this comment.
Should we handle the return value?
terror96
reviewed
Jun 30, 2026
| uint16_t bytes_read = push__bytebuf(auxmap->data, | ||
| &auxmap->payload_pos, | ||
| (unsigned long)iovec[j].iov_base, | ||
| iovec[j].iov_len, |
Contributor
There was a problem hiding this comment.
Wondering if we should already here limit the amount of reading not to exceed the remaining budget i.e. len_to_read - total_size_to_read ... the overshoot will be caught by the next iteration, though.
Member
|
@deepskyblue86 any update on this? |
Member
@deepskyblue86 ping 👼 |
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
4 participants
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.
What type of PR is this?
/kind bug
Any specific area of the project related to this PR?
/area drivers
/area driver-modern-bpf
Does this PR require a change in the driver versions?
What this PR does / why we need it:
Getting
libbpf: prog 'sendmmsg_x': BPF program load failed: Argument list too longin Fedora 42, kernel 6.19.14-108.fc42, both aarch64 and x86_64.This change converts
auxmap__store_iovec_data_param_64from having a regular for loop to using a bpf loop.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: