All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 1/2] bpf: Fix an issue in bpf_prog_test_run_xdp when page size greater than 4K
@ 2025-06-08 16:55 Yonghong Song
  2025-06-08 16:55 ` [PATCH bpf-next 2/2] selftests/bpf: Fix two net related test failures with 64K page size Yonghong Song
  2025-06-09 22:45 ` [PATCH bpf-next 1/2] bpf: Fix an issue in bpf_prog_test_run_xdp when page size greater than 4K Stanislav Fomichev
  0 siblings, 2 replies; 6+ messages in thread
From: Yonghong Song @ 2025-06-08 16:55 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
	Martin KaFai Lau

The bpf selftest xdp_adjust_tail/xdp_adjust_frags_tail_grow failed on
arm64 with 64KB page:
   xdp_adjust_tail/xdp_adjust_frags_tail_grow:FAIL

In bpf_prog_test_run_xdp(), the xdp->frame_sz is set to 4K, but later on
when constructing frags, with 64K page size, the frag data_len could
be more than 4K. This will cause problems in bpf_xdp_frags_increase_tail().

Limiting the data_len to be 4K for each frag fixed the above test failure.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 net/bpf/test_run.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index aaf13a7d58ed..5529ec007954 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -1214,6 +1214,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
 	u32 repeat = kattr->test.repeat;
 	struct netdev_rx_queue *rxqueue;
 	struct skb_shared_info *sinfo;
+	const u32 frame_sz = 4096;
 	struct xdp_buff xdp = {};
 	int i, ret = -EINVAL;
 	struct xdp_md *ctx;
@@ -1255,7 +1256,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
 		headroom -= ctx->data;
 	}
 
-	max_data_sz = 4096 - headroom - tailroom;
+	max_data_sz = frame_sz - headroom - tailroom;
 	if (size > max_data_sz) {
 		/* disallow live data mode for jumbo frames */
 		if (do_live)
@@ -1301,7 +1302,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
 			frag = &sinfo->frags[sinfo->nr_frags++];
 
 			data_len = min_t(u32, kattr->test.data_size_in - size,
-					 PAGE_SIZE);
+					 frame_sz);
 			skb_frag_fill_page_desc(frag, page, 0, data_len);
 
 			if (copy_from_user(page_address(page), data_in + size,
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH bpf-next 2/2] selftests/bpf: Fix two net related test failures with 64K page size
  2025-06-08 16:55 [PATCH bpf-next 1/2] bpf: Fix an issue in bpf_prog_test_run_xdp when page size greater than 4K Yonghong Song
@ 2025-06-08 16:55 ` Yonghong Song
  2025-06-09 22:55   ` Stanislav Fomichev
  2025-06-09 22:45 ` [PATCH bpf-next 1/2] bpf: Fix an issue in bpf_prog_test_run_xdp when page size greater than 4K Stanislav Fomichev
  1 sibling, 1 reply; 6+ messages in thread
From: Yonghong Song @ 2025-06-08 16:55 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
	Martin KaFai Lau

When running BPF selftests on arm64 with a 64K page size, I encountered
the following two test failures:
  sockmap_basic/sockmap skb_verdict change tail:FAIL
  tc_change_tail:FAIL

With further debugging, I identified the root cause in the following
kernel code within __bpf_skb_change_tail():

    u32 max_len = BPF_SKB_MAX_LEN;
    u32 min_len = __bpf_skb_min_len(skb);
    int ret;

    if (unlikely(flags || new_len > max_len || new_len < min_len))
        return -EINVAL;

With a 4K page size, new_len = 65535 and max_len = 16064, the function
returns -EINVAL. However, With a 64K page size, max_len increases to
261824, allowing execution to proceed further in the function. This is
because BPF_SKB_MAX_LEN scales with the page size and larger page sizes
result in higher max_len values.

Updating the new_len parameter in both tests from 65535 to 262143 (0x3ffff)
resolves the failures.

Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
---
 tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c | 2 +-
 tools/testing/selftests/bpf/progs/test_tc_change_tail.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c b/tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c
index 2796dd8545eb..4f7f08364c75 100644
--- a/tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c
+++ b/tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c
@@ -31,7 +31,7 @@ int prog_skb_verdict(struct __sk_buff *skb)
 		change_tail_ret = bpf_skb_change_tail(skb, skb->len + 1, 0);
 		return SK_PASS;
 	} else if (data[0] == 'E') { /* Error */
-		change_tail_ret = bpf_skb_change_tail(skb, 65535, 0);
+		change_tail_ret = bpf_skb_change_tail(skb, 262143, 0);
 		return SK_PASS;
 	}
 	return SK_PASS;
diff --git a/tools/testing/selftests/bpf/progs/test_tc_change_tail.c b/tools/testing/selftests/bpf/progs/test_tc_change_tail.c
index 28edafe803f0..b1057fda58a0 100644
--- a/tools/testing/selftests/bpf/progs/test_tc_change_tail.c
+++ b/tools/testing/selftests/bpf/progs/test_tc_change_tail.c
@@ -94,7 +94,7 @@ int change_tail(struct __sk_buff *skb)
 			bpf_skb_change_tail(skb, len, 0);
 		return TCX_PASS;
 	} else if (payload[0] == 'E') { /* Error */
-		change_tail_ret = bpf_skb_change_tail(skb, 65535, 0);
+		change_tail_ret = bpf_skb_change_tail(skb, 262143, 0);
 		return TCX_PASS;
 	} else if (payload[0] == 'Z') { /* Zero */
 		change_tail_ret = bpf_skb_change_tail(skb, 0, 0);
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next 1/2] bpf: Fix an issue in bpf_prog_test_run_xdp when page size greater than 4K
  2025-06-08 16:55 [PATCH bpf-next 1/2] bpf: Fix an issue in bpf_prog_test_run_xdp when page size greater than 4K Yonghong Song
  2025-06-08 16:55 ` [PATCH bpf-next 2/2] selftests/bpf: Fix two net related test failures with 64K page size Yonghong Song
@ 2025-06-09 22:45 ` Stanislav Fomichev
  2025-06-10  1:31   ` Yonghong Song
  1 sibling, 1 reply; 6+ messages in thread
From: Stanislav Fomichev @ 2025-06-09 22:45 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	kernel-team, Martin KaFai Lau

On 06/08, Yonghong Song wrote:
> The bpf selftest xdp_adjust_tail/xdp_adjust_frags_tail_grow failed on
> arm64 with 64KB page:
>    xdp_adjust_tail/xdp_adjust_frags_tail_grow:FAIL
> 
> In bpf_prog_test_run_xdp(), the xdp->frame_sz is set to 4K, but later on
> when constructing frags, with 64K page size, the frag data_len could
> be more than 4K. This will cause problems in bpf_xdp_frags_increase_tail().
> 
> Limiting the data_len to be 4K for each frag fixed the above test failure.
> 
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
>  net/bpf/test_run.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index aaf13a7d58ed..5529ec007954 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -1214,6 +1214,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
>  	u32 repeat = kattr->test.repeat;
>  	struct netdev_rx_queue *rxqueue;
>  	struct skb_shared_info *sinfo;
> +	const u32 frame_sz = 4096;
>  	struct xdp_buff xdp = {};
>  	int i, ret = -EINVAL;
>  	struct xdp_md *ctx;
> @@ -1255,7 +1256,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
>  		headroom -= ctx->data;
>  	}
>  

[..]

> -	max_data_sz = 4096 - headroom - tailroom;
> +	max_data_sz = frame_sz - headroom - tailroom;

I wonder whether we should do s/4096/PAGE_SIZE/ here instead. Have you
tried that? If we are on a 64K page arch, we should not try to preserve
4K page limits.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next 2/2] selftests/bpf: Fix two net related test failures with 64K page size
  2025-06-08 16:55 ` [PATCH bpf-next 2/2] selftests/bpf: Fix two net related test failures with 64K page size Yonghong Song
@ 2025-06-09 22:55   ` Stanislav Fomichev
  2025-06-10  1:24     ` Yonghong Song
  0 siblings, 1 reply; 6+ messages in thread
From: Stanislav Fomichev @ 2025-06-09 22:55 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	kernel-team, Martin KaFai Lau

On 06/08, Yonghong Song wrote:
> When running BPF selftests on arm64 with a 64K page size, I encountered
> the following two test failures:
>   sockmap_basic/sockmap skb_verdict change tail:FAIL
>   tc_change_tail:FAIL
> 
> With further debugging, I identified the root cause in the following
> kernel code within __bpf_skb_change_tail():
> 
>     u32 max_len = BPF_SKB_MAX_LEN;
>     u32 min_len = __bpf_skb_min_len(skb);
>     int ret;
> 
>     if (unlikely(flags || new_len > max_len || new_len < min_len))
>         return -EINVAL;
> 
> With a 4K page size, new_len = 65535 and max_len = 16064, the function
> returns -EINVAL. However, With a 64K page size, max_len increases to
> 261824, allowing execution to proceed further in the function. This is
> because BPF_SKB_MAX_LEN scales with the page size and larger page sizes
> result in higher max_len values.
> 
> Updating the new_len parameter in both tests from 65535 to 262143 (0x3ffff)
> resolves the failures.
> 
> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
> ---
>  tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c | 2 +-
>  tools/testing/selftests/bpf/progs/test_tc_change_tail.c      | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c b/tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c
> index 2796dd8545eb..4f7f08364c75 100644
> --- a/tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c
> +++ b/tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c
> @@ -31,7 +31,7 @@ int prog_skb_verdict(struct __sk_buff *skb)
>  		change_tail_ret = bpf_skb_change_tail(skb, skb->len + 1, 0);
>  		return SK_PASS;
>  	} else if (data[0] == 'E') { /* Error */
> -		change_tail_ret = bpf_skb_change_tail(skb, 65535, 0);
> +		change_tail_ret = bpf_skb_change_tail(skb, 262143, 0);
>  		return SK_PASS;
>  	}
>  	return SK_PASS;
> diff --git a/tools/testing/selftests/bpf/progs/test_tc_change_tail.c b/tools/testing/selftests/bpf/progs/test_tc_change_tail.c
> index 28edafe803f0..b1057fda58a0 100644
> --- a/tools/testing/selftests/bpf/progs/test_tc_change_tail.c
> +++ b/tools/testing/selftests/bpf/progs/test_tc_change_tail.c
> @@ -94,7 +94,7 @@ int change_tail(struct __sk_buff *skb)
>  			bpf_skb_change_tail(skb, len, 0);
>  		return TCX_PASS;
>  	} else if (payload[0] == 'E') { /* Error */
> -		change_tail_ret = bpf_skb_change_tail(skb, 65535, 0);
> +		change_tail_ret = bpf_skb_change_tail(skb, 262143, 0);
>  		return TCX_PASS;
>  	} else if (payload[0] == 'Z') { /* Zero */
>  		change_tail_ret = bpf_skb_change_tail(skb, 0, 0);

nit: this seems to be exercising BPF_SKB_MAX_LEN case. To make it easier to
spot in the future, can we do the following in both tests?

#define PAGE_SIZE 65536 /* make it work on 64K page arches */
#define BPF_SKB_MAX_LEN (PAGE_SIZE << 2)

... = bpf_skb_change_tail(skb, BPF_SKB_MAX_LEN, 0);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next 2/2] selftests/bpf: Fix two net related test failures with 64K page size
  2025-06-09 22:55   ` Stanislav Fomichev
@ 2025-06-10  1:24     ` Yonghong Song
  0 siblings, 0 replies; 6+ messages in thread
From: Yonghong Song @ 2025-06-10  1:24 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	kernel-team, Martin KaFai Lau



On 6/9/25 3:55 PM, Stanislav Fomichev wrote:
> On 06/08, Yonghong Song wrote:
>> When running BPF selftests on arm64 with a 64K page size, I encountered
>> the following two test failures:
>>    sockmap_basic/sockmap skb_verdict change tail:FAIL
>>    tc_change_tail:FAIL
>>
>> With further debugging, I identified the root cause in the following
>> kernel code within __bpf_skb_change_tail():
>>
>>      u32 max_len = BPF_SKB_MAX_LEN;
>>      u32 min_len = __bpf_skb_min_len(skb);
>>      int ret;
>>
>>      if (unlikely(flags || new_len > max_len || new_len < min_len))
>>          return -EINVAL;
>>
>> With a 4K page size, new_len = 65535 and max_len = 16064, the function
>> returns -EINVAL. However, With a 64K page size, max_len increases to
>> 261824, allowing execution to proceed further in the function. This is
>> because BPF_SKB_MAX_LEN scales with the page size and larger page sizes
>> result in higher max_len values.
>>
>> Updating the new_len parameter in both tests from 65535 to 262143 (0x3ffff)
>> resolves the failures.
>>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c | 2 +-
>>   tools/testing/selftests/bpf/progs/test_tc_change_tail.c      | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c b/tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c
>> index 2796dd8545eb..4f7f08364c75 100644
>> --- a/tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c
>> +++ b/tools/testing/selftests/bpf/progs/test_sockmap_change_tail.c
>> @@ -31,7 +31,7 @@ int prog_skb_verdict(struct __sk_buff *skb)
>>   		change_tail_ret = bpf_skb_change_tail(skb, skb->len + 1, 0);
>>   		return SK_PASS;
>>   	} else if (data[0] == 'E') { /* Error */
>> -		change_tail_ret = bpf_skb_change_tail(skb, 65535, 0);
>> +		change_tail_ret = bpf_skb_change_tail(skb, 262143, 0);
>>   		return SK_PASS;
>>   	}
>>   	return SK_PASS;
>> diff --git a/tools/testing/selftests/bpf/progs/test_tc_change_tail.c b/tools/testing/selftests/bpf/progs/test_tc_change_tail.c
>> index 28edafe803f0..b1057fda58a0 100644
>> --- a/tools/testing/selftests/bpf/progs/test_tc_change_tail.c
>> +++ b/tools/testing/selftests/bpf/progs/test_tc_change_tail.c
>> @@ -94,7 +94,7 @@ int change_tail(struct __sk_buff *skb)
>>   			bpf_skb_change_tail(skb, len, 0);
>>   		return TCX_PASS;
>>   	} else if (payload[0] == 'E') { /* Error */
>> -		change_tail_ret = bpf_skb_change_tail(skb, 65535, 0);
>> +		change_tail_ret = bpf_skb_change_tail(skb, 262143, 0);
>>   		return TCX_PASS;
>>   	} else if (payload[0] == 'Z') { /* Zero */
>>   		change_tail_ret = bpf_skb_change_tail(skb, 0, 0);
> nit: this seems to be exercising BPF_SKB_MAX_LEN case. To make it easier to
> spot in the future, can we do the following in both tests?
>
> #define PAGE_SIZE 65536 /* make it work on 64K page arches */
> #define BPF_SKB_MAX_LEN (PAGE_SIZE << 2)
>
> ... = bpf_skb_change_tail(skb, BPF_SKB_MAX_LEN, 0);

Thanks for suggestions, Will do.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf-next 1/2] bpf: Fix an issue in bpf_prog_test_run_xdp when page size greater than 4K
  2025-06-09 22:45 ` [PATCH bpf-next 1/2] bpf: Fix an issue in bpf_prog_test_run_xdp when page size greater than 4K Stanislav Fomichev
@ 2025-06-10  1:31   ` Yonghong Song
  0 siblings, 0 replies; 6+ messages in thread
From: Yonghong Song @ 2025-06-10  1:31 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	kernel-team, Martin KaFai Lau



On 6/9/25 3:45 PM, Stanislav Fomichev wrote:
> On 06/08, Yonghong Song wrote:
>> The bpf selftest xdp_adjust_tail/xdp_adjust_frags_tail_grow failed on
>> arm64 with 64KB page:
>>     xdp_adjust_tail/xdp_adjust_frags_tail_grow:FAIL
>>
>> In bpf_prog_test_run_xdp(), the xdp->frame_sz is set to 4K, but later on
>> when constructing frags, with 64K page size, the frag data_len could
>> be more than 4K. This will cause problems in bpf_xdp_frags_increase_tail().
>>
>> Limiting the data_len to be 4K for each frag fixed the above test failure.
>>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   net/bpf/test_run.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
>> index aaf13a7d58ed..5529ec007954 100644
>> --- a/net/bpf/test_run.c
>> +++ b/net/bpf/test_run.c
>> @@ -1214,6 +1214,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
>>   	u32 repeat = kattr->test.repeat;
>>   	struct netdev_rx_queue *rxqueue;
>>   	struct skb_shared_info *sinfo;
>> +	const u32 frame_sz = 4096;
>>   	struct xdp_buff xdp = {};
>>   	int i, ret = -EINVAL;
>>   	struct xdp_md *ctx;
>> @@ -1255,7 +1256,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
>>   		headroom -= ctx->data;
>>   	}
>>   
> [..]
>
>> -	max_data_sz = 4096 - headroom - tailroom;
>> +	max_data_sz = frame_sz - headroom - tailroom;
> I wonder whether we should do s/4096/PAGE_SIZE/ here instead. Have you
> tried that? If we are on a 64K page arch, we should not try to preserve
> 4K page limits.

The user space test_run input looks like below (in prog_tests/xdp_adjust_tail.c):

	buf = malloc(16384);
	...
	topts.data_in = buf;
	topts.data_out = buf;
	topts.data_size_in = 9000;
	topts.data_size_out = 16384;
	err = bpf_prog_test_run_opts(prog_fd, &topts);

Allowing s/4096/PAGE_SIZE (64K) will have the test failure for the above
user space input. I think I can increse buf size data_size_in/data_size_out
properly so in the kernel we can do s/4096/PAGE_SIZE.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-06-10  1:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-08 16:55 [PATCH bpf-next 1/2] bpf: Fix an issue in bpf_prog_test_run_xdp when page size greater than 4K Yonghong Song
2025-06-08 16:55 ` [PATCH bpf-next 2/2] selftests/bpf: Fix two net related test failures with 64K page size Yonghong Song
2025-06-09 22:55   ` Stanislav Fomichev
2025-06-10  1:24     ` Yonghong Song
2025-06-09 22:45 ` [PATCH bpf-next 1/2] bpf: Fix an issue in bpf_prog_test_run_xdp when page size greater than 4K Stanislav Fomichev
2025-06-10  1:31   ` Yonghong Song

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.