* [PATCH mptcp-next] Squash to "selftests: mptcp: more stable join tests-cases"
@ 2022-01-07 9:51 Geliang Tang
2022-01-07 11:03 ` Matthieu Baerts
2022-01-10 11:11 ` Paolo Abeni
0 siblings, 2 replies; 11+ messages in thread
From: Geliang Tang @ 2022-01-07 9:51 UTC (permalink / raw)
To: mptcp; +Cc: Geliang Tang
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH mptcp-next] Squash to "selftests: mptcp: more stable join tests-cases" 2022-01-07 9:51 [PATCH mptcp-next] Squash to "selftests: mptcp: more stable join tests-cases" Geliang Tang @ 2022-01-07 11:03 ` Matthieu Baerts 2022-01-10 7:43 ` Geliang Tang 2022-01-10 11:11 ` Paolo Abeni 1 sibling, 1 reply; 11+ messages in thread From: Matthieu Baerts @ 2022-01-07 11:03 UTC (permalink / raw) To: Geliang Tang, mptcp 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH mptcp-next] Squash to "selftests: mptcp: more stable join tests-cases" 2022-01-07 11:03 ` Matthieu Baerts @ 2022-01-10 7:43 ` Geliang Tang 2022-01-12 17:42 ` Matthieu Baerts 0 siblings, 1 reply; 11+ messages in thread From: Geliang Tang @ 2022-01-10 7:43 UTC (permalink / raw) To: Matthieu Baerts; +Cc: mptcp 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH mptcp-next] Squash to "selftests: mptcp: more stable join tests-cases" 2022-01-10 7:43 ` Geliang Tang @ 2022-01-12 17:42 ` Matthieu Baerts 0 siblings, 0 replies; 11+ messages in thread From: Matthieu Baerts @ 2022-01-12 17:42 UTC (permalink / raw) To: Geliang Tang; +Cc: mptcp 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH mptcp-next] Squash to "selftests: mptcp: more stable join tests-cases" 2022-01-07 9:51 [PATCH mptcp-next] Squash to "selftests: mptcp: more stable join tests-cases" Geliang Tang 2022-01-07 11:03 ` Matthieu Baerts @ 2022-01-10 11:11 ` Paolo Abeni 2022-01-11 3:45 ` Geliang Tang 1 sibling, 1 reply; 11+ messages in thread From: Paolo Abeni @ 2022-01-10 11:11 UTC (permalink / raw) To: Geliang Tang, mptcp 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH mptcp-next] Squash to "selftests: mptcp: more stable join tests-cases" 2022-01-10 11:11 ` Paolo Abeni @ 2022-01-11 3:45 ` Geliang Tang 2022-01-11 12:02 ` Paolo Abeni 0 siblings, 1 reply; 11+ messages in thread From: Geliang Tang @ 2022-01-11 3:45 UTC (permalink / raw) To: Paolo Abeni; +Cc: mptcp [-- Attachment #1: Type: text/plain, Size: 5203 bytes --] 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 > [-- Attachment #2: mp_join-01-ns1-0-Ns6MYA.pcap --] [-- Type: application/vnd.tcpdump.pcap, Size: 22724 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH mptcp-next] Squash to "selftests: mptcp: more stable join tests-cases" 2022-01-11 3:45 ` Geliang Tang @ 2022-01-11 12:02 ` Paolo Abeni 2022-01-13 10:35 ` Geliang Tang 0 siblings, 1 reply; 11+ messages in thread From: Paolo Abeni @ 2022-01-11 12:02 UTC (permalink / raw) To: Geliang Tang; +Cc: mptcp 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH mptcp-next] Squash to "selftests: mptcp: more stable join tests-cases" 2022-01-11 12:02 ` Paolo Abeni @ 2022-01-13 10:35 ` Geliang Tang 2022-01-13 10:52 ` Paolo Abeni 0 siblings, 1 reply; 11+ messages in thread From: Geliang Tang @ 2022-01-13 10:35 UTC (permalink / raw) To: Paolo Abeni; +Cc: mptcp 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH mptcp-next] Squash to "selftests: mptcp: more stable join tests-cases" 2022-01-13 10:35 ` Geliang Tang @ 2022-01-13 10:52 ` Paolo Abeni 2022-01-13 11:36 ` Matthieu Baerts 2022-01-13 12:58 ` Geliang Tang 0 siblings, 2 replies; 11+ messages in thread From: Paolo Abeni @ 2022-01-13 10:52 UTC (permalink / raw) To: Geliang Tang; +Cc: mptcp 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH mptcp-next] Squash to "selftests: mptcp: more stable join tests-cases" 2022-01-13 10:52 ` Paolo Abeni @ 2022-01-13 11:36 ` Matthieu Baerts 2022-01-13 12:58 ` Geliang Tang 1 sibling, 0 replies; 11+ messages in thread From: Matthieu Baerts @ 2022-01-13 11:36 UTC (permalink / raw) To: Paolo Abeni, Geliang Tang; +Cc: mptcp 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH mptcp-next] Squash to "selftests: mptcp: more stable join tests-cases" 2022-01-13 10:52 ` Paolo Abeni 2022-01-13 11:36 ` Matthieu Baerts @ 2022-01-13 12:58 ` Geliang Tang 1 sibling, 0 replies; 11+ messages in thread From: Geliang Tang @ 2022-01-13 12:58 UTC (permalink / raw) To: Paolo Abeni; +Cc: mptcp 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 > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-01-13 12:58 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-01-07 9:51 [PATCH mptcp-next] Squash to "selftests: mptcp: more stable join tests-cases" Geliang Tang 2022-01-07 11:03 ` Matthieu Baerts 2022-01-10 7:43 ` Geliang Tang 2022-01-12 17:42 ` Matthieu Baerts 2022-01-10 11:11 ` Paolo Abeni 2022-01-11 3:45 ` Geliang Tang 2022-01-11 12:02 ` Paolo Abeni 2022-01-13 10:35 ` Geliang Tang 2022-01-13 10:52 ` Paolo Abeni 2022-01-13 11:36 ` Matthieu Baerts 2022-01-13 12:58 ` Geliang Tang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.