[nfsv4] Review of draft-ietf-nfsv4-flex-files-08 (part three of three)
David Noveck <davenoveck@gmail.com> Sat, 21 May 2016 14:01 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 AA0DF12B030 for <nfsv4@ietfa.amsl.com>; Sat, 21 May 2016 07:01:46 -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 OWJpo-I-Cebp for <nfsv4@ietfa.amsl.com>; Sat, 21 May 2016 07:01:44 -0700 (PDT)
Received: from mail-oi0-x235.google.com (mail-oi0-x235.google.com [IPv6:2607:f8b0:4003:c06::235]) (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 2F6E612D559 for <nfsv4@ietf.org>; Sat, 21 May 2016 07:01:40 -0700 (PDT)
Received: by mail-oi0-x235.google.com with SMTP id x19so218995156oix.2 for <nfsv4@ietf.org>; Sat, 21 May 2016 07:01:40 -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:cc; bh=RCRu5LospcQctnqVxj92M8esdgsWroc2Jc7jXypoLEQ=; b=pjSU3Am+vis1xTCwKdjuuHfkyQZ6j7TPoDXlnNlcrZ6QlRWFa0HDPXSTfTo8Bhe2dp 5ck6NPdDQgFQVaBS4w6/AHWC0TJe1FzrZwgaRgaRzjakpM/I7WXfFZnHNu3K+sVM1RqW DcFdjjEgYYE7oxg4uB04YVEKG2XXtjguo5GDYdkNbdkRE6qvjc9ymi5BjHtzrHwbhg/T DNUnRfGLzmnJ+HLJ4NiF2iWiMw+xeSDe6i87D//u04n4uJeSMuyUcUB792R/kg3IyuSB G7DnNTfRPuoWScWnlL823dpZfMatYFGuyb9A3KsDrbd9TUHY/uDhIHQy8P7Q1pGLqSDx RLDQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:date:message-id:subject:from:to:cc; bh=RCRu5LospcQctnqVxj92M8esdgsWroc2Jc7jXypoLEQ=; b=WYp9WGSL8+xglZNzU7kCnbt2cQQeeqW4Pa4P+EIIwMzbyc748fDlvXbYle+XD8xNTk c2vF/dF7TiZ9oiM0B4yAgJM4PmdzeAY28J2hCLzJeUPnJoE4GZcCxtfox3Fe9z8WbxMT J1swtiLRGKahyrAHuQGkAszanKPrmvKN6VuaA9QTLz7FpFfnliKxd7x5DVS5eYiwBobN 0msqET33Y7JypPX/RqI+8QrYE6HPTQuPYvlwKzrnz0RGDrTbW4FTQt9IKx4VboAOuGlt 0SiimyzXlP+z84vFFVUhtFNKZxZsFd0WnxC0QL1jfZoFPJQx0LowMZh5H7V7Ahzv2nil zuLw==
X-Gm-Message-State: ALyK8tIL/k0q3aW3Lrxtw0cVaoblpOub5aZY+A5kteoWV4e+p8RD8thebUxs0VRK5LNlqMhYTa6wjv18LSYFpw==
MIME-Version: 1.0
X-Received: by 10.202.4.139 with SMTP id 133mr494630oie.67.1463839298843; Sat, 21 May 2016 07:01:38 -0700 (PDT)
Received: by 10.182.29.166 with HTTP; Sat, 21 May 2016 07:01:38 -0700 (PDT)
Date: Sat, 21 May 2016 10:01:38 -0400
Message-ID: <CADaq8jcSkvOauXXkciaC_K0wKJUCT36qsmFi7VGb6aunuRKdxg@mail.gmail.com>
From: David Noveck <davenoveck@gmail.com>
To: Thomas Haynes <thomas.haynes@primarydata.com>, Benny Halevy <bhalevy@gmail.com>
Content-Type: multipart/alternative; boundary="001a11c037fa6a7a5305335aa5ea"
Archived-At: <http://mailarchive.ietf.org/arch/msg/nfsv4/M1SH6h_SgTvJSCFNbtZbRgwoW1Q>
Cc: "nfsv4@ietf.org" <nfsv4@ietf.org>
Subject: [nfsv4] Review of draft-ietf-nfsv4-flex-files-08 (part three of three)
X-BeenThere: nfsv4@ietf.org
X-Mailman-Version: 2.1.17
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: Sat, 21 May 2016 14:01:47 -0000
*Review Structure* This email is the third part of a three-part review. Note that the overall comments are contained in the first part of this review. These contain: - *Background of Review* - *General Evaluation * - *Issues Blocking Working Group Last Call* - *Other Noteworthy Issues* *Per-section Comments (From Section 5.1.2 Onwards)* *5.1.2. Client Interactions with FF_FLAGS_NO_IO_THRU_MDS* There's some issues with the second sentence that should be noted: - As there appears to be no logical connection between the first two sentences, i suggest deleting "Thus, ". - The clause "it can recall the layout and either not set the flag on the new layout or not provide a layout" seems to be written implying that by doing either of these things, the metadata server somehow makes it OK for the client to o IO through the metadata server. In the pNFS model it is always OK to do IO through the metadata server. One the should-vs.-SHOULD question about this flag is resolved, this should be rewritten. I'm unclear as to the implications of the last sentence: - In the NFS4_OK case, what the client can do is dependent on resolution of the should-vs.-SHOULD issue. - In the NFS4ERR_LAYOUTUNAVALIABLE case, it is clear metadara-server irected IO will be done. - The NFS4ERR_{LAYOUTTRYLATER, DELAY} case is not clear since *Section 5.1.1. Error Codes from LAYOUTGET* only speaks, for these errors, about cases in which the client has a layout, an, in this case, it has already been returned. *7. Recovering from Client I/O Errors:* The last paragraph is quite problematic with regard to use of the RFC2119 terms, for the following reasons: · In a number of places, we are told that he client MAY do X and SHOULD do Y, where X and Y are mutually exclusive. While this might make sense with "may" and "should", it doesn't make sense with "MAY" and "SHOULD". If you say you SHOULD do Y, then you can only do something else (e.g. X), under some very restrictive conditions. On the other hand, if you say you MAY do X, you can just choose to do X, with no such restrictions. · In some cases the thing that is introduced by "MAY" or "SHOULD" is not something that an implementation is actually doing. For example, in "MAY be acceptable to propagate", propagating an error is something that the client can do, but since it relates to the interface between the application and the client, it is not in-scope for this document. In this case,, something being acceptable or not is an attitude issue which is not an appropriate target of an RFC2119 term. Because of these issues, I suggest rewriting this paragraph as follows: Although a client always has the option to propagate a corresponding error to the application that initiated the I/O operation and drop any unwritten data, most clients will require that retry be attempted. In this case, the client will either retry the original I/O operation by requesting a new layout using LAYOUTGET and retrying the I/O operation(s) using the new layout, or by retrying the I/O operation(s) using regular NFS READ or WRITE operations via the metadata server. When doing such retries, the client SHOULD attempt retrieving a new layout and retry the I/O operation using the storage device first and only if the error persists or a layout is unavailable, retry the I/O operation via the metadata server. *8. Mirroring* Suggest making the following changes in the last sentence of the first paragraph: · replace "construct" by "consequence" · replace "that" by "in which" Suggest making the following changes to the last paragraph: · In the second sentence, replace "get to" by "are made on" · In the fourth sentence, delete "e.g. using an earlier version of the protocol," *8.2. Writing to **Mirrors* I think the third sentence of the second paragraph is not using the RFC2119 terms correctly. These terms are about what implementations may or may not *do. *When you say "MUST NOT make assumptions", you are basically referring to what is in the implementer's head. Since the following sentence clearly states the real REQUIREMENT as to what the client must do, I would rewrite the sentence in question to being "It is invalid for the client to assume". In the last sentence sentence, suggest replacing "committed" by "committed to all mirrors" *8.3. Metadata Server Resilvering of the File* There are some issues that relate to the last two sentences: · It doesn't seem right to me for the client to respond to NFS4ERR_LAYOUTTRYLATER by giving up and trying something else. While it allowed to do that, it should not be encouraged or presented as the only/preferred way to do something. This also appear to conflict with the last sentence of *Section 7. **Recovering from Client Errors.* · NFS4ERR_LAYOUTUNAVAIL should be mentioned as a possible valid return here. In fact, this is the one that makes it valid desirable to do the retry to the metadata server and I have proposed that the last sentence of *Section 7. **Recovering from Client Errors *be rewritten to allow that. *9. Flexible Files Layout Type Return* This is unduly confusing. The document need to be more explicit that the layoutreturn_type4 has to be LAYOUTRETURN4_FILE and that there is no LAYOUTRETURN4_FLEXFILE. *9.1.1. ff_ioerr4* This is one place where an NFSv4.2 structure is imported. As a result it isn't very clear what approach you take to using a NFSv4.1 MDS. It may be clear to the authors but the reader is left to wonder. There should probably be something in *Introduction* to address this. *9.2.3. ff_iostat4* This is another place where an NFSv4.2 structure. is imported. The issues are the same as noted in* 9.1.1. ff_ioerr4* above. In the first post-CODE paragraph suggest making the following changes: · In the first sentence, delete the "the" in "the data transfers". · In the second sentence, replace "no visibility to the I/O stream" by "no direct knowledge of the I/O operations being done" · Also in the second sentence, replace "cannot use any" by "and thus cannot create on its own" · In the third sentence, change "infeasible" to "not feasible". In the first sentence of the second post-CODE paragraph, suggest replacing "identify" by "identifies". *13. Recalling layouts:* It is not clear exactly what is meant by "when there are sharing conflicts." I suggest you be a little more clear about what kind(s) of conflicts you mean. Some possibilities: · When existing layouts are inconsistent with the need to enforce locking constraints. · When existing layouts are inconsistent with the requirements regarding resilvering as described in Section 8.3. *13.1. CB_RECALL_ANY:* With regard to AI13, I don't see any reasonable prospect of these constants being added to RFC5661. Also, the first paragraph of this section says that these are in [RFC5661]. One could add this stuff in a minor version but v4.2 iis no longer open to change and v4.3 looks a long way off. One possibility is to do this stuff as an extension as described in draft-ietf-nfsv4-versioning-01. If the stuff in this section is not critical to going forward, perhaps it could dropped and this stuff would be defined in a separate extension document. That document would have to reference draft-ietf-nfsv4-versioning (or a possible successor) normatively so there might be a considerable delay. *15. Security Considerations* This is a big improvement from the -04. In the second paragraph, it isn't clear how the authorization used at LAYOUTGET time, relates to the one at done at OPEN/ACCESS time. I realize that in a security considerations section one might want to indicate to the IESG that you have belt and suspenders working. However, implementers may need to understand what is actually necessary to be safe. There are a couple of issues that arise from recalling of layouts being a SHOULD. Some possible changes: · In the sixth sentence suggest rewriting all the material after the word "and it" as follows: then MUST fence off any clients still holding outstanding layouts forr the respective files by implicitly invalidating the previously distributed credential on all data file comprising the file in question. It is REQUIRED that this be done before committing to the new ACL and/or permissions. · suggest rewriting the penultimate sentence as follows: Doing this will require the clients reauthorize access by requesting new layouts, to be obtained according to the modified access control metadata, before doing any further IO operations *15.1.1 Loosely coupled* I'm going to propose a number of stylistic changes with the goal of eliminating sentences beginning with "I.e". In some cases, I'm taking the liberty of guessing about your motivations in cases in which these are simply left unexpressed. My main point is that the motivations need to be clearer and am not suggesting that my interpretation is the correct one. · In the first paragraph, suggest replacing "I.e." by "Given that," · In the second paragraph, suggest replacing "I.e." by "In this case,there would be a need for the metadata server, storage device, and client to each implement RPCSEC_GSSV3. As that process would unreasonably extend the deployment process, it must, for the moment, be considered out of scope. I also suggest it would be clearer to rewrite the final paragraph as follows: To summarize the current situation, methods to automatically authenticate principals or use RPCSEC_GSSv3-aware clients, metadata server, and storage devices could be designed and deployed. However, the specifics of such approaches are not addressed in this document. Currently, if more secure authentication is desired using the flex-files layout type, tight coupling should be used, as described in the next section. *15.1.2 Tightly coupled* In the second sentence, suggest replacing "Thus" by "As a result,". *17.1. Normative References:* Are you sure you want to make the reference to pNFSLayouts normative? Although the treatment of these issues is not ideal in RFC5661 (and I think the pNFSLayouts work should go forward as soon as possible), a number of layout types have gone forward so far under what is stated in RFC5661. Given that we now have a better understanding of the issues from drafting the layouts draft, it could be argued that publication of the corresponding RFC as a Proposed Standard is not a requirement for flex-files to go forward, and that the reference could be Informative. *Acknowledgements:* In the fourth paragraph suggest: · Changing "a comprehensive review" to "comprehensive reviews". · Changing "call" to "calls" In the penultimate paragraph, propose changing "lead" to "led". Alternatively, you might want to "replace led the charge" by "made a convincing case".
- [nfsv4] Fwd: Review of draft-ietf-nfsv4-flex-file… David Noveck
- [nfsv4] Review of draft-ietf-nfsv4-flex-files-08 … David Noveck
- Re: [nfsv4] Review of draft-ietf-nfsv4-flex-files… Thomas Haynes