Re: [PATCH mptcp-next v8 0/5] add mp_fail testcases

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

On Tue, 15 Feb 2022, Geliang Tang wrote:

> v8:
> - move two MP_RST mibs patches into the "add fastclose testcases" series.
> - allow multiple checksum errors in patch 3.
> - depends on "add fastclose testcases v3".
>

Thanks Geliang. This series (plus the two associated squash-to patches in 
patchwork) look good to me. Matthieu, note the dependency on the fastclose 
test series that has a pending question about intermittent failures.

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>


> RESEND:
> - no code modified, just updated the commit logs and comments.
> v7:
> This version makes the multiple subflows test more stable.
> - Add delays and drop 'retry' in the multiple subflows test in patch 7.
> - Don't add two addresses for the multiple subflows test in patch 7, add
> one address 10.0.2.2 is enough, this make it more stable.
> - Add a comment in patch 6.
> - Rebased to export/20220211T054659.
> - A 500 times loop test log of v7 will be attached.
>
> 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.
>
> - 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:
>
> # 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:
>
> > 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
>
> 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.
>
> v5:
> - update patch 5 as Matt suggested.
> - use '|| exit 1'
> - drop jq
> - drop pedit_action
>
> v4:
> - add the mibs for MP_RST
> - patch 4 "selftests: mptcp: add MP_RST mibs check" uses the variable
> $nr_blank, so it depends on the commit "selftests: mptcp: adjust output
> alignment for more tests".
>
> v3:
> - check the exit code of iptables.
> - add ip6tables support for reset_with_fail too.
> - add the null check for $packets
> - rename nr_mp_fail to pedit_action and get_nr_mp_fail to
> pedit_action_happened
>
> This is v12 of the mp_fail testcases with Matt's changes. It works well
> and it's very stable.
>
> Geliang Tang (5):
>  Squash to "mptcp: infinite mapping receiving"
>  Squash to "selftests: mptcp: add infinite map mibs check"
>  selftests: mptcp: add more arguments for chk_join_nr
>  selftests: mptcp: reuse linkfail to make given size files
>  selftests: mptcp: add the MP_FAIL testcases
>
> net/mptcp/subflow.c                           |   1 +
> tools/testing/selftests/net/mptcp/config      |   8 +
> .../testing/selftests/net/mptcp/mptcp_join.sh | 216 +++++++++++++++---
> 3 files changed, 192 insertions(+), 33 deletions(-)
>
> -- 
> 2.34.1
>
>
>

--
Mat Martineau
Intel