[PATCH mptcp-net v2 5/6] selftests: mptcp: join: fastclose: drop plain RST

Matthieu Baerts (NGI0) posted 6 patches 3 weeks, 4 days ago
[PATCH mptcp-net v2 5/6] selftests: mptcp: join: fastclose: drop plain RST
Posted by Matthieu Baerts (NGI0) 3 weeks, 4 days ago
These tests are sending data without any throttling. Quickly, one side
will shutdown the connection, which will force sending a RST with
MP_FASTCLOSE.

On the kernel side, the following path will be taken:

  mptcp_close
  --> __mptcp_close
    --> mptcp_do_fastclose
      --> __mptcp_close_ssk
        --> tcp_disconnect
          --> tcp_send_active_reset

The subflow will be closed and the reset containing the MP_FASTCLOSE
will be queued to be sent. If some data are received at the same time,
it is possible a "plain" reset -- without ACK -- is sent from
tcp_v[46]_send_reset, because no socket has been found for this packet
(no_tcp_socket). In some cases, this "plain" reset can be sent before
the one with the MP_FASTCLOSE. When this happens, the receiver will
first close the connection upon the first RST reception, and then drop
the second one because the connection has already been closed. In this
case, the connection has been correctly reset, but the MPFastcloseRx and
MPRstRx MIB counters have not been incremented, which is not what the
test expects.

To solve this issue, the "plain" reset are now dropped to make sure the
one with the MP_FASTCLOSE is parsed by the receiver.

Note that this instability seems to be more visible since commit
8cc6e542f150 ("mptcp: propagate shutdown to subflows when possible"),
certainly because the subflows are closed slightly quicker.

Another solution is also possible: reset the connection with no ongoing
data. But this means more changes: data being sent, probably set
SO_LINGER, etc.

With this small modification, the test should no longer be flaky. It is
then OK to remove the MPTCP_LIB_SUBTEST_FLAKY mark.

Fixes: 01542c9bf9ab ("selftests: mptcp: add fastclose testcase")
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 1af96a63516c..0b084c502122 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -335,6 +335,23 @@ reset_check_counter()
 	fi
 }
 
+# $1: test name ; $2: counter to check
+reset_fastclose()
+{
+	reset_check_counter "${1}" "${2}" || return 1
+
+	local netns
+	for netns in "$ns1" "$ns2"; do
+		# Drop "plain" RST sent because the connection is closed
+		if ! ip netns exec "${netns}" "${iptables}" -A OUTPUT -p tcp \
+				--tcp-flags ALL RST \
+				-j DROP; then
+			mark_as_skipped "unable to set the 'fastclose' rule"
+			return 1
+		fi
+	done
+}
+
 # $1: test name
 reset_with_cookies()
 {
@@ -3650,8 +3667,7 @@ fullmesh_tests()
 
 fastclose_tests()
 {
-	if reset_check_counter "fastclose test" "MPTcpExtMPFastcloseTx"; then
-		MPTCP_LIB_SUBTEST_FLAKY=1
+	if reset_fastclose "fastclose test" "MPTcpExtMPFastcloseTx"; then
 		test_linkfail=1024 fastclose=client \
 			run_tests $ns1 $ns2 10.0.1.1
 		chk_join_nr 0 0 0
@@ -3659,8 +3675,7 @@ fastclose_tests()
 		chk_rst_nr 1 1 invert
 	fi
 
-	if reset_check_counter "fastclose server test" "MPTcpExtMPFastcloseRx"; then
-		MPTCP_LIB_SUBTEST_FLAKY=1
+	if reset_fastclose "fastclose server test" "MPTcpExtMPFastcloseRx"; then
 		test_linkfail=1024 fastclose=server \
 			run_tests $ns1 $ns2 10.0.1.1
 		join_rst_nr=1 \

-- 
2.51.0
Re: [PATCH mptcp-net v2 5/6] selftests: mptcp: join: fastclose: drop plain RST
Posted by Geliang Tang 3 weeks, 3 days ago
Hi Matt,

On Sun, 2025-11-02 at 12:30 +0100, Matthieu Baerts (NGI0) wrote:
> These tests are sending data without any throttling. Quickly, one
> side
> will shutdown the connection, which will force sending a RST with
> MP_FASTCLOSE.
> 
> On the kernel side, the following path will be taken:
> 
>   mptcp_close
>   --> __mptcp_close
>     --> mptcp_do_fastclose
>       --> __mptcp_close_ssk
>         --> tcp_disconnect
>           --> tcp_send_active_reset
> 
> The subflow will be closed and the reset containing the MP_FASTCLOSE
> will be queued to be sent. If some data are received at the same
> time,
> it is possible a "plain" reset -- without ACK -- is sent from
> tcp_v[46]_send_reset, because no socket has been found for this
> packet
> (no_tcp_socket). In some cases, this "plain" reset can be sent before
> the one with the MP_FASTCLOSE. When this happens, the receiver will
> first close the connection upon the first RST reception, and then
> drop
> the second one because the connection has already been closed. In
> this
> case, the connection has been correctly reset, but the MPFastcloseRx
> and
> MPRstRx MIB counters have not been incremented, which is not what the
> test expects.
> 
> To solve this issue, the "plain" reset are now dropped to make sure
> the
> one with the MP_FASTCLOSE is parsed by the receiver.
> 
> Note that this instability seems to be more visible since commit
> 8cc6e542f150 ("mptcp: propagate shutdown to subflows when possible"),
> certainly because the subflows are closed slightly quicker.
> 
> Another solution is also possible: reset the connection with no
> ongoing
> data. But this means more changes: data being sent, probably set
> SO_LINGER, etc.
> 
> With this small modification, the test should no longer be flaky. It
> is
> then OK to remove the MPTCP_LIB_SUBTEST_FLAKY mark.
> 
> Fixes: 01542c9bf9ab ("selftests: mptcp: add fastclose testcase")
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

This patch looks good to me.

    Reviewed-by: Geliang Tang <geliang@kernel.org>
    Tested-by: Geliang Tang <geliang@kernel.org>

Thanks,
-Geliang

> ---
>  tools/testing/selftests/net/mptcp/mptcp_join.sh | 23
> +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index 1af96a63516c..0b084c502122 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -335,6 +335,23 @@ reset_check_counter()
>  	fi
>  }
>  
> +# $1: test name ; $2: counter to check
> +reset_fastclose()
> +{
> +	reset_check_counter "${1}" "${2}" || return 1
> +
> +	local netns
> +	for netns in "$ns1" "$ns2"; do
> +		# Drop "plain" RST sent because the connection is
> closed
> +		if ! ip netns exec "${netns}" "${iptables}" -A
> OUTPUT -p tcp \
> +				--tcp-flags ALL RST \
> +				-j DROP; then
> +			mark_as_skipped "unable to set the
> 'fastclose' rule"
> +			return 1
> +		fi
> +	done
> +}
> +
>  # $1: test name
>  reset_with_cookies()
>  {
> @@ -3650,8 +3667,7 @@ fullmesh_tests()
>  
>  fastclose_tests()
>  {
> -	if reset_check_counter "fastclose test"
> "MPTcpExtMPFastcloseTx"; then
> -		MPTCP_LIB_SUBTEST_FLAKY=1
> +	if reset_fastclose "fastclose test" "MPTcpExtMPFastcloseTx";
> then
>  		test_linkfail=1024 fastclose=client \
>  			run_tests $ns1 $ns2 10.0.1.1
>  		chk_join_nr 0 0 0
> @@ -3659,8 +3675,7 @@ fastclose_tests()
>  		chk_rst_nr 1 1 invert
>  	fi
>  
> -	if reset_check_counter "fastclose server test"
> "MPTcpExtMPFastcloseRx"; then
> -		MPTCP_LIB_SUBTEST_FLAKY=1
> +	if reset_fastclose "fastclose server test"
> "MPTcpExtMPFastcloseRx"; then
>  		test_linkfail=1024 fastclose=server \
>  			run_tests $ns1 $ns2 10.0.1.1
>  		join_rst_nr=1 \
>