Re: [PATCH mptcp-next] Squash to "selftests: mptcp: more stable join tests-cases"

[Paolo Abeni <pabeni@redhat.com>]

Hello,

On Fri, 2022-01-07 at 17:51 +0800, Geliang Tang wrote:
> IPv6 removing test failed on my test every time, this patch fixed it.
> 
> 12 remove subflow and signal IPv6       syn[ ok ] - synack[fail] got 1 JOIN[s] synack expected 2
>  - ack[fail] got 1 JOIN[s] ack expected 2
> Server ns stats
> TcpPassiveOpens                 2                  0.0
> TcpAttemptFails                 1                  0.0
> TcpInSegs                       54                 0.0
> TcpOutSegs                      58                 0.0
> TcpRetransSegs                  2                  0.0
> TcpExtEmbryonicRsts             1                  0.0
> TcpExtDelayedACKs               15                 0.0
> TcpExtTCPPureAcks               23                 0.0
> TcpExtTCPTimeouts               1                  0.0
> TcpExtTCPSynRetrans             2                  0.0
> TcpExtTCPOrigDataSent           24                 0.0
> TcpExtTCPDelivered              24                 0.0
> MPTcpExtMPCapableSYNRX          1                  0.0
> MPTcpExtMPCapableACKRX          1                  0.0
> MPTcpExtMPJoinSynRx             2                  0.0
> MPTcpExtMPJoinAckRx             1                  0.0
> MPTcpExtEchoAdd                 1                  0.0
> MPTcpExtRmSubflow               1                  0.0
> Client ns stats
> TcpActiveOpens                  3                  0.0
> TcpInSegs                       58                 0.0
> TcpOutSegs                      53                 0.0
> TcpRetransSegs                  1                  0.0
> TcpOutRsts                      3                  0.0
> TcpExtDelayedACKs               3                  0.0
> TcpExtTCPPureAcks               29                 0.0
> TcpExtTCPTimeouts               1                  0.0
> TcpExtTCPSynRetrans             1                  0.0
> TcpExtTCPOrigDataSent           24                 0.0
> TcpExtTCPDelivered              26                 0.0
> TcpExtTcpTimeoutRehash          1                  0.0
> MPTcpExtMPCapableSYNTX          1                  0.0
> MPTcpExtMPCapableSYNACKRX       1                  0.0
> MPTcpExtMPJoinSynAckRx          1                  0.0
> MPTcpExtAddAddr                 1                  0.0
> MPTcpExtRmAddr                  1                  0.0
> MPTcpExtRmSubflow               1                  0.0
>                                         add[ ok ] - echo  [ ok ]
>                                         rm [fail] got 0 RM_ADDR[s] expected 1
>  - sf    [ ok ]
> Server ns stats
> TcpPassiveOpens                 2                  0.0
> TcpAttemptFails                 1                  0.0
> TcpInSegs                       54                 0.0
> TcpOutSegs                      58                 0.0
> TcpRetransSegs                  2                  0.0
> TcpExtEmbryonicRsts             1                  0.0
> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index e48ce23d2386..41a2510e2f97 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -393,8 +393,8 @@ do_transfer()
>  				do
>  					id=${dump[$pos]}
>  					rm_addr=$(rm_addr_count ${connector_ns})
> -					ip netns exec ${listener_ns} ./pm_nl_ctl del $id
>  					wait_rm_addr ${connector_ns} ${rm_addr}
> +					ip netns exec ${listener_ns} ./pm_nl_ctl del $id
>  					let counter+=1
>  					let pos+=5

Uhmm.... this is equivalent to replacing 'wait_rm_addr()' with 'sleep
1'. It defeats the purpouse of wait_rm_addr itself: reducing the delay
required for 'del address' related operation. Additionally, an
unconditional 'sleep 1' could impact negatively in other test-cases.

Possibly some alike:

---
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index e48ce23d2386..b209a303d1fe 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -366,7 +366,7 @@ do_transfer()
 
        # let the mptcp subflow be established in background before
        # do endpoint manipulation
-       [ $addr_nr_ns1 = "0" -a $addr_nr_ns2 = "0" ] || sleep 1
+       [ $addr_nr_ns1 = "0" -a $addr_nr_ns2 = "0" ] || sleep 2
 
        if [ $addr_nr_ns1 -gt 0 ]; then
                let add_nr_ns1=addr_nr_ns1
---

could be more appropriate, but 2secs to complete MPC + 2 MPJ HS looks
really too much...

Can you please capture a pcap trace with the failing test?

Thanks!

Paolo