From: Geliang Tang <tanggeliang@kylinos.cn>

Just the same as userspace PM, a new parameter needs_id is added for
in-kernel PM mptcp_pm_nl_append_new_local_addr() too.

Add a new helper mptcp_pm_has_addr_attr_id() to check whether an address
ID is set from PM or not. It will be used in the next two commits.

In mptcp_pm_nl_get_local_id(), needs_id is always true, but in
mptcp_pm_nl_add_addr_doit(), pass mptcp_pm_has_addr_attr_id() to
needs_it.

Fixes: fd5a4c04e18 ("mptcp: add the address ID assignment bitmap")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm_netlink.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index d9ad45959219..9367ab506908 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -901,7 +901,8 @@ static void __mptcp_pm_release_addr_entry(struct mptcp_pm_addr_entry *entry)
 }
 
 static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
-					     struct mptcp_pm_addr_entry *entry)
+					     struct mptcp_pm_addr_entry *entry,
+					     bool needs_id)
 {
 	struct mptcp_pm_addr_entry *cur, *del_entry = NULL;
 	unsigned int addr_max;
@@ -949,7 +950,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
 		}
 	}
 
-	if (!entry->addr.id) {
+	if (!entry->addr.id && needs_id) {
 find_next:
 		entry->addr.id = find_next_zero_bit(pernet->id_bitmap,
 						    MPTCP_PM_MAX_ADDR_ID + 1,
@@ -960,7 +961,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
 		}
 	}
 
-	if (!entry->addr.id)
+	if (!entry->addr.id && needs_id)
 		goto out;
 
 	__set_bit(entry->addr.id, pernet->id_bitmap);
@@ -1092,7 +1093,7 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc
 	entry->ifindex = 0;
 	entry->flags = MPTCP_PM_ADDR_FLAG_IMPLICIT;
 	entry->lsk = NULL;
-	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
+	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry, true);
 	if (ret < 0)
 		kfree(entry);
 
@@ -1285,6 +1286,18 @@ static int mptcp_nl_add_subflow_or_signal_addr(struct net *net)
 	return 0;
 }
 
+static bool mptcp_pm_has_addr_attr_id(const struct nlattr *attr,
+				      struct genl_info *info)
+{
+	struct nlattr *tb[MPTCP_PM_ADDR_ATTR_MAX + 1];
+
+	if (!nla_parse_nested_deprecated(tb, MPTCP_PM_ADDR_ATTR_MAX, attr,
+					 mptcp_pm_address_nl_policy, info->extack) &&
+	    tb[MPTCP_PM_ADDR_ATTR_ID])
+		return true;
+	return false;
+}
+
 int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
 {
 	struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
@@ -1326,7 +1339,8 @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
 			goto out_free;
 		}
 	}
-	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
+	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry,
+						!mptcp_pm_has_addr_attr_id(attr, info));
 	if (ret < 0) {
 		GENL_SET_ERR_MSG_FMT(info, "too many addresses or duplicate one: %d", ret);
 		goto out_free;
-- 
2.40.1

On Thu, 1 Feb 2024, Geliang Tang wrote:

> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Just the same as userspace PM, a new parameter needs_id is added for
> in-kernel PM mptcp_pm_nl_append_new_local_addr() too.
>
> Add a new helper mptcp_pm_has_addr_attr_id() to check whether an address
> ID is set from PM or not. It will be used in the next two commits.
>
> In mptcp_pm_nl_get_local_id(), needs_id is always true, but in
> mptcp_pm_nl_add_addr_doit(), pass mptcp_pm_has_addr_attr_id() to
> needs_it.
>
> Fixes: fd5a4c04e18 ("mptcp: add the address ID assignment bitmap")
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>

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

> ---
> net/mptcp/pm_netlink.c | 24 +++++++++++++++++++-----
> 1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index d9ad45959219..9367ab506908 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -901,7 +901,8 @@ static void __mptcp_pm_release_addr_entry(struct mptcp_pm_addr_entry *entry)
> }
>
> static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
> -					     struct mptcp_pm_addr_entry *entry)
> +					     struct mptcp_pm_addr_entry *entry,
> +					     bool needs_id)
> {
> 	struct mptcp_pm_addr_entry *cur, *del_entry = NULL;
> 	unsigned int addr_max;
> @@ -949,7 +950,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
> 		}
> 	}
>
> -	if (!entry->addr.id) {
> +	if (!entry->addr.id && needs_id) {
> find_next:
> 		entry->addr.id = find_next_zero_bit(pernet->id_bitmap,
> 						    MPTCP_PM_MAX_ADDR_ID + 1,
> @@ -960,7 +961,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
> 		}
> 	}
>
> -	if (!entry->addr.id)
> +	if (!entry->addr.id && needs_id)
> 		goto out;
>
> 	__set_bit(entry->addr.id, pernet->id_bitmap);
> @@ -1092,7 +1093,7 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc
> 	entry->ifindex = 0;
> 	entry->flags = MPTCP_PM_ADDR_FLAG_IMPLICIT;
> 	entry->lsk = NULL;
> -	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
> +	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry, true);
> 	if (ret < 0)
> 		kfree(entry);
>
> @@ -1285,6 +1286,18 @@ static int mptcp_nl_add_subflow_or_signal_addr(struct net *net)
> 	return 0;
> }
>
> +static bool mptcp_pm_has_addr_attr_id(const struct nlattr *attr,
> +				      struct genl_info *info)
> +{
> +	struct nlattr *tb[MPTCP_PM_ADDR_ATTR_MAX + 1];
> +
> +	if (!nla_parse_nested_deprecated(tb, MPTCP_PM_ADDR_ATTR_MAX, attr,
> +					 mptcp_pm_address_nl_policy, info->extack) &&
> +	    tb[MPTCP_PM_ADDR_ATTR_ID])
> +		return true;
> +	return false;
> +}
> +
> int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
> {
> 	struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
> @@ -1326,7 +1339,8 @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
> 			goto out_free;
> 		}
> 	}
> -	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
> +	ret = mptcp_pm_nl_append_new_local_addr(pernet, entry,
> +						!mptcp_pm_has_addr_attr_id(attr, info));
> 	if (ret < 0) {
> 		GENL_SET_ERR_MSG_FMT(info, "too many addresses or duplicate one: %d", ret);
> 		goto out_free;
> -- 
> 2.40.1
>
>
>

>
> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Just the same as userspace PM, a new parameter needs_id is added for
> in-kernel PM mptcp_pm_nl_append_new_local_addr() too.
>
> Add a new helper mptcp_pm_has_addr_attr_id() to check whether an address
> ID is set from PM or not. It will be used in the next two commits.
>
> In mptcp_pm_nl_get_local_id(), needs_id is always true, but in
> mptcp_pm_nl_add_addr_doit(), pass mptcp_pm_has_addr_attr_id() to
> needs_it.
>
> Fixes: fd5a4c04e18 ("mptcp: add the address ID assignment bitmap")

 Sorry, should be efd5a4c04e18 here.

-Geliang

> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/pm_netlink.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index d9ad45959219..9367ab506908 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -901,7 +901,8 @@ static void __mptcp_pm_release_addr_entry(struct mptcp_pm_addr_entry *entry)
>  }
>
>  static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
> -                                            struct mptcp_pm_addr_entry *entry)
> +                                            struct mptcp_pm_addr_entry *entry,
> +                                            bool needs_id)
>  {
>         struct mptcp_pm_addr_entry *cur, *del_entry = NULL;
>         unsigned int addr_max;
> @@ -949,7 +950,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
>                 }
>         }
>
> -       if (!entry->addr.id) {
> +       if (!entry->addr.id && needs_id) {
>  find_next:
>                 entry->addr.id = find_next_zero_bit(pernet->id_bitmap,
>                                                     MPTCP_PM_MAX_ADDR_ID + 1,
> @@ -960,7 +961,7 @@ static int mptcp_pm_nl_append_new_local_addr(struct pm_nl_pernet *pernet,
>                 }
>         }
>
> -       if (!entry->addr.id)
> +       if (!entry->addr.id && needs_id)
>                 goto out;
>
>         __set_bit(entry->addr.id, pernet->id_bitmap);
> @@ -1092,7 +1093,7 @@ int mptcp_pm_nl_get_local_id(struct mptcp_sock *msk, struct mptcp_addr_info *skc
>         entry->ifindex = 0;
>         entry->flags = MPTCP_PM_ADDR_FLAG_IMPLICIT;
>         entry->lsk = NULL;
> -       ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
> +       ret = mptcp_pm_nl_append_new_local_addr(pernet, entry, true);
>         if (ret < 0)
>                 kfree(entry);
>
> @@ -1285,6 +1286,18 @@ static int mptcp_nl_add_subflow_or_signal_addr(struct net *net)
>         return 0;
>  }
>
> +static bool mptcp_pm_has_addr_attr_id(const struct nlattr *attr,
> +                                     struct genl_info *info)
> +{
> +       struct nlattr *tb[MPTCP_PM_ADDR_ATTR_MAX + 1];
> +
> +       if (!nla_parse_nested_deprecated(tb, MPTCP_PM_ADDR_ATTR_MAX, attr,
> +                                        mptcp_pm_address_nl_policy, info->extack) &&
> +           tb[MPTCP_PM_ADDR_ATTR_ID])
> +               return true;
> +       return false;
> +}
> +
>  int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
>  {
>         struct nlattr *attr = info->attrs[MPTCP_PM_ENDPOINT_ADDR];
> @@ -1326,7 +1339,8 @@ int mptcp_pm_nl_add_addr_doit(struct sk_buff *skb, struct genl_info *info)
>                         goto out_free;
>                 }
>         }
> -       ret = mptcp_pm_nl_append_new_local_addr(pernet, entry);
> +       ret = mptcp_pm_nl_append_new_local_addr(pernet, entry,
> +                                               !mptcp_pm_has_addr_attr_id(attr, info));
>         if (ret < 0) {
>                 GENL_SET_ERR_MSG_FMT(info, "too many addresses or duplicate one: %d", ret);
>                 goto out_free;
> --
> 2.40.1
>
>

Hi Geliang, Mat,

On 01/02/2024 06:35, Geliang Tang wrote:
>>
>> From: Geliang Tang <tanggeliang@kylinos.cn>
>>
>> Just the same as userspace PM, a new parameter needs_id is added for
>> in-kernel PM mptcp_pm_nl_append_new_local_addr() too.
>>
>> Add a new helper mptcp_pm_has_addr_attr_id() to check whether an address
>> ID is set from PM or not. It will be used in the next two commits.
>>
>> In mptcp_pm_nl_get_local_id(), needs_id is always true, but in
>> mptcp_pm_nl_add_addr_doit(), pass mptcp_pm_has_addr_attr_id() to
>> needs_it.
>>
>> Fixes: fd5a4c04e18 ("mptcp: add the address ID assignment bitmap")
> 
>  Sorry, should be efd5a4c04e18 here.

Thank you for these patches, and the review!

This patch and the parent one are now in our tree (fixes for -net) with
the fix for the sha, and also without "It will be used in the next two
commits." from above.

New patches for t/upstream-net and t/upstream:
- 1505f3076aa3: mptcp: add needs_id for userspace appending addr
- 073a9771f40a: mptcp: add needs_id for netlink appending addr
- Results: d93302b7258d..8f954145aaee (export-net)
- Results: c2469b38e369..8a133f7f21a3 (export)

Tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export-net/20240208T093459
https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20240208T093459

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