[PATCHv2 6.6.y 0/3] Fix missing lib.sh for net/unicast

Linux kernel -stable discussions
 help / color / mirror / Atom feed

* [PATCHv2 6.6.y 0/3] Fix missing lib.sh for net/unicast_extensions.sh and net/pmtu.sh tests
@ 2024-06-18  7:53 Po-Hsu Lin
  2024-06-18  7:53 ` [PATCHv2 6.6.y 1/3] selftests/net: add lib.sh Po-Hsu Lin
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Po-Hsu Lin @ 2024-06-18  7:53 UTC (permalink / raw)
  To: stable
  Cc: po-hsu.lin, gregkh, petrm, liuhangbin, pabeni, kuba, bpoirier,
	idosch

Since upstream commit:
  * 0f4765d0 "selftests/net: convert unicast_extensions.sh to run it in
    unique namespace"
  * 378f082e "selftests/net: convert pmtu.sh to run it in unique namespace"

The lib.sh from commit 25ae948b "selftests/net: add lib.sh" will be needed.
Otherwise these test will complain about missing files and fail:
$ sudo ./unicast_extensions.sh
./unicast_extensions.sh: line 31: lib.sh: No such file or directory
...

$ sudo ./pmtu.sh
./pmtu.sh: line 201: lib.sh: No such file or directory
./pmtu.sh: line 941: cleanup_all_ns: command not found
...

Another commit b6925b4e "selftests/net: add variable NS_LIST for lib.sh" is
needed to add support for the cleanup_all_ns above.

And 2114e833 "selftests: forwarding: Avoid failures to source net/lib.sh" is
a follow-up fix for tests inside the net/forwarding directory.

V2: Add 2114e833 "selftests: forwarding: Avoid failures to source net/lib.sh"
    as suggested by Hangbin Liu.

Benjamin Poirier (1):
  selftests: forwarding: Avoid failures to source net/lib.sh

Hangbin Liu (2):
  selftests/net: add lib.sh
  selftests/net: add variable NS_LIST for lib.sh

 tools/testing/selftests/net/Makefile          |  2 +-
 tools/testing/selftests/net/forwarding/lib.sh | 52 +++++++--------
 tools/testing/selftests/net/lib.sh            | 93 +++++++++++++++++++++++++++
 3 files changed, 120 insertions(+), 27 deletions(-)
 create mode 100644 tools/testing/selftests/net/lib.sh

-- 
2.7.4

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCHv2 6.6.y 1/3] selftests/net: add lib.sh
  2024-06-18  7:53 [PATCHv2 6.6.y 0/3] Fix missing lib.sh for net/unicast_extensions.sh and net/pmtu.sh tests Po-Hsu Lin
@ 2024-06-18  7:53 ` Po-Hsu Lin
  2024-06-19  8:49   ` Greg KH
  2024-06-18  7:53 ` [PATCHv2 6.6.y 2/3] selftests/net: add variable NS_LIST for lib.sh Po-Hsu Lin
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 6+ messages in thread
From: Po-Hsu Lin @ 2024-06-18  7:53 UTC (permalink / raw)
  To: stable
  Cc: po-hsu.lin, gregkh, petrm, liuhangbin, pabeni, kuba, bpoirier,
	idosch

From: Hangbin Liu <liuhangbin@gmail.com>

commit 25ae948b447881bf689d459cd5bd4629d9c04b20 upstream.

Add a lib.sh for net selftests. This file can be used to define commonly
used variables and functions. Some commonly used functions can be moved
from forwarding/lib.sh to this lib file. e.g. busywait().

Add function setup_ns() for user to create unique namespaces with given
prefix name.

Reviewed-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 tools/testing/selftests/net/Makefile          |  2 +-
 tools/testing/selftests/net/forwarding/lib.sh | 27 +--------
 tools/testing/selftests/net/lib.sh            | 85 +++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 27 deletions(-)
 create mode 100644 tools/testing/selftests/net/lib.sh

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index de4506e..7dbdc5e 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -54,7 +54,7 @@ TEST_PROGS += ip_local_port_range.sh
 TEST_PROGS += rps_default_mask.sh
 TEST_PROGS += big_tcp.sh
 TEST_PROGS_EXTENDED := in_netns.sh setup_loopback.sh setup_veth.sh
-TEST_PROGS_EXTENDED += toeplitz_client.sh toeplitz.sh
+TEST_PROGS_EXTENDED += toeplitz_client.sh toeplitz.sh lib.sh
 TEST_GEN_FILES =  socket nettest
 TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy reuseport_addr_any
 TEST_GEN_FILES += tcp_mmap tcp_inq psock_snd txring_overwrite
diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index e37a15e..8f6ca45 100755
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -4,9 +4,6 @@
 ##############################################################################
 # Defines
 
-# Kselftest framework requirement - SKIP code is 4.
-ksft_skip=4
-
 # Can be overridden by the configuration file.
 PING=${PING:=ping}
 PING6=${PING6:=ping6}
@@ -41,6 +38,7 @@ if [[ -f $relative_path/forwarding.config ]]; then
 	source "$relative_path/forwarding.config"
 fi
 
+source ../lib.sh
 ##############################################################################
 # Sanity checks
 
@@ -395,29 +393,6 @@ log_info()
 	echo "INFO: $msg"
 }
 
-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
-}
-
 not()
 {
 	"$@"
diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
new file mode 100644
index 00000000..518eca5
--- /dev/null
+++ b/tools/testing/selftests/net/lib.sh
@@ -0,0 +1,85 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+##############################################################################
+# Defines
+
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+
+##############################################################################
+# Helpers
+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
+}
+
+cleanup_ns()
+{
+	local ns=""
+	local errexit=0
+	local ret=0
+
+	# disable errexit temporary
+	if [[ $- =~ "e" ]]; then
+		errexit=1
+		set +e
+	fi
+
+	for ns in "$@"; do
+		ip netns delete "${ns}" &> /dev/null
+		if ! busywait 2 ip netns list \| grep -vq "^$ns$" &> /dev/null; then
+			echo "Warn: Failed to remove namespace $ns"
+			ret=1
+		fi
+	done
+
+	[ $errexit -eq 1 ] && set -e
+	return $ret
+}
+
+# setup netns with given names as prefix. e.g
+# setup_ns local remote
+setup_ns()
+{
+	local ns=""
+	local ns_name=""
+	local ns_list=""
+	for ns_name in "$@"; do
+		# Some test may setup/remove same netns multi times
+		if unset ${ns_name} 2> /dev/null; then
+			ns="${ns_name,,}-$(mktemp -u XXXXXX)"
+			eval readonly ${ns_name}="$ns"
+		else
+			eval ns='$'${ns_name}
+			cleanup_ns "$ns"
+
+		fi
+
+		if ! ip netns add "$ns"; then
+			echo "Failed to create namespace $ns_name"
+			cleanup_ns "$ns_list"
+			return $ksft_skip
+		fi
+		ip -n "$ns" link set lo up
+		ns_list="$ns_list $ns"
+	done
+}
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 6.6.y 1/3] selftests/net: add lib.sh
  2024-06-18  7:53 ` [PATCHv2 6.6.y 1/3] selftests/net: add lib.sh Po-Hsu Lin
@ 2024-06-19  8:49   ` Greg KH
  0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2024-06-19  8:49 UTC (permalink / raw)
  To: Po-Hsu Lin; +Cc: stable, petrm, liuhangbin, pabeni, kuba, bpoirier, idosch

On Tue, Jun 18, 2024 at 03:53:04PM +0800, Po-Hsu Lin wrote:
> From: Hangbin Liu <liuhangbin@gmail.com>
> 
> commit 25ae948b447881bf689d459cd5bd4629d9c04b20 upstream.
> 
> Add a lib.sh for net selftests. This file can be used to define commonly
> used variables and functions. Some commonly used functions can be moved
> from forwarding/lib.sh to this lib file. e.g. busywait().
> 
> Add function setup_ns() for user to create unique namespaces with given
> prefix name.
> 
> Reviewed-by: Petr Machata <petrm@nvidia.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
> ---
>  tools/testing/selftests/net/Makefile          |  2 +-
>  tools/testing/selftests/net/forwarding/lib.sh | 27 +--------
>  tools/testing/selftests/net/lib.sh            | 85 +++++++++++++++++++++++++++
>  3 files changed, 87 insertions(+), 27 deletions(-)
>  create mode 100644 tools/testing/selftests/net/lib.sh

This patch fails to apply on the lates 6.6.y tree:
	checking file tools/testing/selftests/net/Makefile
	Hunk #1 FAILED at 54.
	1 out of 1 hunk FAILED
	checking file tools/testing/selftests/net/forwarding/lib.sh
	checking file tools/testing/selftests/net/lib.sh

Please rebase and resubmit.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCHv2 6.6.y 2/3] selftests/net: add variable NS_LIST for lib.sh
  2024-06-18  7:53 [PATCHv2 6.6.y 0/3] Fix missing lib.sh for net/unicast_extensions.sh and net/pmtu.sh tests Po-Hsu Lin
  2024-06-18  7:53 ` [PATCHv2 6.6.y 1/3] selftests/net: add lib.sh Po-Hsu Lin
@ 2024-06-18  7:53 ` Po-Hsu Lin
  2024-06-18  7:53 ` [PATCHv2 6.6.y 3/3] selftests: forwarding: Avoid failures to source net/lib.sh Po-Hsu Lin
  2024-06-18  9:19 ` [PATCHv2 6.6.y 0/3] Fix missing lib.sh for net/unicast_extensions.sh and net/pmtu.sh tests Hangbin Liu
  3 siblings, 0 replies; 6+ messages in thread
From: Po-Hsu Lin @ 2024-06-18  7:53 UTC (permalink / raw)
  To: stable
  Cc: po-hsu.lin, gregkh, petrm, liuhangbin, pabeni, kuba, bpoirier,
	idosch

From: Hangbin Liu <liuhangbin@gmail.com>

commit b6925b4ed57cccf42ca0fb46c7446f0859e7ad4b upstream.

Add a global variable NS_LIST to store all the namespaces that setup_ns
created, so the caller could call cleanup_all_ns() instead of remember
all the netns names when using cleanup_ns().

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
Link: https://lore.kernel.org/r/20231213060856.4030084-2-liuhangbin@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 tools/testing/selftests/net/lib.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
index 518eca5..dca5494 100644
--- a/tools/testing/selftests/net/lib.sh
+++ b/tools/testing/selftests/net/lib.sh
@@ -6,6 +6,8 @@
 
 # Kselftest framework requirement - SKIP code is 4.
 ksft_skip=4
+# namespace list created by setup_ns
+NS_LIST=""
 
 ##############################################################################
 # Helpers
@@ -56,6 +58,11 @@ cleanup_ns()
 	return $ret
 }
 
+cleanup_all_ns()
+{
+	cleanup_ns $NS_LIST
+}
+
 # setup netns with given names as prefix. e.g
 # setup_ns local remote
 setup_ns()
@@ -82,4 +89,5 @@ setup_ns()
 		ip -n "$ns" link set lo up
 		ns_list="$ns_list $ns"
 	done
+	NS_LIST="$NS_LIST $ns_list"
 }
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCHv2 6.6.y 3/3] selftests: forwarding: Avoid failures to source net/lib.sh
  2024-06-18  7:53 [PATCHv2 6.6.y 0/3] Fix missing lib.sh for net/unicast_extensions.sh and net/pmtu.sh tests Po-Hsu Lin
  2024-06-18  7:53 ` [PATCHv2 6.6.y 1/3] selftests/net: add lib.sh Po-Hsu Lin
  2024-06-18  7:53 ` [PATCHv2 6.6.y 2/3] selftests/net: add variable NS_LIST for lib.sh Po-Hsu Lin
@ 2024-06-18  7:53 ` Po-Hsu Lin
  2024-06-18  9:19 ` [PATCHv2 6.6.y 0/3] Fix missing lib.sh for net/unicast_extensions.sh and net/pmtu.sh tests Hangbin Liu
  3 siblings, 0 replies; 6+ messages in thread
From: Po-Hsu Lin @ 2024-06-18  7:53 UTC (permalink / raw)
  To: stable
  Cc: po-hsu.lin, gregkh, petrm, liuhangbin, pabeni, kuba, bpoirier,
	idosch

From: Benjamin Poirier <bpoirier@nvidia.com>

commit 2114e83381d3289a88378850f43069e79f848083 upstream.

The expression "source ../lib.sh" added to net/forwarding/lib.sh in commit
25ae948b4478 ("selftests/net: add lib.sh") does not work for tests outside
net/forwarding which source net/forwarding/lib.sh (1). It also does not
work in some cases where only a subset of tests are exported (2).

Avoid the problems mentioned above by replacing the faulty expression with
a copy of the content from net/lib.sh which is used by files under
net/forwarding.

A more thorough solution which avoids duplicating content between
net/lib.sh and net/forwarding/lib.sh has been posted here:
https://lore.kernel.org/netdev/20231222135836.992841-1-bpoirier@nvidia.com/

The approach in the current patch is a stopgap solution to avoid submitting
large changes at the eleventh hour of this development cycle.

Example of problem 1)

tools/testing/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 ]

An error message is printed but since the test does not use functions from
net/lib.sh, the test results are not affected.

Example of problem 2)

tools/testing/selftests$ make install TARGETS="net/forwarding"
tools/testing/selftests$ cd kselftest_install/net/forwarding/
tools/testing/selftests/kselftest_install/net/forwarding$ ./pedit_ip.sh veth{0..3}
lib.sh: line 41: ../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 this case, the test results are affected.

Fixes: 25ae948b4478 ("selftests/net: add lib.sh")
Suggested-by: Ido Schimmel <idosch@nvidia.com>
Suggested-by: Petr Machata <petrm@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Petr Machata <petrm@nvidia.com>
Signed-off-by: Benjamin Poirier <bpoirier@nvidia.com>
Reviewed-by: Hangbin Liu <liuhangbin@gmail.com>
Link: https://lore.kernel.org/r/20240104141109.100672-1-bpoirier@nvidia.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 tools/testing/selftests/net/forwarding/lib.sh | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh
index 8f6ca45..97e7675 100755
--- a/tools/testing/selftests/net/forwarding/lib.sh
+++ b/tools/testing/selftests/net/forwarding/lib.sh
@@ -38,7 +38,32 @@ if [[ -f $relative_path/forwarding.config ]]; then
 	source "$relative_path/forwarding.config"
 fi
 
-source ../lib.sh
+# 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
+}
+
 ##############################################################################
 # Sanity checks
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCHv2 6.6.y 0/3] Fix missing lib.sh for net/unicast_extensions.sh and net/pmtu.sh tests
  2024-06-18  7:53 [PATCHv2 6.6.y 0/3] Fix missing lib.sh for net/unicast_extensions.sh and net/pmtu.sh tests Po-Hsu Lin
                   ` (2 preceding siblings ...)
  2024-06-18  7:53 ` [PATCHv2 6.6.y 3/3] selftests: forwarding: Avoid failures to source net/lib.sh Po-Hsu Lin
@ 2024-06-18  9:19 ` Hangbin Liu
  3 siblings, 0 replies; 6+ messages in thread
From: Hangbin Liu @ 2024-06-18  9:19 UTC (permalink / raw)
  To: Po-Hsu Lin; +Cc: stable, gregkh, petrm, pabeni, kuba, bpoirier, idosch

On Tue, Jun 18, 2024 at 03:53:03PM +0800, Po-Hsu Lin wrote:
> Since upstream commit:
>   * 0f4765d0 "selftests/net: convert unicast_extensions.sh to run it in
>     unique namespace"
>   * 378f082e "selftests/net: convert pmtu.sh to run it in unique namespace"
> 
> The lib.sh from commit 25ae948b "selftests/net: add lib.sh" will be needed.
> Otherwise these test will complain about missing files and fail:
> $ sudo ./unicast_extensions.sh
> ./unicast_extensions.sh: line 31: lib.sh: No such file or directory
> ...
> 
> $ sudo ./pmtu.sh
> ./pmtu.sh: line 201: lib.sh: No such file or directory
> ./pmtu.sh: line 941: cleanup_all_ns: command not found
> ...
> 
> Another commit b6925b4e "selftests/net: add variable NS_LIST for lib.sh" is
> needed to add support for the cleanup_all_ns above.
> 
> And 2114e833 "selftests: forwarding: Avoid failures to source net/lib.sh" is
> a follow-up fix for tests inside the net/forwarding directory.
> 
> V2: Add 2114e833 "selftests: forwarding: Avoid failures to source net/lib.sh"
>     as suggested by Hangbin Liu.
> 
> Benjamin Poirier (1):
>   selftests: forwarding: Avoid failures to source net/lib.sh
> 
> Hangbin Liu (2):
>   selftests/net: add lib.sh
>   selftests/net: add variable NS_LIST for lib.sh
> 
>  tools/testing/selftests/net/Makefile          |  2 +-
>  tools/testing/selftests/net/forwarding/lib.sh | 52 +++++++--------
>  tools/testing/selftests/net/lib.sh            | 93 +++++++++++++++++++++++++++
>  3 files changed, 120 insertions(+), 27 deletions(-)
>  create mode 100644 tools/testing/selftests/net/lib.sh
> 
> -- 
> 2.7.4
> 

For the series

Reviewed-by: Hangbin Liu <liuhangbin@gmail.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-06-19  8:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-18  7:53 [PATCHv2 6.6.y 0/3] Fix missing lib.sh for net/unicast_extensions.sh and net/pmtu.sh tests Po-Hsu Lin
2024-06-18  7:53 ` [PATCHv2 6.6.y 1/3] selftests/net: add lib.sh Po-Hsu Lin
2024-06-19  8:49   ` Greg KH
2024-06-18  7:53 ` [PATCHv2 6.6.y 2/3] selftests/net: add variable NS_LIST for lib.sh Po-Hsu Lin
2024-06-18  7:53 ` [PATCHv2 6.6.y 3/3] selftests: forwarding: Avoid failures to source net/lib.sh Po-Hsu Lin
2024-06-18  9:19 ` [PATCHv2 6.6.y 0/3] Fix missing lib.sh for net/unicast_extensions.sh and net/pmtu.sh tests Hangbin Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox