[PATCH] nvme-tcp: add additional info for nvme_tcp_timeout log

[Engel, Amit]

Sure, I will add it and apply the patch for your review

Thanks
Amit

-----Original Message-----
From: Sagi Grimberg <sagi at grimberg.me> 
Sent: Wednesday, 7 December 2022 17:17
To: Engel, Amit <Amit.Engel at Dell.com>; linux-nvme at lists.infradead.org
Subject: Re: [PATCH] nvme-tcp: add additional info for nvme_tcp_timeout log


[EXTERNAL EMAIL] 


> Hi Sagi,
> 
> Do you mean show_opcode_name() macro which is defined as part of TRACE_EVENT?
> 
> As far as I understand, I can't use show_opcode_name() directly 
> outside of TRACE_EVENT
> 
> Can you please share an example?

Sorry, I meant something like nvme_get_opcode_str()...

Something like (untested and needs a split):
--
diff --git a/drivers/nvme/host/constants.c b/drivers/nvme/host/constants.c index e958d5015585..eacf282f04d8 100644
--- a/drivers/nvme/host/constants.c
+++ b/drivers/nvme/host/constants.c
@@ -54,6 +54,14 @@ static const char * const nvme_admin_ops[] = {
         [nvme_admin_get_lba_status] = "Get LBA Status",
  };

+static const char * const nvme_fabrics_ops[] = {
+       [nvme_fabrics_type_property_set] = "Property Set",
+       [nvme_fabrics_type_connect] = "Connect",
+       [nvme_fabrics_type_property_get] = "Property Get",
+       [nvme_fabrics_type_auth_send] = "Authentication Send",
+       [nvme_fabrics_type_auth_receive] = "Authentication Receive", };
+
  static const char * const nvme_statuses[] = {
         [NVME_SC_SUCCESS] = "Success",
         [NVME_SC_INVALID_OPCODE] = "Invalid Command Opcode", @@ -185,3 +193,12 @@ const unsigned char *nvme_get_admin_opcode_str(u8
opcode)
                 return nvme_admin_ops[opcode];
         return "Unknown";
  }
+EXPORT_SYMBOL_GPL(nvme_get_admin_opcode_str);
+
+const unsigned char *nvme_get_fabrics_opcode_str(u8 opcode) {
+       if (opcode < ARRAY_SIZE(nvme_admin_ops) && nvme_fabrics_ops[opcode])
+               return nvme_fabrics_ops[opcode];
+       return "Unknown";
+}
+EXPORT_SYMBOL_GPL(nvme_get_fabrics_opcode_str);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h index e01fa9f696fa..56f2cec6e5b1 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -1086,19 +1086,30 @@ static inline bool nvme_multi_css(struct nvme_ctrl *ctrl)
  const unsigned char *nvme_get_error_status_str(u16 status);
  const unsigned char *nvme_get_opcode_str(u8 opcode);
  const unsigned char *nvme_get_admin_opcode_str(u8 opcode);
+const unsigned char *nvme_get_fabrics_opcode_str(u8 opcode);
  #else /* CONFIG_NVME_VERBOSE_ERRORS */
  static inline const unsigned char *nvme_get_error_status_str(u16 status)
  {
-       return "I/O Error";
+       return __stringify(status);
  }
  static inline const unsigned char *nvme_get_opcode_str(u8 opcode)
  {
-       return "I/O Cmd";
+       return __stringify(opcode);
  }
  static inline const unsigned char *nvme_get_admin_opcode_str(u8 opcode)
  {
-       return "Admin Cmd";
+       return __stringify(opcode);
+}
+static inline const unsigned char *nvme_get_fabrics_opcode_str(u8 
+opcode) {
+       return __stringify(opcode);
  }
  #endif /* CONFIG_NVME_VERBOSE_ERRORS */
-
+static inline const unsigned char *nvme_opcode_str(int qid, u8 opcode,
u8 fctype)
+{
+       if (opcode == nvme_fabrics_command)
+               return nvme_get_fabrics_opcode_str(fctype);
+       return qid ? nvme_get_opcode_str(opcode) :
+               nvme_get_admin_opcode_str(opcode);
+}
  #endif /* _NVME_H */
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 79789daddeac..77450f52c146 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -2275,10 +2275,13 @@ static enum blk_eh_timer_return nvme_tcp_timeout(struct request *rq)
         struct nvme_tcp_request *req = blk_mq_rq_to_pdu(rq);
         struct nvme_ctrl *ctrl = &req->queue->ctrl->ctrl;
         struct nvme_tcp_cmd_pdu *pdu = req->pdu;
+       u8 opcode = pdu->cmd.common.opcode, fctype =
pdu->cmd.fabrics.fctype;
+       int qid = nvme_tcp_queue_id(req->queue);

         dev_warn(ctrl->device,
-               "queue %d: timeout request %#x type %d\n",
-               nvme_tcp_queue_id(req->queue), rq->tag, pdu->hdr.type);
+               "queue %d: timeout cid %#x type %d op %s\n",
+               qid, nvme_cid(rq), pdu->hdr.type,
+               nvme_opcode_str(qid, opcode, fctype));

         if (ctrl->state != NVME_CTRL_LIVE) {
                 /*
--