Re: [storm] MPA Draft - Review

[Steve Wise <[e-mail]>]

Hey Arkady,

It does seem like you did the section 9 changes Bob and I requested:

----
   Change the IRD definition on the request from "In request: the 
Initiator requested responder IRD for the
   connection." to "In request: the Initiator initial IRD setting for 
the connection."
----

Thanks,

Steve.

On 4/2/2011 8:35 PM, arkady kanevsky wrote:
> All,
> updated version 04 is attached.
>
> Hemal,
> Thanks for catching it.
> I had fixed the first issue. I had added reference to FPDU in the 
> FULPDU definition for the second.
>
> David,
> Please, check to see that you comments are addressed.
>
> Steve and Robert,
> please, check that you comment is fixed correctly.
>
> Once I get positive feedback from all of you, I will submit the version.
>
> Thanks,
> Arkady
>
> On Thu, Mar 31, 2011 at 2:43 PM, Hemal Shah <[e-mail] 
> <mailto:[e-mail]>> wrote:
>
>     I have some comments on -03 draft:
>
>        1. In section 10, it is written that "Enhanced MPA Initiator
>           and Responder:  If a responder receives an enhanced MPA
>           message, it MUST respond with an unenhanced MPA message." I
>           think it should be written that the responder must respond
>           with an enhanced MPA message. It appears like a typo to me.
>        2. I find the use of FULPDU confusing in this draft. RFC5044
>           does not define term FULPDU. RFC5044 uses term FPDU to refer
>           to Framed Protocol Data Unit. I suggest that we use term
>           FPDU instead of FULPDU in the draft.
>
>     Hemal
>     -----Original Message-----
>     From: [e-mail] <mailto:[e-mail]>
>     [mailto:[e-mail]] On Behalf Of [e-mail]
>     <mailto:[e-mail]>
>     Sent: Thursday, March 31, 2011 7:48 AM
>     To: [e-mail] <mailto:[e-mail]>
>     Subject: Re: [storm] MPA Draft - Review
>     Importance: High
>     The -03 version of the MPA draft has addressed all of the issues
>     from my review, and .  Unfortunately, I need some minor edits for
>     clarity before I can send this on to our AD with a publication
>     request.  Would the authors please submit a -04 version with the
>     following two changes quickly.
>     Section 9 (end)
>     OLD
>        The peer-to-peer negotiation for the RTR message follows the
>        following order:
>        Initiator -->: Sets Control Flags it is capable to send for RTR
>        Responder <--: Sets Control Flags it is capable to receive for RTR
>        Initiator -->: The first message send MUST be a negotiated RTR
>     NEW
>        The peer-to-peer negotiation for the RTR message follows the
>        following order:
>        Initiator -->: Sets Control Flags to indicate
>     Initiator-supported forms of RTR
>        Responder <--: Sets Control Flags to indicate
>     Responder-supported forms of RTR
>        Initiator -->: If at least one form of RTR is supported by both
>     Initiator and
>             Responder, then the first message sent MUST be an RTR
>     using a form supported
>             by both the Initiator and Responder.
>     Section 10
>     OLD
>           In
>           this case initiator CAN attempt to establish RDMA connection
>     using
>           unenhanced MPA protocol as defined in [RFC5044] and let ULP deal
>           with ORD and IRD, and peer-to-peer negotiations.
>     NEW
>           In
>           this case initiator MAY attempt to establish RDMA connection
>     using
>     ------------------------->^^^
>           unenhanced MPA protocol as defined in [RFC5044] if this
>     protocol is
>             compatible with the application, and let ULP deal with ORD
>     and IRD,
>           and peer-to-peer negotiations.
>     Ordinarily, I'd write an RFC Editor Note for small changes like
>     these, but they're sufficiently critical to interoperability that
>     I'd prefer to have a new draft version that contains them.
>     Thanks,
>     --David
>     > -----Original Message-----
>     > From: Black, David
>     > Sent: Wednesday, December 22, 2010 9:26 PM
>     > To: [e-mail] <mailto:[e-mail]>
>     > Cc: Black, David
>     > Subject: MPA Draft - Review
>     >
>     > WG Last Call on this draft has run its course:
>     >
>     >                  Enhanced RDMA Connection Establishment
>     >                   draft-ietf-storm-mpa-peer-connect-02
>     >
>     > I've done my review as a WG chair (and the person who will be
>     shepherding this draft to the ADs and
>     > IESG):
>     >
>     > - This draft is on the right track, but has open issues.
>     > - Another version of the draft will be needed.
>     >
>     > Also, it would be greatly appreciated if a few people other than
>     the authors could take a look at
>     > this draft.  We have a very good author team on this draft,
>     whose expertise is beyond doubt, but
>     > more eyes on this draft would help.
>     >
>     > [1] My primary concern is that Section 9 on interoperability is
>     inadequate:
>     >
>     >    An initiator SHOULD NOT use the Enhanced DDP Connection
>     Establishment
>     >    formats or function codes when no enhanced functionality is
>     desired.
>     >
>     >    A responder SHOULD continue to accept the unenhanced connection
>     >    requests.
>     >
>     > The good news is that the first sentence is ok.
>     > The bad news is that the second sentence has significant problems:
>     >        - It uses SHOULD instead of MUST.
>     >        - It doesn't lay out behavior for initiator and responder
>     >                Revision mixes.
>     > IETF interoperability requirements are usually expressed with
>     MUST, including backwards
>     > compatibility.  If interop with unenhanced implementations is
>     only a SHOULD, that will need a
>     > convincing explanation.
>     >
>     > There are 3 Initiator/Responder cases that need attention
>     (New/Old, Old/New and New/New).  I think
>     > they lead to roughly the following:
>     >
>     > New/Old:
>     > - Explain error or failure that the New Initiator will see
>     because the Old responder
>     >        doesn't support Revision 2 of the MPA protocol.
>     > - Explain what the Initiator does when it sees that error or
>     failure.  The
>     >        easiest approach is to always retry with Revision 1, but
>     that won't work
>     >        if the Initiator has to send an RTR (that's the
>     "convincing explanation"
>     >        for why backwards compatibility is not always possible). 
>     The result
>     >        might be two requirements:
>     >        - If the Initiator has data to send, it MUST retry with
>     Revision 1.
>     >        - If the Initiator has no data to send, and hence has to
>     send an RTR,
>     >                the connection setup fails, the TCP connection
>     closes and that
>     >                failure MUST to be reported to the application.
>     >
>     > Old/New:
>     > - If a responder receives a Revision 1 message, it MUST respond
>     with a Revision 1 message.
>     >
>     > New/New:
>     > - If a responder receives a Revision 2 message, it MUST respond
>     with a Revision 2 message.
>     >
>     > I found a few other concerns:
>     >
>     > [B]In Section 7, we need to get the listing of all the SCTP
>     function codes into one place.  Either
>     > repeat the definitions of codes 1-4 from RFC 5043, or create an
>     IANA registry in Section 10 and list
>     > all 7 codes as its initial contents.
>     >
>     > [C] In Section 8, what happens if the responder sends an IRD or
>     ORD value that's different from the
>     > corresponding initiator value?  Is the responder allowed to
>     increase the value that was sent?  An
>     > important case to cover is that the initiator sends a valid
>     value (e.g., 0x2000) but the responder
>     > returns the 0x3FFF value indicating that negotiation is not
>     supported.  Also, what is the behavior
>     > of an IRD or ORD that is set to 0x0000?
>     >
>     > [D] In contrast, the Section 8 discussion of Control Flag
>     functionality is in better shape.  It
>     > would be helpful to add a sentence or two indicating when the
>     RTR occurs (Request ->, <- Reply, RTR
>     > ->), even though that is discussed earlier in the draft.  Also,
>     it's necessary to state whether
>     > negotiation of RTR functionality commits the Initiator to using
>     an RTR (e.g., suppose the initiator
>     > negotiates control flags to allow an RTR and instead sends an
>     FULPDU with payload data after
>     > receiving the Reply - is that ok or is it an error?).
>     >
>     > [E] Nit: In the definition of Control Flag A: ULPDU -> FULPDU
>     >
>     > Thanks,
>     > --David
>     > ----------------------------------------------------
>     > David L. Black, Distinguished Engineer
>     > EMC Corporation, 176 South St., Hopkinton, MA  01748
>     > [phone] <tel:%2B1%20%28508%29%20293-7953>            
>     FAX: [phone] <tel:%2B1%20%28508%29%20293-7786>
>     > [e-mail] <mailto:[e-mail]>        Mobile:
>     [phone] <tel:%2B1%20%28978%29%20394-7754>
>     > ----------------------------------------------------
>     _______________________________________________
>     storm mailing list
>     [e-mail] <mailto:[e-mail]>
>     https://www.ietf.org/mailman/listinfo/storm
>
>     _______________________________________________
>     storm mailing list
>     [e-mail] <mailto:[e-mail]>
>     https://www.ietf.org/mailman/listinfo/storm
>
>
>
>
> -- 
> Cheers,
> Arkady Kanevsky
>
>
> _______________________________________________
> storm mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/storm