[PATCH mptcp-net 1/4] mptcp: add needs_id for userspace appending addr

Geliang Tang posted 4 patches 3 months, 4 weeks ago
[PATCH mptcp-net 1/4] mptcp: add needs_id for userspace appending addr
Posted by Geliang Tang 3 months, 4 weeks ago
From: Geliang Tang <tanggeliang@kylinos.cn>

When userspace PM requires to create an ID 0 subflow in "userspace pm
create id 0 subflow" test like this:

        userspace_pm_add_sf $ns2 10.0.3.2 0

An ID 1 subflow, in fact, is created.

Since in mptcp_pm_nl_append_new_local_addr(), 'id 0' will be treated as
no ID is set by userspace, and will allocate a new ID immediately:

     if (!e->addr.id)
             e->addr.id = find_next_zero_bit(pernet->id_bitmap,
                                             MPTCP_PM_MAX_ADDR_ID + 1,
                                             1);

To solve this issue, a new parameter needs_id is added for
mptcp_userspace_pm_append_new_local_addr() to distinguish between
whether userspace PM has set an ID 0 or whether userspace PM has
not set any address.

needs_id is true in mptcp_userspace_pm_get_local_id(), but false in
mptcp_pm_nl_announce_doit() and mptcp_pm_nl_subflow_create_doit().

Fixes: e5ed101a6028 ("mptcp: userspace pm allow creating id 0 subflow")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm_userspace.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 4f3901d5b8ef..e582b3b2d174 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -26,7 +26,8 @@ void mptcp_free_local_addr_list(struct mptcp_sock *msk)
 }
 
 static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
-						    struct mptcp_pm_addr_entry *entry)
+						    struct mptcp_pm_addr_entry *entry,
+						    bool needs_id)
 {
 	DECLARE_BITMAP(id_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
 	struct mptcp_pm_addr_entry *match = NULL;
@@ -41,7 +42,7 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
 	spin_lock_bh(&msk->pm.lock);
 	list_for_each_entry(e, &msk->pm.userspace_pm_local_addr_list, list) {
 		addr_match = mptcp_addresses_equal(&e->addr, &entry->addr, true);
-		if (addr_match && entry->addr.id == 0)
+		if (addr_match && entry->addr.id == 0 && needs_id)
 			entry->addr.id = e->addr.id;
 		id_match = (e->addr.id == entry->addr.id);
 		if (addr_match && id_match) {
@@ -64,7 +65,7 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
 		}
 
 		*e = *entry;
-		if (!e->addr.id)
+		if (!e->addr.id && needs_id)
 			e->addr.id = find_next_zero_bit(id_bitmap,
 							MPTCP_PM_MAX_ADDR_ID + 1,
 							1);
@@ -153,7 +154,7 @@ int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
 	if (new_entry.addr.port == msk_sport)
 		new_entry.addr.port = 0;
 
-	return mptcp_userspace_pm_append_new_local_addr(msk, &new_entry);
+	return mptcp_userspace_pm_append_new_local_addr(msk, &new_entry, true);
 }
 
 int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
@@ -198,7 +199,7 @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
 		goto announce_err;
 	}
 
-	err = mptcp_userspace_pm_append_new_local_addr(msk, &addr_val);
+	err = mptcp_userspace_pm_append_new_local_addr(msk, &addr_val, false);
 	if (err < 0) {
 		GENL_SET_ERR_MSG(info, "did not match address and id");
 		goto announce_err;
@@ -378,7 +379,7 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	local.addr = addr_l;
-	err = mptcp_userspace_pm_append_new_local_addr(msk, &local);
+	err = mptcp_userspace_pm_append_new_local_addr(msk, &local, false);
 	if (err < 0) {
 		GENL_SET_ERR_MSG(info, "did not match address and id");
 		goto create_err;
-- 
2.40.1
Re: [PATCH mptcp-net 1/4] mptcp: add needs_id for userspace appending addr
Posted by Mat Martineau 3 months, 3 weeks ago
On Thu, 1 Feb 2024, Geliang Tang wrote:

> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> When userspace PM requires to create an ID 0 subflow in "userspace pm
> create id 0 subflow" test like this:
>
>        userspace_pm_add_sf $ns2 10.0.3.2 0
>
> An ID 1 subflow, in fact, is created.
>
> Since in mptcp_pm_nl_append_new_local_addr(), 'id 0' will be treated as
> no ID is set by userspace, and will allocate a new ID immediately:
>
>     if (!e->addr.id)
>             e->addr.id = find_next_zero_bit(pernet->id_bitmap,
>                                             MPTCP_PM_MAX_ADDR_ID + 1,
>                                             1);
>
> To solve this issue, a new parameter needs_id is added for
> mptcp_userspace_pm_append_new_local_addr() to distinguish between
> whether userspace PM has set an ID 0 or whether userspace PM has
> not set any address.
>
> needs_id is true in mptcp_userspace_pm_get_local_id(), but false in
> mptcp_pm_nl_announce_doit() and mptcp_pm_nl_subflow_create_doit().
>
> Fixes: e5ed101a6028 ("mptcp: userspace pm allow creating id 0 subflow")
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>

Hi Geliang -

LGTM:

Reviewed-by: Mat Martineau <martineau@kernel.org>

> ---
> net/mptcp/pm_userspace.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 4f3901d5b8ef..e582b3b2d174 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -26,7 +26,8 @@ void mptcp_free_local_addr_list(struct mptcp_sock *msk)
> }
>
> static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
> -						    struct mptcp_pm_addr_entry *entry)
> +						    struct mptcp_pm_addr_entry *entry,
> +						    bool needs_id)
> {
> 	DECLARE_BITMAP(id_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
> 	struct mptcp_pm_addr_entry *match = NULL;
> @@ -41,7 +42,7 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
> 	spin_lock_bh(&msk->pm.lock);
> 	list_for_each_entry(e, &msk->pm.userspace_pm_local_addr_list, list) {
> 		addr_match = mptcp_addresses_equal(&e->addr, &entry->addr, true);
> -		if (addr_match && entry->addr.id == 0)
> +		if (addr_match && entry->addr.id == 0 && needs_id)
> 			entry->addr.id = e->addr.id;
> 		id_match = (e->addr.id == entry->addr.id);
> 		if (addr_match && id_match) {
> @@ -64,7 +65,7 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
> 		}
>
> 		*e = *entry;
> -		if (!e->addr.id)
> +		if (!e->addr.id && needs_id)
> 			e->addr.id = find_next_zero_bit(id_bitmap,
> 							MPTCP_PM_MAX_ADDR_ID + 1,
> 							1);
> @@ -153,7 +154,7 @@ int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
> 	if (new_entry.addr.port == msk_sport)
> 		new_entry.addr.port = 0;
>
> -	return mptcp_userspace_pm_append_new_local_addr(msk, &new_entry);
> +	return mptcp_userspace_pm_append_new_local_addr(msk, &new_entry, true);
> }
>
> int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
> @@ -198,7 +199,7 @@ int mptcp_pm_nl_announce_doit(struct sk_buff *skb, struct genl_info *info)
> 		goto announce_err;
> 	}
>
> -	err = mptcp_userspace_pm_append_new_local_addr(msk, &addr_val);
> +	err = mptcp_userspace_pm_append_new_local_addr(msk, &addr_val, false);
> 	if (err < 0) {
> 		GENL_SET_ERR_MSG(info, "did not match address and id");
> 		goto announce_err;
> @@ -378,7 +379,7 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
> 	}
>
> 	local.addr = addr_l;
> -	err = mptcp_userspace_pm_append_new_local_addr(msk, &local);
> +	err = mptcp_userspace_pm_append_new_local_addr(msk, &local, false);
> 	if (err < 0) {
> 		GENL_SET_ERR_MSG(info, "did not match address and id");
> 		goto create_err;
> -- 
> 2.40.1
>
>
>