[PATCH] RDMA/rxe: fix retry forever when rnr_retry >= 7

[PATCH] RDMA/rxe: fix retry forever when rnr_retry >= 7

[Fan Yang]

Currently when an error occurs and the completion state becomes
COMPST_RNR_RETRY, qp->comp.rnr_retry is only decreased when
qp->comp.rnr_retry != 7.

If the user happens to config the rnr retry count to be >= 7, the
driver will retry forever, instead of exposing IB_WC_RNR_RETRY_EXC_ERR.

---
 drivers/infiniband/sw/rxe/rxe_comp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
index 4bc88708b355..16c1870b6482 100644
--- a/drivers/infiniband/sw/rxe/rxe_comp.c
+++ b/drivers/infiniband/sw/rxe/rxe_comp.c
@@ -745,8 +745,7 @@ int rxe_completer(void *arg)
 
 		case COMPST_RNR_RETRY:
 			if (qp->comp.rnr_retry > 0) {
-				if (qp->comp.rnr_retry != 7)
-					qp->comp.rnr_retry--;
+				qp->comp.rnr_retry--;
 
 				qp->req.need_retry = 1;
 				pr_debug("qp#%d set rnr nak timer\n",
-- 
2.27.0

Re: [PATCH] RDMA/rxe: fix retry forever when rnr_retry >= 7

[Zhu Yanjun]

On Thu, Jul 23, 2020 at 9:28 PM Fan Yang <yangfan.fan@bytedance.com> wrote:
>
> Currently when an error occurs and the completion state becomes
> COMPST_RNR_RETRY, qp->comp.rnr_retry is only decreased when
> qp->comp.rnr_retry != 7.
>
> If the user happens to config the rnr retry count to be >= 7, the
> driver will retry forever, instead of exposing IB_WC_RNR_RETRY_EXC_ERR.

Please read the following from IB specification

"

The RNR NAK retry counter is decremented each time the responder returns

an RNR NAK. If the requester’s RNR NAK retry counter is zero, and
an RNR NAK packet is received, an RNR NAK retry error occurs. Each
time an RNR NAK is cleared (i.e., an acknowledge message other than
an RNR NAK is returned), the retry counter is reloaded. An exception to
the following is if the RNR NAK retry counter is set to 7. This value indicates

infinite retry and the counter is not decremented.

"


>
> ---
>  drivers/infiniband/sw/rxe/rxe_comp.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_comp.c b/drivers/infiniband/sw/rxe/rxe_comp.c
> index 4bc88708b355..16c1870b6482 100644
> --- a/drivers/infiniband/sw/rxe/rxe_comp.c
> +++ b/drivers/infiniband/sw/rxe/rxe_comp.c
> @@ -745,8 +745,7 @@ int rxe_completer(void *arg)
>
>                 case COMPST_RNR_RETRY:
>                         if (qp->comp.rnr_retry > 0) {
> -                               if (qp->comp.rnr_retry != 7)
> -                                       qp->comp.rnr_retry--;
> +                               qp->comp.rnr_retry--;
>
>                                 qp->req.need_retry = 1;
>                                 pr_debug("qp#%d set rnr nak timer\n",
> --
> 2.27.0
>
>

Re: [External] [PATCH] RDMA/rxe: fix retry forever when rnr_retry >= 7

[杨帆]

> 
> Please read the following from IB specification
> 

Oh I see, I missed that part.  Thanks for your explanation.


> An exception to
> the following is if the RNR NAK retry counter is set to 7. This value indicates
> infinite retry and the counter is not decremented.

I wonder what if the user set RNR retry larger than 7?  Say the user set the RNR
retry attribute to 10, is he expecting a 10-time retry or an infinite retry?  In
the current implementation, any larger-than-7 retry count causes infinite retry.
Is this an expected behavior?

Regards,
Fan

Re: [External] [PATCH] RDMA/rxe: fix retry forever when rnr_retry >= 7

[Jason Gunthorpe]

On Fri, Jul 24, 2020 at 10:04:01PM +0800, 杨帆 wrote:
> > 
> > Please read the following from IB specification
> > 
> 
> Oh I see, I missed that part.  Thanks for your explanation.
> 
> 
> > An exception to
> > the following is if the RNR NAK retry counter is set to 7. This value indicates
> > infinite retry and the counter is not decremented.
> 
> I wonder what if the user set RNR retry larger than 7?  Say the user set the RNR
> retry attribute to 10, is he expecting a 10-time retry or an infinite retry?  In
> the current implementation, any larger-than-7 retry count causes infinite retry.
> Is this an expected behavior?

Out of range values should be rejected at modify_qp time

Jason

Re: [External] [PATCH] RDMA/rxe: fix retry forever when rnr_retry >= 7

[Fan Yang]

> 
> Out of range values should be rejected at modify_qp time
> 

OK, I see.  Thank you all!

Fan