mLDP Extensions for Multi-Topology Routing
draft-ietf-mpls-mldp-multi-topology-09

Many thanks to Christian Huitema's secdir review of this draft.  Please consider his recommendation for the Security Consideration section:

"The security section says that "This extension to mLDP does not introduce any
new security considerations beyond that already apply to the base LDP
specification [RFC5036], base mLDP specification [RFC6388], and MPLS security
framework [RFC5920]."

That may very well be true, but I would encourage the authors to examine at
least two risks: creating multiple topologies create additional work in the
"control plane", thus potential resource exhaustion if trying to support too
many topologies; traffic carried by multiple topologies may end up competing
for finite data plane resource, thus risking local overload. I am speculating,
but have the authors studied the corresponding failure modes? How hard is it
for configuration mistakes or adversarial actions to exploit such failure modes?"


Nits:

Glossary:  It would be easier if this was in some sort of alphabetical order (either by the acronym or the expansion).  

Missing from Glossary:  There are a bagillion undefined acronyms.  LIB, for example, which is not expanded anywhere that I can find, nor is it in the Editor's list.  Please consider adding those acronyms that are not contained here:  https://www.rfc-editor.org/materials/abbrev.expansion.txt

# Gunter Van de Velde, RTG AD, comments for draft-ietf-mpls-mldp-multi-topology-05.txt

Please find https://www.ietf.org/blog/handling-iesg-ballot-positions/ documenting the handling of ballots.

Many thanks for the RTG_DIR reviews from Mike McBride, Yingzhen Qu and Gyan Mishra 

#GENERIC COMMENTS
#================
##Text and concept is reasonably clear explained in the document, although in some sections i did get a slightly lost and had to search internet to understand the flow of formal procedures

## There are some potentially normative RFC2119 words in the document that are not capitalized.

#DETAILED COMMENTS
#=================

129	   A more light weight mechanism to define constraint-based topologies
130	   is the Flexible Algorithm (FA) [RFC9350].  FA can be seen as creating
131	   a sub-topology within a topology using defined topology constraints
132	   and computation algorithm.  This can be done within a MTR topology or
133	   the default Topology.  An instance of such a sub-topology is
134	   identified by a 1 octet value as documented in [RFC9350]).  Flexible
135	   Algorithm is a mechanism to create a sub-topology, but in the future
136	   different algorithms might be defined on how to achieve that.  For
137	   that reason, in the remainder of this document, we'll refer to this
138	   as the IGP Algorithm (IPA).

We should explicit mention the name of this field referenced above 
as "identified by a 1 octet value" is the IGP Algorithm Types defined in RFC8665 
where values 128-255 are reserved for IGP Flexible Algorithms. Maybe this can 
be weaved into the text accordingly.
ref: https://www.iana.org/assignments/igp-parameters/igp-parameters.xhtml#igp-algorithm-types


170	   The same applies to the IPA.  The IPA needs to be encoded as part of
171	   the mLDP FEC to create unique MP-LSPs and at the same time is used to
172	   signal to mLDP (hop-by-hop) which Algorithm needs to be used to
173	   create the MP-LSP.

I find the term IPA slightly odd for a reader. It is one extra term used. Usage 
of "IGP Algorithm Type" seems to read well and does not add so many letters 
and does not add a new abbreviation. I understand that IPA is used in the 
fields encoded and hence i assume it is used in this text blob.

180	   Following subsections propose the extensions to bind an mLDP FEC to a
181	   topology.  The mLDP MT extensions reuse some of the extensions
182	   specified in [RFC7307].

What about following rewrite:
"The following subsections propose extensions to bind an mLDP FEC to a topology. 
These mLDP MT extensions reuse some of the extensions specified in [RFC7307]."

202	   Where "Root Node Address" encoding is as defined for given "Address
203	   Family", and whose length (in octets) is specified by the "AF Length"
204	   field.

this reads rather odd. Is this trying to say:
"The "Root Node Address" encoding is defined according to the given "Address Family," with 
its length (in octets) specified by the "AF Length" field."

Is there need for normative language here?

206	   To extend MP FEC elements for MT, the {MT-ID, IPA} is a tuple that is
207	   relevant in the context of the root address of the MP LSP.  The {MT-
208	   ID, IPA} tuple determines in which (sub)-topology the root address
209	   needs to be resolved.  Since the {MT-ID, IPA} tuple should be
210	   considered part of the mLDP FEC, the most natural place to encode
211	   this tuple is as part of the root address.  While encoding it, we
212	   also propose to use "MT IP" Address Families as described in
213	   following sub section.

What about following readability rewrite. It also removes the "we":
"To extend MP FEC elements for MT, the {MT-ID, IPA} tuple is relevant in the 
context of the root address of the MP LSP. This tuple determines the (sub)-topology 
in which the root address needs to be resolved. As the {MT-ID, IPA} tuple should 
be considered part of the mLDP FEC, it is most naturally encoded as part of the 
root address. Additionally, we propose using "MT IP" Address Families as described 
in the following subsection for this encoding.
"

211	   this tuple is as part of the root address.  While encoding it, we
212	   also propose to use "MT IP" Address Families as described in
213	   following sub section.

I am not sure what this phrase is trying to say at this stage. Maybe i 
got thrown of the rails by the usage of the word "propose". Is such statement not 
requiring normative language? 

217	   [RFC7307] has specified new address families, named "MT IP" and "MT
218	   IPv6", to allow specification of an IP prefix within a topology
219	   scope.  In addition to using this address family for mLDP, we also
220	   use 8 bits of the 16 bits Reserved field to encode the IGP Algorithm
221	   (IPA) Registry.  The resulting format of the data associated with
222	   these new Address Families is as follows:

I am not a fan of using the word "we" in formal procedures. It i sunclear who 
is "we". (WG, IETF, authors, etc?)

What about following rewrite:
"[RFC7307] specifies new address families, named "MT IP" and "MT IPv6," to 
allow for the specification of an IP prefix within a topology scope. In addition 
to using these address families for mLDP, 8 bits of the 16-bit Reserved field 
are utilized to encode the IGP Algorithm (IPA) Registry. The resulting format 
of the data associated with these new Address Families is as follows:"

250	      IPA: The IGP Algorithm, values are from the IGP Algorithm
251	      registry.

This is almost correct. The registry is known as "IGP Algorithm Types"
https://www.iana.org/assignments/igp-parameters/igp-parameters.xhtml#igp-algorithm-types

258	   By using extended MT IP Address Family, the resultant MT MP FEC
259	   element is to be encoded as follows:

I suspect you are trying to say:

"By using the extended MT IP Address Family, the resulting MT MP FEC element 
should be encoded as follows:
"

277	   In the context of this document, the applicable LDP FECs for MT mLDP
278	   include:

I had to look up where these code points were defined. AFter some searching was RC6388.
The RFC was previously suggested, but the reference here could maybe be more explite

288	   *  Typed Wildcard FEC Element (type 0x5)

Here similar as previous code points. The reference rfc5918 would be helpfull
source: https://www.iana.org/assignments/ldp-namespaces/ldp-namespaces.xhtml#fec-type

290	   In case of "Typed Wildcard FEC Element", the sub FEC Element type
291	   MUST be one of the MP FECs listed above.

The figure 3 does not show the sub FEC Element type. Maybe a single phrase could be 
added how it fits together high level. (i assume this may be well known in mLDP 
environment though) 

305	   This document assumes the same definitions and procedures associated
306	   with MPLS MT-ID as defined in [RFC7307] specification.

s/defined/specified/

310	   "MT Multipoint Capability" is a new LDP capability, defined in
311	   accordance with LDP Capability definition guidelines [RFC5561], that
312	   is to be advertised to its peers by an mLDP speaker to announce its
313	   capability to support MTR and the procedures specified in this
314	   document.  This capability MAY be sent either in an Initialization
315	   message at the session establishment time, or in a Capability message
316	   dynamically during the lifetime of a session (only if "Dynamic
317	   Announcement" capability [RFC5561] has been successfully negotiated
318	   with the peer).

This text blob reads not naturally flowing.
What about the following readability re-edit, and i hope that i kept the 
intended formal procedures intact:
"The "MT Multipoint Capability" is a new LDP capability, defined in accordance 
with the LDP Capability definition guidelines outlined in [RFC 5561]. An mLDP 
speaker advertises this capability to its peers to announce its support for MTR 
and the procedures specified in this document. This capability MAY be sent 
either in an Initialization message at session establishment or dynamically 
during the session's lifetime via a Capability message, provided that 
the "Dynamic Announcement" capability from [RFC 5561] has been 
successfully negotiated with the peer.
"

339	      Length: The length (in octets) of TLV.  The value of this field
340	      MUST be 1 as there is no Capability-specific data [RFC5561] that
341	      follows in the TLV.

There seems some word missing in "of TLV".
Maybe you intented to say:
"Length: This field specifies the length of the TLV in octets. The value 
of this field MUST be 1, as there is no Capability-specific data [RFC 5561 
following the TLV].
"

361	   The MT extensions proposed in document do not require any extension
362	   in procedures for Typed Wildcard FEC Element support [RFC5918], and
363	   these procedures apply as-is to Multipoint MT FEC wildcarding.  Like

What does "proposed in document" mean? is potentially a word 
missing (proposed in 'this' document")?

367	   operations for MP FECs in the context of a given (sub)-topology as
368	   identified by the MT-ID and IPA field.

As mentioned in an earlier observation the (sub)-topology is not displayed 
in the figure5. WHere is the sub-topology corelated?

391	      IPA: The IGP Algorithm, values are from the IGP Algorithm
392	      registry.

The IANA registry is the "IGP Algorithm Types" registry to be 
formally correct


399	   [RFC5919] specifies extensions and procedures that allows an LDP
400	   speaker to signal its End-of-LIB (i.e. convergence) for a given FEC
401	   type towards a peer.  MT extensions for MP FEC do not require any
402	   change in these procedures and they apply as-is to MT MP FEC
403	   elements.  This means that an MT mLDP speaker MAY signal its
404	   convergence per (sub-)topology using MT Typed Wildcard MP FEC
405	   element.

I took liberty to add a little more context around End-of-LIB for 
improved for readability. I hope it retains the original intent 
of the authors:

"[RFC5919] specifies extensions and procedures that allow an LDP speaker 
to signal its End-of-LIB for a given FEC type to a peer. By leveraging 
the End-of-LIB message, LDP ensures that label distribution remains 
consistent and reliable, even during network disruptions or maintenance 
activities. The MT extensions for MP FEC do not require any modifications 
to these procedures and apply as-is to MT MP FEC elements. Consequently, an 
MT mLDP speaker MAY signal its convergence per (sub-)topology using 
the MT Typed Wildcard MP FEC element."

445	   To support LSP ping for MT Multipoint LSPs, this document uses
446	   existing sub-types "P2MP LDP FEC Stack" and "MP2MP LDP FEC Stack"
447	   defined in [RFC6425].  The proposed extension is to specify "MT IP"
448	   or "MT IPv6" in the "Address Family" field, set the "Address Length"
449	   field to 8 (for MT IP) or 20 (for MT IPv6), and encode the sub-TLV
450	   with additional {MT-ID, IPA} information as an extension to the "Root
451	   LSR Address" field.  The resultant format of sub-tlv is as follows:

I believe that the text should be more prescriptive. We are not longer 
proposing an extension, but are providing the extension. A proposal implies 
that that others may have other proposals to be considered, while that is 
obviously not the intent. Hence maybe:

s/proposed/LSP Ping/

515	10.  Security Considerations

Should the security considerations of RFC7307 be mentioned?

522	11.  IANA Considerations

In section 9.1 there is suggestion of an implementation. Should the code 
point used by that implementation not be suggested to be allocated by 
IANA to avoid code-point squatting?

GENERAL COMMENT:

Does this capability need to be configured, or is it enabled by default? If it needs to be configured, is there a plan to develop an augment to the YANG model to enable this capability?

Thanks to Roni Even for the General Area Review Team (Gen-ART) review. I do agree with his comments and would like to see the authors address it.
(https://mailarchive.ietf.org/arch/msg/gen-art/uETdASJBc4Pwq9qwkSA61R2MPvY).

-------------------------------------------------------------------------------
COMMENT
-------------------------------------------------------------------------------

Section 4.1.2, paragraph 8
>       Reserved: This 8-bit field MUST be zero on transmission and
>       ignored on receipt.

You mean to say "MUST be zero on transmission and MUST be ignored on receipt". Just make it explicit.

Section 4.1.3, paragraph 12
>    [RFC6514] defines the PMSI tunnel attribute for MVPN.  When the
>    Tunnel Type is set to mLDP P2MP LSP, the Tunnel Identifier is a P2MP
>    FEC Element.  When the Tunnel Type is set to mLDP Multipoint-to-
>    Multipoint (MP2MP) LSP, the Tunnel Identifier is an MP2MP FEC
>    Element.  For deploying mLDP in a network that supports MTR and FA,
>    New MT MP FEC element SHOULD be used as the Tunnel Identifier.

Do not see PMSI expanded anywhere in the document. Same for MVPN.

Found IP blocks or addresses not inside RFC5737/RFC3849 example ranges:
"2.4.1.2" and "2.4.1.1".

-------------------------------------------------------------------------------
NIT
-------------------------------------------------------------------------------

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

"Abstract", paragraph 1
> h that when building a Multipoint LSPs(Label Switched Paths) it can follow a
>                                       ^
It appears that a white space is missing.

Section 1, paragraph 9
> to make LDP MT-aware and be able to setup unicast Label Switched Paths (LSPs)
>                                     ^^^^^
The verb "set up" is spelled as two words. The noun "setup" is spelled as one.

Section 1, paragraph 12
> ng IGP MT routing paths. A more light weight mechanism to define constraint-b
>                                 ^^^^^^^^^^^^
Did you mean "lightweight"?

Section 2, paragraph 1
> on algorithm. This can be done within a MTR topology or the default Topology.
>                                       ^
Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
"an article", "an hour".

Section 2, paragraph 2
> LDP) refers to extensions in LDP to setup multi-point LSPs (point-to-multipoi
>                                     ^^^^^
The verb "set up" is spelled as two words. The noun "setup" is spelled as one.

Section 2, paragraph 3
> oding so that LDP peers are able to setup an MP LSP via their own defined MTR
>                                     ^^^^^
The verb "set up" is spelled as two words. The noun "setup" is spelled as one.

Section 6.1, paragraph 2
> ies extensions and procedures that allows an LDP speaker to signal its End-o
>                                    ^^^^^^
Possible subject-verb agreement error detected.

Thanks to Roni Evan for the GENART review.

** Per the GENART review:

-- Section 4.1.2
   In addition to using this address family for mLDP, we also
   use 8 bits of the 16 bits Reserved field to encode the IGP Algorithm
   (IPA) Registry.

Should RFC7307 be added to the document meta-data as being updated since the reserved fields are affected?

-- Section 4.1.3
   In the context of this document, the applicable LDP FECs for MT mLDP
   include:

   *  MP FEC Elements:

      -  P2MP (type 0x6)

      -  MP2MP-up (type 0x7)

      -  MP2MP-down (type 0x8)

Should this document be added to the following entries in the FEC Type Name Space registry (https://www.iana.org/assignments/ldp-namespaces/ldp-namespaces.xhtml#fec-type) as a reference?

** Section 6.1. Editorial.

      IPA: The IGP Algorithm, values are from the IGP Algorithm
      registry.

Provide a reference to the IGP Algorithms registry

Thanks for your work, I am clearing my DISCUSS. There’s one issue with the new text:

“The IGP Algorithm (IPA) Field Section 4.1.2 Section 6.1 is an 8-bit identifier for the algorithm. The permissible values are tracked in the IANA IGP Algorithm Types registry ([RFC9350] section 18.1.2).”

But, if I look at that reference, section 18.1.2 is “IGP Metric-Type Registry”. I won’t continue to hold a DISCUSS on this, I rely on the authors and responsible AD to make the correction.

Question 11 on the shepherd writeup is incomplete.

The solitary SHOULD in this document is in Section 4.1.3, but it doesn't tell me why I might legitimately decide not to do what it says.

Nits:

* Section 5: s/Capaility/Capability/

* The syntax of the references in Sections 7.1 and 7.2 is curious.

* I don't think you need "(IANA)" in the diagram in Section 5.

On further reflection, I have cleared my discuss; I would still like to see an answer, but am not holding it as a discuss