Hi Murray, Thanks for the detailed reviewed! I've cc'ed int-area WG list also since GUE in a WG item there now.
A few replies are inline... On Wed, Mar 8, 2017 at 1:07 AM, Murray S. Kucherawy <[email protected]> wrote: > Hi all, > > Tom asked me to review this and some related drafts. These are mostly raw > notes; hopefully you can make some sense of them. If not, I'm happy to > clarify what any of them mean. > > The document seems to me (with my ART area eyes) to be generally in good > shape. The vast majority of my feedback is editorial stuff, but there are a > few key points I want to highlight: > > 1. In a few places you use MUST, which I assume is meant to indicate a > normative requirement. To do that in an RFC, you have to reference RFC2119 > and include in your Introduction section some boilerplate that it specifies. > Then, depending on how strict the IESG feels like being, all your uses of > MUST/SHOULD/SHOULD NOT/MAY (in lowercase or in all-caps) now need to be > all-caps (or changed to synonyms). In addition, sometimes they would like > you to explain, given a SHOULD or SHOULD NOT, when one might legitimately > deviate from that advice. You might even be asked to explain some of the > MUSTs if they aren’t already obvious. In my raw notes below I've called out > the lowercase must, may, etc. to which this applies. Will be more specific with normative language. > 2. There are several acronyms that need to be expanded and/or > references provided on first use. > 3. The IANA Considerations section establishes Tom as the reference > point for the port registration, but it uses an old work email address. If > that’s actually in the registry, this one should update it, preferably to > something that will be useful long-term. Yes, we will update contact information. > 4. Again with the IANA Considerations section, you’re using Standards > Action as the gate for registering anything. The IESG might ask you to > explain why such strict rules are being used. Specifically, why would one > of the lower requirements for registry changes be inadequate? Changed to "RFC required" > 5. Although I think I managed to figure out the rest of the document, > Section 3.6 confused me. Maybe if you explain what’s going on there I can > suggest some alternative text. Rewrote this a bit. > 6. As far as I know, appendices are never normative, so there’s no > need to say that. > Okay. > -MSK > > Abstract: > - such tunnels => such as tunnels > > Section 1: > - such as virtual => such as a virtual (or “the”) > > Section 1.1: > - don't end the "C-bit" entry with a period, or do it for all of them > > Section 2: > - expand OAM on first use > - what's going to happen when versions 2 and 3 are exhausted? > Then we need a new protocol :-). In reality, I think it is unlikely this will ever happen in the lifetime of the protocol. Because GUE is arbitrarily extensible, the version number is mostly a fail-safe so that if we found a deficiency in the primary GUE header we could fix this with a new version. Note that version 1 is a convenient means to do header compression for the most common use case and not really a another version, we don't foresee burning any more version numbers for anything like that. > Section 3.1: > - do we need to define or reference a definition for "local tuple"? > - define or reference ECMP > - "not applied" => "not applied," > - using "MUST" needs a reference to RFC2119 somewhere in Section 1 with the > usual boilerplate > - "C-bit: When set, indicates" ... > - Hlen: "header_len" is bytes, correct? > - Proto/ctype: "When the C-bit is set, this ..." > - "Flags." => "Flags:" > - "Private data: ... If private data are present, that portion of the > payload immediately follows the extension fields in the header. The length > ..." > > Section 3.2: > - where are control message types enumerated? > > Section 3.2.1: > - "When the C-bit is not set, the ..." > - When the outer IP protocol is IPv4, the proto ..." > - "the proto field can be set" > - "MUST NOT"? > > Section 3.2.2: > - "MUST" > - include a reference to where type 0 is defined > > Section 3.3: > - missing comma > - "may" > - "optional" > > Section 3.3.1: > - "may" > - why "may" a flag indicate the presence of an extension field? should that > be "some flags indicate..."? > - "must" > - "may" again > - don't understand "if two flag bits are paired, a field may possibly be > three different lengths" I added an example. > - "must" > - "... for example ..." is a run-on sentence > - "must not" > - "may" > - "... is set, which indicates ..." > - don't understand how I would know bits 17-19 are paired > > Section 3.4: > - "may" > - starting sentence with "Where" > - "... necessary, for instance it might contain ..." > - "... from an encapsulator, the packet ..." x2 > - "may" > > Section 3.5.1 > - "carry formatted message" => "carry formatted data" > - "may" > - the mdash needs a space before it > > Section 3.6: > - this part is confusing > - what is a “GUE Transform Field” and where is it defined? > - "... obfuscated, that is the transport..." needs a comma or mdash > - "a trivial destination options" > - Where is "PadN" defined? > - "In this case, 'Next header' MUST refer ..." > > Section 4.2: > - title says IPv6, first line says IPv4 > > Section 5: > - "Server 1" should be "Host 1" to match the diagram > > Section 5.3: > - "may" > - "for instance" > - "packets, that is encapsulating ... " needs an extra comma or an mdash > - "should" > - "GUE then" needs a comma > > Section 5.4: > - "may" > > Section 5.4.1: > - "received" needs a comma > - "in tact" > - comma before "which" between the boxes > > Section 5.4.2: > - comma after "received" > - "must" > > Section 5.5: > - when would one legitimately deviate from the "SHOULD NOT" advice? > - "may" > - "In this case the router..." > > Section 5.6: > - "may" > - comma after "For instance" > - "must not" > - "because" should be "merely because"? > > Section 5.6.1: > - "may" > - comma after "For instance" > - "may" > - "must" x3 > - "should" > > Section 5.6.2: > - "must" > - "may" > > Section 5.7: > - "must" => "needs to be" > - "may" > > Section 5.7.1: > - "must" > - "permissable" => "permissible" > - semicolon or period after "IPv4" > - comma after "IPv6" > - "may" > > Section 5.7.2: > - comma before "it SHOULD" > - comma after "By default" > - comma after "for instance" > > Section 5.7.3: > - comma after "IPv6" > - "must" x2; weird use of "must" before a list of non-MUSTard clauses > - "may" > - "should not" > - comma after "By default" > > Section 5.8: > - "should" > - missing period at end of first paragraph > - "must" > - "should" > > Section 5.9: > - "Note" should be "Note that" > - "may" > - "finer-grained" > - "may" > > Section 5.10: > - "may" > - "should" > - "may" > > Section 5.11.1: > - "finer-grained" > - "contents of clear-text packet" > - final paragraph is a run-on sentence > > Section 5.11.2: > - "should" x4, and when would you not? > - remove "to one", add a comma > - "may" x3 > - "should not", and when would you deviate? > > Section 5.12: > - "must" > - "optional" > - "but" or "however" before "the details" > - last bullet: "If security is used that would..." => "Use of security would > ..." > > Section 6.1: > - expand and reference all the acronyms in the last bullet > > Section 6.2: > - expand TCAM > > Section 7: > - end first bullet with a period > - "may" > > Section 8.1: > - do you still want to use your google.com address? what if you move from > FB? > - is draft-herbert-gue correct? > > Section 8.2-8.4: > - is Standards Action necessary? why not Specification Required? > > Sections 10.1 and 10.2: > - should be ordered > > Appendix A: > - aren't all appendices informational? > > Appendix A.1: > - "must" > - "should" x3 > - "out-of-order" > > Appendix A.2: > - comma after "encapsulation" > > Appendix A.2.1: > - "may" > - comma after "method" > - comma after "offload" > - comma after "method" > - "cover" should be "covered" > - maybe quote "not", as is done later in the section > - comma after "So", and maybe use "Thus" or "Therefore" > > Appendix A.3: > - comma before "which" > - parenthesize "if tunneling" > - "recommended" > - faulty mdash > - "should not" > > Appendix B.2: > - "should" > > Appendix B.3: > - don't understand the first sentence > > _______________________________________________ > nvo3 mailing list > [email protected] > https://www.ietf.org/mailman/listinfo/nvo3 > _______________________________________________ Int-area mailing list [email protected] https://www.ietf.org/mailman/listinfo/int-area
