diff mbox series

[mptcp-next,v10,05/12] mptcp: pm: init and release mptcp_pm_ops

Message ID e1a69a1ee9ca25f12a244e5470341c23364de424.1741258415.git.tanggeliang@kylinos.cn (mailing list archive)
State Superseded
Commit 9c827852b0571eca7e43528ccfa781a75e2c91be
Headers show
Series BPF path manager, part 5 | expand

Checks

Context Check Description
matttbe/checkpatch success total: 0 errors, 0 warnings, 0 checks, 119 lines checked
matttbe/shellcheck success MPTCP selftests files have not been modified
matttbe/build success Build and static analysis OK
matttbe/KVM_Validation__normal success Success! ✅
matttbe/KVM_Validation__debug success Success! ✅
matttbe/KVM_Validation__btf-normal__only_bpftest_all_ success Success! ✅
matttbe/KVM_Validation__btf-debug__only_bpftest_all_ success Success! ✅

Commit Message

Geliang Tang March 6, 2025, 11:01 a.m. UTC 
From: Geliang Tang <tanggeliang@kylinos.cn>

Add a struct mptcp_pm_ops pointer "ops" in struct mptcp_pm_data, and two
helpers mptcp_pm_ops_init() and mptcp_pm_ops_release(), to set and release
this pointer. mptcp_pm_ops_init() is invoked in mptcp_pm_data_reset(),
while mptcp_pm_ops_release() is invoked in mptcp_pm_destroy().

mptcp_pm_kernel_init() is defined as init() interface of the in-kernel PM,
which sets work_pending, accept_addr and accept_subflow.

mptcp_pm_userspace_is_release() is defined as release() interface of the
userspace PM, which is a wrapper of userspace_pm_free_local_addr_list().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm.c           | 55 ++++++++++++++++++++++++++--------------
 net/mptcp/pm_kernel.c    | 19 ++++++++++++++
 net/mptcp/pm_userspace.c |  6 +++++
 net/mptcp/protocol.h     |  1 +
 4 files changed, 62 insertions(+), 19 deletions(-)

Comments

Matthieu Baerts (NGI0) March 10, 2025, 11:18 p.m. UTC | #1 
Hi Geliang,

On 06/03/2025 12:01, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Add a struct mptcp_pm_ops pointer "ops" in struct mptcp_pm_data, and two
> helpers mptcp_pm_ops_init() and mptcp_pm_ops_release(), to set and release
> this pointer. mptcp_pm_ops_init() is invoked in mptcp_pm_data_reset(),
> while mptcp_pm_ops_release() is invoked in mptcp_pm_destroy().
> 
> mptcp_pm_kernel_init() is defined as init() interface of the in-kernel PM,
> which sets work_pending, accept_addr and accept_subflow.
> 
> mptcp_pm_userspace_is_release() is defined as release() interface of the
> userspace PM, which is a wrapper of userspace_pm_free_local_addr_list().
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm.c           | 55 ++++++++++++++++++++++++++--------------
>  net/mptcp/pm_kernel.c    | 19 ++++++++++++++
>  net/mptcp/pm_userspace.c |  6 +++++
>  net/mptcp/protocol.h     |  1 +
>  4 files changed, 62 insertions(+), 19 deletions(-)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 1400bfed4b0d..1a2f1715871c 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c

(...)

>  void mptcp_pm_data_reset(struct mptcp_sock *msk)
>  {
> -	u8 pm_type = mptcp_get_pm_type(sock_net((struct sock *)msk));
> +	const struct net *net = sock_net((struct sock *)msk);
> +	const char *pm_name = mptcp_get_path_manager(net);
> +	u8 pm_type = mptcp_get_pm_type(net);
>  	struct mptcp_pm_data *pm = &msk->pm;
>  
>  	memset(&pm->reset, 0, sizeof(pm->reset));
>  	WRITE_ONCE(pm->pm_type, pm_type);
>  
> -	if (pm_type == MPTCP_PM_TYPE_KERNEL) {
> -		bool subflows_allowed = !!mptcp_pm_get_subflows_max(msk);
> -
> -		/* pm->work_pending must be only be set to 'true' when
> -		 * pm->pm_type is set to MPTCP_PM_TYPE_KERNEL
> -		 */
> -		WRITE_ONCE(pm->work_pending,
> -			   (!!mptcp_pm_get_local_addr_max(msk) &&
> -			    subflows_allowed) ||
> -			   !!mptcp_pm_get_add_addr_signal_max(msk));
> -		WRITE_ONCE(pm->accept_addr,
> -			   !!mptcp_pm_get_add_addr_accept_max(msk) &&
> -			   subflows_allowed);
> -		WRITE_ONCE(pm->accept_subflow, subflows_allowed);
> -	}
> +	rcu_read_lock();
> +	mptcp_pm_ops_init(msk, mptcp_pm_find(pm_name));
> +	rcu_read_unlock();
>  
>  	bitmap_fill(pm->id_avail_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);

I just noticed that this bitmap is currently only used by the in-kernel
PM, so this could be moved to mptcp_pm_kernel_init() as well, no?

I guess we will not need this bitmap for the future BPF PM, because
future BPF PM can use a BPF map instead if they need that I suppose, no?

Cheers,
Matt
diff mbox series

Patch

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 1400bfed4b0d..1a2f1715871c 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -970,37 +970,54 @@  void mptcp_pm_worker(struct mptcp_sock *msk)
 	spin_unlock_bh(&msk->pm.lock);
 }
 
+static void mptcp_pm_ops_init(struct mptcp_sock *msk,
+			      struct mptcp_pm_ops *pm_ops)
+{
+	if (!pm_ops || !bpf_try_module_get(pm_ops, pm_ops->owner)) {
+		pr_warn_once("pm %s fails, fallback to default pm",
+			     pm_ops->name);
+		pm_ops = &mptcp_pm_kernel;
+	}
+
+	msk->pm.ops = pm_ops;
+	if (msk->pm.ops->init)
+		msk->pm.ops->init(msk);
+
+	pr_debug("pm %s initialized\n", pm_ops->name);
+}
+
+static void mptcp_pm_ops_release(struct mptcp_sock *msk)
+{
+	struct mptcp_pm_ops *pm_ops = msk->pm.ops;
+
+	msk->pm.ops = NULL;
+	if (pm_ops->release)
+		pm_ops->release(msk);
+
+	bpf_module_put(pm_ops, pm_ops->owner);
+
+	pr_debug("pm %s released\n", pm_ops->name);
+}
+
 void mptcp_pm_destroy(struct mptcp_sock *msk)
 {
 	mptcp_pm_free_anno_list(msk);
-
-	if (mptcp_pm_is_userspace(msk))
-		mptcp_userspace_pm_free_local_addr_list(msk);
+	mptcp_pm_ops_release(msk);
 }
 
 void mptcp_pm_data_reset(struct mptcp_sock *msk)
 {
-	u8 pm_type = mptcp_get_pm_type(sock_net((struct sock *)msk));
+	const struct net *net = sock_net((struct sock *)msk);
+	const char *pm_name = mptcp_get_path_manager(net);
+	u8 pm_type = mptcp_get_pm_type(net);
 	struct mptcp_pm_data *pm = &msk->pm;
 
 	memset(&pm->reset, 0, sizeof(pm->reset));
 	WRITE_ONCE(pm->pm_type, pm_type);
 
-	if (pm_type == MPTCP_PM_TYPE_KERNEL) {
-		bool subflows_allowed = !!mptcp_pm_get_subflows_max(msk);
-
-		/* pm->work_pending must be only be set to 'true' when
-		 * pm->pm_type is set to MPTCP_PM_TYPE_KERNEL
-		 */
-		WRITE_ONCE(pm->work_pending,
-			   (!!mptcp_pm_get_local_addr_max(msk) &&
-			    subflows_allowed) ||
-			   !!mptcp_pm_get_add_addr_signal_max(msk));
-		WRITE_ONCE(pm->accept_addr,
-			   !!mptcp_pm_get_add_addr_accept_max(msk) &&
-			   subflows_allowed);
-		WRITE_ONCE(pm->accept_subflow, subflows_allowed);
-	}
+	rcu_read_lock();
+	mptcp_pm_ops_init(msk, mptcp_pm_find(pm_name));
+	rcu_read_unlock();
 
 	bitmap_fill(pm->id_avail_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
 }
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index d39e7c178460..03c4584cca7a 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -1398,7 +1398,26 @@  static struct pernet_operations mptcp_pm_pernet_ops = {
 	.size = sizeof(struct pm_nl_pernet),
 };
 
+static void mptcp_pm_kernel_init(struct mptcp_sock *msk)
+{
+	bool subflows_allowed = !!mptcp_pm_get_subflows_max(msk);
+	struct mptcp_pm_data *pm = &msk->pm;
+
+	/* pm->work_pending must be only be set to 'true' when
+	 * pm->pm_type is set to MPTCP_PM_TYPE_KERNEL
+	 */
+	WRITE_ONCE(pm->work_pending,
+		   (!!mptcp_pm_get_local_addr_max(msk) &&
+		    subflows_allowed) ||
+		   !!mptcp_pm_get_add_addr_signal_max(msk));
+	WRITE_ONCE(pm->accept_addr,
+		   !!mptcp_pm_get_add_addr_accept_max(msk) &&
+		   subflows_allowed);
+	WRITE_ONCE(pm->accept_subflow, subflows_allowed);
+}
+
 struct mptcp_pm_ops mptcp_pm_kernel = {
+	.init			= mptcp_pm_kernel_init,
 	.name			= "kernel",
 	.owner			= THIS_MODULE,
 };
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 2cb62f026b1f..6afab631580f 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -683,7 +683,13 @@  int mptcp_userspace_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
 	return ret;
 }
 
+static void mptcp_pm_userspace_release(struct mptcp_sock *msk)
+{
+	mptcp_userspace_pm_free_local_addr_list(msk);
+}
+
 static struct mptcp_pm_ops mptcp_pm_userspace = {
+	.release		= mptcp_pm_userspace_release,
 	.name			= "userspace",
 	.owner			= THIS_MODULE,
 };
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 47710db243f4..fa9bb385feb9 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -220,6 +220,7 @@  struct mptcp_pm_data {
 	struct mptcp_addr_info remote;
 	struct list_head anno_list;
 	struct list_head userspace_pm_local_addr_list;
+	struct mptcp_pm_ops *ops;
 
 	spinlock_t	lock;		/*protects the whole PM data */