Re: [storm] MPA Draft - Review

[arkady kanevsky <[e-mail]>]

Submitted!

On Tue, Apr 5, 2011 at 10:37 AM, Sharp, Robert O
<[e-mail]>wrote:

>  Looks good to me as well.
>
>
>
> Thanks,
>
> Bob
>
>
>
> *From:* Steve Wise [mailto:[e-mail]]
> *Sent:* Monday, April 04, 2011 9:19 PM
> *To:* arkady kanevsky
> *Cc:* Sharp, Robert O; [e-mail]
>
> *Subject:* Re: [storm] MPA Draft - Review
>
>
>
> Looks good.
>
> Bob Sharp should ACK this too though.
>
> Thanks,
>
> Steve.
>
>
> On 4/4/2011 8:50 PM, arkady kanevsky wrote:
>
> Got it. Thanks Steve.
> How about now?
> Arkady
>
> On Mon, Apr 4, 2011 at 9:41 PM, Steve Wise <[e-mail]>
> wrote:
>
> The key word to remove here is "responder".  The IRD in the MPA start
> request is the initiators desired IRD _of the initiator_.  Not the
> initiators desired _responder_ IRD.
>
> If you want to change "desiired" to "initial" thats ok with me.  But the
> key is to get rid of the word "responder" in that sentence.
>
> Steve.
>
>
>
> On 4/4/2011 6:26 PM, arkady kanevsky wrote:
>
> Steve and Bob,
> I changed it to
> "In request: the Initiator desired responder IRD
> for the connection." as you asked.
> I can change it to "initial" instead of "desired".
> Arkady
>
> On Sun, Apr 3, 2011 at 5:36 PM, Steve Wise <[e-mail]>
> wrote:
>
>  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]> 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]<[e-mail]>]
> On Behalf Of [e-mail]
> Sent: Thursday, March 31, 2011 7:48 AM
> To: [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]
>
> > 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]             FAX: [phone]<%2B1%20%28508%29%20293-7786>
>
> > [e-mail]        Mobile: [phone]<%2B1%20%28978%29%20394-7754>
>
> > ----------------------------------------------------
>
>
>
> _______________________________________________
>
> storm mailing list
>
> [e-mail]
>
> https://www.ietf.org/mailman/listinfo/storm
>
>
>
>
>
>
> _______________________________________________
> storm mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/storm
>
>
>
>
> --
> Cheers,
> Arkady Kanevsky
>
>
>
> _______________________________________________
>
> storm mailing list
>
> [e-mail]
>
> https://www.ietf.org/mailman/listinfo/storm
>
>
>
>
>
>
> --
> Cheers,
> Arkady Kanevsky
>
>
>
>
>
>
> --
> Cheers,
> Arkady Kanevsky
>
>
>



-- 
Cheers,
Arkady Kanevsky