| Message ID | 1482179477-100780-1-git-send-email-tatyana.e.nikolova@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 12/19/2016 3:31 PM, Tatyana Nikolova wrote: > RQ WQE size other than 128B is not supported. > Correct RQ size calculation to use 128B only. > > Since this breaks ABI, add code to provide > compatibility with V4 kernel driver, i40iw. > > Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com> Thanks, applied.
On Thu, Dec 22, 2016 at 11:53:20AM -0500, Doug Ledford wrote: > On 12/19/2016 3:31 PM, Tatyana Nikolova wrote: > > RQ WQE size other than 128B is not supported. > > Correct RQ size calculation to use 128B only. > > > > Since this breaks ABI, add code to provide > > compatibility with V4 kernel driver, i40iw. > > > > Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com> > > Thanks, applied. Hi Doug, Please can you clarify a process here? In previous discussions, you mentioned that the reason of not accepting some code to rdma-core was because it is part of next kernel and we patiently waited for release. This code is definitely not relevant for current kernel, but you are already took it before we ever released first version of rdma-core. So why are we waiting with libpvrdma and RoCE address handle? > > -- > Doug Ledford <dledford@redhat.com> > GPG Key ID: B826A3330E572FDD > Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD >
On 12/22/2016 12:13 PM, Leon Romanovsky wrote: > On Thu, Dec 22, 2016 at 11:53:20AM -0500, Doug Ledford wrote: >> On 12/19/2016 3:31 PM, Tatyana Nikolova wrote: >>> RQ WQE size other than 128B is not supported. >>> Correct RQ size calculation to use 128B only. >>> >>> Since this breaks ABI, add code to provide >>> compatibility with V4 kernel driver, i40iw. >>> >>> Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com> >> >> Thanks, applied. > > Hi Doug, > > Please can you clarify a process here? > > In previous discussions, you mentioned that the reason of not accepting > some code to rdma-core was because it is part of next kernel and we > patiently waited for release. This code is definitely not relevant for > current kernel, but you are already took it before we ever released first > version of rdma-core. > > So why are we waiting with libpvrdma and RoCE address handle? You're right, that was a mistake on my part. I'll probably have to revert this patch, then reapply it for the next version since the 4.10 kernel is the first one to provide the abi field to the user space library. >> >> -- >> Doug Ledford <dledford@redhat.com> >> GPG Key ID: B826A3330E572FDD >> Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD >> > > >
On Thu, Dec 22, 2016 at 01:19:35PM -0500, Doug Ledford wrote: > On 12/22/2016 12:13 PM, Leon Romanovsky wrote: > > On Thu, Dec 22, 2016 at 11:53:20AM -0500, Doug Ledford wrote: > >> On 12/19/2016 3:31 PM, Tatyana Nikolova wrote: > >>> RQ WQE size other than 128B is not supported. > >>> Correct RQ size calculation to use 128B only. > >>> > >>> Since this breaks ABI, add code to provide > >>> compatibility with V4 kernel driver, i40iw. > >>> > >>> Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com> > >> > >> Thanks, applied. > > > > Hi Doug, > > > > Please can you clarify a process here? > > > > In previous discussions, you mentioned that the reason of not accepting > > some code to rdma-core was because it is part of next kernel and we > > patiently waited for release. This code is definitely not relevant for > > current kernel, but you are already took it before we ever released first > > version of rdma-core. > > > > So why are we waiting with libpvrdma and RoCE address handle? > > You're right, that was a mistake on my part. I'll probably have to > revert this patch, then reapply it for the next version since the 4.10 > kernel is the first one to provide the abi field to the user space library. I assume that you are waiting for RHEL spec fixes to do actual release, but maybe we should release it anyway now (replace rc3 with actual release)? We are preparing fixes for Debian package which is not in perfect shape too, and I don't want to let people wait till all these packages are finalized. What do you think? > > >> > >> -- > >> Doug Ledford <dledford@redhat.com> > >> GPG Key ID: B826A3330E572FDD > >> Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD > >> > > > > > > > > > -- > Doug Ledford <dledford@redhat.com> > GPG Key ID: B826A3330E572FDD > Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD >
On 12/22/2016 2:17 PM, Leon Romanovsky wrote: > On Thu, Dec 22, 2016 at 01:19:35PM -0500, Doug Ledford wrote: >> On 12/22/2016 12:13 PM, Leon Romanovsky wrote: >>> On Thu, Dec 22, 2016 at 11:53:20AM -0500, Doug Ledford wrote: >>>> On 12/19/2016 3:31 PM, Tatyana Nikolova wrote: >>>>> RQ WQE size other than 128B is not supported. >>>>> Correct RQ size calculation to use 128B only. >>>>> >>>>> Since this breaks ABI, add code to provide >>>>> compatibility with V4 kernel driver, i40iw. >>>>> >>>>> Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com> >>>> >>>> Thanks, applied. >>> >>> Hi Doug, >>> >>> Please can you clarify a process here? >>> >>> In previous discussions, you mentioned that the reason of not accepting >>> some code to rdma-core was because it is part of next kernel and we >>> patiently waited for release. This code is definitely not relevant for >>> current kernel, but you are already took it before we ever released first >>> version of rdma-core. >>> >>> So why are we waiting with libpvrdma and RoCE address handle? >> >> You're right, that was a mistake on my part. I'll probably have to >> revert this patch, then reapply it for the next version since the 4.10 >> kernel is the first one to provide the abi field to the user space library. > > I assume that you are waiting for RHEL spec fixes to do actual release, but > maybe we should release it anyway now (replace rc3 with actual release)? Can't. rc3 had the wrong patch in it. I've reverted it and have pushed it out. > We are preparing fixes for Debian package which is not in perfect shape too, > and I don't want to let people wait till all these packages are finalized. > > What do you think? > >> >>>> >>>> -- >>>> Doug Ledford <dledford@redhat.com> >>>> GPG Key ID: B826A3330E572FDD >>>> Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD >>>> >>> >>> >>> >> >> >> -- >> Doug Ledford <dledford@redhat.com> >> GPG Key ID: B826A3330E572FDD >> Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD >> > > >
On Mon, Dec 19, 2016 at 02:31:17PM -0600, Tatyana Nikolova wrote: > RQ WQE size other than 128B is not supported. > Correct RQ size calculation to use 128B only. > > Since this breaks ABI, add code to provide > compatibility with V4 kernel driver, i40iw. > > Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com> > --- > providers/i40iw/i40iw-abi.h | 3 +-- > providers/i40iw/i40iw_uk.c | 18 +++++++++++++----- > providers/i40iw/i40iw_umain.c | 15 +++++++++++---- > providers/i40iw/i40iw_umain.h | 1 + > providers/i40iw/i40iw_user.h | 4 +++- > providers/i40iw/i40iw_uverbs.c | 20 +++++++++++++++----- > 6 files changed, 44 insertions(+), 17 deletions(-) > Thanks, https://github.com/linux-rdma/rdma-core/commit/1919a5d436712c58bac8480a0aae6ee2b2f92f17 I applied it again.
diff --git a/providers/i40iw/i40iw-abi.h b/providers/i40iw/i40iw-abi.h index 88c8c72..df2df07 100644 --- a/providers/i40iw/i40iw-abi.h +++ b/providers/i40iw/i40iw-abi.h @@ -37,8 +37,7 @@ #include <infiniband/kern-abi.h> -#define I40IW_ABI_USERSPACE_VER 4 -#define I40IW_ABI_KERNEL_VER 4 +#define I40IW_ABI_VER 5 struct i40iw_get_context { struct ibv_get_context cmd; diff --git a/providers/i40iw/i40iw_uk.c b/providers/i40iw/i40iw_uk.c index 392e858..ea3255f 100644 --- a/providers/i40iw/i40iw_uk.c +++ b/providers/i40iw/i40iw_uk.c @@ -970,11 +970,8 @@ enum i40iw_status_code i40iw_qp_uk_init(struct i40iw_qp_uk *qp, if (info->max_rq_frag_cnt > I40IW_MAX_WQ_FRAGMENT_COUNT) return I40IW_ERR_INVALID_FRAG_COUNT; - ret_code = i40iw_get_wqe_shift(info->sq_size, info->max_sq_frag_cnt, info->max_inline_data, &sqshift); - if (ret_code) - return ret_code; - ret_code = i40iw_get_wqe_shift(info->rq_size, info->max_rq_frag_cnt, 0, &rqshift); + ret_code = i40iw_get_wqe_shift(info->sq_size, info->max_sq_frag_cnt, info->max_inline_data, &sqshift); if (ret_code) return ret_code; @@ -1006,8 +1003,19 @@ enum i40iw_status_code i40iw_qp_uk_init(struct i40iw_qp_uk *qp, if (!qp->use_srq) { qp->rq_size = info->rq_size; qp->max_rq_frag_cnt = info->max_rq_frag_cnt; - qp->rq_wqe_size = rqshift; I40IW_RING_INIT(qp->rq_ring, qp->rq_size); + switch (info->abi_ver) { + case 4: + ret_code = i40iw_get_wqe_shift(info->rq_size, info->max_rq_frag_cnt, 0, &rqshift); + if (ret_code) + return ret_code; + break; + case 5: /* fallthrough until next ABI version */ + default: + rqshift = I40IW_MAX_RQ_WQE_SHIFT; + break; + } + qp->rq_wqe_size = rqshift; qp->rq_wqe_size_multiplier = 4 << rqshift; } qp->ops = iw_qp_uk_ops; diff --git a/providers/i40iw/i40iw_umain.c b/providers/i40iw/i40iw_umain.c index 8d7b655..a1e98ac 100644 --- a/providers/i40iw/i40iw_umain.c +++ b/providers/i40iw/i40iw_umain.c @@ -154,15 +154,21 @@ static struct ibv_context *i40iw_ualloc_context(struct ibv_device *ibdev, int cm memset(iwvctx, 0, sizeof(*iwvctx)); iwvctx->ibv_ctx.cmd_fd = cmd_fd; - cmd.userspace_ver = I40IW_ABI_USERSPACE_VER; + cmd.userspace_ver = I40IW_ABI_VER; memset(&resp, 0, sizeof(resp)); if (ibv_cmd_get_context(&iwvctx->ibv_ctx, (struct ibv_get_context *)&cmd, + sizeof(cmd), &resp.ibv_resp, sizeof(resp))) { + + cmd.userspace_ver = 4; + if (ibv_cmd_get_context(&iwvctx->ibv_ctx, (struct ibv_get_context *)&cmd, sizeof(cmd), &resp.ibv_resp, sizeof(resp))) - goto err_free; + goto err_free; + + } - if (resp.kernel_ver != I40IW_ABI_KERNEL_VER) { + if (resp.kernel_ver > I40IW_ABI_VER) { fprintf(stderr, PFX "%s: incompatible kernel driver version: %d. Need version %d\n", - __func__, resp.kernel_ver, I40IW_ABI_KERNEL_VER); + __func__, resp.kernel_ver, I40IW_ABI_VER); goto err_free; } @@ -171,6 +177,7 @@ static struct ibv_context *i40iw_ualloc_context(struct ibv_device *ibdev, int cm iwvctx->max_pds = resp.max_pds; iwvctx->max_qps = resp.max_qps; iwvctx->wq_size = resp.wq_size; + iwvctx->abi_ver = resp.kernel_ver; i40iw_device_init_uk(&iwvctx->dev); ibv_pd = i40iw_ualloc_pd(&iwvctx->ibv_ctx); diff --git a/providers/i40iw/i40iw_umain.h b/providers/i40iw/i40iw_umain.h index 719aefc..06d38a2 100644 --- a/providers/i40iw/i40iw_umain.h +++ b/providers/i40iw/i40iw_umain.h @@ -95,6 +95,7 @@ struct i40iw_uvcontext { uint32_t max_qps; /* maximum qps allowed for this user process */ uint32_t wq_size; /* size of the WQs (sq+rq) + shadow allocated to the mmaped area */ struct i40iw_dev_uk dev; + int abi_ver; }; struct i40iw_uqp; diff --git a/providers/i40iw/i40iw_user.h b/providers/i40iw/i40iw_user.h index 8d345b3..1e95c23 100644 --- a/providers/i40iw/i40iw_user.h +++ b/providers/i40iw/i40iw_user.h @@ -77,6 +77,7 @@ enum i40iw_device_capabilities_const { I40IW_MAX_WQ_ENTRIES = 2048, I40IW_MAX_ORD_SIZE = 32, I40IW_Q2_BUFFER_SIZE = (248 + 100), + I40IW_MAX_WQE_SIZE_RQ = 128, I40IW_QP_CTX_SIZE = 248 }; @@ -103,6 +104,7 @@ enum i40iw_device_capabilities_const { #define I40IW_STAG_INDEX_FROM_STAG(stag) (((stag) && 0xFFFFFF00) >> 8) #define I40IW_MAX_MR_SIZE 0x10000000000L +#define I40IW_MAX_RQ_WQE_SHIFT 2 struct i40iw_qp_uk; struct i40iw_cq_uk; @@ -411,7 +413,7 @@ struct i40iw_qp_uk_init_info { u32 max_sq_frag_cnt; u32 max_rq_frag_cnt; u32 max_inline_data; - + int abi_ver; }; struct i40iw_cq_uk_init_info { diff --git a/providers/i40iw/i40iw_uverbs.c b/providers/i40iw/i40iw_uverbs.c index 4a868ea..a946fcc 100644 --- a/providers/i40iw/i40iw_uverbs.c +++ b/providers/i40iw/i40iw_uverbs.c @@ -661,12 +661,21 @@ struct ibv_qp *i40iw_ucreate_qp(struct ibv_pd *pd, struct ibv_qp_init_attr *attr return NULL; } - rqdepth = i40iw_qp_get_qdepth(rq_attr, attr->cap.max_recv_sge, 0); - if (!rqdepth) { - fprintf(stderr, PFX "%s: invalid RQ attributes, max_recv_wr=%d max_recv_sge=%d\n", - __func__, attr->cap.max_recv_wr, attr->cap.max_recv_sge); - return NULL; + switch (iwvctx->abi_ver) { + case 4: + rqdepth = i40iw_qp_get_qdepth(rq_attr, attr->cap.max_recv_sge, 0); + if (!rqdepth) { + fprintf(stderr, PFX "%s: invalid RQ attributes, max_recv_wr=%d max_recv_sge=%d\n", + __func__, attr->cap.max_recv_wr, attr->cap.max_recv_sge); + return NULL; + } + break; + case 5: /* fallthrough until next ABI version */ + default: + rqdepth = rq_attr << I40IW_MAX_RQ_WQE_SHIFT; + break; } + iwuqp = memalign(1024, sizeof(*iwuqp)); if (!iwuqp) return NULL; @@ -687,6 +696,7 @@ struct ibv_qp *i40iw_ucreate_qp(struct ibv_pd *pd, struct ibv_qp_init_attr *attr info.wqe_alloc_reg = (u32 *)iwvctx->iwupd->db; info.sq_wrtrk_array = calloc(sqdepth, sizeof(*info.sq_wrtrk_array)); + info.abi_ver = iwvctx->abi_ver; if (!info.sq_wrtrk_array) { fprintf(stderr, PFX "%s: failed to allocate memory for SQ work array\n", __func__);
RQ WQE size other than 128B is not supported. Correct RQ size calculation to use 128B only. Since this breaks ABI, add code to provide compatibility with V4 kernel driver, i40iw. Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com> --- providers/i40iw/i40iw-abi.h | 3 +-- providers/i40iw/i40iw_uk.c | 18 +++++++++++++----- providers/i40iw/i40iw_umain.c | 15 +++++++++++---- providers/i40iw/i40iw_umain.h | 1 + providers/i40iw/i40iw_user.h | 4 +++- providers/i40iw/i40iw_uverbs.c | 20 +++++++++++++++----- 6 files changed, 44 insertions(+), 17 deletions(-)