[MPTCP] [PATCH v2 1/7] Squash-to: "mptcp: Handle MP_CAPABLE options for outgoing connections"

[MPTCP] [PATCH v2 1/7] Squash-to: "mptcp: Handle MP_CAPABLE options for outgoing connections"

[Paolo Abeni]

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

After upstream feedback, I just noticed that we actually
don't clear the is_mptcp field on clone allocation failure.

Do that and additionally don't allocate at all the subflow if MPC handshake
failed.

Note: we need to update accordingly accept on later patch

v1  -> v2:
 - use sk_is_mptcp() in __mptcp_tcp_fallback() (Mat)

RFC -> v1:
 - avoid assign statement inside the if statement (Florian)
 - avoid rendundant  conn = NULL  (Mat)
 - clear ulp data on fallback, or to avoid double free at close
   time

Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
---
 net/mptcp/protocol.c |  2 +-
 net/mptcp/subflow.c  | 33 +++++++++++++++++++++------------
 2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 8bb3ecf87ee5..9120f75659b5 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -42,7 +42,7 @@ static struct socket *__mptcp_tcp_fallback(const struct mptcp_sock *msk)
 
 	sock_owned_by_me((const struct sock *)msk);
 
-	if (!ssock || tcp_sk(ssock->sk)->is_mptcp)
+	if (!ssock || sk_is_mptcp(ssock->sk))
 		return NULL;
 
 	return ssock;
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index b9aca17b0b91..cf4c05a6cfac 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -254,6 +254,15 @@ static void subflow_ulp_release(struct sock *sk)
 	kfree_rcu(ctx, rcu);
 }
 
+static void subflow_ulp_fallback(struct sock *sk)
+{
+	struct inet_connection_sock *icsk = inet_csk(sk);
+
+	icsk->icsk_ulp_ops = NULL;
+	rcu_assign_pointer(icsk->icsk_ulp_data, NULL);
+	tcp_sk(sk)->is_mptcp = 0;
+}
+
 static void subflow_ulp_clone(const struct request_sock *req,
 			      struct sock *newsk,
 			      const gfp_t priority)
@@ -262,23 +271,23 @@ static void subflow_ulp_clone(const struct request_sock *req,
 	struct mptcp_subflow_context *old_ctx = mptcp_subflow_ctx(newsk);
 	struct mptcp_subflow_context *new_ctx;
 
-	/* newsk->sk_socket is NULL at this point */
+	if (!subflow_req->mp_capable) {
+		subflow_ulp_fallback(newsk);
+		return;
+	}
+
 	new_ctx = subflow_create_ctx(newsk, priority);
-	if (!new_ctx)
+	if (new_ctx == NULL) {
+		subflow_ulp_fallback(newsk);
 		return;
+	}
 
-	new_ctx->conn = NULL;
 	new_ctx->conn_finished = 1;
 	new_ctx->icsk_af_ops = old_ctx->icsk_af_ops;
-
-	if (subflow_req->mp_capable) {
-		new_ctx->mp_capable = 1;
-		new_ctx->fourth_ack = 1;
-		new_ctx->remote_key = subflow_req->remote_key;
-		new_ctx->local_key = subflow_req->local_key;
-	} else {
-		tcp_sk(newsk)->is_mptcp = 0;
-	}
+	new_ctx->mp_capable = 1;
+	new_ctx->fourth_ack = 1;
+	new_ctx->remote_key = subflow_req->remote_key;
+	new_ctx->local_key = subflow_req->local_key;
 }
 
 static struct tcp_ulp_ops subflow_ulp_ops __read_mostly = {
-- 
2.21.0