* [Qemu-devel] [PATCH] hw/pvrdma: Post CQE when receive invalid gid index
@ 2019-01-09 20:15 Yuval Shaia
2019-01-12 15:11 ` Marcel Apfelbaum
2019-01-18 13:55 ` Marcel Apfelbaum
0 siblings, 2 replies; 7+ messages in thread
From: Yuval Shaia @ 2019-01-09 20:15 UTC (permalink / raw)
To: yuval.shaia, marcel.apfelbaum, qemu-devel
This error should propagate back to guest.
Signed-off-by: Yuval Shaia <yuval.shaia@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);
+ 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) {
--
2.17.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] hw/pvrdma: Post CQE when receive invalid gid index
2019-01-09 20:15 [Qemu-devel] [PATCH] hw/pvrdma: Post CQE when receive invalid gid index Yuval Shaia
@ 2019-01-12 15:11 ` Marcel Apfelbaum
2019-01-13 19:19 ` Yuval Shaia
2019-01-18 13:55 ` Marcel Apfelbaum
1 sibling, 1 reply; 7+ messages in thread
From: Marcel Apfelbaum @ 2019-01-12 15:11 UTC (permalink / raw)
To: Yuval Shaia, qemu-devel
On 1/9/19 10:15 PM, Yuval Shaia wrote:
> This error should propagate back to guest.
>
> Signed-off-by: Yuval Shaia <yuval.shaia@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);
Informing the guest is OK, but are you sure it makes sense
to continue sending the other work requests?
Is maybe safer to return with error as before?
> + 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;
Same question here.
Thanks,
Marcel
> }
>
> if (wqe->hdr.num_sge > dev->dev_attr.max_sge) {
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] hw/pvrdma: Post CQE when receive invalid gid index
2019-01-12 15:11 ` Marcel Apfelbaum
@ 2019-01-13 19:19 ` Yuval Shaia
2019-01-18 13:04 ` Marcel Apfelbaum
0 siblings, 1 reply; 7+ messages in thread
From: Yuval Shaia @ 2019-01-13 19:19 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel
On Sat, Jan 12, 2019 at 05:11:39PM +0200, Marcel Apfelbaum wrote:
>
>
> On 1/9/19 10:15 PM, Yuval Shaia wrote:
> > This error should propagate back to guest.
> >
> > Signed-off-by: Yuval Shaia <yuval.shaia@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);
>
> Informing the guest is OK, but are you sure it makes sense
> to continue sending the other work requests?
Yes, it is exactly what is expected. RDMA application (or driver) would
usually push several work requests to ring (optimally populating the entire
ring) and then ring the doorbell.
An error in a specific WR does not means all WRs in ring should not be
processed.
> Is maybe safer to return with error as before?
And what about the remaining WR in the ring? any way the device would have
to process them on next doorbell.
>
> > + 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;
>
> Same question here.
>
> Thanks,
> Marcel
> > }
> > if (wqe->hdr.num_sge > dev->dev_attr.max_sge) {
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] hw/pvrdma: Post CQE when receive invalid gid index
2019-01-13 19:19 ` Yuval Shaia
@ 2019-01-18 13:04 ` Marcel Apfelbaum
0 siblings, 0 replies; 7+ messages in thread
From: Marcel Apfelbaum @ 2019-01-18 13:04 UTC (permalink / raw)
To: Yuval Shaia; +Cc: qemu-devel
On 1/13/19 9:19 PM, Yuval Shaia wrote:
> On Sat, Jan 12, 2019 at 05:11:39PM +0200, Marcel Apfelbaum wrote:
>>
>> On 1/9/19 10:15 PM, Yuval Shaia wrote:
>>> This error should propagate back to guest.
>>>
>>> Signed-off-by: Yuval Shaia <yuval.shaia@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);
>> Informing the guest is OK, but are you sure it makes sense
>> to continue sending the other work requests?
> Yes, it is exactly what is expected. RDMA application (or driver) would
> usually push several work requests to ring (optimally populating the entire
> ring) and then ring the doorbell.
> An error in a specific WR does not means all WRs in ring should not be
> processed.
Thanks for the explanation.
Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Thanks,
Marcel
>> Is maybe safer to return with error as before?
> And what about the remaining WR in the ring? any way the device would have
> to process them on next doorbell.
>
>>> + 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;
>> Same question here.
>>
>> Thanks,
>> Marcel
>>> }
>>> if (wqe->hdr.num_sge > dev->dev_attr.max_sge) {
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/pvrdma: Post CQE when receive invalid gid index
2019-01-09 20:15 [Qemu-devel] [PATCH] hw/pvrdma: Post CQE when receive invalid gid index Yuval Shaia
2019-01-12 15:11 ` Marcel Apfelbaum
@ 2019-01-18 13:55 ` Marcel Apfelbaum
2019-01-18 16:23 ` Yuval Shaia
1 sibling, 1 reply; 7+ messages in thread
From: Marcel Apfelbaum @ 2019-01-18 13:55 UTC (permalink / raw)
To: Yuval Shaia, qemu-devel
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.shaia@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
> + 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) {
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] hw/pvrdma: Post CQE when receive invalid gid index
2019-01-18 13:55 ` Marcel Apfelbaum
@ 2019-01-18 16:23 ` Yuval Shaia
2019-01-19 9:42 ` Marcel Apfelbaum
0 siblings, 1 reply; 7+ messages in thread
From: Yuval Shaia @ 2019-01-18 16:23 UTC (permalink / raw)
To: Marcel Apfelbaum, liq3ea, philmd; +Cc: qemu-devel
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.shaia@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) {
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] hw/pvrdma: Post CQE when receive invalid gid index
2019-01-18 16:23 ` Yuval Shaia
@ 2019-01-19 9:42 ` Marcel Apfelbaum
0 siblings, 0 replies; 7+ messages in thread
From: Marcel Apfelbaum @ 2019-01-19 9:42 UTC (permalink / raw)
To: Yuval Shaia, liq3ea, philmd; +Cc: qemu-devel
On 1/18/19 6:23 PM, Yuval Shaia wrote:
> 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.shaia@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?
Sounds right to me. I'll add Reported-by tag and keep the coverity info.
Thanks,
Marcel
> 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) {
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-01-19 9:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-09 20:15 [Qemu-devel] [PATCH] hw/pvrdma: Post CQE when receive invalid gid index Yuval Shaia
2019-01-12 15:11 ` Marcel Apfelbaum
2019-01-13 19:19 ` Yuval Shaia
2019-01-18 13:04 ` Marcel Apfelbaum
2019-01-18 13:55 ` Marcel Apfelbaum
2019-01-18 16:23 ` Yuval Shaia
2019-01-19 9:42 ` Marcel Apfelbaum
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.