tools/testing/selftests/net/mptcp/mptcp_join.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
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
done
@@ -442,8 +442,8 @@ do_transfer()
# rm_addr are serialized, allow the previous one to complete
id=${dump[$pos]}
rm_addr=$(rm_addr_count ${listener_ns})
- ip netns exec ${connector_ns} ./pm_nl_ctl del $id
wait_rm_addr ${listener_ns} ${rm_addr}
+ ip netns exec ${connector_ns} ./pm_nl_ctl del $id
let counter+=1
let pos+=5
done
--
2.31.1
Hi Geliang, On 07/01/2022 10:51, Geliang Tang wrote: > IPv6 removing test failed on my test every time, this patch fixed it. Thank you for looking at that! The public CI has many similar errors and maybe (hard to say) more with your patch, e.g.: https://api.cirrus-ci.com/v1/artifact/task/5889173428109312/summary/summary.txt > Our CI did some validations and here is its report: > > - KVM Validation: normal: > - Unstable: 1 failed test(s): selftest_mptcp_join 🔴: > - Task: https://cirrus-ci.com/task/5889173428109312 > - Summary: https://api.cirrus-ci.com/v1/artifact/task/5889173428109312/summary/summary.txt > > - KVM Validation: debug: > - Unstable: 2 failed test(s): selftest_diag selftest_mptcp_join 🔴: > - Task: https://cirrus-ci.com/task/5326223474688000 > - Summary: https://api.cirrus-ci.com/v1/artifact/task/5326223474688000/summary/summary.txt > > Initiator: Patchew Applier > Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/3b5258004128 Could it be linked? Should we eventually wait before and after having emitted the del command with different counters? Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
On Fri, Jan 07, 2022 at 12:03:00PM +0100, Matthieu Baerts wrote: > Hi Geliang, > > On 07/01/2022 10:51, Geliang Tang wrote: > > IPv6 removing test failed on my test every time, this patch fixed it. > > Thank you for looking at that! > > The public CI has many similar errors and maybe (hard to say) more with > your patch, e.g.: > > https://api.cirrus-ci.com/v1/artifact/task/5889173428109312/summary/summary.txt > > > > Our CI did some validations and here is its report: > > > > - KVM Validation: normal: > > - Unstable: 1 failed test(s): selftest_mptcp_join 🔴: > > - Task: https://cirrus-ci.com/task/5889173428109312 > > - Summary: > https://api.cirrus-ci.com/v1/artifact/task/5889173428109312/summary/summary.txt > > > > - KVM Validation: debug: > > - Unstable: 2 failed test(s): selftest_diag selftest_mptcp_join 🔴: > > - Task: https://cirrus-ci.com/task/5326223474688000 > > - Summary: > https://api.cirrus-ci.com/v1/artifact/task/5326223474688000/summary/summary.txt > > > > Initiator: Patchew Applier > > Commits: > https://github.com/multipath-tcp/mptcp_net-next/commits/3b5258004128 > > Could it be linked? Hi Matt, I tested this on my Thinkpad X1 laptop, only two failures occur. But I think it's somehow linked with the commit "selftests: mptcp: more stable join tests-cases", it dropped many latencies (sleep 1) in the selftests. The CI is runing in a KVM, right? I think we didn't tune the latencies of the selftests for a KVM yet (At least for me). I think we need to login into a KVM to run the selftests there and change the latencies there. I think different latencies are needed. We need to make sure the selftests could be passed on both a real machine and a KVM. Could you tell me how to get the CI KVM image? Or could you do this test? Thanks -Geliang > > Should we eventually wait before and after having emitted the del > command with different counters? > > Cheers, > Matt > -- > Tessares | Belgium | Hybrid Access Solutions > www.tessares.net >
Hi Geliang, On 10/01/2022 08:43, Geliang Tang wrote: > On Fri, Jan 07, 2022 at 12:03:00PM +0100, Matthieu Baerts wrote: >> Hi Geliang, >> >> On 07/01/2022 10:51, Geliang Tang wrote: >>> IPv6 removing test failed on my test every time, this patch fixed it. >> >> Thank you for looking at that! >> >> The public CI has many similar errors and maybe (hard to say) more with >> your patch, e.g.: >> >> https://api.cirrus-ci.com/v1/artifact/task/5889173428109312/summary/summary.txt >> >> >>> Our CI did some validations and here is its report: >>> >>> - KVM Validation: normal: >>> - Unstable: 1 failed test(s): selftest_mptcp_join 🔴: >>> - Task: https://cirrus-ci.com/task/5889173428109312 >>> - Summary: >> https://api.cirrus-ci.com/v1/artifact/task/5889173428109312/summary/summary.txt >>> >>> - KVM Validation: debug: >>> - Unstable: 2 failed test(s): selftest_diag selftest_mptcp_join 🔴: >>> - Task: https://cirrus-ci.com/task/5326223474688000 >>> - Summary: >> https://api.cirrus-ci.com/v1/artifact/task/5326223474688000/summary/summary.txt >>> >>> Initiator: Patchew Applier >>> Commits: >> https://github.com/multipath-tcp/mptcp_net-next/commits/3b5258004128 >> >> Could it be linked? > > Hi Matt, > > I tested this on my Thinkpad X1 laptop, only two failures occur. But > I think it's somehow linked with the commit "selftests: mptcp: more > stable join tests-cases", it dropped many latencies (sleep 1) in the > selftests. > > The CI is runing in a KVM, right? I think we didn't tune the latencies > of the selftests for a KVM yet (At least for me). I think we need to login > into a KVM to run the selftests there and change the latencies there. I > think different latencies are needed. We need to make sure the selftests > could be passed on both a real machine and a KVM. > > Could you tell me how to get the CI KVM image? Or could you do this > test? Sorry for the late reply. To test, it is very easy, you just need to install a recent version of docker before: cd <kernel source code> docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \ --pull always mptcp/mptcp-upstream-virtme-docker:latest \ auto-all This will do what the CI is doing: running all tests, once without and once with a "debug" kernel config. There are different arguments you can give instead of "auto-all": only "normal" or "debug" kernel config, having a shell instead of running all tests, etc.. Please see: https://github.com/multipath-tcp/mptcp-upstream-virtme-docker#entrypoint-options Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
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
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. Thanks, -Geliang > > Thanks! > > Paolo >
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
On Tue, Jan 11, 2022 at 01:02:15PM +0100, Paolo Abeni wrote: > 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 > --- Hi Paolo, This patch works on my test. Should we change this "single subflow IPv6" test too? --- @@ -1654,7 +1655,7 @@ ipv6_tests() reset pm_nl_set_limits $ns1 0 1 pm_nl_set_limits $ns2 0 1 - pm_nl_add_endpoint $ns2 dead:beef:3::2 flags subflow + pm_nl_add_endpoint $ns2 dead:beef:3::2 dev ns2eth3 flags subflow run_tests $ns1 $ns2 dead:beef:1::1 0 0 0 slow chk_join_nr "single subflow IPv6" 1 1 1 --- This "single subflow IPv6" test passes every time with or without this change, but the "remove subflow and signal IPv6" test fails. What's the difference between the two tests? Thanks, -Geliang > /P >
On Thu, 2022-01-13 at 18:35 +0800, Geliang Tang wrote: > On Tue, Jan 11, 2022 at 01:02:15PM +0100, Paolo Abeni wrote: > > 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 > > --- > > Hi Paolo, > > This patch works on my test. Should we change this "single subflow IPv6" test too? In theory, yes... > > --- > > @@ -1654,7 +1655,7 @@ ipv6_tests() > reset > pm_nl_set_limits $ns1 0 1 > pm_nl_set_limits $ns2 0 1 > - pm_nl_add_endpoint $ns2 dead:beef:3::2 flags subflow > + pm_nl_add_endpoint $ns2 dead:beef:3::2 dev ns2eth3 flags subflow > run_tests $ns1 $ns2 dead:beef:1::1 0 0 0 slow > chk_join_nr "single subflow IPv6" 1 1 1 > > --- > > This "single subflow IPv6" test passes every time with or without this > change, ... in pratice, as you noted, it's not needed. > but the "remove subflow and signal IPv6" test fails. What's the > difference between the two tests? I'm wild guessing, as I still fail to understand the full picture completely. In the "single subflow IPv6" test, is irrelevant if the additional subflow uses the "wrong" interface, because we just need to establish it. In the "remove subflow and signal IPv6" we have additional time constrains due to multiple subflow creation and removal, and the wrong routing setup causes the large delay I mentioned before, ence the failure. I'm very suprised the wrong routing setup casues a delay instead of an unreachable error and that puzzle me much. Do you prefer sending a new version of the squash-to patch, or do you prefer I'll go ahead with that? Please, let me know, /P
Hello, On 13/01/2022 11:52, Paolo Abeni wrote: > Do you prefer sending a new version of the squash-to patch, or do you > prefer I'll go ahead with that? Small detail just to be sure: as you probably know the patch "selftests: mptcp: more stable join tests-cases" has been accepted in net-next. So we can no longer have a "squash-to" patch. Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net
On Thu, Jan 13, 2022 at 11:52:20AM +0100, Paolo Abeni wrote: > On Thu, 2022-01-13 at 18:35 +0800, Geliang Tang wrote: > > On Tue, Jan 11, 2022 at 01:02:15PM +0100, Paolo Abeni wrote: > > > 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 > > > --- > > > > Hi Paolo, > > > > This patch works on my test. Should we change this "single subflow IPv6" test too? > > In theory, yes... > > > > --- > > > > @@ -1654,7 +1655,7 @@ ipv6_tests() > > reset > > pm_nl_set_limits $ns1 0 1 > > pm_nl_set_limits $ns2 0 1 > > - pm_nl_add_endpoint $ns2 dead:beef:3::2 flags subflow > > + pm_nl_add_endpoint $ns2 dead:beef:3::2 dev ns2eth3 flags subflow > > run_tests $ns1 $ns2 dead:beef:1::1 0 0 0 slow > > chk_join_nr "single subflow IPv6" 1 1 1 > > > > --- > > > > This "single subflow IPv6" test passes every time with or without this > > change, > > ... in pratice, as you noted, it's not needed. > > > but the "remove subflow and signal IPv6" test fails. What's the > > difference between the two tests? > > I'm wild guessing, as I still fail to understand the full picture > completely. In the "single subflow IPv6" test, is irrelevant if the > additional subflow uses the "wrong" interface, because we just need to > establish it. > > In the "remove subflow and signal IPv6" we have additional time > constrains due to multiple subflow creation and removal, and the wrong > routing setup causes the large delay I mentioned before, ence the > failure. > > I'm very suprised the wrong routing setup casues a delay instead of an > unreachable error and that puzzle me much. > > Do you prefer sending a new version of the squash-to patch, or do you > prefer I'll go ahead with that? > > Please, let me know, Hi Paolo, Please send the new version. I'll test it if needed. Thanks, -Geliang > > /P >
© 2016 - 2024 Red Hat, Inc.