Re: [MPTCP][PATCH mptcp-next] Squash to "selftests: mptcp: add testcase for active-back"

[Geliang Tang <geliangtang@gmail.com>]

Hi Matt,

Matthieu Baerts <matthieu.baerts@tessares.net> 于2021年7月22日周四 下午4:55写道：
>
> Hi Geliang,
>
> On 21/07/2021 10:56, Geliang Tang wrote:
> > Drop the debug logs and make the output aligned:
>
> Good idea!
>
> > 05 backup flow used, bidi, link failure syn[ ok ] - synack[ ok ] - ack[ ok ]
> >                                         add[ ok ] - echo  [ ok ]
> >                                         stale             [ ok ]
> >                                         link usage        [ ok ]
>
> I wonder if it would not be even clearer to display one item per line, e.g.:
>
>
> 05 backup flow used, bidi, link failure:
>      syn        [ ok ]
>      synack     [ ok ]
>      ack        [ ok ]
>      add        [ ok ]
>      echo       [ ok ]
>      stale      [ ok ]
>      link usage [ ok ]
> 06 (...)
>

I think the original display is much better than one item per line. Since
it put the related items in one line, and do the different tests in each
line. I think that's more clearer than one item per line. Do you agree?

> But that is for another patch anyway. I can look at that later when
> trying to make this test more reliable by removing fixed values for
> "sleep" and others if nobody is already looking at that.
>
> >
> > Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> > ---
> >  tools/testing/selftests/net/mptcp/mptcp_join.sh | 13 ++++++-------
> >  1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > index 220154cb92a7..d0518487b874 100755
> > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > @@ -228,7 +228,6 @@ link_failure()
> >
> >       for l in $FAILING_LINKS; do
> >               veth="ns1eth$l"
> > -             echo "link $veth down" 1>&2
> >               ip -net "$ns" link set "$veth" down
> >       done
> >  }
> > @@ -634,7 +633,7 @@ chk_stale_nr()
> >       local stale_nr
> >       local recover_nr
> >
> > -     printf "%-39s %s" " " "stale"
> > +     printf "%-39s %s" " " "stale             "
> >       stale_nr=`ip netns exec $ns nstat -as | grep MPTcpExtSubflowStale | awk '{print $2}'`
> >       [ -z "$stale_nr" ] && stale_nr=0
> >       recover_nr=`ip netns exec $ns nstat -as | grep MPTcpExtSubflowRecover | awk '{print $2}'`
> > @@ -878,7 +877,7 @@ chk_link_usage()
> >       local tx_rate=$((tx_link * 100 / $tx_total))
> >       local tolerance=5
> >
> > -     printf "%-39s %s" " " "link usage "
> > +     printf "%-39s %s" " " "link usage        "
>
> Do you mind if I change this when applying the patch to avoid plenty of
> spaces?
>
>   printf "%-39s %-18s" " " "stale"
>   printf "%-39s %-18s" " " "link usage"

It's much better to avoid spaces like this, thanks Matt.

-Geliang

>
> Cheers,
> Matt
> --
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net