[PATCH] nvme-core: update NS Attr Changed AEN handling for ANA group

[Knight, Frederick]

"Deeper mess" - I do not know, but this DOES sort out the misunderstandings.  Whether it does what we want is a different question.

Create and attach are discrete actions in NVMe.  VS management tools may have a single command that invokes both actions (making it appear they are a single action, but they are not when it comes to how NVMe describes what happens):

1) Creation of a namespace (no matter how it happens) adds a namespace ID to IDENTIFY (CNS=10h).  This action does NOT change IDENTIFY (CNS=2h).
If we look at the AENs, the Namespace Attribute Changed AEN happens for a change to one or both of the following:
a) the Identify Namespace Data Structure for one or more namespaces; or
b) the Namespace List returned for IDENTIFY (CNS=2h).

Clearly, a create NS changes CNS=10h, and does NOT change CNS=02h, so b) isn't happening.  As for a), that is a little more murky.  Consider, that CNS=2h (attached namespaces) is specifically mentioned, and CNS=10h is specifically absent. Does that suggest the intention was to only send the AEN when something related to an ATTACHED namespaces changes?  Well, the exact text of a) includes CNS=11h.  CNS=11h does contain an Identify Namespace Data Structure, and it DOES change when you create a namespace (although, the namespace is only allocated, and not attached).  So was the intent to send an AEN for the creation of an allocated NS, or just for an attached NS?

What happens to the ANA Log page when a Namespace is created - nothing, therefore NO AEN about changes to that log page (it is only about attached namespaces).

Neither the Identify Namespace data changed AEN, nor the CNS=11h exist prior to rev 1.2.  They were added as part of 1.2 development.  I have not yet been able to dig out the history to see if there are any hints as to the original intent.  Is the a) item supposed to be only about CNS = 0h, or is it intended to apply to CNS = 11h too? When we invented  CNS = 11h, did we forget to exclude that from a) (did we forget to add the word "attached" to a))? OR, was it intentional?

2) Attach of a namespace (no matter how it happens) adds a namespace ID to IDENTIFY (CNS=02h).  This action DOES change IDENTIFY (CNS=0h and CNS=2h). That means the Identify Namespace Attribute Changed AEN is DOES happen (case b) above, and case a) above (CNS = 0h changes)).

In this case, the ANA Log page has a new entry added (either adding just 1 NSID - to an existing ANA Group, or adding a new ANA Group that contains the newly attached NS).  Either way, the Log page changes, and that means the ANA Change AEN happens.

So for case 2 (the attach), there are 2 AENs for effectively the same event.  This is where we are at with the existing text.

Whether this is what the hosts want, that is a good question. What changes are desired (if any)?

	Fred

-----Original Message-----
From: hch at lst.de <hch at lst.de> 
Sent: Wednesday, December 9, 2020 12:47 PM
To: Keith Busch <kbusch at kernel.org>
Cc: Hannes Reinecke <hare at suse.de>; Knight, Frederick <Frederick.Knight at netapp.com>; hch at lst.de; Meneghini, John <John.Meneghini at netapp.com>; George, Martin <Martin.George at netapp.com>; linux-nvme at lists.infradead.org; sagi at grimberg.me; Hannes Reinecke <hare at suse.com>
Subject: Re: [PATCH] nvme-core: update NS Attr Changed AEN handling for ANA group

NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe.




On Thu, Dec 10, 2020 at 02:39:36AM +0900, Keith Busch wrote:
> > The use-case I'm trying to describe is that the admin on the storage 
> > array is creating a namespace and attaching it to an existing 
> > subsystem, completely without interaction from the host. It just so 
> > happens that this new namespace has a different ANA group than the 
> > existing namespaces in this subsystem.
> > Then the array has to notify the host about this.
> > And the whole discussion is about which AENs this controller/storage 
> > array should be sending.
>
> Fred keeps saying the spec's rules for NVMe's namespace create command 
> from section 5.20 allow him to not send events, but it turns out 
> you're not even using this command? Why would the spec's defined 
> behavior apply to this proprietary use case?

I think we are in an even deeper mess here than I though.

 - one issue is the fact that the exception gets creating vs attaching
   and deleting vs detaching wrong, and that probably is my fault.
 - the other one is the wording should be shall not send the event
   when the creation is ONLY due to the creation of a namespace, that
   is not when a new ANA group shows up (and the way how ANA groups
   are created is out of scope)

So if we do a textual interpretation of the spec, the controller does not only need to send the ANA AEN for the case we are debatting here, but also for the case I specifically wanted to exclude.  Sigh.