Subject: [Idr] AD Review of draft-ietf-idr-bgp-ls-sbfd-extensions-06

From: Alvaro Retana <[email-masked]>
Date: Mon, 11 Apr 2022 07:41:27 -0700

Dear authors:

Please take a look at the comments below.  I will wait for them to be
addressed before moving this document forward.

** Chairs: 6 authors are listed on the front page.  Please provide a
justification for having more than 5.

Thanks!

Alvaro.

[Line numbers from idnits.]

...
81	1.  Introduction
...
90	   For monitoring of a service path end-to-end via S-BFD, the headend
91	   node (i.e.  Initiator) needs to know the S-BFD Discriminator of the
92	   destination/tail-end node (i.e.  Responder) of that service.  The
93	   link-state routing protocols (IS-IS, OSPF and OSPFv3) have been
94	   extended to advertise the S-BFD Discriminators.  With this a
95	   Initiator can learn the S-BFD discriminator for all Responders within
96	   its IGP area/level, or optionally within the domain.  With networks
97	   being divided into multiple IGP domains for scaling and operational
98	   considerations, the service endpoints that require end to end S-BFD
99	   monitoring often span across IGP domains.

[minor] "(IS-IS, OSPF and OSPFv3) have been extended"

This would be a good place to introduce the references to rfc7883/rfc7884.

[nit] s/a Initiator/an Initiator

...
123	3.  Problem and Requirement

[] I appreciate the additional background, but this section is not
needed.  The Introduction already does a good job in providing a
justification when it says this:

   With networks being divided into multiple IGP domains for scaling and
   operational considerations, the service endpoints that require end to
   end S-BFD monitoring often span across IGP domains.

I strongly suggest that you remove this section.

I am including other comments below in case you decide to keep it.

125	   Seamless MPLS [I-D.ietf-mpls-seamless-mpls] extends the core domain
126	   and integrates aggregation and access domains into a single MPLS
127	   domain.  In a large network, the core and aggregation networks can be
128	   organized as different ASes.  Although the core and aggregation
129	   networks are segmented into different ASes, an end-to-end label
130	   switched path (LSP) can be created using hierarchical BGP signaled
131	   LSPs based on internal-BGP (IBGP) labeled unicast within each AS, and
132	   external-BGP (EBGP) labeled unicast to extend the LSP across AS
133	   boundaries.  This provides a seamless MPLS transport connectivity for
134	   any two service end-points across the entire domain.  In order to
135	   detect failures for such end to end services and trigger faster
136	   protection and/or re-routing, S-BFD MAY be used for the Service Layer
137	   (e.g. for MPLS VPNs, pseudowires, etc. ) or the Transport Layer
138	   monitoring.  This creates the need for setting up S-BFD session
139	   spanning across AS domains.

[major] s/S-BFD MAY be used/S-BFD may be used

This is just a statement of fact, not a normative statement.

[] The Introduction already justified the use of BGP-LS to carry the
IGP-derived information.  Do we really need this additional
justification based on a draft that expired more than 7 years ago?

141	   In a similar Segment Routing (SR) [RFC8402] multi-domain network, an
142	   end to end SR Policy [I-D.ietf-spring-segment-routing-policy] path
143	   may be provisioned between service end-points across domains either
144	   via local provisioning, or by a controller or signalled from a Path
145	   Computation Engine (PCE) [RFC4655] . Monitoring using S-BFD can
146	   similarly be setup for such a SR Policy.

[] Again, these are just use examples of what it already justified.

148	   Extending the automatic discovery of S-BFD discriminators of nodes
149	   from within the IGP domain to cross an administrative domain using
150	   BGP-LS enables creating S-BFD sessions on demand across IGP domains.
151	   The S-BFD discriminators for service end point nodes MAY be learnt by
152	   the PCE or a controller via the BGP-LS feed that it gets from across
153	   IGP domains, and it can signal or provision the remote S-BFD
154	   discriminator on the Initiator on demand when S-BFD monitoring is
155	   required.  The mechanisms for the signaling of the S-BFD
156	   discriminator from the PCE/controller to the Initiator and setup of
157	   the S-BFD session are outside the scope of this document.

[major] s/MAY be learnt/may be learnt

This is just a statement of fact, not a normative statement.

[] This paragraph goes into PCE and other unrelated things to carrying
the IGP-derived information in BGP-LS, and ends up with declaring it
out of scope anyway.

159	   Additionally, the service end-points themselves MAY also learn the
160	   S-BFD discriminator of the remote nodes themselves by receiving the
161	   BGP-LS feed via a route reflector (RR) [RFC4456] or a centralized BGP
162	   Speaker that is consolidating the topology information across the
163	   domains.  The Initiator can then itself setup the S-BFD session to
164	   the remote node without a controller/PCE assistance.

[major] s/MAY also learn/may also learn
This is just a statement of fact, not a normative statement.

[] We're now going into how BGP-LS works and deployment models.  Is
there anything specific to this extension that would require
additional considerations (vs other BGP-LS information)?

166	   While this document takes examples of MPLS and SR paths, the S-BFD
167	   discriminator advertisement mechanism is applicable for any S-BFD
168	   use-case in general.

[] Right!!  No need to go into any of these examples!!!

170	4.  BGP-LS Extensions for S-BFD Discriminator

172	   The BGP-LS [RFC7752] specifies the Node NLRI for advertisement of
173	   nodes and their attributes using the BGP-LS Attribute.  The S-BFD
174	   discriminators of a node are considered as its node level attribute
175	   and advertised as such.

[nit] s/considered as its node level attribute/considered a node level attribute

...
200	   o  Length: variable.  Minimum of 4 octets and increments of 4 octets
201	      there on for each additional discriminator

[major] NEW>
     Length: variable, as defined in [RFC7752].  It MUST be a minimum of 4
     octets and increments of 4 octets for each additional discriminator.

[major] What should the receiver do if the length is wrong?

203	   o  Discriminators : multiples of 4 octets, each carrying a S-BFD
204	      local discriminator value of the node.  At least one discriminator
205	      MUST be included in the TLV.

[minor] s/Discriminators : multiples of 4 octets, each/Discriminator n
: 4 octets each,

207	   The S-BFD Discriminators TLV can be added to the BGP-LS Attribute
208	   associated with the Node NLRI that originates the corresponding
209	   underlying IGP TLV/sub-TLV as described below.  This information is
210	   derived from the protocol specific advertisements as below..

[nit] s/as below../as follows:

...
215	   o  OSPFv2/OSPFv3, as defined by the S-BFD Discriminators TLV in
216	      [RFC7884].

[major] s/S-BFD Discriminators TLV/S-BFD Discriminator TLV

218	   When the node is not running any of the IGPs but running a protocol
219	   like BGP, then the locally provisioned S-BFD discriminators of the
220	   node MAY be originated as part of the BGP-LS attribute within the
221	   Node NLRI corresponding to the local node.

[major] The subject of including non-IGP information, BGP
specifically, has come up in other BGP-LS-related drafts.  The
conclusion has been that the process is currently not specified and
that draft-ietf-idr-bgp-ls-bgp-only-fabric may be the best place to do
that -- at least that was the conclusion the last time we discussed it
[1].  Has anything changed?  If not, then please remove the paragraph
above.

[1] https://mailarchive.ietf.org/arch/msg/idr/7nNtfqJ6OyktdP4QETX2xHSHbTw/

223	5.  IANA Considerations

225	   This document requests assigning code-points from the registry "BGP-
226	   LS Node Descriptor, Link Descriptor, Prefix Descriptor, and Attribute
227	   TLVs" based on table below which reflects the values assigned via the
228	   early allocation process.  The column "IS-IS TLV/Sub-TLV" defined in
229	   the registry does not require any value and should be left empty.

[] NEW>
   IANA is requested to permanently allocate the following codepoints from
   the "BGP-LS Node Descriptor, Link Descriptor, Prefix Descriptor, and
   Attribute TLVs" registry. The column "IS-IS TLV/Sub-TLV" defined in the
   registry does not require any value and should be left empty.

231	          +---------------+--------------------------+----------+
232	          |  Code Point   | Description              | Length   |
233	          +---------------+--------------------------+----------+
234	          |     1032      | S-BFD Discriminators TLV | variable |
235	          +---------------+--------------------------+----------+

[minor] Please add a table number.

237	6.  Manageability Considerations

239	   This section is structured as recommended in [RFC5706].

[minor] I don't have a real objection to this sentence, but given that
§6.1 and §6.2 are empty, it may just result in more questions than
needed.  I recommend that you eliminate the sentence and the
subsections.

...
258	7.  Security Considerations

260	   The new protocol extensions introduced in this document augment the
261	   existing IGP topology information that was distributed via [RFC7752].
262	   Procedures and protocol extensions defined in this document do not
263	   affect the BGP security model other than as discussed in the Security
264	   Considerations section of [RFC7752].  More specifically the aspects
265	   related to limiting the nodes and consumers with which the topology
266	   information is shared via BGP-LS to trusted entities within an
267	   administrative domain.

[nit] s/information that was distributed/information that can be distributed

269	   Advertising the S-BFD Discriminators via BGP-LS makes it possible for
270	   attackers to initiate S-BFD sessions using the advertised
271	   information.  The vulnerabilities this poses and how to mitigate them
272	   are discussed in [RFC7752].

[minor] s/rfc7752/rfc7880

[major] Please add text (sample below) talking about why it is ok to
transport the new information in BGP.

   The TLV introduced in this document is used to propagate IGP defined
   information ([RFC7783] and [RFC7784]). The TLV represents information
   used to monitor network resources. The IGP instances originating this
   information are assumed to support any required security and authentication
   mechanisms (as described in [RFC7783] and [RFC7784]) in order to prevent
   any security issue when propagating the information into BGP-LS.

[EoR -06]

---

From: Ketan Talaulikar <[email-masked]>
Date: Tue, 12 Apr 2022 09:33:57 +0530

Hi Alvaro,

Thanks for your detailed review and please check inline below for responses
to your comments.

We've also posted an update with the changes as discussed below:
https://datatracker.ietf.org/doc/html/draft-ietf-idr-bgp-ls-sbfd-extensions-07

> This would be a good place to introduce the references to rfc7883/rfc7884.

KT> Ack

> [nit] s/a Initiator/an Initiator

KT> Ack

> I strongly suggest that you remove this section.

KT> Ack - removed.

> [nit] s/considered as its node level attribute/considered a node level

KT> Ack

>      octets and increments of 4 octets for each additional discriminator.

KT> Agree with the change. I don't follow the need for the RFC7752
reference though. Can you please clarify?

> [major] What should the receiver do if the length is wrong?

KT> The base BGP-LS spec, RFC7752 was not clear on such checks - it only
talked about fixed-length TLVs. The draft-ietf-idr-rfc7752bis clarifies
this aspect and in such case, the TLV would be confirmed malformed and the
handling is explained in
https://datatracker.ietf.org/doc/html/draft-ietf-idr-rfc7752bis-10#section-7.2.2.
This document is referring to fault handling to RFC7752 and I believe the
RFC7752 bis should address this question for all BGP-LS specs.

> [minor] s/Discriminators : multiples of 4 octets, each/Discriminator n

KT> Ack

> [nit] s/as below../as follows:

KT> Ack

> [major] s/S-BFD Discriminators TLV/S-BFD Discriminator TLV

KT> Ack

> [1].  Has anything changed?  If not, then please remove the paragraph

KT> Ack - paragraph removed.

>    registry does not require any value and should be left empty.

KT> Ack

KT> Ack

> needed.  I recommend that you eliminate the sentence and the

KT> Ack

> [nit] s/information that was distributed/information that can be

KT> Ack

> [minor] s/rfc7752/rfc7880

KT> Ack

>    any security issue when propagating the information into BGP-LS.

KT> Ack. Instead of "monitor network resources", "set up of S-BFD" is being
used.

Thanks,
Ketan

---

From: Alvaro Retana <[email-masked]>
Date: Tue, 12 Apr 2022 13:11:20 -0700

On April 12, 2022 at 12:04:10 AM, Ketan Talaulikar wrote:

Ketan:

Hi!

...
> KT> Agree with the change. I don't follow the need for the RFC7752 reference
> though. Can you please clarify?

Just to point at the fact that the TLV format is defined there.  I'm
fine with the text in -07.

> talked about fixed-length TLVs. The draft-ietf-idr-rfc7752bis clarifies this
> aspect and in such case, the TLV would be confirmed malformed and the handling
> is explained in

I agree.  No need to do anything else.

Thanks for the quick turnaround!

I'm starting the IETF LC.

Alvaro.