Re: [PATCH net-next v3 03/15] selftests: mptcp: add chk_subflows_total helper

[Geliang Tang <geliang.tang@suse.com>]

Hi Mat,

On Wed, Nov 15, 2023 at 04:31:31PM -0800, Mat Martineau wrote:
> From: Geliang Tang <geliang.tang@suse.com>
> 
> This patch adds a new helper chk_subflows_total(), in it use the newly
> added counter mptcpi_subflows_total to get the "correct" amount of
> subflows, including the initial one.
> 
> To be compatible with old 'ss' or kernel versions not supporting this
> counter, get the total subflows by listing TCP connections that are
> MPTCP subflows:
> 
>     ss -ti state state established state syn-sent state syn-recv |
>         grep -c tcp-ulp-mptcp.
> 
> Reviewed-by: Matthieu Baerts <matttbe@kernel.org>
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> Signed-off-by: Mat Martineau <martineau@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 41 ++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index f064803071f1..2130e3b7790f 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -1867,7 +1867,7 @@ chk_mptcp_info()
>  	local cnt2
>  	local dump_stats
>  
> -	print_check "mptcp_info ${info1:0:8}=$exp1:$exp2"
> +	print_check "mptcp_info ${info1:0:15}=$exp1:$exp2"
>  
>  	cnt1=$(ss -N $ns1 -inmHM | mptcp_lib_get_info_value "$info1" "$info1")
>  	cnt2=$(ss -N $ns2 -inmHM | mptcp_lib_get_info_value "$info2" "$info2")
> @@ -1888,6 +1888,41 @@ chk_mptcp_info()
>  	fi
>  }
>  
> +# $1: subflows in ns1 ; $2: subflows in ns2
> +# number of all subflows, including the initial subflow.
> +chk_subflows_total()
> +{
> +	local cnt1
> +	local cnt2
> +	local info="subflows_total"

Sorry, this line should be added here:

	local dump_stats

Otherwise, no place to set 0 to dump_stats, if a test fails. Then
unexpected dump infos will be showed in all the subsequent outputs
of chk_subflows_total().

I'll send a squash-to patch to fix this.

Thanks,
-Geliang


> +
> +	# if subflows_total counter is supported, use it:
> +	if [ -n "$(ss -N $ns1 -inmHM | mptcp_lib_get_info_value $info $info)" ]; then
> +		chk_mptcp_info $info $1 $info $2
> +		return
> +	fi
> +
> +	print_check "$info $1:$2"
> +
> +	# if not, count the TCP connections that are in fact MPTCP subflows
> +	cnt1=$(ss -N $ns1 -ti state established state syn-sent state syn-recv |
> +	       grep -c tcp-ulp-mptcp)
> +	cnt2=$(ss -N $ns2 -ti state established state syn-sent state syn-recv |
> +	       grep -c tcp-ulp-mptcp)
> +
> +	if [ "$1" != "$cnt1" ] || [ "$2" != "$cnt2" ]; then
> +		fail_test "got subflows $cnt1:$cnt2 expected $1:$2"
> +		dump_stats=1
> +	else
> +		print_ok
> +	fi
> +
> +	if [ "$dump_stats" = 1 ]; then
> +		ss -N $ns1 -ti
> +		ss -N $ns2 -ti
> +	fi
> +}
> +
>  chk_link_usage()
>  {
>  	local ns=$1
> @@ -3431,10 +3466,12 @@ userspace_tests()
>  		chk_join_nr 1 1 1
>  		chk_add_nr 1 1
>  		chk_mptcp_info subflows 1 subflows 1
> +		chk_subflows_total 2 2
>  		chk_mptcp_info add_addr_signal 1 add_addr_accepted 1
>  		userspace_pm_rm_sf_addr_ns1 10.0.2.1 10
>  		chk_rm_nr 1 1 invert
>  		chk_mptcp_info subflows 0 subflows 0
> +		chk_subflows_total 1 1
>  		kill_events_pids
>  		wait $tests_pid
>  	fi
> @@ -3451,9 +3488,11 @@ userspace_tests()
>  		userspace_pm_add_sf 10.0.3.2 20
>  		chk_join_nr 1 1 1
>  		chk_mptcp_info subflows 1 subflows 1
> +		chk_subflows_total 2 2
>  		userspace_pm_rm_sf_addr_ns2 10.0.3.2 20
>  		chk_rm_nr 1 1
>  		chk_mptcp_info subflows 0 subflows 0
> +		chk_subflows_total 1 1
>  		kill_events_pids
>  		wait $tests_pid
>  	fi
> 
> -- 
> 2.41.0
>