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

[Knight, Frederick]

It is clear that the NS attribute change is required:
Namespace Attribute Changed: Indicates a change to one or both of the following:
•	... ; or
•	the Namespace List returned when the Identify command is issued with the CNS field set to 02h.

It seems clear to me that the ANA change IS prohibited:
Asymmetric Namespace Access Change: The Asymmetric Namespace Access information (refer to section 5.14.1.12) related to an ANA Group that contains namespaces attached to this controller has changed (e.g., an ANA state has changed, an ANAGRPID has changed). The current Asymmetric Namespace Access information for attached namespaces is indicated in the Asymmetric Namespace Access log page (refer to section 5.14.1.12).

We CANNOT deliver this AEN unless it is related to an ANA group that contains namespaces attached to this controller.  The creating of a new ANA group (which therefore has no namespaces) does not meet the criteria of "containing namespaces attached to this controller".  So there are 2 possible orders - 1) the ANA group is created before the namespace, or 2) the namespace is created before the namespace.  If the Group is created before the namespace, then there are NO attached namespaces in that (brand new) ANA group, therefore, we are prohibited from sending the AEN.  If the namespace is created before the ANA group, then we hit the next language, that prohibits sending the ANA changed AEN because is it "due to the creation of a namespace".

We are also prohibited from sending this AEN by the following language:
A controller shall not send this event if:
a)	the change is due to the creation of a namespace (refer to section 5.20); or
b)	the change is due to the deletion of a namespace (refer to section 5.20),
as the Namespace Attribute Changed event is sent for these changes.

Therefore, if the change "is due to the creating of a namespace" (as in sending a NS Management command to create a new NS, or other administrative action), then the ANA Change AEN shall not be sent.

So, there are TWO prohibitions, and ZERO requirements for sending an ANA Changed AEN for this situation.  Please show me the text that requires an ANA Changed AEN for this situation.  I just can't find it.  You're welcome to bring a new TPAR to add that requirement, but from my reading, it is a new requirement.

	Fred Knight


-----Original Message-----
From: hch at lst.de <hch at lst.de> 
Sent: Monday, December 7, 2020 10:26 AM
To: Knight, Frederick <Frederick.Knight at netapp.com>
Cc: Meneghini, John <John.Meneghini at netapp.com>; hch at lst.de; George, Martin <Martin.George at netapp.com>; kbusch at kernel.org; 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.




Getting back to this while I'm reviewing the latest competing patches.

On Mon, Nov 23, 2020 at 03:27:35PM +0000, Knight, Frederick wrote:
> Here is what I would expect to:
> A) create a namespace in existing ANA group:
>       1) NS Attribute Changed AEN (because the NS List returned has changed)
>       2) Host gets the list - discovers the NEW NS
>               Including: Determine which ANAGRPID that NS belongs to (for which the host already knows the state).

Yes.

>
> B) create a namespace in a NEW ANA group
>       1) NS Attribute Changed AEN (because the NS List returned has changed)
>       2) Host gets the list - discovers the new NS
>               Including: Determine there is a BRAND NEW ANAGRPID that the host knows NOTHING about.
>               Since the host knows NOTHING about that ANAGRPID, it seems the host should go look.

I strongly disagree.

> It seems that the existing text says there is NO ANA AEN for a "new" namespace.  John quoted the text:
>
> A controller shall not send this event if:
> a)    the change is due to the creation of a namespace (refer to section 5.20); or
> b)    the change is due to the deletion of a namespace (refer to section 5.20),
> as the Namespace Attribute Changed event is sent for these changes.

Exactly!  But only if the change is due the creation or deletion of a namespaces.  Note the creation of both a namespace and a ANA group at the same time.  In fact I don't see anything in NVMe which would allow for this compound operations.  You always need to create an ANA group first, and can then create a namespace using it, and you need to send the Asymmetric Namespace Access Change AEN that a new ANA group has been created, and then a Namespace Attribute Changed one.


> Christoph claims that the ANA AEN should be send before the NS Attribute Changed AEN.  And, that could be true for an ANAGRPID that changed (although, I don't see text describing any precedence or order requirements for delivery of AENs), but the NS creation case is explicitly excluded.  FWIW – Christoph was one of the people that requested this exclusion to prevent spaming the host with multiple AENs for the same event (a NS create).

Yes, and the exclusion as it exists in the spec makes perfect sense - the host does not need two AEN to notify that a namespace has been created and as part of the creation was added to an AEN group.  But the host does need an AEN when a new group has been created, period.  There also is no language that allows skipping that.