Hi Robert,
Thank you very much for your time and valuable comments.
Answers to them are inline below with prefix [HC].
Would you mind reviewing them to see if they address the issues?
Best Regards,
Huaimo
-----Original Message-----
From: Robert Sparks [mailto:[email protected]]
Sent: Wednesday, February 21, 2018 12:20 PM
To: [email protected]
Cc: [email protected]; [email protected];
[email protected]
Subject: Genart last call review of draft-ietf-teas-rsvp-ingress-protection-13
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.
[HC] We have presented the core ideas early and simplify the language used in
the document accordingly.
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>?
[HC] Yes. You are right. We have corrected it in the document.
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.
[HC] We are considering a new C-Type under the existing Protection Class.
The related text has be updated accordingly as below:
Upon approval of this document, IANA is requested to assign a new
Class Type or C-Type under Class Number 37 and Class Name PROTECTION
located at <https://www.iana.org/assignments/rsvp-parameters/
rsvpparameters.xhtml#rsvp-parameters-39>, as follows:
Value Description Reference
----- ----------- ---------
4 Type 4 INGRESS_PROTECTION This Document
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.
[HC] We have revised the text according to your suggestion as below:
It is anticipated that the future document that moves the idea to the
standard track expects IANA to create and maintain a new registry under
PROTECTION object class, Class Number 37, C-Type 4. Initial values for
the registry are given below. The future assignments are to be made through
IETF Review.
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."
[HC] We have updated the text accordingly as below:
After one approach is selected, the document will be revised
to reflect that selection and any other items learned from
the experiment. The revised document 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.
[HC] We have corrected the inconsistency and updated the document according to
your suggestions.
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.
[HC] Yes. We have used "verify" instead of "reliably detect" and updated the
text accordingly. We have added more details on source-detect and
backup-source-detect as below:
Note that one of the differences between Source-Detect and Backup-
Source-Detect is: in the former, the backup ingress verifies the
failure of the primary ingress slowly such as in seconds; in the
latter, the backup ingress detects the failure fast such as in a few
or tens of milliseconds.
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.
[HC] We have added a brief description on the roles of the object as below:
The primary ingress of a primary LSP sends the
backup ingress this object in a PATH message. In this case, the
object contains the information needed to set up ingress protection.
The information includes:
o Backup ingress IP address indicating the backup ingress,
o Traffic Descriptor describing the traffic that the primary LSP
transports, this traffic is imported into the backup LSP(s) on the
backup ingress when the primary ingress fails,
o Label and Routes indicating the first hops of the primary LSP,
each of which is paired with its label, and
o Desire options on ingress protection such as P2MP option
indicating a desire to use a backup P2MP LSP to protect the
primary ingress of a primary P2MP LSP.
The backup ingress sends the primary ingress this object in a RESV
message. In this case, the object contains the information about the
status on the ingress protection.
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.
[HC] We have added the following text and updated the document accordingly.
When the previously failed primary ingress of a primary LSP becomes available
again and the primary LSP is recovered from its primary ingress, the source
may switches the traffic to the primary ingress from the backup ingress. A
operator may control the traffic switch through using a command on the
source node after seeing that the primary LSP has recovered.
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?
[HC] It is the former. We have updated the text accordingly as below:
o Backup Ingress Address: The primary ingress MUST know the IP
address of the backup ingress it wants to be used before it can
use the INGRESS_PROTECTION 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.)
[HC] We have added more as follows:
It extends the fast-reroute (FRR) protection for transit nodes
of an LSP to the ingress node of the LSP.
The first sentence of the second paragraph of the Introduction is complex.
Please break it apart into several sentences.
[HC] We have rephrased it as below:
To protect against the failure of the (primary) ingress node of a
primary end to end P2MP (or P2P) TE LSP, a typical existing solution
is to set up a secondary backup end to end P2MP (or P2P) TE LSP.
The backup LSP is from a backup ingress node to backup egress nodes (or node).
The backup ingress node is different from the primary ingress node.
The backup egress nodes (or node) are (or is) different from the primary
egress nodes (or node) of the primary LSP.
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:".
[HC] We have revised the text according to your suggestion.
At the second bullet in this list of issues, I suggest "configuration" instead
of "configurations".
[HC] We have changed it to "configuration".
At the third bullet in this list of issues, what does "the BFD down" mean?
[HC] We have updated the text as below:
Any failure on the path of the BFD from an egress node
to an ingress node may cause the BFD to indicate the failure
of the ingress node.
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?
[HC] We have revised the related text in the document regarding to off-path and
on-path. "configured or computed" says that a backup ingress is "configured"
statically or "computed" automatically.
"A backup ingress is computed" means
that some software suggests a backup ingress automatically for the given
information about the primary LSP.
In the introduction to section 4, say _what_ the new object is backwards
compatible with.
[HC] We have removed "It is backward compatible."
In section 4.1, it's unclear what "keeps it" means. Please try reworking this
sentence without using "it".
[HC] "keeps it" means records it (this flag).
We have rephrased this sentence accordingly.
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).
[HC] We have added some explanations.
A node X is configured as a backup ingress on the primary ingress of an LSP.
The node X does not know whether it is a backup ingress for any primary ingress
of any LSP until the node X receives a PATH message with backup ingress
sub-object for an LSP.
In the 3rd paragraph of section 5.1.1, do you mean "efficient processing" where
you say "efficient process"?
[HC] Yes. We have revised it.
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.
[HC] We have deleted it.
At the first sentence of the last paragraph of 5.1.2, did you mean "required"
instead of "requires"?
[HC] Yes. We have changed it to required.
At the second paragraph of 5.3, what are "administrative-colors"?
[HC] They are administrative-groups. We have changed it to
administrative-groups.
_______________________________________________
Gen-art mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/gen-art
