[nfsv4] Review of draft-ietf-nfsv4-scsi-layout-nvme-01

[David Noveck <[e-mail]>]

*General comments *

*Overall evaluation*

Document is pretty far along but has issues, primarily editorial, that need
to be addressed.  With regard to Christoph's hypothesis of "technical
perfection", I am not able to judge but am prone to feel that further
review from knowledgeable working group  members will be necessary.

*Remnants of Informational Approach*

Although the details will be more completely discussed under *Per-section
Comments* there seem to  be  cases in which the wording suggests an
informational approach that is no longer appropriate  For example, the word
"explain" is used in many contexts where it is the job of a standards-track
document  to "define" or  "specify" something.

*Issues with bcp14 Terms*

These terms will need to be wrapped in "<bcp14>...</bcp14>.

There are also a number of problems with some uses of the terms "MUST" and
"SHOULD" :

   - it is often not made sufficiently clear exactly who is to be
   constrained or is to follow the requirement/recommendation.
   - For "SHOULD", it is sometimes not clear what could "valid reasons" to
   bypass the recommendation.

*Per-section comments *

*Authors*

I think you need to designate an editor.

*Title*

Given the decision to make this a standards-track document, feel something
like "Use of NVMe-based Transports to Access Block Devices using the SCSI
Layout Type" would be more appropriate.

*Abstract *

I feel "explains" is now inappropriate and suggest a replacement like the
following:

Defines the use of block devices accessed by NVMe-based Transports in
connection with the pNFS SCSI layout type.


*1. Introduction *

I feel that the first paragraph presumes a lot of knowledge that the
authors and most working group members have but that many readers might not
have. An example is the use of the term MDS which is never defined in this
document.

I think you need a brief statement of the prerequisite knowledge to read
this document. I'm thinking of something like.

NFSv4.1 (in [RFC8881]) includes a pNFS feature that allows reads and writes
to be performed by other means than directing read and write operations to
the server.  Through use of this feature. The server, in the role of
metadata server is responsible for managing file and directory metadata
while separate means are provided for execution of reads and writes.


These other means of effecting file read and writes are defined  by
individual mapping types which often have their own specifications.  The
SCSI Layout Type, defined in RFC8154, describes how IO is to be done
directly to block storage devices.


In the existing second paragraph, use  of "explains" is now wrong so
suggest something like the following.

This document defines how block devices exported by NVMe Controllers
implementing the NVMe Base specification ([NVME-BASE)] are to be used
within the SCSI Layout Type. The definition  is independent  of the
underlying transport used by the NVMe Controller and thus supports
Controllers implementing a wide variety of transports, including PCIe
Express, RDMA, TCP and FibreChannel.


In the third paragraph, have the same issue with "explains" and so suggest:

This document does not amend the existing SCSI layout document.  Rather, it
defines how NVM-accessed devices can  be used within the SCSI Layout by
establishing a mapping of the SCSI constructs used in the SCSI layout
document to  corresponding NVMe constructs.


Note that while I support this as a goal, I have some questions (in *2.2
Volume Identification* and *2.3 Write Caches*) as to whether it is
currently followed.

*1.2 General Definitions*

With regard to the definition of "client", I have problems with the use of
the word "traditional" in connection with clients embedded within OS's that
support multiple applications.  I don't think this belongs here.

With regard to "server", I note that "server" is defined here but never
really used in this document, except in the definition of "client".
Elsewhere there are references to the metadata server and to the data
server. I  think these need to be defined rather than the orphaned term
"server".

*2. SCSI Layout Mapping to NVM*

In the first sentence,

Suggest replacing "only references few" by "references only a few".

Believe "SCSI specific" should be hyphenated.

With regard to the use of the word  "SHOULD", It is unclear to whom this
recommendation is directed.

Recommendations are generally as to what an implementation might do or not
do and mapping of concepts doesn't fit that model.

It is hard to conceive of what might be "valid reasons" to do other than
what is recommended.

*2. 1 Volume Identification *

I have concerns regarding the "SHOULD" about the situation in which both
EUI64 and  NGUId are available.  If the shorter id is allowed and it is
allowed if the longer one is "unavailable" then it is OK functionally to
use it and it is unclear how the availability of the longer id would change
that or require that some  "valid reasons" exist to allow use of the
shorter id.  Feel this would  be better either as "should" or "is
preferable".

Regarding the banning of the use serial numbers, I agree that they not be
allowed but I'm guessing that they were allowed in RFC8154, which calls
into question the idea that this spec does not amend RFC8154.  It seems
that it does, although for a good reason.

Along these lines, there might need to be some consideration of the case in
which the SCSI layout is used to support both SCSI and NVMe-connected
devices.  Is it possible that SCSI serial number might conflict with EUI64
or NGUID device ids.

The third paragraph does not make it sufficiently clear what exactly  are
the "unique addressing needs" that need to be satisfied.

*2.2 Client Fencing*

In the second sentence of the first paragraph, suggest replacing "For this"
by "For this to be achieved".

In the second paragraph suggest replacing "The following is" by "The
following sub-sections (Section 2.2.1 through 2.2.4) provides".

*2.3 Write Caches*

In this case, it appears that we are not, as with other subsections of *2.0
SCSI Layout Mapping to NVMe*, mapping a SCSI operation to an NVMe one. If
so,
this should be moved to its own top-level section and further tinkering
with the statement about not amending RFC8154 might be required.

*4. Security Considerations *

Think you need to add a new paragraph regarding the obligation of the
clients regarding the enforcement of MDS decisions.   I am proposing
something like the following after the current first paragraph:

Decisions regarding authorization of particular users to access file
objects are made by the MDS and communicated to the clients by the ranting,
recall, and revocation of layouts, as described in the definition of
the pNFS feature (see [RFC8881}, [RFC8534]).  As a result, the security of
the data made available to clients depends on clients not making

IO requests without the appropriate layout.   When clients cannot be
trusted to conform to this requirement, direct access to block devices MUST
NOT be provided.


*5.Normative References *

Feel RFC8534 should be added