Skip to contentSkip to conversation

nvme_fc: add uevent to allow dynamic connects

PATCH Superseded James Smart 2 participants 8 messages May 13, 2017 – May 30, 2017

Subsystems (2)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover nvme_fc: add uevent to allow dynamic connects

These patches add support for generating a uevent which can be caugth and converted to a nvme-cli connect-all request for the FC initiator and target ports.

The patches were cut on nvme-4.12 after apply the patches contained in
http://lists.infradead.org/pipermail/linux-nvme/2017-May/010156.html

James Smart (2):
nvme_fc: create fc class and transport device
nvme_fc: add uevent for auto-connect

 drivers/nvme/host/fc.c         | 102 +++++++++++++++++++++++++++++++++++++++--
 include/linux/nvme-fc-driver.h |   2 +
 2 files changed, 100 insertions(+), 4 deletions(-)
Signature · 1 line
2.11.0
#1 lore raw

PATCH v2 1/2 nvme_fc: create fc class and transport device

Added a new fc class, and a device node for the nvme_fc transport under it. I expect the fc class will eventually be the location the SCSI and FC transports merge in the future.

Signed-off-by James Smart <james.smart at broadcom.com>

---
v2: cut on nvme-4.12

 drivers/nvme/host/fc.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)
drivers/nvme/host/fc.c +31 -4
index 8dd06e97736d..4ae99d546378 100644
@@ -217,6 +217,9 @@ static DEFINE_IDA(nvme_fc_ctrl_cnt);
 
 static struct workqueue_struct *nvme_fc_wq;
 
+static struct class *fc_class;
+static struct device *nvmefc_device;
+
 
 
 /* *********************** FC-NVME Port Management ************************ */
@@ -2959,17 +2962,38 @@ static int __init nvme_fc_init_module(void)
 {
 	int ret;
 
+	fc_class = class_create(THIS_MODULE, "fc");
+	if (IS_ERR(fc_class)) {
+		pr_err("couldn't register class fc\n");
+		return PTR_ERR(fc_class);
+	}
+
+	nvmefc_device = device_create(fc_class, NULL, MKDEV(0, 0), NULL,
+				"nvme_fc_transport");
+	if (IS_ERR(nvmefc_device)) {
+		pr_err("couldn't create nvme_fc device!\n");
+		ret = PTR_ERR(nvmefc_device);
+		goto out_destroy_class;
+	}
+
 	nvme_fc_wq = create_workqueue("nvme_fc_wq");
-	if (!nvme_fc_wq)
-		return -ENOMEM;
+	if (!nvme_fc_wq) {
+		ret = -ENOMEM;
+		goto out_destroy_device;
+	}
 
 	ret = nvmf_register_transport(&nvme_fc_transport);
 	if (ret)
-		goto err;
+		goto out_destroy_workqueue;
 
 	return 0;
-err:
+
+out_destroy_workqueue:
 	destroy_workqueue(nvme_fc_wq);
+out_destroy_device:
+	device_destroy(fc_class, MKDEV(0, 0));
+out_destroy_class:
+	class_destroy(fc_class);
 	return ret;
 }
 
@@ -2985,6 +3009,9 @@ static void __exit nvme_fc_exit_module(void)
 
 	ida_destroy(&nvme_fc_local_port_cnt);
 	ida_destroy(&nvme_fc_ctrl_cnt);
+
+	device_destroy(fc_class, MKDEV(0, 0));
+	class_destroy(fc_class);
 }
 
 module_init(nvme_fc_init_module);
Signature · 1 line
2.11.0
#2 lore raw

PATCH v2 1/2 nvme_fc: create fc class and transport device

Quoted 3 lines · James Smart

Added a new fc class, and a device node for the nvme_fc transport under it. I expect the fc class will eventually be the location the SCSI and FC transports merge in the future.

If we chose a nvme name for it that pretty much keeps us from doing that. It would seem a bit silly to create a new module for just these few lines, but I think we should keep the name neutral and document the purpose with extensive comments. And maybe (just maybe) move it into a separate source file.

#3 lore raw

PATCH v2 2/2 nvme_fc: add uevent for auto-connect

In reply to James Smart

To support auto-connecting to FC-NVME devices upon their dynamic appearance, add a uevent that can kick off connection scripts. uevent is posted against the nvme_fc transport device.

Added checking to stop the "feature" of connecting to the same subsystem multiple times on FC devices.

Added routine nvme_fc_rescan_remoteport() to allow lldd to request nvme scan. For example, lldd may invoke this after receiving an RSCN for a logged in device containing a discovery controller.

Tested with the following rule to kick an nvme-cli connect-all for the FC initiator and FC target ports. This is just an example for testing and not intended for real life use.

ACTION=="change", SUBSYSTEM=="fc", ENV{FC_EVENT}=="nvmediscovery", \
        ENV{NVMEFC_HOST_TRADDR}=="*", ENV{NVMEFC_TRADDR}=="*", \
	RUN+="/bin/sh -c '/usr/local/sbin/nvme connect-all --transport=fc --host-traddr=$env{NVMEFC_HOST_TRADDR} --traddr=$env{NVMEFC_TRADDR} >> /tmp/nvme_fc.log'"
Signed-off-by James Smart <james.smart at broadcom.com>

---
v2: changed syntax of event to specify a fc event type with FC_EVENT
variable, and independent variables for host_traddr and traddr. Added nvme_fc_rescan_remoteport().

 drivers/nvme/host/fc.c         | 67 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/nvme-fc-driver.h |  2 ++
 2 files changed, 69 insertions(+)
drivers/nvme/host/fc.c +67 -0
index 4ae99d546378..9a594ea323e8 100644
@@ -392,6 +392,26 @@ nvme_fc_unregister_localport(struct nvme_fc_local_port *portptr)
 }
 EXPORT_SYMBOL_GPL(nvme_fc_unregister_localport);
 
+static void
+nvme_fc_signal_discovery_scan(struct nvme_fc_lport *lport,
+		struct nvme_fc_rport *rport)
+{
+	char hostaddr[80];	/* NVMEFC_HOST_TRADDR=...*/
+	char tgtaddr[80];	/* NVMEFC_TRADDR=...*/
+	char *envp[4] = { "FC_EVENT=nvmediscovery", hostaddr, tgtaddr, NULL };
+
+	if (!(rport->remoteport.port_role & FC_PORT_ROLE_NVME_DISCOVERY))
+		return;
+
+	snprintf(hostaddr, sizeof(hostaddr),
+		"NVMEFC_HOST_TRADDR=nn-0x%016llx:pn-0x%016llx",
+		lport->localport.node_name, lport->localport.port_name);
+	snprintf(tgtaddr, sizeof(tgtaddr),
+		"NVMEFC_TRADDR=nn-0x%016llx:pn-0x%016llx",
+		rport->remoteport.node_name, rport->remoteport.port_name);
+	kobject_uevent_env(&nvmefc_device->kobj, KOBJ_CHANGE, envp);
+}
+
 /**
  * nvme_fc_register_remoteport - transport entry point called by an
  *                              LLDD to register the existence of a NVME
@@ -456,6 +476,8 @@ nvme_fc_register_remoteport(struct nvme_fc_local_port *localport,
 	list_add_tail(&newrec->endp_list, &lport->endp_list);
 	spin_unlock_irqrestore(&nvme_fc_lock, flags);
 
+	nvme_fc_signal_discovery_scan(lport, newrec);
+
 	*portptr = &newrec->remoteport;
 	return 0;
 
@@ -574,6 +596,23 @@ nvme_fc_unregister_remoteport(struct nvme_fc_remote_port *portptr)
 }
 EXPORT_SYMBOL_GPL(nvme_fc_unregister_remoteport);
 
+/**
+ * nvme_fc_rescan_remoteport - transport entry point called by an
+ *                              LLDD to request a nvme device rescan.
+ * @remoteport: pointer to the (registered) remote port that is to be
+ *              rescanned.
+ *
+ * Returns: N/A
+ */
+void
+nvme_fc_rescan_remoteport(struct nvme_fc_remote_port *remoteport)
+{
+	struct nvme_fc_rport *rport = remoteport_to_rport(remoteport);
+
+	nvme_fc_signal_discovery_scan(rport->lport, rport);
+}
+EXPORT_SYMBOL_GPL(nvme_fc_rescan_remoteport);
+
 
 /* *********************** FC-NVME DMA Handling **************************** */
 
@@ -2699,6 +2738,19 @@ static const struct blk_mq_ops nvme_fc_admin_mq_ops = {
 };
 
 
+static inline bool
+__nvme_fc_options_match(struct nvmf_ctrl_options *opts,
+			struct nvme_fc_ctrl *ctrl)
+{
+	if (strcmp(opts->subsysnqn, ctrl->ctrl.opts->subsysnqn) ||
+	    strcmp(opts->host->nqn, ctrl->ctrl.opts->host->nqn) ||
+	    memcmp(&opts->host->id, &ctrl->ctrl.opts->host->id,
+				sizeof(uuid_be)))
+		return false;
+
+	return true;
+}
+
 static struct nvme_ctrl *
 nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
 	struct nvme_fc_lport *lport, struct nvme_fc_rport *rport)
@@ -2706,6 +2758,7 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
 	struct nvme_fc_ctrl *ctrl;
 	unsigned long flags;
 	int ret, idx;
+	bool found = false;
 
 	if (!(rport->remoteport.port_role &
 	    (FC_PORT_ROLE_NVME_DISCOVERY | FC_PORT_ROLE_NVME_TARGET))) {
@@ -2713,6 +2766,20 @@ nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts,
 		goto out_fail;
 	}
 
+	spin_lock_irqsave(&rport->lock, flags);
+	list_for_each_entry(ctrl, &rport->ctrl_list, ctrl_list) {
+		if (__nvme_fc_options_match(opts, ctrl)) {
+			found = true;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&rport->lock, flags);
+
+	if (found) {
+		ret = -EALREADY;
+		goto out_fail;
+	}
+
 	ctrl = kzalloc(sizeof(*ctrl), GFP_KERNEL);
 	if (!ctrl) {
 		ret = -ENOMEM;
include/linux/nvme-fc-driver.h +2 -0
index 492e9f402223..c42313f9a451 100644
@@ -446,6 +446,8 @@ int nvme_fc_register_remoteport(struct nvme_fc_local_port *localport,
 
 int nvme_fc_unregister_remoteport(struct nvme_fc_remote_port *remoteport);
 
+void nvme_fc_rescan_remoteport(struct nvme_fc_remote_port *remoteport);
+
 
 
 /*
Signature · 1 line
2.11.0
#4 lore raw

PATCH v2 2/2 nvme_fc: add uevent for auto-connect

Quoted 7 lines
drivers/nvme/host/fc.c +7 -0
+nvme_fc_signal_discovery_scan(struct nvme_fc_lport *lport,
+		struct nvme_fc_rport *rport)
+{
+	char hostaddr[80];	/* NVMEFC_HOST_TRADDR=...*/
+	char tgtaddr[80];	/* NVMEFC_TRADDR=...*/
+	char *envp[4] = { "FC_EVENT=nvmediscovery", hostaddr, tgtaddr, NULL };
+

drivers/nvme/host/fc.c nvme_fc_unregister_localport(struct nvme_fc_local_port *portptr) · line 412

Quoted 10 lines
drivers/nvme/host/fc.c +10 -0
+	if (!(rport->remoteport.port_role & FC_PORT_ROLE_NVME_DISCOVERY))
+		return;
+
+	snprintf(hostaddr, sizeof(hostaddr),
+		"NVMEFC_HOST_TRADDR=nn-0x%016llx:pn-0x%016llx",
+		lport->localport.node_name, lport->localport.port_name);
+	snprintf(tgtaddr, sizeof(tgtaddr),
+		"NVMEFC_TRADDR=nn-0x%016llx:pn-0x%016llx",
+		rport->remoteport.node_name, rport->remoteport.port_name);
+	kobject_uevent_env(&nvmefc_device->kobj, KOBJ_CHANGE, envp);

Please use kasprintf so that we have a dynamic allocation and don't need to hardcode buffer sizes.

drivers/nvme/host/fc.c nvme_fc_init_ctrl(struct device *dev, struct nvmf_ctrl_options *opts, · line 2781

Quoted 13 lines
drivers/nvme/host/fc.c +13 -0
+	spin_lock_irqsave(&rport->lock, flags);
+	list_for_each_entry(ctrl, &rport->ctrl_list, ctrl_list) {
+		if (__nvme_fc_options_match(opts, ctrl)) {
+			found = true;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&rport->lock, flags);
+
+	if (found) {
+		ret = -EALREADY;
+		goto out_fail;
+	}

Thus seems neither related to the intent of the patch, nor does it make sense at all.

#5 lore raw

v2 cover nvme_fc: add uevent to allow dynamic connects

In reply to James Smart
Quoted 7 lines · James Smart

These patches add support for generating a uevent which can be caugth and converted to a nvme-cli connect-all request for the FC initiator and target ports.

The patches were cut on nvme-4.12 after apply the patches contained in
http://lists.infradead.org/pipermail/linux-nvme/2017-May/010156.html

Quoted 8 lines

James Smart (2):
nvme_fc: create fc class and transport device
nvme_fc: add uevent for auto-connect

 drivers/nvme/host/fc.c         | 102 +++++++++++++++++++++++++++++++++++++++--
 include/linux/nvme-fc-driver.h |   2 +
 2 files changed, 100 insertions(+), 4 deletions(-)

Any comments on this series ?

-- james

#6 lore raw

v2 cover nvme_fc: add uevent to allow dynamic connects

In reply to James Smart
Quoted 3 lines · James Smart

These patches add support for generating a uevent which can be caugth and converted to a nvme-cli connect-all request for the FC initiator and target ports.

Can you post the nvme-cli side as well so that it can be reviewed together?

Quoted 4 lines

The patches were cut on nvme-4.12 after apply the patches contained in
http://lists.infradead.org/pipermail/linux-nvme/2017-May/010156.html

Quoted 16 lines

James Smart (2):
nvme_fc: create fc class and transport device
nvme_fc: add uevent for auto-connect

 drivers/nvme/host/fc.c         | 102 +++++++++++++++++++++++++++++++++++++++--
 include/linux/nvme-fc-driver.h |   2 +
 2 files changed, 100 insertions(+), 4 deletions(-)
Signature · 7 lines
2.11.0


_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

---end quoted text---

#7 lore raw

v2 cover nvme_fc: add uevent to allow dynamic connects

Quoted 7 lines · Christoph Hellwig
Quoted 3 lines · James Smart

These patches add support for generating a uevent which can be caugth and converted to a nvme-cli connect-all request for the FC initiator and target ports.

Can you post the nvme-cli side as well so that it can be reviewed together?

See the patch description in Patch 2. It has the udev event rule, which invokes nvme cli, that was used to test with.

-- james