Subject: Re: [nfsv4] Review of draft-ietf-nfsv4-rfc5666bis-03

From: Chuck Lever <[email-masked]>
Date: Mon, 1 Feb 2016 15:25:20 -0500

> On Feb 1, 2016, at 2:14 PM, David Noveck <[email-masked]> wrote:
> 
> General Comments
> 
> Overall Evaluation
> 
> I basically agree with Chuck that, that except for extensibility-related issues, the document is pretty far along, and only needs wordsmithing.  This review will not concern itself with that wordsmithing.  I anticipate going through the document in its entirety, as part of WGLC, which I don't think is very far off.
> 
> The issues with extensibility will be discussed below but they are not limited to section 9.  I will deal with wordsmithing issues in the sections I do deal with,

This is thorough but extensive. I largely agree with the
changes you've proposed, especially the suggestions to add
an XDR extraction script, and to break up the rpcrdma1_body
union. It will take me some time to review everything and
integrate the individual changes.

One remark: I don't believe the RFC XML DTD allows section
numbering such as "9.a.1". If you know how to do this, do
clue me in. Otherwise, I will adjust these to accommodate
what is possible with XML.

> Issues with Extensibility
> 
> One important issue with extensibility concerns the ability to extend the XDR without forcing each extension document to publish a complete updated description that updates RFC5666bis.  To avoid the need for that, I've proposed some restructuring of the XDR description while maintaining over-the-wire compatibility with RFC5666.
> 
> Within Section 9 itself, the major issue is to accommodate the current Version One error reporting model.  I've proposed dealing with that be explaining the issues in more abstract terms for the general case and providing a new sub-section (now 9.b) to deal with the special limited case of extension for version One.
> 
> Review Format
> 
> I use gmail formatting facilities extensively in this review, including use of multiple fonts, font sizes, and faces.  To accommodate those for whom this might be a problem, I've attached a PDF of the review.  Note that if people want to offer comments in the form of Adobe Reader XI annotations applied to the review PDF, I'm OK with that. 
> 
> Comments by Section (Regarding Extensibility)
> 
> 1.a.  Code Components Licensing Notice
> 
> The text for this proposed new section (following 1.1) is as follows:
> 
> The XDR description and scripts for extracting the XDR description are Code Components as described in Section 4 of "Legal Provisions Relating to IETF Documents" [LEGAL].
> 2.  Changes Since RFC 5666
> A reorganization of this section into three (from two) subsections is proposed,
> 
> 
> 2.1.  Changes To The Specification
> 
> Suggest deleting the fourth bullet as these changes are best handled separately, in the new Section 2.a.
> 2.a.  Changes to the XDR Definition
> For this new section, which follows the existing 2.1, the following text is proposed:
> 
> This section discusses changes to the XDR definition which do not involve any change to the over-the-wire message format relative to that described in [RFC5666].  Changes  made to the XDR which do change the over-the-wire message format (i.e., to make it match actual interoperating implementations) are discussed in section 2.3.
> These changes have been made to make it easier to extend the protocol, to better organize the definition, and to make names in the description more consonant with actual protocol function.  The specific changes are:
> 	• The XDR description has been given an extraction script using a sentinel sequence to match the approach used in [RFC5662] and [RFC7531].
> 	• Constructs which need to be the same in all RPC/RDMA versions are put in their own section of the XDR and given names that reflect the fact that are not version-specific.
> 	• In order to allow extensions to be added without modification to the existing XDR, the header types previously members of the enum rpcrdma1_proc have been defined as constants.
> 	• RDMA_ERR_CHUNK has been renamed as RDMA_ERR_BAD.
> 	• In order to clarify internal layering and to enable protocol extension, the union rpcrdma1_body was deleted.  For details of the consequent reorganization, See Section 5.1.4.
> 5.1.  XDR Protocol Definition
> Suggest adding the following new material:
> This document contains the XDR [RFC4506] description of the core features of the RPC-over-RDMA Version One protocol.  
> The XDR description is provided in this document in a way that makes it simple for the reader to extract into ready-to-compile form.  The reader can apply the following shell script to this document to  produce a machine-readable XDR description of the RPC-over-RDMA protocol described in this document, i.e., RPC-over-RDMA Version One without any OPTIONAL extensions.
> <CODE BEGINS>
> #!/bin/sh
> grep '^ *///' | sed 's?^ */// ??' | sed 's?^ *///$??'
> <CODE ENDS>
> That is, if the above script is stored in a file called "extract.sh", and this document is in a file called "spec.txt", then the reader can do the following to extract an XDR description file.
> <CODE BEGINS>
> sh extract.sh < spec.txt > rpcrdma_corev1.x
> <CODE ENDS>
> As described in Section 9.b, extensions to Version One, published as Proposed Standards, will have similar means of providing an XDR description appropriate to those extensions.  Once  this is done for all extensions to be supported, the XDR for the extensions can be appended to the XDR description file extracted from this document to produce a consolidated XDR description file reflecting all extensions selected. 
> Note that, because RPC/RDMA is not an RPC program, and to enable protocol extension, there is no XDR single entity which describes the format of the messages exchanged within the protocol.  Instead, implementers need to follow the instructions in Section 5.1.4 to appropriately encode and decode protocol messages. 
> 5.1.1 Code Component License
> It is suggested that the existing first paragraph of Section 5.1 be replaced by the following:
> Code components extracted from this document must include the following license text.  Note that in case in which the extracted XDR code is combined with other complementary XDR code which itself has an identical license, only a single copy of the license needs to be preserved.
> After this, the section will continue with the existing XDR Code license comment.  However:
> 	• Each line of the comment will have the sentinel sequence ("///") prepended.
> 	• After the end of the comment, the section will end.
> 	• There will be a "<CODE ENDS>" just before the end of the section.
> 5.1.2.  XDR Applying to All Versions of RPC-over-RDMA
> 
> The following introductory paragraph is proposed:
> 
> This section contains XDR describing parts of the RPC-over-RDMA protocol which are not subject to change in future versions of the protocol.  For a full discussion of the versioning/extensibility model, see Section 9 
> 
> After that, the following XDR section would appear:
> 
> <CODE BEGINS>
> ///typedef uint32 rdma_htype;
> ///
> ///struct rpcrdma_prefix {
> ///     uint32        rdma_xid;
> ///     uint32        rdma_vers;
> ///     uint32        rdma_credit;
> ///     rpcrdma_htype rdma_which;
> ///};
> ////*
> /// * The following header types are currently defined and may be added to.
> /// */
> ///const RDMA_MSG = 0;
> ///const RDMA_NOMSG = 1;
> ///const RDMA_MSGP = 2;  /* Currently unused and reserved. */
> ///const RDMA_DONE = 3;  /* Currently unused and reserved. */
> ///const RDMA_ERROR = 4;
> ///
> ///struct rpcrdma_vers {
> ///     uint32 rdma_vers_low;
> ///     uint32 rdma_vers_high;
> ///};
> ///<CODE ENDS>
> 
> 5.1.2.   XDR Specifc to Version One of RPC-over-RDMA
> The following introductory paragraph is proposed:
> This section contains XDR that is subject to change in subsequent RPC-over-RDMA versions.  There are a few things to be noted:
> 	• Even though many of the structure and union names begin "rpcrdma1_", these are not necessarily restricted to use in Version One.  Instead structures will normally be carried over unchanged to subsequent versions while unions are subject to extension according to the rules for compatible XDR extension as discussed in Section 9. 
> 	• There are some specific items that cannot be changed in subsequent versions.  Comments will identify these.
> Except as noted below, the rest of this section will consist of the XDR within Section 5.1 of -03, with the license comment excluded.  However:
> 	• There will be a "<CODE BEGINS>" in front of it.
> 	• Each line will have the sentinel sequence ("///") prepended.
> 	• The existing "<CODE ENDS>" will remain.
> The following changes to the existing XDR are proposed:
> 	• Deleting the definitions of struct rpcrdma1_header and enum rpcrma1_proc.
> 	• Adding a comment to the definition of RDMA_ERR_VERS indicating that this value must never be changed.
> 	• Adding a comment to the case for RDMA_ERR_VERS in union rpcrdma1_error indicating that the contents of this union arm must never be changed.
> 	• Changing RDMA_ERR_CHUNK to RDMA_ERR_BAD.
> 	• Replacing the two lines now defining the RDMA_ERR_VERS case of union rpcrdma1_error by "struct rpcrdma_vers rdma_err_vers;"
> 	• Deleting the definition of union rpcrdma1_body.
> 5.1.4.  Use of XDR Descriptions
> Suggest the following section text:
> Because RPC-over-RDMA, while described by XDR, is not an RPC program, several functions normally provided by the RPC layer need to be addressed by the RPC-over-RDMA definition itself. In particular, the following functions, normally provided by RPC, need to be provided for as part of the RPC-over-RDMA XDR description:
> 	• selection/negotiation of protocol versions
> 	• selection of the particular message bodies to go with specific header types
> In [RFC5666], the XDR description did not take account of the natural layering between that part of RPC-over-RDMA functionality that performed the RPC-layer-like functions described above and that which implemented individual  transport functions.  As a result:
> 	• the four 32-bit words which must be the same in all versions of RPC-over-RDMA are split, with three of those words in struct rprdma1_header and the remaining word part of union rpcrdma1_body, together with each of the message bodies.
> 	• It is impossible, within the resulting structure, to add a new message type, without modifying the existing XDR description.
> The XDR description used in this document reorganizes the XDR in line with this natural layering, which maintaining over-the-wire equivalence.  As a result of this reorganization, the 32-bit big-endian field starting twelve bytes into header is no longer the discriminator field of union rpcrdma1_body.  Instead, it is the last 32-bit word within struct rpcrdma_heder which defines the common (i.e., for all RPC-over-RDMA versions) header prefix.  it retains its role of indicating the header type and deciding which particular form of header body is to follow.
> As a result of this restructuring, there is no longer a single XDR item that encompasses the entire RPC-over-RDMA  header. Instead, each RPC-over-RDMA message consists of up to three items and those using XDR encode an decode must be aware that they proceed in sequence as follows:
> 	• A struct rpcrdma_prefix  
> 	• Depending on the rdma_which field of the rpc_rdma, the appropriate header body for that heaer type as given by Table 1.
>     In cases in which there is an undefined header type, this is to be treated as an XDR decode/encode error.
> 	• If allowed for that header type as defined in Table 1, an XDR stream for the RPC message being transported
> ------------------------------------------------|
> |   Type      |         Body            |  Msg  |
> -------------------------------------------------
> | RDMA_MSG    | struct rpcrdma1_chunks  |  Yes  |
> |-------------|-------------------------|-------|
> | RDMA_NOMSG  | struct rpcrdma1_chunks  |  No   |
> |-------------|-------------------------|-------|
> | RDMA_ERROR  | union rpcrdma1_error    |  No   |
> -------------------------------------------------
>        Table 1.  Header Type Characteristics
> 
> 5.5.2.  XDR Errors
> In the last sentence of the first paragraph, suggest replacing "RDMA_ERR_CHUNK" by "RDMA_ERR_BAD".
> 5.5.3.  Responder Operational Errors
> In the last sentence of the last paragraph, suggest replacing "RDMA_ERR_CHUNK" by "RDMA_ERR_BAD".
> 9.  Protocol Extensibility
> 
> I propose changing the title of this section as indicated above.
> 
> With regard to the first paragraph, I suggest:
> 	• Replacing the second sentence by the two following sentences:
> This framework allows minor issues with the transport protocol to be addressed and optional features to be introduced, by making compatible extensions to the protocol XDR as described in Section 9.1.  Such changes can be made without a change to the protocol version number.
> 	• In the current third sentence, replacing "needed" by "are to be made"
> 	• Replacing the current fourth sentence by the following:
> In either case, any changes to the RPC-over-RDMA protocol can only be effected by publication of a Standards-track document, with appropriate review by the  Working Group discussion and the IESG.
> Suggest adding the following new paragraph after the first paragraph:
> 
> While it possible to make XDR changes which are not limited to the use of compatible extensions in new RPC-over-RDMA versions, such changes should only be done when absolutely necessary, as they limit interoperability with existing implementations.  It is appropriate for the Working Group to consider alternatives carefully before proceeding using this approach. 
> 
> With regard to the current second paragraph, I suggest:
> 	• In the first sentence, replace "Section 9" by "Section 9 (except for Section 9.b)"
> 	• In the second sentence, replace "requires updating" by "requires publication of a standards-track document updating"
> 9.1.  Changes to RPC-over-RDMA Header XDR
> 
> I propose changing the title of this section as indicated above.  Note that my proposal changes the character of this section by moving much of the material now in it into a new Section 9.b.
> 
> With regard to the first paragraph, I suggest:
> 	• In the first sentence, after "header', add "(now in struct rpcrdma_prefix)"
> 	• In the second sentence, replace "for" by "to allow".
> 	• In the third sentence, after "union', add "(now in struct rpcrdma_vers)"
> I propose the following new material to close out the section:
> 
> Most changes to the RPC-over-RDMA XDR will take the form of a compatible extension to the existing XDR.  Changes which do not change the version number (see Section 9.a) must take this form.
> 
> For an XDR description B to be a compatible extension of an XDR description A, the following must be the case:
> 	• All input recognized as description valid by A must be recognized as valid by description B.
> 	• Any input recognized  as valid by both descriptions must be interpreted as having the same structure according to both descriptions.
> 	• Any input recognized as valid by description B but not by description A cab be recognize as part of an supported/unknown extension using description A.
> In the context of RPC-over-RDMA (in general), the following sorts of changes can be made compatibly:
> 	• Addition of a new header type and associated header body
> 	• Addition of new enum values and associated arms to unions which do not include a default case.
> 	• Addition of previously undefined flag bits to flag words that are including in existing heaer bodies.
> Each such addition is referred to as a "protocol element" while a set of protocol elements that are defined together and such that all must be supported together or not supported together by any particular receiver is called a "feature"
> 
> Note that because of the simplicity of the existing protocol and some deficiencies in the existing error reporting structure, some of the above techniques are not realizable within RPC-over-RDMA Version One.  For a discussion of protocol extension practices within Version One, including XDR extension, see section 9.b.
> 9.2.  Feature Statuses with RPC-over-RDMA Versions
> Within a given RPC-over-RDMA version, every known feature is either OPTIONAL, REQUIRED, or not allowed. 
> 	• REQUIRED features MUST be supported by all receivers and senders may depend on them being supported.
> 	• OPTIONAL features MAY be supported by particular receivers and senders need to be prepared for the absence of support.
> 	• not allowed features are typically those that were formerly OPTIONAL or  REQUIRED, but for which support has been removed,
> All features defined in this document are considered REQUIRED in RPC-over-RDMA Version One.  OPTIONAL features may be added to Version One as specified in section 9.b.
> The terms "OPTIONAL" and "REQUIRED" are used as specified in [RFC2119] as indicated in section 1.1.  In this context, it is important to note that these status values are assigned by those writing additional specifications (e.g., new RPC-over-RDMA versions, extensions) and that their choice in this regard is their guidance to implementers.  As used in this document, these terms are only directed to implementers as used with regard to RPC-over-RDMA Version One.
> The status of features may change between RPC-over-RDMA protocol versions.  For details, see Section 9.a. 
> 9.a.  RPC-over-RDMA Version Numbering
> I suggest adding the following introductory paragraph:
> Version numbers are used within RPC-over-RDMA to allow both parties to agree on a set of interoperable behaviors and create an environment in which support  for OPTIONAL features can be determined and agreed upon.
> I suggest replacing the existing first paragraph by the following material:
> An expected pattern of protocol development is to introduce OPTIONAL features within a given version using  XDR extension.  Such features often need a significant period of optional general use to ensure they are mature And capable of being implemented by a wide range of implementations.  This is especially true for infrastructural features that others will build upon.  When it is appropriate for OPTIONAL features to become REQUIRED, that would be an occasion to create a new RPC-over-RDMA protocol version.
> 9.a.1.  Incrementing The Version Number
> I am proposing that this be deleted as a separate sub-section and the existing material, as modified, be merged into Section 9.a.
> I will refer to existing paragraphs in terms of their position within the existing section 9.2.1.
> In the second sentence of the first paragraph, suggest replacing "Two" by "Some".
> In the first bullet, suggest replacing "or whenever a REQUIRED protocol element is removed" by "or whenever an existing OPTIONAL feature is made REQUIRED"
> Suggest adding the following to the first set of bullets:
> 	• Whenever a REQUIRED feature is made OPTIONAL
> 	• Whenever an existing feature is converted to be "not allowed".
> 	• Whenever an XDR change is made which is not a compatible extension as defined in Section 9.1.
> Suggest rewriting the rest of the section as follows:
> When a version number bump is to be made (e..g., because there is a change in the set of REQUIRED features), the Working Group cretae a Standards-track document which does one of the following:
> 	• Documents the entire protocol as amended
> 	• Documents the changes made relative to the previous minor version.
> 	• Documents extensions made since the previous minor versions by normatively referencing the documents defining those extensions.
> 	• Documents all REQUIRED functionality, and includes OPTIONAL features by normatively referencing the documents defining those extensions.
> The Working Group retains all these options but the last is typically preferred.  In case in which an XDR change is made which is not a compatible extension, the first is most desirable.  In any case, if there are features which there features whose status has been change to not allowed, the document needs to explain that change and how it is intended that existing implementations address the feature removal
> 9.b.  Version One Extension Practices
> Suggest the following text for this new sub-section:
> Unlike other sub-sections within Section 9, this section is not meant to apply to all RPC-over-RDMA versions.  Instead it applies primarily to Version One but will remain in effect unless modified by statements in documents defining future Versions.  Such documents need not update this document.
> RPC-over-RDMA Version One may be extended by defining a new header type and providing an XDR description of the corresponding header body.  A set of such new protocol elements may be introduced by a Standards-track document and are together consiered an OPTIONAL feature.  Working group and IESG review, together with appropriate testing of prototype implementations, should ensure continued interoperation with existing implementations.
> For the sender to determine whether a receiver supports a particular extension, he can issue a simple test request using the specified header type:
> 	• In RPC-over-RDMA Version One, a receiver can indicate that it does not recognize an rdma_proc enum value only by returning an RDMA_ERROR procedure with the rdma_err field set to RDMA_ERR_BAD (see Section 5.5.2) and with an xid field that matches the one sent.  Although this is indistinguishable from a situation where the receiver does indeed support the procedure, but the XDR is malformed, this fact should pose no difficulty in practice.  If the sender always tests for receiver support using a simple instance of the header type to be tested, such an error at this point inicates that the sender and receiver have no prospect of using the protocol element interoperability and so a lack of feature support can be reasonably assumed.
> 	• The extension specification needs to specify some behavior that the receiver can implement that the original sender can use to determine that support is in fact present.  This may include sending a response header, which may itself be an extension such that the original sender by sending his test header, indicates that he is capable of accepting and using the response header.  The original header and the response can be tied together using the xid field.
> Documents describing extension to Version One should contain in addition to an explanation of the purpose and use of each protocol element:
> 	• An XDR description and a script to extract it.  The resulting XDR description should be able to be concatenated with the XDR description extracted from this document to produce a,combined XDR description for the Version One protocol with the specified extension.
> 	• A description of any interactions with existing features (e.g., any requirement that another feature needs to be supported for the new one  to be supported).\
> 13.1.  Normative References
> Suggest adding the following reference with anchor "LEGAL":
> IETF Trust, "Legal Provisions Relating to IETF Documents", November 2008.
> <Review of draft-ietf-nfsv4-rfc5666bis-03.pdf>

--
Chuck Lever

---

From: David Noveck <[email-masked]>
Date: Mon, 1 Feb 2016 15:31:55 -0500

> what is possible with XML.

That was the original intention.  Saying "Section 9.a" was just a device to
indicate that, while I could guess what "a" would be, I didn't really know,
since you could decide to move things around.

As a result, solving for a is left as an exercise for the editor.

On Mon, Feb 1, 2016 at 3:25 PM, Chuck Lever <[email-masked]> wrote:

> > I basically agree with Chuck that, that except for extensibility-related
> issues, the document is pretty far along, and only needs wordsmithing.
> This review will not concern itself with that wordsmithing.  I anticipate
> going through the document in its entirety, as part of WGLC, which I don't
> think is very far off.
> >
> > The issues with extensibility will be discussed below but they are not
> limited to section 9.  I will deal with wordsmithing issues in the sections
> I do deal with,
>
> > One important issue with extensibility concerns the ability to extend
> the XDR without forcing each extension document to publish a complete
> updated description that updates RFC5666bis.  To avoid the need for that,
> I've proposed some restructuring of the XDR description while maintaining
> over-the-wire compatibility with RFC5666.
> >
> > Within Section 9 itself, the major issue is to accommodate the current
> Version One error reporting model.  I've proposed dealing with that be
> explaining the issues in more abstract terms for the general case and
> providing a new sub-section (now 9.b) to deal with the special limited case
> of extension for version One.
> >
> > Review Format
> >
> > I use gmail formatting facilities extensively in this review, including
> use of multiple fonts, font sizes, and faces.  To accommodate those for
> whom this might be a problem, I've attached a PDF of the review.  Note that
> if people want to offer comments in the form of Adobe Reader XI annotations
> applied to the review PDF, I'm OK with that.
> >
> > The XDR description and scripts for extracting the XDR description are
> Code Components as described in Section 4 of "Legal Provisions Relating to
> IETF Documents" [LEGAL].
> > This section discusses changes to the XDR definition which do not
> involve any change to the over-the-wire message format relative to that
> described in [RFC5666].  Changes  made to the XDR which do change the
> over-the-wire message format (i.e., to make it match actual interoperating
> implementations) are discussed in section 2.3.
> > These changes have been made to make it easier to extend the protocol,
> to better organize the definition, and to make names in the description
> more consonant with actual protocol function.  The specific changes are:
> >       • The XDR description has been given an extraction script using a
> sentinel sequence to match the approach used in [RFC5662] and [RFC7531].
> >       • Constructs which need to be the same in all RPC/RDMA versions
> are put in their own section of the XDR and given names that reflect the
> fact that are not version-specific.
> >       • In order to allow extensions to be added without modification to
> the existing XDR, the header types previously members of the enum
> rpcrdma1_proc have been defined as constants.
> >       • In order to clarify internal layering and to enable protocol
> extension, the union rpcrdma1_body was deleted.  For details of the
> consequent reorganization, See Section 5.1.4.
> > This document contains the XDR [RFC4506] description of the core
> features of the RPC-over-RDMA Version One protocol.
> > The XDR description is provided in this document in a way that makes it
> simple for the reader to extract into ready-to-compile form.  The reader
> can apply the following shell script to this document to  produce a
> machine-readable XDR description of the RPC-over-RDMA protocol described in
> this document, i.e., RPC-over-RDMA Version One without any OPTIONAL
> extensions.
> > <CODE BEGINS>
> > #!/bin/sh
> > That is, if the above script is stored in a file called "extract.sh",
> and this document is in a file called "spec.txt", then the reader can do
> the following to extract an XDR description file.
> > <CODE BEGINS>
> > As described in Section 9.b, extensions to Version One, published as
> Proposed Standards, will have similar means of providing an XDR description
> appropriate to those extensions.  Once  this is done for all extensions to
> be supported, the XDR for the extensions can be appended to the XDR
> description file extracted from this document to produce a consolidated XDR
> description file reflecting all extensions selected.
> > Note that, because RPC/RDMA is not an RPC program, and to enable
> protocol extension, there is no XDR single entity which describes the
> format of the messages exchanged within the protocol.  Instead,
> implementers need to follow the instructions in Section 5.1.4 to
> appropriately encode and decode protocol messages.
> > Code components extracted from this document must include the following
> license text.  Note that in case in which the extracted XDR code is
> combined with other complementary XDR code which itself has an identical
> license, only a single copy of the license needs to be preserved.
> > This section contains XDR describing parts of the RPC-over-RDMA protocol
> which are not subject to change in future versions of the protocol.  For a
> full discussion of the versioning/extensibility model, see Section 9
> >
> > This section contains XDR that is subject to change in subsequent
> RPC-over-RDMA versions.  There are a few things to be noted:
> >       • Even though many of the structure and union names begin
> "rpcrdma1_", these are not necessarily restricted to use in Version One.
> Instead structures will normally be carried over unchanged to subsequent
> versions while unions are subject to extension according to the rules for
> compatible XDR extension as discussed in Section 9.
> >       • There are some specific items that cannot be changed in
> subsequent versions.  Comments will identify these.
> > Except as noted below, the rest of this section will consist of the XDR
> within Section 5.1 of -03, with the license comment excluded.  However:
> >       • Adding a comment to the case for RDMA_ERR_VERS in union
> rpcrdma1_error indicating that the contents of this union arm must never be
> changed.
> >       • Replacing the two lines now defining the RDMA_ERR_VERS case of
> union rpcrdma1_error by "struct rpcrdma_vers rdma_err_vers;"
> > Because RPC-over-RDMA, while described by XDR, is not an RPC program,
> several functions normally provided by the RPC layer need to be addressed
> by the RPC-over-RDMA definition itself. In particular, the following
> functions, normally provided by RPC, need to be provided for as part of the
> RPC-over-RDMA XDR description:
> > In [RFC5666], the XDR description did not take account of the natural
> layering between that part of RPC-over-RDMA functionality that performed
> the RPC-layer-like functions described above and that which implemented
> individual  transport functions.  As a result:
> >       • the four 32-bit words which must be the same in all versions of
> RPC-over-RDMA are split, with three of those words in struct rprdma1_header
> and the remaining word part of union rpcrdma1_body, together with each of
> the message bodies.
> >       • It is impossible, within the resulting structure, to add a new
> message type, without modifying the existing XDR description.
> > The XDR description used in this document reorganizes the XDR in line
> with this natural layering, which maintaining over-the-wire equivalence.
> As a result of this reorganization, the 32-bit big-endian field starting
> twelve bytes into header is no longer the discriminator field of union
> rpcrdma1_body.  Instead, it is the last 32-bit word within struct
> rpcrdma_heder which defines the common (i.e., for all RPC-over-RDMA
> versions) header prefix.  it retains its role of indicating the header type
> and deciding which particular form of header body is to follow.
> > As a result of this restructuring, there is no longer a single XDR item
> that encompasses the entire RPC-over-RDMA  header. Instead, each
> RPC-over-RDMA message consists of up to three items and those using XDR
> encode an decode must be aware that they proceed in sequence as follows:
> >       • Depending on the rdma_which field of the rpc_rdma, the
> appropriate header body for that heaer type as given by Table 1.
> >       • If allowed for that header type as defined in Table 1, an XDR
> stream for the RPC message being transported
> > This framework allows minor issues with the transport protocol to be
> addressed and optional features to be introduced, by making compatible
> extensions to the protocol XDR as described in Section 9.1.  Such changes
> can be made without a change to the protocol version number.
> > In either case, any changes to the RPC-over-RDMA protocol can only be
> effected by publication of a Standards-track document, with appropriate
> review by the  Working Group discussion and the IESG.
> > While it possible to make XDR changes which are not limited to the use
> of compatible extensions in new RPC-over-RDMA versions, such changes should
> only be done when absolutely necessary, as they limit interoperability with
> existing implementations.  It is appropriate for the Working Group to
> consider alternatives carefully before proceeding using this approach.
> >
> >       • In the second sentence, replace "requires updating" by "requires
> publication of a standards-track document updating"
> > I propose changing the title of this section as indicated above.  Note
> that my proposal changes the character of this section by moving much of
> the material now in it into a new Section 9.b.
> >
> > Most changes to the RPC-over-RDMA XDR will take the form of a compatible
> extension to the existing XDR.  Changes which do not change the version
> number (see Section 9.a) must take this form.
> >
> > For an XDR description B to be a compatible extension of an XDR
> description A, the following must be the case:
> >       • Any input recognized  as valid by both descriptions must be
> interpreted as having the same structure according to both descriptions.
> >       • Any input recognized as valid by description B but not by
> description A cab be recognize as part of an supported/unknown extension
> using description A.
> > Each such addition is referred to as a "protocol element" while a set of
> protocol elements that are defined together and such that all must be
> supported together or not supported together by any particular receiver is
> called a "feature"
> >
> > Note that because of the simplicity of the existing protocol and some
> deficiencies in the existing error reporting structure, some of the above
> techniques are not realizable within RPC-over-RDMA Version One.  For a
> discussion of protocol extension practices within Version One, including
> XDR extension, see section 9.b.
> >       • OPTIONAL features MAY be supported by particular receivers and
> senders need to be prepared for the absence of support.
> >       • not allowed features are typically those that were formerly
> OPTIONAL or  REQUIRED, but for which support has been removed,
> > All features defined in this document are considered REQUIRED in
> RPC-over-RDMA Version One.  OPTIONAL features may be added to Version One
> as specified in section 9.b.
> > The terms "OPTIONAL" and "REQUIRED" are used as specified in [RFC2119]
> as indicated in section 1.1.  In this context, it is important to note that
> these status values are assigned by those writing additional specifications
> (e.g., new RPC-over-RDMA versions, extensions) and that their choice in
> this regard is their guidance to implementers.  As used in this document,
> these terms are only directed to implementers as used with regard to
> RPC-over-RDMA Version One.
> > Version numbers are used within RPC-over-RDMA to allow both parties to
> agree on a set of interoperable behaviors and create an environment in
> which support  for OPTIONAL features can be determined and agreed upon.
> > An expected pattern of protocol development is to introduce OPTIONAL
> features within a given version using  XDR extension.  Such features often
> need a significant period of optional general use to ensure they are mature
> And capable of being implemented by a wide range of implementations.  This
> is especially true for infrastructural features that others will build
> upon.  When it is appropriate for OPTIONAL features to become REQUIRED,
> that would be an occasion to create a new RPC-over-RDMA protocol version.
> > I am proposing that this be deleted as a separate sub-section and the
> existing material, as modified, be merged into Section 9.a.
> > In the first bullet, suggest replacing "or whenever a REQUIRED protocol
> element is removed" by "or whenever an existing OPTIONAL feature is made
> REQUIRED"
> > When a version number bump is to be made (e..g., because there is a
> change in the set of REQUIRED features), the Working Group cretae a
> Standards-track document which does one of the following:
> >       • Documents extensions made since the previous minor versions by
> normatively referencing the documents defining those extensions.
> >       • Documents all REQUIRED functionality, and includes OPTIONAL
> features by normatively referencing the documents defining those extensions.
> > The Working Group retains all these options but the last is typically
> preferred.  In case in which an XDR change is made which is not a
> compatible extension, the first is most desirable.  In any case, if there
> are features which there features whose status has been change to not
> allowed, the document needs to explain that change and how it is intended
> that existing implementations address the feature removal
> > Unlike other sub-sections within Section 9, this section is not meant to
> apply to all RPC-over-RDMA versions.  Instead it applies primarily to
> Version One but will remain in effect unless modified by statements in
> documents defining future Versions.  Such documents need not update this
> document.
> > RPC-over-RDMA Version One may be extended by defining a new header type
> and providing an XDR description of the corresponding header body.  A set
> of such new protocol elements may be introduced by a Standards-track
> document and are together consiered an OPTIONAL feature.  Working group and
> IESG review, together with appropriate testing of prototype
> implementations, should ensure continued interoperation with existing
> implementations.
> > For the sender to determine whether a receiver supports a particular
> extension, he can issue a simple test request using the specified header
> type:
> >       • In RPC-over-RDMA Version One, a receiver can indicate that it
> does not recognize an rdma_proc enum value only by returning an RDMA_ERROR
> procedure with the rdma_err field set to RDMA_ERR_BAD (see Section 5.5.2)
> and with an xid field that matches the one sent.  Although this is
> indistinguishable from a situation where the receiver does indeed support
> the procedure, but the XDR is malformed, this fact should pose no
> difficulty in practice.  If the sender always tests for receiver support
> using a simple instance of the header type to be tested, such an error at
> this point inicates that the sender and receiver have no prospect of using
> the protocol element interoperability and so a lack of feature support can
> be reasonably assumed.
> >       • The extension specification needs to specify some behavior that
> the receiver can implement that the original sender can use to determine
> that support is in fact present.  This may include sending a response
> header, which may itself be an extension such that the original sender by
> sending his test header, indicates that he is capable of accepting and
> using the response header.  The original header and the response can be
> tied together using the xid field.
> > Documents describing extension to Version One should contain in addition
> to an explanation of the purpose and use of each protocol element:
> >       • An XDR description and a script to extract it.  The resulting
> XDR description should be able to be concatenated with the XDR description
> extracted from this document to produce a,combined XDR description for the
> Version One protocol with the specified extension.
> >       • A description of any interactions with existing features (e.g.,
> any requirement that another feature needs to be supported for the new one
> to be supported).\

---

From: David Noveck <[email-masked]>
Date: Mon, 1 Feb 2016 14:14:11 -0500

*General Comments*

*Overall Evaluation*

*Issues with Extensibility*

One important issue with extensibility concerns the ability to extend the
XDR without forcing each extension document to publish a complete updated
description that updates RFC5666bis.  To avoid the need for that, I've
proposed some restructuring of the XDR description while maintaining
over-the-wire compatibility with RFC5666.

*Review Format*

I use gmail formatting facilities extensively in this review, including use
of multiple fonts, font sizes, and faces.  To accommodate those for whom
this might be a problem, I've attached a PDF of the review.  Note that if
people want to offer comments in the form of Adobe Reader XI annotations
applied to the review PDF, I'm OK with that.

*Comments by Section (Regarding Extensibility)*

1. <https://tools.ietf.org/html/rfc5662#section-1.2>a.  Code
Components Licensing Notice

The XDR description and scripts for extracting the XDR description are
Code Components as described in Section 4 of "Legal Provisions
Relating to IETF Documents" [LEGAL].

*2.  Changes Since RFC 5666*

A reorganization of this section into three (from two) subsections is proposed,*

2.1.  Changes To The Specification*

Suggest deleting the fourth bullet as these changes are best handled
separately, in the new Section 2.a.

*2.a.  Changes to the XDR Definition*

For this new section, which follows the existing 2.1, the following
text is proposed:

This section discusses changes to the XDR definition which do not
involve any change to the over-the-wire message format relative to
that described in [RFC5666].  Changes  made to the XDR which do change
the over-the-wire message format (i.e., to make it match actual
interoperating implementations) are discussed in section 2.3.

These changes have been made to make it easier to extend the protocol,
to better organize the definition, and to make names in the
description more consonant with actual protocol function.  The
specific changes are:

   - The XDR description has been given an extraction script using a
sentinel sequence to match the approach used in [RFC5662] and
[RFC7531].
   - Constructs which need to be the same in all RPC/RDMA versions are
put in their own section of the XDR and given names that reflect the
fact that are not version-specific.
   - In order to allow extensions to be added without modification to
the existing XDR, the header types previously members of the enum
rpcrdma1_proc have been defined as constants.
   - RDMA_ERR_CHUNK has been renamed as RDMA_ERR_BAD.
   - In order to clarify internal layering and to enable protocol
extension, the union rpcrdma1_body was deleted.  For details of the
consequent reorganization, See Section 5.1.4.

*5.1.  XDR Protocol Definition*

The XDR description is provided in this document in a way that makes
it simple for the reader to extract into ready-to-compile form.  The
reader can apply the following shell script to this document to
produce a machine-readable XDR description of the RPC-over-RDMA
protocol described in this document, i.e., RPC-over-RDMA Version One
without any OPTIONAL extensions.

<CODE BEGINS>

#!/bin/sh

<CODE ENDS>

That is, if the above script is stored in a file called "extract.sh",
and this document is in a file called "spec.txt", then the reader can
do the following to extract an XDR description file.

<CODE BEGINS>

<CODE ENDS>

As described in Section 9.b, extensions to Version One, published as
Proposed Standards, will have similar means of providing an XDR
description appropriate to those extensions.  Once  this is done for
all extensions to be supported, the XDR for the extensions can be
appended to the XDR description file extracted from this document to
produce a consolidated XDR description file reflecting all extensions
selected.

*5.1.1 Code Component License*

It is suggested that the existing first paragraph of Section 5.1 be
replaced by the following:

Code components extracted from this document must include the
following license text.  Note that in case in which the extracted XDR
code is combined with other complementary XDR code which itself has an
identical license, only a single copy of the license needs to be
preserved.

After this, the section will continue with the existing XDR Code license
comment. However:

   - Each line of the comment will have the sentinel sequence ("///")
   prepended.
   - After the end of the comment, the section will end.
   - There will be a "<CODE ENDS>" just before the end of the section.

*5.1.2. XDR Applying to All Versions of RPC-over-RDMA*

<CODE BEGINS>

///const RDMA_ERROR = 4;
///

///struct rpcrdma_vers {

*5.1.2.   XDR Specifc to Version One of RPC-over-RDMA*

   - Even though many of the structure and union names begin
"rpcrdma1_", these are not necessarily restricted to use in Version
One.  Instead structures will normally be carried over unchanged to
subsequent versions while unions are subject to extension according to
the rules for compatible XDR extension as discussed in Section 9.
   - There are some specific items that cannot be changed in
subsequent versions.  Comments will identify these.

   - There will be a "<CODE BEGINS>" in front of it.
   - Each line will have the sentinel sequence ("///") prepended.
   - The existing "<CODE ENDS>" will remain.

   - Deleting the definitions of struct rpcrdma1_header and enum
   rpcrma1_proc.
   - Adding a comment to the definition of RDMA_ERR_VERS indicating that
   this value must never be changed.
   - Adding a comment to the case for RDMA_ERR_VERS in union rpcrdma1_error
   indicating that the contents of this union arm must never be changed.
   - Changing RDMA_ERR_CHUNK to RDMA_ERR_BAD.
   - Replacing the two lines now defining the RDMA_ERR_VERS case of union
   rpcrdma1_error by "struct rpcrdma_vers rdma_err_vers;"
   - Deleting the definition of union rpcrdma1_body.

*5.1.4.  Use of XDR Descriptions*

Because RPC-over-RDMA, while described by XDR, is not an RPC program,
several functions normally provided by the RPC layer need to be
addressed by the RPC-over-RDMA definition itself. In particular, the
following functions, normally provided by RPC, need to be provided for
as part of the RPC-over-RDMA XDR description:

   - selection/negotiation of protocol versions
   - selection of the particular message bodies to go with specific header types

In [RFC5666], the XDR description did not take account of the natural
layering between that part of RPC-over-RDMA functionality that
performed the RPC-layer-like functions described above and that which
implemented individual  transport functions.  As a result:

   - the four 32-bit words which must be the same in all versions of
RPC-over-RDMA are split, with three of those words in struct
rprdma1_header and the remaining word part of union rpcrdma1_body,
together with each of the message bodies.
   - It is impossible, within the resulting structure, to add a new
message type, without modifying the existing XDR description.

The XDR description used in this document reorganizes the XDR in line
with this natural layering, which maintaining over-the-wire
equivalence.  As a result of this reorganization, the 32-bit
big-endian field starting twelve bytes into header is no longer the
discriminator field of union rpcrdma1_body.  Instead, it is the last
32-bit word within struct rpcrdma_heder which defines the common
(i.e., for all RPC-over-RDMA versions) header prefix.  it retains its
role of indicating the header type and deciding which particular form
of header body is to follow.

As a result of this restructuring, there is no longer a single XDR
item that encompasses the entire RPC-over-RDMA  header. Instead, each
RPC-over-RDMA message consists of up to three items and those using
XDR encode an decode must be aware that they proceed in sequence as
follows:

   - A struct rpcrdma_prefix
   - Depending on the rdma_which field of the rpc_rdma, the
appropriate header body for that heaer type as given by Table 1.

    In cases in which there is an undefined header type, this is to be
treated as an XDR decode/encode error.

   - If allowed for that header type as defined in Table 1, an XDR
stream for the RPC message being transported

*5.5.2. XDR Errors*

In the last sentence of the first paragraph, suggest replacing
"RDMA_ERR_CHUNK" by "RDMA_ERR_BAD".

*5.5.3.  **Responder Operational Errors*

In the last sentence of the last paragraph, suggest replacing
"RDMA_ERR_CHUNK" by "RDMA_ERR_BAD".

*9. Protocol Extensibility*

   - Replacing the second sentence by the two following sentences:

   - In the current third sentence, replacing "needed" by "are to be made"
   - Replacing the current fourth sentence by the following:

In either case, any changes to the RPC-over-RDMA protocol can only be
effected by publication of a Standards-track document, with
appropriate review by the  Working Group discussion and the IESG.

While it possible to make XDR changes which are not limited to the use of
compatible extensions in new RPC-over-RDMA versions, such changes should
only be done when absolutely necessary, as they limit interoperability with
existing implementations. It is appropriate for the Working Group to
consider alternatives carefully before proceeding using this approach.

   - In the first sentence, replace "Section 9" by "Section 9 (except for
   Section 9.b)"
   - In the second sentence, replace "requires updating" by "requires
   publication of a standards-track document updating"

*9.1. Changes to RPC-over-RDMA Header XDR*

I propose changing the title of this section as indicated above. Note that
my proposal changes the character of this section by moving much of the
material now in it into a new Section 9.b.

   - In the first sentence, after "header', add "(now in struct
   rpcrdma_prefix)"
   - In the second sentence, replace "for" by "to allow".
   - In the third sentence, after "union', add "(now in struct
   rpcrdma_vers)"

For an XDR description B to be a compatible extension of an XDR description
A, the following must be the case:

   - All input recognized as description valid by A must be recognized as
   valid by description B.
   - Any input recognized as valid by both descriptions must be interpreted
   as having the same structure according to both descriptions.
   - Any input recognized as valid by description B but not by description
   A cab be recognize as part of an supported/unknown extension using
   description A.

In the context of RPC-over-RDMA (in general), the following sorts of
changes can be made compatibly:

   - Addition of a new header type and associated header body
   - Addition of new enum values and associated arms to unions which do not
   include a default case.
   - Addition of previously undefined flag bits to flag words that are
   including in existing heaer bodies.

*9.2.  Feature Statuses with RPC-over-RDMA Versions*

Within a given RPC-over-RDMA version, every known feature is either
OPTIONAL, REQUIRED, or not allowed.

   - REQUIRED features MUST be supported by all receivers and senders
may depend on them being supported.
   - OPTIONAL features MAY be supported by particular receivers and
senders need to be prepared for the absence of support.
   - not allowed features are typically those that were formerly
OPTIONAL or  REQUIRED, but for which support has been removed,

All features defined in this document are considered REQUIRED in
RPC-over-RDMA Version One.  OPTIONAL features may be added to Version
One as specified in section 9.b.

The terms "OPTIONAL" and "REQUIRED" are used as specified in [RFC2119]
as indicated in section 1.1.  In this context, it is important to note
that these status values are assigned by those writing additional
specifications (e.g., new RPC-over-RDMA versions, extensions) and that
their choice in this regard is their guidance to implementers.  As
used in this document, these terms are only directed to implementers
as used with regard to RPC-over-RDMA Version One.

The status of features may change between RPC-over-RDMA protocol
versions.  For details, see Section 9.a.

*9.a.  RPC-over-RDMA Version Numbering*

Version numbers are used within RPC-over-RDMA to allow both parties to
agree on a set of interoperable behaviors and create an environment in
which support  for OPTIONAL features can be determined and agreed
upon.

An expected pattern of protocol development is to introduce OPTIONAL
features within a given version using  XDR extension.  Such features
often need a significant period of optional general use to ensure they
are mature And capable of being implemented by a wide range of
implementations.  This is especially true for infrastructural features
that others will build upon.  When it is appropriate for OPTIONAL
features to become REQUIRED, that would be an occasion to create a new
RPC-over-RDMA protocol version.

*9.a.1.  Incrementing The Version Number*

I will refer to existing paragraphs in terms of their position within
the existing section 9.2.1.

In the second sentence of the first paragraph, suggest replacing "Two"
by "Some".

In the first bullet, suggest replacing "or whenever a REQUIRED
protocol element is removed" by "or whenever an existing OPTIONAL
feature is made REQUIRED"

   - Whenever a REQUIRED feature is made OPTIONAL
   - Whenever an existing feature is converted to be "not allowed".
   - Whenever an XDR change is made which is not a compatible
extension as defined in Section 9.1.

   1. Documents the entire protocol as amended
   2. Documents the changes made relative to the previous minor version.
   3. Documents extensions made since the previous minor versions by
normatively referencing the documents defining those extensions.
   4. Documents all REQUIRED functionality, and includes OPTIONAL
features by normatively referencing the documents defining those
extensions.

The Working Group retains all these options but the last is typically
preferred.  In case in which an XDR change is made which is not a
compatible extension, the first is most desirable.  In any case, if
there are features which there features whose status has been change
to not allowed, the document needs to explain that change and how it
is intended that existing implementations address the feature removal

*9.b.  Version One Extension Practices*

Unlike other sub-sections within Section 9, this section is not meant
to apply to all RPC-over-RDMA versions.  Instead it applies primarily
to Version One but will remain in effect unless modified by statements
in documents defining future Versions.  Such documents need not update
this document.

RPC-over-RDMA Version One may be extended by defining a new header
type and providing an XDR description of the corresponding header
body.  A set of such new protocol elements may be introduced by a
Standards-track document and are together consiered an OPTIONAL
feature.  Working group and IESG review, together with appropriate
testing of prototype implementations, should ensure continued
interoperation with existing implementations.

For the sender to determine whether a receiver supports a particular
extension, he can issue a simple test request using the specified
header type:

   - In RPC-over-RDMA Version One, a receiver can indicate that it
does not recognize an rdma_proc enum value only by returning an
RDMA_ERROR procedure with the rdma_err field set to RDMA_ERR_BAD (see
Section 5.5.2) and with an xid field that matches the one sent.
Although this is indistinguishable from a situation where the receiver
does indeed support the procedure, but the XDR is malformed, this fact
should pose no difficulty in practice.  If the sender always tests for
receiver support using a simple instance of the header type to be
tested, such an error at this point inicates that the sender and
receiver have no prospect of using the protocol element
interoperability and so a lack of feature support can be reasonably
assumed.
   - The extension specification needs to specify some behavior that
the receiver can implement that the original sender can use to
determine that support is in fact present.  This may include sending a
response header, which may itself be an extension such that the
original sender by sending his test header, indicates that he is
capable of accepting and using the response header.  The original
header and the response can be tied together using the xid field.

Documents describing extension to Version One should contain in
addition to an explanation of the purpose and use of each protocol
element:

   - An XDR description and a script to extract it.  The resulting XDR
description should be able to be concatenated with the XDR description
extracted from this document to produce a,combined XDR description for
the Version One protocol with the specified extension.
   - A description of any interactions with existing features (e.g.,
any requirement that another feature needs to be supported for the new
one  to be supported).\

*13.1.  Normative References*