Reviewer: Robert Sparks Review result: Not Ready 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-teas-rsvp-ingress-protection-13 Reviewer: Robert Sparks Review Date: 2018-02-21 IETF LC End Date: 2018-03-01 IESG Telechat date: 2018-03-08 Summary: Not Ready for publication as an Experimental RFC I found this document hard to read. I encourage a significant editorial pass that focuses on presenting the core ideas early, and simplifies the language used throughout the document. Major Issues: 1) The IANA considerations section is unclear. You ask IANA to put something in "Class Names, Class Numbers, and Class Types" at <http://www.iana.org/assignments/rsvp-te-parameters/rsvp-te-parameters.xhtml>. But there is no registry with that title on that page. (Nor is there a registry with a structure matching the row you are asking IANA to add.) Did you mean the registry at <https://www.iana.org/assignments/rsvp-parameters/rsvp-parameters.xhtml#rsvp-parameters-4>? If so, the number you are suggesting is in a "Reserved for Private Use" range. If that _was_ your target, the structure of your requested record does not match the structure of the existing registry. Please clarify. The instructions to IANA for what to do when this document moves to the standards track are inappropriate. You can say that you anticipate that the future document that moves the idea to the standard track expects to create a new registry under rsvp-te-parameters, but this document cannot instruct IANA to do so. 2) "After one approach is selected, the document SHOULD become proposed standard." is an innappropriate use of SHOULD, and doesn't correctly reflect the process that would be followed in any case. You could, instead, say "After one approach is selected, a document revising the ideas here to reflect that selection and any other items learned from the experiment is expected to be submitted for publication on the standards track." 3) It is unclear until you get to section 5 (11 pages into the document) which elements provide and which elements consume the information in the proposed new object. The document is inconsistent about which messages the object can/should appear in. Moving the overview of behavior earlier in the document would help. Simplifying the description of that behavior will help more. Making the other sections consistent with what that overview describes is necessary. Minor Issues: a) It is not clear what the difference between source-detect and backup-source-detect really are (I agree with the comments in the rtgdir review on these sections). In section 2.1, where you say "reliably detect", do you really mean "verify"? The detection has already been done by the source. b) Section 4.1 is particularly hard to extract roles and responsibilities from. Calling out early which elements will use the object and in which messages will clarify the definition of behavior. Perhaps you should more carefully separate the definition of the object from normative statements about how the object gets used. c) The document is vague about source behavior, though it requires behaviors of the source when it detects failure, and when a previously failed primary becomes available again. A clearer description of what you are requiring of the source, and what you are intentionally leaving unspecified would help. d) The MUST in the first bullet of section 5.2 is unclear. As written it's an incorrect use of 2119. Are you trying to say that the primary ingress must know the IP address of the backup it wants to be used before it can use the INGRESS_PROTECTION object, or are you trying to say that if primary ingress doesn't know the address of the backup, it MUST NOT include a backup ingress address sub-object? Nits and editorial comments: FRR is used in the title, but does not appear in the abstract. (The abstract could say more about what the document is about.) The first sentence of the second paragraph of the Introduction is complex. Please break it apart into several sentences. In the introduction, please delete ", which are briefed as follows". The introduction to the issues can simply start "There are a number of issues in this solution:". At the second bullet in this list of issues, I suggest "configuration" instead of "configurations". At the third bullet in this list of issues, what does "the BFD down" mean? In section 3, I suggest replacing ", we call it is off-path" with "it is off-path". Please make a similar change to the sentence about on-path. In the third sentence, why do you say "configured or computed"? What does "computed" mean in this context? In the introduction to section 4, say _what_ the new object is backwards compatible with. In section 4.1, it's unclear what "keeps it" means. Please try reworking this sentence without using "it". It was not clear in section 4.1.1 that a backup would use the presence of the backup ingress IPv4 (or 6) sub-objects to discover that the primary wanted it to behave as a backup (as described in section 5.3). In the 3rd paragraph of section 5.1.1, do you mean "efficient processing" where you say "efficient process"? The last paragraph of 5.1.1 does not help the document. "simple" compared to what? "less" compared to what? I suggest deleting the paragraph, or significantly expanding on the discussion. At the first sentence of the last paragraph of 5.1.2, did you mean "required" instead of "requires"? At the second paragraph of 5.3, what are "administrative-colors"?
