[RFC PATCH] nvmf: allow user to choose cid-genctr feature

[RFC PATCH] nvmf: allow user to choose cid-genctr feature

[Chaitanya Kulkarni]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="y", Size: 3362 bytes --]

From: Chaitanya Kulkarni <kch@nvidia.com>

Some fabrics controllers use the command id as an index to
implement specific data structures and will fail if the value is
out of bounds. The nvme driver's recently introduced command sequence
number breaks these controllers.
 
Allow the user to choose the use of the genctr in calculating the command
id when connecting to the fabrics controller.
 
The driver will not have the ability to detect bad completions when
this flag is set to false for a ctrl, but we weren't previously
checking this anyway.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
Hi,
 
Please let know if this approach is acceptable unless there is better one.

The one side effect I see is bloating the nvmf controller options but I
think it is still reasonable.

Also, we can reverse the name of the param from use_genctr ->
skip_genctr to match the NVME_QUIRK_SKIP_CID_GEN macro name along with
the respective logic.

-ck
---
 drivers/nvme/host/fabrics.c | 4 ++++
 drivers/nvme/host/fabrics.h | 2 ++
 drivers/nvme/host/rdma.c    | 3 +++
 drivers/nvme/host/tcp.c     | 3 +++
 4 files changed, 12 insertions(+)

diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index c5a2b71c5268..6e7dafaff750 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -549,6 +549,7 @@ static const match_table_t opt_tokens = {
 	{ NVMF_OPT_TOS,			"tos=%d"		},
 	{ NVMF_OPT_FAIL_FAST_TMO,	"fast_io_fail_tmo=%d"	},
 	{ NVMF_OPT_DISCOVERY,		"discovery"		},
+	{ NVMF_OPT_USE_GENCTR,		"use_genctr"		},
 	{ NVMF_OPT_ERR,			NULL			}
 };
 
@@ -772,6 +773,9 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,
 			}
 			kfree(p);
 			break;
+		case NVMF_OPT_USE_GENCTR:
+			opts->use_genctr = true;
+			break;
 		case NVMF_OPT_DUP_CONNECT:
 			opts->duplicate_connect = true;
 			break;
diff --git a/drivers/nvme/host/fabrics.h b/drivers/nvme/host/fabrics.h
index c3203ff1c654..8fd845fba386 100644
--- a/drivers/nvme/host/fabrics.h
+++ b/drivers/nvme/host/fabrics.h
@@ -68,6 +68,7 @@ enum {
 	NVMF_OPT_FAIL_FAST_TMO	= 1 << 20,
 	NVMF_OPT_HOST_IFACE	= 1 << 21,
 	NVMF_OPT_DISCOVERY	= 1 << 22,
+	NVMF_OPT_USE_GENCTR	= 1 << 23,
 };
 
 /**
@@ -128,6 +129,7 @@ struct nvmf_ctrl_options {
 	unsigned int		nr_poll_queues;
 	int			tos;
 	int			fast_io_fail_tmo;
+	bool			use_genctr;
 };
 
 /*
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 9317f26e51e0..41a170c77e6f 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -2333,6 +2333,9 @@ static struct nvme_ctrl *nvme_rdma_create_ctrl(struct device *dev,
 	ctrl->ctrl.opts = opts;
 	INIT_LIST_HEAD(&ctrl->list);
 
+	if (!opts->use_genctr)
+		ctrl->ctrl.quirks |= NVME_QUIRK_SKIP_CID_GEN;
+
 	if (!(opts->mask & NVMF_OPT_TRSVCID)) {
 		opts->trsvcid =
 			kstrdup(__stringify(NVME_RDMA_IP_PORT), GFP_KERNEL);
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 07156ea9d1a8..1ca2625ffdef 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -2536,6 +2536,9 @@ static struct nvme_ctrl *nvme_tcp_create_ctrl(struct device *dev,
 		goto out_free_ctrl;
 	}
 
+	if (!opts->use_genctr)
+		ctrl->ctrl.quirks |= NVME_QUIRK_SKIP_CID_GEN;
+
 	if (opts->mask & NVMF_OPT_HOST_TRADDR) {
 		ret = inet_pton_with_scope(&init_net, AF_UNSPEC,
 			opts->host_traddr, NULL, &ctrl->src_addr);
-- 
2.29.0

Re: [RFC PATCH] nvmf: allow user to choose cid-genctr feature

[Chaitanya Kulkarni]

On 11/1/2021 4:17 PM, Chaitanya Kulkarni wrote:
> From: Chaitanya Kulkarni <kch@nvidia.com>
> 
> Some fabrics controllers use the command id as an index to
> implement specific data structures and will fail if the value is
> out of bounds. The nvme driver's recently introduced command sequence
> number breaks these controllers.
>   
> Allow the user to choose the use of the genctr in calculating the command
> id when connecting to the fabrics controller.
>   
> The driver will not have the ability to detect bad completions when
> this flag is set to false for a ctrl, but we weren't previously
> checking this anyway.
> 
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.co

Please ignore this sorry for the noise.