[PATCH] nvme-auth: retry command if DNR bit is not set

[PATCH] nvme-auth: retry command if DNR bit is not set

[Hannes Reinecke]

If the cqe returns a status with the DNR bit not set we should
retry the command; otherwise we might incur spurious failures.

Reported-by: Martin George <marting@netapp.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/nvme/host/auth.c | 9 ++++++---
 drivers/nvme/host/core.c | 3 ++-
 drivers/nvme/host/nvme.h | 2 ++
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
index c8a6db7c4498..18f3765a387f 100644
--- a/drivers/nvme/host/auth.c
+++ b/drivers/nvme/host/auth.c
@@ -53,7 +53,7 @@ static int nvme_auth_submit(struct nvme_ctrl *ctrl, int qid,
 	struct nvme_command cmd = {};
 	blk_mq_req_flags_t flags = nvme_auth_flags_from_qid(qid);
 	struct request_queue *q = nvme_auth_queue_from_qid(ctrl, qid);
-	int ret;
+	int ret, retries = nvme_max_retries;
 
 	cmd.auth_common.opcode = nvme_fabrics_command;
 	cmd.auth_common.secp = NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER;
@@ -67,13 +67,16 @@ static int nvme_auth_submit(struct nvme_ctrl *ctrl, int qid,
 		cmd.auth_receive.al = cpu_to_le32(data_len);
 	}
 
+retry:
 	ret = __nvme_submit_sync_cmd(q, &cmd, NULL, data, data_len,
 				     qid == 0 ? NVME_QID_ANY : qid,
 				     0, flags);
-	if (ret > 0)
+	if (ret > 0) {
+		if (!(ret & NVME_SC_DNR) && --retries)
+			goto retry;
 		dev_warn(ctrl->device,
 			"qid %d auth_send failed with status %d\n", qid, ret);
-	else if (ret < 0)
+	} else if (ret < 0)
 		dev_err(ctrl->device,
 			"qid %d auth_send failed with error %d\n", qid, ret);
 	return ret;
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 2429b11eb9a8..21f0fd5e1ce7 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -54,9 +54,10 @@ static unsigned char shutdown_timeout = 5;
 module_param(shutdown_timeout, byte, 0644);
 MODULE_PARM_DESC(shutdown_timeout, "timeout in seconds for controller shutdown");
 
-static u8 nvme_max_retries = 5;
+u8 nvme_max_retries = 5;
 module_param_named(max_retries, nvme_max_retries, byte, 0644);
 MODULE_PARM_DESC(max_retries, "max number of retries a command may have");
+EXPORT_SYMBOL_GPL(nvme_max_retries);
 
 static unsigned long default_ps_max_latency_us = 100000;
 module_param(default_ps_max_latency_us, ulong, 0644);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index bdc0ff7ed9ab..ddf42cdb19a7 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -25,6 +25,8 @@ extern unsigned int nvme_io_timeout;
 extern unsigned int admin_timeout;
 #define NVME_ADMIN_TIMEOUT	(admin_timeout * HZ)
 
+extern u8 nvme_max_retries;
+
 #define NVME_DEFAULT_KATO	5
 
 #ifdef CONFIG_ARCH_NO_SG_CHAIN
-- 
2.35.3

Re: [PATCH] nvme-auth: retry command if DNR bit is not set

[Chaitanya Kulkarni]

On 7/26/22 23:11, Hannes Reinecke wrote:
> If the cqe returns a status with the DNR bit not set we should
> retry the command; otherwise we might incur spurious failures.
> 
> Reported-by: Martin George <marting@netapp.com>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH] nvme-auth: retry command if DNR bit is not set

[Christoph Hellwig]

On Wed, Jul 27, 2022 at 08:11:44AM +0200, Hannes Reinecke wrote:
> If the cqe returns a status with the DNR bit not set we should
> retry the command; otherwise we might incur spurious failures.

If we want to start retrying passthrough commands we should do
that properly by setting a flag in the nvme_request and use the
normal retry code instead of open coding that in the callers.