[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZXok5cRZDKdjX1nj@d3>
Date: Wed, 13 Dec 2023 16:40:53 -0500
From: Benjamin Poirier <benjamin.poirier@...il.com>
To: Hangbin Liu <liuhangbin@...il.com>
Cc: Petr Machata <petrm@...dia.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
netdev@...r.kernel.org, Shuah Khan <shuah@...nel.org>,
mlxsw@...dia.com, Jay Vosburgh <j.vosburgh@...il.com>
Subject: Re: [PATCH net-next] selftests: forwarding: Import top-level lib.sh
through $lib_dir
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
Powered by blists - more mailing lists