Re: [nfsv4] Review comments for WGLC of draft-ietf-nfsv4-layout-types
Thomas Haynes <loghyr@primarydata.com> Tue, 29 August 2017 20:59 UTC
Return-Path: <loghyr@primarydata.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 15F89124E15 for <nfsv4@ietfa.amsl.com>; Tue, 29 Aug 2017 13:59:25 -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, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=primarydata.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 EmbDxBOTNGcB for <nfsv4@ietfa.amsl.com>; Tue, 29 Aug 2017 13:59:20 -0700 (PDT)
Received: from us-smtp-delivery-194.mimecast.com (us-smtp-delivery-194.mimecast.com [63.128.21.194]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 776B6132D90 for <nfsv4@ietf.org>; Tue, 29 Aug 2017 13:59:20 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=primarydata.com; s=mimecast20170802; t=1504040359; h=from:subject:date:message-id:to:cc:mime-version:content-type:in-reply-to:references; bh=1qe44G4+Q+YJS7ljzb5pOoY28Kjpq8UgSz3/Wrscwlk=; b=HO5mjYWD0J0JT8RLzZetvItUnwgqh8szVGujwx61b0VEYK6wT2QKhzxvf1l4xze0PyRWmMdxK5MMPzxWcZYy2hh6ZR6ksnvFVhzSI1t05fljjm0lS5Syw9Tjd8o4TBfp/nY1auWOOlbN9G/Y6Rt8EBoN6wWZip7pDyGVIUSDLsg=
Received: from NAM02-CY1-obe.outbound.protection.outlook.com (mail-cys01nam02lp0051.outbound.protection.outlook.com [207.46.163.51]) (Using TLS) by us-smtp-1.mimecast.com with ESMTP id us-mta-188-3QgR-Kj-OD-07XyINVwmRA-1; Tue, 29 Aug 2017 16:59:13 -0400
Received: from BY2PR1101MB1093.namprd11.prod.outlook.com (10.164.166.21) by BY2PR1101MB1144.namprd11.prod.outlook.com (10.164.166.142) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.1.1385.9; Tue, 29 Aug 2017 20:59:09 +0000
Received: from BY2PR1101MB1093.namprd11.prod.outlook.com ([10.164.166.21]) by BY2PR1101MB1093.namprd11.prod.outlook.com ([10.164.166.21]) with mapi id 15.01.1385.014; Tue, 29 Aug 2017 20:59:09 +0000
From: Thomas Haynes <loghyr@primarydata.com>
To: Chuck Lever <chuck.lever@oracle.com>
CC: "nfsv4@ietf.org" <nfsv4@ietf.org>, hch <hch@lst.de>
Thread-Topic: [nfsv4] Review comments for WGLC of draft-ietf-nfsv4-layout-types
Thread-Index: AQHTDHrZiISsN7s3MU64UU3+W1eRPaKF1gEAgAHU+gCAFE9fAA==
Date: Tue, 29 Aug 2017 20:59:09 +0000
Message-ID: <EF669094-DF5B-45F7-9A19-0760A13541C1@primarydata.com>
References: <418E308E-5057-499A-B403-38486BDEF772@oracle.com> <7572D8F6-D1FF-4723-AF89-1AD2B8C0F4F8@primarydata.com> <2F4E7FEE-0F61-4E73-8077-3D3AE134A85F@oracle.com>
In-Reply-To: <2F4E7FEE-0F61-4E73-8077-3D3AE134A85F@oracle.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-originating-ip: [63.157.6.18]
x-ms-publictraffictype: Email
x-microsoft-exchange-diagnostics: 1; BY2PR1101MB1144; 20:O6FQFrp0XtmfsUN2w+Lce4o/pC/D7c6Vrbu4ndWbJ+yHU/Pgh455pulQsWGYVG8+McVnwn2JTU2smM/ZDyIMypNwKPCcqHEyAO2/t+bxAPRYL5EbJrFKKFGPdNjcYTEBkDr/R7NF/HdUJLeHwA8DH29XPGTfcK7qIaP5ieaevRM=
x-ms-exchange-antispam-srfa-diagnostics: SSOS;
x-ld-processed: 03193ed6-8726-4bb3-a832-18ab0d28adb7,ExtAddr
x-ms-office365-filtering-correlation-id: 11fd057c-60dc-4e01-9f59-08d4ef20cb48
x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(300000500095)(300135000095)(300000501095)(300135300095)(22001)(300000502095)(300135100095)(2017030254152)(2017082002075)(300000503095)(300135400095)(2017052603199)(201703131423075)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095); SRVR:BY2PR1101MB1144;
x-ms-traffictypediagnostic: BY2PR1101MB1144:
x-exchange-antispam-report-test: UriScan:(158342451672863)(120809045254105)(192374486261705)(146099531331640);
x-microsoft-antispam-prvs: <BY2PR1101MB1144B3DBB9CD14BDE3FA1589CE9F0@BY2PR1101MB1144.namprd11.prod.outlook.com>
x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(601004)(2401047)(5005006)(8121501046)(100000703101)(100105400095)(93006095)(93001095)(10201501046)(3002001)(6041248)(20161123558100)(2016111802025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123564025)(20161123562025)(20161123555025)(20161123560025)(6072148)(6043046)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:BY2PR1101MB1144; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:BY2PR1101MB1144;
x-forefront-prvs: 0414DF926F
x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(39830400002)(24454002)(377454003)(189002)(199003)(85644002)(966005)(2950100002)(6916009)(7736002)(97736004)(6436002)(68736007)(236005)(6306002)(106356001)(3280700002)(8676002)(54906002)(53946003)(3660700001)(99286003)(6512007)(83716003)(8936002)(102836003)(6116002)(81156014)(81166006)(230783001)(3846002)(345774005)(54896002)(229853002)(5660300001)(53936002)(110136004)(6506006)(54356999)(105586002)(6246003)(36756003)(76176999)(50986999)(6486002)(77096006)(561944003)(2906002)(101416001)(14454004)(2900100001)(33656002)(189998001)(25786009)(53546010)(606006)(86362001)(4326008)(66066001)(82746002)(478600001)(42262002)(579004)(559001); DIR:OUT; SFP:1102; SCL:1; SRVR:BY2PR1101MB1144; H:BY2PR1101MB1093.namprd11.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en;
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
MIME-Version: 1.0
X-OriginatorOrg: primarydata.com
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Aug 2017 20:59:09.3046 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 03193ed6-8726-4bb3-a832-18ab0d28adb7
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY2PR1101MB1144
X-MC-Unique: 3QgR-Kj-OD-07XyINVwmRA-1
Content-Type: multipart/alternative; boundary="_000_EF669094DF5B45F79A190760A13541C1primarydatacom_"
Archived-At: <https://mailarchive.ietf.org/arch/msg/nfsv4/ho55ZBg7RAkw0WPLhe7pi-PG2lo>
Subject: Re: [nfsv4] Review comments for WGLC of draft-ietf-nfsv4-layout-types
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: Tue, 29 Aug 2017 20:59:25 -0000
On Aug 16, 2017, at 3:49 PM, Chuck Lever <chuck.lever@oracle.com<mailto:chuck.lever@oracle.com>> wrote: On Aug 15, 2017, at 2:51 PM, Thomas Haynes <loghyr@primarydata.com<mailto:loghyr@primarydata.com>> wrote: On Aug 3, 2017, at 10:06 AM, Chuck Lever <chuck.lever@oracle.com<mailto:chuck.lever@oracle.com>> wrote: Thanks to Tom for writing this one. No, thank you for the review! I'm not qualified to address the technical issues covered by this document, thus my comments are limited to editorial and mechanical suggestions. It would be great if current authors of new layouts (other than the author of this document) could review this one. Christoph, can you take a look at this one? It's short. The document is a valuable addition to the canon of pNFS specifications, but is still rough around the edges IMO. Nothing that is impossible to fix, but seems like section 4 needs some face lifting. Document status: This is a "Standards Track" document. Section 4 appears to update three RFCs which are Standards Track. Only one of these is mentioned in the document header and Abstract. Section 3 appears to be advice to authors of layout specifications, and advice to the WG on how to evaluate new layouts. Is the use of RFC 2119 terms appropriate for this section of this document? I think it is clear I am not a paragon of virtue when it comes to the correct use of RFC 2119 terms. == Abstract == "as a whole and those those specifically directed" Consider: "as a whole and those specifically directed” Ugh, thanks == 1. Introduction == "As a consequence, new internet drafts (see [FlexFiles] and [Lustre]) may struggle to meet the requirements to be a pNFS layout type." The Lustre document referenced here does not appear to be an RFC or an active I-D: https://datatracker.ietf.org/doc/draft-faibish-nfsv4-pnfs-lustre-layout/ Perhaps the reference should be removed or replaced. Which is why it is not normative. Note that the URL is stable. This may come out during the last editorial phase. It lays the groundwork for why we want the document. "This document specifies the layout type independent requirements placed on all layout types, whether one of the original three or any new variant." Consider this simplification: "This document specifies layout type-independent requirements placed on all layout types.” We have after Dave’s review: This document specifies the requirements placed on all layout types independent of the particular type chosen, whether one of the original three or any new additional one. and I think you would argue for: This document specifies the requirements placed on all layout types independent of the particular type chosen. And taking in mind your last point about the Intro, how about this as a last paragraph: As a consequence, new internet drafts (see [FlexFiles] and [Lustre]) may struggle to meet the requirements to be a pNFS layout type. This document gathers the requirements from all of the original layout type standard documents and then specifies the requirements placed on all layout types independent of the particular type chosen. The introduction should also announce what is happening in section 4, which provides updates to existing layout types. == 2. Definitions == In general the syntax of each definition should be consistent. You have "defines yada", "is yada", and sometimes complete sentences. Some examples follow: "control communication requirements: define for a layout type the details regarding information on layouts, stateids, file metadata, and file data which must be communicated between the metadata server and the storage devices." Consider: "control communication requirements: layout type requirements regarding information on layouts, stateids, file metadata, and file data which must be communicated between the metadata server and storage devices." "(file) data: is that part of the file system object which contains the data to read or writen. It is the contents of the object and not the attributes of the object." Consider: "(file) data: data content that is opaque to storage services. File data that is written is expected to be unaltered when it is subsequently read back. In particular, data content is not the file's attributes." "fencing: is the process by which the metadata server prevents the storage devices from processing I/O from a specific client to a specific file. Consider: "fencing: the mechanism by which a metadata server prevents storage devices from processing I/O from a specific client to a specific file." And so on. Went with <def>: [is | are] and complete sentences. "layout iomode: see Section 1." Strikes me as needing expansion. If a single paragraph is not adequate, perhaps another subsection of 2 can be added. This had already been fixed. "loose coupling: describes when the control protocol, between a metadata server and storage device, is a storage protocol." Loose and tight coupling might be better served in a separate subsection. This definition strikes me as vague for a first time reader. At least refer to section 3 here. The terms "layout" and "layout type" are key to the entire discussion. They might benefit greatly from an independent subsection in section 2, or even some discussion in section 1. I think there is a difference of opinion on what the definitions section means. To me, the section provides the smallest possible definition that the reader can look at to get the gist of the term. It is not a comprehensive definition of the term. When the reader comes across “layout type” later in the document, they can flip back to the definitions section to see the broad intent of the term. I accept your definition of "definition". There are certain terms, however, that you've already chosen to elaborate on. Elaborating on one or two other terms could be helpful, but I'm not going to be insistent. You might find it handy to have distinct definitions of "file access protocols" and "block access protocols" as separate classes of storage protocols. Throughout the document, you might consider using a more abstract term than "I/O operation". "File data access" for example might be more independent of which storage protocol is in use, or whether the access is direct to a storage device or through the MDS. I’m fine with I/O. == 2.1. Use of the terms ... == "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.” Ouch, in isolation this reads very badly. Consider: ... "is a storage device, but storage devices which use protocols which are not file access protocols (such as NFS) are not data servers.” Taken. "while simultaneously acting as a non-data-server storage device for others." I'm not quite sure what you are saying here. I don't see a definition for "non-data-server storage device". In context, perhaps you are referring to a storage device that does not simultaneously provide file access? A storage device can be a data server for one storage protocol and at the same time, not a data server for another storage protocol. Changed it to: Since a given 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 storage device for others. This discussion could benefit from adding definitions of "file data access" and "block data access", for instance. == 3. The Control Protocol == "In Section 12.2.6 of [RFC5661], the control protocol was introduced." Consider: "In Section 12.2.6 of [RFC5661], the concept of a layout type control protocol was introduced.” I’m struggling with adding this differentiation and I think I have a fix: One of the key requirements of a layout type is the need for a mechanism to be used to meet the requirements that apply to the interaction between the metadata server and the storage device such that they present a consistent interface to the client (Section 12.2.6 of [RFC5661]). I'm having some trouble with this. Mechanically, the word "requirements" is repeated. The last bit doesn't make sense to me: NFS and the storage protocols already make up a "consistent interface to clients," so I think you mean something else, like "a consistent view of stored data and lock state”. I see why you are struggling with it, but replacing it is a struggle for me. What you have is that each of the: 1) NFS protocol between client and mds 2) Storage protocol between client and storage device are already consistent interfaces. What I am saying is that the union of the two has to be consistent. Argh, I can read mine in yours but not yours in mine: A layout type has to meet the requirements that apply to the interaction between the metadata server and the storage device such that they present to the client a consistent view of stored data and lock state (Section 12.2.6 of [RFC5661]). It might be easier to parse if you broke it into two or three sentences. Particular implementations may satisfy this requirement in any manner they choose and the mechanism chosen may not be described as a protocol. In normal English, "may not be described as a protocol" can mean "please do not describe it as a protocol." A less ambiguous wording might be: "The chosen control mechanism is not required to be described as [or perhaps, specified as] a protocol.” In fixing the above, I actually struggled over this before reading this, I had gone with: Particular implementations may satisfy these requirements in any manner they choose and the mechanism chosen need not be described as a protocol. But now I am thinking of Particular implementations may satisfy these requirements in any manner they choose and the mechanism chosen need not be specified as a protocol. Specifications defining layout types need to clearly show how implementations can meet the Last nit picked, I promise: "to show clearly” Over the lifetime of this document or the author? :-) requirements discussed below, especially with respect to 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. Notice I also deleted a some sentences. This text is repeated in front of the third and fourth paragraphs: "In some cases, there may be no control protocol other than the storage” Fixed elsewhere "for metadata serverss and storage devices" Consider: "for metadata servers and storage devices” Fixed elsewhere "(2) The metadata server MUST be able to restrict access to a file on the storage devices when it revokes a layout." This section should make clear that after layout revocation, a client is still allowed to access file data via the MDS. Stating that is redundant to point 1. "(4) Locking MUST be respected." This section feels like it needs expansion. Even "NFSv4.1 file locking semantics MUST be respected." Otherwise, this is not an enforceable MUST. Already reworked. "(5) The metadata server and the storage devices MUST agree on attributes like modify time, the change attribute, and the end- of-file (EOF) position." It would be better to attempt to provide a list of the attributes where this requirement is necessary instead of providing examples. I think this section would be more clear if the requirement is constructed from solely the client's view. I’m fine with the current POV. This item and its discussion don't make sense for storage protocols that are not file access protocols. It needs to be rephrased to apply strictly to storage devices that are accessed via a file access protocol, which would expose file attribute information to clients. Oh, good catch! == 3.2. Undocumented ... == "(4) Clients MUST NOT perform I/O operations which would be inconsistent with the iomode specified in the layout segments it holds." It's not immediately obvious to me what you are getting at in this item. If a client has a READ iomode, then it cannot WRITE using it. "(5) The metadata server MUST be able to do allocation and deallocation of storage. I.e., creating and deleting files." Perhaps the language here could be refined. "storage" can mean anything and hasn't been previously defined. I think the rewording from Dave’s review satisfies this? (3) The metadata server MUST be able to do storage allocation, whether that is to create, delete, extend, or truncate files. I think you are talking about managing layout extents on storage devices? == 4.1 File Layout Type == Throughout this section, the discussion is not clear when it switches between describing behavior specified by RFC 5661, current implementation behavior, and fresh advice to new implementers. I’m torn here, despite this statement: This section is not normative with regards to each of the presented types. This document does not update the specification of either the block layout type (see [RFC5663]) or the object layout type (see [RFC5664]). Nor does it update Section 13 of [RFC5661], but rather Section 12 of that document. In other words, it is the pNFS requirements being updated rather than the specification of the file layout type. Now I understand what you're going for. I think it would be more clear if you start with what _is_ being updated. How about: "This section updates Section 12 of [RFC5661], which enumerates the requirements of pNFS layout type specifications. It is not normative with regards to the layout type presented in Section 13 of RFC5661, the block layout type [RFC5663], and the object layout type [RFC5664]. These are discussed here only to illuminate the updates made to RFC5661 Section 12.” I am fine with this change. Section 4 is not updating the pNFS requirements. That occurs in Section 3. I can see a rewhack of the draft to have in order: Section 3.0 Section 4.0 Section 4.1 Section 4.2 Section 4.3 A new Section 5 to introduce the following Section 3.1 as Section 5.1 Section 3.2 as Section 5.2 Section 3.3 as Section 5.3 Section 5 as Section 6 … So the new Section 3 declares the main issue. The new Section 4 uses the existing layout types to illustrate the issues. The new Section 5 presents the resulting requirements. Hmm, the old Section 5 could be the new Section 5.0. Thoughts? My feeble mind can't imagine the result with this brief description, but your proposal could very well be a better organization of the material. Hah, “feeble”, now I have to try to remember what I wrote! I am going to follow Dave’s suggestion here, apply the edits above, publish v7 and see if the WG can flow with the changes. There are no reasons given about why these changes are being made (was the spec language inadequate, or are you addressing bugs in the protocol, for example)? Please make it clear what the original spec says, what it should now say, and what is new advice to implementers of the file layout type. And separately discuss why the update is necessary. See Noveck's migration update doc series for examples of how to clearly describe an update to specification language. Similar comments apply to sections 4.2 and 4.3: it's not clear what is being updated. I'm not at all arguing about the technical content of this section. These are strictly observations about how this material is presented. == 5. Summary == I'm not sure what purpose this section would play once this document becomes an RFC. == 6. Security Considerations == "It may do this directly, by providing that appropriate checks be performed at the time the access is performed." Consider: ... "at the time each access is performed.” Ack "The client the has the opportunity to re-acquire the layout" Consider: "The client has a subsequent opportunity to re-acquire the layout” Ack and thanks There is a strong discussion of client-side access checking in section 4.2. Would that be more appropriate to move into this section in a more generic form? I’m fine with it as is. -- Chuck Lever _______________________________________________ nfsv4 mailing list nfsv4@ietf.org<mailto:nfsv4@ietf.org> https://www.ietf.org/mailman/listinfo/nfsv4 -- Chuck Lever
- [nfsv4] Review comments for WGLC of draft-ietf-nf… Chuck Lever
- Re: [nfsv4] Review comments for WGLC of draft-iet… Thomas Haynes
- Re: [nfsv4] Review comments for WGLC of draft-iet… David Noveck
- Re: [nfsv4] Review comments for WGLC of draft-iet… Christoph Hellwig
- Re: [nfsv4] Review comments for WGLC of draft-iet… Thomas Haynes
- Re: [nfsv4] Review comments for WGLC of draft-iet… Thomas Haynes
- Re: [nfsv4] Review comments for WGLC of draft-iet… Thomas Haynes
- Re: [nfsv4] Review comments for WGLC of draft-iet… Chuck Lever
- Re: [nfsv4] Review comments for WGLC of draft-iet… David Noveck
- Re: [nfsv4] Review comments for WGLC of draft-iet… hch
- Re: [nfsv4] Review comments for WGLC of draft-iet… Thomas Haynes
- Re: [nfsv4] Review comments for WGLC of draft-iet… Chuck Lever
- Re: [nfsv4] Review comments for WGLC of draft-iet… David Noveck