[PATCH mptcp-next v3 0/9] BPF path manager, part 1

[PATCH mptcp-next v3 0/9] BPF path manager, part 1

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

v3:
 - address Matt's comments in v2 (thanks)
 - only include cleanups and refactoring patches in this set.

v2:
 - add BPF-related code in this set (32-36).

In order to implement BPF userspace path manager, it is 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 userspace path manager
like this:

struct mptcp_pm_ops {
        int (*address_announce)(struct mptcp_sock *msk,
                                struct mptcp_pm_addr_entry *local);
        int (*address_remove)(struct mptcp_sock *msk, u8 id);
        int (*subflow_create)(struct mptcp_sock *msk,
                              struct mptcp_pm_addr_entry *local,
                              struct mptcp_addr_info *remote);
        int (*subflow_destroy)(struct mptcp_sock *msk,
                               struct mptcp_pm_addr_entry *local,
                               struct mptcp_addr_info *remote);
        int (*get_local_id)(struct mptcp_sock *msk,
                            struct mptcp_pm_addr_entry *local);
        u8 (*get_flags)(struct mptcp_sock *msk,
                        struct mptcp_addr_info *skc);
        struct mptcp_pm_addr_entry *(*get_addr)(struct mptcp_sock *msk,
                                                u8 id);
	int (*dump_addr)(struct mptcp_sock *msk,
			 mptcp_pm_addr_id_bitmap_t *bitmap);
        int (*set_flags)(struct mptcp_sock *msk,
                         struct mptcp_pm_addr_entry *local,
                         struct mptcp_addr_info *remote);

        u8                      type;
        struct module           *owner;
        struct list_head        list;

        void (*init)(struct mptcp_sock *msk);
        void (*release)(struct mptcp_sock *msk);
} ____cacheline_aligned_in_smp;

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/74

Geliang Tang (9):
  mptcp: add mptcp_userspace_pm_lookup_addr helper
  mptcp: add mptcp_for_each_userspace_pm_addr macro
  mptcp: add mptcp_userspace_pm_get_sock helper
  mptcp: move mptcp_pm_remove_addrs into pm_userspace
  mptcp: drop free_list for deleting entries
  mptcp: use mptcp_pm_local in pm_netlink only
  mptcp: drop struct mptcp_pm_add_entry
  mptcp: change local addr type of subflow_destroy
  mptcp: drop useless "err = 0" in subflow_destroy

 net/mptcp/pm_netlink.c   |  97 +++++--------
 net/mptcp/pm_userspace.c | 306 +++++++++++++++++----------------------
 net/mptcp/protocol.h     |  35 +++--
 net/mptcp/subflow.c      |   2 +-
 4 files changed, 198 insertions(+), 242 deletions(-)

-- 
2.45.2

[PATCH mptcp-next v3 1/9] mptcp: add mptcp_userspace_pm_lookup_addr helper

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Like __lookup_addr() helper in pm_netlink.c, a new helper
mptcp_userspace_pm_lookup_addr() is also defined in pm_userspace.c.
It looks up the corresponding mptcp_pm_addr_entry address in
userspace_pm_local_addr_list through the passed "addr" parameter
and returns the found address entry.

This helper can be used in mptcp_userspace_pm_delete_local_addr(),
mptcp_userspace_pm_set_flags(), mptcp_userspace_pm_get_local_id()
and mptcp_userspace_pm_is_backup() to simplify the code.

Please note that with this change now list_for_each_entry() is used in
mptcp_userspace_pm_append_new_local_addr(), not list_for_each_entry_safe(),
but that's OK to do so because mptcp_userspace_pm_lookup_addr() only
returns an entry from the list, the list hasn't been modified here.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm_userspace.c | 71 ++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 35 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index e35178f5205f..3664f3c1572e 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -26,6 +26,19 @@ void mptcp_free_local_addr_list(struct mptcp_sock *msk)
 	}
 }
 
+static struct mptcp_pm_addr_entry *
+mptcp_userspace_pm_lookup_addr(struct mptcp_sock *msk,
+			       const struct mptcp_addr_info *addr)
+{
+	struct mptcp_pm_addr_entry *entry;
+
+	list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) {
+		if (mptcp_addresses_equal(&entry->addr, addr, false))
+			return entry;
+	}
+	return NULL;
+}
+
 static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
 						    struct mptcp_pm_addr_entry *entry,
 						    bool needs_id)
@@ -90,22 +103,20 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
 static int mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk,
 						struct mptcp_pm_addr_entry *addr)
 {
-	struct mptcp_pm_addr_entry *entry, *tmp;
 	struct sock *sk = (struct sock *)msk;
+	struct mptcp_pm_addr_entry *entry;
 
-	list_for_each_entry_safe(entry, tmp, &msk->pm.userspace_pm_local_addr_list, list) {
-		if (mptcp_addresses_equal(&entry->addr, &addr->addr, false)) {
-			/* TODO: a refcount is needed because the entry can
-			 * be used multiple times (e.g. fullmesh mode).
-			 */
-			list_del_rcu(&entry->list);
-			sock_kfree_s(sk, entry, sizeof(*entry));
-			msk->pm.local_addr_used--;
-			return 0;
-		}
-	}
-
-	return -EINVAL;
+	entry = mptcp_userspace_pm_lookup_addr(msk, &addr->addr);
+	if (!entry)
+		return -EINVAL;
+
+	/* TODO: a refcount is needed because the entry can
+	 * be used multiple times (e.g. fullmesh mode).
+	 */
+	list_del_rcu(&entry->list);
+	sock_kfree_s(sk, entry, sizeof(*entry));
+	msk->pm.local_addr_used--;
+	return 0;
 }
 
 static struct mptcp_pm_addr_entry *
@@ -123,17 +134,12 @@ mptcp_userspace_pm_lookup_addr_by_id(struct mptcp_sock *msk, unsigned int id)
 int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
 				    struct mptcp_addr_info *skc)
 {
-	struct mptcp_pm_addr_entry *entry = NULL, *e, new_entry;
+	struct mptcp_pm_addr_entry *entry = NULL, new_entry;
 	__be16 msk_sport =  ((struct inet_sock *)
 			     inet_sk((struct sock *)msk))->inet_sport;
 
 	spin_lock_bh(&msk->pm.lock);
-	list_for_each_entry(e, &msk->pm.userspace_pm_local_addr_list, list) {
-		if (mptcp_addresses_equal(&e->addr, skc, false)) {
-			entry = e;
-			break;
-		}
-	}
+	entry = mptcp_userspace_pm_lookup_addr(msk, skc);
 	spin_unlock_bh(&msk->pm.lock);
 	if (entry)
 		return entry->addr.id;
@@ -153,15 +159,11 @@ bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk,
 				  struct mptcp_addr_info *skc)
 {
 	struct mptcp_pm_addr_entry *entry;
-	bool backup = false;
+	bool backup;
 
 	spin_lock_bh(&msk->pm.lock);
-	list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) {
-		if (mptcp_addresses_equal(&entry->addr, skc, false)) {
-			backup = !!(entry->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
-			break;
-		}
-	}
+	entry = mptcp_userspace_pm_lookup_addr(msk, skc);
+	backup = entry && !!(entry->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
 	spin_unlock_bh(&msk->pm.lock);
 
 	return backup;
@@ -606,13 +608,12 @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 		bkup = 1;
 
 	spin_lock_bh(&msk->pm.lock);
-	list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) {
-		if (mptcp_addresses_equal(&entry->addr, &loc.addr, false)) {
-			if (bkup)
-				entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
-			else
-				entry->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP;
-		}
+	entry = mptcp_userspace_pm_lookup_addr(msk, &loc.addr);
+	if (entry) {
+		if (bkup)
+			entry->flags |= MPTCP_PM_ADDR_FLAG_BACKUP;
+		else
+			entry->flags &= ~MPTCP_PM_ADDR_FLAG_BACKUP;
 	}
 	spin_unlock_bh(&msk->pm.lock);
 
-- 
2.45.2

[PATCH mptcp-next v3 2/9] mptcp: add mptcp_for_each_userspace_pm_addr macro

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Similar to mptcp_for_each_subflow() macro, this patch adds a new macro
mptcp_for_each_userspace_pm_addr() for userspace PM to iterate over the
address entries on the local address list userspace_pm_local_addr_list
of the mptcp socket.

This patch doesn't change the behaviour of the code, just refactoring.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm_userspace.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 3664f3c1572e..c99ec28c1bd5 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -8,6 +8,9 @@
 #include "mib.h"
 #include "mptcp_pm_gen.h"
 
+#define mptcp_for_each_userspace_pm_addr(__msk, __entry)	\
+	list_for_each_entry(__entry, &((__msk)->pm.userspace_pm_local_addr_list), list)
+
 void mptcp_free_local_addr_list(struct mptcp_sock *msk)
 {
 	struct mptcp_pm_addr_entry *entry, *tmp;
@@ -32,7 +35,7 @@ mptcp_userspace_pm_lookup_addr(struct mptcp_sock *msk,
 {
 	struct mptcp_pm_addr_entry *entry;
 
-	list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) {
+	mptcp_for_each_userspace_pm_addr(msk, entry) {
 		if (mptcp_addresses_equal(&entry->addr, addr, false))
 			return entry;
 	}
@@ -54,7 +57,7 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
 	bitmap_zero(id_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
 
 	spin_lock_bh(&msk->pm.lock);
-	list_for_each_entry(e, &msk->pm.userspace_pm_local_addr_list, list) {
+	mptcp_for_each_userspace_pm_addr(msk, e) {
 		addr_match = mptcp_addresses_equal(&e->addr, &entry->addr, true);
 		if (addr_match && entry->addr.id == 0 && needs_id)
 			entry->addr.id = e->addr.id;
@@ -124,7 +127,7 @@ mptcp_userspace_pm_lookup_addr_by_id(struct mptcp_sock *msk, unsigned int id)
 {
 	struct mptcp_pm_addr_entry *entry;
 
-	list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) {
+	mptcp_for_each_userspace_pm_addr(msk, entry) {
 		if (entry->addr.id == id)
 			return entry;
 	}
@@ -659,7 +662,7 @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
 
 	lock_sock(sk);
 	spin_lock_bh(&msk->pm.lock);
-	list_for_each_entry(entry, &msk->pm.userspace_pm_local_addr_list, list) {
+	mptcp_for_each_userspace_pm_addr(msk, entry) {
 		if (test_bit(entry->addr.id, bitmap->map))
 			continue;
 
-- 
2.45.2

[PATCH mptcp-next v3 3/9] mptcp: add mptcp_userspace_pm_get_sock helper

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Each userspace pm netlink function uses nla_get_u32() to get the msk
token value, then pass it to mptcp_token_get_sock() to get the msk.
Finally check whether userspace PM is selected on this msk. It makes
sense to wrap them into a helper, named mptcp_userspace_pm_get_sock(),
to do this.

This patch doesn't change the behaviour of the code, just refactoring.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm_userspace.c | 144 +++++++++++++--------------------------
 1 file changed, 47 insertions(+), 97 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index c99ec28c1bd5..a6de837d8958 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -172,36 +172,50 @@ bool mptcp_userspace_pm_is_backup(struct mptcp_sock *msk,
 	return backup;
 }
 
-int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
+static struct mptcp_sock *mptcp_userspace_pm_get_sock(const struct genl_info *info)
 {
 	struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
+	struct mptcp_sock *msk;
+
+	if (!token) {
+		GENL_SET_ERR_MSG(info, "missing required token");
+		return NULL;
+	}
+
+	msk = mptcp_token_get_sock(genl_info_net(info), nla_get_u32(token));
+	if (!msk) {
+		NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
+		return NULL;
+	}
+
+	if (!mptcp_pm_is_userspace(msk)) {
+		GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");
+		sock_put((struct sock *)msk);
+		return NULL;
+	}
+
+	return msk;
+}
+
+int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
+{
 	struct nlattr *addr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	struct mptcp_pm_addr_entry addr_val;
 	struct mptcp_sock *msk;
 	int err = -EINVAL;
 	struct sock *sk;
-	u32 token_val;
 
-	if (!addr || !token) {
-		GENL_SET_ERR_MSG(info, "missing required inputs");
+	if (!addr) {
+		GENL_SET_ERR_MSG(info, "missing required address");
 		return err;
 	}
 
-	token_val = nla_get_u32(token);
-
-	msk = mptcp_token_get_sock(sock_net(skb->sk), token_val);
-	if (!msk) {
-		NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
+	msk = mptcp_userspace_pm_get_sock(info);
+	if (!msk)
 		return err;
-	}
 
 	sk = (struct sock *)msk;
 
-	if (!mptcp_pm_is_userspace(msk)) {
-		GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");
-		goto announce_err;
-	}
-
 	err = mptcp_pm_parse_entry(addr, info, true, &addr_val);
 	if (err < 0) {
 		GENL_SET_ERR_MSG(info, "error parsing local address");
@@ -274,7 +288,6 @@ static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
 
 int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 {
-	struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
 	struct nlattr *id = info->attrs[MPTCP_PM_ATTR_LOC_ID];
 	struct mptcp_pm_addr_entry *match;
 	struct mptcp_pm_addr_entry *entry;
@@ -282,30 +295,21 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 	LIST_HEAD(free_list);
 	int err = -EINVAL;
 	struct sock *sk;
-	u32 token_val;
 	u8 id_val;
 
-	if (!id || !token) {
-		GENL_SET_ERR_MSG(info, "missing required inputs");
+	if (!id) {
+		GENL_SET_ERR_MSG(info, "missing required ID");
 		return err;
 	}
 
 	id_val = nla_get_u8(id);
-	token_val = nla_get_u32(token);
 
-	msk = mptcp_token_get_sock(sock_net(skb->sk), token_val);
-	if (!msk) {
-		NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
+	msk = mptcp_userspace_pm_get_sock(info);
+	if (!msk)
 		return err;
-	}
 
 	sk = (struct sock *)msk;
 
-	if (!mptcp_pm_is_userspace(msk)) {
-		GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");
-		goto out;
-	}
-
 	if (id_val == 0) {
 		err = mptcp_userspace_pm_remove_id_zero_address(msk, info);
 		goto out;
@@ -342,7 +346,6 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct nlattr *raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
-	struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
 	struct nlattr *laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	struct mptcp_pm_addr_entry entry = { 0 };
 	struct mptcp_addr_info addr_r;
@@ -350,28 +353,18 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 	struct mptcp_sock *msk;
 	int err = -EINVAL;
 	struct sock *sk;
-	u32 token_val;
 
-	if (!laddr || !raddr || !token) {
-		GENL_SET_ERR_MSG(info, "missing required inputs");
+	if (!laddr || !raddr) {
+		GENL_SET_ERR_MSG(info, "missing required address(es)");
 		return err;
 	}
 
-	token_val = nla_get_u32(token);
-
-	msk = mptcp_token_get_sock(genl_info_net(info), token_val);
-	if (!msk) {
-		NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
+	msk = mptcp_userspace_pm_get_sock(info);
+	if (!msk)
 		return err;
-	}
 
 	sk = (struct sock *)msk;
 
-	if (!mptcp_pm_is_userspace(msk)) {
-		GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");
-		goto create_err;
-	}
-
 	err = mptcp_pm_parse_entry(laddr, info, true, &entry);
 	if (err < 0) {
 		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
@@ -474,35 +467,24 @@ static struct sock *mptcp_nl_find_ssk(struct mptcp_sock *msk,
 int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct nlattr *raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
-	struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
 	struct nlattr *laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	struct mptcp_addr_info addr_l;
 	struct mptcp_addr_info addr_r;
 	struct mptcp_sock *msk;
 	struct sock *sk, *ssk;
 	int err = -EINVAL;
-	u32 token_val;
 
-	if (!laddr || !raddr || !token) {
-		GENL_SET_ERR_MSG(info, "missing required inputs");
+	if (!laddr || !raddr) {
+		GENL_SET_ERR_MSG(info, "missing required address(es)");
 		return err;
 	}
 
-	token_val = nla_get_u32(token);
-
-	msk = mptcp_token_get_sock(genl_info_net(info), token_val);
-	if (!msk) {
-		NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
+	msk = mptcp_userspace_pm_get_sock(info);
+	if (!msk)
 		return err;
-	}
 
 	sk = (struct sock *)msk;
 
-	if (!mptcp_pm_is_userspace(msk)) {
-		GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");
-		goto destroy_err;
-	}
-
 	err = mptcp_pm_parse_addr(laddr, info, &addr_l);
 	if (err < 0) {
 		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
@@ -565,31 +547,19 @@ int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info)
 	struct mptcp_pm_addr_entry loc = { .addr = { .family = AF_UNSPEC }, };
 	struct mptcp_pm_addr_entry rem = { .addr = { .family = AF_UNSPEC }, };
 	struct nlattr *attr_rem = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
-	struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
 	struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
-	struct net *net = sock_net(skb->sk);
 	struct mptcp_pm_addr_entry *entry;
 	struct mptcp_sock *msk;
 	int ret = -EINVAL;
 	struct sock *sk;
-	u32 token_val;
 	u8 bkup = 0;
 
-	token_val = nla_get_u32(token);
-
-	msk = mptcp_token_get_sock(net, token_val);
-	if (!msk) {
-		NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
+	msk = mptcp_userspace_pm_get_sock(info);
+	if (!msk)
 		return ret;
-	}
 
 	sk = (struct sock *)msk;
 
-	if (!mptcp_pm_is_userspace(msk)) {
-		GENL_SET_ERR_MSG(info, "userspace PM not selected");
-		goto set_flags_err;
-	}
-
 	ret = mptcp_pm_parse_entry(attr, info, false, &loc);
 	if (ret < 0)
 		goto set_flags_err;
@@ -636,30 +606,20 @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
 		DECLARE_BITMAP(map, MPTCP_PM_MAX_ADDR_ID + 1);
 	} *bitmap;
 	const struct genl_info *info = genl_info_dump(cb);
-	struct net *net = sock_net(msg->sk);
 	struct mptcp_pm_addr_entry *entry;
 	struct mptcp_sock *msk;
-	struct nlattr *token;
 	int ret = -EINVAL;
 	struct sock *sk;
 	void *hdr;
 
 	bitmap = (struct id_bitmap *)cb->ctx;
-	token = info->attrs[MPTCP_PM_ATTR_TOKEN];
 
-	msk = mptcp_token_get_sock(net, nla_get_u32(token));
-	if (!msk) {
-		NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
+	msk = mptcp_userspace_pm_get_sock(info);
+	if (!msk)
 		return ret;
-	}
 
 	sk = (struct sock *)msk;
 
-	if (!mptcp_pm_is_userspace(msk)) {
-		GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");
-		goto out;
-	}
-
 	lock_sock(sk);
 	spin_lock_bh(&msk->pm.lock);
 	mptcp_for_each_userspace_pm_addr(msk, entry) {
@@ -684,7 +644,6 @@ int mptcp_userspace_pm_dump_addr(struct sk_buff *msg,
 	release_sock(sk);
 	ret = msg->len;
 
-out:
 	sock_put(sk);
 	return ret;
 }
@@ -693,28 +652,19 @@ int mptcp_userspace_pm_get_addr(struct sk_buff *skb,
 				struct genl_info *info)
 {
 	struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
-	struct nlattr *token = info->attrs[MPTCP_PM_ATTR_TOKEN];
 	struct mptcp_pm_addr_entry addr, *entry;
-	struct net *net = sock_net(skb->sk);
 	struct mptcp_sock *msk;
 	struct sk_buff *msg;
 	int ret = -EINVAL;
 	struct sock *sk;
 	void *reply;
 
-	msk = mptcp_token_get_sock(net, nla_get_u32(token));
-	if (!msk) {
-		NL_SET_ERR_MSG_ATTR(info->extack, token, "invalid token");
+	msk = mptcp_userspace_pm_get_sock(info);
+	if (!msk)
 		return ret;
-	}
 
 	sk = (struct sock *)msk;
 
-	if (!mptcp_pm_is_userspace(msk)) {
-		GENL_SET_ERR_MSG(info, "invalid request; userspace PM not selected");
-		goto out;
-	}
-
 	ret = mptcp_pm_parse_entry(attr, info, false, &addr);
 	if (ret < 0)
 		goto out;
-- 
2.45.2

[PATCH mptcp-next v3 4/9] mptcp: move mptcp_pm_remove_addrs into pm_userspace

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Since mptcp_pm_remove_addrs is only called from the userspace PM, this
patch moves it into pm_userspace.c.

For this, lookup_subflow_by_saddr() and remove_anno_list_by_saddr()
helpers need to be exported in protocol.h. Also add "mptcp_" prefix for
these helpers.

This patch doesn't change the behaviour of the code, just refactoring.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm_netlink.c   | 45 +++++++---------------------------------
 net/mptcp/pm_userspace.c | 28 +++++++++++++++++++++++++
 net/mptcp/protocol.h     |  4 ++++
 3 files changed, 40 insertions(+), 37 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 618289aac0ab..8aba7670345d 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -107,8 +107,8 @@ static void remote_address(const struct sock_common *skc,
 #endif
 }
 
-static bool lookup_subflow_by_saddr(const struct list_head *list,
-				    const struct mptcp_addr_info *saddr)
+bool mptcp_lookup_subflow_by_saddr(const struct list_head *list,
+				   const struct mptcp_addr_info *saddr)
 {
 	struct mptcp_subflow_context *subflow;
 	struct mptcp_addr_info cur;
@@ -1453,8 +1453,8 @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
 	return ret;
 }
 
-static bool remove_anno_list_by_saddr(struct mptcp_sock *msk,
-				      const struct mptcp_addr_info *addr)
+bool mptcp_remove_anno_list_by_saddr(struct mptcp_sock *msk,
+				     const struct mptcp_addr_info *addr)
 {
 	struct mptcp_pm_add_entry *entry;
 
@@ -1482,7 +1482,7 @@ static bool mptcp_pm_remove_anno_addr(struct mptcp_sock *msk,
 
 	list.ids[list.nr++] = mptcp_endp_get_local_id(msk, addr);
 
-	ret = remove_anno_list_by_saddr(msk, addr);
+	ret = mptcp_remove_anno_list_by_saddr(msk, addr);
 	if (ret || force) {
 		spin_lock_bh(&msk->pm.lock);
 		if (ret) {
@@ -1526,7 +1526,7 @@ static int mptcp_nl_remove_subflow_and_signal_addr(struct net *net,
 		}
 
 		lock_sock(sk);
-		remove_subflow = lookup_subflow_by_saddr(&msk->conn_list, addr);
+		remove_subflow = mptcp_lookup_subflow_by_saddr(&msk->conn_list, addr);
 		mptcp_pm_remove_anno_addr(msk, addr, remove_subflow &&
 					  !(entry->flags & MPTCP_PM_ADDR_FLAG_IMPLICIT));
 
@@ -1639,35 +1639,6 @@ int mptcp_pm_nl_del_addr_doit(struct sk_buff *skb, struct genl_info *info)
 	return ret;
 }
 
-/* Called from the userspace PM only */
-void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct list_head *rm_list)
-{
-	struct mptcp_rm_list alist = { .nr = 0 };
-	struct mptcp_pm_addr_entry *entry;
-	int anno_nr = 0;
-
-	list_for_each_entry(entry, rm_list, list) {
-		if (alist.nr >= MPTCP_RM_IDS_MAX)
-			break;
-
-		/* only delete if either announced or matching a subflow */
-		if (remove_anno_list_by_saddr(msk, &entry->addr))
-			anno_nr++;
-		else if (!lookup_subflow_by_saddr(&msk->conn_list,
-						  &entry->addr))
-			continue;
-
-		alist.ids[alist.nr++] = entry->addr.id;
-	}
-
-	if (alist.nr) {
-		spin_lock_bh(&msk->pm.lock);
-		msk->pm.add_addr_signaled -= anno_nr;
-		mptcp_pm_remove_addr(msk, &alist);
-		spin_unlock_bh(&msk->pm.lock);
-	}
-}
-
 /* Called from the in-kernel PM only */
 static void mptcp_pm_flush_addrs_and_subflows(struct mptcp_sock *msk,
 					      struct list_head *rm_list)
@@ -1677,11 +1648,11 @@ static void mptcp_pm_flush_addrs_and_subflows(struct mptcp_sock *msk,
 
 	list_for_each_entry(entry, rm_list, list) {
 		if (slist.nr < MPTCP_RM_IDS_MAX &&
-		    lookup_subflow_by_saddr(&msk->conn_list, &entry->addr))
+		    mptcp_lookup_subflow_by_saddr(&msk->conn_list, &entry->addr))
 			slist.ids[slist.nr++] = mptcp_endp_get_local_id(msk, &entry->addr);
 
 		if (alist.nr < MPTCP_RM_IDS_MAX &&
-		    remove_anno_list_by_saddr(msk, &entry->addr))
+		    mptcp_remove_anno_list_by_saddr(msk, &entry->addr))
 			alist.ids[alist.nr++] = mptcp_endp_get_local_id(msk, &entry->addr);
 	}
 
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index a6de837d8958..737a07f5defe 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -286,6 +286,34 @@ static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
 	return err;
 }
 
+void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct list_head *rm_list)
+{
+	struct mptcp_rm_list alist = { .nr = 0 };
+	struct mptcp_pm_addr_entry *entry;
+	int anno_nr = 0;
+
+	list_for_each_entry(entry, rm_list, list) {
+		if (alist.nr >= MPTCP_RM_IDS_MAX)
+			break;
+
+		/* only delete if either announced or matching a subflow */
+		if (mptcp_remove_anno_list_by_saddr(msk, &entry->addr))
+			anno_nr++;
+		else if (!mptcp_lookup_subflow_by_saddr(&msk->conn_list,
+							&entry->addr))
+			continue;
+
+		alist.ids[alist.nr++] = entry->addr.id;
+	}
+
+	if (alist.nr) {
+		spin_lock_bh(&msk->pm.lock);
+		msk->pm.add_addr_signaled -= anno_nr;
+		mptcp_pm_remove_addr(msk, &alist);
+		spin_unlock_bh(&msk->pm.lock);
+	}
+}
+
 int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct nlattr *id = info->attrs[MPTCP_PM_ATTR_LOC_ID];
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index b4c72a73594f..80d355c1dfb4 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1031,6 +1031,10 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk,
 struct mptcp_pm_add_entry *
 mptcp_lookup_anno_list_by_saddr(const struct mptcp_sock *msk,
 				const struct mptcp_addr_info *addr);
+bool mptcp_lookup_subflow_by_saddr(const struct list_head *list,
+				   const struct mptcp_addr_info *saddr);
+bool mptcp_remove_anno_list_by_saddr(struct mptcp_sock *msk,
+				     const struct mptcp_addr_info *addr);
 int mptcp_pm_set_flags(struct sk_buff *skb, struct genl_info *info);
 int mptcp_pm_nl_set_flags(struct sk_buff *skb, struct genl_info *info);
 int mptcp_userspace_pm_set_flags(struct sk_buff *skb, struct genl_info *info);
-- 
2.45.2

Re: [PATCH mptcp-next v3 4/9] mptcp: move mptcp_pm_remove_addrs into pm_userspace

[Matthieu Baerts]

Hi Geliang,

On 07/11/2024 07:45, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Since mptcp_pm_remove_addrs is only called from the userspace PM, this
> patch moves it into pm_userspace.c.
> 
> For this, lookup_subflow_by_saddr() and remove_anno_list_by_saddr()
> helpers need to be exported in protocol.h. Also add "mptcp_" prefix for
> these helpers.
> 
> This patch doesn't change the behaviour of the code, just refactoring.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm_netlink.c   | 45 +++++++---------------------------------
>  net/mptcp/pm_userspace.c | 28 +++++++++++++++++++++++++
>  net/mptcp/protocol.h     |  4 ++++
>  3 files changed, 40 insertions(+), 37 deletions(-)
> 
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 618289aac0ab..8aba7670345d 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c

(...)

> @@ -1639,35 +1639,6 @@ int mptcp_pm_nl_del_addr_doit(struct sk_buff *skb, struct genl_info *info)
>  	return ret;
>  }
>  
> -/* Called from the userspace PM only */
> -void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct list_head *rm_list)
> -{
> -	struct mptcp_rm_list alist = { .nr = 0 };
> -	struct mptcp_pm_addr_entry *entry;
> -	int anno_nr = 0;
> -
> -	list_for_each_entry(entry, rm_list, list) {
> -		if (alist.nr >= MPTCP_RM_IDS_MAX)
> -			break;
> -
> -		/* only delete if either announced or matching a subflow */
> -		if (remove_anno_list_by_saddr(msk, &entry->addr))
> -			anno_nr++;
> -		else if (!lookup_subflow_by_saddr(&msk->conn_list,
> -						  &entry->addr))
> -			continue;
> -
> -		alist.ids[alist.nr++] = entry->addr.id;
> -	}
> -
> -	if (alist.nr) {
> -		spin_lock_bh(&msk->pm.lock);
> -		msk->pm.add_addr_signaled -= anno_nr;
> -		mptcp_pm_remove_addr(msk, &alist);
> -		spin_unlock_bh(&msk->pm.lock);
> -	}
> -}
> -
>  /* Called from the in-kernel PM only */

I guess we can remove this command as well.

(Something I can change when applying the patch if there is nothing else.)

>  static void mptcp_pm_flush_addrs_and_subflows(struct mptcp_sock *msk,
>  					      struct list_head *rm_list)
> @@ -1677,11 +1648,11 @@ static void mptcp_pm_flush_addrs_and_subflows(struct mptcp_sock *msk,
>  
>  	list_for_each_entry(entry, rm_list, list) {
>  		if (slist.nr < MPTCP_RM_IDS_MAX &&
> -		    lookup_subflow_by_saddr(&msk->conn_list, &entry->addr))
> +		    mptcp_lookup_subflow_by_saddr(&msk->conn_list, &entry->addr))
>  			slist.ids[slist.nr++] = mptcp_endp_get_local_id(msk, &entry->addr);
>  
>  		if (alist.nr < MPTCP_RM_IDS_MAX &&
> -		    remove_anno_list_by_saddr(msk, &entry->addr))
> +		    mptcp_remove_anno_list_by_saddr(msk, &entry->addr))
>  			alist.ids[alist.nr++] = mptcp_endp_get_local_id(msk, &entry->addr);
>  	}
>  
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index a6de837d8958..737a07f5defe 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -286,6 +286,34 @@ static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
>  	return err;
>  }
>  
> +void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct list_head *rm_list)

Can it not be 'static' now that it is only used in the userspace pm?

(...)

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

Re: [PATCH mptcp-next v3 4/9] mptcp: move mptcp_pm_remove_addrs into pm_userspace

[Geliang Tang]

Hi Matt,

Thanks for the review.

On Wed, 2024-12-04 at 18:48 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 07/11/2024 07:45, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > Since mptcp_pm_remove_addrs is only called from the userspace PM,
> > this
> > patch moves it into pm_userspace.c.
> > 
> > For this, lookup_subflow_by_saddr() and remove_anno_list_by_saddr()
> > helpers need to be exported in protocol.h. Also add "mptcp_" prefix
> > for
> > these helpers.
> > 
> > This patch doesn't change the behaviour of the code, just
> > refactoring.
> > 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> >  net/mptcp/pm_netlink.c   | 45 +++++++-----------------------------
> > ----
> >  net/mptcp/pm_userspace.c | 28 +++++++++++++++++++++++++
> >  net/mptcp/protocol.h     |  4 ++++
> >  3 files changed, 40 insertions(+), 37 deletions(-)
> > 
> > diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> > index 618289aac0ab..8aba7670345d 100644
> > --- a/net/mptcp/pm_netlink.c
> > +++ b/net/mptcp/pm_netlink.c
> 
> (...)
> 
> > @@ -1639,35 +1639,6 @@ int mptcp_pm_nl_del_addr_doit(struct sk_buff
> > *skb, struct genl_info *info)
> >  	return ret;
> >  }
> >  
> > -/* Called from the userspace PM only */
> > -void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct
> > list_head *rm_list)
> > -{
> > -	struct mptcp_rm_list alist = { .nr = 0 };
> > -	struct mptcp_pm_addr_entry *entry;
> > -	int anno_nr = 0;
> > -
> > -	list_for_each_entry(entry, rm_list, list) {
> > -		if (alist.nr >= MPTCP_RM_IDS_MAX)
> > -			break;
> > -
> > -		/* only delete if either announced or matching a
> > subflow */
> > -		if (remove_anno_list_by_saddr(msk, &entry->addr))
> > -			anno_nr++;
> > -		else if (!lookup_subflow_by_saddr(&msk->conn_list,
> > -						  &entry->addr))
> > -			continue;
> > -
> > -		alist.ids[alist.nr++] = entry->addr.id;
> > -	}
> > -
> > -	if (alist.nr) {
> > -		spin_lock_bh(&msk->pm.lock);
> > -		msk->pm.add_addr_signaled -= anno_nr;
> > -		mptcp_pm_remove_addr(msk, &alist);
> > -		spin_unlock_bh(&msk->pm.lock);
> > -	}
> > -}
> > -
> >  /* Called from the in-kernel PM only */
> 
> I guess we can remove this command as well.

Dropped this in v4.

> 
> (Something I can change when applying the patch if there is nothing
> else.)
> 
> >  static void mptcp_pm_flush_addrs_and_subflows(struct mptcp_sock
> > *msk,
> >  					      struct list_head
> > *rm_list)
> > @@ -1677,11 +1648,11 @@ static void
> > mptcp_pm_flush_addrs_and_subflows(struct mptcp_sock *msk,
> >  
> >  	list_for_each_entry(entry, rm_list, list) {
> >  		if (slist.nr < MPTCP_RM_IDS_MAX &&
> > -		    lookup_subflow_by_saddr(&msk->conn_list,
> > &entry->addr))
> > +		    mptcp_lookup_subflow_by_saddr(&msk->conn_list,
> > &entry->addr))
> >  			slist.ids[slist.nr++] =
> > mptcp_endp_get_local_id(msk, &entry->addr);
> >  
> >  		if (alist.nr < MPTCP_RM_IDS_MAX &&
> > -		    remove_anno_list_by_saddr(msk, &entry->addr))
> > +		    mptcp_remove_anno_list_by_saddr(msk, &entry-
> > >addr))
> >  			alist.ids[alist.nr++] =
> > mptcp_endp_get_local_id(msk, &entry->addr);
> >  	}
> >  
> > diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> > index a6de837d8958..737a07f5defe 100644
> > --- a/net/mptcp/pm_userspace.c
> > +++ b/net/mptcp/pm_userspace.c
> > @@ -286,6 +286,34 @@ static int
> > mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
> >  	return err;
> >  }
> >  
> > +void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct
> > list_head *rm_list)
> 
> Can it not be 'static' now that it is only used in the userspace pm?

It will be invoked in BPF path manager, 'static' doesn't work.

> 
> (...)
> 
> Cheers,
> Matt

Re: [PATCH mptcp-next v3 4/9] mptcp: move mptcp_pm_remove_addrs into pm_userspace

[Matthieu Baerts]

Hi Geliang,

On 05/12/2024 08:26, Geliang Tang wrote:
> On Wed, 2024-12-04 at 18:48 +0100, Matthieu Baerts wrote:
>> On 07/11/2024 07:45, Geliang Tang wrote:

(...)

>>> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
>>> index a6de837d8958..737a07f5defe 100644
>>> --- a/net/mptcp/pm_userspace.c
>>> +++ b/net/mptcp/pm_userspace.c
>>> @@ -286,6 +286,34 @@ static int
>>> mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
>>>  	return err;
>>>  }
>>>  
>>> +void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct
>>> list_head *rm_list)
>>
>> Can it not be 'static' now that it is only used in the userspace pm?
> 
> It will be invoked in BPF path manager, 'static' doesn't work.

OK. Hopefully people will not send a patch in between to add 'static'.

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

Re: [PATCH mptcp-next v3 4/9] mptcp: move mptcp_pm_remove_addrs into pm_userspace

[Geliang Tang]

On Thu, 2024-12-05 at 10:27 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 05/12/2024 08:26, Geliang Tang wrote:
> > On Wed, 2024-12-04 at 18:48 +0100, Matthieu Baerts wrote:
> > > On 07/11/2024 07:45, Geliang Tang wrote:
> 
> (...)
> 
> > > > diff --git a/net/mptcp/pm_userspace.c
> > > > b/net/mptcp/pm_userspace.c
> > > > index a6de837d8958..737a07f5defe 100644
> > > > --- a/net/mptcp/pm_userspace.c
> > > > +++ b/net/mptcp/pm_userspace.c
> > > > @@ -286,6 +286,34 @@ static int
> > > > mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock
> > > > *msk,
> > > >  	return err;
> > > >  }
> > > >  
> > > > +void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct
> > > > list_head *rm_list)
> > > 
> > > Can it not be 'static' now that it is only used in the userspace
> > > pm?
> > 
> > It will be invoked in BPF path manager, 'static' doesn't work.
> 
> OK. Hopefully people will not send a patch in between to add
> 'static'.

I've mentioned this in the commit log of v4:

'''
Here, mptcp_pm_remove_addrs() is not changed to a static function
because it will be used in BPF Path Manager.
''' 
Thanks,
-Geliang

> 
> Cheers,
> Matt

Re: [PATCH mptcp-next v3 4/9] mptcp: move mptcp_pm_remove_addrs into pm_userspace

[Matthieu Baerts]

On 05/12/2024 10:36, Geliang Tang wrote:
> On Thu, 2024-12-05 at 10:27 +0100, Matthieu Baerts wrote:
>> Hi Geliang,
>>
>> On 05/12/2024 08:26, Geliang Tang wrote:
>>> On Wed, 2024-12-04 at 18:48 +0100, Matthieu Baerts wrote:
>>>> On 07/11/2024 07:45, Geliang Tang wrote:
>>
>> (...)
>>
>>>>> diff --git a/net/mptcp/pm_userspace.c
>>>>> b/net/mptcp/pm_userspace.c
>>>>> index a6de837d8958..737a07f5defe 100644
>>>>> --- a/net/mptcp/pm_userspace.c
>>>>> +++ b/net/mptcp/pm_userspace.c
>>>>> @@ -286,6 +286,34 @@ static int
>>>>> mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock
>>>>> *msk,
>>>>>  	return err;
>>>>>  }
>>>>>  
>>>>> +void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct
>>>>> list_head *rm_list)
>>>>
>>>> Can it not be 'static' now that it is only used in the userspace
>>>> pm?
>>>
>>> It will be invoked in BPF path manager, 'static' doesn't work.
>>
>> OK. Hopefully people will not send a patch in between to add
>> 'static'.
> 
> I've mentioned this in the commit log of v4:
> 
> '''
> Here, mptcp_pm_remove_addrs() is not changed to a static function
> because it will be used in BPF Path Manager.
> '''

Good, thank you (I saw that after I sent my previous email).

If someone sends a patch, we can refer to this commit then.

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

[PATCH mptcp-next v3 5/9] mptcp: drop free_list for deleting entries

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

mptcp_pm_remove_addrs() actually only deletes one address, which does
not match its name. This patch renames it to mptcp_pm_remove_addr_entry()
and changes the parameter "rm_list" to "entry".

With the help of mptcp_pm_remove_addr_entry(), it's no longer necessary to
move the entry to be deleted to free_list and then traverse the list to
delete the entry, which is not allowed in BPF. The entry can be directly
deleted through list_del_rcu() and sock_kfree_s() now.

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

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 737a07f5defe..a98da9a44bfa 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -286,26 +286,21 @@ static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
 	return err;
 }
 
-void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct list_head *rm_list)
+void mptcp_pm_remove_addr_entry(struct mptcp_sock *msk,
+				struct mptcp_pm_addr_entry *entry)
 {
 	struct mptcp_rm_list alist = { .nr = 0 };
-	struct mptcp_pm_addr_entry *entry;
 	int anno_nr = 0;
 
-	list_for_each_entry(entry, rm_list, list) {
-		if (alist.nr >= MPTCP_RM_IDS_MAX)
-			break;
-
-		/* only delete if either announced or matching a subflow */
-		if (mptcp_remove_anno_list_by_saddr(msk, &entry->addr))
-			anno_nr++;
-		else if (!mptcp_lookup_subflow_by_saddr(&msk->conn_list,
-							&entry->addr))
-			continue;
+	/* only delete if either announced or matching a subflow */
+	if (mptcp_remove_anno_list_by_saddr(msk, &entry->addr))
+		anno_nr++;
+	else if (!mptcp_lookup_subflow_by_saddr(&msk->conn_list, &entry->addr))
+		goto out;
 
-		alist.ids[alist.nr++] = entry->addr.id;
-	}
+	alist.ids[alist.nr++] = entry->addr.id;
 
+out:
 	if (alist.nr) {
 		spin_lock_bh(&msk->pm.lock);
 		msk->pm.add_addr_signaled -= anno_nr;
@@ -318,9 +313,7 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct nlattr *id = info->attrs[MPTCP_PM_ATTR_LOC_ID];
 	struct mptcp_pm_addr_entry *match;
-	struct mptcp_pm_addr_entry *entry;
 	struct mptcp_sock *msk;
-	LIST_HEAD(free_list);
 	int err = -EINVAL;
 	struct sock *sk;
 	u8 id_val;
@@ -354,16 +347,14 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 		goto out;
 	}
 
-	list_move(&match->list, &free_list);
+	list_del_rcu(&match->list);
 	spin_unlock_bh(&msk->pm.lock);
 
-	mptcp_pm_remove_addrs(msk, &free_list);
+	mptcp_pm_remove_addr_entry(msk, match);
 
 	release_sock(sk);
 
-	list_for_each_entry_safe(match, entry, &free_list, list) {
-		sock_kfree_s(sk, match, sizeof(*match));
-	}
+	sock_kfree_s(sk, match, sizeof(*match));
 
 	err = 0;
 out:
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 80d355c1dfb4..19a811220621 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1042,7 +1042,8 @@ int mptcp_pm_announce_addr(struct mptcp_sock *msk,
 			   const struct mptcp_addr_info *addr,
 			   bool echo);
 int mptcp_pm_remove_addr(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_list);
-void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct list_head *rm_list);
+void mptcp_pm_remove_addr_entry(struct mptcp_sock *msk,
+				struct mptcp_pm_addr_entry *entry);
 
 void mptcp_free_local_addr_list(struct mptcp_sock *msk);
 
-- 
2.45.2

Re: [PATCH mptcp-next v3 5/9] mptcp: drop free_list for deleting entries

[Matthieu Baerts]

Hi Geliang,

On 07/11/2024 07:45, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> mptcp_pm_remove_addrs() actually only deletes one address, which does
> not match its name. This patch renames it to mptcp_pm_remove_addr_entry()
> and changes the parameter "rm_list" to "entry".
> 
> With the help of mptcp_pm_remove_addr_entry(), it's no longer necessary to
> move the entry to be deleted to free_list and then traverse the list to
> delete the entry, which is not allowed in BPF. The entry can be directly
> deleted through list_del_rcu() and sock_kfree_s() now.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm_userspace.c | 33 ++++++++++++---------------------
>  net/mptcp/protocol.h     |  3 ++-
>  2 files changed, 14 insertions(+), 22 deletions(-)
> 
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 737a07f5defe..a98da9a44bfa 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -286,26 +286,21 @@ static int mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
>  	return err;
>  }
>  
> -void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct list_head *rm_list)
> +void mptcp_pm_remove_addr_entry(struct mptcp_sock *msk,
> +				struct mptcp_pm_addr_entry *entry)
>  {
>  	struct mptcp_rm_list alist = { .nr = 0 };
> -	struct mptcp_pm_addr_entry *entry;
>  	int anno_nr = 0;
>  
> -	list_for_each_entry(entry, rm_list, list) {
> -		if (alist.nr >= MPTCP_RM_IDS_MAX)
> -			break;
> -
> -		/* only delete if either announced or matching a subflow */
> -		if (mptcp_remove_anno_list_by_saddr(msk, &entry->addr))
> -			anno_nr++;
> -		else if (!mptcp_lookup_subflow_by_saddr(&msk->conn_list,
> -							&entry->addr))
> -			continue;
> +	/* only delete if either announced or matching a subflow */
> +	if (mptcp_remove_anno_list_by_saddr(msk, &entry->addr))
> +		anno_nr++;
> +	else if (!mptcp_lookup_subflow_by_saddr(&msk->conn_list, &entry->addr))
> +		goto out;

Here, you can 'return', no need to use this new 'out' label.

>  
> -		alist.ids[alist.nr++] = entry->addr.id;
> -	}
> +	alist.ids[alist.nr++] = entry->addr.id;
>  
> +out:
>  	if (alist.nr) {

If the 'out' label is removed, you can also remove this if-statement.

>  		spin_lock_bh(&msk->pm.lock);
>  		msk->pm.add_addr_signaled -= anno_nr;

(...)

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

Re: [PATCH mptcp-next v3 5/9] mptcp: drop free_list for deleting entries

[Geliang Tang]

On Wed, 2024-12-04 at 18:49 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 07/11/2024 07:45, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > mptcp_pm_remove_addrs() actually only deletes one address, which
> > does
> > not match its name. This patch renames it to
> > mptcp_pm_remove_addr_entry()
> > and changes the parameter "rm_list" to "entry".
> > 
> > With the help of mptcp_pm_remove_addr_entry(), it's no longer
> > necessary to
> > move the entry to be deleted to free_list and then traverse the
> > list to
> > delete the entry, which is not allowed in BPF. The entry can be
> > directly
> > deleted through list_del_rcu() and sock_kfree_s() now.
> > 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> >  net/mptcp/pm_userspace.c | 33 ++++++++++++---------------------
> >  net/mptcp/protocol.h     |  3 ++-
> >  2 files changed, 14 insertions(+), 22 deletions(-)
> > 
> > diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> > index 737a07f5defe..a98da9a44bfa 100644
> > --- a/net/mptcp/pm_userspace.c
> > +++ b/net/mptcp/pm_userspace.c
> > @@ -286,26 +286,21 @@ static int
> > mptcp_userspace_pm_remove_id_zero_address(struct mptcp_sock *msk,
> >  	return err;
> >  }
> >  
> > -void mptcp_pm_remove_addrs(struct mptcp_sock *msk, struct
> > list_head *rm_list)
> > +void mptcp_pm_remove_addr_entry(struct mptcp_sock *msk,
> > +				struct mptcp_pm_addr_entry *entry)
> >  {
> >  	struct mptcp_rm_list alist = { .nr = 0 };
> > -	struct mptcp_pm_addr_entry *entry;
> >  	int anno_nr = 0;
> >  
> > -	list_for_each_entry(entry, rm_list, list) {
> > -		if (alist.nr >= MPTCP_RM_IDS_MAX)
> > -			break;
> > -
> > -		/* only delete if either announced or matching a
> > subflow */
> > -		if (mptcp_remove_anno_list_by_saddr(msk, &entry-
> > >addr))
> > -			anno_nr++;
> > -		else if (!mptcp_lookup_subflow_by_saddr(&msk-
> > >conn_list,
> > -							&entry-
> > >addr))
> > -			continue;
> > +	/* only delete if either announced or matching a subflow
> > */
> > +	if (mptcp_remove_anno_list_by_saddr(msk, &entry->addr))
> > +		anno_nr++;
> > +	else if (!mptcp_lookup_subflow_by_saddr(&msk->conn_list,
> > &entry->addr))
> > +		goto out;
> 
> Here, you can 'return', no need to use this new 'out' label.
> 
> >  
> > -		alist.ids[alist.nr++] = entry->addr.id;
> > -	}
> > +	alist.ids[alist.nr++] = entry->addr.id;
> >  
> > +out:
> >  	if (alist.nr) {
> 
> If the 'out' label is removed, you can also remove this if-statement.

Good idea! Thanks.

> 
> >  		spin_lock_bh(&msk->pm.lock);
> >  		msk->pm.add_addr_signaled -= anno_nr;
> 
> (...)
> 
> Cheers,
> Matt

[PATCH mptcp-next v3 6/9] mptcp: use mptcp_pm_local in pm_netlink only

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

struct mptcp_pm_local is used in pm_netlink to reduce memory usage, but
it has less effect in pm_userspace because userspace pm doesn't use an
array of struct mptcp_pm_addr_entry type.

So this patch moves struct mptcp_pm_local to pm_netlink and restores the
use of mptcp_pm_addr_entry type parameters in __mptcp_subflow_connect().
In this case, only one "struct mptcp_pm_addr_entry" is needed, that's not
reserving too much memory.

This patch makes the path manager code simpler, and easier to implement
the BPF path manager.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm_netlink.c   | 26 ++++++++++++++++++++++----
 net/mptcp/pm_userspace.c |  7 +------
 net/mptcp/protocol.h     |  8 +-------
 net/mptcp/subflow.c      |  2 +-
 4 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 8aba7670345d..00911fae5d88 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -40,6 +40,12 @@ struct pm_nl_pernet {
 	DECLARE_BITMAP(id_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
 };
 
+struct mptcp_pm_local {
+	struct mptcp_addr_info	addr;
+	u8			flags;
+	int			ifindex;
+};
+
 #define MPTCP_PM_ADDR_MAX	8
 #define ADD_ADDR_RETRANS_MAX	3
 
@@ -638,8 +644,14 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 			continue;
 
 		spin_unlock_bh(&msk->pm.lock);
-		for (i = 0; i < nr; i++)
-			__mptcp_subflow_connect(sk, &local, &addrs[i]);
+		for (i = 0; i < nr; i++) {
+			struct mptcp_pm_addr_entry entry = { 0 };
+
+			entry.addr = local.addr;
+			entry.flags = local.flags;
+			entry.ifindex = local.ifindex;
+			__mptcp_subflow_connect(sk, &entry, &addrs[i]);
+		}
 		spin_lock_bh(&msk->pm.lock);
 	}
 	mptcp_pm_nl_check_work_pending(msk);
@@ -755,9 +767,15 @@ static void mptcp_pm_nl_add_addr_received(struct mptcp_sock *msk)
 		return;
 
 	spin_unlock_bh(&msk->pm.lock);
-	for (i = 0; i < nr; i++)
-		if (__mptcp_subflow_connect(sk, &locals[i], &remote) == 0)
+	for (i = 0; i < nr; i++) {
+		struct mptcp_pm_addr_entry entry = { 0 };
+
+		entry.addr = locals[i].addr;
+		entry.flags = locals[i].flags;
+		entry.ifindex = locals[i].ifindex;
+		if (__mptcp_subflow_connect(sk, &entry, &remote) == 0)
 			sf_created = true;
+	}
 	spin_lock_bh(&msk->pm.lock);
 
 	if (sf_created) {
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index a98da9a44bfa..db09350b5022 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -368,7 +368,6 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 	struct nlattr *laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	struct mptcp_pm_addr_entry entry = { 0 };
 	struct mptcp_addr_info addr_r;
-	struct mptcp_pm_local local;
 	struct mptcp_sock *msk;
 	int err = -EINVAL;
 	struct sock *sk;
@@ -415,12 +414,8 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 		goto create_err;
 	}
 
-	local.addr = entry.addr;
-	local.flags = entry.flags;
-	local.ifindex = entry.ifindex;
-
 	lock_sock(sk);
-	err = __mptcp_subflow_connect(sk, &local, &addr_r);
+	err = __mptcp_subflow_connect(sk, &entry, &addr_r);
 	release_sock(sk);
 
 	spin_lock_bh(&msk->pm.lock);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 19a811220621..775ac2fd6854 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -236,12 +236,6 @@ struct mptcp_pm_data {
 	struct mptcp_rm_list rm_list_rx;
 };
 
-struct mptcp_pm_local {
-	struct mptcp_addr_info	addr;
-	u8			flags;
-	int			ifindex;
-};
-
 struct mptcp_pm_addr_entry {
 	struct list_head	list;
 	struct mptcp_addr_info	addr;
@@ -736,7 +730,7 @@ bool mptcp_addresses_equal(const struct mptcp_addr_info *a,
 void mptcp_local_address(const struct sock_common *skc, struct mptcp_addr_info *addr);
 
 /* called with sk socket lock held */
-int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_local *local,
+int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_addr_entry *local,
 			    const struct mptcp_addr_info *remote);
 int mptcp_subflow_create_socket(struct sock *sk, unsigned short family,
 				struct socket **new_sock);
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 07352b15f145..2ae8f467abc1 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1586,7 +1586,7 @@ void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
 #endif
 }
 
-int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_local *local,
+int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_pm_addr_entry *local,
 			    const struct mptcp_addr_info *remote)
 {
 	struct mptcp_sock *msk = mptcp_sk(sk);
-- 
2.45.2

Re: [PATCH mptcp-next v3 6/9] mptcp: use mptcp_pm_local in pm_netlink only

[Geliang Tang]

As I mentioned in [1], I decided to deprecate this patch and keep the
code for struct mptcp_pm_local as it is.

I changed this patch as "Rejected" in patchwork. Other patches in this
set are still valid.

Thanks,
-Geliang

[1]
https://patchwork.kernel.org/project/mptcp/patch/5b8a8e318c9d661f495b6d0be2b7a776de7da7a1.1729588019.git.tanggeliang@kylinos.cn/

On Thu, 2024-11-07 at 14:45 +0800, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> struct mptcp_pm_local is used in pm_netlink to reduce memory usage,
> but
> it has less effect in pm_userspace because userspace pm doesn't use
> an
> array of struct mptcp_pm_addr_entry type.
> 
> So this patch moves struct mptcp_pm_local to pm_netlink and restores
> the
> use of mptcp_pm_addr_entry type parameters in
> __mptcp_subflow_connect().
> In this case, only one "struct mptcp_pm_addr_entry" is needed, that's
> not
> reserving too much memory.
> 
> This patch makes the path manager code simpler, and easier to
> implement
> the BPF path manager.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm_netlink.c   | 26 ++++++++++++++++++++++----
>  net/mptcp/pm_userspace.c |  7 +------
>  net/mptcp/protocol.h     |  8 +-------
>  net/mptcp/subflow.c      |  2 +-
>  4 files changed, 25 insertions(+), 18 deletions(-)
> 
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 8aba7670345d..00911fae5d88 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -40,6 +40,12 @@ struct pm_nl_pernet {
>  	DECLARE_BITMAP(id_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
>  };
>  
> +struct mptcp_pm_local {
> +	struct mptcp_addr_info	addr;
> +	u8			flags;
> +	int			ifindex;
> +};
> +
>  #define MPTCP_PM_ADDR_MAX	8
>  #define ADD_ADDR_RETRANS_MAX	3
>  
> @@ -638,8 +644,14 @@ static void
> mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
>  			continue;
>  
>  		spin_unlock_bh(&msk->pm.lock);
> -		for (i = 0; i < nr; i++)
> -			__mptcp_subflow_connect(sk, &local,
> &addrs[i]);
> +		for (i = 0; i < nr; i++) {
> +			struct mptcp_pm_addr_entry entry = { 0 };
> +
> +			entry.addr = local.addr;
> +			entry.flags = local.flags;
> +			entry.ifindex = local.ifindex;
> +			__mptcp_subflow_connect(sk, &entry,
> &addrs[i]);
> +		}
>  		spin_lock_bh(&msk->pm.lock);
>  	}
>  	mptcp_pm_nl_check_work_pending(msk);
> @@ -755,9 +767,15 @@ static void mptcp_pm_nl_add_addr_received(struct
> mptcp_sock *msk)
>  		return;
>  
>  	spin_unlock_bh(&msk->pm.lock);
> -	for (i = 0; i < nr; i++)
> -		if (__mptcp_subflow_connect(sk, &locals[i], &remote)
> == 0)
> +	for (i = 0; i < nr; i++) {
> +		struct mptcp_pm_addr_entry entry = { 0 };
> +
> +		entry.addr = locals[i].addr;
> +		entry.flags = locals[i].flags;
> +		entry.ifindex = locals[i].ifindex;
> +		if (__mptcp_subflow_connect(sk, &entry, &remote) ==
> 0)
>  			sf_created = true;
> +	}
>  	spin_lock_bh(&msk->pm.lock);
>  
>  	if (sf_created) {
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index a98da9a44bfa..db09350b5022 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -368,7 +368,6 @@ int mptcp_pm_nl_subflow_create_doit(struct
> sk_buff *skb, struct genl_info *info)
>  	struct nlattr *laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
>  	struct mptcp_pm_addr_entry entry = { 0 };
>  	struct mptcp_addr_info addr_r;
> -	struct mptcp_pm_local local;
>  	struct mptcp_sock *msk;
>  	int err = -EINVAL;
>  	struct sock *sk;
> @@ -415,12 +414,8 @@ int mptcp_pm_nl_subflow_create_doit(struct
> sk_buff *skb, struct genl_info *info)
>  		goto create_err;
>  	}
>  
> -	local.addr = entry.addr;
> -	local.flags = entry.flags;
> -	local.ifindex = entry.ifindex;
> -
>  	lock_sock(sk);
> -	err = __mptcp_subflow_connect(sk, &local, &addr_r);
> +	err = __mptcp_subflow_connect(sk, &entry, &addr_r);
>  	release_sock(sk);
>  
>  	spin_lock_bh(&msk->pm.lock);
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index 19a811220621..775ac2fd6854 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -236,12 +236,6 @@ struct mptcp_pm_data {
>  	struct mptcp_rm_list rm_list_rx;
>  };
>  
> -struct mptcp_pm_local {
> -	struct mptcp_addr_info	addr;
> -	u8			flags;
> -	int			ifindex;
> -};
> -
>  struct mptcp_pm_addr_entry {
>  	struct list_head	list;
>  	struct mptcp_addr_info	addr;
> @@ -736,7 +730,7 @@ bool mptcp_addresses_equal(const struct
> mptcp_addr_info *a,
>  void mptcp_local_address(const struct sock_common *skc, struct
> mptcp_addr_info *addr);
>  
>  /* called with sk socket lock held */
> -int __mptcp_subflow_connect(struct sock *sk, const struct
> mptcp_pm_local *local,
> +int __mptcp_subflow_connect(struct sock *sk, const struct
> mptcp_pm_addr_entry *local,
>  			    const struct mptcp_addr_info *remote);
>  int mptcp_subflow_create_socket(struct sock *sk, unsigned short
> family,
>  				struct socket **new_sock);
> diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
> index 07352b15f145..2ae8f467abc1 100644
> --- a/net/mptcp/subflow.c
> +++ b/net/mptcp/subflow.c
> @@ -1586,7 +1586,7 @@ void mptcp_info2sockaddr(const struct
> mptcp_addr_info *info,
>  #endif
>  }
>  
> -int __mptcp_subflow_connect(struct sock *sk, const struct
> mptcp_pm_local *local,
> +int __mptcp_subflow_connect(struct sock *sk, const struct
> mptcp_pm_addr_entry *local,
>  			    const struct mptcp_addr_info *remote)
>  {
>  	struct mptcp_sock *msk = mptcp_sk(sk);

[PATCH mptcp-next v3 7/9] mptcp: drop struct mptcp_pm_add_entry

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

There is no need to add a dedicated address entry type "mptcp_pm_add_entry"
to represent ADD_ADDR addresses. Additional fields for ADD_ADDR addresses
can be added into struct mptcp_pm_addr_entry directly. This makes the path
manager code simpler.

Here "union" can be used to merge struct mptcp_pm_addr_entry and struct
mptcp_pm_add_entry into one. Then all mptcp_pm_add_entry can be replaced by
mptcp_pm_addr_entry.

Although this increases the size of the structure even more, but that's OK
to do so because it is not used in an array.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm_netlink.c | 26 +++++++++-----------------
 net/mptcp/protocol.h   | 20 +++++++++++++++-----
 2 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 00911fae5d88..3f3eaa18ffae 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -18,14 +18,6 @@
 
 static int pm_nl_pernet_id;
 
-struct mptcp_pm_add_entry {
-	struct list_head	list;
-	struct mptcp_addr_info	addr;
-	u8			retrans_times;
-	struct timer_list	add_timer;
-	struct mptcp_sock	*sock;
-};
-
 struct pm_nl_pernet {
 	/* protects pernet updates */
 	spinlock_t		lock;
@@ -257,11 +249,11 @@ bool mptcp_pm_nl_check_work_pending(struct mptcp_sock *msk)
 	return true;
 }
 
-struct mptcp_pm_add_entry *
+struct mptcp_pm_addr_entry *
 mptcp_lookup_anno_list_by_saddr(const struct mptcp_sock *msk,
 				const struct mptcp_addr_info *addr)
 {
-	struct mptcp_pm_add_entry *entry;
+	struct mptcp_pm_addr_entry *entry;
 
 	lockdep_assert_held(&msk->pm.lock);
 
@@ -275,7 +267,7 @@ mptcp_lookup_anno_list_by_saddr(const struct mptcp_sock *msk,
 
 bool mptcp_pm_sport_in_anno_list(struct mptcp_sock *msk, const struct sock *sk)
 {
-	struct mptcp_pm_add_entry *entry;
+	struct mptcp_pm_addr_entry *entry;
 	struct mptcp_addr_info saddr;
 	bool ret = false;
 
@@ -296,7 +288,7 @@ bool mptcp_pm_sport_in_anno_list(struct mptcp_sock *msk, const struct sock *sk)
 
 static void mptcp_pm_add_timer(struct timer_list *timer)
 {
-	struct mptcp_pm_add_entry *entry = from_timer(entry, timer, add_timer);
+	struct mptcp_pm_addr_entry *entry = from_timer(entry, timer, add_timer);
 	struct mptcp_sock *msk = entry->sock;
 	struct sock *sk = (struct sock *)msk;
 
@@ -338,11 +330,11 @@ static void mptcp_pm_add_timer(struct timer_list *timer)
 	__sock_put(sk);
 }
 
-struct mptcp_pm_add_entry *
+struct mptcp_pm_addr_entry *
 mptcp_pm_del_add_timer(struct mptcp_sock *msk,
 		       const struct mptcp_addr_info *addr, bool check_id)
 {
-	struct mptcp_pm_add_entry *entry;
+	struct mptcp_pm_addr_entry *entry;
 	struct sock *sk = (struct sock *)msk;
 	struct timer_list *add_timer = NULL;
 
@@ -366,7 +358,7 @@ mptcp_pm_del_add_timer(struct mptcp_sock *msk,
 bool mptcp_pm_alloc_anno_list(struct mptcp_sock *msk,
 			      const struct mptcp_addr_info *addr)
 {
-	struct mptcp_pm_add_entry *add_entry = NULL;
+	struct mptcp_pm_addr_entry *add_entry = NULL;
 	struct sock *sk = (struct sock *)msk;
 	struct net *net = sock_net(sk);
 
@@ -402,7 +394,7 @@ bool mptcp_pm_alloc_anno_list(struct mptcp_sock *msk,
 
 void mptcp_pm_free_anno_list(struct mptcp_sock *msk)
 {
-	struct mptcp_pm_add_entry *entry, *tmp;
+	struct mptcp_pm_addr_entry *entry, *tmp;
 	struct sock *sk = (struct sock *)msk;
 	LIST_HEAD(free_list);
 
@@ -1474,7 +1466,7 @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
 bool mptcp_remove_anno_list_by_saddr(struct mptcp_sock *msk,
 				     const struct mptcp_addr_info *addr)
 {
-	struct mptcp_pm_add_entry *entry;
+	struct mptcp_pm_addr_entry *entry;
 
 	entry = mptcp_pm_del_add_timer(msk, addr, false);
 	if (entry) {
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 775ac2fd6854..1414e79564c7 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -239,9 +239,19 @@ struct mptcp_pm_data {
 struct mptcp_pm_addr_entry {
 	struct list_head	list;
 	struct mptcp_addr_info	addr;
-	u8			flags;
-	int			ifindex;
-	struct socket		*lsk;
+	union {
+		struct {
+			u8			flags;
+			int			ifindex;
+			struct socket		*lsk;
+		};
+		/* mptcp_pm_add_entry */
+		struct {
+			u8			retrans_times;
+			struct timer_list	add_timer;
+			struct mptcp_sock	*sock;
+		};
+	};
 };
 
 struct mptcp_data_frag {
@@ -1019,10 +1029,10 @@ bool mptcp_pm_alloc_anno_list(struct mptcp_sock *msk,
 			      const struct mptcp_addr_info *addr);
 void mptcp_pm_free_anno_list(struct mptcp_sock *msk);
 bool mptcp_pm_sport_in_anno_list(struct mptcp_sock *msk, const struct sock *sk);
-struct mptcp_pm_add_entry *
+struct mptcp_pm_addr_entry *
 mptcp_pm_del_add_timer(struct mptcp_sock *msk,
 		       const struct mptcp_addr_info *addr, bool check_id);
-struct mptcp_pm_add_entry *
+struct mptcp_pm_addr_entry *
 mptcp_lookup_anno_list_by_saddr(const struct mptcp_sock *msk,
 				const struct mptcp_addr_info *addr);
 bool mptcp_lookup_subflow_by_saddr(const struct list_head *list,
-- 
2.45.2

Re: [PATCH mptcp-next v3 7/9] mptcp: drop struct mptcp_pm_add_entry

[Matthieu Baerts]

On 07/11/2024 07:45, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> There is no need to add a dedicated address entry type "mptcp_pm_add_entry"
> to represent ADD_ADDR addresses. Additional fields for ADD_ADDR addresses
> can be added into struct mptcp_pm_addr_entry directly. This makes the path
> manager code simpler.

To be honest, I don't know if the "union" simplifies stuff: we will need
to make sure some fields are not overridden by mistake. Always a bit risky.

Do you need this to simplify another patch later on?

> Here "union" can be used to merge struct mptcp_pm_addr_entry and struct
> mptcp_pm_add_entry into one. Then all mptcp_pm_add_entry can be replaced by
> mptcp_pm_addr_entry.
> 
> Although this increases the size of the structure even more, but that's OK
> to do so because it is not used in an array.
Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH mptcp-next v3 7/9] mptcp: drop struct mptcp_pm_add_entry

[Geliang Tang]

On Wed, 2024-12-04 at 18:49 +0100, Matthieu Baerts wrote:
> 
> 
> On 07/11/2024 07:45, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > There is no need to add a dedicated address entry type
> > "mptcp_pm_add_entry"
> > to represent ADD_ADDR addresses. Additional fields for ADD_ADDR
> > addresses
> > can be added into struct mptcp_pm_addr_entry directly. This makes
> > the path
> > manager code simpler.
> 
> To be honest, I don't know if the "union" simplifies stuff: we will
> need
> to make sure some fields are not overridden by mistake. Always a bit
> risky.
> 
> Do you need this to simplify another patch later on?

No one needs it, just a cleanup. I dropped it in v4.

> 
> > Here "union" can be used to merge struct mptcp_pm_addr_entry and
> > struct
> > mptcp_pm_add_entry into one. Then all mptcp_pm_add_entry can be
> > replaced by
> > mptcp_pm_addr_entry.
> > 
> > Although this increases the size of the structure even more, but
> > that's OK
> > to do so because it is not used in an array.
> Cheers,
> Matt

[PATCH mptcp-next v3 8/9] mptcp: change local addr type of subflow_destroy

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Generally, in the path manager interfaces, the local address is defined as
an mptcp_pm_addr_entry type address, while the remote address is defined as
an mptcp_addr_info type one:

        (struct mptcp_pm_addr_entry *local, struct mptcp_addr_info *remote)

But subflow_destroy() interface uses two mptcp_addr_info type parameters.
This patch changes the first one to mptcp_pm_addr_entry type and use helper
mptcp_pm_parse_entry() to parse it instead of using mptcp_pm_parse_addr().

This patch doesn't change the behaviour of the code, just refactoring.

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

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index db09350b5022..07e0c7259494 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -482,7 +482,7 @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 {
 	struct nlattr *raddr = info->attrs[MPTCP_PM_ATTR_ADDR_REMOTE];
 	struct nlattr *laddr = info->attrs[MPTCP_PM_ATTR_ADDR];
-	struct mptcp_addr_info addr_l;
+	struct mptcp_pm_addr_entry addr_l;
 	struct mptcp_addr_info addr_r;
 	struct mptcp_sock *msk;
 	struct sock *sk, *ssk;
@@ -499,7 +499,7 @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 
 	sk = (struct sock *)msk;
 
-	err = mptcp_pm_parse_addr(laddr, info, &addr_l);
+	err = mptcp_pm_parse_entry(laddr, info, true, &addr_l);
 	if (err < 0) {
 		NL_SET_ERR_MSG_ATTR(info->extack, laddr, "error parsing local addr");
 		goto destroy_err;
@@ -512,35 +512,34 @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 	}
 
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
-	if (addr_l.family == AF_INET && ipv6_addr_v4mapped(&addr_r.addr6)) {
-		ipv6_addr_set_v4mapped(addr_l.addr.s_addr, &addr_l.addr6);
-		addr_l.family = AF_INET6;
+	if (addr_l.addr.family == AF_INET && ipv6_addr_v4mapped(&addr_r.addr6)) {
+		ipv6_addr_set_v4mapped(addr_l.addr.addr.s_addr, &addr_l.addr.addr6);
+		addr_l.addr.family = AF_INET6;
 	}
-	if (addr_r.family == AF_INET && ipv6_addr_v4mapped(&addr_l.addr6)) {
-		ipv6_addr_set_v4mapped(addr_r.addr.s_addr, &addr_r.addr6);
+	if (addr_r.family == AF_INET && ipv6_addr_v4mapped(&addr_l.addr.addr6)) {
+		ipv6_addr_set_v4mapped(addr_r.addr.s_addr, &addr_l.addr.addr6);
 		addr_r.family = AF_INET6;
 	}
 #endif
-	if (addr_l.family != addr_r.family) {
+	if (addr_l.addr.family != addr_r.family) {
 		GENL_SET_ERR_MSG(info, "address families do not match");
 		err = -EINVAL;
 		goto destroy_err;
 	}
 
-	if (!addr_l.port || !addr_r.port) {
+	if (!addr_l.addr.port || !addr_r.port) {
 		GENL_SET_ERR_MSG(info, "missing local or remote port");
 		err = -EINVAL;
 		goto destroy_err;
 	}
 
 	lock_sock(sk);
-	ssk = mptcp_nl_find_ssk(msk, &addr_l, &addr_r);
+	ssk = mptcp_nl_find_ssk(msk, &addr_l.addr, &addr_r);
 	if (ssk) {
 		struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
-		struct mptcp_pm_addr_entry entry = { .addr = addr_l };
 
 		spin_lock_bh(&msk->pm.lock);
-		mptcp_userspace_pm_delete_local_addr(msk, &entry);
+		mptcp_userspace_pm_delete_local_addr(msk, &addr_l);
 		spin_unlock_bh(&msk->pm.lock);
 		mptcp_subflow_shutdown(sk, ssk, RCV_SHUTDOWN | SEND_SHUTDOWN);
 		mptcp_close_ssk(sk, ssk, subflow);
-- 
2.45.2

[PATCH mptcp-next v3 9/9] mptcp: drop useless "err = 0" in subflow_destroy

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Upon successful return, mptcp_pm_parse_addr() returns 0. There is no need
to set "err = 0" after this. So after mptcp_nl_find_ssk() returns, just
need to set "err = -ESRCH", then release and free msk socket if it returns
NULL.

Also, no need to define the veriable "subflow" in subflow_destroy(), use
mptcp_subflow_ctx(ssk) directly.

This patch doesn't change the behaviour of the code, just refactoring.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm_userspace.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 07e0c7259494..8545212f023e 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -535,19 +535,18 @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 
 	lock_sock(sk);
 	ssk = mptcp_nl_find_ssk(msk, &addr_l.addr, &addr_r);
-	if (ssk) {
-		struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
-
-		spin_lock_bh(&msk->pm.lock);
-		mptcp_userspace_pm_delete_local_addr(msk, &addr_l);
-		spin_unlock_bh(&msk->pm.lock);
-		mptcp_subflow_shutdown(sk, ssk, RCV_SHUTDOWN | SEND_SHUTDOWN);
-		mptcp_close_ssk(sk, ssk, subflow);
-		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RMSUBFLOW);
-		err = 0;
-	} else {
+	if (!ssk) {
 		err = -ESRCH;
+		release_sock(sk);
+		goto destroy_err;
 	}
+
+	spin_lock_bh(&msk->pm.lock);
+	mptcp_userspace_pm_delete_local_addr(msk, &addr_l);
+	spin_unlock_bh(&msk->pm.lock);
+	mptcp_subflow_shutdown(sk, ssk, RCV_SHUTDOWN | SEND_SHUTDOWN);
+	mptcp_close_ssk(sk, ssk, mptcp_subflow_ctx(ssk));
+	MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RMSUBFLOW);
 	release_sock(sk);
 
 destroy_err:
-- 
2.45.2

Re: [PATCH mptcp-next v3 9/9] mptcp: drop useless "err = 0" in subflow_destroy

[Matthieu Baerts]

Hi Geliang,

On 07/11/2024 07:45, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Upon successful return, mptcp_pm_parse_addr() returns 0. There is no need
> to set "err = 0" after this. So after mptcp_nl_find_ssk() returns, just
> need to set "err = -ESRCH", then release and free msk socket if it returns
> NULL.
> 
> Also, no need to define the veriable "subflow" in subflow_destroy(), use
> mptcp_subflow_ctx(ssk) directly.
> 
> This patch doesn't change the behaviour of the code, just refactoring.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm_userspace.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 07e0c7259494..8545212f023e 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -535,19 +535,18 @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
>  
>  	lock_sock(sk);
>  	ssk = mptcp_nl_find_ssk(msk, &addr_l.addr, &addr_r);
> -	if (ssk) {
> -		struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> -
> -		spin_lock_bh(&msk->pm.lock);
> -		mptcp_userspace_pm_delete_local_addr(msk, &addr_l);
> -		spin_unlock_bh(&msk->pm.lock);
> -		mptcp_subflow_shutdown(sk, ssk, RCV_SHUTDOWN | SEND_SHUTDOWN);
> -		mptcp_close_ssk(sk, ssk, subflow);
> -		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RMSUBFLOW);
> -		err = 0;

OK to drop 'err = 0' but...

> -	} else {
> +	if (!ssk) {
>  		err = -ESRCH;
> +		release_sock(sk);
> +		goto destroy_err;

... I think it is always best to reduce the number of exit path: so
here, I think it is best to add a new label before release_sock(sk)
below, instead of duplicating this release_sock(sk). Something like:

      ssk = mptcp_nl_find_ssk(...);
      if (!ssk) {
          err = -ESRCH;
          goto release_sock;
      }

      (...)

  release_sock:
      release_sock(sk);

  destroy_err:
      (...)

WDYT?

>  	}
> +
> +	spin_lock_bh(&msk->pm.lock);
> +	mptcp_userspace_pm_delete_local_addr(msk, &addr_l);
> +	spin_unlock_bh(&msk->pm.lock);
> +	mptcp_subflow_shutdown(sk, ssk, RCV_SHUTDOWN | SEND_SHUTDOWN);
> +	mptcp_close_ssk(sk, ssk, mptcp_subflow_ctx(ssk));
> +	MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RMSUBFLOW);
>  	release_sock(sk);
>  
>  destroy_err:

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

Re: [PATCH mptcp-next v3 9/9] mptcp: drop useless "err = 0" in subflow_destroy

[Geliang Tang]

On Wed, 2024-12-04 at 18:49 +0100, Matthieu Baerts wrote:
> Hi Geliang,
> 
> On 07/11/2024 07:45, Geliang Tang wrote:
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > Upon successful return, mptcp_pm_parse_addr() returns 0. There is
> > no need
> > to set "err = 0" after this. So after mptcp_nl_find_ssk() returns,
> > just
> > need to set "err = -ESRCH", then release and free msk socket if it
> > returns
> > NULL.
> > 
> > Also, no need to define the veriable "subflow" in
> > subflow_destroy(), use
> > mptcp_subflow_ctx(ssk) directly.
> > 
> > This patch doesn't change the behaviour of the code, just
> > refactoring.
> > 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> >  net/mptcp/pm_userspace.c | 21 ++++++++++-----------
> >  1 file changed, 10 insertions(+), 11 deletions(-)
> > 
> > diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> > index 07e0c7259494..8545212f023e 100644
> > --- a/net/mptcp/pm_userspace.c
> > +++ b/net/mptcp/pm_userspace.c
> > @@ -535,19 +535,18 @@ int mptcp_pm_nl_subflow_destroy_doit(struct
> > sk_buff *skb, struct genl_info *info
> >  
> >  	lock_sock(sk);
> >  	ssk = mptcp_nl_find_ssk(msk, &addr_l.addr, &addr_r);
> > -	if (ssk) {
> > -		struct mptcp_subflow_context *subflow =
> > mptcp_subflow_ctx(ssk);
> > -
> > -		spin_lock_bh(&msk->pm.lock);
> > -		mptcp_userspace_pm_delete_local_addr(msk,
> > &addr_l);
> > -		spin_unlock_bh(&msk->pm.lock);
> > -		mptcp_subflow_shutdown(sk, ssk, RCV_SHUTDOWN |
> > SEND_SHUTDOWN);
> > -		mptcp_close_ssk(sk, ssk, subflow);
> > -		MPTCP_INC_STATS(sock_net(sk),
> > MPTCP_MIB_RMSUBFLOW);
> > -		err = 0;
> 
> OK to drop 'err = 0' but...
> 
> > -	} else {
> > +	if (!ssk) {
> >  		err = -ESRCH;
> > +		release_sock(sk);
> > +		goto destroy_err;
> 
> ... I think it is always best to reduce the number of exit path: so
> here, I think it is best to add a new label before release_sock(sk)
> below, instead of duplicating this release_sock(sk). Something like:
> 
>       ssk = mptcp_nl_find_ssk(...);
>       if (!ssk) {
>           err = -ESRCH;
>           goto release_sock;
>       }
> 
>       (...)
> 
>   release_sock:
>       release_sock(sk);
> 
>   destroy_err:
>       (...)
> 
> WDYT?

I agree. Updated in v4.

Thanks,
-Geliang

> 
> >  	}
> > +
> > +	spin_lock_bh(&msk->pm.lock);
> > +	mptcp_userspace_pm_delete_local_addr(msk, &addr_l);
> > +	spin_unlock_bh(&msk->pm.lock);
> > +	mptcp_subflow_shutdown(sk, ssk, RCV_SHUTDOWN |
> > SEND_SHUTDOWN);
> > +	mptcp_close_ssk(sk, ssk, mptcp_subflow_ctx(ssk));
> > +	MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RMSUBFLOW);
> >  	release_sock(sk);
> >  
> >  destroy_err:
> 
> Cheers,
> Matt

Re: [PATCH mptcp-next v3 0/9] BPF path manager, part 1

[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: 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): Unstable: 1 failed test(s): bpftest_test_progs-cpuv4_mptcp 🔴
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/11718116131

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


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 v3 0/9] BPF path manager, part 1

[Matthieu Baerts]

Hi Geliang,

On 07/11/2024 07:45, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> v3:

Thank you for the v3. Sorry, I forgot about this series...

>  - address Matt's comments in v2 (thanks)

Please next time add a short individual changelog in each modified
patch: here I obviously forgot my previous comments, but even when the
review of the new version is done the day after, I don't think I would
remember all of them :)

>  - only include cleanups and refactoring patches in this set.

Good idea!

I had just a few small comments, but globally it looks good.

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