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

[Benjamin Kaduk via Datatracker <[e-mail]>]

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://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-mpls-ri-rsvp-frr/



----------------------------------------------------------------------
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?

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?

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

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?

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.

   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?

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)

   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?

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?

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'?

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?

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

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

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

Section 4.6.2

To double-check: the procedures in the subsections only apply when the
ingress has requested protection, right?

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?

Section 5

Aren't there more documents whose security considerations are also
relevant (e.g., RFC 8370, RFC 2961, etc.)?

Why is utilizing the authentication key for Node-ID Hello messages with
TTL>1 only a MAY and not a 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).

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.

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.