Re: [PATCH bpf-next v2] selftests/bpf: improve test coverage for kfunc call

[Hari Bathini <hbathini@linux.ibm.com>]

On 12/03/26 11:52 pm, Yonghong Song wrote:
> 
> 
> On 3/12/26 1:01 AM, Hari Bathini wrote:
>> On powerpc, immediate load instructions are sign extended. In case
>> of unsigned types, arguments should be explicitly zero-extended by
>> the caller. For kfunc call, this needs to be handled in the JIT code.
>> In bpf_kfunc_call_test4(), that tests for sign-extension of signed
>> argument types in kfunc calls, add some additional failure checks.
>> And add bpf_kfunc_call_test5() to test zero-extension of unsigned
>> argument types in kfunc calls.
>>
>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> 
> LGTM with a nit below.
> 
> Acked-by: Yonghong Song <yonghong.song@linux.dev>

Thanks for the review, Yonghong.

> 
>> ---
>>
>> Changes in v2:
>> - Added asm version of the selftest for consistent testing across
>>    different BPF ISA versions.
>> - Added comments clearly stating the intent of the test cases.
>> - Updated sign-extension selftest to have additional failure checks.
>>
>>
>>   .../selftests/bpf/prog_tests/kfunc_call.c     |  2 +
>>   .../selftests/bpf/progs/kfunc_call_test.c     | 98 +++++++++++++++++++
>>   .../selftests/bpf/test_kmods/bpf_testmod.c    | 54 +++++++++-
>>   .../bpf/test_kmods/bpf_testmod_kfunc.h        |  1 +
>>   4 files changed, 154 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c b/ 
>> tools/testing/selftests/bpf/prog_tests/kfunc_call.c
>> index f79c8e53cb3e..62f3fb79f5d1 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
>> @@ -74,6 +74,8 @@ static struct kfunc_test_params kfunc_tests[] = {
>>       TC_TEST(kfunc_call_test1, 12),
>>       TC_TEST(kfunc_call_test2, 3),
>>       TC_TEST(kfunc_call_test4, -1234),
>> +    TC_TEST(kfunc_call_test5, 0),
>> +    TC_TEST(kfunc_call_test5_asm, 0),
>>       TC_TEST(kfunc_call_test_ref_btf_id, 0),
>>       TC_TEST(kfunc_call_test_get_mem, 42),
>>       SYSCALL_TEST(kfunc_syscall_test, 0),
>> diff --git a/tools/testing/selftests/bpf/progs/kfunc_call_test.c b/ 
>> tools/testing/selftests/bpf/progs/kfunc_call_test.c
>> index 8b86113a0126..5edc51564f71 100644
>> --- a/tools/testing/selftests/bpf/progs/kfunc_call_test.c
>> +++ b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
>> @@ -2,8 +2,106 @@
>>   /* Copyright (c) 2021 Facebook */
>>   #include <vmlinux.h>
>>   #include <bpf/bpf_helpers.h>
>> +#include "bpf_misc.h"
>>   #include "../test_kmods/bpf_testmod_kfunc.h"
>> +SEC("tc")
>> +int kfunc_call_test5(struct __sk_buff *skb)
>> +{
>> +    struct bpf_sock *sk = skb->sk;
>> +    int ret;
>> +    u32 val32;
>> +    u16 val16;
>> +    u8 val8;
>> +
>> +    if (!sk)
>> +        return -1;
>> +
>> +    sk = bpf_sk_fullsock(sk);
>> +    if (!sk)
>> +        return -1;
>> +
>> +    /*
>> +     * Test with constant values to verify zero-extension.
>> +     * ISA-dependent BPF asm:
>> +     *   With ALU32:    w1 = 0xFF; w2 = 0xFFFF; w3 = 0xFFFFffff
>> +     *   Without ALU32: r1 = 0xFF; r2 = 0xFFFF; r3 = 0xFFFFffff
>> +     * Both zero-extend to 64-bit before the kfunc call.
>> +     */
>> +    ret = bpf_kfunc_call_test5(0xFF, 0xFFFF, 0xFFFFffffULL);
> 
> Can we just use 0xFFFFffff instead of 0xFFFFffffULL?

Alexei, can you confirm if I need to respin with this change?

- Hari