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

sashiko-bot
2026-04-15T21:18:29Z

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