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

David Noveck <davenoveck@gmail.com> Wed, 26 July 2017 00:19 UTC

Return-Path: <davenoveck@gmail.com>
X-Original-To: nfsv4@ietfa.amsl.com
Delivered-To: nfsv4@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 32A04132150 for <nfsv4@ietfa.amsl.com>; Tue, 25 Jul 2017 17:19:33 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.699
X-Spam-Level:
X-Spam-Status: No, score=-2.699 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
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 a-7V6x_B6PG0 for <nfsv4@ietfa.amsl.com>; Tue, 25 Jul 2017 17:19:29 -0700 (PDT)
Received: from mail-it0-x236.google.com (mail-it0-x236.google.com [IPv6:2607:f8b0:4001:c0b::236]) (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 60FF0132152 for <nfsv4@ietf.org>; Tue, 25 Jul 2017 17:19:28 -0700 (PDT)
Received: by mail-it0-x236.google.com with SMTP id h199so59482680ith.0 for <nfsv4@ietf.org>; Tue, 25 Jul 2017 17:19:28 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:from:date:message-id:subject:to:cc; bh=gkwhzch5YLBm3rff2GOfN8Il1dmw2JCVqOK+voIPdsc=; b=EZqD4Bccce1gKe7xtvkywMaGV9SSrn++UebuF3FII+ucxNRrIzja+5nDAxSk95aW4u fdeW1BnyNWJoVqKFw4MD1djxj3LjcIg7HWiT+JfCPpEJ6F9UwvSL6MEoexeiJUGqgBWt yBM7ftNfKIn4YNSk4ykoqT8ri+lb8ibnHQggMhnwjdP5zVKRrUSyZHHlmeEBD2H9FvNe 3qB7oFDAuIe5zuHolN8rynQSdYViGtFCJFj8z6Us5D5+2Zu2LIXtbHfWFV1zb42yUNee HiapKmTKNGmY/gXTO8W8fTCi4D9OYtFpebBktd3E4xzo6N5+T9xPi5ZtgRXDl0+DKa/x GFQw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to:cc; bh=gkwhzch5YLBm3rff2GOfN8Il1dmw2JCVqOK+voIPdsc=; b=crTABFxiXXDgRuFTmmgL4FmrJONQwWFIQidtqtahKNzLH3CyB9OLVadMg/Ps7+lXDN QUVA9t6PSowO1420LkRNqgrCPV+H1BrJTc9C4a5Avzgxp4jQg35DB5693jz64iTkcIGk e8BK6kfQeFKAxdJ+S3rzqMGab8+B0N8GWBtQH42Do6t5tRUGq4Jf1iBnhgHFC67M4l9l XrqFU99yX+uZSIkPQ4+VYVIHzMj37GSFbs69n+m/fz0FEVFuWWg4n9agu3FP2xqIOo6x oV9DbRaXT8USaoucu9giOPADpRIgEsQJbtrw1hAFPMqYpZCXoSRGqims1tzENx5aFO0H adiA==
X-Gm-Message-State: AIVw112TLTCN1MoMYDNTpf9PTa7ZwbXXpNqAKc6yFG+AoJ+sR0YY5XyI GMonMFzekdJgPMtRYqFIREDq3twK6g==
X-Received: by 10.36.173.2 with SMTP id c2mr2612353itf.16.1501028367360; Tue, 25 Jul 2017 17:19:27 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.107.142.72 with HTTP; Tue, 25 Jul 2017 17:19:26 -0700 (PDT)
From: David Noveck <davenoveck@gmail.com>
Date: Tue, 25 Jul 2017 20:19:26 -0400
Message-ID: <CADaq8jfFg-C8=DFHyEyHxB1jRC03-7nzq0M1V9-BB4wBsg=otA@mail.gmail.com>
To: Tom Haynes <loghyr@primarydata.com>
Cc: "nfsv4@ietf.org" <nfsv4@ietf.org>
Content-Type: multipart/alternative; boundary="94eb2c1fd1e8a2670a05552d6600"
Archived-At: <https://mailarchive.ietf.org/arch/msg/nfsv4/ESR_PhysQKDu8nTB7FKqzdYEWtI>
Subject: [nfsv4] Review of draft-ietf-nfsv4-layout-types-05
X-BeenThere: nfsv4@ietf.org
X-Mailman-Version: 2.1.22
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: Wed, 26 Jul 2017 00:19:33 -0000

*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".

*1. Introduction*

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

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

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

   - replacing "and the" by "while the"
   - deleting the phrase "in turn".

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

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

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

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


   - replacing "or any new variant" by "or any additional one".

*2.  Definitions*

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

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

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

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

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".
   - 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.

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".

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.

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

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.

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

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

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

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.

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.



*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".


*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.


*3.  The Control Protocol*

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

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".

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.

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

*3.1.  Protocol REQUIREMENTS*

Suggest changing the section title to "Control Protocol REQUIREMENTS"

In the first sentence, suggest deleting "such",

If the first sentence of (1) suggest:

   - replacing "decides" by"chooses".
   - replacing "instead of" by "rather than".

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

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.


(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.

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 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.

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

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


*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".

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


*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".

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

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

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

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.

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

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

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


*4.2.  Block Layout Type*

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

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.

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


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".

*8.2.  Informative References*

Need to add a reference to RFC 8154.