Re: [PATCH 1/3] nvmet: expose discovery subsystem in sysfs

[John Meneghini <jmeneghi@redhat.com>]

Sorry I'm late to the party.  Please see my comments below.

On 3/15/22 05:49, Christoph Hellwig wrote:
> On Tue, Mar 15, 2022 at 10:06:26AM +0100, Hannes Reinecke wrote:
>> The core question really is: do we _want_ to expose the discovery subsystem
>> in configfs?
> 
> Well, if you want a freely configurable one we kinda have to, right?
> 
>> Unfortunately, exposing the discovery subsystem and trying to configure it
>> with configfs does _not_ match with the way discovery is implemented today.
>> While we currently only have a single discovery subsystem, it will only
>> ever return the subsystems visible from this particular port.

I don't see why this would need to change. What is it that you want to configure
in the new unique discovery subsystem(s) that would be any different from the
existing well known discovery subsystem?

> Well.  The original Fabrics spec had this concept of that magic discovery
> NQN, which implies that there is one subsystem (or many pretending to be
> one).  And that is what the implementation followed.  The varipus 80?? TPs
> then made a complete mess off that.

I agree that FMDS has made an overly complicated mess of NVMe-oF Discovery
with the new Discovery TPs. However, I am hoping that TP-8013 and TP-8014
could be used to help with some of the problem.

>> Hence this rather simple approach, having the 'normal' discovery subsystem
>> exposed, and let the admin configure it accordingly.
>>
>> I can look at keeping the internal implementation, and only expose unique
>> discovery controller (ie those with a unique subsystem NQN).
>> That would remove the need to having the 'discovery_nqn' attribute, and
>> address Christophs concerns.
> 
> I suspect if we want to support all the new mess from the FMDS group
> (and maybe we need to question the why a little more), then we should
> so something like:
> 
>   (1) keep the existing global NQN-based discovery as-is.

I agree that we need some way to support legacy hosts and legacy controllers
in the same fabric. What ever we do with TP-8010, etc., we need to be sure that
all hosts and all discovery controllers interoperate cleanly.

>   (2) maybe add a per-port known to allow disabling it if people really care
>   (3) allow creating additional discovery subsystems with non-default
>       NQNs that do not automatically get anything added to them and will
>       just be configured as needed through configfs
> 
> But maybe first we should take a step back and figure out what supporting
> TPAR8013 even buys us?

TP-8013 was designed to work with TP-8014. In fact, at one point we talked about
combining these two TPs into a single TP. The idea behind TP-8013 & 8014 was:

  1. All hosts will connect to the existing Discovery Service with the Well Known
     Discovery NQN and retrieve the Discovery Log pages for the HostNQN provided
     in the Fabric Connect command, as it is done today.

     It was assumed that Authenticating with the Well Known Discovery NQN would
     would not be needed or supported because:
     a) The Discovery Controller controls the Authenticate work flow and
        returning AUTHREQ=1 in the connect response would break legacy hosts.
     b) It doesn't make sense to have a Well Known Discovery NQN as a part of a psk.

  2. Discovery Controllers which support Authentication can return Discovery
     Log Page Entries with Subsystem Type (SUBTYPE): 03h - as defined by TP-8014.
     These DLPEs will contain Unique Discovery NQNs - as defined by TP-8013

  3. Hosts that support Authentication can then disconnect from the Well Known
     Discovery Controller and re-connect with the Unique Discovery NQN. These
     hosts should expect an AUTHREQ=1 response.

  4. Hosts that don't want to support Authentication can ignore the SUBTYPE 03h
     Log Page Entries and operate normally. This would include legacy hosts.

Hopefully, with some kind of a design like this, both legacy (non-authenticating)
and new (authenticating) hosts and discovery controllers can interoperate.

/John