| Message ID | 20240912063119.1277322-1-anders.roxell@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests: Makefile: add missing 'net/lib' to targets | expand |
On Thu, Sep 12, 2024 at 2:31 AM Anders Roxell <anders.roxell@linaro.org> wrote: > > Fixes: 1d0dc857b5d8 ("selftests: drv-net: add checksum tests") > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> This target is automatically built for targets that depend on it. See the commit that introduced it, b86761ff6374. +++ b/tools/testing/selftests/Makefile @@ -116,6 +116,13 @@ TARGETS += zram TARGETS_HOTPLUG = cpu-hotplug TARGETS_HOTPLUG += memory-hotplug +# Networking tests want the net/lib target, include it automatically +ifneq ($(filter net,$(TARGETS)),) +ifeq ($(filter net/lib,$(TARGETS)),) + INSTALL_DEP_TARGETS := net/lib +endif +endif If you believe that it needs to be included directly, please expand the commit message with the reasoning.
On Thu, 12 Sep 2024 08:31:18 +0200 Anders Roxell wrote: > Fixes: 1d0dc857b5d8 ("selftests: drv-net: add checksum tests") > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > --- > tools/testing/selftests/Makefile | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile > index 3b7df5477317..fc3681270afe 100644 > --- a/tools/testing/selftests/Makefile > +++ b/tools/testing/selftests/Makefile > @@ -64,6 +64,7 @@ TARGETS += net > TARGETS += net/af_unix > TARGETS += net/forwarding > TARGETS += net/hsr > +TARGETS += net/lib > TARGETS += net/mptcp > TARGETS += net/netfilter > TARGETS += net/openvswitch Please make sure you always include a commit message. Among other things writing one would force you to understand the code, and in this case understand that this target is intentionally left out. Look around the Makefile for references to net/lib, you'll figure it out. The patch is incorrect.
On 9/12/24 09:23, Jakub Kicinski wrote: > On Thu, 12 Sep 2024 08:31:18 +0200 Anders Roxell wrote: >> Fixes: 1d0dc857b5d8 ("selftests: drv-net: add checksum tests") >> Signed-off-by: Anders Roxell <anders.roxell@linaro.org> >> --- >> tools/testing/selftests/Makefile | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile >> index 3b7df5477317..fc3681270afe 100644 >> --- a/tools/testing/selftests/Makefile >> +++ b/tools/testing/selftests/Makefile >> @@ -64,6 +64,7 @@ TARGETS += net >> TARGETS += net/af_unix >> TARGETS += net/forwarding >> TARGETS += net/hsr >> +TARGETS += net/lib >> TARGETS += net/mptcp >> TARGETS += net/netfilter >> TARGETS += net/openvswitch > > Please make sure you always include a commit message. Among other > things writing one would force you to understand the code, and > in this case understand that this target is intentionally left out. > Look around the Makefile for references to net/lib, you'll figure > it out. > +1 - thank you for outlining the benefits of writing a change log which includes the details. This patch is missing the changelog completely - change log is an important part of sending a patch. > The patch is incorrect. thanks, -- Shuah
On Thu, 12 Sept 2024 at 17:23, Jakub Kicinski <kuba@kernel.org> wrote: > > On Thu, 12 Sep 2024 08:31:18 +0200 Anders Roxell wrote: > > Fixes: 1d0dc857b5d8 ("selftests: drv-net: add checksum tests") > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > > --- > > tools/testing/selftests/Makefile | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile > > index 3b7df5477317..fc3681270afe 100644 > > --- a/tools/testing/selftests/Makefile > > +++ b/tools/testing/selftests/Makefile > > @@ -64,6 +64,7 @@ TARGETS += net > > TARGETS += net/af_unix > > TARGETS += net/forwarding > > TARGETS += net/hsr > > +TARGETS += net/lib > > TARGETS += net/mptcp > > TARGETS += net/netfilter > > TARGETS += net/openvswitch > > Please make sure you always include a commit message. Among other > things writing one would force you to understand the code, and > in this case understand that this target is intentionally left out. > Look around the Makefile for references to net/lib, you'll figure > it out. > > The patch is incorrect. You’re right, the patch is incorrect, I could have explained better. I’m seeing an issue with an out-of-tree cross compilation build of kselftest and can’t figure out what’s wrong. make --keep-going --jobs=32 O=/tmp/build INSTALL_PATH=/tmp/build/kselftest_install \ ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- \ CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- kselftest-install [...] make[4]: Entering directory '/home/anders/src/kernel/linux/tools/testing/selftests/net/lib' CC csum /usr/lib/gcc-cross/aarch64-linux-gnu/13/../../../../aarch64-linux-gnu/bin/ld: cannot open output file /tmp/build/kselftest/net/lib/csum: No such file or directory collect2: error: ld returned 1 exit status [...] Any thoughts on what might be causing this? Cheers, Anders
On Sun, Sep 15, 2024 at 8:45 AM Anders Roxell <anders.roxell@linaro.org> wrote: > > On Thu, 12 Sept 2024 at 17:23, Jakub Kicinski <kuba@kernel.org> wrote: > > > > On Thu, 12 Sep 2024 08:31:18 +0200 Anders Roxell wrote: > > > Fixes: 1d0dc857b5d8 ("selftests: drv-net: add checksum tests") > > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > > > --- > > > tools/testing/selftests/Makefile | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile > > > index 3b7df5477317..fc3681270afe 100644 > > > --- a/tools/testing/selftests/Makefile > > > +++ b/tools/testing/selftests/Makefile > > > @@ -64,6 +64,7 @@ TARGETS += net > > > TARGETS += net/af_unix > > > TARGETS += net/forwarding > > > TARGETS += net/hsr > > > +TARGETS += net/lib > > > TARGETS += net/mptcp > > > TARGETS += net/netfilter > > > TARGETS += net/openvswitch > > > > Please make sure you always include a commit message. Among other > > things writing one would force you to understand the code, and > > in this case understand that this target is intentionally left out. > > Look around the Makefile for references to net/lib, you'll figure > > it out. > > > > The patch is incorrect. > > You’re right, the patch is incorrect, I could have explained better. > I’m seeing an issue with an out-of-tree cross compilation build of > kselftest and can’t figure out what’s wrong. > > make --keep-going --jobs=32 O=/tmp/build > INSTALL_PATH=/tmp/build/kselftest_install \ > ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- \ > CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- kselftest-install > > [...] > make[4]: Entering directory > '/home/anders/src/kernel/linux/tools/testing/selftests/net/lib' > CC csum > /usr/lib/gcc-cross/aarch64-linux-gnu/13/../../../../aarch64-linux-gnu/bin/ld: > cannot open output file /tmp/build/kselftest/net/lib/csum: No such > file or directory > collect2: error: ld returned 1 exit status > [...] > > Any thoughts on what might be causing this? I wonder if this is due to the O= argument. Last week I noticed that some TARGETs explicitly have support for this, like x86. Added in 2016 in commit a8ba798bc8ec6 ("selftests: enable O and KBUILD_OUTPUT"). But by now this support is hardly universal. amd-pstate does not have this infra, for instance. Though if the only breakage is in net/lib, then that does not explain it fully.
On Sun, 15 Sep 2024 09:36:10 +0200 Willem de Bruijn wrote: > > You’re right, the patch is incorrect, I could have explained better. > > I’m seeing an issue with an out-of-tree cross compilation build of > > kselftest and can’t figure out what’s wrong. > > > > make --keep-going --jobs=32 O=/tmp/build > > INSTALL_PATH=/tmp/build/kselftest_install \ > > ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- \ > > CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- kselftest-install > > > > [...] > > make[4]: Entering directory > > '/home/anders/src/kernel/linux/tools/testing/selftests/net/lib' > > CC csum > > /usr/lib/gcc-cross/aarch64-linux-gnu/13/../../../../aarch64-linux-gnu/bin/ld: > > cannot open output file /tmp/build/kselftest/net/lib/csum: No such > > file or directory > > collect2: error: ld returned 1 exit status > > [...] > > > > Any thoughts on what might be causing this? > > I wonder if this is due to the O= argument. > > Last week I noticed that some TARGETs explicitly have support for > this, like x86. Added in 2016 in commit a8ba798bc8ec6 ("selftests: > enable O and KBUILD_OUTPUT"). But by now this support is hardly > universal. amd-pstate does not have this infra, for instance. > > Though if the only breakage is in net/lib, then that does not explain it fully. Some funny business with this install target, I haven't investigated fully but the dependency on all doesn't seem to do its job, and the install target has a copy/paste of all with this line missing: diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 3b7df5477317..3aee8e7b9993 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -261,6 +261,7 @@ ifdef INSTALL_PATH @ret=1; \ for TARGET in $(TARGETS) $(INSTALL_DEP_TARGETS); do \ BUILD_TARGET=$$BUILD/$$TARGET; \ + mkdir -p $$BUILD_TARGET; \ $(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET install \ INSTALL_PATH=$(INSTALL_PATH)/$$TARGET \ SRC_PATH=$(shell readlink -e $$(pwd)) \ Andres, please feel free to test / write commit message and submit this one liner, but even with that the build for some targets fails for me. "make [..] install" seems wobbly.
On Sun, 15 Sept 2024 at 16:46, Jakub Kicinski <kuba@kernel.org> wrote: > > On Sun, 15 Sep 2024 09:36:10 +0200 Willem de Bruijn wrote: > > > You’re right, the patch is incorrect, I could have explained better. > > > I’m seeing an issue with an out-of-tree cross compilation build of > > > kselftest and can’t figure out what’s wrong. > > > > > > make --keep-going --jobs=32 O=/tmp/build > > > INSTALL_PATH=/tmp/build/kselftest_install \ > > > ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- \ > > > CROSS_COMPILE_COMPAT=arm-linux-gnueabihf- kselftest-install > > > > > > [...] > > > make[4]: Entering directory > > > '/home/anders/src/kernel/linux/tools/testing/selftests/net/lib' > > > CC csum > > > /usr/lib/gcc-cross/aarch64-linux-gnu/13/../../../../aarch64-linux-gnu/bin/ld: > > > cannot open output file /tmp/build/kselftest/net/lib/csum: No such > > > file or directory > > > collect2: error: ld returned 1 exit status > > > [...] > > > > > > Any thoughts on what might be causing this? > > > > I wonder if this is due to the O= argument. > > > > Last week I noticed that some TARGETs explicitly have support for > > this, like x86. Added in 2016 in commit a8ba798bc8ec6 ("selftests: > > enable O and KBUILD_OUTPUT"). But by now this support is hardly > > universal. amd-pstate does not have this infra, for instance. > > > > Though if the only breakage is in net/lib, then that does not explain it fully. > > Some funny business with this install target, I haven't investigated > fully but the dependency on all doesn't seem to do its job, and the > install target has a copy/paste of all with this line missing: > > diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile > index 3b7df5477317..3aee8e7b9993 100644 > --- a/tools/testing/selftests/Makefile > +++ b/tools/testing/selftests/Makefile > @@ -261,6 +261,7 @@ ifdef INSTALL_PATH > @ret=1; \ > for TARGET in $(TARGETS) $(INSTALL_DEP_TARGETS); do \ > BUILD_TARGET=$$BUILD/$$TARGET; \ > + mkdir -p $$BUILD_TARGET; \ > $(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET install \ > INSTALL_PATH=$(INSTALL_PATH)/$$TARGET \ > SRC_PATH=$(shell readlink -e $$(pwd)) \ > > > Andres, please feel free to test / write commit message and submit this > one liner, but even with that the build for some targets fails for me. Thank you Jakub, that solved this issue, I'll send a patch shortly. > "make [..] install" seems wobbly. Yes it is. Cheers, Anders
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 3b7df5477317..fc3681270afe 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -64,6 +64,7 @@ TARGETS += net TARGETS += net/af_unix TARGETS += net/forwarding TARGETS += net/hsr +TARGETS += net/lib TARGETS += net/mptcp TARGETS += net/netfilter TARGETS += net/openvswitch
Fixes: 1d0dc857b5d8 ("selftests: drv-net: add checksum tests") Signed-off-by: Anders Roxell <anders.roxell@linaro.org> --- tools/testing/selftests/Makefile | 1 + 1 file changed, 1 insertion(+)