1. Patchew
  2. MPTCP
  3. [PATCH mptcp-next 0/5] add helpers and vars in mptcp_lib.sh, part 4
  4. View patch

[PATCH mptcp-next 2/5] selftests: mptcp: add mptcp_lib_verify_listener_events

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

The helper verify_listener_events() is defined both in mptcp_join.sh and
userspace_pm.sh, export it into mptcp_lib.sh and rename it with mptcp_lib_
prefix. Use this new helper in both scripts.

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

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index cecf09d14f46..f28b10a61268 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -2792,16 +2792,12 @@ AF_INET6=10
 
 verify_listener_events()
 {
-	local evt=$1
 	local e_type=$2
 	local e_family=$3
 	local e_saddr=$4
 	local e_sport=$5
-	local type
-	local family
-	local saddr
-	local sport
 	local name
+	local rc=0
 
 	if [ $e_type = $LISTENER_CREATED ]; then
 		name="LISTENER_CREATED"
@@ -2818,23 +2814,12 @@ verify_listener_events()
 		return
 	fi
 
-	type=$(mptcp_lib_evts_get_info type "$evt" "$e_type")
-	family=$(mptcp_lib_evts_get_info family "$evt" "$e_type")
-	sport=$(mptcp_lib_evts_get_info sport "$evt" "$e_type")
-	if [ $family ] && [ $family = $AF_INET6 ]; then
-		saddr=$(mptcp_lib_evts_get_info saddr6 "$evt" "$e_type")
-	else
-		saddr=$(mptcp_lib_evts_get_info saddr4 "$evt" "$e_type")
-	fi
-
-	if [ $type ] && [ $type = $e_type ] &&
-	   [ $family ] && [ $family = $e_family ] &&
-	   [ $saddr ] && [ $saddr = $e_saddr ] &&
-	   [ $sport ] && [ $sport = $e_sport ]; then
+	mptcp_lib_verify_listener_events "${@}" || rc="${?}"
+	if [ "${rc}" -eq 0 ]; then
 		print_ok
 		return 0
 	fi
-	fail_test "$e_type:$type $e_family:$family $e_saddr:$saddr $e_sport:$sport"
+	fail_test "$e_type $e_family $e_saddr $e_sport"
 }
 
 add_addr_ports_tests()
diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 44491f18ed17..aefe4db47656 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -468,3 +468,29 @@ mptcp_lib_check_expected() {
 
 	return "${rc}"
 }
+
+# shellcheck disable=SC2034
+mptcp_lib_verify_listener_events() {
+	local evt=$1
+	local e_type=$2
+	local e_family=$3
+	local e_saddr=$4
+	local e_sport=$5
+	local type
+	local family
+	local saddr
+	local sport
+	local rc=0
+
+	type=$(mptcp_lib_evts_get_info type "$evt" "$e_type")
+	family=$(mptcp_lib_evts_get_info family "$evt" "$e_type")
+	if [ "$family" ] && [ "$family" = "$AF_INET6" ]; then
+		saddr=$(mptcp_lib_evts_get_info saddr6 "$evt" "$e_type")
+	else
+		saddr=$(mptcp_lib_evts_get_info saddr4 "$evt" "$e_type")
+	fi
+	sport=$(mptcp_lib_evts_get_info sport "$evt" "$e_type")
+
+	mptcp_lib_check_expected "type" "family" "saddr" "sport" || rc="${?}"
+	return "${rc}"
+}
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 6ab9efda8627..28669e49de80 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -836,32 +836,15 @@ test_prio()
 
 verify_listener_events()
 {
-	local evt=$1
-	local e_type=$2
-	local e_family=$3
-	local e_saddr=$4
-	local e_sport=$5
-	local type
-	local family
-	local saddr
-	local sport
-
-	if [ $e_type = $LISTENER_CREATED ]; then
-		print_test "CREATE_LISTENER $e_saddr:$e_sport"
-	elif [ $e_type = $LISTENER_CLOSED ]; then
-		print_test "CLOSE_LISTENER $e_saddr:$e_sport"
-	fi
+	local rc=0
 
-	type=$(mptcp_lib_evts_get_info type $evt $e_type)
-	family=$(mptcp_lib_evts_get_info family $evt $e_type)
-	sport=$(mptcp_lib_evts_get_info sport $evt $e_type)
-	if [ $family ] && [ $family = $AF_INET6 ]; then
-		saddr=$(mptcp_lib_evts_get_info saddr6 $evt $e_type)
+	mptcp_lib_verify_listener_events "${@}" || rc="${?}"
+	if [ "${rc}" -eq 0 ]; then
+		test_pass
 	else
-		saddr=$(mptcp_lib_evts_get_info saddr4 $evt $e_type)
+		ret=1
+		mptcp_lib_result_fail "${test_name}"
 	fi
-
-	check_expected "type" "family" "saddr" "sport"
 }
 
 test_listener()
@@ -877,6 +860,7 @@ test_listener()
 	# Capture events on the network namespace running the client
 	:>$client_evts
 
+	print_test "Listener event LISTENER_CREATED 10.0.2.2:$client4_port"
 	# Attempt to add a listener at 10.0.2.2:<subflow-port>
 	ip netns exec $ns2 ./pm_nl_ctl listen 10.0.2.2\
 		$client4_port &
@@ -895,6 +879,7 @@ test_listener()
 		rport $client4_port token $server4_token
 	sleep 0.5
 
+	print_test "Listener event LISTENER_CLOSED 10.0.2.2:$client4_port"
 	# Delete the listener from the client ns, if one was created
 	mptcp_lib_kill_wait $listener_pid
 
-- 
2.40.1
Re: [PATCH mptcp-next 2/5] selftests: mptcp: add mptcp_lib_verify_listener_events
Posted by Matthieu Baerts 1 year, 11 months ago 
Hi Geliang,

On 07/03/2024 13:48, 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.
> 
> The helper verify_listener_events() is defined both in mptcp_join.sh and
> userspace_pm.sh, export it into mptcp_lib.sh and rename it with mptcp_lib_
> prefix. Use this new helper in both scripts.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  .../testing/selftests/net/mptcp/mptcp_join.sh | 23 +++-----------
>  .../testing/selftests/net/mptcp/mptcp_lib.sh  | 26 ++++++++++++++++
>  .../selftests/net/mptcp/userspace_pm.sh       | 31 +++++--------------
>  3 files changed, 38 insertions(+), 42 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index cecf09d14f46..f28b10a61268 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -2792,16 +2792,12 @@ AF_INET6=10
>  
>  verify_listener_events()
>  {
> -	local evt=$1
>  	local e_type=$2
>  	local e_family=$3
>  	local e_saddr=$4
>  	local e_sport=$5
> -	local type
> -	local family
> -	local saddr
> -	local sport
>  	local name
> +	local rc=0
>  
>  	if [ $e_type = $LISTENER_CREATED ]; then
>  		name="LISTENER_CREATED"
> @@ -2818,23 +2814,12 @@ verify_listener_events()
>  		return
>  	fi
>  
> -	type=$(mptcp_lib_evts_get_info type "$evt" "$e_type")
> -	family=$(mptcp_lib_evts_get_info family "$evt" "$e_type")
> -	sport=$(mptcp_lib_evts_get_info sport "$evt" "$e_type")
> -	if [ $family ] && [ $family = $AF_INET6 ]; then
> -		saddr=$(mptcp_lib_evts_get_info saddr6 "$evt" "$e_type")
> -	else
> -		saddr=$(mptcp_lib_evts_get_info saddr4 "$evt" "$e_type")
> -	fi
> -
> -	if [ $type ] && [ $type = $e_type ] &&
> -	   [ $family ] && [ $family = $e_family ] &&
> -	   [ $saddr ] && [ $saddr = $e_saddr ] &&
> -	   [ $sport ] && [ $sport = $e_sport ]; then
> +	mptcp_lib_verify_listener_events "${@}" || rc="${?}"
> +	if [ "${rc}" -eq 0 ]; then

Same here, you don't need 'rc':

  if mptcp_lib_verify_listener_events "${@}"; then
     (...)

>  		print_ok
>  		return 0
>  	fi
> -	fail_test "$e_type:$type $e_family:$family $e_saddr:$saddr $e_sport:$sport"
> +	fail_test "$e_type $e_family $e_saddr $e_sport"

Mmh, this will call "mptcp_lib_pr_fail" again, with info that have
already been displayed, no?

What about calling "fail_test" without argument, and adapt this helper
not to call "print_fail" in this case, no?

  if [ ${#} -gt 0 ]; then
      print_fail "${@}"
  fi

>  }
>  
>  add_addr_ports_tests()
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> index 44491f18ed17..aefe4db47656 100644
> --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> @@ -468,3 +468,29 @@ mptcp_lib_check_expected() {
>  
>  	return "${rc}"
>  }
> +
> +# shellcheck disable=SC2034

Please *always* document why you need to disable a check

  # shellcheck disable=(...) # <why> (or above it is a long description)


> +mptcp_lib_verify_listener_events() {
> +	local evt=$1
> +	local e_type=$2
> +	local e_family=$3
> +	local e_saddr=$4
> +	local e_sport=$5

Please use {} around the variable name in mptcp_lib.sh: same style as
the rest of the code in this file.

> +	local type
> +	local family
> +	local saddr
> +	local sport
> +	local rc=0
> +
> +	type=$(mptcp_lib_evts_get_info type "$evt" "$e_type")
> +	family=$(mptcp_lib_evts_get_info family "$evt" "$e_type")
> +	if [ "$family" ] && [ "$family" = "$AF_INET6" ]; then
> +		saddr=$(mptcp_lib_evts_get_info saddr6 "$evt" "$e_type")
> +	else
> +		saddr=$(mptcp_lib_evts_get_info saddr4 "$evt" "$e_type")
> +	fi
> +	sport=$(mptcp_lib_evts_get_info sport "$evt" "$e_type")

Same here above for the whole block for the {}.

> +
> +	mptcp_lib_check_expected "type" "family" "saddr" "sport" || rc="${?}"
> +	return "${rc}"
> +}
> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> index 6ab9efda8627..28669e49de80 100755
> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> @@ -836,32 +836,15 @@ test_prio()
>  
>  verify_listener_events()
>  {
> -	local evt=$1
> -	local e_type=$2
> -	local e_family=$3
> -	local e_saddr=$4
> -	local e_sport=$5
> -	local type
> -	local family
> -	local saddr
> -	local sport
> -
> -	if [ $e_type = $LISTENER_CREATED ]; then
> -		print_test "CREATE_LISTENER $e_saddr:$e_sport"
> -	elif [ $e_type = $LISTENER_CLOSED ]; then
> -		print_test "CLOSE_LISTENER $e_saddr:$e_sport"
> -	fi
> +	local rc=0
>  
> -	type=$(mptcp_lib_evts_get_info type $evt $e_type)
> -	family=$(mptcp_lib_evts_get_info family $evt $e_type)
> -	sport=$(mptcp_lib_evts_get_info sport $evt $e_type)
> -	if [ $family ] && [ $family = $AF_INET6 ]; then
> -		saddr=$(mptcp_lib_evts_get_info saddr6 $evt $e_type)
> +	mptcp_lib_verify_listener_events "${@}" || rc="${?}"
> +	if [ "${rc}" -eq 0 ]; then

Same here for 'rc': not needed

> +		test_pass
>  	else
> -		saddr=$(mptcp_lib_evts_get_info saddr4 $evt $e_type)
> +		ret=1
> +		mptcp_lib_result_fail "${test_name}"

Same here for ret=1: don't do that, use test_fail without parameters

>  	fi
> -
> -	check_expected "type" "family" "saddr" "sport"
>  }
>  
>  test_listener()
> @@ -877,6 +860,7 @@ test_listener()
>  	# Capture events on the network namespace running the client
>  	:>$client_evts
>  
> +	print_test "Listener event LISTENER_CREATED 10.0.2.2:$client4_port"

Please don't change the name, it is used in the TAP output.

>  	# Attempt to add a listener at 10.0.2.2:<subflow-port>
>  	ip netns exec $ns2 ./pm_nl_ctl listen 10.0.2.2\
>  		$client4_port &
> @@ -895,6 +879,7 @@ test_listener()
>  		rport $client4_port token $server4_token
>  	sleep 0.5
>  
> +	print_test "Listener event LISTENER_CLOSED 10.0.2.2:$client4_port"

Same here, keep the same name.

>  	# Delete the listener from the client ns, if one was created
>  	mptcp_lib_kill_wait $listener_pid
>  

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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.