On Wednesday, July 2, 2025 10:23 AM, Song, Yoong Siang <yoong.siang.song@xxxxxxxxx> wrote:
>On Wednesday, July 2, 2025 12:31 AM, Stanislav Fomichev <stfomichev@xxxxxxxxx>
>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@xxxxxxxxx>
>>> ---
>>> 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@xxxxxxxxx/
>
>>
>>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
