[PATCH net-next 0/3] mptcp: Improve DATA_FIN transmission

[PATCH net-next 0/3] mptcp: Improve DATA_FIN transmission

[Mat Martineau]

MPTCP's DATA_FIN flag is sent in a DSS option when closing the
MPTCP-level connection. This patch series prepares for correct DATA_FIN
handling across multiple subflows (where individual subflows may
disconnect without closing the entire MPTCP connection) by changing the
way the MPTCP-level socket requests a DATA_FIN on a subflow and
propagates the necessary data for the TCP option.


Mat Martineau (3):
  mptcp: Check connection state before attempting send
  mptcp: Use per-subflow storage for DATA_FIN sequence number
  mptcp: Only send DATA_FIN with final mapping

 net/mptcp/options.c  | 16 ++++++++--------
 net/mptcp/protocol.c | 32 +++++++++++++++++++++++++++-----
 net/mptcp/protocol.h |  2 ++
 3 files changed, 37 insertions(+), 13 deletions(-)


base-commit: e955376277839db92774ec24d559ab42442b95fc
-- 
2.25.1

[PATCH net-next 1/3] mptcp: Check connection state before attempting send

[Mat Martineau]

MPTCP should wait for an active connection or skip sending depending on
the connection state, as TCP does. This happens before the possible
passthrough to a regular TCP sendmsg because the subflow's socket type
(MPTCP or TCP fallback) is not known until the connection is
complete. This is also relevent at disconnect time, where data should
not be sent in certain MPTCP-level connection states.

Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 net/mptcp/protocol.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index a8445407d25a..07559b45eec5 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -419,6 +419,15 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		return -EOPNOTSUPP;
 
 	lock_sock(sk);
+
+	timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
+
+	if ((1 << sk->sk_state) & ~(TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)) {
+		ret = sk_stream_wait_connect(sk, &timeo);
+		if (ret)
+			goto out;
+	}
+
 	ssock = __mptcp_tcp_fallback(msk);
 	if (unlikely(ssock)) {
 fallback:
@@ -427,8 +436,6 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		return ret >= 0 ? ret + copied : (copied ? copied : ret);
 	}
 
-	timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
-
 	ssk = mptcp_subflow_get(msk);
 	if (!ssk) {
 		release_sock(sk);
@@ -460,6 +467,7 @@ static int mptcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 
 	ssk_check_wmem(msk, ssk);
 	release_sock(ssk);
+out:
 	release_sock(sk);
 	return ret;
 }
-- 
2.25.1

[PATCH net-next 2/3] mptcp: Use per-subflow storage for DATA_FIN sequence number

[Mat Martineau]

Instead of reading the MPTCP-level sequence number when sending DATA_FIN,
store the data in the subflow so it can be safely accessed when the
subflow TCP headers are written to the packet without the MPTCP-level
lock held. This also allows the MPTCP-level socket to close individual
subflows without closing the MPTCP connection.

Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 net/mptcp/options.c  |  5 ++---
 net/mptcp/protocol.c | 20 +++++++++++++++++---
 net/mptcp/protocol.h |  2 ++
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 45acd877bef3..90c81953ec2c 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -312,7 +312,7 @@ static void mptcp_write_data_fin(struct mptcp_subflow_context *subflow,
 		 */
 		ext->use_map = 1;
 		ext->dsn64 = 1;
-		ext->data_seq = mptcp_sk(subflow->conn)->write_seq;
+		ext->data_seq = subflow->data_fin_tx_seq;
 		ext->subflow_seq = 0;
 		ext->data_len = 1;
 	} else {
@@ -354,8 +354,7 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
 		if (mpext)
 			opts->ext_copy = *mpext;
 
-		if (skb && tcp_fin &&
-		    subflow->conn->sk_state != TCP_ESTABLISHED)
+		if (skb && tcp_fin && subflow->data_fin_tx_enable)
 			mptcp_write_data_fin(subflow, &opts->ext_copy);
 		ret = true;
 	}
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 07559b45eec5..4c075a9f7ed0 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -720,7 +720,8 @@ static void mptcp_cancel_work(struct sock *sk)
 		sock_put(sk);
 }
 
-static void mptcp_subflow_shutdown(struct sock *ssk, int how)
+static void mptcp_subflow_shutdown(struct sock *ssk, int how,
+				   bool data_fin_tx_enable, u64 data_fin_tx_seq)
 {
 	lock_sock(ssk);
 
@@ -733,6 +734,14 @@ static void mptcp_subflow_shutdown(struct sock *ssk, int how)
 		tcp_disconnect(ssk, O_NONBLOCK);
 		break;
 	default:
+		if (data_fin_tx_enable) {
+			struct mptcp_subflow_context *subflow;
+
+			subflow = mptcp_subflow_ctx(ssk);
+			subflow->data_fin_tx_seq = data_fin_tx_seq;
+			subflow->data_fin_tx_enable = 1;
+		}
+
 		ssk->sk_shutdown |= how;
 		tcp_shutdown(ssk, how);
 		break;
@@ -749,6 +758,7 @@ static void mptcp_close(struct sock *sk, long timeout)
 	struct mptcp_subflow_context *subflow, *tmp;
 	struct mptcp_sock *msk = mptcp_sk(sk);
 	LIST_HEAD(conn_list);
+	u64 data_fin_tx_seq;
 
 	lock_sock(sk);
 
@@ -757,11 +767,15 @@ static void mptcp_close(struct sock *sk, long timeout)
 
 	list_splice_init(&msk->conn_list, &conn_list);
 
+	data_fin_tx_seq = msk->write_seq;
+
 	release_sock(sk);
 
 	list_for_each_entry_safe(subflow, tmp, &conn_list, node) {
 		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
 
+		subflow->data_fin_tx_seq = data_fin_tx_seq;
+		subflow->data_fin_tx_enable = 1;
 		__mptcp_close_ssk(sk, ssk, subflow, timeout);
 	}
 
@@ -854,7 +868,7 @@ static struct sock *mptcp_accept(struct sock *sk, int flags, int *err,
 			*err = -ENOBUFS;
 			local_bh_enable();
 			release_sock(sk);
-			mptcp_subflow_shutdown(newsk, SHUT_RDWR + 1);
+			mptcp_subflow_shutdown(newsk, SHUT_RDWR + 1, 0, 0);
 			tcp_close(newsk, 0);
 			return NULL;
 		}
@@ -1309,7 +1323,7 @@ static int mptcp_shutdown(struct socket *sock, int how)
 	mptcp_for_each_subflow(msk, subflow) {
 		struct sock *tcp_sk = mptcp_subflow_tcp_sock(subflow);
 
-		mptcp_subflow_shutdown(tcp_sk, how);
+		mptcp_subflow_shutdown(tcp_sk, how, 1, msk->write_seq);
 	}
 
 out_unlock:
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 6c0b2c8ab674..313558fa8185 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -125,7 +125,9 @@ struct mptcp_subflow_context {
 		mpc_map : 1,
 		data_avail : 1,
 		rx_eof : 1,
+		data_fin_tx_enable : 1,
 		can_ack : 1;	    /* only after processing the remote a key */
+	u64	data_fin_tx_seq;
 
 	struct	sock *tcp_sock;	    /* tcp sk backpointer */
 	struct	sock *conn;	    /* parent mptcp_sock */
-- 
2.25.1

[PATCH net-next 3/3] mptcp: Only send DATA_FIN with final mapping

[Mat Martineau]

When a DATA_FIN is sent in a MPTCP DSS option that contains a data
mapping, the DATA_FIN consumes one byte of space in the mapping. In this
case, the DATA_FIN should only be included in the DSS option if its
sequence number aligns with the end of the mapped data. Otherwise the
subflow can send an incorrect implicit sequence number for the DATA_FIN,
and the DATA_ACK for that sequence number would not close the
MPTCP-level connection correctly.

Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 net/mptcp/options.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 90c81953ec2c..b9a8305bd934 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -304,21 +304,22 @@ static bool mptcp_established_options_mp(struct sock *sk, struct sk_buff *skb,
 static void mptcp_write_data_fin(struct mptcp_subflow_context *subflow,
 				 struct mptcp_ext *ext)
 {
-	ext->data_fin = 1;
-
 	if (!ext->use_map) {
 		/* RFC6824 requires a DSS mapping with specific values
 		 * if DATA_FIN is set but no data payload is mapped
 		 */
+		ext->data_fin = 1;
 		ext->use_map = 1;
 		ext->dsn64 = 1;
 		ext->data_seq = subflow->data_fin_tx_seq;
 		ext->subflow_seq = 0;
 		ext->data_len = 1;
-	} else {
-		/* If there's an existing DSS mapping, DATA_FIN consumes
-		 * 1 additional byte of mapping space.
+	} else if (ext->data_seq + ext->data_len == subflow->data_fin_tx_seq) {
+		/* If there's an existing DSS mapping and it is the
+		 * final mapping, DATA_FIN consumes 1 additional byte of
+		 * mapping space.
 		 */
+		ext->data_fin = 1;
 		ext->data_len++;
 	}
 }
-- 
2.25.1

Re: [PATCH net-next 0/3] mptcp: Improve DATA_FIN transmission

[David Miller]

From: Mat Martineau <mathew.j.martineau@linux.intel.com>
Date: Fri, 28 Feb 2020 15:47:38 -0800

> MPTCP's DATA_FIN flag is sent in a DSS option when closing the
> MPTCP-level connection. This patch series prepares for correct DATA_FIN
> handling across multiple subflows (where individual subflows may
> disconnect without closing the entire MPTCP connection) by changing the
> way the MPTCP-level socket requests a DATA_FIN on a subflow and
> propagates the necessary data for the TCP option.

Series applied, thanks Mat.