On Fri, Jan 18, 2019 at 03:55:36PM +0200, Marcel Apfelbaum wrote:
> Hi Yuval,
>
> On 1/9/19 10:15 PM, Yuval Shaia wrote:
> > This error should propagate back to guest.
> >
> > Signed-off-by: Yuval Shaia <yuval.sh...@oracle.com>
> > ---
> > hw/rdma/rdma_backend.h | 1 +
> > hw/rdma/vmw/pvrdma_qp_ops.c | 6 ++++--
> > 2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/rdma/rdma_backend.h b/hw/rdma/rdma_backend.h
> > index a9ba40ae48..5114c90e67 100644
> > --- a/hw/rdma/rdma_backend.h
> > +++ b/hw/rdma/rdma_backend.h
> > @@ -32,6 +32,7 @@
> > #define VENDOR_ERR_INVLKEY 0x207
> > #define VENDOR_ERR_MR_SMALL 0x208
> > #define VENDOR_ERR_INV_MAD_BUFF 0x209
> > +#define VENDOR_ERR_INV_GID_IDX 0x210
> > /* Add definition for QP0 and QP1 as there is no userspace enums for them
> > */
> > enum ibv_special_qp_type {
> > diff --git a/hw/rdma/vmw/pvrdma_qp_ops.c b/hw/rdma/vmw/pvrdma_qp_ops.c
> > index 465bee8641..0565eba981 100644
> > --- a/hw/rdma/vmw/pvrdma_qp_ops.c
> > +++ b/hw/rdma/vmw/pvrdma_qp_ops.c
> > @@ -178,7 +178,8 @@ int pvrdma_qp_send(PVRDMADev *dev, uint32_t qp_handle)
> > sgid = rdma_rm_get_gid(&dev->rdma_dev_res,
> > wqe->hdr.wr.ud.av.gid_index);
> > if (!sgid) {
> > pr_dbg("Fail to get gid for idx %d\n",
> > wqe->hdr.wr.ud.av.gid_index);
> > - return -EIO;
> > + complete_with_error(VENDOR_ERR_INV_GID_IDX, comp_ctx);
>
> There may be a problem here, comp_ctx may be uninitialized at this point.
> I see that comp_ctx gets initalized after this call:
>
> /* Prepare CQE */
> comp_ctx = g_malloc(sizeof(CompHandlerCtx));
> comp_ctx->dev = dev;
>
>
> What do you think?
>
> Thanks,
> Marcel
Applying this patch on top of upstream make sense.
Problem start because you probably applied Li Qiang's patch "[PATCH v2] hw:
pvrdma: fix memory leak in error path" which moves the initialization of
comp_ctx to be right after all checks are done.
More than that, accepting this patch makes Li Qiang's patch redundant since
the cleanup of comp_ctx will be taken care by the usual process of
'completion' (see the callback function pvrdma_qp_ops_comp_handler).
Li Qiang, can you confirm?
Yuval
>
> > + continue;
> > }
> > pr_dbg("sgid_id=%d, sgid=0x%llx\n", wqe->hdr.wr.ud.av.gid_index,
> > sgid->global.interface_id);
> > @@ -189,7 +190,8 @@ int pvrdma_qp_send(PVRDMADev *dev, uint32_t qp_handle)
> > if (sgid_idx <= 0) {
> > pr_dbg("Fail to get bk sgid_idx for sgid_idx %d\n",
> > wqe->hdr.wr.ud.av.gid_index);
> > - return -EIO;
> > + complete_with_error(VENDOR_ERR_INV_GID_IDX, comp_ctx);
> > + continue;
> > }
> > if (wqe->hdr.num_sge > dev->dev_attr.max_sge) {
>
