Re: [PATCH mptcp-next v3 3/8] selftests: mptcp: join: reduce join_nr params

[Geliang Tang <geliang@kernel.org>]

On Tue, 2024-08-06 at 13:18 +0200, Matthieu Baerts (NGI0) wrote:
> chk_join_nr() currently takes 9 positional parameters, 6 of them are
> optional. It makes it hard to read:
> 
>   chk_join_nr 1 1 1 1 0 1 1 0 4
> 
> Naming these vars helps to make it easier to read:
> 
>   join_csum_ns1=1 join_csum_ns2=0 \
>     join_fail_nr=1 join_rst_nr=1 join_infi_nr=0 \
>     join_corrupted_pkts=4 \
>     chk_join_nr 1 1 1
> 
> It will then be easier to add new optional parameters.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 31
> ++++++++++++++++++-------
>  1 file changed, 22 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index a1f80dac59a7..0401ba1aaf1b 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -61,6 +61,12 @@ unset sflags
>  unset fastclose
>  unset fullmesh
>  unset speed
> +unset join_csum_ns1
> +unset join_csum_ns2
> +unset join_fail_nr
> +unset join_rst_nr
> +unset join_infi_nr
> +unset join_corrupted_pkts
>  
>  # generated using "nfbpf_compile '(ip && (ip[54] & 0xf0) == 0x30) ||
>  #				  (ip6 && (ip6[74] & 0xf0) ==
> 0x30)'"
> @@ -1314,12 +1320,12 @@ chk_join_nr()
>  	local syn_nr=$1
>  	local syn_ack_nr=$2
>  	local ack_nr=$3
> -	local csum_ns1=${4:-0}
> -	local csum_ns2=${5:-0}
> -	local fail_nr=${6:-0}
> -	local rst_nr=${7:-0}
> -	local infi_nr=${8:-0}
> -	local corrupted_pkts=${9:-0}
> +	local csum_ns1=${join_csum_ns1:-0}
> +	local csum_ns2=${join_csum_ns2:-0}
> +	local fail_nr=${join_fail_nr:-0}
> +	local rst_nr=${join_rst_nr:-0}
> +	local infi_nr=${join_infi_nr:-0}
> +	local corrupted_pkts=${join_corrupted_pkts:-0}
>  	local count
>  	local with_cookie
>  
> @@ -3138,7 +3144,8 @@ fastclose_tests()
>  		MPTCP_LIB_SUBTEST_FLAKY=1
>  		test_linkfail=1024 fastclose=server \
>  			run_tests $ns1 $ns2 10.0.1.1
> -		chk_join_nr 0 0 0 0 0 0 1
> +		join_rst_nr=1 \
> +			chk_join_nr 0 0 0
>  		chk_fclose_nr 1 1 invert
>  		chk_rst_nr 1 1
>  	fi
> @@ -3157,7 +3164,10 @@ fail_tests()
>  		MPTCP_LIB_SUBTEST_FLAKY=1
>  		test_linkfail=128 \
>  			run_tests $ns1 $ns2 10.0.1.1
> -		chk_join_nr 0 0 0 +1 +0 1 0 1 "$(pedit_action_pkts)"
> +		join_csum_ns1=+1 join_csum_ns2=+0 \
> +			join_fail_nr=1 join_rst_nr=0 join_infi_nr=1
> \

Can we drop this "join_rst_nr=0"?

> +			join_corrupted_pkts="$(pedit_action_pkts)" \
> +			chk_join_nr 0 0 0
>  		chk_fail_nr 1 -1 invert
>  	fi
>  
> @@ -3170,7 +3180,10 @@ fail_tests()
>  		pm_nl_add_endpoint $ns2 10.0.2.2 dev ns2eth2 flags
> subflow
>  		test_linkfail=1024 \
>  			run_tests $ns1 $ns2 10.0.1.1
> -		chk_join_nr 1 1 1 1 0 1 1 0 "$(pedit_action_pkts)"
> +		join_csum_ns1=1 join_csum_ns2=0 \
> +			join_fail_nr=1 join_rst_nr=1 join_infi_nr=0
> \

And this "join_infi_nr=0".

> +			join_corrupted_pkts="$(pedit_action_pkts)" \
> +			chk_join_nr 1 1 1
>  	fi
>  }
>  
>