[PATCH mptcp-next v3 0/4] use helpers in lib.sh and net_helpers.sh

[PATCH mptcp-next v3 0/4] use helpers in lib.sh and net_helpers.sh

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This set uses the public helpers defined in net selftests to replace
helpers in mptcp_lib.sh.

v3:
 - address Matt's comments in v2 (thanks, it's very useful.)
 - don't rename ns in diag.sh, but in lib.sh
 - ns in get_counter needs to be renamed too
 - add a ns_name valid check in setup_ns
 - drop $NS_LIST in mptcp_lib_ns_init
 - don't drop mptcp_lib_ns_init, use cleanup_ns in it
 - don't use cleanup_all_ns, still use mptcp_lib_ns_init

v2:
 - only patch 3 updated, mptcp_join.sh, move cleanup_all_ns from
   cleanup_partial to cleanup, setup_ns will delete existing
   namespaces automatically.
 - update commit logs.

Geliang Tang (4):
  selftests: net: rename ns in setup/cleanup_ns
  selftests: mptcp: rename ns to _ns in get_counter
  selftests: mptcp: use setup/cleanup_ns helpers
  selftests: mptcp: use wait_local_port_listen helper

 tools/testing/selftests/net/lib.sh            | 33 +++++++++++-------
 .../testing/selftests/net/mptcp/mptcp_lib.sh  | 34 +++++--------------
 2 files changed, 29 insertions(+), 38 deletions(-)

-- 
2.43.0

[PATCH mptcp-next v3 1/4] selftests: net: rename ns in setup/cleanup_ns

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

The helpers setup_ns and cleanup_ns don't work when a namespace named
"ns" is passed to them.

For example, in net/mptcp/diag.sh, the name of the namespace is "ns".
If "setup_ns ns" is used in it, diag.sh fails with errors:

 Invalid netns name "./mptcp_connect"
 Cannot open network namespace "10000": No such file or directory
 Cannot open network namespace "10000": No such file or directory

That is because "ns" is also a local variable in both setup_ns and
cleanup_ns. To solve this, this patch renames the local variable "ns"
as "_ns".

Also a ns_name valid check has been added to setup_ns. If "_ns" is
passed in as a ns_name, setup_ns helper exits.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/lib.sh | 33 ++++++++++++++++++------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh
index edc030e81a46..1401f5bf961a 100644
--- a/tools/testing/selftests/net/lib.sh
+++ b/tools/testing/selftests/net/lib.sh
@@ -128,7 +128,7 @@ slowwait_for_counter()
 
 cleanup_ns()
 {
-	local ns=""
+	local _ns=""
 	local errexit=0
 	local ret=0
 
@@ -138,10 +138,11 @@ cleanup_ns()
 		set +e
 	fi
 
-	for ns in "$@"; do
-		ip netns delete "${ns}" &> /dev/null
-		if ! busywait $BUSYWAIT_TIMEOUT ip netns list \| grep -vq "^$ns$" &> /dev/null; then
-			echo "Warn: Failed to remove namespace $ns"
+	for _ns in "$@"; do
+		ip netns delete "${_ns}" &> /dev/null
+		if ! busywait $BUSYWAIT_TIMEOUT ip netns list \
+		   \| grep -vq "^$_ns$" &> /dev/null; then
+			echo "Warn: Failed to remove namespace $_ns"
 			ret=1
 		fi
 	done
@@ -159,29 +160,35 @@ cleanup_all_ns()
 # setup_ns local remote
 setup_ns()
 {
-	local ns=""
+	local _ns=""
 	local ns_name=""
 	local ns_list=""
 	local ns_exist=
 	for ns_name in "$@"; do
+		if [ "${ns_name}" == "_ns" ]; then
+			echo "ns_name shouldn't be _ns"
+			cleanup_ns "$ns_list"
+			set -e
+			return $ksft_fail
+		fi
 		# 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"
+			_ns="${ns_name,,}-$(mktemp -u XXXXXX)"
+			eval readonly ${ns_name}="$_ns"
 			ns_exist=false
 		else
-			eval ns='$'${ns_name}
-			cleanup_ns "$ns"
+			eval _ns='$'${ns_name}
+			cleanup_ns "$_ns"
 			ns_exist=true
 		fi
 
-		if ! ip netns add "$ns"; then
+		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_exist && ns_list="$ns_list $ns"
+		ip -n "$_ns" link set lo up
+		! $ns_exist && ns_list="$ns_list $_ns"
 	done
 	NS_LIST="$NS_LIST $ns_list"
 }
-- 
2.43.0

[PATCH mptcp-next v3 2/4] selftests: mptcp: rename ns to _ns in get_counter

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

If "setup_ns ns" is used in diag.sh, it fails with errors:

 ./mptcp_lib.sh: line 289: local: ns: readonly variable
 13 ....chk 2 cestab                                  [ OK ]
 14 ....chk 2->0 msk in use after flush               [ OK ]

That is because "ns" is also a local variable in mptcp_lib_get_counter.
To solve this, this patch renames it as "_ns".

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_lib.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index ad2ebda5cb64..16aa080c8229 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -286,11 +286,11 @@ mptcp_lib_is_v6() {
 
 # $1: ns, $2: MIB counter
 mptcp_lib_get_counter() {
-	local ns="${1}"
+	local _ns="${1}"
 	local counter="${2}"
 	local count
 
-	count=$(ip netns exec "${ns}" nstat -asz "${counter}" |
+	count=$(ip netns exec "${_ns}" nstat -asz "${counter}" |
 		awk 'NR==1 {next} {print $2}')
 	if [ -z "${count}" ]; then
 		mptcp_lib_fail_if_expected_feature "${counter} counter"
-- 
2.43.0

[PATCH mptcp-next v3 3/4] selftests: mptcp: use setup/cleanup_ns helpers

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch includes lib.sh into mptcp_lib.sh, uses setup_ns helper
defined in lib.sh to set up namespaces in mptcp_lib_ns_init(), and
uses cleanup_ns to delete namespaces in mptcp_lib_ns_exit().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_lib.sh | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 16aa080c8229..642a0ee255ef 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -1,6 +1,8 @@
 #! /bin/bash
 # SPDX-License-Identifier: GPL-2.0
 
+. "$(dirname "${0}")/../lib.sh"
+
 readonly KSFT_PASS=0
 readonly KSFT_FAIL=1
 readonly KSFT_SKIP=4
@@ -412,17 +414,10 @@ mptcp_lib_check_tools() {
 }
 
 mptcp_lib_ns_init() {
-	local sec rndh
-
-	sec=$(date +%s)
-	rndh=$(printf %x "${sec}")-$(mktemp -u XXXXXX)
+	setup_ns "${@}"
 
 	local netns
 	for netns in "${@}"; do
-		eval "${netns}=${netns}-${rndh}"
-
-		ip netns add "${!netns}" || exit ${KSFT_SKIP}
-		ip -net "${!netns}" link set lo up
 		ip netns exec "${!netns}" sysctl -q net.mptcp.enabled=1
 		ip netns exec "${!netns}" sysctl -q net.ipv4.conf.all.rp_filter=0
 		ip netns exec "${!netns}" sysctl -q net.ipv4.conf.default.rp_filter=0
@@ -430,9 +425,10 @@ mptcp_lib_ns_init() {
 }
 
 mptcp_lib_ns_exit() {
+	cleanup_ns "${@}"
+
 	local netns
 	for netns in "${@}"; do
-		ip netns del "${netns}"
 		rm -f /tmp/"${netns}".{nstat,out}
 	done
 }
-- 
2.43.0

[PATCH mptcp-next v3 4/4] selftests: mptcp: use wait_local_port_listen helper

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch includes net_helper.sh into mptcp_lib.sh, uses the helper
wait_local_port_listen() defined in it to implement the similar mptcp
helper. This can drop some duplicate code.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_lib.sh | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 642a0ee255ef..5ccb302e3fd2 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -2,6 +2,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 . "$(dirname "${0}")/../lib.sh"
+. "$(dirname "${0}")/../net_helper.sh"
 
 readonly KSFT_PASS=0
 readonly KSFT_FAIL=1
@@ -337,20 +338,7 @@ mptcp_lib_check_transfer() {
 
 # $1: ns, $2: port
 mptcp_lib_wait_local_port_listen() {
-	local listener_ns="${1}"
-	local port="${2}"
-
-	local port_hex
-	port_hex="$(printf "%04X" "${port}")"
-
-	local _
-	for _ in $(seq 10); do
-		ip netns exec "${listener_ns}" cat /proc/net/tcp* | \
-			awk "BEGIN {rc=1} {if (\$2 ~ /:${port_hex}\$/ && \$4 ~ /0A/) \
-			     {rc=0; exit}} END {exit rc}" &&
-			break
-		sleep 0.1
-	done
+	wait_local_port_listen "${@}" "tcp"
 }
 
 mptcp_lib_check_output() {
-- 
2.43.0

Re: [PATCH mptcp-next v3 0/4] use helpers in lib.sh and net_helpers.sh

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/9218177603

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/3f489d2ea6c4
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=855553


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)