[PATCH rdma-rc v1] xprtrdma: Don't require LOCAL_DMA_LKEY support for fasterg

[PATCH rdma-rc v1] xprtrdma: Don't require LOCAL_DMA_LKEY support for fasterg

[Sagi Grimberg]

There is no need to require LOCAL_DMA_LKEY support as the
PD allocation makes sure that there is a local_dma_lkey. Also
correctly set a return value in error path.

This caused a NULL pointer dereference in mlx5 which removed
the support for LOCAL_DMA_LKEY.

Fixes: bb6c96d72879 ("xprtrdma: Replace global lkey with lkey local to PD")
Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Reviewed-by: Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
---
Changes from v0:
- Added error path rc assignment.

 net/sunrpc/xprtrdma/verbs.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
index eb081ad..81e8d31 100644
--- a/net/sunrpc/xprtrdma/verbs.c
+++ b/net/sunrpc/xprtrdma/verbs.c
@@ -543,11 +543,8 @@ rpcrdma_ia_open(struct rpcrdma_xprt *xprt, struct sockaddr *addr, int memreg)
 	}
 
 	if (memreg == RPCRDMA_FRMR) {
-		/* Requires both frmr reg and local dma lkey */
-		if (((devattr->device_cap_flags &
-		     (IB_DEVICE_MEM_MGT_EXTENSIONS|IB_DEVICE_LOCAL_DMA_LKEY)) !=
-		    (IB_DEVICE_MEM_MGT_EXTENSIONS|IB_DEVICE_LOCAL_DMA_LKEY)) ||
-		      (devattr->max_fast_reg_page_list_len == 0)) {
+		if (!(devattr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) ||
+		    (devattr->max_fast_reg_page_list_len == 0)) {
 			dprintk("RPC:       %s: FRMR registration "
 				"not supported by HCA\n", __func__);
 			memreg = RPCRDMA_MTHCAFMR;
@@ -557,6 +554,7 @@ rpcrdma_ia_open(struct rpcrdma_xprt *xprt, struct sockaddr *addr, int memreg)
 		if (!ia->ri_device->alloc_fmr) {
 			dprintk("RPC:       %s: MTHCAFMR registration "
 				"not supported by HCA\n", __func__);
+			rc = -EINVAL;
 			goto out3;
 		}
 	}
-- 
1.7.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-rc v1] xprtrdma: Don't require LOCAL_DMA_LKEY support for fasterg

[Anna Schumaker]

Hi Sagi,

On 10/06/2015 12:52 PM, Sagi Grimberg wrote:
> There is no need to require LOCAL_DMA_LKEY support as the
> PD allocation makes sure that there is a local_dma_lkey. Also
> correctly set a return value in error path.
> 
> This caused a NULL pointer dereference in mlx5 which removed
> the support for LOCAL_DMA_LKEY.

Looks good to me!  This is another patch going directly to an RDMA tree somewhere, right?

Anna

> 
> Fixes: bb6c96d72879 ("xprtrdma: Replace global lkey with lkey local to PD")
> Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
> Reviewed-by: Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> ---
> Changes from v0:
> - Added error path rc assignment.
> 
>  net/sunrpc/xprtrdma/verbs.c |    8 +++-----
>  1 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index eb081ad..81e8d31 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -543,11 +543,8 @@ rpcrdma_ia_open(struct rpcrdma_xprt *xprt, struct sockaddr *addr, int memreg)
>  	}
>  
>  	if (memreg == RPCRDMA_FRMR) {
> -		/* Requires both frmr reg and local dma lkey */
> -		if (((devattr->device_cap_flags &
> -		     (IB_DEVICE_MEM_MGT_EXTENSIONS|IB_DEVICE_LOCAL_DMA_LKEY)) !=
> -		    (IB_DEVICE_MEM_MGT_EXTENSIONS|IB_DEVICE_LOCAL_DMA_LKEY)) ||
> -		      (devattr->max_fast_reg_page_list_len == 0)) {
> +		if (!(devattr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) ||
> +		    (devattr->max_fast_reg_page_list_len == 0)) {
>  			dprintk("RPC:       %s: FRMR registration "
>  				"not supported by HCA\n", __func__);
>  			memreg = RPCRDMA_MTHCAFMR;
> @@ -557,6 +554,7 @@ rpcrdma_ia_open(struct rpcrdma_xprt *xprt, struct sockaddr *addr, int memreg)
>  		if (!ia->ri_device->alloc_fmr) {
>  			dprintk("RPC:       %s: MTHCAFMR registration "
>  				"not supported by HCA\n", __func__);
> +			rc = -EINVAL;
>  			goto out3;
>  		}
>  	}
> 

--
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-rc v1] xprtrdma: Don't require LOCAL_DMA_LKEY support for fasterg

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 2527 bytes --]

On 10/06/2015 01:25 PM, Anna Schumaker wrote:
> Hi Sagi,
> 
> On 10/06/2015 12:52 PM, Sagi Grimberg wrote:
>> There is no need to require LOCAL_DMA_LKEY support as the
>> PD allocation makes sure that there is a local_dma_lkey. Also
>> correctly set a return value in error path.
>>
>> This caused a NULL pointer dereference in mlx5 which removed
>> the support for LOCAL_DMA_LKEY.
> 
> Looks good to me!  This is another patch going directly to an RDMA tree somewhere, right?

Yeah, I'll pick this one up.  Thanks!

> Anna
> 
>>
>> Fixes: bb6c96d72879 ("xprtrdma: Replace global lkey with lkey local to PD")
>> Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>> Reviewed-by: Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>> ---
>> Changes from v0:
>> - Added error path rc assignment.
>>
>>  net/sunrpc/xprtrdma/verbs.c |    8 +++-----
>>  1 files changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
>> index eb081ad..81e8d31 100644
>> --- a/net/sunrpc/xprtrdma/verbs.c
>> +++ b/net/sunrpc/xprtrdma/verbs.c
>> @@ -543,11 +543,8 @@ rpcrdma_ia_open(struct rpcrdma_xprt *xprt, struct sockaddr *addr, int memreg)
>>  	}
>>  
>>  	if (memreg == RPCRDMA_FRMR) {
>> -		/* Requires both frmr reg and local dma lkey */
>> -		if (((devattr->device_cap_flags &
>> -		     (IB_DEVICE_MEM_MGT_EXTENSIONS|IB_DEVICE_LOCAL_DMA_LKEY)) !=
>> -		    (IB_DEVICE_MEM_MGT_EXTENSIONS|IB_DEVICE_LOCAL_DMA_LKEY)) ||
>> -		      (devattr->max_fast_reg_page_list_len == 0)) {
>> +		if (!(devattr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) ||
>> +		    (devattr->max_fast_reg_page_list_len == 0)) {
>>  			dprintk("RPC:       %s: FRMR registration "
>>  				"not supported by HCA\n", __func__);
>>  			memreg = RPCRDMA_MTHCAFMR;
>> @@ -557,6 +554,7 @@ rpcrdma_ia_open(struct rpcrdma_xprt *xprt, struct sockaddr *addr, int memreg)
>>  		if (!ia->ri_device->alloc_fmr) {
>>  			dprintk("RPC:       %s: MTHCAFMR registration "
>>  				"not supported by HCA\n", __func__);
>> +			rc = -EINVAL;
>>  			goto out3;
>>  		}
>>  	}
>>
> 
> --
> 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
> 


-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]

Re: [PATCH rdma-rc v1] xprtrdma: Don't require LOCAL_DMA_LKEY support for fasterg

[Anna Schumaker]

On 10/06/2015 01:36 PM, Doug Ledford wrote:
> On 10/06/2015 01:25 PM, Anna Schumaker wrote:
>> Hi Sagi,
>>
>> On 10/06/2015 12:52 PM, Sagi Grimberg wrote:
>>> There is no need to require LOCAL_DMA_LKEY support as the
>>> PD allocation makes sure that there is a local_dma_lkey. Also
>>> correctly set a return value in error path.
>>>
>>> This caused a NULL pointer dereference in mlx5 which removed
>>> the support for LOCAL_DMA_LKEY.
>>
>> Looks good to me!  This is another patch going directly to an RDMA tree somewhere, right?
> 
> Yeah, I'll pick this one up.  Thanks!

Cool!  In that case you can add:

Acked-by: Anna Schumaker <Anna.Schumaker-ZwjVKphTwtPQT0dZR+AlfA@public.gmane.org>

> 
>> Anna
>>
>>>
>>> Fixes: bb6c96d72879 ("xprtrdma: Replace global lkey with lkey local to PD")
>>> Signed-off-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>> Reviewed-by: Chuck Lever <chuck.lever-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
>>> ---
>>> Changes from v0:
>>> - Added error path rc assignment.
>>>
>>>  net/sunrpc/xprtrdma/verbs.c |    8 +++-----
>>>  1 files changed, 3 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
>>> index eb081ad..81e8d31 100644
>>> --- a/net/sunrpc/xprtrdma/verbs.c
>>> +++ b/net/sunrpc/xprtrdma/verbs.c
>>> @@ -543,11 +543,8 @@ rpcrdma_ia_open(struct rpcrdma_xprt *xprt, struct sockaddr *addr, int memreg)
>>>  	}
>>>  
>>>  	if (memreg == RPCRDMA_FRMR) {
>>> -		/* Requires both frmr reg and local dma lkey */
>>> -		if (((devattr->device_cap_flags &
>>> -		     (IB_DEVICE_MEM_MGT_EXTENSIONS|IB_DEVICE_LOCAL_DMA_LKEY)) !=
>>> -		    (IB_DEVICE_MEM_MGT_EXTENSIONS|IB_DEVICE_LOCAL_DMA_LKEY)) ||
>>> -		      (devattr->max_fast_reg_page_list_len == 0)) {
>>> +		if (!(devattr->device_cap_flags & IB_DEVICE_MEM_MGT_EXTENSIONS) ||
>>> +		    (devattr->max_fast_reg_page_list_len == 0)) {
>>>  			dprintk("RPC:       %s: FRMR registration "
>>>  				"not supported by HCA\n", __func__);
>>>  			memreg = RPCRDMA_MTHCAFMR;
>>> @@ -557,6 +554,7 @@ rpcrdma_ia_open(struct rpcrdma_xprt *xprt, struct sockaddr *addr, int memreg)
>>>  		if (!ia->ri_device->alloc_fmr) {
>>>  			dprintk("RPC:       %s: MTHCAFMR registration "
>>>  				"not supported by HCA\n", __func__);
>>> +			rc = -EINVAL;
>>>  			goto out3;
>>>  		}
>>>  	}
>>>
>>
>> --
>> 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
>>
> 
> 

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