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

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

*Review Structure*

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

This part consists of:

   - *General Comments*
   -
*Comments by Section (through Section 3) *

Later emails will contain the rest of the per-section comments.

*General Comments*

*Overall Evaluation*

I think this document is fairly far along in meeting the original
requirements, to replace RPC5667 and supplement it with a reasonable
treatment of NFSv4, not really dealt with in RFC5667.

However, in a number of respects, the current text has issues that need to
be addressed before going forward and I've made some suggestions as to how
those issues could be addressed.  These issues are:

   - Adaptation to the possiblility of multiple RPC_over-RDMA versions,
   whether that is done as outlined in *draft-cel-nfsv4-rpcrdma-version-two* or
   not.  This issue is discussed in *Handling of RPC-over-RDMA Transport
   Versions.*
   - The need to address some fundamental issues regarding the ability to
   bound reply size.  I have suggested a way to do that in the proposed new
   section *2.x. Difficulties in Reply Size Estimation*

*Handling of RPC-over-RDMA Transport Versions*

Even though the I-D for Version Two has not yet been adopted as a working
group document and there may be some dispute as to the proper direction
such a new version might take, it is now clear that further development of
RPC-over-RDMA is likely to take place.  In light of the fact that we can no
longer assume Version One will be the only version, we have a number of
choices:

   - Make the current document the ULB for NFS applying to RDMA-over-RDMA
   Version One only or to Version One plus a very small Version Two.  Note
   that later changes to an extensible transport version might make a lot of
   the current statements about how DDP will be done, inappropriate. For
   example, a version two based on *draft-cel-nfsv4-rpcrdma-version-two* might
   not require much change but the later addition of features for message
   continuation and  send-based DDP, as set out in
   *draft-noveck-nfsv4-rpcrma-rtrext* would essentially force a rewriting
   of the document, even if those feature were OPTIONAL.
   - Cut back the scope of the current to eliminate most of the explanatory
   material tied to Version One of the transport and leave it oly containg the
   mateial that rfc5666bis requires for ULBs.
   - Restructure the document to distinguish material required by rfc566bis
   and applicable to all transport versions from explanatory/illustrative
    material tied to Version One.

In my revision suggestions, I have adopted the last of these approaches.

*Comments by Section (through Section 3)*

*Abstract*

In the first sentence, suggest replacing " to RPC-over-RDMA transports" by "to
the RPC-over-RDMA transport"

Also suggest a parallel change in the second sentence.

*1.  Introduction*

In the first sentence of the second paragraph, suggest the following
changes:

   - Replacing "facilitate" by "allow successful".
   -

   Replacing "must agree in advance on what " by "must be in agreement
as to which".

   -

   Replacing "in which" by "sent or received in specific".

I suggest adding the following new closing paragraph:

This Upper Layer Binding information is applicable to all versions of the
RPC-over-RDMA transport. However, text describng how DDP-eligibility (for
example) affects actual transport operation is written assuming the use of
RPC-over-RDMA Version One.

*2. Conveying NFS Operations On RPC-Over-RDMA Transports*

Suggest replacing the title by "Conveying NFS Operations On the
RPC-Over-RDMA Version One Transport"

There are a number of issues with the text of this section:

   - The use of the singular "protocol" is a problem, since there are
   multiple NFS protocols. Unfortunately simply switching to the plural would
   not work since you would be referring to "specifics of those protocols
   which is exactly what you don't want to do here.
   - There is a need to be clearer about where RPC-over-RDMA in general is
   being referred to and where Version One is being referred to.
   - "principals" is wrong.

Suggest the following possible replacement for this section:

Definitions of terminology and a general discussion of how
RPC-over-RDMA Version One provides Direct Data Placement and uses it
when conveying RPC transactions can be found in
[I-D.ietf-nfsv4-rfc5666bis]. In this section, these general principles
are applied to issues that arise in the context of the NFS protocols.

*2.1. Use of the Read List*

In the first sentence of the first paragraph, suggest replacing
"RPC-over-RDMA" by "RPC-over-RDMA Version One".

*2.2. Use of the Write List*

In the first sentence of the first paragraph, suggest replacing
"RPC-over-RDMA" by "RPC-over-RDMA Version One".

*2.2.1.  Empty Write Chunks*

Suggest replacing the first sentence of the second paragraph by the following:

Empty write chunks are useful in situations in which an NFS client
wants to have an NFS server return a DDP-eligible result inline.

In the second sentence of the second paragraph, suggest replacing
"simply specfies" by "can simply specify".

Suggest replacing the first sentence of the last paragraph by the following:

In some situation, an NFS server will generate an XDR union result
defined such that there are arms that contain a DDP-eligible result,
as well as other arms that do not.

*2.3.  Use Of Long Calls A**nd Replies*

Suggest changing "And" to "and" in the title.

In the second sentence of the first paragraph, suggest replacing
"using" by "as" or "in the form of".

In the first sentence of the second paragraph, suggest replacing
"predicts" by "determines".

*2.4.  Scatter-Gather Considerations*

In the first sentence, suggest replacing "comprises" by "corresponds to".

In the last sentence, suggest replacing "in which to send or receive"
by "into which to send or from which to receive"

*2.x.  Difficulties in Reply Size Estimation*

For a number of protocols discussed in this document, there are
procedures for which it is nor possible to determine, on the basis XDR
definition, the maxiumum reply size.  These protocols are:

   - NFSACL, discussed in Section 3.1.
   - NFSV4.0, discussed in Section 4.2 [may wind up being numbered differently

For these procedures, it not clear now we can conform to the
requirement, stated in section 7.2 of rfc5666bis, to provide a
dependable means of determining the maximum reply size.  One could
arbitrarily pick a sufficiently large value, such as a gigabyte and be
assured that no larger reply size will ever be encountered.  However,
such an approach would case enormous reply chunks to be allocated
unnecessarily, leading to the adoption of a more flexible approach in
which the maximum is determined during protocol operation.

In the approach used for these procedures, replies for which no a
priori determination of reply size bound is possible, initially use a
reply chunk size determined by the requester implementation, but
subject to adjustment in the light of experience.  When a particular
reply chunk size is inadequate, as reflect in an trasport-reported
error, that size is adjusted upward (e.g. doubled), and the operation
retried.  As a result, the maximum, at any given time is aequate to
accommoate all replies actually encountered and no more than twice the
size of the reply chunk size actually needed.

Although all of the operations with such uncertain rely sizes are
idempotent, the use of COMPOUND operations (e.g. in NFSv4.0) raises
the possibility of non-idempotent requests that combine a
non-idempotent operation with an operation whose reply size is
uncertain, causing difficulties with the use of retry.  In this
onnection, it is important to note that many operations normally
thought of as non-idempotent (e.g WRITE, SETATTR) are actually
idempotent and that truly non-idempotent are quite unusual in
COMPOUNDs that include operations with uncertain reply sizes.  For
further detail, see  *4.x.y.  Reply Size Estimation for NFSv4.0*

*3.  NFS Versions 2 And 3 Upper Layer Binding*

Suggest replacing title by "Upper Layer Binding for NFS Versions 2 and 3".

I'm suggesting a re-organization of this section for a number of reasons:

   - rfc5666bis makes the definition of which XDR item are
DDP-eligible, the main job of the ULB.  While it is possible to infer
that from the existing text, I feel it is better to derive what the
client and server should do, given explicit eligibility
specifications, and the rules in rfc5666bis, rather than having
multiple, possibly conflicting descriptions.
   - It makes it easier to distinguish the part of the ULB which
applies to all RPC-over-RDMA Versions from the discussion of the
consequences for Version One.
   - There appear to be a number of instances in which the
descriptions of what implementations are to do contradict rfc5666bis.

Suggest replacing the first two paragraphs by a new subsection 3.1,
entitled *DDP-Eligibility*,  as follows:

Within NFS Version 2 or version 3 requests, the only DDP-eligible XDR items are:

   - The opaque file data argument for an NFS WRITE procedure.
   - The pathname argument for an NFS SYMLINK procedure.

Within the responses for these NFS versions, the DDP-eligible XDR
items in responses are:

   - The opaque data result for an NFS READ procedure.
   - The pathname result for an NFS READLINK procedure.

For these NFS versions, no other DDP-eligible arguments or results exist.

At this point, I am suggesting that we start a new subsection 3.2, to be
entitled, *Handling in RPC-over-RDMA Version One.*

Some of the specifics will depend on how we address the mismatches between
this document and rfc5666bis noted below:

   - Regarding read chunks that do not correspond to a DDP-eligible data
   item, rfc5666bis treats these as DDP-eligibility violations, rather than
   REQUIRING the server to ignore them.
   - Regarding unused write chunks, rfc5666 bis requires that the server
   return an indication of their non-use, rather than ignoring them.

Propose adding the following new material before the current third
paragraph of *3. NFS Versions 2 And 3 Upper Layer Binding*:

When there are chunks that cannot be properly associated with a
DDP-eligible data item, according to the rules in [rfc566bis], that
document defines the appropriate handling:

   - Read chunks result in DDP-eligibility violations.
   - Write chunks must be returned returned to the client as empty in order
   to clearly indicate that the data in question was not used.

Suggest replacing the last sentence of the existing fourth paragraph by the
following:

However, when a client using  NFS version 2 or 3 sends a long Call or
prepares for the reception of a long Reply combination of multiple
chunk types can arise:

   - A position-zero read chunk can be combined with a non-empty write list.
   - A reply chunk can be combined with a non-empty read list, which
may include position zero read chunks or directly placed data items.

The first sentence of the last paragraph is confusing in that it isn't
clear what sort of of clients are "successfully" estimating the
maximum reply size.

   - They can't be clients using RPC-over-RDMA since you can't base
this standard on supposed implementations of this very standard. .
   - If, on the other hand, they are TCP clients, it isn't clear how
their success in making this determination can be determined.

Suggest replacing the last paragraph by the following:

The structure of these NFS versions is such that clients can determine
the maximum reply size for each operation. A client provides a reply
chunk when the maximum reply size, exclusive of any directly placed
data represented by write chunks is larger than the client's inline
threshold.

---

From: Chuck Lever <[email-masked]>
Date: Wed, 21 Dec 2016 11:50:05 -0500

Hi Dave-

Thanks for the review. Responses to your General Comments are below.

> On Dec 19, 2016, at 5:56 PM, David Noveck <[email-masked]> wrote:
> 
> Review Structure
> 
> This is the first part of a multi-part review.  It is split into multiple emails to avoid running into mailing list size limits.
> 
> This part consists of:
> 	• General Comments
> 	• Comments by Section (through Section 3)
> General Comments
> Overall Evaluation
> I think this document is fairly far along in meeting the original requirements, to replace RPC5667 and supplement it with a reasonable treatment of NFSv4, not really dealt with in RFC5667.
> 
> However, in a number of respects, the current text has issues that need to be addressed before going forward and I've made some suggestions as to how those issues could be addressed.  These issues are:
> 	• Adaptation to the possiblility of multiple RPC_over-RDMA versions, whether that is done as outlined in draft-cel-nfsv4-rpcrdma-version-two or not.  This issue is discussed in Handling of RPC-over-RDMA Transport Versions.
> 	• The need to address some fundamental issues regarding the ability to bound reply size.  I have suggested a way to do that in the proposed new section 2.x.  Difficulties in Reply Size Estimation

I predict some resistance to section titles in a specification that include
the word "Difficulties".

The point has been made, however, that without proper Reply Size Estimation,
successful interoperation is at risk. This is why we now call out Reply Size
Estimation in RFC 5666bis as a necessary part of an Upper Layer Binding.

I agree that there are plenty of details that need to be sorted here. Your
comments in this area are most helpful, and I plan to integrate many of them
into this document. I hope we can resolve most RSE issues so that the use of
the word "Difficulties" will be rare or altogether unnecessary.

> Handling of RPC-over-RDMA Transport Versions
> 
> Even though the I-D for Version Two has not yet been adopted as a working group document and there may be some dispute as to the proper direction such a new version might take, it is now clear that further development of RPC-over-RDMA is likely to take place.  In light of the fact that we can no longer assume Version One will be the only version, we have a number of choices:
> 	• Make the current document the ULB for NFS applying to RDMA-over-RDMA Version One only or to Version One plus a very small Version Two.  Note that later changes to an extensible transport version might make a lot of the current statements about how DDP will be done, inappropriate. For example, a version two based on draft-cel-nfsv4-rpcrdma-version-two might not require much change but the later addition of features for message continuation and  send-based DDP, as set out in draft-noveck-nfsv4-rpcrma-rtrext would essentially force a rewriting of the document, even if those feature were OPTIONAL.
> 	• Cut back the scope of the current to eliminate most of the explanatory material tied to Version One of the transport and leave it oly containg the mateial that rfc5666bis requires for ULBs.
> 	• Restructure the document to distinguish material required by rfc566bis and applicable to all transport versions from explanatory/illustrative  material tied to Version One.

I prefer to keep the NFS ULB document as agnostic to transport protocol
version as possible. That would increase its useful lifespan, rather 
than requiring a fresh ULB later to handle subsequent versions of
RPC-over-RDMA.

I assume you object to the use of terminology that mentions a particular
RDMA mechanism (eg. RDMA Read, or Read chunk), since that is incompatible
with potential extensions to RPC-over-RDMA which might use some other
mechanism (eg. structured Send buffers).

On considering replacing that language with abstract terminology such as
"DDP-eligible", that could make the document more difficult for implementers
to interpret. It would certainly bear little resemblance to the original RFC
5667.

Likely all of Section 2 would need to be rewritten or removed. I believe
removal of this section would be a disservice to implementers.

IMO it's treading a fine line to consider non-RDMA Direct Data Placement
mechanisms in a transport protocol called "RPC-over-RDMA".

Be that as it may, I will see what can be done to address your concerns in
this document. Because I'm not taking "the last of these approaches" (see
above), at least for now, I will probably not merge many of your proposed
changes in this area, but will use the body of your remarks as a guide to
rework the text to be agnostic to data placement mechanism.

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Wed, 21 Dec 2016 13:16:08 -0500

>I predict some resistance to section titles in a specification that include

I see your point.  How about "Issues with"?

> version as possible.

That's in line with the requirements for ULBs presented in rfc5666bis.

> That would increase its useful lifespan, rather

Which, I presume, is why you wrote the ULB requirements in rfc5666bis
the way you did.

> mechanism (eg. structured Send buffers).

That's the specific issue that I'm most concerned about, but I think you
have summarized the general issue above.  We want to make this
a ULB document that applies to RDMA-over-RPC in general and not iie
it to the specifics of Version One.

Another area of concern is that much of the current text implicitly assumes
that
message continuation is not (and will never be) available.  That also is
Version-One-specific

> to interpret.

I don't think it will.  Consider what happened in rfc5666bis where that
terminology
was introduced.  While RFC5666 mentioned only the specifics of the
implementation,
rfc566bis introduced the abstract concept (i.e DDP-eligibility) and was
able to describe
how, that abstract concept was implemented.  I found the result easier to
interpret because
it presented both the abstraction and the implementation.

> It would certainly bear little resemblance to the original RFC 5667.

I think that is overstating it.  In the end I expect the difference between
RFC5667 and
rfc5667bis to be about the same as between RFC5666 and rfc5666bis.

> removal of this section would be a disservice to implementers.

I don't believe that either of those should be done and I din't propose
major
changes to Section 2.  Instead I think that this material which
Version-One-Specific
be identified as Version-One-Specific.  I think it can serve as a helpful
bridge,
supplementing rfc566bis between the level of abstract concepts and that of
Chunks
and explicit RDMA operations.

> mechanisms in a transport protocol called "RPC-over-RDMA"

I don't think the phrase "non-RDMA Direct Data Placement mechanisms" is
correct.  If we adopt the terminology of rfc5666bis, the use of SEND-based
DDP
(using structured receive buffers) does use RDMA since RDMA SEND is defined
as an RDMA operation.  Such operations as RDMA READ and RDMA WRITE are
referred to as "explicit RDMA operations".

In my view, if this is a line we cannot tread (or consider approaching), we
are forced
into a cramped subset of the possible RDMA operations.  RDMA-over-RPC
should
be free to use the most efficient operation to do what it needs to do,
including to implement
DDP.  Using explicit RDMA operations is one possible implementation but it
isn't a goal.

> Because I'm not taking "the last of these approaches" (see
> above),

This third approach was:

Restructure the document to distinguish material required by rfc566bis and
applicable to all transport versions from explanatory/illustrative
material tied to Version One.

I'm not clear why you object to this and what you propose to do instead.
You are very clear why you don't want to adopt either of the first two
approaches listed above.

> at least for now, I will probably not merge many of your proposed

OK, but your point above is that this might result in a document that is
unclear to implementers, so I think it will need to supplemented by
material which is not transport-version-agnostic.  If you have such
material, it needs to be clearly labeled as transport-version specific.
.

On Wed, Dec 21, 2016 at 11:50 AM, Chuck Lever <[email-masked]>
wrote:

> > This is the first part of a multi-part review.  It is split into
> multiple emails to avoid running into mailing list size limits.
> >
> > This part consists of:
> >       • General Comments
> > However, in a number of respects, the current text has issues that need
> to be addressed before going forward and I've made some suggestions as to
> how those issues could be addressed.  These issues are:
> >       • Adaptation to the possiblility of multiple RPC_over-RDMA
> versions, whether that is done as outlined in draft-cel-nfsv4-rpcrdma-version-two
> or not.  This issue is discussed in Handling of RPC-over-RDMA Transport
> Versions.
> >       • The need to address some fundamental issues regarding the
> ability to bound reply size.  I have suggested a way to do that in the
> proposed new section 2.x.  Difficulties in Reply Size Estimation
>
> > Even though the I-D for Version Two has not yet been adopted as a
> working group document and there may be some dispute as to the proper
> direction such a new version might take, it is now clear that further
> development of RPC-over-RDMA is likely to take place.  In light of the fact
> that we can no longer assume Version One will be the only version, we have
> a number of choices:
> >       • Make the current document the ULB for NFS applying to
> RDMA-over-RDMA Version One only or to Version One plus a very small Version
> Two.  Note that later changes to an extensible transport version might make
> a lot of the current statements about how DDP will be done, inappropriate.
> For example, a version two based on draft-cel-nfsv4-rpcrdma-version-two
> might not require much change but the later addition of features for
> message continuation and  send-based DDP, as set out in
> draft-noveck-nfsv4-rpcrma-rtrext would essentially force a rewriting of
> the document, even if those feature were OPTIONAL.
> >       • Cut back the scope of the current to eliminate most of the
> explanatory material tied to Version One of the transport and leave it oly
> containg the mateial that rfc5666bis requires for ULBs.
> >       • Restructure the document to distinguish material required by
> rfc566bis and applicable to all transport versions from
> explanatory/illustrative  material tied to Version One.

---

From: Chuck Lever <[email-masked]>
Date: Wed, 21 Dec 2016 13:43:54 -0500

> On Dec 21, 2016, at 1:16 PM, David Noveck <[email-masked]> wrote:
> 
> Restructure the document to distinguish material required by rfc566bis and applicable to all transport versions from explanatory/illustrative  material tied to Version One.
> 
> I'm not clear why you object to this and what you propose to do instead.  You are very clear why you don't want to adopt either of the first two approaches listed above.

I plan to replace data placement mechanism-specific language that you
objected to (but did not replace in your proposed changes) with the generic
terminology we introduce in rfc5666bis.

That's probably closest to your "approach two" but with as little redaction
as I can get away with.

It's not the approach you took in your proposed changes. So as I said, I
will use your proposals as guidance, but probably not merge them wholesale.

I stated some risks to this approach. I'm willing to try it anyway. It's
only spilled electrons, trapped in a change control system, that can be
reverted if we end up not liking them.

> OK, but your point above is that this might result in a document that is unclear to implementers, so I think it will need to supplemented by material which is not transport-version-agnostic.  If you have such material, it needs to be clearly labeled as transport-version specific.

Agreed. In any event, I will produce a revision soon, and you can see if
I've corrected the issues you identified.

--
Chuck Lever