From: Geliang Tang <tanggeliang@kylinos.cn>

The helper mptcp_pm_is_userspace() is used to distinguish userspace PM
operations from in-kernel PM in mptcp_pm_allow_new_subflow(). It seems
reasonable to add a mandatory .allow_new_subflow interface for struct
mptcp_pm_ops.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 include/net/mptcp.h   |  3 +++
 net/mptcp/pm.c        | 13 ++-----------
 net/mptcp/pm_kernel.c | 21 +++++++++++++++++++++
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 8c1ac7368693..aedabc7f4190 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -135,6 +135,9 @@ struct mptcp_pm_ops {
 			      const struct mptcp_addr_info *addr);
 	bool (*accept_new_subflow)(const struct mptcp_sock *msk);
 
+	/* optional */
+	bool (*allow_new_subflow)(struct mptcp_sock *msk);
+
 	char			name[MPTCP_PM_NAME_MAX];
 	struct module		*owner;
 	struct list_head	list;
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index ca105bbd03ea..215b3a4d24be 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -453,23 +453,14 @@ 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;
 	bool ret = true;
 
 	if (!pm->ops->accept_new_subflow(msk))
 		return false;
 
 	spin_lock_bh(&pm->lock);
-	if (!mptcp_pm_is_userspace(msk) && READ_ONCE(pm->accept_subflow)) {
-		subflows_max = mptcp_pm_get_subflows_max(msk);
-
-		pr_debug("msk=%p subflows=%d max=%d allow=%d\n", msk, pm->subflows,
-			 subflows_max, READ_ONCE(pm->accept_subflow));
-
-		ret = pm->subflows < subflows_max;
-		if (ret && pm->subflows == subflows_max - 1)
-			WRITE_ONCE(pm->accept_subflow, false);
-	}
+	if (pm->ops->allow_new_subflow)
+		ret = pm->ops->allow_new_subflow(msk);
 	if (ret)
 		pm->subflows++;
 	spin_unlock_bh(&pm->lock);
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 7ec8fafeda0e..5ae64392a316 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -1409,6 +1409,26 @@ static bool mptcp_pm_kernel_accept_new_subflow(const struct mptcp_sock *msk)
 	return READ_ONCE(msk->pm.accept_subflow);
 }
 
+static bool mptcp_pm_kernel_allow_new_subflow(struct mptcp_sock *msk)
+{
+	struct mptcp_pm_data *pm = &msk->pm;
+	unsigned int subflows_max;
+	bool ret = false;
+
+	subflows_max = mptcp_pm_get_subflows_max(msk);
+
+	pr_debug("msk=%p subflows=%d max=%d allow=%d\n", msk, pm->subflows,
+		 subflows_max, READ_ONCE(pm->accept_subflow));
+
+	if (READ_ONCE(pm->accept_subflow)) {
+		ret = pm->subflows < subflows_max;
+		if (ret && pm->subflows == subflows_max - 1)
+			WRITE_ONCE(pm->accept_subflow, false);
+	}
+
+	return ret;
+}
+
 static void mptcp_pm_kernel_init(struct mptcp_sock *msk)
 {
 	bool subflows_allowed = !!mptcp_pm_get_subflows_max(msk);
@@ -1438,6 +1458,7 @@ struct mptcp_pm_ops mptcp_pm_kernel = {
 	.rm_addr_received	= mptcp_pm_kernel_rm_addr_received,
 	.add_addr_echo		= mptcp_pm_kernel_add_addr_echo,
 	.accept_new_subflow	= mptcp_pm_kernel_accept_new_subflow,
+	.allow_new_subflow	= mptcp_pm_kernel_allow_new_subflow,
 	.init			= mptcp_pm_kernel_init,
 	.name			= "kernel",
 	.owner			= THIS_MODULE,
-- 
2.43.0

Hi Geliang,

On 24/03/2025 09:19, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> The helper mptcp_pm_is_userspace() is used to distinguish userspace PM
> operations from in-kernel PM in mptcp_pm_allow_new_subflow(). It seems
> reasonable to add a mandatory .allow_new_subflow interface for struct
> mptcp_pm_ops.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  include/net/mptcp.h   |  3 +++
>  net/mptcp/pm.c        | 13 ++-----------
>  net/mptcp/pm_kernel.c | 21 +++++++++++++++++++++
>  3 files changed, 26 insertions(+), 11 deletions(-)
> 
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index 8c1ac7368693..aedabc7f4190 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -135,6 +135,9 @@ struct mptcp_pm_ops {
>  			      const struct mptcp_addr_info *addr);
>  	bool (*accept_new_subflow)(const struct mptcp_sock *msk);
>  
> +	/* optional */
> +	bool (*allow_new_subflow)(struct mptcp_sock *msk);

Maybe not needed, see my comment in patch 8/9.

> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index ca105bbd03ea..215b3a4d24be 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -453,23 +453,14 @@ 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;
>  	bool ret = true;
>  
>  	if (!pm->ops->accept_new_subflow(msk))
>  		return false;
>  
>  	spin_lock_bh(&pm->lock);
> -	if (!mptcp_pm_is_userspace(msk) && READ_ONCE(pm->accept_subflow)) {
> -		subflows_max = mptcp_pm_get_subflows_max(msk);
> -
> -		pr_debug("msk=%p subflows=%d max=%d allow=%d\n", msk, pm->subflows,
> -			 subflows_max, READ_ONCE(pm->accept_subflow));
> -
> -		ret = pm->subflows < subflows_max;
> -		if (ret && pm->subflows == subflows_max - 1)
> -			WRITE_ONCE(pm->accept_subflow, false);
> -	}
> +	if (pm->ops->allow_new_subflow)
> +		ret = pm->ops->allow_new_subflow(msk);

From what I understood, callbacks should not be called under a lock. But
maybe this new callback is not needed, see my comment on patch 8/9.

>  	if (ret)
>  		pm->subflows++;
>  	spin_unlock_bh(&pm->lock);

(...)

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