[MPTCP] Re: [PATCH mptcp-net] mptcp: fix DATA_FIN processing for orphaned sockets.

[MPTCP] Re: [PATCH mptcp-net] mptcp: fix DATA_FIN processing for orphaned sockets.

[Mat Martineau]

[-- Attachment #1: Type: text/plain, Size: 2026 bytes --]

On Mon, 15 Feb 2021, Paolo Abeni wrote:

> Currently we move orphaned msk socket directly from FIN_WAIT2
> state to close, with the rationale that incoming additional
> data could be just dropped by the TCP stack/TW sockets.
>
> Anyhow we miss sending MPTCP-level ack on incoming DATA_FIN,
> and that may hang the peers.
>
> Fixes: e16163b6e2b7 ("mptcp: refactor shutdown and close")
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
> ---
> I hope nobody is keeping track of nr of follow-ups x commit,
> because the referenced one could score a world record :(((
> ---
> net/mptcp/protocol.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>

Thanks for catching this. Fix looks ready for the export branch, and -net 
(if tests are good). Also a candidate for 5.11-stable?


Mat


> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index ee5a2981df6da..f15c79ff6a53c 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2297,13 +2297,12 @@ static void mptcp_worker(struct work_struct *work)
> 	__mptcp_check_send_data_fin(sk);
> 	mptcp_check_data_fin(sk);
>
> -	/* if the msk data is completely acked, or the socket timedout,
> -	 * there is no point in keeping around an orphaned sk
> +	/* There is no point in keeping aroud an orphaned sk timedout or closed,
> +	 * but we need the msk around to reply to incoming DATA_FIN, even if
> +	 * orphaned and in FIN_WAIT2 state
> 	 */
> 	if (sock_flag(sk, SOCK_DEAD) &&
> -	    (mptcp_check_close_timeout(sk) ||
> -	    (state != sk->sk_state &&
> -	    ((1 << inet_sk_state_load(sk)) & (TCPF_CLOSE | TCPF_FIN_WAIT2))))) {
> +	    (mptcp_check_close_timeout(sk) || sk->sk_state == TCP_CLOSE)) {
> 		inet_sk_state_store(sk, TCP_CLOSE);
> 		__mptcp_destroy_sock(sk);
> 		goto unlock;
> -- 
> 2.26.2
> _______________________________________________
> mptcp mailing list -- mptcp(a)lists.01.org
> To unsubscribe send an email to mptcp-leave(a)lists.01.org
>

--
Mat Martineau
Intel

[MPTCP] Re: [PATCH mptcp-net] mptcp: fix DATA_FIN processing for orphaned sockets.

[Paolo Abeni]

[-- Attachment #1: Type: text/plain, Size: 1059 bytes --]

On Tue, 2021-02-16 at 18:05 -0800, Mat Martineau wrote:
> On Mon, 15 Feb 2021, Paolo Abeni wrote:
> 
> > Currently we move orphaned msk socket directly from FIN_WAIT2
> > state to close, with the rationale that incoming additional
> > data could be just dropped by the TCP stack/TW sockets.
> > 
> > Anyhow we miss sending MPTCP-level ack on incoming DATA_FIN,
> > and that may hang the peers.
> > 
> > Fixes: e16163b6e2b7 ("mptcp: refactor shutdown and close")
> > Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
> > ---
> > I hope nobody is keeping track of nr of follow-ups x commit,
> > because the referenced one could score a world record :(((
> > ---
> > net/mptcp/protocol.c | 9 ++++-----
> > 1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> 
> Thanks for catching this. Fix looks ready for the export branch, and -net 
> (if tests are good). Also a candidate for 5.11-stable?

I guess so, since the fixed commit in is since 5.11-rc1. I usually let
the fix be propagated according to the fixes tag.

Cheers,

Paolo

[MPTCP] Re: [PATCH mptcp-net] mptcp: fix DATA_FIN processing for orphaned sockets.

[Matthieu Baerts]

[-- Attachment #1: Type: text/plain, Size: 929 bytes --]

Hi Paolo, Mat,

On 15/02/2021 15:14, Paolo Abeni wrote:
> Currently we move orphaned msk socket directly from FIN_WAIT2
> state to close, with the rationale that incoming additional
> data could be just dropped by the TCP stack/TW sockets.
> 
> Anyhow we miss sending MPTCP-level ack on incoming DATA_FIN,
> and that may hang the peers.
> 
> Fixes: e16163b6e2b7 ("mptcp: refactor shutdown and close")
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>

Thank you for the patch and the review!

Just applied in our tree with Mat's RvB tag, without the dot at the end 
of the commit title and typos spotted by codespell.
@Paolo: please use this version if you send that to netdev :)

- 97e35721a7af: mptcp: fix DATA_FIN processing for orphaned sockets
- Results: 230a407eae19..afc69f9d3c92

Tests + export have been queued!

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