[PATCH mptcp-next 0/5] add helpers and vars in mptcp_lib.sh, part 2

[PATCH mptcp-next 0/5] add helpers and vars in mptcp_lib.sh, part 2

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Depends on:
 - dump for userspace pm
 - add helpers and vars in mptcp_lib.sh, part 1

Geliang Tang (5):
  selftests: mptcp: unify namespace names to ns1/2/3/4
  selftests: mptcp: add mptcp_lib_ns_* helpers
  selftests: mptcp: add mptcp_lib_cleanup helper
  selftests: mptcp: add mptcp_lib_check_output helper
  selftests: mptcp: add mptcp_lib_evts_* helpers

 tools/testing/selftests/net/mptcp/diag.sh     |  49 +++---
 .../selftests/net/mptcp/mptcp_connect.sh      |  21 +--
 .../testing/selftests/net/mptcp/mptcp_join.sh | 103 +++---------
 .../testing/selftests/net/mptcp/mptcp_lib.sh  | 158 ++++++++++++++++++
 .../selftests/net/mptcp/mptcp_sockopt.sh      |  24 +--
 .../testing/selftests/net/mptcp/pm_netlink.sh |  33 ++--
 .../selftests/net/mptcp/simult_flows.sh       |  19 +--
 .../selftests/net/mptcp/userspace_pm.sh       |  44 +----
 8 files changed, 242 insertions(+), 209 deletions(-)

-- 
2.40.1

[PATCH mptcp-next 1/5] selftests: mptcp: unify namespace names to ns1/2/3/4

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Most scripts use ns1, ns2, ns3 and ns4 as namespace names, but ns and
ns_sbox are used in diag.sh and mptcp_sockopt.sh. To maintain consistency
with other scripts, this patch renames these variables:

    ns        -> ns1         in diag.sh
    ns_sbox   -> ns3         in mptcp_sockopt.sh

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/diag.sh     | 48 +++++++++----------
 .../selftests/net/mptcp/mptcp_sockopt.sh      | 12 ++---
 2 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index 60a7009ce1b5..e91dde816543 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -5,7 +5,7 @@
 
 sec=$(date +%s)
 rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
-ns="ns1-$rndh"
+ns1="ns1-$rndh"
 ksft_skip=4
 test_cnt=1
 timeout_poll=30
@@ -18,19 +18,19 @@ flush_pids()
 	# give it some time
 	sleep 1.1
 
-	ip netns pids "${ns}" | xargs --no-run-if-empty kill -SIGUSR1 &>/dev/null
+	ip netns pids "${ns1}" | xargs --no-run-if-empty kill -SIGUSR1 &>/dev/null
 
 	for _ in $(seq 10); do
-		[ -z "$(ip netns pids "${ns}")" ] && break
+		[ -z "$(ip netns pids "${ns1}")" ] && break
 		sleep 0.1
 	done
 }
 
 cleanup()
 {
-	ip netns pids "${ns}" | xargs --no-run-if-empty kill -SIGKILL &>/dev/null
+	ip netns pids "${ns1}" | xargs --no-run-if-empty kill -SIGKILL &>/dev/null
 
-	ip netns del $ns
+	ip netns del $ns1
 }
 
 mptcp_lib_check_mptcp
@@ -38,7 +38,7 @@ mptcp_lib_check_tools ip ss
 
 get_msk_inuse()
 {
-	ip netns exec $ns cat /proc/net/protocols | awk '$1~/^MPTCP$/{print $3}'
+	ip netns exec $ns1 cat /proc/net/protocols | awk '$1~/^MPTCP$/{print $3}'
 }
 
 __chk_nr()
@@ -73,7 +73,7 @@ __chk_msk_nr()
 	local condition=$1
 	shift 1
 
-	__chk_nr "ss -inmHMN $ns | $condition" "$@"
+	__chk_nr "ss -inmHMN $ns1 | $condition" "$@"
 }
 
 chk_msk_nr()
@@ -94,7 +94,7 @@ wait_msk_nr()
 	msg=$*
 
 	while [ $i -lt $timeout ]; do
-		nr=$(ss -inmHMN $ns | $condition)
+		nr=$(ss -inmHMN $ns1 | $condition)
 		[ $nr == $expected ] && break;
 		[ $nr -gt $max ] && max=$nr
 		i=$((i + 1))
@@ -133,7 +133,7 @@ __chk_listen()
 	local expected=$2
 	local msg="$3"
 
-	__chk_nr "ss -N $ns -Ml '$filter' | grep -c LISTEN" "$expected" "$msg" 0
+	__chk_nr "ss -N $ns1 -Ml '$filter' | grep -c LISTEN" "$expected" "$msg" 0
 }
 
 chk_msk_listen()
@@ -163,7 +163,7 @@ chk_msk_inuse()
 		msg+=" after flush"
 	fi
 
-	listen_nr=$(ss -N "${ns}" -Ml | grep -c LISTEN)
+	listen_nr=$(ss -N "${ns1}" -Ml | grep -c LISTEN)
 	expected=$((expected + listen_nr))
 
 	for _ in $(seq 10); do
@@ -186,7 +186,7 @@ chk_msk_cestab()
 		msg+=" after flush"
 	fi
 
-	__chk_nr "mptcp_lib_get_counter ${ns} MPTcpExtMPCurrEstab" \
+	__chk_nr "mptcp_lib_get_counter ${ns1} MPTcpExtMPCurrEstab" \
 		 "${expected}" "${msg}" ""
 }
 
@@ -205,24 +205,24 @@ wait_connected()
 }
 
 trap cleanup EXIT
-ip netns add $ns
-ip -n $ns link set dev lo up
+ip netns add $ns1
+ip -n $ns1 link set dev lo up
 
 echo "a" | \
 	timeout ${timeout_test} \
-		ip netns exec $ns \
+		ip netns exec $ns1 \
 			./mptcp_connect -p 10000 -l -t ${timeout_poll} -w 20 \
 				0.0.0.0 >/dev/null &
-mptcp_lib_wait_local_port_listen $ns 10000
+mptcp_lib_wait_local_port_listen $ns1 10000
 chk_msk_nr 0 "no msk on netns creation"
 chk_msk_listen 10000
 
 echo "b" | \
 	timeout ${timeout_test} \
-		ip netns exec $ns \
+		ip netns exec $ns1 \
 			./mptcp_connect -p 10000 -r 0 -t ${timeout_poll} -w 20 \
 				127.0.0.1 >/dev/null &
-wait_connected $ns 10000
+wait_connected $ns1 10000
 chk_msk_nr 2 "after MPC handshake "
 chk_msk_remote_key_nr 2 "....chk remote_key"
 chk_msk_fallback_nr 0 "....chk no fallback"
@@ -235,16 +235,16 @@ chk_msk_cestab 0 "2->0"
 
 echo "a" | \
 	timeout ${timeout_test} \
-		ip netns exec $ns \
+		ip netns exec $ns1 \
 			./mptcp_connect -p 10001 -l -s TCP -t ${timeout_poll} -w 20 \
 				0.0.0.0 >/dev/null &
-mptcp_lib_wait_local_port_listen $ns 10001
+mptcp_lib_wait_local_port_listen $ns1 10001
 echo "b" | \
 	timeout ${timeout_test} \
-		ip netns exec $ns \
+		ip netns exec $ns1 \
 			./mptcp_connect -p 10001 -r 0 -t ${timeout_poll} -w 20 \
 				127.0.0.1 >/dev/null &
-wait_connected $ns 10001
+wait_connected $ns1 10001
 chk_msk_fallback_nr 1 "check fallback"
 chk_msk_inuse 1
 chk_msk_cestab 1
@@ -257,16 +257,16 @@ NR_CLIENTS=100
 for I in `seq 1 $NR_CLIENTS`; do
 	echo "a" | \
 		timeout ${timeout_test} \
-			ip netns exec $ns \
+			ip netns exec $ns1 \
 				./mptcp_connect -p $((I+10001)) -l -w 20 \
 					-t ${timeout_poll} 0.0.0.0 >/dev/null &
 done
-mptcp_lib_wait_local_port_listen $ns $((NR_CLIENTS + 10001))
+mptcp_lib_wait_local_port_listen $ns1 $((NR_CLIENTS + 10001))
 
 for I in `seq 1 $NR_CLIENTS`; do
 	echo "b" | \
 		timeout ${timeout_test} \
-			ip netns exec $ns \
+			ip netns exec $ns1 \
 				./mptcp_connect -p $((I+10001)) -w 20 \
 					-t ${timeout_poll} 127.0.0.1 >/dev/null &
 done
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index fd7de1b3dc55..eb06c3f6184b 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -18,7 +18,7 @@ sec=$(date +%s)
 rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
 ns1="ns1-$rndh"
 ns2="ns2-$rndh"
-ns_sbox="ns_sbox-$rndh"
+ns3="ns3-$rndh"
 
 add_mark_rules()
 {
@@ -41,7 +41,7 @@ add_mark_rules()
 init()
 {
 	local netns
-	for netns in "$ns1" "$ns2" "$ns_sbox";do
+	for netns in "$ns1" "$ns2" "$ns3";do
 		ip netns add $netns || exit $ksft_skip
 		ip -net $netns link set lo up
 		ip netns exec $netns sysctl -q net.mptcp.enabled=1
@@ -80,7 +80,7 @@ init()
 cleanup()
 {
 	local netns
-	for netns in "$ns1" "$ns2" "$ns_sbox"; do
+	for netns in "$ns1" "$ns2" "$ns3"; do
 		ip netns del $netns
 	done
 	rm -f "$cin" "$cout"
@@ -223,7 +223,7 @@ do_mptcp_sockopt_tests()
 		return
 	fi
 
-	ip netns exec "$ns_sbox" ./mptcp_sockopt
+	ip netns exec "$ns3" ./mptcp_sockopt
 	lret=$?
 
 	if [ $lret -ne 0 ]; then
@@ -234,7 +234,7 @@ do_mptcp_sockopt_tests()
 	fi
 	mptcp_lib_result_pass "sockopt v4"
 
-	ip netns exec "$ns_sbox" ./mptcp_sockopt -6
+	ip netns exec "$ns3" ./mptcp_sockopt -6
 	lret=$?
 
 	if [ $lret -ne 0 ]; then
@@ -265,7 +265,7 @@ run_tests()
 
 do_tcpinq_test()
 {
-	ip netns exec "$ns_sbox" ./mptcp_inq "$@"
+	ip netns exec "$ns3" ./mptcp_inq "$@"
 	local lret=$?
 	if [ $lret -ne 0 ];then
 		ret=$lret
-- 
2.40.1

[PATCH mptcp-next 2/5] selftests: mptcp: add mptcp_lib_ns_* helpers

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Add helpers mptcp_lib_ns_init() and mptcp_lib_ns_exit() in mptcp_lib.sh to
init all namespaces ns1, ns2, ns3 and ns4. Then every test script can
invoke these helpers and use all namespaces.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/diag.sh     |  8 +--
 .../selftests/net/mptcp/mptcp_connect.sh      | 20 ++-----
 .../testing/selftests/net/mptcp/mptcp_join.sh | 19 +-----
 .../testing/selftests/net/mptcp/mptcp_lib.sh  | 59 +++++++++++++++++++
 .../selftests/net/mptcp/mptcp_sockopt.sh      | 15 +----
 .../testing/selftests/net/mptcp/pm_netlink.sh |  8 +--
 .../selftests/net/mptcp/simult_flows.sh       | 18 +-----
 .../selftests/net/mptcp/userspace_pm.sh       | 12 +---
 8 files changed, 75 insertions(+), 84 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index e91dde816543..40fc04e8e592 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -3,9 +3,7 @@
 
 . "$(dirname "${0}")/mptcp_lib.sh"
 
-sec=$(date +%s)
-rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
-ns1="ns1-$rndh"
+mptcp_lib_ns_init 1
 ksft_skip=4
 test_cnt=1
 timeout_poll=30
@@ -30,7 +28,7 @@ cleanup()
 {
 	ip netns pids "${ns1}" | xargs --no-run-if-empty kill -SIGKILL &>/dev/null
 
-	ip netns del $ns1
+	mptcp_lib_ns_exit 1
 }
 
 mptcp_lib_check_mptcp
@@ -205,8 +203,6 @@ wait_connected()
 }
 
 trap cleanup EXIT
-ip netns add $ns1
-ip -n $ns1 link set dev lo up
 
 echo "a" | \
 	timeout ${timeout_test} \
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index ea52110c3fbc..00527f4c3b98 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -121,12 +121,7 @@ while getopts "$optstring" option;do
 	esac
 done
 
-sec=$(date +%s)
-rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
-ns1="ns1-$rndh"
-ns2="ns2-$rndh"
-ns3="ns3-$rndh"
-ns4="ns4-$rndh"
+mptcp_lib_ns_init
 
 TEST_COUNT=0
 TEST_GROUP=""
@@ -138,11 +133,7 @@ cleanup()
 	rm -f "$sin" "$sout"
 	rm -f "$capout"
 
-	local netns
-	for netns in "$ns1" "$ns2" "$ns3" "$ns4";do
-		ip netns del $netns
-		rm -f /tmp/$netns.{nstat,out}
-	done
+	mptcp_lib_ns_exit
 }
 
 mptcp_lib_check_mptcp
@@ -158,11 +149,6 @@ cin_disconnect="$cin".disconnect
 cout_disconnect="$cout".disconnect
 trap cleanup EXIT
 
-for i in "$ns1" "$ns2" "$ns3" "$ns4";do
-	ip netns add $i || exit $ksft_skip
-	ip -net $i link set lo up
-done
-
 #  "$ns1"              ns2                    ns3                     ns4
 # ns1eth2    ns2eth1   ns2eth3      ns3eth2   ns3eth4       ns4eth3
 #                           - drop 1% ->            reorder 25%
@@ -251,6 +237,8 @@ fi
 
 check_mptcp_disabled()
 {
+	: "${rndh:?}"
+
 	local disabled_ns="ns_disabled-$rndh"
 	ip netns add ${disabled_ns} || exit $ksft_skip
 
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index a8d08554efc9..7e354ff5d717 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -23,8 +23,6 @@ tmpfile=""
 cout=""
 check_output_err=""
 capout=""
-ns1=""
-ns2=""
 ksft_skip=4
 iptables="iptables"
 ip6tables="ip6tables"
@@ -86,21 +84,12 @@ init_partial()
 {
 	capout=$(mktemp)
 
-	local sec rndh
-	sec=$(date +%s)
-	rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
-
-	ns1="ns1-$rndh"
-	ns2="ns2-$rndh"
+	mptcp_lib_ns_init 2
 
 	local netns
 	for netns in "$ns1" "$ns2"; do
-		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.mptcp.pm_type=0 2>/dev/null || true
-		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
 		if $checksum; then
 			ip netns exec $netns sysctl -q net.mptcp.checksum_enabled=1
 		fi
@@ -145,11 +134,7 @@ cleanup_partial()
 {
 	rm -f "$capout"
 
-	local netns
-	for netns in "$ns1" "$ns2"; do
-		ip netns del $netns
-		rm -f /tmp/$netns.{nstat,out}
-	done
+	mptcp_lib_ns_exit 2
 }
 
 init() {
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 6d9a2af85a8d..bfc535594675 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -350,3 +350,62 @@ mptcp_lib_check_tools() {
 		esac
 	done
 }
+
+rndh=""
+ns1=""
+ns2=""
+ns3=""
+ns4=""
+
+mptcp_lib_ns_init() {
+	: "${rndh?}"
+	: "${ns1?}"
+	: "${ns2?}"
+	: "${ns3?}"
+	: "${ns4?}"
+
+	local nr="${1:-4}"
+	local i=1
+	local sec
+
+	sec=$(date +%s)
+	rndh=$(printf %x "$sec")-$(mktemp -u XXXXXX)
+
+	ns1="ns1-$rndh"
+	ns2="ns2-$rndh"
+	ns3="ns3-$rndh"
+	ns4="ns4-$rndh"
+
+	local netns
+	for netns in "$ns1" "$ns2" "$ns3" "$ns4"; do
+		if [ "$i" -gt "$nr" ]; then
+			break
+		fi
+		ip netns add "$netns" || exit ${KSFT_SKIP}
+		ip -net "$netns" link set lo up
+
+		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
+		i=$((i + 1))
+	done
+}
+
+mptcp_lib_ns_exit() {
+	: "${ns1:?}"
+	: "${ns2:?}"
+	: "${ns3:?}"
+	: "${ns4:?}"
+
+	local nr="${1:-4}"
+	local i=1
+
+	local netns
+	for netns in "$ns1" "$ns2" "$ns3" "$ns4"; do
+		if [ "$i" -gt "$nr" ]; then
+			break
+		fi
+		ip netns del "$netns"
+		rm -f /tmp/"$netns".{nstat,out}
+		i=$((i + 1))
+	done
+}
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index eb06c3f6184b..5d8881d11fb0 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -14,11 +14,7 @@ timeout_test=$((timeout_poll * 2 + 1))
 iptables="iptables"
 ip6tables="ip6tables"
 
-sec=$(date +%s)
-rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
-ns1="ns1-$rndh"
-ns2="ns2-$rndh"
-ns3="ns3-$rndh"
+mptcp_lib_ns_init 3
 
 add_mark_rules()
 {
@@ -42,11 +38,7 @@ init()
 {
 	local netns
 	for netns in "$ns1" "$ns2" "$ns3";do
-		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
 	done
 
 	local i
@@ -79,10 +71,7 @@ init()
 
 cleanup()
 {
-	local netns
-	for netns in "$ns1" "$ns2" "$ns3"; do
-		ip netns del $netns
-	done
+	mptcp_lib_ns_exit 3
 	rm -f "$cin" "$cout"
 	rm -f "$sin" "$sout"
 }
diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
index cb6ea67e688b..2ea80f1b0aee 100755
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -24,15 +24,13 @@ while getopts "$optstring" option;do
 	esac
 done
 
-sec=$(date +%s)
-rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
-ns1="ns1-$rndh"
+mptcp_lib_ns_init 1
 err=$(mktemp)
 
 cleanup()
 {
 	rm -f $err
-	ip netns del $ns1
+	mptcp_lib_ns_exit 1
 }
 
 mptcp_lib_check_mptcp
@@ -40,8 +38,6 @@ mptcp_lib_check_tools ip
 
 trap cleanup EXIT
 
-ip netns add $ns1 || exit $ksft_skip
-ip -net $ns1 link set lo up
 ip netns exec $ns1 sysctl -q net.mptcp.enabled=1
 
 check()
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index 7d8388ecc966..a45b6ad41aea 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -3,11 +3,7 @@
 
 . "$(dirname "${0}")/mptcp_lib.sh"
 
-sec=$(date +%s)
-rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
-ns1="ns1-$rndh"
-ns2="ns2-$rndh"
-ns3="ns3-$rndh"
+mptcp_lib_ns_init 3
 capture=false
 ksft_skip=4
 timeout_poll=30
@@ -36,10 +32,7 @@ cleanup()
 	rm -f "$large" "$small"
 	rm -f "$capout"
 
-	local netns
-	for netns in "$ns1" "$ns2" "$ns3";do
-		ip netns del $netns
-	done
+	mptcp_lib_ns_exit 3
 }
 
 mptcp_lib_check_mptcp
@@ -65,13 +58,6 @@ setup()
 
 	trap cleanup EXIT
 
-	for i in "$ns1" "$ns2" "$ns3";do
-		ip netns add $i || exit $ksft_skip
-		ip -net $i link set lo up
-		ip netns exec $i sysctl -q net.ipv4.conf.all.rp_filter=0
-		ip netns exec $i sysctl -q net.ipv4.conf.default.rp_filter=0
-	done
-
 	ip link add ns1eth1 netns "$ns1" type veth peer name ns2eth1 netns "$ns2"
 	ip link add ns1eth2 netns "$ns1" type veth peer name ns2eth2 netns "$ns2"
 	ip link add ns2eth3 netns "$ns2" type veth peer name ns3eth1 netns "$ns3"
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 629fc5d0ecc5..71caa6b449a8 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -50,10 +50,7 @@ app6_port=50004
 client_addr_id=${RANDOM:0:2}
 server_addr_id=${RANDOM:0:2}
 
-sec=$(date +%s)
-rndh=$(printf %x "$sec")-$(mktemp -u XXXXXX)
-ns1="ns1-$rndh"
-ns2="ns2-$rndh"
+mptcp_lib_ns_init 2
 ret=0
 test_name=""
 
@@ -118,10 +115,7 @@ cleanup()
 		mptcp_lib_kill_wait $pid
 	done
 
-	local netns
-	for netns in "$ns1" "$ns2" ;do
-		ip netns del "$netns"
-	done
+	mptcp_lib_ns_exit 2
 
 	rm -rf $file $client_evts $server_evts
 
@@ -132,8 +126,6 @@ trap cleanup EXIT
 
 # Create and configure network namespaces for testing
 for i in "$ns1" "$ns2" ;do
-	ip netns add "$i" || exit 1
-	ip -net "$i" link set lo up
 	ip netns exec "$i" sysctl -q net.mptcp.enabled=1
 	ip netns exec "$i" sysctl -q net.mptcp.pm_type=1
 done
-- 
2.40.1

[PATCH mptcp-next 3/5] selftests: mptcp: add mptcp_lib_cleanup helper

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch adds a new helper mptcp_lib_cleanup() in mptcp_lib.sh, it's
a public cleanup interface, being invoked in every cleanup() in all
scripts.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/diag.sh          | 1 +
 tools/testing/selftests/net/mptcp/mptcp_connect.sh | 1 +
 tools/testing/selftests/net/mptcp/mptcp_join.sh    | 1 +
 tools/testing/selftests/net/mptcp/mptcp_lib.sh     | 4 ++++
 tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 1 +
 tools/testing/selftests/net/mptcp/pm_netlink.sh    | 1 +
 tools/testing/selftests/net/mptcp/simult_flows.sh  | 1 +
 tools/testing/selftests/net/mptcp/userspace_pm.sh  | 1 +
 8 files changed, 11 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index 40fc04e8e592..bd6c8def8b5b 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -29,6 +29,7 @@ cleanup()
 	ip netns pids "${ns1}" | xargs --no-run-if-empty kill -SIGKILL &>/dev/null
 
 	mptcp_lib_ns_exit 1
+	mptcp_lib_cleanup
 }
 
 mptcp_lib_check_mptcp
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 00527f4c3b98..5273650a78f2 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -134,6 +134,7 @@ cleanup()
 	rm -f "$capout"
 
 	mptcp_lib_ns_exit
+	mptcp_lib_cleanup
 }
 
 mptcp_lib_check_mptcp
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 7e354ff5d717..ceaaf155cc32 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -166,6 +166,7 @@ cleanup()
 	rm -f "$tmpfile"
 	rm -rf $evts_ns1 $evts_ns2
 	rm -f $check_output_err
+	mptcp_lib_cleanup
 	cleanup_partial
 }
 
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index bfc535594675..01b0e43eaa80 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -409,3 +409,7 @@ mptcp_lib_ns_exit() {
 		i=$((i + 1))
 	done
 }
+
+mptcp_lib_cleanup() {
+	echo "cleanup"
+}
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 5d8881d11fb0..f82553c56512 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -74,6 +74,7 @@ cleanup()
 	mptcp_lib_ns_exit 3
 	rm -f "$cin" "$cout"
 	rm -f "$sin" "$sout"
+	mptcp_lib_cleanup
 }
 
 mptcp_lib_check_mptcp
diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
index 2ea80f1b0aee..5da9ccafc47e 100755
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -31,6 +31,7 @@ cleanup()
 {
 	rm -f $err
 	mptcp_lib_ns_exit 1
+	mptcp_lib_cleanup
 }
 
 mptcp_lib_check_mptcp
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index a45b6ad41aea..e9820aec7ffa 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -33,6 +33,7 @@ cleanup()
 	rm -f "$capout"
 
 	mptcp_lib_ns_exit 3
+	mptcp_lib_cleanup
 }
 
 mptcp_lib_check_mptcp
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 71caa6b449a8..dab9115738d7 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -119,6 +119,7 @@ cleanup()
 
 	rm -rf $file $client_evts $server_evts
 
+	mptcp_lib_cleanup
 	_printf "Done\n"
 }
 
-- 
2.40.1

[PATCH mptcp-next 4/5] selftests: mptcp: add mptcp_lib_check_output helper

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Unify check_output() in mptcp_join.sh and check() in pm_netlink.sh into
a new public function mptcp_lib_check_output() in mptcp_lib.sh. And use
mptcp_lib_print_ok() and _err() in it to print test results with colors.

Use this new helper instead of check_output() and check().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 21 +-----------
 .../testing/selftests/net/mptcp/mptcp_lib.sh  | 32 +++++++++++++++++++
 .../testing/selftests/net/mptcp/pm_netlink.sh | 24 ++++----------
 3 files changed, 40 insertions(+), 37 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index ceaaf155cc32..f75d22dbb28d 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -21,7 +21,6 @@ cinfail=""
 cinsent=""
 tmpfile=""
 cout=""
-check_output_err=""
 capout=""
 ksft_skip=4
 iptables="iptables"
@@ -151,7 +150,6 @@ init() {
 	cout=$(mktemp)
 	evts_ns1=$(mktemp)
 	evts_ns2=$(mktemp)
-	check_output_err=$(mktemp)
 
 	trap cleanup EXIT
 
@@ -165,7 +163,6 @@ cleanup()
 	rm -f "$sin" "$sout" "$cinsent" "$cinfail"
 	rm -f "$tmpfile"
 	rm -rf $evts_ns1 $evts_ns2
-	rm -f $check_output_err
 	mptcp_lib_cleanup
 	cleanup_partial
 }
@@ -3345,26 +3342,10 @@ userspace_pm_get_addr()
 
 check_output()
 {
-	local cmd="$1"
-	local expected="$2"
 	local msg="$3"
-	local out=`$cmd 2>$check_output_err`
-	local cmd_ret=$?
 
 	printf "%-42s" "$msg"
-	if [ $cmd_ret -ne 0 ]; then
-		mptcp_lib_print_err "[FAIL] command execution '$cmd' stderr "
-		cat $check_output_err
-		ret=${KSFT_FAIL}
-		return $cmd_ret
-	elif [ "$out" = "$expected" ]; then
-		mptcp_lib_print_ok "[ OK ]"
-		return 0
-	else
-		mptcp_lib_print_err "[FAIL] expected '$expected' got '$out'"
-		ret=${KSFT_FAIL}
-		return 1
-	fi
+	mptcp_lib_check_output "${1}" "${2}"
 }
 
 userspace_tests()
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 01b0e43eaa80..fd2ed3b5137c 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -410,6 +410,38 @@ mptcp_lib_ns_exit() {
 	done
 }
 
+check_output_err=$(mktemp)
+
+mptcp_lib_check_output() {
+	: "${check_output_err:?}"
+	: "${ret:?}"
+
+	local cmd="$1"
+	local expected="$2"
+	local out
+	local cmd_ret
+
+	out=$($cmd 2>"$check_output_err")
+	cmd_ret=$?
+
+	if [ $cmd_ret -ne 0 ]; then
+		mptcp_lib_print_err "[FAIL] command execution '$cmd' stderr "
+		cat "$check_output_err"
+		ret=${KSFT_FAIL}
+		return $cmd_ret
+	elif [ "$out" = "$expected" ]; then
+		mptcp_lib_print_ok "[ OK ]"
+		return 0
+	else
+		mptcp_lib_print_err "[FAIL] expected '$expected' got '$out'"
+		ret=${KSFT_FAIL}
+		return 1
+	fi
+}
+
 mptcp_lib_cleanup() {
+	: "${check_output_err:?}"
+
 	echo "cleanup"
+	rm -f "$check_output_err"
 }
diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
index 5da9ccafc47e..eb3440d2620e 100755
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -25,11 +25,9 @@ while getopts "$optstring" option;do
 done
 
 mptcp_lib_ns_init 1
-err=$(mktemp)
 
 cleanup()
 {
-	rm -f $err
 	mptcp_lib_ns_exit 1
 	mptcp_lib_cleanup
 }
@@ -43,26 +41,18 @@ ip netns exec $ns1 sysctl -q net.mptcp.enabled=1
 
 check()
 {
-	local cmd="$1"
-	local expected="$2"
 	local msg="$3"
-	local out=`$cmd 2>$err`
-	local cmd_ret=$?
 
 	printf "%-50s" "$msg"
-	if [ $cmd_ret -ne 0 ]; then
-		echo "[FAIL] command execution '$cmd' stderr "
-		cat $err
-		mptcp_lib_result_fail "${msg} # error ${cmd_ret}"
-		ret=1
-	elif [ "$out" = "$expected" ]; then
-		echo "[ OK ]"
+	# ${*} doesn't work here since there're spaces in some arguments.
+	mptcp_lib_check_output "${1}" "${2}"
+	local rc=$?
+	if [ ${rc} -eq 0 ]; then
 		mptcp_lib_result_pass "${msg}"
-	else
-		echo -n "[FAIL] "
-		echo "expected '$expected' got '$out'"
+	elif [ ${rc} -eq 1 ]; then
 		mptcp_lib_result_fail "${msg} # different output"
-		ret=1
+	else
+		mptcp_lib_result_fail "${msg} # error ${rc}"
 	fi
 }
 
-- 
2.40.1

[PATCH mptcp-next 5/5] selftests: mptcp: add mptcp_lib_evts_* helpers

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

To avoid duplicated code in different MPTCP selftests, we can add and
use helpers defined in mptcp_lib.sh.

This patch unifies "pm_nl_ctl events" related code in userspace_pm.sh
and mptcp_join.sh into four helpers: mptcp_lib_evts_init(), _start(),
_kill() and _remove(). Define them in mptcp_lib.sh and use these new
helpers in both scripts.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 62 +++++++-----------
 .../testing/selftests/net/mptcp/mptcp_lib.sh  | 63 +++++++++++++++++++
 .../selftests/net/mptcp/userspace_pm.sh       | 31 ++-------
 3 files changed, 92 insertions(+), 64 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index f75d22dbb28d..d4798495a17d 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -33,10 +33,6 @@ ip_mptcp=0
 check_invert=0
 validate_checksum=false
 init=0
-evts_ns1=""
-evts_ns2=""
-evts_ns1_pid=0
-evts_ns2_pid=0
 last_test_failed=0
 last_test_skipped=0
 last_test_ignored=1
@@ -148,8 +144,7 @@ init() {
 	cin=$(mktemp)
 	cinsent=$(mktemp)
 	cout=$(mktemp)
-	evts_ns1=$(mktemp)
-	evts_ns2=$(mktemp)
+	mptcp_lib_evts_init
 
 	trap cleanup EXIT
 
@@ -162,7 +157,7 @@ cleanup()
 	rm -f "$cin" "$cout" "$sinfail"
 	rm -f "$sin" "$sout" "$cinsent" "$cinfail"
 	rm -f "$tmpfile"
-	rm -rf $evts_ns1 $evts_ns2
+	mptcp_lib_evts_remove
 	mptcp_lib_cleanup
 	cleanup_partial
 }
@@ -427,12 +422,7 @@ reset_with_events()
 {
 	reset "${1}" || return 1
 
-	:> "$evts_ns1"
-	:> "$evts_ns2"
-	ip netns exec $ns1 ./pm_nl_ctl events >> "$evts_ns1" 2>&1 &
-	evts_ns1_pid=$!
-	ip netns exec $ns2 ./pm_nl_ctl events >> "$evts_ns2" 2>&1 &
-	evts_ns2_pid=$!
+	mptcp_lib_evts_start "${ns1}" "${ns2}"
 }
 
 reset_with_tcp_filter()
@@ -602,12 +592,6 @@ wait_mpj()
 	done
 }
 
-kill_events_pids()
-{
-	mptcp_lib_kill_wait $evts_ns1_pid
-	mptcp_lib_kill_wait $evts_ns2_pid
-}
-
 pm_nl_set_limits()
 {
 	local ns=$1
@@ -2877,9 +2861,9 @@ add_addr_ports_tests()
 		chk_add_nr 1 1 1
 		chk_rm_nr 1 1 invert
 
-		verify_listener_events $evts_ns1 $LISTENER_CREATED $AF_INET 10.0.2.1 10100
-		verify_listener_events $evts_ns1 $LISTENER_CLOSED $AF_INET 10.0.2.1 10100
-		kill_events_pids
+		verify_listener_events $server_evts $LISTENER_CREATED $AF_INET 10.0.2.1 10100
+		verify_listener_events $server_evts $LISTENER_CLOSED $AF_INET 10.0.2.1 10100
+		mptcp_lib_evts_kill
 	fi
 
 	# subflow and signal with port, remove
@@ -3252,10 +3236,10 @@ fail_tests()
 # $1: ns ; $2: addr ; $3: id
 userspace_pm_add_addr()
 {
-	local evts=$evts_ns1
+	local evts=$server_evts
 	local tk
 
-	[ "$1" == "$ns2" ] && evts=$evts_ns2
+	[ "$1" == "$ns2" ] && evts=$client_evts
 	tk=$(mptcp_lib_evts_get_info token "$evts")
 
 	ip netns exec $1 ./pm_nl_ctl ann $2 token $tk id $3
@@ -3265,11 +3249,11 @@ userspace_pm_add_addr()
 # $1: ns ; $2: id
 userspace_pm_rm_addr()
 {
-	local evts=$evts_ns1
+	local evts=$server_evts
 	local tk
 	local cnt
 
-	[ "$1" == "$ns2" ] && evts=$evts_ns2
+	[ "$1" == "$ns2" ] && evts=$client_evts
 	tk=$(mptcp_lib_evts_get_info token "$evts")
 
 	cnt=$(rm_addr_count ${1})
@@ -3280,10 +3264,10 @@ userspace_pm_rm_addr()
 # $1: ns ; $2: addr ; $3: id
 userspace_pm_add_sf()
 {
-	local evts=$evts_ns1
+	local evts=$server_evts
 	local tk da dp
 
-	[ "$1" == "$ns2" ] && evts=$evts_ns2
+	[ "$1" == "$ns2" ] && evts=$client_evts
 	tk=$(mptcp_lib_evts_get_info token "$evts")
 	da=$(mptcp_lib_evts_get_info daddr4 "$evts")
 	dp=$(mptcp_lib_evts_get_info dport "$evts")
@@ -3296,13 +3280,13 @@ userspace_pm_add_sf()
 # $1: ns ; $2: addr $3: event type
 userspace_pm_rm_sf()
 {
-	local evts=$evts_ns1
+	local evts=$server_evts
 	local t=${3:-1}
 	local ip
 	local tk da dp sp
 	local cnt
 
-	[ "$1" == "$ns2" ] && evts=$evts_ns2
+	[ "$1" == "$ns2" ] && evts=$client_evts
 	[ -n "$(mptcp_lib_evts_get_info "saddr4" "$evts" $t)" ] && ip=4
 	[ -n "$(mptcp_lib_evts_get_info "saddr6" "$evts" $t)" ] && ip=6
 	tk=$(mptcp_lib_evts_get_info token "$evts")
@@ -3319,10 +3303,10 @@ userspace_pm_rm_sf()
 # $1: ns
 userspace_pm_dump()
 {
-	local evts=$evts_ns1
+	local evts=$server_evts
 	local tk
 
-	[ "$1" == "$ns2" ] && evts=$evts_ns2
+	[ "$1" == "$ns2" ] && evts=$client_evts
 	tk=$(mptcp_lib_evts_get_info token "$evts")
 
 	ip netns exec $1 ./pm_nl_ctl dump token $tk
@@ -3331,10 +3315,10 @@ userspace_pm_dump()
 # $1: ns ; $2: id
 userspace_pm_get_addr()
 {
-	local evts=$evts_ns1
+	local evts=$server_evts
 	local tk
 
-	[ "$1" == "$ns2" ] && evts=$evts_ns2
+	[ "$1" == "$ns2" ] && evts=$client_evts
 	tk=$(mptcp_lib_evts_get_info token "$evts")
 
 	ip netns exec $1 ./pm_nl_ctl get $2 token $tk
@@ -3468,7 +3452,7 @@ userspace_tests()
 		chk_rm_nr 2 2 invert
 		chk_mptcp_info subflows 0 subflows 0
 		chk_subflows_total 1 1
-		kill_events_pids
+		mptcp_lib_evts_kill
 		mptcp_lib_kill_wait $tests_pid
 	fi
 
@@ -3505,7 +3489,7 @@ userspace_tests()
 		chk_rm_nr 1 1
 		chk_mptcp_info subflows 0 subflows 0
 		chk_subflows_total 1 1
-		kill_events_pids
+		mptcp_lib_evts_kill
 		mptcp_lib_kill_wait $tests_pid
 	fi
 
@@ -3529,7 +3513,7 @@ userspace_tests()
 		chk_join_nr 1 1 1
 		chk_mptcp_info subflows 1 subflows 1
 		chk_subflows_total 2 2
-		kill_events_pids
+		mptcp_lib_evts_kill
 		mptcp_lib_kill_wait $tests_pid
 	fi
 
@@ -3553,7 +3537,7 @@ userspace_tests()
 		chk_rst_nr 0 0 invert
 		chk_mptcp_info subflows 1 subflows 1
 		chk_subflows_total 1 1
-		kill_events_pids
+		mptcp_lib_evts_kill
 		mptcp_lib_kill_wait $tests_pid
 	fi
 
@@ -3579,7 +3563,7 @@ userspace_tests()
 		chk_rst_nr 0 0 invert
 		chk_mptcp_info subflows 1 subflows 1
 		chk_subflows_total 1 1
-		kill_events_pids
+		mptcp_lib_evts_kill
 		mptcp_lib_kill_wait $tests_pid
 	fi
 }
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index fd2ed3b5137c..67ceefe70059 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -439,6 +439,69 @@ mptcp_lib_check_output() {
 	fi
 }
 
+server_evts=""
+client_evts=""
+server_evts_pid=0
+client_evts_pid=0
+
+# server_evts(_pid) and client_evts(_pid) are needed
+# by mptcp_lib_evts_init, _start, _kill and _remove.
+mptcp_lib_evts_init() {
+	: "${server_evts?}"
+	: "${client_evts?}"
+
+	if [ -z "${server_evts}" ]; then
+		server_evts=$(mktemp)
+	fi
+	if [ -z "${client_evts}" ]; then
+		client_evts=$(mktemp)
+	fi
+}
+
+# $1 ns1, $2 ns2
+mptcp_lib_evts_start() {
+	: "${server_evts:?}"
+	: "${client_evts:?}"
+	: "${server_evts_pid:?}"
+	: "${client_evts_pid:?}"
+
+	local ns_1="${1}"
+	local ns_2="${2}"
+
+	:>"$server_evts"
+	:>"$client_evts"
+
+	if [ "${server_evts_pid}" -ne 0 ]; then
+		mptcp_lib_kill_wait "${server_evts_pid}"
+	fi
+	ip netns exec "${ns_1}" ./pm_nl_ctl events >> "${server_evts}" 2>&1 &
+	server_evts_pid=$!
+
+	if [ "${client_evts_pid}" -ne 0 ]; then
+		mptcp_lib_kill_wait "${client_evts_pid}"
+	fi
+	ip netns exec "${ns_2}" ./pm_nl_ctl events >> "${client_evts}" 2>&1 &
+	client_evts_pid=$!
+}
+
+mptcp_lib_evts_kill() {
+	: "${server_evts_pid:?}"
+	: "${client_evts_pid:?}"
+
+	mptcp_lib_kill_wait "${server_evts_pid}"
+	mptcp_lib_kill_wait "${client_evts_pid}"
+
+	server_evts_pid=0
+	client_evts_pid=0
+}
+
+mptcp_lib_evts_remove() {
+	: "${server_evts:?}"
+	: "${client_evts:?}"
+
+	rm -rf "${server_evts}" "${client_evts}"
+}
+
 mptcp_lib_cleanup() {
 	: "${check_output_err:?}"
 
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index dab9115738d7..57fb990cefe9 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -30,10 +30,6 @@ AF_INET=2
 AF_INET6=10
 
 file=""
-server_evts=""
-client_evts=""
-server_evts_pid=0
-client_evts_pid=0
 client4_pid=0
 server4_pid=0
 client6_pid=0
@@ -109,15 +105,16 @@ cleanup()
 
 	# Terminate the MPTCP connection and related processes
 	local pid
-	for pid in $client4_pid $server4_pid $client6_pid $server6_pid\
-		   $server_evts_pid $client_evts_pid
+	for pid in $client4_pid $server4_pid $client6_pid $server6_pid
 	do
 		mptcp_lib_kill_wait $pid
 	done
+	mptcp_lib_evts_kill
 
 	mptcp_lib_ns_exit 2
 
-	rm -rf $file $client_evts $server_evts
+	rm -rf $file
+	mptcp_lib_evts_remove
 
 	mptcp_lib_cleanup
 	_printf "Done\n"
@@ -175,24 +172,8 @@ make_connection()
 
 	# Capture netlink events over the two network namespaces running
 	# the MPTCP client and server
-	if [ -z "$client_evts" ]; then
-		client_evts=$(mktemp)
-	fi
-	:>"$client_evts"
-	if [ $client_evts_pid -ne 0 ]; then
-		mptcp_lib_kill_wait $client_evts_pid
-	fi
-	ip netns exec "$ns2" ./pm_nl_ctl events >> "$client_evts" 2>&1 &
-	client_evts_pid=$!
-	if [ -z "$server_evts" ]; then
-		server_evts=$(mktemp)
-	fi
-	:>"$server_evts"
-	if [ $server_evts_pid -ne 0 ]; then
-		mptcp_lib_kill_wait $server_evts_pid
-	fi
-	ip netns exec "$ns1" ./pm_nl_ctl events >> "$server_evts" 2>&1 &
-	server_evts_pid=$!
+	mptcp_lib_evts_init
+	mptcp_lib_evts_start "${ns1}" "${ns2}"
 	sleep 0.5
 
 	# Run the server
-- 
2.40.1

Re: [PATCH mptcp-next 0/5] add helpers and vars in mptcp_lib.sh, part 2

[Matthieu Baerts]

Hi Geliang,

On 19/02/2024 10:29, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Depends on:
>  - dump for userspace pm
>  - add helpers and vars in mptcp_lib.sh, part 1
> 
> Geliang Tang (5):
>   selftests: mptcp: unify namespace names to ns1/2/3/4
>   selftests: mptcp: add mptcp_lib_ns_* helpers
>   selftests: mptcp: add mptcp_lib_cleanup helper
>   selftests: mptcp: add mptcp_lib_check_output helper
>   selftests: mptcp: add mptcp_lib_evts_* helpers

Good idea to reduce duplicated code between selftests. Still, I think we
should not go "too far", and hide the use of some variables and files.
For example, I think it is important not to have hidden global
variables, or declare it on one file, and use it elsewhere (except if
they are prefixed with the name of the file: MPTCP_LIB_xxx).

Please see my comments on the individual patches.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next 1/5] selftests: mptcp: unify namespace names to ns1/2/3/4

[Matthieu Baerts]

Hi Geliang,

On 19/02/2024 10:29, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Most scripts use ns1, ns2, ns3 and ns4 as namespace names, but ns and
> ns_sbox are used in diag.sh and mptcp_sockopt.sh. To maintain consistency
> with other scripts, this patch renames these variables:

If it is just for consistency, I don't think we should rename these
variables:

>     ns        -> ns1         in diag.sh

For me, adding a number as suffix means there are multiple netns
interacting with each others. Here, there is only one, that would be
confusing.

If this rename is not needed to fix something, I would then prefer to
avoid such modifications: that will "annoy" us in case of backports →
more risks of having conflicts, but mainly backports done by the stable
team will not detect issues where the wrong variable is used (the stable
team only compiles the .c code).

>     ns_sbox   -> ns3         in mptcp_sockopt.sh

To be honest, I'm not a big fan of having numbers as suffix: when
looking at the names, it is not clear what is the difference between
'ns1' and 'ns2' for example. We should have probably used "_client",
"_server", "_router", "_shaper", etc. from the beginning. Too late now.

Hence, I would prefer here to keep 'ns_sbox', otherwise we might think
that ns3 is connected to ns1 and/or ns2, which is not the case.

WDYT?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next 2/5] selftests: mptcp: add mptcp_lib_ns_* helpers

[Matthieu Baerts]

Hi Geliang,

On 19/02/2024 10:29, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Add helpers mptcp_lib_ns_init() and mptcp_lib_ns_exit() in mptcp_lib.sh to
> init all namespaces ns1, ns2, ns3 and ns4. Then every test script can
> invoke these helpers and use all namespaces.

Good idea to move duplicated code to the _lib.sh file.

> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>

(...)

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index 6d9a2af85a8d..bfc535594675 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -350,3 +350,62 @@ mptcp_lib_check_tools() {
>  		esac
>  	done
>  }
> +
> +rndh=""

I guess you "export" it because it is needed for the captured files,
right? If yes, why not using the "${ns}" variables in a dedicated commit
before to ease this refactoring?

> +ns1=""
> +ns2=""
> +ns3=""
> +ns4=""

I really don't think it is a good idea to hide these variables: if you
open 'mptcp_connect.sh', it will be confusing not to see where these
variables have been declared. If you declare variables (or functions) in
the lib, they should be prefixed MPTCP_LIB_ (or mptcp_lib_ for the
functions) I think.

Instead, I think you should keep the ns* variables declared at the top
of each file, but call "mptcp_lib_ns_init" with the variables to
initiate, e.g.

  ns1=""
  ns2=""
  ns_disabled=""

  (...)

  mptcp_lib_ns_init ns1 ns2 ns_disabled

in mptcp_lib_ns_init(), you can do something like:

  (...)

  local ns_i
  for ns_i in "${@}"; do
      eval "${ns_i}=${ns_i}-${rndh}"

      ip netns add "${!ns_i}" || (...)

(so you can use the version with '!' in the for loop)

> +mptcp_lib_ns_init() {
> +	: "${rndh?}"
> +	: "${ns1?}"
> +	: "${ns2?}"
> +	: "${ns3?}"
> +	: "${ns4?}"
> +
> +	local nr="${1:-4}"
> +	local i=1
> +	local sec
> +
> +	sec=$(date +%s)
> +	rndh=$(printf %x "$sec")-$(mktemp -u XXXXXX)
> +
> +	ns1="ns1-$rndh"
> +	ns2="ns2-$rndh"
> +	ns3="ns3-$rndh"
> +	ns4="ns4-$rndh"

In this mptcp_lib.sh, probably good to continue to use {} around
variables ('${rndh}' instead of '$rndh') like in the rest of the file:
clearer, and we avoid typos.

> +
> +	local netns
> +	for netns in "$ns1" "$ns2" "$ns3" "$ns4"; do
> +		if [ "$i" -gt "$nr" ]; then
> +			break
> +		fi

(see above)

> +		ip netns add "$netns" || exit ${KSFT_SKIP}
> +		ip -net "$netns" link set lo up
> +
> +		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

Mmh, is it OK to do that for all netns? We were not doing that before
for diag.sh, but I suppose that's because it was not communicating with
other netns, right? So fine to do that everywhere?

That's a behaviour change. Please always clearly mention behaviour
changes in the commit message when you do such refactoring, where we
would expect no behaviour changes.

While at it, maybe we want to add:

  ip netns exec "${!ns_i}" sysctl -q net.mptcp.enabled=1

That should be the default value. If not, good to override. (I think it
is needed for some RHEL versions.) If yes, please add that in the commit
message as well.

> +		i=$((i + 1))
> +	done
> +}
> +
> +mptcp_lib_ns_exit() {
> +	: "${ns1:?}"
> +	: "${ns2:?}"
> +	: "${ns3:?}"
> +	: "${ns4:?}"
> +
> +	local nr="${1:-4}"
> +	local i=1
> +
> +	local netns
> +	for netns in "$ns1" "$ns2" "$ns3" "$ns4"; do
> +		if [ "$i" -gt "$nr" ]; then
> +			break
> +		fi

Same here, best to clearer to pass the variables names and accept a
variable number of parameters:


  mptcp_lib_ns_exit "${ns1}" "${ns2}"

  local ns_i
  for ns_i in "${@}"; do
      ip netns del "${ns_i}"

> +		ip netns del "$netns"
> +		rm -f /tmp/"$netns".{nstat,out}

Here, it is also a behaviour change. I guess that's fine, but still,
please mention that in the commit message.

> +		i=$((i + 1))
> +	done
> +}

(...)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next 3/5] selftests: mptcp: add mptcp_lib_cleanup helper

[Matthieu Baerts]

Hi Geliang,

On 19/02/2024 10:29, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch adds a new helper mptcp_lib_cleanup() in mptcp_lib.sh, it's
> a public cleanup interface, being invoked in every cleanup() in all
> scripts.

When looking at the commit alone, it is difficult to understand why it
is needed. In such cases, to help reviewers and dev later, please add
something like: This will be used in the next commit(s) for XXX.

(...)

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index bfc535594675..01b0e43eaa80 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -409,3 +409,7 @@ mptcp_lib_ns_exit() {
>  		i=$((i + 1))
>  	done
>  }
> +
> +mptcp_lib_cleanup() {
> +	echo "cleanup"

Do we want to print this everywhere?

> +}

(...)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next 4/5] selftests: mptcp: add mptcp_lib_check_output helper

[Matthieu Baerts]

On 19/02/2024 10:29, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Unify check_output() in mptcp_join.sh and check() in pm_netlink.sh into
> a new public function mptcp_lib_check_output() in mptcp_lib.sh. And use
> mptcp_lib_print_ok() and _err() in it to print test results with colors.
> 
> Use this new helper instead of check_output() and check().
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  .../testing/selftests/net/mptcp/mptcp_join.sh | 21 +-----------
>  .../testing/selftests/net/mptcp/mptcp_lib.sh  | 32 +++++++++++++++++++
>  .../testing/selftests/net/mptcp/pm_netlink.sh | 24 ++++----------
>  3 files changed, 40 insertions(+), 37 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index ceaaf155cc32..f75d22dbb28d 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -21,7 +21,6 @@ cinfail=""
>  cinsent=""
>  tmpfile=""
>  cout=""
> -check_output_err=""

Same as in a previous patch: to avoid some confusions, I think we should
keep global variables declared here and not in mptcp_lib.sh (or they
need to be prefixed).

(same in userspace_pm.sh)

>  capout=""
>  ksft_skip=4
>  iptables="iptables"
> @@ -151,7 +150,6 @@ init() {
>  	cout=$(mktemp)
>  	evts_ns1=$(mktemp)
>  	evts_ns2=$(mktemp)
> -	check_output_err=$(mktemp)
>  
>  	trap cleanup EXIT
>  
> @@ -165,7 +163,6 @@ cleanup()
>  	rm -f "$sin" "$sout" "$cinsent" "$cinfail"
>  	rm -f "$tmpfile"
>  	rm -rf $evts_ns1 $evts_ns2
> -	rm -f $check_output_err
>  	mptcp_lib_cleanup
>  	cleanup_partial
>  }

I think it is clearer to keep the assignment and cleanup here, than
hiding it in mptcp_lib.sh and doing that everywhere, even when not required.

(same in userspace_pm.sh)

> @@ -3345,26 +3342,10 @@ userspace_pm_get_addr()
>  
>  check_output()
>  {
> -	local cmd="$1"
> -	local expected="$2"

Best to keep that, it is useful to act as a doc. Otherwise, we have to
look at mptcp_lib.sh to find out what is $1 and $2.

(same in userspace_pm.sh)

>  	local msg="$3"
> -	local out=`$cmd 2>$check_output_err`
> -	local cmd_ret=$?
>  
>  	printf "%-42s" "$msg"
> -	if [ $cmd_ret -ne 0 ]; then
> -		mptcp_lib_print_err "[FAIL] command execution '$cmd' stderr "
> -		cat $check_output_err
> -		ret=${KSFT_FAIL}
> -		return $cmd_ret
> -	elif [ "$out" = "$expected" ]; then
> -		mptcp_lib_print_ok "[ OK ]"
> -		return 0
> -	else
> -		mptcp_lib_print_err "[FAIL] expected '$expected' got '$out'"
> -		ret=${KSFT_FAIL}
> -		return 1
> -	fi
> +	mptcp_lib_check_output "${1}" "${2}"

(see above: best to pass "${check_output_err}" to mptcp_lib_check_output)

(same in userspace_pm.sh)

>  }
>  
>  userspace_tests()
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index 01b0e43eaa80..fd2ed3b5137c 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -410,6 +410,38 @@ mptcp_lib_ns_exit() {
>  	done
>  }
>  
> +check_output_err=$(mktemp)
> +
> +mptcp_lib_check_output() {
> +	: "${check_output_err:?}"
> +	: "${ret:?}"
> +
> +	local cmd="$1"

In mptcp_lib.sh, please always use {} as in the rest of the file. (same
below).

> +	local expected="$2"

(see above: "check_output_err" will not be passed as argument)

> +	local out
> +	local cmd_ret
> +
> +	out=$($cmd 2>"$check_output_err")
> +	cmd_ret=$?
> +
> +	if [ $cmd_ret -ne 0 ]; then
> +		mptcp_lib_print_err "[FAIL] command execution '$cmd' stderr "
> +		cat "$check_output_err"
> +		ret=${KSFT_FAIL}

Mmh, I don't think it is a good idea to assign a global variable
declared in another file from here.

Can you not return ${ret} instead? So we would do something like:

  mptcp_lib_check_output (...) || ret=${?}

or:

  if mptcp_lib_check_output (...); then
      # OK (...)
  else
      ret=1
      (...)
  fi

(do you need the value of "${cmd_ret}" later? I think just having 0 or
${KSFT_FAIL} is enough, no?)

> +		return $cmd_ret
> +	elif [ "$out" = "$expected" ]; then
> +		mptcp_lib_print_ok "[ OK ]"
> +		return 0
> +	else
> +		mptcp_lib_print_err "[FAIL] expected '$expected' got '$out'"
> +		ret=${KSFT_FAIL}
> +		return 1
> +	fi
> +}
> +
>  mptcp_lib_cleanup() {
> +	: "${check_output_err:?}"
> +
>  	echo "cleanup"
> +	rm -f "$check_output_err"
>  }

As mentioned above, I don't think we should do that here. Maybe we don't
need patch 3/5 then?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next 5/5] selftests: mptcp: add mptcp_lib_evts_* helpers

[Matthieu Baerts]

Hi Geliang,

On 19/02/2024 10:29, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> To avoid duplicated code in different MPTCP selftests, we can add and
> use helpers defined in mptcp_lib.sh.
> 
> This patch unifies "pm_nl_ctl events" related code in userspace_pm.sh
> and mptcp_join.sh into four helpers: mptcp_lib_evts_init(), _start(),
> _kill() and _remove(). Define them in mptcp_lib.sh and use these new
> helpers in both scripts.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  .../testing/selftests/net/mptcp/mptcp_join.sh | 62 +++++++-----------
>  .../testing/selftests/net/mptcp/mptcp_lib.sh  | 63 +++++++++++++++++++
>  .../selftests/net/mptcp/userspace_pm.sh       | 31 ++-------
>  3 files changed, 92 insertions(+), 64 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index f75d22dbb28d..d4798495a17d 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -33,10 +33,6 @@ ip_mptcp=0
>  check_invert=0
>  validate_checksum=false
>  init=0
> -evts_ns1=""
> -evts_ns2=""
> -evts_ns1_pid=0
> -evts_ns2_pid=0
>  last_test_failed=0
>  last_test_skipped=0
>  last_test_ignored=1
> @@ -148,8 +144,7 @@ init() {
>  	cin=$(mktemp)
>  	cinsent=$(mktemp)
>  	cout=$(mktemp)
> -	evts_ns1=$(mktemp)
> -	evts_ns2=$(mktemp)
> +	mptcp_lib_evts_init
>  
>  	trap cleanup EXIT
>  
> @@ -162,7 +157,7 @@ cleanup()
>  	rm -f "$cin" "$cout" "$sinfail"
>  	rm -f "$sin" "$sout" "$cinsent" "$cinfail"
>  	rm -f "$tmpfile"
> -	rm -rf $evts_ns1 $evts_ns2
> +	mptcp_lib_evts_remove
>  	mptcp_lib_cleanup
>  	cleanup_partial
>  }

Same here, I think it is best to keep these global variables declared
here and pass their names to mptcp_lib_(...) functions.

By doing that, you can keep a different name in mptcp_join.sh and
userspace_pm.sh, and reduce the modifications here below.

(...)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.