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

On Thu, Apr 16, 2026 at 5:53 AM <[email protected]> wrote:
>
> Sashiko AI review found 2 potential issue(s):
> - [Medium] Relative paths passed via `O=` are incorrectly evaluated from the kernel source root instead of the invocation directory.
> - [Medium] The `make clean` target unconditionally fails and aborts the build if the kernel source tree is unconfigured and distro headers are missing.
> --
>
> commit 533e8d39cc98f56f3dff4f8c5bff0dcd49fc6ef1
> Author: Ricardo B. Marlière <[email protected]>
>
> selftests/bpf: Fix test_kmods KDIR to honor O= and distro kernels
>
> This commit modifies test_kmods/Makefile to honor O= and KBUILD_OUTPUT, and
> adds a fallback to /lib/modules/$(uname -r)/build for distro kernels. It also
> makes missing KDIR silently skipped when building in permissive mode.
>
> > diff --git a/tools/testing/selftests/bpf/test_kmods/Makefile b/tools/testing/selftests/bpf/test_kmods/Makefile
> > index 63c4d3f6a12f6..10f202bd0d8c2 100644
> > --- a/tools/testing/selftests/bpf/test_kmods/Makefile
> > +++ b/tools/testing/selftests/bpf/test_kmods/Makefile
> > @@ -1,5 +1,7 @@
> >  TEST_KMOD_DIR := $(realpath $(dir $(abspath $(lastword $(MAKEFILE_LIST)))))
> > -KDIR ?= $(abspath $(TEST_KMOD_DIR)/../../../../..)
> > +SRCTREE_KDIR := $(abspath $(TEST_KMOD_DIR)/../../../../..)
> > +KMOD_O := $(or $(O),$(KBUILD_OUTPUT))
>
> Will this break out-of-tree builds if a user passes a relative path?
>
> If a user builds selftests with a relative O= path (e.g. make O=out), KMOD_O
> captures this relative string and passes it unaltered to the kernel build
> system via make -C $(KDIR) O=$(KMOD_O).
>
> Because make -C changes the working directory to $(KDIR) before evaluating
> O=, the relative path will be evaluated relative to the kernel root rather
> than the directory where make was originally invoked.
>
> [ ... ]
> > @@ -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
>
> [ ... ]
>
> >  clean:
> > -     $(Q)$(MAKE) -C $(KDIR) M=$(TEST_KMOD_DIR) clean
> > +ifeq ($(PERMISSIVE),)
> > +     $(Q)$(MAKE) -C $(KDIR) $(if $(KMOD_O),O=$(KMOD_O),KBUILD_OUTPUT=) \
> > +             M=$(TEST_KMOD_DIR) clean
>
> Does this unconditionally abort make clean if the kernel source tree is
> unconfigured and distro headers are missing?

I think both issues flagged are valid and need to be addressed.