[mpls] Re: John Scudder's Discuss on draft-ietf-mpls-ri-rsvp-frr-18: (with DISCUSS and COMMENT)

[Chandrasekar Ramachandran <[e-mail]>]

As stated in the responses to all the comments (under COMMENT section) in the previous mail, the comments are addressed in https://datatracker.ietf.org/doc/html/draft-ietf-mpls-ri-rsvp-frr-21.

Thanks,
Chandra.


Juniper Business Use Only
> -----Original Message-----
> From: Chandrasekar Ramachandran <csekar=[e-mail]>
> Sent: Wednesday, June 26, 2024 4:37 PM
> To: John Scudder <[e-mail]>; The IESG <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail]
> Subject: [mpls] Re: John Scudder's Discuss on draft-ietf-mpls-ri-rsvp-frr-18:
> (with DISCUSS and COMMENT)
>
> [External Email. Be cautious of content]
>
>
> Thanks for the detailed review. Please see inline (prefixed [CR]) for responses.
>
> Thanks,
> Chandra.
>
>
> Juniper Business Use Only
>
> Juniper Business Use Only
> > -----Original Message-----
> > From: John Scudder via Datatracker <[e-mail]>
> > Sent: Wednesday, May 29, 2024 6:06 AM
> > To: The IESG <[e-mail]>
> > Cc: [e-mail]; [e-mail];
> > [e-mail]; Nicolai Leymann <[e-mail]>;
> > [e-mail]
> > Subject: John Scudder's Discuss on draft-ietf-mpls-ri-rsvp-frr-18:
> > (with DISCUSS and COMMENT)
> >
> > [External Email. Be cautious of content]
> >
> >
> > John Scudder has entered the following ballot position for
> > draft-ietf-mpls-ri-rsvp-frr-18: Discuss
> >
> > When responding, please keep the subject line intact and reply to all
> > email addresses included in the To and CC lines. (Feel free to cut
> > this introductory paragraph, however.)
> >
> >
> > Please refer to
> > https://urldefense.com/v3/__https://www.ietf.org/about/groups/iesg/sta
> > tem
> > ents/handling-ballot-positions/__;!!NEt6yMaO-gk!E-
> > wtrMVo1qpsxwkeOZLVQjhalO1sPdAEG-o4lpb0Wn4W-
> > lL70xoGf8s0cEIBtrEpTeWgFiMeSkm3cw$
> > for more information about how to handle DISCUSS and COMMENT
> > positions.
> >
> >
> > The document, along with other ballot positions, can be found here:
> > https://urldefense.com/v3/__https://datatracker.ietf.org/doc/draft-iet
> > f-mpls-
> > ri-rsvp-frr/__;!!NEt6yMaO-gk!E-wtrMVo1qpsxwkeOZLVQjhalO1sPdAEG-
> > o4lpb0Wn4W-lL70xoGf8s0cEIBtrEpTeWgFiN5pzldoQ$
> >
> >
> >
> > ----------------------------------------------------------------------
> > DISCUSS:
> > ----------------------------------------------------------------------
> >
> > # John Scudder, RTG AD, comments for draft-ietf-mpls-ri-rsvp-frr-18 CC
> > @jgscudder
> >
> > Thanks for this document. It's a dense read for someone like me who is
> > not an expert in RSVP, but it seems important, useful, and carefully
> > done. I appreciate the precise and detailed work.
> >
>
> [CR] Thank you.
>
> > I have one concern I've flagged as a DISCUSS point, which may turn out
> > not to be a big deal if I've just misunderstood some nuance, but in
> > any case, we should talk through it. I also have some other comments I
> > hope might be helpful.
> >
> > ## DISCUSS
> >
> > ### Section 4.1
> >
> >    A node supporting facility backup protection [RFC4090] MUST set the
> >    RI-RSVP flag (I bit) that is defined in Section 3.1 of RSVP-TE
> >    Scaling Techniques [RFC8370] only if it supports all the extensions
> >    specified in the rest of this document.
> >
> > I have several concerns about this. At least some of them probably
> > relate to my limited expertise in the subject area, but maybe I can at
> > least expose some areas where additional explanation might be helpful in
> the document.
> >
>
> [CR] Please also refer to the responses sent to Ketan
> (https://urldefense.com/v3/__https://mailarchive.ietf.org/arch/msg/mpls/gM
> pvz7ez80a9AfOdPOvPSUZEcpM/__;!!NEt6yMaO-
> gk!AApy1LDqx27yOJ5pG3J1_v75FHupuF7zr_cBmXmLEHR7onL-
> qonFSeMqQXpPvRFsUTAJa7QH46-scxKMB6tBXcN-sk8WHrY$ ).
>
> > First and foremost, as written this sentence doesn't seem like it's
> > achievable in practice. Couldn't there be a legacy router in the field
> > that supports both RFC
> > 4090 and RFC 8370? Wouldn't that router then advertise the I bit, at
> > least in some circumstances?
> >
>
> [CR] As noted in the response to Ketan, the authors believe that it is
> extremely unlikely that an MPLS RSVP-TE implementation that supports
> RFC4090 would implement only RI-RSVP and not RI-RSVP-FRR. So, we don't
> envision there being a legacy MPLS router in the field that supports both RFC
> 4090 and RFC 8370. In the unlikely scenario where such an implementation
> choice was made, the requirements on the establishment of signaling
> handshake and the backwards compatibility procedures discussed in the
> document provide for sufficient cover.
>
> > Second, this MUST seems to conflict with a SHOULD in section 4.6.1,
> > which says,
> >
> >    An implementation supporting RI-RSVP-FRR extensions SHOULD set the
> >    flag "Refresh interval Independent RSVP" or RI-RSVP flag in the
> >    CAPABILITY object carried in Hello messages as specified in RSVP-TE
> >    Scaling Techniques [RFC8370].
> >
>
> [CR] We will update this to a "MUST".
>
> > It makes me wonder if the section 4.1 paragraph should be more like this.
> >
> > NEW:
> >    A node supporting facility backup protection [RFC4090] MUST NOT set the
> >    RI-RSVP flag (I bit) that is defined in Section 3.1 of RSVP-TE
> >    Scaling Techniques [RFC8370] unless it supports all the extensions
> >    specified in the rest of this document.
> >
>
> [CR] Based on the three responses above, is this change still necessary?
>
> > (Although even though that might be clearer, the first point stands,
> > that it might not be achievable.)
> >
>
> [CR] As noted above, we don't envision there being a legacy MPLS router in
> the field that supports both RFC 4090 and RFC 8370. There was some text in a
> draft version of RFC8370, that explicitly stated "use RI-RSVP-FRR procedures if
> RFC4090 procedures are implemented" (see section 2.2 of
> https://urldefense.com/v3/__https://datatracker.ietf.org/doc/draft-ietf-teas-
> rsvp-te-scaling-rec/02/__;!!NEt6yMaO-
> gk!AApy1LDqx27yOJ5pG3J1_v75FHupuF7zr_cBmXmLEHR7onL-
> qonFSeMqQXpPvRFsUTAJa7QH46-scxKMB6tBXcN--UsV_kk$ ), but it was
> removed in later stages based on WG feedback.
>
> > Third, moving on to the end of the paragraph,
> >
> >    Procedures for backward compatibility (see Section 4.6.2.3 of this
> >    document) delves on this in detail.
> >
> > As far as I can tell, despite the title of Section 4.6.2, Subsection
> > 4.6.2.3 isn't a procedure for backward compatibility, it's a warning
> > about the terrible things that can happen if some node in the network is
> incompatible.
> >
>
> [CR]  Subsection 4.6.2.3 was added based on the discussion in the following
> thread (ask was to add some text describing the operational considerations
> for this particular scenario):
> https://urldefense.com/v3/__https://mailarchive.ietf.org/arch/msg/mpls/KDR
> VWBAajl08pnXDkit0ykRyZDg/__;!!NEt6yMaO-
> gk!AApy1LDqx27yOJ5pG3J1_v75FHupuF7zr_cBmXmLEHR7onL-
> qonFSeMqQXpPvRFsUTAJa7QH46-scxKMB6tBXcN-A_oZoIA$
>
> > Is all of this right? That there could be a legacy node in the
> > network, with no way to detect that it doesn't comply with the present
> > specification, and that terrible things such as 4.6.2.3 describes, could
> happen as a result?
> >
>
> [CR] Please refer to the previous response above.
>
> > (I see Ketan Talaulikar raised a related concern in his RTGDIR
> > review.)
> >
> >
> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> >
> > ## COMMENT
> >
> > ### RTGDIR review
> >
> > Please take a look at Ketan Talaulikar's RTGDIR review that was posted
> > yesterday,
> > (https://urldefense.com/v3/__https://mailarchive.ietf.org/arch/msg/rtg
> > -
> > dir/PRZJa7LH9b3J1aRFo3BvYZ3DQUQ/__;!!NEt6yMaO-gk!E-
> > wtrMVo1qpsxwkeOZLVQjhalO1sPdAEG-o4lpb0Wn4W-
> > lL70xoGf8s0cEIBtrEpTeWgFiP0TOSq2g$ ). I won't repeat his points here,
> > other than the one that appeared in my DISCUSS, but please consider all of
> them.
> >
> > ### Section 4.3.3, use of RFC 2119 keyword in example
> >
> > You have a number of sections that include examples. The examples are
> > very helpful for understanding the specification, thank you! However,
> > you use RFC 2119/BCP 14 keywords in the examples, and I think you
> > shouldn't. Those keywords are reserved for specifying procedures, and
> > an example isn't specifying procedure, it's demonstrating it.
> >
> > I'll call out each example separately but only provide the explanation here.
> >
> > In this section, what I noticed was,
> >
> >    1.  When C receives a Conditional PathTear from B, it decides to
> >       retain LSP state as it is the NP-MP of the PLR A.  C also MUST
> >       check whether Phop B had previously signaled availability of node
> >       protection.  As B had previously signaled NP availability by
> >       including B-SFRR-Ready Extended Association object, C MUST remove
> >       the B-SFRR-Ready Extended Association object containing
> >       Association Source set to B from the Path message and trigger a
> >       Path to D.
> >
> > There are various ways this could be rewritten, the absolute easiest
> > would be to simply substitute "must" for "MUST", although it would be
> > cleaner to fully rewrite in terms of actions instead of expectations,
> > as in,
> >
> > NEW:
> >    1.  When C receives a Conditional PathTear from B, it decides to
> >       retain LSP state as it is the NP-MP of the PLR A.  It also
> >       checks whether Phop B had previously signaled availability of node
> >       protection.  As B had previously signaled NP availability by
> >       including B-SFRR-Ready Extended Association object, C removes
> >       the B-SFRR-Ready Extended Association object containing
> >       Association Source set to B from the Path message and triggers a
> >       Path to D.
> >
> [CR] Ack. Will make the change in all appropriate places.
>
> > ### Section 4.4.3, RFC 2119 keyword misuse
> >
> >    As any implementation that does not support Conditional PathTear MUST
> >    ignore the new object but process the message as a normal PathTear
> >    without generating any error, the Class-Num of the new object MUST be
> >    10bbbbbb where 'b' represents a bit (from Section 3.10 of [RFC2205]).
> >
> > I think the first MUST is unnecessary since you're stating a need, not
> > a requirement. The need is fulfilled as a natural consequence of your
> > choice of code point and the underlying requirement from RFC 2205. The
> > second MUST is also unnecessary, you are not telling the implementer
> > anything, you're just explaining why you chose the type code you did.
> >
> > One possible fix would be to delete the paragraph, but it's probably
> > nicer to the reader if you leave the explanation in place. Perhaps
> > something like,
> >
> > NEW:
> >    Any implementation that does not support Conditional PathTear needs
> >    to ignore the new object but process the message as a normal PathTear
> >    without generating any error. For this reason, the Class-Num of the
> >    new object follows the pattern 10bbbbbb where 'b' represents a bit.
> >    (The behavior for objects of this type is specified in Section 3.10
> >    of [RFC2205]).
> >
> [CR] Thanks. Will make the suggested change.
>
> > ### Section 4.5, clarification of requirement
> >
> >    If the ingress wants to tear down the LSP because of a management
> >    event while the LSP is being locally repaired at a transit PLR, it
> >    would not be desirable to wait till the completion of backup LSP
> >    signaling to perform state cleanup.  To enable LSP state cleanup when
> >    the LSP is being locally repaired, the PLR MUST send a "Remote"
> >    PathTear message instructing the MP to delete the PSB and RSB states
> >    corresponding to the LSP.  The TTL in the "Remote" PathTear message
> >    MUST be set to 255.
> >
> > In this paragraph, I am unclear exactly what the sentence "To enable
> > LSP state cleanup when the LSP is being locally repaired, the PLR MUST
> > send a "Remote"
> > PathTear message instructing the MP to delete the PSB and RSB states
> > corresponding to the LSP" is telling me. My best guess is,
> >
> > NEW:
> >    If the ingress wants to tear down the LSP because of a management
> >    event while the LSP is being locally repaired at a transit PLR, it
> >    would not be desirable to wait till the completion of backup LSP
> >    signaling to perform state cleanup.  In this case, the PLR MUST send
> >    a "Remote" PathTear message instructing the MP to delete the PSB and
> >    RSB states corresponding to the LSP.  The TTL in the "Remote"
> >    PathTear message MUST be set to 255. Doing this enables LSP state
> >    cleanup when the LSP is being locally repaired,
> >
> > Is that rewrite faithful to what you intended? If so, I suggest using
> > it, or any other rewrite of your choosing that clarifies matters. In
> > particular, my intent in the rewrite is to make it clear that the PLR
> > is unequivocally required to do this, which IMO wasn't clear before.
> >
>
> [CR] Yes, the rewrite is faithful. Will make the suggested edit.
>
> > ### Section 4.5.1, clarification of requirement
> >
> >    If local repair fails on the PLR after a failure, then this MUST be
> >    considered as a case for cleaning up LSP state from the PLR to the
> >    Egress.  The PLR achieves state cleanup by sending "Remote" PathTear
> >    to the MP.  The MP MUST delete the states corresponding to the LSP
> >    also propagate the PathTear downstream thereby achieving state
> >    cleanup from all downstream nodes up to the LSP egress.  Note that in
> >    the case of link protection, the PathTear MUST be directed to the LP-
> >    MP's Node-ID IP address rather than the Nhop interface address.
> >
> > Similar to the previous case, the combination of the RFC 2119 keyword
> > with the casual writing style leaves the intent of the requirement unclear to
> me.
> > Here's an attempt at a rewrite, in the same spirit.
> >
> > NEW:
> >    If local repair fails on the PLR after a failure, the PLR MUST send a
> >    "Remote" PathTear to the MP.  The purpose of doing this is to clean
> >    up LSP state from the PLR to the Egress. Upon receiving the PathTear,
> >    the MP will delete the states corresponding to the LSP and also
> >    propagate the PathTear downstream thereby achieving state cleanup
> >    from all downstream nodes up to the LSP egress.  Note that in the
> >    case of link protection, the PathTear MUST be directed to the LP-
> >    MP's Node-ID IP address rather than the Nhop interface address.
> >
> > Note that I removed the second MUST, on the assumption that you aren't
> > specifying a new requirement for the MP, but stating the natural
> > consequence of a requirement you've already specified elsewhere.
> > Please double-check that I haven't broken something!
> >
>
> [CR] Regarding the second MUST, we believe it is necessary (please refer to
> section 4.2.4). We will adopt the rest of the suggested changes as shown
> below.
>
> NEW:
>    If local repair fails on the PLR after a failure, the PLR MUST send a
>    "Remote" PathTear to the MP.  The purpose of doing this is to clean
>    up LSP state from the PLR to the Egress. Upon receiving the PathTear,
>    the MP MUST delete the states corresponding to the LSP and also
>    propagate the PathTear downstream thereby achieving state cleanup
>    from all downstream nodes up to the LSP egress.  Note that in the
>    case of link protection, the PathTear MUST be directed to the LP-
>    MP's Node-ID IP address rather than the Nhop interface address.
>
> > ### Section 4.5.2, use of RFC 2119 keyword in example
> >
> > As with my 4.3.3 comment. In this case my suggestion is,
> >
> > OLD:
> >    RRO of the LSP carried in the Resv will not contain C.  When A
> >    processes the Resv message with a new RRO not containing C - its
> >    former NP-MP, A MUST send a "Remote" PathTear to C.  When C
> > receives
> >
> > NEW:
> >    RRO of the LSP carried in the Resv will not contain C.  When A
> >    processes the Resv message with a new RRO not containing C - its
> >    former NP-MP, A sends a "Remote" PathTear to C.  When C receives
> >
> [CR] Ack. Will make the suggested change.
>
> > ### Section 4.5.3.1, I'm confused
> >
> >    If an LSP is preempted on an LP-MP after its Phop or the incoming
> >    link has already failed
> >
> > Why "Phop or the incoming link" and not just "incoming link"? It's a
> > link- protecting merge point, so why does it care about the failure of
> > an upstream *node*? Also, this seems as though it conflicts with the
> > title of the subsection, which is "Preemption on LP-MP after Phop Link
> Failure" and not "...
> > after Phop or Phop Link Failure".
> >
> > Could it be rewritten as follows?
> >
> >    If an LSP is preempted on an LP-MP after its Phop
> >    link has already failed
> >
>
> [CR] You are right. Will make the change as suggested.
>
> > ### Section 4.5.3.2, I'm still confused but in the other direction
> >
> >    If an LSP is preempted on an NP-MP after its Phop link has already
> >    failed
> >
> > If it's a node-protecting merge point, shouldn't this one care about
> > the Phop as well as the Phop link? (Also in the title of the
> > subsection.)
> >
>
> [CR] When the phop link goes down, the NP-MP can't determine if it is just
> the phop link that went down or the phop that went down. So, we don't need
> a separate section to discuss "Phop Failure".
>
> > ### Section 4.5.3.2, use of RFC 2119 keyword in example
> >
> > As with my 4.3.3 comment. In this case my suggestion is,
> >
> > OLD:
> >    3.  As the only reason for C having retained state after Phop node
> >       failure was that it was an NP-MP, C MUST send a normal PathTear to
> >       D and delete its PSB state also.  D would also delete the PSB and
> >       RSB states on receiving a PathTear from C.
> >
> > NEW:
> >    3.  As the only reason for C having retained state after Phop node
> >       failure was that it was an NP-MP, C sends a normal PathTear to
> >       D and deletes its PSB state also.  D would also delete the PSB and
> >       RSB states on receiving a PathTear from C.
> >
>
> [CR] Ack. Will make the suggested change.
>
> > ### Section 4.6, ALL CAPS
> >
> > I probably wouldn't even flag this if I weren't doing a full review
> > already, but
> >
> >    DOES NOT support RI-RSVP-FRR extensions.  Note that for LSPs
> >
> > Although RFC 2119 doesn't forbid the use of all caps for non-reserved
> > keywords, it's often considered to be inadvisable. I would suggest,
> >
> > NEW:
> >    does not support RI-RSVP-FRR extensions.  Note that for LSPs
> >
>
> [CR] Ack. Will make the suggested edit.
>
> > ### Section 4.6.1, SHOULD set
> >
> >    An implementation supporting RI-RSVP-FRR extensions SHOULD set the
> >    flag "Refresh interval Independent RSVP" or RI-RSVP flag in the
> >    CAPABILITY object carried in Hello messages as specified in RSVP-TE
> >    Scaling Techniques [RFC8370].
> >
> > Depending on the resolution of my DISCUSS point, I guess you might
> > want to revisit this SHOULD.
> >
>
> [CR] As noted above in one of the responses, we will make this a "MUST".
>
> > ### Section 4.6.2.1, use of RFC 2119 keyword in example
> >
> > As with my 4.3.3 comment. In this case my suggestion is,
> >
> > OLD:
> >    -  A and B MUST reduce the refresh time to the default short refresh
> >       interval of 30 seconds and trigger a Path message
> >
> >    -  If B is not an MP and if Phop link of B fails, B cannot send
> >       Conditional PathTear to C but MUST time out the PSB state from A
> >       normally.  Note that B can time out the PSB state A normally only
> >       if A did not set long refresh in the TIME_VALUES object carried in
> >       the Path messages sent earlier.
> >
> > NEW:
> >    -  A and B reduce the refresh time to the default short refresh
> >       interval of 30 seconds and trigger a Path message
> >
> >    -  If B is not an MP and if Phop link of B fails, B cannot send
> >       Conditional PathTear to C but times out the PSB state from A
> >       normally.  Note that B can time out the PSB state A normally only
> >       if A did not set long refresh in the TIME_VALUES object carried in
> >       the Path messages sent earlier.
> >
>
> [CR] Thanks. Will make the changes as suggested.
>
> > ## Notes
> >
> > This review is in the ["IETF Comments" Markdown format][ICMF], You can
> > use the [`ietf-comments` tool][ICT] to automatically convert this
> > review into individual GitHub issues.
> >
> > [ICMF]: https://urldefense.com/v3/__https://github.com/mnot/ietf-
> > comments/blob/main/format.md__;!!NEt6yMaO-gk!E-
> > wtrMVo1qpsxwkeOZLVQjhalO1sPdAEG-o4lpb0Wn4W-
> > lL70xoGf8s0cEIBtrEpTeWgFiOieqUlIQ$
> > [ICT]: https://urldefense.com/v3/__https://github.com/mnot/ietf-
> > comments__;!!NEt6yMaO-gk!E-wtrMVo1qpsxwkeOZLVQjhalO1sPdAEG-
> > o4lpb0Wn4W-lL70xoGf8s0cEIBtrEpTeWgFiP1IMYQ5w$
> >
> >
>
> _______________________________________________
> mpls mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]