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

Geliang Tang posted 7 patches 1 year, 11 months ago
There is a newer version of this series
[PATCH mptcp-next v2 7/7] selftests: mptcp: add mptcp_lib_check_tools helper
Posted by Geliang Tang 1 year, 11 months ago
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: [PATCH mptcp-next v2 7/7] selftests: mptcp: add mptcp_lib_check_tools helper
Posted by Matthieu Baerts 1 year, 11 months ago
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.