Re: [PATCH mptcp-next v12 1/5] mptcp: avoid resetting when another subflow available

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 09/10/2023 08:02, Geliang Tang wrote:
> When closing the msk->first socket in __mptcp_close_ssk(), if there's
> another subflow available, it's better to avoid resetting it.
> 
> Signed-off-by: Geliang Tang <geliang.tang@suse.com>
> ---
>  net/mptcp/protocol.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 30e0c29ae0a4..6346a164ed66 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2396,7 +2396,7 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
>  		goto out_release;
>  	}
>  
> -	dispose_it = msk->free_first || ssk != msk->first;
> +	dispose_it = msk->free_first || ssk != msk->first || !list_is_singular(&msk->conn_list);
>  	if (dispose_it)
>  		list_del(&subflow->node);
>  
> @@ -2446,7 +2446,7 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
>  
>  	sock_put(ssk);
>  
> -	if (ssk == msk->first)
> +	if (ssk == msk->first && list_is_singular(&msk->conn_list))
>  		WRITE_ONCE(msk->first, NULL);

I don't think we should 'dispose it' nor set 'msk->first' to NULL but
instead, we should not call tcp_disconnect() directly: if the connection
is still in ESTABLISHED, tcp_disconnect() will send a RST if I'm not
mistaken. That's what we want to avoid, no?

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