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

[John Scudder <[e-mail]>]

Hi Chandra,

My replies are inline.

Thanks,

—John

> On Jul 15, 2024, at 12:30 PM, Chandrasekar Ramachandran <[e-mail]> wrote:
> 
> Hi John,
> Please see inline (prefixed [CR2]) for responses.
> 
> 
> Juniper Business Use Only
>> -----Original Message-----
>> From: John Scudder <[e-mail]>
>> Sent: Thursday, June 27, 2024 11:59 PM
>> To: Chandrasekar Ramachandran <[e-mail]>
>> Cc: The IESG <[e-mail]>; [e-mail]; mpls-
>> [e-mail]; [e-mail]; Nicolai Leymann <[e-mail]>
>> Subject: Re: John Scudder's Discuss on draft-ietf-mpls-ri-rsvp-frr-18: (with
>> DISCUSS and COMMENT)
>> 
>> Hi Chandra,
>> 
>> Thanks for your reply. I hadn’t realized there was such a long history to the
>> issue raised in my DISCUSS. Based on the fact I ended up DISCUSSing on
>> virtually the same issue Alvaro did in 2019, I have two immediate reactions:
>> 
>> - I’ll clear the DISCUSS because even though I don’t love it, this issue has been
>> raised and considered already.
>> 
>> - However, some additional (editorial level) work should be done to further
>> clarify the document in these areas. See more in line below.
>> 
>> —John
>> 
>>> On Jun 26, 2024, at 7:07 AM, Chandrasekar Ramachandran
>> <[e-mail]> wrote:
>>> 
>>> Thanks for the detailed review. Please see inline (prefixed [CR]) for
>> responses.
>>> 
>>> Thanks,
>>> Chandra.
>>> 
>>> 
>>> 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/st
>>>> atem
>>>> 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-ie
>>>> tf-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://mailarchive.ietf.org/arch/msg/mpls/gMpvz7ez80a9AfOdPOvPSUZEcp
>> M/).
>>> 
>>>> 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?
>> 
>> I’m going to move to No Objection, which means that I think the document
>> *can* proceed without further changes if you and the responsible AD agree
>> to it, but I still think you *should* make a change here. Let me try again to
>> explain my point. The current Section 4.1 text is,
>> 
>> OLD:
>>   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.
>> 
>> IMO, the way you've chosen to express this makes the intent unclear. I had
>> offered the rewrite,
>> 
>> 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.
>> 
>> This is intended to simply be a rearrangement of the clauses in a way that
>> makes it easier for a reader to understand what your intent is. If you think I've
>> changed your meaning vs. the original, please help me understand what you
>> think the difference is.
> 
> [CR2] The authors are fine with the proposed text. We will update the text in the next version of the draft.

Great, thanks.

>>>> (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://datatracker.ietf.org/doc/draft-ietf-teas-rsvp-te-scaling-rec/02/) 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://mailarchive.ietf.org/arch/msg/mpls/KDRVWBAajl08pnXDkit0ykRyZDg
>>> /
>> 
>> OK. I accept this subsection was the negotiated settlement to the objection
>> Alvaro raised, and then I later unknowingly re-raised. I think it should be
>> moved to a different section, however, since it is indeed not a procedure for
>> backward compatibility at all (right?). I suggest you retitle it,
>> 
>> OLD:
>>   Advertising RI-RSVP without RI-RSVP-FRR
>> 
>> NEW:
>>   Consequence of Advertising RI-RSVP without RI-RSVP-FRR
>> 
>> And that you move it up a couple of levels, making it Section 4.7 for instance.
>> 
> 
> [CR2] Agree it will be appropriate not to include under backward compatibility section but instead move it to a different section as suggested. We will move and change the title in the next version of the draft.

Great, thanks.

>> Finally, I think you could make exactly what this section is doing more
>> transparent with a small edit, viz,
>> 
>> OLD:
>>   If a node supporting facility backup protection [RFC4090] sets the
>>   RI-RSVP capability (I bit) but does not support the RI-RSVP-FRR
>>   extensions, then it leaves room for stale state to linger around for
>>   an inordinate period of time or disruption of normal FRR operation
>>   (see Section 3 of this document).
>> 
>> NEW:
>>   If a node supporting facility backup protection [RFC4090] sets the
>>   RI-RSVP capability (I bit) but does not support the RI-RSVP-FRR
>>   extensions, for example because it is a legacy node, or due to an
>>   implementation bug or configuration error, then it leaves room for
>>   stale state to linger around for an inordinate period of time or
>>   disruption of normal FRR operation (see Section 3 of this document).
>> 
> 
> [CR2] As pointed out in an earlier mail that the authors consider that it is extremely unlikely that an MPLS RSVP-TE implementation that supports RFC4090 would implement only RI-RSVP and not RI-RSVP-FRR, we will adopt the NEW text but only removing the phrase on legacy node.
> 
> NEW:
>   If a node supporting facility backup protection [RFC4090] sets the
>   RI-RSVP capability (I bit) but does not support the RI-RSVP-FRR
>   extensions, due to an implementation bug or configuration error,
>   then it leaves room for stale state to linger around for an inordinate
>   period of time or disruption of normal FRR operation (see Section 3
>   of this document).

Works for me.

>>>> 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
>> 
>> Thanks for resolving my previous comments. I have one new comment,
>> related to the changes made since my last review.
>> 
>> ### Section 4.4.3, what other bits could there be?
>> 
>> Version 21 has,
>> 
>>      If no bit is set, then the PathTear message MUST be processed as a
>>      normal PathTear message for the LSP.
>> 
>> I see this was introduced after my last review. I'm curious as to the reason for
>> the change (I suppose it was the result of somebody else's review). I ask
>> because, the previous text spoke specifically about "the M-bit" but now you
>> speak generically about "no bit", presumably to include the M-bit, but not
>> exclusively. The problem is, I don't see any other bit flags in the object header,
>> even potentially. The fields are Length, Class, C–type, a one-bit field called M-
>> bit, and a field 31 bits wide, called "Reserved", but notably, not called "flags"
>> or something like that.
>> 
>> So, what is the difference between the old text, and the new? As far as I can
>> tell the new text while correct, is less transparent than the old text without
>> bringing any benefit.
>> 
> 
> [CR2] The above change was made to address the following questions from IANA:
> **
> RFC 3936 says that new Class Types registries have 256 values. The current version of this document assigns value 1, but doesn't tell us what to do with value 0. Should it be reserved?

I’m curious what your resolution to that question from IANA was? In version 21 I still see nothing about value 0 or for that matter values 2-255. I do see that other class types registries are also silent about value 0 and unassigned values, so maybe you decided that’s fine. I’m also curious what the registration policy is for the c-types conditions sub-registry. The header before all the c-types, https://www.iana.org/assignments/rsvp-parameters/rsvp-parameters.xhtml#rsvp-parameters-4 says that "any new Class Number definition must specify an appropriate IANA Considerations policy for assigning additional Class Type values” so it seems you’re supposed to supply one (although I don’t see them for the other conditions sub-registries).

> Can you confirm that the new CONDITIONS Objects Flags registry shouldn't have a registration or reservation that has the value 0x000?
> **
> Please also refer to the corresponding changes in the IANA Considerations Section (Section 6.1).
> 
> This document defines just one bit flag, but our intent was to leave room for other bit flags to be introduced in future documents. In case, some implementation includes the CONDITIONS object in a Tear message and does not set any flag, then it should be treated like a regular unconditional Tear message. That was the motivation behind saying – “If no bit is set..”.
> 
> Would the following changes address your concern?
> 
> In Section 4.4.3:
> Class: TBA1
> C-type: 1
> Reserved bits: Reserved bits MUST be set to zero on transmission and MUST be ignored on receipt.

That’s good enough. IMO if you intend that the entire field marked “Reserved” is a flags field, it’s even better to write it like,

```
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Length | Class | C-type |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
| Flags (Reserved) |M|
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

    • Class: TBA1
• C-type: 1
• Flags is a 32-bit field. Bit 31 is the Merge-point condition (M) bit: If the M bit is set, then the PathTear message MUST be processed according to the receiver router role, i.e. if the receiving router is an MP or not for the LSP. If it is not set, then the PathTear message MUST be processed as a normal PathTear message for the LSP. Bits 0-30 are reserved, they MUST be set to zero on transmission and MUST be ignored on receipt.
```

The point of the suggested revision is to clarify that the field isn’t just 31 unstructured bits with no intended use, but that it’s all flags. (And knowing that sometimes these decisions get revised in the future, too — at some point if bits 0-15 remain unallocated, some later RFC could be written to redefine them as a 16-bit integer field, for example. But that’s OK, we should document your current intent as well as we can.)

By the way, I noticed you left off the customary bit number ruler at the top of the figure. The RFCEd would probably fix this, but you should do so in your next rev.

> Merge-point condition (M) bit: If the M bit is set to 1, then the PathTear message MUST be processed according to the receiver router role, i.e. if the receiving router is an MP or not for the LSP.  If the M bit is set to 0, then the PathTear message MUST be processed as a normal PathTear message for the LSP.
> 
> In Section 6.1:
> s/sub-registry for "CONDITIONS Object Flags"/sub-registry for "CONDITIONS Object Values"

I think it makes better sense as flags, again based on your statement that "our intent was to leave room for other bit flags to be introduced”. Your current text is,

OLD:
Bit Number Hex Value Name Reference
- 0x0000 No Condition This document
31 0x0001 Merge-point This document

It seems to me as though the actual confusion arises from the “-“ line, so possibly, imitating the convention used by the other flags sub-registry already under RSVP parameters, https://www.iana.org/assignments/rsvp-parameters/rsvp-parameters.xhtml#rsvp-parameters-119,

NEW:
Bit Number  32-Bit Value  Name  Reference 
0-30  Unassigned 
31 0x0001 Merge-point This document

> thanks,
> Chandra.