Re: [PATCH bpf-next 2/3] selftests/bpf: report placeholder tests as SKIP, not OK

On 8/21/26 2:00 PM, [email protected] wrote:
>> diff --git a/tools/testing/selftests/bpf/progs/bpf_misc.h b/tools/testing/selftests/bpf/progs/bpf_misc.h
>> index 5eacf1b43252..88b0bfba83bb 100644
>> --- a/tools/testing/selftests/bpf/progs/bpf_misc.h
>> +++ b/tools/testing/selftests/bpf/progs/bpf_misc.h
>> @@ -106,6 +106,11 @@
>>    * __description     Text to be used for display and as an additional filter
>>    *                   alias, while the original program name stays matchable.
>>    *
>> + * __skip            Report the test as SKIP with the given reason instead of
>> + *                   running it. For placeholder programs that stand in for a
>> + *                   feature the toolchain or JIT cannot provide, so a run does
>> + *                   not report OK for coverage it never executed.
>> + *
>>    * __log_level       Log level to use for the program, numeric value expected.
>>    *
>>    * __flag            Adds one flag use for the program, the following values are valid:
>> @@ -139,6 +144,7 @@
>>   #define __failure		__test_tag("test_expect_failure")
>>   #define __success		__test_tag("test_expect_success")
>>   #define __description(desc)	__test_tag("test_description=" desc)
>> +#define __skip(reason)		__test_tag("test_skip=" reason)
>>   #define __msg_unpriv(msg)	__test_tag("test_expect_msg_unpriv=" msg)
> [ ... ]
>
>> diff --git a/tools/testing/selftests/bpf/progs/verifier_ldsx.c b/tools/testing/selftests/bpf/progs/verifier_ldsx.c
>> index 41340877dc9d..e58ea69e3854 100644
>> --- a/tools/testing/selftests/bpf/progs/verifier_ldsx.c
>> +++ b/tools/testing/selftests/bpf/progs/verifier_ldsx.c
>> @@ -436,6 +436,7 @@ void kfunc_root(void)
>>
>>   SEC("socket")
>>   __description("cpuv4 is not supported by compiler or jit, use a dummy test")
>> +__skip("cpuv4 is not supported by compiler or jit")
>>   __success
>>   int dummy_test(void)
>>   {
> [ ... ]
>
>> diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c
>> index 07807757b518..794a7dfb0579 100644
>> --- a/tools/testing/selftests/bpf/test_loader.c
>> +++ b/tools/testing/selftests/bpf/test_loader.c
>> @@ -70,6 +70,7 @@ struct test_spec {
>>   	int arch_mask;
>>   	int load_mask;
>>   	int linear_sz;
>> +	const char *skip_reason;
>>   	bool auxiliary;
>>   	bool valid;
>>   };
>> @@ -456,6 +457,8 @@ static int parse_test_spec(struct test_loader *tester,
>>   			continue;
>>   		if ((val = str_has_pfx(s, "test_description="))) {
>>   			description = val;
>> +		} else if ((val = str_has_pfx(s, "test_skip="))) {
>> +			spec->skip_reason = val;
>>   		} else if (strcmp(s, "test_expect_failure") == 0) {
>>   			spec->priv.expect_failure = true;
>>   			spec->mode_mask |= PRIV;
>> @@ -1327,6 +1330,12 @@ void run_subtest(struct test_loader *tester,
>>   	if (!test__start_subtest_with_desc(subspec->name, subspec->description))
>>   		return;
>>
>> +	if (spec->skip_reason) {
>> +		printf("%s:SKIP: %s\n", __func__, spec->skip_reason);
>> +		test__skip();
>> +		return;
>> +	}
>> +
>>   	if ((get_current_arch() & spec->arch_mask) == 0) {
>>   		test__skip();
>>   		return;
> The commit converts 13 placeholder programs to use the new __skip()
> annotation, but two feature-gated placeholders with the identical
> pattern were not converted. Were these intentionally excluded?
>
> The preceding commit b1831d82fe2c states "Eleven of the thirteen
> feature-gated placeholder programs are called dummy_test", but grep
> finds fifteen such programs.
>
> tools/testing/selftests/bpf/progs/verifier_aggregate_ret.c has a
> compile-time gate at line 8:
>
>    #if defined(__clang_major__) && __clang_major__ >= 23
>
> with its #else arm at lines 166-172 supplying the same __success
> placeholder pattern that this commit converts everywhere else:
>
>    #else
>
>    SEC("socket")
>    __description("verifier_aggregate_ret: needs LLVM 23, dummy test")
>    __success
>    int dummy_test(void)
>
> Similarly, tools/testing/selftests/bpf/progs/verifier_percpu_addr.c has
> a gate at line 7:
>
>    #if defined(__TARGET_ARCH_x86)
>
> with its #else placeholder at lines 60-66:
>
>    #else
>
>    SEC("raw_tp")
>    __description("percpu addr dummy")
>    __success
>    int dummy_test(void)
>
> Both reach the modified test_loader.c code path via
> prog_tests/verifier.c (test_verifier_aggregate_ret and
> test_verifier_percpu_addr), so __skip() would work for them.
>
> The gates fire today, so the placeholders are what actually gets built
> and reported:
> - verifier_aggregate_ret.c: any toolchain older than clang 23
> - verifier_percpu_addr.c: every non-x86 build (arm64, s390x, riscv64,
>    loongarch)
>
> On those builds these two tests still report OK for coverage that never
> executed.

Bot is correct. The reason is the older baseline I first worked against 
had the aforementioned numbers. After rebase I did remember to rerun the 
numbers but obviously there was some snafu. I'll fix this in v2.

Thx,
-Vineet