Re: [PATCH mptcp-next 2/5] selftests: mptcp: move stats info in case of errors to lib.sh

[Geliang Tang <geliang@kernel.org>]

Hi Matt,

On Thu, 2025-01-09 at 16:30 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> Thank you for your review!
> 
> On 09/01/2025 10:24, Geliang Tang wrote:
> > Hi Matt,
> > 
> > Thanks for this set.
> > 
> > On Wed, 2025-01-08 at 19:40 +0100, Matthieu Baerts (NGI0) wrote:
> > > A few MPTCP selftests are using the same code to print stats in
> > > case
> > > of
> > > error. This code can then be moved to mptcp_lib.sh.
> > > 
> > > No behaviour changes intended, except to print the error in red
> > > and
> > > to
> > > stderr, like most error messages.
> 
> (...)
> 
> > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > index
> > > 975d4d4c862afff2e685e86dc08a892dbd09d783..50ca64357053b14cd4989e6
> > > 647d
> > > 825337978fdba 100644
> > > --- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > +++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
> > > @@ -107,6 +107,27 @@ mptcp_lib_pr_info() {
> > >  	mptcp_lib_print_info "INFO: ${*}"
> > >  }
> > >  
> > > +# $1-2: listener/connector ns ; $3 port ; [ $4-5
> > > listener/connector
> > > stat file ]
> > > +mptcp_lib_pr_err_stats() {
> > > +	local lns="${1}"
> > > +	local cns="${2}"
> > > +	local port="${3}"
> > > +	local lstat="${4:-}"
> > > +	local cstat="${5:-}"
> > 
> > I think it's better to add a separate patch to add
> > 
> > 	cat /tmp/${listener_ns}.out
> > 
> > and
> > 
> > 	cat /tmp/${connector_ns}.out
> > 
> > in mptcp_sockopt.sh,
> 
> mptcp_sockopt.sh currently doesn't have such file. I hesitated to add
> them using the content of nstat like in the other ones, but I didn't
> really see how it would help to fix issues with socket options. Or do
> you have something in mind?
> 
> I guess we could add them "just in case", but...

I just sent a patch named "selftests: mptcp: sockopt: save nstat infos"
to do this.

> 
> > then we can drop the last two arguments "lstat"
> > and "cstat" of this helper.
> 
> ... sorry, I'm not sure to understand what you meant here. We still
> want
> to print the content of these files if available.

And a squash-to patch for this.

Thanks,
-Geliang

> 
> 
> Cheers,
> Matt