Re: [mpls] Benjamin Kaduk's Discuss on draft-ietf-mpls-ri-rsvp-frr-07: (with DISCUSS and COMMENT)

[Chandrasekar Ramachandran <[e-mail]>]

Hi Benjamin,
Apologies for the long delay in addressing your comments.
I have uploaded the latest (09) version of the draft to address your comments.
Please refer inline for my responses.

Thanks,
Chandra.


Juniper Public

> -----Original Message-----
> From: Benjamin Kaduk via Datatracker <[e-mail]>
> Sent: Thursday, December 5, 2019 9:19 AM
> To: The IESG <[e-mail]>
> Cc: [e-mail]; Nicolai Leymann
> <[e-mail]>; [e-mail]; [e-mail];
> [e-mail]
> Subject: Benjamin Kaduk's Discuss on draft-ietf-mpls-ri-rsvp-frr-07: (with
> DISCUSS and COMMENT)
> 
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-mpls-ri-rsvp-frr-07: 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/iesg/statement/discuss-
> criteria.html__;!8WoA6RjC81c!XPFLd1R4I69sL6cGmbqayx8cWhrcyYjpXuWoPe
> iiF1bqelXp2isNHnA9Av0p1Aw$
> for more information about IESG 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-ietf-
> mpls-ri-rsvp-
> frr/__;!8WoA6RjC81c!XPFLd1R4I69sL6cGmbqayx8cWhrcyYjpXuWoPeiiF1bqel
> Xp2isNHnA9nOkvW4A$
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> I think there's a lot of SHOULDs in this document that, if ignored,
> would cause the implementation to fail to achieve its stated purpose
> (elimination of stale state retention).  Therefore, I don't understand
> why they are given as SHOULD rather than MUST.  I have noted many (but
> probably not all) such instances in the COMMENT section.
> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Thanks for this document; the core procedures seem solid and
> well-thought-out.
> 
> I agree with Barry's comment.
>
> Section 1
> 
> Just to check my understanding: since we only mention the bypass-tunnel
> mode of RFC 4090 FRR, I infer that the detour LSPs do not need separate
> handling, since they have a one-to-one correspondence with the main LSP,
> which RI-RSVP is already tracking explicitly?
> 
[Chandra] Yes, only the facility protection FRR in RFC 4090 needs the additional extensions to support RFC 8370.

> Section 4.1
> 
>    A node supporting [RFC4090] facility protection FRR MAY set the RI-
>    RSVP capability (I bit) defined in Section 3 of RSVP-TE Scaling
>    Techniques [RFC8370] only if it supports all the extensions specified
> 
> nit: Section 3.1, no?
> 
[Chandra] Yes, fixed.

>    in the rest of this document.  A node supporting [RFC4090] facility
>    bypass FRR but not supporting the extensions specified in this
>    document MUST reset the RI-RSVP capability (I bit) in the outgoing
>    Node-ID based Hello messages.  Hence, this document updates [RFC4090]
>    by defining extensions and additional procedures over facility
>    protection FRR defined in [RFC4090] in order to advertise RI-RSVP
>    capability [RFC8370].
> 
> This sounds like it is changing the semantics of the I bit that was
> already defined, to mean support for more extentions than it originally
> was used for.  How is this backwards compatible (that is, how will an
> implementation know that the peer supports this updated semantics vs.
> the original semantics)?
> [I think this is exactly the core of Alvaro's Discuss point, which I
> support.]
> 
[Chandra] Reproducing the response sent to Alvaro:
---
draft-ietf-teas-rsvp-te-scaling-rec (which went on to become RFC8370) and draft-ietf-mpls-ri-rsvp-frr started off as companion documents. Early WG draft versions of RFC8370 (https://tools.ietf.org/html/draft-ietf-teas-rsvp-te-scaling-rec-02, Sec 2.2) had explicit text saying that if an RFC4090 bypass implementation opts to support RI-RSVP, then it must also implement procedures specified in draft-ietf-mpls-ri-rsvp-frr (RI-RSVP-FRR). But based on WG feedback (rightly so), it was decided that draft-ietf-teas-rsvp-te-scaling-rec should be kept fairly generic (applicable to all data-plane technologies) and shouldn’t include any MPLS specific implementation recommendations. 

If an RFC4090 bypass implementation adds support for just RI-RSVP [RFC8370] and does not add support for RI-RSVP-FRR, it leaves room for stale state to linger around for a long time (given the long refresh-interval) after a failover event. Section 3 "Problem Description" of RI-RSVP-FRR draft delves on this in detail. Hence, RI-RSVP-FRR updates RFC4090 bypass FRR procedures and makes them refresh interval independent, i.e. plugs the gap in applying RI-RSVP [RFC8370] technique to RFC4090 bypass procedures. So, the recommendation for RFC4090 bypass implementations is to not turn on RI-RSVP if RI-RSVP-FRR is not supported.
---
I have also reworded the text in Section 4.1 to spell out the requirements on setting or not setting the I-bit by implementations that support RFC 4090 and RFC 8370. I have also included a new Section 4.6.2.3 “Advertising RI-RSVP without RI-RSVP-FRR” describing the impact of ignoring the requirements for setting the I-bit.

> Section 4.2.1
> 
>    -  The PLR MUST also include its router ID in a Node-ID sub-object in
>       RRO object carried in a Path message.  While including its router
>       ID in the Node-ID sub-object carried in the outgoing Path message,
>       the PLR MUST include the Node-ID sub-object after including its
>       IPv4/IPv6 address or unnumbered interface ID sub-object.
> 
> Remind me why the ordering is important?
> 
[Chandra] We have observed in production networks that when nodes from different vendors along the LSP path use different ordering rules, the node that processes the message cannot always correlate the addresses to the correct hop beyond the IGP area boundary. As long as the address sub-objects contain addresses within an IGP area boundary, the processing node can use its local TED to associate the node-id sub-objects to their corresponding interface address sub-objects.

> Section 4.2.3
> 
>    A node receiving Path messages should determine whether they contain
>    a B-SFRR-Ready Extended Association object with the Node-ID address
>    of the PLR as the source and its own Node-ID as the destination.  In
> 
> To be clear, the node receiving Path messages is just looking for
> whether its Node-ID is the destination in the B-SFRR-Ready Extended
> Association object, and treating the source address as the Node-ID of
> the PLR; the node does not a priori know that this other node is "the
> PLR", right?  This should probably be reworded, if so.
> 
[Chandra] Yes, I have reworded the section.

>    If a matching B-SFRR-Ready Extended Association object is found in
>    the Path message and if there is an operational remote signaling
>    adjacency with the PLR that has advertised RI-RSVP capability (I-bit)
>    [RFC8370] in its Node-ID based Hello messages, then the node SHOULD
>    consider itself as the MP for the corresponding PLR.  The matching
> 
> "corresponding PLR" for this specific LSP, right?
> 
[Chandra] That is right. I have gone through most of the sections of the draft and attempted to address your comment on scoping.

> Section 4.2.4
> 
>    -  The MP later receives a Path with no matching B-SFRR-Ready
>       Extended Association object corresponding to the PLR's IP address
>       contained in the Path RRO, or
> 
> Again, this is for the specific LSP in question?
> 
>    -  The MP receives a PathTear, or
> 
> (and some scoping may be needed here, too -- *any* PathTear is probably
> too broad a criterion)
> 
[Chandra] I have gone through most of the sections of the draft and attempted to address your comment on scoping.

>    Unlike the normal path state that is either locally generated on the
>    ingress or created by a Path message from the Phop node, the "remote"
> 
> I'm not sure why the ingress case is a relevant comparison; the ingress
> is not ever going to be the MP, is it?
> 
[Chandra] I have reworded the remote state section without any change to the actual meaning.

> Section 4.3.3
> 
>       protection.  As B had previously signaled NP availability by
>       including B-SFRR-Ready Extended Association object, C SHOULD
>       remove the B-SFRR-Ready Extended Association object containing
>       Association Source set to B from the Path message and trigger a
>       Path to D.
> 
> This is only a SHOULD-level requirement.  How does D learn to clean up
> the associated state if the SHOULD is ignored?
> 
> Section 4.4
> 
>    does not require the receiving node to unconditionally delete the LSP
>    state immediately.  For this, B SHOULD add a new optional CONDITIONS
>    object in the PathTear.  The CONDITIONS object is defined in
>    Section 4.4.3.  If node C also understands the new object, then C
>    SHOULD delete LSP state only if it is not an NP-MP - in other words C
>    SHOULD delete LSP state if there is no "remote" PLR path state on C.
> 
> This is only a SHOULD-level requirement, but won't C tear down the whole
> LSP if the CONDITIONS are not {present and understood}?  Similarly, what
> happens if C does not understand the CONDITIONS object -- won't the
> whole LSP be torn down?
> 
[Chandra] I have converted some of the SHOULDs in the document to MUST including the above relating to your two comments above.

> Section 4.4.3
> 
> I'd probably have an introductory note that CONDITIONS is intended to
> see generic usage, and we only define one ('M') condition in this
> document.
> Also, what's the mnemonic for choosing 'M'?
> 
[Chandra] 'M' stands for 'Merge point'. The particular bit in the flags field contained in the CONDITIONS object must be processed based on the 'Merge point' condition of the receiving node. I have re-worded this section to explain the reasoning behind choosing 'M'.

> Section 4.5
> 
>    5. On D there would be a remote signaling adjacency with B and so D
>       SHOULD accept the "Remote" PathTear and delete the PSB and RSB
>       states corresponding to the LSP.
> 
> Just to check my understanding: this would also tear down any other LSPs
> that were using the same node protection path through F, right?
> 
[Chandra] Yes, that follows from the basic N:1 protection offered by bypass tunnels in RFC 4090.

> Section 4.5.2
> 
>    When a PLR router that has already made NP available detects a change
>    in the RRO carried in the Resv message indicating that the router's
>    former NP-MP is no longer present in the LSP path, then the router
>    SHOULD send a "Remote" PathTear directly to its former NP-MP.
> 
> Why is this only SHOULD?  Won't the former NP-MP retain stale state
> otherwise?
> 
>    The new RRO of the LSP carried in the Resv will not contain C.  When
>    A processes the Resv with a new RRO not containing C - its former NP-
>    MP, A SHOULD send a "Remote" PathTear to C.  When C receives the
> 
> (I'm not sure that I'd use the normative "SHOULD" here.)
> 
[Chandra] I have converted some of the SHOULDs in the document to MUST including the above relating to your two comments above.

>    "Remote" PathTear for its PSB state, C will send a normal PathTear
>    downstream to D and delete both the PSB and RSB states corresponding
>    to the LSP.  As D has already received backup LSP signaling from B, D
>    will retain control plane and forwarding states corresponding to the
>    LSP.
> 
> Remind me where this behavior of D is specified (to ignore the PathTear
> since it already got the backup LSP signaling).  It's intuitive/obvious,
> but I forget where it's written down.
> 
[Chandra] Yes, this is the basic RSVP state refresh timeout logic in RFC 4090 that has been explained in Section 3 Problem Description.

   4. As the link on C, over which the LSP states are refreshed, has
      failed, C will no longer receive state refreshes.  Consequently
      the protected LSP states on C will time out and C will send the
      tear down messages for all LSPs.  As each router should consider
      itself as an MP, C will time out the state only after waiting for
      an additional duration equal to refresh timeout.

> Section 4.6
> 
>    Note that for LSPs requesting only link protection, the PLR and the
>    LP-MP need to support the extensions.
> 
> I think this comparison would be more poigniant if the word "only" was
> inserted, for "only the PLR and the LP need to support".
> 
[Chandra] Yes, I have moved "only" to the appropriate location.

> Section 4.6.2
> 
> To double-check: the procedures in the subsections only apply when the
> ingress has requested protection, right?
> 
[Chandra] Even though this draft adds these procedures for LSPs using RFC 4090 facility protection, the text pertaining to setting short refresh interval in Path or Resv message if the Phop node or the Nhop node (i.e. immediate neighbor) does not advertise RI-RSVP capability is a logical consequence of Section 3.1 of RFC 8370. I have also re-worded this section slightly to convey the applicability of these procedures.

> Section 4.6.2.2
> 
>    -  If the node reduces the refresh time from the above procedures, it
>       SHOULD also not execute MP procedures specified in Section 4.3 of
>       this document.
> 
> Just to check: a reduced refresh time in either Resv *or* Path suffices
> to prohibit *all* the MP procedures?
> 
[Chandra] Reducing refresh time is not the only action. There is also explicit text in backward compatibility to prohibit nodes from sending Remote or Conditional PathTear to any downstream node. 

   If a node reduces the refresh time using the above procedures, it
   MUST NOT send any "Remote" PathTear or Conditional PathTear message
   to the downstream node.

> Section 5
> 
> Aren't there more documents whose security considerations are also
> relevant (e.g., RFC 8370, RFC 2961, etc.)?
> 
[Chandra] Yes, I have included RFC 8370 (that in turns includes considerations from RFC 2961).

> Why is utilizing the authentication key for Node-ID Hello messages with
> TTL>1 only a MAY and not a SHOULD?
> 
[Chandra] Yes, I have changed it to SHOULD.

> I'd also suggest to acknowledge the inherent risk when sending
> non-immediate-neighbor Hellos that the intermediate could tamper with
> them and disrupt the connection (though any such intermediate is in a
> position to do worse mischief).
> 
[Chandra] The text making a SHOULD recommendation to support node-id specific authentication is to address this concern.

> If we're going to (per my Discuss) keep all the SHOULDs and not MUSTs,
> we should talk about how failing to follow the SHOULDs will lead to
> increased state usage on peer nodes and potentially DoS.
> 
> It would be reasonable to mention that we have a solid negotiation story
> so that we don't expect to send conditional PathTears to nodes that
> don't comprehend them, which reduces the risk of misinterpretation and
> having a LSP get torn down unnecessarily.
> 
[Chandra] I went through all SHOULDs and converted the appropriate ones to MUSTs. Could you check the diff and comment on them?

> Section 6
> 
> When reading Section 4.4.3 I had the distinct impression that the
> "Reserved" field was intended to have future flags allocated from it.
> Wouldn't it make sense to create a registry with which to do so?
> If I was wrong about this, I might have to revise my previous comments
> on that section.
> 
[Chandra] Yes, asking for IANA registry for the flags field in the CONDITIONS object was the intention. I have added sub-section explicitly requesting that.

Thanks,
Chandra.