Re: [PATCH bpf-next v3 4/6] bpf: Support specifying linear xdp packet data size for BPF_PROG_TEST_RUN

[Martin KaFai Lau <martin.lau@linux.dev>]

On 9/15/25 3:47 PM, Amery Hung wrote:
> To test bpf_xdp_pull_data(), an xdp packet containing fragments as well
> as free linear data area after xdp->data_end needs to be created.
> However, bpf_prog_test_run_xdp() always fills the linear area with
> data_in before creating fragments, leaving no space to pull data. This
> patch will allow users to specify the linear data size through
> ctx->data_end.
> 
> Currently, ctx_in->data_end must match data_size_in and will not be the
> final ctx->data_end seen by xdp programs. This is because ctx->data_end
> is populated according to the xdp_buff passed to test_run. The linear
> data area available in an xdp_buff, max_data_sz, is alawys filled up
> before copying data_in into fragments.
> 
> This patch will allow users to specify the size of data that goes into
> the linear area. When ctx_in->data_end is different from data_size_in,
> only ctx_in->data_end bytes of data will be put into the linear area when
> creating the xdp_buff.
> 
> While ctx_in->data_end will be allowed to be different from data_size_in,
> it cannot be larger than the data_size_in as there will be no data to
> copy from user space. If it is larger than the maximum linear data area
> size, the layout suggested by the user will not be honored. Data beyond
> max_data_sz bytes will still be copied into fragments.
> 
> Finally, since it is possible for a NIC to produce a xdp_buff with empty
> linear data area, allow it when calling bpf_test_init() from
> bpf_prog_test_run_xdp() so that we can test XDP kfuncs with such
> xdp_buff.
> 
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---
>   net/bpf/test_run.c                            | 26 ++++++++++++-------
>   .../bpf/prog_tests/xdp_context_test_run.c     |  4 +--
>   2 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 4a862d605386..558126bbd180 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -660,12 +660,15 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_memb_release, KF_RELEASE)
>   BTF_KFUNCS_END(test_sk_check_kfunc_ids)
>   
>   static void *bpf_test_init(const union bpf_attr *kattr, u32 user_size,
> -			   u32 size, u32 headroom, u32 tailroom)
> +			   u32 size, u32 headroom, u32 tailroom, bool is_xdp)

Understood that the patch has inherited this function. I found it hard to read 
when it is called by xdp but this could be just me. For example, what is passed 
as "size" from the bpf_prog_test_run_xdp(), which ends up being "PAGE_SIZE - 
headroom - tailroom". I am not sure how to fix it. e.g. can we always allocate a 
PAGE_SIZE for non xdp callers also. or may be the xdp should not reuse this 
function. This probably is a fruit of thoughts for later. Not asking to consider 
it in this set.

I think at least the first step is to avoid adding "is_xdp" specific logic here.

>   {
>   	void __user *data_in = u64_to_user_ptr(kattr->test.data_in);
>   	void *data;
>   
> -	if (user_size < ETH_HLEN || user_size > PAGE_SIZE - headroom - tailroom)
> +	if (!is_xdp && user_size < ETH_HLEN)

Move the lower bound check to its caller. test_run_xdp() does not need this 
check. test_run_flow_dissector() and test_run_nf() already have its own check. 
test_run_nf() actually has a different bound. test_run_skb() is the only one 
that needs this check, so it can be explicitly done in there like other callers.

> +		return ERR_PTR(-EINVAL);
> +
> +	if (user_size > PAGE_SIZE - headroom - tailroom)
>   		return ERR_PTR(-EINVAL);
>   
>   	size = SKB_DATA_ALIGN(size);
> @@ -1003,7 +1006,8 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
>   
>   	data = bpf_test_init(kattr, kattr->test.data_size_in,
>   			     size, NET_SKB_PAD + NET_IP_ALIGN,
> -			     SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
> +			     SKB_DATA_ALIGN(sizeof(struct skb_shared_info)),
> +			     false);
>   	if (IS_ERR(data))
>   		return PTR_ERR(data);
>   
> @@ -1207,8 +1211,8 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
>   {
>   	bool do_live = (kattr->test.flags & BPF_F_TEST_XDP_LIVE_FRAMES);
>   	u32 tailroom = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> +	u32 retval = 0, duration, max_data_sz, data_sz;
>   	u32 batch_size = kattr->test.batch_size;
> -	u32 retval = 0, duration, max_data_sz;
>   	u32 size = kattr->test.data_size_in;
>   	u32 headroom = XDP_PACKET_HEADROOM;
>   	u32 repeat = kattr->test.repeat;
> @@ -1246,7 +1250,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
>   
>   	if (ctx) {
>   		/* There can't be user provided data before the meta data */
> -		if (ctx->data_meta || ctx->data_end != size ||
> +		if (ctx->data_meta || ctx->data_end > size ||
>   		    ctx->data > ctx->data_end ||
>   		    unlikely(xdp_metalen_invalid(ctx->data)) ||
>   		    (do_live && (kattr->test.data_out || kattr->test.ctx_out)))
> @@ -1256,14 +1260,15 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
>   	}
>   
>   	max_data_sz = PAGE_SIZE - headroom - tailroom;
> -	if (size > max_data_sz) {
> +	data_sz = (ctx && ctx->data_end < max_data_sz) ? ctx->data_end : max_data_sz;

hmm... can the "size" (not data_sz) be directly updated to ctx->data_end in the 
above "if (ctx)".

> +	if (size > data_sz) {
>   		/* disallow live data mode for jumbo frames */
>   		if (do_live)
>   			goto free_ctx;
> -		size = max_data_sz;
> +		size = data_sz;
>   	}
>   
> -	data = bpf_test_init(kattr, size, max_data_sz, headroom, tailroom);
> +	data = bpf_test_init(kattr, size, max_data_sz, headroom, tailroom, true);
>   	if (IS_ERR(data)) {
>   		ret = PTR_ERR(data);
>   		goto free_ctx;
> @@ -1386,7 +1391,7 @@ int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
>   	if (size < ETH_HLEN)
>   		return -EINVAL;
>   
> -	data = bpf_test_init(kattr, kattr->test.data_size_in, size, 0, 0);
> +	data = bpf_test_init(kattr, kattr->test.data_size_in, size, 0, 0, false);
>   	if (IS_ERR(data))
>   		return PTR_ERR(data);
>   
> @@ -1659,7 +1664,8 @@ int bpf_prog_test_run_nf(struct bpf_prog *prog,
>   
>   	data = bpf_test_init(kattr, kattr->test.data_size_in, size,
>   			     NET_SKB_PAD + NET_IP_ALIGN,
> -			     SKB_DATA_ALIGN(sizeof(struct skb_shared_info)));
> +			     SKB_DATA_ALIGN(sizeof(struct skb_shared_info)),
> +			     false);
>   	if (IS_ERR(data))
>   		return PTR_ERR(data);
>   
> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c
> index 46e0730174ed..178292d1251a 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_context_test_run.c
> @@ -97,9 +97,7 @@ void test_xdp_context_test_run(void)
>   	/* Meta data must be 255 bytes or smaller */
>   	test_xdp_context_error(prog_fd, opts, 0, 256, sizeof(data), 0, 0, 0);
>   
> -	/* Total size of data must match data_end - data_meta */
> -	test_xdp_context_error(prog_fd, opts, 0, sizeof(__u32),
> -			       sizeof(data) - 1, 0, 0, 0);
> +	/* Total size of data must be data_end - data_meta or larger */
>   	test_xdp_context_error(prog_fd, opts, 0, sizeof(__u32),
>   			       sizeof(data) + 1, 0, 0, 0);
>