Re: [PATCH net-next] selftests: forwarding: Import top-level lib.sh through $lib_dir

[Benjamin Poirier <benjamin.poirier@gmail.com>]

On 2023-12-13 14:00 +0800, Hangbin Liu wrote:
> On Tue, Dec 12, 2023 at 03:17:01PM -0500, Benjamin Poirier wrote:
> > On 2023-12-12 18:22 +0100, Petr Machata wrote:
[...]
> > There is also another related issue which is that generating a test
> > archive using gen_tar for the tests under drivers/net/bonding does not
> > include the new lib.sh. This is similar to the issue reported here:
> > https://lore.kernel.org/netdev/40f04ded-0c86-8669-24b1-9a313ca21076@redhat.com/
> > 
> > /tmp/x# ./run_kselftest.sh
> > TAP version 13
> > [...]
> > # timeout set to 120
> > # selftests: drivers/net/bonding: dev_addr_lists.sh
> > # ./net_forwarding_lib.sh: line 41: ../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 4 selftests: drivers/net/bonding: dev_addr_lists.sh
> > [...]
> 
> Hmm.. Is it possible to write a rule in the Makefile to create the net/
> and net/forwarding folder so we can source the relative path directly. e.g.
> 
> ]# tree
> .
> ├── drivers
> │   └── net
> │       └── bonding
> │           ├── bond-arp-interval-causes-panic.sh
> │           ├── ...
> │           └── settings
> ├── kselftest
> │   ├── module.sh
> │   ├── prefix.pl
> │   └── runner.sh
> ├── kselftest-list.txt
> ├── net
> │   ├── forwarding
> │   │   └── lib.sh
> │   └── lib.sh
> └── run_kselftest.sh

That sounds like a good idea. I started to work on that approach but I'm
missing recursive inclusion. For instance

cd tools/testing/selftests
make install TARGETS="drivers/net/bonding"
./kselftest_install/run_kselftest.sh -t drivers/net/bonding:dev_addr_lists.sh

includes net/forwarding/lib.sh but is missing net/lib.sh. I feel that my
'make' skills are rusty but I guess that with enough make code, it could
be done. A workaround is simply to manually list the transitive
dependencies in TEST_SH_LIBS:
 TEST_SH_LIBS := \
-	net/forwarding/lib.sh
+	net/forwarding/lib.sh \
+	net/lib.sh

I only converted a few files to validate that the approach is viable. I
used the following tests:
drivers/net/bonding/dev_addr_lists.sh
net/test_vxlan_vnifiltering.sh
net/forwarding/pedit_ip.sh

Let me know what you think.

---
 tools/testing/selftests/Makefile                            | 5 ++++-
 tools/testing/selftests/drivers/net/bonding/Makefile        | 6 ++++--
 .../testing/selftests/drivers/net/bonding/dev_addr_lists.sh | 2 +-
 .../selftests/drivers/net/bonding/net_forwarding_lib.sh     | 1 -
 tools/testing/selftests/lib.mk                              | 3 +++
 tools/testing/selftests/net/forwarding/Makefile             | 3 +++
 tools/testing/selftests/net/forwarding/lib.sh               | 3 ++-
 7 files changed, 17 insertions(+), 6 deletions(-)
 delete mode 120000 tools/testing/selftests/drivers/net/bonding/net_forwarding_lib.sh

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 3b2061d1c1a5..0aaa7efa3015 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -256,7 +256,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 \
+				SH_LIBS_PATH=$(INSTALL_PATH) \
+				BASE_PATH=$(shell pwd) \
 				O=$(abs_objtree)		\
 				$(if $(FORCE_TARGETS),|| exit);	\
 		ret=$$((ret * $$?));		\
diff --git a/tools/testing/selftests/drivers/net/bonding/Makefile b/tools/testing/selftests/drivers/net/bonding/Makefile
index 8a72bb7de70f..6682f8c1fe79 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_SH_LIBS := \
+	net/forwarding/lib.sh
 
 include ../../../lib.mk
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/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
diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk
index 118e0964bda9..94b7af7d6610 100644
--- a/tools/testing/selftests/lib.mk
+++ b/tools/testing/selftests/lib.mk
@@ -137,6 +137,9 @@ endef
 install: all
 ifdef INSTALL_PATH
 	$(INSTALL_RULE)
+	$(if $(TEST_SH_LIBS), \
+		cd $(BASE_PATH) && rsync -aR $(TEST_SH_LIBS) $(SH_LIBS_PATH)/ \
+	)
 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 df593b7b3e6b..4f6921638bae 100644
--- a/tools/testing/selftests/net/forwarding/Makefile
+++ b/tools/testing/selftests/net/forwarding/Makefile
@@ -128,4 +128,7 @@ TEST_PROGS_EXTENDED := devlink_lib.sh \
 	sch_tbf_etsprio.sh \
 	tc_common.sh
 
+TEST_SH_LIBS := \
+	net/lib.sh
+
 include ../../lib.mk
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 8f6ca458af9a..b99fae42e3c0 100755
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -38,7 +38,8 @@ if [[ -f $relative_path/forwarding.config ]]; then
 	source "$relative_path/forwarding.config"
 fi
 
-source ../lib.sh
+libdir=$(dirname "$(readlink -f "$BASH_SOURCE")")
+source "$libdir"/../lib.sh
 ##############################################################################
 # Sanity checks
 
-- 
2.43.0