Re: [PATCH v3 2/2] RDMA/rxe: Generate error completion for error requester QP state

[Bob Pearson <rpearsonhpe@gmail.com>]

On 5/15/22 20:53, Li Zhijian wrote:
> As per IBTA specification, all subsequent WQEs while QP is in error
> state should be completed with a flush error.
> 
> Here we check QP_STATE_ERROR after req_next_wqe() so that rxe_completer()
> has chance to be called where it will set CQ state to FLUSH ERROR and the
> completion can associate with its WQE.
> 
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> V3: unlikely() optimization # Cheng Xu <chengyou@linux.alibaba.com>
>     update commit log # Haakon Bugge <haakon.bugge@oracle.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_req.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
> index 8bdd0b6b578f..c1f1c19f26b2 100644
> --- a/drivers/infiniband/sw/rxe/rxe_req.c
> +++ b/drivers/infiniband/sw/rxe/rxe_req.c
> @@ -624,7 +624,7 @@ int rxe_requester(void *arg)
>  	rxe_get(qp);
>  
>  next_wqe:
> -	if (unlikely(!qp->valid || qp->req.state == QP_STATE_ERROR))
> +	if (unlikely(!qp->valid))
>  		goto exit;
>  
>  	if (unlikely(qp->req.state == QP_STATE_RESET)) {
> @@ -646,6 +646,14 @@ int rxe_requester(void *arg)
>  	if (unlikely(!wqe))
>  		goto exit;
>  
> +	if (unlikely(qp->req.state == QP_STATE_ERROR)) {
> +		/*
> +		 * Generate an error completion so that user space is able to
> +		 * poll this completion.
> +		 */
> +		goto err;
> +	}
> +
>  	if (wqe->mask & WR_LOCAL_OP_MASK) {
>  		ret = rxe_do_local_ops(qp, wqe);
>  		if (unlikely(ret))

There may be issues with moving this after the retry check since the retransmit timer can
fire at any time and may race with the completer setting the error state and result in
a retry flow occurring while you are trying to flush out all the wqes. Perhaps better
to to duplicate setting wqe in the error state check.

Bob