All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups
@ 2025-09-18 17:42 Matthieu Baerts (NGI0)
  2025-09-18 17:42 ` [PATCH mptcp-next 01/10] Squash to "mptcp: pm: in-kernel: usable client side with C-flag" Matthieu Baerts (NGI0)
                   ` (10 more replies)
  0 siblings, 11 replies; 18+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-09-18 17:42 UTC (permalink / raw)
  To: MPTCP Upstream; +Cc: Matthieu Baerts (NGI0)

I'm currently looking at adding ADD-ADDR endpoint type, see [1].

When working on this, I decided to do something we should have done from
a long time: renaming some variables with confusing names, and split
some functions.

Flushing this now, just to ease the reviews: this should be pure
refactoring, no functional changes intended in all these commits.

Based-on: <20250916-pm-c-flag-client-default-v2-0-3be2c5bc4d6a@kernel.org>
Link: https://github.com/multipath-tcp/mptcp_net-next/issues/503
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Matthieu Baerts (NGI0) (10):
      Squash to "mptcp: pm: in-kernel: usable client side with C-flag"
      mptcp: pm: in-kernel: refactor fill_local_addresses_vec
      mptcp: pm: in-kernel: refactor fill_remote_addresses_vec
      mptcp: pm: rename 'subflows' to 'extra_subflows'
      mptcp: pm: in-kernel: rename 'subflows_max' to 'limit_extra_subflows'
      mptcp: pm: in-kernel: rename 'add_addr_signal_max' to 'endp_signal_max'
      mptcp: pm: in-kernel: rename 'add_addr_accept_max' to 'limit_add_addr_accepted'
      mptcp: pm: in-kernel: rename 'local_addr_max' to 'endp_subflow_max'
      mptcp: pm: in-kernel: rename 'local_addr_list' to 'endp_list'
      mptcp: pm: in-kernel: rename 'addrs' to 'endpoints'

 include/uapi/linux/mptcp.h                        |   5 +
 net/mptcp/pm.c                                    |  16 +-
 net/mptcp/pm_kernel.c                             | 451 ++++++++++++----------
 net/mptcp/pm_userspace.c                          |   2 +-
 net/mptcp/protocol.h                              |  15 +-
 net/mptcp/sockopt.c                               |  20 +-
 tools/testing/selftests/bpf/progs/mptcp_subflow.c |   2 +-
 7 files changed, 284 insertions(+), 227 deletions(-)
---
base-commit: 32778f6dc930afb30f995997c12cd8b829ac1600
change-id: 20250916-pm-kern-endp-add_addr-95cfceaef3fc

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH mptcp-next 01/10] Squash to "mptcp: pm: in-kernel: usable client side with C-flag"
  2025-09-18 17:42 [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups Matthieu Baerts (NGI0)
@ 2025-09-18 17:42 ` Matthieu Baerts (NGI0)
  2025-09-19  3:18   ` Geliang Tang
  2025-09-18 17:42 ` [PATCH mptcp-next 02/10] mptcp: pm: in-kernel: refactor fill_local_addresses_vec Matthieu Baerts (NGI0)
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-09-18 17:42 UTC (permalink / raw)
  To: MPTCP Upstream; +Cc: Matthieu Baerts (NGI0)

The id_avail_bitmap is only used when either the 'subflow' or 'signal'
flag is used, but not with 'fullmesh' only. Here, it is replacing the
'subflow' action, so check if this flag is set.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_kernel.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index d7cd89fa6a11a1ea7703edbfbdf2bbe86a6a3054..277f81f38134d07918143331746a50bc316d81ca 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -411,7 +411,8 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
 			locals[i].flags = entry->flags;
 			locals[i].ifindex = entry->ifindex;
 
-			if (c_flag_case)
+			if (c_flag_case &&
+			    (entry->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW))
 				__clear_bit(locals[i].addr.id,
 					    msk->pm.id_avail_bitmap);
 

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH mptcp-next 02/10] mptcp: pm: in-kernel: refactor fill_local_addresses_vec
  2025-09-18 17:42 [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups Matthieu Baerts (NGI0)
  2025-09-18 17:42 ` [PATCH mptcp-next 01/10] Squash to "mptcp: pm: in-kernel: usable client side with C-flag" Matthieu Baerts (NGI0)
@ 2025-09-18 17:42 ` Matthieu Baerts (NGI0)
  2025-09-19  6:51   ` Geliang Tang
  2025-09-18 17:42 ` [PATCH mptcp-next 03/10] mptcp: pm: in-kernel: refactor fill_remote_addresses_vec Matthieu Baerts (NGI0)
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-09-18 17:42 UTC (permalink / raw)
  To: MPTCP Upstream; +Cc: Matthieu Baerts (NGI0)

Before this modification, this function was quite long with many levels
of indentations.

Each case can be split in a dedicated function: fullmesh, C flag, any.

To remove one level of indentation, msk->pm.subflows >= subflows_max is
now checked upfront.

No functional changes intended.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_kernel.c | 182 ++++++++++++++++++++++++++++++--------------------
 net/mptcp/protocol.h  |   3 +-
 2 files changed, 110 insertions(+), 75 deletions(-)

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 277f81f38134d07918143331746a50bc316d81ca..5bdcfcc26308841c49375ce35205097f30592279 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -377,25 +377,20 @@ static void mptcp_pm_nl_subflow_established(struct mptcp_sock *msk)
 	mptcp_pm_create_subflow_or_signal_addr(msk);
 }
 
-/* Fill all the local addresses into the array addrs[],
- * and return the array size.
- */
-static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
-					     struct mptcp_addr_info *remote,
-					     struct mptcp_pm_local *locals)
+static unsigned int
+fill_local_addresses_vec_fullmesh(struct mptcp_sock *msk,
+				  struct mptcp_addr_info *remote,
+				  struct mptcp_pm_local *locals,
+				  bool c_flag_case)
 {
+	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
+	unsigned int subflows_max = mptcp_pm_get_subflows_max(msk);
 	struct sock *sk = (struct sock *)msk;
 	struct mptcp_pm_addr_entry *entry;
 	struct mptcp_addr_info mpc_addr;
-	struct pm_nl_pernet *pernet;
-	unsigned int subflows_max;
-	bool c_flag_case;
+	struct mptcp_pm_local *local;
 	int i = 0;
 
-	pernet = pm_nl_get_pernet_from_msk(msk);
-	subflows_max = mptcp_pm_get_subflows_max(msk);
-	c_flag_case = remote->id && mptcp_pm_add_addr_c_flag_case(msk);
-
 	mptcp_local_address((struct sock_common *)msk, &mpc_addr);
 
 	rcu_read_lock();
@@ -406,77 +401,116 @@ static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
 		if (!mptcp_pm_addr_families_match(sk, &entry->addr, remote))
 			continue;
 
-		if (msk->pm.subflows < subflows_max) {
-			locals[i].addr = entry->addr;
-			locals[i].flags = entry->flags;
-			locals[i].ifindex = entry->ifindex;
+		local = &locals[i];
+		local->addr = entry->addr;
+		local->flags = entry->flags;
+		local->ifindex = entry->ifindex;
 
-			if (c_flag_case &&
-			    (entry->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW))
-				__clear_bit(locals[i].addr.id,
-					    msk->pm.id_avail_bitmap);
+		if (c_flag_case && (entry->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW))
+			__clear_bit(local->addr.id, msk->pm.id_avail_bitmap);
 
-			/* Special case for ID0: set the correct ID */
-			if (mptcp_addresses_equal(&locals[i].addr, &mpc_addr, locals[i].addr.port))
-				locals[i].addr.id = 0;
+		/* Special case for ID0: set the correct ID */
+		if (mptcp_addresses_equal(&local->addr, &mpc_addr,
+					  local->addr.port))
+			local->addr.id = 0;
 
-			msk->pm.subflows++;
-			i++;
-		}
+		msk->pm.subflows++;
+		i++;
+
+		if (msk->pm.subflows >= subflows_max)
+			break;
 	}
 	rcu_read_unlock();
 
+	return i;
+}
+
+static unsigned int
+fill_local_addresses_vec_c_flag(struct mptcp_sock *msk,
+				struct mptcp_addr_info *remote,
+				struct mptcp_pm_local *locals)
+{
+	unsigned int local_addr_max = mptcp_pm_get_local_addr_max(msk);
+	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
+	unsigned int subflows_max = mptcp_pm_get_subflows_max(msk);
+	struct sock *sk = (struct sock *)msk;
+	struct mptcp_addr_info mpc_addr;
+	struct mptcp_pm_local *local;
+	int i = 0;
+
+	mptcp_local_address((struct sock_common *)msk, &mpc_addr);
+
+	while (msk->pm.local_addr_used < local_addr_max) {
+		local = &locals[i];
+
+		if (!select_local_address(pernet, msk, local))
+			break;
+
+		__clear_bit(local->addr.id, msk->pm.id_avail_bitmap);
+
+		if (!mptcp_pm_addr_families_match(sk, &local->addr, remote))
+			continue;
+
+		if (mptcp_addresses_equal(&local->addr, &mpc_addr,
+					  local->addr.port))
+			continue;
+
+		msk->pm.local_addr_used++;
+		msk->pm.subflows++;
+		i++;
+
+		if (msk->pm.subflows >= subflows_max)
+			break;
+	}
+
+	return i;
+}
+
+static unsigned int
+fill_local_address_any(struct mptcp_sock *msk, struct mptcp_addr_info *remote,
+		       struct mptcp_pm_local *local)
+{
+	struct sock *sk = (struct sock *)msk;
+
+	memset(local, 0, sizeof(*local));
+	local->addr.family =
+#if IS_ENABLED(CONFIG_MPTCP_IPV6)
+			remote->family == AF_INET6 &&
+			ipv6_addr_v4mapped(&remote->addr6) ? AF_INET :
+#endif
+			remote->family;
+
+	if (!mptcp_pm_addr_families_match(sk, &local->addr, remote))
+		return 0;
+
+	msk->pm.subflows++;
+
+	return 1;
+}
+
+/* Fill all the local addresses into the array addrs[],
+ * and return the array size.
+ */
+static unsigned int
+fill_local_addresses_vec(struct mptcp_sock *msk, struct mptcp_addr_info *remote,
+			 struct mptcp_pm_local *locals)
+{
+	bool c_flag_case = remote->id && mptcp_pm_add_addr_c_flag_case(msk);
+	int i;
+
+	/* If there is at least one MPTCP endpoint with a fullmesh flag */
+	i = fill_local_addresses_vec_fullmesh(msk, remote, locals, c_flag_case);
+	if (i)
+		return i;
+
 	/* Special case: peer sets the C flag, accept one ADD_ADDR if default
 	 * limits are used -- accepting no ADD_ADDR -- and use subflow endpoints
 	 */
-	if (!i && c_flag_case) {
-		unsigned int local_addr_max = mptcp_pm_get_local_addr_max(msk);
+	if (c_flag_case)
+		return fill_local_addresses_vec_c_flag(msk, remote, locals);
 
-		while (msk->pm.local_addr_used < local_addr_max &&
-		       msk->pm.subflows < subflows_max) {
-			struct mptcp_pm_local *local = &locals[i];
-
-			if (!select_local_address(pernet, msk, local))
-				break;
-
-			__clear_bit(local->addr.id, msk->pm.id_avail_bitmap);
-
-			if (!mptcp_pm_addr_families_match(sk, &local->addr,
-							  remote))
-				continue;
-
-			if (mptcp_addresses_equal(&local->addr, &mpc_addr,
-						  local->addr.port))
-				continue;
-
-			msk->pm.local_addr_used++;
-			msk->pm.subflows++;
-			i++;
-		}
-
-		return i;
-	}
-
-	/* If the array is empty, fill in the single
-	 * 'IPADDRANY' local address
-	 */
-	if (!i) {
-		memset(&locals[i], 0, sizeof(locals[i]));
-		locals[i].addr.family =
-#if IS_ENABLED(CONFIG_MPTCP_IPV6)
-			       remote->family == AF_INET6 &&
-			       ipv6_addr_v4mapped(&remote->addr6) ? AF_INET :
-#endif
-			       remote->family;
-
-		if (!mptcp_pm_addr_families_match(sk, &locals[i].addr, remote))
-			return 0;
-
-		msk->pm.subflows++;
-		i++;
-	}
-
-	return i;
+	/* No special case: fill in the single 'IPADDRANY' local address */
+	return fill_local_address_any(msk, remote, &locals[0]);
 }
 
 static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index dd0662defd41c84474e44c559c571e3594b85d9e..0d6dae37c9daf4ec8990b9a87036aa393add585c 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1200,7 +1200,8 @@ static inline bool mptcp_pm_add_addr_c_flag_case(struct mptcp_sock *msk)
 {
 	return READ_ONCE(msk->pm.remote_deny_join_id0) &&
 	       msk->pm.local_addr_used == 0 &&
-	       mptcp_pm_get_add_addr_accept_max(msk) == 0;
+	       mptcp_pm_get_add_addr_accept_max(msk) == 0 &&
+	       msk->pm.subflows < mptcp_pm_get_subflows_max(msk);
 }
 
 void mptcp_sockopt_sync_locked(struct mptcp_sock *msk, struct sock *ssk);

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH mptcp-next 03/10] mptcp: pm: in-kernel: refactor fill_remote_addresses_vec
  2025-09-18 17:42 [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups Matthieu Baerts (NGI0)
  2025-09-18 17:42 ` [PATCH mptcp-next 01/10] Squash to "mptcp: pm: in-kernel: usable client side with C-flag" Matthieu Baerts (NGI0)
  2025-09-18 17:42 ` [PATCH mptcp-next 02/10] mptcp: pm: in-kernel: refactor fill_local_addresses_vec Matthieu Baerts (NGI0)
@ 2025-09-18 17:42 ` Matthieu Baerts (NGI0)
  2025-09-19  6:52   ` Geliang Tang
  2025-09-18 17:42 ` [PATCH mptcp-next 04/10] mptcp: pm: rename 'subflows' to 'extra_subflows' Matthieu Baerts (NGI0)
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 18+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-09-18 17:42 UTC (permalink / raw)
  To: MPTCP Upstream; +Cc: Matthieu Baerts (NGI0)

Before this modification, this function was quite long with many levels
of indentations.

Each case can be split in a dedicated function: fullmesh, non-fullmesh.

To remove one level of indentation, msk->pm.subflows >= subflows_max is
now checked after having added one subflow, and stops the loop if it is
no longer possible to add new subflows. This is fine to do this because
this function should only be called if msk->pm.subflows < subflows_max.

No functional changes intended.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_kernel.c | 112 ++++++++++++++++++++++++++++----------------------
 1 file changed, 63 insertions(+), 49 deletions(-)

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 5bdcfcc26308841c49375ce35205097f30592279..8d5df9b98589e5cd69f16dc54e9140e88a1835e2 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -159,74 +159,88 @@ select_signal_address(struct pm_nl_pernet *pernet, const struct mptcp_sock *msk,
 	return found;
 }
 
-/* Fill all the remote addresses into the array addrs[],
- * and return the array size.
- */
-static unsigned int fill_remote_addresses_vec(struct mptcp_sock *msk,
-					      struct mptcp_addr_info *local,
-					      bool fullmesh,
-					      struct mptcp_addr_info *addrs)
+static unsigned int
+fill_remote_addr(struct mptcp_sock *msk, struct mptcp_addr_info *local,
+		 struct mptcp_addr_info *addrs)
 {
 	bool deny_id0 = READ_ONCE(msk->pm.remote_deny_join_id0);
+	struct sock *sk = (struct sock *)msk;
+	struct mptcp_addr_info remote = { 0 };
+
+	if (deny_id0)
+		return 0;
+
+	mptcp_remote_address((struct sock_common *)sk, &remote);
+
+	if (!mptcp_pm_addr_families_match(sk, local, &remote))
+		return 0;
+
+	msk->pm.subflows++;
+	*addrs = remote;
+
+	return 1;
+}
+
+static unsigned int
+fill_remote_addresses_fullmesh(struct mptcp_sock *msk,
+			       struct mptcp_addr_info *local,
+			       struct mptcp_addr_info *addrs)
+{
+	bool deny_id0 = READ_ONCE(msk->pm.remote_deny_join_id0);
+	DECLARE_BITMAP(unavail_id, MPTCP_PM_MAX_ADDR_ID + 1);
 	struct sock *sk = (struct sock *)msk, *ssk;
 	struct mptcp_subflow_context *subflow;
-	struct mptcp_addr_info remote = { 0 };
 	unsigned int subflows_max;
 	int i = 0;
 
 	subflows_max = mptcp_pm_get_subflows_max(msk);
-	mptcp_remote_address((struct sock_common *)sk, &remote);
 
-	/* Non-fullmesh endpoint, fill in the single entry
-	 * corresponding to the primary MPC subflow remote address
+	/* Forbid creation of new subflows matching existing ones, possibly
+	 * already created by incoming ADD_ADDR
 	 */
-	if (!fullmesh) {
-		if (deny_id0)
-			return 0;
+	bitmap_zero(unavail_id, MPTCP_PM_MAX_ADDR_ID + 1);
+	mptcp_for_each_subflow(msk, subflow)
+		if (READ_ONCE(subflow->local_id) == local->id)
+			__set_bit(subflow->remote_id, unavail_id);
 
-		if (!mptcp_pm_addr_families_match(sk, local, &remote))
-			return 0;
+	mptcp_for_each_subflow(msk, subflow) {
+		ssk = mptcp_subflow_tcp_sock(subflow);
+		mptcp_remote_address((struct sock_common *)ssk, &addrs[i]);
+		addrs[i].id = READ_ONCE(subflow->remote_id);
+		if (deny_id0 && !addrs[i].id)
+			continue;
 
+		if (test_bit(addrs[i].id, unavail_id))
+			continue;
+
+		if (!mptcp_pm_addr_families_match(sk, local, &addrs[i]))
+			continue;
+
+		/* forbid creating multiple address towards this id */
+		__set_bit(addrs[i].id, unavail_id);
 		msk->pm.subflows++;
-		addrs[i++] = remote;
-	} else {
-		DECLARE_BITMAP(unavail_id, MPTCP_PM_MAX_ADDR_ID + 1);
+		i++;
 
-		/* Forbid creation of new subflows matching existing
-		 * ones, possibly already created by incoming ADD_ADDR
-		 */
-		bitmap_zero(unavail_id, MPTCP_PM_MAX_ADDR_ID + 1);
-		mptcp_for_each_subflow(msk, subflow)
-			if (READ_ONCE(subflow->local_id) == local->id)
-				__set_bit(subflow->remote_id, unavail_id);
-
-		mptcp_for_each_subflow(msk, subflow) {
-			ssk = mptcp_subflow_tcp_sock(subflow);
-			mptcp_remote_address((struct sock_common *)ssk, &addrs[i]);
-			addrs[i].id = READ_ONCE(subflow->remote_id);
-			if (deny_id0 && !addrs[i].id)
-				continue;
-
-			if (test_bit(addrs[i].id, unavail_id))
-				continue;
-
-			if (!mptcp_pm_addr_families_match(sk, local, &addrs[i]))
-				continue;
-
-			if (msk->pm.subflows < subflows_max) {
-				/* forbid creating multiple address towards
-				 * this id
-				 */
-				__set_bit(addrs[i].id, unavail_id);
-				msk->pm.subflows++;
-				i++;
-			}
-		}
+		if (msk->pm.subflows >= subflows_max)
+			break;
 	}
 
 	return i;
 }
 
+/* Fill all the remote addresses into the array addrs[],
+ * and return the array size.
+ */
+static unsigned int
+fill_remote_addresses_vec(struct mptcp_sock *msk, struct mptcp_addr_info *local,
+			  bool fullmesh, struct mptcp_addr_info *addrs)
+{
+	if (fullmesh)
+		return fill_remote_addresses_fullmesh(msk, local, addrs);
+
+	return fill_remote_addr(msk, local, addrs);
+}
+
 static struct mptcp_pm_addr_entry *
 __lookup_addr_by_id(struct pm_nl_pernet *pernet, unsigned int id)
 {

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH mptcp-next 04/10] mptcp: pm: rename 'subflows' to 'extra_subflows'
  2025-09-18 17:42 [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups Matthieu Baerts (NGI0)
                   ` (2 preceding siblings ...)
  2025-09-18 17:42 ` [PATCH mptcp-next 03/10] mptcp: pm: in-kernel: refactor fill_remote_addresses_vec Matthieu Baerts (NGI0)
@ 2025-09-18 17:42 ` Matthieu Baerts (NGI0)
  2025-09-18 17:42 ` [PATCH mptcp-next 05/10] mptcp: pm: in-kernel: rename 'subflows_max' to 'limit_extra_subflows' Matthieu Baerts (NGI0)
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-09-18 17:42 UTC (permalink / raw)
  To: MPTCP Upstream; +Cc: Matthieu Baerts (NGI0)

A few variables linked to the Path-Managers are confusing, and it would
help current and future developers, to clarify them.

One of them is 'subflows', which in fact represents the number of extra
subflows: all the additional subflows created after the initial one, and
not the total number of subflows.

While at it, add an additional name for the corresponding variable in
MPTCP INFO: mptcpi_extra_subflows. Not to break the current uAPI, the
new name is added as a 'define' pointing to the former name. This will
then also help userspace devs.

No functional changes intended.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 include/uapi/linux/mptcp.h                        |  1 +
 net/mptcp/pm.c                                    | 10 +++++-----
 net/mptcp/pm_kernel.c                             | 24 +++++++++++------------
 net/mptcp/pm_userspace.c                          |  2 +-
 net/mptcp/protocol.h                              |  6 +++---
 net/mptcp/sockopt.c                               |  4 ++--
 tools/testing/selftests/bpf/progs/mptcp_subflow.c |  2 +-
 7 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index 15eef878690b8556af21be8d959b6a2c9fe617d3..f807c8dba56e7eb278fce0ad3184de3d0e24399b 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -42,6 +42,7 @@
 
 struct mptcp_info {
 	__u8	mptcpi_subflows;
+	#define mptcpi_extra_subflows mptcpi_subflows
 	__u8	mptcpi_add_addr_signal;
 	__u8	mptcpi_add_addr_accepted;
 	__u8	mptcpi_subflows_max;
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index edaf93fe6f86b32aee8e9bb88318b1dc2a7fcb5e..2e0b7509b8a91cba14f656480f425e0f7241890e 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -489,7 +489,7 @@ bool mptcp_pm_allow_new_subflow(struct mptcp_sock *msk)
 	if (mptcp_pm_is_userspace(msk)) {
 		if (mptcp_userspace_pm_active(msk)) {
 			spin_lock_bh(&pm->lock);
-			pm->subflows++;
+			pm->extra_subflows++;
 			spin_unlock_bh(&pm->lock);
 			return true;
 		}
@@ -498,7 +498,7 @@ bool mptcp_pm_allow_new_subflow(struct mptcp_sock *msk)
 
 	subflows_max = mptcp_pm_get_subflows_max(msk);
 
-	pr_debug("msk=%p subflows=%d max=%d allow=%d\n", msk, pm->subflows,
+	pr_debug("msk=%p subflows=%d max=%d allow=%d\n", msk, pm->extra_subflows,
 		 subflows_max, READ_ONCE(pm->accept_subflow));
 
 	/* try to avoid acquiring the lock below */
@@ -507,8 +507,8 @@ bool mptcp_pm_allow_new_subflow(struct mptcp_sock *msk)
 
 	spin_lock_bh(&pm->lock);
 	if (READ_ONCE(pm->accept_subflow)) {
-		ret = pm->subflows < subflows_max;
-		if (ret && ++pm->subflows == subflows_max)
+		ret = pm->extra_subflows < subflows_max;
+		if (ret && ++pm->extra_subflows == subflows_max)
 			WRITE_ONCE(pm->accept_subflow, false);
 	}
 	spin_unlock_bh(&pm->lock);
@@ -594,7 +594,7 @@ void mptcp_pm_subflow_check_next(struct mptcp_sock *msk,
 	if (mptcp_pm_is_userspace(msk)) {
 		if (update_subflows) {
 			spin_lock_bh(&pm->lock);
-			pm->subflows--;
+			pm->extra_subflows--;
 			spin_unlock_bh(&pm->lock);
 		}
 		return;
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 8d5df9b98589e5cd69f16dc54e9140e88a1835e2..6439f877f334083d86c85d3c19caebba8da7902c 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -175,7 +175,7 @@ fill_remote_addr(struct mptcp_sock *msk, struct mptcp_addr_info *local,
 	if (!mptcp_pm_addr_families_match(sk, local, &remote))
 		return 0;
 
-	msk->pm.subflows++;
+	msk->pm.extra_subflows++;
 	*addrs = remote;
 
 	return 1;
@@ -218,10 +218,10 @@ fill_remote_addresses_fullmesh(struct mptcp_sock *msk,
 
 		/* forbid creating multiple address towards this id */
 		__set_bit(addrs[i].id, unavail_id);
-		msk->pm.subflows++;
+		msk->pm.extra_subflows++;
 		i++;
 
-		if (msk->pm.subflows >= subflows_max)
+		if (msk->pm.extra_subflows >= subflows_max)
 			break;
 	}
 
@@ -309,7 +309,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 	pr_debug("local %d:%d signal %d:%d subflows %d:%d\n",
 		 msk->pm.local_addr_used, local_addr_max,
 		 msk->pm.add_addr_signaled, add_addr_signal_max,
-		 msk->pm.subflows, subflows_max);
+		 msk->pm.extra_subflows, subflows_max);
 
 	/* check first for announce */
 	if (msk->pm.add_addr_signaled < add_addr_signal_max) {
@@ -349,7 +349,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 subflow:
 	/* check if should create a new subflow */
 	while (msk->pm.local_addr_used < local_addr_max &&
-	       msk->pm.subflows < subflows_max) {
+	       msk->pm.extra_subflows < subflows_max) {
 		struct mptcp_addr_info addrs[MPTCP_PM_ADDR_MAX];
 		bool fullmesh;
 		int i, nr;
@@ -428,10 +428,10 @@ fill_local_addresses_vec_fullmesh(struct mptcp_sock *msk,
 					  local->addr.port))
 			local->addr.id = 0;
 
-		msk->pm.subflows++;
+		msk->pm.extra_subflows++;
 		i++;
 
-		if (msk->pm.subflows >= subflows_max)
+		if (msk->pm.extra_subflows >= subflows_max)
 			break;
 	}
 	rcu_read_unlock();
@@ -470,10 +470,10 @@ fill_local_addresses_vec_c_flag(struct mptcp_sock *msk,
 			continue;
 
 		msk->pm.local_addr_used++;
-		msk->pm.subflows++;
+		msk->pm.extra_subflows++;
 		i++;
 
-		if (msk->pm.subflows >= subflows_max)
+		if (msk->pm.extra_subflows >= subflows_max)
 			break;
 	}
 
@@ -497,7 +497,7 @@ fill_local_address_any(struct mptcp_sock *msk, struct mptcp_addr_info *remote,
 	if (!mptcp_pm_addr_families_match(sk, &local->addr, remote))
 		return 0;
 
-	msk->pm.subflows++;
+	msk->pm.extra_subflows++;
 
 	return 1;
 }
@@ -573,7 +573,7 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
 		if (remote.id)
 			msk->pm.add_addr_accepted++;
 		if (msk->pm.add_addr_accepted >= add_addr_accept_max ||
-		    msk->pm.subflows >= subflows_max)
+		    msk->pm.extra_subflows >= subflows_max)
 			WRITE_ONCE(msk->pm.accept_addr, false);
 	}
 }
@@ -1415,7 +1415,7 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk)
 {
 	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
 
-	if (msk->pm.subflows == mptcp_pm_get_subflows_max(msk) ||
+	if (msk->pm.extra_subflows == mptcp_pm_get_subflows_max(msk) ||
 	    (find_next_and_bit(pernet->id_bitmap, msk->pm.id_avail_bitmap,
 			       MPTCP_PM_MAX_ADDR_ID + 1, 0) == MPTCP_PM_MAX_ADDR_ID + 1)) {
 		WRITE_ONCE(msk->pm.work_pending, false);
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 1911fe1799fa38a53381247a830a9a0daf1c4492..d838e8ea65fd4ef8b5d0698f0d4586d565fab32f 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -419,7 +419,7 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 	if (err)
 		mptcp_userspace_pm_delete_local_addr(msk, &entry);
 	else
-		msk->pm.subflows++;
+		msk->pm.extra_subflows++;
 	spin_unlock_bh(&msk->pm.lock);
 
  create_err:
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 0d6dae37c9daf4ec8990b9a87036aa393add585c..33f7bf9a8c1344a01432c18d183a5d1b2d1f2c9e 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -236,7 +236,7 @@ struct mptcp_pm_data {
 	u8		add_addr_accepted;
 	u8		local_addr_used;
 	u8		pm_type;
-	u8		subflows;
+	u8		extra_subflows;
 	u8		status;
 
 	);
@@ -1185,7 +1185,7 @@ unsigned int mptcp_pm_get_local_addr_max(const struct mptcp_sock *msk);
 /* called under PM lock */
 static inline void __mptcp_pm_close_subflow(struct mptcp_sock *msk)
 {
-	if (--msk->pm.subflows < mptcp_pm_get_subflows_max(msk))
+	if (--msk->pm.extra_subflows < mptcp_pm_get_subflows_max(msk))
 		WRITE_ONCE(msk->pm.accept_subflow, true);
 }
 
@@ -1201,7 +1201,7 @@ static inline bool mptcp_pm_add_addr_c_flag_case(struct mptcp_sock *msk)
 	return READ_ONCE(msk->pm.remote_deny_join_id0) &&
 	       msk->pm.local_addr_used == 0 &&
 	       mptcp_pm_get_add_addr_accept_max(msk) == 0 &&
-	       msk->pm.subflows < mptcp_pm_get_subflows_max(msk);
+	       msk->pm.extra_subflows < mptcp_pm_get_subflows_max(msk);
 }
 
 void mptcp_sockopt_sync_locked(struct mptcp_sock *msk, struct sock *ssk);
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 2abe6f1e99400498e915176c360be9281fd524f6..17966da80239d731de925f3e4211b3ee00f802e4 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -962,7 +962,7 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 
 	memset(info, 0, sizeof(*info));
 
-	info->mptcpi_subflows = READ_ONCE(msk->pm.subflows);
+	info->mptcpi_extra_subflows = READ_ONCE(msk->pm.extra_subflows);
 	info->mptcpi_add_addr_signal = READ_ONCE(msk->pm.add_addr_signaled);
 	info->mptcpi_add_addr_accepted = READ_ONCE(msk->pm.add_addr_accepted);
 	info->mptcpi_local_addr_used = READ_ONCE(msk->pm.local_addr_used);
@@ -996,7 +996,7 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 	info->mptcpi_bytes_sent = msk->bytes_sent;
 	info->mptcpi_bytes_received = msk->bytes_received;
 	info->mptcpi_bytes_retrans = msk->bytes_retrans;
-	info->mptcpi_subflows_total = info->mptcpi_subflows +
+	info->mptcpi_subflows_total = info->mptcpi_extra_subflows +
 		__mptcp_has_initial_subflow(msk);
 	now = tcp_jiffies32;
 	info->mptcpi_last_data_sent = jiffies_to_msecs(now - msk->last_data_sent);
diff --git a/tools/testing/selftests/bpf/progs/mptcp_subflow.c b/tools/testing/selftests/bpf/progs/mptcp_subflow.c
index 70302477e326eecaef6aad4ecf899aa3d6606f23..41389e579578b7d6c0d9ffff520c9f3e930abb51 100644
--- a/tools/testing/selftests/bpf/progs/mptcp_subflow.c
+++ b/tools/testing/selftests/bpf/progs/mptcp_subflow.c
@@ -117,7 +117,7 @@ int _getsockopt_subflow(struct bpf_sockopt *ctx)
 		return 1;
 
 	msk = bpf_core_cast(sk, struct mptcp_sock);
-	if (msk->pm.subflows != 1) {
+	if (msk->pm.extra_subflows != 1) {
 		ctx->retval = -1;
 		return 1;
 	}

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH mptcp-next 05/10] mptcp: pm: in-kernel: rename 'subflows_max' to 'limit_extra_subflows'
  2025-09-18 17:42 [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups Matthieu Baerts (NGI0)
                   ` (3 preceding siblings ...)
  2025-09-18 17:42 ` [PATCH mptcp-next 04/10] mptcp: pm: rename 'subflows' to 'extra_subflows' Matthieu Baerts (NGI0)
@ 2025-09-18 17:42 ` Matthieu Baerts (NGI0)
  2025-09-18 17:42 ` [PATCH mptcp-next 06/10] mptcp: pm: in-kernel: rename 'add_addr_signal_max' to 'endp_signal_max' Matthieu Baerts (NGI0)
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-09-18 17:42 UTC (permalink / raw)
  To: MPTCP Upstream; +Cc: Matthieu Baerts (NGI0)

A few variables linked to the in-kernel Path-Manager are confusing, and
it would help current and future developers, to clarify them.

One of them is 'subflows_max', which in fact represents the limit of
extra subflows: the limit set via 'ip mptcp limit subflows X' for
example. It is not linked to the maximum number of created / possible
subflows.

While at it, add an additional name for the corresponding variable in
MPTCP INFO: mptcpi_limit_extra_subflows. Not to break the current uAPI,
the new name is added as a 'define' pointing to the former name. This
will then also help userspace devs.

No functional changes intended.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 include/uapi/linux/mptcp.h |  1 +
 net/mptcp/pm.c             | 10 +++++-----
 net/mptcp/pm_kernel.c      | 50 ++++++++++++++++++++++++----------------------
 net/mptcp/protocol.h       |  6 +++---
 net/mptcp/sockopt.c        |  4 ++--
 5 files changed, 37 insertions(+), 34 deletions(-)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index f807c8dba56e7eb278fce0ad3184de3d0e24399b..314200c61f150da445ff87debb154bfd98f324fc 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -46,6 +46,7 @@ struct mptcp_info {
 	__u8	mptcpi_add_addr_signal;
 	__u8	mptcpi_add_addr_accepted;
 	__u8	mptcpi_subflows_max;
+	#define mptcpi_limit_extra_subflows mptcpi_subflows_max
 	__u8	mptcpi_add_addr_signal_max;
 	__u8	mptcpi_add_addr_accepted_max;
 	__u32	mptcpi_flags;
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 2e0b7509b8a91cba14f656480f425e0f7241890e..ea40f31f17ba0d6c82db250d17f5a051ce6fabfa 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -483,7 +483,7 @@ void mptcp_pm_new_connection(struct mptcp_sock *msk, const struct sock *ssk, int
 bool mptcp_pm_allow_new_subflow(struct mptcp_sock *msk)
 {
 	struct mptcp_pm_data *pm = &msk->pm;
-	unsigned int subflows_max;
+	unsigned int limit_extra_subflows;
 	int ret = 0;
 
 	if (mptcp_pm_is_userspace(msk)) {
@@ -496,10 +496,10 @@ bool mptcp_pm_allow_new_subflow(struct mptcp_sock *msk)
 		return false;
 	}
 
-	subflows_max = mptcp_pm_get_subflows_max(msk);
+	limit_extra_subflows = mptcp_pm_get_limit_extra_subflows(msk);
 
 	pr_debug("msk=%p subflows=%d max=%d allow=%d\n", msk, pm->extra_subflows,
-		 subflows_max, READ_ONCE(pm->accept_subflow));
+		 limit_extra_subflows, READ_ONCE(pm->accept_subflow));
 
 	/* try to avoid acquiring the lock below */
 	if (!READ_ONCE(pm->accept_subflow))
@@ -507,8 +507,8 @@ bool mptcp_pm_allow_new_subflow(struct mptcp_sock *msk)
 
 	spin_lock_bh(&pm->lock);
 	if (READ_ONCE(pm->accept_subflow)) {
-		ret = pm->extra_subflows < subflows_max;
-		if (ret && ++pm->extra_subflows == subflows_max)
+		ret = pm->extra_subflows < limit_extra_subflows;
+		if (ret && ++pm->extra_subflows == limit_extra_subflows)
 			WRITE_ONCE(pm->accept_subflow, false);
 	}
 	spin_unlock_bh(&pm->lock);
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 6439f877f334083d86c85d3c19caebba8da7902c..e61d417da2154809ad83da5a1b8eede1216a2b5a 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -23,7 +23,7 @@ struct pm_nl_pernet {
 	unsigned int		add_addr_signal_max;
 	unsigned int		add_addr_accept_max;
 	unsigned int		local_addr_max;
-	unsigned int		subflows_max;
+	unsigned int		limit_extra_subflows;
 	unsigned int		next_id;
 	DECLARE_BITMAP(id_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
 };
@@ -62,13 +62,13 @@ unsigned int mptcp_pm_get_add_addr_accept_max(const struct mptcp_sock *msk)
 }
 EXPORT_SYMBOL_GPL(mptcp_pm_get_add_addr_accept_max);
 
-unsigned int mptcp_pm_get_subflows_max(const struct mptcp_sock *msk)
+unsigned int mptcp_pm_get_limit_extra_subflows(const struct mptcp_sock *msk)
 {
 	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
 
-	return READ_ONCE(pernet->subflows_max);
+	return READ_ONCE(pernet->limit_extra_subflows);
 }
-EXPORT_SYMBOL_GPL(mptcp_pm_get_subflows_max);
+EXPORT_SYMBOL_GPL(mptcp_pm_get_limit_extra_subflows);
 
 unsigned int mptcp_pm_get_local_addr_max(const struct mptcp_sock *msk)
 {
@@ -190,10 +190,10 @@ fill_remote_addresses_fullmesh(struct mptcp_sock *msk,
 	DECLARE_BITMAP(unavail_id, MPTCP_PM_MAX_ADDR_ID + 1);
 	struct sock *sk = (struct sock *)msk, *ssk;
 	struct mptcp_subflow_context *subflow;
-	unsigned int subflows_max;
+	unsigned int limit_extra_subflows;
 	int i = 0;
 
-	subflows_max = mptcp_pm_get_subflows_max(msk);
+	limit_extra_subflows = mptcp_pm_get_limit_extra_subflows(msk);
 
 	/* Forbid creation of new subflows matching existing ones, possibly
 	 * already created by incoming ADD_ADDR
@@ -221,7 +221,7 @@ fill_remote_addresses_fullmesh(struct mptcp_sock *msk,
 		msk->pm.extra_subflows++;
 		i++;
 
-		if (msk->pm.extra_subflows >= subflows_max)
+		if (msk->pm.extra_subflows >= limit_extra_subflows)
 			break;
 	}
 
@@ -270,18 +270,18 @@ __lookup_addr(struct pm_nl_pernet *pernet, const struct mptcp_addr_info *info)
 static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 {
 	struct sock *sk = (struct sock *)msk;
+	unsigned int limit_extra_subflows;
 	unsigned int add_addr_signal_max;
 	bool signal_and_subflow = false;
 	unsigned int local_addr_max;
 	struct pm_nl_pernet *pernet;
 	struct mptcp_pm_local local;
-	unsigned int subflows_max;
 
 	pernet = pm_nl_get_pernet(sock_net(sk));
 
 	add_addr_signal_max = mptcp_pm_get_add_addr_signal_max(msk);
 	local_addr_max = mptcp_pm_get_local_addr_max(msk);
-	subflows_max = mptcp_pm_get_subflows_max(msk);
+	limit_extra_subflows = mptcp_pm_get_limit_extra_subflows(msk);
 
 	/* do lazy endpoint usage accounting for the MPC subflows */
 	if (unlikely(!(msk->pm.status & BIT(MPTCP_PM_MPC_ENDPOINT_ACCOUNTED))) && msk->first) {
@@ -309,7 +309,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 	pr_debug("local %d:%d signal %d:%d subflows %d:%d\n",
 		 msk->pm.local_addr_used, local_addr_max,
 		 msk->pm.add_addr_signaled, add_addr_signal_max,
-		 msk->pm.extra_subflows, subflows_max);
+		 msk->pm.extra_subflows, limit_extra_subflows);
 
 	/* check first for announce */
 	if (msk->pm.add_addr_signaled < add_addr_signal_max) {
@@ -349,7 +349,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 subflow:
 	/* check if should create a new subflow */
 	while (msk->pm.local_addr_used < local_addr_max &&
-	       msk->pm.extra_subflows < subflows_max) {
+	       msk->pm.extra_subflows < limit_extra_subflows) {
 		struct mptcp_addr_info addrs[MPTCP_PM_ADDR_MAX];
 		bool fullmesh;
 		int i, nr;
@@ -398,14 +398,15 @@ fill_local_addresses_vec_fullmesh(struct mptcp_sock *msk,
 				  bool c_flag_case)
 {
 	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
-	unsigned int subflows_max = mptcp_pm_get_subflows_max(msk);
 	struct sock *sk = (struct sock *)msk;
 	struct mptcp_pm_addr_entry *entry;
+	unsigned int limit_extra_subflows;
 	struct mptcp_addr_info mpc_addr;
 	struct mptcp_pm_local *local;
 	int i = 0;
 
 	mptcp_local_address((struct sock_common *)msk, &mpc_addr);
+	limit_extra_subflows = mptcp_pm_get_limit_extra_subflows(msk);
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(entry, &pernet->local_addr_list, list) {
@@ -431,7 +432,7 @@ fill_local_addresses_vec_fullmesh(struct mptcp_sock *msk,
 		msk->pm.extra_subflows++;
 		i++;
 
-		if (msk->pm.extra_subflows >= subflows_max)
+		if (msk->pm.extra_subflows >= limit_extra_subflows)
 			break;
 	}
 	rcu_read_unlock();
@@ -446,13 +447,14 @@ fill_local_addresses_vec_c_flag(struct mptcp_sock *msk,
 {
 	unsigned int local_addr_max = mptcp_pm_get_local_addr_max(msk);
 	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
-	unsigned int subflows_max = mptcp_pm_get_subflows_max(msk);
 	struct sock *sk = (struct sock *)msk;
+	unsigned int limit_extra_subflows;
 	struct mptcp_addr_info mpc_addr;
 	struct mptcp_pm_local *local;
 	int i = 0;
 
 	mptcp_local_address((struct sock_common *)msk, &mpc_addr);
+	limit_extra_subflows = mptcp_pm_get_limit_extra_subflows(msk);
 
 	while (msk->pm.local_addr_used < local_addr_max) {
 		local = &locals[i];
@@ -473,7 +475,7 @@ fill_local_addresses_vec_c_flag(struct mptcp_sock *msk,
 		msk->pm.extra_subflows++;
 		i++;
 
-		if (msk->pm.extra_subflows >= subflows_max)
+		if (msk->pm.extra_subflows >= limit_extra_subflows)
 			break;
 	}
 
@@ -531,14 +533,14 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
 {
 	struct mptcp_pm_local locals[MPTCP_PM_ADDR_MAX];
 	struct sock *sk = (struct sock *)msk;
+	unsigned int limit_extra_subflows;
 	unsigned int add_addr_accept_max;
 	struct mptcp_addr_info remote;
-	unsigned int subflows_max;
 	bool sf_created = false;
 	int i, nr;
 
 	add_addr_accept_max = mptcp_pm_get_add_addr_accept_max(msk);
-	subflows_max = mptcp_pm_get_subflows_max(msk);
+	limit_extra_subflows = mptcp_pm_get_limit_extra_subflows(msk);
 
 	pr_debug("accepted %d:%d remote family %d\n",
 		 msk->pm.add_addr_accepted, add_addr_accept_max,
@@ -573,7 +575,7 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
 		if (remote.id)
 			msk->pm.add_addr_accepted++;
 		if (msk->pm.add_addr_accepted >= add_addr_accept_max ||
-		    msk->pm.extra_subflows >= subflows_max)
+		    msk->pm.extra_subflows >= limit_extra_subflows)
 			WRITE_ONCE(msk->pm.accept_addr, false);
 	}
 }
@@ -1273,13 +1275,13 @@ int mptcp_pm_nl_set_limits_doit(struct sk_buff *skb, struct genl_info *info)
 	if (ret)
 		goto unlock;
 
-	subflows = pernet->subflows_max;
+	subflows = pernet->limit_extra_subflows;
 	ret = parse_limit(info, MPTCP_PM_ATTR_SUBFLOWS, &subflows);
 	if (ret)
 		goto unlock;
 
 	WRITE_ONCE(pernet->add_addr_accept_max, rcv_addrs);
-	WRITE_ONCE(pernet->subflows_max, subflows);
+	WRITE_ONCE(pernet->limit_extra_subflows, subflows);
 
 unlock:
 	spin_unlock_bh(&pernet->lock);
@@ -1306,7 +1308,7 @@ int mptcp_pm_nl_get_limits_doit(struct sk_buff *skb, struct genl_info *info)
 		goto fail;
 
 	if (nla_put_u32(msg, MPTCP_PM_ATTR_SUBFLOWS,
-			READ_ONCE(pernet->subflows_max)))
+			READ_ONCE(pernet->limit_extra_subflows)))
 		goto fail;
 
 	genlmsg_end(msg, reply);
@@ -1415,7 +1417,7 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk)
 {
 	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
 
-	if (msk->pm.extra_subflows == mptcp_pm_get_subflows_max(msk) ||
+	if (msk->pm.extra_subflows == mptcp_pm_get_limit_extra_subflows(msk) ||
 	    (find_next_and_bit(pernet->id_bitmap, msk->pm.id_avail_bitmap,
 			       MPTCP_PM_MAX_ADDR_ID + 1, 0) == MPTCP_PM_MAX_ADDR_ID + 1)) {
 		WRITE_ONCE(msk->pm.work_pending, false);
@@ -1450,7 +1452,7 @@ static int __net_init pm_nl_init_net(struct net *net)
 	INIT_LIST_HEAD_RCU(&pernet->local_addr_list);
 
 	/* Cit. 2 subflows ought to be enough for anybody. */
-	pernet->subflows_max = 2;
+	pernet->limit_extra_subflows = 2;
 	pernet->next_id = 1;
 	pernet->stale_loss_cnt = 4;
 	spin_lock_init(&pernet->lock);
@@ -1486,7 +1488,7 @@ static struct pernet_operations mptcp_pm_pernet_ops = {
 
 static void mptcp_pm_kernel_init(struct mptcp_sock *msk)
 {
-	bool subflows_allowed = !!mptcp_pm_get_subflows_max(msk);
+	bool subflows_allowed = !!mptcp_pm_get_limit_extra_subflows(msk);
 	struct mptcp_pm_data *pm = &msk->pm;
 
 	/* pm->work_pending must be only be set to 'true' when
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 33f7bf9a8c1344a01432c18d183a5d1b2d1f2c9e..acfe46e530cd75b14b33d9f4940b90fbf2867517 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1179,13 +1179,13 @@ void mptcp_pm_worker(struct mptcp_sock *msk);
 void __mptcp_pm_kernel_worker(struct mptcp_sock *msk);
 unsigned int mptcp_pm_get_add_addr_signal_max(const struct mptcp_sock *msk);
 unsigned int mptcp_pm_get_add_addr_accept_max(const struct mptcp_sock *msk);
-unsigned int mptcp_pm_get_subflows_max(const struct mptcp_sock *msk);
+unsigned int mptcp_pm_get_limit_extra_subflows(const struct mptcp_sock *msk);
 unsigned int mptcp_pm_get_local_addr_max(const struct mptcp_sock *msk);
 
 /* called under PM lock */
 static inline void __mptcp_pm_close_subflow(struct mptcp_sock *msk)
 {
-	if (--msk->pm.extra_subflows < mptcp_pm_get_subflows_max(msk))
+	if (--msk->pm.extra_subflows < mptcp_pm_get_limit_extra_subflows(msk))
 		WRITE_ONCE(msk->pm.accept_subflow, true);
 }
 
@@ -1201,7 +1201,7 @@ static inline bool mptcp_pm_add_addr_c_flag_case(struct mptcp_sock *msk)
 	return READ_ONCE(msk->pm.remote_deny_join_id0) &&
 	       msk->pm.local_addr_used == 0 &&
 	       mptcp_pm_get_add_addr_accept_max(msk) == 0 &&
-	       msk->pm.extra_subflows < mptcp_pm_get_subflows_max(msk);
+	       msk->pm.extra_subflows < mptcp_pm_get_limit_extra_subflows(msk);
 }
 
 void mptcp_sockopt_sync_locked(struct mptcp_sock *msk, struct sock *ssk);
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 17966da80239d731de925f3e4211b3ee00f802e4..4e82bcfcd34e3a1dffb05799cf181ee2940e75d7 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -972,8 +972,8 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 
 	/* The following limits only make sense for the in-kernel PM */
 	if (mptcp_pm_is_kernel(msk)) {
-		info->mptcpi_subflows_max =
-			mptcp_pm_get_subflows_max(msk);
+		info->mptcpi_limit_extra_subflows =
+			mptcp_pm_get_limit_extra_subflows(msk);
 		info->mptcpi_add_addr_signal_max =
 			mptcp_pm_get_add_addr_signal_max(msk);
 		info->mptcpi_add_addr_accepted_max =

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH mptcp-next 06/10] mptcp: pm: in-kernel: rename 'add_addr_signal_max' to 'endp_signal_max'
  2025-09-18 17:42 [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups Matthieu Baerts (NGI0)
                   ` (4 preceding siblings ...)
  2025-09-18 17:42 ` [PATCH mptcp-next 05/10] mptcp: pm: in-kernel: rename 'subflows_max' to 'limit_extra_subflows' Matthieu Baerts (NGI0)
@ 2025-09-18 17:42 ` Matthieu Baerts (NGI0)
  2025-09-18 17:42 ` [PATCH mptcp-next 07/10] mptcp: pm: in-kernel: rename 'add_addr_accept_max' to 'limit_add_addr_accepted' Matthieu Baerts (NGI0)
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-09-18 17:42 UTC (permalink / raw)
  To: MPTCP Upstream; +Cc: Matthieu Baerts (NGI0)

A few variables linked to the in-kernel Path-Manager are confusing, and
it would help current and future developers, to clarify them.

One of them is 'add_addr_signal_max', which in fact represents the
maximum number of 'signal' endpoints that can be used to announced
addresses, and not the number of ADD_ADDR that can be signalled.

While at it, add an additional name for the corresponding variable in
MPTCP INFO: mptcpi_endp_signal_max. Not to break the current uAPI, the
new name is added as a 'define' pointing to the former name. This will
then also help userspace devs.

No functional changes intended.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 include/uapi/linux/mptcp.h |  1 +
 net/mptcp/pm_kernel.c      | 28 ++++++++++++++--------------
 net/mptcp/protocol.h       |  2 +-
 net/mptcp/sockopt.c        |  4 ++--
 4 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index 314200c61f150da445ff87debb154bfd98f324fc..69fc20db1c2f731d5f93cfee4e58a23119ff7a97 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -48,6 +48,7 @@ struct mptcp_info {
 	__u8	mptcpi_subflows_max;
 	#define mptcpi_limit_extra_subflows mptcpi_subflows_max
 	__u8	mptcpi_add_addr_signal_max;
+	#define mptcpi_endp_signal_max mptcpi_add_addr_signal_max
 	__u8	mptcpi_add_addr_accepted_max;
 	__u32	mptcpi_flags;
 	__u32	mptcpi_token;
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index e61d417da2154809ad83da5a1b8eede1216a2b5a..eb046d4b0d72ffe20a56a07781a8862b389c93bf 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -20,7 +20,7 @@ struct pm_nl_pernet {
 	struct list_head	local_addr_list;
 	unsigned int		addrs;
 	unsigned int		stale_loss_cnt;
-	unsigned int		add_addr_signal_max;
+	unsigned int		endp_signal_max;
 	unsigned int		add_addr_accept_max;
 	unsigned int		local_addr_max;
 	unsigned int		limit_extra_subflows;
@@ -46,13 +46,13 @@ static struct pm_nl_pernet *genl_info_pm_nl(struct genl_info *info)
 	return pm_nl_get_pernet(genl_info_net(info));
 }
 
-unsigned int mptcp_pm_get_add_addr_signal_max(const struct mptcp_sock *msk)
+unsigned int mptcp_pm_get_endp_signal_max(const struct mptcp_sock *msk)
 {
 	const struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
 
-	return READ_ONCE(pernet->add_addr_signal_max);
+	return READ_ONCE(pernet->endp_signal_max);
 }
-EXPORT_SYMBOL_GPL(mptcp_pm_get_add_addr_signal_max);
+EXPORT_SYMBOL_GPL(mptcp_pm_get_endp_signal_max);
 
 unsigned int mptcp_pm_get_add_addr_accept_max(const struct mptcp_sock *msk)
 {
@@ -271,15 +271,15 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 {
 	struct sock *sk = (struct sock *)msk;
 	unsigned int limit_extra_subflows;
-	unsigned int add_addr_signal_max;
 	bool signal_and_subflow = false;
+	unsigned int endp_signal_max;
 	unsigned int local_addr_max;
 	struct pm_nl_pernet *pernet;
 	struct mptcp_pm_local local;
 
 	pernet = pm_nl_get_pernet(sock_net(sk));
 
-	add_addr_signal_max = mptcp_pm_get_add_addr_signal_max(msk);
+	endp_signal_max = mptcp_pm_get_endp_signal_max(msk);
 	local_addr_max = mptcp_pm_get_local_addr_max(msk);
 	limit_extra_subflows = mptcp_pm_get_limit_extra_subflows(msk);
 
@@ -308,11 +308,11 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 
 	pr_debug("local %d:%d signal %d:%d subflows %d:%d\n",
 		 msk->pm.local_addr_used, local_addr_max,
-		 msk->pm.add_addr_signaled, add_addr_signal_max,
+		 msk->pm.add_addr_signaled, endp_signal_max,
 		 msk->pm.extra_subflows, limit_extra_subflows);
 
 	/* check first for announce */
-	if (msk->pm.add_addr_signaled < add_addr_signal_max) {
+	if (msk->pm.add_addr_signaled < endp_signal_max) {
 		/* due to racing events on both ends we can reach here while
 		 * previous add address is still running: if we invoke now
 		 * mptcp_pm_announce_addr(), that will fail and the
@@ -686,8 +686,8 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
 		pernet->next_id = entry->addr.id;
 
 	if (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL) {
-		addr_max = pernet->add_addr_signal_max;
-		WRITE_ONCE(pernet->add_addr_signal_max, addr_max + 1);
+		addr_max = pernet->endp_signal_max;
+		WRITE_ONCE(pernet->endp_signal_max, addr_max + 1);
 	}
 	if (entry->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW) {
 		addr_max = pernet->local_addr_max;
@@ -1086,8 +1086,8 @@ int mptcp_pm_nl_del_addr_doit(struct sk_buff *skb, struct genl_info *info)
 		return -EINVAL;
 	}
 	if (entry->flags & MPTCP_PM_ADDR_FLAG_SIGNAL) {
-		addr_max = pernet->add_addr_signal_max;
-		WRITE_ONCE(pernet->add_addr_signal_max, addr_max - 1);
+		addr_max = pernet->endp_signal_max;
+		WRITE_ONCE(pernet->endp_signal_max, addr_max - 1);
 	}
 	if (entry->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW) {
 		addr_max = pernet->local_addr_max;
@@ -1173,7 +1173,7 @@ static void __flush_addrs(struct list_head *list)
 
 static void __reset_counters(struct pm_nl_pernet *pernet)
 {
-	WRITE_ONCE(pernet->add_addr_signal_max, 0);
+	WRITE_ONCE(pernet->endp_signal_max, 0);
 	WRITE_ONCE(pernet->local_addr_max, 0);
 	pernet->addrs = 0;
 }
@@ -1497,7 +1497,7 @@ static void mptcp_pm_kernel_init(struct mptcp_sock *msk)
 	WRITE_ONCE(pm->work_pending,
 		   (!!mptcp_pm_get_local_addr_max(msk) &&
 		    subflows_allowed) ||
-		   !!mptcp_pm_get_add_addr_signal_max(msk));
+		   !!mptcp_pm_get_endp_signal_max(msk));
 	WRITE_ONCE(pm->accept_addr,
 		   !!mptcp_pm_get_add_addr_accept_max(msk) &&
 		   subflows_allowed);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index acfe46e530cd75b14b33d9f4940b90fbf2867517..31da0aa0f52443c16324dfe2a266252994bf656c 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1177,7 +1177,7 @@ void __init mptcp_pm_userspace_register(void);
 void __init mptcp_pm_nl_init(void);
 void mptcp_pm_worker(struct mptcp_sock *msk);
 void __mptcp_pm_kernel_worker(struct mptcp_sock *msk);
-unsigned int mptcp_pm_get_add_addr_signal_max(const struct mptcp_sock *msk);
+unsigned int mptcp_pm_get_endp_signal_max(const struct mptcp_sock *msk);
 unsigned int mptcp_pm_get_add_addr_accept_max(const struct mptcp_sock *msk);
 unsigned int mptcp_pm_get_limit_extra_subflows(const struct mptcp_sock *msk);
 unsigned int mptcp_pm_get_local_addr_max(const struct mptcp_sock *msk);
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 4e82bcfcd34e3a1dffb05799cf181ee2940e75d7..4688e0f25d15d9363b33bdcf3ad75c8295a810e3 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -974,8 +974,8 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 	if (mptcp_pm_is_kernel(msk)) {
 		info->mptcpi_limit_extra_subflows =
 			mptcp_pm_get_limit_extra_subflows(msk);
-		info->mptcpi_add_addr_signal_max =
-			mptcp_pm_get_add_addr_signal_max(msk);
+		info->mptcpi_endp_signal_max =
+			mptcp_pm_get_endp_signal_max(msk);
 		info->mptcpi_add_addr_accepted_max =
 			mptcp_pm_get_add_addr_accept_max(msk);
 		info->mptcpi_local_addr_max =

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH mptcp-next 07/10] mptcp: pm: in-kernel: rename 'add_addr_accept_max' to 'limit_add_addr_accepted'
  2025-09-18 17:42 [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups Matthieu Baerts (NGI0)
                   ` (5 preceding siblings ...)
  2025-09-18 17:42 ` [PATCH mptcp-next 06/10] mptcp: pm: in-kernel: rename 'add_addr_signal_max' to 'endp_signal_max' Matthieu Baerts (NGI0)
@ 2025-09-18 17:42 ` Matthieu Baerts (NGI0)
  2025-09-18 17:42 ` [PATCH mptcp-next 08/10] mptcp: pm: in-kernel: rename 'local_addr_max' to 'endp_subflow_max' Matthieu Baerts (NGI0)
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-09-18 17:42 UTC (permalink / raw)
  To: MPTCP Upstream; +Cc: Matthieu Baerts (NGI0)

A few variables linked to the in-kernel Path-Manager are confusing, and
it would help current and future developers, to clarify them.

One of them is 'add_addr_accept_max', which in fact represents the limit
of ADD_ADDR that can be accepted:  the limit set via 'ip mptcp limit
add_addr_accepted X' for example. It is not linked to the maximum number
of accepted ADD_ADDR.

While at it, add an additional name for the corresponding variable in
MPTCP INFO: mptcpi_limit_add_addr_accepted. Not to break the current
uAPI, the new name is added as a 'define' pointing to the former name.
This will then also help userspace devs.

No functional changes intended.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 include/uapi/linux/mptcp.h |  1 +
 net/mptcp/pm_kernel.c      | 26 +++++++++++++-------------
 net/mptcp/protocol.h       |  4 ++--
 net/mptcp/sockopt.c        |  4 ++--
 4 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index 69fc20db1c2f731d5f93cfee4e58a23119ff7a97..1c275ce96b524cf1525b80967f28b57a59c24a0f 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -50,6 +50,7 @@ struct mptcp_info {
 	__u8	mptcpi_add_addr_signal_max;
 	#define mptcpi_endp_signal_max mptcpi_add_addr_signal_max
 	__u8	mptcpi_add_addr_accepted_max;
+	#define mptcpi_limit_add_addr_accepted mptcpi_add_addr_accepted_max
 	__u32	mptcpi_flags;
 	__u32	mptcpi_token;
 	__u64	mptcpi_write_seq;
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index eb046d4b0d72ffe20a56a07781a8862b389c93bf..fbe7a0a930b23a5aaaba9420ccb4c914695675a4 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -21,7 +21,7 @@ struct pm_nl_pernet {
 	unsigned int		addrs;
 	unsigned int		stale_loss_cnt;
 	unsigned int		endp_signal_max;
-	unsigned int		add_addr_accept_max;
+	unsigned int		limit_add_addr_accepted;
 	unsigned int		local_addr_max;
 	unsigned int		limit_extra_subflows;
 	unsigned int		next_id;
@@ -54,13 +54,13 @@ unsigned int mptcp_pm_get_endp_signal_max(const struct mptcp_sock *msk)
 }
 EXPORT_SYMBOL_GPL(mptcp_pm_get_endp_signal_max);
 
-unsigned int mptcp_pm_get_add_addr_accept_max(const struct mptcp_sock *msk)
+unsigned int mptcp_pm_get_limit_add_addr_accepted(const struct mptcp_sock *msk)
 {
 	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
 
-	return READ_ONCE(pernet->add_addr_accept_max);
+	return READ_ONCE(pernet->limit_add_addr_accepted);
 }
-EXPORT_SYMBOL_GPL(mptcp_pm_get_add_addr_accept_max);
+EXPORT_SYMBOL_GPL(mptcp_pm_get_limit_add_addr_accepted);
 
 unsigned int mptcp_pm_get_limit_extra_subflows(const struct mptcp_sock *msk)
 {
@@ -534,16 +534,16 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
 	struct mptcp_pm_local locals[MPTCP_PM_ADDR_MAX];
 	struct sock *sk = (struct sock *)msk;
 	unsigned int limit_extra_subflows;
-	unsigned int add_addr_accept_max;
+	unsigned int limit_add_addr_accepted;
 	struct mptcp_addr_info remote;
 	bool sf_created = false;
 	int i, nr;
 
-	add_addr_accept_max = mptcp_pm_get_add_addr_accept_max(msk);
+	limit_add_addr_accepted = mptcp_pm_get_limit_add_addr_accepted(msk);
 	limit_extra_subflows = mptcp_pm_get_limit_extra_subflows(msk);
 
 	pr_debug("accepted %d:%d remote family %d\n",
-		 msk->pm.add_addr_accepted, add_addr_accept_max,
+		 msk->pm.add_addr_accepted, limit_add_addr_accepted,
 		 msk->pm.remote.family);
 
 	remote = msk->pm.remote;
@@ -574,7 +574,7 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
 		/* add_addr_accepted is not decr for ID 0 */
 		if (remote.id)
 			msk->pm.add_addr_accepted++;
-		if (msk->pm.add_addr_accepted >= add_addr_accept_max ||
+		if (msk->pm.add_addr_accepted >= limit_add_addr_accepted ||
 		    msk->pm.extra_subflows >= limit_extra_subflows)
 			WRITE_ONCE(msk->pm.accept_addr, false);
 	}
@@ -586,7 +586,7 @@ void mptcp_pm_nl_rm_addr(struct mptcp_sock *msk, u8 rm_id)
 		/* Note: if the subflow has been closed before, this
 		 * add_addr_accepted counter will not be decremented.
 		 */
-		if (--msk->pm.add_addr_accepted < mptcp_pm_get_add_addr_accept_max(msk))
+		if (--msk->pm.add_addr_accepted < mptcp_pm_get_limit_add_addr_accepted(msk))
 			WRITE_ONCE(msk->pm.accept_addr, true);
 	}
 }
@@ -1270,7 +1270,7 @@ int mptcp_pm_nl_set_limits_doit(struct sk_buff *skb, struct genl_info *info)
 	int ret;
 
 	spin_lock_bh(&pernet->lock);
-	rcv_addrs = pernet->add_addr_accept_max;
+	rcv_addrs = pernet->limit_add_addr_accepted;
 	ret = parse_limit(info, MPTCP_PM_ATTR_RCV_ADD_ADDRS, &rcv_addrs);
 	if (ret)
 		goto unlock;
@@ -1280,7 +1280,7 @@ int mptcp_pm_nl_set_limits_doit(struct sk_buff *skb, struct genl_info *info)
 	if (ret)
 		goto unlock;
 
-	WRITE_ONCE(pernet->add_addr_accept_max, rcv_addrs);
+	WRITE_ONCE(pernet->limit_add_addr_accepted, rcv_addrs);
 	WRITE_ONCE(pernet->limit_extra_subflows, subflows);
 
 unlock:
@@ -1304,7 +1304,7 @@ int mptcp_pm_nl_get_limits_doit(struct sk_buff *skb, struct genl_info *info)
 		goto fail;
 
 	if (nla_put_u32(msg, MPTCP_PM_ATTR_RCV_ADD_ADDRS,
-			READ_ONCE(pernet->add_addr_accept_max)))
+			READ_ONCE(pernet->limit_add_addr_accepted)))
 		goto fail;
 
 	if (nla_put_u32(msg, MPTCP_PM_ATTR_SUBFLOWS,
@@ -1499,7 +1499,7 @@ static void mptcp_pm_kernel_init(struct mptcp_sock *msk)
 		    subflows_allowed) ||
 		   !!mptcp_pm_get_endp_signal_max(msk));
 	WRITE_ONCE(pm->accept_addr,
-		   !!mptcp_pm_get_add_addr_accept_max(msk) &&
+		   !!mptcp_pm_get_limit_add_addr_accepted(msk) &&
 		   subflows_allowed);
 	WRITE_ONCE(pm->accept_subflow, subflows_allowed);
 
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 31da0aa0f52443c16324dfe2a266252994bf656c..63c46a00c9d48c4e6d41753b5f020f7cf39ffba0 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1178,7 +1178,7 @@ void __init mptcp_pm_nl_init(void);
 void mptcp_pm_worker(struct mptcp_sock *msk);
 void __mptcp_pm_kernel_worker(struct mptcp_sock *msk);
 unsigned int mptcp_pm_get_endp_signal_max(const struct mptcp_sock *msk);
-unsigned int mptcp_pm_get_add_addr_accept_max(const struct mptcp_sock *msk);
+unsigned int mptcp_pm_get_limit_add_addr_accepted(const struct mptcp_sock *msk);
 unsigned int mptcp_pm_get_limit_extra_subflows(const struct mptcp_sock *msk);
 unsigned int mptcp_pm_get_local_addr_max(const struct mptcp_sock *msk);
 
@@ -1200,7 +1200,7 @@ static inline bool mptcp_pm_add_addr_c_flag_case(struct mptcp_sock *msk)
 {
 	return READ_ONCE(msk->pm.remote_deny_join_id0) &&
 	       msk->pm.local_addr_used == 0 &&
-	       mptcp_pm_get_add_addr_accept_max(msk) == 0 &&
+	       mptcp_pm_get_limit_add_addr_accepted(msk) == 0 &&
 	       msk->pm.extra_subflows < mptcp_pm_get_limit_extra_subflows(msk);
 }
 
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 4688e0f25d15d9363b33bdcf3ad75c8295a810e3..5ab9909dbe799bed5c59d2b1b04cc0e88e960574 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -976,8 +976,8 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 			mptcp_pm_get_limit_extra_subflows(msk);
 		info->mptcpi_endp_signal_max =
 			mptcp_pm_get_endp_signal_max(msk);
-		info->mptcpi_add_addr_accepted_max =
-			mptcp_pm_get_add_addr_accept_max(msk);
+		info->mptcpi_limit_add_addr_accepted =
+			mptcp_pm_get_limit_add_addr_accepted(msk);
 		info->mptcpi_local_addr_max =
 			mptcp_pm_get_local_addr_max(msk);
 	}

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH mptcp-next 08/10] mptcp: pm: in-kernel: rename 'local_addr_max' to 'endp_subflow_max'
  2025-09-18 17:42 [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups Matthieu Baerts (NGI0)
                   ` (6 preceding siblings ...)
  2025-09-18 17:42 ` [PATCH mptcp-next 07/10] mptcp: pm: in-kernel: rename 'add_addr_accept_max' to 'limit_add_addr_accepted' Matthieu Baerts (NGI0)
@ 2025-09-18 17:42 ` Matthieu Baerts (NGI0)
  2025-09-18 17:42 ` [PATCH mptcp-next 09/10] mptcp: pm: in-kernel: rename 'local_addr_list' to 'endp_list' Matthieu Baerts (NGI0)
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 18+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-09-18 17:42 UTC (permalink / raw)
  To: MPTCP Upstream; +Cc: Matthieu Baerts (NGI0)

A few variables linked to the in-kernel Path-Manager are confusing, and
it would help current and future developers, to clarify them.

One of them is 'local_addr_max', which in fact represents the maximum
number of 'subflow' endpoints that can be used to create new subflows,
and not the number of local addresses that have been used to create
subflows.

While at it, add an additional name for the corresponding variable in
MPTCP INFO: mptcpi_endp_subflow_max. Not to break the current uAPI, the
new name is added as a 'define' pointing to the former name. This will
then also help userspace devs.

Also move the variable and function next to the other 'endp_X_max' ones.

No functional changes intended.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 include/uapi/linux/mptcp.h |  1 +
 net/mptcp/pm_kernel.c      | 42 +++++++++++++++++++++---------------------
 net/mptcp/protocol.h       |  2 +-
 net/mptcp/sockopt.c        |  4 ++--
 4 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/include/uapi/linux/mptcp.h b/include/uapi/linux/mptcp.h
index 1c275ce96b524cf1525b80967f28b57a59c24a0f..5ec996977b3fa2351222e6d01b814770b34348e9 100644
--- a/include/uapi/linux/mptcp.h
+++ b/include/uapi/linux/mptcp.h
@@ -58,6 +58,7 @@ struct mptcp_info {
 	__u64	mptcpi_rcv_nxt;
 	__u8	mptcpi_local_addr_used;
 	__u8	mptcpi_local_addr_max;
+	#define mptcpi_endp_subflow_max mptcpi_local_addr_max
 	__u8	mptcpi_csum_enabled;
 	__u32	mptcpi_retransmits;
 	__u64	mptcpi_bytes_retrans;
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index fbe7a0a930b23a5aaaba9420ccb4c914695675a4..e65449a1395270d46ee550a0814c94525586b4b4 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -21,8 +21,8 @@ struct pm_nl_pernet {
 	unsigned int		addrs;
 	unsigned int		stale_loss_cnt;
 	unsigned int		endp_signal_max;
+	unsigned int		endp_subflow_max;
 	unsigned int		limit_add_addr_accepted;
-	unsigned int		local_addr_max;
 	unsigned int		limit_extra_subflows;
 	unsigned int		next_id;
 	DECLARE_BITMAP(id_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
@@ -54,6 +54,14 @@ unsigned int mptcp_pm_get_endp_signal_max(const struct mptcp_sock *msk)
 }
 EXPORT_SYMBOL_GPL(mptcp_pm_get_endp_signal_max);
 
+unsigned int mptcp_pm_get_endp_subflow_max(const struct mptcp_sock *msk)
+{
+	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
+
+	return READ_ONCE(pernet->endp_subflow_max);
+}
+EXPORT_SYMBOL_GPL(mptcp_pm_get_endp_subflow_max);
+
 unsigned int mptcp_pm_get_limit_add_addr_accepted(const struct mptcp_sock *msk)
 {
 	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
@@ -70,14 +78,6 @@ unsigned int mptcp_pm_get_limit_extra_subflows(const struct mptcp_sock *msk)
 }
 EXPORT_SYMBOL_GPL(mptcp_pm_get_limit_extra_subflows);
 
-unsigned int mptcp_pm_get_local_addr_max(const struct mptcp_sock *msk)
-{
-	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
-
-	return READ_ONCE(pernet->local_addr_max);
-}
-EXPORT_SYMBOL_GPL(mptcp_pm_get_local_addr_max);
-
 static bool lookup_subflow_by_daddr(const struct list_head *list,
 				    const struct mptcp_addr_info *daddr)
 {
@@ -272,15 +272,15 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 	struct sock *sk = (struct sock *)msk;
 	unsigned int limit_extra_subflows;
 	bool signal_and_subflow = false;
+	unsigned int endp_subflow_max;
 	unsigned int endp_signal_max;
-	unsigned int local_addr_max;
 	struct pm_nl_pernet *pernet;
 	struct mptcp_pm_local local;
 
 	pernet = pm_nl_get_pernet(sock_net(sk));
 
 	endp_signal_max = mptcp_pm_get_endp_signal_max(msk);
-	local_addr_max = mptcp_pm_get_local_addr_max(msk);
+	endp_subflow_max = mptcp_pm_get_endp_subflow_max(msk);
 	limit_extra_subflows = mptcp_pm_get_limit_extra_subflows(msk);
 
 	/* do lazy endpoint usage accounting for the MPC subflows */
@@ -307,7 +307,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 	}
 
 	pr_debug("local %d:%d signal %d:%d subflows %d:%d\n",
-		 msk->pm.local_addr_used, local_addr_max,
+		 msk->pm.local_addr_used, endp_subflow_max,
 		 msk->pm.add_addr_signaled, endp_signal_max,
 		 msk->pm.extra_subflows, limit_extra_subflows);
 
@@ -348,7 +348,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 
 subflow:
 	/* check if should create a new subflow */
-	while (msk->pm.local_addr_used < local_addr_max &&
+	while (msk->pm.local_addr_used < endp_subflow_max &&
 	       msk->pm.extra_subflows < limit_extra_subflows) {
 		struct mptcp_addr_info addrs[MPTCP_PM_ADDR_MAX];
 		bool fullmesh;
@@ -445,7 +445,7 @@ fill_local_addresses_vec_c_flag(struct mptcp_sock *msk,
 				struct mptcp_addr_info *remote,
 				struct mptcp_pm_local *locals)
 {
-	unsigned int local_addr_max = mptcp_pm_get_local_addr_max(msk);
+	unsigned int endp_subflow_max = mptcp_pm_get_endp_subflow_max(msk);
 	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
 	struct sock *sk = (struct sock *)msk;
 	unsigned int limit_extra_subflows;
@@ -456,7 +456,7 @@ fill_local_addresses_vec_c_flag(struct mptcp_sock *msk,
 	mptcp_local_address((struct sock_common *)msk, &mpc_addr);
 	limit_extra_subflows = mptcp_pm_get_limit_extra_subflows(msk);
 
-	while (msk->pm.local_addr_used < local_addr_max) {
+	while (msk->pm.local_addr_used < endp_subflow_max) {
 		local = &locals[i];
 
 		if (!select_local_address(pernet, msk, local))
@@ -690,8 +690,8 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
 		WRITE_ONCE(pernet->endp_signal_max, addr_max + 1);
 	}
 	if (entry->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW) {
-		addr_max = pernet->local_addr_max;
-		WRITE_ONCE(pernet->local_addr_max, addr_max + 1);
+		addr_max = pernet->endp_subflow_max;
+		WRITE_ONCE(pernet->endp_subflow_max, addr_max + 1);
 	}
 
 	pernet->addrs++;
@@ -1090,8 +1090,8 @@ int mptcp_pm_nl_del_addr_doit(struct sk_buff *skb, struct genl_info *info)
 		WRITE_ONCE(pernet->endp_signal_max, addr_max - 1);
 	}
 	if (entry->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW) {
-		addr_max = pernet->local_addr_max;
-		WRITE_ONCE(pernet->local_addr_max, addr_max - 1);
+		addr_max = pernet->endp_subflow_max;
+		WRITE_ONCE(pernet->endp_subflow_max, addr_max - 1);
 	}
 
 	pernet->addrs--;
@@ -1174,7 +1174,7 @@ static void __flush_addrs(struct list_head *list)
 static void __reset_counters(struct pm_nl_pernet *pernet)
 {
 	WRITE_ONCE(pernet->endp_signal_max, 0);
-	WRITE_ONCE(pernet->local_addr_max, 0);
+	WRITE_ONCE(pernet->endp_subflow_max, 0);
 	pernet->addrs = 0;
 }
 
@@ -1495,7 +1495,7 @@ static void mptcp_pm_kernel_init(struct mptcp_sock *msk)
 	 * pm->pm_type is set to MPTCP_PM_TYPE_KERNEL
 	 */
 	WRITE_ONCE(pm->work_pending,
-		   (!!mptcp_pm_get_local_addr_max(msk) &&
+		   (!!mptcp_pm_get_endp_subflow_max(msk) &&
 		    subflows_allowed) ||
 		   !!mptcp_pm_get_endp_signal_max(msk));
 	WRITE_ONCE(pm->accept_addr,
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 63c46a00c9d48c4e6d41753b5f020f7cf39ffba0..a65aedc84ef5db3d8fc489b39d85ca56eb50e30e 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1178,9 +1178,9 @@ void __init mptcp_pm_nl_init(void);
 void mptcp_pm_worker(struct mptcp_sock *msk);
 void __mptcp_pm_kernel_worker(struct mptcp_sock *msk);
 unsigned int mptcp_pm_get_endp_signal_max(const struct mptcp_sock *msk);
+unsigned int mptcp_pm_get_endp_subflow_max(const struct mptcp_sock *msk);
 unsigned int mptcp_pm_get_limit_add_addr_accepted(const struct mptcp_sock *msk);
 unsigned int mptcp_pm_get_limit_extra_subflows(const struct mptcp_sock *msk);
-unsigned int mptcp_pm_get_local_addr_max(const struct mptcp_sock *msk);
 
 /* called under PM lock */
 static inline void __mptcp_pm_close_subflow(struct mptcp_sock *msk)
diff --git a/net/mptcp/sockopt.c b/net/mptcp/sockopt.c
index 5ab9909dbe799bed5c59d2b1b04cc0e88e960574..92a2a274262732a345b9ab185efd7da1f0a5773a 100644
--- a/net/mptcp/sockopt.c
+++ b/net/mptcp/sockopt.c
@@ -978,8 +978,8 @@ void mptcp_diag_fill_info(struct mptcp_sock *msk, struct mptcp_info *info)
 			mptcp_pm_get_endp_signal_max(msk);
 		info->mptcpi_limit_add_addr_accepted =
 			mptcp_pm_get_limit_add_addr_accepted(msk);
-		info->mptcpi_local_addr_max =
-			mptcp_pm_get_local_addr_max(msk);
+		info->mptcpi_endp_subflow_max =
+			mptcp_pm_get_endp_subflow_max(msk);
 	}
 
 	if (__mptcp_check_fallback(msk))

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH mptcp-next 09/10] mptcp: pm: in-kernel: rename 'local_addr_list' to 'endp_list'
  2025-09-18 17:42 [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups Matthieu Baerts (NGI0)
                   ` (7 preceding siblings ...)
  2025-09-18 17:42 ` [PATCH mptcp-next 08/10] mptcp: pm: in-kernel: rename 'local_addr_max' to 'endp_subflow_max' Matthieu Baerts (NGI0)
@ 2025-09-18 17:42 ` Matthieu Baerts (NGI0)
  2025-09-18 17:42 ` [PATCH mptcp-next 10/10] mptcp: pm: in-kernel: rename 'addrs' to 'endpoints' Matthieu Baerts (NGI0)
  2025-09-18 19:29 ` [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups MPTCP CI
  10 siblings, 0 replies; 18+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-09-18 17:42 UTC (permalink / raw)
  To: MPTCP Upstream; +Cc: Matthieu Baerts (NGI0)

A few variables linked to the in-kernel Path-Manager are confusing, and
it would help current and future developers, to clarify them.

One of them is 'local_addr_list', which in fact represents the list of
endpoints, and not only the 'subflow' endpoints.

No functional changes intended.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_kernel.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index e65449a1395270d46ee550a0814c94525586b4b4..eee931f1466afc166ad34658e8071e9b30cf9906 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -17,7 +17,7 @@ static int pm_nl_pernet_id;
 struct pm_nl_pernet {
 	/* protects pernet updates */
 	spinlock_t		lock;
-	struct list_head	local_addr_list;
+	struct list_head	endp_list;
 	unsigned int		addrs;
 	unsigned int		stale_loss_cnt;
 	unsigned int		endp_signal_max;
@@ -110,7 +110,7 @@ select_local_address(const struct pm_nl_pernet *pernet,
 	msk_owned_by_me(msk);
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(entry, &pernet->local_addr_list, list) {
+	list_for_each_entry_rcu(entry, &pernet->endp_list, list) {
 		if (!(entry->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW))
 			continue;
 
@@ -141,7 +141,7 @@ select_signal_address(struct pm_nl_pernet *pernet, const struct mptcp_sock *msk,
 	 * Note: removal from the local address list during the msk life-cycle
 	 * can lead to additional addresses not being announced.
 	 */
-	list_for_each_entry_rcu(entry, &pernet->local_addr_list, list) {
+	list_for_each_entry_rcu(entry, &pernet->endp_list, list) {
 		if (!test_bit(entry->addr.id, msk->pm.id_avail_bitmap))
 			continue;
 
@@ -246,7 +246,7 @@ __lookup_addr_by_id(struct pm_nl_pernet *pernet, unsigned int id)
 {
 	struct mptcp_pm_addr_entry *entry;
 
-	list_for_each_entry_rcu(entry, &pernet->local_addr_list, list,
+	list_for_each_entry_rcu(entry, &pernet->endp_list, list,
 				lockdep_is_held(&pernet->lock)) {
 		if (entry->addr.id == id)
 			return entry;
@@ -259,7 +259,7 @@ __lookup_addr(struct pm_nl_pernet *pernet, const struct mptcp_addr_info *info)
 {
 	struct mptcp_pm_addr_entry *entry;
 
-	list_for_each_entry_rcu(entry, &pernet->local_addr_list, list,
+	list_for_each_entry_rcu(entry, &pernet->endp_list, list,
 				lockdep_is_held(&pernet->lock)) {
 		if (mptcp_addresses_equal(&entry->addr, info, entry->addr.port))
 			return entry;
@@ -409,7 +409,7 @@ fill_local_addresses_vec_fullmesh(struct mptcp_sock *msk,
 	limit_extra_subflows = mptcp_pm_get_limit_extra_subflows(msk);
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(entry, &pernet->local_addr_list, list) {
+	list_for_each_entry_rcu(entry, &pernet->endp_list, list) {
 		if (!(entry->flags & MPTCP_PM_ADDR_FLAG_FULLMESH))
 			continue;
 
@@ -634,7 +634,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
 	 */
 	if (!address_use_port(entry))
 		entry->addr.port = 0;
-	list_for_each_entry(cur, &pernet->local_addr_list, list) {
+	list_for_each_entry(cur, &pernet->endp_list, list) {
 		if (mptcp_addresses_equal(&cur->addr, &entry->addr,
 					  cur->addr.port || entry->addr.port)) {
 			/* allow replacing the exiting endpoint only if such
@@ -696,9 +696,9 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
 
 	pernet->addrs++;
 	if (!entry->addr.port)
-		list_add_tail_rcu(&entry->list, &pernet->local_addr_list);
+		list_add_tail_rcu(&entry->list, &pernet->endp_list);
 	else
-		list_add_rcu(&entry->list, &pernet->local_addr_list);
+		list_add_rcu(&entry->list, &pernet->endp_list);
 	ret = entry->addr.id;
 
 out:
@@ -1184,7 +1184,7 @@ int mptcp_pm_nl_flush_addrs_doit(struct sk_buff *skb, struct genl_info *info)
 	LIST_HEAD(free_list);
 
 	spin_lock_bh(&pernet->lock);
-	list_splice_init(&pernet->local_addr_list, &free_list);
+	list_splice_init(&pernet->endp_list, &free_list);
 	__reset_counters(pernet);
 	pernet->next_id = 1;
 	bitmap_zero(pernet->id_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
@@ -1449,7 +1449,7 @@ static int __net_init pm_nl_init_net(struct net *net)
 {
 	struct pm_nl_pernet *pernet = pm_nl_get_pernet(net);
 
-	INIT_LIST_HEAD_RCU(&pernet->local_addr_list);
+	INIT_LIST_HEAD_RCU(&pernet->endp_list);
 
 	/* Cit. 2 subflows ought to be enough for anybody. */
 	pernet->limit_extra_subflows = 2;
@@ -1475,7 +1475,7 @@ static void __net_exit pm_nl_exit_net(struct list_head *net_list)
 		 * other modifiers, also netns core already waited for a
 		 * RCU grace period.
 		 */
-		__flush_addrs(&pernet->local_addr_list);
+		__flush_addrs(&pernet->endp_list);
 	}
 }
 

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH mptcp-next 10/10] mptcp: pm: in-kernel: rename 'addrs' to 'endpoints'
  2025-09-18 17:42 [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups Matthieu Baerts (NGI0)
                   ` (8 preceding siblings ...)
  2025-09-18 17:42 ` [PATCH mptcp-next 09/10] mptcp: pm: in-kernel: rename 'local_addr_list' to 'endp_list' Matthieu Baerts (NGI0)
@ 2025-09-18 17:42 ` Matthieu Baerts (NGI0)
  2025-09-18 19:29 ` [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups MPTCP CI
  10 siblings, 0 replies; 18+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-09-18 17:42 UTC (permalink / raw)
  To: MPTCP Upstream; +Cc: Matthieu Baerts (NGI0)

A few variables linked to the in-kernel Path-Manager are confusing, and
it would help current and future developers, to clarify them.

One of them is 'addrs', which in fact represents the number of declared
endpoints, and not only the 'signal' endpoints.

No functional changes intended.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/pm_kernel.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index eee931f1466afc166ad34658e8071e9b30cf9906..1f214e4c7078ad7518b2a501efa86633ad9e96ba 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -18,7 +18,7 @@ struct pm_nl_pernet {
 	/* protects pernet updates */
 	spinlock_t		lock;
 	struct list_head	endp_list;
-	unsigned int		addrs;
+	unsigned int		endpoints;
 	unsigned int		stale_loss_cnt;
 	unsigned int		endp_signal_max;
 	unsigned int		endp_subflow_max;
@@ -620,7 +620,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
 	 */
 	if (pernet->next_id == MPTCP_PM_MAX_ADDR_ID)
 		pernet->next_id = 1;
-	if (pernet->addrs >= MPTCP_PM_ADDR_MAX) {
+	if (pernet->endpoints >= MPTCP_PM_ADDR_MAX) {
 		ret = -ERANGE;
 		goto out;
 	}
@@ -659,7 +659,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
 				goto out;
 			}
 
-			pernet->addrs--;
+			pernet->endpoints--;
 			entry->addr.id = cur->addr.id;
 			list_del_rcu(&cur->list);
 			del_entry = cur;
@@ -694,7 +694,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
 		WRITE_ONCE(pernet->endp_subflow_max, addr_max + 1);
 	}
 
-	pernet->addrs++;
+	pernet->endpoints++;
 	if (!entry->addr.port)
 		list_add_tail_rcu(&entry->list, &pernet->endp_list);
 	else
@@ -1094,7 +1094,7 @@ int mptcp_pm_nl_del_addr_doit(struct sk_buff *skb, struct genl_info *info)
 		WRITE_ONCE(pernet->endp_subflow_max, addr_max - 1);
 	}
 
-	pernet->addrs--;
+	pernet->endpoints--;
 	list_del_rcu(&entry->list);
 	__clear_bit(entry->addr.id, pernet->id_bitmap);
 	spin_unlock_bh(&pernet->lock);
@@ -1175,7 +1175,7 @@ static void __reset_counters(struct pm_nl_pernet *pernet)
 {
 	WRITE_ONCE(pernet->endp_signal_max, 0);
 	WRITE_ONCE(pernet->endp_subflow_max, 0);
-	pernet->addrs = 0;
+	pernet->endpoints = 0;
 }
 
 int mptcp_pm_nl_flush_addrs_doit(struct sk_buff *skb, struct genl_info *info)

-- 
2.51.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups
  2025-09-18 17:42 [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups Matthieu Baerts (NGI0)
                   ` (9 preceding siblings ...)
  2025-09-18 17:42 ` [PATCH mptcp-next 10/10] mptcp: pm: in-kernel: rename 'addrs' to 'endpoints' Matthieu Baerts (NGI0)
@ 2025-09-18 19:29 ` MPTCP CI
  10 siblings, 0 replies; 18+ messages in thread
From: MPTCP CI @ 2025-09-18 19:29 UTC (permalink / raw)
  To: Matthieu Baerts; +Cc: mptcp

Hi Matthieu,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Success! ✅
- KVM Validation: debug: Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/17837220771

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/481fd3636aa0
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1003950


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


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 (NGI0 Core)

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH mptcp-next 01/10] Squash to "mptcp: pm: in-kernel: usable client side with C-flag"
  2025-09-18 17:42 ` [PATCH mptcp-next 01/10] Squash to "mptcp: pm: in-kernel: usable client side with C-flag" Matthieu Baerts (NGI0)
@ 2025-09-19  3:18   ` Geliang Tang
  2025-09-19  8:19     ` Matthieu Baerts
  0 siblings, 1 reply; 18+ messages in thread
From: Geliang Tang @ 2025-09-19  3:18 UTC (permalink / raw)
  To: Matthieu Baerts (NGI0), MPTCP Upstream

Hi Matt,

Thanks for this patch.

On Thu, 2025-09-18 at 19:42 +0200, Matthieu Baerts (NGI0) wrote:
> The id_avail_bitmap is only used when either the 'subflow' or
> 'signal'
> flag is used, but not with 'fullmesh' only. Here, it is replacing the
> 'subflow' action, so check if this flag is set.

I recall that 'fullmesh' can only be used together with 'subflow'. If
it has already been determined earlier that the flags contain
'fullmesh', shouldn't the flags here already include 'subflow'? I'm not
entirely certain, but directly checking for 'subflow' as done in this
patch seems like a better approach.

So this patch looks good to me.

As mentioned in my previous comment, if possible, the refactoring of
the fill_local_addresses_vec_c_flag helper in patch 2 could also be
added to this squash-to patch.

Thanks,
-Geliang

> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  net/mptcp/pm_kernel.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index
> d7cd89fa6a11a1ea7703edbfbdf2bbe86a6a3054..277f81f38134d07918143331746
> a50bc316d81ca 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -411,7 +411,8 @@ static unsigned int
> fill_local_addresses_vec(struct mptcp_sock *msk,
>  			locals[i].flags = entry->flags;
>  			locals[i].ifindex = entry->ifindex;
>  
> -			if (c_flag_case)
> +			if (c_flag_case &&
> +			    (entry->flags &
> MPTCP_PM_ADDR_FLAG_SUBFLOW))
>  				__clear_bit(locals[i].addr.id,
>  					    msk-
> >pm.id_avail_bitmap);
>  

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH mptcp-next 02/10] mptcp: pm: in-kernel: refactor fill_local_addresses_vec
  2025-09-18 17:42 ` [PATCH mptcp-next 02/10] mptcp: pm: in-kernel: refactor fill_local_addresses_vec Matthieu Baerts (NGI0)
@ 2025-09-19  6:51   ` Geliang Tang
  2025-09-19  8:25     ` Matthieu Baerts
  0 siblings, 1 reply; 18+ messages in thread
From: Geliang Tang @ 2025-09-19  6:51 UTC (permalink / raw)
  To: Matthieu Baerts (NGI0), MPTCP Upstream

Hi Matt,

On Thu, 2025-09-18 at 19:42 +0200, Matthieu Baerts (NGI0) wrote:
> Before this modification, this function was quite long with many
> levels
> of indentations.
> 
> Each case can be split in a dedicated function: fullmesh, C flag,
> any.

If "C flag" function is squash into patch 1, how about splitting this
patch into two, one for 'fullmesh' and one for 'any'.

> 
> To remove one level of indentation, msk->pm.subflows >= subflows_max
> is
> now checked upfront.
> 
> No functional changes intended.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  net/mptcp/pm_kernel.c | 182 ++++++++++++++++++++++++++++++----------
> ----------
>  net/mptcp/protocol.h  |   3 +-
>  2 files changed, 110 insertions(+), 75 deletions(-)
> 
> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index
> 277f81f38134d07918143331746a50bc316d81ca..5bdcfcc26308841c49375ce3520
> 5097f30592279 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -377,25 +377,20 @@ static void
> mptcp_pm_nl_subflow_established(struct mptcp_sock *msk)
>  	mptcp_pm_create_subflow_or_signal_addr(msk);
>  }
>  
> -/* Fill all the local addresses into the array addrs[],
> - * and return the array size.
> - */
> -static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
> -					     struct mptcp_addr_info
> *remote,
> -					     struct mptcp_pm_local
> *locals)
> +static unsigned int
> +fill_local_addresses_vec_fullmesh(struct mptcp_sock *msk,
> +				  struct mptcp_addr_info *remote,
> +				  struct mptcp_pm_local *locals,
> +				  bool c_flag_case)
>  {
> +	struct pm_nl_pernet *pernet =
> pm_nl_get_pernet_from_msk(msk);
> +	unsigned int subflows_max = mptcp_pm_get_subflows_max(msk);
>  	struct sock *sk = (struct sock *)msk;
>  	struct mptcp_pm_addr_entry *entry;
>  	struct mptcp_addr_info mpc_addr;
> -	struct pm_nl_pernet *pernet;
> -	unsigned int subflows_max;
> -	bool c_flag_case;
> +	struct mptcp_pm_local *local;
>  	int i = 0;
>  
> -	pernet = pm_nl_get_pernet_from_msk(msk);
> -	subflows_max = mptcp_pm_get_subflows_max(msk);
> -	c_flag_case = remote->id &&
> mptcp_pm_add_addr_c_flag_case(msk);
> -
>  	mptcp_local_address((struct sock_common *)msk, &mpc_addr);
>  
>  	rcu_read_lock();
> @@ -406,77 +401,116 @@ static unsigned int
> fill_local_addresses_vec(struct mptcp_sock *msk,
>  		if (!mptcp_pm_addr_families_match(sk, &entry->addr,
> remote))
>  			continue;
>  
> -		if (msk->pm.subflows < subflows_max) {
> -			locals[i].addr = entry->addr;
> -			locals[i].flags = entry->flags;
> -			locals[i].ifindex = entry->ifindex;
> +		local = &locals[i];
> +		local->addr = entry->addr;
> +		local->flags = entry->flags;
> +		local->ifindex = entry->ifindex;
>  
> -			if (c_flag_case &&
> -			    (entry->flags &
> MPTCP_PM_ADDR_FLAG_SUBFLOW))
> -				__clear_bit(locals[i].addr.id,
> -					    msk-
> >pm.id_avail_bitmap);
> +		if (c_flag_case && (entry->flags &
> MPTCP_PM_ADDR_FLAG_SUBFLOW))
> +			__clear_bit(local->addr.id, msk-
> >pm.id_avail_bitmap);
>  
> -			/* Special case for ID0: set the correct ID
> */
> -			if (mptcp_addresses_equal(&locals[i].addr,
> &mpc_addr, locals[i].addr.port))
> -				locals[i].addr.id = 0;
> +		/* Special case for ID0: set the correct ID */
> +		if (mptcp_addresses_equal(&local->addr, &mpc_addr,
> +					  local->addr.port))
> +			local->addr.id = 0;
>  
> -			msk->pm.subflows++;
> -			i++;
> -		}
> +		msk->pm.subflows++;
> +		i++;
> +
> +		if (msk->pm.subflows >= subflows_max)
> +			break;
>  	}
>  	rcu_read_unlock();
>  
> +	return i;
> +}
> +
> +static unsigned int
> +fill_local_addresses_vec_c_flag(struct mptcp_sock *msk,
> +				struct mptcp_addr_info *remote,
> +				struct mptcp_pm_local *locals)
> +{
> +	unsigned int local_addr_max =
> mptcp_pm_get_local_addr_max(msk);
> +	struct pm_nl_pernet *pernet =
> pm_nl_get_pernet_from_msk(msk);
> +	unsigned int subflows_max = mptcp_pm_get_subflows_max(msk);
> +	struct sock *sk = (struct sock *)msk;
> +	struct mptcp_addr_info mpc_addr;
> +	struct mptcp_pm_local *local;
> +	int i = 0;
> +
> +	mptcp_local_address((struct sock_common *)msk, &mpc_addr);
> +
> +	while (msk->pm.local_addr_used < local_addr_max) {
> +		local = &locals[i];
> +
> +		if (!select_local_address(pernet, msk, local))
> +			break;
> +
> +		__clear_bit(local->addr.id, msk-
> >pm.id_avail_bitmap);
> +
> +		if (!mptcp_pm_addr_families_match(sk, &local->addr,
> remote))
> +			continue;
> +
> +		if (mptcp_addresses_equal(&local->addr, &mpc_addr,
> +					  local->addr.port))
> +			continue;
> +
> +		msk->pm.local_addr_used++;
> +		msk->pm.subflows++;
> +		i++;
> +
> +		if (msk->pm.subflows >= subflows_max)
> +			break;
> +	}
> +
> +	return i;
> +}
> +
> +static unsigned int
> +fill_local_address_any(struct mptcp_sock *msk, struct
> mptcp_addr_info *remote,
> +		       struct mptcp_pm_local *local)
> +{
> +	struct sock *sk = (struct sock *)msk;
> +
> +	memset(local, 0, sizeof(*local));
> +	local->addr.family =
> +#if IS_ENABLED(CONFIG_MPTCP_IPV6)
> +			remote->family == AF_INET6 &&
> +			ipv6_addr_v4mapped(&remote->addr6) ? AF_INET
> :
> +#endif
> +			remote->family;
> +
> +	if (!mptcp_pm_addr_families_match(sk, &local->addr, remote))
> +		return 0;
> +
> +	msk->pm.subflows++;
> +
> +	return 1;
> +}
> +
> +/* Fill all the local addresses into the array addrs[],
> + * and return the array size.
> + */
> +static unsigned int
> +fill_local_addresses_vec(struct mptcp_sock *msk, struct
> mptcp_addr_info *remote,
> +			 struct mptcp_pm_local *locals)

How about keeping this unchanged:

static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
					     struct mptcp_addr_info
*remote,
					     struct mptcp_pm_local
*locals)

This can make the patch smaller.

> +{
> +	bool c_flag_case = remote->id &&
> mptcp_pm_add_addr_c_flag_case(msk);
> +	int i;
> +
> +	/* If there is at least one MPTCP endpoint with a fullmesh
> flag */
> +	i = fill_local_addresses_vec_fullmesh(msk, remote, locals,
> c_flag_case);
> +	if (i)
> +		return i;
> +
>  	/* Special case: peer sets the C flag, accept one ADD_ADDR
> if default
>  	 * limits are used -- accepting no ADD_ADDR -- and use
> subflow endpoints
>  	 */
> -	if (!i && c_flag_case) {
> -		unsigned int local_addr_max =
> mptcp_pm_get_local_addr_max(msk);
> +	if (c_flag_case)
> +		return fill_local_addresses_vec_c_flag(msk, remote,
> locals);
>  
> -		while (msk->pm.local_addr_used < local_addr_max &&
> -		       msk->pm.subflows < subflows_max) {
> -			struct mptcp_pm_local *local = &locals[i];
> -
> -			if (!select_local_address(pernet, msk,
> local))
> -				break;
> -
> -			__clear_bit(local->addr.id, msk-
> >pm.id_avail_bitmap);
> -
> -			if (!mptcp_pm_addr_families_match(sk,
> &local->addr,
> -							  remote))
> -				continue;
> -
> -			if (mptcp_addresses_equal(&local->addr,
> &mpc_addr,
> -						  local->addr.port))
> -				continue;
> -
> -			msk->pm.local_addr_used++;
> -			msk->pm.subflows++;
> -			i++;
> -		}
> -
> -		return i;
> -	}
> -
> -	/* If the array is empty, fill in the single
> -	 * 'IPADDRANY' local address
> -	 */
> -	if (!i) {
> -		memset(&locals[i], 0, sizeof(locals[i]));
> -		locals[i].addr.family =
> -#if IS_ENABLED(CONFIG_MPTCP_IPV6)
> -			       remote->family == AF_INET6 &&
> -			       ipv6_addr_v4mapped(&remote->addr6) ?
> AF_INET :
> -#endif
> -			       remote->family;
> -
> -		if (!mptcp_pm_addr_families_match(sk,
> &locals[i].addr, remote))
> -			return 0;
> -
> -		msk->pm.subflows++;
> -		i++;
> -	}
> -
> -	return i;
> +	/* No special case: fill in the single 'IPADDRANY' local
> address */
> +	return fill_local_address_any(msk, remote, &locals[0]);
>  }
>  
>  static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index
> dd0662defd41c84474e44c559c571e3594b85d9e..0d6dae37c9daf4ec8990b9a8703
> 6aa393add585c 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -1200,7 +1200,8 @@ static inline bool
> mptcp_pm_add_addr_c_flag_case(struct mptcp_sock *msk)
>  {
>  	return READ_ONCE(msk->pm.remote_deny_join_id0) &&
>  	       msk->pm.local_addr_used == 0 &&
> -	       mptcp_pm_get_add_addr_accept_max(msk) == 0;
> +	       mptcp_pm_get_add_addr_accept_max(msk) == 0 &&
> +	       msk->pm.subflows < mptcp_pm_get_subflows_max(msk);

This needs to be squash to patch 1 too.

Thanks,
-Geliang

>  }
>  
>  void mptcp_sockopt_sync_locked(struct mptcp_sock *msk, struct sock
> *ssk);

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH mptcp-next 03/10] mptcp: pm: in-kernel: refactor fill_remote_addresses_vec
  2025-09-18 17:42 ` [PATCH mptcp-next 03/10] mptcp: pm: in-kernel: refactor fill_remote_addresses_vec Matthieu Baerts (NGI0)
@ 2025-09-19  6:52   ` Geliang Tang
  2025-09-19  8:41     ` Matthieu Baerts
  0 siblings, 1 reply; 18+ messages in thread
From: Geliang Tang @ 2025-09-19  6:52 UTC (permalink / raw)
  To: Matthieu Baerts (NGI0), MPTCP Upstream

Hi Matt,

On Thu, 2025-09-18 at 19:42 +0200, Matthieu Baerts (NGI0) wrote:
> Before this modification, this function was quite long with many
> levels
> of indentations.
> 
> Each case can be split in a dedicated function: fullmesh, non-
> fullmesh.

How about splitting this patch into two, one for 'fullmesh', one for
'non-fullmesh'.

> 
> To remove one level of indentation, msk->pm.subflows >= subflows_max
> is
> now checked after having added one subflow, and stops the loop if it
> is
> no longer possible to add new subflows. This is fine to do this
> because
> this function should only be called if msk->pm.subflows <
> subflows_max.
> 
> No functional changes intended.
> 
> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
> ---
>  net/mptcp/pm_kernel.c | 112 ++++++++++++++++++++++++++++------------
> ----------
>  1 file changed, 63 insertions(+), 49 deletions(-)
> 
> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index
> 5bdcfcc26308841c49375ce35205097f30592279..8d5df9b98589e5cd69f16dc54e9
> 140e88a1835e2 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -159,74 +159,88 @@ select_signal_address(struct pm_nl_pernet
> *pernet, const struct mptcp_sock *msk,
>  	return found;
>  }
>  
> -/* Fill all the remote addresses into the array addrs[],
> - * and return the array size.
> - */
> -static unsigned int fill_remote_addresses_vec(struct mptcp_sock
> *msk,
> -					      struct mptcp_addr_info
> *local,
> -					      bool fullmesh,
> -					      struct mptcp_addr_info
> *addrs)
> +static unsigned int
> +fill_remote_addr(struct mptcp_sock *msk, struct mptcp_addr_info
> *local,
> +		 struct mptcp_addr_info *addrs)
>  {
>  	bool deny_id0 = READ_ONCE(msk->pm.remote_deny_join_id0);
> +	struct sock *sk = (struct sock *)msk;
> +	struct mptcp_addr_info remote = { 0 };

This breaks 'Reverse X-Mas Tree' order.

> +
> +	if (deny_id0)
> +		return 0;
> +
> +	mptcp_remote_address((struct sock_common *)sk, &remote);
> +
> +	if (!mptcp_pm_addr_families_match(sk, local, &remote))
> +		return 0;
> +
> +	msk->pm.subflows++;
> +	*addrs = remote;
> +
> +	return 1;

How about still using:

addrs[i++] = remote;
return i;

here, to keep it consistent with other fill_*_addr helpers.

> +}
> +
> +static unsigned int
> +fill_remote_addresses_fullmesh(struct mptcp_sock *msk,
> +			       struct mptcp_addr_info *local,
> +			       struct mptcp_addr_info *addrs)
> +{
> +	bool deny_id0 = READ_ONCE(msk->pm.remote_deny_join_id0);
> +	DECLARE_BITMAP(unavail_id, MPTCP_PM_MAX_ADDR_ID + 1);
>  	struct sock *sk = (struct sock *)msk, *ssk;
>  	struct mptcp_subflow_context *subflow;
> -	struct mptcp_addr_info remote = { 0 };
>  	unsigned int subflows_max;
>  	int i = 0;
>  
>  	subflows_max = mptcp_pm_get_subflows_max(msk);
> -	mptcp_remote_address((struct sock_common *)sk, &remote);
>  
> -	/* Non-fullmesh endpoint, fill in the single entry
> -	 * corresponding to the primary MPC subflow remote address
> +	/* Forbid creation of new subflows matching existing ones,
> possibly
> +	 * already created by incoming ADD_ADDR
>  	 */
> -	if (!fullmesh) {
> -		if (deny_id0)
> -			return 0;
> +	bitmap_zero(unavail_id, MPTCP_PM_MAX_ADDR_ID + 1);
> +	mptcp_for_each_subflow(msk, subflow)
> +		if (READ_ONCE(subflow->local_id) == local->id)
> +			__set_bit(subflow->remote_id, unavail_id);
>  
> -		if (!mptcp_pm_addr_families_match(sk, local,
> &remote))
> -			return 0;
> +	mptcp_for_each_subflow(msk, subflow) {
> +		ssk = mptcp_subflow_tcp_sock(subflow);
> +		mptcp_remote_address((struct sock_common *)ssk,
> &addrs[i]);
> +		addrs[i].id = READ_ONCE(subflow->remote_id);
> +		if (deny_id0 && !addrs[i].id)
> +			continue;
>  
> +		if (test_bit(addrs[i].id, unavail_id))
> +			continue;
> +
> +		if (!mptcp_pm_addr_families_match(sk, local,
> &addrs[i]))
> +			continue;
> +
> +		/* forbid creating multiple address towards this id
> */
> +		__set_bit(addrs[i].id, unavail_id);
>  		msk->pm.subflows++;
> -		addrs[i++] = remote;
> -	} else {
> -		DECLARE_BITMAP(unavail_id, MPTCP_PM_MAX_ADDR_ID +
> 1);
> +		i++;
>  
> -		/* Forbid creation of new subflows matching existing
> -		 * ones, possibly already created by incoming
> ADD_ADDR
> -		 */
> -		bitmap_zero(unavail_id, MPTCP_PM_MAX_ADDR_ID + 1);
> -		mptcp_for_each_subflow(msk, subflow)
> -			if (READ_ONCE(subflow->local_id) == local-
> >id)
> -				__set_bit(subflow->remote_id,
> unavail_id);
> -
> -		mptcp_for_each_subflow(msk, subflow) {
> -			ssk = mptcp_subflow_tcp_sock(subflow);
> -			mptcp_remote_address((struct sock_common
> *)ssk, &addrs[i]);
> -			addrs[i].id = READ_ONCE(subflow->remote_id);
> -			if (deny_id0 && !addrs[i].id)
> -				continue;
> -
> -			if (test_bit(addrs[i].id, unavail_id))
> -				continue;
> -
> -			if (!mptcp_pm_addr_families_match(sk, local,
> &addrs[i]))
> -				continue;
> -
> -			if (msk->pm.subflows < subflows_max) {
> -				/* forbid creating multiple address
> towards
> -				 * this id
> -				 */
> -				__set_bit(addrs[i].id, unavail_id);
> -				msk->pm.subflows++;
> -				i++;
> -			}
> -		}
> +		if (msk->pm.subflows >= subflows_max)
> +			break;
>  	}
>  
>  	return i;
>  }
>  
> +/* Fill all the remote addresses into the array addrs[],
> + * and return the array size.
> + */
> +static unsigned int
> +fill_remote_addresses_vec(struct mptcp_sock *msk, struct
> mptcp_addr_info *local,
> +			  bool fullmesh, struct mptcp_addr_info
> *addrs)

How about keeping this unchanged:

static unsigned int fill_remote_addresses_vec(struct mptcp_sock
*msk,
					      struct mptcp_addr_info
*local,
					      bool fullmesh,
					      struct mptcp_addr_info
*addrs)

This can make the patch smaller.

> +{

-	/* Non-fullmesh endpoint, fill in the single entry
-	 * corresponding to the primary MPC subflow remote address

We can keep this comment here.

-	if (!fullmesh) {

... and test "!fullmesh" just like the original code.

Thanks,
-Geliang

> +	if (fullmesh)
> +		return fill_remote_addresses_fullmesh(msk, local,
> addrs);
> +
> +	return fill_remote_addr(msk, local, addrs);
> +}
> +
>  static struct mptcp_pm_addr_entry *
>  __lookup_addr_by_id(struct pm_nl_pernet *pernet, unsigned int id)
>  {

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH mptcp-next 01/10] Squash to "mptcp: pm: in-kernel: usable client side with C-flag"
  2025-09-19  3:18   ` Geliang Tang
@ 2025-09-19  8:19     ` Matthieu Baerts
  0 siblings, 0 replies; 18+ messages in thread
From: Matthieu Baerts @ 2025-09-19  8:19 UTC (permalink / raw)
  To: Geliang Tang, MPTCP Upstream

Hi Geliang,

On 19/09/2025 05:18, Geliang Tang wrote:
> Hi Matt,
> 
> Thanks for this patch.

Thank you for the review!

> On Thu, 2025-09-18 at 19:42 +0200, Matthieu Baerts (NGI0) wrote:
>> The id_avail_bitmap is only used when either the 'subflow' or
>> 'signal'
>> flag is used, but not with 'fullmesh' only. Here, it is replacing the
>> 'subflow' action, so check if this flag is set.
> 
> I recall that 'fullmesh' can only be used together with 'subflow'. If
> it has already been determined earlier that the flags contain
> 'fullmesh', shouldn't the flags here already include 'subflow'? I'm not
> entirely certain, but directly checking for 'subflow' as done in this
> patch seems like a better approach.

We have a restriction not to have "fullmesh" with "signal" (even if I
think we should remove it: I think there is no technical reason to
forbid), but we can have "fullmesh" without "subflow".

> So this patch looks good to me.

Thanks!

> As mentioned in my previous comment, if possible, the refactoring of
> the fill_local_addresses_vec_c_flag helper in patch 2 could also be
> added to this squash-to patch.

I would prefer not to.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH mptcp-next 02/10] mptcp: pm: in-kernel: refactor fill_local_addresses_vec
  2025-09-19  6:51   ` Geliang Tang
@ 2025-09-19  8:25     ` Matthieu Baerts
  0 siblings, 0 replies; 18+ messages in thread
From: Matthieu Baerts @ 2025-09-19  8:25 UTC (permalink / raw)
  To: Geliang Tang, MPTCP Upstream

Hi Geliang,

On 19/09/2025 08:51, Geliang Tang wrote:
> Hi Matt,
> 
> On Thu, 2025-09-18 at 19:42 +0200, Matthieu Baerts (NGI0) wrote:
>> Before this modification, this function was quite long with many
>> levels
>> of indentations.
>>
>> Each case can be split in a dedicated function: fullmesh, C flag,
>> any.
> 
> If "C flag" function is squash into patch 1, how about splitting this
> patch into two, one for 'fullmesh' and one for 'any'.

I would prefer not to. I don't think it is really needed to split that
in multiple patches.

(...)

>> +/* Fill all the local addresses into the array addrs[],
>> + * and return the array size.
>> + */
>> +static unsigned int
>> +fill_local_addresses_vec(struct mptcp_sock *msk, struct
>> mptcp_addr_info *remote,
>> +			 struct mptcp_pm_local *locals)
> 
> How about keeping this unchanged:
> 
> static unsigned int fill_local_addresses_vec(struct mptcp_sock *msk,
> 					     struct mptcp_addr_info
> *remote,
> 					     struct mptcp_pm_local
> *locals)
> 
> This can make the patch smaller.

I did these modifications because that's the style I used in the new
functions, plus I think it is better to avoid big indentations.

I don't think we really need to make this patch smaller, it is just a
"pure" refactoring.

> 
>> +{
>> +	bool c_flag_case = remote->id &&
>> mptcp_pm_add_addr_c_flag_case(msk);
>> +	int i;
>> +
>> +	/* If there is at least one MPTCP endpoint with a fullmesh
>> flag */
>> +	i = fill_local_addresses_vec_fullmesh(msk, remote, locals,
>> c_flag_case);
>> +	if (i)
>> +		return i;
>> +
>>  	/* Special case: peer sets the C flag, accept one ADD_ADDR
>> if default
>>  	 * limits are used -- accepting no ADD_ADDR -- and use
>> subflow endpoints
>>  	 */
>> -	if (!i && c_flag_case) {
>> -		unsigned int local_addr_max =
>> mptcp_pm_get_local_addr_max(msk);
>> +	if (c_flag_case)
>> +		return fill_local_addresses_vec_c_flag(msk, remote,
>> locals);
>>  
>> -		while (msk->pm.local_addr_used < local_addr_max &&
>> -		       msk->pm.subflows < subflows_max) {
>> -			struct mptcp_pm_local *local = &locals[i];
>> -
>> -			if (!select_local_address(pernet, msk,
>> local))
>> -				break;
>> -
>> -			__clear_bit(local->addr.id, msk-
>>> pm.id_avail_bitmap);
>> -
>> -			if (!mptcp_pm_addr_families_match(sk,
>> &local->addr,
>> -							  remote))
>> -				continue;
>> -
>> -			if (mptcp_addresses_equal(&local->addr,
>> &mpc_addr,
>> -						  local->addr.port))
>> -				continue;
>> -
>> -			msk->pm.local_addr_used++;
>> -			msk->pm.subflows++;
>> -			i++;
>> -		}
>> -
>> -		return i;
>> -	}
>> -
>> -	/* If the array is empty, fill in the single
>> -	 * 'IPADDRANY' local address
>> -	 */
>> -	if (!i) {
>> -		memset(&locals[i], 0, sizeof(locals[i]));
>> -		locals[i].addr.family =
>> -#if IS_ENABLED(CONFIG_MPTCP_IPV6)
>> -			       remote->family == AF_INET6 &&
>> -			       ipv6_addr_v4mapped(&remote->addr6) ?
>> AF_INET :
>> -#endif
>> -			       remote->family;
>> -
>> -		if (!mptcp_pm_addr_families_match(sk,
>> &locals[i].addr, remote))
>> -			return 0;
>> -
>> -		msk->pm.subflows++;
>> -		i++;
>> -	}
>> -
>> -	return i;
>> +	/* No special case: fill in the single 'IPADDRANY' local
>> address */
>> +	return fill_local_address_any(msk, remote, &locals[0]);
>>  }
>>  
>>  static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
>> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
>> index
>> dd0662defd41c84474e44c559c571e3594b85d9e..0d6dae37c9daf4ec8990b9a8703
>> 6aa393add585c 100644
>> --- a/net/mptcp/protocol.h
>> +++ b/net/mptcp/protocol.h
>> @@ -1200,7 +1200,8 @@ static inline bool
>> mptcp_pm_add_addr_c_flag_case(struct mptcp_sock *msk)
>>  {
>>  	return READ_ONCE(msk->pm.remote_deny_join_id0) &&
>>  	       msk->pm.local_addr_used == 0 &&
>> -	       mptcp_pm_get_add_addr_accept_max(msk) == 0;
>> +	       mptcp_pm_get_add_addr_accept_max(msk) == 0 &&
>> +	       msk->pm.subflows < mptcp_pm_get_subflows_max(msk);
> 
> This needs to be squash to patch 1 too.

It is currently not needed, see this message in the commit message:

> To remove one level of indentation, msk->pm.subflows >= subflows_max is
> now checked upfront.
But I guess it doesn't hurt to move it there, just to be on the safe
side, and avoid this note.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH mptcp-next 03/10] mptcp: pm: in-kernel: refactor fill_remote_addresses_vec
  2025-09-19  6:52   ` Geliang Tang
@ 2025-09-19  8:41     ` Matthieu Baerts
  0 siblings, 0 replies; 18+ messages in thread
From: Matthieu Baerts @ 2025-09-19  8:41 UTC (permalink / raw)
  To: Geliang Tang, MPTCP Upstream

Hi Geliang,

On 19/09/2025 08:52, Geliang Tang wrote:
> Hi Matt,
> 
> On Thu, 2025-09-18 at 19:42 +0200, Matthieu Baerts (NGI0) wrote:
>> Before this modification, this function was quite long with many
>> levels
>> of indentations.
>>
>> Each case can be split in a dedicated function: fullmesh, non-
>> fullmesh.
> 
> How about splitting this patch into two, one for 'fullmesh', one for
> 'non-fullmesh'.

I don't think that's really needed: I'm simply moving what is in the
if-statement in one helper, and the rest in another one. Or do you think
it would really help?

> 
>>
>> To remove one level of indentation, msk->pm.subflows >= subflows_max
>> is
>> now checked after having added one subflow, and stops the loop if it
>> is
>> no longer possible to add new subflows. This is fine to do this
>> because
>> this function should only be called if msk->pm.subflows <
>> subflows_max.
>>
>> No functional changes intended.
>>
>> Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
>> ---
>>  net/mptcp/pm_kernel.c | 112 ++++++++++++++++++++++++++++------------
>> ----------
>>  1 file changed, 63 insertions(+), 49 deletions(-)
>>
>> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
>> index
>> 5bdcfcc26308841c49375ce35205097f30592279..8d5df9b98589e5cd69f16dc54e9
>> 140e88a1835e2 100644
>> --- a/net/mptcp/pm_kernel.c
>> +++ b/net/mptcp/pm_kernel.c
>> @@ -159,74 +159,88 @@ select_signal_address(struct pm_nl_pernet
>> *pernet, const struct mptcp_sock *msk,
>>  	return found;
>>  }
>>  
>> -/* Fill all the remote addresses into the array addrs[],
>> - * and return the array size.
>> - */
>> -static unsigned int fill_remote_addresses_vec(struct mptcp_sock
>> *msk,
>> -					      struct mptcp_addr_info
>> *local,
>> -					      bool fullmesh,
>> -					      struct mptcp_addr_info
>> *addrs)
>> +static unsigned int
>> +fill_remote_addr(struct mptcp_sock *msk, struct mptcp_addr_info
>> *local,
>> +		 struct mptcp_addr_info *addrs)
>>  {
>>  	bool deny_id0 = READ_ONCE(msk->pm.remote_deny_join_id0);
>> +	struct sock *sk = (struct sock *)msk;
>> +	struct mptcp_addr_info remote = { 0 };
> 
> This breaks 'Reverse X-Mas Tree' order.

Good catch, fixed!

> 
>> +
>> +	if (deny_id0)
>> +		return 0;
>> +
>> +	mptcp_remote_address((struct sock_common *)sk, &remote);
>> +
>> +	if (!mptcp_pm_addr_families_match(sk, local, &remote))
>> +		return 0;
>> +
>> +	msk->pm.subflows++;
>> +	*addrs = remote;
>> +
>> +	return 1;
> 
> How about still using:
> 
> addrs[i++] = remote;
> return i;
> 
> here, to keep it consistent with other fill_*_addr helpers.

We will only fill one address, then no need to declare 'i', no?
It looks clearer to me like that, so we clearly understand only one
address is filled, no?

> 
>> +}
>> +
>> +static unsigned int
>> +fill_remote_addresses_fullmesh(struct mptcp_sock *msk,
>> +			       struct mptcp_addr_info *local,
>> +			       struct mptcp_addr_info *addrs)
>> +{
>> +	bool deny_id0 = READ_ONCE(msk->pm.remote_deny_join_id0);
>> +	DECLARE_BITMAP(unavail_id, MPTCP_PM_MAX_ADDR_ID + 1);
>>  	struct sock *sk = (struct sock *)msk, *ssk;
>>  	struct mptcp_subflow_context *subflow;
>> -	struct mptcp_addr_info remote = { 0 };
>>  	unsigned int subflows_max;
>>  	int i = 0;
>>  
>>  	subflows_max = mptcp_pm_get_subflows_max(msk);
>> -	mptcp_remote_address((struct sock_common *)sk, &remote);
>>  
>> -	/* Non-fullmesh endpoint, fill in the single entry
>> -	 * corresponding to the primary MPC subflow remote address
>> +	/* Forbid creation of new subflows matching existing ones,
>> possibly
>> +	 * already created by incoming ADD_ADDR
>>  	 */
>> -	if (!fullmesh) {
>> -		if (deny_id0)
>> -			return 0;
>> +	bitmap_zero(unavail_id, MPTCP_PM_MAX_ADDR_ID + 1);
>> +	mptcp_for_each_subflow(msk, subflow)
>> +		if (READ_ONCE(subflow->local_id) == local->id)
>> +			__set_bit(subflow->remote_id, unavail_id);
>>  
>> -		if (!mptcp_pm_addr_families_match(sk, local,
>> &remote))
>> -			return 0;
>> +	mptcp_for_each_subflow(msk, subflow) {
>> +		ssk = mptcp_subflow_tcp_sock(subflow);
>> +		mptcp_remote_address((struct sock_common *)ssk,
>> &addrs[i]);
>> +		addrs[i].id = READ_ONCE(subflow->remote_id);
>> +		if (deny_id0 && !addrs[i].id)
>> +			continue;
>>  
>> +		if (test_bit(addrs[i].id, unavail_id))
>> +			continue;
>> +
>> +		if (!mptcp_pm_addr_families_match(sk, local,
>> &addrs[i]))
>> +			continue;
>> +
>> +		/* forbid creating multiple address towards this id
>> */
>> +		__set_bit(addrs[i].id, unavail_id);
>>  		msk->pm.subflows++;
>> -		addrs[i++] = remote;
>> -	} else {
>> -		DECLARE_BITMAP(unavail_id, MPTCP_PM_MAX_ADDR_ID +
>> 1);
>> +		i++;
>>  
>> -		/* Forbid creation of new subflows matching existing
>> -		 * ones, possibly already created by incoming
>> ADD_ADDR
>> -		 */
>> -		bitmap_zero(unavail_id, MPTCP_PM_MAX_ADDR_ID + 1);
>> -		mptcp_for_each_subflow(msk, subflow)
>> -			if (READ_ONCE(subflow->local_id) == local-
>>> id)
>> -				__set_bit(subflow->remote_id,
>> unavail_id);
>> -
>> -		mptcp_for_each_subflow(msk, subflow) {
>> -			ssk = mptcp_subflow_tcp_sock(subflow);
>> -			mptcp_remote_address((struct sock_common
>> *)ssk, &addrs[i]);
>> -			addrs[i].id = READ_ONCE(subflow->remote_id);
>> -			if (deny_id0 && !addrs[i].id)
>> -				continue;
>> -
>> -			if (test_bit(addrs[i].id, unavail_id))
>> -				continue;
>> -
>> -			if (!mptcp_pm_addr_families_match(sk, local,
>> &addrs[i]))
>> -				continue;
>> -
>> -			if (msk->pm.subflows < subflows_max) {
>> -				/* forbid creating multiple address
>> towards
>> -				 * this id
>> -				 */
>> -				__set_bit(addrs[i].id, unavail_id);
>> -				msk->pm.subflows++;
>> -				i++;
>> -			}
>> -		}
>> +		if (msk->pm.subflows >= subflows_max)
>> +			break;
>>  	}
>>  
>>  	return i;
>>  }
>>  
>> +/* Fill all the remote addresses into the array addrs[],
>> + * and return the array size.
>> + */
>> +static unsigned int
>> +fill_remote_addresses_vec(struct mptcp_sock *msk, struct
>> mptcp_addr_info *local,
>> +			  bool fullmesh, struct mptcp_addr_info
>> *addrs)
> 
> How about keeping this unchanged:
> 
> static unsigned int fill_remote_addresses_vec(struct mptcp_sock
> *msk,
> 					      struct mptcp_addr_info
> *local,
> 					      bool fullmesh,
> 					      struct mptcp_addr_info
> *addrs)
> 
> This can make the patch smaller.

I don't think that's needed, for the same reason.

> 
>> +{
> 
> -	/* Non-fullmesh endpoint, fill in the single entry
> -	 * corresponding to the primary MPC subflow remote address
> 
> We can keep this comment here.

Indeed.

> 
> -	if (!fullmesh) {
> 
> ... and test "!fullmesh" just like the original code.

It looks "strange" to treat the opposite condition first, but if I leave
the comment, I can keep '!fullmesh' I guess.

> 
> Thanks,
> -Geliang
> 
>> +	if (fullmesh)
>> +		return fill_remote_addresses_fullmesh(msk, local,
>> addrs);
>> +
>> +	return fill_remote_addr(msk, local, addrs);
>> +}
>> +
>>  static struct mptcp_pm_addr_entry *
>>  __lookup_addr_by_id(struct pm_nl_pernet *pernet, unsigned int id)
>>  {

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2025-09-19  8:41 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-18 17:42 [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups Matthieu Baerts (NGI0)
2025-09-18 17:42 ` [PATCH mptcp-next 01/10] Squash to "mptcp: pm: in-kernel: usable client side with C-flag" Matthieu Baerts (NGI0)
2025-09-19  3:18   ` Geliang Tang
2025-09-19  8:19     ` Matthieu Baerts
2025-09-18 17:42 ` [PATCH mptcp-next 02/10] mptcp: pm: in-kernel: refactor fill_local_addresses_vec Matthieu Baerts (NGI0)
2025-09-19  6:51   ` Geliang Tang
2025-09-19  8:25     ` Matthieu Baerts
2025-09-18 17:42 ` [PATCH mptcp-next 03/10] mptcp: pm: in-kernel: refactor fill_remote_addresses_vec Matthieu Baerts (NGI0)
2025-09-19  6:52   ` Geliang Tang
2025-09-19  8:41     ` Matthieu Baerts
2025-09-18 17:42 ` [PATCH mptcp-next 04/10] mptcp: pm: rename 'subflows' to 'extra_subflows' Matthieu Baerts (NGI0)
2025-09-18 17:42 ` [PATCH mptcp-next 05/10] mptcp: pm: in-kernel: rename 'subflows_max' to 'limit_extra_subflows' Matthieu Baerts (NGI0)
2025-09-18 17:42 ` [PATCH mptcp-next 06/10] mptcp: pm: in-kernel: rename 'add_addr_signal_max' to 'endp_signal_max' Matthieu Baerts (NGI0)
2025-09-18 17:42 ` [PATCH mptcp-next 07/10] mptcp: pm: in-kernel: rename 'add_addr_accept_max' to 'limit_add_addr_accepted' Matthieu Baerts (NGI0)
2025-09-18 17:42 ` [PATCH mptcp-next 08/10] mptcp: pm: in-kernel: rename 'local_addr_max' to 'endp_subflow_max' Matthieu Baerts (NGI0)
2025-09-18 17:42 ` [PATCH mptcp-next 09/10] mptcp: pm: in-kernel: rename 'local_addr_list' to 'endp_list' Matthieu Baerts (NGI0)
2025-09-18 17:42 ` [PATCH mptcp-next 10/10] mptcp: pm: in-kernel: rename 'addrs' to 'endpoints' Matthieu Baerts (NGI0)
2025-09-18 19:29 ` [PATCH mptcp-next 00/10] mptcp: pm: a few cleanups MPTCP CI

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.