mptcp: avoid dup events + error + misc.

[PATCH mptcp-next 08/11] selftests: mptcp: join chk_stale_nr: avoid dup stats

Posted by Matthieu Baerts (NGI0) 1 week, 6 days ago

nstat outputs are already printed when calling 'fail_test', no need to
do it again.

While at it, no need to use the dump_stats variable, print the extra
stats directly. And use 'ip -n $ns' instead of 'ip netns exec $ns',
shorter and clearer.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 63a84a4d43a6..80e03168b419 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -1648,7 +1648,6 @@ chk_stale_nr()
 	local stale_min=$2
 	local stale_max=$3
 	local stale_delta=$4
-	local dump_stats
 	local stale_nr
 	local recover_nr
 
@@ -1664,16 +1663,11 @@ chk_stale_nr()
 		fail_test "got $stale_nr stale[s] $recover_nr recover[s], " \
 		     " expected stale in range [$stale_min..$stale_max]," \
 		     " stale-recover delta $stale_delta"
-		dump_stats=1
+		echo $ns stats
+		ip -n $ns -s link show
 	else
 		print_ok
 	fi
-
-	if [ "${dump_stats}" = 1 ]; then
-		echo $ns stats
-		ip netns exec $ns ip -s link show
-		ip netns exec $ns nstat -as | grep MPTcp
-	fi
 }
 
 chk_add_nr()

-- 
2.51.0

Re: [PATCH mptcp-next 08/11] selftests: mptcp: join chk_stale_nr: avoid dup stats

Posted by Geliang Tang 1 week, 4 days ago

Hi Matt,

Thanks for this patch.

On Fri, 2025-12-26 at 07:40 +0100, Matthieu Baerts (NGI0) wrote:
> nstat outputs are already printed when calling 'fail_test', no need
> to
> do it again.
> 
> While at it, no need to use the dump_stats variable, print the extra
> stats directly. And use 'ip -n $ns' instead of 'ip netns exec $ns',
> shorter and clearer.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 63a84a4d43a6..80e03168b419 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -1648,7 +1648,6 @@ chk_stale_nr()
>  	local stale_min=$2
>  	local stale_max=$3
>  	local stale_delta=$4
> -	local dump_stats
>  	local stale_nr
>  	local recover_nr
>  
> @@ -1664,16 +1663,11 @@ chk_stale_nr()
>  		fail_test "got $stale_nr stale[s] $recover_nr
> recover[s], " \
>  		     " expected stale in range
> [$stale_min..$stale_max]," \
>  		     " stale-recover delta $stale_delta"
> -		dump_stats=1
> +		echo $ns stats
> +		ip -n $ns -s link show
>  	else
>  		print_ok
>  	fi
> -
> -	if [ "${dump_stats}" = 1 ]; then
> -		echo $ns stats
> -		ip netns exec $ns ip -s link show
> -		ip netns exec $ns nstat -as | grep MPTcp

mptcp_lib_pr_nstat() is called in fail_test(), it's slightly different
from the above line:

        ip netns exec "${ns}" nstat -as | grep Tcp

So, is it necessary for us to keep the above line?

Thanks,
-Geliang

> -	fi
>  }
>  
>  chk_add_nr()

Re: [PATCH mptcp-next 08/11] selftests: mptcp: join chk_stale_nr: avoid dup stats

Posted by Geliang Tang 1 week, 4 days ago

On Sun, 2025-12-28 at 10:25 +0800, Geliang Tang wrote:
> Hi Matt,
> 
> Thanks for this patch.
> 
> On Fri, 2025-12-26 at 07:40 +0100, Matthieu Baerts (NGI0) wrote:
> > nstat outputs are already printed when calling 'fail_test', no need
> > to
> > do it again.
> > 
> > While at it, no need to use the dump_stats variable, print the
> > extra
> > stats directly. And use 'ip -n $ns' instead of 'ip netns exec $ns',
> > shorter and clearer.
> > 
> > Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> > ---
> >  tools/testing/selftests/net/mptcp/mptcp_join.sh | 10 ++--------
> >  1 file changed, 2 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > index 63a84a4d43a6..80e03168b419 100755
> > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > @@ -1648,7 +1648,6 @@ chk_stale_nr()
> >  	local stale_min=$2
> >  	local stale_max=$3
> >  	local stale_delta=$4
> > -	local dump_stats
> >  	local stale_nr
> >  	local recover_nr
> >  
> > @@ -1664,16 +1663,11 @@ chk_stale_nr()
> >  		fail_test "got $stale_nr stale[s] $recover_nr
> > recover[s], " \
> >  		     " expected stale in range
> > [$stale_min..$stale_max]," \
> >  		     " stale-recover delta $stale_delta"
> > -		dump_stats=1
> > +		echo $ns stats
> > +		ip -n $ns -s link show
> >  	else
> >  		print_ok
> >  	fi
> > -
> > -	if [ "${dump_stats}" = 1 ]; then
> > -		echo $ns stats
> > -		ip netns exec $ns ip -s link show
> > -		ip netns exec $ns nstat -as | grep MPTcp
> 
> mptcp_lib_pr_nstat() is called in fail_test(), it's slightly
> different
> from the above line:
> 
>         ip netns exec "${ns}" nstat -as | grep Tcp
> 
> So, is it necessary for us to keep the above line?

Sorry, I made a mistake. The output of 'grep Tcp' includes the output
of 'grep MPTcp', so the information of dump_stats is indeed redundant.

So this patch looks good to me.

    Reviewed-by: Geliang Tang <geliang@kernel.org>

Thanks,
-Geliang

> 
> Thanks,
> -Geliang
> 
> > -	fi
> >  }
> >  
> >  chk_add_nr()