Re: [PATCH bpf-next v3 14/15] selftests/bpf: Add tests for by-value kfunc arguments

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

On 9/11/26 9:47 AM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c b/tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c
>> new file mode 100644
>> index 000000000000..8b0e7aed1ff1
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/progs/aggregate_arg_kfunc.c
> [ ... ]
>
>> +SEC("tc")
>> +__arch_x86_64 __arch_arm64 __load_if_JITed()
>> +__description("aggregate kfunc argument: struct at the tail")
>> +__success __retval(0)
>> +int aggregate_arg_kfunc_tail(struct __sk_buff *skb)
>> +{
>> +	u64 a = 1, b = 2;
>> +	struct prog_test_pair_arg s = { .lo = a, .hi = b };
>> +
>> +	if (bpf_kfunc_call_test_pair_arg_tail(1, 2, 3, 4, 5, s, 6) != a + b + 21)
>> +		return 1;
>> +	return 0;
>> +}
> Do these tests actually verify that arguments end up in the correct slots?
>
> Looking at the test pattern: bpf_kfunc_call_test_pair_arg_tail(1, 2, 3,
> 4, 5, s, 6) expects 'a + b + 21', but the kfunc returns the sum of all
> its arguments unweighted:
>
> tools/testing/selftests/bpf/test_kmods/bpf_testmod.c:
>      __bpf_kfunc u64 bpf_kfunc_call_test_pair_arg_tail(
>          u64 a, u64 b, u64 c, u64 d, u64 e,
>          struct prog_test_pair_arg s, u64 f)
>      {
>          return a + b + c + d + e + s.lo + s.hi + f;
>      }
>
> The check is invariant under any permutation of values across argument
> slots. It will catch a value that is lost, duplicated or garbage, but
> not a value that ends up in the wrong slot.
>
> Concrete case in this test: aggregate_arg_kfunc_tail is the one case
> where x86-64 moves an argument down into a still-live register (BPF
> slots a..e -> 0..4, s -> 6,7, f -> 5, so f travels through
> BPF_JIT_ARG_TMP). If the scratch handling swapped f with s.lo, the
> callee would read s.lo = 6, f = a, and 1+2+3+4+5 + 6 + b + a still
> equals a + b + 21, so the test passes.
>
> The same holds for the two halves of every 16-byte argument:
> '(u64)(v >> 64) + (u64)v' and 's.lo + s.hi' are symmetric, so a hi/lo
> swap of an __int128 or of prog_test_pair_arg is invisible in all nine
> value tests.
>
> The commit message states "Every test checks the value the kfunc
> returns, so a misplaced argument shows up rather than passing quietly."
> Does this hold if the values are commutative?

I will change kfunc to have different values with different multiplier
for each argument to make the test more robust.

>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34620351527