* Issue in "Introduce create/destroy QP commands over ioctl"? @ 2020-05-27 12:34 Gal Pressman 2020-05-27 12:53 ` Jason Gunthorpe 0 siblings, 1 reply; 3+ messages in thread From: Gal Pressman @ 2020-05-27 12:34 UTC (permalink / raw) To: Yishai Hadas, Leon Romanovsky; +Cc: linux-rdma, Leybovich, Yossi Hi, The recent transition of create/destroy QP commands to ioctl broke the EFA provider in [1]. With the new ioctl the 'ibv_resp' part of the response is all zero'd, opposed to the write method. Any idea what went wrong? Is that an intended change? Thanks, Gal [1] https://github.com/linux-rdma/rdma-core/blob/a02ba0449c1409742261d68bd251fe0ad0662b15/providers/efa/verbs.c#L562 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Issue in "Introduce create/destroy QP commands over ioctl"? 2020-05-27 12:34 Issue in "Introduce create/destroy QP commands over ioctl"? Gal Pressman @ 2020-05-27 12:53 ` Jason Gunthorpe 2020-05-27 13:18 ` Gal Pressman 0 siblings, 1 reply; 3+ messages in thread From: Jason Gunthorpe @ 2020-05-27 12:53 UTC (permalink / raw) To: Gal Pressman; +Cc: Yishai Hadas, Leon Romanovsky, linux-rdma, Leybovich, Yossi On Wed, May 27, 2020 at 03:34:33PM +0300, Gal Pressman wrote: > Hi, > > The recent transition of create/destroy QP commands to ioctl broke the EFA > provider in [1]. > With the new ioctl the 'ibv_resp' part of the response is all zero'd, opposed to > the write method. It is a bug in the efa provider. It is not allowed to touch the 'ibv_resp' parts of the structure from a provider. Something like this diff --git a/providers/efa/verbs.c b/providers/efa/verbs.c index 5f8e7b800210bb..92bb7432a92a2b 100644 --- a/providers/efa/verbs.c +++ b/providers/efa/verbs.c @@ -541,7 +541,9 @@ static void efa_sq_terminate(struct efa_qp *qp) efa_wq_terminate(&qp->sq.wq); } -static int efa_sq_initialize(struct efa_qp *qp, struct efa_create_qp_resp *resp) +static int efa_sq_initialize(struct efa_qp *qp, + const struct ibv_qp_init_attr_ex *attr, + struct efa_create_qp_resp *resp) { struct efa_dev *dev = to_efa_dev(qp->verbs_qp.qp.context->device); size_t desc_ring_size; @@ -559,7 +561,7 @@ static int efa_sq_initialize(struct efa_qp *qp, struct efa_create_qp_resp *resp) desc_ring_size = qp->sq.wq.wqe_cnt * sizeof(struct efa_io_tx_wqe); qp->sq.desc_ring_mmap_size = align(desc_ring_size + qp->sq.desc_offset, qp->page_size); - qp->sq.max_inline_data = resp->ibv_resp.max_inline_data; + qp->sq.max_inline_data = attr->cap.max_inline_data; qp->sq.local_queue = malloc(desc_ring_size); if (!qp->sq.local_queue) { @@ -840,7 +842,7 @@ static struct ibv_qp *create_qp(struct ibv_context *ibvctx, if (err) goto err_destroy_qp; - err = efa_sq_initialize(qp, &resp); + err = efa_sq_initialize(qp, attr, &resp); if (err) goto err_terminate_rq; ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: Issue in "Introduce create/destroy QP commands over ioctl"? 2020-05-27 12:53 ` Jason Gunthorpe @ 2020-05-27 13:18 ` Gal Pressman 0 siblings, 0 replies; 3+ messages in thread From: Gal Pressman @ 2020-05-27 13:18 UTC (permalink / raw) To: Jason Gunthorpe Cc: Yishai Hadas, Leon Romanovsky, linux-rdma, Leybovich, Yossi On 27/05/2020 15:53, Jason Gunthorpe wrote: > On Wed, May 27, 2020 at 03:34:33PM +0300, Gal Pressman wrote: >> Hi, >> >> The recent transition of create/destroy QP commands to ioctl broke the EFA >> provider in [1]. >> With the new ioctl the 'ibv_resp' part of the response is all zero'd, opposed to >> the write method. > > It is a bug in the efa provider. It is not allowed to touch the > 'ibv_resp' parts of the structure from a provider. Thanks Jason, I did not know that. > Something like this > > diff --git a/providers/efa/verbs.c b/providers/efa/verbs.c > index 5f8e7b800210bb..92bb7432a92a2b 100644 > --- a/providers/efa/verbs.c > +++ b/providers/efa/verbs.c > @@ -541,7 +541,9 @@ static void efa_sq_terminate(struct efa_qp *qp) > efa_wq_terminate(&qp->sq.wq); > } > > -static int efa_sq_initialize(struct efa_qp *qp, struct efa_create_qp_resp *resp) > +static int efa_sq_initialize(struct efa_qp *qp, > + const struct ibv_qp_init_attr_ex *attr, > + struct efa_create_qp_resp *resp) > { > struct efa_dev *dev = to_efa_dev(qp->verbs_qp.qp.context->device); > size_t desc_ring_size; > @@ -559,7 +561,7 @@ static int efa_sq_initialize(struct efa_qp *qp, struct efa_create_qp_resp *resp) > desc_ring_size = qp->sq.wq.wqe_cnt * sizeof(struct efa_io_tx_wqe); > qp->sq.desc_ring_mmap_size = align(desc_ring_size + qp->sq.desc_offset, > qp->page_size); > - qp->sq.max_inline_data = resp->ibv_resp.max_inline_data; > + qp->sq.max_inline_data = attr->cap.max_inline_data; > > qp->sq.local_queue = malloc(desc_ring_size); > if (!qp->sq.local_queue) { > @@ -840,7 +842,7 @@ static struct ibv_qp *create_qp(struct ibv_context *ibvctx, > if (err) > goto err_destroy_qp; > > - err = efa_sq_initialize(qp, &resp); > + err = efa_sq_initialize(qp, attr, &resp); > if (err) > goto err_terminate_rq; > > I'll send a PR. ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-27 13:19 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-27 12:34 Issue in "Introduce create/destroy QP commands over ioctl"? Gal Pressman 2020-05-27 12:53 ` Jason Gunthorpe 2020-05-27 13:18 ` Gal Pressman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).