[PATCH mptcp-next v8 07/12] mptcp: pm: userspace: register mptcp_pm_userspace

Geliang Tang posted 12 patches 11 months, 1 week ago
There is a newer version of this series
[PATCH mptcp-next v8 07/12] mptcp: pm: userspace: register mptcp_pm_userspace
Posted by Geliang Tang 11 months, 1 week ago
From: Geliang Tang <tanggeliang@kylinos.cn>

This patch defines the original userspace path manager as a new
struct mptcp_pm_ops named "mptcp_userspace_pm", and register it
in mptcp_pm_init(). mptcp_userspace_pm_is_release() is a wrapper
of mptcp_userspace_pm_free_local_addr_list().

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm.c           |  1 +
 net/mptcp/pm_userspace.c | 26 ++++++++++++++++++++++++++
 net/mptcp/protocol.h     |  1 +
 3 files changed, 28 insertions(+)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 28ea8bdaa8b0..5018ed3c575f 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -1028,6 +1028,7 @@ void mptcp_pm_data_init(struct mptcp_sock *msk)
 void __init mptcp_pm_init(void)
 {
 	mptcp_pm_kernel_register();
+	mptcp_pm_userspace_register();
 	mptcp_pm_nl_init();
 }
 
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 13856df22673..412d6c912148 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -682,3 +682,29 @@ int mptcp_userspace_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
 	sock_put(sk);
 	return ret;
 }
+
+static void mptcp_userspace_pm_init(struct mptcp_sock *msk)
+{
+	struct mptcp_pm_data *pm = &msk->pm;
+
+	WRITE_ONCE(pm->work_pending, 0);
+	WRITE_ONCE(pm->accept_addr, 0);
+	WRITE_ONCE(pm->accept_subflow, 0);
+}
+
+static void mptcp_userspace_pm_release(struct mptcp_sock *msk)
+{
+	mptcp_userspace_pm_free_local_addr_list(msk);
+}
+
+static struct mptcp_pm_ops mptcp_pm_userspace = {
+	.init			= mptcp_userspace_pm_init,
+	.release		= mptcp_userspace_pm_release,
+	.name			= "userspace",
+	.owner			= THIS_MODULE,
+};
+
+void __init mptcp_pm_userspace_register(void)
+{
+	mptcp_pm_register(&mptcp_pm_userspace);
+}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index f700cb55bf49..658bc60d4cd8 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1162,6 +1162,7 @@ static inline u8 subflow_get_local_id(const struct mptcp_subflow_context *subflo
 }
 
 void __init mptcp_pm_kernel_register(void);
+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);
-- 
2.43.0
Re: [PATCH mptcp-next v8 07/12] mptcp: pm: userspace: register mptcp_pm_userspace
Posted by Matthieu Baerts 11 months, 1 week ago
Hi Geliang,

On 04/03/2025 12:40, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch defines the original userspace path manager as a new
> struct mptcp_pm_ops named "mptcp_userspace_pm", and register it
> in mptcp_pm_init(). mptcp_userspace_pm_is_release() is a wrapper
> of mptcp_userspace_pm_free_local_addr_list().

(...)

> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 13856df22673..412d6c912148 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -682,3 +682,29 @@ int mptcp_userspace_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
>  	sock_put(sk);
>  	return ret;
>  }
> +
> +static void mptcp_userspace_pm_init(struct mptcp_sock *msk)
> +{
> +	struct mptcp_pm_data *pm = &msk->pm;
> +
> +	WRITE_ONCE(pm->work_pending, 0);
> +	WRITE_ONCE(pm->accept_addr, 0);
> +	WRITE_ONCE(pm->accept_subflow, 0);

I would not do that here: these variables are not used by the userspace
PM, that doesn't make sense for this PM to do that. I think these
variables should be reset all the time, then the init callback is called
if set.

In other words, only the in-kernel PM needs this init callback.

> +}
> +
> +static void mptcp_userspace_pm_release(struct mptcp_sock *msk)
> +{
> +	mptcp_userspace_pm_free_local_addr_list(msk);
> +}

Same as in patch 6, I would also move this release part in patch 8
("mptcp: pm: initialize and release mptcp_pm_ops"), to show that you are
moving code. Otherwise, it is difficult to know from where it comes from.

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