| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <IA3PR11MB9254C961FD048793FD0013EAD840A@IA3PR11MB9254.namprd11.prod.outlook.com>
Date: Wed, 2 Jul 2025 03:55:38 +0000
From: "Song, Yoong Siang" <yoong.siang.song@...el.com>
To: "Song, Yoong Siang" <yoong.siang.song@...el.com>, Stanislav Fomichev
<stfomichev@...il.com>
CC: "David S . Miller" <davem@...emloft.net>, Eric Dumazet
<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
<pabeni@...hat.com>, Simon Horman <horms@...nel.org>, Jonathan Corbet
<corbet@....net>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann
<daniel@...earbox.net>, Jesper Dangaard Brouer <hawk@...nel.org>, "John
Fastabend" <john.fastabend@...il.com>, Stanislav Fomichev <sdf@...ichev.me>,
Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>,
Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>, "Yonghong
Song" <yonghong.song@...ux.dev>, KP Singh <kpsingh@...nel.org>, Hao Luo
<haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, Mykola Lysenko
<mykolal@...com>, Shuah Khan <shuah@...nel.org>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>, "linux-doc@...r.kernel.org"
<linux-doc@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
"linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>
Subject: RE: [PATCH bpf-next 2/2] selftests/bpf: Enhance XDP Rx Metadata
Handling
On Wednesday, July 2, 2025 10:23 AM, Song, Yoong Siang <yoong.siang.song@...el.com> wrote:
>On Wednesday, July 2, 2025 12:31 AM, Stanislav Fomichev <stfomichev@...il.com>
>wrote:
>>On 07/01, Song Yoong Siang wrote:
>>> Introduce the XDP_METADATA_SIZE macro to ensure that user applications can
>>> consistently retrieve the correct location of struct xdp_meta.
>>>
>>> Prior to this commit, the XDP program adjusted the data_meta backward by
>>> the size of struct xdp_meta, while the user application retrieved the data
>>> by calculating backward from the data pointer. This approach only worked if
>>> xdp_buff->data_meta was equal to xdp_buff->data before calling
>>> bpf_xdp_adjust_meta.
>>>
>>> With the introduction of XDP_METADATA_SIZE, both the XDP program and user
>>> application now calculate and identify the location of struct xdp_meta from
>>> the data pointer. This ensures the implementation remains functional even
>>> when there is device-reserved metadata, making the tests more portable
>>> across different NICs.
>>>
>>> Signed-off-by: Song Yoong Siang <yoong.siang.song@...el.com>
>>> ---
>>> tools/testing/selftests/bpf/prog_tests/xdp_metadata.c | 2 +-
>>> tools/testing/selftests/bpf/progs/xdp_hw_metadata.c | 10 +++++++++-
>>> tools/testing/selftests/bpf/progs/xdp_metadata.c | 8 +++++++-
>>> tools/testing/selftests/bpf/xdp_hw_metadata.c | 2 +-
>>> tools/testing/selftests/bpf/xdp_metadata.h | 7 +++++++
>>> 5 files changed, 25 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
>>b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
>>> index 19f92affc2da..8d6c2633698b 100644
>>> --- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
>>> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
>>> @@ -302,7 +302,7 @@ static int verify_xsk_metadata(struct xsk *xsk, bool
>>sent_from_af_xdp)
>>>
>>> /* custom metadata */
>>>
>>> - meta = data - sizeof(struct xdp_meta);
>>> + meta = data - XDP_METADATA_SIZE;
>>>
>>> if (!ASSERT_NEQ(meta->rx_timestamp, 0, "rx_timestamp"))
>>> return -1;
>>> diff --git a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c
>>b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c
>>> index 330ece2eabdb..72242ac1cdcd 100644
>>> --- a/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c
>>> +++ b/tools/testing/selftests/bpf/progs/xdp_hw_metadata.c
>>> @@ -27,6 +27,7 @@ extern int bpf_xdp_metadata_rx_vlan_tag(const struct
>>xdp_md *ctx,
>>> SEC("xdp.frags")
>>> int rx(struct xdp_md *ctx)
>>> {
>>> + int metalen_used, metalen_to_adjust;
>>> void *data, *data_meta, *data_end;
>>> struct ipv6hdr *ip6h = NULL;
>>> struct udphdr *udp = NULL;
>>> @@ -72,7 +73,14 @@ int rx(struct xdp_md *ctx)
>>> return XDP_PASS;
>>> }
>>>
>>> - err = bpf_xdp_adjust_meta(ctx, -(int)sizeof(struct xdp_meta));
>>
>>[..]
>>
>>> + metalen_used = ctx->data - ctx->data_meta;
>>
>>Is the intent here to query how much metadata has been consumed/reserved
>>by the driver?
>Yes.
>
>>Looking at IGC it has the following code/comment:
>>
>> bi->xdp->data += IGC_TS_HDR_LEN;
>>
>> /* HW timestamp has been copied into local variable. Metadata
>> * length when XDP program is called should be 0.
>> */
>> bi->xdp->data_meta += IGC_TS_HDR_LEN;
>>
>>Are you sure that metadata size is correctly exposed to the bpf program?
>You are right, the current igc driver didn't expose the metadata size correctly.
>I submitted [1] to fix it.
>
>[1] https://patchwork.ozlabs.org/project/intel-wired-
>lan/patch/20250701080955.3273137-1-yoong.siang.song@...el.com/
>
>>
>>My assumptions was that we should just unconditionally do bpf_xdp_adjust_meta
>>with -XDP_METADATA_SIZE and that should be good enough.
>
>The checking is just for precautions. No problem if directly adjust the meta
>unconditionally.
>That will save processing time for each packet as well.
>I will remove the checking and submit v2.
>
>Thanks & Regards
>Siang
>
Hi Stanislav Fomichev,
I submitted v2. But after that, I think twice. IMHO,
err = bpf_xdp_adjust_meta(ctx, (int)(ctx->data - ctx->data_meta - XDP_METADATA_SIZE));
is better than
err = bpf_xdp_adjust_meta(ctx, -(int)XDP_METADATA_SIZE);
because it is more robust.
Any thoughts?
Thanks & Regards
Siang
Powered by blists - more mailing lists