Re: [nfsv4] Review of draft-ietf-nfsv4-layout-types-05

[Thomas Haynes <[e-mail]>]

I have addressed all of these issues.

I also did a subsequent spell check run and caught 8 other issues. :-)


On Jul 25, 2017, at 5:19 PM, David Noveck <[e-mail]<mailto:[e-mail]>> wrote:

General Comments

Overall Evaluation

This document is in pretty good good shape.  I expect that it will be able to go forward fairly soon after WGLC is completed.

There are no technical issues to address.  All of the issues noted below are editorial in nature.  Most of them
are minor wording/spelling/grammar issues which can be trivially dealt with and should not affect other sections
of the document.

The exceptions are noted in the following sections:

  *   Multi-section Issues discusses issues, that while still editorial in nature involve multiple sections of the document
  *   Other Siginificant Issues lists a number of areas where special attention needs to be paid because the needed changes, while editorial in nature, are more significant than a minor textual correction.

Multi-section Issues:

There's a bunch of issues to reflect the fact that RFC8154 is now a published layout type.  I think these can be
dealt with without explicit discussion of the SCSI layout.  See the per-section comments for details.

Other Significant Issues

Special attention needs to be paid to :

  *   The proposed additional paragraph in 2.2.  Requirements Language.
  *   The changes proposed for (3) and (5) in 3.1.  Protocol REQUIREMENTS.
  *   The changes suggested in 3.2.  Undocumented Protocol REQUIREMENTS

Administrative Issues

This document is not officially in WGLC according to DataTracker. Given the tangled
history of this document, it may be necessary for someone to document the fact that
this document had a WGLC before going forward to the IESG.

Comments by Section

Abstract

In the second sentence, suggest deleting the word "more”.

Ack


1. Introduction

In the last sentence of the first paragraph suggest replacing "in" by "as to”.

Ack



In the last sentence of the second unbulleted paragraph, suggest replacing "type" by "types”.

Took a while to figure out which ‘type’, but Ack


In the last sentence of the third unbulleted paragraph, suggest:

  *   replacing "and the" by "while the"
  *   deleting the phrase "in turn”.

Ack


Suggest adding the following new sentence to the end of the third (unbulleted) paragraph:

Subsequently, the SCSI layout type was defined in [RFC8154].

Ack

In the second sentence of the penultimate paragraph, suggest replacing "like" by "such as”.

Ack



In the final sentence of the last paragraph, suggest:

  *   replacing "layout type independent" by  "layout-type-independent". An alternative is to rewrite the object of the sentence as follows:

requirements placed on all layout types independent of the particular type chosen

Picked rewrite

  *   replacing "or any new variant" by "or any additional one”.

Ack

2.  Definitions

In the second sentence of the "control protocol" section, suggest replacing "my" by "may”.

Ack


In the second sentence of the "(file) data" section, suggest replacing "and not" by "rather than”.

Ack


In the first sentence of the "data server (DS)" section, suggest replacing "pNFS server" by "server used to support pNFS access".


This goes hand in hand with the approach used
in the "metadata server (MDS)” section, they are both pNFS servers.

In the second sentence of the "data server (DS)" section, suggest replacing "one or more" by "several”.

I like the original better because it connotates that a stripe can be of width 1



In the third sentence of the "data server (DS)" section, suggest:

  *   replacing "strictly accessed over the NFSv4.1 protocol" by "is accessed using the NFSv4.1 protocol”.

“strictly” in contrast to “any” (implied in the last half of the sentence.



  *   replacing the material after the initial comma by the following:

the data server could be accessed via any file access protocol allowed by the layout type, assuming that the pNFS requirements. are met.

Same number of commas, so I’ll go with this:

      While
      the metadata server is strictly accessed over the NFSv4.1
      protocol, the data server could be accessed via any file access
      protocol that meets the pNFS requirements.

I think we’ve beat in the point by now that it is the layout which determines the file access protocol.


In the second paragraph of the "layout" section", suggest replacing "may specify their own interpretation of layout data" by "are responsible for defining the format of the layout dara”.

but individual layout
        types are responsible for specifying the format of the layout data.


With regard to the "layout iomode" section, there is a reference to Section 1 but I don't understand what,specifically, is being referred to.  Please clarify.


I think I was trying to replace the ‘layout type’ definition.

Yes, you had in your review:


  *   Layout Type: propose removing this in favor of the introductory paragraph suggested above

So, I meant to point the reader to the definition in Section 1.

Restored to its former glory and keeping the ‘layout type’ def..



In the "layout type" section, suggest replacing "describes" by "specifies”.

Ack


In the "loose coupling" section, suggest replacing the text by the following:

describes a situation in which the control protocol, used between a metadata server and a storage device, is also a storage protocol.

Why are we redefining control protocol here?

I’m going with:

        describes when the control protocol is a storage protocol.




In the "recalling a layout" section, suggest replacing "could be able to" by "would have the opportunity to".


Ack


In the "recalling a layout" section, a period is needed after "specific layout".


Ack

In the "storage protocol" section, need to convert the initial comma to a period.

Ack


In the second sentence of the "storage protocol" section, suggest replacing "may specify its own storage protocol" by "specifies the set of storage protocols which may be validly used".

Suggest the following possible replacement for the third sentence of the "storage protocol" section:

It is possible for a layout type to allow the use of multiple storage access protocols.



I understand where you are going with the “valid”, but it sounds awkward. I’m going to replace the
def with:

        is the protocol used by clients to do I/O operations to the
        storage device.  Each layout type specifies the set of storage
        protocols.




In the "tight coupling" section, suggest replacing the text by the following:

describes a situation in which the control protocol, used between metadata server and storage device, is one designed specifically for that purpose.  It may be either a proprietary protocol, adapted specifically to a a particular metadata server or one based on a standards-track document.

Again, we both redefine control protocol.

Went with:

        describes a situation in which the control protocol is one
        designed specifically for that purpose.  It may be either a
        proprietary protocol, adapted specifically to a a particular
        metadata server, or one based on a standards-track document.





2.1.  Use of the Terms "Data Server" and "Storage Device"

In the first sentence of the first paragraph, suggest replacing "these the" by "these”.

Ack


2.2.  Requirements Language

Suggest adding the following additional paragraph:

It should be noted that this document differs from most standards-track documents in that it that it specifies requirement for those defining future layout types, who may then go on to defne the requirements for those implementing those layout types, rather than defining the requirements for implementations directly.  The document makes clear, and the reader should be aware of whether any particular requirement applies to implementations, to those defining layout types, or is a general requirement which implementations need to conform to, with the specific  means left to layout type definitions type to specify.

Remind me where you got your law degree? :-)

I went with this instead:

   This document differs from most standards-track documents in that it
   specifies requirements for those defining future layout types rather
   than defining the requirements for implementations directly.  This
   document makes clear whether:

   (1)  any particular requirement applies to implementations.

   (2)  any particular requirement applies to those defining layout
        types.

   (3)  the requirement is a general requirement which implementations
        need to conform to, with the specific means left to layout type
        definitions type to specify.




3.  The Control Protocol

In the first sentence of the first paragraph, suggest replacing "the control" by "the concept of a control”.

Ack


In the second sentence of the first paragraph, suggest replacing "no published specifications for control protocols as yet" by "no specifications for control protocols published so far”.

Ack


In the third and fourth paragraphs, the material at the start ("In some cases, there may be no control protocol other than the storage") needs to be removed.

Wow, was prepared to fight and then I see it must be a cut and paste error. :-)


In the first sentence of the third paragraph, suggest replacing "may be" "will be a”.

ack


3.1.  Protocol REQUIREMENTS

Suggest changing the section title to "Control Protocol REQUIREMENTS”

Ack


In the first sentence, suggest deleting "such",


ack

If the first sentence of (1) suggest:

  *   replacing "decides" by"chooses".
  *   replacing "instead of" by "rather than”.

I’m fine with what is there


In the first sentence of (3), suggest replacing "remove" by "allow the violation of”.

Ack


The second sentence of (3) as written is not very clear about the potential "requirement" and its possible source.

One possible revision is:

While Section 12.9 of [RFC5661] specifically lays the burden of enforcing these controls on the combination of clients, storage devices, and the metadata server, certain implementations might have a need for the metadata server to update the storage device so that it can enforce security.

Another is:

While Section 12.9 of [RFC5661] specifically lays the burden of enforcing these controls on the combination of clients, storage devices, and the metadata server, individual layout type might creare requirements as to how this is to be done, including a possible requirement for the  metadata server to update the storage device so that it can enforce security.

I think the “burden” is on the layout type and not the implementation.

I went with:

   (3)  A pNFS impelementation MUST NOT allow the violation of NFSv4.1's
        access controls: ACLs and file open modes.  Section 12.9 of
        [RFC5661] specifically lays this burden on the combination of
        clients, storage devices, and the metadata server.  However the
        specification of the individual layout type might create
        requirements as to how this is to be done.  This may include a
        possible requirement for the metadata server to update the
        storage device so that it can enforce security.

        The file layout requires the storage device to enforce access
        whereas the flex file layout requires both the storage device
        and the client to enforce security.




(4) as written, while correct, needs to be more specific.  Suggest the following replacement text:

Interactions between locking and IO operations must obey existing semantic restrictions.  In particular, if an IO operation would be invalid when directed at the metadata server, it is not to be allowed when performed on the storage device.


Took almost as is, “must” -> “MUST” to conform to all of the other points.



For (5), a very broad general proposition is stated and then so many holes are poked in it that it is not quite clear what is left.  Some suggestions to consider in revising:

  *   Many storage devices do not have a concept of "modification time" or "the change attribute" making the concept of agreement nebulous.
  *   Given that, it is better to start with requiring that the metadata server and the storage devices "not disagree" which is easier thn requiring that they agree and then patching around the fact there is no real agreement.

So here is a possible replacement along these lines:

Any disagreement between the metadata server and the storage devices as to the value of attributes such as modify time, the change attribute, and the end- of-file (EOF) position MUST be of limited duration with clear means of resolution of any discrepancies being provided.  Note that

  1.   Discrepancies need not be resolved unless the client has accessed the file in question via


Changed ‘the client’ -> ‘any client’.


  1.  the metadata server, typically by performing a GETATTR.
  2.  A particular storage device might be striped such it has no information regarding the EOF position.
  3.  Both clock skew and network delay can lead to the metadata server and the storage device having different values of the time attributes.  As long as those differences can be accounted for what is presented to the client in a GETATTR, then no violation results.
  4.  A LAYOUTCOMMIT requires that changes in attributes resulting from operations on the storage device need to be reflected in the metadata server by the completion of the operation.

Ack

In the second sentence of the penultimate paragraph, suggest replacing "does use" by "uses”.

Ack


In the final  paragraph, suggest replacing "interact" by "are to interact”.

Ack


3.2.  Undocumented Protocol REQUIREMENTS

Suggest changing the section title to "Previously Undocumented Protocol REQUIREMENTS".

I think attention should paid to the differences between/among these requirements and to the fact that these all seem to be directed to the clients (and in one case the MDS) while the following paragraph refers to the ability of storage devices to meet them. ???  Note that:

  *   Items (1), (3), and (4) , are really not about what the client will or won't do.  Whatever rfc5661 says explicitly, it is clear that clients are supposed to respect layouts.  The real issue is whether the data storge evices are able to enforce these.
  *   Item (2) is a requirement on the MDS which doesn't reallly fit with the others.
  *   Item 5 is unclear.  Part of the difficulty is that responsibility for storage allocation is different for different layout types.  Another issue is the "i..e.".  Creating files is only one nsance of storge allocation but simply changing this to an "e.g." will not work.

I suggest starting the section as follows:

If one summarizes the requirements stated in Section 12 of [RFC5661], there are a number of matters in which it is clear that certain behaviors are necessary for the protocol to work while not being stated as explicit requirements.

A number of these concern the obligation of clients to respect layouts when making IO requests on the data storage devices:

This would be followed by what are currently  (1), (3), and (4). and then the following:

 Under the file layout type, the storage devices are able to reject any request made not conforming to these requirements.  However,for other known layout types, this not possible so that the burden of conforming is solely on the client with the data storage devices unable to directly detect violations.  For these layout types, special fencing operations may be necessary to enforce layout revocation.

Also included in this category are:

  *   The requirement for the MDS to perform IO operations directed to it.
  *   The need to provide appropriate storage allocation,whether to create or delete files or to extend or truncate existing ones.

The means to address these requirements will vary with the layout type.  Generally, the control protocol will be used to effect these, whether a purpose-built one, one identical to the storage protocol or a new standards-track control protocol.

The above may not be the final answer to the issues in this section but suggest starting there.

4.  Specifications of Existing Layout Types

I think the section title needs to b changed to "4.  Specifications of Original Layout Types”.

Ack


In the last sentence, suggest replacing "not" by "rather than”.

Ack


4.1.  File Layout Type

In the following, I'm going to include the indented paragraphs, including the numbered ones, when citing particular paragraphs.

In the second sentence of the first paragraph, suggest replacing "would" by "would have".


Ack

In the second sentence of the first paragraph, suggest replacing "apply" by "are applied".



Ack

In the sixth paragraph, suggest replacing "presented" by "presented in [RFC5661]”

Only if I strip the  [RFC5661] from both the following points



In the first sentence of the eleventh paragraph, suggest replacing, "However," by "It should be noted that”.

Ack


Suggest replacing the second sentence of the eleventh paragraph by the following:

The storage devices MUST make,  on each READ or WRITE I/O,  all of the required access checks as determined by the NFSv4.1 protocol.

I’m fine with the current wording...


In the final sentence of the eleventh paragraph, suggest replacing "And note that" by "It should also be noted that,”

Went with "And note that” -> “Also”


In the penultimate paragraph, suggest replacing "is sufficient' by "provides sufficient context to enable the data server to ensure”

Ack


In the first sentence of the final paragraph, suggest replacing  "such"  by "in order”.

Ack


4.2.  Block Layout Type

In the first sentence of the first paragraph, suggest replacing "not guaranteed to be able" by  "generally not able”

Ack



Suggest replacing he second sentence of the first paragraph by:

Typically, storage area network (SAN) disk arrays and SAN protocols provide coarse-grained access control mechanisms (e.g., Logical Unit Number (LUN) mapping and/or masking), with a target granularity of disks rather than individual blocks and a source granularity of individual hosts rather than of users or owners.

Ack


In the third sentence if the furst paragraph, suggest replacing ", and hence" by ".  As a result", creating a new final sentence.

Ack


In the third sntence of the third paragraph, suggest replacng "as a local dumb disk' by "in the same fashion as a local disc device".

5.  Summary

In the first sentence, need to replace "published" by "original”.

Ack


8.2.  Informative References

Need to add a reference to RFC 8154.

Ack