Re: [PATCH mptcp-next v3 1/8] selftests: mptcp: join: no extra msg if no counter

[Geliang Tang <geliang@kernel.org>]

Hi Matt,

Thanks for these patches.

On Tue, 2024-08-06 at 13:18 +0200, Matthieu Baerts (NGI0) wrote:
> The checksum and fail counters might not be available. Then no need
> to
> display an extra message with missing info.
> 
> While at it, fix the indentation around, which is wrong since the
> same
> commit.
> 
> Fixes: 47867f0a7e83 ("selftests: mptcp: join: skip check if MIB
> counter not supported")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 01c1e0871aca..a1f80dac59a7 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -1112,7 +1112,7 @@ chk_csum_nr()
>  
>  	print_check "sum"
>  	count=$(mptcp_lib_get_counter ${ns1} "MPTcpExtDataCsumErr")
> -	if [ "$count" != "$csum_ns1" ]; then
> +	if [ -n "$count" ] && [ "$count" != "$csum_ns1" ]; then

We have checked [ -z "$count" ] in mptcp_lib_get_counter() already, so
I think no need to double check it here. Can we just let
mptcp_lib_get_counter() return "0" in this [ -z "$count" ] case, then
we can drop all these [ -n "$count" ] or [ -z "$count" ] (like in
chk_cestab_nr()) checks after mptcp_lib_get_counter()? WDYT?

Regards,
-Geliang

>  		extra_msg+=" ns1=$count"
>  	fi
>  	if [ -z "$count" ]; then
> @@ -1125,7 +1125,7 @@ chk_csum_nr()
>  	fi
>  	print_check "csum"
>  	count=$(mptcp_lib_get_counter ${ns2} "MPTcpExtDataCsumErr")
> -	if [ "$count" != "$csum_ns2" ]; then
> +	if [ -n "$count" ] && [ "$count" != "$csum_ns2" ]; then
>  		extra_msg+=" ns2=$count"
>  	fi
>  	if [ -z "$count" ]; then
> @@ -1169,13 +1169,13 @@ chk_fail_nr()
>  
>  	print_check "ftx"
>  	count=$(mptcp_lib_get_counter ${ns_tx} "MPTcpExtMPFailTx")
> -	if [ "$count" != "$fail_tx" ]; then
> +	if [ -n "$count" ] && [ "$count" != "$fail_tx" ]; then
>  		extra_msg+=",tx=$count"
>  	fi
>  	if [ -z "$count" ]; then
>  		print_skip
>  	elif { [ "$count" != "$fail_tx" ] && [ $allow_tx_lost -eq 0
> ]; } ||
> -	   { [ "$count" -gt "$fail_tx" ] && [ $allow_tx_lost -eq 1
> ]; }; then
> +	     { [ "$count" -gt "$fail_tx" ] && [ $allow_tx_lost -eq 1
> ]; }; then
>  		fail_test "got $count MP_FAIL[s] TX expected
> $fail_tx"
>  	else
>  		print_ok
> @@ -1183,13 +1183,13 @@ chk_fail_nr()
>  
>  	print_check "failrx"
>  	count=$(mptcp_lib_get_counter ${ns_rx} "MPTcpExtMPFailRx")
> -	if [ "$count" != "$fail_rx" ]; then
> +	if [ -n "$count" ] && [ "$count" != "$fail_rx" ]; then
>  		extra_msg+=",rx=$count"
>  	fi
>  	if [ -z "$count" ]; then
>  		print_skip
>  	elif { [ "$count" != "$fail_rx" ] && [ $allow_rx_lost -eq 0
> ]; } ||
> -	   { [ "$count" -gt "$fail_rx" ] && [ $allow_rx_lost -eq 1
> ]; }; then
> +	     { [ "$count" -gt "$fail_rx" ] && [ $allow_rx_lost -eq 1
> ]; }; then
>  		fail_test "got $count MP_FAIL[s] RX expected
> $fail_rx"
>  	else
>  		print_ok
>