Re: [PATCH mptcp-next v6 0/7] add mp_fail testcases

[Geliang Tang <geliang.tang@suse.com>]

On Thu, Feb 10, 2022 at 02:00:31PM +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 10/02/2022 07:00, Geliang Tang wrote:
> > v6:
> >  - Split two patches from the last one.
> >  - Retry the multiple subflows test three times to fix this
> > "MP_FAIL MP_RST: 0 corrupted pkts" failure reported by me in v5:
> > 
> > Created /tmp/tmp.e4nE5Q14mj (size 1024 KB) containing data sent by client
> > Created /tmp/tmp.QwpQYClFnm (size 1024 KB) containing data sent by server
> > 001 MP_FAIL MP_RST: 0 corrupted pkts     syn[ ok ] - synack[ ok ] - ack[ ok ]
> >                                          sum[fail] got 0 data checksum error[s] expected 1
> >                                          ftx[fail] got 0 MP_FAIL[s] TX expected 1
> >                                          rtx[fail] got 0 MP_RST[s] TX expected 1
> >                                          itx[ ok ] - infirx[ ok ]
> > 
> > A test log of running v6 500 times is attached, named v6-loop-500-times.log,
> > in it, we can see retry happend 8 times (116, 136, 236, 295, 297, 402, 444,
> > 457), and no "0 corrupted pkts" any more.
> 
> I'm still not sure I understand what happened. Could it be because the
> subflow was never used to carry (enough) data and we never corrupt anything?
> 
> If that's the source of the issue, we can probably fix it by adding more
> delay on the other paths or something similar, no?

Matt, you're right! Adding more delay seems work on my test.

I changed the code like this:

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index dbc73e28cc06..e27f668b0134 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -219,6 +219,11 @@ reset_with_fail()
                action pedit munge offset 148 u8 invert \
                pipe csum tcp \
                index 100 || exit 1
+
+       if [ $i -eq 2 ]; then
+               tc -n $ns2 qdisc add dev ns2eth1 root netem rate 20mbit delay 1
+               tc -n $ns2 qdisc add dev ns2eth3 root netem rate 20mbit delay 1
+       fi
 }

Is this the right way to add delay?

Thanks,
-Geliang

> 
> If not and if you can reproduce it, may you also check IPTables and
> counters if you don't mind?
> 
>   ip netns exec $ns2 iptables -t mangle -L OUTPUT -v
>   ip -s -s link
> 
> Or because packets are dropped before MPTCP processing? We can also
> corrupt more packets or even all the ones carrying enough data.
> 
> I think we should really avoid retrying except if we have a very good
> reason to, e.g. something "normal" we cannot control, happening ~10% of
> the time and we retry only if we confirmed before it was due to that.
> 
> >  - Reduce the single subflow test files size from 1024KB to 128KB to fix
> > this "file received by client does not match" failure reported by CI and
> > Matt in v5:
> 
> As long as we don't hide another bug :)
> 
> > # Created /tmp/tmp.crkOA4p7hr (size 1024 KB) containing data sent by client
> > # Created /tmp/tmp.jFbZEAnYZa (size 1024 KB) containing data sent by server
> > # file received by server has inverted byte at 195585
> > # 100 MP_FAIL MP_RST: 1 corrupted pkts     syn[ ok ] - synack[ ok ] - ack[ ok ]
> > #                                          sum[ ok ] - csum  [ ok ]
> > #                                          ftx[ ok ] - failrx[ ok ]
> > #                                          rtx[ ok ] - rstrx [ ok ]
> > #                                          itx[ ok ] - infirx[ ok ]
> > # Created /tmp/tmp.crkOA4p7hr (size 1024 KB) containing data sent by client
> > # Created /tmp/tmp.jFbZEAnYZa (size 1024 KB) containing data sent by server
> > # [ FAIL ] file received by client does not match (in, out):
> > # -rw------- 1 root root 1048604 Feb  9 11:37 /tmp/tmp.jFbZEAnYZa
> > # Trailing bytes are:
> > # MPTCP_TEST_FILE_END_MARKER
> > # -rw------- 1 root root 1048606 Feb  9 11:37 /tmp/tmp.ghV0iWPhu5
> > # Trailing bytes are:
> > # MPTCP_TEST_FILE_END_MARKER
> > # file received by server has inverted byte at 169
> > # 101 Infinite map: 5 corrupted pkts       syn[ ok ] - synack[ ok ] - ack[ ok ]
> > #                                          sum[ ok ] - csum  [ ok ]
> > #                                          ftx[ ok ] - failrx[ ok ]
> > #                                          rtx[ ok ] - rstrx [ ok ]
> > #                                          itx[ ok ] - infirx[ ok ]
> > 
> > In the attached v6-loop-500-times.log, no "file received by client does
> > not match" any more.
> > 
> > I think this v6 is very stable, but there are still 6 tests failed in the
> > 500 time tests log (68 77 97 112 161 243). These failures are all due to
> > get one more unexpected checksum failure:
> 
> OK so probably at the end, we didn't have issues because we had a hash
> collision (no MP_FAIL while there were corrupted packets), that's good.
> 
> >  > cat v6-loop-500-times.log  | grep "\[fail"
> >                                          sum[fail] got 2 data checksum error[s] expected 1
> >                                          ftx[fail] got 2 MP_FAIL[s] TX expected 1
> >  - failrx[fail] got 2 MP_FAIL[s] RX expected 1
> >                                          rtx[fail] got 2 MP_RST[s] TX expected 1
> >  - rstrx [fail] got 2 MP_RST[s] RX expected 1
> >                                          sum[ ok ] - csum  [fail] got 1 data checksum error[s] expected 0
> >                                          sum[fail] got 2 data checksum error[s] expected 1
> >                                          ftx[fail] got 2 MP_FAIL[s] TX expected 1
> >  - failrx[fail] got 2 MP_FAIL[s] RX expected 1
> >                                          rtx[fail] got 2 MP_RST[s] TX expected 1
> >  - rstrx [fail] got 2 MP_RST[s] RX expected 1
> >                                          sum[ ok ] - csum  [fail] got 1 data checksum error[s] expected 0
> >                                          rtx[fail] got 2 MP_RST[s] TX expected 1
> >  - rstrx [fail] got 2 MP_RST[s] RX expected 1
> >                                          sum[fail] got 2 data checksum error[s] expected 1
> >                                          ftx[fail] got 2 MP_FAIL[s] TX expected 1
> >  - failrx[fail] got 2 MP_FAIL[s] RX expected 1
> >                                          rtx[fail] got 2 MP_RST[s] TX expected 1
> >  - rstrx [fail] got 2 MP_RST[s] RX expected 1
> 
> I see it is also happening when only one packet has been corrupted. So
> it is not because a few packets in a row have been corrupted I suppose.
> 
> I guess we don't have retransmitted MP_FAIL/RST here, right?
> 
> > These failures are related the checksum bug reported by me, issue #255.
> > When transferring a larger file, the checksum sometimes fails. Running
> > "./mptcp_connect.sh -C" in 10 times, we will the MP_FAILs. If we solve
> > issue #255 in the future, this mp_fail testcases will be more stable.
> 
> Indeed, maybe linked.
> 
> Cheers,
> Matt
> -- 
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net
>