[PATCH mptcp-net 0/4] fixes for userspace PM

[PATCH mptcp-net 0/4] fixes for userspace PM

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Fixes for "create an ID 0 subflow" and "map v4 address to v6".

Geliang Tang (4):
  mptcp: add needs_id for userspace appending addr
  mptcp: add needs_id for netlink appending addr
  mptcp: map v4 address to v6 when destroying subflow
  selftests: mptcp: rm subflow with v4/v4mapped addr

 net/mptcp/pm_netlink.c                        | 24 +++++++++++++++----
 net/mptcp/pm_userspace.c                      | 23 +++++++++++++-----
 .../testing/selftests/net/mptcp/mptcp_join.sh | 11 ++++++---
 3 files changed, 44 insertions(+), 14 deletions(-)

-- 
2.40.1

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

[Geliang Tang]

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

[Mat Martineau]

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

[PATCH mptcp-net 2/4] mptcp: add needs_id for netlink appending addr

[Geliang Tang]

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

Re: [PATCH mptcp-net 2/4] mptcp: add needs_id for netlink appending addr

[Geliang Tang]

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

Re: [PATCH mptcp-net 2/4] mptcp: add needs_id for netlink appending addr

[Matthieu Baerts]

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.

Re: [PATCH mptcp-net 2/4] mptcp: add needs_id for netlink appending addr

[Mat Martineau]

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

[PATCH mptcp-net 3/4] mptcp: map v4 address to v6 when destroying subflow

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Address family of server side mismatches with that of client side, like
in "userspace pm add & remove address" test:

    userspace_pm_add_addr $ns1 10.0.2.1 10
    userspace_pm_rm_sf $ns1 "::ffff:10.0.2.1" $SUB_ESTABLISHED

That's because on the server side, the family is set to AF_INET6 and the
v4 address is mapped in a v6 one.

This patch fixes this issue. In mptcp_pm_nl_subflow_destroy_doit(), before
checking local address family with remote address family, map an IPv4
address to an IPv6 address if the pair is a v4-mapped address.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/387
Fixes: 702c2f646d42 ("mptcp: netlink: allow userspace-driven subflow establishment")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/mptcp/pm_userspace.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index e582b3b2d174..b40a69649fe1 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -495,6 +495,16 @@ int mptcp_pm_nl_subflow_destroy_doit(struct sk_buff *skb, struct genl_info *info
 		goto destroy_err;
 	}
 
+#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_r.family == AF_INET && ipv6_addr_v4mapped(&addr_l.addr6)) {
+		ipv6_addr_set_v4mapped(addr_r.addr.s_addr, &addr_r.addr6);
+		addr_r.family = AF_INET6;
+	}
+#endif
 	if (addr_l.family != addr_r.family) {
 		GENL_SET_ERR_MSG(info, "address families do not match");
 		err = -EINVAL;
-- 
2.40.1

[PATCH mptcp-net 4/4] selftests: mptcp: rm subflow with v4/v4mapped addr

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

Now both a v4 address and a v4-mapped address are supported when
destroying a userspace pm subflow, this patch adds random tests for both
addresses.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/387
Fixes: 48d73f609dcc ("selftests: mptcp: update userspace pm addr tests")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 tools/testing/selftests/net/mptcp/mptcp_join.sh | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
index c07386e21e0a..a3bdbc896c6f 100755
--- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
@@ -3333,12 +3333,13 @@ userspace_pm_rm_sf()
 {
 	local evts=$evts_ns1
 	local t=${3:-1}
-	local ip=4
+	local ip
 	local tk da dp sp
 	local cnt
 
 	[ "$1" == "$ns2" ] && evts=$evts_ns2
-	if mptcp_lib_is_v6 $2; then ip=6; fi
+	[ -n "$(mptcp_lib_evts_get_info "saddr4" "$evts" $t)" ] && ip=4
+	[ -n "$(mptcp_lib_evts_get_info "saddr6" "$evts" $t)" ] && ip=6
 	tk=$(mptcp_lib_evts_get_info token "$evts")
 	da=$(mptcp_lib_evts_get_info "daddr$ip" "$evts" $t)
 	dp=$(mptcp_lib_evts_get_info dport "$evts" $t)
@@ -3441,7 +3442,11 @@ userspace_tests()
 		chk_subflows_total 2 2
 		chk_mptcp_info add_addr_signal 1 add_addr_accepted 1
 		userspace_pm_rm_addr $ns1 10
-		userspace_pm_rm_sf $ns1 "::ffff:10.0.2.1" $SUB_ESTABLISHED
+		if [ $((RANDOM%2)) -eq 0 ]; then
+			userspace_pm_rm_sf $ns1 ::ffff:10.0.2.1 $SUB_ESTABLISHED
+		else
+			userspace_pm_rm_sf $ns1 10.0.2.1 $SUB_ESTABLISHED
+		fi
 		chk_rm_nr 1 1 invert
 		chk_mptcp_info subflows 0 subflows 0
 		chk_subflows_total 1 1
-- 
2.40.1

Re: selftests: mptcp: rm subflow with v4/v4mapped addr: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7736093723

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/4625d77bcf04


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: selftests: mptcp: rm subflow with v4/v4mapped addr: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/6118213113610240
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/6118213113610240/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5555263160188928
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5555263160188928/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/4625d77bcf04


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

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-net 4/4] selftests: mptcp: rm subflow with v4/v4mapped addr

[Mat Martineau]

On Thu, 1 Feb 2024, Geliang Tang wrote:

> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> Now both a v4 address and a v4-mapped address are supported when
> destroying a userspace pm subflow, this patch adds random tests for both
> addresses.
>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/387
> Fixes: 48d73f609dcc ("selftests: mptcp: update userspace pm addr tests")
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> tools/testing/selftests/net/mptcp/mptcp_join.sh | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> index c07386e21e0a..a3bdbc896c6f 100755
> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> @@ -3333,12 +3333,13 @@ userspace_pm_rm_sf()
> {
> 	local evts=$evts_ns1
> 	local t=${3:-1}
> -	local ip=4
> +	local ip
> 	local tk da dp sp
> 	local cnt
>
> 	[ "$1" == "$ns2" ] && evts=$evts_ns2
> -	if mptcp_lib_is_v6 $2; then ip=6; fi
> +	[ -n "$(mptcp_lib_evts_get_info "saddr4" "$evts" $t)" ] && ip=4
> +	[ -n "$(mptcp_lib_evts_get_info "saddr6" "$evts" $t)" ] && ip=6
> 	tk=$(mptcp_lib_evts_get_info token "$evts")
> 	da=$(mptcp_lib_evts_get_info "daddr$ip" "$evts" $t)
> 	dp=$(mptcp_lib_evts_get_info dport "$evts" $t)
> @@ -3441,7 +3442,11 @@ userspace_tests()
> 		chk_subflows_total 2 2
> 		chk_mptcp_info add_addr_signal 1 add_addr_accepted 1
> 		userspace_pm_rm_addr $ns1 10
> -		userspace_pm_rm_sf $ns1 "::ffff:10.0.2.1" $SUB_ESTABLISHED
> +		if [ $((RANDOM%2)) -eq 0 ]; then
> +			userspace_pm_rm_sf $ns1 ::ffff:10.0.2.1 $SUB_ESTABLISHED
> +		else
> +			userspace_pm_rm_sf $ns1 10.0.2.1 $SUB_ESTABLISHED
> +		fi

Instead of randomizing this test, does it work to add a second subflow to 
this test case?

Then the two subflows could be removed two different ways (one with the 
v4mapped and one with v4)? That should avoid adding too much test time.

- Mat


> 		chk_rm_nr 1 1 invert
> 		chk_mptcp_info subflows 0 subflows 0
> 		chk_subflows_total 1 1
> -- 
> 2.40.1
>
>
>

Re: [PATCH mptcp-net 4/4] selftests: mptcp: rm subflow with v4/v4mapped addr

[Geliang Tang]

Hi Mat,

On Mon, Feb 05, 2024 at 05:40:00PM -0800, Mat Martineau wrote:
> On Thu, 1 Feb 2024, Geliang Tang wrote:
> 
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > Now both a v4 address and a v4-mapped address are supported when
> > destroying a userspace pm subflow, this patch adds random tests for both
> > addresses.
> > 
> > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/387
> > Fixes: 48d73f609dcc ("selftests: mptcp: update userspace pm addr tests")
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> > tools/testing/selftests/net/mptcp/mptcp_join.sh | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > index c07386e21e0a..a3bdbc896c6f 100755
> > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
> > @@ -3333,12 +3333,13 @@ userspace_pm_rm_sf()
> > {
> > 	local evts=$evts_ns1
> > 	local t=${3:-1}
> > -	local ip=4
> > +	local ip
> > 	local tk da dp sp
> > 	local cnt
> > 
> > 	[ "$1" == "$ns2" ] && evts=$evts_ns2
> > -	if mptcp_lib_is_v6 $2; then ip=6; fi
> > +	[ -n "$(mptcp_lib_evts_get_info "saddr4" "$evts" $t)" ] && ip=4
> > +	[ -n "$(mptcp_lib_evts_get_info "saddr6" "$evts" $t)" ] && ip=6
> > 	tk=$(mptcp_lib_evts_get_info token "$evts")
> > 	da=$(mptcp_lib_evts_get_info "daddr$ip" "$evts" $t)
> > 	dp=$(mptcp_lib_evts_get_info dport "$evts" $t)
> > @@ -3441,7 +3442,11 @@ userspace_tests()
> > 		chk_subflows_total 2 2
> > 		chk_mptcp_info add_addr_signal 1 add_addr_accepted 1
> > 		userspace_pm_rm_addr $ns1 10
> > -		userspace_pm_rm_sf $ns1 "::ffff:10.0.2.1" $SUB_ESTABLISHED
> > +		if [ $((RANDOM%2)) -eq 0 ]; then
> > +			userspace_pm_rm_sf $ns1 ::ffff:10.0.2.1 $SUB_ESTABLISHED
> > +		else
> > +			userspace_pm_rm_sf $ns1 10.0.2.1 $SUB_ESTABLISHED
> > +		fi
> 
> Instead of randomizing this test, does it work to add a second subflow to
> this test case?

Yes, it works.

> 
> Then the two subflows could be removed two different ways (one with the
> v4mapped and one with v4)? That should avoid adding too much test time.

I just sent a v2 for this with only patch 3 and patch 4 in it.

Thanks,
-Geliang

> 
> - Mat
> 
> 
> > 		chk_rm_nr 1 1 invert
> > 		chk_mptcp_info subflows 0 subflows 0
> > 		chk_subflows_total 1 1
> > -- 
> > 2.40.1
> > 
> > 
> > 

Re: selftests: mptcp: rm subflow with v4/v4mapped addr: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI (GitHub Action) did some validations and here is its report:

- KVM Validation: normal:
  - Success! ✅:
  - Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/7793464637

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/483bc6889d53


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: selftests: mptcp: rm subflow with v4/v4mapped addr: Tests Results

[MPTCP CI]

Hi Geliang,

Thank you for your modifications, that's great!

Our CI (Cirrus) did some validations with a debug kernel and here is its report:

- KVM Validation: debug (except selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5780927813517312
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5780927813517312/summary/summary.txt

- KVM Validation: debug (only selftest_mptcp_join):
  - Success! ✅:
  - Task: https://cirrus-ci.com/task/5217977860096000
  - Summary: https://api.cirrus-ci.com/v1/artifact/task/5217977860096000/summary/summary.txt

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/483bc6889d53


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

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)