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

[Alissa Cooper <[e-mail]>]

Robert, thanks for your review. Huaimo, thanks for your responses. I think with the final change agreed between Huaimo and IANA, Section 8 should be good to go. I entered a No Objection ballot.

Alissa

> On Mar 1, 2018, at 10:43 AM, Robert Sparks <[e-mail]> wrote:
> 
> I'm fine with the way you handled all my editorial suggestions.
> 
> I'm still uncomfortable with what you are asking IANA to do. I think at this point you need to have a conversation with your AD.
> 
> As -14 is written, you are trying to put a value in a part of the class names, class numbers, and class types registry at a codepoint that requires "Standards Action" (Range 0-119). Maybe I'm misremembering, but I don't _think_ you can do that with an Experimental document.
> 
> The language you have for the subregistry that you are _NOT_ asking IANA to create at this time still implies that it exists as a registry (with discussions of initial values and future assignment policies). Instead, I think you need something like "During this experiment, the types of the new subobjects are defined by the following table in this document. If the concept proceeds to the standards track, these will move to an IANA maintained registry" or some similar language.
> 
> Again, get your ADs advice on what to do next here.
> 
> RjS
> 
> 
> On 2/28/18 8:18 PM, Huaimo Chen wrote:
>> 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:[e-mail]]
>> Sent: Wednesday, February 21, 2018 12:20 PM
>> To: [e-mail]
>> Cc: [e-mail]; [e-mail]; [e-mail]
>> 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
> [e-mail]
> https://www.ietf.org/mailman/listinfo/gen-art