Re: [PATCH bpf-next 09/12] selftests/bpf: Add C tests for 16-byte returns in R0:R2

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

On 7/8/26 2:10 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/testing/selftests/bpf/prog_tests/aggregate_ret.c b/tools/testing/selftests/bpf/prog_tests/aggregate_ret.c
>> new file mode 100644
>> index 0000000000000..d4ecc19684cef
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/aggregate_ret.c
>> @@ -0,0 +1,96 @@
> [ ... ]
>
>> +/*
>> + * Run one program and check its result. The program returns 0 on success, or
>> + * 1..4 identifying which half of which aggregate (R0:R2) return value came back
>> + * wrong. A return of -1 means the program was built with an LLVM that lacks
>> + * R0:R2 16-byte return support, in which case the subtest is skipped.
>> + */
> [ ... ]
>
>> +	/* C variant (__int128): relies on LLVM 23 R0:R2 codegen, skipped otherwise. */
>> +	if (test__start_subtest("int128_c"))
>> +		run_prog(skel_c->progs.aggregate_ret_test, true);
>> +
>> +	aggregate_ret_int128_c__destroy(skel_c);
>> +
>> +	skel_struct_c = aggregate_ret_struct_c__open_and_load();
>> +	if (!ASSERT_OK_PTR(skel_struct_c, "skel_struct_c_open_load"))
>> +		return;
>> +
>> +	/*
>> +	 * C variant returning a 16-byte struct by value: exercises the struct
>> +	 * branch of bpf_ret_reg_pair(). Also LLVM 23 only.
>> +	 */
>> +	if (test__start_subtest("struct_c"))
>> +		run_prog(skel_struct_c->progs.aggregate_ret_struct_c_test, true);
>> +
>> +	/*
>> +	 * Same 16-byte struct return, but from a global (non-static) callee:
>> +	 * exercises the compiler lowering a global struct-returning function to
>> +	 * R0:R2 and the verifier's global return path end to end. Also LLVM 23
>> +	 * only.
>> +	 */
>> +	if (test__start_subtest("global_struct_c"))
>> +		run_prog(skel_struct_c->progs.aggregate_ret_global_struct_c_test, true);
>> +
>> +	/*
>> +	 * Same 16-byte struct, but returned by value from a kfunc: exercises
>> +	 * the struct branch of check_kfunc_call() and the JIT RDX->R2 move.
>> +	 * Also LLVM 23 only.
>> +	 */
>> +	if (test__start_subtest("kfunc_struct_c"))
>> +		run_prog(skel_struct_c->progs.aggregate_ret_kfunc_struct_c_test, true);
> this isn't a bug, but the "Also LLVM 23 only." note is repeated on each
> of these per-subtest comments, and the run_prog() comment above already
> states that programs built without R0:R2 support return -1 and get
> skipped.  Could that note be stated once rather than on every block?

Ack. Will do.

>
> Related, do the per-subtest comments need to name the specific internals
> (bpf_ret_reg_pair(), check_kfunc_call(), "the JIT RDX->R2 move"), or
> would a shorter note serve here?

I guess I can rename these comments, function names themselves already have
enough information.

>
>> 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 c36bb911defa7..34d0a352e70dd 100644
>> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
>> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod_kfunc.h
>> @@ -55,6 +55,15 @@ struct prog_test_big_arg {
>>   	__u64 b;
>>   };
>>
>> +/*
>> + * A 16-byte struct returned by value from a kfunc, returned in the R0:R2
>> + * register pair.
>> + */
>> +struct bpf_kfunc_ret_ll {	/* 16 bytes: R0:R2 */
> this isn't a bug, but the block comment already says the struct is 16
> bytes and returned in the R0:R2 register pair, and the trailing
> /* 16 bytes: R0:R2 */ on the next line repeats that.  Could one of the
> two be dropped?

Yes, will drop.

>
>> +	__u64 a;
>> +	__u64 b;
>> +};
>
> ---
> 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/28973410219