[PATCH rdma-core 0/2] Support rq and cq record doorbell

[PATCH rdma-core 0/2] Support rq and cq record doorbell

[Yixian Liu]

This patch-set updates to support rq record doorbell and
cq record doorbell in the user space.

Yixian Liu (2):
  libhns: Support rq record doorbell
  libhns: Support cq record doorbell

 providers/hns/CMakeLists.txt     |   1 +
 providers/hns/hns_roce_u.h       |  25 +++++
 providers/hns/hns_roce_u_db.c    | 196 +++++++++++++++++++++++++++++++++++++++
 providers/hns/hns_roce_u_db.h    |   5 +
 providers/hns/hns_roce_u_hw_v2.c |  52 +++--------
 providers/hns/hns_roce_u_verbs.c |  38 +++++++-
 6 files changed, 274 insertions(+), 43 deletions(-)
 create mode 100644 providers/hns/hns_roce_u_db.c

-- 
1.9.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 rdma-core 1/2] libhns: Support rq record doorbell

[Yixian Liu]

This patch updates to support rq record doorbell in the
user space driver.

Signed-off-by: Yixian Liu <liuyixian-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Shaobo Xu <xushaobo2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 providers/hns/CMakeLists.txt     |   1 +
 providers/hns/hns_roce_u.h       |  25 +++++
 providers/hns/hns_roce_u_db.c    | 196 +++++++++++++++++++++++++++++++++++++++
 providers/hns/hns_roce_u_db.h    |   5 +
 providers/hns/hns_roce_u_hw_v2.c |  25 ++---
 providers/hns/hns_roce_u_verbs.c |  17 ++++
 6 files changed, 251 insertions(+), 18 deletions(-)
 create mode 100644 providers/hns/hns_roce_u_db.c

diff --git a/providers/hns/CMakeLists.txt b/providers/hns/CMakeLists.txt
index f136151..697dbd7 100644
--- a/providers/hns/CMakeLists.txt
+++ b/providers/hns/CMakeLists.txt
@@ -1,6 +1,7 @@
 rdma_provider(hns
   hns_roce_u.c
   hns_roce_u_buf.c
+  hns_roce_u_db.c
   hns_roce_u_hw_v1.c
   hns_roce_u_hw_v2.c
   hns_roce_u_verbs.c
diff --git a/providers/hns/hns_roce_u.h b/providers/hns/hns_roce_u.h
index 101320b..63b7f5c 100644
--- a/providers/hns/hns_roce_u.h
+++ b/providers/hns/hns_roce_u.h
@@ -93,6 +93,26 @@ struct hns_roce_buf {
 	unsigned int			length;
 };
 
+#define BIT_CNT_PER_BYTE       8
+
+/* the sw db length, on behalf of the qp/cq/srq length from left to right; */
+static const unsigned int db_size[] = {4, 4};
+
+/* the sw doorbell type; */
+enum hns_roce_db_type {
+	HNS_ROCE_QP_TYPE_DB,
+	HNS_ROCE_CQ_TYPE_DB,
+	HNS_ROCE_DB_TYPE_NUM
+};
+
+struct hns_roce_db_page {
+	struct hns_roce_db_page	*prev, *next;
+	struct hns_roce_buf	buf;
+	unsigned int		num_db;
+	unsigned int		use_cnt;
+	unsigned long		*bitmap;
+};
+
 struct hns_roce_context {
 	struct ibv_context		ibv_ctx;
 	void				*uar;
@@ -110,6 +130,10 @@ struct hns_roce_context {
 	int				num_qps;
 	int				qp_table_shift;
 	int				qp_table_mask;
+
+	struct hns_roce_db_page		*db_list[HNS_ROCE_DB_TYPE_NUM];
+	pthread_mutex_t			db_list_mutex;
+
 	unsigned int			max_qp_wr;
 	unsigned int			max_sge;
 	int				max_cqe;
@@ -188,6 +212,7 @@ struct hns_roce_qp {
 	unsigned int			sq_signal_bits;
 	struct hns_roce_wq		sq;
 	struct hns_roce_wq		rq;
+	unsigned int			*rdb;
 	struct hns_roce_sge_ex		sge;
 	unsigned int			next_sge;
 	int				port_num;
diff --git a/providers/hns/hns_roce_u_db.c b/providers/hns/hns_roce_u_db.c
new file mode 100644
index 0000000..f7bb41b
--- /dev/null
+++ b/providers/hns/hns_roce_u_db.c
@@ -0,0 +1,196 @@
+/*
+ * Copyright (c) 2017 Hisilicon Limited.
+ *
+ * This software is available to you under a choice of one of two
+ * licenses.  You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available from the file
+ * COPYING in the main directory of this source tree, or the
+ * OpenIB.org BSD license below:
+ *
+ *     Redistribution and use in source and binary forms, with or
+ *     without modification, are permitted provided that the following
+ *     conditions are met:
+ *
+ *      - Redistributions of source code must retain the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer.
+ *
+ *      - Redistributions in binary form must reproduce the above
+ *        copyright notice, this list of conditions and the following
+ *        disclaimer in the documentation and/or other materials
+ *        provided with the distribution.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <string.h>
+#include <stdio.h>
+#include "hns_roce_u.h"
+#include "hns_roce_u_db.h"
+
+static inline uint32_t align_size(uint32_t size, uint32_t align)
+{
+	return (uint32_t) (size + align - 1) & (~(align - 1));
+}
+
+static struct hns_roce_db_page *hns_roce_add_db_page(
+						struct hns_roce_context *ctx,
+						enum hns_roce_db_type type)
+{
+	struct hns_roce_db_page *page;
+	uint32_t bitmap_num;
+	uint32_t page_size;
+	int i;
+
+	/* alloc db page */
+	page_size = (uint32_t)to_hr_dev(ctx->ibv_ctx.device)->page_size;
+	page = (struct hns_roce_db_page *)calloc(1, sizeof(*page));
+	if (!page)
+		goto err_page;
+
+	/* allocate bitmap space for sw db */
+	page->num_db = page_size / db_size[type];
+	page->use_cnt = 0;
+	bitmap_num = align_size(page->num_db,
+				BIT_CNT_PER_BYTE * sizeof(uint64_t));
+
+	page->bitmap = calloc(1, bitmap_num / BIT_CNT_PER_BYTE);
+	if (!page->bitmap)
+		goto err_map;
+
+	/* bitmap_num indicate the mount of u64 */
+	bitmap_num = bitmap_num / (BIT_CNT_PER_BYTE * sizeof(uint64_t));
+
+	if (hns_roce_alloc_buf(&page->buf, page_size, (int)page_size))
+		goto err;
+
+	/* init the sw db bitmap */
+	for (i = 0; i < bitmap_num; ++i)
+		page->bitmap[i] = ~(0UL);
+
+	/* add the set ctx->db_list */
+	page->prev = NULL;
+	page->next = ctx->db_list[type];
+	ctx->db_list[type] = page;
+	if (page->next)
+		page->next->prev = page;
+
+	return page;
+err:
+	free(page->bitmap);
+	page->bitmap = NULL;
+
+err_map:
+	free(page);
+	page = NULL;
+
+err_page:
+	return NULL;
+}
+
+static void hns_roce_clear_db_page(struct hns_roce_db_page *page)
+{
+	if (!page) {
+		fprintf(stderr, "error clear: db page is NULL!\n");
+		return;
+	}
+
+	if (page->bitmap) {
+		free(page->bitmap);
+		page->bitmap = NULL;
+	}
+
+	hns_roce_free_buf(&page->buf);
+}
+
+void *hns_roce_alloc_db(struct hns_roce_context *ctx,
+			enum hns_roce_db_type type)
+{
+	struct hns_roce_db_page *page;
+	void *db = NULL;
+	uint32_t bit_num;
+	uint32_t i;
+
+	pthread_mutex_lock((pthread_mutex_t *)&ctx->db_list_mutex);
+
+	for (page = ctx->db_list[type]; page != NULL; page = page->next)
+		if (page->use_cnt < page->num_db)
+			goto found;
+
+	page = hns_roce_add_db_page(ctx, type);
+	if (!page)
+		goto out;
+
+found:
+	++page->use_cnt;
+	/* if the bitmap is allocated, the bitmap[i] is set zero */
+	for (i = 0; page->bitmap[i] == 0; ++i)
+		;
+
+	bit_num = (uint32_t)ffsl(page->bitmap[i]);
+	page->bitmap[i] &= ~(1ULL << (bit_num - 1));
+
+	db = (void *)((uint8_t *)(page->buf.buf) +
+			(size_t)((i * sizeof(uint64_t) * BIT_CNT_PER_BYTE +
+			(bit_num - 1)) * db_size[type]));
+
+out:
+	pthread_mutex_unlock((pthread_mutex_t *)&ctx->db_list_mutex);
+
+	return db;
+}
+
+void hns_roce_free_db(struct hns_roce_context *ctx, unsigned int *db,
+		      enum hns_roce_db_type type)
+{
+	struct hns_roce_db_page *page;
+	uint32_t bit_num;
+	uint32_t page_size;
+
+	pthread_mutex_lock((pthread_mutex_t *)&ctx->db_list_mutex);
+
+	page_size = (uint32_t)to_hr_dev(ctx->ibv_ctx.device)->page_size;
+	for (page = ctx->db_list[type]; page != NULL; page = page->next)
+		if (((uintptr_t)db & (~((uintptr_t)page_size - 1))) ==
+						(uintptr_t)(page->buf.buf))
+			goto found;
+
+	fprintf(stderr, "db page can't be found!\n");
+	goto out;
+
+found:
+	--page->use_cnt;
+	if (!page->use_cnt) {
+		if (page->prev)
+			page->prev->next = page->next;
+		else
+			ctx->db_list[type] = page->next;
+
+		if (page->next)
+			page->next->prev = page->prev;
+
+		hns_roce_clear_db_page(page);
+		free(page);
+		page = NULL;
+
+		goto out;
+	}
+
+	bit_num = (uint32_t)(((uintptr_t)db - (uintptr_t)page->buf.buf) /
+								db_size[type]);
+	page->bitmap[bit_num / (sizeof(uint64_t) * BIT_CNT_PER_BYTE)] |=
+			     ((uint64_t)1ULL) <<
+			     (bit_num % (sizeof(uint64_t) * BIT_CNT_PER_BYTE));
+
+out:
+	pthread_mutex_unlock((pthread_mutex_t *)&ctx->db_list_mutex);
+}
diff --git a/providers/hns/hns_roce_u_db.h b/providers/hns/hns_roce_u_db.h
index 76d13ce..b44e64d 100644
--- a/providers/hns/hns_roce_u_db.h
+++ b/providers/hns/hns_roce_u_db.h
@@ -51,4 +51,9 @@ static inline void hns_roce_write64(uint32_t val[2],
 	*(volatile uint64_t *) (ctx->uar + offset) = HNS_ROCE_PAIR_TO_64(val);
 }
 
+void *hns_roce_alloc_db(struct hns_roce_context *ctx,
+			enum hns_roce_db_type type);
+void hns_roce_free_db(struct hns_roce_context *ctx, unsigned int *db,
+		      enum hns_roce_db_type type);
+
 #endif /* _HNS_ROCE_U_DB_H */
diff --git a/providers/hns/hns_roce_u_hw_v2.c b/providers/hns/hns_roce_u_hw_v2.c
index 226f66d..c0e0f26 100644
--- a/providers/hns/hns_roce_u_hw_v2.c
+++ b/providers/hns/hns_roce_u_hw_v2.c
@@ -153,22 +153,9 @@ static int hns_roce_v2_wq_overflow(struct hns_roce_wq *wq, int nreq,
 	return cur + nreq >= wq->max_post;
 }
 
-static void hns_roce_update_rq_db(struct hns_roce_context *ctx,
-				  unsigned int qpn, unsigned int rq_head)
+static void hns_roce_update_rq_db(struct hns_roce_qp *qp)
 {
-	struct hns_roce_db rq_db;
-
-	rq_db.byte_4 = 0;
-	rq_db.parameter = 0;
-
-	roce_set_field(rq_db.byte_4, DB_BYTE_4_TAG_M, DB_BYTE_4_TAG_S, qpn);
-	roce_set_field(rq_db.byte_4, DB_BYTE_4_CMD_M, DB_BYTE_4_CMD_S, 0x1);
-	roce_set_field(rq_db.parameter, DB_PARAM_RQ_PRODUCER_IDX_M,
-		       DB_PARAM_RQ_PRODUCER_IDX_S, rq_head);
-
-	udma_to_device_barrier();
-
-	hns_roce_write64((uint32_t *)&rq_db, ctx, ROCEE_VF_DB_CFG0_OFFSET);
+	*qp->rdb = qp->rq.head & 0xffff;
 }
 
 static void hns_roce_update_sq_db(struct hns_roce_context *ctx,
@@ -763,7 +750,6 @@ static int hns_roce_u_v2_post_recv(struct ibv_qp *ibvqp, struct ibv_recv_wr *wr,
 	int nreq;
 	int ind;
 	struct hns_roce_qp *qp = to_hr_qp(ibvqp);
-	struct hns_roce_context *ctx = to_hr_ctx(ibvqp->context);
 	struct hns_roce_v2_wqe_data_seg *dseg;
 	struct hns_roce_rinl_sge *sge_list;
 	void *wqe;
@@ -829,8 +815,7 @@ out:
 	if (nreq) {
 		qp->rq.head += nreq;
 
-		hns_roce_update_rq_db(ctx, qp->ibv_qp.qp_num,
-				     qp->rq.head & ((qp->rq.wqe_cnt << 1) - 1));
+		hns_roce_update_rq_db(qp);
 	}
 
 	pthread_spin_unlock(&qp->rq.lock);
@@ -971,6 +956,10 @@ static int hns_roce_u_v2_destroy_qp(struct ibv_qp *ibqp)
 	hns_roce_unlock_cqs(ibqp);
 	pthread_mutex_unlock(&to_hr_ctx(ibqp->context)->qp_table_mutex);
 
+	if (qp->rq.max_gs)
+		hns_roce_free_db(to_hr_ctx(ibqp->context), qp->rdb,
+				 HNS_ROCE_QP_TYPE_DB);
+
 	hns_roce_free_buf(&qp->buf);
 	if (qp->rq_rinl_buf.wqe_list) {
 		if (qp->rq_rinl_buf.wqe_list[0].sg_list) {
diff --git a/providers/hns/hns_roce_u_verbs.c b/providers/hns/hns_roce_u_verbs.c
index 046f0a4..cde8568 100644
--- a/providers/hns/hns_roce_u_verbs.c
+++ b/providers/hns/hns_roce_u_verbs.c
@@ -41,6 +41,7 @@
 #include <ccan/minmax.h>
 #include "hns_roce_u.h"
 #include "hns_roce_u_abi.h"
+#include "hns_roce_u_db.h"
 #include "hns_roce_u_hw_v1.h"
 #include "hns_roce_u_hw_v2.h"
 
@@ -546,6 +547,19 @@ struct ibv_qp *hns_roce_u_create_qp(struct ibv_pd *pd,
 		goto err_free;
 	}
 
+	if ((to_hr_dev(pd->context->device)->hw_version != HNS_ROCE_HW_VER1) &&
+	    attr->cap.max_recv_sge) {
+		qp->rdb = hns_roce_alloc_db(context, HNS_ROCE_QP_TYPE_DB);
+		if (!qp->rdb) {
+			fprintf(stderr, "alloc rdb buffer failed!\n");
+			goto err_free;
+		}
+
+		*qp->rdb = 0;
+		cmd.db_addr = (uintptr_t) qp->rdb;
+	} else
+		cmd.db_addr = 0;
+
 	cmd.buf_addr = (uintptr_t) qp->buf.buf;
 	cmd.log_sq_stride = qp->sq.wqe_shift;
 	for (cmd.log_sq_bb_count = 0; qp->sq.wqe_cnt > 1 << cmd.log_sq_bb_count;
@@ -589,6 +603,9 @@ err_destroy:
 
 err_rq_db:
 	pthread_mutex_unlock(&to_hr_ctx(pd->context)->qp_table_mutex);
+	if ((to_hr_dev(pd->context->device)->hw_version != HNS_ROCE_HW_VER1) &&
+	    attr->cap.max_recv_sge)
+		hns_roce_free_db(context, qp->rdb, HNS_ROCE_QP_TYPE_DB);
 
 err_free:
 	free(qp->sq.wrid);
-- 
1.9.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 rdma-core 2/2] libhns: Support cq record doorbell

[Yixian Liu]

This patch updates to support cq record doorbell in
user space driver.

Signed-off-by: Yixian Liu <liuyixian-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Shaobo Xu <xushaobo2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 providers/hns/hns_roce_u_hw_v2.c | 27 +++++----------------------
 providers/hns/hns_roce_u_verbs.c | 21 ++++++++++++++++++---
 2 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/providers/hns/hns_roce_u_hw_v2.c b/providers/hns/hns_roce_u_hw_v2.c
index c0e0f26..f009839 100644
--- a/providers/hns/hns_roce_u_hw_v2.c
+++ b/providers/hns/hns_roce_u_hw_v2.c
@@ -182,25 +182,10 @@ static void hns_roce_update_sq_db(struct hns_roce_context *ctx,
 	hns_roce_write64((uint32_t *)&sq_db, ctx, ROCEE_VF_DB_CFG0_OFFSET);
 }
 
-static void hns_roce_v2_update_cq_cons_index(struct hns_roce_context *ctx,
-					     struct hns_roce_cq *cq)
+static void hns_roce_v2_update_cq_cons_index(struct hns_roce_cq *cq)
 {
-	struct hns_roce_v2_cq_db cq_db;
-
-	cq_db.byte_4 = 0;
-	cq_db.parameter = 0;
-
-	roce_set_field(cq_db.byte_4, DB_BYTE_4_TAG_M, DB_BYTE_4_TAG_S, cq->cqn);
-	roce_set_field(cq_db.byte_4, DB_BYTE_4_CMD_M, DB_BYTE_4_CMD_S, 0x3);
-
-	roce_set_field(cq_db.parameter, CQ_DB_PARAMETER_CQ_CONSUMER_IDX_M,
-		       CQ_DB_PARAMETER_CQ_CONSUMER_IDX_S,
-		       cq->cons_index & ((cq->cq_depth << 1) - 1));
-	roce_set_field(cq_db.parameter, CQ_DB_PARAMETER_CMD_SN_M,
-		       CQ_DB_PARAMETER_CMD_SN_S, 1);
-	roce_set_bit(cq_db.parameter, CQ_DB_PARAMETER_NOTIFY_S, 0);
-
-	hns_roce_write64((uint32_t *)&cq_db, ctx, ROCEE_VF_DB_CFG0_OFFSET);
+	*cq->set_ci_db = (unsigned short)(cq->cons_index &
+					  ((cq->cq_depth << 1) - 1));
 }
 
 static struct hns_roce_qp *hns_roce_v2_find_qp(struct hns_roce_context *ctx,
@@ -450,7 +435,6 @@ static int hns_roce_u_v2_poll_cq(struct ibv_cq *ibvcq, int ne,
 	int err = V2_CQ_OK;
 	struct hns_roce_qp *qp = NULL;
 	struct hns_roce_cq *cq = to_hr_cq(ibvcq);
-	struct hns_roce_context *ctx = to_hr_ctx(ibvcq->context);
 
 	pthread_spin_lock(&cq->lock);
 
@@ -463,7 +447,7 @@ static int hns_roce_u_v2_poll_cq(struct ibv_cq *ibvcq, int ne,
 	if (npolled) {
 		mmio_ordered_writes_hack();
 
-		hns_roce_v2_update_cq_cons_index(ctx, cq);
+		hns_roce_v2_update_cq_cons_index(cq);
 	}
 
 	pthread_spin_unlock(&cq->lock);
@@ -830,7 +814,6 @@ static void __hns_roce_v2_cq_clean(struct hns_roce_cq *cq, uint32_t qpn,
 	uint32_t prod_index;
 	uint8_t owner_bit = 0;
 	struct hns_roce_v2_cqe *cqe, *dest;
-	struct hns_roce_context *ctx = to_hr_ctx(cq->ibv_cq.context);
 
 	for (prod_index = cq->cons_index; get_sw_cqe_v2(cq, prod_index);
 	     ++prod_index)
@@ -856,7 +839,7 @@ static void __hns_roce_v2_cq_clean(struct hns_roce_cq *cq, uint32_t qpn,
 	if (nfreed) {
 		cq->cons_index += nfreed;
 		udma_to_device_barrier();
-		hns_roce_v2_update_cq_cons_index(ctx, cq);
+		hns_roce_v2_update_cq_cons_index(cq);
 	}
 }
 
diff --git a/providers/hns/hns_roce_u_verbs.c b/providers/hns/hns_roce_u_verbs.c
index cde8568..7037a1c 100644
--- a/providers/hns/hns_roce_u_verbs.c
+++ b/providers/hns/hns_roce_u_verbs.c
@@ -276,6 +276,16 @@ struct ibv_cq *hns_roce_u_create_cq(struct ibv_context *context, int cqe,
 
 	cmd.buf_addr = (uintptr_t) cq->buf.buf;
 
+	if (to_hr_dev(context->device)->hw_version != HNS_ROCE_HW_VER1) {
+		cq->set_ci_db = hns_roce_alloc_db(to_hr_ctx(context),
+						  HNS_ROCE_CQ_TYPE_DB);
+		if (!cq->set_ci_db) {
+			fprintf(stderr, "alloc cq db buffer failed!\n");
+			goto err_buf;
+		}
+		cmd.db_addr  = (uintptr_t) cq->set_ci_db;
+	}
+
 	ret = ibv_cmd_create_cq(context, cqe, channel, comp_vector,
 				&cq->ibv_cq, &cmd.ibv_cmd, sizeof(cmd),
 				&resp.ibv_resp, sizeof(resp));
@@ -287,9 +297,6 @@ struct ibv_cq *hns_roce_u_create_cq(struct ibv_context *context, int cqe,
 
 	if (to_hr_dev(context->device)->hw_version == HNS_ROCE_HW_VER1)
 		cq->set_ci_db = to_hr_ctx(context)->cq_tptr_base + cq->cqn * 2;
-	else
-		cq->set_ci_db = to_hr_ctx(context)->uar +
-				ROCEE_VF_DB_CFG0_OFFSET;
 
 	cq->arm_db    = cq->set_ci_db;
 	cq->arm_sn    = 1;
@@ -299,6 +306,11 @@ struct ibv_cq *hns_roce_u_create_cq(struct ibv_context *context, int cqe,
 	return &cq->ibv_cq;
 
 err_db:
+	if (to_hr_dev(context->device)->hw_version != HNS_ROCE_HW_VER1)
+		hns_roce_free_db(to_hr_ctx(context), cq->set_ci_db,
+				 HNS_ROCE_CQ_TYPE_DB);
+
+err_buf:
 	hns_roce_free_buf(&cq->buf);
 
 err:
@@ -320,6 +332,9 @@ int hns_roce_u_destroy_cq(struct ibv_cq *cq)
 	if (ret)
 		return ret;
 
+	if (to_hr_dev(cq->context->device)->hw_version != HNS_ROCE_HW_VER1)
+		hns_roce_free_db(to_hr_ctx(cq->context),
+				 to_hr_cq(cq)->set_ci_db, HNS_ROCE_CQ_TYPE_DB);
 	hns_roce_free_buf(&to_hr_cq(cq)->buf);
 	free(to_hr_cq(cq));
 
-- 
1.9.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 rdma-core 2/2] libhns: Support cq record doorbell

[Jason Gunthorpe]

On Thu, Jan 18, 2018 at 10:36:01AM +0800, Yixian Liu wrote:

> diff --git a/providers/hns/hns_roce_u_verbs.c b/providers/hns/hns_roce_u_verbs.c
> index cde8568..7037a1c 100644
> +++ b/providers/hns/hns_roce_u_verbs.c
> @@ -276,6 +276,16 @@ struct ibv_cq *hns_roce_u_create_cq(struct ibv_context *context, int cqe,
>  
>  	cmd.buf_addr = (uintptr_t) cq->buf.buf;
>  
> +	if (to_hr_dev(context->device)->hw_version != HNS_ROCE_HW_VER1) {
> +		cq->set_ci_db = hns_roce_alloc_db(to_hr_ctx(context),
> +						  HNS_ROCE_CQ_TYPE_DB);
> +		if (!cq->set_ci_db) {
> +			fprintf(stderr, "alloc cq db buffer failed!\n");
> +			goto err_buf;
> +		}
> +		cmd.db_addr  = (uintptr_t) cq->set_ci_db;

Uhh.. why does the userspace already have the 'db_addr' member of
hns_roce_create_cq when the kernel doesn't?

What is going on here? How does forward and backwards compatibility of
the kABI work?

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 rdma-core 2/2] libhns: Support cq record doorbell

[Liuyixian (Eason)]

On 2018/1/18 12:37, Jason Gunthorpe wrote:
> On Thu, Jan 18, 2018 at 10:36:01AM +0800, Yixian Liu wrote:
> 
>> diff --git a/providers/hns/hns_roce_u_verbs.c b/providers/hns/hns_roce_u_verbs.c
>> index cde8568..7037a1c 100644
>> +++ b/providers/hns/hns_roce_u_verbs.c
>> @@ -276,6 +276,16 @@ struct ibv_cq *hns_roce_u_create_cq(struct ibv_context *context, int cqe,
>>  
>>  	cmd.buf_addr = (uintptr_t) cq->buf.buf;
>>  
>> +	if (to_hr_dev(context->device)->hw_version != HNS_ROCE_HW_VER1) {
>> +		cq->set_ci_db = hns_roce_alloc_db(to_hr_ctx(context),
>> +						  HNS_ROCE_CQ_TYPE_DB);
>> +		if (!cq->set_ci_db) {
>> +			fprintf(stderr, "alloc cq db buffer failed!\n");
>> +			goto err_buf;
>> +		}
>> +		cmd.db_addr  = (uintptr_t) cq->set_ci_db;
> 
> Uhh.. why does the userspace already have the 'db_addr' member of
> hns_roce_create_cq when the kernel doesn't?
> 
> What is going on here? How does forward and backwards compatibility of
> the kABI work?
> 
> Jason
>
I have checked the history log, it seems that we have missed to add 'db_addr'
for the kernel when adding it for the userspace.
Up to now, we haven't referred this field in current driver both in kernel
and userspace, that's why we haven't found this bug.

Thanks for your doubt!

Eason








--
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 rdma-core 2/2] libhns: Support cq record doorbell

[Jason Gunthorpe]

On Thu, Jan 18, 2018 at 03:25:04PM +0800, Liuyixian (Eason) wrote:
> 
> 
> On 2018/1/18 12:37, Jason Gunthorpe wrote:
> > On Thu, Jan 18, 2018 at 10:36:01AM +0800, Yixian Liu wrote:
> > 
> >> diff --git a/providers/hns/hns_roce_u_verbs.c b/providers/hns/hns_roce_u_verbs.c
> >> index cde8568..7037a1c 100644
> >> +++ b/providers/hns/hns_roce_u_verbs.c
> >> @@ -276,6 +276,16 @@ struct ibv_cq *hns_roce_u_create_cq(struct ibv_context *context, int cqe,
> >>  
> >>  	cmd.buf_addr = (uintptr_t) cq->buf.buf;
> >>  
> >> +	if (to_hr_dev(context->device)->hw_version != HNS_ROCE_HW_VER1) {
> >> +		cq->set_ci_db = hns_roce_alloc_db(to_hr_ctx(context),
> >> +						  HNS_ROCE_CQ_TYPE_DB);
> >> +		if (!cq->set_ci_db) {
> >> +			fprintf(stderr, "alloc cq db buffer failed!\n");
> >> +			goto err_buf;
> >> +		}
> >> +		cmd.db_addr  = (uintptr_t) cq->set_ci_db;
> > 
> > Uhh.. why does the userspace already have the 'db_addr' member of
> > hns_roce_create_cq when the kernel doesn't?
> > 
> > What is going on here? How does forward and backwards compatibility of
> > the kABI work?
> > 
> > Jason
> >
> I have checked the history log, it seems that we have missed to add 'db_addr'
> for the kernel when adding it for the userspace.
> Up to now, we haven't referred this field in current driver both in kernel
> and userspace, that's why we haven't found this bug.
> 
> Thanks for your doubt!

Please explain how forward and backwards compatibility of
the kABI will work with this new db_addr capability.

Normally we'd rely on the kernel seeing that the udata is longer to
signal that userspace supports an optional feature, but that is broken
in this case.

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 rdma-core 2/2] libhns: Support cq record doorbell

[Liuyixian (Eason)]

On 2018/1/19 0:19, Jason Gunthorpe wrote:
> On Thu, Jan 18, 2018 at 03:25:04PM +0800, Liuyixian (Eason) wrote:
>>
>>
>> On 2018/1/18 12:37, Jason Gunthorpe wrote:
>>> On Thu, Jan 18, 2018 at 10:36:01AM +0800, Yixian Liu wrote:
>>>
>>>> diff --git a/providers/hns/hns_roce_u_verbs.c b/providers/hns/hns_roce_u_verbs.c
>>>> index cde8568..7037a1c 100644
>>>> +++ b/providers/hns/hns_roce_u_verbs.c
>>>> @@ -276,6 +276,16 @@ struct ibv_cq *hns_roce_u_create_cq(struct ibv_context *context, int cqe,
>>>>  
>>>>  	cmd.buf_addr = (uintptr_t) cq->buf.buf;
>>>>  
>>>> +	if (to_hr_dev(context->device)->hw_version != HNS_ROCE_HW_VER1) {
>>>> +		cq->set_ci_db = hns_roce_alloc_db(to_hr_ctx(context),
>>>> +						  HNS_ROCE_CQ_TYPE_DB);
>>>> +		if (!cq->set_ci_db) {
>>>> +			fprintf(stderr, "alloc cq db buffer failed!\n");
>>>> +			goto err_buf;
>>>> +		}
>>>> +		cmd.db_addr  = (uintptr_t) cq->set_ci_db;
>>>
>>> Uhh.. why does the userspace already have the 'db_addr' member of
>>> hns_roce_create_cq when the kernel doesn't?
>>>
>>> What is going on here? How does forward and backwards compatibility of
>>> the kABI work?
>>>
>>> Jason
>>>
>> I have checked the history log, it seems that we have missed to add 'db_addr'
>> for the kernel when adding it for the userspace.
>> Up to now, we haven't referred this field in current driver both in kernel
>> and userspace, that's why we haven't found this bug.
>>
>> Thanks for your doubt!
> 
> Please explain how forward and backwards compatibility of
> the kABI will work with this new db_addr capability.
> 
> Normally we'd rely on the kernel seeing that the udata is longer to
> signal that userspace supports an optional feature, but that is broken
> in this case.
> 
> Jason
> 
> 
Hi Jason,

In my understand, there are two possible aspects when you mentioned compatibility
for the new kernel driver with db_addr field:
1. the new kernel driver needed to support old user application using old userspace driver.
2. whether the new kernel driver can work well with the new userspace driver or not.

I am not quite sure about which case you care about.
For case 1, the new kernel driver can't support old user application or old userspace driver.
Because we use the record db solution (what this patchset does and why add new db_addr in kernel)
to substitute with operating register, which used by the old userspace driver.
Therefore, users need to update the userspace driver to the version which support record db.

For case 2, after we add db_addr field in the struct of hns_roce_ib_create_cq for the kernel,
it will have **the same length** with the userspace, that will be ok if we run an application
based on the new kernel and userspace driver, which both support record db.

Eason


--
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 rdma-core 2/2] libhns: Support cq record doorbell

[Jason Gunthorpe]

On Fri, Jan 19, 2018 at 07:04:42PM +0800, Liuyixian (Eason) wrote:

> For case 1, the new kernel driver can't support old user application or old userspace driver.
> Because we use the record db solution (what this patchset does and why add new db_addr in kernel)
> to substitute with operating register, which used by the old userspace driver.
> Therefore, users need to update the userspace driver to the version which support record db.

This is not acceptable for the kernel. You must maintain compatability
with old userspace.

Also, new userspace must work with older kernels.

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 rdma-core 2/2] libhns: Support cq record doorbell

[Liuyixian (Eason)]

On 2018/1/20 4:00, Jason Gunthorpe wrote:
> On Fri, Jan 19, 2018 at 07:04:42PM +0800, Liuyixian (Eason) wrote:
> 
>> For case 1, the new kernel driver can't support old user application or old userspace driver.
>> Because we use the record db solution (what this patchset does and why add new db_addr in kernel)
>> to substitute with operating register, which used by the old userspace driver.
>> Therefore, users need to update the userspace driver to the version which support record db.
> 
> This is not acceptable for the kernel. You must maintain compatability
> with old userspace.
> 
> Also, new userspace must work with older kernels.
> 
> Jason

Hi Jason,

Maybe my previous email caused some confusions to people.
Let me illustrate the compatibility more clear.

For hip06, which not support record db, even while it creates cq in userspace
with hns_roce_create_cq (db_addr already there), it will do nothing with db_addr.
The kernel also will not access db_addr, both for old kernel and new kernel(adding db_addr).
Because hip06 pushes db by operating register, this manner keeps unchanged.
Thus, the old/new kernel support both old/new userspace for hip06,

For hip08, which is still under development to support record db, considering
forward compatibility that new kernel with old userspace, as old userspace operates
register and new kernel still provide those registers, there is no forward compatibility
problem. The reverse (backward compatibility) is not true. It is not possible to take
new userspace will still run correctly if the kernel is old. As new userspace use
db_addr substitute operating register, it can run correctly only on new kernel
(the patchset goes with this patchset submitted to for-next) and later kernel versions.
I think it is reasonable for this case.

With carefully release-management, there would be no compatibility problem
both for hip06 and hip08.

Eason

> 
> 


--
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 rdma-core 2/2] libhns: Support cq record doorbell

[Jason Gunthorpe]

On Sat, Jan 20, 2018 at 07:41:03PM +0800, Liuyixian (Eason) wrote:

> new userspace will still run correctly if the kernel is old. As new userspace use
> db_addr substitute operating register, it can run correctly only on new kernel
> (the patchset goes with this patchset submitted to for-next) and later kernel versions.
> I think it is reasonable for this case.

Still not OK, we expect the user space to work with old kernels too.

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 rdma-core 2/2] libhns: Support cq record doorbell

[Leon Romanovsky]

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

On Sat, Jan 20, 2018 at 07:41:03PM +0800, Liuyixian (Eason) wrote:
>

<...>
>
> With carefully release-management, there would be no compatibility problem
> both for hip06 and hip08.

Sorry, for interrupting your discussion, but what does it mean "carefully
release-management"?

Thanks

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

Re: [PATCH rdma-core 2/2] libhns: Support cq record doorbell

[Liuyixian (Eason)]

On 2018/1/21 14:57, Leon Romanovsky wrote:
> On Sat, Jan 20, 2018 at 07:41:03PM +0800, Liuyixian (Eason) wrote:
>>
> 
> <...>
>>
>> With carefully release-management, there would be no compatibility problem
>> both for hip06 and hip08.
> 
> Sorry, for interrupting your discussion, but what does it mean "carefully
> release-management"?
> 
> Thanks
> 

Hi Leon,

Here release-management mainly means that our driver (kernel and userspace)
for hisilicon SoC is released progressively with linux kernel versions. Take
hip08 for example, our first version Roce driver for hip08 appear in linux
kernel 4.14, as more and more features are added in, some features may be
supported only in 4.16. I call this process as a release-management for
hisilicon SoC.

Eason

--
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 rdma-core 2/2] libhns: Support cq record doorbell

[Leon Romanovsky]

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

On Mon, Jan 22, 2018 at 10:01:32PM +0800, Liuyixian (Eason) wrote:
>
>
> On 2018/1/21 14:57, Leon Romanovsky wrote:
> > On Sat, Jan 20, 2018 at 07:41:03PM +0800, Liuyixian (Eason) wrote:
> >>
> >
> > <...>
> >>
> >> With carefully release-management, there would be no compatibility problem
> >> both for hip06 and hip08.
> >
> > Sorry, for interrupting your discussion, but what does it mean "carefully
> > release-management"?
> >
> > Thanks
> >
>
> Hi Leon,
>
> Here release-management mainly means that our driver (kernel and userspace)
> for hisilicon SoC is released progressively with linux kernel versions. Take
> hip08 for example, our first version Roce driver for hip08 appear in linux
> kernel 4.14, as more and more features are added in, some features may be
> supported only in 4.16. I call this process as a release-management for
> hisilicon SoC.

Are you aware that rdma-core and kernel are independent and can be
installed in any combinations of old/new kernel/library mix?

Thanks

>
> Eason
>
> --
> 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 --]

Re: [PATCH rdma-core 2/2] libhns: Support cq record doorbell

[Liuyixian (Eason)]

On 2018/1/22 23:13, Leon Romanovsky wrote:
> On Mon, Jan 22, 2018 at 10:01:32PM +0800, Liuyixian (Eason) wrote:
>>
>>
>> On 2018/1/21 14:57, Leon Romanovsky wrote:
>>> On Sat, Jan 20, 2018 at 07:41:03PM +0800, Liuyixian (Eason) wrote:
>>>>
>>>
>>> <...>
>>>>
>>>> With carefully release-management, there would be no compatibility problem
>>>> both for hip06 and hip08.
>>>
>>> Sorry, for interrupting your discussion, but what does it mean "carefully
>>> release-management"?
>>>
>>> Thanks
>>>
>>
>> Hi Leon,
>>
>> Here release-management mainly means that our driver (kernel and userspace)
>> for hisilicon SoC is released progressively with linux kernel versions. Take
>> hip08 for example, our first version Roce driver for hip08 appear in linux
>> kernel 4.14, as more and more features are added in, some features may be
>> supported only in 4.16. I call this process as a release-management for
>> hisilicon SoC.
> 
> Are you aware that rdma-core and kernel are independent and can be
> installed in any combinations of old/new kernel/library mix?
> 
> Thanks
> 
Hi Leon,

Yes, I have already consider the mix case in my previous email responded to Jason.

The following table illustrates our compatibility cases.

+-----------------------+-------------------------------+
|			|	rdma-core release	|
|			+-------------------------------+
|			|   release 12	|   release 16	|
|			|     (old)	|     (new)	|
+-------+---------------+---------------+---------------+
| 	|     v4.9	|   compatible	| NOT compatible|
|kernel	|     (old)	|    (hip06)	|    (hip06)	|
|version|---------------+---------------+---------------+
|	|     v4.14	|   compatible	|  compatible	|
|	|     (new)	|    (hip06)	| (hip06/hip08)	|
+-------+---------------+---------------+---------------+

The above table illustrates that old/new kernel supports old rdma-core,
while the new rdma-core needs to update the kernel (driver) to new version.

I think this release-management strategy is reasonable.	What's your opinion?

Eason



--
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 rdma-core 2/2] libhns: Support cq record doorbell

[Liuyixian (Eason)]

On 2018/1/21 0:46, Jason Gunthorpe wrote:
> On Sat, Jan 20, 2018 at 07:41:03PM +0800, Liuyixian (Eason) wrote:
> 
>> new userspace will still run correctly if the kernel is old. As new userspace use
>> db_addr substitute operating register, it can run correctly only on new kernel
>> (the patchset goes with this patchset submitted to for-next) and later kernel versions.
>> I think it is reasonable for this case.
> 
> Still not OK, we expect the user space to work with old kernels too.
> 

Hi Jason,

I just send an email to Leon for further illustrating my consideration
on the compatibility problem.

Could you please give me further comments on that email?  If you don't
agree with what I said in that email, can you give some suggestions
to resolve compatibility issue?

Thanks
Eason

--
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 rdma-core 2/2] libhns: Support cq record doorbell

[Jason Gunthorpe]

On Tue, Jan 23, 2018 at 08:51:09PM +0800, Liuyixian (Eason) wrote:
> Could you please give me further comments on that email?  If you don't
> agree with what I said in that email, can you give some suggestions
> to resolve compatibility issue?

You can't have a not compatible in your table.

You need to add some kind of mechanism for userspace to enable the new
features conditionally.

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 rdma-core 2/2] libhns: Support cq record doorbell

[Liuyixian (Eason)]

On 2018/1/23 23:04, Jason Gunthorpe wrote:
> On Tue, Jan 23, 2018 at 08:51:09PM +0800, Liuyixian (Eason) wrote:
>> Could you please give me further comments on that email?  If you don't
>> agree with what I said in that email, can you give some suggestions
>> to resolve compatibility issue?
> 
> You can't have a not compatible in your table.
> 
> You need to add some kind of mechanism for userspace to enable the new
> features conditionally.
> 
> Jason
> 
Thanks, I will consider your suggestion in next version patch set.

Eason

--
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 rdma-core 2/2] libhns: Support cq record doorbell

[Leon Romanovsky]

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

On Wed, Jan 24, 2018 at 06:35:20PM +0800, Liuyixian (Eason) wrote:
>
>
> On 2018/1/23 23:04, Jason Gunthorpe wrote:
> > On Tue, Jan 23, 2018 at 08:51:09PM +0800, Liuyixian (Eason) wrote:
> >> Could you please give me further comments on that email?  If you don't
> >> agree with what I said in that email, can you give some suggestions
> >> to resolve compatibility issue?
> >
> > You can't have a not compatible in your table.
> >
> > You need to add some kind of mechanism for userspace to enable the new
> > features conditionally.
> >
> > Jason
> >
> Thanks, I will consider your suggestion in next version patch set.

ABI_VERSION cam help you to achieve that.

Thanks

>
> Eason
>

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

Re: [PATCH rdma-core 2/2] libhns: Support cq record doorbell

[Jason Gunthorpe]

On Wed, Jan 24, 2018 at 01:51:41PM +0200, Leon Romanovsky wrote:
> On Wed, Jan 24, 2018 at 06:35:20PM +0800, Liuyixian (Eason) wrote:
> >
> >
> > On 2018/1/23 23:04, Jason Gunthorpe wrote:
> > > On Tue, Jan 23, 2018 at 08:51:09PM +0800, Liuyixian (Eason) wrote:
> > >> Could you please give me further comments on that email?  If you don't
> > >> agree with what I said in that email, can you give some suggestions
> > >> to resolve compatibility issue?
> > >
> > > You can't have a not compatible in your table.
> > >
> > > You need to add some kind of mechanism for userspace to enable the new
> > > features conditionally.
> > >
> > > Jason
> > >
> > Thanks, I will consider your suggestion in next version patch set.
> 
> ABI_VERSION cam help you to achieve that.

ABI_VERSION is mostly totally broken in verbs and breaks
compatability with. It is a last emergency resort.

Probably in this case you just need to lengthen the response udata
struct and the kernel can detect if the longer struct is provided to
enable the feature and userspace can detect !0 to determine if the
kernel supports it.

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 rdma-core 2/2] libhns: Support cq record doorbell

[Liuyixian (Eason)]

On 2018/1/25 0:30, Jason Gunthorpe wrote:
> On Wed, Jan 24, 2018 at 01:51:41PM +0200, Leon Romanovsky wrote:
>> On Wed, Jan 24, 2018 at 06:35:20PM +0800, Liuyixian (Eason) wrote:
>>>
>>>
>>> On 2018/1/23 23:04, Jason Gunthorpe wrote:
>>>> On Tue, Jan 23, 2018 at 08:51:09PM +0800, Liuyixian (Eason) wrote:
>>>>> Could you please give me further comments on that email?  If you don't
>>>>> agree with what I said in that email, can you give some suggestions
>>>>> to resolve compatibility issue?
>>>>
>>>> You can't have a not compatible in your table.
>>>>
>>>> You need to add some kind of mechanism for userspace to enable the new
>>>> features conditionally.
>>>>
>>>> Jason
>>>>
>>> Thanks, I will consider your suggestion in next version patch set.
>>
>> ABI_VERSION cam help you to achieve that.
> 
> ABI_VERSION is mostly totally broken in verbs and breaks
> compatability with. It is a last emergency resort.
> 
> Probably in this case you just need to lengthen the response udata
> struct and the kernel can detect if the longer struct is provided to
> enable the feature and userspace can detect !0 to determine if the
> kernel supports it.
> 
> Jason

Okay, thanks both of you! I will fix the compatibility problem ASAP.

--
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