On Tue, 2016-08-16@12:05 +0300, Sagi Grimberg wrote:

On 15/08/16 19:47, Jay Freyensee wrote:

quoted

Currently under debate due to spec confusion, increase hrqsize
to one more than hsqsize.

Signed-off-by: Jay Freyensee <james_p_freyensee at linux.intel.com>
---
?drivers/nvme/host/rdma.c???| 4 ++--
?drivers/nvme/target/rdma.c | 7 ++++---
?2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 6aa913e..524c2b3 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c

@@ -1289,10 +1289,10 @@ static int nvme_rdma_route_resolved(struct

nvme_rdma_queue *queue)
?	?* specified by the Fabrics standard.
?	?*/
?	if (priv.qid == 0) {
-		priv.hrqsize = cpu_to_le16(NVMF_AQ_DEPTH - 1);
+		priv.hrqsize = cpu_to_le16(NVMF_AQ_DEPTH);
?		priv.hsqsize = cpu_to_le16(NVMF_AQ_DEPTH - 1);
?	} else {
-		priv.hrqsize = cpu_to_le16(queue->ctrl-

quoted

ctrl.sqsize);

+		priv.hrqsize = cpu_to_le16(queue->ctrl-

quoted

ctrl.sqsize + 1);

?		priv.hsqsize = cpu_to_le16(queue->ctrl-

quoted

ctrl.sqsize);

?	}

diff --git a/drivers/nvme/target/rdma.c

b/drivers/nvme/target/rdma.c
index 68b7b04..3557980 100644

--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c

@@ -1004,11 +1004,12 @@ nvmet_rdma_parse_cm_connect_req(struct

rdma_conn_param *conn,
?	queue->host_qid = le16_to_cpu(req->qid);

?	/*
-	?* req->hsqsize corresponds to our recv queue size plus 1
-	?* req->hrqsize corresponds to our send queue size plus 1
+	?* req->hsqsize corresponds to our recv queue size plus 1
as
+	?* this value is based on sqsize, a 0-based value.
+	?* req->hrqsize corresponds to our send queue size plus 2
?	?*/
?	queue->recv_queue_size = le16_to_cpu(req->hsqsize) + 1;
-	queue->send_queue_size = le16_to_cpu(req->hrqsize) + 1;
+	queue->send_queue_size = le16_to_cpu(req->hrqsize) + 2;

?	if (!queue->host_qid && queue->recv_queue_size >
NVMF_AQ_DEPTH)
?		return NVME_RDMA_CM_INVALID_HSQSIZE;

Something doesn't look right here.

?From my understanding of the WG discussion. hsqsize is 0's based and
hrqsize isn't.

Not really, hrqsize being 0's based or 1's based is a bit of moot point
to the fundamental problem, the spec really doesn't explain how to use
hrqsize. ?More following.

So this host seems to do the right thing. but why does the target
inc hrqsize+2? you just allocated 2 more recv queue entries when the
host will never send you more than hrqsize.

Am I missing something?

Yah, the spec is missing the explanation of how to actually use
hrqsize. Dave Minturn agreed the NVMe committee/consortium needs to
explain how to use this field better in the fabrics spec.

Here is his summary he sent to the NVMe organization:

"The HRQSIZE description needs to explain that the host uses HRQSIZE to
resource the RDMA QP resources for the NVMe CQ, both on the host and on
the controller.??The value of HRQSIZE is based on the sizing of the
NVMe CQ which may be sized to match the size of the NVMe SQ or up to
MAXCMD, if MAXCMD is enabled and the host is using SQHD flow control."

This came out after I submitted this patch series. ?So from his
explanation, I think we can drop this patch all together and just make
NVMe SQ == hsqsize == hrqsize.

J