diff mbox series

[RFC,net-next,10/10] selftests: dsa: Replace symlinks by wrapper script

Message ID 20231222135836.992841-11-bpoirier@nvidia.com (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series selftests: Add TEST_INCLUDES directive and adjust tests to use it | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers fail 1 blamed authors not CCed: pabeni@redhat.com; 4 maintainers not CCed: pabeni@redhat.com f.fainelli@gmail.com linux-kselftest@vger.kernel.org andrew@lunn.ch
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest fail Script lib.sh not found in tools/testing/selftests/drivers/net/dsa/Makefile Script tc_common.sh not found in tools/testing/selftests/drivers/net/dsa/Makefile
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch warning WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: adding a line without newline at end of file
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Benjamin Poirier Dec. 22, 2023, 1:58 p.m. UTC 
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

Comments

Vladimir Oltean Dec. 27, 2023, 8:11 p.m. UTC | #1 
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?
Benjamin Poirier Dec. 28, 2023, 7:36 p.m. UTC | #2 
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" "$@"
Vladimir Oltean Dec. 28, 2023, 8:33 p.m. UTC | #3 
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.
diff mbox series

Patch

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"