[PATCH mptcp-net] selftests: mptcp: fix fastclose with csum failure

Paolo Abeni posted 1 patch 5 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/720d67a275179d96aba05cccd56a98f10da47aa1.1699647725.git.pabeni@redhat.com
Maintainers: Matthieu Baerts <matttbe@kernel.org>, Mat Martineau <martineau@kernel.org>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Shuah Khan <shuah@kernel.org>
tools/testing/selftests/net/mptcp/mptcp_join.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH mptcp-net] selftests: mptcp: fix fastclose with csum failure
Posted by Paolo Abeni 5 months, 3 weeks ago
Running the mp_join selftest manually with the following command line:

  ./mptcp_join.sh -z -C

lead to some failures:

  002 fastclose server test
  # ...
  rtx                                 [fail] got 1 MP_RST[s] TX expected 0
  # ...
  rstrx                               [fail] got 1 MP_RST[s] RX expected 0

the problem is really in the wrong expectations for the RST checks
implied by the csum validation. Note that the same check is repeated
explicitly in the same test-case, with the correct expectation and
pass successfully.

Address the issue explicitly setting the correct expectation for
the failing checks.

Reported-by: Reported-by: Xiumei Mu <xmu@redhat.com>
Fixes: ("6bf41020b72b selftests: mptcp: update and extend fastclose test-cases")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 995280882428..1606474232f6 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -3220,7 +3220,7 @@ fastclose_tests()
 	if reset_check_counter "fastclose server test" "MPTcpExtMPFastcloseRx"; then
 		test_linkfail=1024 fastclose=server \
 			run_tests $ns1 $ns2 10.0.1.1
-		chk_join_nr 0 0 0
+		chk_join_nr 0 0 0 0 0 0 1
 		chk_fclose_nr 1 1 invert
 		chk_rst_nr 1 1
 	fi
-- 
2.41.0
Re: [PATCH mptcp-net] selftests: mptcp: fix fastclose with csum failure
Posted by Matthieu Baerts 5 months, 3 weeks ago
Hi Paolo,

On 10/11/2023 21:24, Paolo Abeni wrote:
> Running the mp_join selftest manually with the following command line:
> 
>   ./mptcp_join.sh -z -C
> 
> lead to some failures:
> 
>   002 fastclose server test
>   # ...
>   rtx                                 [fail] got 1 MP_RST[s] TX expected 0
>   # ...
>   rstrx                               [fail] got 1 MP_RST[s] RX expected 0
> 
> the problem is really in the wrong expectations for the RST checks
> implied by the csum validation. Note that the same check is repeated
> explicitly in the same test-case, with the correct expectation and
> pass successfully.
> 
> Address the issue explicitly setting the correct expectation for
> the failing checks.

Thank you for the patch!

Now in our tree (fixes for -net):

New patches for t/upstream-net and t/upstream:
- 925598813f6a: selftests: mptcp: fix fastclose with csum failure
- Results: cecbfe325d48..5d8953c9cae1 (export-net)
- Results: 69a6cbb603df..2d9a700b962e (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20231113T094142
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20231113T094142

Cheers,
Matt
Re: [PATCH mptcp-net] selftests: mptcp: fix fastclose with csum failure
Posted by Matthieu Baerts 5 months, 3 weeks ago
Hi Paolo,

On 10/11/2023 21:24, Paolo Abeni wrote:
> Running the mp_join selftest manually with the following command line:
> 
>   ./mptcp_join.sh -z -C
> 
> lead to some failures:
> 
>   002 fastclose server test
>   # ...
>   rtx                                 [fail] got 1 MP_RST[s] TX expected 0
>   # ...
>   rstrx                               [fail] got 1 MP_RST[s] RX expected 0
> 
> the problem is really in the wrong expectations for the RST checks
> implied by the csum validation. Note that the same check is repeated
> explicitly in the same test-case, with the correct expectation and
> pass successfully.

Good catch! LGTM:

Reviewed-by: Matthieu Baerts <matttbe@kernel.org>

Note that I was a bit confused to see the CI complaining about this test
while not testing it with '-C':

> # 103 fastclose server test
> #       Info: Test file (size 1024 KB) for client
> #       Info: Test file (size 1024 KB) for server
> # copyfd_io_poll: poll timed out (events: POLLIN 1, POLLOUT 4)
> # [fail] client exit code 2, server 0
> # Server ns stats
> # TcpPassiveOpens                 1                  0.0
> # TcpEstabResets                  1                  0.0
> # TcpInSegs                       6                  0.0
> # TcpOutSegs                      7                  0.0
> # TcpOutRsts                      2                  0.0
> # TcpExtTCPPureAcks               2                  0.0
> # TcpExtTCPOrigDataSent           2                  0.0
> # TcpExtTCPDelivered              2                  0.0
> # MPTcpExtMPCapableSYNRX          1                  0.0
> # MPTcpExtMPCapableACKRX          1                  0.0
> # MPTcpExtMPFastcloseTx           1                  0.0
> # MPTcpExtMPRstTx                 1                  0.0
> # Client ns stats
> # TcpActiveOpens                  1                  0.0
> # TcpEstabResets                  1                  0.0
> # TcpInSegs                       7                  0.0
> # TcpOutSegs                      10                 0.0
> # TcpExtTCPPureAcks               2                  0.0
> # TcpExtTCPOrigDataSent           7                  0.0
> # TcpExtTCPDelivered              7                  0.0
> # MPTcpExtMPCapableSYNTX          1                  0.0
> # MPTcpExtMPCapableSYNACKRX       1                  0.0
> # 
> # netns ns1-654eb1d7-Nk17nk socket stat for 10102:
> # Netid State Recv-Q Send-Q Local Address:Port Peer Address:PortProcess
> # TcpPassiveOpens                 1                  0.0
> # TcpEstabResets                  1                  0.0
> # TcpInSegs                       6                  0.0
> # TcpOutSegs                      7                  0.0
> # TcpOutRsts                      2                  0.0
> # TcpExtTCPPureAcks               2                  0.0
> # TcpExtTCPOrigDataSent           2                  0.0
> # TcpExtTCPDelivered              2                  0.0
> # MPTcpExtMPCapableSYNRX          1                  0.0
> # MPTcpExtMPCapableACKRX          1                  0.0
> # MPTcpExtMPFastcloseTx           1                  0.0
> # MPTcpExtMPRstTx                 1                  0.0
> # 
> # netns ns2-654eb1d7-Nk17nk socket stat for 10102:
> # Netid State Recv-Q Send-Q Local Address:Port Peer Address:PortProcess
> # TcpActiveOpens                  1                  0.0
> # TcpEstabResets                  1                  0.0
> # TcpInSegs                       7                  0.0
> # TcpOutSegs                      10                 0.0
> # TcpExtTCPPureAcks               2                  0.0
> # TcpExtTCPOrigDataSent           7                  0.0
> # TcpExtTCPDelivered              7                  0.0
> # MPTcpExtMPCapableSYNTX          1                  0.0
> # MPTcpExtMPCapableSYNACKRX       1                  0.0
> #       syn                                 [ ok ]
> #       synack                              [ ok ]
> #       ack                                 [ ok ]
> #       ctx                                 [ ok ]
> #       fclzrx                              [fail] got 0 MP_FASTCLOSE[s] RX expected 1
> #       Info: invert,rx=0
> #       rtx                                 [ ok ]
> #       rstrx                               [fail] got 0 MP_RST[s] RX expected 1

See:
https://api.cirrus-ci.com/v1/artifact/task/5358215681015808/summary/summary.txt

But the error is different here because the client got an error (poll
timed out). It looks like it is similar to a known issue:

https://github.com/multipath-tcp/mptcp_net-next/issues/324

So all good, I can apply your patch!

Cheers,
Matt