diff mbox series

[net-next,v3] mptcp: Wake up MPTCP worker when DATA_FIN found on a TCP FIN packet

Message ID 20200921145759.1302197-1-matthieu.baerts@tessares.net
State Accepted
Delegated to: David Miller
Headers show
Series [net-next,v3] mptcp: Wake up MPTCP worker when DATA_FIN found on a TCP FIN packet | expand

Commit Message

Matthieu Baerts Sept. 21, 2020, 2:57 p.m. UTC 
From: Mat Martineau <mathew.j.martineau@linux.intel.com>

When receiving a DATA_FIN MPTCP option on a TCP FIN packet, the DATA_FIN
information would be stored but the MPTCP worker did not get
scheduled. In turn, the MPTCP socket state would remain in
TCP_ESTABLISHED and no blocked operations would be awakened.

TCP FIN packets are seen by the MPTCP socket when moving skbs out of the
subflow receive queues, so schedule the MPTCP worker when a skb with
DATA_FIN but no data payload is moved from a subflow queue. Other cases
(DATA_FIN on a bare TCP ACK or on a packet with data payload) are
already handled.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/84
Fixes: 43b54c6ee382 ("mptcp: Use full MPTCP-level disconnect state machine")
Acked-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---

Notes:
    This is a resend of v1 and v2 with the same code modification. The
    previous versions did not get delivered to vger.kernel.org but did go
    to other recipients. There is a known issue with the SMTP server on
    the Intel side. Sorry for the multiple versions.
    
    The only modifications compared to v1 and v2 are in the commit message:
    - Paolo's ACK is back: it was accidentaly dropped.
    - My SOB is at the end, being the new sender of this patch from Mat.

 net/mptcp/subflow.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Matthieu Baerts Sept. 21, 2020, 3:15 p.m. UTC | #1 
Hi David, Jakub,

On 21/09/2020 16:57, Matthieu Baerts wrote:
> From: Mat Martineau <mathew.j.martineau@linux.intel.com>
> 
> When receiving a DATA_FIN MPTCP option on a TCP FIN packet, the DATA_FIN
> information would be stored but the MPTCP worker did not get
> scheduled. In turn, the MPTCP socket state would remain in
> TCP_ESTABLISHED and no blocked operations would be awakened.
> 
> TCP FIN packets are seen by the MPTCP socket when moving skbs out of the
> subflow receive queues, so schedule the MPTCP worker when a skb with
> DATA_FIN but no data payload is moved from a subflow queue. Other cases
> (DATA_FIN on a bare TCP ACK or on a packet with data payload) are
> already handled.
> 
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/84
> Fixes: 43b54c6ee382 ("mptcp: Use full MPTCP-level disconnect state machine")

A small mistake from my side, this patch is for -net and not net-next :/

Sorry, with these 3 patches with the same code, it really looks like we 
don't want to have this patch sent properly :)
I can resend a v4 with the proper subject if you prefer.

Cheers,
Matt
David Miller Sept. 24, 2020, 12:31 a.m. UTC | #2 
From: Matthieu Baerts <matthieu.baerts@tessares.net>
Date: Mon, 21 Sep 2020 16:57:58 +0200

> From: Mat Martineau <mathew.j.martineau@linux.intel.com>
> 
> When receiving a DATA_FIN MPTCP option on a TCP FIN packet, the DATA_FIN
> information would be stored but the MPTCP worker did not get
> scheduled. In turn, the MPTCP socket state would remain in
> TCP_ESTABLISHED and no blocked operations would be awakened.
> 
> TCP FIN packets are seen by the MPTCP socket when moving skbs out of the
> subflow receive queues, so schedule the MPTCP worker when a skb with
> DATA_FIN but no data payload is moved from a subflow queue. Other cases
> (DATA_FIN on a bare TCP ACK or on a packet with data payload) are
> already handled.
> 
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/84
> Fixes: 43b54c6ee382 ("mptcp: Use full MPTCP-level disconnect state machine")
> Acked-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>

Applied to 'net', thanks.
diff mbox series

Patch

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 34d6230df017..31316cfeb4d0 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -733,7 +733,7 @@  static enum mapping_status get_mapping_status(struct sock *ssk,
 
 	if (mpext->data_fin == 1) {
 		if (data_len == 1) {
-			mptcp_update_rcv_data_fin(msk, mpext->data_seq);
+			bool updated = mptcp_update_rcv_data_fin(msk, mpext->data_seq);
 			pr_debug("DATA_FIN with no payload seq=%llu", mpext->data_seq);
 			if (subflow->map_valid) {
 				/* A DATA_FIN might arrive in a DSS
@@ -744,6 +744,9 @@  static enum mapping_status get_mapping_status(struct sock *ssk,
 				skb_ext_del(skb, SKB_EXT_MPTCP);
 				return MAPPING_OK;
 			} else {
+				if (updated && schedule_work(&msk->work))
+					sock_hold((struct sock *)msk);
+
 				return MAPPING_DATA_FIN;
 			}
 		} else {