[PATCH 0/5] libbnxt_re: Critical bug fix series

[PATCH 0/5] libbnxt_re: Critical bug fix series

[Devesh Sharma]

These are set of bug fix patches for the libbnxt_re user
space RDMA provider library. This patch series goes
hand-in-hand with recent bnxt_re kernel driver bug fixes.

Devesh Sharma (2):
  libbnxt_re: unmap DB page during uninit ucontext
  libbnxt_re: reset head and tail when moving to RST

Somnath Kotur (3):
  libbnxt_re: sq needs to be augmented by 128B
  libbnxt_re: fix wqe size for the 0-len posting
  libbnxt_re: move rts to rts after a threshold

 providers/bnxt_re/bnxt_re-abi.h |  2 +
 providers/bnxt_re/main.c        |  9 +++++
 providers/bnxt_re/main.h        |  3 ++
 providers/bnxt_re/memory.h      | 10 ++++-
 providers/bnxt_re/verbs.c       | 81 +++++++++++++++++++++++++++++++----------
 5 files changed, 84 insertions(+), 21 deletions(-)

-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/5] libbnxt_re: sq needs to be augmented by 128B

[Devesh Sharma]

From: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

In order to avoid out of order completions there is a
requirement to augment the SQ of any QP by 128B.
This patch adds (128 + 1)B delta while SQ allocation and
updates the queue-full determining logic.

bnxt_re_query_qp is relying on whatever attributes driver
returns. Thus, dropping to update the max_send_wr and
max_recv_wr caps during bnxt_re_create_qp.

Signed-off-by: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Devesh Sharma <devesh.sharma-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 providers/bnxt_re/bnxt_re-abi.h |  2 ++
 providers/bnxt_re/memory.h      | 10 +++++++++-
 providers/bnxt_re/verbs.c       |  9 ++++++---
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/providers/bnxt_re/bnxt_re-abi.h b/providers/bnxt_re/bnxt_re-abi.h
index 205d8c4..73beef5 100644
--- a/providers/bnxt_re/bnxt_re-abi.h
+++ b/providers/bnxt_re/bnxt_re-abi.h
@@ -43,6 +43,8 @@
 
 #define BNXT_RE_ABI_VERSION 1
 
+#define BNXT_RE_FULL_FLAG_DELTA        0x80
+
 enum bnxt_re_wr_opcode {
 	BNXT_RE_WR_OPCD_SEND		= 0x00,
 	BNXT_RE_WR_OPCD_SEND_IMM	= 0x01,
diff --git a/providers/bnxt_re/memory.h b/providers/bnxt_re/memory.h
index 0150d80..aac0ff3 100644
--- a/providers/bnxt_re/memory.h
+++ b/providers/bnxt_re/memory.h
@@ -49,6 +49,14 @@ struct bnxt_re_queue {
 	uint32_t head;
 	uint32_t tail;
 	uint32_t stride;
+	/* Represents the difference between the real queue depth allocated in
+	 * HW and the user requested queue depth and is used to correctly flag
+	 * queue full condition based on user supplied queue depth.
+	 * This value can vary depending on the type of queue and any HW
+	 * requirements that mandate keeping a fixed gap between the producer
+	 * and the consumer indices in the queue
+	 */
+	uint32_t diff;
 	pthread_spinlock_t qlock;
 };
 
@@ -86,7 +94,7 @@ static inline void iowrite32(__u32 *dst, __le32 *src)
 /* Basic queue operation */
 static inline uint32_t bnxt_re_is_que_full(struct bnxt_re_queue *que)
 {
-	return (((que->tail + 1) & (que->depth - 1)) == que->head);
+	return (((que->diff + que->tail) & (que->depth - 1)) == que->head);
 }
 
 static inline uint32_t bnxt_re_is_que_empty(struct bnxt_re_queue *que)
diff --git a/providers/bnxt_re/verbs.c b/providers/bnxt_re/verbs.c
index 2768a56..35eb12d 100644
--- a/providers/bnxt_re/verbs.c
+++ b/providers/bnxt_re/verbs.c
@@ -793,7 +793,11 @@ static int bnxt_re_alloc_queues(struct bnxt_re_qp *qp,
 
 	que = qp->sqq;
 	que->stride = bnxt_re_get_sqe_sz();
-	que->depth = roundup_pow_of_two(attr->cap.max_send_wr + 1);
+	/* 8916 adjustment */
+	que->depth = roundup_pow_of_two(attr->cap.max_send_wr + 1 +
+					BNXT_RE_FULL_FLAG_DELTA);
+	que->diff = que->depth - attr->cap.max_send_wr;
+
 	/* psn_depth extra entries of size que->stride */
 	psn_depth = (que->depth * sizeof(struct bnxt_re_psns)) /
 		     que->stride;
@@ -828,6 +832,7 @@ static int bnxt_re_alloc_queues(struct bnxt_re_qp *qp,
 		que = qp->rqq;
 		que->stride = bnxt_re_get_rqe_sz();
 		que->depth = roundup_pow_of_two(attr->cap.max_recv_wr + 1);
+		que->diff = que->depth - attr->cap.max_recv_wr;
 		ret = bnxt_re_alloc_aligned(qp->rqq, pg_size);
 		if (ret)
 			goto fail;
@@ -888,9 +893,7 @@ struct ibv_qp *bnxt_re_create_qp(struct ibv_pd *ibvpd,
 	qp->rcq = to_bnxt_re_cq(attr->recv_cq);
 	qp->udpi = &cntx->udpi;
 	/* Save/return the altered Caps. */
-	attr->cap.max_send_wr = cap->max_swr;
 	cap->max_ssge = attr->cap.max_send_sge;
-	attr->cap.max_recv_wr = cap->max_rwr;
 	cap->max_rsge = attr->cap.max_recv_sge;
 	cap->max_inline = attr->cap.max_inline_data;
 	cap->sqsig = attr->sq_sig_all;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] libbnxt_re: fix wqe size for the 0-len posting

[Devesh Sharma]

From: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

rstream does not include any SGEs in the traffic it generates
with zero length. In this case wqe preparation logic is
calculating WQE-size=2. This is resulting WQE_FORMAT_ERROR
when h/w is processing the wqe. Our h/w requires host to supply
WQE Size with room for atleast one SGE. Thus, this patch is
increasing the wqe-size by 1 whenever wr->num_sges are zero.

Signed-off-by: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Devesh Sharma <devesh.sharma-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 providers/bnxt_re/verbs.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/providers/bnxt_re/verbs.c b/providers/bnxt_re/verbs.c
index 35eb12d..7cf0b86 100644
--- a/providers/bnxt_re/verbs.c
+++ b/providers/bnxt_re/verbs.c
@@ -1081,6 +1081,11 @@ static int bnxt_re_build_send_sqe(struct bnxt_re_qp *qp, void *wqe,
 	} else {
 		qesize = wr->num_sge;
 	}
+	/* HW requires wqe size has room for atleast one sge even if none was
+	 * supplied by application
+	 */
+	if (!wr->num_sge)
+		qesize++;
 	qesize += (bnxt_re_get_sqe_hdr_sz() >> 4);
 	hdrval |= (qesize & BNXT_RE_HDR_WS_MASK) << BNXT_RE_HDR_WS_SHIFT;
 	hdr->rsv_ws_fl_wt |= htole32(hdrval);
@@ -1259,6 +1264,11 @@ static int bnxt_re_build_rqe(struct bnxt_re_qp *qp, struct ibv_recv_wr *wr,
 
 	len = bnxt_re_build_sge(sge, wr->sg_list, wr->num_sge, false);
 	wqe_sz = wr->num_sge + (bnxt_re_get_rqe_hdr_sz() >> 4); /* 16B align */
+	/* HW requires wqe size has room for atleast one sge even if none was
+	 * supplied by application
+	 */
+	if (!wr->num_sge)
+		wqe_sz++;
 	hdrval = BNXT_RE_WR_OPCD_RECV;
 	hdrval |= ((wqe_sz & BNXT_RE_HDR_WS_MASK) << BNXT_RE_HDR_WS_SHIFT);
 	hdr->rsv_ws_fl_wt = htole32(hdrval);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5] libbnxt_re: move rts to rts after a threshold

[Devesh Sharma]

From: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

Due to a know bug, h/w is stopping UD-WQE processing after
0x800000 WQEs. Library needs to move the QP from RTS to RTS
around half-way mark. This patch adds a simple wqe-counter
and modfies the qp to circumvent the problem.

Signed-off-by: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Devesh Sharma <devesh.sharma-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 providers/bnxt_re/main.h  |  3 +++
 providers/bnxt_re/verbs.c | 12 ++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/providers/bnxt_re/main.h b/providers/bnxt_re/main.h
index 4cc8abd..9688fec 100644
--- a/providers/bnxt_re/main.h
+++ b/providers/bnxt_re/main.h
@@ -54,6 +54,8 @@
 
 #define DEV	"bnxt_re : "
 
+#define BNXT_RE_UD_QP_HW_STALL 0x400000
+
 struct bnxt_re_dpi {
 	__u32 dpindx;
 	__u64 *dbpage;
@@ -113,6 +115,7 @@ struct bnxt_re_qp {
 	uint32_t tbl_indx;
 	uint32_t sq_psn;
 	uint32_t pending_db;
+	uint64_t wqe_cnt;
 	uint16_t mtu;
 	uint16_t qpst;
 	uint8_t qptyp;
diff --git a/providers/bnxt_re/verbs.c b/providers/bnxt_re/verbs.c
index 7cf0b86..abe4e2e 100644
--- a/providers/bnxt_re/verbs.c
+++ b/providers/bnxt_re/verbs.c
@@ -1240,8 +1240,20 @@ int bnxt_re_post_send(struct ibv_qp *ibvqp, struct ibv_send_wr *wr,
 		bnxt_re_fill_wrid(wrid, wr, bytes, qp->cap.sqsig);
 		bnxt_re_fill_psns(qp, psns, wr->opcode, bytes);
 		bnxt_re_incr_tail(sq);
+		qp->wqe_cnt++;
 		wr = wr->next;
 		bnxt_re_ring_sq_db(qp);
+		if (qp->wqe_cnt == BNXT_RE_UD_QP_HW_STALL && qp->qptyp ==
+		    IBV_QPT_UD) {
+			/* Move RTS to RTS since it is time. */
+			struct ibv_qp_attr attr;
+			int attr_mask;
+
+			attr_mask = IBV_QP_STATE;
+			attr.qp_state = IBV_QPS_RTS;
+			bnxt_re_modify_qp(&qp->ibvqp, &attr, attr_mask);
+			qp->wqe_cnt = 0;
+		}
 	}
 
 	pthread_spin_unlock(&sq->qlock);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/5] libbnxt_re: unmap DB page during uninit ucontext

[Devesh Sharma]

Library is trying to map DPI page every time a new PD is
allocated on a given user-context. Similarly, the free-PD
is trying to unmap the PD in every call. However, dapltest
is trying to ring the DB after a free-PD.

Library needs to mmap the DB page only once during first
PD allocation on a given user-context. The unmap of DB page
should be done during uninit_ucontext. This function is
called during ibv_close.

Signed-off-by: Devesh Sharma <devesh.sharma-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 providers/bnxt_re/main.c  |  9 +++++++++
 providers/bnxt_re/verbs.c | 27 ++++++++++++---------------
 2 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/providers/bnxt_re/main.c b/providers/bnxt_re/main.c
index da4dd06..29ac7cd 100644
--- a/providers/bnxt_re/main.c
+++ b/providers/bnxt_re/main.c
@@ -159,6 +159,15 @@ static void bnxt_re_uninit_context(struct verbs_device *vdev,
 	if (cntx->shpg)
 		munmap(cntx->shpg, dev->pg_size);
 	pthread_spin_destroy(&cntx->fqlock);
+
+	/* Un-map DPI only for the first PD that was
+	 * allocated in this context.
+	 */
+	if (cntx->udpi.dbpage && cntx->udpi.dbpage != MAP_FAILED) {
+		pthread_spin_destroy(&cntx->udpi.db_lock);
+		munmap(cntx->udpi.dbpage, dev->pg_size);
+		cntx->udpi.dbpage = NULL;
+	}
 }
 
 static struct verbs_device_ops bnxt_re_dev_ops = {
diff --git a/providers/bnxt_re/verbs.c b/providers/bnxt_re/verbs.c
index abe4e2e..d337514 100644
--- a/providers/bnxt_re/verbs.c
+++ b/providers/bnxt_re/verbs.c
@@ -96,14 +96,17 @@ struct ibv_pd *bnxt_re_alloc_pd(struct ibv_context *ibvctx)
 	dbr = *(uint64_t *)((uint32_t *)&resp + 3);
 
 	/* Map DB page now. */
-	cntx->udpi.dpindx = resp.dpi;
-	cntx->udpi.dbpage = mmap(NULL, dev->pg_size, PROT_WRITE, MAP_SHARED,
-				 ibvctx->cmd_fd, dbr);
-	if (cntx->udpi.dbpage == MAP_FAILED) {
-		(void)ibv_cmd_dealloc_pd(&pd->ibvpd);
-		goto out;
-	}
-	pthread_spin_init(&cntx->udpi.db_lock, PTHREAD_PROCESS_PRIVATE);
+	if (!cntx->udpi.dbpage) {
+		cntx->udpi.dpindx = resp.dpi;
+		cntx->udpi.dbpage = mmap(NULL, dev->pg_size, PROT_WRITE,
+					 MAP_SHARED, ibvctx->cmd_fd, dbr);
+		if (cntx->udpi.dbpage == MAP_FAILED) {
+			(void)ibv_cmd_dealloc_pd(&pd->ibvpd);
+			goto out;
+		}
+		pthread_spin_init(&cntx->udpi.db_lock,
+				  PTHREAD_PROCESS_PRIVATE);
+        }
 
 	return &pd->ibvpd;
 out:
@@ -114,18 +117,12 @@ out:
 int bnxt_re_free_pd(struct ibv_pd *ibvpd)
 {
 	struct bnxt_re_pd *pd = to_bnxt_re_pd(ibvpd);
-	struct bnxt_re_context *cntx = to_bnxt_re_context(ibvpd->context);
-	struct bnxt_re_dev *dev = to_bnxt_re_dev(cntx->ibvctx.device);
 	int status;
 
 	status = ibv_cmd_dealloc_pd(ibvpd);
 	if (status)
 		return status;
-
-	pthread_spin_destroy(&cntx->udpi.db_lock);
-	if (cntx->udpi.dbpage && (cntx->udpi.dbpage != MAP_FAILED))
-		munmap(cntx->udpi.dbpage, dev->pg_size);
-
+	/* DPI un-mapping will be during uninit_ucontext */
 	free(pd);
 
 	return 0;
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] libbnxt_re: reset head and tail when moving to RST

[Devesh Sharma]

When the QP is moving to RESET state from any state
the head and tail of SQ and RQ(if present) should move
to initial state (which is 0).
This patch adds the code to handle reseting head and tail.

Signed-off-by: Devesh Sharma <devesh.sharma-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 providers/bnxt_re/verbs.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/providers/bnxt_re/verbs.c b/providers/bnxt_re/verbs.c
index d337514..1fe7967 100644
--- a/providers/bnxt_re/verbs.c
+++ b/providers/bnxt_re/verbs.c
@@ -419,6 +419,11 @@ static void bnxt_re_poll_success_rcqe(struct bnxt_re_qp *qp,
 	ibvwc->wc_flags = 0;
 	if (is_imm) {
 		ibvwc->wc_flags |= IBV_WC_WITH_IMM;
+		/* Completion reports the raw-data in LE format, While
+		 * user expects it in BE format. Thus, swapping on outgoing
+		 * data is needed. On a BE platform le32toh will do the swap
+		 * while on LE platform htobe32 will do the job.
+		 */
 		ibvwc->imm_data = htobe32(le32toh(rcqe->imm_key));
 		if (is_rdma)
 			ibvwc->opcode = IBV_WC_RECV_RDMA_WITH_IMM;
@@ -917,9 +922,18 @@ int bnxt_re_modify_qp(struct ibv_qp *ibvqp, struct ibv_qp_attr *attr,
 
 	rc = ibv_cmd_modify_qp(ibvqp, attr, attr_mask, &cmd, sizeof(cmd));
 	if (!rc) {
-		if (attr_mask & IBV_QP_STATE)
+		if (attr_mask & IBV_QP_STATE) {
 			qp->qpst = attr->qp_state;
-
+			/* transition to reset */
+			if (qp->qpst == IBV_QPS_RESET) {
+				qp->sqq->head = 0;
+				qp->sqq->tail = 0;
+				if (qp->rqq) {
+					qp->rqq->head = 0;
+					qp->rqq->tail = 0;
+				}
+			}
+		}
 		if (attr_mask & IBV_QP_SQ_PSN)
 			qp->sq_psn = attr->sq_psn;
 		if (attr_mask & IBV_QP_PATH_MTU)
@@ -1200,6 +1214,11 @@ int bnxt_re_post_send(struct ibv_qp *ibvqp, struct ibv_send_wr *wr,
 						  qp->cap.sqsig);
 		switch (wr->opcode) {
 		case IBV_WR_SEND_WITH_IMM:
+			/* Since our h/w is LE and user supplies raw-data in
+			 * BE format. Swapping on incoming data is needed.
+			 * On a BE platform htole32 will do the swap while on
+			 * LE platform be32toh will do the job.
+			 */
 			hdr->key_immd = htole32(be32toh(wr->imm_data));
 		case IBV_WR_SEND:
 			if (qp->qptyp == IBV_QPT_UD)
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] libbnxt_re: reset head and tail when moving to RST

[Jason Gunthorpe]

On Fri, May 12, 2017 at 06:24:26AM -0400, Devesh Sharma wrote:

>  		ibvwc->wc_flags |= IBV_WC_WITH_IMM;
> +		/* Completion reports the raw-data in LE format, While
> +		 * user expects it in BE format. Thus, swapping on outgoing
> +		 * data is needed. On a BE platform le32toh will do the swap
> +		 * while on LE platform htobe32 will do the job.
> +		 */
>  		ibvwc->imm_data = htobe32(le32toh(rcqe->imm_key));

Just to be clear, getting this right is an interop requirement.

Code like the above means the hardware takes byte 0 of the ImmDt
header and writes to byte 3 of rcqe->imm_key.

verbs requires that byte 0 of the ImmDt header is written to byte 0 of
wc->imm_data

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] libbnxt_re: Critical bug fix series

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]

On Fri, May 12, 2017 at 06:24:21AM -0400, Devesh Sharma wrote:
> These are set of bug fix patches for the libbnxt_re user
> space RDMA provider library. This patch series goes
> hand-in-hand with recent bnxt_re kernel driver bug fixes.
>
> Devesh Sharma (2):
>   libbnxt_re: unmap DB page during uninit ucontext
>   libbnxt_re: reset head and tail when moving to RST
>
> Somnath Kotur (3):
>   libbnxt_re: sq needs to be augmented by 128B
>   libbnxt_re: fix wqe size for the 0-len posting
>   libbnxt_re: move rts to rts after a threshold
>
>  providers/bnxt_re/bnxt_re-abi.h |  2 +
>  providers/bnxt_re/main.c        |  9 +++++
>  providers/bnxt_re/main.h        |  3 ++
>  providers/bnxt_re/memory.h      | 10 ++++-
>  providers/bnxt_re/verbs.c       | 81 +++++++++++++++++++++++++++++++----------
>  5 files changed, 84 insertions(+), 21 deletions(-)

Hi Davesh,

Please use "[PATCH rdma-core ..] ..." in your submissions for rdma-core
library.

This series applied.

Thanks

>
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]