[PATCH] RDMA/cma: fix first byte overwritten for AF_IB

[PATCH] RDMA/cma: fix first byte overwritten for AF_IB

[Jack Wang]

If user attach private data for AF_IB, the first byte will
be overwritten, because we always set the cma version no matter
family is AF_IB, so move the version set inside if condition.

Reported-by: Fabian Holler <fabian.holler-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Signed-off-by: Jack Wang <jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
---
 drivers/infiniband/core/cma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index d570030..22a22e2 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2618,10 +2618,10 @@ static int cma_format_hdr(void *hdr, struct rdma_id_private *id_priv)
 	struct cma_hdr *cma_hdr;
 
 	cma_hdr = hdr;
-	cma_hdr->cma_version = CMA_VERSION;
 	if (cma_family(id_priv) == AF_INET) {
 		struct sockaddr_in *src4, *dst4;
 
+		cma_hdr->cma_version = CMA_VERSION;
 		src4 = (struct sockaddr_in *) cma_src_addr(id_priv);
 		dst4 = (struct sockaddr_in *) cma_dst_addr(id_priv);
 
@@ -2632,6 +2632,7 @@ static int cma_format_hdr(void *hdr, struct rdma_id_private *id_priv)
 	} else if (cma_family(id_priv) == AF_INET6) {
 		struct sockaddr_in6 *src6, *dst6;
 
+		cma_hdr->cma_version = CMA_VERSION;
 		src6 = (struct sockaddr_in6 *) cma_src_addr(id_priv);
 		dst6 = (struct sockaddr_in6 *) cma_dst_addr(id_priv);
 
-- 
1.9.1

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

RE: [PATCH] RDMA/cma: fix first byte overwritten for AF_IB

[Hefty, Sean]

> If user attach private data for AF_IB, the first byte will
> be overwritten, because we always set the cma version no matter
> family is AF_IB, so move the version set inside if condition.
> 
> Reported-by: Fabian Holler <fabian.holler-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
> Signed-off-by: Jack Wang <jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
> ---
>  drivers/infiniband/core/cma.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index d570030..22a22e2 100644
> --- a/drivers/infiniband/core/cma.c
> +++ b/drivers/infiniband/core/cma.c
> @@ -2618,10 +2618,10 @@ static int cma_format_hdr(void *hdr, struct
> rdma_id_private *id_priv)
>  	struct cma_hdr *cma_hdr;
> 
>  	cma_hdr = hdr;
> -	cma_hdr->cma_version = CMA_VERSION;
>  	if (cma_family(id_priv) == AF_INET) {
>  		struct sockaddr_in *src4, *dst4;
> 
> +		cma_hdr->cma_version = CMA_VERSION;
>  		src4 = (struct sockaddr_in *) cma_src_addr(id_priv);
>  		dst4 = (struct sockaddr_in *) cma_dst_addr(id_priv);
> 
> @@ -2632,6 +2632,7 @@ static int cma_format_hdr(void *hdr, struct
> rdma_id_private *id_priv)
>  	} else if (cma_family(id_priv) == AF_INET6) {
>  		struct sockaddr_in6 *src6, *dst6;
> 
> +		cma_hdr->cma_version = CMA_VERSION;
>  		src6 = (struct sockaddr_in6 *) cma_src_addr(id_priv);
>  		dst6 = (struct sockaddr_in6 *) cma_dst_addr(id_priv);

I don't think this is sufficient.  The RDMA CM private data header is defined by the IB spec.  If the service ID starts with the prefix 0x0000000001, it's reasonable to assume that the header is part of the private data.  The receive side should probably even check the version and discard any unknown values.

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

Re: [PATCH] RDMA/cma: fix first byte overwritten for AF_IB

[Jinpu Wang]

Hi Sean,

Thanks for looking into this.

I checked nfiniBand TM Architecture Release 1.2.1 VOLUME 1 - GENERAL
SPECIFICATIONS, for RDMA IP CM service,
the private data for CM REQ is defined in:
Table 532 IP Addressing CM REQ Message Private Data Format
So you mean even for RDMA IB, the private data format is also
required, could you point me a little more detail?



On Tue, Dec 16, 2014 at 10:29 PM, Hefty, Sean <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>> If user attach private data for AF_IB, the first byte will
>> be overwritten, because we always set the cma version no matter
>> family is AF_IB, so move the version set inside if condition.
>>
>> Reported-by: Fabian Holler <fabian.holler-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
>> Signed-off-by: Jack Wang <jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
>> ---
>>  drivers/infiniband/core/cma.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
>> index d570030..22a22e2 100644
>> --- a/drivers/infiniband/core/cma.c
>> +++ b/drivers/infiniband/core/cma.c
>> @@ -2618,10 +2618,10 @@ static int cma_format_hdr(void *hdr, struct
>> rdma_id_private *id_priv)
>>       struct cma_hdr *cma_hdr;
>>
>>       cma_hdr = hdr;
>> -     cma_hdr->cma_version = CMA_VERSION;
>>       if (cma_family(id_priv) == AF_INET) {
>>               struct sockaddr_in *src4, *dst4;
>>
>> +             cma_hdr->cma_version = CMA_VERSION;
>>               src4 = (struct sockaddr_in *) cma_src_addr(id_priv);
>>               dst4 = (struct sockaddr_in *) cma_dst_addr(id_priv);
>>
>> @@ -2632,6 +2632,7 @@ static int cma_format_hdr(void *hdr, struct
>> rdma_id_private *id_priv)
>>       } else if (cma_family(id_priv) == AF_INET6) {
>>               struct sockaddr_in6 *src6, *dst6;
>>
>> +             cma_hdr->cma_version = CMA_VERSION;
>>               src6 = (struct sockaddr_in6 *) cma_src_addr(id_priv);
>>               dst6 = (struct sockaddr_in6 *) cma_dst_addr(id_priv);
>
> I don't think this is sufficient.  The RDMA CM private data header is defined by the IB spec.  If the service ID starts with the prefix 0x0000000001, it's reasonable to assume that the header is part of the private data.  The receive side should probably even check the version and discard any unknown values.
>
> - Sean



-- 
Mit freundlichen Grüßen,
Best Regards,

Jack Wang

Linux Kernel Developer Storage
ProfitBricks GmbH  The IaaS-Company.

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin
Tel: +49 30 5770083-42
Fax: +49 30 5770085-98
Email: jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org
URL: http://www.profitbricks.de

Sitz der Gesellschaft: Berlin.
Registergericht: Amtsgericht Charlottenburg, HRB 125506 B.
Geschäftsführer: Andreas Gauger, Achim Weiss.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] RDMA/cma: fix first byte overwritten for AF_IB

[Or Gerlitz]

On 12/16/2014 5:43 PM, Jack Wang wrote:
> If user attach private data for AF_IB,

Just out of curiosity, can you share what was the motivation for you to 
use AF_IB?


Or.

> the first byte will
> be overwritten, because we always set the cma version no matter
> family is AF_IB, so move the version set inside if condition.

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

Re: [PATCH] RDMA/cma: fix first byte overwritten for AF_IB

[Jinpu Wang]

Hi Or,

We have some trouble in the past about IPoIB support about neighbour
discovery, AF_IB doesn't relay on IPoIB, so
we think have another option maybe give us more safe in some cases.



On Wed, Dec 17, 2014 at 11:40 AM, Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> On 12/16/2014 5:43 PM, Jack Wang wrote:
>>
>> If user attach private data for AF_IB,
>
>
> Just out of curiosity, can you share what was the motivation for you to use
> AF_IB?
>
>
> Or.
>
>
>> the first byte will
>> be overwritten, because we always set the cma version no matter
>> family is AF_IB, so move the version set inside if condition.
>
>



-- 
Mit freundlichen Grüßen,
Best Regards,

Jack Wang

Linux Kernel Developer Storage
ProfitBricks GmbH  The IaaS-Company.

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin
Tel: +49 30 5770083-42
Fax: +49 30 5770085-98
Email: jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org
URL: http://www.profitbricks.de

Sitz der Gesellschaft: Berlin.
Registergericht: Amtsgericht Charlottenburg, HRB 125506 B.
Geschäftsführer: Andreas Gauger, Achim Weiss.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] RDMA/cma: fix first byte overwritten for AF_IB

[Hefty, Sean]

> I checked nfiniBand TM Architecture Release 1.2.1 VOLUME 1 - GENERAL
> SPECIFICATIONS, for RDMA IP CM service,
> the private data for CM REQ is defined in:
> Table 532 IP Addressing CM REQ Message Private Data Format
> So you mean even for RDMA IB, the private data format is also
> required, could you point me a little more detail?

The RDMA IP CM service Annex in the IB spec defines a range of service IDs that apply to the RDMA CM.  If a service ID falls into this range, the private data format should also match.

For example, an app could start with AF_INET, use an alternate method to resolve the IP address to a GID (such as a user space daemon), then provide the mapped address directly to the kernel using AF_IB.  This prevents the kernel from needing to perform a duplicate address resolution.  The listening side of the connection may still be using AF_INET.  In this example, the client side is still required to format the private data as indicated in the Annex.  I.e. the original IP addressing that was used should appear as part of the private data.

The port space (which maps to the service ID) needs to be included as part of the check that determines the format of the private data, and not simply the address family.

- Sean

Re: [PATCH] RDMA/cma: fix first byte overwritten for AF_IB

[Jinpu Wang]

Hi Sean,

On Wed, Dec 17, 2014 at 6:58 PM, Hefty, Sean <sean.hefty-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>> I checked nfiniBand TM Architecture Release 1.2.1 VOLUME 1 - GENERAL
>> SPECIFICATIONS, for RDMA IP CM service,
>> the private data for CM REQ is defined in:
>> Table 532 IP Addressing CM REQ Message Private Data Format
>> So you mean even for RDMA IB, the private data format is also
>> required, could you point me a little more detail?
>
> The RDMA IP CM service Annex in the IB spec defines a range of service IDs that apply to the RDMA CM.  If a service ID falls into this range, the private data format should also match.
>
> For example, an app could start with AF_INET, use an alternate method to resolve the IP address to a GID (such as a user space daemon), then provide the mapped address directly to the kernel using AF_IB.  This prevents the kernel from needing to perform a duplicate address resolution.  The listening side of the connection may still be using AF_INET.  In this example, the client side is still required to format the private data as indicated in the Annex.  I.e. the original IP addressing that was used should appear as part of the private data.

So it's possible on server side listen using AF_INET, but client
rdma_resolve_addr/rdma_resolve_route/rdma_connect with AF_IB?

Jack


>
> The port space (which maps to the service ID) needs to be included as part of the check that determines the format of the private data, and not simply the address family.
>



> - Sean



-- 
Mit freundlichen Grüßen,
Best Regards,

Jack Wang

Linux Kernel Developer Storage
ProfitBricks GmbH  The IaaS-Company.

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin
Tel: +49 30 5770083-42
Fax: +49 30 5770085-98
Email: jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org
URL: http://www.profitbricks.de

Sitz der Gesellschaft: Berlin.
Registergericht: Amtsgericht Charlottenburg, HRB 125506 B.
Geschäftsführer: Andreas Gauger, Achim Weiss.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] RDMA/cma: fix first byte overwritten for AF_IB

[Hefty, Sean]

> So it's possible on server side listen using AF_INET, but client
> rdma_resolve_addr/rdma_resolve_route/rdma_connect with AF_IB?

Yes

[PATCH]RDMA/cma: fix first byte overwritten for AF_IB

[Jinpu Wang]

commit 56e620c453f2588cfc9898a41b110477f6417a5d
Author: Jack Wang <jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
Date:   Tue Dec 16 15:44:17 2014 +0100

    RDMA/cma: fix first byte overwritten for AF_IB

    If user attach private data for AF_IB, the first byte will
    be overwritten, because we always set the cma version no matter
    family is AF_IB, so move the version set inside if condition.

    Reported-by: Fabian Holler <fabian.holler-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>
    Signed-off-by: Jack Wang <jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org>

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index d570030..22a22e2 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -2618,10 +2618,10 @@ static int cma_format_hdr(void *hdr, struct
rdma_id_private *id_priv)
        struct cma_hdr *cma_hdr;

        cma_hdr = hdr;
-       cma_hdr->cma_version = CMA_VERSION;
        if (cma_family(id_priv) == AF_INET) {
                struct sockaddr_in *src4, *dst4;

+               cma_hdr->cma_version = CMA_VERSION;
                src4 = (struct sockaddr_in *) cma_src_addr(id_priv);
                dst4 = (struct sockaddr_in *) cma_dst_addr(id_priv);

@@ -2632,6 +2632,7 @@ static int cma_format_hdr(void *hdr, struct
rdma_id_private *id_priv)
        } else if (cma_family(id_priv) == AF_INET6) {
                struct sockaddr_in6 *src6, *dst6;

+               cma_hdr->cma_version = CMA_VERSION;
                src6 = (struct sockaddr_in6 *) cma_src_addr(id_priv);
                dst6 = (struct sockaddr_in6 *) cma_dst_addr(id_priv);



-- 
Mit freundlichen Grüßen,
Best Regards,

Jack Wang

Linux Kernel Developer Storage
ProfitBricks GmbH  The IaaS-Company.

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin
Tel: +49 30 5770083-42
Fax: +49 30 5770085-98
Email: jinpu.wang-EIkl63zCoXaH+58JC4qpiA@public.gmane.org
URL: http://www.profitbricks.de

Sitz der Gesellschaft: Berlin.
Registergericht: Amtsgericht Charlottenburg, HRB 125506 B.
Geschäftsführer: Andreas Gauger, Achim Weiss.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html