[PATCH mptcp-next v10 00/12] BPF path manager, part 5

[PATCH mptcp-next v10 00/12] BPF path manager, part 5

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

v10:
 - drop pm->init in mptcp_pm_validate, move mptcp_pm_validate into a new
   patch.
 - remove tlb, use ctl instead in proc_pm_type.
 - use "strncmp() == 0" instead of "!strncmp()".
 - set pm_type __MPTCP_PM_TYPE_NR for other names.
 - add WARN_ON_ONCE in mptcp_pm_unregister.
 - drop mptcp_userspace_pm_init.
 - rename "struct mptcp_pm_ops *pm" to "struct mptcp_pm_ops *pm_ops".
 - drop pm_ops NULL check in mptcp_pm_release.
 - add struct_group for struct mptcp_pm_data.
 - move selftests into userspace_pm.sh.

v9:
 - rename mptcp_pm_nl_ prefix to mptcp_pm_kernel_.
 - rename mptcp_userspace_pm_ prefix to mptcp_pm_userspace_.
 - update commit logs.

v8:
 - address Matt's comments in v7.

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.

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 (12):
  mptcp: pm: define struct mptcp_pm_ops
  mptcp: pm: register in-kernel and userspace PM
  mptcp: sysctl: set path manager by name
  mptcp: add struct_group in mptcp_pm_data
  mptcp: pm: init and release mptcp_pm_ops
  mptcp: pm: add get_local_id() interface
  mptcp: pm: add get_priority() interface
  mptcp: pm: validate mandatory ops
  mptcp: sysctl: map path_manager to pm_type
  mptcp: sysctl: map pm_type to path_manager
  mptcp: sysctl: add available_path_managers
  selftests: mptcp: add path_manager sysctl tests

 Documentation/networking/mptcp-sysctl.rst     |  23 +++
 include/net/mptcp.h                           |  19 +++
 net/mptcp/ctrl.c                              | 111 ++++++++++++-
 net/mptcp/pm.c                                | 157 +++++++++++++-----
 net/mptcp/pm_kernel.c                         |  35 +++-
 net/mptcp/pm_userspace.c                      |  26 ++-
 net/mptcp/protocol.h                          |  22 ++-
 .../selftests/net/mptcp/userspace_pm.sh       |  58 ++++++-
 8 files changed, 395 insertions(+), 56 deletions(-)

-- 
2.43.0

[PATCH mptcp-next v10 01/12] mptcp: pm: define struct mptcp_pm_ops

[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 an MPTCP path
manager, which contains a set of interfaces. Currently only init()
and release() interfaces are included, subsequent patches will add
others step by step.

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

"list" is used to add this path manager to mptcp_pm_list list when
it is registered. "name" is used to identify this path manager.
mptcp_pm_find() uses "name" to find a path manager on the list.

mptcp_pm_unregister is not used in this set, but will be invoked
in .unreg of struct bpf_struct_ops.

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

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index e9832a8f981e..69f7b35e57c3 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 */
@@ -112,6 +113,17 @@ struct mptcp_sched_ops {
 	void (*release)(struct mptcp_sock *msk);
 } ____cacheline_aligned_in_smp;
 
+#define MPTCP_PM_NAME_MAX	16
+
+struct mptcp_pm_ops {
+	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 04a156395aad..5ea5048dfd9b 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,
@@ -1025,3 +1030,37 @@ void __init mptcp_pm_init(void)
 	mptcp_pm_kernel_register();
 	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_ops;
+
+	list_for_each_entry_rcu(pm_ops, &mptcp_pm_list, list) {
+		if (!strcmp(pm_ops->name, name))
+			return pm_ops;
+	}
+
+	return NULL;
+}
+
+int mptcp_pm_register(struct mptcp_pm_ops *pm_ops)
+{
+	spin_lock(&mptcp_pm_list_lock);
+	if (mptcp_pm_find(pm_ops->name)) {
+		spin_unlock(&mptcp_pm_list_lock);
+		return -EEXIST;
+	}
+	list_add_tail_rcu(&pm_ops->list, &mptcp_pm_list);
+	spin_unlock(&mptcp_pm_list_lock);
+
+	pr_debug("%s registered\n", pm_ops->name);
+	return 0;
+}
+
+void mptcp_pm_unregister(struct mptcp_pm_ops *pm_ops)
+{
+	spin_lock(&mptcp_pm_list_lock);
+	list_del_rcu(&pm_ops->list);
+	spin_unlock(&mptcp_pm_list_lock);
+}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 3492b256ecba..dfb981a83f31 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1047,6 +1047,10 @@ 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_register(struct mptcp_pm_ops *pm_ops);
+void mptcp_pm_unregister(struct mptcp_pm_ops *pm_ops);
+
 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

[PATCH mptcp-next v10 02/12] mptcp: pm: register in-kernel and userspace PM

[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_pm_kernel", and register it in
mptcp_pm_kernel_register(). Also define the userspace path manager
as a new struct mptcp_pm_ops named "mptcp_pm_userspace", and register
it in mptcp_pm_init().

To ensure that there's always a valid path manager available, the
default path manager "mptcp_pm_kernel" will be skipped in
mptcp_pm_unregister().

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

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 5ea5048dfd9b..eefed554dcc9 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();
 }
 
@@ -1060,6 +1061,10 @@ int mptcp_pm_register(struct mptcp_pm_ops *pm_ops)
 
 void mptcp_pm_unregister(struct mptcp_pm_ops *pm_ops)
 {
+	/* skip unregistering the default path manager */
+	if (WARN_ON_ONCE(pm_ops == &mptcp_pm_kernel))
+		return;
+
 	spin_lock(&mptcp_pm_list_lock);
 	list_del_rcu(&pm_ops->list);
 	spin_unlock(&mptcp_pm_list_lock);
diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 806a9b5b3c07..d39e7c178460 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -1398,8 +1398,15 @@ static struct pernet_operations mptcp_pm_pernet_ops = {
 	.size = sizeof(struct pm_nl_pernet),
 };
 
+struct mptcp_pm_ops mptcp_pm_kernel = {
+	.name			= "kernel",
+	.owner			= THIS_MODULE,
+};
+
 void __init mptcp_pm_kernel_register(void)
 {
 	if (register_pernet_subsys(&mptcp_pm_pernet_ops) < 0)
 		panic("Failed to register MPTCP PM pernet subsystem.\n");
+
+	mptcp_pm_register(&mptcp_pm_kernel);
 }
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 13856df22673..2cb62f026b1f 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -682,3 +682,13 @@ int mptcp_userspace_pm_get_addr(u8 id, struct mptcp_pm_addr_entry *addr,
 	sock_put(sk);
 	return ret;
 }
+
+static struct mptcp_pm_ops mptcp_pm_userspace = {
+	.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 dfb981a83f31..3f820a9e453f 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1047,6 +1047,9 @@ 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);
 
+/* the default path manager, used in mptcp_pm_unregister */
+extern struct mptcp_pm_ops mptcp_pm_kernel;
+
 struct mptcp_pm_ops *mptcp_pm_find(const char *name);
 int mptcp_pm_register(struct mptcp_pm_ops *pm_ops);
 void mptcp_pm_unregister(struct mptcp_pm_ops *pm_ops);
@@ -1154,6 +1157,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

[PATCH mptcp-next v10 03/12] mptcp: sysctl: set path manager by name

[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 | 19 +++++++++
 net/mptcp/ctrl.c                          | 50 +++++++++++++++++++++++
 net/mptcp/protocol.h                      |  1 +
 3 files changed, 70 insertions(+)

diff --git a/Documentation/networking/mptcp-sysctl.rst b/Documentation/networking/mptcp-sysctl.rst
index 03e1d3610333..b78a2254d452 100644
--- a/Documentation/networking/mptcp-sysctl.rst
+++ b/Documentation/networking/mptcp-sysctl.rst
@@ -72,6 +72,23 @@ 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.
+
+	This is a per-namespace sysctl.
+
+	* "kernel"          - In-kernel path manager
+	* "userspace"       - Userspace path manager
+
+	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 +101,8 @@ pm_type - INTEGER
 
 	This is a per-namespace sysctl.
 
+	Deprecated since v6.15, 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..1f405be6bc00 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 pm_name[MPTCP_PM_NAME_MAX];
+	const struct ctl_table tbl = {
+		.data = pm_name,
+		.maxlen = MPTCP_PM_NAME_MAX,
+	};
+	int ret;
+
+	strscpy(pm_name, *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, pm_name);
+
+	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 3f820a9e453f..0ef758d233b7 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

[PATCH mptcp-next v10 04/12] mptcp: add struct_group in mptcp_pm_data

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch adds a "struct_group(reset, ...)" in struct mptcp_pm_data to
simplify the reset, and make sure we don't miss any.

Suggested-by: Matthieu Baerts <matttbe@kernel.org>
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm.c       | 14 +-------------
 net/mptcp/protocol.h |  4 ++++
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index eefed554dcc9..1400bfed4b0d 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -983,12 +983,7 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk)
 	u8 pm_type = mptcp_get_pm_type(sock_net((struct sock *)msk));
 	struct mptcp_pm_data *pm = &msk->pm;
 
-	pm->add_addr_signaled = 0;
-	pm->add_addr_accepted = 0;
-	pm->local_addr_used = 0;
-	pm->subflows = 0;
-	pm->rm_list_tx.nr = 0;
-	pm->rm_list_rx.nr = 0;
+	memset(&pm->reset, 0, sizeof(pm->reset));
 	WRITE_ONCE(pm->pm_type, pm_type);
 
 	if (pm_type == MPTCP_PM_TYPE_KERNEL) {
@@ -1005,15 +1000,8 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk)
 			   !!mptcp_pm_get_add_addr_accept_max(msk) &&
 			   subflows_allowed);
 		WRITE_ONCE(pm->accept_subflow, subflows_allowed);
-	} else {
-		WRITE_ONCE(pm->work_pending, 0);
-		WRITE_ONCE(pm->accept_addr, 0);
-		WRITE_ONCE(pm->accept_subflow, 0);
 	}
 
-	WRITE_ONCE(pm->addr_signal, 0);
-	WRITE_ONCE(pm->remote_deny_join_id0, false);
-	pm->status = 0;
 	bitmap_fill(pm->id_avail_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
 }
 
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 0ef758d233b7..47710db243f4 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -223,6 +223,8 @@ struct mptcp_pm_data {
 
 	spinlock_t	lock;		/*protects the whole PM data */
 
+	struct_group(reset,
+
 	u8		addr_signal;
 	bool		server_side;
 	bool		work_pending;
@@ -238,6 +240,8 @@ struct mptcp_pm_data {
 	DECLARE_BITMAP(id_avail_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
 	struct mptcp_rm_list rm_list_tx;
 	struct mptcp_rm_list rm_list_rx;
+
+	);
 };
 
 struct mptcp_pm_local {
-- 
2.43.0

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

[Geliang Tang]

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
@@ -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 */
 
-- 
2.43.0

[PATCH mptcp-next v10 06/12] mptcp: pm: add get_local_id() interface

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch adds get_local_id() interface for struct mptcp_sched_ops,
then 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.

Rename
	mptcp_pm_nl_get_local_id()/mptcp_userspace_pm_get_local_id()
to
	mptcp_pm_kernel_get_local_id()/mptcp_pm_userspace_get_local_id()

and make them static.

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

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 69f7b35e57c3..169e19440b03 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -116,6 +116,9 @@ struct mptcp_sched_ops {
 #define MPTCP_PM_NAME_MAX	16
 
 struct mptcp_pm_ops {
+	int (*get_local_id)(struct mptcp_sock *msk,
+			    struct mptcp_pm_addr_entry *skc);
+
 	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 1a2f1715871c..d2dfd91cab46 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 03c4584cca7a..a67e94c02c64 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_kernel_get_local_id(struct mptcp_sock *msk,
+					struct mptcp_pm_addr_entry *skc)
 {
 	struct mptcp_pm_addr_entry *entry;
 	struct pm_nl_pernet *pernet;
@@ -1417,6 +1417,7 @@ static void mptcp_pm_kernel_init(struct mptcp_sock *msk)
 }
 
 struct mptcp_pm_ops mptcp_pm_kernel = {
+	.get_local_id		= mptcp_pm_kernel_get_local_id,
 	.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 6afab631580f..81ddc14bc273 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -126,8 +126,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_pm_userspace_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;
@@ -689,6 +689,7 @@ static void mptcp_pm_userspace_release(struct mptcp_sock *msk)
 }
 
 static struct mptcp_pm_ops mptcp_pm_userspace = {
+	.get_local_id		= mptcp_pm_userspace_get_local_id,
 	.release		= mptcp_pm_userspace_release,
 	.name			= "userspace",
 	.owner			= THIS_MODULE,
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index fa9bb385feb9..6cae1ae5e9be 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

[PATCH mptcp-next v10 07/12] mptcp: pm: add get_priority() interface

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch adds get_priority() interface for struct mptcp_sched_ops,
then 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.

Rename
	mptcp_pm_nl_is_backup()/mptcp_userspace_pm_is_backup()
to
	mptcp_pm_kernel_get_priority()/mptcp_pm_userspace_get_priority()

and make them static.

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

diff --git a/include/net/mptcp.h b/include/net/mptcp.h
index 169e19440b03..b344b909e749 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -118,6 +118,8 @@ struct mptcp_sched_ops {
 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;
diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index d2dfd91cab46..29bc903658f7 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 a67e94c02c64..9fc140d8fe26 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -722,7 +722,8 @@ static int mptcp_pm_kernel_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_kernel_get_priority(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;
@@ -1418,6 +1419,7 @@ static void mptcp_pm_kernel_init(struct mptcp_sock *msk)
 
 struct mptcp_pm_ops mptcp_pm_kernel = {
 	.get_local_id		= mptcp_pm_kernel_get_local_id,
+	.get_priority		= mptcp_pm_kernel_get_priority,
 	.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 81ddc14bc273..7fc19b844384 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -145,8 +145,8 @@ static int mptcp_pm_userspace_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_pm_userspace_get_priority(struct mptcp_sock *msk,
+					    struct mptcp_addr_info *skc)
 {
 	struct mptcp_pm_addr_entry *entry;
 	bool backup;
@@ -690,6 +690,7 @@ static void mptcp_pm_userspace_release(struct mptcp_sock *msk)
 
 static struct mptcp_pm_ops mptcp_pm_userspace = {
 	.get_local_id		= mptcp_pm_userspace_get_local_id,
+	.get_priority		= mptcp_pm_userspace_get_priority,
 	.release		= mptcp_pm_userspace_release,
 	.name			= "userspace",
 	.owner			= THIS_MODULE,
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 6cae1ae5e9be..d26e89d960a1 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

[PATCH mptcp-next v10 08/12] mptcp: pm: validate mandatory ops

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch adds a helper mptcp_pm_validate() to check whether required
ops are defined. It will be invoked in .validate of struct bpf_struct_ops.

Currently mandatory ops of mptcp_pm_ops are get_local_id and get_priority.

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

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 29bc903658f7..5d666b0891b3 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -1045,8 +1045,24 @@ struct mptcp_pm_ops *mptcp_pm_find(const char *name)
 	return NULL;
 }
 
+int mptcp_pm_validate(struct mptcp_pm_ops *pm_ops)
+{
+	if (!pm_ops->get_local_id || !pm_ops->get_priority) {
+		pr_err("%s does not implement required ops\n", pm_ops->name);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 int mptcp_pm_register(struct mptcp_pm_ops *pm_ops)
 {
+	int ret;
+
+	ret = mptcp_pm_validate(pm_ops);
+	if (ret)
+		return ret;
+
 	spin_lock(&mptcp_pm_list_lock);
 	if (mptcp_pm_find(pm_ops->name)) {
 		spin_unlock(&mptcp_pm_list_lock);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index d26e89d960a1..1ce6c22cb295 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1059,6 +1059,7 @@ extern struct mptcp_pm_ops mptcp_pm_kernel;
 struct mptcp_pm_ops *mptcp_pm_find(const char *name);
 int mptcp_pm_register(struct mptcp_pm_ops *pm_ops);
 void mptcp_pm_unregister(struct mptcp_pm_ops *pm_ops);
+int mptcp_pm_validate(struct mptcp_pm_ops *pm_ops);
 
 void mptcp_userspace_pm_free_local_addr_list(struct mptcp_sock *msk);
 
-- 
2.43.0

[PATCH mptcp-next v10 09/12] mptcp: sysctl: map path_manager to pm_type

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch maps the newly added path manager sysctl "path_manager"
to the old one "pm_type".

	path_manager   pm_type

	"kernel"    -> MPTCP_PM_TYPE_KERNEL
	"userspace" -> MPTCP_PM_TYPE_USERSPACE
	others      -> __MPTCP_PM_TYPE_NR

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

diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 1f405be6bc00..6a35e6bb0a63 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -200,6 +200,9 @@ static int mptcp_set_path_manager(char *path_manager, const char *name)
 static int proc_path_manager(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,
+						   path_manager);
 	char (*path_manager)[MPTCP_PM_NAME_MAX] = ctl->data;
 	char pm_name[MPTCP_PM_NAME_MAX];
 	const struct ctl_table tbl = {
@@ -211,8 +214,16 @@ static int proc_path_manager(const struct ctl_table *ctl, int write,
 	strscpy(pm_name, *path_manager, MPTCP_PM_NAME_MAX);
 
 	ret = proc_dostring(&tbl, write, buffer, lenp, ppos);
-	if (write && ret == 0)
+	if (write && ret == 0) {
+		u8 pm_type = __MPTCP_PM_TYPE_NR;
+
+		if (strncmp(pm_name, "kernel", MPTCP_PM_NAME_MAX) == 0)
+			pm_type = MPTCP_PM_TYPE_KERNEL;
+		else if (strncmp(pm_name, "userspace", MPTCP_PM_NAME_MAX) == 0)
+			pm_type = MPTCP_PM_TYPE_USERSPACE;
+		pernet->pm_type = pm_type;
 		ret = mptcp_set_path_manager(*path_manager, pm_name);
+	}
 
 	return ret;
 }
-- 
2.43.0

[PATCH mptcp-next v10 10/12] mptcp: sysctl: map pm_type to path_manager

[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".

	path_manager		   pm_type

	MPTCP_PM_TYPE_KERNEL    -> "kernel"
	MPTCP_PM_TYPE_USERSPACE -> "userspace"

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

diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
index 6a35e6bb0a63..357083977ba5 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -228,6 +228,29 @@ 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);
+	int ret;
+
+	ret = proc_dou8vec_minmax(ctl, write, buffer, lenp, ppos);
+	if (write && ret == 0) {
+		u8 pm_type = READ_ONCE(*(u8 *)ctl->data);
+		char *pm_name = "unknown";
+
+		if (pm_type == MPTCP_PM_TYPE_KERNEL)
+			pm_name = "kernel";
+		else if (pm_type == MPTCP_PM_TYPE_USERSPACE)
+			pm_name = "userspace";
+		mptcp_set_path_manager(pernet->path_manager, pm_name);
+	}
+
+	return ret;
+}
+
 static struct ctl_table mptcp_sysctl_table[] = {
 	{
 		.procname = "enabled",
@@ -272,7 +295,7 @@ static struct ctl_table mptcp_sysctl_table[] = {
 		.procname = "pm_type",
 		.maxlen = sizeof(u8),
 		.mode = 0644,
-		.proc_handler = proc_dou8vec_minmax,
+		.proc_handler = proc_pm_type,
 		.extra1       = SYSCTL_ZERO,
 		.extra2       = &mptcp_pm_type_max
 	},
-- 
2.43.0

[PATCH mptcp-next v10 11/12] mptcp: sysctl: add available_path_managers

[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 managers.

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 b78a2254d452..5bfab01eff5a 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 b344b909e749..6a08ac862bbe 100644
--- a/include/net/mptcp.h
+++ b/include/net/mptcp.h
@@ -114,6 +114,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 357083977ba5..14c32d06f51e 100644
--- a/net/mptcp/ctrl.c
+++ b/net/mptcp/ctrl.c
@@ -251,6 +251,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",
@@ -336,6 +354,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)
@@ -362,6 +386,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 5d666b0891b3..ef76939435bf 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -1085,3 +1085,22 @@ void mptcp_pm_unregister(struct mptcp_pm_ops *pm_ops)
 	list_del_rcu(&pm_ops->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_ops;
+	size_t offs = 0;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(pm_ops, &mptcp_pm_list, list) {
+		offs += snprintf(buf + offs, maxlen - offs, "%s%s",
+				 offs == 0 ? "" : " ", pm_ops->name);
+
+		if (WARN_ON_ONCE(offs >= maxlen))
+			break;
+	}
+	rcu_read_unlock();
+}
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 1ce6c22cb295..5a64ed3049d1 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1060,6 +1060,7 @@ struct mptcp_pm_ops *mptcp_pm_find(const char *name);
 int mptcp_pm_register(struct mptcp_pm_ops *pm_ops);
 void mptcp_pm_unregister(struct mptcp_pm_ops *pm_ops);
 int mptcp_pm_validate(struct mptcp_pm_ops *pm_ops);
+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

[PATCH mptcp-next v10 12/12] selftests: mptcp: add path_manager sysctl tests

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

This patch checks if the newly added net.mptcp.path_manager is mapped
successfully from or to the old net.mptcp.pm_type. And add a new helper
set_path_manager() to set the newly added net.mptcp.path_manager.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>

fix

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 .../selftests/net/mptcp/userspace_pm.sh       | 58 ++++++++++++++++++-
 1 file changed, 55 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 3651f73451cf..35ba4edc0fa2 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -118,9 +118,61 @@ trap cleanup EXIT
 
 # Create and configure network namespaces for testing
 mptcp_lib_ns_init ns1 ns2
-for i in "$ns1" "$ns2" ;do
-	ip netns exec "$i" sysctl -q net.mptcp.pm_type=1
-done
+
+set_path_manager()
+{
+	local ns=$1
+	local pm=$2
+
+	if ! ip netns exec ${ns} sysctl net.mptcp.available_path_managers |
+	     grep -wq "${pm}"; then
+		test_fail "path manager ${pm} not found"
+		return 1
+	fi
+	ip netns exec ${ns} sysctl -q net.mptcp.path_manager="${pm}"
+}
+
+if [ -f /proc/sys/net/mptcp/path_manager ]; then
+	ip netns exec "$ns1" sysctl -q net.mptcp.pm_type=0
+	pm_name="$(ip netns exec "$ns1" sysctl -n net.mptcp.path_manager)"
+	if [ "${pm_name}" != "kernel" ]; then
+		test_fail "unexpected pm_name: ${pm_name}"
+		mptcp_lib_result_print_all_tap
+		exit ${KSFT_FAIL}
+	fi
+
+	ip netns exec "$ns1" sysctl -q net.mptcp.pm_type=1
+	pm_name="$(ip netns exec "$ns1" sysctl -n net.mptcp.path_manager)"
+	if [ "${pm_name}" != "userspace" ]; then
+		test_fail "unexpected pm_name: ${pm_name}"
+		mptcp_lib_result_print_all_tap
+		exit ${KSFT_FAIL}
+	fi
+
+	set_path_manager "$ns1" "kernel"
+	pm_type="$(ip netns exec "$ns1" sysctl -n net.mptcp.pm_type)"
+	if [ "${pm_type}" != "0" ]; then
+		test_fail "unexpected pm_type: ${pm_type}"
+		mptcp_lib_result_print_all_tap
+		exit ${KSFT_FAIL}
+	fi
+
+	set_path_manager "$ns1" "userspace"
+	pm_type="$(ip netns exec "$ns1" sysctl -n net.mptcp.pm_type)"
+	if [ "${pm_type}" != "1" ]; then
+		test_fail "unexpected pm_type: ${pm_type}"
+		mptcp_lib_result_print_all_tap
+		exit ${KSFT_FAIL}
+	fi
+
+	set_path_manager "$ns2" "userspace"
+	print_test "check path_manager and pm_type sysctl mapping"
+	test_pass
+else
+	for i in "$ns1" "$ns2"; do
+		ip netns exec "$i" sysctl -q net.mptcp.pm_type=1
+	done
+fi
 
 #  "$ns1"              ns2
 #     ns1eth2    ns2eth1
-- 
2.43.0

Re: BPF path manager, part 5

[MPTCP CI]

Hi Geliang,

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/13697375040

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


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)

Re: [PATCH mptcp-next v10 00/12] BPF path manager, part 5

[Matthieu Baerts]

Hi Geliang,

On 06/03/2025 12:01, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> v10:
>  - drop pm->init in mptcp_pm_validate, move mptcp_pm_validate into a new
>    patch.
>  - remove tlb, use ctl instead in proc_pm_type.
>  - use "strncmp() == 0" instead of "!strncmp()".
>  - set pm_type __MPTCP_PM_TYPE_NR for other names.
>  - add WARN_ON_ONCE in mptcp_pm_unregister.
>  - drop mptcp_userspace_pm_init.
>  - rename "struct mptcp_pm_ops *pm" to "struct mptcp_pm_ops *pm_ops".
>  - drop pm_ops NULL check in mptcp_pm_release.
>  - add struct_group for struct mptcp_pm_data.
>  - move selftests into userspace_pm.sh.

Thank you for the new version and the changelog!

I have some comments, almost there. If you already have patches adding
the remaining function pointers to the new mptcp_pm_ops, feel free to
add them in the next version.

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

Re: [PATCH mptcp-next v10 04/12] mptcp: add struct_group in mptcp_pm_data

[Matthieu Baerts]

Hi Geliang,

On 06/03/2025 12:01, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch adds a "struct_group(reset, ...)" in struct mptcp_pm_data to
> simplify the reset, and make sure we don't miss any.

Do you mind checking if, before this patch, we didn't already miss the
reset of some fields? (I think I quickly checked last time and
everything was reset here, or a bit later (e.g. server_side). If we
missed something, we will need a dedicated patch reseting the missing
fields first, with a Fixes tag, for -net, then this patch using
'struct_group' to ease the reset.

> Suggested-by: Matthieu Baerts <matttbe@kernel.org>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm.c       | 14 +-------------
>  net/mptcp/protocol.h |  4 ++++
>  2 files changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index eefed554dcc9..1400bfed4b0d 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -983,12 +983,7 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk)
>  	u8 pm_type = mptcp_get_pm_type(sock_net((struct sock *)msk));
>  	struct mptcp_pm_data *pm = &msk->pm;
>  
> -	pm->add_addr_signaled = 0;
> -	pm->add_addr_accepted = 0;
> -	pm->local_addr_used = 0;
> -	pm->subflows = 0;
> -	pm->rm_list_tx.nr = 0;
> -	pm->rm_list_rx.nr = 0;

Just to be sure, is it OK to reset the list with memset(0)? (I didn't check)

> +	memset(&pm->reset, 0, sizeof(pm->reset));
>  	WRITE_ONCE(pm->pm_type, pm_type);
>  
>  	if (pm_type == MPTCP_PM_TYPE_KERNEL) {
> @@ -1005,15 +1000,8 @@ void mptcp_pm_data_reset(struct mptcp_sock *msk)
>  			   !!mptcp_pm_get_add_addr_accept_max(msk) &&
>  			   subflows_allowed);
>  		WRITE_ONCE(pm->accept_subflow, subflows_allowed);
> -	} else {
> -		WRITE_ONCE(pm->work_pending, 0);
> -		WRITE_ONCE(pm->accept_addr, 0);
> -		WRITE_ONCE(pm->accept_subflow, 0);
>  	}
>  
> -	WRITE_ONCE(pm->addr_signal, 0);
> -	WRITE_ONCE(pm->remote_deny_join_id0, false);
> -	pm->status = 0;
>  	bitmap_fill(pm->id_avail_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
>  }
>  
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 0ef758d233b7..47710db243f4 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -223,6 +223,8 @@ struct mptcp_pm_data {
>  
>  	spinlock_t	lock;		/*protects the whole PM data */
>  
> +	struct_group(reset,
> +
>  	u8		addr_signal;
>  	bool		server_side;
>  	bool		work_pending;
> @@ -238,6 +240,8 @@ struct mptcp_pm_data {
>  	DECLARE_BITMAP(id_avail_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);

Maybe better to move this bitmap after the reset group, because it is
large, and it will be overwritten with 1 just after the reset.

>  	struct mptcp_rm_list rm_list_tx;
>  	struct mptcp_rm_list rm_list_rx;
> +
> +	);
>  };
>  
>  struct mptcp_pm_local {
Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

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

[Matthieu Baerts]

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
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next v10 08/12] mptcp: pm: validate mandatory ops

[Matthieu Baerts]

Hi Geliang,

On 06/03/2025 12:01, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch adds a helper mptcp_pm_validate() to check whether required
> ops are defined. It will be invoked in .validate of struct bpf_struct_ops.
> 
> Currently mandatory ops of mptcp_pm_ops are get_local_id and get_priority.

Because more ops will be added soon, it looks a bit strange to introduce
this helper now, no? If you have to change something else in this
series, maybe best to squash this in patch 2/12 ("mptcp: pm: define
struct mptcp_pm_ops") but ...

> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm.c       | 16 ++++++++++++++++
>  net/mptcp/protocol.h |  1 +
>  2 files changed, 17 insertions(+)
> 
> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index 29bc903658f7..5d666b0891b3 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c
> @@ -1045,8 +1045,24 @@ struct mptcp_pm_ops *mptcp_pm_find(const char *name)
>  	return NULL;
>  }
>  
> +int mptcp_pm_validate(struct mptcp_pm_ops *pm_ops)
> +{
> +	if (!pm_ops->get_local_id || !pm_ops->get_priority) {
> +		pr_err("%s does not implement required ops\n", pm_ops->name);
> +		return -EINVAL;
> +	}

... without this if-statement: this can be introduced when adding each
mandatory ops. I think this will help better understanding that they are
mandatory when they are being introduced (patch 7 and 8/12). WDYT?

(Please then add something like this in the commit message of patch 2/12
after the squash:)

  mptcp_pm_validate() will be invoked in .validate of struct
  bpf_struct_ops. That's why this function is exported.

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

Re: [PATCH mptcp-next v10 09/12] mptcp: sysctl: map path_manager to pm_type

[Matthieu Baerts]

Hi Geliang,

On 06/03/2025 12:01, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch maps the newly added path manager sysctl "path_manager"
> to the old one "pm_type".
> 
> 	path_manager   pm_type
> 
> 	"kernel"    -> MPTCP_PM_TYPE_KERNEL
> 	"userspace" -> MPTCP_PM_TYPE_USERSPACE
> 	others      -> __MPTCP_PM_TYPE_NR
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/ctrl.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> index 1f405be6bc00..6a35e6bb0a63 100644
> --- a/net/mptcp/ctrl.c
> +++ b/net/mptcp/ctrl.c
> @@ -200,6 +200,9 @@ static int mptcp_set_path_manager(char *path_manager, const char *name)
>  static int proc_path_manager(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,
> +						   path_manager);
>  	char (*path_manager)[MPTCP_PM_NAME_MAX] = ctl->data;
>  	char pm_name[MPTCP_PM_NAME_MAX];
>  	const struct ctl_table tbl = {
> @@ -211,8 +214,16 @@ static int proc_path_manager(const struct ctl_table *ctl, int write,
>  	strscpy(pm_name, *path_manager, MPTCP_PM_NAME_MAX);
>  
>  	ret = proc_dostring(&tbl, write, buffer, lenp, ppos);
> -	if (write && ret == 0)
> +	if (write && ret == 0) {
> +		u8 pm_type = __MPTCP_PM_TYPE_NR;
> +
> +		if (strncmp(pm_name, "kernel", MPTCP_PM_NAME_MAX) == 0)
> +			pm_type = MPTCP_PM_TYPE_KERNEL;
> +		else if (strncmp(pm_name, "userspace", MPTCP_PM_NAME_MAX) == 0)
> +			pm_type = MPTCP_PM_TYPE_USERSPACE;
> +		pernet->pm_type = pm_type;

This should be done after mptcp_set_path_manager(), and if there were no
errors (ret == 0), because if the name is not valid, the path-manager
will not be modified, so pm_type should not be modified as well.

BTW, did you check that setting "pm_type = __MPTCP_PM_TYPE_NR" is OK?
Can we display 2, but not allow to set 2 for pm_type here? In other
words, please check that this is OK with your modifications:

  # sysctl net.mptcp.pm_type=2
  sysctl: setting key "net.mptcp.pm_type": Invalid argument
  # sysctl -q net.mptcp.path_manager = "$BPF_PM"
  # sysctl -n net.mptcp.pm_type
  2

>  		ret = mptcp_set_path_manager(*path_manager, pm_name);
> +	}
>  
>  	return ret;
>  }
Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next v10 10/12] mptcp: sysctl: map pm_type to path_manager

[Matthieu Baerts]

Hi Geliang,

On 06/03/2025 12:01, 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".
> 
> 	path_manager		   pm_type
> 
> 	MPTCP_PM_TYPE_KERNEL    -> "kernel"
> 	MPTCP_PM_TYPE_USERSPACE -> "userspace"
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/ctrl.c | 25 ++++++++++++++++++++++++-
>  1 file changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> index 6a35e6bb0a63..357083977ba5 100644
> --- a/net/mptcp/ctrl.c
> +++ b/net/mptcp/ctrl.c
> @@ -228,6 +228,29 @@ 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);
> +	int ret;
> +
> +	ret = proc_dou8vec_minmax(ctl, write, buffer, lenp, ppos);
> +	if (write && ret == 0) {
> +		u8 pm_type = READ_ONCE(*(u8 *)ctl->data);
> +		char *pm_name = "unknown";

Small detail: thanks to proc_dou8vec_minmax, pm_name cannot be
"unknown": it should only be 0 or 1. Then you can either not assign it,
or assign it to an empty string "just in case"?

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

Re: [PATCH mptcp-next v10 12/12] selftests: mptcp: add path_manager sysctl tests

[Matthieu Baerts]

Hi Geliang,

On 06/03/2025 12:01, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> This patch checks if the newly added net.mptcp.path_manager is mapped
> successfully from or to the old net.mptcp.pm_type. And add a new helper
> set_path_manager() to set the newly added net.mptcp.path_manager.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> 
> fix

(typo)

> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  .../selftests/net/mptcp/userspace_pm.sh       | 58 ++++++++++++++++++-
>  1 file changed, 55 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> index 3651f73451cf..35ba4edc0fa2 100755
> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> @@ -118,9 +118,61 @@ trap cleanup EXIT
>  
>  # Create and configure network namespaces for testing

I see that you didn't move the "print_title" and "print_test" here. I
think you should do that to be able to print test_fail properly, no?

>  mptcp_lib_ns_init ns1 ns2
> -for i in "$ns1" "$ns2" ;do
> -	ip netns exec "$i" sysctl -q net.mptcp.pm_type=1
> -done
> +
> +set_path_manager()
> +{
> +	local ns=$1
> +	local pm=$2
> +
> +	if ! ip netns exec ${ns} sysctl net.mptcp.available_path_managers |
> +	     grep -wq "${pm}"; then
> +		test_fail "path manager ${pm} not found"
> +		return 1
> +	fi
> +	ip netns exec ${ns} sysctl -q net.mptcp.path_manager="${pm}"
> +}
> +

Maybe add a comment here to explain what's the goal, e.g.

  # check path_manager and pm_type sysctl mapping

> +if [ -f /proc/sys/net/mptcp/path_manager ]; then
> +	ip netns exec "$ns1" sysctl -q net.mptcp.pm_type=0
> +	pm_name="$(ip netns exec "$ns1" sysctl -n net.mptcp.path_manager)"
> +	if [ "${pm_name}" != "kernel" ]; then
> +		test_fail "unexpected pm_name: ${pm_name}"
> +		mptcp_lib_result_print_all_tap
> +		exit ${KSFT_FAIL}
> +	fi
I don't think this step should be moved below, because here we don't
change values: pm_type=0 is the default value.

> +	ip netns exec "$ns1" sysctl -q net.mptcp.pm_type=1
> +	pm_name="$(ip netns exec "$ns1" sysctl -n net.mptcp.path_manager)"
> +	if [ "${pm_name}" != "userspace" ]; then
> +		test_fail "unexpected pm_name: ${pm_name}"
> +		mptcp_lib_result_print_all_tap
> +		exit ${KSFT_FAIL}
> +	fi

I don't think you need this step if you keep the for-loop out of the
'else', like before, see below. It implicitly check that setting pm_type
to 1 also changes "path_manager", otherwise the rest of the tests will fail.

> +
> +	set_path_manager "$ns1" "kernel"
> +	pm_type="$(ip netns exec "$ns1" sysctl -n net.mptcp.pm_type)"
> +	if [ "${pm_type}" != "0" ]; then
> +		test_fail "unexpected pm_type: ${pm_type}"
> +		mptcp_lib_result_print_all_tap
> +		exit ${KSFT_FAIL}
> +	fi
> +
> +	set_path_manager "$ns1" "userspace"
> +	pm_type="$(ip netns exec "$ns1" sysctl -n net.mptcp.pm_type)"
> +	if [ "${pm_type}" != "1" ]; then
> +		test_fail "unexpected pm_type: ${pm_type}"
> +		mptcp_lib_result_print_all_tap
> +		exit ${KSFT_FAIL}
> +	fi
> +
> +	set_path_manager "$ns2" "userspace"
> +	print_test "check path_manager and pm_type sysctl mapping"
> +	test_pass
> +else
> +	for i in "$ns1" "$ns2"; do
> +		ip netns exec "$i" sysctl -q net.mptcp.pm_type=1
> +	done
> +fi

I do think there is no need to do repeating tests here, Just having
these 3 steps are enough to cover all cases directly and indirectly,
easier to maintain without the repeat, and only focus on the mapping
aspect, not on the final values we set (so keeping the 'pm_type=1' at
the end for all kernel versions is easier).

In other words:

  print_title "Init"
  print_test "Created network namespaces ns1, ns2"
  mptcp_lib_ns_init ns1 ns2

  # check path_manager and pm_type sysctl mapping
  if [ -f /proc/sys/net/mptcp/path_manager ]; then
      ip netns exec "$ns1" sysctl -q net.mptcp.path_manager=userspace
      pm_type="$(ip netns exec "$ns1" sysctl -n net.mptcp.pm_type)"
      if [ "${pm_type}" != "1" ]; then
          test_fail "unexpected pm_type: ${pm_type}"
          mptcp_lib_result_print_all_tap
          exit ${KSFT_FAIL}
      fi

      ip netns exec "$ns1" sysctl -q net.mptcp.pm_type=0
      pm="$(ip netns exec "$ns1" sysctl -n net.mptcp.path_manager)"
      if [ "${pm}" != "kernel" ]; then
          test_fail "unexpected path-manager: ${pm}"
          mptcp_lib_result_print_all_tap
          exit ${KSFT_FAIL}
      fi
  fi

  for i in "$ns1" "$ns2"; do
      ip netns exec "$i" sysctl -q net.mptcp.pm_type=1
  done

  (...)

Small and enough, no?

(If you want, you can also add an extra test setting "path_manager" to
"error", and checking that "pm_type" is unchanged, e.g. place this in
the middle of the two.)

      ip netns exec "$ns1" sysctl -q net.mptcp.path_manager=error
2>/dev/null
      pm_type="$(ip netns exec "$ns1" sysctl -n net.mptcp.pm_type)"
      if [ "${pm_type}" != "1" ]; then
          test_fail "unexpected pm_type after error: ${pm_type}"
          mptcp_lib_result_print_all_tap
          exit ${KSFT_FAIL}
      fi

And no need to have the same tests on ns2. Easier to keep the for-loop
setting pm_type=1 like before.

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

Re: [PATCH mptcp-next v10 04/12] mptcp: add struct_group in mptcp_pm_data

[Geliang Tang]

Hi Matt,

Thanks for the review.

On Tue, 2025-03-11 at 00:17 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 06/03/2025 12:01, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > This patch adds a "struct_group(reset, ...)" in struct
> > mptcp_pm_data to
> > simplify the reset, and make sure we don't miss any.
> 
> Do you mind checking if, before this patch, we didn't already miss
> the
> reset of some fields? (I think I quickly checked last time and
> everything was reset here, or a bit later (e.g. server_side). If we

Yes, I did check this, and nothing is missing.

> missed something, we will need a dedicated patch reseting the missing
> fields first, with a Fixes tag, for -net, then this patch using
> 'struct_group' to ease the reset.
> 
> > Suggested-by: Matthieu Baerts <matttbe@kernel.org>
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> >  net/mptcp/pm.c       | 14 +-------------
> >  net/mptcp/protocol.h |  4 ++++
> >  2 files changed, 5 insertions(+), 13 deletions(-)
> > 
> > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> > index eefed554dcc9..1400bfed4b0d 100644
> > --- a/net/mptcp/pm.c
> > +++ b/net/mptcp/pm.c
> > @@ -983,12 +983,7 @@ void mptcp_pm_data_reset(struct mptcp_sock
> > *msk)
> >  	u8 pm_type = mptcp_get_pm_type(sock_net((struct sock
> > *)msk));
> >  	struct mptcp_pm_data *pm = &msk->pm;
> >  
> > -	pm->add_addr_signaled = 0;
> > -	pm->add_addr_accepted = 0;
> > -	pm->local_addr_used = 0;
> > -	pm->subflows = 0;
> > -	pm->rm_list_tx.nr = 0;
> > -	pm->rm_list_rx.nr = 0;
> 
> Just to be sure, is it OK to reset the list with memset(0)? (I didn't
> check)

I kept this unchanged in v11.

> 
> > +	memset(&pm->reset, 0, sizeof(pm->reset));
> >  	WRITE_ONCE(pm->pm_type, pm_type);
> >  
> >  	if (pm_type == MPTCP_PM_TYPE_KERNEL) {
> > @@ -1005,15 +1000,8 @@ void mptcp_pm_data_reset(struct mptcp_sock
> > *msk)
> >  			   !!mptcp_pm_get_add_addr_accept_max(msk)
> > &&
> >  			   subflows_allowed);
> >  		WRITE_ONCE(pm->accept_subflow, subflows_allowed);
> > -	} else {
> > -		WRITE_ONCE(pm->work_pending, 0);
> > -		WRITE_ONCE(pm->accept_addr, 0);
> > -		WRITE_ONCE(pm->accept_subflow, 0);
> >  	}
> >  
> > -	WRITE_ONCE(pm->addr_signal, 0);
> > -	WRITE_ONCE(pm->remote_deny_join_id0, false);
> > -	pm->status = 0;
> >  	bitmap_fill(pm->id_avail_bitmap, MPTCP_PM_MAX_ADDR_ID +
> > 1);
> >  }
> >  
> > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> > index 0ef758d233b7..47710db243f4 100644
> > --- a/net/mptcp/protocol.h
> > +++ b/net/mptcp/protocol.h
> > @@ -223,6 +223,8 @@ struct mptcp_pm_data {
> >  
> >  	spinlock_t	lock;		/*protects the whole PM
> > data */
> >  
> > +	struct_group(reset,
> > +
> >  	u8		addr_signal;
> >  	bool		server_side;
> >  	bool		work_pending;
> > @@ -238,6 +240,8 @@ struct mptcp_pm_data {
> >  	DECLARE_BITMAP(id_avail_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
> 
> Maybe better to move this bitmap after the reset group, because it is
> large, and it will be overwritten with 1 just after the reset.

I agree. I moved this, together with rm_list_tx and rm_list_rx, after
the reset group in v11.

Thanks,
-Geliang

> 
> >  	struct mptcp_rm_list rm_list_tx;
> >  	struct mptcp_rm_list rm_list_rx;
> > +
> > +	);
> >  };
> >  
> >  struct mptcp_pm_local {
> Cheers,
> Matt

Re: [PATCH mptcp-next v10 09/12] mptcp: sysctl: map path_manager to pm_type

[Geliang Tang]

On Tue, 2025-03-11 at 00:19 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 06/03/2025 12:01, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > This patch maps the newly added path manager sysctl "path_manager"
> > to the old one "pm_type".
> > 
> > 	path_manager   pm_type
> > 
> > 	"kernel"    -> MPTCP_PM_TYPE_KERNEL
> > 	"userspace" -> MPTCP_PM_TYPE_USERSPACE
> > 	others      -> __MPTCP_PM_TYPE_NR
> > 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> >  net/mptcp/ctrl.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/net/mptcp/ctrl.c b/net/mptcp/ctrl.c
> > index 1f405be6bc00..6a35e6bb0a63 100644
> > --- a/net/mptcp/ctrl.c
> > +++ b/net/mptcp/ctrl.c
> > @@ -200,6 +200,9 @@ static int mptcp_set_path_manager(char
> > *path_manager, const char *name)
> >  static int proc_path_manager(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,
> > +						   path_manager);
> >  	char (*path_manager)[MPTCP_PM_NAME_MAX] = ctl->data;
> >  	char pm_name[MPTCP_PM_NAME_MAX];
> >  	const struct ctl_table tbl = {
> > @@ -211,8 +214,16 @@ static int proc_path_manager(const struct
> > ctl_table *ctl, int write,
> >  	strscpy(pm_name, *path_manager, MPTCP_PM_NAME_MAX);
> >  
> >  	ret = proc_dostring(&tbl, write, buffer, lenp, ppos);
> > -	if (write && ret == 0)
> > +	if (write && ret == 0) {
> > +		u8 pm_type = __MPTCP_PM_TYPE_NR;
> > +
> > +		if (strncmp(pm_name, "kernel", MPTCP_PM_NAME_MAX)
> > == 0)
> > +			pm_type = MPTCP_PM_TYPE_KERNEL;
> > +		else if (strncmp(pm_name, "userspace",
> > MPTCP_PM_NAME_MAX) == 0)
> > +			pm_type = MPTCP_PM_TYPE_USERSPACE;
> > +		pernet->pm_type = pm_type;
> 
> This should be done after mptcp_set_path_manager(), and if there were
> no
> errors (ret == 0), because if the name is not valid, the path-manager
> will not be modified, so pm_type should not be modified as well.

Good catch! Done in v11.

> 
> BTW, did you check that setting "pm_type = __MPTCP_PM_TYPE_NR" is OK?
> Can we display 2, but not allow to set 2 for pm_type here? In other
> words, please check that this is OK with your modifications:
> 
>   # sysctl net.mptcp.pm_type=2
>   sysctl: setting key "net.mptcp.pm_type": Invalid argument
>   # sysctl -q net.mptcp.path_manager = "$BPF_PM"
>   # sysctl -n net.mptcp.pm_type
>   2

I did check this and it matches the expectation.

Thanks,
-Geliang

> 
> >  		ret = mptcp_set_path_manager(*path_manager,
> > pm_name);
> > +	}
> >  
> >  	return ret;
> >  }
> Cheers,
> Matt

Re: [PATCH mptcp-next v10 12/12] selftests: mptcp: add path_manager sysctl tests

[Geliang Tang]

Hi Matt,

On Tue, 2025-03-11 at 00:25 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 06/03/2025 12:01, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > This patch checks if the newly added net.mptcp.path_manager is
> > mapped
> > successfully from or to the old net.mptcp.pm_type. And add a new
> > helper
> > set_path_manager() to set the newly added net.mptcp.path_manager.
> > 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > fix
> 
> (typo)
> 
> > 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> >  .../selftests/net/mptcp/userspace_pm.sh       | 58
> > ++++++++++++++++++-
> >  1 file changed, 55 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> > b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> > index 3651f73451cf..35ba4edc0fa2 100755
> > --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> > +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> > @@ -118,9 +118,61 @@ trap cleanup EXIT
> >  
> >  # Create and configure network namespaces for testing
> 
> I see that you didn't move the "print_title" and "print_test" here. I
> think you should do that to be able to print test_fail properly, no?
> 
> >  mptcp_lib_ns_init ns1 ns2
> > -for i in "$ns1" "$ns2" ;do
> > -	ip netns exec "$i" sysctl -q net.mptcp.pm_type=1
> > -done
> > +
> > +set_path_manager()
> > +{
> > +	local ns=$1
> > +	local pm=$2
> > +
> > +	if ! ip netns exec ${ns} sysctl
> > net.mptcp.available_path_managers |
> > +	     grep -wq "${pm}"; then
> > +		test_fail "path manager ${pm} not found"
> > +		return 1
> > +	fi
> > +	ip netns exec ${ns} sysctl -q
> > net.mptcp.path_manager="${pm}"
> > +}
> > +
> 
> Maybe add a comment here to explain what's the goal, e.g.
> 
>   # check path_manager and pm_type sysctl mapping
> 
> > +if [ -f /proc/sys/net/mptcp/path_manager ]; then
> > +	ip netns exec "$ns1" sysctl -q net.mptcp.pm_type=0
> > +	pm_name="$(ip netns exec "$ns1" sysctl -n
> > net.mptcp.path_manager)"
> > +	if [ "${pm_name}" != "kernel" ]; then
> > +		test_fail "unexpected pm_name: ${pm_name}"
> > +		mptcp_lib_result_print_all_tap
> > +		exit ${KSFT_FAIL}
> > +	fi
> I don't think this step should be moved below, because here we don't
> change values: pm_type=0 is the default value.
> 
> > +	ip netns exec "$ns1" sysctl -q net.mptcp.pm_type=1
> > +	pm_name="$(ip netns exec "$ns1" sysctl -n
> > net.mptcp.path_manager)"
> > +	if [ "${pm_name}" != "userspace" ]; then
> > +		test_fail "unexpected pm_name: ${pm_name}"
> > +		mptcp_lib_result_print_all_tap
> > +		exit ${KSFT_FAIL}
> > +	fi
> 
> I don't think you need this step if you keep the for-loop out of the
> 'else', like before, see below. It implicitly check that setting
> pm_type
> to 1 also changes "path_manager", otherwise the rest of the tests
> will fail.
> 
> > +
> > +	set_path_manager "$ns1" "kernel"
> > +	pm_type="$(ip netns exec "$ns1" sysctl -n
> > net.mptcp.pm_type)"
> > +	if [ "${pm_type}" != "0" ]; then
> > +		test_fail "unexpected pm_type: ${pm_type}"
> > +		mptcp_lib_result_print_all_tap
> > +		exit ${KSFT_FAIL}
> > +	fi
> > +
> > +	set_path_manager "$ns1" "userspace"
> > +	pm_type="$(ip netns exec "$ns1" sysctl -n
> > net.mptcp.pm_type)"
> > +	if [ "${pm_type}" != "1" ]; then
> > +		test_fail "unexpected pm_type: ${pm_type}"
> > +		mptcp_lib_result_print_all_tap
> > +		exit ${KSFT_FAIL}
> > +	fi
> > +
> > +	set_path_manager "$ns2" "userspace"
> > +	print_test "check path_manager and pm_type sysctl mapping"
> > +	test_pass
> > +else
> > +	for i in "$ns1" "$ns2"; do
> > +		ip netns exec "$i" sysctl -q net.mptcp.pm_type=1
> > +	done
> > +fi
> 
> I do think there is no need to do repeating tests here, Just having
> these 3 steps are enough to cover all cases directly and indirectly,
> easier to maintain without the repeat, and only focus on the mapping
> aspect, not on the final values we set (so keeping the 'pm_type=1' at
> the end for all kernel versions is easier).
> 
> In other words:
> 
>   print_title "Init"
>   print_test "Created network namespaces ns1, ns2"
>   mptcp_lib_ns_init ns1 ns2

test_pass should be followed this print_test to print a [OK].
So this may be better:

print_title "Init"
mptcp_lib_ns_init ns1 ns2

if []; then
	test_fail "..."
fi

print_test "Created network namespaces ns1, ns2"
test_pass

So in v11, I just moved 'print_title "Init"' forward but kept
"print_test" and "test_pass" unchanged.

Thanks,
-Geliang

> 
>   # check path_manager and pm_type sysctl mapping
>   if [ -f /proc/sys/net/mptcp/path_manager ]; then
>       ip netns exec "$ns1" sysctl -q net.mptcp.path_manager=userspace
>       pm_type="$(ip netns exec "$ns1" sysctl -n net.mptcp.pm_type)"
>       if [ "${pm_type}" != "1" ]; then
>           test_fail "unexpected pm_type: ${pm_type}"
>           mptcp_lib_result_print_all_tap
>           exit ${KSFT_FAIL}
>       fi
> 
>       ip netns exec "$ns1" sysctl -q net.mptcp.pm_type=0
>       pm="$(ip netns exec "$ns1" sysctl -n net.mptcp.path_manager)"
>       if [ "${pm}" != "kernel" ]; then
>           test_fail "unexpected path-manager: ${pm}"
>           mptcp_lib_result_print_all_tap
>           exit ${KSFT_FAIL}
>       fi
>   fi
> 
>   for i in "$ns1" "$ns2"; do
>       ip netns exec "$i" sysctl -q net.mptcp.pm_type=1
>   done
> 
>   (...)
> 
> Small and enough, no?
> 
> (If you want, you can also add an extra test setting "path_manager"
> to
> "error", and checking that "pm_type" is unchanged, e.g. place this in
> the middle of the two.)
> 
>       ip netns exec "$ns1" sysctl -q net.mptcp.path_manager=error
> 2>/dev/null
>       pm_type="$(ip netns exec "$ns1" sysctl -n net.mptcp.pm_type)"
>       if [ "${pm_type}" != "1" ]; then
>           test_fail "unexpected pm_type after error: ${pm_type}"
>           mptcp_lib_result_print_all_tap
>           exit ${KSFT_FAIL}
>       fi
> 
> And no need to have the same tests on ns2. Easier to keep the for-
> loop
> setting pm_type=1 like before.
> 
> Cheers,
> Matt

Re: [PATCH mptcp-next v10 04/12] mptcp: add struct_group in mptcp_pm_data

[Matthieu Baerts]

Hi Geliang,

On 11/03/2025 05:24, Geliang Tang wrote:
> Hi Matt,
> 
> Thanks for the review.
> 
> On Tue, 2025-03-11 at 00:17 +0100, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 06/03/2025 12:01, Geliang Tang wrote:
>>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>>
>>> This patch adds a "struct_group(reset, ...)" in struct
>>> mptcp_pm_data to
>>> simplify the reset, and make sure we don't miss any.
>>
>> Do you mind checking if, before this patch, we didn't already miss
>> the
>> reset of some fields? (I think I quickly checked last time and
>> everything was reset here, or a bit later (e.g. server_side). If we
> 
> Yes, I did check this, and nothing is missing.

Great, thank you for having checked!

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