Re: [PATCH mptcp-next v5 11/12] selftests: mptcp: add mptcp_lib_verify_listener_events

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 26/02/2024 10:43, 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 | 25 +++------------
>  .../testing/selftests/net/mptcp/mptcp_lib.sh  | 25 +++++++++++++++
>  .../selftests/net/mptcp/userspace_pm.sh       | 31 +++++--------------
>  3 files changed, 38 insertions(+), 43 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index c9b779510528..fa7d07c51973 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -2791,16 +2791,12 @@ backup_tests()
>  
>  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 = $MPTCP_LIB_LISTENER_CREATED ]; then
>  		name="LISTENER_CREATED"
> @@ -2817,23 +2813,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
> -		print_ok

Should you not keep 'print_ok' here?

> +	mptcp_lib_verify_listener_events "${@}" || rc="${?}"
> +	if [ "${rc}" -eq 0 ]
> +	then

Keep the same style as in the rest of the file: 'then' at the previous line.

>  		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()

(...)

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