Subject: Re: [Idr] AD Review of draft-ietf-idr-bgp-ls-segment-routing-ext-11

From: Alvaro Retana <[email-masked]>
Date: Wed, 8 May 2019 12:57:43 -0700

On April 18, 2019 at 7:22:50 AM, Ketan Talaulikar (ketant) ([email-masked])
wrote:

Ketan:

Hi!

...

Until we agree on the formatting I’m not considering the related comments
(yours or mine) below, with a couple of exceptions.

Now that we seem to have agreed to disagree (which is ok - I understand
your reasons), I looked again at the specifics of the TLVs…. I put some
comments at the bottom of this message.  My focus was just on the TLVs and
the intent of being consistent.

...

1009 6.  Security Considerations

...

[major] "SR operates within a trusted domain...[RFC8402]...and its security
considerations also apply to BGP-LS sessions when carrying SR information."
 The Security Considerations in rfc8404 really only talk about the data
plane -- I don't see how they apply to the BGP-LS sessions.

*[KT] The concept of trusted domain in SR is being extended to BGP-LS
sessions as well.*

Sorry, but I don’t know what this means.

[major] "...precaution is necessary to ensure that the SR information
collected via BGP-LS is limited to specific controllers or applications..."
 This sounds as if you're referring to information that (once collected)
can be shared between controllers -- I think that case is out of scope.  If
you are trying to talk about the BGP sessions, then I think the language
needs a little more work.  BTW, the paragraph below also talks about BGP
sessions; suggestion: keep the common topics together.

*[KT] This is about not propagating BGP-LS SR information to controllers
and applications that are not part of this SR trusted domain. It is not
applicable to general BGP sessions since they might be setup to other
networks – e.g. eBGP for Internet. The point was not to setup BGP-LS
sessions to other peers which are outside the SR trusted domain.*

[major] The end of the last sentence says "...in a secure manner within
this SR domain".  Assuming that we're talking about BGP sessions, what does
that mean?  Does it mean anything beyond what BGP is normally specified to
do?  If not, then I would recommend taking that piece of text out to not
invite more questions than needed.

*[KT] As mentioned above, the BGP sessions may be setup outside the SR
domain as well. So we are saying that the BGP-LS sessions which exchange
the SR information do not go outside the SR domain.*

Thanks for the explanation.  Please reflect some of this on the document
itself — so that it is clear there.

BTW, it looks like idnits doesn’t like the formatting on Tables 5-7…take a
look at the errors at the top of
https://tools.ietf.org/idnits?url=https://tools.ietf.org/id/draft-ietf-idr-bgp-ls-segment-routing-ext-13.txt
I’m not sure what to do to fix that...

Thanks!

Alvaro.

[The line numbers come from idnits for -13.]

...

221 2.1.1.  SID/Label Sub-TLV

223   The SID/Label TLV is used as a sub-TLV by the SR Capabilities

224   (Section 2.1.2) and Segment Routing Local Block (SRLB)

225   (Section 2.1.4) TLVs and has the following format:

[minor] If we want to be consistent, this description is missing the "This
information is derived from..." text; there are sub-TLVs defined in both
IS-IS and OSPF.

...

249 2.1.2.  SR Capabilities TLV

...

257   o  IS-IS, as defined by the SR Capabilities sub-TLV in

258      [I-D.ietf-isis-segment-routing-extensions].

260   o  OSPFv2/OSPFv3, as defined by the SID/Label Range TLV in

261      [I-D.ietf-ospf-segment-routing-extensions] and

262      [I-D.ietf-ospf-ospfv3-segment-routing-extensions].

[mayor] The SID/Label Range TLV is not defined in the OSPFv3 draft.

...

305 2.1.3.  SR Algorithm TLV

307   The SR Algorithm TLV is used in order to advertise the SR Algorithms

308   supported by the node.  This information is derived from the protocol

309   specific advertisements.

311   o  IS-IS, as defined by the SR Algorithm sub-TLV in

312      [I-D.ietf-isis-segment-routing-extensions].

[nit] The name is "SR-Algorithm Sub-TLV".

314   o  OSPFv2/OSPFv3, as defined by the SR Algorithm TLV in

315      [I-D.ietf-ospf-segment-routing-extensions] and

316      [I-D.ietf-ospf-ospfv3-segment-routing-extensions].

[nit] The name is "SR-Algorithm TLV".

[major] It is only defined in the OSPFv2 draft.

...

340 2.1.4.  SR Local Block TLV

...

357   o  OSPFv2/OSPFv3, as defined by the SR Local Block TLV in

358      [I-D.ietf-ospf-segment-routing-extensions] and

359      [I-D.ietf-ospf-ospfv3-segment-routing-extensions].

[major] This one is also not defined in the OSPFv3 document.

...

385      Flags: 1 octet of flags.  None are defined at this stage.

[major] I need to point out again that I really don't like the way this
field is treated.  Note that by decoupling the Flags from whatever is
defined on the IGP drafts introduces complexity and may result in
divergence.  Also, it is not consistent with other fields defined in this
document (e.g. Flags in §2.1.2, 2.2.1, 2.2.2, 2.3.1, 2.3.2, 2.3.4..), and
it doesn't comply with the description above ("This information is derived
from...").

...

400 2.1.5.  SRMS Preference TLV

...

414   o  OSPFv2/OSPFv3, as defined by the SRMS Preference TLV in

415      [I-D.ietf-ospf-segment-routing-extensions] and

416      [I-D.ietf-ospf-ospfv3-segment-routing-extensions].

[major] Again, only defined in the OSPFv2 draft...

...

438   The use of the SRMS Preference TLV is defined in

439   [I-D.ietf-isis-segment-routing-extensions],

440   [I-D.ietf-ospf-segment-routing-extensions] and

441   [I-D.ietf-ospf-ospfv3-segment-routing-extensions].

[minor] This last paragraph is redundant: the text above already says where
the information comes from.  It is obvious that the use of the TLV is
defined there too.

...

461 2.2.1.  Adjacency SID TLV

463   The Adjacency SID TLV is used in order to advertise information

464   related to an Adjacency SID.  This information is originated as in

465   Adj-SID sub-TLV of IS-IS [I-D.ietf-isis-segment-routing-extensions],

466   OSPFv2 [I-D.ietf-ospf-segment-routing-extensions] and OSPFv3

467   [I-D.ietf-ospf-ospfv3-segment-routing-extensions

[minor] It may be a simple change in the text, but I want to make sure...
The text above "This information is originated as in..." represents a
change (going forward) from the text used before: "This information is
derived from..."   The latter seems a better fit for this document.  Is
there a deeper reason for the change?

...

490      Flags. 1 octet value which sould be parsed as:

[nit] s/sould/should

[minor] s/be parsed/be set

[] Same comment for 2.2.2 and 2.3.1.

492      *  IS-IS Adj-SID flags are defined in

493         [I-D.ietf-isis-segment-routing-extensions] section 2.2.1.

495      *  OSPFv2 Adj-SID flags are defined in

496         [I-D.ietf-ospf-segment-routing-extensions] section 6.1.

498      *  OSPFv3 Adj-SID flags are defined in

499         [I-D.ietf-ospf-segment-routing-extensions] section 7.1.

[minor] This information about the flags, and the information below about
the SID/Index/Label, is redundant: the text above already says where the
information comes from.

[] Same comment for 2.2.2 and 2.3.1.

501      Weight: Weight used for load-balancing purposes.

503      Reserved: 2 octets that SHOULD be set to 0 and MUST be ignored on

504      receipt.

506      SID/Index/Label:

508      *  IS-IS: Label or index value as defined in

509         [I-D.ietf-isis-segment-routing-extensions],

511      *  OSPFv2: Label or index value as defined in

512         [I-D.ietf-ospf-segment-routing-extensions],

514      *  OSPFv3: Label or index value as defined in

515         [I-D.ietf-ospf-ospfv3-segment-routing-extensions],

517   The Flags and, as an extension, the SID/Index/Label fields of this

518   TLV need to be interpreted accordingly to the respective underlying

519   IS-IS, OSPFv2 or OSPFv3 protocol.  The Protocol-ID of the BGP-LS Link

520   NLRI should be used to determine the underlying protocol

521   specification for parsing these fields.

[minor] This last paragraph is true for all the (sub-)TLVs, and not just
for the ones where the text exists.  That is true, right?

[major] I know that I made a comment before about the text originally in
§2, and that that comment resulted in this new text.  My main concern about
the original text was its Normative nature.  I think that the current text
is ok, but the placement is inconsistent and putting it in a neutral place
may be better.  Yes, I'm asking you to take it back to §2: a generic,
non-normative statement.

523 2.2.2.  LAN Adjacency SID TLV

...

534   This information is originated as in LAN Adj-SID sub-TLV of IS-IS

535   [I-D.ietf-isis-segment-routing-extensions], OSPFv2

536   [I-D.ietf-ospf-segment-routing-extensions] and OSPFv3

537   [I-D.ietf-ospf-ospfv3-segment-routing-extensions].

[nit] For IS-IS, the name is "LAN Adj-SID".

...

605 2.2.3.  L2 Bundle Member Attribute TLV

...

617   This information is originated as in L2 Bundle Member Attributes TLV

618   of IS-IS [I-D.ietf-isis-l2bundles].  The equivalent functionality has

619   not been specified as yet for OSPF.

[nit] s/as in L2/as in the L2

...

696 2.3.1.  Prefix SID TLV

698   The Prefix SID TLV is used in order to advertise information related

699   to a Prefix SID.  This information is originated as in Prefix-SID

700   sub-TLV of IS-IS [I-D.ietf-isis-segment-routing-extensions], OSPFv2

701   [I-D.ietf-ospf-segment-routing-extensions] and OSPFv3

702   [I-D.ietf-ospf-ospfv3-segment-routing-extensions].

[nit] For OSPF, the name is "Prefix SID".

...

800 2.3.3.  Source Router Identifier (Source Router-ID) TLV

802   The Source Router-ID TLV contains the IPv4 or IPv6 Router-ID of the

803   originator of the Prefix.  For IS-IS protocol this is as defined in

804   [RFC7794] IPv4 or IPv6 Router-ID of the originating router.  For OSPF

805   protocol, this is as defined in [I-D.ietf-lsr-ospf-prefix-originator]

806   and is a 32 bit OSPF Router-ID of the originating router..

[nit] s/For xxx protocol/For the xxx protocol

[minor] To be consistent with the other sections...  rfc7794 talks about
the IPv4/IPv6 Source Router ID sub-TLV...and
I-D.ietf-lsr-ospf-prefix-originator about the Prefix Source Router-ID
sub-TLV.

...

829 2.3.4.  Range TLV

831   The range TLV is used in order to advertise a range of prefix-to-SID

832   mappings as part of the Segment Routing Mapping Server (SRMS)

833   functionality [I-D.ietf-spring-segment-routing-ldp-interop], as

834   defined in the respective underlying IGP SR extensions

835   [I-D.ietf-ospf-segment-routing-extensions],

837   [I-D.ietf-ospf-ospfv3-segment-routing-extensions] and

838   [I-D.ietf-isis-segment-routing-extensions].

[nit] There's an extra line...

[nit] s/range TLV/Range TLV

[major] Unlike other sections, this one doesn't explicitly mention which
(sub-)TLV the information is derived from.  This is specially important
because the Flags field below doesn't have a specific reference.

These explicit mentions show up in the "Advertisement Procedure"
sub-sections, which basically just say where the information is derived
from.  I don't understand why those sub-sections are needed for this case
and not all the other TLVs in this document.  ???    Suggestion: delete the
sub-sections.

...

884   Range TLV

885      Prefix-SID TLV (used as a sub-TLV in this context)

887   Where:

889   o  The Range TLV is defined in Section 2.3.4.

891   o  The Prefix-SID TLV (used as sub-TLV in this context) is defined in

892      Section 2.3.1.

[minor] These last few sentences are a little confusing, specially since
there's a reference to §2.3.4, which is this section!  Perhaps take the
text starting with "Where" out and simply put a reference to §2.3.1 where
the Prefix-SID TLV is mentioned.

...

922 2.4.  Equivalent IS-IS Segment Routing TLVs/Sub-TLVs

[major] The Tables below mention the reference draft names (good!), but
also specific sections.  Mentioning the sections is not great becasue they
may change -- for example §3.2 doesn't exist in the OSPFv3 draft.  Perhaps
a better idea would be to mention the name of the (sub-)TLVs along with the
draft reference (and no sections).

---

From: "Ketan Talaulikar (ketant)" <[email-masked]>
Date: Thu, 9 May 2019 07:06:18 +0000

Hi Alvaro,

Thanks again for your review.

I’ve posted the v14 of the draft with the changes to address your comments below.

  https://tools.ietf.org/html/draft-ietf-idr-bgp-ls-segment-routing-ext-14

Please check inline below for responses to your comments.

From: Alvaro Retana <[email-masked]>
Sent: 09 May 2019 01:28
To: Ketan Talaulikar (ketant) <[email-masked]>
Cc: Susan Hares <[email-masked]>; [email-masked]; idr@ietf. org <[email-masked]>; [email-masked]
Subject: RE: AD Review of draft-ietf-idr-bgp-ls-segment-routing-ext-11

On April 18, 2019 at 7:22:50 AM, Ketan Talaulikar (ketant) ([email-masked]<mailto:[email-masked]>) wrote:

Ketan:

Hi!

...
Until we agree on the formatting I’m not considering the related comments (yours or mine) below, with a couple of exceptions.

Now that we seem to have agreed to disagree (which is ok - I understand your reasons), I looked again at the specifics of the TLVs…. I put some comments at the bottom of this message.  My focus was just on the TLVs and the intent of being consistent.

...

...

[major] "SR operates within a trusted domain...[RFC8402]...and its security considerations also apply to BGP-LS sessions when carrying SR information."  The Security Considerations in rfc8404 really only talk about the data plane -- I don't see how they apply to the BGP-LS sessions.

[KT] The concept of trusted domain in SR is being extended to BGP-LS sessions as well.

[KT] I think I understand your point. I will update as below. Further statements describe the application of SR trusted domain to BGP-LS sessions.

   SR operates within a trusted domain [RFC8402<https://tools.ietf.org/html/rfc8402>]. The SR traffic engineering
   policies using the SIDs advertised via BGP-LS are expected to be used
   entirely within this trusted SR domain (e.g. between multiple AS/
   domains within a single provider network).  Therefore, precaution is
   necessary to ensure that the SR information collected via BGP-LS is
   limited to specific consumers in a secure manner within this SR
   domain.

[major] "...precaution is necessary to ensure that the SR information collected via BGP-LS is limited to specific controllers or applications..."  This sounds as if you're referring to information that (once collected) can be shared between controllers -- I think that case is out of scope.  If you are trying to talk about the BGP sessions, then I think the language needs a little more work.  BTW, the paragraph below also talks about BGP sessions; suggestion: keep the common topics together.

[KT] This is about not propagating BGP-LS SR information to controllers and applications that are not part of this SR trusted domain. It is not applicable to general BGP sessions since they might be setup to other networks – e.g. eBGP for Internet. The point was not to setup BGP-LS sessions to other peers which are outside the SR trusted domain.

[major] The end of the last sentence says "...in a secure manner within this SR domain".  Assuming that we're talking about BGP sessions, what does that mean?  Does it mean anything beyond what BGP is normally specified to do?  If not, then I would recommend taking that piece of text out to not invite more questions than needed.

[KT] As mentioned above, the BGP sessions may be setup outside the SR domain as well. So we are saying that the BGP-LS sessions which exchange the SR information do not go outside the SR domain.

Thanks for the explanation.  Please reflect some of this on the document itself — so that it is clear there.

[KT] Ack – I’ve updated the text.

BTW, it looks like idnits doesn’t like the formatting on Tables 5-7…take a look at the errors at the top of https://tools.ietf.org/idnits?url=https://tools.ietf.org/id/draft-ietf-idr-bgp-ls-segment-routing-ext-13.txt  I’m not sure what to do to fix that...

[KT] That’s strange, since I don’t see any errors at the link above. The tables don’t look very neat at this time though due to long draft names. I hope it would fix itself once the IGP draft names are replaced by their RFC numbers.

Thanks!

Alvaro.

...

[minor] If we want to be consistent, this description is missing the "This information is derived from..." text; there are sub-TLVs defined in both IS-IS and OSPF.

[KT] ack – updated.

...

...

[KT] You are right. The OSPFv3 spec re-uses the same TLV defined in the OSPFv2 spec (https://tools.ietf.org/html/draft-ietf-ospf-ospfv3-segment-routing-extensions-23#section-4). This change was done during IESG review in v19 of the OSPFv3 spec and we missed updating this document for the same. I will remove the reference to OSPFv3 draft.

...

[KT] Ack - fixed

[KT] ack – fixed.

[KT] ack fixed as mentioned in previous response.

...

...

[KT] Ack – fixed. Same as previous – the OSPFv3 spec points to the OSPFv2 for these TLVs in RI LSA.

...

[major] I need to point out again that I really don't like the way this field is treated.  Note that by decoupling the Flags from whatever is defined on the IGP drafts introduces complexity and may result in divergence.  Also, it is not consistent with other fields defined in this document (e.g. Flags in §2.1.2, 2.2.1, 2.2.2, 2.3.1, 2.3.2, 2.3.4..), and it doesn't comply with the description above ("This information is derived from...").

[KT] Fixed. I refer to the ISIS spec for the ISIS flags and none are specified for OSPFv2/v3. So this aligns with 2.1.2 now.

...

...

[KT] Ack – fixed as described previously.

...

[minor] This last paragraph is redundant: the text above already says where the information comes from.  It is obvious that the use of the TLV is defined there too.

[KT] Agree – removed this para.

...

[minor] It may be a simple change in the text, but I want to make sure...  The text above "This information is originated as in..." represents a change (going forward) from the text used before: "This information is derived from..."   The latter seems a better fit for this document.  Is there a deeper reason for the change?

[KT] No deeper reason really that I know of. I will use “derived from” everywhere to be consistent.

...

[nit] s/sould/should

[KT] Fixed all of the above

[minor] This information about the flags, and the information below about the SID/Index/Label, is redundant: the text above already says where the information comes from.

[KT] The “where information comes from” is common to all TLVs. The specific fields whose values are IGP protocol specific are only in select TLVs and hence this information is clarified for those fields.

504             receipt.

506             SID/Index/Label:

[minor] This last paragraph is true for all the (sub-)TLVs, and not just for the ones where the text exists.  That is true, right?

[KT] It is true only for the specific TLVs and their specific fields that are mentioned.

[major] I know that I made a comment before about the text originally in §2, and that that comment resulted in this new text.  My main concern about the original text was its Normative nature.  I think that the current text is ok, but the placement is inconsistent and putting it in a neutral place may be better.  Yes, I'm asking you to take it back to §2: a generic, non-normative statement.

[KT] As explained above, this text placement is being made specifically for those TLVs/fields where this IGP protocol specific encoding is being used.

...

[KT] Fixed to LAN-Adj-SID for IS-IS

...

...

[KT] Fixed (changed to “derived from”).

...

[KT] Fixed

...

[KT] fixed

[minor] To be consistent with the other sections...  rfc7794 talks about the IPv4/IPv6 Source Router ID sub-TLV...and I-D.ietf-lsr-ospf-prefix-originator about the Prefix Source Router-ID sub-TLV.

[KT] Fixed.

...

829        2.3.4.  Range TLV

[KT] I don’t see it in the XML and there is a page break in the txt file

[KT] Fixed

[major] Unlike other sections, this one doesn't explicitly mention which (sub-)TLV the information is derived from.  This is specially important because the Flags field below doesn't have a specific reference.

These explicit mentions show up in the "Advertisement Procedure" sub-sections, which basically just say where the information is derived from.  I don't understand why those sub-sections are needed for this case and not all the other TLVs in this document.  ???    Suggestion: delete the sub-sections.

[KT] I have re-organized to remove these sub-sections. The text is now incorporated in the main TLV description itself so that it is consistent with other TLVs in this document.

...

884          Range TLV

887          Where:

892             Section 2.3.1.

[minor] These last few sentences are a little confusing, specially since there's a reference to §2.3.4, which is this section!  Perhaps take the text starting with "Where" out and simply put a reference to §2.3.1 where the Prefix-SID TLV is mentioned.

[KT] Updated based on previous comment.

...

[major] The Tables below mention the reference draft names (good!), but also specific sections.  Mentioning the sections is not great becasue they may change -- for example §3.2 doesn't exist in the OSPFv3 draft.  Perhaps a better idea would be to mention the name of the (sub-)TLVs along with the draft reference (and no sections).

[KT] I’ve updated the tables to list the BGP-LS TLV names, their IGP TLV name+codepoint and the draft/RFC reference.

I hope we didn’t get into mail truncation issue this time 😊

Thanks,

Ketan

---

From: Alvaro Retana <[email-masked]>
Date: Thu, 9 May 2019 06:21:42 -0700

On May 9, 2019 at 3:06:22 AM, Ketan Talaulikar (ketant) ([email-masked])
wrote:

No, we didn’t. :-)

Thanks for the quick turnaround!

I’m starting the IETF LC.

Alvaro.