[PATCH nvme-cli] nvme_fc auto-connect scripts

[PATCH nvme-cli] nvme_fc auto-connect scripts

[Hannes Reinecke]

Here's a set of script to enable FC-NVMe autoconnect, based on
the initial posting from James Smart.
FC-NVMe already generates an uevent whenever a new FC-NVMe remote
port is registered, so we can create an udev rule to parse this
event and start connecting to it.

The problem here is that the 'nvme connect-all' call might be taking
some time or might even stall altogether, so we cannot call it
directly from the udev rule. Rather, the udev rule starts a systemd
service, which then calls 'nvme connect-all' as a 'simple' service.
With this approach we are insulated from any stalls in udev rules.
Additionally we can implement a boot service to restart any missed
connections attempts.

To stop autoconnect an additional nvmefc-connect.target has been
added, which will instruct systemd to cancel all outstanding
autoconnect services.

Cc: James Smart <james.smart at broadcom.com>
Signed-off-by: Simon Schricker <sschricker at suse.com>
Signed-off-by: Hannes Reinecke <hare at suse.com>
---
 nvme-fc-autoconnect/70-nvmefc-autoconnect.conf      |  1 +
 nvme-fc-autoconnect/70-nvmefc-autoconnect.rules     |  8 ++++++++
 nvme-fc-autoconnect/nvmefc-boot-connections.service |  9 +++++++++
 nvme-fc-autoconnect/nvmefc-connect.target           |  2 ++
 nvme-fc-autoconnect/nvmefc-connect at .service         | 13 +++++++++++++
 5 files changed, 33 insertions(+)
 create mode 100644 nvme-fc-autoconnect/70-nvmefc-autoconnect.conf
 create mode 100644 nvme-fc-autoconnect/70-nvmefc-autoconnect.rules
 create mode 100644 nvme-fc-autoconnect/nvmefc-boot-connections.service
 create mode 100644 nvme-fc-autoconnect/nvmefc-connect.target
 create mode 100644 nvme-fc-autoconnect/nvmefc-connect at .service

diff --git a/nvme-fc-autoconnect/70-nvmefc-autoconnect.conf b/nvme-fc-autoconnect/70-nvmefc-autoconnect.conf
new file mode 100644
index 0000000..b92d94f
--- /dev/null
+++ b/nvme-fc-autoconnect/70-nvmefc-autoconnect.conf
@@ -0,0 +1 @@
+install_items+="/usr/lib/udev/rules.d/70-nvmefc-autoconnect.rules"
diff --git a/nvme-fc-autoconnect/70-nvmefc-autoconnect.rules b/nvme-fc-autoconnect/70-nvmefc-autoconnect.rules
new file mode 100644
index 0000000..aa4f9bf
--- /dev/null
+++ b/nvme-fc-autoconnect/70-nvmefc-autoconnect.rules
@@ -0,0 +1,8 @@
+#
+# nvme_fc: udev event to automatically scan (via discovery controller)
+#   new FC nvme remote ports and auto-connect to the subsystems they report.
+#
+
+ACTION=="change", SUBSYSTEM=="fc", ENV{FC_EVENT}=="nvmediscovery", \
+  ENV{NVMEFC_HOST_TRADDR}=="*",  ENV{NVMEFC_TRADDR}=="*", \
+  RUN+="/usr/bin/systemctl --no-block start nvmefc-connect at --host-traddr=$env{NVMEFC_HOST_TRADDR}\\x20--traddr=$env{NVMEFC_TRADDR}.service"
diff --git a/nvme-fc-autoconnect/nvmefc-boot-connections.service b/nvme-fc-autoconnect/nvmefc-boot-connections.service
new file mode 100644
index 0000000..aa35c15
--- /dev/null
+++ b/nvme-fc-autoconnect/nvmefc-boot-connections.service
@@ -0,0 +1,9 @@
+[Unit]
+Description=Auto-connect to subsystems on FC-NVME devices during boot
+
+[Service]
+Type=oneshot
+ExecStart=/bin/sh -c "echo add > /sys/class/fc/fc_udev_device/nvme_discovery"
+
+[Install]
+WantedBy=default.target
diff --git a/nvme-fc-autoconnect/nvmefc-connect.target b/nvme-fc-autoconnect/nvmefc-connect.target
new file mode 100644
index 0000000..953c0b7
--- /dev/null
+++ b/nvme-fc-autoconnect/nvmefc-connect.target
@@ -0,0 +1,2 @@
+[Unit]
+Description=All instances of nvme-fc-autoconnect daemon
diff --git a/nvme-fc-autoconnect/nvmefc-connect at .service b/nvme-fc-autoconnect/nvmefc-connect at .service
new file mode 100644
index 0000000..a8ff271
--- /dev/null
+++ b/nvme-fc-autoconnect/nvmefc-connect at .service
@@ -0,0 +1,13 @@
+#
+# Unit file used by 70-nvmefc-autoconnect.rules.
+#
+
+[Unit]
+Description=Auto-connect to subsystems on FC-NVME devices
+After=syslog.target
+PartOf=nvmefc-connect.target
+Requires=nvmefc-connect.target
+
+[Service]
+Type=simple
+ExecStart=/bin/sh -c "/usr/sbin/nvme connect-all --transport=fc `/usr/bin/echo -e '%i'`"
-- 
2.16.4

[PATCH nvme-cli] nvme_fc auto-connect scripts

[Sagi Grimberg]

Hannes,

I added these to the discovery log change stuff, which
makes nvme-fc-autoconnect service a unified service not just for
FC. I thought you had a look at the series.

[PATCH nvme-cli] nvme_fc auto-connect scripts

[Hannes Reinecke]

On 4/30/19 9:15 AM, Sagi Grimberg wrote:
> Hannes,
> 
> I added these to the discovery log change stuff, which
> makes nvme-fc-autoconnect service a unified service not just for
> FC. I thought you had a look at the series.

There had been some fixes to it (namely changing it to a 'simple' 
service instead of 'oneshot', and adding an nvmefc-connect.target to 
stop all running instances).
So I've just posted it here for completeness; I've found it rather weird 
sending a patch to a non-merged patchset.
Do feel free to include it in your patchset.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare at suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG N?rnberg)

[PATCH nvme-cli] nvme_fc auto-connect scripts

[Sagi Grimberg]

>> Hannes,
>>
>> I added these to the discovery log change stuff, which
>> makes nvme-fc-autoconnect service a unified service not just for
>> FC. I thought you had a look at the series.
> 
> There had been some fixes to it (namely changing it to a 'simple' 
> service instead of 'oneshot', and adding an nvmefc-connect.target to 
> stop all running instances).
> So I've just posted it here for completeness; I've found it rather weird 
> sending a patch to a non-merged patchset.
> Do feel free to include it in your patchset.

Sure, thanks.

[PATCH nvme-cli] nvme_fc auto-connect scripts

[Hannes Reinecke]

On 12/19/18 2:13 AM, James Smart wrote:
> where do we sit with this ?
> 
I'll be working on updating nvme-cli to accept 'none' parameters, and 
then I'll update the patch to be transport-agnostic.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare at suse.com			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: F. Imend?rffer, J. Smithard, D. Upmanyu, G. Norton
HRB 21284 (AG N?rnberg)

[PATCH nvme-cli] nvme_fc auto-connect scripts

[James Smart]

where do we sit with this ?

-- james


On 11/20/2018 8:05 AM, Hannes Reinecke wrote:
> On 11/19/18 11:53 PM, Sagi Grimberg wrote:
>>
>>> From: James Smart <jsmart2021 at gmail.com>
>>>
>>> Sagi recently posted a "one-shot" call to nvme-connect via a systemd
>>> script. FC has no problems operating with it assuming all the required
>>> connect fields are present (host_traddr required).
>>
>>
>> I think that this is a good approach. Can we add a preparatory patch
>> that will allow nvme-cli to accept nil/none arguments? This way
>> we can have the kernel shoot consistent udev events where parameters
>> can stay don't-care and we can have the same rule...
>>
>> If nvme-cli can accept a command in the form:
>> nvme connect-all --traddr=<a> --host-traddr=<b> --trsvcid=none
>>
> Good point, ans something which I had been thinking about, too.
> But really it only makes sense if these scripts are supposed to work 
> on different fabrics.
>
>> I think we can consolidate a bit more (it will be a different device
>> because for IP stuff the device will be the controller char device).
>>
>> Perhaps we should change the name 70-nvmefc-autoconnect.rules to
>> 70-nvmf-autoconnect.rules that will contain rules fc and IP in the
>> same rule file...
>>
> Of course we can make it more general.
>
> It was just that buy-in from other subsystems into doing autoconnect 
> was rather reluctant.
>
> I'll be resending the patches.
>
> Cheers,
>
> Hannes

[PATCH nvme-cli] nvme_fc auto-connect scripts

[Hannes Reinecke]

On 11/19/18 11:53 PM, Sagi Grimberg wrote:
> 
>> From: James Smart <jsmart2021 at gmail.com>
>>
>> Sagi recently posted a "one-shot" call to nvme-connect via a systemd
>> script. FC has no problems operating with it assuming all the required
>> connect fields are present (host_traddr required).
> 
> 
> I think that this is a good approach. Can we add a preparatory patch
> that will allow nvme-cli to accept nil/none arguments? This way
> we can have the kernel shoot consistent udev events where parameters
> can stay don't-care and we can have the same rule...
> 
> If nvme-cli can accept a command in the form:
> nvme connect-all --traddr=<a> --host-traddr=<b> --trsvcid=none
> 
Good point, ans something which I had been thinking about, too.
But really it only makes sense if these scripts are supposed to work on 
different fabrics.

> I think we can consolidate a bit more (it will be a different device
> because for IP stuff the device will be the controller char device).
> 
> Perhaps we should change the name 70-nvmefc-autoconnect.rules to
> 70-nvmf-autoconnect.rules that will contain rules fc and IP in the
> same rule file...
> 
Of course we can make it more general.

It was just that buy-in from other subsystems into doing autoconnect was 
rather reluctant.

I'll be resending the patches.

Cheers,

Hannes

[PATCH nvme-cli] nvme_fc auto-connect scripts

[Sagi Grimberg]

> From: James Smart <jsmart2021 at gmail.com>
> 
> Sagi recently posted a "one-shot" call to nvme-connect via a systemd
> script. FC has no problems operating with it assuming all the required
> connect fields are present (host_traddr required).


I think that this is a good approach. Can we add a preparatory patch
that will allow nvme-cli to accept nil/none arguments? This way
we can have the kernel shoot consistent udev events where parameters
can stay don't-care and we can have the same rule...

If nvme-cli can accept a command in the form:
nvme connect-all --traddr=<a> --host-traddr=<b> --trsvcid=none

I think we can consolidate a bit more (it will be a different device
because for IP stuff the device will be the controller char device).

Perhaps we should change the name 70-nvmefc-autoconnect.rules to
70-nvmf-autoconnect.rules that will contain rules fc and IP in the
same rule file...

[PATCH nvme-cli] nvme_fc auto-connect scripts

[Hannes Reinecke]

From: James Smart <jsmart2021@gmail.com>

Sagi recently posted a "one-shot" call to nvme-connect via a systemd
script. FC has no problems operating with it assuming all the required
connect fields are present (host_traddr required).

However FC is a bit more dynamic, and we've built up scripts that can
be triggered whenever a new FC remote port with a discovery subsytem is
detected. Whenever such a port is detected (by registration of the
remoteport with the nvme_fc transport by the lldd), the transport
generates a udev event.  A udev rule is created that parses the event
information into the FC remote port (traddr) that supports a discovery
subsystem and the FC host port that is connected to the FC remote port
(host_traddr). The rule then invokes an instance-based systemd service
to perform a "nvme connect-all" with the traddr and host-traddr info.

To resolve some issues seen by discovery of remote ports before the udev
service was running we've added an additional system service
nvmefc-boot-connections.service which will retrigger the event by
writint 'add' into the 'nvme_discovery' sysfs attribute.

This patch has been adapted from an earlier patch from James Smart.

Signed-off-by: James Smart <james.smart at broadcom.com>
Signed-off-by: Hannes Reinecke <hare at suse.com>
---
 Makefile                                |  9 +++++++++
 nvme-fc/70-nvmefc-autoconnect.rules     |  8 ++++++++
 nvme-fc/nvmefc-boot-connections.service |  9 +++++++++
 nvme-fc/nvmefc-connect at .service         | 14 ++++++++++++++
 4 files changed, 40 insertions(+)
 create mode 100644 nvme-fc/70-nvmefc-autoconnect.rules
 create mode 100644 nvme-fc/nvmefc-boot-connections.service
 create mode 100644 nvme-fc/nvmefc-connect at .service

diff --git a/Makefile b/Makefile
index 70541cb..8b9d0e1 100644
--- a/Makefile
+++ b/Makefile
@@ -8,6 +8,8 @@ DESTDIR =
 PREFIX ?= /usr/local
 SYSCONFDIR = /etc
 SBINDIR = $(PREFIX)/sbin
+SYSTEMDDIR = /usr/lib/systemd
+UDEVDIR = /usr/lib/udev
 LIB_DEPENDS =
 
 ifeq ($(LIBUUID),0)
@@ -86,6 +88,13 @@ install-bash-completion:
 	$(INSTALL) -d $(DESTDIR)$(PREFIX)/share/bash-completion/completions
 	$(INSTALL) -m 644 -T ./completions/bash-nvme-completion.sh $(DESTDIR)$(PREFIX)/share/bash-completion/completions/nvme
 
+install-udev:
+	$(INSTALL) -d $(DESTDIR)$(SYSTEMDDIR)/system
+	$(INSTALL) -m 644 ./nvme-fc/nvme-fc-connect at .service $(DESTDIR)$(SYSTEMDDIR)/system
+	$(INSTALL) -m 644 ./nvme-fc/nvmefc-boot-connections.service $(DESTDIR)$(SYSTEMDIR)/system
+	$(INSTALL) -d $(DESTDIR)$(UDEVDIR)/rules.d
+	$(INSTALL) -m 644 ./nvme-fc/70-nvme-fc-autoconnect.rules $(DESTDIR)$(UDEVDIR)/rules.d
+
 install: install-bin install-man install-bash-completion
 
 nvme.spec: nvme.spec.in NVME-VERSION-FILE
diff --git a/nvme-fc/70-nvmefc-autoconnect.rules b/nvme-fc/70-nvmefc-autoconnect.rules
new file mode 100644
index 0000000..aa4f9bf
--- /dev/null
+++ b/nvme-fc/70-nvmefc-autoconnect.rules
@@ -0,0 +1,8 @@
+#
+# nvme_fc: udev event to automatically scan (via discovery controller)
+#   new FC nvme remote ports and auto-connect to the subsystems they report.
+#
+
+ACTION=="change", SUBSYSTEM=="fc", ENV{FC_EVENT}=="nvmediscovery", \
+  ENV{NVMEFC_HOST_TRADDR}=="*",  ENV{NVMEFC_TRADDR}=="*", \
+  RUN+="/usr/bin/systemctl --no-block start nvmefc-connect at --host-traddr=$env{NVMEFC_HOST_TRADDR}\\x20--traddr=$env{NVMEFC_TRADDR}.service"
diff --git a/nvme-fc/nvmefc-boot-connections.service b/nvme-fc/nvmefc-boot-connections.service
new file mode 100644
index 0000000..aa35c15
--- /dev/null
+++ b/nvme-fc/nvmefc-boot-connections.service
@@ -0,0 +1,9 @@
+[Unit]
+Description=Auto-connect to subsystems on FC-NVME devices during boot
+
+[Service]
+Type=oneshot
+ExecStart=/bin/sh -c "echo add > /sys/class/fc/fc_udev_device/nvme_discovery"
+
+[Install]
+WantedBy=default.target
diff --git a/nvme-fc/nvmefc-connect at .service b/nvme-fc/nvmefc-connect at .service
new file mode 100644
index 0000000..c2370d6
--- /dev/null
+++ b/nvme-fc/nvmefc-connect at .service
@@ -0,0 +1,14 @@
+#
+# Unit file used by 70-nvmefc-autoconnect.rules.
+#
+
+[Unit]
+Description=Auto-connect to subsystems on FC-NVME devices
+After=syslog.target
+
+[Service]
+Type=oneshot
+ExecStart=/bin/sh -c "/usr/sbin/nvme connect-all --transport=fc `/usr/bin/echo -e '%i'`"
+
+[Install]
+WantedBy=default.target
-- 
2.13.7