diff mbox

[for-next] RDMA/CMA: Mark IPv4 addresses correctly when the listener is IPv6

Message ID 1415893935-4583-1-git-send-email-ogerlitz@mellanox.com (mailing list archive)
State Rejected
Headers show

Commit Message

Or Gerlitz Nov. 13, 2014, 3:52 p.m. UTC 
From: Yotam Kenneth <yotamke@mellanox.com>

When accepting a new connection with the listener being IPv6, the
family of the new connection is set as IPv6. This causes cma_zero_addr
function to return true on an non-zero address. As a result, the wrong
code path is taken. This causes the connection request to be rejected,
as the RDMA-CM code looks for the wrong type of device.

Since copying the ip address is done in different function depending
on the family (cma_save_ip4_info/cma_save_ip6_info) this is fixed by
hard coding the family of the IP address according to the function.

Signed-off-by: Yotam Kenneth <yotamke@mellanox.com>
Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
---
 drivers/infiniband/core/cma.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

Comments

Hefty, Sean Nov. 13, 2014, 4:24 p.m. UTC | #1 
> From: Yotam Kenneth <yotamke@mellanox.com>
> 
> When accepting a new connection with the listener being IPv6, the
> family of the new connection is set as IPv6. This causes cma_zero_addr
> function to return true on an non-zero address. As a result, the wrong
> code path is taken. This causes the connection request to be rejected,
> as the RDMA-CM code looks for the wrong type of device.
> 
> Since copying the ip address is done in different function depending
> on the family (cma_save_ip4_info/cma_save_ip6_info) this is fixed by
> hard coding the family of the IP address according to the function.
> 
> Signed-off-by: Yotam Kenneth <yotamke@mellanox.com>
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> ---
>  drivers/infiniband/core/cma.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index d570030..6e5e11c 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -866,12 +866,12 @@ static void cma_save_ip4_info(struct rdma_cm_id *id,
> struct rdma_cm_id *listen_i
> 
>  	listen4 = (struct sockaddr_in *) &listen_id->route.addr.src_addr;
>  	ip4 = (struct sockaddr_in *) &id->route.addr.src_addr;
> -	ip4->sin_family = listen4->sin_family;
> +	ip4->sin_family = AF_INET;
>  	ip4->sin_addr.s_addr = hdr->dst_addr.ip4.addr;
>  	ip4->sin_port = listen4->sin_port;
> 
>  	ip4 = (struct sockaddr_in *) &id->route.addr.dst_addr;
> -	ip4->sin_family = listen4->sin_family;
> +	ip4->sin_family = AF_INET;
>  	ip4->sin_addr.s_addr = hdr->src_addr.ip4.addr;
>  	ip4->sin_port = hdr->port;
>  }
> @@ -883,12 +883,12 @@ static void cma_save_ip6_info(struct rdma_cm_id *id,
> struct rdma_cm_id *listen_i
> 
>  	listen6 = (struct sockaddr_in6 *) &listen_id->route.addr.src_addr;
>  	ip6 = (struct sockaddr_in6 *) &id->route.addr.src_addr;
> -	ip6->sin6_family = listen6->sin6_family;
> +	ip6->sin6_family = AF_INET6;
>  	ip6->sin6_addr = hdr->dst_addr.ip6;
>  	ip6->sin6_port = listen6->sin6_port;
> 
>  	ip6 = (struct sockaddr_in6 *) &id->route.addr.dst_addr;
> -	ip6->sin6_family = listen6->sin6_family;
> +	ip6->sin6_family = AF_INET6;
>  	ip6->sin6_addr = hdr->src_addr.ip6;
>  	ip6->sin6_port = hdr->port;

I can't say that I understand what the problem is or how the change fixes it.  Is listen4->sin_port above not AF_INET?  If that's the case, then aren't we still taking the wrong code path and just masking some bug further up in the code path?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shachar Raindel Nov. 16, 2014, 9:04 a.m. UTC | #2 
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Hefty, Sean
> Sent: Thursday, November 13, 2014 6:24 PM
> To: Or Gerlitz
> Cc: linux-rdma@vger.kernel.org; Roland Dreier; Yotam Kenneth
> Subject: RE: [PATCH for-next] RDMA/CMA: Mark IPv4 addresses correctly
> when the listener is IPv6
> 
> > From: Yotam Kenneth <yotamke@mellanox.com>
> >
> > When accepting a new connection with the listener being IPv6, the
> > family of the new connection is set as IPv6. This causes cma_zero_addr
> > function to return true on an non-zero address. As a result, the wrong
> > code path is taken. This causes the connection request to be rejected,
> > as the RDMA-CM code looks for the wrong type of device.
> >
> > Since copying the ip address is done in different function depending
> > on the family (cma_save_ip4_info/cma_save_ip6_info) this is fixed by
> > hard coding the family of the IP address according to the function.
> >
> > Signed-off-by: Yotam Kenneth <yotamke@mellanox.com>
> > Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> > ---
> >  drivers/infiniband/core/cma.c |    8 ++++----
> >  1 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/cma.c
> b/drivers/infiniband/core/cma.c
> > index d570030..6e5e11c 100644
> > --- a/drivers/infiniband/core/cma.c
> > +++ b/drivers/infiniband/core/cma.c
> > @@ -866,12 +866,12 @@ static void cma_save_ip4_info(struct rdma_cm_id
> *id,
> > struct rdma_cm_id *listen_i
> >
> >  	listen4 = (struct sockaddr_in *) &listen_id->route.addr.src_addr;
> >  	ip4 = (struct sockaddr_in *) &id->route.addr.src_addr;
> > -	ip4->sin_family = listen4->sin_family;
> > +	ip4->sin_family = AF_INET;
> >  	ip4->sin_addr.s_addr = hdr->dst_addr.ip4.addr;
> >  	ip4->sin_port = listen4->sin_port;
> >
> >  	ip4 = (struct sockaddr_in *) &id->route.addr.dst_addr;
> > -	ip4->sin_family = listen4->sin_family;
> > +	ip4->sin_family = AF_INET;
> >  	ip4->sin_addr.s_addr = hdr->src_addr.ip4.addr;
> >  	ip4->sin_port = hdr->port;
> >  }
> > @@ -883,12 +883,12 @@ static void cma_save_ip6_info(struct rdma_cm_id
> *id,
> > struct rdma_cm_id *listen_i
> >
> >  	listen6 = (struct sockaddr_in6 *) &listen_id->route.addr.src_addr;
> >  	ip6 = (struct sockaddr_in6 *) &id->route.addr.src_addr;
> > -	ip6->sin6_family = listen6->sin6_family;
> > +	ip6->sin6_family = AF_INET6;
> >  	ip6->sin6_addr = hdr->dst_addr.ip6;
> >  	ip6->sin6_port = listen6->sin6_port;
> >
> >  	ip6 = (struct sockaddr_in6 *) &id->route.addr.dst_addr;
> > -	ip6->sin6_family = listen6->sin6_family;
> > +	ip6->sin6_family = AF_INET6;
> >  	ip6->sin6_addr = hdr->src_addr.ip6;
> >  	ip6->sin6_port = hdr->port;
> 
> I can't say that I understand what the problem is or how the change
> fixes it.  Is listen4->sin_port above not AF_INET?  If that's the case,
> then aren't we still taking the wrong code path and just masking some
> bug further up in the code path?


An IPv6 listener socket can accept IPv4 connection, especially
when listening to the "any" interface. This behavior is
configurable with the net.ipv6.bindv6only sysctl flag. In
RDMA-CM, we use this flag as the baseline value for the CM-ID
flag "afonly". If afonly is set to 0, a listening RDMA-CM ID for
IPv6 any address will happily accept an IPv4 connection. The IP
addresses saved in cma_save_ip[46]_info are only used internally
in the RDMA-CM code for selecting the appropriate device for
answering and sending a reply. As such, there is no problem with
hard coding the appropriate address family in the functions.

Thanks,
--Shachar
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Or Gerlitz Dec. 16, 2014, 11:34 a.m. UTC | #3 
On 11/16/2014 11:04 AM, Shachar Raindel wrote:
>
>> -----Original Message-----
>> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
>> owner@vger.kernel.org] On Behalf Of Hefty, Sean
>> Sent: Thursday, November 13, 2014 6:24 PM
>> To: Or Gerlitz
>> Cc: linux-rdma@vger.kernel.org; Roland Dreier; Yotam Kenneth
>> Subject: RE: [PATCH for-next] RDMA/CMA: Mark IPv4 addresses correctly
>> when the listener is IPv6
>>
>>> From: Yotam Kenneth <yotamke@mellanox.com>
>>>
>>> When accepting a new connection with the listener being IPv6, the
>>> family of the new connection is set as IPv6. This causes cma_zero_addr
>>> function to return true on an non-zero address. As a result, the wrong
>>> code path is taken. This causes the connection request to be rejected,
>>> as the RDMA-CM code looks for the wrong type of device.
>>>
>>> Since copying the ip address is done in different function depending
>>> on the family (cma_save_ip4_info/cma_save_ip6_info) this is fixed by
>>> hard coding the family of the IP address according to the function.
>>>
>>> Signed-off-by: Yotam Kenneth <yotamke@mellanox.com>
>>> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
>>> ---
>>>   drivers/infiniband/core/cma.c |    8 ++++----
>>>   1 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/infiniband/core/cma.c
>> b/drivers/infiniband/core/cma.c
>>> index d570030..6e5e11c 100644
>>> --- a/drivers/infiniband/core/cma.c
>>> +++ b/drivers/infiniband/core/cma.c
>>> @@ -866,12 +866,12 @@ static void cma_save_ip4_info(struct rdma_cm_id
>> *id,
>>> struct rdma_cm_id *listen_i
>>>
>>>   	listen4 = (struct sockaddr_in *) &listen_id->route.addr.src_addr;
>>>   	ip4 = (struct sockaddr_in *) &id->route.addr.src_addr;
>>> -	ip4->sin_family = listen4->sin_family;
>>> +	ip4->sin_family = AF_INET;
>>>   	ip4->sin_addr.s_addr = hdr->dst_addr.ip4.addr;
>>>   	ip4->sin_port = listen4->sin_port;
>>>
>>>   	ip4 = (struct sockaddr_in *) &id->route.addr.dst_addr;
>>> -	ip4->sin_family = listen4->sin_family;
>>> +	ip4->sin_family = AF_INET;
>>>   	ip4->sin_addr.s_addr = hdr->src_addr.ip4.addr;
>>>   	ip4->sin_port = hdr->port;
>>>   }
>>> @@ -883,12 +883,12 @@ static void cma_save_ip6_info(struct rdma_cm_id
>> *id,
>>> struct rdma_cm_id *listen_i
>>>
>>>   	listen6 = (struct sockaddr_in6 *) &listen_id->route.addr.src_addr;
>>>   	ip6 = (struct sockaddr_in6 *) &id->route.addr.src_addr;
>>> -	ip6->sin6_family = listen6->sin6_family;
>>> +	ip6->sin6_family = AF_INET6;
>>>   	ip6->sin6_addr = hdr->dst_addr.ip6;
>>>   	ip6->sin6_port = listen6->sin6_port;
>>>
>>>   	ip6 = (struct sockaddr_in6 *) &id->route.addr.dst_addr;
>>> -	ip6->sin6_family = listen6->sin6_family;
>>> +	ip6->sin6_family = AF_INET6;
>>>   	ip6->sin6_addr = hdr->src_addr.ip6;
>>>   	ip6->sin6_port = hdr->port;
>> I can't say that I understand what the problem is or how the change
>> fixes it.  Is listen4->sin_port above not AF_INET?  If that's the case,
>> then aren't we still taking the wrong code path and just masking some
>> bug further up in the code path?
>
> An IPv6 listener socket can accept IPv4 connection, especially
> when listening to the "any" interface. This behavior is
> configurable with the net.ipv6.bindv6only sysctl flag. In
> RDMA-CM, we use this flag as the baseline value for the CM-ID
> flag "afonly". If afonly is set to 0, a listening RDMA-CM ID for
> IPv6 any address will happily accept an IPv4 connection. The IP
> addresses saved in cma_save_ip[46]_info are only used internally
> in the RDMA-CM code for selecting the appropriate device for
> answering and sending a reply. As such, there is no problem with
> hard coding the appropriate address family in the functions.
>
>

Hi Sean,

Any comment? can you please ack the patch or continue the discussion?


Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hefty, Sean Dec. 16, 2014, 6:38 p.m. UTC | #4 
> Any comment? can you please ack the patch or continue the discussion?

My concerns were addressed, so I have no further issues.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Or Gerlitz Dec. 17, 2014, 4:22 a.m. UTC | #5 
On 12/16/2014 8:38 PM, Hefty, Sean wrote:
>> Any comment? can you please ack the patch or continue the discussion?
> My concerns were addressed, so I have no further issues.

OK, thanks, Roland, can you please move fwd and pick this one for the 
merge window too?

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index d570030..6e5e11c 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -866,12 +866,12 @@  static void cma_save_ip4_info(struct rdma_cm_id *id, struct rdma_cm_id *listen_i
 
 	listen4 = (struct sockaddr_in *) &listen_id->route.addr.src_addr;
 	ip4 = (struct sockaddr_in *) &id->route.addr.src_addr;
-	ip4->sin_family = listen4->sin_family;
+	ip4->sin_family = AF_INET;
 	ip4->sin_addr.s_addr = hdr->dst_addr.ip4.addr;
 	ip4->sin_port = listen4->sin_port;
 
 	ip4 = (struct sockaddr_in *) &id->route.addr.dst_addr;
-	ip4->sin_family = listen4->sin_family;
+	ip4->sin_family = AF_INET;
 	ip4->sin_addr.s_addr = hdr->src_addr.ip4.addr;
 	ip4->sin_port = hdr->port;
 }
@@ -883,12 +883,12 @@  static void cma_save_ip6_info(struct rdma_cm_id *id, struct rdma_cm_id *listen_i
 
 	listen6 = (struct sockaddr_in6 *) &listen_id->route.addr.src_addr;
 	ip6 = (struct sockaddr_in6 *) &id->route.addr.src_addr;
-	ip6->sin6_family = listen6->sin6_family;
+	ip6->sin6_family = AF_INET6;
 	ip6->sin6_addr = hdr->dst_addr.ip6;
 	ip6->sin6_port = listen6->sin6_port;
 
 	ip6 = (struct sockaddr_in6 *) &id->route.addr.dst_addr;
-	ip6->sin6_family = listen6->sin6_family;
+	ip6->sin6_family = AF_INET6;
 	ip6->sin6_addr = hdr->src_addr.ip6;
 	ip6->sin6_port = hdr->port;
 }