Re: [nfsv4] Spencer Dawkins' Yes on draft-ietf-nfsv4-rfc3530bis-26: (with COMMENT)

["Haynes, Tom" <[e-mail]>]

Spencer,

Thanks for the review!

Tom

On May 27, 2013, at 4:43 PM, Spencer Dawkins <[e-mail]> wrote:

> Spencer Dawkins has entered the following ballot position for
> draft-ietf-nfsv4-rfc3530bis-26: Yes
> 
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
> 
> 
> Please refer to http://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
> 
> 
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> I'm balloting "YES", but I'm providing a number of comments. Please don't
> be discouraged. I have 14 comments, for an average of one comment every
> 23 pages :-)
> 
> Several are nits. Several others are places where I didn't think the
> meaning was clear. I do have three questions.
> 
> None of these questions are blocking, but I'd appreciate it if you could
> look at 3.1, 5.6 and 9.14.
> 
> In this text:
> 
> 1.2.  Inconsistencies of this Document with the companion document NFS
>      Version 4 Protocol
> 
>   [I-D.ietf-nfsv4-rfc3530bis-dot-x], NFS Version 4 Protocol, contains
>   the definitions in XDR description language of the constructs used by
>   the protocol.  Inside this document, several of the constructs are
>   reproduced for purposes of explanation.  The reader is warned of the
>   possibility of errors in the reproduced constructs outside of
>   [I-D.ietf-nfsv4-rfc3530bis-dot-x].  For any part of the document that
>   is inconsistent with [I-D.ietf-nfsv4-rfc3530bis-dot-x],
>   [I-D.ietf-nfsv4-rfc3530bis-dot-x] is to be considered authoritative.
> 
> It took me a few reads to be somewhat confident that I understood what
> this text says - the first few times, I was reading it as "these are the
> inconsistencies between the two documents, but we're publishing both
> documents without fixing them". I think the section title was about half
> of my problem. I know that's not what you meant :-)
> 
> Am I reading it correctly, if I restate it this way?
> 
> 1.2.  Definitions in the companion document NFS Version 4 Protocol are
>      Authoritative

ok

> 
>   [I-D.ietf-nfsv4-rfc3530bis-dot-x], NFS Version 4 Protocol, contains
>   the definitions in XDR description language of the constructs used by
>   the protocol.  Inside this document, several of those constructs are
>   reproduced for purposes of explanation.  The reader is warned of the
>   possibility of errors in the reproduced constructs in this
>   document.  For any part of this document that
>   is inconsistent with [I-D.ietf-nfsv4-rfc3530bis-dot-x],
>   [I-D.ietf-nfsv4-rfc3530bis-dot-x] is to be considered authoritative.
> 
> In this text:
> 
> 1.3.3.  Filesystem Model
> 
>   The NFSv4 protocol does not require a separate protocol to provide
>   for the initial mapping between path name and filehandle.  Instead of
>   using the older MOUNT protocol for this mapping, the server provides
>   a ROOT filehandle that represents the logical root or top of the
>   filesystem tree provided by the server.  The server provides multiple
>   filesystems by gluing them together with pseudo filesystems.  These
>   pseudo filesystems provide for potential gaps in the path names
>   between real filesystems.
> 
> Is "potential gaps" a term of art in the NFS community? It hasn't been
> defined in the spec, and "gap" only appears one other place in the spec,
> in Section 8.6. I can figure it out from the discussion in 8.6, but
> that's more than 100 pages into the spec.

I'm okay with leaving this as is.


> 
> In this text:
> 
> 1.3.3.3.  Multi-server Namespace
> 
>   These attributes may be used together with the concept of absent file
>   systems, which provide specifications for additional locations but no
>   actual file system content.
> 
> Is "absent file system" a term of art in the NFS community? It's defined
> in section 7.2, but that's the 7th occurrence in the document, about 75
> pages in.
> 

Added to general definitions

> In this text:
> 
> 3.1.  Ports and Transports
> 
>   To date, all NFSv4 implementations are TCP based, i.e., there are
>   none for SCTP nor UDP.  UDP by itself is not sufficient as a
>   transport for NFSv4, neither is UDP in combination with some other
>   mechanism (e.g., DCCP [RFC4340], NORM [RFC5740]).
> 
> "some other mechanism" wasn't clear to me. Could you help me understand
> what this sentence is saying about DCCP and NORM, and what the problems
> are if you tried to use (for example) NFSv4 over DCCP? I believe you,
> it's just unclear to me from the text what reason you have for saying
> this.

I'm going to let someone from the WG chime in here.

> 
> In this text:
> 
> 5.6.  REQUIRED Attributes - List and Definition References
> 
>   o  Id: The number assigned to the attribute.  In the event of
>      conflicts between the assigned number and
>      [I-D.ietf-nfsv4-rfc3530bis-dot-x], the latter is likely
>      authoritative, but should be resolved with Errata to this document
>      and/or [I-D.ietf-nfsv4-rfc3530bis-dot-x].  See [ISEG_errata] for
>      the Errata process.
> 
> I'm struggling with "is likely authoritative". I suspect that part of my
> struggle is that you're talking about how to resolve possible conflicts
> between this spec, another spec, and an IANA registry. You're sort of
> picking a designated tie-breaker, but the reader still has to try to
> figure out what the right answer should be. Given that you're telling
> people to be prepared to resolve conflicts using the errata process, do
> you need to say more than that?


I've touched this one up.


> 
> In this text:
> 
> 5.8.2.26.  Attribute 40: quota_used
> 
>   The value in bytes that represents the amount of disc space used by
>   this file or directory and possibly a number of other similar files
>   or directories, where the set of "similar" meets at least the
>   criterion that allocating space to any file or directory in the set
>   will reduce the "quota_avail_hard" of every other file or directory
>   in the set.
> 
> I noted a "disc" among the "disk"s.

fixed

> 
> In this text:
> 
> 7.7.  Effecting File System Transitions
> 
>   Transitions between file system instances, whether due to switching
>   between replicas upon server unavailability or to server-initiated
>   migration events, are best dealt with together.  This is so even
>   though, for the server, pragmatic considerations will normally force
>   different implementation strategies for planned and unplanned
>   transitions.  
> 
> these two sentences would have made more sense to me if they used the
> same words to describe the same thing. If I'm getting the parallels
> right, one way to do that, would be 
> 
> 7.7.  Effecting File System Transitions
> 
>   Transitions between file system instances, whether due to switching
>                                                            ^ unplanned
>   between replicas upon server unavailability or to server-initiated
>                                                    ^ planned
>   migration events, are best dealt with together.  This is so even
>   though, for the server, pragmatic considerations will normally force
>   different implementation strategies for planned and unplanned
>   transitions.  


That section has been removed.


> 
> In this text:
> 
> 9.14.  Migration, Replication and State
> 
>   If a server replica or a server immigrating a filesystem agrees to,
>   or is expected to, accept opaque values from the client that
>   originated from another server, then it is a wise implementation
>   practice for the servers to encode the "opaque" values in network
>   byte order.  This way, servers acting as replicas or immigrating
>   filesystems will be able to parse values like stateids, directory
>   cookies, filehandles, etc. even if their native byte order is
>   different from other servers cooperating in the replication and
>   migration of the filesystem.
> 
> I'm having a difficult time connecting "it is a wise implementation
> practice" to "*will* be able to parse" - is it obvious to the replica
> server that the opaque values have been encoded in network byte order, so
> it's safe to parse values based on that? Or are these servers expected to
> be tightly coupled enough that each server knows whether other servers
> will encode opaque values "wisely"?

Moved to a SHOULD


> 
> In this text:
> 
> 10.2.1.  Delegation Recovery
> 
>   o  Upon reclaim, a client reporting resources assigned to it by an
>      earlier server instance must be granted those resources.
> 
> Is "reporting" the right word? This may be a term of art, but I'm not
> seeing other places in the specification where clients "report" resources
> to servers. 

Someone was probably trying to avoid "claiming". I had no such fear here!


> 
> Just a little further down:
> 
>   Situations in which there us a series of client and server restarts
> 
> I'm pretty sure this is "there is a series"

ok

> 
> In this text: 
> 
> 12.4.1.  Processing of Principal Strings
> 
>   The string should be scanned for at-signs.  If there is more that one
>   at-sign, the string is considered invalid. 
> 
> I'm pretty sure this is "more than one"

Rewritten section

> 
> In this text:
> 
> 15.18.3.  RESULT
> 
>    nfs_space_limit4
>              space_limit; /* Defines condition that
>                              the client must check to
>                              determine whether the
>                              file needs to be flushed
>                              to the server on close.  */
> 
> I'm no expert, but could I ask you to check whether this is the right
> description for this struct? nfs_space_limit4 looks like it's either a
> file size or a number of blocks, and I wasn't understanding how that was
> a "condition" or how the limit had anything to do with flushing a file to
> the server on close, so I'm wondering about a cut-and-paste error.

A 10 year old cut and paste.

Anyone on the WG alias care to comment?


> 
> In this text:
> 
> 15.33.5.  IMPLEMENTATION
> 
>   Note that a server MAY use the AUTH_NONE flavor to signify that the
>   client is allowed to attempt to use authentication flavors that are
>   not explicitly listed in the SECINFO results.  Instead of using a
>   listed flavor, the client might then, for instance opt to use an
>   otherwise unlisted RPCSEC_GSS mechanism instead of AUTH_NONE.  It may
>   wish to do so in order to meet an application requirement for data
>   integrity or privacy.  In choosing to use an unlisted flavor, the
>   client SHOULD always be prepared to handle a failure by falling back
>   to using AUTH_NONE or another listed flavor.  It MUST NOT assume that
>   identity mapping is supported, and should be prepared for the fact
>   that its identity is squashed.
>                        ^^^^^^^^
> 
> Is "squashed" a term of art in the NFS community? It's not defined, it
> only appears once in the document, and it wasn't obvious to me what
> reference I should be checking to understand the sentence.
> 

Yes, it is.


> In this text:
> 
> 18.1.  Named Attribute Definitions
> 
>   Under the NFSv4 specification, the name of a named attribute can in
>   theory be up to 2^32 - 1 bytes in length, but in practice NFSv4
>   clients and servers will be unable to a handle string that long.
> 
> Something is garbled here. I'm guessing "will be unable to handle a
> string that long", but that's guessing ...
> 
>