[nfsv4] Review of draft-ietf-nfsv4-fex-files-13 (part two of two)

[David Noveck <[e-mail]>]

*Review Structure*

This review has been split up into multiple emails to avoid running afoul
of mailing list size restrictions.

This is the second section.

*Previous Section*

This section contains all the general (non-section-specific) comments and
the first section of the detailed per-section comments.   Generally it
should be read first.

*Current Section*

Contains the bulk of the per-section comments, including those related to
security considerations.

*Comments by Section (from Section 2.1 onward)*

*2.1.  LAYOUTCOMMIT*

The reference to section 12.5.4 of RFC5661 may have to be updated to
point to something within layout-types, given that that Section 12 of
RFC5661 is updated by layout-types.

*2.3.  State and Locking Models*

The sentence* "*The choice of locking models is governed by the
following rules", is confusng since it is actually the server
implementation who makes this choice, when there is a choice

Suggest replacing this by the following:

A client can determine which couplig model has been implemented as follows:

*4.1.  ff_device_addr4*

In the first sentence, need to replace "storage protool specific "by
"layout-type-specific".

*5.  Flexible File Layout Type*

The placement of the last sentence in the section is confusing.  It
should either be moved to Section 5.1 or "This" should be changed to
"The next".

*5.1.  ff_layout4*

The first pargraph, as currently written, seems to suggest that
mirroring is always in effect.  It needs to be clarified.

There are a number of issues in the ninth (non-code) paragraph.
Sucggest replacing the last two sentences by the following:

In the case of loose coupling when accessing an NFSv4 storage device,
the client will use an anonymous stateid to perform I/O on the storage
device as there is no use for the metadata server stateid in the
absence of a control protocol to provide a global stateid model. In
such cases, the server MUST set the ffds_stateid to be the anonymous
stateid.

There are a number of issues in the thirteenth (non-code) paragraph:

   - It is unclear what the referent of the intial "These" is.
   - The statement that "it can not be fixed" is not correct. It could be
   fixed although there are good resons not to fix it. Those reason should be
   explained.
   - The statement that an implementation might require the protocol to be
   changed sounds backward to me and the IESG is likely to find it
   unacceptable.

Suggest the following replcement paragraph:

A number of issues stem from a mismatch between the fact that
ffds_stateid is defined as a single item while ffds_fh_vers is defined
as an array.  It is possible for each open file on the storage device
to require its own open stateid.  Because there are established
loosely coupled implementations of the version of the protocol
described in this document, such potential issues have not been
addressed here.  It is possible for future layout types to be defined
that adress these issues, should it become important to provide
multiple stateids for the same underlying file.

*5.1.2.  Client Interactions with FF_FLAGS_NO_IO_THRU_MDS*

Suggest replacing the second sentence by "The flag functions as a hint".

To clarify the third sentence suggest replacing it by the following:

The flag indicates to the client that the metadata server prefers to
separate  the MDS I/O from the data I/O, most likely for peformance
reasons.

The material beyond the current third sentece is just a complicated
way of saying that the MDS has no effective way to deal with a client
that ignores this flag.  It would be better to just delete that
material

*14.  Client Fencing*

In the first sentece,suggest deleting the phrase "at the least".

*15.  Security Considerations*

The way the material before section 15.1 is written, it isn't clear
that the first pargraph is dscussing security for pNFS as whole, while
the following two paragraphs are about security with regard to the
loose coupling model of the flex-files layout type.

In order to clarify, suggest:


   - not describing pNFS as an extension since it is inconsistent with
the watythis term is used in RFC8178.  "Feature" would be better.
   - shortening the first pargraph to put the stress on the fct that
this issue was ealt with in RFC5661.
   - Adding some introductory material to the second paragraph to make
clear what you are talkig about.

*15.1.  RPCSEC_GSS and Security Services*

There needs to be some sort of introductory material to introduce this
section as a whole.  Suggest the following:

Because of the special use of principals within the loose coupling
model, the  issues are different in the case of loose coupling
(Section 15.1.1) and tight coupling (Section 15.1.2)

*15.1.1.  Loosely Coupled*

Although I understand why RPCSEC_GSSv3 is not being used, many
readers, not being familiar with the discussions about it, may well be
confused, especually given the reference to "the intent of the loosely
coupled model that the storage device not be modified" which is
mentioned nowhere else in this document.  Suggest the following
replacement:

RPCSEC_GSS version 3 (RPCSEC_GSSv3) [RFC7861] contains facilities that
would allow it to be used to authorize the client to the storage
device on behalf of the metadata server.  Doing so  would require that
each of the metadata server, storage device, and client would need to
implement RPCSEC_GSSv3 using an RPC-application-defined structured
privilege assertion in a manner described in Section 4.9.1 of
[RFC7862].  The specifics necessary to do so are not described in this
document.  This is principally because any such specification would
require extensive implementation work on a wide range of data servers,
which would be unlikely  to result in a widely usable specification
for a considerable time.

As a result, the layout type described in this document will not
provide support for use of RPCSEC_GSS togther with the loosely coupled
model.  However, future layout types could be specified which would
allow such support, either through the use of RPCSEC_GSSv3, or in
other ways.

*17.1.  Normative References*

The reference for [pNFSLayouts] needs to be updated.

I believe RFC7861 belongs here (see below).

*17.2.  Informative References*

Given how this is used in Section 15.1.1, I believe the reference to
RFC 7861 is more appropriate as a normative reference.