[PATCH mptcp-net] Squash-to: "mptcp: fix accept vs worker race"

[PATCH mptcp-net] Squash-to: "mptcp: fix accept vs worker race"

[Paolo Abeni]

Avoid the ugly timeout hack, we can rely simple on
the in_accept_queue flag.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index c131315d833b..045570ebad96 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2387,8 +2387,6 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
 
 		/* ensure later check in mptcp_worker will dispose the msk */
 		sock_set_flag(sk, SOCK_DEAD);
-		inet_csk(sk)->icsk_mtup.probe_timestamp = tcp_jiffies32 -
-							  TCP_TIMEWAIT_LEN -1;
 	}
 
 	dispose_it = !msk->subflow || ssk != msk->subflow->sk;
@@ -2495,12 +2493,12 @@ static void __mptcp_close_subflow(struct sock *sk)
 
 }
 
-static bool mptcp_check_close_timeout(const struct sock *sk)
+static bool mptcp_should_close(const struct sock *sk)
 {
 	s32 delta = tcp_jiffies32 - inet_csk(sk)->icsk_mtup.probe_timestamp;
 	struct mptcp_subflow_context *subflow;
 
-	if (delta >= TCP_TIMEWAIT_LEN)
+	if (delta >= TCP_TIMEWAIT_LEN || mptcp_sk(sk)->in_accept_queue)
 		return true;
 
 	/* if all subflows are in closed status don't bother with additional
@@ -2723,7 +2721,7 @@ static void mptcp_worker(struct work_struct *work)
 	 * even if it is orphaned and in FIN_WAIT2 state
 	 */
 	if (sock_flag(sk, SOCK_DEAD)) {
-		if (mptcp_check_close_timeout(sk)) {
+		if (mptcp_should_close(sk)) {
 			inet_sk_state_store(sk, TCP_CLOSE);
 			mptcp_do_fastclose(sk);
 		}
-- 
2.39.2

Re: Squash-to: "mptcp: fix accept vs worker race": Tests Results

[MPTCP CI]

Hi Paolo,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6313513459646464
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6313513459646464/summary/summary.txt

- KVM Validation: normal (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/4906138576093184
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4906138576093184/summary/summary.txt

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/4764250036502528
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4764250036502528/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5469088529514496
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5469088529514496/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/4338375c43fb


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)

Re: [PATCH mptcp-net] Squash-to: "mptcp: fix accept vs worker race"

[Matthieu Baerts]

Hi Paolo,

On 06/04/2023 21:41, Paolo Abeni wrote:
> Avoid the ugly timeout hack, we can rely simple on
> the in_accept_queue flag.

Thank you for this patch!

Indeed cleaner and good idea to have renamed the function.

Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>

Just applied:

New patches for t/upstream-net and t/upstream:
- e64fe31d095e: "squashed" in "mptcp: fix accept vs worker race"
- Results: db0ea133bb6b..38f9e8a70d76 (export-net)
- Results: 930f9c85e067..016e68a14412 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20230407T092153
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230407T092153

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Re: Squash-to: "mptcp: fix accept vs worker race": Tests Results

[MPTCP CI]

Hi Paolo,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6728750830714880
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6728750830714880/summary/summary.txt

- KVM Validation: normal (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/4529727575162880
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/4529727575162880/summary/summary.txt

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5655627482005504
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5655627482005504/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5092677528584192
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5092677528584192/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/f3b6a3c53df1


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-debug

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)