Re: [PATCH v2 07/10] selftests/bpf: Tolerate benchmark build failures

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Masking benchmark compilation failures breaks `make install` unconditionally due to the missing `bench` executable, as partial linking of `bench` is impossible.
--

--- Patch [7]: [PATCH v2 07/10] selftests/bpf: Tolerate benchmark build failures ---
commit d38880b3d361e7ca90a0ac6bc884115d3f936eda
Author: Ricardo B. Marlière <[email protected]>

selftests/bpf: Tolerate benchmark build failures
    
This commit makes benchmark object and binary compilation non-fatal during
the selftests/bpf build. It emits SKIP-BENCH or SKIP-LINK and removes
failed outputs to prevent stale object reuse.

> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index ca64a7678dfe1..6c4f854594862 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
[ ... ]
> @@ -918,7 +919,8 @@ $(OUTPUT)/bench: $(OUTPUT)/bench.o \
>  		 $(OUTPUT)/usdt_2.o \
>  		 #
>  	$(call msg,BINARY,,$@)
> -	$(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.a %.o,$^) $(LDLIBS) -o $@
> +	$(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.a %.o,$(wildcard $^)) $(LDLIBS) -o $@ || \
> +		($(RM) $@; printf '  %-12s %s\n' 'SKIP-LINK' '$(notdir $@) (some benchmarks may have been skipped)' 1>&2)

[Severity: Medium]
Could this cause the installation pipeline to fail unconditionally when a
benchmark object fails to build?

When a benchmark compilation fails, the linker step fails and deletes the
target executable. Because the bench binary is listed in
TEST_GEN_PROGS_EXTENDED, the rsync step in tools/testing/selftests/lib.mk
during make install will fail when it attempts to copy the missing file:

tools/testing/selftests/lib.mk:
INSTALL_SINGLE_RULE:
    ...
    $(if $(INSTALL_LIST),rsync -a --copy-unsafe-links $(INSTALL_LIST) $(INSTALL_PATH)/)

Also, is it possible to partially link the bench binary when some objects
are missing? Looking at tools/testing/selftests/bpf/bench.c, it hardcodes
explicit references to all benchmark structures:

tools/testing/selftests/bpf/bench.c:
static const struct bench *benchs[] = {
	&bench_count_global,
	&bench_count_local,
	&bench_rename_base,
    ...
};

This guarantees an "undefined reference" linker failure if any object is
skipped, meaning the bench binary will always fail to link and get deleted on
a partial build. Should missing benchmarks be excluded from the benchs array,
or should the installation step be updated to handle the missing executable?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7