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

[Paolo Abeni <pabeni@redhat.com>]

On Tue, 2022-01-11 at 11:45 +0800, Geliang Tang wrote:
> Hi Paolo,
> 
> On Mon, Jan 10, 2022 at 12:11:38PM +0100, Paolo Abeni wrote:
> > 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
> > ---
> 
> This patch doesn't work on my test either. I changed it to "sleep 2",
> the "remove subflow and signal IPv6" test still failed.
> 
> > 
> > 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?
> 
> The pcap is attached.

Thank you! 

I'm really fail to understand why that happens, but the server replies
to the subflow endpoint MPJ syn with more than 1s delay, and that
causes the failure.

The interesting part is that the ipv6 routing configuration is actually
broken, as we lack an ipv6 route for the server on each link. The
following addresses the issue here, but I don't see how things were
somewhat working so far...

---
diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index e48ce23d2386..cdfb7da47384 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -76,6 +76,7 @@ init()
 
 		# let $ns2 reach any $ns1 address from any interface
 		ip -net "$ns2" route add default via 10.0.$i.1 dev ns2eth$i metric 10$i
+		ip -net "$ns2" route add default via dead:beef:$i::1 dev ns2eth$i metric 10$i
 	done
 }
 
@@ -1545,7 +1546,7 @@ ipv6_tests()
 	ip netns exec $ns1 ./pm_nl_ctl limits 0 2
 	ip netns exec $ns1 ./pm_nl_ctl add dead:beef:2::1 flags signal
 	ip netns exec $ns2 ./pm_nl_ctl limits 1 2
-	ip netns exec $ns2 ./pm_nl_ctl add dead:beef:3::2 flags subflow
+	ip netns exec $ns2 ./pm_nl_ctl add dead:beef:3::2 dev ns2eth3 flags subflow
 	run_tests $ns1 $ns2 dead:beef:1::1 0 -1 -1 slow
 	chk_join_nr "remove subflow and signal IPv6" 2 2 2
 	chk_add_nr 1 1
---
/P