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 <[email protected]> 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
