nvme-discover: Use 'nvme_info' sysfs attribute for fc transport

Hi all,

here's a small patchset to use the 'nvme_info' sysfs attribute from the SCSI host to provide the parameters for 'nvme discover'.
So with this patch it's sufficient to state

nvme discover -t fc

and all discovery log entries are returned.

I know that sysfs attribute is non-standard, but I'll be working on a patch to implement it for qla2xxx, too.

As usual, comments and reviews are welcome.

Hannes Reinecke (2):
nvme-discover: use ASCII characters in documentation
nvme-discover: parse 'nvme_info' sysfs attribute for fc transport

 Documentation/nvme-discover.txt |   7 +-
 fabrics.c                       | 193 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 198 insertions(+), 2 deletions(-)

2.16.4

Somehow a non-ASCII 's' slipped into the documentation for 'nvme discover'.

---
 Documentation/nvme-discover.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 51d63e4..b4ae2fc 100644
@@ -29,7 +29,7 @@ Discovery commands to run.  If no /etc/nvme/discovery.conf file
 exists, the command will quit with an error.
 
 Otherwise, a specific Discovery Controller should be specified using the
---transport, --traddr, and if necessary the --trsvcid flags. A Di?covery
+--transport, --traddr, and if necessary the --trsvcid flags. A Discovery
 request will then be sent to the specified Discovery Controller.
 
 BACKGROUND

2.16.4

The 'nvme_info' sysfs attribute for the SCSI host already contains all relevant information to issue a NVMe discovery. So for those cases we could use that information to format a discovery command and don't have to specify a discovery.conf file.

This patch adds the infrastructure for parsing the information in the 'nvme_info' sysfs attribute and uses that information to attempt to connect to the discovery controller on that transport address.

The user can select this feature by using 'nvme discover -t fc'.

---
 Documentation/nvme-discover.txt |   5 ++
 fabrics.c                       | 193 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 197 insertions(+), 1 deletion(-)

index b4ae2fc..7a9fad2 100644
@@ -28,6 +28,11 @@ find a /etc/nvme/discovery.conf file to use to supply a list of
 Discovery commands to run.  If no /etc/nvme/discovery.conf file
 exists, the command will quit with an error.
 
+If just --transport=fc is specified, then 'nvme discover' will attempt
+to read the sysfs attribute 'nvme_info' from the SCSI Host sysfs
+directory, and send the Discovery request to the Discovery Controllers
+specified by the transport addresses found in that file.
+
 Otherwise, a specific Discovery Controller should be specified using the
 --transport, --traddr, and if necessary the --trsvcid flags. A Discovery
 request will then be sent to the specified Discovery Controller.

2.16.4

Well....?? I know it probably works well, but it was not in the direction I was going.

Here's my issues:
1. the nvme_info sysfs parameter is not part of any transport and is a lpfc-specific attribute. So adding this into nvmecli concerns me. I don't know that lpfc has even considered the text format sufficient for being an "api" that can be called by "nvmecli" - meaning I don't know that it will pass the backward-compatibility tests.
2. What you are looking for is unnecessary: The patch to repost connectivity events has been post. Christoph just needs to review it again. I'll repost.
3. I also don't like parsing scsi stuff in the nvme cli. If we really need to pull fc-isms from it, then we should start the work for moving the fc transport out of scsi and into it's own layer that binds with scsi and nvme.? But I'd rather finish the fixes in nvme cli to address async connections, retries while controller being reconnected to, etc - that all enable the simple udev script.

-- james

Well, _actually_ I sort-of-expected this response :-)
It was primarily a reminder to folks that there are open issues where we need to look into.
(And actually I'm waiting for TP8002 to be ratified, as this implements quite some key logic I'd need...)

My plan here is as follows:

- Implement an 'async-connect' option to nvme-cli, which would instruct the kernel code to _not_ wait for the connect/reconnect workqueue function but rather return immediately.
- Implement a workqueue function for the discovery controller to retrieve the discovery log page, store it in a sysfs attribute, and send an uevent for each found subsystem NQN.
- Invoke this workqueue whenever 'nvme discover' is called with the async-connect option enabled.

With this we have the following workflow:

1. FC-NVMe posts an uevent whenever an FC-NVMe target port has been found
2. A udev rule parses this event and calls 'nvme discover --async-connect' to the target address of these ports
3. The 'discovery' workqueue function retrieves the discovery log page and issues uevents for each found subsystem NQN
4. A udev rule parses this event and calls 'nvme connect --async-connect' for each of these events

The first step is actually optional; we can invoke 'nvme discover --async-connect' even manually from a systemd service during boot up. Which has the nice advantage of working for RDMA, too.

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: F. Imend?rffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG N?rnberg)

This is something I'd love to see actually. Something like net/fc or drivers/fc. IIRC we had drivers/fc back in the 2.4 days, didn't we?

Byte,
Johannes

Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Using arbitrary nvme info from scsi lpfc implementation is not going to work... We need something that can be more generic and bound within nvme.

Besides, fc already has udev infra to auto connect IIRC.