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

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

[Alexei Starovoitov]

On Thu, Apr 16, 2026 at 5:53 AM <sashiko-bot@kernel.org> 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 <rbm@suse.com>
>
> 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.

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

[Ricardo B. Marlière]

On Sun Apr 26, 2026 at 8:40 PM -03, Alexei Starovoitov wrote:
> On Thu, Apr 16, 2026 at 5:53 AM <sashiko-bot@kernel.org> 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 <rbm@suse.com>
>>
>> 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.

Yes, indeed! I will respin a v8 (this is v6) later addressing these and
a few other points. Thanks for checking, it means there's still hope for this
series :)

I wish there was a way to trigger sashiko without having to send it to
the ML.