After commit 25ae948b4478 ("selftests/net: add lib.sh") but before commit 2114e83381d3 ("selftests: forwarding: Avoid failures to source net/lib.sh"), some net selftests encountered errors when they were being exported and run. This was because the new net/lib.sh was not exported along with the tests. The errors were crudely avoided by duplicating some content between net/lib.sh and net/forwarding/lib.sh in 2114e83381d3.
In order to restore the sourcing of net/lib.sh from net/forwarding/lib.sh and remove the duplicated content, this series introduces a new selftests Makefile variable to list extra files to export from other directories and makes use of it to avoid reintroducing the errors mentioned above.
v1: * "selftests: Introduce Makefile variable to list shared bash scripts" Changed TEST_INCLUDES to take relative paths, like other TEST_* variables. Paths are adjusted accordingly in the subsequent patches. (Vladimir Oltean)
* selftests: bonding: Change script interpreter selftests: forwarding: Remove executable bits from lib.sh Removed from this series, submitted separately.
Since commit 2114e83381d3 ("selftests: forwarding: Avoid failures to source net/lib.sh") resolved the test errors, this version of the series is focused on removing the duplication that was added in that commit. Directly rebasing the series would reintroduce the problems that 2114e83381d3 avoided before fixing them again. In order to prevent such breakage partway through the series, patches are reordered and content changed slightly but there is no diff at the end compared with the simple rebasing approach. I have dropped most review tags on account of this reordering.
RFC: https://lore.kernel.org/netdev/20231222135836.992841-1-bpoirier@nvidia.com/
Link: https://lore.kernel.org/netdev/ZXu7dGj7F9Ng8iIX@Laptop-X1/
Benjamin Poirier (5): selftests: Introduce Makefile variable to list shared bash scripts selftests: bonding: Add net/forwarding/lib.sh to TEST_INCLUDES selftests: team: Add shared library scripts to TEST_INCLUDES selftests: dsa: Replace test symlinks by wrapper script selftests: forwarding: Redefine relative_path variable
Petr Machata (1): selftests: forwarding: Remove duplicated lib.sh content
Documentation/dev-tools/kselftest.rst | 10 +++++ tools/testing/selftests/Makefile | 7 +++- .../selftests/drivers/net/bonding/Makefile | 7 +++- .../net/bonding/bond-eth-type-change.sh | 2 +- .../drivers/net/bonding/bond_topo_2d1c.sh | 2 +- .../drivers/net/bonding/dev_addr_lists.sh | 2 +- .../net/bonding/mode-1-recovery-updelay.sh | 2 +- .../net/bonding/mode-2-recovery-updelay.sh | 2 +- .../drivers/net/bonding/net_forwarding_lib.sh | 1 - .../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 +- .../testing/selftests/drivers/net/dsa/lib.sh | 1 - .../drivers/net/dsa/local_termination.sh | 2 +- .../drivers/net/dsa/no_forwarding.sh | 2 +- .../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 +- .../selftests/drivers/net/team/Makefile | 7 ++-- .../drivers/net/team/dev_addr_lists.sh | 4 +- .../selftests/drivers/net/team/lag_lib.sh | 1 - .../drivers/net/team/net_forwarding_lib.sh | 1 - tools/testing/selftests/lib.mk | 19 ++++++++++ .../testing/selftests/net/forwarding/Makefile | 3 ++ tools/testing/selftests/net/forwarding/lib.sh | 37 +++---------------- .../net/forwarding/mirror_gre_lib.sh | 2 +- .../net/forwarding/mirror_gre_topo_lib.sh | 2 +- 32 files changed, 96 insertions(+), 64 deletions(-) delete mode 120000 tools/testing/selftests/drivers/net/bonding/net_forwarding_lib.sh 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 delete mode 120000 tools/testing/selftests/drivers/net/team/lag_lib.sh delete mode 120000 tools/testing/selftests/drivers/net/team/net_forwarding_lib.sh
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 was not handled and was reverted in commit 2114e83381d3 ("selftests: forwarding: Avoid failures to source net/lib.sh"). In order to allow reinstating the inclusion of net/lib.sh from net/forwarding/lib.sh, 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, if net/forwarding/lib.sh sources net/lib.sh; the Makefile related to a test that sources net/forwarding/lib.sh from a parent directory must list: TEST_INCLUDES := \ ../../../net/forwarding/lib.sh \ ../../../net/lib.sh
v1 (from RFC): * changed TEST_INCLUDES to take relative paths, like other TEST_* variables (Vladimir Oltean) * preserved common "$(MAKE) OUTPUT=... -C ... target" ordering in Makefile (Petr Machata)
Reviewed-by: Petr Machata petrm@nvidia.com Tested-by: Petr Machata petrm@nvidia.com Signed-off-by: Benjamin Poirier bpoirier@nvidia.com --- Documentation/dev-tools/kselftest.rst | 10 ++++++++++ tools/testing/selftests/Makefile | 7 ++++++- tools/testing/selftests/lib.mk | 19 +++++++++++++++++++ 3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/Documentation/dev-tools/kselftest.rst b/Documentation/dev-tools/kselftest.rst index ab376b316c36..470cc7913647 100644 --- a/Documentation/dev-tools/kselftest.rst +++ b/Documentation/dev-tools/kselftest.rst @@ -255,9 +255,19 @@ 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 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 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 15b6a111c3be..082db6b68060 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -191,6 +191,8 @@ run_tests: all @for TARGET in $(TARGETS); do \ BUILD_TARGET=$$BUILD/$$TARGET; \ $(MAKE) OUTPUT=$$BUILD_TARGET -C $$TARGET run_tests \ + SRC_PATH=$(shell readlink -e $$(pwd)) \ + OBJ_PATH=$(BUILD) \ O=$(abs_objtree); \ done;
@@ -241,7 +243,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) OUTPUT=$$BUILD_TARGET -C $$TARGET install \ + INSTALL_PATH=$(INSTALL_PATH)/$$TARGET \ + SRC_PATH=$(shell readlink -e $$(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..087fee22dd53 100644 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -69,11 +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 + @$(INSTALL_INCLUDES) @if [ "X$(TEST_PROGS)" != "X" ]; then \ $(call RUN_TESTS, $(TEST_GEN_PROGS) $(TEST_CUSTOM_PROGS) \ $(addprefix $(OUTPUT)/,$(TEST_PROGS))) ; \ @@ -103,6 +121,7 @@ endef install: all ifdef INSTALL_PATH $(INSTALL_RULE) + $(INSTALL_INCLUDES) else $(error Error: set INSTALL_PATH to use install) endif
On Wed, 24 Jan 2024 12:02:17 -0500 Benjamin Poirier wrote:
--- a/Documentation/dev-tools/kselftest.rst +++ b/Documentation/dev-tools/kselftest.rst @@ -255,9 +255,19 @@ 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 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.
I think that this chunk causes a warning when doing make htmldocs:
Documentation/dev-tools/kselftest.rst:267: WARNING: Unexpected indentation. Documentation/dev-tools/kselftest.rst:268: WARNING: Block quote ends without a blank line; unexpected unindent.
Could you double-check?
On 2024-01-24 20:46 -0800, Jakub Kicinski wrote:
On Wed, 24 Jan 2024 12:02:17 -0500 Benjamin Poirier wrote:
--- a/Documentation/dev-tools/kselftest.rst +++ b/Documentation/dev-tools/kselftest.rst @@ -255,9 +255,19 @@ 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 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.
I think that this chunk causes a warning when doing make htmldocs:
Documentation/dev-tools/kselftest.rst:267: WARNING: Unexpected indentation. Documentation/dev-tools/kselftest.rst:268: WARNING: Block quote ends without a blank line; unexpected unindent.
Could you double-check?
You're right, thanks for pointing it out. I'll send a v2.
In order to avoid duplicated files when both the bonding and forwarding tests are exported together, add net/forwarding/lib.sh to TEST_INCLUDES and include it via its relative path.
Reviewed-by: Petr Machata petrm@nvidia.com Tested-by: Petr Machata petrm@nvidia.com Signed-off-by: Benjamin Poirier bpoirier@nvidia.com --- tools/testing/selftests/drivers/net/bonding/Makefile | 6 ++++-- .../selftests/drivers/net/bonding/bond-eth-type-change.sh | 2 +- .../testing/selftests/drivers/net/bonding/bond_topo_2d1c.sh | 2 +- .../testing/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, 9 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..1e10a1f06faf 100644 --- a/tools/testing/selftests/drivers/net/bonding/Makefile +++ b/tools/testing/selftests/drivers/net/bonding/Makefile @@ -15,7 +15,9 @@ 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
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
Benjamin Poirier bpoirier@nvidia.com wrote:
In order to avoid duplicated files when both the bonding and forwarding tests are exported together, add net/forwarding/lib.sh to TEST_INCLUDES and include it via its relative path.
Reviewed-by: Petr Machata petrm@nvidia.com Tested-by: Petr Machata petrm@nvidia.com Signed-off-by: Benjamin Poirier bpoirier@nvidia.com
tools/testing/selftests/drivers/net/bonding/Makefile | 6 ++++-- .../selftests/drivers/net/bonding/bond-eth-type-change.sh | 2 +- .../testing/selftests/drivers/net/bonding/bond_topo_2d1c.sh | 2 +- .../testing/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, 9 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..1e10a1f06faf 100644 --- a/tools/testing/selftests/drivers/net/bonding/Makefile +++ b/tools/testing/selftests/drivers/net/bonding/Makefile @@ -15,7 +15,9 @@ 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
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
Is there a way to pass TEST_INCLUDES via the environment or as a parameter, so that it's not necessary to hard code the path name here and in the similar cases below?
-J
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
On 2024-01-24 10:24 -0800, Jay Vosburgh wrote: [...]
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
Is there a way to pass TEST_INCLUDES via the environment or as a parameter, so that it's not necessary to hard code the path name here and in the similar cases below?
It think would be possible but I see two issues:
1) Tests can be run in a myriad ways. Some of them (`make run_tests`, `run_kselftest.sh`) use runner.sh which would be the place to set environment variables for a test. However it's also possible to run tests directly:
tools/testing/selftests/drivers/net/bonding# ./dev_addr_lists.sh
In that case, there's nothing to automatically set an environment variable for the test.
I think that could be addressed, for example by putting the content of TEST_INCLUDES in a file and having the test read it itself, but ...
2) As can be seen in the dsa case and in the bonding and team cases after patch 6, the relationship between the files listed in TEST_INCLUDES and the files sourced in a test is not 1:1. So automatically sourcing all files listed in TEST_INCLUDES is not generally applicable.
Given these two points, I'm inclined to stick with the current approach. What do you think?
Benjamin Poirier bpoirier@nvidia.com wrote:
On 2024-01-24 10:24 -0800, Jay Vosburgh wrote: [...]
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
Is there a way to pass TEST_INCLUDES via the environment or as a parameter, so that it's not necessary to hard code the path name here and in the similar cases below?
It think would be possible but I see two issues:
- Tests can be run in a myriad ways. Some of them (`make run_tests`,
`run_kselftest.sh`) use runner.sh which would be the place to set environment variables for a test. However it's also possible to run tests directly:
tools/testing/selftests/drivers/net/bonding# ./dev_addr_lists.sh
In that case, there's nothing to automatically set an environment variable for the test.
I think that could be addressed, for example by putting the content of TEST_INCLUDES in a file and having the test read it itself, but ...
As can be seen in the dsa case and in the bonding and team cases after patch 6, the relationship between the files listed in TEST_INCLUDES and the files sourced in a test is not 1:1. So automatically sourcing all files listed in TEST_INCLUDES is not generally applicable.
Given these two points, I'm inclined to stick with the current approach. What do you think?
That rationale sounds fine; I was wondering if there was a straightforward way to centralize the naming of the "library" without having to literally hard code it all over the place, but that doesn't seem to be the case. Thanks for the explanation.
-J
--- -Jay Vosburgh, jay.vosburgh@canonical.com
In order to avoid duplicated files when both the team and bonding tests are exported together, add lag_lib.sh to TEST_INCLUDES.
Do likewise for net/forwarding/lib.sh regarding team and forwarding tests.
Reviewed-by: Petr Machata petrm@nvidia.com Tested-by: Petr Machata petrm@nvidia.com Signed-off-by: Benjamin Poirier bpoirier@nvidia.com --- tools/testing/selftests/drivers/net/team/Makefile | 6 +++--- tools/testing/selftests/drivers/net/team/dev_addr_lists.sh | 4 ++-- tools/testing/selftests/drivers/net/team/lag_lib.sh | 1 - .../selftests/drivers/net/team/net_forwarding_lib.sh | 1 - 4 files changed, 5 insertions(+), 7 deletions(-) delete mode 120000 tools/testing/selftests/drivers/net/team/lag_lib.sh 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..708bdfce37f5 100644 --- a/tools/testing/selftests/drivers/net/team/Makefile +++ b/tools/testing/selftests/drivers/net/team/Makefile @@ -3,8 +3,8 @@
TEST_PROGS := dev_addr_lists.sh
-TEST_FILES := \ - lag_lib.sh \ - net_forwarding_lib.sh +TEST_INCLUDES := \ + ../bonding/lag_lib.sh \ + ../../../net/forwarding/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..b1ec7755b783 100755 --- a/tools/testing/selftests/drivers/net/team/dev_addr_lists.sh +++ b/tools/testing/selftests/drivers/net/team/dev_addr_lists.sh @@ -11,9 +11,9 @@ 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 +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 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
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.
In the last patch of this series, net/forwarding/lib.sh will source lib.sh from its parent directory (ie. net/lib.sh). This would not work for dsa tests 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, as a preparation for that last patch, replace the test symlinks by a wrapper script which runs the original tests under net/forwarding/. Following from that, the links to shared library scripts in dsa/ are no longer used so remove them and add all the original files needed from parent directories to TEST_INCLUDES.
Suggested-by: Hangbin Liu liuhangbin@gmail.com Reviewed-by: Petr Machata petrm@nvidia.com Tested-by: Petr Machata petrm@nvidia.com Signed-off-by: Benjamin Poirier bpoirier@nvidia.com --- .../testing/selftests/drivers/net/dsa/Makefile | 17 +++++++++++++++-- .../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, 34 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..83da1d721017 100644 --- a/tools/testing/selftests/drivers/net/dsa/Makefile +++ b/tools/testing/selftests/drivers/net/dsa/Makefile @@ -11,8 +11,21 @@ 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
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"
The following code which is part of lib.sh: relative_path="${BASH_SOURCE%/*}" if [[ "$relative_path" == "${BASH_SOURCE}" ]]; then relative_path="." fi
reimplements functionality that is part of `dirname`: $ dirname "" .
To avoid this duplication, replace "relative_path" by "net_forwarding_dir", a new variable defined using dirname.
Furthermore, to avoid the potential confusion about what "relative_path" is about (cwd, test script directory or test library directory), define "net_forwarding_dir" as the absolute path to net/forwarding/.
Tested-by: Petr Machata petrm@nvidia.com Signed-off-by: Benjamin Poirier bpoirier@nvidia.com --- tools/testing/selftests/net/forwarding/lib.sh | 9 +++------ tools/testing/selftests/net/forwarding/mirror_gre_lib.sh | 2 +- .../selftests/net/forwarding/mirror_gre_topo_lib.sh | 2 +- 3 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index 8a61464ab6eb..cf0ba4bfe50d 100644 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -29,13 +29,10 @@ 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 +net_forwarding_dir=$(dirname "$(readlink -e "${BASH_SOURCE[0]}")")
-if [[ -f $relative_path/forwarding.config ]]; then - source "$relative_path/forwarding.config" +if [[ -f $net_forwarding_dir/forwarding.config ]]; then + source "$net_forwarding_dir/forwarding.config" fi
# Kselftest framework requirement - SKIP code is 4. diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_lib.sh b/tools/testing/selftests/net/forwarding/mirror_gre_lib.sh index fac486178ef7..0c36546e131e 100644 --- a/tools/testing/selftests/net/forwarding/mirror_gre_lib.sh +++ b/tools/testing/selftests/net/forwarding/mirror_gre_lib.sh @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0
-source "$relative_path/mirror_lib.sh" +source "$net_forwarding_dir/mirror_lib.sh"
quick_test_span_gre_dir_ips() { diff --git a/tools/testing/selftests/net/forwarding/mirror_gre_topo_lib.sh b/tools/testing/selftests/net/forwarding/mirror_gre_topo_lib.sh index 39c03e2867f4..6e615fffa4ef 100644 --- a/tools/testing/selftests/net/forwarding/mirror_gre_topo_lib.sh +++ b/tools/testing/selftests/net/forwarding/mirror_gre_topo_lib.sh @@ -33,7 +33,7 @@ # | | # +-------------------------------------------------------------------------+
-source "$relative_path/mirror_topo_lib.sh" +source "$net_forwarding_dir/mirror_topo_lib.sh"
mirror_gre_topo_h3_create() {
From: Petr Machata petrm@nvidia.com
commit 25ae948b4478 ("selftests/net: add lib.sh") added net/lib.sh to contain code shared by tests under net/ and net/forwarding/. However, this caused issues with selftests from directories other than net/forwarding/, in particular those under drivers/net/. Those issues were avoided in a simple way by duplicating some content in commit 2114e83381d3 ("selftests: forwarding: Avoid failures to source net/lib.sh").
In order to remove the duplicated content, restore the inclusion of net/lib.sh from net/forwarding/lib.sh but with the following changes:
* net/lib.sh is imported through the net_forwarding_dir path The original expression "source ../lib.sh" would look for lib.sh in the directory above the script file's one, which did not work for tests under drivers/net/.
* net/lib.sh is added to TEST_INCLUDES Since net/forwarding/lib.sh now sources net/lib.sh, both of those files must be exported along with tests which source net/forwarding/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 --- .../selftests/drivers/net/bonding/Makefile | 3 ++- .../selftests/drivers/net/dsa/Makefile | 3 ++- .../selftests/drivers/net/team/Makefile | 3 ++- .../testing/selftests/net/forwarding/Makefile | 3 +++ tools/testing/selftests/net/forwarding/lib.sh | 26 +------------------ 5 files changed, 10 insertions(+), 28 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/bonding/Makefile b/tools/testing/selftests/drivers/net/bonding/Makefile index 1e10a1f06faf..03a089165d3f 100644 --- a/tools/testing/selftests/drivers/net/bonding/Makefile +++ b/tools/testing/selftests/drivers/net/bonding/Makefile @@ -18,6 +18,7 @@ TEST_FILES := \ bond_topo_3d1c.sh
TEST_INCLUDES := \ - ../../../net/forwarding/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 83da1d721017..cd6817fe5be6 100644 --- a/tools/testing/selftests/drivers/net/dsa/Makefile +++ b/tools/testing/selftests/drivers/net/dsa/Makefile @@ -26,6 +26,7 @@ TEST_INCLUDES := \ ../../../net/forwarding/local_termination.sh \ ../../../net/forwarding/no_forwarding.sh \ ../../../net/forwarding/tc_actions.sh \ - ../../../net/forwarding/tc_common.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 708bdfce37f5..2d5a76d99181 100644 --- a/tools/testing/selftests/drivers/net/team/Makefile +++ b/tools/testing/selftests/drivers/net/team/Makefile @@ -5,6 +5,7 @@ TEST_PROGS := dev_addr_lists.sh
TEST_INCLUDES := \ ../bonding/lag_lib.sh \ - ../../../net/forwarding/lib.sh + ../../../net/forwarding/lib.sh \ + ../../../net/lib.sh
include ../../../lib.mk diff --git a/tools/testing/selftests/net/forwarding/Makefile b/tools/testing/selftests/net/forwarding/Makefile index 452693514be4..1fba2717738d 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 := \ + ../lib.sh + include ../../lib.mk diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index cf0ba4bfe50d..a7ecfc8cae98 100644 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -35,31 +35,7 @@ if [[ -f $net_forwarding_dir/forwarding.config ]]; then source "$net_forwarding_dir/forwarding.config" fi
-# Kselftest framework requirement - SKIP code is 4. -ksft_skip=4 - -busywait() -{ - local timeout=$1; shift - - local start_time="$(date -u +%s%3N)" - while true - do - local out - out=$("$@") - local ret=$? - if ((!ret)); then - echo -n "$out" - return 0 - fi - - local current_time="$(date -u +%s%3N)" - if ((current_time - start_time > timeout)); then - echo -n "$out" - return 1 - fi - done -} +source "$net_forwarding_dir/../lib.sh"
############################################################################## # Sanity checks
On Wed, Jan 24, 2024 at 12:02:16PM -0500, Benjamin Poirier wrote:
After commit 25ae948b4478 ("selftests/net: add lib.sh") but before commit 2114e83381d3 ("selftests: forwarding: Avoid failures to source net/lib.sh"), some net selftests encountered errors when they were being exported and run. This was because the new net/lib.sh was not exported along with the tests. The errors were crudely avoided by duplicating some content between net/lib.sh and net/forwarding/lib.sh in 2114e83381d3.
In order to restore the sourcing of net/lib.sh from net/forwarding/lib.sh and remove the duplicated content, this series introduces a new selftests Makefile variable to list extra files to export from other directories and makes use of it to avoid reintroducing the errors mentioned above.
v1:
- "selftests: Introduce Makefile variable to list shared bash scripts"
Changed TEST_INCLUDES to take relative paths, like other TEST_* variables. Paths are adjusted accordingly in the subsequent patches. (Vladimir Oltean)
- selftests: bonding: Change script interpreter selftests: forwarding: Remove executable bits from lib.sh
Removed from this series, submitted separately.
Since commit 2114e83381d3 ("selftests: forwarding: Avoid failures to source net/lib.sh") resolved the test errors, this version of the series is focused on removing the duplication that was added in that commit. Directly rebasing the series would reintroduce the problems that 2114e83381d3 avoided before fixing them again. In order to prevent such breakage partway through the series, patches are reordered and content changed slightly but there is no diff at the end compared with the simple rebasing approach. I have dropped most review tags on account of this reordering.
RFC: https://lore.kernel.org/netdev/20231222135836.992841-1-bpoirier@nvidia.com/
Link: https://lore.kernel.org/netdev/ZXu7dGj7F9Ng8iIX@Laptop-X1/
Reviewed-by: Hangbin Liu liuhangbin@gmail.com
linux-kselftest-mirror@lists.linaro.org
-
Benjamin Poirier -
Hangbin Liu -
Jakub Kicinski -
Jay Vosburgh