Subject: [nfsv4] Review of draft-ietf-nfsv4-rfc5667bis-03 (part three of three)

From: David Noveck <[email-masked]>
Date: Mon, 19 Dec 2016 17:57:36 -0500

*Review Structure*

This is the final part of a multi-part review.  It is split into multiple
emails to avoid running into mailing list size limits.

This part has per-section comments starting at Section 4.2 and going
through to the end of the document.

Note that all general comments are in the first email.

*Comments by Section (from Section 4.2)*

*4.2.  Reply Size Estimation*

This section is likely to wind up numbered 4.3.

I don't see he point of the second sentence of the first paragraph:

   - TCP clients do not need to prepare buffers to accommodate replies.
   - There is no real significance in dealing with most operations
successfully as long as there are some you do not successfully deal
with.

I have a number of issues with the bulleted list that appears in this section:

   - Regarding the introductory phrase, "These include but are not
limited to", that makes things too vague.  If there are other cases,
we have to know about them.  For now I'm going to assume that this is
the complete list.
   - I'm unclear of the significance of the reputed opacity of these
items, which are not truly opaque anyway.

*4.x.y.  Reply Size Estimation for NFSv4.0*

Propose ddeleting the lat paragraph of the current Section 4.2 andd
replacing it with tis new section, as follows:

With regard to NFS version 4.0, the items discussed in Section  4.2
[numbering may change] make it impossible to determine an a priori
limit on the reply size of GEATTR operations interrogating certain
attributes.  While it is possible for client implementations to rely
on their own architectural limits to bound the reply size, such limits
are not guaranteed to be reliable.   Because of this, the techniques
discussed in Section 2.x to make sure that a GETATTR  with an
appropriately sized reply chunk can be done, when an anomalously large
reply is encountered.

As a result, the reply size for most operations is determinable using
the XDR for the specific operations within the COMPOUND issued.  In
the case of operations such as READ and READDIR, it can be assume that
variable length items in the reply will be no larger than the size
limit presented in the request.  In the case of a response to GETATTR,
where only attributes of fixed size are requested, it may be assume
that the response size is no larger than then size of the set of
attributes requested.

This leaves the following operations as those for which no a priori
size limits can be determined:

   - READLINK
   - GETATTR where the set of attributes requested include acl, owner,
group, fs_locations.

Compound requests that include any of the operations for which reply size
limits cannot be determined are to be treated as iscussed in Section 2.x.
Specfically:

   - For COMPOUNDs including such operations, a reply size limit of at
   least 4K SHOULD be initially used. When the inline buffer size is smaller
   than this size, a reply chunk MUST be provided.

   - When a transport error indicates that this reply size was inadequate,
   the COMPOUND MUST be retried with a larger reply chunk. This larger reply
   chunk SHOULD be at least twice the size previously used.

   - If a retry is needed, the requester SHOULD issue future requests for
   such COMPOUNDs, providing a reply chunk at least as large as the largest
   one already used.

Since the presence of COMPOUNDs that contain non-idempotent operations
(.g. RENAME, REMOVE, CREATE) interferes with the ability to retry, it
is best to avoids COMPOUNDs that contain such operations together with
operations for which reply limits are uncertain. Clients SHOULD avoid
such COMPOUNDs by issuing multiple COMPOUNDs to replace COMPOUNDs
containing this combination.  In this connection, it is important to
note that many operation currently thought of as non-idempotent (e.g.
SETATTR, WRITE) are in fact idempotent and are not affected by this
restriction.

*4.2.1.  Managing READ_PLUS Replies*

I don't see this section as related to *Reply Size Estimation*, so it
should probably wind up numbered 4.4

*4.3.  NFS Version 4 COMPOUND Requests*

I think this section will wind up numbered 4.5.

In the last sentence of the first paragraph, suggest replacing "as
long as it observes the restrictions in Section 4.1" by "as long as
chunks are only used to represent DDP-eligible data items, as
described in section 4.1."

With regard to the third paragraph, it is unclear what is being
refereed to by the phrase "with some additional restrictions.".  If
such restrictions exist, they need to be called out explicitly.

*4.4.1.  NFS Version 4.0 Callback*

I don't see the point of not providing a ULB, given that one would be
so trivial (i.e. No DDP-eligible items, smalll messages).  I think the
text seems to be focused on the fact that there is no big demand for
this as yet, but it seems to be that given that this is so easy to do,
you might as well just indicate how it would be done and not force a
new document to be written when and if this is needed.

*4.4.2.  NFS Version 4.1 Callback*

In the first sentence of the third paragraph, suggest deleting "or Reply".

Suggest replacing the third sentence of the third paragraph by the following:

When the NFS version 4 client reports a backchannel ca_maxrequestsize
that is larger than the connection's inline threshold, the NFS version
4.1 client can support Long Call messages (by using a Position-zero
Read Chunk). Otherwise an NFS version 4.1 server MUST use Short
messages to convey backchannel operations.

* 5.  Extending NFS Upper Layer Bindings*

Suggest rewriting the last sentence as follows:

This includes extensions to a given NFSv4 minor version whose
definition is published separately, as described in [nfsv4-vers].

*8.1.  Normative References*

This needs the following updates:

   - *I-D.ietf-nfsv4-minorversion2* needs to be updated to point to RFC7862
   - *I-D.ietf-nfsv4-rfc5666bis* needs to be updated to -08.

*8.2.  Informative References*

Suggest you add a reference to *draft-ietf-nfsv4-versioning-09*.