-
Notifications
You must be signed in to change notification settings - Fork 143
skb meta/safeproof helpers/data move helper #10033
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
Open
jsitnicki
wants to merge
16
commits into
kernel-patches:bpf-next_base
Choose a base branch
from
jsitnicki:skb-meta/safeproof-helpers/data_move_helper
base: bpf-next_base
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
skb meta/safeproof helpers/data move helper #10033
jsitnicki
wants to merge
16
commits into
kernel-patches:bpf-next_base
from
jsitnicki:skb-meta/safeproof-helpers/data_move_helper
Conversation
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
This patch set continues our work [1] to allow BPF programs and user-space applications to attach multiple bytes of metadata to packets via the XDP/skb metadata area. The focus of this patch set it to ensure that skb metadata remains intact when packets pass through a chain of TC BPF programs that call helpers which operate on skb head. Currently, several helpers that either adjust the skb->data pointer or reallocate skb->head do not preserve metadata at its expected location, that is immediately in front of the MAC header. These are: - bpf_skb_adjust_room - bpf_skb_change_head - bpf_skb_change_proto - bpf_skb_change_tail - bpf_skb_vlan_pop - bpf_skb_vlan_push In TC BPF context, metadata must be moved whenever skb->data changes to keep the skb->data_meta pointer valid. I don't see any way around it. Creative ideas how to avoid that would be very welcome. We can patch the helpers in at least two different ways: 1. Integrate metadata move into header move Replace the existing memmove, which follows skb_push/pull, with a helper that moves both headers and metadata in a single call. This avoids an extra memmove but reduces transparency. skb_pull(skb, len); - memmove(skb->data, skb->data - len, n); + skb_postpull_data_move(skb, len, n); skb->mac_header += len; skb_push(skb, len) - memmove(skb->data, skb->data + len, n); + skb_postpush_data_move(skb, len, n); skb->mac_header -= len; 2. Move metadata separately Add a dedicated metadata move after the header move. This is more explicit but costs an additional memmove. skb_pull(skb, len); memmove(skb->data, skb->data - len, n); + skb_metadata_postpull_move(skb, len); skb->mac_header += len; skb_push(skb, len) + skb_metadata_postpush_move(skb, len); memmove(skb->data, skb->data + len, n); skb->mac_header -= len; This patch set implements option (1), expecting that "you can have just one memmove" will be the most obvious feedback, while readability is a, somewhat subjective, matter of taste which I don't claim to have ;-) The structure of the patch set is as follows: - patches 1-3 prepare ground for safe-proofing the BPF helpers - patches 4-8 modify the BPF helpers to preserve skb metadata - patches 9-10 prepare ground for verifying metadata after BPF helper calls - patches 11-15 adapt and expand tests to cover the made changes Thanks, -jkbs [1] https://lore.kernel.org/all/20250814-skb-metadata-thru-dynptr-v7-0-8a39e636e0fb@cloudflare.com/ To: [email protected] Cc: David S. Miller <[email protected]> Cc: Eric Dumazet <[email protected]> Cc: Jakub Kicinski <[email protected]> Cc: Paolo Abeni <[email protected]> Cc: Simon Horman <[email protected]> Cc: Martin KaFai Lau <[email protected]> Cc: Daniel Borkmann <[email protected]> Cc: John Fastabend <[email protected]> Cc: Stanislav Fomichev <[email protected]> Cc: Alexei Starovoitov <[email protected]> Cc: Andrii Nakryiko <[email protected]> Cc: Eduard Zingerman <[email protected]> Cc: Song Liu <[email protected]> Cc: Yonghong Song <[email protected]> Cc: KP Singh <[email protected]> Cc: Hao Luo <[email protected]> Cc: Jiri Olsa <[email protected]> Cc: [email protected] Cc: [email protected] --- Changes in v2: - Tweak WARN_ON_ONCE check in skb_data_move() (patch 2) - Convert all tests to verify skb metadata in BPF (patches 9-10) - Add test coverage for modified BPF helpers (patches 12-15) - Link to v1: https://lore.kernel.org/r/[email protected] --- b4-submit-tracking --- # This section is used internally by b4 prep for tracking purposes. { "series": { "revision": 2, "change-id": "20250903-skb-meta-rx-path-be50b3a17af9", "prefixes": [ "bpf-next" ], "history": { "v1": [ "[email protected]" ] } } }
pskb_expand_head() copies headroom, including skb metadata, into the newly allocated head, but then clears the metadata. As a result, metadata is lost when BPF helpers trigger a headroom reallocation. Let the skb metadata be in the newly created copy of head. Signed-off-by: Jakub Sitnicki <[email protected]>
Lay groundwork for fixing BPF helpers available to TC(X) programs. When skb_push() or skb_pull() is called in a TC(X) ingress BPF program, the skb metadata must be kept in front of the MAC header. Otherwise, BPF programs using the __sk_buff->data_meta pseudo-pointer lose access to it. Introduce a helper that moves both metadata and a specified number of packet data bytes together, suitable as a drop-in replacement for memmove(). Signed-off-by: Jakub Sitnicki <[email protected]>
All callers ignore the return value. Prepare to reorder memmove() after skb_pull() which is a common pattern. Signed-off-by: Jakub Sitnicki <[email protected]>
Use the metadata-aware helper to move packet bytes after skb_pull(), ensuring metadata remains valid after calling the BPF helper. Signed-off-by: Jakub Sitnicki <[email protected]>
Use the metadata-aware helper to move packet bytes after skb_push(), ensuring metadata remains valid after calling the BPF helper. Also, take care to reserve sufficient headroom for metadata to fit. Signed-off-by: Jakub Sitnicki <[email protected]>
bpf_skb_adjust_room() may push or pull bytes from skb->data. In both cases, skb metadata must be moved accordingly to stay accessible. Replace existing memmove() calls, which only move payload, with a helper that also handles metadata. Reserve enough space for metadata to fit after skb_push. Signed-off-by: Jakub Sitnicki <[email protected]>
bpf_skb_change_proto reuses the same headroom operations as bpf_skb_adjust_room, already updated to handle metadata safely. The remaining step is to ensure that there is sufficient headroom to accommodate metadata on skb_push(). Signed-off-by: Jakub Sitnicki <[email protected]>
Although bpf_skb_change_head() doesn't move packet data after skb_push(), skb metadata still needs to be relocated. Use the dedicated helper to handle it. Signed-off-by: Jakub Sitnicki <[email protected]>
Move metadata verification into the BPF TC programs. Previously, userspace read metadata from a map and verified it once at test end. Now TC programs compare metadata directly using __builtin_memcmp() and set a test_pass flag. This enables verification at multiple points during test execution rather than a single final check. Signed-off-by: Jakub Sitnicki <[email protected]>
Add diagnostic output when metadata verification fails to help with troubleshooting test failures. Introduce a check_metadata() helper that prints both expected and received metadata to the BPF program's stderr stream on mismatch. The userspace test reads and dumps this stream on failure. Signed-off-by: Jakub Sitnicki <[email protected]>
Since pskb_expand_head() no longer clears metadata on unclone, update tests for cloned packets to expect metadata to remain intact. Also simplify the clone_dynptr_kept_on_{data,meta}_slice_write tests. Creating an r/w dynptr slice is sufficient to trigger an unclone in the prologue, so remove the extraneous writes to the data/meta slice. Signed-off-by: Jakub Sitnicki <[email protected]>
Add a test to verify that skb metadata remains accessible after calling bpf_skb_vlan_push() and bpf_skb_vlan_pop(), which modify the packet headroom. Signed-off-by: Jakub Sitnicki <[email protected]>
Add a test to verify that skb metadata remains accessible after calling bpf_skb_adjust_room(), which modifies the packet headroom and can trigger head reallocation. The helper expects an Ethernet frame carrying an IP packet so switch test packet identification by source MAC address since we can no longer rely on Ethernet proto being set to zero. Signed-off-by: Jakub Sitnicki <[email protected]>
Add a test to verify that skb metadata remains accessible after calling bpf_skb_change_head() and bpf_skb_change_tail(), which modify packet headroom/tailroom and can trigger head reallocation. Signed-off-by: Jakub Sitnicki <[email protected]>
Add a test to verify that skb metadata remains accessible after calling bpf_skb_change_proto(), which modifies packet headroom to accommodate different IP header sizes. Signed-off-by: Jakub Sitnicki <[email protected]>
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.
No description provided.