Re: [nfsv4] Review of draft-ietf-nfsv4-flex-files-13 (part one of two)

Thomas Haynes <loghyr@primarydata.com> Thu, 31 August 2017 00:39 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 CDC21132494 for <nfsv4@ietfa.amsl.com>; Wed, 30 Aug 2017 17:39:59 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.7
X-Spam-Level:
X-Spam-Status: No, score=-2.7 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] 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 O01LYGBZnRqj for <nfsv4@ietfa.amsl.com>; Wed, 30 Aug 2017 17:39:56 -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 CD8411323B8 for <nfsv4@ietf.org>; Wed, 30 Aug 2017 17:39:55 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=primarydata.com; s=mimecast20170802; t=1504139994; h=from:subject:date:message-id:to:cc:mime-version:content-type:in-reply-to:references; bh=Gyla4VJLJSY8ckqVlBnxV/ZMxVkcpPfjt+GarsbxvBg=; b=gIkhO40N4yHUXZgV9E1JRVFeKhFi+NHtFu5s1v6G30HyDfC8k/S1j2ECSUuN0sfbUx2Ub4sY/i3RrZncrIZGVi4ecsipaJ2jCbblDAR04egKWY+j+p8aE29kU7pD8sI5jDhTT2bVtC63hw6b+PKW3oFyVZI5dv3yE0hagEgcu6s=
Received: from NAM02-CY1-obe.outbound.protection.outlook.com (mail-cys01nam02lp0052.outbound.protection.outlook.com [207.46.163.52]) (Using TLS) by us-smtp-1.mimecast.com with ESMTP id us-mta-196-iTL3sphUN3CVm3Cqi58DHQ-1; Wed, 30 Aug 2017 20:39:52 -0400
Received: from DM3PR1101MB1104.namprd11.prod.outlook.com (10.164.196.24) by DM3PR1101MB1088.namprd11.prod.outlook.com (10.164.196.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.13.10; Thu, 31 Aug 2017 00:39:50 +0000
Received: from DM3PR1101MB1104.namprd11.prod.outlook.com ([fe80::2548:eb18:ffa6:1ca3]) by DM3PR1101MB1104.namprd11.prod.outlook.com ([fe80::2548:eb18:ffa6:1ca3%13]) with mapi id 15.20.0013.011; Thu, 31 Aug 2017 00:39:49 +0000
From: Thomas Haynes <loghyr@primarydata.com>
To: Dave Noveck <davenoveck@gmail.com>
CC: "nfsv4@ietf.org" <nfsv4@ietf.org>
Thread-Topic: Review of draft-ietf-nfsv4-flex-files-13 (part one of two)
Thread-Index: AQHTGzwkBhubT73k5Em0SrmftRifr6KdrNSA
Date: Thu, 31 Aug 2017 00:39:49 +0000
Message-ID: <E22DB76D-17BD-4F96-BCC5-A7C557BC3B08@primarydata.com>
References: <CADaq8jfHL4o=f6FC-pTbck=B-=4x_PRJ8tPht2u5C-hf4tVt_Q@mail.gmail.com>
In-Reply-To: <CADaq8jfHL4o=f6FC-pTbck=B-=4x_PRJ8tPht2u5C-hf4tVt_Q@mail.gmail.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; DM3PR1101MB1088; 20:1tc6JR+lrOasDuML552T0o+dMlR7HF+pAj3EUESXqcDq12AeV69XEzbgaSHkfirZOH36B9AtpneQU+XmT6S4qYi9Z1NPHCD8ldYnobyA3U75NlqpEzrifaS9xaERR/yZMmR1zGW7tpUvB6P7+454UJZH63RWGe6IwRbMHdZQZpY=
x-ms-exchange-antispam-srfa-diagnostics: SSOS;
x-ms-office365-filtering-correlation-id: 12f6c033-39bb-44b1-e73f-08d4f008c9a6
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:DM3PR1101MB1088;
x-ms-traffictypediagnostic: DM3PR1101MB1088:
x-exchange-antispam-report-test: UriScan:(158342451672863)(192374486261705);
x-microsoft-antispam-prvs: <DM3PR1101MB10884FA3DC0B67CAECD9AD9DCE9D0@DM3PR1101MB1088.namprd11.prod.outlook.com>
x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(601004)(2401047)(8121501046)(5005006)(3002001)(93006095)(93001095)(100000703101)(100105400095)(10201501046)(6041248)(2016111802025)(20161123564025)(20161123555025)(20161123558100)(20161123562025)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(6043046)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:DM3PR1101MB1088; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:DM3PR1101MB1088;
x-forefront-prvs: 04163EF38A
x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(39830400002)(189002)(24454002)(199003)(51914003)(377454003)(2900100001)(3846002)(102836003)(6116002)(236005)(6916009)(2950100002)(39060400002)(54896002)(53936002)(6506006)(76176999)(54356999)(6436002)(25786009)(3280700002)(106356001)(50986999)(53546010)(3660700001)(6246003)(1411001)(110136004)(2906002)(66066001)(229853002)(4326008)(6486002)(97736004)(6512007)(99286003)(83716003)(189998001)(82746002)(86362001)(8936002)(81156014)(68736007)(33656002)(5250100002)(8676002)(105586002)(14454004)(5660300001)(478600001)(36756003)(81166006)(7736002)(101416001)(230783001)(42262002); DIR:OUT; SFP:1102; SCL:1; SRVR:DM3PR1101MB1088; H:DM3PR1101MB1104.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: 31 Aug 2017 00:39:49.8239 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 03193ed6-8726-4bb3-a832-18ab0d28adb7
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM3PR1101MB1088
X-MC-Unique: iTL3sphUN3CVm3Cqi58DHQ-1
Content-Type: multipart/alternative; boundary="_000_E22DB76D17BD4F96BCC5A7C557BC3B08primarydatacom_"
Archived-At: <https://mailarchive.ietf.org/arch/msg/nfsv4/KXhyrxTJAdZvbUsujYuta91cTlI>
Subject: Re: [nfsv4] Review of draft-ietf-nfsv4-flex-files-13 (part one of two)
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: Thu, 31 Aug 2017 00:40:00 -0000

Ack with no text means I made the change...

And thanks for the comments!


On Aug 22, 2017, at 4:45 AM, David Noveck <davenoveck@gmail.com<mailto:davenoveck@gmail.com>> wrote:

Review Structure

This review hs been split up into multiple emails to avoid running afoul of mailing list size restrictions.

This is the first section.

Current Section

This section contains all the general (non-section-specific) comments and the first section of the detailed per-section comments.

Next Section

Contains the bulk of the per-section comments, including those related to security considerations.

General Comments

Background of Review

There are two significant recent changes that need to be addressed:

  *   The major revision of security for loose coupling that was made in -13.

  *   The need to co-ordinate this document with layout-types, which is also in last call.

In addition, I feel there is a need for a review of other sections of the document because I never had a chance to review -12 and because the document has made enough progress that it is clearly headed to the IESG soon.  Because of that, I am shifting the focus of the review to look more closely at the kind of issues that might arise during IESG review.

Overall Evaluation

In terms of technical content, this document is ready to go forward.  The protocol is sound and it is explained in a manner that can be understood by the members of the working group who are familiar with pNFS.

There are some issues of presentation that need to be addressed before the document goes forward:

  *   There are still some typo-level issues that need to be addressed.

  *   In some cases, the presentation, while understandable to those with the appropriate background, may be confusing to naive readers, who the IESG generally wants to accommodate.

It appears that all of these issues can be addressed by changes of limited scope, allowing a -14 to go forward without a new last call.

Comments by Section (until Section 2.1)

Abstract

In the last sentence, "Client side" needs to be replaced by "Client-side”.

Ack



1.  Introduction

There are a number of problems with the first sentence of the second paragraph:

  *   The introductory clause seems to be coming out of the blue, given that may readers, including the IESG members, may not have the necessary background to understand it .  One possibility is to add a reference to section 13 of RFC5661, but I don't think that fully solves the problem.

I am fine with the out of the blue approach.



  *   The reference to "a back-end control protocol" would be similarly confusing, especially since the definition of "control protocol" although not "back-end control protocol" appears later, in Section 1.1.

I am fine with the reader being able to infer that back-end applies to the protocol between the mds and the dses.



  *   The use of the "MAY" in the second clause is confusing.  It is true that when the storage devices are NFSv4.1+, this is. formally speaking, an option, most implementers will for various reasons have no such choice.

ACK


The following suggested replacement is one way to address these issues:

When the storage devices support the NFSv4.1 protocol, it possible for implementations to  provide support for a global state model equivalent to that described in Section 13 of [RFC5661].  See Section  2.3.2 for further description.  Any such implementation would a communiation mechanism between the MDS and the data storage devices,contituting a control protocol as defined below.

With regard to the second sentece of the second paragraph:

  *   If this is important enough to raise in the introduction, you cannot simply say "not my job.  Goodbye."  If it is not your job, which it isn't, you should give some indication of whose job it might be.
  *   If the phrase "out of scope" is to be retained, it should be hyphenated.  In any case, that phrase is jargon-ish and unhelpful.

I understand your point here, I have replaced the second paragraph with:

   To provide a global state model equivalent to that of the files
   layout type, a back-end control protocol might be implemented between
   the metadata server and NFSv4.1+ storage devices.  This document does
   not provide a standard's track control protocol.  An implemementation
   can either define its own mechanism or it could define a control
   protocol in a standard's track document.  The requirements for the a
   control protocol are specified in [RFC5661] and clarified in
   [pNFSLayouts].



I'm going to suggest the following as a posible replacement:

The definition of such a mechanism will not be provided in this document.  An implementation is free to define its own mechanism.  Alternatively, a standard protocol could be defined, allowing new implementations the option of choosing either it or an implementation-specific mechanism.

1.1.  Definitions

I'm going to start by discussing the role of defnitions.  In connection with a review of layout-types-05, Chuck suggested some elaboration of certain terms.  While I agree with Tom's response that a "comprehensive" defnintion is not, in general, required, there were certain things about the specifics of Tom's response that were concerning and need to be addressed here.

Tom seemed to say that he had his own definition of "definition". I'll be using the following that I got using Google as the standard I will use in my review:

a statement of the exact meaning of a word, especially in a dictionary

I don't think the IESG will be OK with anyone having a significantly different approach and there is no point in presenting the document to the IESG, if one thinks one can somehow get a waiver from the IESG's expectations, which are generally to expect that terms be clearly defined before use.

These seem to be different from Tom's expectations, which treat these terms as well-understood and anticipates that they only be used later as a reminder of the gist of a term already used.


My prior post-WG level reviews have certainly shown that those reviewers do validate the definition of terms used in prior documents.


The good news here is that the problems with the definitions in this section are quite limited and can be easily addressed.

Another general issue that needs to be addressed up-front concerns the relationship between these definitions and those in layout-types-06. Where the same term is defined in both documents, the definitions should agree and I'll be noting cases where they do not.

In the definition of "control protocol', the term "data access protocol" is used, although it is not defined either here of in layout-types-06 .  Suggest using "storage protocol", which is defined in layout-types, instead.

Ack



In the definition of  "client-side mirroring", suggest replacing "is when" by "is a feature in which".


Ack


The definition for the term "data file" matches that for the term "(file) data" in layout-types-06.  The cases of "metadata file" and "(file metadata" are similar. It would be best if the same terminology ere used in these related documents.  Otherwise the differences should be mentioned.

Ack, going with (file) data approach.



For the following definitions, it would be better (clearer) to use the definitions in layout-types-06: "fencing", "layout" (first paragraph only), "layout iomode", "layout stateid", "layout type", "loose coupling", "metadata sever (MDS)”.

Ack

And for those keeping score, as the layout-types draft was modified by
Chuck’s suggestion to follow a certain format, I have applied that same
format here in any term Dave did not point out.



In the case of "tight coupling", the two documents need to be in alignment, but the phrase "is when" needs to be replaced by "is an arrangement in which".


Ack

Suggest addIng a defnition of "storage protocol" matching that in layout-types-06.

Ack

2.  Coupling of Storage Devices

Given that function of the section is to provide alternative coupling models I'd like to suggest:

  *   That the section title be changed to something like: "Coupling Models Supported”.

I’m okay with “Coupling Models”, but not the “Supported”.


  *   That the first sentence needs to be revised to make it clearer exactly who is choosing the coupling model.  Suggest:

A server implemetation may choose to implement either a loose or tight coupling model.

Ack - although I am choking on heavy use of implement here - I just can’t get rid of the second one.

Went with:

    A server implemetation may choose either a loose or tight coupling
    model between the metadata server and the storage devices.


With regard to the description of the two coupling models, while all of the statements are true, the text is not clear about what is necessary to provide these states of affairs and who is responsible for providing the necessary  pre-requisites.

For the tight coupling section, suggest the following replacement:

To implement the tight coupling model, there needs to be a control protocol defined capable of managing security and LAYOUTCOMMITs, providing a global stateid model and related functions, all without imposing special requiremets on the client.

I don’t like the last phrase, while it is true for this layout type, it is not true for all layout types.


For the loose coupling section, besides the same sort of lack of clarity, the following issues need to be adressed:

  *   The statement that the control protcol "might be a version of NFS is confusing because it is impossible for the control protocol to be anything other than a version of NFS.

Actually, no, either:

1) There is another storage protocol available.
2) There is some management protocol available - i.e. NetApp has zapis

Although in practice, it is NFS…


  *   It is unclear who the statement that the semanntics "MUST be defined" is directed to. Normally such an RFC2119 term is directed to a client or servrer implementation. In this case, it is confusingly directed to the rest of this document.

For the loose coupling section suggest that the following replacement be used:

When implementing the loose coupling model, the only control protocol will be a version of NFS, with no ability to provide a global stateid model or to prevent clients from using layouts inappropriately. To enable client use in that environment, this document will specify how security, state, and locking are to be managed.


Ack

So the complete set of changes to Section 2 are:

2.  Coupling of Storage Devices

   A server implementation may choose either a loose or tight coupling
   model between the metadata server and the storage devices.  To
   implement the tight coupling model, a control protocol has to be
   defined.  As the flex file layout imposes no special requirements on
   the client, the control protocol will need to provide:

   (1)  for the management of both security and LAYOUTCOMMITs, and,

   (2)  a global stateid model and management of these stateids.

   When implementing the loose coupling model, the only control protocol
   will be a version of NFS, with no ability to provide a global stateid
   model or to prevent clients from using layouts inappropriately.  To
   enable client use in that environment, this document will specify how
   security, state, and locking are to be managed.