Re: [PATCH mptcp-net 3/5] selftests: mptcp: add ss mptcp support checks

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 13/02/2024 05:14, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Commands 'ss -M' are used in scripts mptcp_connect.sh, mptcp_join.sh,
> and mptcp_sockopt.sh to display only MPTCP sockets. So it must be checked
> if ss tool supports MPTCP in these script.
> 
> Fixes: 767389c8dd55 ("selftests: mptcp: dump more info on errors")
> Fixes: c2a55e8fd80f ("selftests: mptcp: dump more info on mpjoin errors")
> Fixes: dc65fe82fb07 ("selftests: mptcp: add packet mark test case")

If we only use it to get more debug in these selftests, maybe this check
is not needed? I think it would be better to let other CI running our
tests, and accept we can have less debug info. (Also easier for the
backports if there is only one thing to fix at a time)


I would then add...

 Fixes: e274f7154008 ("selftests: mptcp: add subflow limits test-cases")

... because here it is different, it is used as a feature.

So I think we only need it for mptcp_join.sh, right?

> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_connect.sh | 6 ++++++
>  tools/testing/selftests/net/mptcp/mptcp_join.sh    | 6 ++++++
>  tools/testing/selftests/net/mptcp/mptcp_sockopt.sh | 6 ++++++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.sh b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> index 7898d62fce0b..c49351d85cae 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_connect.sh
> @@ -154,6 +154,12 @@ if [ $? -ne 0 ];then
>  	exit $ksft_skip
>  fi
>  
> +ss -h | grep -q MPTCP
> +if [ $? -ne 0 ];then
> +	echo "SKIP: ss tool does not support MPTCP"
> +	exit $ksft_skip
> +fi
> +
>  sin=$(mktemp)
>  sout=$(mktemp)
>  cin=$(mktemp)
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index c07386e21e0a..5ca1512c3eae 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -161,6 +161,12 @@ check_tools()
>  		exit $ksft_skip
>  	fi
>  
> +	ss -h | grep -q MPTCP
> +	if [ $? -ne 0 ];then

(Even if you might have taken that from diag.sh) This will not pass
shellcheck's validation, I think, right?

Better like this?

  if ! ss -h | grep -q MPTCP; then

(Also, please add a space after ";", like in most of the scripts we have)

Same in the other files (if you need to modify them).

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