diff mbox series

Re: [MPTCP][PATCH v2 mptcp-next 1/2] mptcp: convert IPv4 address to IPv4-mapped

Message ID d3cbdcc0-4471-7a4d-1b1e-d69be80d231c@tessares.net
State RFC, archived
Headers show
Series Re: [MPTCP][PATCH v2 mptcp-next 1/2] mptcp: convert IPv4 address to IPv4-mapped | expand

Commit Message

Matthieu Baerts Dec. 17, 2020, 11:06 p.m. UTC 
Hi Geliang, Mat,

On 17/12/2020 01:23, Mat Martineau wrote:
> On Wed, 16 Dec 2020, Geliang Tang wrote:
> 
>> Currently, PM doesn't create subflow with IPv4-mapped IPv6 socket. This
>> patch converts the IPv4 address to IPv4-mapped IPv6 address to fix it.
>>
>> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
>> ---
>> net/mptcp/pm_netlink.c | 25 ++++++++++++++++++++++---
>> 1 file changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>> index cc4ca013a06b..987e83cdee11 100644
>> --- a/net/mptcp/pm_netlink.c
>> +++ b/net/mptcp/pm_netlink.c
>> @@ -145,6 +158,8 @@ select_local_address(const struct pm_nl_pernet 
>> *pernet,
>>         if (!(entry->addr.flags & MPTCP_PM_ADDR_FLAG_SUBFLOW))
>>             continue;
>>
>> +        mptcp_pm_addr_convert_v4mapped(msk, &entry->addr);
>> +
> 
> This is modifying the contents of pernet->local_addr_list, not a copy of 
> the entry, when the msk has a v4mapped address. I'm not sure that's the 
> way to fix this, and might leave some corner cases unaddressed.
> 
> Did you look at modifying addresses_equal() so it can detect equivalent 
> IPv4 and v4mapped addresses?

Maybe something like that the patch I joined?

I didn't try much but maybe more what we want?

Cheers,
Matt

Comments

Mat Martineau Dec. 17, 2020, 11:16 p.m. UTC | #1 
On Fri, 18 Dec 2020, Matthieu Baerts wrote:

> Hi Geliang, Mat,
>
> On 17/12/2020 01:23, Mat Martineau wrote:
>> On Wed, 16 Dec 2020, Geliang Tang wrote:
>> 
>>> Currently, PM doesn't create subflow with IPv4-mapped IPv6 socket. This
>>> patch converts the IPv4 address to IPv4-mapped IPv6 address to fix it.
>>> 
>>> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
>>> ---
>>> net/mptcp/pm_netlink.c | 25 ++++++++++++++++++++++---
>>> 1 file changed, 22 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
>>> index cc4ca013a06b..987e83cdee11 100644
>>> --- a/net/mptcp/pm_netlink.c
>>> +++ b/net/mptcp/pm_netlink.c
>>> @@ -145,6 +158,8 @@ select_local_address(const struct pm_nl_pernet 
>>> *pernet,
>>>         if (!(entry->addr.flags & MPTCP_PM_ADDR_FLAG_SUBFLOW))
>>>             continue;
>>> 
>>> +        mptcp_pm_addr_convert_v4mapped(msk, &entry->addr);
>>> +
>> 
>> This is modifying the contents of pernet->local_addr_list, not a copy of 
>> the entry, when the msk has a v4mapped address. I'm not sure that's the way 
>> to fix this, and might leave some corner cases unaddressed.
>> 
>> Did you look at modifying addresses_equal() so it can detect equivalent 
>> IPv4 and v4mapped addresses?
>
> Maybe something like that the patch I joined?
>
> I didn't try much but maybe more what we want?
>

I haven't tested it out, but yes, that's what I had in mind.

--
Mat Martineau
Intel
diff mbox series

Patch

From 36f5e1f9bade5329e82a85fb6330db57a165c04c Mon Sep 17 00:00:00 2001
From: Matthieu Baerts <matthieu.baerts@tessares.net>
Date: Fri, 18 Dec 2020 00:03:12 +0100
Subject: [PATCH] mptcp: support v4mappedv6

Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 net/mptcp/pm_netlink.c | 22 +++++++++++++---------
 net/mptcp/subflow.c    | 17 +++++++++++------
 2 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/net/mptcp/pm_netlink.c b/net/mptcp/pm_netlink.c
index 9b1f6298bbdb..6c89a9e0099b 100644
--- a/net/mptcp/pm_netlink.c
+++ b/net/mptcp/pm_netlink.c
@@ -60,15 +60,20 @@  static bool addresses_equal(const struct mptcp_addr_info *a,
 {
 	bool addr_equals = false;
 
-	if (a->family != b->family)
-		return false;
-
-	if (a->family == AF_INET)
-		addr_equals = a->addr.s_addr == b->addr.s_addr;
+	if (a->family == b->family) {
+		if (a->family == AF_INET)
+			addr_equals = a->addr.s_addr == b->addr.s_addr;
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
-	else
-		addr_equals = !ipv6_addr_cmp(&a->addr6, &b->addr6);
+		else
+			addr_equals = !ipv6_addr_cmp(&a->addr6, &b->addr6);
+	} else if (a->family == AF_INET6) {
+		if (b->family == AF_INET && ipv6_addr_v4mapped(&a->addr6))
+			addr_equals = a->addr6.s6_addr32[3] == b->addr.s_addr;
+	} else if (b->family == AF_INET6) {
+		if (a->family == AF_INET && ipv6_addr_v4mapped(&b->addr6))
+			addr_equals = a->addr.s_addr == b->addr6.s6_addr32[3];
 #endif
+	}
 
 	if (!addr_equals)
 		return false;
@@ -147,8 +152,7 @@  select_local_address(const struct pm_nl_pernet *pernet,
 		/* avoid any address already in use by subflows and
 		 * pending join
 		 */
-		if (entry->addr.family == ((struct sock *)msk)->sk_family &&
-		    !lookup_subflow_by_saddr(&msk->conn_list, &entry->addr)) {
+		if (!lookup_subflow_by_saddr(&msk->conn_list, &entry->addr)) {
 			ret = entry;
 			break;
 		}
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 278cbe3e539e..11cc0ef3ae3c 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1074,10 +1074,11 @@  void mptcpv6_handle_mapped(struct sock *sk, bool mapped)
 #endif
 
 static void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
-				struct sockaddr_storage *addr)
+				struct sockaddr_storage *addr,
+				unsigned short family)
 {
 	memset(addr, 0, sizeof(*addr));
-	addr->ss_family = info->family;
+	addr->ss_family = family;
 	if (addr->ss_family == AF_INET) {
 		struct sockaddr_in *in_addr = (struct sockaddr_in *)addr;
 
@@ -1088,7 +1089,11 @@  static void mptcp_info2sockaddr(const struct mptcp_addr_info *info,
 	else if (addr->ss_family == AF_INET6) {
 		struct sockaddr_in6 *in6_addr = (struct sockaddr_in6 *)addr;
 
-		in6_addr->sin6_addr = info->addr6;
+		if (info->family == AF_INET)
+			ipv6_addr_set_v4mapped(info->addr.s_addr,
+					       &in6_addr->sin6_addr);
+		else
+			in6_addr->sin6_addr = info->addr6;
 		in6_addr->sin6_port = info->port;
 	}
 #endif
@@ -1132,11 +1137,11 @@  int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
 	subflow->remote_key = msk->remote_key;
 	subflow->local_key = msk->local_key;
 	subflow->token = msk->token;
-	mptcp_info2sockaddr(loc, &addr);
+	mptcp_info2sockaddr(loc, &addr, ssk->sk_family);
 
 	addrlen = sizeof(struct sockaddr_in);
 #if IS_ENABLED(CONFIG_MPTCP_IPV6)
-	if (loc->family == AF_INET6)
+	if (addr.ss_family == AF_INET6)
 		addrlen = sizeof(struct sockaddr_in6);
 #endif
 	ssk->sk_bound_dev_if = loc->ifindex;
@@ -1152,7 +1157,7 @@  int __mptcp_subflow_connect(struct sock *sk, const struct mptcp_addr_info *loc,
 	subflow->remote_id = remote_id;
 	subflow->request_join = 1;
 	subflow->request_bkup = !!(loc->flags & MPTCP_PM_ADDR_FLAG_BACKUP);
-	mptcp_info2sockaddr(remote, &addr);
+	mptcp_info2sockaddr(remote, &addr, ssk->sk_family);
 
 	mptcp_add_pending_subflow(msk, subflow);
 	err = kernel_connect(sf, (struct sockaddr *)&addr, addrlen, O_NONBLOCK);
-- 
2.29.2