Re: [PATCH v2 2/2] perf test make: Add BPF skeleton build tests for clang and bpf-gcc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] perf: bpf_skel test skipped due to missing pkg-config for OpenSSL
- [Low] perf/tests: bash negation operator ! masks test framework crashes
--

commit b1fbd7e0bcb15ba961fd484277db525d4021ba13
Author: Ian Rogers <irogers@google.com>

perf test make: Add BPF skeleton build tests for clang and bpf-gcc

Add make_bpf_skel_clang (CLANG=clang BPF_GCC=) and make_bpf_skel_bpf_gcc
(BPF_GCC=bpf-gcc CLANG=) targets to tools/perf/tests/make when the
corresponding compiler and BPF skeleton dependencies are available on
the host.

> diff --git a/tools/perf/tests/make b/tools/perf/tests/make
> index 7524de5228029..4e66b93cf12c9 100644
> --- a/tools/perf/tests/make
> +++ b/tools/perf/tests/make

[ ... ]

> @@ -142,6 +144,20 @@ have_libllvm := $(shell printf '#include <llvm/Support/ManagedStatic.h>\n#includ
>  		$(shell $(LLVM_CONFIG) --system-libs 2>/dev/null) >/dev/null 2>&1 && echo y)
>  endif
>  
> +have_bpf_skel_deps := $(shell printf '#include <libelf.h>\n#include <zlib.h>\n#include <linux/bpf.h>\n#include <openssl/opensslv.h>\nint main(){return 0;}\n' | \
> +	$(CC) -x c -I$(srctree)/tools/include/uapi - -o /dev/null -lelf -lz -lcrypto >/dev/null 2>&1 && echo y)

[Severity: Low]
Does this hardcoded check fail to find OpenSSL on systems that require explicit
paths via pkg-config?

Unlike tools/build/feature/Makefile which correctly utilizes pkg-config, it
looks like missing pkg-config usage here could lead to silent test evasion on
environments where OpenSSL is not in the default compiler search path.

> +
> +ifneq ($(have_bpf_skel_deps),)

[ ... ]

> @@ -313,6 +335,13 @@ test_make_clang_nondistro     := $(test_make_clang)
>  test_make_clang_nondistro_O   := $(test_make_clang_O)
>  endif
>  
> +test_make_no_bpf_skel         := test -x $(PERF_O)/perf && ! $(PERF_O)/perf check -q feature bpf_skeletons
> +test_make_no_bpf_skel_O       := test -x $$TMP_O/perf && ! $$TMP_O/perf check -q feature bpf_skeletons

[Severity: Low]
By using the shell negation operator !, does this prevent the framework from
distinguishing between a graceful exit 1 (feature correctly absent) and a
catastrophic crash (exit code >128)?

Would using an explicit exit code validation (e.g., ; test $? -eq 1) avoid
masking crashes in the built perf check utility?

> +test_make_bpf_skel_clang      := test -x $(PERF_O)/perf && $(PERF_O)/perf check -q feature bpf_skeletons
> +test_make_bpf_skel_clang_O    := test -x $$TMP_O/perf && $$TMP_O/perf check -q feature bpf_skeletons
> +test_make_bpf_skel_bpf_gcc    := test -x $(PERF_O)/perf && $(PERF_O)/perf check -q feature bpf_skeletons
> +test_make_bpf_skel_bpf_gcc_O  := test -x $$TMP_O/perf && $$TMP_O/perf check -q feature bpf_skeletons
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013231430.2163505-1-irogers@google.com?part=2