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

From: David Noveck <[email-masked]>
Date: Tue, 22 Aug 2017 07:45:18 -0400

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

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

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

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.

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

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.

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

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

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

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

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.

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

---

From: Thomas Haynes <[email-masked]>
Date: Thu, 31 Aug 2017 00:39:49 +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 <[email-masked]<mailto:[email-masked]>> wrote:

Review Structure

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

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

  *   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

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.

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

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.

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.