Re: [PATCH bpf-next 2/2] selftests/bpf: Fix two net related test failures with 64K page size

[Yonghong Song <yonghong.song@linux.dev>]

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.