Re: [PATCH v2] RDMA/rxe: Generate a completion on error after getting a wqe

[Leon Romanovsky <leonro@xxxxxxxxxx>]

On Thu, Mar 31, 2022 at 05:55:59AM +0000, yangx.jy@xxxxxxxxxxx wrote:
> On 2022/3/30 15:44, Leon Romanovsky wrote:
> > You will need to open IBTA spec and try to understand from it.
> > But realistically speaking, I think that it will be too hard to do it
> > and not sure if it is really important.
> >
> > Please resubmit your patch with updated diff and commit message.
> 
> Hi Leon,
> 
> It seems that your change is based on old code.
> 
> After looking into the latest code, this change seem not simpler and 
> clearer. do you think so?

No, something like that will do the trick.

diff --git a/drivers/infiniband/sw/rxe/rxe_av.c b/drivers/infiniband/sw/rxe/rxe_av.c
index 3b05314ca739..5938cba936d9 100644
--- a/drivers/infiniband/sw/rxe/rxe_av.c
+++ b/drivers/infiniband/sw/rxe/rxe_av.c
@@ -104,9 +104,6 @@ struct rxe_av *rxe_get_av(struct rxe_pkt_info *pkt, struct rxe_ah **ahp)
        struct rxe_ah *ah;
        u32 ah_num;

-       if (ahp)
-               *ahp = NULL;
-
        if (!pkt || !pkt->qp)
                return NULL;

diff --git a/drivers/infiniband/sw/rxe/rxe_req.c b/drivers/infiniband/sw/rxe/rxe_req.c
index ae5fbc79dd5c..9afc8cf50863 100644
--- a/drivers/infiniband/sw/rxe/rxe_req.c
+++ b/drivers/infiniband/sw/rxe/rxe_req.c
@@ -648,26 +648,30 @@ int rxe_requester(void *arg)
                psn_compare(qp->req.psn, (qp->comp.psn +
                                RXE_MAX_UNACKED_PSNS)) > 0)) {
                qp->req.wait_psn = 1;
-               goto exit;
+               wqe->status = IB_WC_LOC_QP_OP_ERR;
+               goto err;
        }

        /* Limit the number of inflight SKBs per QP */
        if (unlikely(atomic_read(&qp->skb_out) >
                     RXE_INFLIGHT_SKBS_PER_QP_HIGH)) {
                qp->need_req_skb = 1;
-               goto exit;
+               wqe->status = IB_WC_LOC_QP_OP_ERR;
+               goto err;
        }

        opcode = next_opcode(qp, wqe, wqe->wr.opcode);
        if (unlikely(opcode < 0)) {
                wqe->status = IB_WC_LOC_QP_OP_ERR;
-               goto exit;
+               goto err;
        }
 
        mask = rxe_opcode[opcode].mask;
        if (unlikely(mask & RXE_READ_OR_ATOMIC_MASK)) {
-               if (check_init_depth(qp, wqe))
-                       goto exit;
+               if (check_init_depth(qp, wqe)) {
+                       wqe->status = IB_WC_LOC_QP_OP_ERR;
+                       goto err;
+               }
        }
 
        mtu = get_mtu(qp);
@@ -704,32 +708,27 @@ int rxe_requester(void *arg)
        pkt.wqe = wqe;
 
        av = rxe_get_av(&pkt, &ah);
-       if (unlikely(!av)) {
-               pr_err("qp#%d Failed no address vector\n", qp_num(qp));
-               wqe->status = IB_WC_LOC_QP_OP_ERR;
-               goto err_drop_ah;
-       }
+       if (unlikely(!av))
+               goto qp_err;
 
        skb = init_req_packet(qp, av, wqe, opcode, payload, &pkt);
        if (unlikely(!skb)) {
-               pr_err("qp#%d Failed allocating skb\n", qp_num(qp));
-               wqe->status = IB_WC_LOC_QP_OP_ERR;
-               goto err_drop_ah;
+               rxe_put(ah);
+               goto qp_err;
        }
 
        ret = finish_packet(qp, av, wqe, &pkt, skb, payload);
        if (unlikely(ret)) {
-               pr_debug("qp#%d Error during finish packet\n", qp_num(qp));
                if (ret == -EFAULT)
                        wqe->status = IB_WC_LOC_PROT_ERR;
                else
                        wqe->status = IB_WC_LOC_QP_OP_ERR;
                kfree_skb(skb);
-               goto err_drop_ah;
+               rxe_put(ah);
+               goto err;
        }
 
-       if (ah)
-               rxe_put(ah);
+       rxe_put(ah);
 
        /*
         * To prevent a race on wqe access between requester and completer,
@@ -751,17 +750,15 @@ int rxe_requester(void *arg)
                        goto exit;
                }
 
-               wqe->status = IB_WC_LOC_QP_OP_ERR;
-               goto err;
+               goto qp_err;
        }
 
        update_state(qp, wqe, &pkt);
 
        goto next_wqe;
 
-err_drop_ah:
-       if (ah)
-               rxe_put(ah);
+qp_err:
+       wqe->status = IB_WC_LOC_QP_OP_ERR;
 err:
        wqe->state = wqe_state_error;
        __rxe_do_task(&qp->comp.task);


> 
> Best Regards,
> 
> Xiao Yang
> 
> >
> > Thanks