Re: [RTG-DIR] RtgDir Review: draft-ietf-mpls-ri-rsvp-frr-05

[Chandrasekar Ramachandran <[e-mail]>]

Hi Julien,
Draft version 6 uploaded today addresses most of your comments.
Please refer to my detailed responses inline.

Thanks,
Chandra.


Juniper Internal

> -----Original Message-----
> From: [e-mail] <[e-mail]>
> Sent: Thursday, April 11, 2019 10:03 PM
> To: [e-mail]
> Cc: [e-mail]; [e-mail]; [e-mail]
> Subject: RtgDir Review: draft-ietf-mpls-ri-rsvp-frr-05
> 
> Hello,
> 
> I have been selected as the Routing Directorate reviewer for this draft.
> The Routing Directorate seeks to review all routing or routing-related drafts
> as they pass through IETF last call and IESG review, and sometimes on
> special request. The purpose of the review is to provide assistance to the
> Routing ADs. For more information about the Routing Directorate, please
> see ​https://urldefense.proofpoint.com/v2/url?u=http-
> 3A__trac.tools.ietf.org_area_rtg_trac_wiki_RtgDir&d=DwIDaQ&c=HAkYuh63
> rsuhr6Scbfh0UjBXeMK-
> ndb3voDTXcWzoCI&r=iEQmXlRGWdNbtvVr6ghcatwLYhZUbMF-u63wi_-
> VTtA&m=y53rabR7nvvG-
> 1gvqqZPbmO5lH7P9g4v_e1Yj8unbjg&s=fT9WTkI4LNFIV3mjukePVngq8cvop
> WoONP1JCE18yFg&e=
> 
> Although these comments are primarily for the use of the Routing ADs, it
> would be helpful if you could consider them along with any other IETF Last
> Call comments that you receive, and strive to resolve them through
> discussion or by updating the draft.
> 
> Document: draft-ietf-mpls-ri-rsvp-frr-05
> Reviewer: Julien Meuric
> Review Date: April 10, 2019
> Intended Status: Proposed Standard
> 
> *Summary:*
> I have some minor concerns about this document that I think should be
> resolved before publication.
> 
> *Comments:*
> The document is well written and self consistent. The clear problem
> statement and the solution's "spirit" description make the detailed behavior
> convenient to follow. The main issue is about the use of 2119
> language: only 4 MUSTs are used (2 of them being similar), far too many
> SHOULDs.

[Chandra] The motivation is to enable the implementations to seamlessly interoperate with implementations that do not support the draft during incremental deployment. We have converted a few more SHOULD to MUST keeping the above in consideration. That is, wherever we consider MUST will enable better interoperability, we have upgraded SHOULD to MUST.

> *Major Issues:*
> No major issues found.
> 
> *Minor Issues:*
> Unless I missed an assumption somewhere, I think that, for a Standards
> Track document, many of the existing SHOULDs should actually be MUSTs.
> It is a bit odd to read a document trying to be exhaustive about the possible
> situations and backward compatibility consideration (nice
> work!) while only requiring optional behaviors.

[Chandra] Yes, backward compatibility is an important consideration. You could refer to the latest version for the updates.

> *Nits:*
> ------
> Global
> ---
> - Two nits are repeated along the full document and need to be fixed:
>   * Most of the time, PATH and RESV messages are fully capitalized while
> PathErr, PathTear, ResvTear... are not. Replacing them by Path/Resv would
> bring consistency.
>   * When it comes to nodes or messages names, many definite/indefinite
> articles (the/a) are missing. I have spotted some of them but got lazy...

[Chandra] I have gone through the whole draft again and has fixed all sections. You could refer to the latest version.
 
> ------
> Header
> ---
> - Was RFC 8370 considered to be added to the updated document list? The
> specification is clearly about combining RFC 4090 and 8370.
> - s/Refresh Interval Independent/Refresh-Interval Independent/

[Chandra] Fixed.

> ------
> Abstract
> ---
> - s/LSP related states/LSP-related states/  [x2]
> - LSP is not expanded on 1st use (only in introduction), it may be worth to
> expand earlier.
> - s/fast reroute (FRR)/Fast ReRoute (FRR)/
> - s/Refresh-interval/Refresh-Interval/  [for consistency]

[Chandra] Fixed.

> ------
> 1. Introduction
> ---
> - s/label switched path (LSP)/Label Switched Path (LSP)/
> - s/Standard RSVP/Base RSVP/  [x2]
> - OLD
>    eliminate facility backup protection
>    dependency on refresh timeouts for stale state cleanup including the
>    cleanup for facility backup protection.
>   NEW
>    eliminate facility backup protection
>    dependency on refresh timeouts for stale state cleanup.

[Chandra] Fixed.

> ------
> 2. Terminology
> ---
> - The terms Nhop and NNhop, extensively used, are missing.
> - Adding "B-SFRR-Ready" may be considered.
> - The link between "Merge Point" and "MP" is only implicit: MP and PLR
> deserve to be added as defined terms.
> - s/Link-protecting/Link-Protecting/
> - s/Node-protecting/Node-Protecting/

[Chandra] Fixed.

> ------
> 3. Problem Description
> ---
> - s/Figure 1, consider/Figure 1, let us consider/
> - s/Also assume/Also let us assume/
> - s/A is the Point of Local Repair (PLR) and C is Node Protecting Merge Point
> (NP-MP)/A is the PLR and C is the NP-MP/
> - s/D is the Link Protecting Merge Point (LP-MP)/D is the LP-MP/
> - s/are refreshed has failed/are refreshed, has failed/
> - s/send tear down message/send a tear down message/
> - s/as a Merge Point/as an MP/
> - s/receive PathTear/receive a PathTear/
> - Sentences #2 and #3 in the bullet at the top of page 7 seem to duplicate
> the bullet at the bottom of page 6. This need to be skimmed (I personally
> prefer the wording of the 2nd bullet over the 1st one, including s/send
> PathTear/send a PathTear/).

[Chandra] Fixed.

> ------
> 4. Solution Aspects
> ---
> - s/send tear down message/send a tear down message/  [x2]
> ------
> 4.1.
> ---
> - s/RFC 4090/[RFC4090]/  [x4, excluding section title]
> ------
> 4.2.
> ---
> - s/RFC 4090/[RFC4090]/
> - OLD
>       a LP-bypass LSP to Nhop node avoiding only the link that
>       protected LSP takes to reach Nhop
>   NEW
>       an LP-bypass LSP to the Nhop node avoiding only the link that
>       the protected LSP takes to reach the Nhop.
> 
> - OLD
>       a LP-bypass LSP to Nhop node avoiding the link that
>       protected LSP takes to reach Nhop
>   NEW
>       an LP-bypass LSP to the Nhop node avoiding the link that
>       the protected LSP takes to reach the Nhop.
> 
> - s/RFC 8370/[RFC8370]/
> - OLD
>      included RRO object carried in RESV message.
>      If the MP has not included Node-ID sub-object in RESV RRO
>    NEW
>      included in the RRO object carried in the Resv message.
>      If the MP has not included a Node-ID sub-object in the Resv RRO
> 
> - s/PATH message/Path message/  [x2]
> - s/in CAPABILITY object/in the CAPABILITY object/  [x2]
> - OLD
>     then the PLR SHOULD include B-SFRR-Ready Extended Association object
> and triggers PATH message
>   NEW
>     then [I-D.ietf-mpls-summary-frr-rsvpte] applies: the PLR SHOULD include a
> B-SFRR-Ready Extended Association object and triggers a Path message
> 
> - s/PATH message/Path message/
> - s/ordering rules object/object ordering rules/
> - s/RFC 4090/[RFC4090]/
> - s/RFC 8370/[RFC8370]/
> - s/sub-object in the RRO object carried in the RESV message/sub-object of
> the RRO object carried in the Resv message/
> - s/included Node-ID sub-object in the RRO object carried in PATH
> message/included a Node-ID sub-object in the RRO object carried in the
> Path message/
> - s/The node should determine whether the incoming PATH messages
> contains B-SFRR-Ready/A node receiving Path messages should determine
> whether they contain a B-SFRR-Ready/
> - s/followed by implementations supporting/followed by the
> implementations supporting/
> - s/"Remote" state on MP/"Remote" State on MP/
> - OLD
>     The "remote" state is identical to the protected LSP path state except for
> the difference in RSVP_HOP object.
>   NEW
>     The only difference between the "remote" path state and the LSP path
> state is in the RSVP_HOP object.
> 
> - s/in "remote" Path state/in the "remote" path state/
> - s/MP.../The MP.../  [x4]
> - s/Node signaling/The node signaling/
> - s/a PATH with/a Path message with/
> - s/in PATH RRO/in the Path RRO/
> - s/receives PathTear/receives a PathTear message
> - s/on the Ingress or created from a PATH message from/on the ingress or
> created by a Path message from/
> - s/from PLR/from the PLR/
> - s/called "Remote PathTear"/called "Remote" PathTear/

[Chandra] Fixed.
> ------
> 4.3.
> ---
> - s/Immediate node failures/Node failures/
> - s/SHOULD send Conditional PathTear/SHOULD send a "Conditional
> PathTear" downstream/
> - s/Node-ID signaling/The Node-ID signaling/
> - s/MP receives normal or "Remote" PathTear for PSB/The MP receives a
> normal or "Remote" PathTear for its PSB/  [x3]
> - s/MP receives ResvTear RSB/The MP receives a ResvTear for its RSB/  [x3]
> - s/Remote Node-ID/The remote Node-ID/  [x2]
> - s/Receiving Conditional PathTear/Receiving a Conditional PathTear/
> - s/Figure 1, assume/Figure 1, we assume/
> - s/PATH/Path/  [x5]
> - s/MP receives normal or "Remote" PathTear for PSB/The MP receives a
> normal or "Remote" PathTear for its PSB/  [x2]
> - s/MP receives ResvTear RSB/The MP receives a ResvTear for its RSB/  [x2]

[Chandra] Fixed.

> ------
> 4.4.
> ---
> - s/Conditional Path Tear/Conditional PathTear/  [x3]
> - s/Ingress has/The ingress has/
> - s/and PathTear is not received/and no PathTear is received/
> - Section 4.4.2:
>   * need "a/the" before most (conditional/normal) PathTears
>   * s/included B-SFRR-Ready Extended Association/included the B-SFRR-
> Ready Extended Association/
>   * s/PATH/a Path message/
>   * should consider upgrading some SHOULDs into MUST.
> - s/CONDITIONS object/CONDITIONS Object/
> - s/called as "CONDITIONS" object that/called "CONDITIONS" object, that/
> - "Length, Class, C-type, M bit" would better be followed  by ":" and drop
> the trailing carriage return (like the Terminology section).
> - s/If M-bit is set/If the M bit is set/  [x2]
> - s/processed based on the condition if the receiver router is a Merge Point
> or not./processed according to the receiver router role, i.e. if it is an MP or
> not./
> - s/as normal PathTear message./as a normal PathTear message./

[Chandra] Fixed.

> - The M bit is newly defined: is there any reason not to specify it using
> MUSTs?

[Chandra] Please refer to the earlier comment on backward compatibility consideration.

> ------
> 4.5.
> ---
> - s/the Ingress wants/the ingress wants/
> - s/in "remote" PathTear message/in the "Remote" PathTear message/
> - s/Consider node C in example topology (Figure 1) has/Let us consider that
> node C, in the example topology (Figure 1), has/
> - s/sends normal PathTear/send a normal PathTear/
> - s/Assume B/Let us assume that B/
> - s/send "remote" PathTear/sends a "Remote" PathTear/
> - s/deletes PSB and RSB states/deletes the PSB and RSB states/
> - s/the remote PathTear and delete PSB and RSB states/the "Remote"
> PathTear and delete the PSB and RSB states/
> - s/a router that/a PLR that/
> - s/in RESV message, and if the RRO change indicates that/in the Resv
> message indicating that/
> - s/send "Remote" PathTear/send a "Remote" PathTear/
> - s/assume/let us assume/
> - s/NP-MP for A/NP-MP for PLR A/
> - s/trigger RESV/trigger a Resv/
> - s/in RESV/in the Resv/
> - s/the RESV with/the Resv with/
> - s/send "Remote" PathTear/send a "Remote" PathTear/
> - s/send normal PathTear/send a normal PathTear/
> - s/both PSB and RSB states corresponding/both the PSB and the RSB states
> corresponding/
> - s/Phop Link failure/Phop Link Failure/
> - s/send normal PathTear and delete both PSB and RSB states
> corresponding/send a normal PathTear and delete both the PSB and the RSB
> states corresponding/
> - s/send normal PathTear and delete PSB and RSB states corresponding/send
> a normal PathTear and delete the PSB and RSB states corresponding/
> - s/Consider B-C/Let us consider that B-C/
> - s/send PathErr or ResvTear/send a PathErr nor a ResvTear/
> - s/because backup LSP/because the backup LSP/
> - s/send normal PathTear/send a normal PathTear/
> - s/on receiving PathTear/when receiving a PathTear/
> - s/reject backup LSP PATH and send PathErr/reject the backup LSP Path and
> send a PathErr/

[Chandra] Fixed.

> ------
> 4.6.
> ---
> - s/have been proposed in/have been defined in/
> - s/and remote PathTear/and "Remote" PathTear/
> - s/should support/need to support/  [unless moving to 2119 language]
> - s/in CAPABILITY object/in the CAPABILITY object/
> - s/initiate remote Node-ID/initiate a remote Node-ID/
> - s/with NNhop/with its NNhop/
> - s/set RI-RSVP flag in CAPABILITY object/set the RI-RSVP flag in the
> CAPABILITY object/
> - s/that NNhop/that the NNhop/
> - s/PPhop/the PPhop/  [x3]
> - s/in PATH/in its Path messages/
> - s/set RI-RSVP flag in CAPABILITY object/set the RI-RSVP flag in the
> CAPABILITY object/
> - s/for backward compatibility/for Backward Compatibility/
> - s/in TIME_VALUES object carried in PATH to default short refresh default
> value/in the TIME_VALUES object carried in the Path message to a default
> short refresh value/
> - s/in TIME_VALUES object carried in PATH to a short refresh default
> value/in the TIME_VALUES object carried in the Path message to a default
> short refresh value/
> - s/send remote PathTear or/send any "Remote" PathTear nor/
> - s/trigger PATH/trigger a Path message./
> - s/send Conditional PathTear/send a Conditional PathTear/
> - s/in TIME_VALUES object carried in RESV to default short refresh default
> value/in the TIME_VALUES object carried in the Resv message to a default
> short refresh value/
> - s/in TIME_VALUES object carried in PATH to default value/in the
> TIME_VALUES object carried in the *Resv* message to a default value/
> - s/and PPhop node/and the PPhop node/
> - s/in TIME_VALUES object carried in RESV to default value/in the
> TIME_VALUES object carried in the Resv message to a default value/
> - To be consistent with 4.6.2.1, the last paragraph of section 4.6.2.2.
> should NOT start with a bullet.

[Chandra] Fixed.

Thanks,
Chandra.

> ------
> 
> Cheers,
> 
> Julien
> 
> 
> ______________________________________________________________
> ___________________________________________________________
> 
> Ce message et ses pieces jointes peuvent contenir des informations
> confidentielles ou privilegiees et ne doivent donc pas etre diffuses, exploites
> ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez
> le signaler a l'expediteur et le detruire ainsi que les pieces jointes. Les
> messages electroniques etant susceptibles d'alteration, Orange decline
> toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
> 
> This message and its attachments may contain confidential or privileged
> information that may be protected by law; they should not be distributed,
> used or copied without authorisation.
> If you have received this email in error, please notify the sender and delete
> this message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been
> modified, changed or falsified.
> Thank you.