[PATCH] selftests/bpf: improve test coverage for kfunc call

[PATCH] selftests/bpf: improve test coverage for kfunc call

[Hari Bathini]

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.
While kfunc_call_test4 test case already checks for sign-extension of
signed argument types in kfunc calls, zero-extension for unsigned
argument types is being checked with this test case.

Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---

- powerpc BPF JIT was not handling ABI sign-extension & zero-extension
  appropriately for kfunc calls. Fixed with:

    https://lore.kernel.org/all/20260303130208.325249-7-hbathini@linux.ibm.com/


 .../selftests/bpf/prog_tests/kfunc_call.c     |  1 +
 .../selftests/bpf/progs/kfunc_call_test.c     | 34 +++++++++++++++++++
 .../selftests/bpf/test_kmods/bpf_testmod.c    | 28 +++++++++++++++
 .../bpf/test_kmods/bpf_testmod_kfunc.h        |  1 +
 4 files changed, 64 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
index f79c8e53cb3e..fb06f2485197 100644
--- a/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
+++ b/tools/testing/selftests/bpf/prog_tests/kfunc_call.c
@@ -74,6 +74,7 @@ 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_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..a32c3a60fa4f 100644
--- a/tools/testing/selftests/bpf/progs/kfunc_call_test.c
+++ b/tools/testing/selftests/bpf/progs/kfunc_call_test.c
@@ -4,6 +4,40 @@
 #include <bpf/bpf_helpers.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;
+
+	ret = bpf_kfunc_call_test5(0xFF, 0xFFFF, 0xFFFFFFFF);
+	if (ret)
+		return ret;
+
+	val32 = bpf_get_prandom_u32();
+	val16 = val32 & 0xFFFF;
+	val8 = val32 & 0xFF;
+	ret = bpf_kfunc_call_test5(val8, val16, val32);
+	if (ret)
+		return ret;
+
+	ret = bpf_kfunc_call_test5(val8 * 0xFF, val16 * 0xFFFF, val32 * 0xFFFFFFFF);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 SEC("tc")
 int kfunc_call_test4(struct __sk_buff *skb)
 {
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
index e62c6b78657f..de4897ddcff1 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
@@ -766,6 +766,33 @@ __bpf_kfunc long noinline bpf_kfunc_call_test4(signed char a, short b, int c, lo
 	return (long)a + (long)b + (long)c + d;
 }
 
+__bpf_kfunc int bpf_kfunc_call_test5(u8 a, u16 b, u32 c)
+{
+	/* Make val as volatile to avoid compiler optimizations on the below checks */
+	volatile long val = a;
+
+	/* Check zero-extension */
+	if (val != (unsigned long)a)
+		return 1;
+	/* Check no sign-extension */
+	if (val < 0)
+		return 2;
+
+	val = b;
+	if (val != (unsigned long)b)
+		return 3;
+	if (val < 0)
+		return 4;
+
+	val = c;
+	if (val != (unsigned long)c)
+		return 5;
+	if (val < 0)
+		return 6;
+
+	return 0;
+}
+
 static struct prog_test_ref_kfunc prog_test_struct = {
 	.a = 42,
 	.b = 108,
@@ -1228,6 +1255,7 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test1)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test2)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test3)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test4)
+BTF_ID_FLAGS(func, bpf_kfunc_call_test5)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_pass1)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail1)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_mem_len_fail2)
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
index b393bf771131..aa0b8d41e71b 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
@@ -110,6 +110,7 @@ __u64 bpf_kfunc_call_test1(struct sock *sk, __u32 a, __u64 b,
 int bpf_kfunc_call_test2(struct sock *sk, __u32 a, __u32 b) __ksym;
 struct sock *bpf_kfunc_call_test3(struct sock *sk) __ksym;
 long bpf_kfunc_call_test4(signed char a, short b, int c, long d) __ksym;
+int bpf_kfunc_call_test5(__u8 a, __u16 b, __u32 c) __ksym;
 
 void bpf_kfunc_call_test_pass_ctx(struct __sk_buff *skb) __ksym;
 void bpf_kfunc_call_test_pass1(struct prog_test_pass1 *p) __ksym;
-- 
2.53.0

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

[Alexei Starovoitov]

On Tue, Mar 3, 2026 at 5:15 AM Hari Bathini <hbathini@linux.ibm.com> wrote:
>
>
> +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;
> +
> +       ret = bpf_kfunc_call_test5(0xFF, 0xFFFF, 0xFFFFFFFF);

maybe add a comment with bpf asm to highlight what this is ?

Also 0xFFFFffffULL ?
8 "F"s in a row is harder on the eyes.
and ULL to make it explicit ?

> +       if (ret)
> +               return ret;
> +
> +       val32 = bpf_get_prandom_u32();
> +       val16 = val32 & 0xFFFF;
> +       val8 = val32 & 0xFF;
> +       ret = bpf_kfunc_call_test5(val8, val16, val32);
> +       if (ret)
> +               return ret;
> +
> +       ret = bpf_kfunc_call_test5(val8 * 0xFF, val16 * 0xFFFF, val32 * 0xFFFFFFFF);

I'm struggling to decipher it. Pls add a comment with asm to explain.
I think the last multiplication is still done in 32-bit domain ?
or not? 0xFFFFFFFF is a 64-bit constant in C. I think...

Also we have 4 ISA versions. test_progs-no_alu32 and test_progs
compile it differently.
Maybe let's add another version of this test but fully in asm ?
Keep the C version too.

> +       if (ret)
> +               return ret;
> +
> +       return 0;
> +}
> +
>  SEC("tc")
>  int kfunc_call_test4(struct __sk_buff *skb)
>  {
> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> index e62c6b78657f..de4897ddcff1 100644
> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> @@ -766,6 +766,33 @@ __bpf_kfunc long noinline bpf_kfunc_call_test4(signed char a, short b, int c, lo
>         return (long)a + (long)b + (long)c + d;
>  }
>
> +__bpf_kfunc int bpf_kfunc_call_test5(u8 a, u16 b, u32 c)
> +{
> +       /* Make val as volatile to avoid compiler optimizations on the below checks */
> +       volatile long val = a;

Pls add a comment that this is zero extended in C.

> +
> +       /* Check zero-extension */
> +       if (val != (unsigned long)a)
> +               return 1;
> +       /* Check no sign-extension */
> +       if (val < 0)
> +               return 2;
> +
> +       val = b;
> +       if (val != (unsigned long)b)
> +               return 3;
> +       if (val < 0)
> +               return 4;
> +
> +       val = c;
> +       if (val != (unsigned long)c)
> +               return 5;
> +       if (val < 0)
> +               return 6;
> +
> +       return 0;
> +}

Overall this looks very useful.
I would expand with another test where a,b,c are s8,s16,s32.

Please resend with [PATCH bpf-next] in the subject, so that CIs
can pick it up correctly.

pw-bot: cr

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

[Hari Bathini]

Hi Alexei,

Thanks for the review.

On 09/03/26 10:37 pm, Alexei Starovoitov wrote:
> On Tue, Mar 3, 2026 at 5:15 AM Hari Bathini <hbathini@linux.ibm.com> wrote:
>>
>>
>> +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;
>> +
>> +       ret = bpf_kfunc_call_test5(0xFF, 0xFFFF, 0xFFFFFFFF);
> 
> maybe add a comment with bpf asm to highlight what this is ?
> 
> Also 0xFFFFffffULL ?
> 8 "F"s in a row is harder on the eyes.
> and ULL to make it explicit ?

True. Will do that.

>> +       if (ret)
>> +               return ret;
>> +
>> +       val32 = bpf_get_prandom_u32();
>> +       val16 = val32 & 0xFFFF;
>> +       val8 = val32 & 0xFF;
>> +       ret = bpf_kfunc_call_test5(val8, val16, val32);
>> +       if (ret)
>> +               return ret;
>> +
>> +       ret = bpf_kfunc_call_test5(val8 * 0xFF, val16 * 0xFFFF, val32 * 0xFFFFFFFF);
> 
> I'm struggling to decipher it. Pls add a comment with asm to explain.
> I think the last multiplication is still done in 32-bit domain ?
> or not? 0xFFFFFFFF is a 64-bit constant in C. I think...

Sure. Let me add comments to convey the intention of the bpf programs
to avoid ambiguity..

> 
> Also we have 4 ISA versions. test_progs-no_alu32 and test_progs
> compile it differently.
> Maybe let's add another version of this test but fully in asm ?
> Keep the C version too.
> 
>> +       if (ret)
>> +               return ret;
>> +
>> +       return 0;
>> +}
>> +
>>   SEC("tc")
>>   int kfunc_call_test4(struct __sk_buff *skb)
>>   {
>> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
>> index e62c6b78657f..de4897ddcff1 100644
>> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
>> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
>> @@ -766,6 +766,33 @@ __bpf_kfunc long noinline bpf_kfunc_call_test4(signed char a, short b, int c, lo
>>          return (long)a + (long)b + (long)c + d;
>>   }
>>
>> +__bpf_kfunc int bpf_kfunc_call_test5(u8 a, u16 b, u32 c)
>> +{
>> +       /* Make val as volatile to avoid compiler optimizations on the below checks */
>> +       volatile long val = a;
> 
> Pls add a comment that this is zero extended in C.
> 
>> +
>> +       /* Check zero-extension */
>> +       if (val != (unsigned long)a)
>> +               return 1;
>> +       /* Check no sign-extension */
>> +       if (val < 0)
>> +               return 2;
>> +
>> +       val = b;
>> +       if (val != (unsigned long)b)
>> +               return 3;
>> +       if (val < 0)
>> +               return 4;
>> +
>> +       val = c;
>> +       if (val != (unsigned long)c)
>> +               return 5;
>> +       if (val < 0)
>> +               return 6;
>> +
>> +       return 0;
>> +}
> 
> Overall this looks very useful.
> I would expand with another test where a,b,c are s8,s16,s32.

Slightly different approach but kfunc_call_test4/bpf_kfunc_call_test4
cover signed arguments already?

> Please resend with [PATCH bpf-next] in the subject, so that CIs
> can pick it up correctly.
My bad. Will add the suffix while sending v2..

- Hari

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

[Alexei Starovoitov]

On Wed, Mar 11, 2026 at 8:10 AM Hari Bathini <hbathini@linux.ibm.com> wrote:
>
> >
> >> +
> >> +       /* Check zero-extension */
> >> +       if (val != (unsigned long)a)
> >> +               return 1;
> >> +       /* Check no sign-extension */
> >> +       if (val < 0)
> >> +               return 2;
> >> +
> >> +       val = b;
> >> +       if (val != (unsigned long)b)
> >> +               return 3;
> >> +       if (val < 0)
> >> +               return 4;
> >> +
> >> +       val = c;
> >> +       if (val != (unsigned long)c)
> >> +               return 5;
> >> +       if (val < 0)
> >> +               return 6;
> >> +
> >> +       return 0;
> >> +}
> >
> > Overall this looks very useful.
> > I would expand with another test where a,b,c are s8,s16,s32.
>
> Slightly different approach but kfunc_call_test4/bpf_kfunc_call_test4
> cover signed arguments already?

Ahh. Then may be tweak it to adopt similar fine grained
error reporting as your bpf_kfunc_call_test5()
or go other way around an collapse all errors the way test4 is doing it.

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

[Hari Bathini]

On 11/03/26 9:32 pm, Alexei Starovoitov wrote:
> On Wed, Mar 11, 2026 at 8:10 AM Hari Bathini <hbathini@linux.ibm.com> wrote:
>>
>>>
>>>> +
>>>> +       /* Check zero-extension */
>>>> +       if (val != (unsigned long)a)
>>>> +               return 1;
>>>> +       /* Check no sign-extension */
>>>> +       if (val < 0)
>>>> +               return 2;
>>>> +
>>>> +       val = b;
>>>> +       if (val != (unsigned long)b)
>>>> +               return 3;
>>>> +       if (val < 0)
>>>> +               return 4;
>>>> +
>>>> +       val = c;
>>>> +       if (val != (unsigned long)c)
>>>> +               return 5;
>>>> +       if (val < 0)
>>>> +               return 6;
>>>> +
>>>> +       return 0;
>>>> +}
>>>
>>> Overall this looks very useful.
>>> I would expand with another test where a,b,c are s8,s16,s32.
>>
>> Slightly different approach but kfunc_call_test4/bpf_kfunc_call_test4
>> cover signed arguments already?
> 
> Ahh. Then may be tweak it to adopt similar fine grained
> error reporting as your bpf_kfunc_call_test5()

I Prefer this.

Does the below change to bpf_kfunc_call_test4 look fine:

diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c 
b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
index 48dcaf93bb9f..6237c2222633 100644
--- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
@@ -760,8 +760,30 @@ __bpf_kfunc struct sock 
*bpf_kfunc_call_test3(struct sock *sk)

  __bpf_kfunc long noinline bpf_kfunc_call_test4(signed char a, short b, 
int c, long d)
  {
+       /*
+        * Make val as volatile to avoid compiler optimizations.
+        * Verify that negative signed values remain negative after
+        * sign-extension (JIT must sign-extend, not zero-extend).
+        */
+       volatile long val;
+
+       /* val will be positive, if JIT does zero-extension instead of 
sign-extension */
+       val = a;
+       if (val >= 0)
+               return 1;
+
+       val = b;
+       if (val >= 0)
+               return 2;
+
+       val = c;
+       if (val >= 0)
+               return 3;
+
         /* Provoke the compiler to assume that the caller has 
sign-extended a,
          * b and c on platforms where this is required (e.g. s390x).
+        *
+        * Original behavior: return sum for backward compatibility
          */
         return (long)a + (long)b + (long)c + d;
  }


- Hari

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

[Alexei Starovoitov]

On Wed, Mar 11, 2026 at 11:03 AM Hari Bathini <hbathini@linux.ibm.com> wrote:
>
>
>
> On 11/03/26 9:32 pm, Alexei Starovoitov wrote:
> > On Wed, Mar 11, 2026 at 8:10 AM Hari Bathini <hbathini@linux.ibm.com> wrote:
> >>
> >>>
> >>>> +
> >>>> +       /* Check zero-extension */
> >>>> +       if (val != (unsigned long)a)
> >>>> +               return 1;
> >>>> +       /* Check no sign-extension */
> >>>> +       if (val < 0)
> >>>> +               return 2;
> >>>> +
> >>>> +       val = b;
> >>>> +       if (val != (unsigned long)b)
> >>>> +               return 3;
> >>>> +       if (val < 0)
> >>>> +               return 4;
> >>>> +
> >>>> +       val = c;
> >>>> +       if (val != (unsigned long)c)
> >>>> +               return 5;
> >>>> +       if (val < 0)
> >>>> +               return 6;
> >>>> +
> >>>> +       return 0;
> >>>> +}
> >>>
> >>> Overall this looks very useful.
> >>> I would expand with another test where a,b,c are s8,s16,s32.
> >>
> >> Slightly different approach but kfunc_call_test4/bpf_kfunc_call_test4
> >> cover signed arguments already?
> >
> > Ahh. Then may be tweak it to adopt similar fine grained
> > error reporting as your bpf_kfunc_call_test5()
>
> I Prefer this.
>
> Does the below change to bpf_kfunc_call_test4 look fine:
>
> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> index 48dcaf93bb9f..6237c2222633 100644
> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> @@ -760,8 +760,30 @@ __bpf_kfunc struct sock
> *bpf_kfunc_call_test3(struct sock *sk)
>
>   __bpf_kfunc long noinline bpf_kfunc_call_test4(signed char a, short b,
> int c, long d)
>   {
> +       /*
> +        * Make val as volatile to avoid compiler optimizations.
> +        * Verify that negative signed values remain negative after
> +        * sign-extension (JIT must sign-extend, not zero-extend).
> +        */
> +       volatile long val;
> +
> +       /* val will be positive, if JIT does zero-extension instead of
> sign-extension */
> +       val = a;
> +       if (val >= 0)
> +               return 1;
> +
> +       val = b;
> +       if (val >= 0)
> +               return 2;
> +
> +       val = c;
> +       if (val >= 0)
> +               return 3;
> +
>          /* Provoke the compiler to assume that the caller has
> sign-extended a,
>           * b and c on platforms where this is required (e.g. s390x).

While at it reformat the comment to proper kernel style.

the rest lgtm.

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

[Vineet Gupta]

On 3/11/26 11:03 AM, Hari Bathini wrote:
>
> On 11/03/26 9:32 pm, Alexei Starovoitov wrote:
>> On Wed, Mar 11, 2026 at 8:10 AM Hari Bathini <hbathini@linux.ibm.com> 
>> wrote:
>>>
>>>>
>>>>> +
>>>>> +       /* Check zero-extension */
>>>>> +       if (val != (unsigned long)a)
>>>>> +               return 1;
>>>>> +       /* Check no sign-extension */
>>>>> +       if (val < 0)
>>>>> +               return 2;
>>>>> +
>>>>> +       val = b;
>>>>> +       if (val != (unsigned long)b)
>>>>> +               return 3;
>>>>> +       if (val < 0)
>>>>> +               return 4;
>>>>> +
>>>>> +       val = c;
>>>>> +       if (val != (unsigned long)c)
>>>>> +               return 5;
>>>>> +       if (val < 0)
>>>>> +               return 6;
>>>>> +
>>>>> +       return 0;
>>>>> +}
>>>>
>>>> Overall this looks very useful.
>>>> I would expand with another test where a,b,c are s8,s16,s32.
>>>
>>> Slightly different approach but kfunc_call_test4/bpf_kfunc_call_test4
>>> cover signed arguments already?
>>
>> Ahh. Then may be tweak it to adopt similar fine grained
>> error reporting as your bpf_kfunc_call_test5()
>
> I Prefer this.
>
> Does the below change to bpf_kfunc_call_test4 look fine:
>
> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c 
> b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> index 48dcaf93bb9f..6237c2222633 100644
> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> @@ -760,8 +760,30 @@ __bpf_kfunc struct sock 
> *bpf_kfunc_call_test3(struct sock *sk)
>
>  __bpf_kfunc long noinline bpf_kfunc_call_test4(signed char a, short 
> b, int c, long d)

I might regret for bringing this up as it could be yet another ABI 
fiasco between gcc and llvm.

As per C standard, sign of unadorned char (i.e. w/o explicit signed or 
unsigned prefix) is ABI defined.
For gcc-bpf char is specified to be signed.
So test4 has s8, while new test5 has u8. Would it make sense to have an 
additional test without signed/unsigned annotation for char ?
This will flag any discrepancy between the two compilers.

Thx,
-Vineet

>  {
> +       /*
> +        * Make val as volatile to avoid compiler optimizations.
> +        * Verify that negative signed values remain negative after
> +        * sign-extension (JIT must sign-extend, not zero-extend).
> +        */
> +       volatile long val;
> +
> +       /* val will be positive, if JIT does zero-extension instead of 
> sign-extension */
> +       val = a;
> +       if (val >= 0)
> +               return 1;
> +
> +       val = b;
> +       if (val >= 0)
> +               return 2;
> +
> +       val = c;
> +       if (val >= 0)
> +               return 3;
> +
>         /* Provoke the compiler to assume that the caller has 
> sign-extended a,
>          * b and c on platforms where this is required (e.g. s390x).
> +        *
> +        * Original behavior: return sum for backward compatibility
>          */
>         return (long)a + (long)b + (long)c + d;
>  }
>
>
> - Hari

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

[Jose E. Marchesi]

> On 3/11/26 11:03 AM, Hari Bathini wrote:
>>
>> On 11/03/26 9:32 pm, Alexei Starovoitov wrote:
>>> On Wed, Mar 11, 2026 at 8:10 AM Hari Bathini
>>> <hbathini@linux.ibm.com> wrote:
>>>>
>>>>>
>>>>>> +
>>>>>> +       /* Check zero-extension */
>>>>>> +       if (val != (unsigned long)a)
>>>>>> +               return 1;
>>>>>> +       /* Check no sign-extension */
>>>>>> +       if (val < 0)
>>>>>> +               return 2;
>>>>>> +
>>>>>> +       val = b;
>>>>>> +       if (val != (unsigned long)b)
>>>>>> +               return 3;
>>>>>> +       if (val < 0)
>>>>>> +               return 4;
>>>>>> +
>>>>>> +       val = c;
>>>>>> +       if (val != (unsigned long)c)
>>>>>> +               return 5;
>>>>>> +       if (val < 0)
>>>>>> +               return 6;
>>>>>> +
>>>>>> +       return 0;
>>>>>> +}
>>>>>
>>>>> Overall this looks very useful.
>>>>> I would expand with another test where a,b,c are s8,s16,s32.
>>>>
>>>> Slightly different approach but kfunc_call_test4/bpf_kfunc_call_test4
>>>> cover signed arguments already?
>>>
>>> Ahh. Then may be tweak it to adopt similar fine grained
>>> error reporting as your bpf_kfunc_call_test5()
>>
>> I Prefer this.
>>
>> Does the below change to bpf_kfunc_call_test4 look fine:
>>
>> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
>> b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
>> index 48dcaf93bb9f..6237c2222633 100644
>> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
>> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
>> @@ -760,8 +760,30 @@ __bpf_kfunc struct sock
>> *bpf_kfunc_call_test3(struct sock *sk)
>>
>>  __bpf_kfunc long noinline bpf_kfunc_call_test4(signed char a, short
>> b, int c, long d)
>
> I might regret for bringing this up as it could be yet another ABI
> fiasco between gcc and llvm.

char is signed in BPF in both compilers, matching x86.

> As per C standard, sign of unadorned char (i.e. w/o explicit signed or
> unsigned prefix) is ABI defined.
> For gcc-bpf char is specified to be signed.
> So test4 has s8, while new test5 has u8. Would it make sense to have
> an additional test without signed/unsigned annotation for char ?
> This will flag any discrepancy between the two compilers.
>
> Thx,
> -Vineet
>
>>  {
>> +       /*
>> +        * Make val as volatile to avoid compiler optimizations.
>> +        * Verify that negative signed values remain negative after
>> +        * sign-extension (JIT must sign-extend, not zero-extend).
>> +        */
>> +       volatile long val;
>> +
>> +       /* val will be positive, if JIT does zero-extension instead
>> of sign-extension */
>> +       val = a;
>> +       if (val >= 0)
>> +               return 1;
>> +
>> +       val = b;
>> +       if (val >= 0)
>> +               return 2;
>> +
>> +       val = c;
>> +       if (val >= 0)
>> +               return 3;
>> +
>>         /* Provoke the compiler to assume that the caller has
>> sign-extended a,
>>          * b and c on platforms where this is required (e.g. s390x).
>> +        *
>> +        * Original behavior: return sum for backward compatibility
>>          */
>>         return (long)a + (long)b + (long)c + d;
>>  }
>>
>>
>> - Hari