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

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

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

v3:
 - only check ss mptcp support in mptcp_join.sh.
 - use 'switch... case' in mptcp_lib_check_tools.

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 (2):
  selftests: mptcp: join: add ss mptcp support check
  selftests: mptcp: add mptcp_lib_check_tools helper

 tools/testing/selftests/net/mptcp/diag.sh     | 12 +------
 .../selftests/net/mptcp/mptcp_connect.sh      |  7 +----
 .../testing/selftests/net/mptcp/mptcp_join.sh | 23 ++------------
 .../testing/selftests/net/mptcp/mptcp_lib.sh  | 31 +++++++++++++++++++
 .../selftests/net/mptcp/mptcp_sockopt.sh      | 15 +--------
 .../testing/selftests/net/mptcp/pm_netlink.sh |  7 +----
 .../selftests/net/mptcp/simult_flows.sh       |  7 +----
 .../selftests/net/mptcp/userspace_pm.sh       |  6 +---
 8 files changed, 40 insertions(+), 68 deletions(-)

-- 
2.40.1

[PATCH mptcp-next v3 1/2] selftests: mptcp: join: add ss mptcp support check

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Commands 'ss -M' are used in script mptcp_join.sh to display only MPTCP
sockets. So it must be checked if ss tool supports MPTCP in this script.

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

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 5935b1fcfadb..d797b27b7626 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -162,6 +162,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
-- 
2.40.1

[PATCH mptcp-next v3 2/2] 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     | 12 +------
 .../selftests/net/mptcp/mptcp_connect.sh      |  7 +----
 .../testing/selftests/net/mptcp/mptcp_join.sh | 28 ++---------------
 .../testing/selftests/net/mptcp/mptcp_lib.sh  | 31 +++++++++++++++++++
 .../selftests/net/mptcp/mptcp_sockopt.sh      | 15 +--------
 .../testing/selftests/net/mptcp/pm_netlink.sh |  7 +----
 .../selftests/net/mptcp/simult_flows.sh       |  7 +----
 .../selftests/net/mptcp/userspace_pm.sh       |  6 +---
 8 files changed, 40 insertions(+), 73 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
index 8573326d326a..60a7009ce1b5 100755
--- a/tools/testing/selftests/net/mptcp/diag.sh
+++ b/tools/testing/selftests/net/mptcp/diag.sh
@@ -34,17 +34,7 @@ cleanup()
 }
 
 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 7898d62fce0b..ea52110c3fbc 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
@@ -147,12 +147,7 @@ cleanup()
 
 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
+mptcp_lib_check_tools ip
 
 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 d797b27b7626..d9df6eb6a52a 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -152,34 +152,12 @@ 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_mptcp
+	mptcp_lib_check_kallsyms
+	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..b92c5278730b 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -319,3 +319,34 @@ mptcp_lib_wait_local_port_listen() {
 		sleep 0.1
 	done
 }
+
+mptcp_lib_check_tools() {
+	local tool
+
+	for tool in "${@}"; do
+		case "${tool}" in
+		"ip")
+			if ! ip -Version &> /dev/null; then
+				mptcp_lib_print_warn "SKIP: Could not run test without ip tool"
+				exit ${KSFT_SKIP}
+			fi
+			;;
+		"ss")
+			if ! ss -h | grep -q MPTCP; then
+				mptcp_lib_print_warn "SKIP: ss tool does not support MPTCP"
+				exit ${KSFT_SKIP}
+			fi
+			;;
+		"iptables"* | "ip6tables"*)
+			if ! "${tool}" -V &> /dev/null; then
+				mptcp_lib_print_warn "SKIP: Could not run all tests without ${tool} tool"
+				exit ${KSFT_SKIP}
+			fi
+			;;
+		*)
+			mptcp_lib_print_warn "Unsupported tool: ${tool}"
+			exit ${KSFT_SKIP}
+			;;
+		esac
+	done
+}
diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
index dac8e1fc7143..edb71f44be45 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
@@ -89,20 +89,7 @@ cleanup()
 
 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 ! "${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 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..cb6ea67e688b 100755
--- a/tools/testing/selftests/net/mptcp/pm_netlink.sh
+++ b/tools/testing/selftests/net/mptcp/pm_netlink.sh
@@ -36,12 +36,7 @@ cleanup()
 }
 
 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..7d8388ecc966 100755
--- a/tools/testing/selftests/net/mptcp/simult_flows.sh
+++ b/tools/testing/selftests/net/mptcp/simult_flows.sh
@@ -43,12 +43,7 @@ cleanup()
 }
 
 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..629fc5d0ecc5 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -17,11 +17,7 @@ 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
+mptcp_lib_check_tools ip
 
 ANNOUNCED=6        # MPTCP_EVENT_ANNOUNCED
 REMOVED=7          # MPTCP_EVENT_REMOVED
-- 
2.40.1

Re: [PATCH mptcp-next v3 1/2] selftests: mptcp: join: add ss mptcp support check

[Matthieu Baerts]

Hi Geliang,

On 19/02/2024 09:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Commands 'ss -M' are used in script mptcp_join.sh to display only MPTCP
> sockets. So it must be checked if ss tool supports MPTCP in this script.

Thank you for the update, I will queue it for -net, with:

Fixes: e274f7154008 ("selftests: mptcp: add subflow limits test-cases")
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

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

Re: [PATCH mptcp-next v3 2/2] selftests: mptcp: add mptcp_lib_check_tools helper

[Matthieu Baerts]

Hi Geliang,

Thank you for the new version.

On 19/02/2024 09:22, 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

'ip6tables' can be passed too since v3.

> whether to check ip tool, ss tool and iptables tools.
> 
> This helper can be used in every scripts.

(...)

> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index d797b27b7626..d9df6eb6a52a 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -152,34 +152,12 @@ 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_mptcp
> +	mptcp_lib_check_kallsyms
> +	mptcp_lib_check_tools ip ss iptables

You should use "${iptables}" -- just in case we switch to iptables-nft
or something else later -- and add "${ip6tables}" now that the check is
split.

>  	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..b92c5278730b 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -319,3 +319,34 @@ mptcp_lib_wait_local_port_listen() {
>  		sleep 0.1
>  	done
>  }
> +
> +mptcp_lib_check_tools() {
> +	local tool
> +
> +	for tool in "${@}"; do
> +		case "${tool}" in
> +		"ip")
> +			if ! ip -Version &> /dev/null; then
> +				mptcp_lib_print_warn "SKIP: Could not run test without ip tool"
> +				exit ${KSFT_SKIP}
> +			fi
> +			;;
> +		"ss")
> +			if ! ss -h | grep -q MPTCP; then
> +				mptcp_lib_print_warn "SKIP: ss tool does not support MPTCP"
> +				exit ${KSFT_SKIP}
> +			fi
> +			;;
> +		"iptables"* | "ip6tables"*)
> +			if ! "${tool}" -V &> /dev/null; then
> +				mptcp_lib_print_warn "SKIP: Could not run all tests without ${tool} tool"
> +				exit ${KSFT_SKIP}
> +			fi
> +			;;
> +		*)
> +			mptcp_lib_print_warn "Unsupported tool: ${tool}"
> +			exit ${KSFT_SKIP}

If we are here, it means we have an internal error: we are checking a
tool we don't support. I think you should then use 'mptcp_lib_print_err'
and exit an error "${KSFT_FAIL}".

If you prefer, you can also print a more explicit message, e.g.

  Internal error: unsupported tool: ${tool}

Up to you, this error should not happen.

> +			;;
> +		esac
> +	done
> +}
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> index dac8e1fc7143..edb71f44be45 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> @@ -89,20 +89,7 @@ cleanup()
>  
>  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 ! "${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 iptables

Same here: use "${iptables}" instead, and add "${ip6tables}".

>  
>  check_mark()
>  {

(...)

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:
  - Success! ✅:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7956717856

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


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/7957112646

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


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: [PATCH mptcp-next v3 1/2] selftests: mptcp: join: add ss mptcp support check

[Matthieu Baerts]

Hi Geliang,

On 19/02/2024 09:52, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 19/02/2024 09:22, Geliang Tang wrote:
>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>
>> Commands 'ss -M' are used in script mptcp_join.sh to display only MPTCP
>> sockets. So it must be checked if ss tool supports MPTCP in this script.
> 
> Thank you for the update, I will queue it for -net, with:
> 
> Fixes: e274f7154008 ("selftests: mptcp: add subflow limits test-cases")
> Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

Just did:

New patches for t/upstream-net and t/upstream:
- ee636468e74e: selftests: mptcp: join: add ss mptcp support check
- Results: 22c802d10725..43a48b2f8697 (export-net)
- 05d22fbbbb38: conflict in t/selftests-mptcp-stop-forcing-iptables-legacy
- Results: d5aedc5b25ce..1642cc21aa85 (export)

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