Re: [PATCH mptcp-next v5 5/5] selftests: mptcp: add mp_fail testcases

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

Hi Matt,

On Wed, Feb 09, 2022 at 01:36:33PM +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 09/02/2022 11:43, Geliang Tang wrote:
> > Added the test cases for MP_FAIL, the multiple subflows test for the MP_RST
> > case and the single subflow one for the infinite mapping case.
> 
> The tests are passing on the public CI, that's good!
> 
> But the output is less good:
> 
> # 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 ]
> 
> Did you not do a modification earlier not to print "file received by
> client does not match" error while it is "normal" for the moment to have
> that in this case?

My modification works, we got the inverted byte on the server side. This
mismatch is on the client side. This failure rarely happens in my test.
I guess it maybe related the checksum bug reported by me, issue #255.

Thanks,
-Geliang

> 
> Even if it is temporarily, would it not be better to add a variable
> "corrupted_data", set to 0 in the 'init()' side but set to 1 for this
> test? When it happens, we expect to have a mismatch, we can print a
> message if it was (with a ref to a GitHub ticket?) and mark the test as
> failed if not. WDYT?
> 
> (also, because from a TCP point of view, the packet is still OK, we
> could also not drop the data but that's another discussion)
> 
> Cheers,
> Matt
> -- 
> Tessares | Belgium | Hybrid Access Solutions
> www.tessares.net
>