[PATCH mptcp-next v2 0/7] add helpers and vars in mptcp_lib.sh, part 1

[PATCH mptcp-next v2 0/7] add helpers and vars in mptcp_lib.sh, part 1

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

v2:
 - address Matt's comments in v1.
 - all for -next, not -net.
 - add two more patches.

Part 1 of "add helpers and vars in mptcp_lib.sh", some fixes.

Geliang Tang (7):
  selftests: mptcp: netlink: drop duplicate var ret
  selftests: mptcp: simult flows: define missing vars
  selftests: mptcp: join: change capture/checksum as bool
  selftests: mptcp: diag: change timeout_poll to 30
  selftests: mptcp: drop iptables/ip6tables vars
  selftests: mptcp: add ss mptcp support checks
  selftests: mptcp: add mptcp_lib_check_tools helper

 tools/testing/selftests/net/mptcp/diag.sh     | 15 +----
 .../selftests/net/mptcp/mptcp_connect.sh      |  9 +--
 .../testing/selftests/net/mptcp/mptcp_join.sh | 55 ++++++-------------
 .../testing/selftests/net/mptcp/mptcp_lib.sh  | 29 ++++++++++
 .../selftests/net/mptcp/mptcp_sockopt.sh      | 25 ++-------
 .../testing/selftests/net/mptcp/pm_netlink.sh |  9 +--
 .../selftests/net/mptcp/simult_flows.sh       | 14 ++---
 .../selftests/net/mptcp/userspace_pm.sh       |  8 +--
 8 files changed, 62 insertions(+), 102 deletions(-)

-- 
2.40.1

[PATCH mptcp-next v2 1/7] selftests: mptcp: netlink: drop duplicate var ret

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

The variable 'ret' are defined twice in pm_netlink.sh. This patch drops
this duplicate one.

Fixes: eedbc685321b ("selftests: add PM netlink functional tests")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/pm_netlink.sh | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
index 71899a3ffa7a..ebfefae71e13 100755
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -28,7 +28,6 @@ sec=$(date +%s)
 rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
 ns1="ns1-$rndh"
 err=$(mktemp)
-ret=0
 
 cleanup()
 {
-- 
2.40.1

[PATCH mptcp-next v2 2/7] selftests: mptcp: simult flows: define missing vars

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

fhe variables 'large', 'small', 'sout', 'cout', 'capout' and 'size' are
used in multiple functions, so they should be defined as global variables.
This patch redefines them at the beginning of simult_flows.sh.

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

diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index 2f5d3c73db5f..e6e5b933a1b9 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -16,6 +16,12 @@ test_cnt=1
 ret=0
 bail=0
 slack=50
+large=""
+small=""
+sout=""
+cout=""
+capout=""
+size=0
 
 usage() {
 	echo "Usage: $0 [ -b ] [ -c ] [ -d ]"
-- 
2.40.1

[PATCH mptcp-next v2 3/7] selftests: mptcp: join: change capture/checksum as bool

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

To maintain consistency with other scripts, this patch changes vars
'capture' and 'checksum' as bool vars in mptcp_join.

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

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index b807e2a42c61..88074f64f0a7 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -29,11 +29,11 @@ iptables="iptables"
 ip6tables="ip6tables"
 timeout_poll=30
 timeout_test=$((timeout_poll * 2 + 1))
-capture=0
-checksum=0
+capture=false
+checksum=false
 ip_mptcp=0
 check_invert=0
-validate_checksum=0
+validate_checksum=false
 init=0
 evts_ns1=""
 evts_ns2=""
@@ -100,7 +100,7 @@ init_partial()
 		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 -eq 1 ]; then
+		if $checksum; then
 			ip netns exec $netns sysctl -q net.mptcp.checksum_enabled=1
 		fi
 	done
@@ -376,7 +376,7 @@ reset_with_checksum()
 	ip netns exec $ns1 sysctl -q net.mptcp.checksum_enabled=$ns1_enable
 	ip netns exec $ns2 sysctl -q net.mptcp.checksum_enabled=$ns2_enable
 
-	validate_checksum=1
+	validate_checksum=true
 }
 
 reset_with_allow_join_id0()
@@ -409,7 +409,7 @@ reset_with_allow_join_id0()
 setup_fail_rules()
 {
 	check_invert=1
-	validate_checksum=1
+	validate_checksum=true
 	local i="$1"
 	local ip="${2:-4}"
 	local tables
@@ -1013,7 +1013,7 @@ do_transfer()
 	:> "$sout"
 	:> "$capout"
 
-	if [ $capture -eq 1 ]; then
+	if $capture; then
 		local capuser
 		if [ -z $SUDO_USER ] ; then
 			capuser=""
@@ -1115,7 +1115,7 @@ do_transfer()
 	wait $spid
 	local rets=$?
 
-	if [ $capture -eq 1 ]; then
+	if $capture; then
 	    sleep 1
 	    kill $cappid
 	fi
@@ -1503,7 +1503,7 @@ chk_join_nr()
 	else
 		print_ok
 	fi
-	if [ $validate_checksum -eq 1 ]; then
+	if $validate_checksum; then
 		chk_csum_nr $csum_ns1 $csum_ns2
 		chk_fail_nr $fail_nr $fail_nr
 		chk_rst_nr $rst_nr $rst_nr
@@ -3664,10 +3664,10 @@ while getopts "${all_tests_args}cCih" opt; do
 			tests+=("${all_tests[${opt}]}")
 			;;
 		c)
-			capture=1
+			capture=true
 			;;
 		C)
-			checksum=1
+			checksum=true
 			;;
 		i)
 			ip_mptcp=1
-- 
2.40.1

[PATCH mptcp-next v2 4/7] selftests: mptcp: diag: change timeout_poll to 30

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Even if it is set to 100ms from the beginning with commit
df62f2ec3df6 ("selftests/mptcp: add diag interface tests"), there is
no reason not to have it to 30ms like all the other tests. diag.sh is
not supposed to be slower than the other ones. To maintain consistency
with other scripts, this patch changes it to 30.

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

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index 0a58ebb8b04c..8573326d326a 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -8,7 +8,7 @@ rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
 ns="ns1-$rndh"
 ksft_skip=4
 test_cnt=1
-timeout_poll=100
+timeout_poll=30
 timeout_test=$((timeout_poll * 2 + 1))
 ret=0
 
-- 
2.40.1

[PATCH mptcp-next v2 5/7] selftests: mptcp: drop iptables/ip6tables vars

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Since iptables-legacy/ip6tables-legacy are dropped in the commit
5b31f0f6f954 ("selftests: mptcp: stop forcing iptables-legacy"),
there's no need to use ${iptables}/${ip6tables} variables anymore.
This patch drops them, and use iptables/ip6tables directly.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../testing/selftests/net/mptcp/mptcp_join.sh | 20 +++++++++----------
 .../selftests/net/mptcp/mptcp_sockopt.sh      | 16 +++++++--------
 2 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 88074f64f0a7..9b35a16a7c8b 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -25,8 +25,6 @@ capout=""
 ns1=""
 ns2=""
 ksft_skip=4
-iptables="iptables"
-ip6tables="ip6tables"
 timeout_poll=30
 timeout_test=$((timeout_poll * 2 + 1))
 capture=false
@@ -161,11 +159,11 @@ check_tools()
 		exit $ksft_skip
 	fi
 
-	if ! "${iptables}" -V &> /dev/null; then
-		echo "SKIP: Could not run all tests without ${iptables} tool"
+	if ! iptables -V &> /dev/null; then
+		echo "SKIP: Could not run all tests without iptables tool"
 		exit $ksft_skip
-	elif ! "${ip6tables}" -V &> /dev/null; then
-		echo "SKIP: Could not run all tests without ${ip6tables} tool"
+	elif ! ip6tables -V &> /dev/null; then
+		echo "SKIP: Could not run all tests without ip6tables tool"
 		exit $ksft_skip
 	fi
 }
@@ -348,9 +346,9 @@ reset_with_add_addr_timeout()
 
 	reset "${1}" || return 1
 
-	tables="${iptables}"
+	tables="iptables"
 	if [ $ip -eq 6 ]; then
-		tables="${ip6tables}"
+		tables="ip6tables"
 	fi
 
 	ip netns exec $ns1 sysctl -q net.mptcp.add_addr_timeout=1
@@ -414,9 +412,9 @@ setup_fail_rules()
 	local ip="${2:-4}"
 	local tables
 
-	tables="${iptables}"
+	tables="iptables"
 	if [ $ip -eq 6 ]; then
-		tables="${ip6tables}"
+		tables="ip6tables"
 	fi
 
 	ip netns exec $ns2 $tables \
@@ -475,7 +473,7 @@ reset_with_tcp_filter()
 	local src="${2}"
 	local target="${3}"
 
-	if ! ip netns exec "${ns}" ${iptables} \
+	if ! ip netns exec "${ns}" iptables \
 			-A INPUT \
 			-s "${src}" \
 			-p tcp \
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index dac8e1fc7143..722b0872f00d 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -11,8 +11,6 @@ cout=""
 ksft_skip=4
 timeout_poll=30
 timeout_test=$((timeout_poll * 2 + 1))
-iptables="iptables"
-ip6tables="ip6tables"
 
 sec=$(date +%s)
 rndh=$(printf %x $sec)-$(mktemp -u XXXXXX)
@@ -26,7 +24,7 @@ add_mark_rules()
 	local m=$2
 
 	local t
-	for t in ${iptables} ${ip6tables}; do
+	for t in "iptables" "ip6tables"; do
 		# just to debug: check we have multiple subflows connection requests
 		ip netns exec $ns $t -A OUTPUT -p tcp --syn -m mark --mark $m -j ACCEPT
 
@@ -96,11 +94,11 @@ if [ $? -ne 0 ];then
 	exit $ksft_skip
 fi
 
-if ! "${iptables}" -V &> /dev/null; then
-	echo "SKIP: Could not run all tests without ${iptables} tool"
+if ! iptables -V &> /dev/null; then
+	echo "SKIP: Could not run all tests without iptables tool"
 	exit $ksft_skip
-elif ! "${ip6tables}" -V &> /dev/null; then
-	echo "SKIP: Could not run all tests without ${ip6tables} tool"
+elif ! ip6tables -V &> /dev/null; then
+	echo "SKIP: Could not run all tests without ip6tables tool"
 	exit $ksft_skip
 fi
 
@@ -109,10 +107,10 @@ check_mark()
 	local ns=$1
 	local af=$2
 
-	local tables=${iptables}
+	local tables="iptables"
 
 	if [ $af -eq 6 ];then
-		tables=${ip6tables}
+		tables="ip6tables"
 	fi
 
 	local counters values
-- 
2.40.1

Re: [PATCH mptcp-next v2 5/7] selftests: mptcp: drop iptables/ip6tables vars

[Matthieu Baerts]

Hi Geliang,

On 16/02/2024 10:50, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Since iptables-legacy/ip6tables-legacy are dropped in the commit
> 5b31f0f6f954 ("selftests: mptcp: stop forcing iptables-legacy"),
> there's no need to use ${iptables}/${ip6tables} variables anymore.

I agree, there is no need to use variables for the moment. But as I
mentioned in the commit message, I kept "the variables to easily change
them later or add options". They don't hurt, we might want to use
'iptables -w', force using 'iptables-nft' if available, or other options
for the debugging. I don't think we need this not so small patch.

In other words, the patch is OK, but maybe we don't need it?

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

[PATCH mptcp-next v2 6/7] selftests: mptcp: add ss mptcp support checks

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Commands 'ss -M' are used in scripts mptcp_connect.sh, mptcp_join.sh,
and mptcp_sockopt.sh to display only MPTCP sockets. So it must be checked
if ss tool supports MPTCP in these script.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_connect.sh | 5 +++++
 tools/testing/selftests/net/mptcp/mptcp_join.sh    | 5 +++++
 tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 5 +++++
 3 files changed, 15 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index 7898d62fce0b..eb811d257cab 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -154,6 +154,11 @@ if [ $? -ne 0 ];then
 	exit $ksft_skip
 fi
 
+if ! ss -h | grep -q MPTCP; then
+	echo "SKIP: ss tool does not support MPTCP"
+	exit $ksft_skip
+fi
+
 sin=$(mktemp)
 sout=$(mktemp)
 cin=$(mktemp)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 9b35a16a7c8b..34a8d5ab185c 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -159,6 +159,11 @@ check_tools()
 		exit $ksft_skip
 	fi
 
+	if ! ss -h | grep -q MPTCP; then
+		echo "SKIP: ss tool does not support MPTCP"
+		exit $ksft_skip
+	fi
+
 	if ! iptables -V &> /dev/null; then
 		echo "SKIP: Could not run all tests without iptables tool"
 		exit $ksft_skip
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 722b0872f00d..988042912c7a 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -94,6 +94,11 @@ if [ $? -ne 0 ];then
 	exit $ksft_skip
 fi
 
+if ! ss -h | grep -q MPTCP; then
+	echo "SKIP: ss tool does not support MPTCP"
+	exit $ksft_skip
+fi
+
 if ! iptables -V &> /dev/null; then
 	echo "SKIP: Could not run all tests without iptables tool"
 	exit $ksft_skip
-- 
2.40.1

Re: [PATCH mptcp-next v2 6/7] selftests: mptcp: add ss mptcp support checks

[Matthieu Baerts]

Hi Geliang,

On 16/02/2024 10:50, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Commands 'ss -M' are used in scripts mptcp_connect.sh, mptcp_join.sh,
> and mptcp_sockopt.sh to display only MPTCP sockets. So it must be checked
> if ss tool supports MPTCP in these script.

I think you missed my comment from v1: 'ss' is used in mptcp_connect.sh
and mptcp_sockopt.sh, but only in case of errors, to get more info. They
are not strictly needed to run the tests, no?

I think it would be better not to skip the two selftests *entirely* if
'ss' doesn't support MPTCP.

For 'mptcp_join.sh', it is different, because 'ss' with MPTCP support is
needed for some tests, since:

 Fixes: e274f7154008 ("selftests: mptcp: add subflow limits test-cases")

(Even there, it is mandatory just for one subtest... In fact, I wonder
if we should not use 'chk_subflows_total' instead, it doesn't require
'ss -M' support.)

So in other words, I think we should only modify 'mptcp_join.sh', no?

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

[PATCH mptcp-next v2 7/7] selftests: mptcp: add mptcp_lib_check_tools helper

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch exports check_tools() helper from mptcp_join.sh into
mptcp_lib.sh as a public one mptcp_lib_check_tools(). The arguments
"ip", "ss", and "iptables" are passed into this helper to indicate
whether to check ip tool, ss tool and iptables tools.

This helper can be used in every scripts.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/diag.sh     | 13 +--------
 .../selftests/net/mptcp/mptcp_connect.sh      | 14 +--------
 .../testing/selftests/net/mptcp/mptcp_join.sh | 26 +----------------
 .../testing/selftests/net/mptcp/mptcp_lib.sh  | 29 +++++++++++++++++++
 .../selftests/net/mptcp/mptcp_sockopt.sh      | 22 +-------------
 .../testing/selftests/net/mptcp/pm_netlink.sh |  8 +----
 .../selftests/net/mptcp/simult_flows.sh       |  8 +----
 .../selftests/net/mptcp/userspace_pm.sh       |  8 +----
 8 files changed, 36 insertions(+), 92 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index 8573326d326a..7c68160ad3b9 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -33,18 +33,7 @@ cleanup()
 	ip netns del $ns
 }
 
-mptcp_lib_check_mptcp
-
-ip -Version > /dev/null 2>&1
-if [ $? -ne 0 ];then
-	echo "SKIP: Could not run test without ip tool"
-	exit $ksft_skip
-fi
-ss -h | grep -q MPTCP
-if [ $? -ne 0 ];then
-	echo "SKIP: ss tool does not support MPTCP"
-	exit $ksft_skip
-fi
+mptcp_lib_check_tools "ip" "ss"
 
 get_msk_inuse()
 {
diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
index eb811d257cab..6c846db4243f 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -145,19 +145,7 @@ cleanup()
 	done
 }
 
-mptcp_lib_check_mptcp
-mptcp_lib_check_kallsyms
-
-ip -Version > /dev/null 2>&1
-if [ $? -ne 0 ];then
-	echo "SKIP: Could not run test without ip tool"
-	exit $ksft_skip
-fi
-
-if ! ss -h | grep -q MPTCP; then
-	echo "SKIP: ss tool does not support MPTCP"
-	exit $ksft_skip
-fi
+mptcp_lib_check_tools "ip" "ss"
 
 sin=$(mktemp)
 sout=$(mktemp)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 34a8d5ab185c..644740d7d988 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -149,34 +149,10 @@ cleanup_partial()
 	done
 }
 
-check_tools()
-{
-	mptcp_lib_check_mptcp
-	mptcp_lib_check_kallsyms
-
-	if ! ip -Version &> /dev/null; then
-		echo "SKIP: Could not run test without ip tool"
-		exit $ksft_skip
-	fi
-
-	if ! ss -h | grep -q MPTCP; then
-		echo "SKIP: ss tool does not support MPTCP"
-		exit $ksft_skip
-	fi
-
-	if ! iptables -V &> /dev/null; then
-		echo "SKIP: Could not run all tests without iptables tool"
-		exit $ksft_skip
-	elif ! ip6tables -V &> /dev/null; then
-		echo "SKIP: Could not run all tests without ip6tables tool"
-		exit $ksft_skip
-	fi
-}
-
 init() {
 	init=1
 
-	check_tools
+	mptcp_lib_check_tools "ip" "ss" "iptables"
 
 	sin=$(mktemp)
 	sout=$(mktemp)
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 108a1e12436c..cdb8948749eb 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -319,3 +319,32 @@ mptcp_lib_wait_local_port_listen() {
 		sleep 0.1
 	done
 }
+
+mptcp_lib_check_tools() {
+	mptcp_lib_check_mptcp
+	mptcp_lib_check_kallsyms
+
+	if [ "${1:-""}" == "ip" ]; then
+		if ! ip -Version &> /dev/null; then
+			mptcp_lib_print_warn "SKIP: Could not run test without ip tool"
+			exit ${KSFT_SKIP}
+		fi
+	fi
+
+	if [ "${2:-""}" == "ss" ]; then
+		if ! ss -h | grep -q MPTCP; then
+			mptcp_lib_print_warn "SKIP: ss tool does not support MPTCP"
+			exit ${KSFT_SKIP}
+		fi
+	fi
+
+	if [ "${3:-""}" == "iptables" ]; then
+		if ! iptables -V &> /dev/null; then
+			mptcp_lib_print_warn "SKIP: Could not run all tests without iptables tool"
+			exit ${KSFT_SKIP}
+		elif ! ip6tables -V &> /dev/null; then
+			mptcp_lib_print_warn "SKIP: Could not run all tests without ip6tables tool"
+			exit ${KSFT_SKIP}
+		fi
+	fi
+}
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index 988042912c7a..8b6dd6e919c8 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -85,27 +85,7 @@ cleanup()
 	rm -f "$sin" "$sout"
 }
 
-mptcp_lib_check_mptcp
-mptcp_lib_check_kallsyms
-
-ip -Version > /dev/null 2>&1
-if [ $? -ne 0 ];then
-	echo "SKIP: Could not run test without ip tool"
-	exit $ksft_skip
-fi
-
-if ! ss -h | grep -q MPTCP; then
-	echo "SKIP: ss tool does not support MPTCP"
-	exit $ksft_skip
-fi
-
-if ! iptables -V &> /dev/null; then
-	echo "SKIP: Could not run all tests without iptables tool"
-	exit $ksft_skip
-elif ! ip6tables -V &> /dev/null; then
-	echo "SKIP: Could not run all tests without ip6tables tool"
-	exit $ksft_skip
-fi
+mptcp_lib_check_tools "ip" "ss" "iptables"
 
 check_mark()
 {
diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
index ebfefae71e13..e7ab5eca3e75 100755
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -35,13 +35,7 @@ cleanup()
 	ip netns del $ns1
 }
 
-mptcp_lib_check_mptcp
-
-ip -Version > /dev/null 2>&1
-if [ $? -ne 0 ];then
-	echo "SKIP: Could not run test without ip tool"
-	exit $ksft_skip
-fi
+mptcp_lib_check_tools "ip"
 
 trap cleanup EXIT
 
diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
index e6e5b933a1b9..053f40bf25c1 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -42,13 +42,7 @@ cleanup()
 	done
 }
 
-mptcp_lib_check_mptcp
-
-ip -Version > /dev/null 2>&1
-if [ $? -ne 0 ];then
-	echo "SKIP: Could not run test without ip tool"
-	exit $ksft_skip
-fi
+mptcp_lib_check_tools "ip"
 
 #  "$ns1"              ns2                    ns3
 #     ns1eth1    ns2eth1   ns2eth3      ns3eth1
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 1b94a75604fe..9d42fb9bc809 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -10,19 +10,13 @@
 
 . "$(dirname "${0}")/mptcp_lib.sh"
 
-mptcp_lib_check_mptcp
-mptcp_lib_check_kallsyms
+mptcp_lib_check_tools "ip"
 
 if ! mptcp_lib_has_file '/proc/sys/net/mptcp/pm_type'; then
 	echo "userspace pm tests are not supported by the kernel: SKIP"
 	exit ${KSFT_SKIP}
 fi
 
-if ! ip -Version &> /dev/null; then
-	echo "SKIP: Cannot not run test without ip tool"
-	exit ${KSFT_SKIP}
-fi
-
 ANNOUNCED=6        # MPTCP_EVENT_ANNOUNCED
 REMOVED=7          # MPTCP_EVENT_REMOVED
 SUB_ESTABLISHED=10 # MPTCP_EVENT_SUB_ESTABLISHED
-- 
2.40.1

Re: selftests: mptcp: add mptcp_lib_check_tools helper: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

- {"code":404,"message":
  - "Can't find artifacts containing file conclusion.txt"}:
  - Task: https://cirrus-ci.com/task/5854280519778304
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5854280519778304/summary/summary.txt

- {"code":404,"message":
  - "Can't find artifacts containing file conclusion.txt"}:
  - Task: https://cirrus-ci.com/task/5291330566356992
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5291330566356992/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e6d1487a62d3


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-debug

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)

Re: selftests: mptcp: add mptcp_lib_check_tools helper: Tests Results

[Matthieu Baerts]

Hello,

On 16/02/2024 11:14, MPTCP CI wrote:
> Hi Geliang,
> 
> Thank you for your modifications, that's great!
> 
> Our CI (Cirrus) did some validations with a debug kernel and here is its report:
> 
> - {"code":404,"message":
>   - "Can't find artifacts containing file conclusion.txt"}:
>   - Task: https://cirrus-ci.com/task/5854280519778304
>   - Summary: https://api.cirrus-ci.com/v1/artifact/task/5854280519778304/summary/summary.txt
> 
> - {"code":404,"message":
>   - "Can't find artifacts containing file conclusion.txt"}:
>   - Task: https://cirrus-ci.com/task/5291330566356992
>   - Summary: https://api.cirrus-ci.com/v1/artifact/task/5291330566356992/summary/summary.txt
> 
> Initiator: Patchew Applier
> Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e6d1487a62d3

FYI, the recent sync with net, including the last changes from Linus
tree, introduces a KMemLeak detected by our CI:


https://api.cirrus-ci.com/v1/artifact/task/6498522040631296/summary/summary.txt

I just added to our tree, a fix from upstream:

  https://lore.kernel.org/r/20240215213944.508142048@goodmis.org

And I just cancelled running jobs + forced a rebase of this series: in
other words, this message is "normal", new results should come soon.

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

Re: selftests: mptcp: add mptcp_lib_check_tools helper: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7929007344

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e6d1487a62d3


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)

Re: selftests: mptcp: add mptcp_lib_check_tools helper: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7929308311

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/c454c04a88cd


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)

Re: selftests: mptcp: add mptcp_lib_check_tools helper: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5488696795856896
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5488696795856896/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6614596702699520
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6614596702699520/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/c454c04a88cd


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-debug

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)

Re: [PATCH mptcp-next v2 7/7] selftests: mptcp: add mptcp_lib_check_tools helper

[Matthieu Baerts]

Hi Geliang,

On 16/02/2024 10:50, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch exports check_tools() helper from mptcp_join.sh into
> mptcp_lib.sh as a public one mptcp_lib_check_tools(). The arguments
> "ip", "ss", and "iptables" are passed into this helper to indicate
> whether to check ip tool, ss tool and iptables tools.
> 
> This helper can be used in every scripts.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  tools/testing/selftests/net/mptcp/diag.sh     | 13 +--------
>  .../selftests/net/mptcp/mptcp_connect.sh      | 14 +--------
>  .../testing/selftests/net/mptcp/mptcp_join.sh | 26 +----------------
>  .../testing/selftests/net/mptcp/mptcp_lib.sh  | 29 +++++++++++++++++++
>  .../selftests/net/mptcp/mptcp_sockopt.sh      | 22 +-------------
>  .../testing/selftests/net/mptcp/pm_netlink.sh |  8 +----
>  .../selftests/net/mptcp/simult_flows.sh       |  8 +----
>  .../selftests/net/mptcp/userspace_pm.sh       |  8 +----
>  8 files changed, 36 insertions(+), 92 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
> index 8573326d326a..7c68160ad3b9 100755
> --- a/tools/testing/selftests/net/mptcp/diag.sh
> +++ b/tools/testing/selftests/net/mptcp/diag.sh
> @@ -33,18 +33,7 @@ cleanup()
>  	ip netns del $ns
>  }
>  
> -mptcp_lib_check_mptcp

(see below) Here, we don't need to check for mptcp_lib_check_kallsyms.

(...)

> mptcp_lib_check_tools "ip" "ss"

(you can remove the double quotes if you don't use variables)

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index 108a1e12436c..cdb8948749eb 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -319,3 +319,32 @@ mptcp_lib_wait_local_port_listen() {
>  		sleep 0.1
>  	done
>  }
> +
> +mptcp_lib_check_tools() {
> +	mptcp_lib_check_mptcp
> +	mptcp_lib_check_kallsyms

I would not move that here, better to have an explicit check in each
selftest I think. Also, see above/below: some selftests don't need kallsyms.

> +	if [ "${1:-""}" == "ip" ]; then

Probably better to use a switch/case, so the order is not important, no?


  local tool

  for tool in "${@}"; do
      case "${tool}" in
      "ip")
          if ! ip -Version &> /dev/null; then
              (...)
          fi
          ;;
      "ss")
          (...)
          ;;
      "iptables"* | "ip6tables"*)  ## including extensions
          if ! "${tool}" -V &> /dev/null; then
          (...)
      *)
          mptcp_lib_print_warn "Unsupported tool: ${tool}"
          exit ${KSFT_FAIL}
          ;;
      esac
  done


So we can use this helper with parameters in any order, e.g.

  mptcp_lib_check_tools ss iptables-nft


> +		if ! ip -Version &> /dev/null; then
> +			mptcp_lib_print_warn "SKIP: Could not run test without ip tool"
> +			exit ${KSFT_SKIP}
> +		fi
> +	fi
> +
> +	if [ "${2:-""}" == "ss" ]; then
> +		if ! ss -h | grep -q MPTCP; then
> +			mptcp_lib_print_warn "SKIP: ss tool does not support MPTCP"
> +			exit ${KSFT_SKIP}
> +		fi
> +	fi
> +
> +	if [ "${3:-""}" == "iptables" ]; then
> +		if ! iptables -V &> /dev/null; then
> +			mptcp_lib_print_warn "SKIP: Could not run all tests without iptables tool"
> +			exit ${KSFT_SKIP}
> +		elif ! ip6tables -V &> /dev/null; then
> +			mptcp_lib_print_warn "SKIP: Could not run all tests without ip6tables tool"
> +			exit ${KSFT_SKIP}
> +		fi

I would split "iptables" and "ip6tables" cases, please see above: just
not to repeat the same err message, and maybe a selftest will only need one.

> +	fi
> +}

(...)

> diff --git a/tools/testing/selftests/net/mptcp/pm_netlink.sh b/tools/testing/selftests/net/mptcp/pm_netlink.sh
> index ebfefae71e13..e7ab5eca3e75 100755
> --- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
> +++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
> @@ -35,13 +35,7 @@ cleanup()
>  	ip netns del $ns1
>  }
>  
> -mptcp_lib_check_mptcp

(see above) Here, we don't need to check for mptcp_lib_check_kallsyms.

> -ip -Version > /dev/null 2>&1
> -if [ $? -ne 0 ];then
> -	echo "SKIP: Could not run test without ip tool"
> -	exit $ksft_skip
> -fi
> +mptcp_lib_check_tools "ip"
>  
>  trap cleanup EXIT
>  
> diff --git a/tools/testing/selftests/net/mptcp/simult_flows.sh b/tools/testing/selftests/net/mptcp/simult_flows.sh
> index e6e5b933a1b9..053f40bf25c1 100755
> --- a/tools/testing/selftests/net/mptcp/simult_flows.sh
> +++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
> @@ -42,13 +42,7 @@ cleanup()
>  	done
>  }
>  
> -mptcp_lib_check_mptcp

(see above) Here, we don't need to check for mptcp_lib_check_kallsyms.

(...)

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

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

[Matthieu Baerts]

Hi Geliang,

On 16/02/2024 10:50, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> v2:
>  - address Matt's comments in v1.
>  - all for -next, not -net.
>  - add two more patches.

Thank you for the v2.

Patches 1-4 are OK for me. I can apply them already.

I don't think we need 5/7, and I have some comments for the two last ones.

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

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

[Matthieu Baerts]

Hi Geliang,

On 16/02/2024 14:43, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 16/02/2024 10:50, Geliang Tang wrote:
>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>
>> v2:
>>  - address Matt's comments in v1.
>>  - all for -next, not -net.
>>  - add two more patches.
> 
> Thank you for the v2.
> 
> Patches 1-4 are OK for me. I can apply them already.

Just did. I slightly modified some commit messages, I hope that's OK.

Now in our tree, feat. for next.

New patches for t/upstream:
- ce771d245e7a: selftests: mptcp: netlink: drop duplicate var ret
- 980763004b18: selftests: mptcp: simult flows: define missing vars
- 99e3296b940f: selftests: mptcp: join: change capture/checksum as bool
- 247b270f3921: selftests: mptcp: diag: change timeout_poll to 30
- Results: 65bda47e218b..b95a6fc53895 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20240216T135434

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