linux-kernel - RE: [PATCH bpf-next 2/2] selftests/bpf: Enhance XDP Rx Metadata Handling

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <IA3PR11MB925416396633E361F37E819DD840A@IA3PR11MB9254.namprd11.prod.outlook.com>
Date: Wed, 2 Jul 2025 02:23:29 +0000
From: "Song, Yoong Siang" <yoong.siang.song@...el.com>
To: 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 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@intel.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