[Teas] Genart last call review of draft-ietf-teas-rsvp-ingress-protection-13

[Robert Sparks <[e-mail]>]

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"?