Peter, thank you for your review. I have entered a Discuss ballot for this document based on my own review.
Lars > On 2022-5-28, at 23:59, Peter Yee via Datatracker <nore...@ietf.org> wrote: > > Reviewer: Peter Yee > Review result: Ready with Issues > > I am the assigned Gen-ART reviewer for this draft. The General Area Review > Team > (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF > Chair. Please treat these comments just like any other last call comments. > > For more information, please see the FAQ at > > <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. > > Document: draft-ietf-ipsecme-iptfs-12 > Reviewer: Peter Yee > Review Date: 2022-05-27 > IETF LC End Date: 2022-05-18 > IESG Telechat date: Not scheduled for a telechat > > Summary: This draft specifies an improved method for countering traffic > analysis of IPsec tunnels. There are some nits and minor issues that should be > addressed. I did not evaluate the appendices for correctness. [Ready with > issues.] > > Major issues: None > > Minor issues: > > Page 7, 3rd paragraph, 1st sentence (and elsewhere in the document): You make > reference to the “user” and what the “user” is supposed to do. I can’t begin > to > imagine an ordinary user coming up with an optimal window size or do some of > the other things that are being required. Do you really want to put this > requirement on a user, or should it be a different entity, such as the IP-TFS > implementation? > > Page 13, 1st partial paragraph: How would the referenced AGGFRAG_PAYLOAD empty > payload be recognized? The ESP Next Header won’t indicate that the contents is > an AGGFRAG_PAYLOAD because the SA isn’t an AGGFRAG_PAYLOAD SA. > > Page 13, 2nd full paragraph: the unnumbered figure from page 17 would be > really > helpful here given how many disparate header fields are referenced in this and > the following paragraphs. Page 15, section 6.1: RFC 4303 says, “The Next > Header > is a mandatory, 8-bit field that identifies the type of data contained in the > Payload Data field, e.g., an IPv4 or IPv6 packet, or a next layer header and > data. The value of this field is chosen from the set of IP Protocol Numbers > defined on the web page of the IANA, e.g., a value of 4 indicates IPv4, a > value > of 41 indicates IPv6, and a value of 6 indicates TCP.” Thus, I don’t believe > you can arbitrarily choose 0x5. See the registry at > https://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml. > > Nits/editorial comments: > > General: > > Insert a hyphen between “congestion” and “controlled” throughout the document. > This includes the “non-“ cases as well. > > Insert a hyphen between “AGGFRAG_PAYLOAD” and “enabled” throughout the > document. This includes the “non-“ cases as well. > > Change “inner-packet” to “inner packet”. The latter already predominates > throughout the document, noting also that “outer packet” never appears in > hyphenated form. > > Ensure that all the figures have proper captions with numbers. For example, > the > figures on pages 16, 17, 18, and 19 aren’t labeled. The figure on page 17 > could > really use a number so that there can be a pointer from page 13 if the figure > isn’t moved to page 13, as suggested above. > > Specific: > > Page 1, Abstract, 1st sentence: change “payload” to “payloads”. Or change “ESP > payload” to “an ESP payload”. I can see arguments either way, but the sentence > needs one or the other. > > Page 5, 1st paragraph, 2nd sentence: delete a duplicated “the” before “tunnel > packets”. > > Page 5, Figure 1: change “subtype” to “sub-type” to match usage in the rest of > the document. > > Page 6, 3rd paragraph: append a comma after “outer”. > > Page 7, 4th paragraph, last sentence: “one” who? What entity is supposed to be > making this choice? > > Please 7, 4th paragraph, 1st sentence: append a comma after “note”. > > Page 7, 4th paragraph, 3rd sentence: delete “amount of”. > > Page 7, 5th paragraph, 1st sentence: consider changing “with no gaps” to > “consecutively”. > > Page 8, section 2.2.3.1, 1st paragraph, last sentence: delete the comma after > “researching”. > > Page 9, section 2.2.5.3, 1st sentence: append a comma after “default”. > > Page 10, section 2.3, 1st sentence: insert a hyphen between “AGGFRAG_PAYLOAD” > and “enabled”. > > Page 10, section 2.4.1, 2nd paragraph, 3rd sentence: append a comma after > “case”. Append a period after “etc”. > > Page 10, section 2.4.2, 1st paragraph, 2nd sentence: append a comma after > “transport”. > > Page 11, 1st partial paragraph: change “packet” to “packets”. Append a closing > parenthesis after “congestion”. > > Page 11, 1st full paragraph, 1st sentence: insert a hyphen between “TCP” and > “friendly”. > > Page 11, 3rd paragraph, 1st sentence: insert a hyphen between “IP-TFS” and > “enabled”. > > Page 11, 4th paragraph: append a comma after the closing parenthesis. > > Page 12, 1st partial paragraph, 1st full sentence: delete this sentence as it > doesn’t really add anything. But if you are unwilling to delete the sentence, > then change “are” to “is”. > > Page 12, section 2.5, 1st paragraph: insert a hyphen between “AGGFRAG” and > “enabled”. > > Page 12, section 2.5, 2nd paragraph, 3rd sentence: append a comma after “For > partial packets”. Delete “the” before “they”. > > Page 12, section 2.5, 2nd paragraph, 4th sentence: insert “the” before > “AGGFRAG_PAYLOAD”. > > Page 12, section 2.5, 3rd paragraph, 1st sentence: insert “an” before > “in-order”. > > Page 12, section 2.5 3rd paragraph, 2nd sentence: change “make sure” to > “ensure”, if you care. “Tastes light” vs. “less filling”, I suppose. Change > “in-order” to “in order”. Insert “a” between “when” and “lost”. Also consider > breaking up the sentence into multiple sentences because of its sheer length. > For example, the final parenthetical potion could be a whole sentence on its > own. > > Page 12, section 3, 2nd sentence: change “it’s” to “its”. > > Page 13, 2nd full paragraph, 2nd sentence: change “locally, subsequent” to > “locally. Subsequent”. > > Page 13, 3rd paragraph, 1st sentence: expand the initialism “CC”. I’m assuming > “Congestion Control”. It’s not in the RFC Editor’s list of abbreviations. > > Page 13, 4th paragraph, 3rd sentence: change “senders” to “sender’s”. > > Page 16, section 6.1.1, 1st paragraph: change “4 octet” to 4-octet”. > > Page 16, section 6.1.1, “Reserved” definition: delete the comma after > “generation”. > > Page 17, section 6.1.2, “Reserved” definition: delete the comma after > “generation”. > > Page 17, section 6.1.2, “P” and “E” definitions: insert “that” before “if”. > > Page 18, “Echo Delay” and “Transmit Delay” definitions, 2nd sentence: change > “value” to “delay” because by definition, the value cannot be larger than > 0x1FFFFF, while the delay can be. Change “it” to “the value”. > > Page 18, “Datablocks” definition: 2nd sentence: change “an” to “a”. Insert a > hyphen between “non-IP-TFS” and “enabled”. Consider changing “value” to > “field” > because DataBlocks isn’t really a value. > > Page 19, section 6.1.3.1, figure: shouldn’t the “TypeOfService” field be the > “DiffServ” field instead? > > Page 20, section 6.1.4, “0” definition: delete the comma. > > Page 21, 1st paragraph, last sentence: change “it’s” to “its”. > > Page 22, section 8, 1st paragraph, 1st sentence: change “it” to “its”. > > Page 22, section 8, 3rd paragraph: append a comma after “maintained” and after > “would be”. > > Page 24, Appendix A, title: change “Of” to “of”. > > Page 24, Appendix A, 1st paragraph, 1st sentence: append a comma after > “Below”. > > Page 25, Figure 3: Explain what the 1500 means. > > Page 25, 1st paragraph: change “800 octet” to “800-octet” twice. Make a > similar > change for “60”, “240”, and “4000”. > > Page 25, 2nd paragraph, 2nd sentence: place “ESP1” in parentheses. > > Page 25, 2nd paragraph, 3rd sentence: change “packet ESP2s” to “packet’s > (ESP2)”. Change “60 octet” to “60-octet”. > > Page 25, 2nd paragraph, 4th sentence: place “ESP3” in parentheses. Change > “4000 > octet” to “4000-octet”. Change “forth” to “fourth”. > > Page 25, 2nd paragraph, 5th sentence: change “packet ESP4s” to “packet’s > (ESP4)”. Append a comma after “1400”. Change “4000 octet” to “4000-octet”. > > Page 25, Appendix B, 1st sentence: change “TCP friendly” to “TCP-friendly”. > > Page 25, Appendix B, 2nd sentence: change “TCP friendly” to “TCP-friendly”. > > Page 25, Appendix B, 3rd sentence: append a comma after “[RFC4342])”. > > Page 25, Appendix B, 3rd paragraph: append a comma after “addition”. > > Page 26, 1st paragraph, 2nd sentence: append a comma after “[RFC5348]”. > > Page 26, section C.1, 1st paragraph, 1st sentence: append a comma after > “overhead”. > > Page 26, section C.1.1, 1st sentence: append a comma after “For comparison”. > Insert “an” before “AGGFRAG”. > > Page 26 section C.1.1, 2nd sentence: append a comma after “Therefore”. Change > “fractional” to “fractions”. > > Page 27, 1st formula: change “Paylaod” to “Payload”. > > Page 28, section C.3, 3rd sentence: insert a hyphen between “well” and > “understood”. > > Page 28, section C.3.1, 2nd sentence: change the second “and” to “an”. Append > a > comma after the closing parenthesis. > > Page 28, section C.3.1, 3rd sentence: append a comma after “Additionally”. > > Page 30, 1st paragraph, 1st sentence: append a hyphen after “small”. Insert a > hyphen between “medium” and “sized”. > > > _______________________________________________ > Gen-art mailing list > gen-...@ietf.org > https://www.ietf.org/mailman/listinfo/gen-art
signature.asc
Description: Message signed with OpenPGP
_______________________________________________ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec
