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

[David Noveck <[e-mail]>]

General Comments Overall Evaluation

>From a technical point of view this document is on good shape, and should
allow implementers of the SCSI layout to use NVMe-connected devices and
obtain the performance benefits provided by that configuration.


The issues that I found that are mostly editorial.  The remainder deal with
issues that the IESG may be concerned about, but will not really be an
issue for most implementers.  I’ve given my views about these issues but
you should be aware that these comments can be divided into three groups:

·       Those that will help you avoid difficulties with the IESG.

·       Those that address issues that the IESG is not typically concerned
with, such as the correct definition of RFC2119 terms

·       Those where my view is not likely to find favor with the IESG.

You can make your choices on all of these based on whether you agree with
me and how combative you feel.
Path forward for this Document

Although you have not formally requested this step, the working group
should be considering this document as a candidate working group document.
After all, the proposed charter draft was written with the assumption that
such work as this, to take advantage of the work done by RFC8154, would be
done and needed to be in-scope for the working group.


My recommendation is that you make the request whenever you are comfortable
doing so.  In particular, I don't think it should be held up by changes
suggested in the rest of this review.  In my opinion, there are no
technical gaps that would raise issues about moving this to be a working
group document and all the suggested changes could be addressed after that
conversion is made.
Issue of Proposed Status

Regarding the designation of this document as "informational", I'm
uncertain about whether this is right.  I think it makes sense for the
working group to discuss this issue as part of the process of making this a
working group document. Although we could change this later, it is simpler
if we resolved this question soon.

On the one hand, you are providing information about the proper mapping.
However, it is necessary that everybody follow these instructions or things
will not work correctly.  Presumably, this is why you use the RFC2119
keywords.  The information within this document is about the right way to
do things, so perhaps standards-track is more appropriate.


More about this under *1.1. Conventions Used in This Document.*
Comments By Section Title

I'm OK with your current title:


Using the Parallel NFS (pNFS) SCSI Layout with NVMe


But I'm worried that the IESG is going to push for something like:


Using the Parallel NFS (pNFS) Small Computer Systems Interface (SCSI)
Layout with Non-Volatile Memory Express (NVMe).


Perhaps the IESG is so intent on this that resistance is useless, but I
have problem with a very long review process which results in a document
which is less accessible to the audience it is written for. It could be
worse.  They might want:


Using the Parallel Network File System (pNFS) Small Computer Systems
Interface (SCSI) Layout with Non-Volatile Memory Express (NVMe).


*Abstract*



Suggest:

·       Replacing "with transports" by "when accessing devices connected
using"

·       With regard to the object of "using", both NVMe and NVMe over
Fabrics are mentioned so perhaps, if these are both protocols, then you
could pluralize "protocol".  Alternatively, you could say "using the NVMe
interface.  This includes connections using the NVMe over Fabrics protocol."

*1.      Introduction*

In the first sentence suggest:

·       replacing "to directly perform I/O" by "to perform I/O directly".

·       replacing "while bypassing the MDS" by "without interacting with
the Metadata server (MDS)."


The last sentence of the paragraph is run-on.  Suggest ending the sentence
after "way" and starting the next sentence "Instead,"
1.1.  Conventions Used in This Document

The problem here is that RFC2119 only explains how these terms are to be
used in standards-track documents, so it is not clear what they mean in an
informational document.
1.2.  General Definitions

The problem in this section is  that the definitions do not really reflect
the fact that pNFS is being used.


For Client suggest:


A "client" is an entity that accesses the NFS server's resources including
those made available by pNFS layouts.  The client may be an application
that contains the logic to access the NFS server and devices designated by
pNFS layouts directly.  The client may also be the traditional operating
system client that provides remote file system services for a set of
applications.


For Server suggest:



A "server" is an entity responsible for coordinating client access to a set
of file systems, including granting, recalling, and revoking associated
layouts as necessary. It is identified by a server owner.


Another alternative is to have separate definitions of “Server” and
“Metadata Server (MDS)”.


*2.      SCSI Layout mapping to NVMe*

I have problems with the use of the term “SHOULD” in this document, beyond
the fact that this document is supposedly Informational.  In particular,
what are the negative consequences of *not *using the NVMe Translation
reference that an implementer is supposed to be aware when he decides not
to do the translation that way.  It is hard to escape the conclusion that
the result will either fail to interoperate with the other implementation,
or if it does work will not be interoperable with other implementations
that do things using this translation layer.  In other words, this is a
recipe for total chaos.  It appears that, if you do use an RFC2119 term,
the only valid one is “MUST”.


Below I’ve tried to revise this section to not use RFC2119 terms.  This is
more compatible with this being an Informational document.


The SCSI layout definition [RFC8154] only references a few SCSI-specific
concepts directly.  The appropriate mapping of these SCSI concepts to NVM
Express ([NVME]) concepts is provided by the NVMe SCSI Translation
Reference document ([NVME-STLR]).  This mapping allows use of the pNFS SCSI
layout with NVMe devices.

The NVMe SCSI Translation Reference is to be used to define the NVMe
command and concepts that are to be used to implement the pNFS SCSI
layout.   This does not imply that implementations use an actual SCSI to
NVMe translation layer, although they are free to do so.

*2.1. Volume Identification*

With regard to the second sentence, I’m OK with “MUST” and “MUST NOT” but
the phrase “might not be suitable” is too weak to support these.  Do you
really mean something like “as the methods based on the Serial Number for
legacy devices do not provide the sufficient uniqueness to support the pNFS
SCSI layout’s addressing needs”?

In the last sentence, suggest replacing “SHOULD” by “should”.  What this is
saying is that the NGUID value provides a better implementation, and while
everyone should produce the best possible implementation, the RFC2119 term
“SHOULD” is about interoperability and doesn’t apply here.

2.2.  Client Fencing

In the second sentence, suggest replacing “for both” by “To enable this,”

In the third sentence, suggest replacing “SHOULD” by, in order of
preference, “are to”, “MUST”, “should.”   “SHOULD” is just plain rong here.

I don’t understand the last sentence.  Does the following match the intent?

The behavior provided by NVMe is somewhat similar to that obtained by
setting the ALL_TG_PT bit when registering a SCSI Reservation key.
However, NVMe specification is sufficient to provide multi-controller
reservation semantics in a reliable fashion.
2.3.  Volatile write caches

In the first sentence, suggest putting the material after the comma in
parentheses.  Using a semicolon is a possible alternative.

In the final sentence suggest:

·       Replacing “enable on a” by “enabled on an”.

·       Suggest replacing “must” by “needs to”.  While “must” is valid and
appropriate, the lower-case “must” might trigger an IESG suggestion to use
“MUST”.  Using “needs to” might avoid that.

*3.      Security Considerations*

I don’t think “since no protocol changes are proposed here” is going to be
an acceptable justification for not providing a security considerations
section.  After all, you propose that people do something or else there
would be no point in writing the document.


Here’s a possible replacement:


Since the mapping from SCSI to NVMe does not introduce any new
vulnerabilities, the security considerations applicable to use of the pNFS
SCSI layout with NVMe devices are the same as those for the use of the pNFS
SCSI layout on other devices.  These security considerations are described
in [RFC8154].