Last Call Review of draft-ietf-mpls-ri-rsvp-frr-05
review-ietf-mpls-ri-rsvp-frr-05-rtgdir-lc-meuric-2019-04-11-00

• Status
• Email expansions
• History

• Versions
• 00

Prep for IETF Last Call

Review
review-ietf-mpls-ri-rsvp-frr-05-rtgdir-lc-meuric-2019-04-11

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
​http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

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.

*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.

*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...
------
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/
------
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]
------
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.
------
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/
------
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/).
------
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/
------
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]
------
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./
- The M bit is newly defined: is there any reason not to specify it
using MUSTs?
------
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/
------
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.
------

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.