Re: [PATCH bpf-next v5 02/11] selftests/bpf: Fix test_kmods KDIR to honor O= and distro kernels

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] Skipping module compilation in permissive mode fatally breaks `rsync` during out-of-tree builds.
- [Medium] Silently omitting the kernel modules converts a build skip into unconditional runtime test suite failures.
--

commit 7696fe9d292ef9e4b288adf6be9f441665f1447a
Author: Ricardo B. Marlière <rbm@suse.com>

selftests/bpf: Fix test_kmods KDIR to honor O= and distro kernels

This commit updates test_kmods/Makefile to respect out-of-tree builds using
O= and KBUILD_OUTPUT. It also introduces a permissive mode that silently
skips building modules if the kernel directory is missing.

> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -296,7 +296,7 @@ $(addprefix test_kmods/,$(subst .ko,%ko,$(TEST_KMODS))): $(VMLINUX_BTF) $(RESOLV
>  
>  $(TEST_KMOD_TARGETS): $(addprefix test_kmods/,$(TEST_KMODS))
>  	$(call msg,MOD,,$@)
> -	$(Q)cp test_kmods/$(@F) $@
> +	$(Q)$(if $(PERMISSIVE),if [ -f test_kmods/$(@F) ]; then )cp test_kmods/$(@F) $@$(if $(PERMISSIVE),; fi)

Does skipping the cp command leave the target file missing from
TEST_KMOD_TARGETS, which is subsequently included in TRUNNER_EXTRA_FILES?

During an out-of-tree build where O= is set, the rule
$(TRUNNER_BINARY)-extras runs rsync -aq $$^ $(TRUNNER_OUTPUT)/. Since $$^
evaluates to the prerequisites, it passes the missing .ko file path directly
to rsync.

Because the file was never created, does this cause rsync to fail fatally
with a No such file or directory error and abort the build?

Note that while the later patch selftests/bpf: Tolerate missing files
during install adds --ignore-missing-args to fix make install, it does not
modify the $(TRUNNER_BINARY)-extras target, leaving out-of-tree make all
broken.

> diff --git a/tools/testing/selftests/bpf/test_kmods/Makefile b/tools/testing/selftests/bpf/test_kmods/Makefile
> --- a/tools/testing/selftests/bpf/test_kmods/Makefile
> +++ b/tools/testing/selftests/bpf/test_kmods/Makefile
[ ... ]
> @@ -14,8 +16,24 @@ $(foreach m,$(MODULES),$(eval obj-m += $(m:.ko=.o)))
>  
>  CFLAGS_bpf_testmod.o = -I$(src)
>  
> +# When BPF_STRICT_BUILD != 0, a missing KDIR is fatal (the default).
> +# When permissive, skip silently.
> +PERMISSIVE := $(filter 0,$(BPF_STRICT_BUILD))
> +
>  all:
> -	$(Q)$(MAKE) -C $(KDIR) M=$(TEST_KMOD_DIR) modules
> +ifeq ($(PERMISSIVE),)
> +	$(Q)$(MAKE) -C $(KDIR) $(if $(KMOD_O),O=$(KMOD_O),KBUILD_OUTPUT=) \
> +		M=$(TEST_KMOD_DIR) modules
> +else ifneq ("$(wildcard $(KDIR))", "")
> +	$(Q)$(MAKE) -C $(KDIR) $(if $(KMOD_O),O=$(KMOD_O),KBUILD_OUTPUT=) \
> +		M=$(TEST_KMOD_DIR) modules
> +endif

If permissive mode silently skips module compilation, does this convert a
build skip into unconditional runtime test suite failures?

While test_progs.c attempts to gracefully skip tests by checking
env.has_testmod, other runners and tests fail unconditionally.

For example, test_verifier.c explicitly calls load_bpf_testmod() and
immediately returns EXIT_FAILURE if it fails, aborting the entire
verification test suite.

Similarly, tests like bpf_mod_race.c contain strict assertions
(ASSERT_OK(load_bpf_testmod(false), ...)) in their recovery paths that fail
when the module is missing.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260415-selftests-bpf_misconfig-v5-0-03d0a52a898a@suse.com?part=2