[Idr] AD Review of draft-ietf-idr-bgp-ls-segment-routing-msd-09

[Alvaro Retana <[e-mail]>]

Dear authors:

This is my review of draft-ietf-idr-bgp-ls-segment-routing-msd-09.  I
have put some comments in-line below which should be easy to address.

This document, like many other BGP-LS specifications to carry
IGP-derived information is written with that assumption: that the
information will come from the IGPs.  However, §2 reads:

    The BGP-LS speaker may also advertise the MSD information for the local
    node and its links when not running any link-state IGP protocol e.g. when
    running BGP as the only routing protocol.

This case (no IGP) is not fully explained in the text.  Borrowing from
the IGP RFCs, two clear pieces of information that are missing are:

   (1) A definition of what the Node/Link MSDs are.  I borrowed from rfc8476
   and made suggestions for §3 and §4 of this document (in-line).

   (2) "Procedures for Defining and Using Node and Link MSD Advertisements",
   similar to §4 (in both rfc8476/rfc8491).  Suggestion: simply include the
   text from rfc8476 after §4 of this document.


The text in §2 is also not specific on how this local information
should be advertised using BGP-LS.  Should the Direct Protocol
Identifier be used, or, because of "running BGP as the only routing
protocol" should it be BGP?  Please add details to the specification.

One more related point.  The deployment model for collecting
IGP-derived information is that "BGP-LS is configured on a small
number of nodes" [rfc8491], but the model for advertising local
information is different.  Please include some text about that too.


I will wait for a revised document to primarily address this text from
§2 (and the in-line comments marked as [major]) before starting the
IETF LC.

Thanks!

Alvaro.



[Line numbers from idnits.]

...
14	 Signaling MSD (Maximum SID Depth) using Border Gateway Protocol Link-
15	                                 State

[minor] s/Border Gateway Protocol Link-State/Border Gateway Protocol -
Link State/g
That is the form used in rfc7752 -- in the only place where BGP-LS is expanded.


...
79	1.  Introduction

81	   When Segment Routing (SR) [RFC8402] paths are computed by a
82	   centralized controller, it is critical that the controller learns the
83	   Maximum SID Depth (MSD) that can be imposed at each node/link on a
84	   given SR path.  This ensures that the Segment Identifier (SID) stack
85	   depth of a computed path doesn't exceed the number of SIDs the node
86	   is capable of imposing.

[nit] s/learns/learn


...
93	   However, if PCEP is not supported/configured on the head-end of a SR
94	   tunnel or a Binding-SID anchor node, and controller does not
95	   participate in IGP routing, it has no way of learning the MSD of
96	   nodes and links.  BGP-LS [RFC7752] defines a way to advertise
97	   topology and associated attributes and capabilities of the nodes in
98	   that topology to a centralized controller.  This document defines
99	   extensions to BGP-LS to advertise one or more types of MSDs at node
100	   and/or link granularity.

[nit] s/to advertise/to expose
For consistency with the OSPF/ISIS documents.


[minor] Including the last sentence above makes the text not flow
well: it first says that extensions "to advertise one or more types of
MSDs" are defined here...and then it talks about other types of MSDs.
Suggestion:  Move that sentence to be the first one in the last
paragraph.


102	   Other types of MSD are known to be useful.  For example,
103	   [I-D.ietf-ospf-mpls-elc] and [I-D.ietf-isis-mpls-elc] define Readable
104	   Label Depth Capability (RLDC) that is used by a head-end to insert an
105	   Entropy Label (EL) at a depth that can be read by transit nodes.

107	   In the future, it is expected that new MSD-Types will be defined to
108	   signal additional capabilities, e.g., ELs, SIDs that can be imposed
109	   through recirculation, or SIDs associated with another data plane
110	   such as IPv6.  MSD advertisements MAY be useful even if SR itself is
111	   not enabled.  For example, in a non-SR MPLS network, MSD defines the
112	   maximum label depth.

[major] s/MAY/may
There is no Normative action in the sentence; it's just stating a fact.


...
116	1.1.1.  Terminology

118	   BGP-LS: Distribution of Link-State and TE Information using Border
119	   Gateway Protocol

[minor] Don't add a definition for BGP-LS here because it was already
expanded in the Abstract...to mean something else...


121	   MSD: Maximum SID Depth

[minor] Please use the same definition as in rfc8476/rfc8491


...
129	   SID: Segment Identifier

[minor] Please use the same definition as in rfc8476/rfc8491



...
140	   o  pushing one or more new labels onto the label stack.  The number
141	      of labels imposed is then the sum of the number of labels that are
142	      replaced and the number of labels that are pushed.  See [RFC3031]
143	      for further details.

[minor] Split off the last 2 sentences into their own paragraph.


145	1.1.2.  Requirements Language

147	   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
148	   "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
149	   "OPTIONAL" in this document are to be interpreted as described in BCP
150	   14 [RFC2119] [RFC8174] when, and only when, they appear in all
151	   capitals, as shown here .

[nit] s/here ./here.


153	2.  Advertisement of MSD via BGP-LS

155	   This document describes extensions that enable BGP-LS speakers to
156	   signal the MSD capabilities (described in [RFC8491] ) of nodes and
157	   their links in a network to a BGP-LS consumer of network topology
158	   such as a centralized controller.  The centralized controller can
159	   leverage this information in computation of SR paths and their
160	   instantiation on network nodes based on their MSD capabilities.  When
161	   a BGP-LS speaker is originating the topology learnt via link-state
162	   routing protocols like OSPF or IS-IS, the MSD information for the
163	   nodes and their links is sourced from the underlying extensions as
164	   defined in [RFC8476] and [RFC8491] respectively.  The BGP-LS speaker
165	   may also advertise the MSD information for the local node and its
166	   links when not running any link-state IGP protocol e.g. when running
167	   BGP as the only routing protocol.

[nit] s/This document describes extensions that enable BGP-LS speakers
to signal the MSD capabilities (described in [RFC8491] )...The
centralized controller can leverage this information...instantiation
on network nodes based on their MSD capabilities./This document
describes extensions that enable BGP-LS speakers to signal the MSD
capabilities ([RFC8491])...The centralized controller can leverage
this information...instantiation on network nodes.


169	   The extensions introduced in this document allow for advertisement of
170	   different MSD-Types.  This document does not define these MSD-Types
171	   but leverages the definition, guidelines and the code-point registry
172	   specified in [RFC8491].  This enables sharing of MSD-Types that may
173	   be defined in the future by the IGPs in BGP-LS.

[minor] Other MSD-Types, beyond what is defined in rfc8491 have
already been defined...so maybe simplify the paragraph; suggestion:

   The extensions introduced in this document allow for advertisement of
   different MSD-Types, which are defined elsewhere and were introduced in
   [RFC8491].  This enables sharing of MSD-Types that may be defined in the
   future by the IGPs in BGP-LS.


175	3.  Node MSD TLV

177	   Node MSD is encoded in a new Node Attribute TLV [RFC7752] using the
178	   following format:

[major] Suggestion>

   The Node MSD ([RFC8476] [RFC8491]) is encoded in a new Node Attribute TLV
   [RFC7752] to carry the provisioned SID depth of the router identified by the
   corresponding Router-ID.  Node MSD is the smallest MSD supported by the node
   on the set of interfaces configured for use.  MSD values may be learned via
   a hardware API or may be provisioned.  The following format is used:


...
200	      *  MSD-Type : one of the values defined in the IANA registry
201	         titled "IGP MSD-Types" under the "Interior Gateway Protocol
202	         (IGP) Parameters" registry created by [RFC8491].

[nit] s/IANA registry titled "IGP MSD-Types" under the "Interior
Gateway Protocol (IGP) Parameters" registry created by [RFC8491]./"IGP
MSD-Types" registry defined in [RFC8491].


...
210	4.  Link MSD TLV

212	   Link MSD is encoded in a new Link Attribute TLV [RFC7752] using the
213	   following format:

[major] Suggestion>

    The Link MSD ([RFC8476] [RFC8491]) is defined to carry the MSD of the
    interface associated with the link.  It is encoded in a new Link Attribute
    TLV [RFC7752] using the following format:


...
234	      *  MSD-Type : one of the values defined in the IANA registry
235	         titled "IGP MSD-Types" under the "Interior Gateway Protocol
236	         (IGP) Parameters" registry created by [RFC8491].

[nit] s/IANA registry titled "IGP MSD-Types" under the "Interior
Gateway Protocol (IGP) Parameters" registry created by [RFC8491]./"IGP
MSD-Types" registry defined in [RFC8491].


...
243	5.  IANA Considerations
...
250	       +------------+-----------------+---------------------------+
251	       | Code Point |   Description   |     IS-IS TLV/Sub-TLV     |
252	       +------------+-----------------+---------------------------+
253	       |    266     | Node MSD        | 242/23                    |
254	       |    267     | Link MSD        | (22,23,25,141,222,223)/15 |
255	       +------------+-----------------+---------------------------+

[minor] Add a "Reference" column, and include "[This Document]" in it.


257	6.  Manageability Considerations
...
272	   A consumer of the BGP-LS information retrieves this information over
273	   a BGP-LS session (refer Section 1 and 2 of [RFC7752]).  The handling
274	   of semantic or content errors by the consumer would be dictated by
275	   the nature of its application usage and hence is beyond the scope of
276	   this document.

[minor] Personally, I think that pointing to rfc7752 (as has already
been done) is enough...and that the detail in the last paragraph is
unnecessary for this document.  Consider removing it.


278	   This document only introduces new Attribute TLVs and any syntactic
279	   error in them would result in the BGP-LS Attribute being discarded
280	   with an error log.  The MSD information introduced in BGP-LS by this
281	   specification, may be used by BGP-LS consumer applications like a SR
282	   path computation engine (PCE) to learn the SR SID-stack handling
283	   capabilities of the nodes in the topology.  This can enable the SR
284	   PCE to perform path computations taking into consideration the size
285	   of SID Stack that the specific headend node may be able to impose.
286	   Errors in the encoding or decoding of the MSD information may result
287	   in the unavailability of such information to the SR PCE or incorrect
288	   information being made available to it.  This may result in the
289	   headend node not being able to instantiate the desired SR path in its
290	   forwarding and provide the SR based optimization functionality.  The
291	   handling of such errors by applications like SR PCE may be
292	   implementation specific and out of scope of this document.

[major] s/any syntactic error...BGP-LS Attribute being discarded with
an error log./any syntactic error...BGP-LS Attribute being discarded
[RFC7752].


294	   The extensions specified in this document, do not specify any new
295	   configuration or monitoring aspects in BGP or BGP-LS.  The
296	   specification of BGP models BGP and BGP-LS models is an ongoing work
297	   based on the [I-D.ietf-idr-bgp-model].  The management of the MSD
298	   features within an ietf segment-routing stack is also an ongoing work
299	   based on the [I-D.ietf-spring-sr-yang].  Management of the segment
300	   routing in IGPs is ongoing work for ISIS [I-D.ietf-isis-sr-yang] ,
301	   and OSPF [I-D.ietf-ospf-sr-yang].

[nit] "BGP models BGP and BGP-LS models"  I think there's a comma
missing somewhere or maybe too many "BGPs"...  BTW,
I-D.ietf-idr-bgp-model does not include BGP-LS.


[nit] s/an ietf segment-routing stack/an SR stack


[nit] s/of the segment routing/of segment routing


[nit] s/[I-D.ietf-isis-sr-yang] ,/[I-D.ietf-isis-sr-yang],


[minor] I don't think that pointing at all this other work is
necessary in this document.  Please consider removing the paragraph
(except for maybe the first sentence).


303	7.  Security Considerations
...
313	   The document does not introduce additional security issues beyond
314	   discussed in [RFC7752], [RFC8476] and [RFC8491].  However, [RFC7752]
315	   is being revised in [I-D.ietf-idr-rfc7752bis] to provide additional
316	   clarification in several portions of the specification after
317	   receiving feedback from implementers.  One of the places that is
318	   being clarified is the error handling and security.  It is expected
319	   that after [I-D.ietf-idr-rfc7752bis] is released that implementers
320	   will update all BGP-LS base implementations improving the error
321	   handling for protocol work (including this document) that depend on
322	   this function.

[major] "The document does not introduce additional security issues
beyond discussed in [RFC7752], [RFC8476] and [RFC8491]."  True, but
not complete.

Suggestion>

   The procedures and protocol extensions defined in this document do not
   affect the BGP security model.  See the "Security Considerations" section of
   [RFC4271] for a discussion of BGP security.  Also, refer to [RFC4272] and
   [RFC6952] for analyses of security issues for BGP. Security considerations
   for acquiring and distributing BGP-LS information are discussed in [RFC7752].

   The TLVs introduced in this document are used to propagate the MSD IGP
   extensions defined in [RFC8476] and [RFC8491].  It is assumed that the IGP
   instances originating these TLVs will support all the required security (as
   described in [RFC8476] and [RFC8491]) in order to prevent any security
   issues when propagating the TLVs into BGP-LS.

   The advertisement of the node and link attribute information defined in this
   document presents no additional risk beyond that associated with the
   existing node and link attribute information already supported in [RFC7752].

The references to rfc4271, rfc4272 and rfc6952 can be Informative.


[minor] Including the reference to rfc7752bis may raise unnecessary
questions and potentially delay the publication of this document.
Note that the conclusion that "implementers will update all BGP-LS
base implementations" is the natural result of Obsoleting rfc7752.
Please consider removing the references.


...
366	10.2.  Informative References
...
401	   [I-D.ietf-pce-segment-routing]
402	              Sivabalan, S., Filsfils, C., Tantsura, J., Henderickx, W.,
403	              and J. Hardwick, "PCEP Extensions for Segment Routing",
404	              draft-ietf-pce-segment-routing-16 (work in progress),
405	              March 2019.

== Outdated reference: draft-ietf-pce-segment-routing has been published as
   RFC 8664