[nfsv4] AD review of draft-ietf-nfsv4-rfc5667bis-11

[Spencer Dawkins at IETF <[e-mail]>]

Hi, Chuck,

This is another one of the NFSv4 docs that is easy to review. Thanks for
that.

I have some comments, but most of them are about clarity, rather than about
technical concerns.

Please let me know what you think, after you've had an opportunity to look
these over.

Thanks,

Spencer (D)

--

Pretty much every reviewer is going to ask you to update the reference to
draft-ietf-nfsv4-rfc5666bis to point to RFC 8166. Might as well do that
sooner, rather than later, because when reviewers start typing, they rarely
stop ...

When I saw this text

   This document contains material required of Upper Layer Bindings, as
   specified in [I-D.ietf-nfsv4-rfc5666bis], for the following NFS
   protocol versions:

   o  NFS Version 2 [RFC1094]

   o  NFS Version 3 [RFC1813]

   o  NFS Version 4.0 [RFC7530]

   o  NFS Version 4.1 [RFC5661]

   o  NFS Version 4.2 [RFC7862]

   Upper Layer Bindings are also provided for auxiliary protocols used
   with NFS versions 2 and 3.

I found myself wondering "what auxiliary protocols?" When you define the
Upper Layer Bindings for them in Section 4, that's very clear. Could you
consider either moving some of the Section 4 text here or just adding a
forward reference here, pointing to Section 4?

I found this text

   (to avoid hitting in a Duplicate Reply Cache)

a trifle unclear - is this saying

   (to avoid receiving the same error again from a Duplicate Reply Cache)

or something else? But even s/hitting in/hitting/ would be clearer for me.

In this text,

   Unless
   explicitly mentioned as applicable, short Reply chunk retry should
   not be used.

I wondered if you're saying "this is less efficient", or "this will
probably fail". Or some other possibility. But perhaps being clear about
likely consequences of doing something you're recommending against, would
help people talk themselves into believing you.

This text

   A transport error does not give an indication of whether the server
   has processed the arguments of the RPC Call, or whether the server
   has accessed or modified client memory associated with that RPC.

appears in Section 3, which covers NFS 2 and NFS 3/legacy NFS. Is this also
true for more recent versions? Or is that not a meaningful concept in
non-legacy NFS?

Pardon my unfamiliarity, but in this text,

   Historically, NFS/RDMA implementations have chosen to convey the
   MOUNT, NLM, and NSM protocols via TCP.  To enable interoperation of
   these protocols when NFS/RDMA is in use, a legacy NFS server MUST
   provide TCP-based MOUNT, NLM, and NSM services.

are these auxiliary protocols a set (so, they would all be present), or are
they independent (so some might be present while others might not be)? If
they're a set, "MUST provide x, y, and z" is the right answer, but if
they're independent, "MUST provide support for these protocols via TCP"
would be more appropriate. (I only ask about this because this text is
inside a MUST)

I see that NFSACL isn't mentioned in the previous paragraph I asked about.
I see

   Legacy clients and servers that support the NFSACL RPC Program
   typically convey NFSACL procedures on the same connection as NFS RPC
   Programs.  This obviates the need for separate rpcbind queries to
   discover server support for this RPC Program.

in the following section, and I'm assuming that the question of transport
protocol support for NFSACL has already been answered because it's using
the same connection as some other protocol that you've already answered the
question about. Did I get that right?

In this text,

   Specifically, the requirements of
   [I-D.ietf-nfsv4-rfc5666bis] ensure that this choice does not
   introduce new vulnerabilities. ensure that this choice does not
   introduce new vulnerabilities.

I believe you, but the people who write security reviews might appreciate a
pointer to a specific section in [I-D.ietf-nfsv4-rfc5666bis] to bound their
search, if there's one you can point to.