Network File System (NFS) Version 4 Minor Version 2 Protocol
draft-ietf-nfsv4-minorversion2-41

I have no objection to this document, but the relationship with the older versions and how versioning is handled is not clear to me.  Some parts of the document that add to my confusion:

- Section 2. (Minor Versioning) says that “NFSv4.2 only defines extensions to NFSv4.1”, but draft-ietf-nfsv4-versioning [*] says that “minor versions zero and one are not extensible.”

- Section 1.1. (Scope of This Document) says that “NFSv4.2 is a superset of NFSv4.1, with all of the new features being optional”.  Which sounds to me as if this document is defining more optional features for NFSv4.1 and not an extension.

In any case, I am not familiar with the versioning (which is why I wanted to learn about it), but I am now more confused than before.  It may all be a case of terminology..

[*] Why isn’t the reference to draft-ietf-nfsv4-versioning Normative?  Also, the reference ([NFSv4-Versioning]) is not formatted correctly.

I appear to be in the minority here, in that I *did* understand this document's place relative to 4.0, and 4.1.  Still, I agree that clarifying that is really important, and I'll suggest a specific clarification in Section 1.1):

OLD
   This document describes the NFSv4.2 protocol.  With respect to
   NFSv4.0 and NFSv4.1, this document does not:
NEW
   This document describes the NFSv4.2 protocol as extensions to
   the specifications of NFSv4.0 and NFSv4.1. Those specifications
   remain current and form the basis for the additions defined
   herein.  It is necessary to implement those before adding
   NFSv4.2 to the implementation.
   
   With respect to NFSv4.0 and NFSv4.1, this document does not:
END

-- Section 1.2 --

   A major goal of the design of NFSv4.2 is to take common local file
   system features and offer them remotely.

This sounds like it means to be a change in goals relative to 4.0 and 4.1.  I think it would fit better to say it this way, and would add to the clarification above:

NEW
   A major goal of the enhancements provided in NFSv4.2 is to take
   common local file system features that have not been available
   through earlier versions of NFS, and to offer them remotely.
END

-- Section 6.1 --

   Hole:  A byte range within a Sparse file that contains regions of all
      zeroes.  A hole might or might not have space allocated or
      reserved to it.

I'm wondering about the "regions of" here: If I have a byte range that contains two regions of all zeroes and something that's not all zeroes in between those two regions, I do not have a (single) hole, do I?  Why does this say "regions of"?

And a question: Is there any disctinction between a byte-range within a sparse file that happens to contain all zeroes... and one that is recorded in metadata as being all zeroes?  Can some file systems write a region of zeroes without "knowing" that they have created a hole?  Does this distinction matter here?

-- Section 6.2.1 --

   Note that as the client has no a priori
   knowledge of whether a hole is present or not

(No need to respond to this; take it or leave it as you please.)  I have a general preference for avoiding Latin terms, as they're not properly understood by everyone.  In this case, too, "a priori" has a connotation that goes beyond the literal Latin translation.  I think it'd be better to word this as "Because the client does not know in advance whether a hole is present or not".

   READ_PLUS extends the response with a new arm representing holes to
   avoid returning data for portions of the file which are initialized
   to zero and may or may not contain a backing store.  Returning data
   blocks of uninitialized data wastes computational and network
   resources, thus reducing performance.

It wouldn't be "uninitialized" data, would it?  It'd be zeroes.  I think you might just want to say "Returning actual data blocks corresponding to holes", yes?

   By contrast, if a READ_PLUS occurs in the middle of a hole,
   the server can send back a range which starts before the offset and
   extends past the range.

I'm not sure how a range can extend past itself ("a range which ... extends past the range").  I think you just want to say "a range representing the hole."

-- Section 6.2.2 --

   DEALLOCATE can be used to hole punch, which allows the client to
   avoid the transfer of a repetitive pattern of zeros across the
   network.

This is the first time you've mentioned DEALLOCATE where I think I understand that it is a way of doing a WRITE wherein the client sends the representation of a hole to the server, rather than actually doing a WRITE.  (I had previously thought it was used to tell the server to undo an ALLOCATE, but it now seems that they are related things, but are not duals.)

You might want to be more clear about this here, especially if what I say in the previous paragraph is wrong.

-- Section 7 --

   Lesser space MAY be
   consumed for backups after block deallocation.

I don't think this is a proper 2119 MAY; it sounds like a statement of fact, not a protocol option.

I've cleared my DISCUSS position based on email discussion.

=== Substantive Comments ===

-1.1 and 1.2
It would help to clarify the relationship between this draft and NFS 4.0 and 4.1. This section says that this document does not describe or clarify them. This led me to expect that the  NFS 4.2 spec would stand alone, but it seems more like this draft is incremental to at least 4.1.

Also, as mentioned in the opsdir review, it would be helpful to comment on what expectations of interoperability exist between 4.2., 4.1, and 4.0.

-4.10.1.1, last paragraph: "Servers SHOULD reject..."
Why not MUST? Do you envision circumstances where it might be okay to accept COPY_NOTIFY requests over insecure channels? Channels with other forms of privacy protection?

-- Last sentence in same paragraph:
- 9.6.1.1, first paragraph, last sentence:

Does this mean to imply that it may also choose _not_ to evaluate whether the attribute is successful? 

- 9.6.1.2, last paragraph:

Why just MAY? does it ever make sense for a "Full Mode" implementation to not validate the security attributes?

- 18

The reference to [Quigley14] needs to be normative.


=== Editorial and Nits ===

- 3:
This section seems out of context. It jumps into specifics without introduction. Maybe an introductory paragraph would help, or perhaps it would help to move this section into the section on the new operations. 

Also, please expand pNFS on first mention

- 4.2, paragraph 4: "traditional copy authentication approach"

Is "authentication" the right word? The paragraph otherwise seems about authorization.

-4.2.2, paragraph 6:
Please expand ONC on first mention.

- 4.10.1.1, paragraph after first code fragment:
should "cfp_shared_secret" be "cfap_shared_secret"?

- 9.6.1.1, first paragraph, first sentence:
I don’t understand this sentence. Does it add value to the paragraph?

-- [NFSv42xdr]

The reference is missing the short name for the I-D, and should be designated as a work-in-progress. (I know you intend for the RFC editor to replace this, but they have processes in place to do the right thing if you reference the I-D.)

As noted in Elwyn's review, the random number/shared secret should be noted as something that should not be re-used.

At this time I have no objection while awaiting the outcome of the opsdir review performed by sheng jiang

---
Hi, OPS-DIR, Authors,

I have reviewed this document as part of the Operational directorate's ongoing effort to review all IETF documents being processed by the IESG. These comments were written with the intent of improving the operational aspects of the IETF drafts. Comments that are not addressed in last call may be included in AD reviews during the IESG review. Document editors and WG chairs should treat these comments just like any other last call comments.

This Standards Track document specifies describes NFS version 4 minor version two, describing the protocol extensions made from NFS version 4 minor version 1. It has been coupled with another document, "NFSv4 Minor Version 2 Protocol External Data Representation Standard (XDR) Description, draft-ietf-nfsv4-minorversion2-dot-x. They two should be published together, I believe. This document is well written. It is almost ready to be published. There are a major comment below, which may be fixed by two references if there is already specification by another existing document:

In Section 3.3, "As the File Layout Type does not provide a means for informing the client as to which minor version a particular storage device is providing, it will have to negotiate this via the normal RPC semantics of major and minor version discovery." It is not clear how negotiation to be performed and the procedure. I did not find existing specification for minor version discovery, neither. If there were existing specifications, two references could solve my concern. But if there were not, the document may need to define the procedure by itself.

Two more comments between major and minor:

The compatibility and potential interoperation among this 4.2 and 4.1, 4.0, is still not very clear from operational perspective. It would be helpful to add one more subsection in section 1 or 2 to clarify this.

It would help to clarify whether this specification is IP independent, although my guess is positive. This document does give IPv4 examples, but not clear whether it is also working with IPv6.

Minor comments:
 
  RFC 2401 has been obsoleted by RFC 4301,
  RFC 2616 has been obsoleted by RFC 7230 series.

A few abbreviations does not give full name explanation: ONC, NIS, NTFS, HFS, HPC, pNFS, EOF, although they may be well known.

A few typos: Section 4.10.1.1.5, the last paragraph, nounce/nonce, <"copy_to_auth", user id, source list, nounce, nounce MIC, context handle, handle version>.
      Section 4.10.1.2, the second paragraph, uiniquely/uniquely, "The cnr_stateid returned from the COPY_NOTIFY can be used to uiniquely identify the destination server to the source server."
      Section 15.3, the third last paragraph, destination/ destination, "As the source does not know which netloc4 network location the destination might use to establish the copy operation"
      Section15.13.3, the third last paragraph, wlll/will, "the clone block size wlll be zeroed."

Best regards,

Sheng

I am, of course, as confused as the rest of the ADs who have commented about the relationship between 4.0, 4.1, and 4.2. If that could be easily explained in the first part of the document, great.

- 1.3.2: I'm just curious, feel free to ignore. Does anyone
maintain those posix specs these days? (e.g.
[posix_fadvise]).

- 1.4: "a new arm" isn't clear to this reader, maybe
consider re-phrasing if this isn't a common NFS term (if it
is, that's fine)

- 4.10: thanks for providing all that!

- 4.10: possibly dumb question: does this (or NFSv4) support
what a user would perceive as an encrypting file system
where the keys are held-by/derived from something on the
user's machine? If so, and if the two servers involved use
keys known at the user's machine and not by the NFS
infrastructure then I'm not sure how this can work. IOW, if
there's a decryption and then a re-encryption required in a
server-server copy and if that needs any keying material
from the client then could that ever work? Note that I'm not
talking about securing the file during the server-server
copy but de-crypting before sending and then re-encrypting
before storing.

- 9.6: Is it considered a requirement that e.g. a user
cleared to secret cannot tell if there is anything stored
that is of higher classification? If so, did anyone go
through all NFS interfaces to check if e.g. disk or
directory usage information could give away the fact that
there is stuff stored that's not visible to this user?  I'm
not arguing that this definitely needs to be done
exhaustively, but maybe consider adding a caveat emptor
sentence somewhere saying that even with MAC, it could be
that NFS allows for some minor meta-data leakage such as the
above.