Re: [storm] MPA Draft - Review

arkady kanevsky <arkady.kanevsky@gmail.com> Mon, 04 April 2011 23:24 UTC

Return-Path: <arkady.kanevsky@gmail.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 580ED3A6805 for <storm@core3.amsl.com>; Mon, 4 Apr 2011 16:24:20 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -3.598
X-Spam-Level:
X-Spam-Status: No, score=-3.598 tagged_above=-999 required=5 tests=[BAYES_00=-2.599, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-1]
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 0OK-YoMNTNpw for <storm@core3.amsl.com>; Mon, 4 Apr 2011 16:24:18 -0700 (PDT)
Received: from mail-pv0-f172.google.com (mail-pv0-f172.google.com [74.125.83.172]) by core3.amsl.com (Postfix) with ESMTP id 4FB8D3A67ED for <storm@ietf.org>; Mon, 4 Apr 2011 16:24:18 -0700 (PDT)
Received: by pvh1 with SMTP id 1so1044559pvh.31 for <storm@ietf.org>; Mon, 04 Apr 2011 16:26:01 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=YCTPdUYGPgPIE4fY9BNVdHVUlY0PPYzw2U8QDDjsoLE=; b=nDuKY+LYAqk7YWQbkxDI7KEW/p++MrBdqJddhQowYRnL/dlg1Qo8Z/AF/h2SQ9JEWC 5BbN6OiEyKXGvwSLUQeBwBzKN6H4CjoAVI1KrZeZBUibJsxhsONdn9h8NDReMLsv6pP9 QXz8c3/UWdlm9Yq0CxIvlL7b5QsBr5A8oUFNs=
DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; b=nd+PtRnn3bolRFzY4f/SQ0xIDsX4kiDmhdywuD2OfdMSHMc8WEEMACBTF+bGZLjydu VYUCVG3Y6VRXT81RWj/ajnAGIdGBPrHwCh9Tro7PkcaeXQv2yH5C0GAGUlOXb8rtXBs2 Ka1VGL2XUpGBTFn50LQ/Hwv65bOHdox0Drh44=
MIME-Version: 1.0
Received: by 10.142.44.5 with SMTP id r5mr6683117wfr.300.1301959561096; Mon, 04 Apr 2011 16:26:01 -0700 (PDT)
Received: by 10.142.185.20 with HTTP; Mon, 4 Apr 2011 16:26:00 -0700 (PDT)
In-Reply-To: <4D98E86A.60403@opengridcomputing.com>
References: <7C4DFCE962635144B8FAE8CA11D0BF1E03D74C7183@MX14A.corp.emc.com> <7C4DFCE962635144B8FAE8CA11D0BF1E03E5E9530C@MX14A.corp.emc.com> <76DBE161893C324BA6D4BB507EE4C3849510935E96@IRVEXCHCCR02.corp.ad.broadcom.com> <BANLkTimmcdA6XahDQxS7d31ftoV+fbiAOw@mail.gmail.com> <4D98E86A.60403@opengridcomputing.com>
Date: Mon, 4 Apr 2011 19:26:00 -0400
Message-ID: <BANLkTimYTJF_ZmcR3ctu=kFUPB17dsrrtA@mail.gmail.com>
From: arkady kanevsky <arkady.kanevsky@gmail.com>
To: Steve Wise <swise@opengridcomputing.com>, "Sharp, Robert O" <robert.o.sharp@intel.com>
Content-Type: multipart/alternative; boundary=000e0cd2e00e26b05704a020151d
Cc: storm@ietf.org
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: Mon, 04 Apr 2011 23:24:20 -0000

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 <swise@opengridcomputing.com>wrote;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 <hemal@broadcom.com> 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: storm-bounces@ietf.org [mailto:storm-bounces@ietf.org<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 mailing list
>> storm@ietf.org
>> https://www.ietf.org/mailman/listinfo/storm
>>
>>
>
>
> --
> Cheers,
> Arkady Kanevsky
>
>
> _______________________________________________
> storm mailing liststorm@ietf.orghttps://www.ietf.org/mailman/listinfo/storm
>
>
>


-- 
Cheers,
Arkady Kanevsky