While tacking care of the mptcp-level listener I unintentionally
moved the subflow level unhash after the subflow listener backlog
cleanup.

That could cause some nasty race and makes the code harder to read.

Address the issue restoring the proper order of operations.

Fixes: 57fc0f1ceaa4 ("mptcp: ensure listener is unhashed before updating the sk status")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index f2abae254524..03a7c83bc2d2 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2957,10 +2957,10 @@ static void mptcp_check_listen_stop(struct sock *sk)
 		return;
 
 	lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
+	tcp_set_state(ssk, TCP_CLOSE);
 	mptcp_subflow_queue_clean(sk, ssk);
 	inet_csk_listen_stop(ssk);
 	mptcp_event_pm_listener(ssk, MPTCP_EVENT_LISTENER_CLOSED);
-	tcp_set_state(ssk, TCP_CLOSE);
 	release_sock(ssk);
 }
 
-- 
2.40.1

Hi Paolo,

On 23/06/2023 16:51, Paolo Abeni wrote:
> While tacking care of the mptcp-level listener I unintentionally
> moved the subflow level unhash after the subflow listener backlog
> cleanup.
> 
> That could cause some nasty race and makes the code harder to read.
> 
> Address the issue restoring the proper order of operations.
> 
> Fixes: 57fc0f1ceaa4 ("mptcp: ensure listener is unhashed before updating the sk status")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Thank you for the patch!

Now in our tree (fixes for -net):

New patches for t/upstream-net and t/upstream:
- 61f0a42bb0c0: mptcp: ensure subflow is unhashed before cleaning the
backlog
- Results: d3dabc07c83c..4303dba6139f (export-net)
- Results: cc9edd912a76..3b62cbd7cc66 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20230626T134743
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230626T134743

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

Hi Paolo,

On 23/06/2023 16:51, Paolo Abeni wrote:
> While tacking care of the mptcp-level listener I unintentionally
> moved the subflow level unhash after the subflow listener backlog
> cleanup.
> 
> That could cause some nasty race and makes the code harder to read.
> 
> Address the issue restoring the proper order of operations.

Thank you for this fix!

I was surprised to see the CI complaining about the "fastclose server
test" test, with *and* without the debug kernel config. But I don't see
the link with that, it looks like this test is not stable. I just updated:

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

The patch looks then good to me!

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

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

On Mon, 2023-06-26 at 15:20 +0200, Matthieu Baerts wrote:
> Hi Paolo,
> 
> On 23/06/2023 16:51, Paolo Abeni wrote:
> > While tacking care of the mptcp-level listener I unintentionally
> > moved the subflow level unhash after the subflow listener backlog
> > cleanup.
> > 
> > That could cause some nasty race and makes the code harder to read.
> > 
> > Address the issue restoring the proper order of operations.
> 
> Thank you for this fix!
> 
> I was surprised to see the CI complaining about the "fastclose server
> test" test, with *and* without the debug kernel config. 

Same here!

On the plus side, it's more job security ;)

I'll try to have a look when time allows

/P