Re: [storm] MPA Draft - Review

[Steve Wise <[e-mail]>]

On 4/3/2011 4:36 PM, Steve Wise wrote:
> Hey Arkady,
>
> It does seem like you did the section 9 changes Bob and I requested:

To clarify: please modify section 9 as I've shown below..


> ----
>   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
>
>
> _______________________________________________
> storm mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/storm