[PATCHv4 0/7] nvmet: debugfs support

[PATCHv4 0/7] nvmet: debugfs support

[Hannes Reinecke]

Hi all,

taking up the original patchset for nvmet debugfs
I've improved upon that submission by modifying
the layout:

/dev/kernel/debug/nvmet
  <subsysnqn>
    ctrl<cntlid>
      port
      state
      hostnqn
      kato
      host_traddr

The 'state' attribute is the value of the
CSTS register; one can trigger a controller reset
by writing 'fatal' into it (to set CSTS.CFS).

As usual, comments and reviews are welcome.

Changes to v3:
- drop patch to add queue entries
- Reviews from Sagi

Changes to v2:
- Move the 'host_traddr' attribute to the
  controller directory
- Rename callback to 'host_traddr'

Hannes Reinecke (7):
  nvmet: add debugfs support
  nvmet: add 'host_traddr' callback for debugfs
  nvmet-tcp: implement host_traddr()
  nvmet-rdma: implement host_traddr()
  nvmet-fc: implement host_traddr()
  nvme-fcloop: implement 'host_traddr'
  lpfc_nvmet: implement 'host_traddr'

 drivers/nvme/target/Kconfig    |   9 ++
 drivers/nvme/target/Makefile   |   1 +
 drivers/nvme/target/core.c     |  30 ++++-
 drivers/nvme/target/debugfs.c  | 202 +++++++++++++++++++++++++++++++++
 drivers/nvme/target/debugfs.h  |  42 +++++++
 drivers/nvme/target/fc.c       |  33 ++++++
 drivers/nvme/target/fcloop.c   |  11 ++
 drivers/nvme/target/nvmet.h    |  12 +-
 drivers/nvme/target/rdma.c     |  12 ++
 drivers/nvme/target/tcp.c      |  14 +++
 drivers/scsi/lpfc/lpfc_nvmet.c |  11 ++
 include/linux/nvme-fc-driver.h |   4 +
 12 files changed, 378 insertions(+), 3 deletions(-)
 create mode 100644 drivers/nvme/target/debugfs.c
 create mode 100644 drivers/nvme/target/debugfs.h

-- 
2.35.3

[PATCH 1/7] nvmet: add debugfs support

[Hannes Reinecke]

Add a debugfs hierarchy to display the configured subsystems
and the controllers attached to the subsystems.

Suggested-by: Redouane BOUFENGHOUR <redouane.boufenghour@shadow.tech>
Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/nvme/target/Kconfig   |   9 ++
 drivers/nvme/target/Makefile  |   1 +
 drivers/nvme/target/core.c    |  22 +++-
 drivers/nvme/target/debugfs.c | 183 ++++++++++++++++++++++++++++++++++
 drivers/nvme/target/debugfs.h |  42 ++++++++
 drivers/nvme/target/nvmet.h   |   8 +-
 6 files changed, 262 insertions(+), 3 deletions(-)
 create mode 100644 drivers/nvme/target/debugfs.c
 create mode 100644 drivers/nvme/target/debugfs.h

diff --git a/drivers/nvme/target/Kconfig b/drivers/nvme/target/Kconfig
index 872dd1a0acd8..5740893ace6d 100644
--- a/drivers/nvme/target/Kconfig
+++ b/drivers/nvme/target/Kconfig
@@ -18,6 +18,15 @@ config NVME_TARGET
 	  To configure the NVMe target you probably want to use the nvmetcli
 	  tool from http://git.infradead.org/users/hch/nvmetcli.git.
 
+config NVME_TARGET_DEBUGFS
+        bool "NVMe Target debugfs support"
+	depends on NVME_TARGET
+	help
+	  This enables debugfs support to display the connected controllers
+	  to each subsystem
+
+	  If unsure, say N.
+
 config NVME_TARGET_PASSTHRU
 	bool "NVMe Target Passthrough support"
 	depends on NVME_TARGET
diff --git a/drivers/nvme/target/Makefile b/drivers/nvme/target/Makefile
index c66820102493..c402c44350b2 100644
--- a/drivers/nvme/target/Makefile
+++ b/drivers/nvme/target/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_NVME_TARGET_TCP)		+= nvmet-tcp.o
 
 nvmet-y		+= core.o configfs.o admin-cmd.o fabrics-cmd.o \
 			discovery.o io-cmd-file.o io-cmd-bdev.o
+nvmet-$(CONFIG_NVME_TARGET_DEBUGFS)	+= debugfs.o
 nvmet-$(CONFIG_NVME_TARGET_PASSTHRU)	+= passthru.o
 nvmet-$(CONFIG_BLK_DEV_ZONED)		+= zns.o
 nvmet-$(CONFIG_NVME_TARGET_AUTH)	+= fabrics-cmd-auth.o auth.o
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 6bbe4df0166c..7c82e33d9347 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -16,6 +16,7 @@
 #include "trace.h"
 
 #include "nvmet.h"
+#include "debugfs.h"
 
 struct kmem_cache *nvmet_bvec_cache;
 struct workqueue_struct *buffered_io_wq;
@@ -1466,6 +1467,7 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
 	mutex_lock(&subsys->lock);
 	list_add_tail(&ctrl->subsys_entry, &subsys->ctrls);
 	nvmet_setup_p2p_ns_map(ctrl, req);
+	nvmet_debugfs_ctrl_setup(ctrl);
 	mutex_unlock(&subsys->lock);
 
 	*ctrlp = ctrl;
@@ -1500,6 +1502,8 @@ static void nvmet_ctrl_free(struct kref *ref)
 
 	nvmet_destroy_auth(ctrl);
 
+	nvmet_debugfs_ctrl_free(ctrl);
+
 	ida_free(&cntlid_ida, ctrl->cntlid);
 
 	nvmet_async_events_free(ctrl);
@@ -1613,8 +1617,14 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
 	INIT_LIST_HEAD(&subsys->ctrls);
 	INIT_LIST_HEAD(&subsys->hosts);
 
+	ret = nvmet_debugfs_subsys_setup(subsys);
+	if (ret)
+		goto free_subsysnqn;
+
 	return subsys;
 
+free_subsysnqn:
+	kfree(subsys->subsysnqn);
 free_fr:
 	kfree(subsys->firmware_rev);
 free_mn:
@@ -1631,6 +1641,8 @@ static void nvmet_subsys_free(struct kref *ref)
 
 	WARN_ON_ONCE(!xa_empty(&subsys->namespaces));
 
+	nvmet_debugfs_subsys_free(subsys);
+
 	xa_destroy(&subsys->namespaces);
 	nvmet_passthru_subsys_free(subsys);
 
@@ -1684,11 +1696,18 @@ static int __init nvmet_init(void)
 	if (error)
 		goto out_free_nvmet_work_queue;
 
-	error = nvmet_init_configfs();
+	error = nvmet_init_debugfs();
 	if (error)
 		goto out_exit_discovery;
+
+	error = nvmet_init_configfs();
+	if (error)
+		goto out_exit_debugfs;
+
 	return 0;
 
+out_exit_debugfs:
+	nvmet_exit_debugfs();
 out_exit_discovery:
 	nvmet_exit_discovery();
 out_free_nvmet_work_queue:
@@ -1705,6 +1724,7 @@ static int __init nvmet_init(void)
 static void __exit nvmet_exit(void)
 {
 	nvmet_exit_configfs();
+	nvmet_exit_debugfs();
 	nvmet_exit_discovery();
 	ida_destroy(&cntlid_ida);
 	destroy_workqueue(nvmet_wq);
diff --git a/drivers/nvme/target/debugfs.c b/drivers/nvme/target/debugfs.c
new file mode 100644
index 000000000000..45c0483c54fc
--- /dev/null
+++ b/drivers/nvme/target/debugfs.c
@@ -0,0 +1,183 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DebugFS interface for the NVMe target.
+ * Copyright (c) 2022-2024 Shadow
+ * Copyright (c) 2024 SUSE LLC
+ */
+
+#include <linux/debugfs.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+
+#include "nvmet.h"
+#include "debugfs.h"
+
+struct dentry *nvmet_debugfs;
+
+#define NVMET_DEBUGFS_ATTR(field) \
+	static int field##_open(struct inode *inode, struct file *file) \
+	{ return single_open(file, field##_show, inode->i_private); } \
+	\
+	static const struct file_operations field##_fops = { \
+		.open = field##_open, \
+		.read = seq_read, \
+		.release = single_release, \
+	}
+
+#define NVMET_DEBUGFS_RW_ATTR(field) \
+	static int field##_open(struct inode *inode, struct file *file) \
+	{ return single_open(file, field##_show, inode->i_private); } \
+	\
+	static const struct file_operations field##_fops = { \
+		.open = field##_open, \
+		.read = seq_read, \
+		.write = field##_write, \
+		.release = single_release, \
+	}
+
+static int nvmet_ctrl_hostnqn_show(struct seq_file *m, void *p)
+{
+	struct nvmet_ctrl *ctrl = (struct nvmet_ctrl *)m->private;
+
+	seq_printf(m, "%s\n", ctrl->hostnqn);
+	return 0;
+}
+NVMET_DEBUGFS_ATTR(nvmet_ctrl_hostnqn);
+
+static int nvmet_ctrl_kato_show(struct seq_file *m, void *p)
+{
+	struct nvmet_ctrl *ctrl = (struct nvmet_ctrl *)m->private;
+
+	seq_printf(m, "%d\n", ctrl->kato);
+	return 0;
+}
+NVMET_DEBUGFS_ATTR(nvmet_ctrl_kato);
+
+static int nvmet_ctrl_port_show(struct seq_file *m, void *p)
+{
+	struct nvmet_ctrl *ctrl = (struct nvmet_ctrl *)m->private;
+
+	seq_printf(m, "%d\n", le16_to_cpu(ctrl->port->disc_addr.portid));
+	return 0;
+}
+NVMET_DEBUGFS_ATTR(nvmet_ctrl_port);
+
+static const char *const csts_state_names[] = {
+	[NVME_CSTS_RDY] = "ready",
+	[NVME_CSTS_CFS] = "fatal",
+	[NVME_CSTS_NSSRO] = "reset",
+	[NVME_CSTS_SHST_OCCUR] = "shutdown",
+	[NVME_CSTS_SHST_CMPLT] = "completed",
+	[NVME_CSTS_PP] = "paused",
+};
+
+static int nvmet_ctrl_state_show(struct seq_file *m, void *p)
+{
+	struct nvmet_ctrl *ctrl = (struct nvmet_ctrl *)m->private;
+	bool sep = false;
+	int i;
+
+	for (i = 0; i < 7; i++) {
+		int state = BIT(i);
+
+		if (!(ctrl->csts & state))
+			continue;
+		if (sep)
+			seq_puts(m, "|");
+		sep = true;
+		if (csts_state_names[state])
+			seq_puts(m, csts_state_names[state]);
+		else
+			seq_printf(m, "%d", state);
+	}
+	if (sep)
+		seq_printf(m, "\n");
+	return 0;
+}
+
+static ssize_t nvmet_ctrl_state_write(struct file *file, const char __user *buf,
+				      size_t count, loff_t *ppos)
+{
+	struct seq_file *m = file->private_data;
+	struct nvmet_ctrl *ctrl = m->private;
+	char reset[16];
+
+	if (count >= sizeof(reset))
+		return -EINVAL;
+	if (copy_from_user(reset, buf, count))
+		return -EFAULT;
+	if (!memcmp(reset, "fatal", 5))
+		nvmet_ctrl_fatal_error(ctrl);
+	else
+		return -EINVAL;
+	return count;
+}
+NVMET_DEBUGFS_RW_ATTR(nvmet_ctrl_state);
+
+int nvmet_debugfs_ctrl_setup(struct nvmet_ctrl *ctrl)
+{
+	char name[32];
+	struct dentry *parent = ctrl->subsys->debugfs_dir;
+	int ret;
+
+	if (!parent)
+		return -ENODEV;
+	snprintf(name, sizeof(name), "ctrl%d", ctrl->cntlid);
+	ctrl->debugfs_dir = debugfs_create_dir(name, parent);
+	if (IS_ERR(ctrl->debugfs_dir)) {
+		ret = PTR_ERR(ctrl->debugfs_dir);
+		ctrl->debugfs_dir = NULL;
+		return ret;
+	}
+	debugfs_create_file("port", S_IRUSR, ctrl->debugfs_dir, ctrl,
+			    &nvmet_ctrl_port_fops);
+	debugfs_create_file("hostnqn", S_IRUSR, ctrl->debugfs_dir, ctrl,
+			    &nvmet_ctrl_hostnqn_fops);
+	debugfs_create_file("kato", S_IRUSR, ctrl->debugfs_dir, ctrl,
+			    &nvmet_ctrl_kato_fops);
+	debugfs_create_file("state", S_IRUSR | S_IWUSR, ctrl->debugfs_dir, ctrl,
+			    &nvmet_ctrl_state_fops);
+	return 0;
+}
+
+void nvmet_debugfs_ctrl_free(struct nvmet_ctrl *ctrl)
+{
+	debugfs_remove_recursive(ctrl->debugfs_dir);
+}
+
+int nvmet_debugfs_subsys_setup(struct nvmet_subsys *subsys)
+{
+	int ret = 0;
+
+	subsys->debugfs_dir = debugfs_create_dir(subsys->subsysnqn,
+						 nvmet_debugfs);
+	if (IS_ERR(subsys->debugfs_dir)) {
+		ret = PTR_ERR(subsys->debugfs_dir);
+		subsys->debugfs_dir = NULL;
+	}
+	return ret;
+}
+
+void nvmet_debugfs_subsys_free(struct nvmet_subsys *subsys)
+{
+	debugfs_remove_recursive(subsys->debugfs_dir);
+}
+
+int __init nvmet_init_debugfs(void)
+{
+	struct dentry *parent;
+
+	parent = debugfs_create_dir("nvmet", NULL);
+	if (IS_ERR(parent)) {
+		pr_warn("%s: failed to create debugfs directory\n", "nvmet");
+		return PTR_ERR(parent);
+	}
+	nvmet_debugfs = parent;
+	return 0;
+}
+
+void nvmet_exit_debugfs(void)
+{
+	debugfs_remove_recursive(nvmet_debugfs);
+}
diff --git a/drivers/nvme/target/debugfs.h b/drivers/nvme/target/debugfs.h
new file mode 100644
index 000000000000..ff09e5597614
--- /dev/null
+++ b/drivers/nvme/target/debugfs.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * DebugFS interface for the NVMe target.
+ * Copyright (c) 2022-2024 Shadow
+ * Copyright (c) 2024 SUSE LLC
+ */
+#ifndef NVMET_DEBUGFS_H
+#define NVMET_DEBUGFS_H
+
+#include <linux/types.h>
+
+#ifdef CONFIG_NVME_TARGET_DEBUGFS
+int nvmet_debugfs_subsys_setup(struct nvmet_subsys *subsys);
+void nvmet_debugfs_subsys_free(struct nvmet_subsys *subsys);
+int nvmet_debugfs_ctrl_setup(struct nvmet_ctrl *ctrl);
+void nvmet_debugfs_ctrl_free(struct nvmet_ctrl *ctrl);
+
+int __init nvmet_init_debugfs(void);
+void nvmet_exit_debugfs(void);
+#else
+static inline int nvmet_debugfs_subsys_setup(struct nvmet_subsys *subsys)
+{
+	return 0;
+}
+static inline void nvmet_debugfs_subsys_free(struct nvmet_subsys *subsys){}
+
+static inline int nvmet_debugfs_ctrl_setup(struct nvmet_ctrl *ctrl)
+{
+	return 0;
+}
+static inline void nvmet_debugfs_ctrl_free(struct nvmet_ctrl *ctrl) {}
+
+static inline int __init nvmet_init_debugfs(void)
+{
+    return 0;
+}
+
+static inline void nvmet_exit_debugfs(void) {}
+
+#endif
+
+#endif // NVMET_DEBUGFS_H
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index f460728e1df1..4f5b0a2f36b9 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -230,7 +230,9 @@ struct nvmet_ctrl {
 
 	struct device		*p2p_client;
 	struct radix_tree_root	p2p_ns_map;
-
+#ifdef CONFIG_NVME_TARGET_DEBUGFS
+	struct dentry		*debugfs_dir;
+#endif
 	spinlock_t		error_lock;
 	u64			err_counter;
 	struct nvme_error_slot	slots[NVMET_ERROR_LOG_SLOTS];
@@ -262,7 +264,9 @@ struct nvmet_subsys {
 
 	struct list_head	hosts;
 	bool			allow_any_host;
-
+#ifdef CONFIG_NVME_TARGET_DEBUGFS
+	struct dentry		*debugfs_dir;
+#endif
 	u16			max_qid;
 
 	u64			ver;
-- 
2.35.3

[PATCH 2/7] nvmet: add 'host_traddr' callback for debugfs

[Hannes Reinecke]

We want to display the transport address of the connected host
in debugfs, but this is a property of the transport.
So add a callback 'host_traddr' to allow the transport drivers
to fill in the data.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
---
 drivers/nvme/target/core.c    |  8 ++++++++
 drivers/nvme/target/debugfs.c | 19 +++++++++++++++++++
 drivers/nvme/target/nvmet.h   |  4 ++++
 3 files changed, 31 insertions(+)

diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 7c82e33d9347..20b138ed571f 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -1530,6 +1530,14 @@ void nvmet_ctrl_fatal_error(struct nvmet_ctrl *ctrl)
 }
 EXPORT_SYMBOL_GPL(nvmet_ctrl_fatal_error);
 
+ssize_t nvmet_ctrl_host_traddr(struct nvmet_ctrl *ctrl,
+		char *traddr, size_t traddr_len)
+{
+	if (!ctrl->ops->host_traddr)
+		return -EOPNOTSUPP;
+	return ctrl->ops->host_traddr(ctrl, traddr, traddr_len);
+}
+
 static struct nvmet_subsys *nvmet_find_get_subsys(struct nvmet_port *port,
 		const char *subsysnqn)
 {
diff --git a/drivers/nvme/target/debugfs.c b/drivers/nvme/target/debugfs.c
index 45c0483c54fc..60cfb1ab266e 100644
--- a/drivers/nvme/target/debugfs.c
+++ b/drivers/nvme/target/debugfs.c
@@ -115,6 +115,23 @@ static ssize_t nvmet_ctrl_state_write(struct file *file, const char __user *buf,
 }
 NVMET_DEBUGFS_RW_ATTR(nvmet_ctrl_state);
 
+static int nvmet_ctrl_host_traddr_show(struct seq_file *m, void *p)
+{
+	struct nvmet_ctrl *ctrl = (struct nvmet_ctrl *)m->private;
+	ssize_t size;
+	char buf[NVMF_TRADDR_SIZE + 1];
+
+	size = nvmet_ctrl_host_traddr(ctrl, buf, NVMF_TRADDR_SIZE);
+	if (size < 0) {
+		buf[0] = '\0';
+		size = 0;
+	}
+	buf[size] = '\0';
+	seq_printf(m, "%s\n", buf);
+	return 0;
+}
+NVMET_DEBUGFS_ATTR(nvmet_ctrl_host_traddr);
+
 int nvmet_debugfs_ctrl_setup(struct nvmet_ctrl *ctrl)
 {
 	char name[32];
@@ -138,6 +155,8 @@ int nvmet_debugfs_ctrl_setup(struct nvmet_ctrl *ctrl)
 			    &nvmet_ctrl_kato_fops);
 	debugfs_create_file("state", S_IRUSR | S_IWUSR, ctrl->debugfs_dir, ctrl,
 			    &nvmet_ctrl_state_fops);
+	debugfs_create_file("host_traddr", S_IRUSR, ctrl->debugfs_dir, ctrl,
+			    &nvmet_ctrl_host_traddr_fops);
 	return 0;
 }
 
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index 4f5b0a2f36b9..257cace53924 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -354,6 +354,8 @@ struct nvmet_fabrics_ops {
 	void (*delete_ctrl)(struct nvmet_ctrl *ctrl);
 	void (*disc_traddr)(struct nvmet_req *req,
 			struct nvmet_port *port, char *traddr);
+	ssize_t (*host_traddr)(struct nvmet_ctrl *ctrl,
+			char *traddr, size_t traddr_len);
 	u16 (*install_queue)(struct nvmet_sq *nvme_sq);
 	void (*discovery_chg)(struct nvmet_port *port);
 	u8 (*get_mdts)(const struct nvmet_ctrl *ctrl);
@@ -502,6 +504,8 @@ struct nvmet_ctrl *nvmet_ctrl_find_get(const char *subsysnqn,
 				       struct nvmet_req *req);
 void nvmet_ctrl_put(struct nvmet_ctrl *ctrl);
 u16 nvmet_check_ctrl_status(struct nvmet_req *req);
+ssize_t nvmet_ctrl_host_traddr(struct nvmet_ctrl *ctrl,
+		char *traddr, size_t traddr_len);
 
 struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
 		enum nvme_subsys_type type);
-- 
2.35.3

[PATCH 3/7] nvmet-tcp: implement host_traddr()

[Hannes Reinecke]

Implement callback to display the host transport address.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
---
 drivers/nvme/target/tcp.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index a5422e2c979a..9c3c1456af52 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -2170,6 +2170,19 @@ static void nvmet_tcp_disc_port_addr(struct nvmet_req *req,
 	}
 }
 
+static ssize_t nvmet_tcp_host_port_addr(struct nvmet_ctrl *ctrl,
+			char *traddr, size_t traddr_len)
+{
+	struct nvmet_sq *sq = ctrl->sqs[0];
+	struct nvmet_tcp_queue *queue =
+		container_of(sq, struct nvmet_tcp_queue, nvme_sq);
+
+	if (queue->sockaddr_peer.ss_family == AF_UNSPEC)
+		return -EINVAL;
+	return snprintf(traddr, traddr_len, "%pISc",
+			(struct sockaddr *)&queue->sockaddr_peer);
+}
+
 static const struct nvmet_fabrics_ops nvmet_tcp_ops = {
 	.owner			= THIS_MODULE,
 	.type			= NVMF_TRTYPE_TCP,
@@ -2180,6 +2193,7 @@ static const struct nvmet_fabrics_ops nvmet_tcp_ops = {
 	.delete_ctrl		= nvmet_tcp_delete_ctrl,
 	.install_queue		= nvmet_tcp_install_queue,
 	.disc_traddr		= nvmet_tcp_disc_port_addr,
+	.host_traddr		= nvmet_tcp_host_port_addr,
 };
 
 static int __init nvmet_tcp_init(void)
-- 
2.35.3

[PATCH 4/7] nvmet-rdma: implement host_traddr()

[Hannes Reinecke]

Implement callback to display the host transport address.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
---
 drivers/nvme/target/rdma.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
index 5b8c63e74639..26f12c80f7ea 100644
--- a/drivers/nvme/target/rdma.c
+++ b/drivers/nvme/target/rdma.c
@@ -2012,6 +2012,17 @@ static void nvmet_rdma_disc_port_addr(struct nvmet_req *req,
 	}
 }
 
+static ssize_t nvmet_rdma_host_port_addr(struct nvmet_ctrl *ctrl,
+		char *traddr, size_t traddr_len)
+{
+	struct nvmet_sq *nvme_sq = ctrl->sqs[0];
+	struct nvmet_rdma_queue *queue =
+		container_of(nvme_sq, struct nvmet_rdma_queue, nvme_sq);
+
+	return snprintf(traddr, traddr_len, "%pISc",
+			(struct sockaddr *)&queue->cm_id->route.addr.dst_addr);
+}
+
 static u8 nvmet_rdma_get_mdts(const struct nvmet_ctrl *ctrl)
 {
 	if (ctrl->pi_support)
@@ -2036,6 +2047,7 @@ static const struct nvmet_fabrics_ops nvmet_rdma_ops = {
 	.queue_response		= nvmet_rdma_queue_response,
 	.delete_ctrl		= nvmet_rdma_delete_ctrl,
 	.disc_traddr		= nvmet_rdma_disc_port_addr,
+	.host_traddr		= nvmet_rdma_host_port_addr,
 	.get_mdts		= nvmet_rdma_get_mdts,
 	.get_max_queue_size	= nvmet_rdma_get_max_queue_size,
 };
-- 
2.35.3

[PATCH 5/7] nvmet-fc: implement host_traddr()

[Hannes Reinecke]

Implement callback to display the host transport address by
adding a callback 'host_traddr' for nvmet_fc_target_template.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/nvme/target/fc.c       | 33 +++++++++++++++++++++++++++++++++
 include/linux/nvme-fc-driver.h |  4 ++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
index fd229f310c93..b4706a8a9837 100644
--- a/drivers/nvme/target/fc.c
+++ b/drivers/nvme/target/fc.c
@@ -2931,6 +2931,38 @@ nvmet_fc_discovery_chg(struct nvmet_port *port)
 		tgtport->ops->discovery_event(&tgtport->fc_target_port);
 }
 
+static ssize_t
+nvmet_fc_host_traddr(struct nvmet_ctrl *ctrl,
+		char *traddr, size_t traddr_size)
+{
+	struct nvmet_sq *sq = ctrl->sqs[0];
+	struct nvmet_fc_tgt_queue *queue =
+		container_of(sq, struct nvmet_fc_tgt_queue, nvme_sq);
+	struct nvmet_fc_tgtport *tgtport = queue->assoc ? queue->assoc->tgtport : NULL;
+	struct nvmet_fc_hostport *hostport = queue->assoc ? queue->assoc->hostport : NULL;
+	u64 wwnn, wwpn;
+	ssize_t ret = 0;
+
+	if (!tgtport || !nvmet_fc_tgtport_get(tgtport))
+		return -ENODEV;
+	if (!hostport || !nvmet_fc_hostport_get(hostport)) {
+		ret = -ENODEV;
+		goto out_put;
+	}
+
+	if (tgtport->ops->host_traddr) {
+		ret = tgtport->ops->host_traddr(hostport->hosthandle, &wwnn, &wwpn);
+		if (ret)
+			goto out_put_host;
+		ret = snprintf(traddr, traddr_size, "nn-0x%llx:pn-0x%llx", wwnn, wwpn);
+	}
+out_put_host:
+	nvmet_fc_hostport_put(hostport);
+out_put:
+	nvmet_fc_tgtport_put(tgtport);
+	return ret;
+}
+
 static const struct nvmet_fabrics_ops nvmet_fc_tgt_fcp_ops = {
 	.owner			= THIS_MODULE,
 	.type			= NVMF_TRTYPE_FC,
@@ -2940,6 +2972,7 @@ static const struct nvmet_fabrics_ops nvmet_fc_tgt_fcp_ops = {
 	.queue_response		= nvmet_fc_fcp_nvme_cmd_done,
 	.delete_ctrl		= nvmet_fc_delete_ctrl,
 	.discovery_chg		= nvmet_fc_discovery_chg,
+	.host_traddr		= nvmet_fc_host_traddr,
 };
 
 static int __init nvmet_fc_init_module(void)
diff --git a/include/linux/nvme-fc-driver.h b/include/linux/nvme-fc-driver.h
index 4109f1bd6128..89ea1ebd975a 100644
--- a/include/linux/nvme-fc-driver.h
+++ b/include/linux/nvme-fc-driver.h
@@ -920,6 +920,9 @@ struct nvmet_fc_target_port {
  *       further references to hosthandle.
  *       Entrypoint is Mandatory if the lldd calls nvmet_fc_invalidate_host().
  *
+ * @host_traddr: called by the transport to retrieve the node name and
+ *       port name of the host port address.
+ *
  * @max_hw_queues:  indicates the maximum number of hw queues the LLDD
  *       supports for cpu affinitization.
  *       Value is Mandatory. Must be at least 1.
@@ -975,6 +978,7 @@ struct nvmet_fc_target_template {
 	void (*ls_abort)(struct nvmet_fc_target_port *targetport,
 				void *hosthandle, struct nvmefc_ls_req *lsreq);
 	void (*host_release)(void *hosthandle);
+	int (*host_traddr)(void *hosthandle, u64 *wwnn, u64 *wwpn);
 
 	u32	max_hw_queues;
 	u16	max_sgl_segments;
-- 
2.35.3

[PATCH 6/7] nvme-fcloop: implement 'host_traddr'

[Hannes Reinecke]

Implement the 'host_traddr' callback to display the host transport
address for nvmet debugfs.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/nvme/target/fcloop.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/nvme/target/fcloop.c b/drivers/nvme/target/fcloop.c
index 913cd2ec7a6f..e1abb27927ff 100644
--- a/drivers/nvme/target/fcloop.c
+++ b/drivers/nvme/target/fcloop.c
@@ -492,6 +492,16 @@ fcloop_t2h_host_release(void *hosthandle)
 	/* host handle ignored for now */
 }
 
+static int
+fcloop_t2h_host_traddr(void *hosthandle, u64 *wwnn, u64 *wwpn)
+{
+	struct fcloop_rport *rport = hosthandle;
+
+	*wwnn = rport->lport->localport->node_name;
+	*wwpn = rport->lport->localport->port_name;
+	return 0;
+}
+
 /*
  * Simulate reception of RSCN and converting it to a initiator transport
  * call to rescan a remote port.
@@ -1074,6 +1084,7 @@ static struct nvmet_fc_target_template tgttemplate = {
 	.ls_req			= fcloop_t2h_ls_req,
 	.ls_abort		= fcloop_t2h_ls_abort,
 	.host_release		= fcloop_t2h_host_release,
+	.host_traddr		= fcloop_t2h_host_traddr,
 	.max_hw_queues		= FCLOOP_HW_QUEUES,
 	.max_sgl_segments	= FCLOOP_SGL_SEGS,
 	.max_dif_sgl_segments	= FCLOOP_SGL_SEGS,
-- 
2.35.3

[PATCH 7/7] lpfc_nvmet: implement 'host_traddr'

[Hannes Reinecke]

Implement the 'host_traddr' callback to display the host transport
address for nvmet debugfs.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/scsi/lpfc/lpfc_nvmet.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c
index 425328d9c2d8..a315ab03f721 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.c
+++ b/drivers/scsi/lpfc/lpfc_nvmet.c
@@ -1363,6 +1363,16 @@ lpfc_nvmet_ls_abort(struct nvmet_fc_target_port *targetport,
 		atomic_inc(&lpfc_nvmet->xmt_ls_abort);
 }
 
+static int
+lpfc_nvmet_host_traddr(void *hosthandle, u64 *wwnn, u64 *wwpn)
+{
+	struct lpfc_nodelist *ndlp = hosthandle;
+
+	*wwnn = wwn_to_u64(ndlp->nlp_nodename.u.wwn);
+	*wwpn = wwn_to_u64(ndlp->nlp_portname.u.wwn);
+	return 0;
+}
+
 static void
 lpfc_nvmet_host_release(void *hosthandle)
 {
@@ -1413,6 +1423,7 @@ static struct nvmet_fc_target_template lpfc_tgttemplate = {
 	.ls_req         = lpfc_nvmet_ls_req,
 	.ls_abort       = lpfc_nvmet_ls_abort,
 	.host_release   = lpfc_nvmet_host_release,
+	.host_traddr    = lpfc_nvmet_host_traddr,
 
 	.max_hw_queues  = 1,
 	.max_sgl_segments = LPFC_NVMET_DEFAULT_SEGS,
-- 
2.35.3

Re: [PATCHv4 0/7] nvmet: debugfs support

[Sagi Grimberg]

Series looks solid to me,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCHv4 0/7] nvmet: debugfs support

[Daniel Wagner]

I've gave it a quick test run with blktests triggering resets. Works as
expected.

Tested-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Daniel Wagner <dwagner@suse.de>

Re: [PATCHv4 0/7] nvmet: debugfs support

[Daniel Wagner]

Any chance to get this picked up?

Re: [PATCHv4 0/7] nvmet: debugfs support

[Chaitanya Kulkarni]

On 4/8/24 09:07, Daniel Wagner wrote:
> Any chance to get this picked up?
>

you will have my review in next couple of hours ...


-ck

Re: [PATCH 1/7] nvmet: add debugfs support

[Chaitanya Kulkarni]

On 3/26/24 05:03, Hannes Reinecke wrote:
> Add a debugfs hierarchy to display the configured subsystems
> and the controllers attached to the subsystems.
>
> Suggested-by: Redouane BOUFENGHOUR <redouane.boufenghour@shadow.tech>
> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> ---
>   drivers/nvme/target/Kconfig   |   9 ++
>   drivers/nvme/target/Makefile  |   1 +
>   drivers/nvme/target/core.c    |  22 +++-
>   drivers/nvme/target/debugfs.c | 183 ++++++++++++++++++++++++++++++++++
>   drivers/nvme/target/debugfs.h |  42 ++++++++
>   drivers/nvme/target/nvmet.h   |   8 +-
>   6 files changed, 262 insertions(+), 3 deletions(-)
>   create mode 100644 drivers/nvme/target/debugfs.c
>   create mode 100644 drivers/nvme/target/debugfs.h
>
> diff --git a/drivers/nvme/target/Kconfig b/drivers/nvme/target/Kconfig
> index 872dd1a0acd8..5740893ace6d 100644
> --- a/drivers/nvme/target/Kconfig
> +++ b/drivers/nvme/target/Kconfig
> @@ -18,6 +18,15 @@ config NVME_TARGET
>   	  To configure the NVMe target you probably want to use the nvmetcli
>   	  tool from http://git.infradead.org/users/hch/nvmetcli.git.
>   
> +config NVME_TARGET_DEBUGFS
> +        bool "NVMe Target debugfs support"
> +	depends on NVME_TARGET
> +	help
> +	  This enables debugfs support to display the connected controllers
> +	  to each subsystem
> +
> +	  If unsure, say N.
> +
>   config NVME_TARGET_PASSTHRU
>   	bool "NVMe Target Passthrough support"
>   	depends on NVME_TARGET
> diff --git a/drivers/nvme/target/Makefile b/drivers/nvme/target/Makefile
> index c66820102493..c402c44350b2 100644
> --- a/drivers/nvme/target/Makefile
> +++ b/drivers/nvme/target/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_NVME_TARGET_TCP)		+= nvmet-tcp.o
>   
>   nvmet-y		+= core.o configfs.o admin-cmd.o fabrics-cmd.o \
>   			discovery.o io-cmd-file.o io-cmd-bdev.o
> +nvmet-$(CONFIG_NVME_TARGET_DEBUGFS)	+= debugfs.o
>   nvmet-$(CONFIG_NVME_TARGET_PASSTHRU)	+= passthru.o
>   nvmet-$(CONFIG_BLK_DEV_ZONED)		+= zns.o
>   nvmet-$(CONFIG_NVME_TARGET_AUTH)	+= fabrics-cmd-auth.o auth.o
> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
> index 6bbe4df0166c..7c82e33d9347 100644
> --- a/drivers/nvme/target/core.c
> +++ b/drivers/nvme/target/core.c
> @@ -16,6 +16,7 @@
>   #include "trace.h"
>   
>   #include "nvmet.h"
> +#include "debugfs.h"
>   
>   struct kmem_cache *nvmet_bvec_cache;
>   struct workqueue_struct *buffered_io_wq;
> @@ -1466,6 +1467,7 @@ u16 nvmet_alloc_ctrl(const char *subsysnqn, const char *hostnqn,
>   	mutex_lock(&subsys->lock);
>   	list_add_tail(&ctrl->subsys_entry, &subsys->ctrls);
>   	nvmet_setup_p2p_ns_map(ctrl, req);
> +	nvmet_debugfs_ctrl_setup(ctrl);
>   	mutex_unlock(&subsys->lock);
>   
>   	*ctrlp = ctrl;
> @@ -1500,6 +1502,8 @@ static void nvmet_ctrl_free(struct kref *ref)
>   
>   	nvmet_destroy_auth(ctrl);
>   
> +	nvmet_debugfs_ctrl_free(ctrl);
> +
>   	ida_free(&cntlid_ida, ctrl->cntlid);
>   
>   	nvmet_async_events_free(ctrl);
> @@ -1613,8 +1617,14 @@ struct nvmet_subsys *nvmet_subsys_alloc(const char *subsysnqn,
>   	INIT_LIST_HEAD(&subsys->ctrls);
>   	INIT_LIST_HEAD(&subsys->hosts);
>   
> +	ret = nvmet_debugfs_subsys_setup(subsys);
> +	if (ret)
> +		goto free_subsysnqn;
> +
>   	return subsys;
>   
> +free_subsysnqn:
> +	kfree(subsys->subsysnqn);
>   free_fr:
>   	kfree(subsys->firmware_rev);
>   free_mn:
> @@ -1631,6 +1641,8 @@ static void nvmet_subsys_free(struct kref *ref)
>   
>   	WARN_ON_ONCE(!xa_empty(&subsys->namespaces));
>   
> +	nvmet_debugfs_subsys_free(subsys);
> +
>   	xa_destroy(&subsys->namespaces);
>   	nvmet_passthru_subsys_free(subsys);
>   
> @@ -1684,11 +1696,18 @@ static int __init nvmet_init(void)
>   	if (error)
>   		goto out_free_nvmet_work_queue;
>   
> -	error = nvmet_init_configfs();
> +	error = nvmet_init_debugfs();
>   	if (error)
>   		goto out_exit_discovery;
> +
> +	error = nvmet_init_configfs();
> +	if (error)
> +		goto out_exit_debugfs;
> +
>   	return 0;
>   
> +out_exit_debugfs:
> +	nvmet_exit_debugfs();
>   out_exit_discovery:
>   	nvmet_exit_discovery();
>   out_free_nvmet_work_queue:
> @@ -1705,6 +1724,7 @@ static int __init nvmet_init(void)
>   static void __exit nvmet_exit(void)
>   {
>   	nvmet_exit_configfs();
> +	nvmet_exit_debugfs();
>   	nvmet_exit_discovery();
>   	ida_destroy(&cntlid_ida);
>   	destroy_workqueue(nvmet_wq);
> diff --git a/drivers/nvme/target/debugfs.c b/drivers/nvme/target/debugfs.c
> new file mode 100644
> index 000000000000..45c0483c54fc
> --- /dev/null
> +++ b/drivers/nvme/target/debugfs.c
> @@ -0,0 +1,183 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DebugFS interface for the NVMe target.
> + * Copyright (c) 2022-2024 Shadow
> + * Copyright (c) 2024 SUSE LLC
> + */
> +
> +#include <linux/debugfs.h>
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +
> +#include "nvmet.h"
> +#include "debugfs.h"
> +
> +struct dentry *nvmet_debugfs;
> +
> +#define NVMET_DEBUGFS_ATTR(field) \
> +	static int field##_open(struct inode *inode, struct file *file) \
> +	{ return single_open(file, field##_show, inode->i_private); } \
> +	\
> +	static const struct file_operations field##_fops = { \
> +		.open = field##_open, \
> +		.read = seq_read, \
> +		.release = single_release, \
> +	}
> +
> +#define NVMET_DEBUGFS_RW_ATTR(field) \
> +	static int field##_open(struct inode *inode, struct file *file) \
> +	{ return single_open(file, field##_show, inode->i_private); } \
> +	\
> +	static const struct file_operations field##_fops = { \
> +		.open = field##_open, \
> +		.read = seq_read, \
> +		.write = field##_write, \
> +		.release = single_release, \
> +	}
> +
> +static int nvmet_ctrl_hostnqn_show(struct seq_file *m, void *p)
> +{
> +	struct nvmet_ctrl *ctrl = (struct nvmet_ctrl *)m->private;
> +

nit:- do you really need above type cast ? m->private is void * ..

See :-

transaction_log_show()
ptdump_show()
bw_proc_show()
xics_debug_show()
xive_native_debug_show()
pci_perf_show()

> +	seq_printf(m, "%s\n", ctrl->hostnqn);

nit :- I believe preferred way is to use seq_puts() that you have done
it in nvmet_ctrl_state_show(), please check ..

> +	return 0;
> +}
> +NVMET_DEBUGFS_ATTR(nvmet_ctrl_hostnqn);
> +
> +static int nvmet_ctrl_kato_show(struct seq_file *m, void *p)
> +{
> +	struct nvmet_ctrl *ctrl = (struct nvmet_ctrl *)m->private;
> +

no need to cast above ...

> +	seq_printf(m, "%d\n", ctrl->kato);
> +	return 0;
> +}
> +NVMET_DEBUGFS_ATTR(nvmet_ctrl_kato);
> +
> +static int nvmet_ctrl_port_show(struct seq_file *m, void *p)
> +{
> +	struct nvmet_ctrl *ctrl = (struct nvmet_ctrl *)m->private;
> +

same here ...

> +	seq_printf(m, "%d\n", le16_to_cpu(ctrl->port->disc_addr.portid));
> +	return 0;
> +}
> +NVMET_DEBUGFS_ATTR(nvmet_ctrl_port);
> +
> +static const char *const csts_state_names[] = {
> +	[NVME_CSTS_RDY] = "ready",
> +	[NVME_CSTS_CFS] = "fatal",
> +	[NVME_CSTS_NSSRO] = "reset",
> +	[NVME_CSTS_SHST_OCCUR] = "shutdown",
> +	[NVME_CSTS_SHST_CMPLT] = "completed",
> +	[NVME_CSTS_PP] = "paused",
> +};
> +

nit:- based on what you have submitted on host side aligning helps:-

static const char *const csts_state_names[] = {
         [NVME_CSTS_RDY]        = "ready",
         [NVME_CSTS_CFS]        = "fatal",
         [NVME_CSTS_NSSRO]      = "reset",
         [NVME_CSTS_SHST_OCCUR] = "shutdown",
         [NVME_CSTS_SHST_CMPLT] = "completed",
         [NVME_CSTS_PP]         = "paused",
};

> +static int nvmet_ctrl_state_show(struct seq_file *m, void *p)
> +{
> +	struct nvmet_ctrl *ctrl = (struct nvmet_ctrl *)m->private;

no need to cast above ...

> +	bool sep = false;
> +	int i;
> +
> +	for (i = 0; i < 7; i++) {
> +		int state = BIT(i);
> +
> +		if (!(ctrl->csts & state))
> +			continue;
> +		if (sep)
> +			seq_puts(m, "|");
> +		sep = true;
> +		if (csts_state_names[state])
> +			seq_puts(m, csts_state_names[state]);
> +		else
> +			seq_printf(m, "%d", state);
> +	}
> +	if (sep)
> +		seq_printf(m, "\n");
> +	return 0;
> +}
> +
> +static ssize_t nvmet_ctrl_state_write(struct file *file, const char __user *buf,
> +				      size_t count, loff_t *ppos)
> +{
> +	struct seq_file *m = file->private_data;
> +	struct nvmet_ctrl *ctrl = m->private;
> +	char reset[16];
> +
> +	if (count >= sizeof(reset))
> +		return -EINVAL;
> +	if (copy_from_user(reset, buf, count))
> +		return -EFAULT;
> +	if (!memcmp(reset, "fatal", 5))
> +		nvmet_ctrl_fatal_error(ctrl);
> +	else
> +		return -EINVAL;
> +	return count;
> +}
> +NVMET_DEBUGFS_RW_ATTR(nvmet_ctrl_state);
> +
> +int nvmet_debugfs_ctrl_setup(struct nvmet_ctrl *ctrl)
> +{
> +	char name[32];
> +	struct dentry *parent = ctrl->subsys->debugfs_dir;
> +	int ret;
> +
> +	if (!parent)
> +		return -ENODEV;
> +	snprintf(name, sizeof(name), "ctrl%d", ctrl->cntlid);
> +	ctrl->debugfs_dir = debugfs_create_dir(name, parent);
> +	if (IS_ERR(ctrl->debugfs_dir)) {
> +		ret = PTR_ERR(ctrl->debugfs_dir);
> +		ctrl->debugfs_dir = NULL;
> +		return ret;
> +	}
> +	debugfs_create_file("port", S_IRUSR, ctrl->debugfs_dir, ctrl,
> +			    &nvmet_ctrl_port_fops);
> +	debugfs_create_file("hostnqn", S_IRUSR, ctrl->debugfs_dir, ctrl,
> +			    &nvmet_ctrl_hostnqn_fops);
> +	debugfs_create_file("kato", S_IRUSR, ctrl->debugfs_dir, ctrl,
> +			    &nvmet_ctrl_kato_fops);
> +	debugfs_create_file("state", S_IRUSR | S_IWUSR, ctrl->debugfs_dir, ctrl,
> +			    &nvmet_ctrl_state_fops);
> +	return 0;
> +}
> +
> +void nvmet_debugfs_ctrl_free(struct nvmet_ctrl *ctrl)
> +{
> +	debugfs_remove_recursive(ctrl->debugfs_dir);
> +}
> +
> +int nvmet_debugfs_subsys_setup(struct nvmet_subsys *subsys)
> +{
> +	int ret = 0;
> +
> +	subsys->debugfs_dir = debugfs_create_dir(subsys->subsysnqn,
> +						 nvmet_debugfs);
> +	if (IS_ERR(subsys->debugfs_dir)) {
> +		ret = PTR_ERR(subsys->debugfs_dir);
> +		subsys->debugfs_dir = NULL;
> +	}
> +	return ret;
> +}
> +
> +void nvmet_debugfs_subsys_free(struct nvmet_subsys *subsys)
> +{
> +	debugfs_remove_recursive(subsys->debugfs_dir);
> +}
> +
> +int __init nvmet_init_debugfs(void)
> +{
> +	struct dentry *parent;
> +
> +	parent = debugfs_create_dir("nvmet", NULL);
> +	if (IS_ERR(parent)) {
> +		pr_warn("%s: failed to create debugfs directory\n", "nvmet");
> +		return PTR_ERR(parent);
> +	}
> +	nvmet_debugfs = parent;
> +	return 0;
> +}
> +
> +void nvmet_exit_debugfs(void)
> +{
> +	debugfs_remove_recursive(nvmet_debugfs);
> +}
> diff --git a/drivers/nvme/target/debugfs.h b/drivers/nvme/target/debugfs.h
> new file mode 100644
> index 000000000000..ff09e5597614
> --- /dev/null
> +++ b/drivers/nvme/target/debugfs.h
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * DebugFS interface for the NVMe target.
> + * Copyright (c) 2022-2024 Shadow
> + * Copyright (c) 2024 SUSE LLC
> + */
> +#ifndef NVMET_DEBUGFS_H
> +#define NVMET_DEBUGFS_H
> +
> +#include <linux/types.h>
> +
> +#ifdef CONFIG_NVME_TARGET_DEBUGFS
> +int nvmet_debugfs_subsys_setup(struct nvmet_subsys *subsys);
> +void nvmet_debugfs_subsys_free(struct nvmet_subsys *subsys);
> +int nvmet_debugfs_ctrl_setup(struct nvmet_ctrl *ctrl);
> +void nvmet_debugfs_ctrl_free(struct nvmet_ctrl *ctrl);
> +
> +int __init nvmet_init_debugfs(void);
> +void nvmet_exit_debugfs(void);
> +#else
> +static inline int nvmet_debugfs_subsys_setup(struct nvmet_subsys *subsys)
> +{
> +	return 0;
> +}
> +static inline void nvmet_debugfs_subsys_free(struct nvmet_subsys *subsys){}
> +
> +static inline int nvmet_debugfs_ctrl_setup(struct nvmet_ctrl *ctrl)
> +{
> +	return 0;
> +}
> +static inline void nvmet_debugfs_ctrl_free(struct nvmet_ctrl *ctrl) {}
> +
> +static inline int __init nvmet_init_debugfs(void)
> +{
> +    return 0;
> +}
> +
> +static inline void nvmet_exit_debugfs(void) {}
> +
> +#endif
> +
> +#endif // NVMET_DEBUGFS_H

nit:- instead of // use /**/ ?

> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
> index f460728e1df1..4f5b0a2f36b9 100644
> --- a/drivers/nvme/target/nvmet.h
> +++ b/drivers/nvme/target/nvmet.h
> @@ -230,7 +230,9 @@ struct nvmet_ctrl {
>   
>   	struct device		*p2p_client;
>   	struct radix_tree_root	p2p_ns_map;
> -
> +#ifdef CONFIG_NVME_TARGET_DEBUGFS
> +	struct dentry		*debugfs_dir;
> +#endif
>   	spinlock_t		error_lock;
>   	u64			err_counter;
>   	struct nvme_error_slot	slots[NVMET_ERROR_LOG_SLOTS];
> @@ -262,7 +264,9 @@ struct nvmet_subsys {
>   
>   	struct list_head	hosts;
>   	bool			allow_any_host;
> -
> +#ifdef CONFIG_NVME_TARGET_DEBUGFS
> +	struct dentry		*debugfs_dir;
> +#endif
>   	u16			max_qid;
>   
>   	u64			ver;

-ck

Re: [PATCH 2/7] nvmet: add 'host_traddr' callback for debugfs

[Chaitanya Kulkarni]

On 3/26/24 05:03, Hannes Reinecke wrote:
> We want to display the transport address of the connected host
> in debugfs, but this is a property of the transport.
> So add a callback 'host_traddr' to allow the transport drivers
> to fill in the data.
>
> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> ---
>   drivers/nvme/target/core.c    |  8 ++++++++
>   drivers/nvme/target/debugfs.c | 19 +++++++++++++++++++
>   drivers/nvme/target/nvmet.h   |  4 ++++
>   3 files changed, 31 insertions(+)
>
> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
> index 7c82e33d9347..20b138ed571f 100644
> --- a/drivers/nvme/target/core.c
> +++ b/drivers/nvme/target/core.c
> @@ -1530,6 +1530,14 @@ void nvmet_ctrl_fatal_error(struct nvmet_ctrl *ctrl)
>   }
>   EXPORT_SYMBOL_GPL(nvmet_ctrl_fatal_error);
>   
> +ssize_t nvmet_ctrl_host_traddr(struct nvmet_ctrl *ctrl,
> +		char *traddr, size_t traddr_len)
> +{
> +	if (!ctrl->ops->host_traddr)
> +		return -EOPNOTSUPP;
> +	return ctrl->ops->host_traddr(ctrl, traddr, traddr_len);
> +}
> +
>   static struct nvmet_subsys *nvmet_find_get_subsys(struct nvmet_port *port,
>   		const char *subsysnqn)
>   {
> diff --git a/drivers/nvme/target/debugfs.c b/drivers/nvme/target/debugfs.c
> index 45c0483c54fc..60cfb1ab266e 100644
> --- a/drivers/nvme/target/debugfs.c
> +++ b/drivers/nvme/target/debugfs.c
> @@ -115,6 +115,23 @@ static ssize_t nvmet_ctrl_state_write(struct file *file, const char __user *buf,
>   }
>   NVMET_DEBUGFS_RW_ATTR(nvmet_ctrl_state);
>   
> +static int nvmet_ctrl_host_traddr_show(struct seq_file *m, void *p)
> +{
> +	struct nvmet_ctrl *ctrl = (struct nvmet_ctrl *)m->private;

cast can be avoided above ...

-ck

Re: [PATCHv4 0/7] nvmet: debugfs support

[Chaitanya Kulkarni]

On 3/26/24 05:03, Hannes Reinecke wrote:
> Hi all,
>
> taking up the original patchset for nvmet debugfs
> I've improved upon that submission by modifying
> the layout:
>
> /dev/kernel/debug/nvmet
>    <subsysnqn>
>      ctrl<cntlid>
>        port
>        state
>        hostnqn
>        kato
>        host_traddr
>
> The 'state' attribute is the value of the
> CSTS register; one can trigger a controller reset
> by writing 'fatal' into it (to set CSTS.CFS).
>
> As usual, comments and reviews are welcome.
>
>

apart from few nits commented patch-series looks good to me.

Since it does touch all the transport and writes the controller
state transport independent blktest would be very useful to trigger
nvmet_fatal_error_handler() while I/O traffic is running from host ?

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

-ck

Re: [PATCHv4 0/7] nvmet: debugfs support

[Daniel Wagner]

On Mon, Apr 08, 2024 at 06:29:10PM +0000, Chaitanya Kulkarni wrote:
> Since it does touch all the transport and writes the controller
> state transport independent blktest would be very useful to trigger
> nvmet_fatal_error_handler() while I/O traffic is running from host ?

Absolutely, this is one of the test I want to add, obviously with a
multipath setup.

Re: [PATCHv4 0/7] nvmet: debugfs support

[Sagi Grimberg]

On 08/04/2024 19:07, Daniel Wagner wrote:
> Any chance to get this picked up?

I'm fine with it, the only question would be the fc additions, which
perhaps can use a review tag from one of the fc folks.