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

[Mat Martineau <martineau@kernel.org>]

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