Re: [storm] AD Review of draft-ietf-storm-mpa-peer-connect

arkady kanevsky <arkady.kanevsky@gmail.com> Sat, 04 June 2011 01:29 UTC

Return-Path: <arkady.kanevsky@gmail.com>
X-Original-To: storm@ietfa.amsl.com
Delivered-To: storm@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 6B10DE0752 for <storm@ietfa.amsl.com>; Fri, 3 Jun 2011 18:29:21 -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.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ka41xtykdxZA for <storm@ietfa.amsl.com>; Fri, 3 Jun 2011 18:29:19 -0700 (PDT)
Received: from mail-iw0-f172.google.com (mail-iw0-f172.google.com [209.85.214.172]) by ietfa.amsl.com (Postfix) with ESMTP id 7F949E071A for <storm@ietf.org>; Fri, 3 Jun 2011 18:29:19 -0700 (PDT)
Received: by iwn39 with SMTP id 39so2596880iwn.31 for <multiple recipients>; Fri, 03 Jun 2011 18:29:19 -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=l6pkQxkYV+nKLAd6FETz8KnzuEdCIvbCOyijd2SZp/M=; b=sF3HQnvf3FTTOt58oYMVWJuxOzKWVHRGn9cF5ExLNEd2nZztyppFb4E8cKLBs1Lpu/ NZdlDc7Ueo5tW6rpWlmYXdG3o1BFpQM1PS4dPovgh4mCUBo5QHtxpAbgIFX8flUaiHjo 0OovOHr6Cf3LEe1kRviElDU0C/PNlFL2OW4uc=
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=BiJXxFpvHZJuUfIISq4ByV2u+dXL+sSO7RE8PO9LXOZYymaZnDx7gtMjEYGvhXVMSc WhFbF1zIb/dxpncU8T7wnRo9PK2IJT98k5J0n2TNjhM4bVKVUWOrnrGbXWC2OOtiwoxd jItSZ36ijnFRSFhPnCi+eElr3DB6r5L3FB2zc=
MIME-Version: 1.0
Received: by 10.231.74.7 with SMTP id s7mr3511150ibj.172.1307150958793; Fri, 03 Jun 2011 18:29:18 -0700 (PDT)
Received: by 10.231.199.132 with HTTP; Fri, 3 Jun 2011 18:29:18 -0700 (PDT)
In-Reply-To: <B7F58B09EC4B43D7BF8BFA9FA95A8061@davidPC>
References: <AcwW/3OnDAJozBeCSrOODdtKJWAnhg==> <B7F58B09EC4B43D7BF8BFA9FA95A8061@davidPC>
Date: Fri, 3 Jun 2011 21:29:18 -0400
Message-ID: <BANLkTi=sUfHteXgDyHf1cngh3cVpu=YEwQ@mail.gmail.com>
From: arkady kanevsky <arkady.kanevsky@gmail.com>
To: David Harrington <ietfdbh@comcast.net>
Content-Type: multipart/alternative; boundary=000e0cd6b24491159804a4d8cc39
Cc: storm-chairs@ietf.org, draft-ietf-storm-mpa-peer-connect@tools.ietf.org, storm@ietf.org
Subject: Re: [storm] AD Review of draft-ietf-storm-mpa-peer-connect
X-BeenThere: storm@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: Storage Maintenance WG <storm.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/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: Sat, 04 Jun 2011 01:29:21 -0000

David,
Thank you for a very good comments and careful reading of a draft.
We are working on the next version which address your comments, while tardy
on responding to the email.
Thanks,
Arkady

On Fri, May 20, 2011 at 11:06 AM, David Harrington <ietfdbh@comcast.net>wrote;wrote:

> AD Review of draft-ietf-storm-mpa-peer-connect
>
> I have reviewed this document and have comments. I think this work is
> important, but the way this document explains it could be better.
>
> My goal as AD is to have this document go through IETF Last Call and
> IESG review without any issues delaying advancement.
>
> Some comments are technical/process comments that refer to text that
> will likely cause delays during IESG Evaluation.
>
> Some are stylistic, such as capitalization and wording consistency,
> that are distracting and are likely to make readers wonder "is there
> something special here that I need to understand?" and to waste time
> trying to determine if there is or is not something special that led
> to your capitalizing the text, or using different wording than
> elsewhere. These could lead to IESG members raising unnecessary
> questions that would delay advancement.
>
> Others are editorial in nature, mostly about writing clear and
> unambiguous text. Generally, if it is simply a small editorial thing,
> I ignored it, expecting the RFC Editor to clean up the text a bit. But
> sometimes, the lack of clarity could lead to differing interpretations
> of the text, or lack of understanding of the text, and those should be
> fixed before I bring this to the IESG.
>
> In some cases, I have asked a question. If I had to ask a question,
> then the document apparently didn't explain it clearly enough. The
> answer to the question usually needs to be explained "in the
> document", not just in a private email to me.
>
> 1) idnits complains about unreachable reference:
> http://www.rdmaconsortium.org/home/draft-hilland-iwarp-verbs-v1.0-RDMA
> C.pdf
>
> 2) abstracts cannot contain references.
>
> 3) in 4.3.2, the conclusion could be stated more clearly, such as "If
> a nop approach was used, it would require lower layers to know the
> usage model, and would disrupt the calculations ..."
>
> 4) in 4.3.3, I have no idea what this text is saying or why it is
> relevant to the problem at hand. I don't know what an untagged message
> is; I don't know why a WC can only be generated by an untagged
> message. Can you expand on this so readers that are not RDMA experts
> understand what you're talking about? The IESG will need to review and
> approve this document, and to my knowledge none of them are RDMA
> experts.
>
> 5) It would be helpful to explain why taking the bit from RES is
> backwards compatible.
> Explain why older implementation will not have a problem with the new
> bit, and why new implemntations will be able to benefit from the new
> bit.
>
> 6) Rev "MUST be set to two"; would this be better as "two or higher?"
> to allow for a version three+ that would be backward compatible with
> the enhanced features of version two?
>
> 7) I don't understand "The Enhanced Reject function code SHOULD be
> used to indicate a configuration that would have been accepted." Would
> have been accepted except was not because of what condition? Why is
> this only a SHOULD and not a MUST?
>  8) How can the private data be zero length if it must begin with
> Enhanced RDMA Connection establishment data?
> 9) sometimes "Enhanced RDMA Connection Establishment Data" is
> capitalized, sometimes not. Be consistent.
> 10) Received extended ... message SHOULD be reported to the ULP. Why
> is this not a MUST? Anytime there is a SHOULD, the text should
> identify when the rule is not mandatory - what are the valid
> exceptions that make this not a MUST?
> 11) What is "enhanced DDP stream management"?
> 12) "It should be noted that" - you're noting it, so this lead-in
> phrase is totally unnecessary. If you leave it as "It should be noted
> that", then explain WHY it should be noted. If "it is especially
> important to recognize that" then say that, and explain WHY it is
> important to recognize.
> It appears that it is important only because you've made a case for
> peer-to-peer needing to initiate from either side, and the SCTP
> adaption layer allows for that already. So you should probably explain
> why you're duplicating that capability.
> 13) additional legal sequences - but then you don't seem to define any
> **sequences**. Is "Legal Sequences" a special term? If so, please
> explain where it is defined. if not, why is it capitalized?
> 14) I cannot parse "The protocol layers on which RDMA connection
> establishment is layered upon [RFC5040] and [RFC5041] define layers
> and error types."
> 15) if 5043 and 5044 do not define error codes, then how are the error
> codes used? are they carried in messages? which messages?
> 16) In looking for the answer to 15, I found that error codes ARE
> defined in RFC5044 section 8. So your text is apparently wrong.
> 17) in section 9, the responder SHOULD NOT set its IRD higher than
> Initiator ORD. Why is this not a MUST? Under what conditions is it
> acceptable to set this higher than Initiator ORD? What are the
> implications of doing so?
> the responder SHOULD set its ORD lower. What are the valid exceptions
> to this?
> 18) the Initiator MUST set its IRD to a value at least as large as the
> responder ORD if it has sufficient resources
> "MUST if" means this is a SHOULD, and lack of resources is the
> exception. If this is truly a MUST (which the following sentences seem
> to say, requiring a termination if ti doesn't do so, then I would
> remove the if clause from the first sentence, making it a definite
> MUST.
> 19) "Control flag for using" doesn't discuss whatthe values are. Does
> this mean that if A has a value 1, then FULPDU has a zero length?
> This section might be better organized by defining what the fields are
> (control flags and depths), then describing how the values are set.
> You do that for A,B,C but describe IRD and ORD in-line.
> 20) In section 9, "If there is no Standard RDMAP Configuration Data in
> the MPA Reply Frame, and the Enhanced Connection Establishment version
> number is used, it is the equivalent of setting 'A', 'B' and 'C'. "
> Why do we need this extra complexity? Why not require that these be
> set ONE way, not two? I also have concerns about "THE enhanced
> connectiosn establshment version number"; will this document need to
> be updated if a version three is ever developed?
> 21) "Setting no Control Flags in the MPA Reply indicates that an RDMA
> Send message will be required. As this option will require the
> initiator ULP to be involved it SHOULD NOT be used unless necessary. "
> Please define what necessary is. I think this would be better as a
> MUST NOT.
> 22) "Initiator or Responder SHOULD generate the TERM message" - why is
> thi snot a MUST? What are the valid exceptions to this?
> in section 10:
> 23) "An initiator SHOULD NOT use the Enhanced DDP Connection
> Establishment formats or function codes when no enhanced functionality
> is desired. " what are the valid exceptions that make this a SHOULD
> rather than a MUST?
> 24) the paragraph containing "If a responder does not support received
> extended MPA message, then it MUST close the TCP connection and exit
> MPA since MPA frame is improperly formatted for it as stated in
> [RFC5044]." needs a bit of English cleanup.
> 25) section11 This document defines error codes; so does RFC5044.
> Implementers and operators will need to hunt through documents to find
> the various error codes. This document apparently overlooks the error
> codes defined in 5044 section 8. Should there be a registry,
> maintained by IANA, for the MPA error codes?
> section 12:
> 26) I am always concerned when a security coinsiderations section says
> this document does not introduce any new security considerations. This
> is also a red flag during IESG Evaluation. I suggest you document WHY
> these changes do not impact security compared to 5043 and 5044.
> Demonstrate that the WG actually **considered** the security issues.
>
> 27) expand acronyms on first use (MPA, RDMA, RDDP, ULP, iWARP, etc.)
>
> 28) references for sctp, infiniband,
>
> 29) in 4.3.1, "required only for one transport" - mention the one
> transport?
>
> 30) an explanation/definition of MPA Fencing would help readers. Maybe
> a terminology section would be helpful, given all the acronyms, etc.
>
> 31) The information in 4.4 would be helful in the Introduction
> section. This would help to organize the document into "Here's how it
> works now; here is why the client/server model is undesirable in a P2P
> environment; and this is how to modify the existing standard to
> address this problem."
>
> 32) Is section 5 a description of how things currently work, or the
> way this document proposes they work? Section 5 mentions "Enhanced
> Connection Setup"; is that the same as "Enhanced RDMA Connection
> Establishment"? if so, then naming section 5 "Enhanced RDMA Connection
> Establishment" or "Enhanced MPA Connection Setup" might be helful. Use
> terminology consistently, please.
>
> I hope these suggestions help improve the document so we can advance
> it through the approval process quickly,
>
>
>
>
>
>
>
>
>
> David Harrington
> Director, IETF Transport Area
> ietfdbh@comcast.net (preferred for ietf)
> dbharrington@huaweisymantec.com
> +1 603 828 1401 (cell)
>
> _______________________________________________
> storm mailing list
> storm@ietf.org
> https://www.ietf.org/mailman/listinfo/storm
>



-- 
Cheers,
Arkady Kanevsky