[v2] mptcp: allow changing the "backup" bit by endpoint id

[PATCH mptcp-next v2] mptcp: allow changing the "backup" bit by endpoint id

Posted by Davide Caratti 2 years, 5 months ago

a non-zero 'id' is sufficient to identify MPTCP endpoints: allow changing
the value of 'backup' bit by simply specifying the endpoint id.

Link: https://github.com/multipath-tcp/mptcp_net-next/issues/158
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---

Notes:
    v2:
     - remove poltergeist that handles id zero

 net/mptcp/pm_netlink.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 7b96be1e9f14..4ff8d55cbe82 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -1702,22 +1702,28 @@ static int mptcp_nl_addr_backup(struct net *net,
 
 static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
 {
+	struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, }, *entry;
 	struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
 	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
-	struct mptcp_pm_addr_entry addr, *entry;
 	struct net *net = sock_net(skb->sk);
-	u8 bkup = 0;
+	u8 bkup = 0, lookup_by_id = 0;
 	int ret;
 
-	ret = mptcp_pm_parse_addr(attr, info, true, &addr);
+	ret = mptcp_pm_parse_addr(attr, info, false, &addr);
 	if (ret < 0)
 		return ret;
 
 	if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
 		bkup = 1;
+	if (addr.addr.family == AF_UNSPEC) {
+		lookup_by_id = 1;
+		if (!addr.addr.id)
+			return -EOPNOTSUPP;
+	}
 
 	list_for_each_entry(entry, &pernet->local_addr_list, list) {
-		if (addresses_equal(&entry->addr, &addr.addr, true)) {
+		if ((!lookup_by_id && addresses_equal(&entry->addr, &addr.addr, true)) ||
+		    (lookup_by_id && entry->addr.id == addr.addr.id)) {
 			mptcp_nl_addr_backup(net, &entry->addr, bkup);
 
 			if (bkup)
-- 
2.31.1

Re: [PATCH mptcp-next v2] mptcp: allow changing the "backup" bit by endpoint id

Posted by Mat Martineau 2 years, 5 months ago

On Thu, 18 Nov 2021, Davide Caratti wrote:

> a non-zero 'id' is sufficient to identify MPTCP endpoints: allow changing
> the value of 'backup' bit by simply specifying the endpoint id.
>
> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/158
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
> ---
>
> Notes:
>    v2:
>     - remove poltergeist that handles id zero

Thanks Davide - this patch looks good to me. I just noticed today that I 
missed your reply to my v1 comment last Friday, sorry about that. The 
change I requested to the id 0 case was mostly about readability.


Reviewed-by: Mat Martineau <mathew.j.martineau@linux.intel.com>


>
> net/mptcp/pm_netlink.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
> index 7b96be1e9f14..4ff8d55cbe82 100644
> --- a/net/mptcp/pm_netlink.c
> +++ b/net/mptcp/pm_netlink.c
> @@ -1702,22 +1702,28 @@ static int mptcp_nl_addr_backup(struct net *net,
>
> static int mptcp_nl_cmd_set_flags(struct sk_buff *skb, struct genl_info *info)
> {
> +	struct mptcp_pm_addr_entry addr = { .addr = { .family = AF_UNSPEC }, }, *entry;
> 	struct nlattr *attr = info->attrs[MPTCP_PM_ATTR_ADDR];
> 	struct pm_nl_pernet *pernet = genl_info_pm_nl(info);
> -	struct mptcp_pm_addr_entry addr, *entry;
> 	struct net *net = sock_net(skb->sk);
> -	u8 bkup = 0;
> +	u8 bkup = 0, lookup_by_id = 0;
> 	int ret;
>
> -	ret = mptcp_pm_parse_addr(attr, info, true, &addr);
> +	ret = mptcp_pm_parse_addr(attr, info, false, &addr);
> 	if (ret < 0)
> 		return ret;
>
> 	if (addr.flags & MPTCP_PM_ADDR_FLAG_BACKUP)
> 		bkup = 1;
> +	if (addr.addr.family == AF_UNSPEC) {
> +		lookup_by_id = 1;
> +		if (!addr.addr.id)
> +			return -EOPNOTSUPP;
> +	}
>
> 	list_for_each_entry(entry, &pernet->local_addr_list, list) {
> -		if (addresses_equal(&entry->addr, &addr.addr, true)) {
> +		if ((!lookup_by_id && addresses_equal(&entry->addr, &addr.addr, true)) ||
> +		    (lookup_by_id && entry->addr.id == addr.addr.id)) {
> 			mptcp_nl_addr_backup(net, &entry->addr, bkup);
>
> 			if (bkup)
> -- 
> 2.31.1
>
>

--
Mat Martineau
Intel

Re: [PATCH mptcp-next v2] mptcp: allow changing the "backup" bit by endpoint id

Posted by Matthieu Baerts 2 years, 5 months ago

Hi Davide, Mat,

On 18/11/2021 15:00, Davide Caratti wrote:
> a non-zero 'id' is sufficient to identify MPTCP endpoints: allow changing
> the value of 'backup' bit by simply specifying the endpoint id.

Thank you for the patch and the review.

Now in our tree (feature for next) with Mat's RvB tag:

- 61df278130b4: mptcp: allow changing the "backup" bit by endpoint id
- Results: 209223221ddf..ad41cf142fb3

Builds and tests are now in progress:

https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20211119T171947
https://github.com/multipath-tcp/mptcp_net-next/actions/workflows/build-validation.yml?query=branch:export

> Link: https://github.com/multipath-tcp/mptcp_net-next/issues/158

I left the "Link" tag as the ticket cannot be closed yet.

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net