Re: [PATCH bpf-next] selftests/bpf: Tolerate libarena skeleton failures under BPF_STRICT_BUILD=0

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The permissive fallback for libarena sub-makes does not remove the target file on failure, leaving a corrupted or empty skeleton header that permanently breaks incremental builds.
- [Medium] The `bench` executable fails to link and is completely skipped when the libarena benchmark skeleton generation fails, contradicting the commit message's claim that 'bench all link'.
--

commit 03a9a4a189e91e03941dfb620bf2f4bda1071094
Author: HyeongJun An <sammiee5311@gmail.com>

selftests/bpf: Tolerate libarena skeleton failures under BPF_STRICT_BUILD=0

This patch modifies the libarena skeleton build rules to respect
BPF_STRICT_BUILD=0, allowing the build to continue if the libarena
skeletons fail to generate. It adds permissive fallback logic to the
three libarena sub-makes.

> The build then prints SKIP-SKEL and continues, and test_progs,
> test_progs-no_alu32, test_progs-cpuv4 and bench all link.

[Severity: Medium]
Is this claim about bench linking successfully accurate when the benchmark
skeleton fails?

Looking at the bench target in the Makefile, it has a hardcoded
dependency on $(OUTPUT)/bench_libarena.o. The link command unconditionally
passes all objects to the compiler:

$(Q)$(CC) $(CFLAGS) $(LDFLAGS) $(filter %.a %.o,$^) $(LDLIBS) -o $@ ...

If the libarena benchmark skeleton fails under permissive mode,
bench_libarena.o will be missing. Because the link command doesn't
filter out skipped benchmark objects, won't this cause the entire bench
executable to fail to link and trigger SKIP-LINK instead?

> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 7ea5ba1df29e3..a9951774e5c85 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -835,17 +835,20 @@ LIBARENA_SKEL := libarena/libarena.skel.h
>  LIBARENA_BENCH_SKEL := libarena/libarena_bench.skel.h
>  
>  $(LIBARENA_SKEL): $(INCLUDE_DIR)/vmlinux.h $(BPFOBJ) $(LIBARENA_BPF_DEPS)
> -	+$(MAKE) -C libarena libarena.skel.h $(LIBARENA_MAKE_ARGS)
> +	+$(MAKE) -C libarena libarena.skel.h $(LIBARENA_MAKE_ARGS) $(if $(PERMISSIVE),|| \
> +		printf '  %-12s %s\n' 'SKIP-SKEL' '$(notdir $@)' 1>&2)

[Severity: High]
Does this missing cleanup leave behind a corrupted skeleton header?

In other permissive fallback rules, ($(RM) $@; printf ...) is used to
ensure the target file is removed on failure. Because shell redirection
in the sub-make might truncate the skeleton header before bpftool runs,
could an error leave a 0-byte or incomplete file behind?

If the file isn't explicitly removed here, won't subsequent incremental
builds attempt to use the corrupted skeleton header instead of rebuilding
it?

[ ... ]

>  $(LIBARENA_BENCH_SKEL): $(INCLUDE_DIR)/vmlinux.h $(BPFOBJ) $(LIBARENA_BENCH_BPF_DEPS) | $(LIBARENA_SKEL)
> -	+$(MAKE) -C libarena benchmarks $(LIBARENA_MAKE_ARGS)
> +	+$(MAKE) -C libarena benchmarks $(LIBARENA_MAKE_ARGS) $(if $(PERMISSIVE),|| \
> +		printf '  %-12s %s\n' 'SKIP-SKEL' '$(notdir $@)' 1>&2)

[Severity: High]
Similarly, should this fallback also remove the target file with $(RM) $@ to
prevent breaking future incremental builds with a corrupted
libarena_bench.skel.h?

[ ... ]

>  ifneq ($(CLANG_HAS_ARENA_ASAN),)
>  LIBARENA_ASAN_SKEL := libarena/libarena_asan.skel.h
>  CFLAGS += -DHAS_BPF_ARENA_ASAN
>  
>  $(LIBARENA_ASAN_SKEL): $(INCLUDE_DIR)/vmlinux.h $(BPFOBJ) $(LIBARENA_BPF_DEPS)
> -	+$(MAKE) -C libarena libarena_asan.skel.h $(LIBARENA_MAKE_ARGS)
> +	+$(MAKE) -C libarena libarena_asan.skel.h $(LIBARENA_MAKE_ARGS) $(if $(PERMISSIVE),|| \
> +		printf '  %-12s %s\n' 'SKIP-SKEL' '$(notdir $@)' 1>&2)

[Severity: High]
Does this fallback need the same target cleanup for libarena_asan.skel.h?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911141621.1944900-1-sammiee5311@gmail.com?part=1