Re: [storm] MPA Draft - Review
"Hemal Shah" <hemal@broadcom.com> Thu, 31 March 2011 18:43 UTC
Return-Path: <hemal@broadcom.com>
X-Original-To: storm@core3.amsl.com
Delivered-To: storm@core3.amsl.com
Received: from localhost (localhost [127.0.0.1]) by core3.amsl.com (Postfix) with ESMTP id ABBB328C0FC for <storm@core3.amsl.com>; Thu, 31 Mar 2011 11:43:58 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.598
X-Spam-Level:
X-Spam-Status: No, score=-2.598 tagged_above=-999 required=5 tests=[BAYES_00=-2.599, HTML_MESSAGE=0.001]
Received: from mail.ietf.org ([64.170.98.32]) by localhost (core3.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id qNBiB5qUADeJ for <storm@core3.amsl.com>; Thu, 31 Mar 2011 11:43:57 -0700 (PDT)
Received: from mms1.broadcom.com (mms1.broadcom.com [216.31.210.17]) by core3.amsl.com (Postfix) with ESMTP id E8C7628C0FB for <storm@ietf.org>; Thu, 31 Mar 2011 11:43:56 -0700 (PDT)
Received: from [10.9.200.131] by mms1.broadcom.com with ESMTP (Broadcom SMTP Relay (Email Firewall v6.3.2)); Thu, 31 Mar 2011 11:48:24 -0700
X-Server-Uuid: 02CED230-5797-4B57-9875-D5D2FEE4708A
Received: from IRVEXCHCCR02.corp.ad.broadcom.com ([10.9.200.183]) by IRVEXCHHUB01.corp.ad.broadcom.com ([10.9.200.131]) with mapi; Thu, 31 Mar 2011 11:45:26 -0700
From: Hemal Shah <hemal@broadcom.com>
To: "david.black@emc.com" <david.black@emc.com>, "storm@ietf.org" <storm@ietf.org>
Date: Thu, 31 Mar 2011 11:43:46 -0700
Thread-Topic: MPA Draft - Review
Thread-Index: AcuiSMs9BaSwGO4cTKyXNsSoFs8erRNZ1Z5AAAgmV1A=
Message-ID: <76DBE161893C324BA6D4BB507EE4C3849510935E96@IRVEXCHCCR02.corp.ad.broadcom.com>
References: <7C4DFCE962635144B8FAE8CA11D0BF1E03D74C7183@MX14A.corp.emc.com> <7C4DFCE962635144B8FAE8CA11D0BF1E03E5E9530C@MX14A.corp.emc.com>
In-Reply-To: <7C4DFCE962635144B8FAE8CA11D0BF1E03E5E9530C@MX14A.corp.emc.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
acceptlanguage: en-US
MIME-Version: 1.0
X-WSS-ID: 618A13F23TK599730-02-01
Content-Type: multipart/alternative; boundary="_000_76DBE161893C324BA6D4BB507EE4C3849510935E96IRVEXCHCCR02c_"
Subject: Re: [storm] MPA Draft - Review
X-BeenThere: storm@ietf.org
X-Mailman-Version: 2.1.9
Precedence: list
List-Id: Storage Maintenance WG <storm.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/listinfo/storm>, <mailto:storm-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/storm>
List-Post: <mailto:storm@ietf.org>
List-Help: <mailto:storm-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/storm>, <mailto:storm-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 31 Mar 2011 18:43:58 -0000
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: storm-bounces@ietf.org [mailto:storm-bounces@ietf.org] On Behalf Of david.black@emc.com Sent: Thursday, March 31, 2011 7:48 AM To: storm@ietf.org 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: storm@ietf.org > 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 > +1 (508) 293-7953 FAX: +1 (508) 293-7786 > david.black@emc.com Mobile: +1 (978) 394-7754 > ---------------------------------------------------- _______________________________________________ storm mailing list storm@ietf.org https://www.ietf.org/mailman/listinfo/storm
- [storm] MPA Draft - Review david.black
- Re: [storm] MPA Draft - Review arkady kanevsky
- Re: [storm] MPA Draft - Review david.black
- Re: [storm] MPA Draft - Review Hemal Shah
- Re: [storm] MPA Draft - Review david.black
- Re: [storm] MPA Draft - Review arkady kanevsky
- Re: [storm] MPA Draft - Review Steve Wise
- Re: [storm] MPA Draft - Review Steve Wise
- Re: [storm] MPA Draft - Review arkady kanevsky
- Re: [storm] MPA Draft - Review Steve Wise
- Re: [storm] MPA Draft - Review Sharp, Robert O
- Re: [storm] MPA Draft - Review arkady kanevsky
- Re: [storm] MPA Draft - Review arkady kanevsky
- Re: [storm] MPA Draft - Review Steve Wise
- Re: [storm] MPA Draft - Review arkady kanevsky
- [storm] MPA Draft - Implementations? david.black
- Re: [storm] MPA Draft - Implementations? arkady kanevsky