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

[Geliang Tang <geliang@kernel.org>]

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()