All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH mptcp-next v7 00/11] BPF path manager, part 5
@ 2025-03-03  4:22 Geliang Tang
  2025-03-03  4:22 ` [PATCH mptcp-next v7 01/11] mptcp: pm: define struct mptcp_pm_ops Geliang Tang
                   ` (12 more replies)
  0 siblings, 13 replies; 24+ messages in thread
From: Geliang Tang @ 2025-03-03  4:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

v7:
 - addresss Matt's comments in v6 [1].
 - drop "type" from struct mptcp_pm_ops as Matt suggested.
 - map "pm_type" to new sysctl as Matt suggested.

Depends on:
 - mptcp: pm: code reorganisation, v2

Based-on: <20250228-mptcp-pm-reorg-code-v2-0-fa8b2542b7a5@kernel.org>

[1]
https://patchwork.kernel.org/project/mptcp/cover/cover.1740320007.git.tanggeliang@kylinos.cn/

v6:
 - add "name" in struct mptcp_pm_ops.
 - add some "sysctl" patches.
 - drop "struct mptcp_pm_param".
 - drop "pm_type" in mptcp_pm_data.

v5:
 - use "struct mptcp_pm_param *param" as unified parameters for all
   interfaces.
 - register in-kernel mptcp_pm_ops too.
 - only implement two interfaces "get_local_id" and "get_priority" in
   this set.

v4:
 - include a new patch "define BPF path manager type".

 - add new interfaces:
	created established closed
	listerner_created listener_closed

 - rename interfaces as:
	address_announced address_removed
	subflow_established subflow_closed
	get_priority set_priority

 - rename functions as:
	mptcp_pm_validate
	mptcp_pm_register
	mptcp_pm_unregister
	mptcp_pm_initialize
	mptcp_pm_release

v3:
 - rename the 2nd parameter of get_local_id() from 'local' to 'skc'.
 - keep the 'msk_sport' check in mptcp_userspace_pm_get_local_id().
 - return 'err' instead of '0' in userspace_pm_subflow_create().
 - drop 'ret' variable inmptcp_pm_data_reset().
 - fix typos in commit log.

v2:
 - update get_local_id interface in patch 2.

get_addr() and dump_addr() interfaces of BPF userspace pm are dropped
as Matt suggested.

In order to implement BPF path manager, it's necessary to unify the
interfaces of the path manager. This set contains some cleanups and
refactoring to unify the interfaces in kernel space. Finally, define
a struct mptcp_pm_ops for a path manager.

Geliang Tang (11):
  mptcp: pm: define struct mptcp_pm_ops
  mptcp: sysctl: new sysctl to set path manager by name
  mptcp: sysctl: map pm_type to path_manager
  mptcp: sysctl: add available_path_managers
  mptcp: pm: in-kernel: register mptcp_kernel_pm
  mptcp: pm: userspace: register mptcp_userspace_pm
  mptcp: pm: initialize and release mptcp_pm_ops
  mptcp: pm: drop pm_type in mptcp_pm_data
  mptcp: sysctl: drop get_pm_type helper
  mptcp: pm: make get_local_id helpers static
  mptcp: pm: make is_backup helpers static

 Documentation/networking/mptcp-sysctl.rst |  26 +++++
 include/net/mptcp.h                       |  19 ++++
 net/mptcp/ctrl.c                          | 107 ++++++++++++++++-
 net/mptcp/pm.c                            | 133 +++++++++++++++++++---
 net/mptcp/pm_kernel.c                     |  16 ++-
 net/mptcp/pm_userspace.c                  |  26 ++++-
 net/mptcp/protocol.h                      |  25 ++--
 7 files changed, 317 insertions(+), 35 deletions(-)

-- 
2.43.0


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

* [PATCH mptcp-next v7 01/11] mptcp: pm: define struct mptcp_pm_ops
  2025-03-03  4:22 [PATCH mptcp-next v7 00/11] BPF path manager, part 5 Geliang Tang
@ 2025-03-03  4:22 ` Geliang Tang
  2025-03-03 10:39   ` Matthieu Baerts
  2025-03-03  4:22 ` [PATCH mptcp-next v7 02/11] mptcp: sysctl: new sysctl to set path manager by name Geliang Tang
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 24+ messages in thread
From: Geliang Tang @ 2025-03-03  4:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

In order to allow users to develop their own BPF-based path manager,
this patch defines a struct ops "mptcp_pm_ops" for a userspace path
manager, which contains a set of interfaces.

Add a set of functions to register, unregister, find and validate a
given struct ops.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 include/net/mptcp.h  | 17 ++++++++++++++
 net/mptcp/pm.c       | 55 ++++++++++++++++++++++++++++++++++++++++++++
 net/mptcp/protocol.h |  5 ++++
 3 files changed, 77 insertions(+)

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 72d6e6597add..53e67b90c37a 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -14,6 +14,7 @@
 
 struct mptcp_info;
 struct mptcp_sock;
+struct mptcp_pm_addr_entry;
 struct seq_file;
 
 /* MPTCP sk_buff extension data */
@@ -121,6 +122,22 @@ struct mptcp_sched_ops {
 	void (*release)(struct mptcp_sock *msk);
 } ____cacheline_aligned_in_smp;
 
+#define MPTCP_PM_NAME_MAX	16
+
+struct mptcp_pm_ops {
+	int (*get_local_id)(struct mptcp_sock *msk,
+			    struct mptcp_pm_addr_entry *skc);
+	bool (*get_priority)(struct mptcp_sock *msk,
+			     struct mptcp_addr_info *skc);
+
+	char			name[MPTCP_PM_NAME_MAX];
+	struct module		*owner;
+	struct list_head	list;
+
+	void (*init)(struct mptcp_sock *msk);
+	void (*release)(struct mptcp_sock *msk);
+} ____cacheline_aligned_in_smp;
+
 #ifdef CONFIG_MPTCP
 void mptcp_init(void);
 
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 833839d7286e..53a29adf7cae 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -5,6 +5,8 @@
  */
 #define pr_fmt(fmt) "MPTCP: " fmt
 
+#include <linux/rculist.h>
+#include <linux/spinlock.h>
 #include "protocol.h"
 #include "mib.h"
 
@@ -18,6 +20,9 @@ struct mptcp_pm_add_entry {
 	struct mptcp_sock	*sock;
 };
 
+static DEFINE_SPINLOCK(mptcp_pm_list_lock);
+static LIST_HEAD(mptcp_pm_list);
+
 /* path manager helpers */
 
 /* if sk is ipv4 or ipv6_only allows only same-family local and remote addresses,
@@ -1024,3 +1029,53 @@ void __init mptcp_pm_init(void)
 {
 	mptcp_pm_nl_init();
 }
+
+/* Must be called with rcu read lock held */
+struct mptcp_pm_ops *mptcp_pm_find(const char *name)
+{
+	struct mptcp_pm_ops *pm;
+
+	list_for_each_entry_rcu(pm, &mptcp_pm_list, list) {
+		if (!strcmp(pm->name, name))
+			return pm;
+	}
+
+	return NULL;
+}
+
+int mptcp_pm_validate(struct mptcp_pm_ops *pm)
+{
+	if (!pm->get_local_id || !pm->get_priority) {
+		pr_err("%s does not implement required ops\n", pm->name);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+int mptcp_pm_register(struct mptcp_pm_ops *pm)
+{
+	int ret;
+
+	ret = mptcp_pm_validate(pm);
+	if (ret)
+		return ret;
+
+	spin_lock(&mptcp_pm_list_lock);
+	if (mptcp_pm_find(pm->name)) {
+		spin_unlock(&mptcp_pm_list_lock);
+		return -EEXIST;
+	}
+	list_add_tail_rcu(&pm->list, &mptcp_pm_list);
+	spin_unlock(&mptcp_pm_list_lock);
+
+	pr_debug("%s registered\n", pm->name);
+	return 0;
+}
+
+void mptcp_pm_unregister(struct mptcp_pm_ops *pm)
+{
+	spin_lock(&mptcp_pm_list_lock);
+	list_del_rcu(&pm->list);
+	spin_unlock(&mptcp_pm_list_lock);
+}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 9bdfd915d62f..090592c108d6 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1050,6 +1050,11 @@ int mptcp_pm_remove_addr(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_
 void mptcp_pm_remove_addr_entry(struct mptcp_sock *msk,
 				struct mptcp_pm_addr_entry *entry);
 
+struct mptcp_pm_ops *mptcp_pm_find(const char *name);
+int mptcp_pm_validate(struct mptcp_pm_ops *pm);
+int mptcp_pm_register(struct mptcp_pm_ops *pm);
+void mptcp_pm_unregister(struct mptcp_pm_ops *pm);
+
 void mptcp_userspace_pm_free_local_addr_list(struct mptcp_sock *msk);
 
 void mptcp_event(enum mptcp_event_type type, const struct mptcp_sock *msk,
-- 
2.43.0


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

* [PATCH mptcp-next v7 02/11] mptcp: sysctl: new sysctl to set path manager by name
  2025-03-03  4:22 [PATCH mptcp-next v7 00/11] BPF path manager, part 5 Geliang Tang
  2025-03-03  4:22 ` [PATCH mptcp-next v7 01/11] mptcp: pm: define struct mptcp_pm_ops Geliang Tang
@ 2025-03-03  4:22 ` Geliang Tang
  2025-03-03 10:40   ` Matthieu Baerts
  2025-03-03  4:22 ` [PATCH mptcp-next v7 03/11] mptcp: sysctl: map pm_type to path_manager Geliang Tang
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 24+ messages in thread
From: Geliang Tang @ 2025-03-03  4:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

A new net.mptcp.path_manager sysctl is added to determine which path
manager will be used by each newly-created MPTCP socket by setting the
name of it.

This sysctl makes the old one "pm_type" deprecated.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 Documentation/networking/mptcp-sysctl.rst | 22 ++++++++++
 net/mptcp/ctrl.c                          | 50 +++++++++++++++++++++++
 net/mptcp/protocol.h                      |  1 +
 3 files changed, 73 insertions(+)

diff --git a/Documentation/networking/mptcp-sysctl.rst b/Documentation/networking/mptcp-sysctl.rst
index 03e1d3610333..5f6c02c12f5b 100644
--- a/Documentation/networking/mptcp-sysctl.rst
+++ b/Documentation/networking/mptcp-sysctl.rst
@@ -72,6 +72,26 @@ enabled - BOOLEAN
 
 	Default: 1 (enabled)
 
+path_manager - STRING
+	Set the default path manager name to use for each new MPTCP
+	socket. In-kernel path management will control subflow
+	connections and address advertisements according to
+	per-namespace values configured over the MPTCP netlink
+	API. Userspace path management puts per-MPTCP-connection subflow
+	connection decisions and address advertisements under control of
+	a privileged userspace program, at the cost of more netlink
+	traffic to propagate all of the related events and commands.
+	User-defined BPF-based path managers can also be set via this
+	sysctl.
+
+	This is a per-namespace sysctl.
+
+	* "kernel"          - In-kernel path manager
+	* "userspace"       - Userspace path manager
+	* all other strings - BPF-based path managers
+
+	Default: "kernel"
+
 pm_type - INTEGER
 	Set the default path manager type to use for each new MPTCP
 	socket. In-kernel path management will control subflow
@@ -84,6 +104,8 @@ pm_type - INTEGER
 
 	This is a per-namespace sysctl.
 
+	(Deprecated, use path_manager instead.).
+
 	* 0 - In-kernel path manager
 	* 1 - Userspace path manager
 
diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index be6c0237e10b..d64e6b4f6d1d 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -39,6 +39,7 @@ struct mptcp_pernet {
 	u8 allow_join_initial_addr_port;
 	u8 pm_type;
 	char scheduler[MPTCP_SCHED_NAME_MAX];
+	char path_manager[MPTCP_PM_NAME_MAX];
 };
 
 static struct mptcp_pernet *mptcp_get_pernet(const struct net *net)
@@ -83,6 +84,11 @@ int mptcp_get_pm_type(const struct net *net)
 	return mptcp_get_pernet(net)->pm_type;
 }
 
+const char *mptcp_get_path_manager(const struct net *net)
+{
+	return mptcp_get_pernet(net)->path_manager;
+}
+
 const char *mptcp_get_scheduler(const struct net *net)
 {
 	return mptcp_get_pernet(net)->scheduler;
@@ -101,6 +107,7 @@ static void mptcp_pernet_set_defaults(struct mptcp_pernet *pernet)
 	pernet->stale_loss_cnt = 4;
 	pernet->pm_type = MPTCP_PM_TYPE_KERNEL;
 	strscpy(pernet->scheduler, "default", sizeof(pernet->scheduler));
+	strscpy(pernet->path_manager, "kernel", sizeof(pernet->path_manager));
 }
 
 #ifdef CONFIG_SYSCTL
@@ -174,6 +181,42 @@ static int proc_blackhole_detect_timeout(const struct ctl_table *table,
 	return ret;
 }
 
+static int mptcp_set_path_manager(char *path_manager, const char *name)
+{
+	struct mptcp_pm_ops *pm;
+	int ret = 0;
+
+	rcu_read_lock();
+	pm = mptcp_pm_find(name);
+	if (pm)
+		strscpy(path_manager, name, MPTCP_PM_NAME_MAX);
+	else
+		ret = -ENOENT;
+	rcu_read_unlock();
+
+	return ret;
+}
+
+static int proc_path_manager(const struct ctl_table *ctl, int write,
+			     void *buffer, size_t *lenp, loff_t *ppos)
+{
+	char (*path_manager)[MPTCP_PM_NAME_MAX] = ctl->data;
+	char val[MPTCP_PM_NAME_MAX];
+	const struct ctl_table tbl = {
+		.data = val,
+		.maxlen = MPTCP_PM_NAME_MAX,
+	};
+	int ret;
+
+	strscpy(val, *path_manager, MPTCP_PM_NAME_MAX);
+
+	ret = proc_dostring(&tbl, write, buffer, lenp, ppos);
+	if (write && ret == 0)
+		ret = mptcp_set_path_manager(*path_manager, val);
+
+	return ret;
+}
+
 static struct ctl_table mptcp_sysctl_table[] = {
 	{
 		.procname = "enabled",
@@ -253,6 +296,12 @@ static struct ctl_table mptcp_sysctl_table[] = {
 		.mode = 0644,
 		.proc_handler = proc_dou8vec_minmax,
 	},
+	{
+		.procname = "path_manager",
+		.maxlen	= MPTCP_PM_NAME_MAX,
+		.mode = 0644,
+		.proc_handler = proc_path_manager,
+	},
 };
 
 static int mptcp_pernet_new_table(struct net *net, struct mptcp_pernet *pernet)
@@ -278,6 +327,7 @@ static int mptcp_pernet_new_table(struct net *net, struct mptcp_pernet *pernet)
 	table[8].data = &pernet->close_timeout;
 	table[9].data = &pernet->blackhole_timeout;
 	table[10].data = &pernet->syn_retrans_before_tcp_fallback;
+	table[11].data = &pernet->path_manager;
 
 	hdr = register_net_sysctl_sz(net, MPTCP_SYSCTL_PATH, table,
 				     ARRAY_SIZE(mptcp_sysctl_table));
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 090592c108d6..737f148770e3 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -694,6 +694,7 @@ int mptcp_allow_join_id0(const struct net *net);
 unsigned int mptcp_stale_loss_cnt(const struct net *net);
 unsigned int mptcp_close_timeout(const struct sock *sk);
 int mptcp_get_pm_type(const struct net *net);
+const char *mptcp_get_path_manager(const struct net *net);
 const char *mptcp_get_scheduler(const struct net *net);
 
 void mptcp_active_disable(struct sock *sk);
-- 
2.43.0


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

* [PATCH mptcp-next v7 03/11] mptcp: sysctl: map pm_type to path_manager
  2025-03-03  4:22 [PATCH mptcp-next v7 00/11] BPF path manager, part 5 Geliang Tang
  2025-03-03  4:22 ` [PATCH mptcp-next v7 01/11] mptcp: pm: define struct mptcp_pm_ops Geliang Tang
  2025-03-03  4:22 ` [PATCH mptcp-next v7 02/11] mptcp: sysctl: new sysctl to set path manager by name Geliang Tang
@ 2025-03-03  4:22 ` Geliang Tang
  2025-03-03 10:40   ` Matthieu Baerts
  2025-03-03  4:22 ` [PATCH mptcp-next v7 04/11] mptcp: sysctl: add available_path_managers Geliang Tang
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 24+ messages in thread
From: Geliang Tang @ 2025-03-03  4:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch adds a new proc_handler "proc_pm_type" for "pm_type" to
map old path manager sysctl "pm_type" to the newly added "path_manager".

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/ctrl.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index d64e6b4f6d1d..32f13ab7db0a 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -217,6 +217,35 @@ static int proc_path_manager(const struct ctl_table *ctl, int write,
 	return ret;
 }
 
+static int proc_pm_type(const struct ctl_table *ctl, int write,
+			void *buffer, size_t *lenp, loff_t *ppos)
+{
+	struct mptcp_pernet *pernet = container_of(ctl->data,
+						   struct mptcp_pernet,
+						   pm_type);
+	unsigned int val = READ_ONCE(*(u8 *)ctl->data);
+	const struct ctl_table tbl = {
+		.maxlen = sizeof(val),
+		.data = &val,
+	};
+	int ret;
+
+	if (val > mptcp_pm_type_max)
+		return -ERANGE;
+
+	ret = proc_douintvec(&tbl, write, buffer, lenp, ppos);
+	if (write && ret == 0) {
+		char *path_manager = "kernel";
+
+		if (val == MPTCP_PM_TYPE_USERSPACE)
+			path_manager = "userspace";
+		mptcp_set_path_manager(pernet->path_manager, path_manager);
+		WRITE_ONCE(*(u8 *)ctl->data, val);
+	}
+
+	return ret;
+}
+
 static struct ctl_table mptcp_sysctl_table[] = {
 	{
 		.procname = "enabled",
@@ -261,9 +290,7 @@ static struct ctl_table mptcp_sysctl_table[] = {
 		.procname = "pm_type",
 		.maxlen = sizeof(u8),
 		.mode = 0644,
-		.proc_handler = proc_dou8vec_minmax,
-		.extra1       = SYSCTL_ZERO,
-		.extra2       = &mptcp_pm_type_max
+		.proc_handler = proc_pm_type,
 	},
 	{
 		.procname = "scheduler",
-- 
2.43.0


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

* [PATCH mptcp-next v7 04/11] mptcp: sysctl: add available_path_managers
  2025-03-03  4:22 [PATCH mptcp-next v7 00/11] BPF path manager, part 5 Geliang Tang
                   ` (2 preceding siblings ...)
  2025-03-03  4:22 ` [PATCH mptcp-next v7 03/11] mptcp: sysctl: map pm_type to path_manager Geliang Tang
@ 2025-03-03  4:22 ` Geliang Tang
  2025-03-03 10:41   ` Matthieu Baerts
  2025-03-03  4:22 ` [PATCH mptcp-next v7 05/11] mptcp: pm: in-kernel: register mptcp_kernel_pm Geliang Tang
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 24+ messages in thread
From: Geliang Tang @ 2025-03-03  4:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

Similarly to net.mptcp.available_schedulers, this patch adds a new one
net.mptcp.available_path_managers to list the available path mangers.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 Documentation/networking/mptcp-sysctl.rst |  4 ++++
 include/net/mptcp.h                       |  2 ++
 net/mptcp/ctrl.c                          | 25 +++++++++++++++++++++++
 net/mptcp/pm.c                            | 19 +++++++++++++++++
 net/mptcp/protocol.h                      |  1 +
 5 files changed, 51 insertions(+)

diff --git a/Documentation/networking/mptcp-sysctl.rst b/Documentation/networking/mptcp-sysctl.rst
index 5f6c02c12f5b..1f2397c11f65 100644
--- a/Documentation/networking/mptcp-sysctl.rst
+++ b/Documentation/networking/mptcp-sysctl.rst
@@ -30,6 +30,10 @@ allow_join_initial_addr_port - BOOLEAN
 
 	Default: 1
 
+available_path_managers - STRING
+	Shows the available path managers choices that are registered. More
+	path managers may be available, but not loaded.
+
 available_schedulers - STRING
 	Shows the available schedulers choices that are registered. More packet
 	schedulers may be available, but not loaded.
diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 53e67b90c37a..83977fe3dd30 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -123,6 +123,8 @@ struct mptcp_sched_ops {
 } ____cacheline_aligned_in_smp;
 
 #define MPTCP_PM_NAME_MAX	16
+#define MPTCP_PM_MAX		128
+#define MPTCP_PM_BUF_MAX	(MPTCP_PM_NAME_MAX * MPTCP_PM_MAX)
 
 struct mptcp_pm_ops {
 	int (*get_local_id)(struct mptcp_sock *msk,
diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 32f13ab7db0a..643472eb11b2 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -246,6 +246,24 @@ static int proc_pm_type(const struct ctl_table *ctl, int write,
 	return ret;
 }
 
+static int proc_available_path_managers(const struct ctl_table *ctl,
+					int write, void *buffer,
+					size_t *lenp, loff_t *ppos)
+{
+	struct ctl_table tbl = { .maxlen = MPTCP_PM_BUF_MAX, };
+	int ret;
+
+	tbl.data = kmalloc(tbl.maxlen, GFP_USER);
+	if (!tbl.data)
+		return -ENOMEM;
+
+	mptcp_pm_get_available(tbl.data, MPTCP_PM_BUF_MAX);
+	ret = proc_dostring(&tbl, write, buffer, lenp, ppos);
+	kfree(tbl.data);
+
+	return ret;
+}
+
 static struct ctl_table mptcp_sysctl_table[] = {
 	{
 		.procname = "enabled",
@@ -329,6 +347,12 @@ static struct ctl_table mptcp_sysctl_table[] = {
 		.mode = 0644,
 		.proc_handler = proc_path_manager,
 	},
+	{
+		.procname = "available_path_managers",
+		.maxlen	= MPTCP_PM_BUF_MAX,
+		.mode = 0444,
+		.proc_handler = proc_available_path_managers,
+	},
 };
 
 static int mptcp_pernet_new_table(struct net *net, struct mptcp_pernet *pernet)
@@ -355,6 +379,7 @@ static int mptcp_pernet_new_table(struct net *net, struct mptcp_pernet *pernet)
 	table[9].data = &pernet->blackhole_timeout;
 	table[10].data = &pernet->syn_retrans_before_tcp_fallback;
 	table[11].data = &pernet->path_manager;
+	/* table[12] is for available_path_managers which is read-only info */
 
 	hdr = register_net_sysctl_sz(net, MPTCP_SYSCTL_PATH, table,
 				     ARRAY_SIZE(mptcp_sysctl_table));
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 53a29adf7cae..88ff136b3786 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -1079,3 +1079,22 @@ void mptcp_pm_unregister(struct mptcp_pm_ops *pm)
 	list_del_rcu(&pm->list);
 	spin_unlock(&mptcp_pm_list_lock);
 }
+
+/* Build string with list of available path manager values.
+ * Similar to tcp_get_available_congestion_control()
+ */
+void mptcp_pm_get_available(char *buf, size_t maxlen)
+{
+	struct mptcp_pm_ops *pm;
+	size_t offs = 0;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(pm, &mptcp_pm_list, list) {
+		offs += snprintf(buf + offs, maxlen - offs, "%s%s",
+				 offs == 0 ? "" : " ", pm->name);
+
+		if (WARN_ON_ONCE(offs >= maxlen))
+			break;
+	}
+	rcu_read_unlock();
+}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 737f148770e3..9dbfde4027b3 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1055,6 +1055,7 @@ struct mptcp_pm_ops *mptcp_pm_find(const char *name);
 int mptcp_pm_validate(struct mptcp_pm_ops *pm);
 int mptcp_pm_register(struct mptcp_pm_ops *pm);
 void mptcp_pm_unregister(struct mptcp_pm_ops *pm);
+void mptcp_pm_get_available(char *buf, size_t maxlen);
 
 void mptcp_userspace_pm_free_local_addr_list(struct mptcp_sock *msk);
 
-- 
2.43.0


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

* [PATCH mptcp-next v7 05/11] mptcp: pm: in-kernel: register mptcp_kernel_pm
  2025-03-03  4:22 [PATCH mptcp-next v7 00/11] BPF path manager, part 5 Geliang Tang
                   ` (3 preceding siblings ...)
  2025-03-03  4:22 ` [PATCH mptcp-next v7 04/11] mptcp: sysctl: add available_path_managers Geliang Tang
@ 2025-03-03  4:22 ` Geliang Tang
  2025-03-03 10:42   ` Matthieu Baerts
  2025-03-03  4:22 ` [PATCH mptcp-next v7 06/11] mptcp: pm: userspace: register mptcp_userspace_pm Geliang Tang
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 24+ messages in thread
From: Geliang Tang @ 2025-03-03  4:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch defines the original in-kernel netlink path manager as a
new struct mptcp_pm_ops named "mptcp_kernel_pm", and register it in
mptcp_pm_nl_init().

This mptcp_pm_ops will be skipped in mptcp_pm_unregister().

Only get_local_id() and get_priority() interfaces are implemented here.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm.c        | 3 +++
 net/mptcp/pm_kernel.c | 9 +++++++++
 net/mptcp/protocol.h  | 2 ++
 3 files changed, 14 insertions(+)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 88ff136b3786..e648cb522320 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -1075,6 +1075,9 @@ int mptcp_pm_register(struct mptcp_pm_ops *pm)
 
 void mptcp_pm_unregister(struct mptcp_pm_ops *pm)
 {
+	if (pm == &mptcp_kernel_pm)
+		return;
+
 	spin_lock(&mptcp_pm_list_lock);
 	list_del_rcu(&pm->list);
 	spin_unlock(&mptcp_pm_list_lock);
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index daf8f98a3164..8a5966e6e3e3 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -1400,6 +1400,13 @@ static struct pernet_operations mptcp_pm_pernet_ops = {
 	.size = sizeof(struct pm_nl_pernet),
 };
 
+struct mptcp_pm_ops mptcp_kernel_pm = {
+	.get_local_id		= mptcp_pm_nl_get_local_id,
+	.get_priority		= mptcp_pm_nl_is_backup,
+	.name			= "kernel",
+	.owner			= THIS_MODULE,
+};
+
 void __init mptcp_pm_nl_init(void)
 {
 	if (register_pernet_subsys(&mptcp_pm_pernet_ops) < 0)
@@ -1407,4 +1414,6 @@ void __init mptcp_pm_nl_init(void)
 
 	if (genl_register_family(&mptcp_genl_family))
 		panic("Failed to register MPTCP PM netlink family\n");
+
+	mptcp_pm_register(&mptcp_kernel_pm);
 }
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 9dbfde4027b3..56d3a7457f80 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1051,6 +1051,8 @@ int mptcp_pm_remove_addr(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_
 void mptcp_pm_remove_addr_entry(struct mptcp_sock *msk,
 				struct mptcp_pm_addr_entry *entry);
 
+extern struct mptcp_pm_ops mptcp_kernel_pm;
+
 struct mptcp_pm_ops *mptcp_pm_find(const char *name);
 int mptcp_pm_validate(struct mptcp_pm_ops *pm);
 int mptcp_pm_register(struct mptcp_pm_ops *pm);
-- 
2.43.0


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

* [PATCH mptcp-next v7 06/11] mptcp: pm: userspace: register mptcp_userspace_pm
  2025-03-03  4:22 [PATCH mptcp-next v7 00/11] BPF path manager, part 5 Geliang Tang
                   ` (4 preceding siblings ...)
  2025-03-03  4:22 ` [PATCH mptcp-next v7 05/11] mptcp: pm: in-kernel: register mptcp_kernel_pm Geliang Tang
@ 2025-03-03  4:22 ` Geliang Tang
  2025-03-03 10:52   ` Matthieu Baerts
  2025-03-03  4:22 ` [PATCH mptcp-next v7 07/11] mptcp: pm: initialize and release mptcp_pm_ops Geliang Tang
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 24+ messages in thread
From: Geliang Tang @ 2025-03-03  4:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

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_data_init().

Only get_local_id(), get_priority() and release() interfaces are
implemented here. 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 | 18 ++++++++++++++++++
 net/mptcp/protocol.h     |  1 +
 3 files changed, 20 insertions(+)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index e648cb522320..98f81221786f 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_nl_init();
+	mptcp_userspace_pm_init();
 }
 
 /* Must be called with rcu read lock held */
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 8f9e749e9b1a..d53f44df9641 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -683,3 +683,21 @@ int mptcp_userspace_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
 	sock_put(sk);
 	return ret;
 }
+
+static void mptcp_userspace_pm_release(struct mptcp_sock *msk)
+{
+	mptcp_userspace_pm_free_local_addr_list(msk);
+}
+
+static struct mptcp_pm_ops mptcp_userspace_pm = {
+	.get_local_id		= mptcp_userspace_pm_get_local_id,
+	.get_priority		= mptcp_userspace_pm_is_backup,
+	.release		= mptcp_userspace_pm_release,
+	.name			= "userspace",
+	.owner			= THIS_MODULE,
+};
+
+void __init mptcp_userspace_pm_init(void)
+{
+	mptcp_pm_register(&mptcp_userspace_pm);
+}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 56d3a7457f80..979ee8762fd4 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_nl_init(void);
+void __init mptcp_userspace_pm_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);
-- 
2.43.0


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

* [PATCH mptcp-next v7 07/11] mptcp: pm: initialize and release mptcp_pm_ops
  2025-03-03  4:22 [PATCH mptcp-next v7 00/11] BPF path manager, part 5 Geliang Tang
                   ` (5 preceding siblings ...)
  2025-03-03  4:22 ` [PATCH mptcp-next v7 06/11] mptcp: pm: userspace: register mptcp_userspace_pm Geliang Tang
@ 2025-03-03  4:22 ` Geliang Tang
  2025-03-03 10:53   ` Matthieu Baerts
  2025-03-03  4:22 ` [PATCH mptcp-next v7 08/11] mptcp: pm: drop pm_type in mptcp_pm_data Geliang Tang
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 24+ messages in thread
From: Geliang Tang @ 2025-03-03  4:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

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

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

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 98f81221786f..e8b34f2ecb35 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -973,15 +973,15 @@ void mptcp_pm_worker(struct mptcp_sock *msk)
 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_release(msk);
 }
 
 void mptcp_pm_data_reset(struct mptcp_sock *msk)
 {
+	const char *path_manager = mptcp_get_path_manager(sock_net((struct sock *)msk));
 	u8 pm_type = mptcp_get_pm_type(sock_net((struct sock *)msk));
 	struct mptcp_pm_data *pm = &msk->pm;
+	int ret;
 
 	pm->add_addr_signaled = 0;
 	pm->add_addr_accepted = 0;
@@ -991,6 +991,12 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk)
 	pm->rm_list_rx.nr = 0;
 	WRITE_ONCE(pm->pm_type, pm_type);
 
+	rcu_read_lock();
+	ret = mptcp_pm_initialize(msk, mptcp_pm_find(path_manager));
+	rcu_read_unlock();
+	if (ret)
+		return;
+
 	if (pm_type == MPTCP_PM_TYPE_KERNEL) {
 		bool subflows_allowed = !!mptcp_pm_get_subflows_max(msk);
 
@@ -1102,3 +1108,33 @@ void mptcp_pm_get_available(char *buf, size_t maxlen)
 	}
 	rcu_read_unlock();
 }
+
+int mptcp_pm_initialize(struct mptcp_sock *msk, struct mptcp_pm_ops *pm)
+{
+	if (!pm)
+		pm = &mptcp_kernel_pm;
+
+	if (!bpf_try_module_get(pm, pm->owner))
+		return -EBUSY;
+
+	msk->pm.ops = pm;
+	if (msk->pm.ops->init)
+		msk->pm.ops->init(msk);
+
+	pr_debug("pm %s initialized\n", pm->name);
+	return 0;
+}
+
+void mptcp_pm_release(struct mptcp_sock *msk)
+{
+	struct mptcp_pm_ops *pm = msk->pm.ops;
+
+	if (!pm)
+		return;
+
+	msk->pm.ops = NULL;
+	if (pm->release)
+		pm->release(msk);
+
+	bpf_module_put(pm, pm->owner);
+}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 979ee8762fd4..172450455c2a 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 */
 
@@ -1058,6 +1059,8 @@ int mptcp_pm_validate(struct mptcp_pm_ops *pm);
 int mptcp_pm_register(struct mptcp_pm_ops *pm);
 void mptcp_pm_unregister(struct mptcp_pm_ops *pm);
 void mptcp_pm_get_available(char *buf, size_t maxlen);
+int mptcp_pm_initialize(struct mptcp_sock *msk, struct mptcp_pm_ops *pm);
+void mptcp_pm_release(struct mptcp_sock *msk);
 
 void mptcp_userspace_pm_free_local_addr_list(struct mptcp_sock *msk);
 
-- 
2.43.0


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

* [PATCH mptcp-next v7 08/11] mptcp: pm: drop pm_type in mptcp_pm_data
  2025-03-03  4:22 [PATCH mptcp-next v7 00/11] BPF path manager, part 5 Geliang Tang
                   ` (6 preceding siblings ...)
  2025-03-03  4:22 ` [PATCH mptcp-next v7 07/11] mptcp: pm: initialize and release mptcp_pm_ops Geliang Tang
@ 2025-03-03  4:22 ` Geliang Tang
  2025-03-03 10:57   ` Matthieu Baerts
  2025-03-03  4:22 ` [PATCH mptcp-next v7 09/11] mptcp: sysctl: drop get_pm_type helper Geliang Tang
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 24+ messages in thread
From: Geliang Tang @ 2025-03-03  4:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

Now pm->pm_type can be replaced by pm->ops->name, then "pm_type" filed
of struct mptcp_pm_data can be dropped.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm.c       | 4 +---
 net/mptcp/protocol.h | 5 ++---
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index e8b34f2ecb35..1ce58d16370a 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -979,7 +979,6 @@ void mptcp_pm_destroy(struct mptcp_sock *msk)
 void mptcp_pm_data_reset(struct mptcp_sock *msk)
 {
 	const char *path_manager = mptcp_get_path_manager(sock_net((struct sock *)msk));
-	u8 pm_type = mptcp_get_pm_type(sock_net((struct sock *)msk));
 	struct mptcp_pm_data *pm = &msk->pm;
 	int ret;
 
@@ -989,7 +988,6 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk)
 	pm->subflows = 0;
 	pm->rm_list_tx.nr = 0;
 	pm->rm_list_rx.nr = 0;
-	WRITE_ONCE(pm->pm_type, pm_type);
 
 	rcu_read_lock();
 	ret = mptcp_pm_initialize(msk, mptcp_pm_find(path_manager));
@@ -997,7 +995,7 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk)
 	if (ret)
 		return;
 
-	if (pm_type == MPTCP_PM_TYPE_KERNEL) {
+	if (mptcp_pm_is_kernel(msk)) {
 		bool subflows_allowed = !!mptcp_pm_get_subflows_max(msk);
 
 		/* pm->work_pending must be only be set to 'true' when
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 172450455c2a..56eeee1cbccc 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -233,7 +233,6 @@ struct mptcp_pm_data {
 	u8		add_addr_signaled;
 	u8		add_addr_accepted;
 	u8		local_addr_used;
-	u8		pm_type;
 	u8		subflows;
 	u8		status;
 	DECLARE_BITMAP(id_avail_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
@@ -1101,12 +1100,12 @@ static inline bool mptcp_pm_should_rm_signal(struct mptcp_sock *msk)
 
 static inline bool mptcp_pm_is_userspace(const struct mptcp_sock *msk)
 {
-	return READ_ONCE(msk->pm.pm_type) == MPTCP_PM_TYPE_USERSPACE;
+	return !strncmp(msk->pm.ops->name, "userspace", MPTCP_PM_NAME_MAX);
 }
 
 static inline bool mptcp_pm_is_kernel(const struct mptcp_sock *msk)
 {
-	return READ_ONCE(msk->pm.pm_type) == MPTCP_PM_TYPE_KERNEL;
+	return !strncmp(msk->pm.ops->name, "kernel", MPTCP_PM_NAME_MAX);
 }
 
 static inline unsigned int mptcp_add_addr_len(int family, bool echo, bool port)
-- 
2.43.0


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

* [PATCH mptcp-next v7 09/11] mptcp: sysctl: drop get_pm_type helper
  2025-03-03  4:22 [PATCH mptcp-next v7 00/11] BPF path manager, part 5 Geliang Tang
                   ` (7 preceding siblings ...)
  2025-03-03  4:22 ` [PATCH mptcp-next v7 08/11] mptcp: pm: drop pm_type in mptcp_pm_data Geliang Tang
@ 2025-03-03  4:22 ` Geliang Tang
  2025-03-03 10:57   ` Matthieu Baerts
  2025-03-03  4:22 ` [PATCH mptcp-next v7 10/11] mptcp: pm: make get_local_id helpers static Geliang Tang
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 24+ messages in thread
From: Geliang Tang @ 2025-03-03  4:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

The helper mptcp_get_pm_type() is unused now, it's replaced by the
new one mptcp_get_path_manager(). So drop it.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/ctrl.c     | 5 -----
 net/mptcp/protocol.h | 1 -
 2 files changed, 6 deletions(-)

diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 643472eb11b2..fd797f071886 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -79,11 +79,6 @@ unsigned int mptcp_close_timeout(const struct sock *sk)
 	return mptcp_get_pernet(sock_net(sk))->close_timeout;
 }
 
-int mptcp_get_pm_type(const struct net *net)
-{
-	return mptcp_get_pernet(net)->pm_type;
-}
-
 const char *mptcp_get_path_manager(const struct net *net)
 {
 	return mptcp_get_pernet(net)->path_manager;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 56eeee1cbccc..3579c31650fa 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -693,7 +693,6 @@ int mptcp_is_checksum_enabled(const struct net *net);
 int mptcp_allow_join_id0(const struct net *net);
 unsigned int mptcp_stale_loss_cnt(const struct net *net);
 unsigned int mptcp_close_timeout(const struct sock *sk);
-int mptcp_get_pm_type(const struct net *net);
 const char *mptcp_get_path_manager(const struct net *net);
 const char *mptcp_get_scheduler(const struct net *net);
 
-- 
2.43.0


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

* [PATCH mptcp-next v7 10/11] mptcp: pm: make get_local_id helpers static
  2025-03-03  4:22 [PATCH mptcp-next v7 00/11] BPF path manager, part 5 Geliang Tang
                   ` (8 preceding siblings ...)
  2025-03-03  4:22 ` [PATCH mptcp-next v7 09/11] mptcp: sysctl: drop get_pm_type helper Geliang Tang
@ 2025-03-03  4:22 ` Geliang Tang
  2025-03-03 10:58   ` Matthieu Baerts
  2025-03-03  4:22 ` [PATCH mptcp-next v7 11/11] mptcp: pm: make is_backup " Geliang Tang
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 24+ messages in thread
From: Geliang Tang @ 2025-03-03  4:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

Now mptcp_pm_get_local_id() can directly invoke get_local_id() interface
through "ops" of "msk->pm". Instead of using mptcp_pm_is_userspace() to
check which get_local_id() helper to invoke.

Then mptcp_pm_nl_get_local_id() and mptcp_userspace_pm_get_local_id()
helpers can be static.

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

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 1ce58d16370a..848393511997 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -872,9 +872,7 @@ int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc)
 	skc_local.addr.id = 0;
 	skc_local.flags = MPTCP_PM_ADDR_FLAG_IMPLICIT;
 
-	if (mptcp_pm_is_userspace(msk))
-		return mptcp_userspace_pm_get_local_id(msk, &skc_local);
-	return mptcp_pm_nl_get_local_id(msk, &skc_local);
+	return msk->pm.ops->get_local_id(msk, &skc_local);
 }
 
 bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc)
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 8a5966e6e3e3..3d5beaafc3ec 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -693,8 +693,8 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
 	return err;
 }
 
-int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk,
-			     struct mptcp_pm_addr_entry *skc)
+static int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk,
+				    struct mptcp_pm_addr_entry *skc)
 {
 	struct mptcp_pm_addr_entry *entry;
 	struct pm_nl_pernet *pernet;
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index d53f44df9641..05495f6e771a 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -127,8 +127,8 @@ mptcp_userspace_pm_lookup_addr_by_id(struct mptcp_sock *msk, unsigned int id)
 	return NULL;
 }
 
-int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
-				    struct mptcp_pm_addr_entry *skc)
+static int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
+					   struct mptcp_pm_addr_entry *skc)
 {
 	__be16 msk_sport =  ((struct inet_sock *)
 			     inet_sk((struct sock *)msk))->inet_sport;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 3579c31650fa..776f164a21eb 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1137,10 +1137,6 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, const struct sk_buff *skb,
 bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
 			     struct mptcp_rm_list *rm_list);
 int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
-int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk,
-			     struct mptcp_pm_addr_entry *skc);
-int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
-				    struct mptcp_pm_addr_entry *skc);
 bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc);
 bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
 bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
-- 
2.43.0


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

* [PATCH mptcp-next v7 11/11] mptcp: pm: make is_backup helpers static
  2025-03-03  4:22 [PATCH mptcp-next v7 00/11] BPF path manager, part 5 Geliang Tang
                   ` (9 preceding siblings ...)
  2025-03-03  4:22 ` [PATCH mptcp-next v7 10/11] mptcp: pm: make get_local_id helpers static Geliang Tang
@ 2025-03-03  4:22 ` Geliang Tang
  2025-03-03  5:32 ` [PATCH mptcp-next v7 00/11] BPF path manager, part 5 MPTCP CI
  2025-03-03 10:38 ` Matthieu Baerts
  12 siblings, 0 replies; 24+ messages in thread
From: Geliang Tang @ 2025-03-03  4:22 UTC (permalink / raw)
  To: mptcp; +Cc: Geliang Tang

From: Geliang Tang <tanggeliang@kylinos.cn>

Now mptcp_pm_is_backup() can directly invoke get_priority() interface
through "ops" of "msk->pm". Instead of using mptcp_pm_is_userspace()
to check which is_backup() helper to invoke.

Then mptcp_pm_nl_is_backup() and mptcp_userspace_pm_is_backup() helpers
can be static.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm.c           | 5 +----
 net/mptcp/pm_kernel.c    | 3 ++-
 net/mptcp/pm_userspace.c | 4 ++--
 net/mptcp/protocol.h     | 2 --
 4 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 848393511997..89a1a0ba9f79 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -881,10 +881,7 @@ bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc)
 
 	mptcp_local_address((struct sock_common *)skc, &skc_local);
 
-	if (mptcp_pm_is_userspace(msk))
-		return mptcp_userspace_pm_is_backup(msk, &skc_local);
-
-	return mptcp_pm_nl_is_backup(msk, &skc_local);
+	return msk->pm.ops->get_priority(msk, &skc_local);
 }
 
 static void mptcp_pm_subflows_chk_stale(const struct mptcp_sock *msk, struct sock *ssk)
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 3d5beaafc3ec..5cc6cda01ba2 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -723,7 +723,8 @@ static int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk,
 	return ret;
 }
 
-bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc)
+static bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk,
+				  struct mptcp_addr_info *skc)
 {
 	struct pm_nl_pernet *pernet = pm_nl_get_pernet_from_msk(msk);
 	struct mptcp_pm_addr_entry *entry;
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 05495f6e771a..b30c8aa45610 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -146,8 +146,8 @@ static int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
 	return mptcp_userspace_pm_append_new_local_addr(msk, skc, true);
 }
 
-bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk,
-				  struct mptcp_addr_info *skc)
+static bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk,
+					 struct mptcp_addr_info *skc)
 {
 	struct mptcp_pm_addr_entry *entry;
 	bool backup;
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 776f164a21eb..b5f50feb52fa 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1138,8 +1138,6 @@ bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
 			     struct mptcp_rm_list *rm_list);
 int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);
 bool mptcp_pm_is_backup(struct mptcp_sock *msk, struct sock_common *skc);
-bool mptcp_pm_nl_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
-bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk, struct mptcp_addr_info *skc);
 int mptcp_pm_nl_dump_addr(struct sk_buff *msg,
 			  struct netlink_callback *cb);
 int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
-- 
2.43.0


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

* Re: [PATCH mptcp-next v7 00/11] BPF path manager, part 5
  2025-03-03  4:22 [PATCH mptcp-next v7 00/11] BPF path manager, part 5 Geliang Tang
                   ` (10 preceding siblings ...)
  2025-03-03  4:22 ` [PATCH mptcp-next v7 11/11] mptcp: pm: make is_backup " Geliang Tang
@ 2025-03-03  5:32 ` MPTCP CI
  2025-03-03 10:38 ` Matthieu Baerts
  12 siblings, 0 replies; 24+ messages in thread
From: MPTCP CI @ 2025-03-03  5:32 UTC (permalink / raw)
  To: Geliang Tang; +Cc: mptcp

Hi Geliang,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal: Unstable: 1 failed test(s): selftest_mptcp_connect 🔴
- 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/13623533080

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


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] 24+ messages in thread

* Re: [PATCH mptcp-next v7 00/11] BPF path manager, part 5
  2025-03-03  4:22 [PATCH mptcp-next v7 00/11] BPF path manager, part 5 Geliang Tang
                   ` (11 preceding siblings ...)
  2025-03-03  5:32 ` [PATCH mptcp-next v7 00/11] BPF path manager, part 5 MPTCP CI
@ 2025-03-03 10:38 ` Matthieu Baerts
  12 siblings, 0 replies; 24+ messages in thread
From: Matthieu Baerts @ 2025-03-03 10:38 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 03/03/2025 05:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> v7:
>  - addresss Matt's comments in v6 [1].
>  - drop "type" from struct mptcp_pm_ops as Matt suggested.
>  - map "pm_type" to new sysctl as Matt suggested.
> 
> Depends on:
>  - mptcp: pm: code reorganisation, v2

Is it OK for you if I apply this "code reorganisation" series as it is?
If there are no major changes needed, it will be easier to apply it now,
and send it upstream "soon".

Thank you for the v7. I have some comments, no big changes, I think we
are heading in the right direction.

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


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

* Re: [PATCH mptcp-next v7 01/11] mptcp: pm: define struct mptcp_pm_ops
  2025-03-03  4:22 ` [PATCH mptcp-next v7 01/11] mptcp: pm: define struct mptcp_pm_ops Geliang Tang
@ 2025-03-03 10:39   ` Matthieu Baerts
  0 siblings, 0 replies; 24+ messages in thread
From: Matthieu Baerts @ 2025-03-03 10:39 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 03/03/2025 05:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> In order to allow users to develop their own BPF-based path manager,
> this patch defines a struct ops "mptcp_pm_ops" for a userspace path
> manager, which contains a set of interfaces.
> 
> Add a set of functions to register, unregister, find and validate a
> given struct ops.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  include/net/mptcp.h  | 17 ++++++++++++++
>  net/mptcp/pm.c       | 55 ++++++++++++++++++++++++++++++++++++++++++++
>  net/mptcp/protocol.h |  5 ++++
>  3 files changed, 77 insertions(+)
> 
> diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> index 72d6e6597add..53e67b90c37a 100644
> --- a/include/net/mptcp.h
> +++ b/include/net/mptcp.h
> @@ -14,6 +14,7 @@
>  
>  struct mptcp_info;
>  struct mptcp_sock;
> +struct mptcp_pm_addr_entry;
>  struct seq_file;
>  
>  /* MPTCP sk_buff extension data */
> @@ -121,6 +122,22 @@ struct mptcp_sched_ops {
>  	void (*release)(struct mptcp_sock *msk);
>  } ____cacheline_aligned_in_smp;
>  
> +#define MPTCP_PM_NAME_MAX	16
> +
> +struct mptcp_pm_ops {
> +	int (*get_local_id)(struct mptcp_sock *msk,
> +			    struct mptcp_pm_addr_entry *skc);
> +	bool (*get_priority)(struct mptcp_sock *msk,
> +			     struct mptcp_addr_info *skc);

Detail, if you have something else to change in this series: it is
strange to see only two of them defined. I would expect all of them. Or,
probably better, nothing now, and only add them when being used to show
to the reviewers/devs how they are being used and where, no?

By doing that, it will also be clearer to understand which ops are
mandatory when we will see the modification of mptcp_pm_validate().

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


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

* Re: [PATCH mptcp-next v7 02/11] mptcp: sysctl: new sysctl to set path manager by name
  2025-03-03  4:22 ` [PATCH mptcp-next v7 02/11] mptcp: sysctl: new sysctl to set path manager by name Geliang Tang
@ 2025-03-03 10:40   ` Matthieu Baerts
  0 siblings, 0 replies; 24+ messages in thread
From: Matthieu Baerts @ 2025-03-03 10:40 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 03/03/2025 05:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> A new net.mptcp.path_manager sysctl is added to determine which path
> manager will be used by each newly-created MPTCP socket by setting the
> name of it.
> 
> This sysctl makes the old one "pm_type" deprecated.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  Documentation/networking/mptcp-sysctl.rst | 22 ++++++++++
>  net/mptcp/ctrl.c                          | 50 +++++++++++++++++++++++
>  net/mptcp/protocol.h                      |  1 +
>  3 files changed, 73 insertions(+)
> 
> diff --git a/Documentation/networking/mptcp-sysctl.rst b/Documentation/networking/mptcp-sysctl.rst
> index 03e1d3610333..5f6c02c12f5b 100644
> --- a/Documentation/networking/mptcp-sysctl.rst
> +++ b/Documentation/networking/mptcp-sysctl.rst
> @@ -72,6 +72,26 @@ enabled - BOOLEAN
>  
>  	Default: 1 (enabled)
>  
> +path_manager - STRING
> +	Set the default path manager name to use for each new MPTCP
> +	socket. In-kernel path management will control subflow
> +	connections and address advertisements according to
> +	per-namespace values configured over the MPTCP netlink
> +	API. Userspace path management puts per-MPTCP-connection subflow
> +	connection decisions and address advertisements under control of
> +	a privileged userspace program, at the cost of more netlink
> +	traffic to propagate all of the related events and commands.
> +	User-defined BPF-based path managers can also be set via this
> +	sysctl.

Maybe we should add the last sentence about BPF-based PMs later on, when
everything will be ready, to avoid confusions. WDTY?

Also, because there are many patches to upstream, I don't think
everything will be ready for v6.15.

> +
> +	This is a per-namespace sysctl.
> +
> +	* "kernel"          - In-kernel path manager
> +	* "userspace"       - Userspace path manager
> +	* all other strings - BPF-based path managers

Same here.

> +
> +	Default: "kernel"
> +
>  pm_type - INTEGER
>  	Set the default path manager type to use for each new MPTCP
>  	socket. In-kernel path management will control subflow
> @@ -84,6 +104,8 @@ pm_type - INTEGER
>  
>  	This is a per-namespace sysctl.
>  
> +	(Deprecated, use path_manager instead.).

Detail: I think you can remove the parenthesis.

Also, maybe we should have "Deprecated since v6.15, use (...)"? I see
other sysctl knobs have that in their description.

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


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

* Re: [PATCH mptcp-next v7 03/11] mptcp: sysctl: map pm_type to path_manager
  2025-03-03  4:22 ` [PATCH mptcp-next v7 03/11] mptcp: sysctl: map pm_type to path_manager Geliang Tang
@ 2025-03-03 10:40   ` Matthieu Baerts
  0 siblings, 0 replies; 24+ messages in thread
From: Matthieu Baerts @ 2025-03-03 10:40 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 03/03/2025 05:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch adds a new proc_handler "proc_pm_type" for "pm_type" to
> map old path manager sysctl "pm_type" to the newly added "path_manager".

Don't forget to add a selftest checking this new sysctl and the mapping
are correct, e.g. in userspace_pm.sh. See my previous comment:

https://lore.kernel.org/c49517d2-38e2-4848-9fb9-1c7748689cec@kernel.org

> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/ctrl.c | 33 ++++++++++++++++++++++++++++++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> index d64e6b4f6d1d..32f13ab7db0a 100644
> --- a/net/mptcp/ctrl.c
> +++ b/net/mptcp/ctrl.c
> @@ -217,6 +217,35 @@ static int proc_path_manager(const struct ctl_table *ctl, int write,
>  	return ret;
>  }
>  
> +static int proc_pm_type(const struct ctl_table *ctl, int write,
> +			void *buffer, size_t *lenp, loff_t *ppos)
> +{
> +	struct mptcp_pernet *pernet = container_of(ctl->data,
> +						   struct mptcp_pernet,
> +						   pm_type);

Out of curiosity, do you plan to drop pernet->pm_type later on?

When all the mptcp_pm_is_{kernel,userspace}() will be dropped, I think
we can also drop it from the mptcp_pernet structure. At that point, we
could then read info from "path_manager" and write in ctl->data 0, 1, or
2. But only when we will no longer use pernet->pm_type.

> +	unsigned int val = READ_ONCE(*(u8 *)ctl->data);
> +	const struct ctl_table tbl = {
> +		.maxlen = sizeof(val),
> +		.data = &val,
> +	};
> +	int ret;
> +
> +	if (val > mptcp_pm_type_max)
> +		return -ERANGE;

You might not need this if ...

> +
> +	ret = proc_douintvec(&tbl, write, buffer, lenp, ppos);

... you use proc_dou8vec_minmax() here and ...

> +	if (write && ret == 0) {
> +		char *path_manager = "kernel";
> +
> +		if (val == MPTCP_PM_TYPE_USERSPACE)
> +			path_manager = "userspace";
> +		mptcp_set_path_manager(pernet->path_manager, path_manager);
> +		WRITE_ONCE(*(u8 *)ctl->data, val);
> +	}
> +
> +	return ret;
> +}
> +
>  static struct ctl_table mptcp_sysctl_table[] = {
>  	{
>  		.procname = "enabled",
> @@ -261,9 +290,7 @@ static struct ctl_table mptcp_sysctl_table[] = {
>  		.procname = "pm_type",
>  		.maxlen = sizeof(u8),
>  		.mode = 0644,
> -		.proc_handler = proc_dou8vec_minmax,
> -		.extra1       = SYSCTL_ZERO,
> -		.extra2       = &mptcp_pm_type_max

... you keep these two last lines.

> +		.proc_handler = proc_pm_type,
>  	},
>  	{
>  		.procname = "scheduler",
Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


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

* Re: [PATCH mptcp-next v7 04/11] mptcp: sysctl: add available_path_managers
  2025-03-03  4:22 ` [PATCH mptcp-next v7 04/11] mptcp: sysctl: add available_path_managers Geliang Tang
@ 2025-03-03 10:41   ` Matthieu Baerts
  0 siblings, 0 replies; 24+ messages in thread
From: Matthieu Baerts @ 2025-03-03 10:41 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 03/03/2025 05:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Similarly to net.mptcp.available_schedulers, this patch adds a new one
> net.mptcp.available_path_managers to list the available path mangers.

Detail: s/mangers/managers/

("manger" means "to eat" in French, so that looks strange to me :) )

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


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

* Re: [PATCH mptcp-next v7 05/11] mptcp: pm: in-kernel: register mptcp_kernel_pm
  2025-03-03  4:22 ` [PATCH mptcp-next v7 05/11] mptcp: pm: in-kernel: register mptcp_kernel_pm Geliang Tang
@ 2025-03-03 10:42   ` Matthieu Baerts
  0 siblings, 0 replies; 24+ messages in thread
From: Matthieu Baerts @ 2025-03-03 10:42 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 03/03/2025 05:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch defines the original in-kernel netlink path manager as a
> new struct mptcp_pm_ops named "mptcp_kernel_pm", and register it in

Detail: should it not be called "mptcp_pm_kernel" with the usual
"mptcp_pm_" prefix like everywhere else (except in the userspace PM I see)?

> mptcp_pm_nl_init().
> 
> This mptcp_pm_ops will be skipped in mptcp_pm_unregister().

Why this exception here? Please add a comment in the code, and
eventually in the commit message if you need a longer explanation.

Why is it fine to unregister the userspace PM, and not the kernel one?
Can you not check the owner to see if it is an internal module for
example? Or add something in struct mptcp_pm_ops to know if the
unregister part is needed?

Also, mptcp_pm_unregister() is currently unused in this series, is it
normal?

> Only get_local_id() and get_priority() interfaces are implemented here.

Maybe they can all be implemented later on, see my comment on patch 1/11.

> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 9dbfde4027b3..56d3a7457f80 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -1051,6 +1051,8 @@ int mptcp_pm_remove_addr(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_
>  void mptcp_pm_remove_addr_entry(struct mptcp_sock *msk,
>  				struct mptcp_pm_addr_entry *entry);
>  
> +extern struct mptcp_pm_ops mptcp_kernel_pm;

Can you add a comment in the commit message explaining why it needs to
be declared as extern? (or only do that when you need it elsewhere?)

Maybe enough to mention that it needs to be "extern" because it is the
default one?

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


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

* Re: [PATCH mptcp-next v7 06/11] mptcp: pm: userspace: register mptcp_userspace_pm
  2025-03-03  4:22 ` [PATCH mptcp-next v7 06/11] mptcp: pm: userspace: register mptcp_userspace_pm Geliang Tang
@ 2025-03-03 10:52   ` Matthieu Baerts
  0 siblings, 0 replies; 24+ messages in thread
From: Matthieu Baerts @ 2025-03-03 10:52 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang



On 03/03/2025 05:22, 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_data_init().
> 
> Only get_local_id(), get_priority() and release() interfaces are
> implemented here. 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 | 18 ++++++++++++++++++
>  net/mptcp/protocol.h     |  1 +
>  3 files changed, 20 insertions(+)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index e648cb522320..98f81221786f 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_nl_init();
> +	mptcp_userspace_pm_init();

Detail: maybe better to call it mptcp_pm_userspace_init(). So all the
remaining exposed userspace PM helpers will have the same mptcp_pm_
prefix as the rest?

Also, maybe clearer with mptcp_pm_userspace_register()?

BTW, I think we should also rename mptcp_pm_nl_init to
mptcp_pm_kernel_register() later ; or do that as part of patch 5/11?
"While at it, rename the init function to avoid confusions?" WDYT? Or I
do the rename as part of the "code reorganisation"?

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


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

* Re: [PATCH mptcp-next v7 07/11] mptcp: pm: initialize and release mptcp_pm_ops
  2025-03-03  4:22 ` [PATCH mptcp-next v7 07/11] mptcp: pm: initialize and release mptcp_pm_ops Geliang Tang
@ 2025-03-03 10:53   ` Matthieu Baerts
  0 siblings, 0 replies; 24+ messages in thread
From: Matthieu Baerts @ 2025-03-03 10:53 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 03/03/2025 05:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Add a struct mptcp_pm_ops pointer "ops" in struct mptcp_pm_data, and two
> functions mptcp_pm_initialize() and mptcp_pm_release(), to set and release
> this pointer. mptcp_pm_initialize() is invoked in mptcp_pm_data_reset(),
> while mptcp_pm_release() is invoked in mptcp_pm_destroy().
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm.c       | 42 +++++++++++++++++++++++++++++++++++++++---
>  net/mptcp/protocol.h |  3 +++
>  2 files changed, 42 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 98f81221786f..e8b34f2ecb35 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -973,15 +973,15 @@ void mptcp_pm_worker(struct mptcp_sock *msk)
>  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_release(msk);
>  }
>  
>  void mptcp_pm_data_reset(struct mptcp_sock *msk)
>  {
> +	const char *path_manager = mptcp_get_path_manager(sock_net((struct sock *)msk));
>  	u8 pm_type = mptcp_get_pm_type(sock_net((struct sock *)msk));
>  	struct mptcp_pm_data *pm = &msk->pm;
> +	int ret;
>  
>  	pm->add_addr_signaled = 0;
>  	pm->add_addr_accepted = 0;
> @@ -991,6 +991,12 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk)
>  	pm->rm_list_rx.nr = 0;
>  	WRITE_ONCE(pm->pm_type, pm_type);
>  
> +	rcu_read_lock();
> +	ret = mptcp_pm_initialize(msk, mptcp_pm_find(path_manager));
> +	rcu_read_unlock();
> +	if (ret)
> +		return;

Mmh, that's annoying if pm->ops has not been set here! I don't think
mptcp_pm_initialize() can fail and if it does, it should stop the
connection.

> +
>  	if (pm_type == MPTCP_PM_TYPE_KERNEL) {>  		bool subflows_allowed = !!mptcp_pm_get_subflows_max(msk);
>  
> @@ -1102,3 +1108,33 @@ void mptcp_pm_get_available(char *buf, size_t maxlen)
>  	}
>  	rcu_read_unlock();
>  }
> +
> +int mptcp_pm_initialize(struct mptcp_sock *msk, struct mptcp_pm_ops *pm)
> +{
> +	if (!pm)

I guess this should never happen, right? Or maybe yes if a BPF PM has
been unloaded?

> +		pm = &mptcp_kernel_pm;
> +
> +	if (!bpf_try_module_get(pm, pm->owner))
> +		return -EBUSY;

Should it not fallback to the kernel PM + print a warning (once?)?

pr_warn_once("%pm %s couldn't be initialized, falling back to 'kernel'",
             pm->name);
pm = &mptcp_kernel_pm;

> +
> +	msk->pm.ops = pm;
> +	if (msk->pm.ops->init)
> +		msk->pm.ops->init(msk);
> +
> +	pr_debug("pm %s initialized\n", pm->name);
> +	return 0;
> +}
> +
> +void mptcp_pm_release(struct mptcp_sock *msk)
> +{
> +	struct mptcp_pm_ops *pm = msk->pm.ops;
> +
> +	if (!pm)
> +		return;
> +
> +	msk->pm.ops = NULL;
> +	if (pm->release)
> +		pm->release(msk);
> +
> +	bpf_module_put(pm, pm->owner);
> +}

Can you not declare mptcp_pm_release() and mptcp_pm_initialize() as
static and move them above mptcp_pm_destroy() and mptcp_pm_data_reset()?
It would make more sense, and no need to export them in protocol.h.

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


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

* Re: [PATCH mptcp-next v7 08/11] mptcp: pm: drop pm_type in mptcp_pm_data
  2025-03-03  4:22 ` [PATCH mptcp-next v7 08/11] mptcp: pm: drop pm_type in mptcp_pm_data Geliang Tang
@ 2025-03-03 10:57   ` Matthieu Baerts
  0 siblings, 0 replies; 24+ messages in thread
From: Matthieu Baerts @ 2025-03-03 10:57 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 03/03/2025 05:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Now pm->pm_type can be replaced by pm->ops->name, then "pm_type" filed
> of struct mptcp_pm_data can be dropped.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm.c       | 4 +---
>  net/mptcp/protocol.h | 5 ++---
>  2 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index e8b34f2ecb35..1ce58d16370a 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -979,7 +979,6 @@ void mptcp_pm_destroy(struct mptcp_sock *msk)
>  void mptcp_pm_data_reset(struct mptcp_sock *msk)
>  {
>  	const char *path_manager = mptcp_get_path_manager(sock_net((struct sock *)msk));
> -	u8 pm_type = mptcp_get_pm_type(sock_net((struct sock *)msk));
>  	struct mptcp_pm_data *pm = &msk->pm;
>  	int ret;
>  
> @@ -989,7 +988,6 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk)
>  	pm->subflows = 0;
>  	pm->rm_list_tx.nr = 0;
>  	pm->rm_list_rx.nr = 0;
> -	WRITE_ONCE(pm->pm_type, pm_type);
>  
>  	rcu_read_lock();
>  	ret = mptcp_pm_initialize(msk, mptcp_pm_find(path_manager));
> @@ -997,7 +995,7 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk)
>  	if (ret)
>  		return;
>  
> -	if (pm_type == MPTCP_PM_TYPE_KERNEL) {
> +	if (mptcp_pm_is_kernel(msk)) {

The code here could be done in the new init() callback maybe? So what
you introduced in the previous patch 7/11.

>  		bool subflows_allowed = !!mptcp_pm_get_subflows_max(msk);
>  
>  		/* pm->work_pending must be only be set to 'true' when
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 172450455c2a..56eeee1cbccc 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -233,7 +233,6 @@ struct mptcp_pm_data {
>  	u8		add_addr_signaled;
>  	u8		add_addr_accepted;
>  	u8		local_addr_used;
> -	u8		pm_type;
>  	u8		subflows;
>  	u8		status;
>  	DECLARE_BITMAP(id_avail_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
> @@ -1101,12 +1100,12 @@ static inline bool mptcp_pm_should_rm_signal(struct mptcp_sock *msk)
>  
>  static inline bool mptcp_pm_is_userspace(const struct mptcp_sock *msk)
>  {
> -	return READ_ONCE(msk->pm.pm_type) == MPTCP_PM_TYPE_USERSPACE;
> +	return !strncmp(msk->pm.ops->name, "userspace", MPTCP_PM_NAME_MAX);

Please don't do a string comparison here.

I think it is better to drop msk->pm.pm_type when mptcp_pm_is_userspace
and mptcp_pm_is_kernel are both dropped, no?

If you want to drop pm_type before, then you could compare &msk->pm.ops,
but I think that's something that should be done later. I guess
mptcp_pm_is_userspace() will still be needed, but only from
pm_userspace.c with mptcp_userspace_pm_get_sock(). No?

Same for mptcp_pm_is_kernel(), only from pm_kernel.c when iterating over
all connections.  Except if you use introduce a new macro like
mptcp_pm_for_each_msk() taking in argument "&mptcp_pm_kernel"?

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


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

* Re: [PATCH mptcp-next v7 09/11] mptcp: sysctl: drop get_pm_type helper
  2025-03-03  4:22 ` [PATCH mptcp-next v7 09/11] mptcp: sysctl: drop get_pm_type helper Geliang Tang
@ 2025-03-03 10:57   ` Matthieu Baerts
  0 siblings, 0 replies; 24+ messages in thread
From: Matthieu Baerts @ 2025-03-03 10:57 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 03/03/2025 05:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> The helper mptcp_get_pm_type() is unused now, it's replaced by the
> new one mptcp_get_path_manager(). So drop it.

Same as with the previous commit: probably too early to drop it. If we
drop it, we should drop the variable in the mptcp_pernet structure, see
my comment on patch 3/11.

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


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

* Re: [PATCH mptcp-next v7 10/11] mptcp: pm: make get_local_id helpers static
  2025-03-03  4:22 ` [PATCH mptcp-next v7 10/11] mptcp: pm: make get_local_id helpers static Geliang Tang
@ 2025-03-03 10:58   ` Matthieu Baerts
  0 siblings, 0 replies; 24+ messages in thread
From: Matthieu Baerts @ 2025-03-03 10:58 UTC (permalink / raw)
  To: Geliang Tang, mptcp; +Cc: Geliang Tang

Hi Geliang,

On 03/03/2025 05:22, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Now mptcp_pm_get_local_id() can directly invoke get_local_id() interface
> through "ops" of "msk->pm". Instead of using mptcp_pm_is_userspace() to
> check which get_local_id() helper to invoke.

As mentioned in a previous comment, I think it would be clearer to add
get_local_id() in the PM ops structure (+ the validation, + in the
different PM ops) here in this commit.

Same on the next patch, and for all future PM ops:
new/established/closed connections/subflow, ADD/RM_ADDR received, etc. →
everywhere mptcp_pm_is_userspace() or mptcp_pm_is_kernel() is used in pm.c.

Note that when it is needed to do actions from the worker, I guess it
would be better to schedule the worker if the corresponding pm->ops->XXX
is defined, then calling pm->ops->XXX from mptcp_pm_worker(). The idea
would be to get rid of __mptcp_pm_kernel_worker(), everything should be
done from mptcp_pm_worker(). That should also simplify the hooks for the
BPF PMs, only having hooks called while owning the MSK socket. WDYT?

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


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

end of thread, other threads:[~2025-03-03 10:58 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-03  4:22 [PATCH mptcp-next v7 00/11] BPF path manager, part 5 Geliang Tang
2025-03-03  4:22 ` [PATCH mptcp-next v7 01/11] mptcp: pm: define struct mptcp_pm_ops Geliang Tang
2025-03-03 10:39   ` Matthieu Baerts
2025-03-03  4:22 ` [PATCH mptcp-next v7 02/11] mptcp: sysctl: new sysctl to set path manager by name Geliang Tang
2025-03-03 10:40   ` Matthieu Baerts
2025-03-03  4:22 ` [PATCH mptcp-next v7 03/11] mptcp: sysctl: map pm_type to path_manager Geliang Tang
2025-03-03 10:40   ` Matthieu Baerts
2025-03-03  4:22 ` [PATCH mptcp-next v7 04/11] mptcp: sysctl: add available_path_managers Geliang Tang
2025-03-03 10:41   ` Matthieu Baerts
2025-03-03  4:22 ` [PATCH mptcp-next v7 05/11] mptcp: pm: in-kernel: register mptcp_kernel_pm Geliang Tang
2025-03-03 10:42   ` Matthieu Baerts
2025-03-03  4:22 ` [PATCH mptcp-next v7 06/11] mptcp: pm: userspace: register mptcp_userspace_pm Geliang Tang
2025-03-03 10:52   ` Matthieu Baerts
2025-03-03  4:22 ` [PATCH mptcp-next v7 07/11] mptcp: pm: initialize and release mptcp_pm_ops Geliang Tang
2025-03-03 10:53   ` Matthieu Baerts
2025-03-03  4:22 ` [PATCH mptcp-next v7 08/11] mptcp: pm: drop pm_type in mptcp_pm_data Geliang Tang
2025-03-03 10:57   ` Matthieu Baerts
2025-03-03  4:22 ` [PATCH mptcp-next v7 09/11] mptcp: sysctl: drop get_pm_type helper Geliang Tang
2025-03-03 10:57   ` Matthieu Baerts
2025-03-03  4:22 ` [PATCH mptcp-next v7 10/11] mptcp: pm: make get_local_id helpers static Geliang Tang
2025-03-03 10:58   ` Matthieu Baerts
2025-03-03  4:22 ` [PATCH mptcp-next v7 11/11] mptcp: pm: make is_backup " Geliang Tang
2025-03-03  5:32 ` [PATCH mptcp-next v7 00/11] BPF path manager, part 5 MPTCP CI
2025-03-03 10:38 ` Matthieu Baerts

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.