* [RFC PATCH net-next 01/10] selftests: bonding: Change script interpreter
2023-12-22 13:58 [RFC PATCH net-next 00/10] selftests: Add TEST_INCLUDES directive and adjust tests to use it Benjamin Poirier
@ 2023-12-22 13:58 ` Benjamin Poirier
2023-12-22 13:58 ` [RFC PATCH net-next 02/10] selftests: forwarding: Remove executable bits from lib.sh Benjamin Poirier
` (9 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Benjamin Poirier @ 2023-12-22 13:58 UTC (permalink / raw)
To: netdev; +Cc: Shuah Khan, Petr Machata, Hangbin Liu, Vladimir Oltean
The tests changed by this patch, as well as the scripts they source, use
features which are not part of POSIX sh (ex. 'source' and 'local'). As a
result, these tests fail when /bin/sh is dash such as on Debian. Change the
interpreter to bash so that these tests can run successfully.
Fixes: d43eff0b85ae ("selftests: bonding: up/down delay w/ slave link flapping")
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
.../selftests/drivers/net/bonding/mode-1-recovery-updelay.sh | 2 +-
.../selftests/drivers/net/bonding/mode-2-recovery-updelay.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/bonding/mode-1-recovery-updelay.sh b/tools/testing/selftests/drivers/net/bonding/mode-1-recovery-updelay.sh
index ad4c845a4ac7..b76bf5030952 100755
--- a/tools/testing/selftests/drivers/net/bonding/mode-1-recovery-updelay.sh
+++ b/tools/testing/selftests/drivers/net/bonding/mode-1-recovery-updelay.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
# SPDX-License-Identifier: GPL-2.0
# Regression Test:
diff --git a/tools/testing/selftests/drivers/net/bonding/mode-2-recovery-updelay.sh b/tools/testing/selftests/drivers/net/bonding/mode-2-recovery-updelay.sh
index 2330d37453f9..8c2619002147 100755
--- a/tools/testing/selftests/drivers/net/bonding/mode-2-recovery-updelay.sh
+++ b/tools/testing/selftests/drivers/net/bonding/mode-2-recovery-updelay.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
# SPDX-License-Identifier: GPL-2.0
# Regression Test:
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH net-next 02/10] selftests: forwarding: Remove executable bits from lib.sh
2023-12-22 13:58 [RFC PATCH net-next 00/10] selftests: Add TEST_INCLUDES directive and adjust tests to use it Benjamin Poirier
2023-12-22 13:58 ` [RFC PATCH net-next 01/10] selftests: bonding: Change script interpreter Benjamin Poirier
@ 2023-12-22 13:58 ` Benjamin Poirier
2023-12-27 19:24 ` Vladimir Oltean
2023-12-22 13:58 ` [RFC PATCH net-next 03/10] selftests: forwarding: Import top-level lib.sh through absolute path Benjamin Poirier
` (8 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Benjamin Poirier @ 2023-12-22 13:58 UTC (permalink / raw)
To: netdev; +Cc: Shuah Khan, Petr Machata, Hangbin Liu, Vladimir Oltean
The lib.sh script is meant to be sourced from other scripts, not executed
directly. Therefore, remove the executable bits from lib.sh's permissions.
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 0
1 file changed, 0 insertions(+), 0 deletions(-)
mode change 100755 => 100644 tools/testing/selftests/net/forwarding/lib.sh
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
old mode 100755
new mode 100644
--
2.43.0
^ permalink raw reply [flat|nested] 22+ messages in thread* [RFC PATCH net-next 03/10] selftests: forwarding: Import top-level lib.sh through absolute path
2023-12-22 13:58 [RFC PATCH net-next 00/10] selftests: Add TEST_INCLUDES directive and adjust tests to use it Benjamin Poirier
2023-12-22 13:58 ` [RFC PATCH net-next 01/10] selftests: bonding: Change script interpreter Benjamin Poirier
2023-12-22 13:58 ` [RFC PATCH net-next 02/10] selftests: forwarding: Remove executable bits from lib.sh Benjamin Poirier
@ 2023-12-22 13:58 ` Benjamin Poirier
2023-12-22 13:58 ` [RFC PATCH net-next 04/10] selftests: forwarding: Simplify forwarding.config import logic Benjamin Poirier
` (7 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Benjamin Poirier @ 2023-12-22 13:58 UTC (permalink / raw)
To: netdev; +Cc: Shuah Khan, Petr Machata, Hangbin Liu, Vladimir Oltean
From: Petr Machata <petrm@nvidia.com>
The commit cited below moved code from net/forwarding/lib.sh to net/lib.sh,
and had the former source the latter. This causes issues with selftests
from directories other than net/forwarding/, in particular drivers/net/...
For those files, the line "source ../lib.sh" would look for lib.sh in the
directory above the script file's one, which is incorrect.
Instead, use $BASH_SOURCE and dirname to determine which directory the
forwarding/lib.sh resides in, and use that to find net/lib.sh.
Some selftests (namely drivers/net/bonding) use a symbolic link to
forwarding/lib.sh, and source that instead. Therefore pass $BASH_SOURCE
through readlink to resolve to the ultimate file.
Fixes: 25ae948b4478 ("selftests/net: add lib.sh")
Suggested-by: Hangbin Liu <liuhangbin@gmail.com>
Signed-off-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index e3740163c384..f9e32152f23d 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -38,7 +38,9 @@ if [[ -f $relative_path/forwarding.config ]]; then
source "$relative_path/forwarding.config"
fi
-source ../lib.sh
+libdir=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
+source "$libdir"/../lib.sh
+
##############################################################################
# Sanity checks
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH net-next 04/10] selftests: forwarding: Simplify forwarding.config import logic
2023-12-22 13:58 [RFC PATCH net-next 00/10] selftests: Add TEST_INCLUDES directive and adjust tests to use it Benjamin Poirier
` (2 preceding siblings ...)
2023-12-22 13:58 ` [RFC PATCH net-next 03/10] selftests: forwarding: Import top-level lib.sh through absolute path Benjamin Poirier
@ 2023-12-22 13:58 ` Benjamin Poirier
2023-12-27 19:27 ` Vladimir Oltean
2023-12-22 13:58 ` [RFC PATCH net-next 05/10] selftests: Introduce Makefile variable to list shared bash scripts Benjamin Poirier
` (6 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Benjamin Poirier @ 2023-12-22 13:58 UTC (permalink / raw)
To: netdev; +Cc: Shuah Khan, Petr Machata, Hangbin Liu, Vladimir Oltean
The first condition removed by this patch reimplements functionality that
is part of `dirname`:
$ dirname ""
.
Use the libdir variable introduced in the previous patch to import
forwarding.config without duplicating functionality.
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
tools/testing/selftests/net/forwarding/lib.sh | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index f9e32152f23d..481d9b655a40 100644
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -29,16 +29,12 @@ STABLE_MAC_ADDRS=${STABLE_MAC_ADDRS:=no}
TCPDUMP_EXTRA_FLAGS=${TCPDUMP_EXTRA_FLAGS:=}
TROUTE6=${TROUTE6:=traceroute6}
-relative_path="${BASH_SOURCE%/*}"
-if [[ "$relative_path" == "${BASH_SOURCE}" ]]; then
- relative_path="."
-fi
-
-if [[ -f $relative_path/forwarding.config ]]; then
- source "$relative_path/forwarding.config"
-fi
-
libdir=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
+
+if [ -f "$libdir"/forwarding.config ]; then
+ source "$libdir"/forwarding.config
+fi
+
source "$libdir"/../lib.sh
##############################################################################
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [RFC PATCH net-next 04/10] selftests: forwarding: Simplify forwarding.config import logic
2023-12-22 13:58 ` [RFC PATCH net-next 04/10] selftests: forwarding: Simplify forwarding.config import logic Benjamin Poirier
@ 2023-12-27 19:27 ` Vladimir Oltean
2023-12-28 19:07 ` Benjamin Poirier
0 siblings, 1 reply; 22+ messages in thread
From: Vladimir Oltean @ 2023-12-27 19:27 UTC (permalink / raw)
To: Benjamin Poirier; +Cc: netdev, Shuah Khan, Petr Machata, Hangbin Liu
On Fri, Dec 22, 2023 at 08:58:30AM -0500, Benjamin Poirier wrote:
> The first condition removed by this patch reimplements functionality that
> is part of `dirname`:
> $ dirname ""
> .
>
> Use the libdir variable introduced in the previous patch to import
> forwarding.config without duplicating functionality.
>
> Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
> ---
> tools/testing/selftests/net/forwarding/lib.sh | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> index f9e32152f23d..481d9b655a40 100644
> --- a/tools/testing/selftests/net/forwarding/lib.sh
> +++ b/tools/testing/selftests/net/forwarding/lib.sh
> @@ -29,16 +29,12 @@ STABLE_MAC_ADDRS=${STABLE_MAC_ADDRS:=no}
> TCPDUMP_EXTRA_FLAGS=${TCPDUMP_EXTRA_FLAGS:=}
> TROUTE6=${TROUTE6:=traceroute6}
>
> -relative_path="${BASH_SOURCE%/*}"
> -if [[ "$relative_path" == "${BASH_SOURCE}" ]]; then
> - relative_path="."
> -fi
> -
> -if [[ -f $relative_path/forwarding.config ]]; then
> - source "$relative_path/forwarding.config"
> -fi
> -
> libdir=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
> +
> +if [ -f "$libdir"/forwarding.config ]; then
> + source "$libdir"/forwarding.config
Nitpick: this used to be indented with tabs, not spaces. Also, any
reason why only "$libdir" is quoted and not the full path, as before?
Otherwise:
Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> +fi
> +
> source "$libdir"/../lib.sh
>
> ##############################################################################
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH net-next 04/10] selftests: forwarding: Simplify forwarding.config import logic
2023-12-27 19:27 ` Vladimir Oltean
@ 2023-12-28 19:07 ` Benjamin Poirier
0 siblings, 0 replies; 22+ messages in thread
From: Benjamin Poirier @ 2023-12-28 19:07 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: netdev, Shuah Khan, Petr Machata, Hangbin Liu
On 2023-12-27 21:27 +0200, Vladimir Oltean wrote:
> On Fri, Dec 22, 2023 at 08:58:30AM -0500, Benjamin Poirier wrote:
> > The first condition removed by this patch reimplements functionality that
> > is part of `dirname`:
> > $ dirname ""
> > .
> >
> > Use the libdir variable introduced in the previous patch to import
> > forwarding.config without duplicating functionality.
> >
> > Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
> > ---
> > tools/testing/selftests/net/forwarding/lib.sh | 14 +++++---------
> > 1 file changed, 5 insertions(+), 9 deletions(-)
> >
> > diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
> > index f9e32152f23d..481d9b655a40 100644
> > --- a/tools/testing/selftests/net/forwarding/lib.sh
> > +++ b/tools/testing/selftests/net/forwarding/lib.sh
> > @@ -29,16 +29,12 @@ STABLE_MAC_ADDRS=${STABLE_MAC_ADDRS:=no}
> > TCPDUMP_EXTRA_FLAGS=${TCPDUMP_EXTRA_FLAGS:=}
> > TROUTE6=${TROUTE6:=traceroute6}
> >
> > -relative_path="${BASH_SOURCE%/*}"
> > -if [[ "$relative_path" == "${BASH_SOURCE}" ]]; then
> > - relative_path="."
> > -fi
> > -
> > -if [[ -f $relative_path/forwarding.config ]]; then
> > - source "$relative_path/forwarding.config"
> > -fi
> > -
> > libdir=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
> > +
> > +if [ -f "$libdir"/forwarding.config ]; then
> > + source "$libdir"/forwarding.config
>
> Nitpick: this used to be indented with tabs, not spaces.
Thank you for pointing it out. I have fixed it.
> Also, any
> reason why only "$libdir" is quoted and not the full path, as before?
It's not necessary to quote "/forwarding.config" since it doesn't expand
or split, so I did not quote it. Also, the previous code was
inconsistent in its quoting.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC PATCH net-next 05/10] selftests: Introduce Makefile variable to list shared bash scripts
2023-12-22 13:58 [RFC PATCH net-next 00/10] selftests: Add TEST_INCLUDES directive and adjust tests to use it Benjamin Poirier
` (3 preceding siblings ...)
2023-12-22 13:58 ` [RFC PATCH net-next 04/10] selftests: forwarding: Simplify forwarding.config import logic Benjamin Poirier
@ 2023-12-22 13:58 ` Benjamin Poirier
2023-12-27 19:43 ` Vladimir Oltean
2023-12-22 13:58 ` [RFC PATCH net-next 06/10] selftests: forwarding: Add net/lib.sh to TEST_INCLUDES Benjamin Poirier
` (5 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Benjamin Poirier @ 2023-12-22 13:58 UTC (permalink / raw)
To: netdev; +Cc: Shuah Khan, Petr Machata, Hangbin Liu, Vladimir Oltean
Some tests written in bash source other files in a parent directory. For
example, drivers/net/bonding/dev_addr_lists.sh sources
net/forwarding/lib.sh. If a subset of tests is exported and run outside the
source tree (for example by using `make -C tools/testing/selftests gen_tar
TARGETS="drivers/net/bonding"`), these other files must be made available
as well.
Commit ae108c48b5d2 ("selftests: net: Fix cross-tree inclusion of scripts")
addressed this problem by symlinking and copying the sourced files but this
only works for direct dependencies. Commit 25ae948b4478 ("selftests/net:
add lib.sh") changed net/forwarding/lib.sh to source net/lib.sh. As a
result, that latter file must be included as well when the former is
exported. This is not handled currently. Therefore, add a mechanism to list
dependent files in a new Makefile variable and export them. This allows
sourcing those files using the same expression whether tests are run
in-tree or exported.
Dependencies are not resolved recursively so transitive dependencies must
be listed in TEST_INCLUDES. For example, net/forwarding/lib.sh sources
net/lib.sh; so a script that sources net/forwarding/lib.sh from a parent
directory must list:
TEST_INCLUDES := \
net/forwarding/lib.sh \
net/lib.sh
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
Documentation/dev-tools/kselftest.rst | 6 ++++++
tools/testing/selftests/Makefile | 7 ++++++-
tools/testing/selftests/lib.mk | 6 ++++++
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
index ab376b316c36..8b79843ca514 100644
--- a/Documentation/dev-tools/kselftest.rst
+++ b/Documentation/dev-tools/kselftest.rst
@@ -255,9 +255,15 @@ Contributing new tests (details)
TEST_PROGS_EXTENDED, TEST_GEN_PROGS_EXTENDED mean it is the
executable which is not tested by default.
+
TEST_FILES, TEST_GEN_FILES mean it is the file which is used by
test.
+ TEST_INCLUDES lists files which are not in the current directory or one of
+ its descendants but which should be included when exporting or installing
+ the tests. The files are listed with a path relative to
+ tools/testing/selftests/.
+
* First use the headers inside the kernel source and/or git repo, and then the
system headers. Headers for the kernel release as opposed to headers
installed by the distro on the system should be the primary focus to be able
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 81f094e7f0f7..3f494a31b479 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -188,6 +188,8 @@ run_tests: all
@for TARGET in $(TARGETS); do \
BUILD_TARGET=$$BUILD/$$TARGET; \
$(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET run_tests \
+ SRC_PATH=$(shell pwd) \
+ OBJ_PATH=$(BUILD) \
O=$(abs_objtree); \
done;
@@ -238,7 +240,10 @@ ifdef INSTALL_PATH
@ret=1; \
for TARGET in $(TARGETS); do \
BUILD_TARGET=$$BUILD/$$TARGET; \
- $(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET INSTALL_PATH=$(INSTALL_PATH)/$$TARGET install \
+ $(MAKE) install OUTPUT=$$BUILD_TARGET -C $$TARGET \
+ INSTALL_PATH=$(INSTALL_PATH)/$$TARGET \
+ SRC_PATH=$(shell pwd) \
+ OBJ_PATH=$(INSTALL_PATH) \
O=$(abs_objtree) \
$(if $(FORCE_TARGETS),|| exit); \
ret=$$((ret * $$?)); \
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index aa646e0661f3..2b6c2be4f356 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -74,6 +74,9 @@ ifdef building_out_of_srctree
@if [ "X$(TEST_PROGS)$(TEST_PROGS_EXTENDED)$(TEST_FILES)" != "X" ]; then \
rsync -aq --copy-unsafe-links $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(OUTPUT); \
fi
+ $(if $(TEST_INCLUDES), \
+ cd $(SRC_PATH) && rsync -aR $(TEST_INCLUDES) $(OBJ_PATH)/ \
+ )
@if [ "X$(TEST_PROGS)" != "X" ]; then \
$(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) \
$(addprefix $(OUTPUT)/,$(TEST_PROGS))) ; \
@@ -103,6 +106,9 @@ endef
install: all
ifdef INSTALL_PATH
$(INSTALL_RULE)
+ $(if $(TEST_INCLUDES), \
+ cd $(SRC_PATH) && rsync -aR $(TEST_INCLUDES) $(OBJ_PATH)/ \
+ )
else
$(error Error: set INSTALL_PATH to use install)
endif
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [RFC PATCH net-next 05/10] selftests: Introduce Makefile variable to list shared bash scripts
2023-12-22 13:58 ` [RFC PATCH net-next 05/10] selftests: Introduce Makefile variable to list shared bash scripts Benjamin Poirier
@ 2023-12-27 19:43 ` Vladimir Oltean
2023-12-27 19:47 ` Vladimir Oltean
0 siblings, 1 reply; 22+ messages in thread
From: Vladimir Oltean @ 2023-12-27 19:43 UTC (permalink / raw)
To: Benjamin Poirier; +Cc: netdev, Shuah Khan, Petr Machata, Hangbin Liu
On Fri, Dec 22, 2023 at 08:58:31AM -0500, Benjamin Poirier wrote:
> diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
> index ab376b316c36..8b79843ca514 100644
> --- a/Documentation/dev-tools/kselftest.rst
> +++ b/Documentation/dev-tools/kselftest.rst
> @@ -255,9 +255,15 @@ Contributing new tests (details)
>
> TEST_PROGS_EXTENDED, TEST_GEN_PROGS_EXTENDED mean it is the
> executable which is not tested by default.
> +
> TEST_FILES, TEST_GEN_FILES mean it is the file which is used by
> test.
>
> + TEST_INCLUDES lists files which are not in the current directory or one of
> + its descendants but which should be included when exporting or installing
> + the tests. The files are listed with a path relative to
> + tools/testing/selftests/.
> +
> * First use the headers inside the kernel source and/or git repo, and then the
> system headers. Headers for the kernel release as opposed to headers
> installed by the distro on the system should be the primary focus to be able
I've never had to touch this infrastructure, but the fact that TEST_INCLUDES
is relative to tools/testing/selftests/ when all other TEST_* variables
are relative to $PWD seems ... inconsistent?
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH net-next 05/10] selftests: Introduce Makefile variable to list shared bash scripts
2023-12-27 19:43 ` Vladimir Oltean
@ 2023-12-27 19:47 ` Vladimir Oltean
2023-12-28 19:28 ` Benjamin Poirier
0 siblings, 1 reply; 22+ messages in thread
From: Vladimir Oltean @ 2023-12-27 19:47 UTC (permalink / raw)
To: Benjamin Poirier; +Cc: netdev, Shuah Khan, Petr Machata, Hangbin Liu
On Wed, Dec 27, 2023 at 09:43:56PM +0200, Vladimir Oltean wrote:
> On Fri, Dec 22, 2023 at 08:58:31AM -0500, Benjamin Poirier wrote:
> > diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
> > index ab376b316c36..8b79843ca514 100644
> > --- a/Documentation/dev-tools/kselftest.rst
> > +++ b/Documentation/dev-tools/kselftest.rst
> > @@ -255,9 +255,15 @@ Contributing new tests (details)
> >
> > TEST_PROGS_EXTENDED, TEST_GEN_PROGS_EXTENDED mean it is the
> > executable which is not tested by default.
> > +
> > TEST_FILES, TEST_GEN_FILES mean it is the file which is used by
> > test.
> >
> > + TEST_INCLUDES lists files which are not in the current directory or one of
> > + its descendants but which should be included when exporting or installing
> > + the tests. The files are listed with a path relative to
> > + tools/testing/selftests/.
> > +
> > * First use the headers inside the kernel source and/or git repo, and then the
> > system headers. Headers for the kernel release as opposed to headers
> > installed by the distro on the system should be the primary focus to be able
>
> I've never had to touch this infrastructure, but the fact that TEST_INCLUDES
> is relative to tools/testing/selftests/ when all other TEST_* variables
> are relative to $PWD seems ... inconsistent?
To solve the inconsistency, can it be used like this everywhere?
TEST_INCLUDES := \
$(SRC_PATH)/net/lib.sh
(I haven't tried this)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH net-next 05/10] selftests: Introduce Makefile variable to list shared bash scripts
2023-12-27 19:47 ` Vladimir Oltean
@ 2023-12-28 19:28 ` Benjamin Poirier
2023-12-28 20:38 ` Vladimir Oltean
0 siblings, 1 reply; 22+ messages in thread
From: Benjamin Poirier @ 2023-12-28 19:28 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: netdev, Shuah Khan, Petr Machata, Hangbin Liu
On 2023-12-27 21:47 +0200, Vladimir Oltean wrote:
> On Wed, Dec 27, 2023 at 09:43:56PM +0200, Vladimir Oltean wrote:
> > On Fri, Dec 22, 2023 at 08:58:31AM -0500, Benjamin Poirier wrote:
> > > diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
> > > index ab376b316c36..8b79843ca514 100644
> > > --- a/Documentation/dev-tools/kselftest.rst
> > > +++ b/Documentation/dev-tools/kselftest.rst
> > > @@ -255,9 +255,15 @@ Contributing new tests (details)
> > >
> > > TEST_PROGS_EXTENDED, TEST_GEN_PROGS_EXTENDED mean it is the
> > > executable which is not tested by default.
> > > +
> > > TEST_FILES, TEST_GEN_FILES mean it is the file which is used by
> > > test.
> > >
> > > + TEST_INCLUDES lists files which are not in the current directory or one of
> > > + its descendants but which should be included when exporting or installing
> > > + the tests. The files are listed with a path relative to
> > > + tools/testing/selftests/.
> > > +
> > > * First use the headers inside the kernel source and/or git repo, and then the
> > > system headers. Headers for the kernel release as opposed to headers
> > > installed by the distro on the system should be the primary focus to be able
> >
> > I've never had to touch this infrastructure, but the fact that TEST_INCLUDES
> > is relative to tools/testing/selftests/ when all other TEST_* variables
> > are relative to $PWD seems ... inconsistent?
I agree with your point about consistency. I have changed TEST_INCLUDES
to take paths relative to PWD. The implementation is more complicated
since the paths have to be converted to the old values anyways for
`rsync -R` but it works. I pasted the overall diff below and it'll be
part of the next version of the series.
>
> To solve the inconsistency, can it be used like this everywhere?
>
> TEST_INCLUDES := \
> $(SRC_PATH)/net/lib.sh
After the changes, it's possible to list files using SRC_PATH but I
didn't do it. Since the point is to make TEST_INCLUDES be more like
TEST_PROGS, TEST_FILES, ..., I used relative paths.
For example in net/forwarding/Makefile:
TEST_INCLUDES := \
../lib.sh
diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst
index 8b79843ca514..470cc7913647 100644
--- a/Documentation/dev-tools/kselftest.rst
+++ b/Documentation/dev-tools/kselftest.rst
@@ -259,10 +259,14 @@ Contributing new tests (details)
TEST_FILES, TEST_GEN_FILES mean it is the file which is used by
test.
- TEST_INCLUDES lists files which are not in the current directory or one of
- its descendants but which should be included when exporting or installing
- the tests. The files are listed with a path relative to
- tools/testing/selftests/.
+ TEST_INCLUDES is similar to TEST_FILES, it lists files which should be
+ included when exporting or installing the tests, with the following
+ differences:
+ * symlinks to files in other directories are preserved
+ * the part of paths below tools/testing/selftests/ is preserved when copying
+ the files to the output directory
+ TEST_INCLUDES is meant to list dependencies located in other directories of
+ the selftests hierarchy.
* First use the headers inside the kernel source and/or git repo, and then the
system headers. Headers for the kernel release as opposed to headers
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 3f494a31b479..c289505245f5 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -188,7 +188,7 @@ run_tests: all
@for TARGET in $(TARGETS); do \
BUILD_TARGET=$$BUILD/$$TARGET; \
$(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET run_tests \
- SRC_PATH=$(shell pwd) \
+ SRC_PATH=$(shell readlink -e $$(pwd)) \
OBJ_PATH=$(BUILD) \
O=$(abs_objtree); \
done;
@@ -242,7 +242,7 @@ ifdef INSTALL_PATH
BUILD_TARGET=$$BUILD/$$TARGET; \
$(MAKE) install OUTPUT=$$BUILD_TARGET -C $$TARGET \
INSTALL_PATH=$(INSTALL_PATH)/$$TARGET \
- SRC_PATH=$(shell pwd) \
+ SRC_PATH=$(shell readlink -e $$(pwd)) \
OBJ_PATH=$(INSTALL_PATH) \
O=$(abs_objtree) \
$(if $(FORCE_TARGETS),|| exit); \
diff --git a/tools/testing/selftests/drivers/net/bonding/Makefile b/tools/testing/selftests/drivers/net/bonding/Makefile
index a61fe339b9be..03a089165d3f 100644
--- a/tools/testing/selftests/drivers/net/bonding/Makefile
+++ b/tools/testing/selftests/drivers/net/bonding/Makefile
@@ -18,7 +18,7 @@ TEST_FILES := \
bond_topo_3d1c.sh
TEST_INCLUDES := \
- net/forwarding/lib.sh \
- net/lib.sh
+ ../../../net/forwarding/lib.sh \
+ ../../../net/lib.sh
include ../../../lib.mk
diff --git a/tools/testing/selftests/drivers/net/dsa/Makefile b/tools/testing/selftests/drivers/net/dsa/Makefile
index 8259eac80c3b..cd6817fe5be6 100644
--- a/tools/testing/selftests/drivers/net/dsa/Makefile
+++ b/tools/testing/selftests/drivers/net/dsa/Makefile
@@ -16,17 +16,17 @@ TEST_FILES := \
forwarding.config
TEST_INCLUDES := \
- net/forwarding/bridge_locked_port.sh \
- net/forwarding/bridge_mdb.sh \
- net/forwarding/bridge_mld.sh \
- net/forwarding/bridge_vlan_aware.sh \
- net/forwarding/bridge_vlan_mcast.sh \
- net/forwarding/bridge_vlan_unaware.sh \
- net/forwarding/lib.sh \
- net/forwarding/local_termination.sh \
- net/forwarding/no_forwarding.sh \
- net/forwarding/tc_actions.sh \
- net/forwarding/tc_common.sh \
- net/lib.sh
+ ../../../net/forwarding/bridge_locked_port.sh \
+ ../../../net/forwarding/bridge_mdb.sh \
+ ../../../net/forwarding/bridge_mld.sh \
+ ../../../net/forwarding/bridge_vlan_aware.sh \
+ ../../../net/forwarding/bridge_vlan_mcast.sh \
+ ../../../net/forwarding/bridge_vlan_unaware.sh \
+ ../../../net/forwarding/lib.sh \
+ ../../../net/forwarding/local_termination.sh \
+ ../../../net/forwarding/no_forwarding.sh \
+ ../../../net/forwarding/tc_actions.sh \
+ ../../../net/forwarding/tc_common.sh \
+ ../../../net/lib.sh
include ../../../lib.mk
diff --git a/tools/testing/selftests/drivers/net/team/Makefile b/tools/testing/selftests/drivers/net/team/Makefile
index 8a9846b5a209..2d5a76d99181 100644
--- a/tools/testing/selftests/drivers/net/team/Makefile
+++ b/tools/testing/selftests/drivers/net/team/Makefile
@@ -4,8 +4,8 @@
TEST_PROGS := dev_addr_lists.sh
TEST_INCLUDES := \
- drivers/net/bonding/lag_lib.sh \
- net/forwarding/lib.sh \
- net/lib.sh
+ ../bonding/lag_lib.sh \
+ ../../../net/forwarding/lib.sh \
+ ../../../net/lib.sh
include ../../../lib.mk
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 2b6c2be4f356..087fee22dd53 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -69,14 +69,29 @@ define RUN_TESTS
run_many $(1)
endef
+define INSTALL_INCLUDES
+ $(if $(TEST_INCLUDES), \
+ relative_files=""; \
+ for entry in $(TEST_INCLUDES); do \
+ entry_dir=$$(readlink -e "$$(dirname "$$entry")"); \
+ entry_name=$$(basename "$$entry"); \
+ relative_dir=$${entry_dir#"$$SRC_PATH"/}; \
+ if [ "$$relative_dir" = "$$entry_dir" ]; then \
+ echo "Error: TEST_INCLUDES entry \"$$entry\" not located inside selftests directory ($$SRC_PATH)" >&2; \
+ exit 1; \
+ fi; \
+ relative_files="$$relative_files $$relative_dir/$$entry_name"; \
+ done; \
+ cd $(SRC_PATH) && rsync -aR $$relative_files $(OBJ_PATH)/ \
+ )
+endef
+
run_tests: all
ifdef building_out_of_srctree
@if [ "X$(TEST_PROGS)$(TEST_PROGS_EXTENDED)$(TEST_FILES)" != "X" ]; then \
rsync -aq --copy-unsafe-links $(TEST_PROGS) $(TEST_PROGS_EXTENDED) $(TEST_FILES) $(OUTPUT); \
fi
- $(if $(TEST_INCLUDES), \
- cd $(SRC_PATH) && rsync -aR $(TEST_INCLUDES) $(OBJ_PATH)/ \
- )
+ @$(INSTALL_INCLUDES)
@if [ "X$(TEST_PROGS)" != "X" ]; then \
$(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) \
$(addprefix $(OUTPUT)/,$(TEST_PROGS))) ; \
@@ -106,9 +121,7 @@ endef
install: all
ifdef INSTALL_PATH
$(INSTALL_RULE)
- $(if $(TEST_INCLUDES), \
- cd $(SRC_PATH) && rsync -aR $(TEST_INCLUDES) $(OBJ_PATH)/ \
- )
+ $(INSTALL_INCLUDES)
else
$(error Error: set INSTALL_PATH to use install)
endif
diff --git a/tools/testing/selftests/net/forwarding/Makefile b/tools/testing/selftests/net/forwarding/Makefile
index 5b55c0467eed..1fba2717738d 100644
--- a/tools/testing/selftests/net/forwarding/Makefile
+++ b/tools/testing/selftests/net/forwarding/Makefile
@@ -130,6 +130,6 @@ TEST_PROGS_EXTENDED := devlink_lib.sh \
tc_common.sh
TEST_INCLUDES := \
- net/lib.sh
+ ../lib.sh
include ../../lib.mk
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [RFC PATCH net-next 05/10] selftests: Introduce Makefile variable to list shared bash scripts
2023-12-28 19:28 ` Benjamin Poirier
@ 2023-12-28 20:38 ` Vladimir Oltean
0 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2023-12-28 20:38 UTC (permalink / raw)
To: Benjamin Poirier; +Cc: netdev, Shuah Khan, Petr Machata, Hangbin Liu
On Thu, Dec 28, 2023 at 02:28:06PM -0500, Benjamin Poirier wrote:
> I agree with your point about consistency. I have changed TEST_INCLUDES
> to take paths relative to PWD. The implementation is more complicated
> since the paths have to be converted to the old values anyways for
> `rsync -R` but it works. I pasted the overall diff below and it'll be
> part of the next version of the series.
>
> >
> > To solve the inconsistency, can it be used like this everywhere?
> >
> > TEST_INCLUDES := \
> > $(SRC_PATH)/net/lib.sh
>
> After the changes, it's possible to list files using SRC_PATH but I
> didn't do it. Since the point is to make TEST_INCLUDES be more like
> TEST_PROGS, TEST_FILES, ..., I used relative paths.
> For example in net/forwarding/Makefile:
> TEST_INCLUDES := \
> ../lib.sh
I thought you wanted to avoid the cascade of ../../../ which is confusing,
so I recommended $(SRC_PATH) as a way to avoid that by using absolute
paths instead - which makes it easier to track down the include file
being sourced.
My inconsistency issue was with TEST_INCLUDES being relative to a
different directory than the rest. With that being addressed,
I don't think that using absolute paths for some files would be
inconsistent in a similar way. But I don't mind too much either way.
Feel free to keep the ../../../ scheme.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC PATCH net-next 06/10] selftests: forwarding: Add net/lib.sh to TEST_INCLUDES
2023-12-22 13:58 [RFC PATCH net-next 00/10] selftests: Add TEST_INCLUDES directive and adjust tests to use it Benjamin Poirier
` (4 preceding siblings ...)
2023-12-22 13:58 ` [RFC PATCH net-next 05/10] selftests: Introduce Makefile variable to list shared bash scripts Benjamin Poirier
@ 2023-12-22 13:58 ` Benjamin Poirier
2023-12-22 13:58 ` [RFC PATCH net-next 07/10] selftests: bonding: Add lib.sh scripts " Benjamin Poirier
` (4 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Benjamin Poirier @ 2023-12-22 13:58 UTC (permalink / raw)
To: netdev; +Cc: Shuah Khan, Petr Machata, Hangbin Liu, Vladimir Oltean
Since commit 25ae948b4478 ("selftests/net: add lib.sh"), when exporting the
forwarding tests and running them, the tests that import
net/forwarding/lib.sh fail to import net/lib.sh. This is especially a
problem for the tests that use functions from net/lib.sh, like pedit_ip.sh:
$ make install TARGETS="drivers/net/forwarding"
$ cd kselftest_install/net/forwarding
$ ./pedit_ip.sh veth{0..3}
lib.sh: line 38: /src/linux/tools/testing/selftests/kselftest_install/net/forwarding/../lib.sh: No such file or directory
TEST: ping [ OK ]
TEST: ping6 [ OK ]
./pedit_ip.sh: line 135: busywait: command not found
TEST: dev veth1 ingress pedit ip src set 198.51.100.1 [FAIL]
Expected to get 10 packets, but got .
./pedit_ip.sh: line 135: busywait: command not found
TEST: dev veth2 egress pedit ip src set 198.51.100.1 [FAIL]
Expected to get 10 packets, but got .
./pedit_ip.sh: line 135: busywait: command not found
TEST: dev veth1 ingress pedit ip dst set 198.51.100.1 [FAIL]
Expected to get 10 packets, but got .
./pedit_ip.sh: line 135: busywait: command not found
TEST: dev veth2 egress pedit ip dst set 198.51.100.1 [FAIL]
Expected to get 10 packets, but got .
./pedit_ip.sh: line 135: busywait: command not found
TEST: dev veth1 ingress pedit ip6 src set 2001:db8:2::1 [FAIL]
Expected to get 10 packets, but got .
./pedit_ip.sh: line 135: busywait: command not found
TEST: dev veth2 egress pedit ip6 src set 2001:db8:2::1 [FAIL]
Expected to get 10 packets, but got .
./pedit_ip.sh: line 135: busywait: command not found
TEST: dev veth1 ingress pedit ip6 dst set 2001:db8:2::1 [FAIL]
Expected to get 10 packets, but got .
./pedit_ip.sh: line 135: busywait: command not found
TEST: dev veth2 egress pedit ip6 dst set 2001:db8:2::1 [FAIL]
Expected to get 10 packets, but got .
In order for net/lib.sh to be exported along with forwarding tests, add
net/lib.sh to TEST_INCLUDES. This is the result after this commit:
$ ./pedit_ip.sh veth{0..3}
TEST: ping [ OK ]
TEST: ping6 [ OK ]
TEST: dev veth1 ingress pedit ip src set 198.51.100.1 [ OK ]
TEST: dev veth2 egress pedit ip src set 198.51.100.1 [ OK ]
TEST: dev veth1 ingress pedit ip dst set 198.51.100.1 [ OK ]
TEST: dev veth2 egress pedit ip dst set 198.51.100.1 [ OK ]
TEST: dev veth1 ingress pedit ip6 src set 2001:db8:2::1 [ OK ]
TEST: dev veth2 egress pedit ip6 src set 2001:db8:2::1 [ OK ]
TEST: dev veth1 ingress pedit ip6 dst set 2001:db8:2::1 [ OK ]
TEST: dev veth2 egress pedit ip6 dst set 2001:db8:2::1 [ OK ]
Fixes: 25ae948b4478 ("selftests/net: add lib.sh")
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
tools/testing/selftests/net/forwarding/Makefile | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/net/forwarding/Makefile b/tools/testing/selftests/net/forwarding/Makefile
index 452693514be4..5b55c0467eed 100644
--- a/tools/testing/selftests/net/forwarding/Makefile
+++ b/tools/testing/selftests/net/forwarding/Makefile
@@ -129,4 +129,7 @@ TEST_PROGS_EXTENDED := devlink_lib.sh \
sch_tbf_etsprio.sh \
tc_common.sh
+TEST_INCLUDES := \
+ net/lib.sh
+
include ../../lib.mk
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH net-next 07/10] selftests: bonding: Add lib.sh scripts to TEST_INCLUDES
2023-12-22 13:58 [RFC PATCH net-next 00/10] selftests: Add TEST_INCLUDES directive and adjust tests to use it Benjamin Poirier
` (5 preceding siblings ...)
2023-12-22 13:58 ` [RFC PATCH net-next 06/10] selftests: forwarding: Add net/lib.sh to TEST_INCLUDES Benjamin Poirier
@ 2023-12-22 13:58 ` Benjamin Poirier
2023-12-22 13:58 ` [RFC PATCH net-next 08/10] selftests: team: " Benjamin Poirier
` (3 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Benjamin Poirier @ 2023-12-22 13:58 UTC (permalink / raw)
To: netdev; +Cc: Shuah Khan, Petr Machata, Hangbin Liu, Vladimir Oltean
Since commit 25ae948b4478 ("selftests/net: add lib.sh"), when exporting the
bonding tests and running them, the tests that import net/forwarding/lib.sh
fail to import net/lib.sh. This prints an error message but since the
bonding tests do not use functions from net/lib.sh, this does not affect
the test results.
Example:
kselftest_install# ./run_kselftest.sh -t drivers/net/bonding:dev_addr_lists.sh
TAP version 13
1..1
# timeout set to 120
# selftests: drivers/net/bonding: dev_addr_lists.sh
# ./net_forwarding_lib.sh: line 38: /src/linux/tools/testing/selftests/kselftest_install/drivers/net/bonding/../lib.sh: No such file or directory
# TEST: bonding cleanup mode active-backup [ OK ]
# TEST: bonding cleanup mode 802.3ad [ OK ]
# TEST: bonding LACPDU multicast address to slave (from bond down) [ OK ]
# TEST: bonding LACPDU multicast address to slave (from bond up) [ OK ]
ok 1 selftests: drivers/net/bonding: dev_addr_lists.sh
In order to avoid the error message, net/forwarding/lib.sh is exported and
included via its relative path and net/lib.sh is also exported.
Fixes: 25ae948b4478 ("selftests/net: add lib.sh")
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
tools/testing/selftests/drivers/net/bonding/Makefile | 7 +++++--
.../selftests/drivers/net/bonding/bond-eth-type-change.sh | 2 +-
.../selftests/drivers/net/bonding/bond_topo_2d1c.sh | 2 +-
.../selftests/drivers/net/bonding/dev_addr_lists.sh | 2 +-
.../drivers/net/bonding/mode-1-recovery-updelay.sh | 2 +-
.../drivers/net/bonding/mode-2-recovery-updelay.sh | 2 +-
.../selftests/drivers/net/bonding/net_forwarding_lib.sh | 1 -
7 files changed, 10 insertions(+), 8 deletions(-)
delete mode 120000 tools/testing/selftests/drivers/net/bonding/net_forwarding_lib.sh
diff --git a/tools/testing/selftests/drivers/net/bonding/Makefile b/tools/testing/selftests/drivers/net/bonding/Makefile
index 8a72bb7de70f..a61fe339b9be 100644
--- a/tools/testing/selftests/drivers/net/bonding/Makefile
+++ b/tools/testing/selftests/drivers/net/bonding/Makefile
@@ -15,7 +15,10 @@ TEST_PROGS := \
TEST_FILES := \
lag_lib.sh \
bond_topo_2d1c.sh \
- bond_topo_3d1c.sh \
- net_forwarding_lib.sh
+ bond_topo_3d1c.sh
+
+TEST_INCLUDES := \
+ net/forwarding/lib.sh \
+ net/lib.sh
include ../../../lib.mk
diff --git a/tools/testing/selftests/drivers/net/bonding/bond-eth-type-change.sh b/tools/testing/selftests/drivers/net/bonding/bond-eth-type-change.sh
index 862e947e17c7..8293dbc7c18f 100755
--- a/tools/testing/selftests/drivers/net/bonding/bond-eth-type-change.sh
+++ b/tools/testing/selftests/drivers/net/bonding/bond-eth-type-change.sh
@@ -11,7 +11,7 @@ ALL_TESTS="
REQUIRE_MZ=no
NUM_NETIFS=0
lib_dir=$(dirname "$0")
-source "$lib_dir"/net_forwarding_lib.sh
+source "$lib_dir"/../../../net/forwarding/lib.sh
bond_check_flags()
{
diff --git a/tools/testing/selftests/drivers/net/bonding/bond_topo_2d1c.sh b/tools/testing/selftests/drivers/net/bonding/bond_topo_2d1c.sh
index a509ef949dcf..0eb7edfb584c 100644
--- a/tools/testing/selftests/drivers/net/bonding/bond_topo_2d1c.sh
+++ b/tools/testing/selftests/drivers/net/bonding/bond_topo_2d1c.sh
@@ -28,7 +28,7 @@
REQUIRE_MZ=no
NUM_NETIFS=0
lib_dir=$(dirname "$0")
-source ${lib_dir}/net_forwarding_lib.sh
+source "$lib_dir"/../../../net/forwarding/lib.sh
s_ns="s-$(mktemp -u XXXXXX)"
c_ns="c-$(mktemp -u XXXXXX)"
diff --git a/tools/testing/selftests/drivers/net/bonding/dev_addr_lists.sh b/tools/testing/selftests/drivers/net/bonding/dev_addr_lists.sh
index 5cfe7d8ebc25..e6fa24eded5b 100755
--- a/tools/testing/selftests/drivers/net/bonding/dev_addr_lists.sh
+++ b/tools/testing/selftests/drivers/net/bonding/dev_addr_lists.sh
@@ -14,7 +14,7 @@ ALL_TESTS="
REQUIRE_MZ=no
NUM_NETIFS=0
lib_dir=$(dirname "$0")
-source "$lib_dir"/net_forwarding_lib.sh
+source "$lib_dir"/../../../net/forwarding/lib.sh
source "$lib_dir"/lag_lib.sh
diff --git a/tools/testing/selftests/drivers/net/bonding/mode-1-recovery-updelay.sh b/tools/testing/selftests/drivers/net/bonding/mode-1-recovery-updelay.sh
index b76bf5030952..9d26ab4cad0b 100755
--- a/tools/testing/selftests/drivers/net/bonding/mode-1-recovery-updelay.sh
+++ b/tools/testing/selftests/drivers/net/bonding/mode-1-recovery-updelay.sh
@@ -23,7 +23,7 @@ REQUIRE_MZ=no
REQUIRE_JQ=no
NUM_NETIFS=0
lib_dir=$(dirname "$0")
-source "$lib_dir"/net_forwarding_lib.sh
+source "$lib_dir"/../../../net/forwarding/lib.sh
source "$lib_dir"/lag_lib.sh
cleanup()
diff --git a/tools/testing/selftests/drivers/net/bonding/mode-2-recovery-updelay.sh b/tools/testing/selftests/drivers/net/bonding/mode-2-recovery-updelay.sh
index 8c2619002147..2d275b3e47dd 100755
--- a/tools/testing/selftests/drivers/net/bonding/mode-2-recovery-updelay.sh
+++ b/tools/testing/selftests/drivers/net/bonding/mode-2-recovery-updelay.sh
@@ -23,7 +23,7 @@ REQUIRE_MZ=no
REQUIRE_JQ=no
NUM_NETIFS=0
lib_dir=$(dirname "$0")
-source "$lib_dir"/net_forwarding_lib.sh
+source "$lib_dir"/../../../net/forwarding/lib.sh
source "$lib_dir"/lag_lib.sh
cleanup()
diff --git a/tools/testing/selftests/drivers/net/bonding/net_forwarding_lib.sh b/tools/testing/selftests/drivers/net/bonding/net_forwarding_lib.sh
deleted file mode 120000
index 39c96828c5ef..000000000000
--- a/tools/testing/selftests/drivers/net/bonding/net_forwarding_lib.sh
+++ /dev/null
@@ -1 +0,0 @@
-../../../net/forwarding/lib.sh
\ No newline at end of file
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH net-next 08/10] selftests: team: Add lib.sh scripts to TEST_INCLUDES
2023-12-22 13:58 [RFC PATCH net-next 00/10] selftests: Add TEST_INCLUDES directive and adjust tests to use it Benjamin Poirier
` (6 preceding siblings ...)
2023-12-22 13:58 ` [RFC PATCH net-next 07/10] selftests: bonding: Add lib.sh scripts " Benjamin Poirier
@ 2023-12-22 13:58 ` Benjamin Poirier
2023-12-22 13:58 ` [RFC PATCH net-next 09/10] selftests: team: Add shared library script " Benjamin Poirier
` (2 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Benjamin Poirier @ 2023-12-22 13:58 UTC (permalink / raw)
To: netdev; +Cc: Shuah Khan, Petr Machata, Hangbin Liu, Vladimir Oltean
Since commit 25ae948b4478 ("selftests/net: add lib.sh"), when exporting the
team test and running it, the test fails to import net/lib.sh. This prints
an error message but since the test does not use functions from net/lib.sh,
this does not affect the test result.
Example:
# make install TARGETS="drivers/net/team"
# kselftest_install/run_kselftest.sh
TAP version 13
1..1
# timeout set to 45
# selftests: drivers/net/team: dev_addr_lists.sh
# ./net_forwarding_lib.sh: line 38: /src/linux/tools/testing/selftests/kselftest_install/drivers/net/team/../lib.sh:
No such file or directory
# This program is not intended to be run as root.
# TEST: team cleanup mode lacp [ OK ]
ok 1 selftests: drivers/net/team: dev_addr_lists.sh
In order to avoid the error message, net/forwarding/lib.sh is exported and
included via its relative path and net/lib.sh is also exported.
Fixes: 25ae948b4478 ("selftests/net: add lib.sh")
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
tools/testing/selftests/drivers/net/team/Makefile | 5 ++++-
tools/testing/selftests/drivers/net/team/dev_addr_lists.sh | 2 +-
.../testing/selftests/drivers/net/team/net_forwarding_lib.sh | 1 -
3 files changed, 5 insertions(+), 3 deletions(-)
delete mode 120000 tools/testing/selftests/drivers/net/team/net_forwarding_lib.sh
diff --git a/tools/testing/selftests/drivers/net/team/Makefile b/tools/testing/selftests/drivers/net/team/Makefile
index 6a86e61e8bfe..d31af127ca29 100644
--- a/tools/testing/selftests/drivers/net/team/Makefile
+++ b/tools/testing/selftests/drivers/net/team/Makefile
@@ -5,6 +5,9 @@ TEST_PROGS := dev_addr_lists.sh
TEST_FILES := \
lag_lib.sh \
- net_forwarding_lib.sh
+
+TEST_INCLUDES := \
+ net/forwarding/lib.sh \
+ net/lib.sh
include ../../../lib.mk
diff --git a/tools/testing/selftests/drivers/net/team/dev_addr_lists.sh b/tools/testing/selftests/drivers/net/team/dev_addr_lists.sh
index 33913112d5ca..bea2565486f7 100755
--- a/tools/testing/selftests/drivers/net/team/dev_addr_lists.sh
+++ b/tools/testing/selftests/drivers/net/team/dev_addr_lists.sh
@@ -11,7 +11,7 @@ ALL_TESTS="
REQUIRE_MZ=no
NUM_NETIFS=0
lib_dir=$(dirname "$0")
-source "$lib_dir"/net_forwarding_lib.sh
+source "$lib_dir"/../../../net/forwarding/lib.sh
source "$lib_dir"/lag_lib.sh
diff --git a/tools/testing/selftests/drivers/net/team/net_forwarding_lib.sh b/tools/testing/selftests/drivers/net/team/net_forwarding_lib.sh
deleted file mode 120000
index 39c96828c5ef..000000000000
--- a/tools/testing/selftests/drivers/net/team/net_forwarding_lib.sh
+++ /dev/null
@@ -1 +0,0 @@
-../../../net/forwarding/lib.sh
\ No newline at end of file
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH net-next 09/10] selftests: team: Add shared library script to TEST_INCLUDES
2023-12-22 13:58 [RFC PATCH net-next 00/10] selftests: Add TEST_INCLUDES directive and adjust tests to use it Benjamin Poirier
` (7 preceding siblings ...)
2023-12-22 13:58 ` [RFC PATCH net-next 08/10] selftests: team: " Benjamin Poirier
@ 2023-12-22 13:58 ` Benjamin Poirier
2023-12-22 13:58 ` [RFC PATCH net-next 10/10] selftests: dsa: Replace symlinks by wrapper script Benjamin Poirier
2023-12-23 9:27 ` [RFC PATCH net-next 00/10] selftests: Add TEST_INCLUDES directive and adjust tests to use it Hangbin Liu
10 siblings, 0 replies; 22+ messages in thread
From: Benjamin Poirier @ 2023-12-22 13:58 UTC (permalink / raw)
To: netdev; +Cc: Shuah Khan, Petr Machata, Hangbin Liu, Vladimir Oltean
lag_lib.sh is added to TEST_INCLUDES to avoid duplication when both the
bonding and team tests are exported together.
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
tools/testing/selftests/drivers/net/team/Makefile | 4 +---
tools/testing/selftests/drivers/net/team/dev_addr_lists.sh | 2 +-
tools/testing/selftests/drivers/net/team/lag_lib.sh | 1 -
3 files changed, 2 insertions(+), 5 deletions(-)
delete mode 120000 tools/testing/selftests/drivers/net/team/lag_lib.sh
diff --git a/tools/testing/selftests/drivers/net/team/Makefile b/tools/testing/selftests/drivers/net/team/Makefile
index d31af127ca29..8a9846b5a209 100644
--- a/tools/testing/selftests/drivers/net/team/Makefile
+++ b/tools/testing/selftests/drivers/net/team/Makefile
@@ -3,10 +3,8 @@
TEST_PROGS := dev_addr_lists.sh
-TEST_FILES := \
- lag_lib.sh \
-
TEST_INCLUDES := \
+ drivers/net/bonding/lag_lib.sh \
net/forwarding/lib.sh \
net/lib.sh
diff --git a/tools/testing/selftests/drivers/net/team/dev_addr_lists.sh b/tools/testing/selftests/drivers/net/team/dev_addr_lists.sh
index bea2565486f7..b1ec7755b783 100755
--- a/tools/testing/selftests/drivers/net/team/dev_addr_lists.sh
+++ b/tools/testing/selftests/drivers/net/team/dev_addr_lists.sh
@@ -13,7 +13,7 @@ NUM_NETIFS=0
lib_dir=$(dirname "$0")
source "$lib_dir"/../../../net/forwarding/lib.sh
-source "$lib_dir"/lag_lib.sh
+source "$lib_dir"/../bonding/lag_lib.sh
destroy()
diff --git a/tools/testing/selftests/drivers/net/team/lag_lib.sh b/tools/testing/selftests/drivers/net/team/lag_lib.sh
deleted file mode 120000
index e1347a10afde..000000000000
--- a/tools/testing/selftests/drivers/net/team/lag_lib.sh
+++ /dev/null
@@ -1 +0,0 @@
-../bonding/lag_lib.sh
\ No newline at end of file
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [RFC PATCH net-next 10/10] selftests: dsa: Replace symlinks by wrapper script
2023-12-22 13:58 [RFC PATCH net-next 00/10] selftests: Add TEST_INCLUDES directive and adjust tests to use it Benjamin Poirier
` (8 preceding siblings ...)
2023-12-22 13:58 ` [RFC PATCH net-next 09/10] selftests: team: Add shared library script " Benjamin Poirier
@ 2023-12-22 13:58 ` Benjamin Poirier
2023-12-27 20:11 ` Vladimir Oltean
2023-12-23 9:27 ` [RFC PATCH net-next 00/10] selftests: Add TEST_INCLUDES directive and adjust tests to use it Hangbin Liu
10 siblings, 1 reply; 22+ messages in thread
From: Benjamin Poirier @ 2023-12-22 13:58 UTC (permalink / raw)
To: netdev; +Cc: Shuah Khan, Petr Machata, Hangbin Liu, Vladimir Oltean
Since commit 25ae948b4478 ("selftests/net: add lib.sh"), when exporting the
dsa tests and running them, the tests which import net/forwarding/lib.sh
(via the lib.sh symlink) fail to import net/lib.sh. This is especially a
problem for the tc_actions.sh test which uses `busywait` from net/lib.sh:
$ make install TARGETS="drivers/net/dsa"
$ cd kselftest_install/drivers/net/dsa
$ ./tc_actions.sh veth{0..3}
lib.sh: line 38: /src/linux/tools/testing/selftests/kselftest_install/drivers/net/dsa/../lib.sh: No such file or directory
tc_common.sh: line 15: busywait: command not found
tc_common.sh: line 15: busywait: command not found
tc_common.sh: line 15: busywait: command not found
TEST: gact drop and ok (skip_hw) [FAIL]
Packet was not dropped
tc_common.sh: line 15: busywait: command not found
tc_common.sh: line 15: busywait: command not found
TEST: mirred egress flower redirect (skip_hw) [FAIL]
Did not match incoming redirect packet
tc_common.sh: line 15: busywait: command not found
tc_common.sh: line 15: busywait: command not found
TEST: mirred egress flower mirror (skip_hw) [FAIL]
Did not match incoming mirror packet
tc_common.sh: line 15: busywait: command not found
tc_common.sh: line 15: busywait: command not found
TEST: mirred egress matchall mirror (skip_hw) [FAIL]
Did not match incoming mirror packet
tc_common.sh: line 15: busywait: command not found
tc_common.sh: line 15: busywait: command not found
tc_common.sh: line 15: busywait: command not found
tc_common.sh: line 15: busywait: command not found
tc_common.sh: line 15: busywait: command not found
tc_common.sh: line 15: busywait: command not found
TEST: mirred_egress_to_ingress (skip_hw) [FAIL]
didn't mirror first packet
tc_common.sh: line 15: busywait: command not found
tc_common.sh: line 15: busywait: command not found
TEST: mirred_egress_to_ingress_tcp (skip_hw) [FAIL]
didn't mirred redirect ICMP
INFO: Could not test offloaded functionality
(I manually created the veth interfaces just to demonstrate running the
test.)
The dsa tests which are symlinks of tests from net/forwarding/ (like
tc_actions.sh) become regular files after export (because `rsync
--copy-unsafe-links` is used) and expect to source lib.sh
(net/forwarding/lib.sh) from the same directory. That lib.sh then expects
to source net/lib.sh from the parent directory but this does not work
because net/lib.sh is not present under drivers/net/.
Since the tests in net/forwarding/ are not meant to be copied and run from
another directory, the failure to source net/lib.sh is solved by replacing
the test symlinks by a wrapper script which runs the original tests under
net/forwarding/. The links to shared library scripts can then be removed
and all the files needed from parent directories are added to
TEST_INCLUDES.
Fixes: 25ae948b4478 ("selftests/net: add lib.sh")
Suggested-by: Hangbin Liu <liuhangbin@gmail.com>
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
---
.../testing/selftests/drivers/net/dsa/Makefile | 18 ++++++++++++++++--
.../drivers/net/dsa/bridge_locked_port.sh | 2 +-
.../selftests/drivers/net/dsa/bridge_mdb.sh | 2 +-
.../selftests/drivers/net/dsa/bridge_mld.sh | 2 +-
.../drivers/net/dsa/bridge_vlan_aware.sh | 2 +-
.../drivers/net/dsa/bridge_vlan_mcast.sh | 2 +-
.../drivers/net/dsa/bridge_vlan_unaware.sh | 2 +-
tools/testing/selftests/drivers/net/dsa/lib.sh | 1 -
.../drivers/net/dsa/local_termination.sh | 2 +-
.../selftests/drivers/net/dsa/no_forwarding.sh | 2 +-
.../drivers/net/dsa/run_net_forwarding_test.sh | 9 +++++++++
.../selftests/drivers/net/dsa/tc_actions.sh | 2 +-
.../selftests/drivers/net/dsa/tc_common.sh | 1 -
.../drivers/net/dsa/test_bridge_fdb_stress.sh | 2 +-
14 files changed, 35 insertions(+), 14 deletions(-)
delete mode 120000 tools/testing/selftests/drivers/net/dsa/lib.sh
create mode 100755 tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh
delete mode 120000 tools/testing/selftests/drivers/net/dsa/tc_common.sh
diff --git a/tools/testing/selftests/drivers/net/dsa/Makefile b/tools/testing/selftests/drivers/net/dsa/Makefile
index c393e7b73805..8259eac80c3b 100644
--- a/tools/testing/selftests/drivers/net/dsa/Makefile
+++ b/tools/testing/selftests/drivers/net/dsa/Makefile
@@ -11,8 +11,22 @@ TEST_PROGS = bridge_locked_port.sh \
tc_actions.sh \
test_bridge_fdb_stress.sh
-TEST_PROGS_EXTENDED := lib.sh tc_common.sh
+TEST_FILES := \
+ run_net_forwarding_test.sh \
+ forwarding.config
-TEST_FILES := forwarding.config
+TEST_INCLUDES := \
+ net/forwarding/bridge_locked_port.sh \
+ net/forwarding/bridge_mdb.sh \
+ net/forwarding/bridge_mld.sh \
+ net/forwarding/bridge_vlan_aware.sh \
+ net/forwarding/bridge_vlan_mcast.sh \
+ net/forwarding/bridge_vlan_unaware.sh \
+ net/forwarding/lib.sh \
+ net/forwarding/local_termination.sh \
+ net/forwarding/no_forwarding.sh \
+ net/forwarding/tc_actions.sh \
+ net/forwarding/tc_common.sh \
+ net/lib.sh
include ../../../lib.mk
diff --git a/tools/testing/selftests/drivers/net/dsa/bridge_locked_port.sh b/tools/testing/selftests/drivers/net/dsa/bridge_locked_port.sh
index f5eb940c4c7c..d16a65e7595d 120000
--- a/tools/testing/selftests/drivers/net/dsa/bridge_locked_port.sh
+++ b/tools/testing/selftests/drivers/net/dsa/bridge_locked_port.sh
@@ -1 +1 @@
-../../../net/forwarding/bridge_locked_port.sh
\ No newline at end of file
+run_net_forwarding_test.sh
\ No newline at end of file
diff --git a/tools/testing/selftests/drivers/net/dsa/bridge_mdb.sh b/tools/testing/selftests/drivers/net/dsa/bridge_mdb.sh
index 76492da525f7..d16a65e7595d 120000
--- a/tools/testing/selftests/drivers/net/dsa/bridge_mdb.sh
+++ b/tools/testing/selftests/drivers/net/dsa/bridge_mdb.sh
@@ -1 +1 @@
-../../../net/forwarding/bridge_mdb.sh
\ No newline at end of file
+run_net_forwarding_test.sh
\ No newline at end of file
diff --git a/tools/testing/selftests/drivers/net/dsa/bridge_mld.sh b/tools/testing/selftests/drivers/net/dsa/bridge_mld.sh
index 81a7e0df0474..d16a65e7595d 120000
--- a/tools/testing/selftests/drivers/net/dsa/bridge_mld.sh
+++ b/tools/testing/selftests/drivers/net/dsa/bridge_mld.sh
@@ -1 +1 @@
-../../../net/forwarding/bridge_mld.sh
\ No newline at end of file
+run_net_forwarding_test.sh
\ No newline at end of file
diff --git a/tools/testing/selftests/drivers/net/dsa/bridge_vlan_aware.sh b/tools/testing/selftests/drivers/net/dsa/bridge_vlan_aware.sh
index 9831ed74376a..d16a65e7595d 120000
--- a/tools/testing/selftests/drivers/net/dsa/bridge_vlan_aware.sh
+++ b/tools/testing/selftests/drivers/net/dsa/bridge_vlan_aware.sh
@@ -1 +1 @@
-../../../net/forwarding/bridge_vlan_aware.sh
\ No newline at end of file
+run_net_forwarding_test.sh
\ No newline at end of file
diff --git a/tools/testing/selftests/drivers/net/dsa/bridge_vlan_mcast.sh b/tools/testing/selftests/drivers/net/dsa/bridge_vlan_mcast.sh
index 7f3c3f0bf719..d16a65e7595d 120000
--- a/tools/testing/selftests/drivers/net/dsa/bridge_vlan_mcast.sh
+++ b/tools/testing/selftests/drivers/net/dsa/bridge_vlan_mcast.sh
@@ -1 +1 @@
-../../../net/forwarding/bridge_vlan_mcast.sh
\ No newline at end of file
+run_net_forwarding_test.sh
\ No newline at end of file
diff --git a/tools/testing/selftests/drivers/net/dsa/bridge_vlan_unaware.sh b/tools/testing/selftests/drivers/net/dsa/bridge_vlan_unaware.sh
index bf1a57e6bde1..d16a65e7595d 120000
--- a/tools/testing/selftests/drivers/net/dsa/bridge_vlan_unaware.sh
+++ b/tools/testing/selftests/drivers/net/dsa/bridge_vlan_unaware.sh
@@ -1 +1 @@
-../../../net/forwarding/bridge_vlan_unaware.sh
\ No newline at end of file
+run_net_forwarding_test.sh
\ No newline at end of file
diff --git a/tools/testing/selftests/drivers/net/dsa/lib.sh b/tools/testing/selftests/drivers/net/dsa/lib.sh
deleted file mode 120000
index 39c96828c5ef..000000000000
--- a/tools/testing/selftests/drivers/net/dsa/lib.sh
+++ /dev/null
@@ -1 +0,0 @@
-../../../net/forwarding/lib.sh
\ No newline at end of file
diff --git a/tools/testing/selftests/drivers/net/dsa/local_termination.sh b/tools/testing/selftests/drivers/net/dsa/local_termination.sh
index c08166f84501..d16a65e7595d 120000
--- a/tools/testing/selftests/drivers/net/dsa/local_termination.sh
+++ b/tools/testing/selftests/drivers/net/dsa/local_termination.sh
@@ -1 +1 @@
-../../../net/forwarding/local_termination.sh
\ No newline at end of file
+run_net_forwarding_test.sh
\ No newline at end of file
diff --git a/tools/testing/selftests/drivers/net/dsa/no_forwarding.sh b/tools/testing/selftests/drivers/net/dsa/no_forwarding.sh
index b9757466bc97..d16a65e7595d 120000
--- a/tools/testing/selftests/drivers/net/dsa/no_forwarding.sh
+++ b/tools/testing/selftests/drivers/net/dsa/no_forwarding.sh
@@ -1 +1 @@
-../../../net/forwarding/no_forwarding.sh
\ No newline at end of file
+run_net_forwarding_test.sh
\ No newline at end of file
diff --git a/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh b/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh
new file mode 100755
index 000000000000..4106c0a102ea
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh
@@ -0,0 +1,9 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+libdir=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
+testname=$(basename "${BASH_SOURCE[0]}")
+
+source "$libdir"/forwarding.config
+cd "$libdir"/../../../net/forwarding/ || exit 1
+source "./$testname" "$@"
diff --git a/tools/testing/selftests/drivers/net/dsa/tc_actions.sh b/tools/testing/selftests/drivers/net/dsa/tc_actions.sh
index 306213d9430e..d16a65e7595d 120000
--- a/tools/testing/selftests/drivers/net/dsa/tc_actions.sh
+++ b/tools/testing/selftests/drivers/net/dsa/tc_actions.sh
@@ -1 +1 @@
-../../../net/forwarding/tc_actions.sh
\ No newline at end of file
+run_net_forwarding_test.sh
\ No newline at end of file
diff --git a/tools/testing/selftests/drivers/net/dsa/tc_common.sh b/tools/testing/selftests/drivers/net/dsa/tc_common.sh
deleted file mode 120000
index bc3465bdc36b..000000000000
--- a/tools/testing/selftests/drivers/net/dsa/tc_common.sh
+++ /dev/null
@@ -1 +0,0 @@
-../../../net/forwarding/tc_common.sh
\ No newline at end of file
diff --git a/tools/testing/selftests/drivers/net/dsa/test_bridge_fdb_stress.sh b/tools/testing/selftests/drivers/net/dsa/test_bridge_fdb_stress.sh
index 92acab83fbe2..74682151d04d 100755
--- a/tools/testing/selftests/drivers/net/dsa/test_bridge_fdb_stress.sh
+++ b/tools/testing/selftests/drivers/net/dsa/test_bridge_fdb_stress.sh
@@ -19,7 +19,7 @@ REQUIRE_JQ="no"
REQUIRE_MZ="no"
NETIF_CREATE="no"
lib_dir=$(dirname "$0")
-source "$lib_dir"/lib.sh
+source "$lib_dir"/../../../net/forwarding/lib.sh
cleanup() {
echo "Cleaning up"
--
2.43.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [RFC PATCH net-next 10/10] selftests: dsa: Replace symlinks by wrapper script
2023-12-22 13:58 ` [RFC PATCH net-next 10/10] selftests: dsa: Replace symlinks by wrapper script Benjamin Poirier
@ 2023-12-27 20:11 ` Vladimir Oltean
2023-12-28 19:36 ` Benjamin Poirier
0 siblings, 1 reply; 22+ messages in thread
From: Vladimir Oltean @ 2023-12-27 20:11 UTC (permalink / raw)
To: Benjamin Poirier; +Cc: netdev, Shuah Khan, Petr Machata, Hangbin Liu
On Fri, Dec 22, 2023 at 08:58:36AM -0500, Benjamin Poirier wrote:
> diff --git a/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh b/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh
> new file mode 100755
> index 000000000000..4106c0a102ea
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh
> @@ -0,0 +1,9 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +libdir=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
> +testname=$(basename "${BASH_SOURCE[0]}")
> +
> +source "$libdir"/forwarding.config
> +cd "$libdir"/../../../net/forwarding/ || exit 1
> +source "./$testname" "$@"
Thanks for working on this. I don't dislike the solution. Just one
question. Can "run_net_forwarding_test.sh" be one day moved from
tools/testing/selftests/drivers/net/dsa/ without duplicating it,
should anyone else need the same setup?
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [RFC PATCH net-next 10/10] selftests: dsa: Replace symlinks by wrapper script
2023-12-27 20:11 ` Vladimir Oltean
@ 2023-12-28 19:36 ` Benjamin Poirier
2023-12-28 20:33 ` Vladimir Oltean
0 siblings, 1 reply; 22+ messages in thread
From: Benjamin Poirier @ 2023-12-28 19:36 UTC (permalink / raw)
To: Vladimir Oltean; +Cc: netdev, Shuah Khan, Petr Machata, Hangbin Liu
On 2023-12-27 22:11 +0200, Vladimir Oltean wrote:
> On Fri, Dec 22, 2023 at 08:58:36AM -0500, Benjamin Poirier wrote:
> > diff --git a/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh b/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh
> > new file mode 100755
> > index 000000000000..4106c0a102ea
> > --- /dev/null
> > +++ b/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh
> > @@ -0,0 +1,9 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +libdir=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
> > +testname=$(basename "${BASH_SOURCE[0]}")
> > +
> > +source "$libdir"/forwarding.config
> > +cd "$libdir"/../../../net/forwarding/ || exit 1
> > +source "./$testname" "$@"
>
> Thanks for working on this. I don't dislike the solution. Just one
> question. Can "run_net_forwarding_test.sh" be one day moved from
> tools/testing/selftests/drivers/net/dsa/ without duplicating it,
> should anyone else need the same setup?
Yes, it's possible. I didn't think about it before but I tested the
approach below. It applies over the changes I just sent in my previous
mail about patch 5.
Thank you for your review and suggestions.
diff --git a/tools/testing/selftests/drivers/net/dsa/Makefile b/tools/testing/selftests/drivers/net/dsa/Makefile
index cd6817fe5be6..5ff77c851642 100644
--- a/tools/testing/selftests/drivers/net/dsa/Makefile
+++ b/tools/testing/selftests/drivers/net/dsa/Makefile
@@ -12,10 +12,10 @@ TEST_PROGS = bridge_locked_port.sh \
test_bridge_fdb_stress.sh
TEST_FILES := \
- run_net_forwarding_test.sh \
forwarding.config
TEST_INCLUDES := \
+ run_net_forwarding_test.sh \
../../../net/forwarding/bridge_locked_port.sh \
../../../net/forwarding/bridge_mdb.sh \
../../../net/forwarding/bridge_mld.sh \
@@ -25,6 +25,7 @@ TEST_INCLUDES := \
../../../net/forwarding/lib.sh \
../../../net/forwarding/local_termination.sh \
../../../net/forwarding/no_forwarding.sh \
+ ../../../net/forwarding/run_net_forwarding_test.sh \
../../../net/forwarding/tc_actions.sh \
../../../net/forwarding/tc_common.sh \
../../../net/lib.sh
diff --git a/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh b/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh
deleted file mode 100755
index 4106c0a102ea..000000000000
--- a/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh
+++ /dev/null
@@ -1,9 +0,0 @@
-#!/bin/bash
-# SPDX-License-Identifier: GPL-2.0
-
-libdir=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
-testname=$(basename "${BASH_SOURCE[0]}")
-
-source "$libdir"/forwarding.config
-cd "$libdir"/../../../net/forwarding/ || exit 1
-source "./$testname" "$@"
diff --git a/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh b/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh
new file mode 120000
index 000000000000..2e7e656349e6
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh
@@ -0,0 +1 @@
+../../../net/forwarding/run_net_forwarding_test.sh
\ No newline at end of file
diff --git a/tools/testing/selftests/net/forwarding/run_net_forwarding_test.sh b/tools/testing/selftests/net/forwarding/run_net_forwarding_test.sh
new file mode 100755
index 000000000000..cfddadb57fe7
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/run_net_forwarding_test.sh
@@ -0,0 +1,10 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+linkdir=$(readlink -f "$(dirname "$0")")
+libdir=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
+testname=$(basename "${BASH_SOURCE[0]}")
+
+source "$linkdir"/forwarding.config
+cd "$libdir" || exit 1
+source "./$testname" "$@"
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [RFC PATCH net-next 10/10] selftests: dsa: Replace symlinks by wrapper script
2023-12-28 19:36 ` Benjamin Poirier
@ 2023-12-28 20:33 ` Vladimir Oltean
0 siblings, 0 replies; 22+ messages in thread
From: Vladimir Oltean @ 2023-12-28 20:33 UTC (permalink / raw)
To: Benjamin Poirier; +Cc: netdev, Shuah Khan, Petr Machata, Hangbin Liu
On Thu, Dec 28, 2023 at 02:36:58PM -0500, Benjamin Poirier wrote:
> On 2023-12-27 22:11 +0200, Vladimir Oltean wrote:
> > On Fri, Dec 22, 2023 at 08:58:36AM -0500, Benjamin Poirier wrote:
> > > diff --git a/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh b/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh
> > > new file mode 100755
> > > index 000000000000..4106c0a102ea
> > > --- /dev/null
> > > +++ b/tools/testing/selftests/drivers/net/dsa/run_net_forwarding_test.sh
> > > @@ -0,0 +1,9 @@
> > > +#!/bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +
> > > +libdir=$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")
> > > +testname=$(basename "${BASH_SOURCE[0]}")
> > > +
> > > +source "$libdir"/forwarding.config
> > > +cd "$libdir"/../../../net/forwarding/ || exit 1
> > > +source "./$testname" "$@"
> >
> > Thanks for working on this. I don't dislike the solution. Just one
> > question. Can "run_net_forwarding_test.sh" be one day moved from
> > tools/testing/selftests/drivers/net/dsa/ without duplicating it,
> > should anyone else need the same setup?
>
> Yes, it's possible. I didn't think about it before but I tested the
> approach below. It applies over the changes I just sent in my previous
> mail about patch 5.
>
> Thank you for your review and suggestions.
Thanks. I tested it too and it works.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH net-next 00/10] selftests: Add TEST_INCLUDES directive and adjust tests to use it
2023-12-22 13:58 [RFC PATCH net-next 00/10] selftests: Add TEST_INCLUDES directive and adjust tests to use it Benjamin Poirier
` (9 preceding siblings ...)
2023-12-22 13:58 ` [RFC PATCH net-next 10/10] selftests: dsa: Replace symlinks by wrapper script Benjamin Poirier
@ 2023-12-23 9:27 ` Hangbin Liu
10 siblings, 0 replies; 22+ messages in thread
From: Hangbin Liu @ 2023-12-23 9:27 UTC (permalink / raw)
To: Benjamin Poirier; +Cc: netdev, Shuah Khan, Petr Machata, Vladimir Oltean
On Fri, Dec 22, 2023 at 08:58:26AM -0500, Benjamin Poirier wrote:
> From: Benjamin Poirier <benjamin.poirier@gmail.com>
>
> After commit 25ae948b4478 ("selftests/net: add lib.sh"), some net
> selftests encounter errors when they are being exported and run. This is
> because the new net/lib.sh is not exported along with the tests.
>
> After some related fixes to net selftests, this series introduces a new
> selftests Makefile variable to list extra files to export from other
> directories and makes use of it to resolve the errors described above.
>
> Link: https://lore.kernel.org/netdev/ZXu7dGj7F9Ng8iIX@Laptop-X1/
Tested with
make TARGETS="drivers/net/team drivers/net/bonding drivers/net/dsa" \
-j8 -C tools/testing/selftests gen_tar
and all looks good to me. Thanks for your works!
Tested-by: Hangbin Liu <liuhangbin@gmail.com>
Reviewed-by: Hangbin Liu <liuhangbin@gmail.com>
^ permalink raw reply [flat|nested] 22+ messages in thread