[PATCH mptcp-next v9 0/6] mptcp: fixes and enhancements related to path management

[PATCH mptcp-next v9 0/6] mptcp: fixes and enhancements related to path management

[Kishen Maloor]

This patch series incorporates fixes and enhancements related to
path management over MPTCP connections, particularly in support of
userspace PMs. The changes ensure that the required bits of
information are conveyed through MPTCP netlink events which
would be consumed by the path manager in making its decisions.
Further it adds flexibility in establishing paths and from
both ends of MPTCP connections.

v6:
-Dropped framework and functionality around the use of kernel
listening sockets for receiving MPJs.

v7:
-Fixed bug caught by the CI build.

v8:
-Fixed logic to achieve the desired outcome of not responding to
address advertisements when a userspace PM is offline.
-Factored out minor changes to mptcp_pm_is_userspace() to be
squashed into an earlier commit.

v9:
-Removed an explicit 'struct inet_sock *' cast in a patch which was
not necessary.

Kishen Maloor (6):
  Squash-to: mptcp: Bypass kernel PM when userspace PM is enabled
  mptcp: bypass in-kernel PM restrictions for non-kernel PMs
  mptcp: store remote id from MP_JOIN SYN/ACK in local ctx
  mptcp: reflect remote port (not 0) in ANNOUNCED events
  mptcp: establish subflows from either end of connection
  mptcp: expose server_side attribute in MPTCP netlink events

 include/uapi/linux/mptcp.h |  1 +
 net/mptcp/options.c        |  4 ++--
 net/mptcp/pm.c             | 21 +++++++++++++++++----
 net/mptcp/pm_netlink.c     | 24 +++++++++++++++++++++---
 net/mptcp/protocol.c       |  5 +----
 net/mptcp/protocol.h       | 22 ++++++++++++++++------
 net/mptcp/subflow.c        |  5 ++++-
 7 files changed, 62 insertions(+), 20 deletions(-)


base-commit: 8e1fca01ac83e646133bd647abdae74d6d6449ba
--
2.31.1

[PATCH mptcp-next v9 1/6] Squash-to: mptcp: Bypass kernel PM when userspace PM is enabled

[Kishen Maloor]

Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
---
 net/mptcp/protocol.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index fd82fd113113..54d2b3b2d100 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -806,9 +806,9 @@ static inline bool mptcp_pm_should_rm_signal(struct mptcp_sock *msk)
 	return READ_ONCE(msk->pm.addr_signal) & BIT(MPTCP_RM_ADDR_SIGNAL);
 }
 
-static inline bool mptcp_pm_is_userspace(struct mptcp_sock *msk)
+static inline bool mptcp_pm_is_userspace(const struct mptcp_sock *msk)
 {
-	return READ_ONCE(msk->pm.pm_type) != MPTCP_PM_TYPE_KERNEL;
+	return READ_ONCE(msk->pm.pm_type) == MPTCP_PM_TYPE_USERSPACE;
 }
 
 static inline unsigned int mptcp_add_addr_len(int family, bool echo, bool port)
-- 
2.31.1

[PATCH mptcp-next v9 2/6] mptcp: bypass in-kernel PM restrictions for non-kernel PMs

[Kishen Maloor]

Current limits on the # of addresses/subflows must apply only to
in-kernel PM managed sockets. Thus this change removes such
restrictions on connections overseen by non-kernel (e.g. userspace)
PMs. This change also ensures that the kernel does not record stats
inside struct mptcp_pm_data updated along kernel code paths when exercised
via non-kernel PMs.

Additionally, address announcements are acknolwedged and subflow
requests are honored only when it's deemed that	a userspace path
manager	is active at the time.

Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
---
 net/mptcp/pm.c         | 15 +++++++++++++--
 net/mptcp/pm_netlink.c | 10 ++++++++++
 net/mptcp/protocol.h   |  6 ++++++
 net/mptcp/subflow.c    |  4 +++-
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 57f67578a47f..8df9cb28d970 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -87,6 +87,9 @@ bool mptcp_pm_allow_new_subflow(struct mptcp_sock *msk)
 	unsigned int subflows_max;
 	int ret = 0;
 
+	if (mptcp_pm_is_userspace(msk))
+		return mptcp_userspace_pm_active(msk);
+
 	subflows_max = mptcp_pm_get_subflows_max(msk);
 
 	pr_debug("msk=%p subflows=%d max=%d allow=%d", msk, pm->subflows,
@@ -179,7 +182,8 @@ void mptcp_pm_subflow_check_next(struct mptcp_sock *msk, const struct sock *ssk,
 	bool update_subflows;
 
 	update_subflows = (ssk->sk_state == TCP_CLOSE) &&
-			  (subflow->request_join || subflow->mp_join);
+			  (subflow->request_join || subflow->mp_join) &&
+			  mptcp_pm_is_kernel(msk);
 	if (!READ_ONCE(pm->work_pending) && !update_subflows)
 		return;
 
@@ -208,7 +212,14 @@ void mptcp_pm_add_addr_received(struct mptcp_sock *msk,
 
 	spin_lock_bh(&pm->lock);
 
-	if (!READ_ONCE(pm->accept_addr) || mptcp_pm_is_userspace(msk)) {
+	if (mptcp_pm_is_userspace(msk)) {
+		if (mptcp_userspace_pm_active(msk)) {
+			mptcp_pm_announce_addr(msk, addr, true);
+			mptcp_pm_add_addr_send_ack(msk);
+		} else {
+			__MPTCP_INC_STATS(sock_net((struct sock *)msk), MPTCP_MIB_ADDADDRDROP);
+		}
+	} else if (!READ_ONCE(pm->accept_addr)) {
 		mptcp_pm_announce_addr(msk, addr, true);
 		mptcp_pm_add_addr_send_ack(msk);
 	} else if (mptcp_pm_schedule_work(msk, MPTCP_PM_ADD_ADDR_RECEIVED)) {
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 10a73898c8db..491aedc486f1 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -798,6 +798,9 @@ static void mptcp_pm_nl_rm_addr_or_subflow(struct mptcp_sock *msk,
 		if (!removed)
 			continue;
 
+		if (!mptcp_pm_is_kernel(msk))
+			continue;
+
 		if (rm_type == MPTCP_MIB_RMADDR) {
 			msk->pm.add_addr_accepted--;
 			WRITE_ONCE(msk->pm.accept_addr, true);
@@ -1848,6 +1851,13 @@ static void mptcp_nl_mcast_send(struct net *net, struct sk_buff *nlskb, gfp_t gf
 				nlskb, 0, MPTCP_PM_EV_GRP_OFFSET, gfp);
 }
 
+bool mptcp_userspace_pm_active(const struct mptcp_sock *msk)
+{
+	return genl_has_listeners(&mptcp_genl_family,
+				  sock_net((const struct sock *)msk),
+				  MPTCP_PM_EV_GRP_OFFSET);
+}
+
 static int mptcp_event_add_subflow(struct sk_buff *skb, const struct sock *ssk)
 {
 	const struct inet_sock *issk = inet_sk(ssk);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 54d2b3b2d100..85390146944d 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -784,6 +784,7 @@ void mptcp_event(enum mptcp_event_type type, const struct mptcp_sock *msk,
 		 const struct sock *ssk, gfp_t gfp);
 void mptcp_event_addr_announced(const struct mptcp_sock *msk, const struct mptcp_addr_info *info);
 void mptcp_event_addr_removed(const struct mptcp_sock *msk, u8 id);
+bool mptcp_userspace_pm_active(const struct mptcp_sock *msk);
 
 static inline bool mptcp_pm_should_add_signal(struct mptcp_sock *msk)
 {
@@ -811,6 +812,11 @@ static inline bool mptcp_pm_is_userspace(const struct mptcp_sock *msk)
 	return READ_ONCE(msk->pm.pm_type) == MPTCP_PM_TYPE_USERSPACE;
 }
 
+static inline bool mptcp_pm_is_kernel(const struct mptcp_sock *msk)
+{
+	return READ_ONCE(msk->pm.pm_type) == MPTCP_PM_TYPE_KERNEL;
+}
+
 static inline unsigned int mptcp_add_addr_len(int family, bool echo, bool port)
 {
 	u8 len = TCPOLEN_MPTCP_ADD_ADDR_BASE;
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 75c824b67ca9..9567231a4bfa 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -62,7 +62,9 @@ static void subflow_generate_hmac(u64 key1, u64 key2, u32 nonce1, u32 nonce2,
 static bool mptcp_can_accept_new_subflow(const struct mptcp_sock *msk)
 {
 	return mptcp_is_fully_established((void *)msk) &&
-	       READ_ONCE(msk->pm.accept_subflow);
+		((mptcp_pm_is_userspace(msk) &&
+		  mptcp_userspace_pm_active(msk)) ||
+		 READ_ONCE(msk->pm.accept_subflow));
 }
 
 /* validate received token and create truncated hmac and nonce for SYN-ACK */
-- 
2.31.1

[PATCH mptcp-next v9 3/6] mptcp: store remote id from MP_JOIN SYN/ACK in local ctx

[Kishen Maloor]

This change reads the addr id assigned to the remote endpoint
of a subflow from the MP_JOIN SYN/ACK message and stores it
in the related subflow context. The remote id was not being
captured prior to this change, and will now provide a consistent
view of remote endpoints and their ids as seen through netlink
events.

Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
---
 net/mptcp/subflow.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 9567231a4bfa..a0e7af33fb26 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -443,6 +443,7 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
 		subflow->backup = mp_opt.backup;
 		subflow->thmac = mp_opt.thmac;
 		subflow->remote_nonce = mp_opt.nonce;
+		subflow->remote_id = mp_opt.join_id;
 		pr_debug("subflow=%p, thmac=%llu, remote_nonce=%u backup=%d",
 			 subflow, subflow->thmac, subflow->remote_nonce,
 			 subflow->backup);
-- 
2.31.1

[PATCH mptcp-next v9 4/6] mptcp: reflect remote port (not 0) in ANNOUNCED events

[Kishen Maloor]

Per RFC 8684, if no port is specified in an ADD_ADDR message, MPTCP
SHOULD attempt to connect to the specified address on the same port
as the port that is already in use by the subflow on which the
ADD_ADDR signal was sent.

To facilitate that, this change reflects the specific remote port in
use by that subflow in MPTCP_EVENT_ANNOUNCED events.

Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
---
v9: removed explicit 'struct inet_sock *' cast.
---
 net/mptcp/options.c    |  2 +-
 net/mptcp/pm.c         |  6 ++++--
 net/mptcp/pm_netlink.c | 11 ++++++++---
 net/mptcp/protocol.h   |  4 ++--
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 88f4ebbd6515..c9625fea3ef9 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -1133,7 +1133,7 @@ bool mptcp_incoming_options(struct sock *sk, struct sk_buff *skb)
 		if ((mp_opt.suboptions & OPTION_MPTCP_ADD_ADDR) &&
 		    add_addr_hmac_valid(msk, &mp_opt)) {
 			if (!mp_opt.echo) {
-				mptcp_pm_add_addr_received(msk, &mp_opt.addr);
+				mptcp_pm_add_addr_received(sk, &mp_opt.addr);
 				MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ADDADDR);
 			} else {
 				mptcp_pm_add_addr_echoed(msk, &mp_opt.addr);
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 8df9cb28d970..5d6832c4d9f2 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -200,15 +200,17 @@ void mptcp_pm_subflow_check_next(struct mptcp_sock *msk, const struct sock *ssk,
 	spin_unlock_bh(&pm->lock);
 }
 
-void mptcp_pm_add_addr_received(struct mptcp_sock *msk,
+void mptcp_pm_add_addr_received(const struct sock *ssk,
 				const struct mptcp_addr_info *addr)
 {
+	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
+	struct mptcp_sock *msk = mptcp_sk(subflow->conn);
 	struct mptcp_pm_data *pm = &msk->pm;
 
 	pr_debug("msk=%p remote_id=%d accept=%d", msk, addr->id,
 		 READ_ONCE(pm->accept_addr));
 
-	mptcp_event_addr_announced(msk, addr);
+	mptcp_event_addr_announced(ssk, addr);
 
 	spin_lock_bh(&pm->lock);
 
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 491aedc486f1..6b555b20fd6f 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -2012,10 +2012,12 @@ void mptcp_event_addr_removed(const struct mptcp_sock *msk, uint8_t id)
 	kfree_skb(skb);
 }
 
-void mptcp_event_addr_announced(const struct mptcp_sock *msk,
+void mptcp_event_addr_announced(const struct sock *ssk,
 				const struct mptcp_addr_info *info)
 {
-	struct net *net = sock_net((const struct sock *)msk);
+	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
+	struct mptcp_sock *msk = mptcp_sk(subflow->conn);
+	struct net *net = sock_net(ssk);
 	struct nlmsghdr *nlh;
 	struct sk_buff *skb;
 
@@ -2037,7 +2039,10 @@ void mptcp_event_addr_announced(const struct mptcp_sock *msk,
 	if (nla_put_u8(skb, MPTCP_ATTR_REM_ID, info->id))
 		goto nla_put_failure;
 
-	if (nla_put_be16(skb, MPTCP_ATTR_DPORT, info->port))
+	if (nla_put_be16(skb, MPTCP_ATTR_DPORT,
+			 info->port == 0 ?
+			 inet_sk(ssk)->inet_dport :
+			 info->port))
 		goto nla_put_failure;
 
 	switch (info->family) {
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 85390146944d..a762b789f5ab 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -753,7 +753,7 @@ void mptcp_pm_subflow_established(struct mptcp_sock *msk);
 bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk);
 void mptcp_pm_subflow_check_next(struct mptcp_sock *msk, const struct sock *ssk,
 				 const struct mptcp_subflow_context *subflow);
-void mptcp_pm_add_addr_received(struct mptcp_sock *msk,
+void mptcp_pm_add_addr_received(const struct sock *ssk,
 				const struct mptcp_addr_info *addr);
 void mptcp_pm_add_addr_echoed(struct mptcp_sock *msk,
 			      const struct mptcp_addr_info *addr);
@@ -782,7 +782,7 @@ int mptcp_pm_remove_subflow(struct mptcp_sock *msk, const struct mptcp_rm_list *
 
 void mptcp_event(enum mptcp_event_type type, const struct mptcp_sock *msk,
 		 const struct sock *ssk, gfp_t gfp);
-void mptcp_event_addr_announced(const struct mptcp_sock *msk, const struct mptcp_addr_info *info);
+void mptcp_event_addr_announced(const struct sock *ssk, const struct mptcp_addr_info *info);
 void mptcp_event_addr_removed(const struct mptcp_sock *msk, u8 id);
 bool mptcp_userspace_pm_active(const struct mptcp_sock *msk);
 
-- 
2.31.1

[PATCH mptcp-next v9 5/6] mptcp: establish subflows from either end of connection

[Kishen Maloor]

This change updates internal logic to permit subflows to be
established from either the client or server ends of MPTCP
connections. This symmetry and added flexibility may be
harnessed by PM implementations running on either end in
creating new subflows.

The essence of this change lies in not relying on the
"server_side" flag (which continues to be available if needed).

Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
---
 net/mptcp/options.c  | 2 +-
 net/mptcp/protocol.c | 5 +----
 net/mptcp/protocol.h | 8 ++++++--
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index c9625fea3ef9..e05d9458a025 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -931,7 +931,7 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
 		if (TCP_SKB_CB(skb)->seq == subflow->ssn_offset + 1 &&
 		    TCP_SKB_CB(skb)->end_seq == TCP_SKB_CB(skb)->seq &&
 		    subflow->mp_join && (mp_opt->suboptions & OPTIONS_MPTCP_MPJ) &&
-		    READ_ONCE(msk->pm.server_side))
+		    !subflow->request_join)
 			tcp_send_ack(ssk);
 		goto fully_established;
 	}
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index d3887f628b54..b2c654992de0 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3327,15 +3327,12 @@ bool mptcp_finish_join(struct sock *ssk)
 		return false;
 	}
 
-	if (!msk->pm.server_side)
+	if (!list_empty(&subflow->node))
 		goto out;
 
 	if (!mptcp_pm_allow_new_subflow(msk))
 		goto err_prohibited;
 
-	if (WARN_ON_ONCE(!list_empty(&subflow->node)))
-		goto err_prohibited;
-
 	/* active connections are already on conn_list.
 	 * If we can't acquire msk socket lock here, let the release callback
 	 * handle it
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index a762b789f5ab..187c932deef0 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -911,13 +911,17 @@ static inline bool mptcp_check_infinite_map(struct sk_buff *skb)
 	return false;
 }
 
+static inline bool is_active_ssk(struct mptcp_subflow_context *subflow)
+{
+	return (subflow->request_mptcp || subflow->request_join);
+}
+
 static inline bool subflow_simultaneous_connect(struct sock *sk)
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
-	struct sock *parent = subflow->conn;
 
 	return sk->sk_state == TCP_ESTABLISHED &&
-	       !mptcp_sk(parent)->pm.server_side &&
+	       is_active_ssk(subflow) &&
 	       !subflow->conn_finished;
 }
 
-- 
2.31.1

[PATCH mptcp-next v9 6/6] mptcp: expose server_side attribute in MPTCP netlink events

[Kishen Maloor]

This change records the 'server_side' attribute of MPTCP_EVENT_CREATED
and MPTCP_EVENT_ESTABLISHED events to inform their recipient about the
Client/Server role of the running MPTCP application.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/246
Signed-off-by: Kishen Maloor <kishen.maloor@intel.com>
---
 include/uapi/linux/mptcp.h | 1 +
 net/mptcp/pm_netlink.c     | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index 9690efedb5fa..e41ea01a94bb 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -188,6 +188,7 @@ enum mptcp_event_attr {
 	MPTCP_ATTR_IF_IDX,	/* s32 */
 	MPTCP_ATTR_RESET_REASON,/* u32 */
 	MPTCP_ATTR_RESET_FLAGS, /* u32 */
+	MPTCP_ATTR_SERVER_SIDE,	/* u8 */
 
 	__MPTCP_ATTR_AFTER_LAST
 };
diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 6b555b20fd6f..7c92af2ad900 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1978,6 +1978,9 @@ static int mptcp_event_created(struct sk_buff *skb,
 	if (err)
 		return err;
 
+	if (nla_put_u8(skb, MPTCP_ATTR_SERVER_SIDE, READ_ONCE(msk->pm.server_side)))
+		return -EMSGSIZE;
+
 	return mptcp_event_add_subflow(skb, ssk);
 }
 
-- 
2.31.1

Re: mptcp: expose server_side attribute in MPTCP netlink events: Build Failure

[MPTCP CI]

Hi Kishen,

Thank you for your modifications, that's great!

But sadly, our CI spotted some issues with it when trying to build it.

You can find more details there:

  https://patchwork.kernel.org/project/mptcp/patch/20220329021401.1196466-7-kishen.maloor@intel.com/
  https://github.com/multipath-tcp/mptcp_net-next/actions/runs/2066164177

Status: failure
Initiator: MPTCPimporter
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/6a130a46e54f

Feel free to reply to this email if you cannot access logs, if you need
some support to fix the error, if this doesn't seem to be caused by your
modifications or if the error is a false positive one.

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)

Re: mptcp: expose server_side attribute in MPTCP netlink events: Tests Results

[MPTCP CI]

Hi Kishen,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal:
  - Unstable: 1 failed test(s): selftest_mptcp_join 🔴:
  - Task: https://cirrus-ci.com/task/5288976587161600
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5288976587161600/summary/summary.txt

- KVM Validation: debug:
  - Unstable: 2 failed test(s): selftest_mptcp_join selftest_simult_flows 🔴:
  - Task: https://cirrus-ci.com/task/6414876494004224
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6414876494004224/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/6a130a46e54f

Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (Tessares)

Re: [PATCH mptcp-next v9 0/6] mptcp: fixes and enhancements related to path management

[Paolo Abeni]

On Mon, 2022-03-28 at 22:13 -0400, Kishen Maloor wrote:
> This patch series incorporates fixes and enhancements related to
> path management over MPTCP connections, particularly in support of
> userspace PMs. The changes ensure that the required bits of
> information are conveyed through MPTCP netlink events which
> would be consumed by the path manager in making its decisions.
> Further it adds flexibility in establishing paths and from
> both ends of MPTCP connections.
> 
> v6:
> -Dropped framework and functionality around the use of kernel
> listening sockets for receiving MPJs.
> 
> v7:
> -Fixed bug caught by the CI build.
> 
> v8:
> -Fixed logic to achieve the desired outcome of not responding to
> address advertisements when a userspace PM is offline.
> -Factored out minor changes to mptcp_pm_is_userspace() to be
> squashed into an earlier commit.
> 
> v9:
> -Removed an explicit 'struct inet_sock *' cast in a patch which was
> not necessary.
> 
> Kishen Maloor (6):
>   Squash-to: mptcp: Bypass kernel PM when userspace PM is enabled
>   mptcp: bypass in-kernel PM restrictions for non-kernel PMs
>   mptcp: store remote id from MP_JOIN SYN/ACK in local ctx
>   mptcp: reflect remote port (not 0) in ANNOUNCED events
>   mptcp: establish subflows from either end of connection
>   mptcp: expose server_side attribute in MPTCP netlink events
> 
>  include/uapi/linux/mptcp.h |  1 +
>  net/mptcp/options.c        |  4 ++--
>  net/mptcp/pm.c             | 21 +++++++++++++++++----
>  net/mptcp/pm_netlink.c     | 24 +++++++++++++++++++++---
>  net/mptcp/protocol.c       |  5 +----
>  net/mptcp/protocol.h       | 22 ++++++++++++++++------
>  net/mptcp/subflow.c        |  5 ++++-
>  7 files changed, 62 insertions(+), 20 deletions(-)
> 
> 
> base-commit: 8e1fca01ac83e646133bd647abdae74d6d6449ba
> --
> 2.31.1
> 
This series LGTM, thanks!

There is a consistent self-test failure on test:

098 userspace pm type echoes add_addr. 

I *think* it can be addressed even after merging this series. @Kishen,
could you please have a look at that?

Paolo

Re: [PATCH mptcp-next v9 2/6] mptcp: bypass in-kernel PM restrictions for non-kernel PMs

[Mat Martineau]

This patch exposes some incorrect behavior in my userspace PM selftests
that are already in the export branch. If you could add the squash-to
patch at the beginning of your series to revert the bad test, and add
the new self test commit after "mptcp: bypass in-kernel PM restrictions
for non-kernel PMs" I think that addresses the test problem.

Mat Martineau (2):
  Squash-to: selftests: mptcp: Add tests for userspace PM type
  selftests: mptcp: ADD_ADDR echo test with missing userspace daemon

 tools/testing/selftests/net/mptcp/mptcp_join.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.35.1

[PATCH mptcp-next 1/2] Squash-to: selftests: mptcp: Add tests for userspace PM type

[Mat Martineau]

Remove invalid test.

Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 375b087edc7b..650bb062f612 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -2729,17 +2729,6 @@ userspace_tests()
 		chk_add_nr 0 0
 	fi
 
-	# userspace pm type echoes add_addr
-	if reset "userspace pm type echoes add_addr"; then
-		set_userspace_pm $ns2
-		pm_nl_set_limits $ns1 0 2
-		pm_nl_set_limits $ns2 0 2
-		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
-		run_tests $ns1 $ns2 10.0.1.1
-		chk_join_nr 0 0 0
-		chk_add_nr 1 1
-	fi
-
 	# userspace pm type rejects join
 	if reset "userspace pm type rejects join"; then
 		set_userspace_pm $ns1
-- 
2.35.1

[PATCH mptcp-next 2/2] selftests: mptcp: ADD_ADDR echo test with missing userspace daemon

[Mat Martineau]

Check userspace PM behavior to ensure ADD_ADDR echoes are only sent when
there is an active userspace daemon. If the daemon is restarting or
hasn't loaded yet, the missing echo will cause the peer to retransmit
the ADD_ADDR - and hopefully the daemon will be ready to receive it at
that later time.

Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index 650bb062f612..469cd544a8d2 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -2729,6 +2729,17 @@ userspace_tests()
 		chk_add_nr 0 0
 	fi
 
+	# userspace pm type does not echo add_addr without daemon
+	if reset "userspace pm no echo w/o daemon"; then
+		set_userspace_pm $ns2
+		pm_nl_set_limits $ns1 0 2
+		pm_nl_set_limits $ns2 0 2
+		pm_nl_add_endpoint $ns1 10.0.2.1 flags signal
+		run_tests $ns1 $ns2 10.0.1.1
+		chk_join_nr 0 0 0
+		chk_add_nr 1 0
+	fi
+
 	# userspace pm type rejects join
 	if reset "userspace pm type rejects join"; then
 		set_userspace_pm $ns1
-- 
2.35.1

Re: [PATCH mptcp-next v9 0/6] mptcp: fixes and enhancements related to path management

[Mat Martineau]

On Mon, 28 Mar 2022, Kishen Maloor wrote:

> This patch series incorporates fixes and enhancements related to
> path management over MPTCP connections, particularly in support of
> userspace PMs. The changes ensure that the required bits of
> information are conveyed through MPTCP netlink events which
> would be consumed by the path manager in making its decisions.
> Further it adds flexibility in establishing paths and from
> both ends of MPTCP connections.
>
> v6:
> -Dropped framework and functionality around the use of kernel
> listening sockets for receiving MPJs.
>

Thanks Kishen. This first series looks good to me, and Paolo acked too. 
The second series needs a rebase and a few more tweaks.


For this series:

Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>


Matthieu, I sent two additional selftest-only patches so that the behavior 
implemented in this series (which is correct: don't send ADD_ADDR echoes 
if there's no userspace daemon) is correctly tested. They're pretty short 
and simple, hopefully you can review and apply with Kishen's series. As I 
mentioned in 
https://lore.kernel.org/mptcp/20220401193714.10268-1-mathew.j.martineau@linux.intel.com/ 
:

   If you could add the squash-to patch at the beginning of [the] series to
   revert the bad test, and add the new self test commit after "mptcp:
   bypass in-kernel PM restrictions for non-kernel PMs" I think that
   addresses the test problem.


Mat

> v7:
> -Fixed bug caught by the CI build.
>
> v8:
> -Fixed logic to achieve the desired outcome of not responding to
> address advertisements when a userspace PM is offline.
> -Factored out minor changes to mptcp_pm_is_userspace() to be
> squashed into an earlier commit.
>
> v9:
> -Removed an explicit 'struct inet_sock *' cast in a patch which was
> not necessary.
>
> Kishen Maloor (6):
>  Squash-to: mptcp: Bypass kernel PM when userspace PM is enabled
>  mptcp: bypass in-kernel PM restrictions for non-kernel PMs
>  mptcp: store remote id from MP_JOIN SYN/ACK in local ctx
>  mptcp: reflect remote port (not 0) in ANNOUNCED events
>  mptcp: establish subflows from either end of connection
>  mptcp: expose server_side attribute in MPTCP netlink events
>
> include/uapi/linux/mptcp.h |  1 +
> net/mptcp/options.c        |  4 ++--
> net/mptcp/pm.c             | 21 +++++++++++++++++----
> net/mptcp/pm_netlink.c     | 24 +++++++++++++++++++++---
> net/mptcp/protocol.c       |  5 +----
> net/mptcp/protocol.h       | 22 ++++++++++++++++------
> net/mptcp/subflow.c        |  5 ++++-
> 7 files changed, 62 insertions(+), 20 deletions(-)
>
>
> base-commit: 8e1fca01ac83e646133bd647abdae74d6d6449ba
> --
> 2.31.1
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next v9 0/6] mptcp: fixes and enhancements related to path management

[Matthieu Baerts]

Hi Kishen, Paolo, Mat,

On 29/03/2022 04:13, Kishen Maloor wrote:
> This patch series incorporates fixes and enhancements related to
> path management over MPTCP connections, particularly in support of
> userspace PMs. The changes ensure that the required bits of
> information are conveyed through MPTCP netlink events which
> would be consumed by the path manager in making its decisions.
> Further it adds flexibility in establishing paths and from
> both ends of MPTCP connections.

Thank you for the patches and the reviews!

Now in our tree (feat. for net-next next) with Paolo's ACK and Mat's RvB
tags + my RvB tag on Mat's patch:

New patches for t/upstream:
- 427906c8a699: "squashed" patch 1/6 in "mptcp: Bypass kernel PM when
userspace PM is enabled"
- 6640ff7f4de5: "Signed-off-by" + "Co-developed-by"
- Results: 463bac3cb3eb..6ecd31441499 (export)

- ab5c3fe24c5d: "squashed" patch 1/2 in "selftests: mptcp: Add tests for
userspace PM type"
- Results: 6ecd31441499..bdcfe21e5935 (export)

- 3fd4c2a2d672: mptcp: bypass in-kernel PM restrictions for non-kernel PMs
- 564dcdfa9e21: selftests: mptcp: ADD_ADDR echo test with missing
userspace daemon
- f9900267d0b8: mptcp: store remote id from MP_JOIN SYN/ACK in local ctx
- fc1a120f5a70: mptcp: reflect remote port (not 0) in ANNOUNCED events
- 12d4f5bc7b1c: mptcp: establish subflows from either end of connection
- 653a3596c18f: mptcp: expose server_side attribute in MPTCP netlink events
- Results: bdcfe21e5935..580d91d8ba51 (export)

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20220402T105856
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export

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

Re: mptcp: expose server_side attribute in MPTCP netlink events: Build Failure

[MPTCP CI]

Hi Kishen,

Thank you for your modifications, that's great!

But sadly, our CI spotted some issues with it when trying to build it.

You can find more details there:

  https://patchwork.kernel.org/project/mptcp/patch/20220203072508.3072309-9-kishen.maloor@intel.com/
  https://github.com/multipath-tcp/mptcp_net-next/actions/runs/1788178430

Status: failure
Initiator: MPTCPimporter
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/23ad466ddec7

Feel free to reply to this email if you cannot access logs, if you need
some support to fix the error, if this doesn't seem to be caused by your
modifications or if the error is a false positive one.

Cheers,
MPTCP GH Action bot

Re: mptcp: expose server_side attribute in MPTCP netlink events: Build Failure

[MPTCP CI]

Hi Kishen,

Thank you for your modifications, that's great!

But sadly, our CI spotted some issues with it when trying to build it.

You can find more details there:

  https://patchwork.kernel.org/project/mptcp/patch/20220203031331.2996457-9-kishen.maloor@intel.com/
  https://github.com/multipath-tcp/mptcp_net-next/actions/runs/1787467514

Status: failure
Initiator: MPTCPimporter
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/a581d3db61d8

Feel free to reply to this email if you cannot access logs, if you need
some support to fix the error, if this doesn't seem to be caused by your
modifications or if the error is a false positive one.

Cheers,
MPTCP GH Action bot