[nfsv4] (belated) copy of my review of draft-ietf-nfsv4-layout-types-03

David Noveck <davenoveck@gmail.com> Thu, 16 July 2015 14:21 UTC

Return-Path: <davenoveck@gmail.com>
X-Original-To: nfsv4@ietfa.amsl.com
Delivered-To: nfsv4@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 063ED1A90B2 for <nfsv4@ietfa.amsl.com>; Thu, 16 Jul 2015 07:21:32 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: 4.901
X-Spam-Level: ****
X-Spam-Status: No, score=4.901 tagged_above=-999 required=5 tests=[BAYES_50=0.8, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, HTML_FONT_SIZE_LARGE=0.001, HTML_MESSAGE=0.001, J_CHICKENPOX_14=0.6, J_CHICKENPOX_34=0.6, J_CHICKENPOX_55=0.6, MANY_SPAN_IN_TEXT=2.399, SPF_PASS=-0.001] autolearn=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id C0CIVWiPDRdv for <nfsv4@ietfa.amsl.com>; Thu, 16 Jul 2015 07:21:21 -0700 (PDT)
Received: from mail-oi0-x22a.google.com (mail-oi0-x22a.google.com [IPv6:2607:f8b0:4003:c06::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id B80CF1A90A7 for <nfsv4@ietf.org>; Thu, 16 Jul 2015 07:21:20 -0700 (PDT)
Received: by oibn4 with SMTP id n4so51238390oib.3 for <nfsv4@ietf.org>; Thu, 16 Jul 2015 07:21:20 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:date:message-id:subject:from:to:content-type; bh=SSBC0Q9gxWUq8xjJ3vBXfRgxPzr8m0NKBXlcRLlbax0=; b=TjhbiZH40CDbLL34m/Xi32ofEpt9jZUxpwGSFW+VsObKc94efa+wfwZ4fHCHPYr4iM k5FvfCoawjyTzsWsYHEDRv0cElaP7YJYNMRpaF4C5cMgTEyTGDaBACDTvb+UmxzhLzW8 VOYb/cRXNoPx0SS6X2M+/1JhcadbjoUA1YsvBjpNQqJ7FLtzbcc1NUNJYR1wDw/ebUVq 118CzbteULJcuLf0SRCPvwlvhAUbDoT7J+Seonto5V95jkohqhpfSJsVC8GtIcXepcAC 1qK+2b+pg17MBkYkjdZBVGRuIWSTB1h9+gLvhykgosQq8Lsz5VM+Cu9YRfiRJ99ie0SB 4AcQ==
MIME-Version: 1.0
X-Received: by 10.182.210.165 with SMTP id mv5mr8838229obc.82.1437056479983; Thu, 16 Jul 2015 07:21:19 -0700 (PDT)
Received: by 10.182.115.228 with HTTP; Thu, 16 Jul 2015 07:21:19 -0700 (PDT)
Date: Thu, 16 Jul 2015 10:21:19 -0400
Message-ID: <CADaq8jc5ZkHbF=d8-6P+3hU0Y8YeXH_vRBK1RyepAK=vo4g93Q@mail.gmail.com>
From: David Noveck <davenoveck@gmail.com>
To: "nfsv4@ietf.org" <nfsv4@ietf.org>
Content-Type: multipart/alternative; boundary="001a11c2489402fc98051afec992"
Archived-At: <http://mailarchive.ietf.org/arch/msg/nfsv4/5gaiSB1JxydCAuZyswjP1d8akig>
X-Mailman-Approved-At: Fri, 17 Jul 2015 09:30:14 -0700
Subject: [nfsv4] (belated) copy of my review of draft-ietf-nfsv4-layout-types-03
X-BeenThere: nfsv4@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: NFSv4 Working Group <nfsv4.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/nfsv4>, <mailto:nfsv4-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/nfsv4/>
List-Post: <mailto:nfsv4@ietf.org>
List-Help: <mailto:nfsv4-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/nfsv4>, <mailto:nfsv4-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 16 Jul 2015 14:21:32 -0000

I sent the comments below to Tom a while back.  This was the review that
Tom was referring to at the Dallas meeting.  Although Tom characterized my
comments as a "good review", I don't think one can assume that he
necessarily agrees with everything in these comments.  Still, it would be
good if the working group had access to these comments before discussing
next steps for draft-ietf-nfsv4-layout-types.

*_______________*

*Overall Comments*

Overall this document has made a good start in addressing the problems it
set out to solve.  I'd like to thank you for undertaking what might have
seemed a thankless task.  It's now almost thankless but I hope it won't
remain so.

I think the problem it deals with is more difficult than I expected it to
be.  I hadn't recognized the complexity that derives from:

   - The fact that we have three components to provide interoperability
   among, rather than just two.
   - The fact that we are trying to accommodate such different protocols
   within the same framework.
   - The fact that locking and permissions are similar enough that they
   need to be treated together but different enough that doing so is not easy.

There are a couple of issues that I think would be best addressed
relatively soon::

   - The fact that the phrase "control protocol" is  used to mean a number
   of different things (i.e, requirements, and also the mechanism to satisfy
   them) makes things hard to understand.  Phrases like "no control protocol"
   and "real control protocol" compound the difficulty.
   - Re-organization of section 3.

As noted above, I think the material in section 3 needs to be re-organized.
  Much of it is not really related, or related only tangentially, to the
control protocol, at least as I understand the term.  One possible
classification of the material is as follows:

   - Semantic requirements that apply to the protocol as a whole.
   - Issues that relate to the interaction of the Metadata sever and the
   data storage devices.  This covers the control program requirements proper,
   or at least that part of them that is layout-type-specific.
   - Issues that relate to the interaction of clients and the data storage
   devices.  This includes the need to clearly define the data storage
   protocol and to specify who is responsible for making sure that IO requests
   conform to valid layouts.
   - Issues that relate to the interaction between the clients and the
   metadata server.

For all these, it is necessary to distinguish among:

   - REQUIREMENTS specified in this document that apply directly to pNFS
   implementations.
   - requirements for the document defining a layout type to clearly
   explain/define certain things.
   - guidance regarding the need for layout type definition documents to
   impose appropriate REQUIREMENTS on implementations to ensure
   interoperability.

*Comments by Section*

*Abstract*

Suggest replacing the first sentence by the following two sentences:

This document defines the requirements which individual pNFS layout
types need to meet in order to work within the parallel NFS (pNFS)
framework as defined in RFC5661.  In so doing, it aims to more clearly
distinguish between requirements for pNFS as a whole  and those those
specifically directed to the pNFS File Layout.


In the last sentence suggest adding "in this regard" after "RFC5661".

*Introduction*

In the second paragraph suggest the following changes:

   - In the first sentence, replace "being strictly for" by "applying only
   to".
   - In the second sentence, replace "I.e., since" by "Because" and "is
   some" by "has been some".

In the second sentence of the third paragraph, suggest replacing "clarifies
what are" by "specifies".

*Definitions*

I'd like to offer the following suggestions for modification/clarification
of the definitions. There are also proposed additions. In the case of
proposed new definitions, the term to be be defined is underlined

We currently have the issue that it is almost impossible not to use terms
in the definitions before they themselves are defined. Like you, I've
abused to a degree the principle of alphabetical order so as to limit the
difficulties. In addition, given the centrality of the concept of "Layout
Type" . I'm proposing bringing it forward in an introductory paragraphs
such as the following.

The concept of layout type has a central role in the definition and
implementation of parallel NFS. Clients and servers implementing different
layout types behave differently in many ways while conforming to the
overall parallel NFS framework defined in [RFC5661] and this document.
Different layout types may differ in:


   - The method used to do IO operations directed to data storage devices.
   - The requirements for communication between the Metadata server and the
   data storage devices.
   - the means used to ensure that IO requests are only processed when the
   client holds an appropriate layout.
   - The format and interpretation of nominally opaque data fields in
   pNFS-related NFSv4.x data structures.

Such matters are defined in a standards-track layout type specification.
Except for the files layout type, which was defined in chapter 13 of
[RFC5661], existing layout types are defined in their own RFC's and it is
anticipated that new layout type will be defined in similar documents.

It may be that we'll need a new concept (e.g. "layout-subtype" or "layout
type variant") to deal with cases in which a layout type has variant
classes of implementations that differs in some but not all of the items
listed above. For example, loose and tight coupling variants of flex-files
might fit this model and we should consider whether the same would apply
for virtualized and non-virtualized variants of the block layout type.

   - *Control communication requirements:* I think it is helpful here to
   separate requirements and mechanism. So i'm proposing:

For a particular layout type, defines the details regarding
information (e.g layouts, stateids, file metadata, and file data)
which must be  communicated between the metadata server and the
storage devices.


   - *Control protocol:*

Defines a particular mechanism that an implementation of a layout type
would use to meet the control communication requirement for that layout
type. This need not be a protocol as normally understood. In some cases the
same protocol my be used as a control protocol and data access protocol.


   - *(file) data: *suggest the following as a replacement

is that part of the file system object that contains the data to be
read or written,  as opposed to attributes of the object .  That is,
it is the file contents.


   - *Data server (DS):*  I have a suggestion (below) designed to
resolve the confusion and apparent self-contradiction regarding this
term:

is one of the pNFS servers which provide the contents of a file system
object which is a regular file, when the file system object is
accessed over a file-based protocol.  Note that this usage differs
from that in [RFC5661] which applies the term in some cases even when
other sorts of protocols are being used. Depending on the layout,
there might be one or more data servers over which the data is
striped.  Note that while the metadata server is strictly accessed
over the NFSv4.1 protocol, depending on the Layout Type, the data
server could be accessed via any file access  protocol that meets the
pNFS requirements.


See section 2.1 for a comparison of this term and"data storage device".


   - *fencing:  *Suggest replacing "when" by "process by which"


   - *layout:* suggest the following replacement:

contains information a client uses to access file data on pNFS storage
device .  This will include specification of the protocol (layout
type) and the identity of the storage devices to be used.


The bulk of the contents of the layout are defined in [RFC5662] as
nominally opaque, but individual layout types may specify their own
interpretation of layout data.


   - *layout iomode:* Suggest replacing "read or" by "read-only access or"


   - *layout stateid: *Suggest replacing "the difference between a
layout stateid and a normal stateid" by "differences in handling
between layout stateids and other stateids".


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


   - *(file) metadata: *Suggest the follow replacement:

is that part of the file system object that contains various
descriptive data relevant to the file object, as opposed to the file
data itself.  This could include the tie of last modification, access
time, eof position, etc.


   -

*Metadata server (MDS):       *
      - In the second sentence, suggest replacing "generating" by
"generating, recalling, and revoking"
      - Suggest deleting the third sentence and appending the
following material to the end of the second sentence

, for performing directory operations, and for performing I/O
operations to regular files when the clients direct these to the MDS
itself.


   - *recalling a layout:*
      - in the first sentence, suggest replacing "is" by "occurs"
      - also in the first sentence, suggest replacing "uses a back
channel" by "issues a callback".


   - *revoking a layout:  *suggest the following replacement:

occurs when the metadata server invalidates a specific layout  Once
revocation occurs, the metadata server will not accept as valid any
reference to the revoked layout and the data storage device will not
accept any client access based on the layout.


   - *stateid:  *suggest the following replacement:

is a 128-bit quantity returned by a server that uniquely defines the some
set of locking-related state provided by the server. Stateids may designate
state related to open files, to byte-range locks, to delegations, or to
layouts.


   - *storage device: *suggest the following as a replacement

Designates the target to which clients may direct IO requests when they
hold an appropriate layout. Note that each data server is a data storage
device but that some data storage device are not data servers. See section
2.1 for further discussion.


   - *storage protocol:*

is the protocol used by clients to do IO operations to the data storage
device  Each layout type may specify  its own data access protocol.  It is
possible for a layout type to specify multiple data access protocols

*Section 2.1*

First of all, suggest as a new title for this section: *Use of the
Terms "Data Server" and "Data storage Device"*

Proposed new content for this section is below:

In [RFC5661]], these two terms are used somewhat inconsistently:


   - In chapter 12, where pNFS in general is discussed, the term "data
storage device" is used.
   - In chapter 13, there the file layout type is discussed, the term
"data server" is used.
   - In other chapters, the term data "server: is used, even in
contexts where the storage access type is not NFSv4.1 or any other
file access protocol.

As this document deals with pNFS in general, it uses the more generic
term "storage device" in preference to "data server".  The term "data
server" is used only in contexts in which a file server is used as a
data storage device.  Note that every data server is a storage device
but that storage devices which use protocols which are not file access
protocol are not data servers.

Since a given data storage device may support multiple layout types, a
given device can potentially act as a data server for some set of
storage protocols while simultaneously acting as a non-data-server
data storage device for others.

*Requirements Language*

This document poses special issues regarding the RFC2119 terms in that
it states requirements/recommendations that apply both to
implementations and to future specifications.   My preference is to
apply them only to implementations but I will follow your existing
practice in my comments which follow.  Still, whichever way you choose
to go on this particular issue, this section should say *something
a*bout the question.

      *Control Protocol*

I think this group of sections need to be re-organized. The basic problem
is that the control protocol only deals with the interaction of the MDS and
storage devices while, if you look at the requirements in section 3.1 and
the non-requirements in section 3.2 you see a number of different patterns:

   - (1) of section 3.1 primarily specifies a requirement for client-MDS
   interaction. While this may, depending of how it is implemented have
   implications for MDS-storage-device interaction, it is not necessary to
   think of this as part of control protocol.
   - (3) and (4) of section 3.1 are requirements for correct inter-client
   interaction.
   - (2) and (5) of section 3.1 concern MDS-data storage interaction and
   are appropriately considered as part of the control protocol.
   - The items in section 3.2 deal with matters relating to the interaction
   of clients with storage-devices.


Suggest the following replacement for the text introducing the concept of a
control protocol:

In Section 12.2.6 of [RFC5661], the concept of a control protocol was
introduced. There have been no published specifications for control
protocols as yet.  The control protocol denotes any mechanism used to
meet the requirements that apply to the interaction between the
metadata server and the storage device.  Particular implementations my
satisfy this requirement in any manner they choose and the mechanism
chosen may not be described as a protocol.  Specifications defining
Layout Types need to clearly show how implementations can meet the
requirements discussed below, especially with respect to the those
that have security implications.  In addition, such specifications may
find it necessary to impose requirements on implementations of the
layout type to ensure appropriate interoperability.


In some cases, there may be no control protocol other than the storage
protocol.  This is often described as using a "loose coupling" model.
In such cases, the assumption is that the Metadata Server, data
storage devices, and client may be changed independently  and that the
implementation requirements in the layout type specification need to
ensure this degree of interoperability.  This model is used in the
block and object layout type specification.


In other cases, it is assumed that there may be purpose-built control
protocol which may be different for different implementations of the
MDS and data server.  In such cases, the assumption is that the
Metadata server and data servers are designed and implemented as a
unit and interoperability needs to be assured between clients and
MDS-DS pairs, developed independently. This is the model used for the
files layout.


Another possibility, not so far realized, is for the definition of a
control protocol to be specified in a standards-track document.
There are two subcases to consider:


   - A new layout type includes a definition of a particular control
protocol whose use is obligatory for MDS's and data storage devices
implementing the layout type.  In this case the interoperability model
is similar to the first case above and the defining document should
assure interoperability among MDS's, data storage devices and clients
developed independently.
   - A control protocol is defined in a standards-track document which
meets the control protocol requirements for one of the existing layout
types.  In this case, the new document's job is to assure
interoperability between MDS's and data storage devices developed
separately.  The existing  definition document for the selected layout
type retains the function of assuring interoperability between clients
and a given collection of MDS and data storage devices.  In his
context, implementations tht implement the new protocol are treated in
the same way as those that use an internal control protocol or a
functional equivalent.


*Protocol Requirements*


My general comment is that for some of the requirements (e.g. 2, 3, 4,
5) some degree of client co-operation/good-behavior may be required.
Unfortunately, it seems that explicitly allowing general requirements
for co-operation among DS/MDS/client would allow things I don't think
we want to allow.  For example, it might vitiate (1).


In the first sentence, suggest deletion of the word "broad".  Also,
since all of these use "MUST", should they be "REQUIREMENTS"?


As to individual requirements:

(1):     The big issue here is to make clear that this is not a
requirement imposed on the client and to make clear that the
           client may do this at any point of its choosing, if that is
indeed the requirement, I suggest the following replacement:


NFSv4.1 clients MUST be able to access a file by directing IO requests
to the metadata server rather than to the storage device.  When  a
client chooses to do this, the metadata  must be able to retrieve the
data from the constituent storage devices and present it back to the
client.


Whether the metadata server allows access over other protocols (e.g.,
NFSv3, Server Message Block (SMB), etc) is strictly an implementation
choice, just as it is in the case of any other (i.e.
non-pNFS-supporting) NFSv4.1 server.


(2): Propose the following changes for clarification:


   - In the second sentence, suggest replacing "fails to renew its lease in
   time" by "a client's lease is expired due to non-renewal"
   - In the third sentence, suggest replacing "state" by "locking state or
   access permissions".
   - Also in the third sentence, suggest deleting "with the client".
   - Propose adding the following paragraph:

Effective revocation may require client co-operation in using a particular
stateid (files layout) or principal (e,g. flexible files layout) when
performing IO.


(3):      I think there are some substantial issues that need to be
addressed here.


I've been unable to come up with an alternative since I think the working
group needs to discuss the issues so we get some sort of sense which way
people want to go here.


   - As written this doesn't apply to anything except the file layout.
   Block and object devices do not have ACL's. and NFSv3 DS's (for flexible
   files) typically don't either.
   - The stuff about "file open modes" really belongs in (4).
   - If the focus moves from the means of enforcing access restrictions
   (e.g ACLs) to the restrictions themselves, we run into the question of what
   sort of client co-operation is valid.  The issues relating to what might be
   loosely called "principal impersonation" were discussed in connection with
   the review of the flex-files draft but I don't think the working group ever
   came to a consensus in that regard.

(4):      This requires some clarification, if, as I assume is the case,
open modes are part of the locking state that must be respected.  The
following issues need to be addressed:


   - If the metadata server supports mandatory locking, IO operations that
   conflict with existing byte-range locks must noT be done by other clients
   or by other lockowners on the same client.
   - When a  delegation is held by one client conflicting IO operations
   initiated my other clients must not occur, until the delegation is recalled
   and returned, or evoked.
   - When an open specifying a deny mode is in effect, Io operations of the
   denied type must not be done by ther clients or by other openowners in the
   same client.

(5):      There are a number of problems with this point:


   - For most layout types, the DS has no conception of modify time, change
   attribute, or end- of-file (EOF) position.
   - The existing text first says "MUST agree" but then undercuts that so
   as to make it effectively meaningless.
   - There s no reference to LAYOUTCOMMIT and it should be stated that
    when a LAYOUTCOMMIT is done, it is required  that DS-generated changes in
   attributes be reflected at the MDS by completion of the operation.

As to the last paragraph:

   - Suggest replacing the first sentence by "These requirements may be
   satisfied in different ways by different Layout Types".
   - In the third paragraph we reach the crux of the issue we must face.
   Some things to note:
      - While the control protocol is specified as relating to coordination
      of MDS and DS, here we are suddenly speaking of interaction among client,
      MDS, and DS.
      - If you were to allow arbitrary client participation, it isn't clear
       whether the result would be acceptable.  In particular, the client might
      assure that any request made to the MDS is properly honored by simply
      making no such requests.  It is clear that that would represent a sharp
      change from RFC5661.
      - Although this says "the other Layout Types MUST document" the
      required interaction, what actually must do the documenting is the
      layout type specification, so maybe this belongs in section 3.3
      - What the layout type specification needs to do is not only document
      how this interaction might happen but specify the implementation
      requirements which the DS, MDS, and pNFS clients must abide by,  in
      order to ensure that these overall pNFS requirements are met.  In
      other words, it is a requirement (for layout type
specifications) that they
      specify REQUIREMENTS (for implementations of that layout type).

With regard to item (3) of this section, i can see this is not
particularly relevant  to the main issues you are concerned with.
Nevertheless, these items are one of the
primary functions of control protocols and,  when this section
is reorganized, you need to say something about these functions.   I think
the best model for this is to require the document defining a layout type
to specify, at least in general terms, how these
co-ordination functions are performed in the context of
the specific layout type.

*Non-Protocol Requirements*

Some general comments on this section:

   - I'm guessing that a more appropriate title for the contents as it
   stands would be *Protocol Non-requirements*.  Is that correct?
   - Stating non-requirements (i.e. things that are not in RFC5661) is
   inherently confusing.
   - Although these things do not in fact appear in RFC5661, that does not
   dispose of the matter given that we are *updating *RFC6661.  Although
   there are good reasons that many of these items cannot be enforced in many
   of the mapping types, the appropriate facts are not referred to.
   - What we wind up saying, "SHOULD if at all possible" is dangerously
   unclear and give implementations and/or layout types too much leeway to
   create non-interoperable sets of implementations.

I think it is appropriate to re-orient this section away from the
validations which might or might not be done and put the focus on what you
are trying to ensure.  I believe that it is clear that someone has to
ensure:

   - that clients do not perform IO if they do not have layouts for the
   files in question.
   - that client do not perform IO operations outside the ranges specified.
   - that clients do not perform IO operations inconsistent with the
   iomodes specified by the layout held.

'While it is true that, for some layout types, the data storage device is
not in a position to enforce these conditions, it seems to me that the
proper response is not to weaken requirements into recommendations.  I'm
pretty sure that the overall pNFS protocol requirements laid out in the
previous section cannot be met in the absence of someone making sure you
have an appropriate layout to do IO,

Instead a better approach is to allow the different layout types to take
different approaches to allocating responsibilities between the data
storage device and the client as to how the overall requirements are to be
met.  Some possibilities in this regard:

   - The storage device has sufficient knowledge about existing layouts to
   determine whether a given io is valid issued, according to what the layouts
   specify.  This is the approach in effect when validation is possible.
   - The client has the primary responsibility for only issuing IO
   appropriate to the layouts held.  The role of the MDS is ensuring that a
   client which may not be aware of needed changes in available layouts is
   prevented from inappropriately accessing the storage device.


*Editorial Requirements*

Regarding the introductory paragraph, I don't think it is the function of
this section to introduce new  (i.e. "additional") requirements. Here's a
possible replacement:


This section discusses how the protocol requirements discussed above
need to be addressed in documents specifying a new layout Type.
Depending on the interoperability model for the layout type in
question, this may involve the imposition of layout-type-specific
requirements that ensure appropriate interoperability of pNFS
components developed separately..


I don't agree with the material beginning "at a minimum', primarily because
it vitiates the essentially correct statement which starts the last
paragraph of this section. i.e. someone might think he is absolved from
providing other required information because he has met the "minimum". I
think the basic reason for doing this document is to eliminate that sort of
confusion.

I would prefer to strengthen the last paragraph and I think it can be made
appropriately strong without capitalized RFC2119 terms, underlining, or
72-point boldface Arial Black text :--) Here's the kind of thing I'm
thinking of:

The specification of the Layout Type needs to make clear how the
client, metadata server, and storage device act together to meet the
protocol requirements discussed previously. For example, if the
metadata server implements mandatory byte-range locking when accessed
directly by the client, it must do so when data is read or written
using the designated data storage protocol.  If the document does not
impose implementation requirements sufficient to ensure that these
semantic requirements are met, it is not appropriate for the working
group to allow the document to move forward.



*Implementations in Existing Layout Types*

Some issues to address:

   - The section title needs to be changed to reflect the fact that what is
   being described is primarily how the layout types are specified.  If
   implementations are described, those implementations have to
   match the specification.
   - It needs to be stated that the description of block and object layout
   are not normative since this document does not update RFC5663 and RFC5664.
   - You might also choose to state that the description of the files
   layout type is not normative, i.e. that you are only updating chapter 12 of
   RFC5661 and not chapter 13.

We also have a potential issue for all of the mapping type regarding the
potential interaction between open deny modes and use of anonymous
stateids.  The issue is similar to mandatory byte-range locking but I
can't recall
its being addressed directly.

*File Layout Type*

I suggest replacing the first sentence by the following:

Because the storage protocol is a subset of NFSv4.1, the semantics of
the File Layout Type comes closest to the semantics of NFSv4.1 in the
absence of pNFS.


In the second sentence, suggest replacing "stateid" by "stateid and
principal" since many of the validations mentioned relate to authorization.

Also in the second sentence, suggest replacing "was" by "were".

Suggest deleting "in the absence of pNFS" from the second sentence and
adding the following sentence:

The same set of validations apply whether pNFS is in effect or not.

I have some issues that relate to material that starts in the second
paragraph, proceeds through the bulleted items and goes on into the third
(non-bulleted) paragraphs.  Some issues:

   - As far as I can see, chapter 13 of RFC5661 does not say "SHOULD" about
   these validations with regard to the file layout type.
   - If the data server is not enforcing these validations, then who is?  I
   don't see how the semantic requirements for NFSv4 as a whole can be
   enforced if nobody is requiring that appropriate layouts be validly held by
   the client when IO is done to data servers? If this is not a MUST for the
   data server then I think it it has to be a MUST for clients.
   - While section 13.6 of RFC5661 does have the following MUST. it does
   not mention appropriateness of ranges or modes.  Also, since the data
   server must reject the IO, it isn't clear why the server MUST NOT issue it.

As described in Section 12.5.1
<https://tools.ietf.org/html/rfc5661#section-12.5.1>, a client MUST
NOT send an I/O to a data server for which it does not hold a valid
layout; the data server MUST reject such an I/O.


With regard to the last paragraph:

   - It is only the data filehandle that ensures that the client has a
   valid layout for the the I/O being performed. The mention of the stateid
   confuses things since this only used to ensure proper locking.
   - Suggest replacing "fenced off for" by "fenced off from"
   - If you invalidate a stateid, you are not typically fencing ff a
   client. Rather you are fencing off a particular owner or open file. Only in
   the case of a delegation stateid are you fencing off a client.

It appears that because the files layout is able to cleanly deal with
locking issues as well as security/authorization issues, they wind up
confused here. It appears that you have to make a decision to clearly
discuss locking issues as well as security issues. If you do, then. as in
this case, you need to clearly separate locking considerations from
authorization ones when necessary.

*Block Layout Type*

With regard to the phrase "at the granularity of individual hosts, not
individual blocks", this conflates two ways in which SAN access control is
coarser-grained than that for NAS devices.  It seems that you have to
decide which of the following you want to say:

   - at the granularity of LUNs rather than of individual files or
blocks. *[object
   granularity]*
   - at the granularity of hosts rather than of principals*. [subject
   granularity]*

With regard to the last sentence of the first paragraph:

   - Suggest replacing "is very careful to define" by "specifies".
   - Since "SHOULD NOT" does not appear to to meet the definition in
   RFC2219, it should appear in quote marks in this document.
   - A suitable paraphrase of the intent of the last clause might be "use
   of the pNFS Block Layout Type is not appropriate".

Suggest the following replacement for the second paragraph:

As a result of these granularity issues, the security burden has been
shifted from the storage devices to the client.  Those deploying
implementations of this layout type need to be sure that  the client
implementation can be trusted  This is not a new sort of requirement
in the context of SAN protocols.  In such environments,  the client is
expected to provide block-based protection.


With regard to the third paragraph:


   - In the first sentence, suggest replacing "implication" by "shift
of the burden"
   - As ACLs relate  to security, if they need to be mentioned
explicitly (which I don't think is necessary), it should be done in
the previous paragraph.
   - In the second sentence, suggest replacing "might not be" by "is not".
   - The third sentence is confusing.  Suggest the following replacement:

For example, the server may use a layout iomode allowing reading to
enforce a mandatory read-only lock,  In such cases, the client has to
support that use by not sending WRITEs to the storage devices.


   - In the fourth sentence:
      - Suggest replacing "basic" by "fundamental".
      - Suggest replacing "can be treated" by "is treated by this layout
      type".
      - Suggest ending this sentence after "dumb disk" and proceeding with
      a new sentence like the following:

Once the client has access to the storage device, it is able to perform
both READ and WRITE I/O to the entire storage device.


   - The fifth sentence is a good summary but it needs something additional
   to "close the deal".  Suggest:

Therefore, the client is required to provide that enforcement.

With regard to the last paragraph, suggest the following replacement:


In the context of client fencing upon revocation Of a layout, the
abovementioned limitations come into play again, i.e. the granularity
of the fencing can only be at the host/logical-unit level.  Thus, if
one of a client's layouts is revoked by the server, it will
effectively revoke *all* of the client's layouts for files located on
the storage units comprising the logical volume.  This may extend to
the client's layouts for files in other file systems.  Clients need to
be prepared for such revocations and reacquire layouts as needed.


*Object Layout Type*


Within the first paragraph:

   - I don't understand the first sentence as written.  Is the following
    reasonable replacement?

The Object Layout Type provides that security checks occur during the
allocation of a layout.


   - The second sentence as written seems to (typically) require one
   LAYOUTGET per IO.  I'm sure that isn't intended.  Would the following make
   sense?

The client will typically will typically ask for layouts covering all
of the file and may do so for either READ and READ/WRITE.  This
enables the client to do subsequent IO operations without the need to
obtain layouts for specific byte ranges.


   - With regard to the third sentence there are some difficulties that
   from inappropriately combining issues elated to authorization and those
   relating to locking:
      - It doesn't make sense to speak of verifying permissions against
      outstanding locks since locks are owned by owners and not
users/principals.
      - It seems incongruous to speak of verifying permissions against
      layout iomodes. Actually, layout iomodes are verified against permission
      (i.e. mode and acls).


   - As regards the last sentence of the paragraph:
      - Whatever you want to say about authentication her, clearly
      authentication has nothing to do with issuing ACCESS or OPEN calls.
      - I don't see why you would issue ACCESS (as opposed to OPEN) calls.
      Is this to enable IO with anonymous stateids?
      - I don't see how this similar to having a data delegation. One
      always has to do an OPEN whether you have a data delegation or not.

In the first sentence of the second paragraph. the phrase "inside the
layout" seems misplaced.  How about the following proposed replacement?

Upon successful authorization, the client receives, within the layout,
 a set of object capabilities allowing it I/O access to the specified
objects, with the requested iomode.


As regards the long final sentence of the second paragraph, the fact that
it includes the word "MUST" means we need to exercise particular care:

   - Suggest replacing "enforce access control" by "enforce access control
   and loocking semantics"
   - Suggest replacing "Whenever the metadata server detects one of:" by
   "Whenever any of the follow occur:
   - With regard to "the permissions on the object change", it should not
   be necessary to change the capability unless the change make access
   permissions more restrictive.
   - With regard to "a conflicting mandatory byte-range lock is granted",
   since layouts are not owner-specific, it isn't clear exactly what is
   meant here by "conflicting".

It appears that what is meant here is that you need to change the
capability when there is a conflicting mandatory byte range lock obtained
by another client.

It appear to me that the model for locking is that MDS is responsible
enforcing locking between clients while the client is responsible for
enforcing locking among owners within a single client.  I hope that's
right.  It's all I can think of.


   - With regard to, "a layout is revoked and reassigned to another
   client", the capability change should follow the revocation and should occur
   independently of any potential layout reassignment.
   - Should also look at adding some the following:
      - Revocation of a delegation, open or mandatory byte-range lock held
      by the client
   - At the end of the sentence that the bullets are part of, suggest
   replacing "it" by "the metadata server" and "to implicitly invalidate'" by
   "in order to invalidate"


*Summary*

in the second sentence, suggest replacing "decisions seem to be forced" by
"choices are conditioned".

I have a number of problems with the third sentence:

   - Given, that, as indicated above, these choices are essentially
   dictated by the choice of storage protocol, I don't see how implementing
   any particular control protocol could be expected to change that.
   - It's not clear what the phrase "a real control protocol" means given
   the definition of a control protocol as "a set of requirements". I'm
   supposing this means "a 'control protocol', as that phrase might normally
   be understood", but this needs to be clarified. It is possible that the
   definitions I proposed for section 2 might help here. Then again,maybe not.

With regard to the first sentence of the second paragraph, I think "as we
have seen" is not appropriate.  I don't see where this has been shown (or
demonstrated) to be the case.  In fact, the control protocol has been
defined to be a set of requirements and this sentence appear to draw
conclusion from this fact, which is not appropriate.  How about the
following as a replacement?

In the context of this document , we treat the control protocol as  a set
of requirements.

With regard to the second sentence of the second paragraph, I have the
following proposals:


   - Replace "enclosing: by "defining".
   - I have problems with the word "minimally' as it suggests something
   that only meets these minimal requirements is essentially OK.  I can see
   how you might want to stress (1) and (2) as especially important.  So how
   about the following:
   - Drop the word 'minimally".
      - Add the following paragraph at the end:

In addition, the document needs to make clear how other semantic
requirements of NFSv4.1 (e.g. locking) are met in the context of the
proposed layout type.


*Security Considerations*

I think you need a few introductory paragraphs, such as the following:

This section does not deal directly with security considerations for
existing or new layout types. instead, it provides a general framework for
understating security-related issues within the pNFS framework. Specific
security considerations will be addressed in the Security Considerations
sections of documents specifying layout types.


The layout type specification must ensure that only data accesses
consistent with the NFSV4.1 security model are allowed. It may do this
directly, by providing that appropriate checks be performed at the time the
access is performed, or by allowing the client or the data storage device
to be responsible for making the appropriate checks, with the support of
the metadata server. In the latter case,, IO access writes are reflected in
layouts and the layout type must provide a way to prevent inappropriate
access due to permissions changes between the time a layout is granted and
the time the access is performed.

I think the following paragraph could appear after yours:

After such termination of access, the client has the opportunity to
re-acquire the layout and perform the secuirty check in the context of the
newly current access permissions.

Note that I am saying here that revocation of layouts is not required to
enforce access permission chnges in the case of the file layout.  It is
required to enforce locking., but not access permissions.  is this a
problem?