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, 03 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: > 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
- [storm] AD Review of draft-ietf-storm-mpa-peer-co… David Harrington
- Re: [storm] AD Review of draft-ietf-storm-mpa-pee… arkady kanevsky
- Re: [storm] AD Review of draft-ietf-storm-mpa-pee… arkady kanevsky
- Re: [storm] AD Review of draft-ietf-storm-mpa-pee… arkady kanevsky
- Re: [storm] AD Review of draft-ietf-storm-mpa-pee… David B Harrington
- [storm] draft-ietf-storm-mpa-peer-connect - revie… Steve Wise
- Re: [storm] draft-ietf-storm-mpa-peer-connect - r… david.black