Re: [nfsv4] AD Review of draft-ietf-nfsv4-rfc5666bis-08

[David Noveck <[e-mail]>]

> See what you think of

>           Therefore
>           the use of the Read-Read Transfer Model within RPC-over-RDMA
Version
>           One implementations has been removed from this specification.

> Does that sound violent enough? :-)

If you're looking for violence, you could go with  "removed with extreme
prejudice", but it conflicts with
the following sentence.

How about:

            Therefore
            use of the Read-Read Transfer Model has been removed from
RPC-over-RDMA Version
            One.   This transfer model could be adopted by future versions
of RPC-over-RDMA.




On Tue, Jan 10, 2017 at 5:58 PM, Spencer Dawkins at IETF <
[e-mail]> wrote:

> Hi, Chuck,
>
> Again, thanks for the speedy response. My responses are inline.
>
> Mostly they're suggestions that echo your clarifications. See what you
> think, but it looks to me like we're converging pretty well.
>
> Thanks,
>
> Spencer
>
> On Tue, Jan 10, 2017 at 3:35 PM, Chuck Lever <[e-mail]>
> wrote:
>
>> Spencer, thanks for your time and your remarks. I've indicated
>> below where I can immediately merge your suggestions, but
>> there remain open questions.
>>
>>
>> > On Jan 10, 2017, at 3:29 PM, Spencer Dawkins at IETF <
>> [e-mail]> wrote:
>> >
>> > Hi, Spencer (S, where the "S" stands for "Shepherd),
>> >
>> > My apologies for a slow AD evaluation for this draft. I've been down
>> with something flu-ish since December 24, and am only now surfacing.
>> >
>> > I found this specification to be pretty clear, but I did have some
>> questions and suggested wording changes that I'd like for people to
>> consider before I request Last Call.
>> >
>> > Could you let me know when you think the document is good to go?
>> >
>> > In the meantime, I'll do the AD Evaluation for
>> draft-ietf-nfsv4-rpcrdma-bidirection-05.
>> >
>> > Thanks!
>> >
>> > Spencer (D)
>> >
>> > AD Evaluation follows ---
>> >
>> > This could certainly be OK, but I note that RFC 5666 uses the pre-2008
>> text IPR version, while the bis draft does not. Is that intentional? (If
>> you folks were able to obtain releases that allow you to use the default
>> IPR version, I applaud your initiative. It's never going to be easier to do
>> that, than now).
>>
>> You mean this text from RFC 5666:
>>
>>    This document may contain material from IETF Documents or IETF
>>    Contributions published or made publicly available before November
>>    10, 2008.  The person(s) controlling the copyright in some of this
>>    material may not have granted the IETF Trust the right to allow
>>    modifications of such material outside the IETF Standards Process.
>>    Without obtaining an adequate license from the person(s) controlling
>>    the copyright in such materials, this document may not be modified
>>    outside the IETF Standards Process, and derivative works of it may
>>    not be created outside the IETF Standards Process, except to format
>>    it for publication as an RFC or to translate it into languages other
>>    than English.
>>
>> I didn't obtain any permission to delete this material, just simply
>> used the default boilerplate provided by xml2rfc. I see now that was
>> an oversight on my part.
>>
>> Because we are continuing to use the XDR specification from RFC 5666
>> nearly in whole and it was originally submitted in 2006, I think we
>> can say that there remains some pre-2008 contribution in rfc5666bis,
>> and therefore should continue to include the pre-2008 IPR boilerplate.
>
>
> That's actually handling it correctly. My question was whether it's
> possible to get permission from the RFC 5666 authors, so you can use the
> more modern IPR attribute. If not, you're fine.
>
> I only note that it gets harder every year to find early authors, and at
> this rate, the nfsv4 working group will be around another twenty years, so
> maybe it's worth poking people now? :-)
>
>
>> I'm looking at
>>
>>   https://xml2rfc.tools.ietf.org/rfc7749.html#attribute-ipr
>>
>> Which value should I supply as the document's ipr attribute? Possibly
>> one of these:
>>
>>   https://xml2rfc.tools.ietf.org/rfc7749.html#attribute-ipr-200811
>>
>>
>> > Just so that I understand what's being said, is
>> >
>> >    This document obsoletes RFC 5666.  However, the protocol specified by
>> >    this document is based on existing interoperating implementations of
>> >    the RPC-over-RDMA Version One protocol.
>> >
>> > saying
>> >
>> >    This document obsoletes RFC 5666.  However, the protocol specified by
>> >    this document is based on experience gained from existing
>> >                              ^^^^^^^^^^^^^^^^^^^^^^
>> >    interoperating implementations of the RPC-over-RDMA Version One
>> protocol.
>> >
>> > ? (If so, again, I applaud that!)
>>
>> rfc5666bis does benefit from experience gained, but it is also an
>> effort to document the behavior of existing implementations, which
>> may not exactly match what was specified in RFC 5666. All existing
>> implementations continue to interoperate.
>>
>> Is there a way to say that more explicitly here?
>
>
> Perhaps something like this?
>
> This document specifies the RPC-over-RDMA Version One protocol, based on
> existing implementations of RFC 5666 and experience gained through
> deployment. This document obsoletes RFC 5666.
>
>
>> > I would be a bit more comfortable with
>> >
>> >    Open Network Computing Remote Procedure Call (ONC RPC, or simply,
>> >    RPC)
>> >
>> > if the document said
>> >
>> >    Open Network Computing Remote Procedure Call (ONC RPC, often
>> shortened
>> >    in this document as RPC)
>> >
>> > (or, more broadly, "often shortened in NFSv4 documents", if that's
>> correct), because I'm thinking that "RPC" doesn't even refer to ONC RPC in
>> all IETF-stream RFCs.
>>
>> I can make that editorial change (unless my co-authors object,
>> and ditto for subsequent responses).
>>
>>
>> > Is this text
>> >
>> >    Although the RDMA transport described herein can provide relatively
>> >    transparent support for any RPC application, this document also
>> >    describes mechanisms that can optimize data transfer even further,
>> >    given more active participation by RPC applications.
>> >
>> > saying
>> >
>> >    Although the RDMA transport described herein can provide relatively
>> >    transparent support for any RPC application, this document also
>> >    describes mechanisms that can optimize data transfer even further,
>> >    for RPC applications that are willing to exploit awareness of
>> >    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> >    RDMA transport.
>> >    ^^^^^^^^^^^^^^
>> >
>> > ?
>>
>> Yes, that makes more sense. I will use your replacement text.
>>
>>
>> > In this text,
>> >
>> >    o  Section 2 has been expanded to introduce and explain key RPC, XDR,
>> >       and RDMA terminology.  These terms are now used consistently
>> >       throughout the specification.
>> >
>> > I wonder if it's helpful to add a reference for XDR.
>>
>> I will add this reference.
>>
>>
>> > I found myself reading
>> >
>> >    o  The specification of the optional Connection Configuration
>> >       Protocol has been removed from the specification.
>> >
>> > and wondering "where did it go, and why?". Is this going to appear in a
>> separate document, or was this so optional that it wasn't much implemented
>> or deployed? For most of the items in the bullet list in Section 2.1, I
>> don't think you need any explanation, but if you're removing something that
>> was in the obsoleted RFC, it might be nice to explain what's behind that.
>>
>> A) CCP was never implemented, and
>>
>
> I think pointing this out would be helpful. I don't think you'd need to
> say more.
>
>
>> B) A better design would provide a similar information in-band for
>> each new connection, rather than as a separate protocol.
>>
>> I recall we had some of this explanation in earlier revisions, but
>> it was removed for brevity. I can add something here.
>>
>>
>> > In this text,
>> >
>> >    o  A subsection describing the use of RPCSEC_GSS with RPC-over-RDMA
>> >       Version One has been added.
>> >
>> > it might be helpful to add a reference for RPCSEC_GSS.
>>
>> I will add this reference.
>>
>>
>> > In this text,
>> >
>> >    o  Support for the Read-Read transfer model has been removed.  Read-
>> >       Read is a slower transfer model than Read-Write.  As a result,
>> >       implementers have chosen not to support it.  Removal simplifies
>> >       explanatory text, and support for the RDMA_DONE procedure is no
>> >       longer necessary.
>> >
>> > "is no longer necessary" says, to me, that it can still be present, and
>> used. Is that about right, or is "the RDMA_DONE procedure is no longer
>> supported" more accurate? I'm just asking because the description of the
>> RDMA_MSGP message type in the next bullet sounds much blunter, and both of
>> those seem weaker than the descriptions in Section 5.6. If you're able to
>> use the same language in all these places, I'd be clearer on what your
>> intent is.
>>
>> No implementation of Read-Read exists, thus no implementation sends
>> RDMA_DONE. I recall we carefully attempted to avoid the use of the
>> term "deprecated" here.
>>
>
> Understood.
>
> Perhaps something like "support for the RDMA_DONE procedure is no longer
> part of this protocol"?
>
>
>>
>> What tone or element of the text in Section 5.6 would you prefer we
>> use throughout?
>>
>>
>> > It seems to me that
>> >
>> >    The protocol version number has not been changed because the protocol
>> >    specified in this document fully interoperates with implementations
>> >    of the RPC-over-RDMA Version One protocol specified in [RFC5666].
>> >
>> > belongs a lot earlier in the document - at least in the Introduction,
>> and possibly the Abstract as well. It's nearly five pages into the current
>> draft.
>>
>> I will move this text earlier in the document.
>>
>>
>> > I'm not clear on the relationship between RDMA and DDP, and I'm a bit
>> surprised that Section 3.2.1 is called Direct Data Placement, but most of
>> the contents are describing RDMA, only one paragraph mentions DDP, and that
>> paragraph says that RDMA isn't necessary or sufficient for DDP, unless I'm
>> misreading it. I suspect that my hangup is that it's not clear where RDMA
>> and DDP are moving the RPC message, but I'm not sure of that. Could you
>> take a look at this section and make sure it's as clear for an implementer
>> as it needs to be?
>>
>> Direct Data Placement is the most abstract way of defining what this
>> transport does. Remote Direct Memory Access is one mechanism that
>> implements DDP. We are attempting to write this document in terms of
>> data placement, rather than in terms of a specific placement mechanism.
>>
>> Dave Noveck is spearheading an effort to use Send and Receive, rather
>> than RDMA, to achieve Direct Data Placement, for example.
>>
>> As one author of this document, I might not be in the best position
>> to know exactly what is and is not clear to implementers. ;-) I will
>> have a look at this, and I'm open to other suggestions.
>>
>> Though the individual changes will be surgical, this might take one
>> or two iterations to find consistency throughout the entire document.
>>
>>
>> > I'm noticing that "pre-posted" is used for the first time without a
>> reference or definition, and "pinned" is used only once, without a
>> reference or definition.
>>
>> I will investigate replacement terminology.
>>
>
> I think the terminology is fine, and it's certainly used in the
> bidirectional draft. I was wondering if you could define it, either by
> reference or inline in this document.
>
>
>> > In this text,
>> >
>> >               RDMA Read
>> >             The RDMA provider supports an RDMA Read operation to
>> directly
>> >             place peer source data in the read initiator's memory.  The
>> local
>> >             host initiates an RDMA Read, and completion is signaled
>> there; no
>> >             completion is signaled on the remote.
>> >
>> > is "remote" short for "remote peer"? And I'm wondering how this is
>> different from the next paragraph, which begins,
>> >
>> >       The remote peer receives no notification of RDMA Read completion.
>>
>> "remote peer" is correct, and I agree that is redundant. Removing the text
>> that discusses completion handling from the first paragraph is probably
>> best.
>>
>>
>> > On
>> >
>> >          [RFC5666] specifies the use of both the Read-Read and the
>> Read-Write
>> >          Transfer Model.  All current RPC-over-RDMA Version One
>> >          implementations use only the Read-Write Transfer Model.
>> Therefore
>> >          the use of the Read-Read Transfer Model within RPC-over-RDMA
>> Version
>> >          One implementations is no longer supported.  Transfer Models
>> other
>> >          than the Read-Write model may be used in future versions of
>> RPC-over
>> >        RDMA.
>> >
>> > I'm still stumbling over my previous point about Read-Read, but this
>> time, it's "no longer supported", which seems different from "no longer
>> necessary" in my previous comment.
>>
>> "No longer necessary" refers to the use of a particular protocol element
>> (RDMA_DONE). The text here is discussing a particular mode of operation
>> which was never implemented. I'm open to suggestion.
>
>
> See what you think of
>
>             Therefore
>             the use of the Read-Read Transfer Model within RPC-over-RDMA
> Version
>             One implementations has been removed from this specification.
>
> Does that sound violent enough? :-)
>
>
>> > In this text,
>> >
>> >       4.4.2.  DDP-Eligibility
>> >
>> >          Only an XDR data item that might benefit from Direct Data
>> Placement
>> >          may be reduced.  The eligibility of particular XDR data items
>> to be
>> >          reduced is independent of RPC-over-RDMA, and thus is not
>> specified by
>> >          this document.
>> >
>> > is there a pointer/reference you can give, for where this is specified?
>> Is this describing the same "not specified in this document" as the text in
>> 5.4.2. "Communicating DDP-Eligibility"? I had the same question there.
>>
>> I will add text that explains that the eligibility of particular
>> XDR data items is spelled out in a protocol's Upper Layer Binding
>> specification, which is a separate document.
>>
>>
>> > For this text,
>> >
>> >    Except in special cases, a chunk contains exactly one XDR data item.
>> >
>> > is there a pointer/reference/clue you can provide about chunks with
>> more than one XDR data item?
>>
>> I will add a reference to the discussion of Position-Zero Read chunks
>> and Reply chunks.
>>
>>
>> > I struggled with this text a bit (and similar text appears in Section
>> 4.4.6.1. Write Chunk Round-up):
>> >
>> > 3.5.  XDR Decoding with Read Chunks
>> >        XDR requires each encoded data item to start on four-byte
>> alignment.
>> >          When an odd-length data item is encoded, its length is encoded
>> >          literally, while the data is padded so the next data item in
>> the XDR
>> >          stream can start on a four-byte boundary.
>> >
>> > I would think "odd-length" covers one-byte and three-byte fields, but
>> not two-byte fields. If you mean "a data item with a length that's not a
>> multiple of four bytes", that would be more accurate.
>>
>> I will replace the term "odd-length" with more precise language.
>>
>>
>> > I might have said "When an odd-length data item is encoded, its
>> unpadded length is encoded". I think I understand what you meant by
>> "encoded literally", but I was guessing.
>>
>> I agree this is confusing. The actual length of said items precedes them
>> in
>> the XDR stream. This length never includes XDR padding bytes.
>>
>> If the chunk does not include padding, the chunk length is the same as the
>> actual length of the data item. If the chunk includes padding bytes, the
>> chunk length must be increased to include those bytes, but the actual
>> encoded length remains unchanged.
>>
>>
>> > Thanks for getting the code component licensing right (I think) - this
>> is my first RFC to shepherd with a code component, so I'm reading
>> https://trustee.ietf.org/license-info/IETF-TLP-4.htm in parallel.
>>
>> I believe this was copied from recent NFS-related RFCs that
>> have adopted similar clauses. Let me know if we need something
>> different/newer here, IANAL ;-)
>>
>>
>> > In this text:
>> >
>> >          The details of reporting and recovery from RDMA link layer
>> errors are
>> >          outside the scope of this protocol specification.
>> >
>> > are these details described anywhere? If so, a reference would be nice.
>>
>> They would be spelled out in the link layer's API and operational
>> specifications. There's no one reference. Would it be better to
>> remove this text?
>>
>
> I think this is useful information. Perhaps
>
>           The details of reporting and recovery from RDMA link layer
> errors are
>           spelled out in specific link layer APIs and operational
> specification, and are
>           outside the scope of this protocol specification.
>
>
>>
>>
>> > My apologies for asking about text that's unchanged from RFC 5666, but
>> I wasn't sure what you meant here:
>> >
>> >    RPC services normally register with a portmap or rpcbind [RFC1833]
>> >    service, which associates an RPC Program number with a service
>> >    address.  (In the case of UDP or TCP, the service address for NFS is
>> >    normally port 2049.)  This policy is no different with RDMA
>> >    transports, although it may require the allocation of port numbers
>> >    appropriate to each Upper Layer Protocol that uses the RPC framing
>> >    defined here.
>> >
>> > Is this saying that you need multiple port numbers if you use multiple
>> ULPs, or that specific ULPs have different port conventions, or something
>> else? I'm guessing a couple of other possibilities, but I'm guessing.
>>
>> I think this means that sometimes, the Upper Layer Protocol might choose
>> to allocate a separate port number for use when running on RPC-over-RDMA.
>> For example, NFS on traditional transports such as TCP uses port 2049,
>> but NFS/RDMA uses port 20049.
>>
>> I will clarify this text.
>>
>>
>> > It didn't jump out at me, that this paragraph was introducing two cases
>> that result in similar problems, until I read the rest of the section:
>> >
>> >          A DDP-eligibility violation occurs when a requester forms a
>> Call
>> >          message with a non-DDP-eligible data item in a Read chunk.  A
>> >          violation occurs when a responder forms a Reply message without
>> >          reducing a DDP-eligible data item when there is a Write list
>> provided
>> >          by the requester.
>> >
>> > I was at least partially confused because the first case is for a
>> "DDP-eligibility violation", and the second is for "a violation" - is the
>> second also for a "DDP-eligibility violation"?
>>
>> Yes, those are both DDP-eligibility violations.
>>
>>
>> > If I'm getting that right, this text might be clearer as
>> >
>> >          A DDP-eligibility violation occurs when a requester forms a
>> Call
>> >          message with a non-DDP-eligible data item in a Read chunk, or
>> >          when a responder forms a Reply message without
>> >          reducing a DDP-eligible data item when there is a Write list
>> provided
>> >          by the requester.
>>
>> I will use your replacement.
>>
>>
>> > Nice work on Section 8. You may note that Mirja and I have been
>> including migration and coexistence discussions at TSVAREA, the past couple
>> of IETFs.
>>
>> Dave Noveck provided the structure and most of the text for this section.
>>
>>
>> > In Section 8.1, I wonder if you'll get pushback on naming a
>> theoretically short-lived working group (yeah, I know the nfsv4 working
>> group is older than 1998, and that's version 4, for the oldest charter in
>> the datatracker) in a "forever RFC":
>> >
>> >          Such extensions must be specified in a Standards Track
>> document with
>> >          appropriate review by the nfsv4 Working Group and the IESG.
>> >
>> > Let's leave this for now, and see if anyone objects.
>>
>>
>> --
>> Chuck Lever
>>
>>
>>
>>
>
> _______________________________________________
> nfsv4 mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/nfsv4
>
>