Re: [mptcp-next v2 7/7] selftests: mptcp: add chk_sublfow in diag.sh

[Matthieu Baerts <matttbe@kernel.org>]

On 16/04/2025 10:59, Gang Yan wrote:
> This patch aims to add chk_dump_subflow in diag.sh. The subflow's
> info can be obtained through "ss -tin", then use the 'mptcp_diag'
> to verify the token in subflow_info.
> 
> Co-developed-by: Geliang Tang <geliang@kernel.org>
> Signed-off-by: Geliang Tang <geliang@kernel.org>
> Signed-off-by: Gang Yan <yangang@kylinos.cn>
> ---
>  tools/testing/selftests/net/mptcp/diag.sh | 55 +++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/mptcp/diag.sh b/tools/testing/selftests/net/mptcp/diag.sh
> index e7a75341f0f3..20de007e44cd 100755
> --- a/tools/testing/selftests/net/mptcp/diag.sh
> +++ b/tools/testing/selftests/net/mptcp/diag.sh
> @@ -225,6 +225,60 @@ chk_dump_one()
>  	fi
>  }
>  
> +get_endpoint_addr()
> +{
> +	echo $1 | cut -d ":" -f 1
> +}
> +
> +get_endpoint_port()
> +{
> +	echo $1 | cut -d ":" -f 2
> +}
> +
> +chk_dump_subflow()
> +{
> +	local inet_diag_token
> +	local subflow_line
> +	local remote_addr
> +	local remote_port
> +	local local_addr
> +	local local_port
> +	local ss_token
> +	local msg
> +
> +	subflow_line=$(ss -tnN $ns | \
> +		       grep -m1 -Eo '[0-9.]+:[0-9].+ +[0-9.]+:[0-9.]+')
> +
> +	if [ -n "$subflow_line" ]; then
> +		read -r local_endpoint remote_endpoint <<< $subflow_line
> +
> +		local_addr=$(get_endpoint_addr $local_endpoint)
> +		local_port=$(get_endpoint_port $local_endpoint)

(you could use 'read -rd: local_addr local_port' here ; same below)

> +
> +		remote_addr=$(get_endpoint_addr $remote_endpoint)
> +		remote_port=$(get_endpoint_port $remote_endpoint)
> +	fi
> +
> +	ss_token=$(ss -tniN $ns | \
> +		   grep -m1 -Eo 'token:[^ ]+')

It might be safer to call 'ss' only once, then extract the subflow and
the ports later.

> +
> +	inet_diag_token=$(ip netns exec $ns ./mptcp_diag -s \
> +			  "$local_addr $local_port $remote_addr $remote_port" | \

Why don't you call 'mptcp_diag -s "$subflow_line"', then adapt sscanf in
mptcp_diag.c to use: "%s:%d %s:%d"?

You will save a few lines of bash here above by doing that. Or is it
more difficult when handling IPv6 addresses? I don't think you handle
IPv6 addresses anyway, right?

> +			  grep -Eo 'token:[^ ]+')
> +
> +	msg="....chk dump_subflow"
> +
> +	mptcp_lib_print_title "$msg"
> +	if [[ $ss_token == $inet_diag_token ]]; then

Please use the same style as elsewhere in the file:

  if [ -n "$ss_token" ] && [ "$ss_token" = "$inet_diag_token" ]; then

> +		mptcp_lib_pr_ok
> +		mptcp_lib_result_pass "${msg}"
> +	else
> +		mptcp_lib_pr_fail "expected $ss_token but $inet_diag_token"

Same here for the style:

  "expected $ss_token found $inet_diag_token"

> +		mptcp_lib_result_fail "${msg}"
> +		ret=${KSFT_FAIL}
> +	fi
> +}
> +
>  msk_info_get_value()
>  {
>  	local port="${1}"
> @@ -316,6 +370,7 @@ chk_msk_fallback_nr 0 "....chk no fallback"
>  chk_msk_inuse 2
>  chk_msk_cestab 2
>  chk_dump_one
> +chk_dump_subflow
>  flush_pids
>  
>  chk_msk_inuse 0 "2->0"

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