Subject: Re: [RTG-DIR] Rtgdir early review of draft-ietf-idr-bgpls-srv6-ext-07

From: "Ketan Talaulikar (ketant)" <[email-masked]>
Date: Tue, 1 Jun 2021 14:42:15 +0000

Hi Adrian,

Thanks for your detailed review and comments. This is a quick note to let you know that we are working on addressing your comments.

Thanks,
Ketan

-----Original Message-----
From: Adrian Farrel via Datatracker <[email-masked]> 
Sent: 30 May 2021 02:50
To: [email-masked]
Cc: [email-masked]; [email-masked]
Subject: Rtgdir early review of draft-ietf-idr-bgpls-srv6-ext-07

Reviewer: Adrian Farrel
Review result: Has Issues

I have been selected to do a routing directorate "early" review of this draft. https://datatracker.ietf.org/doc/draft-ietf-idr-bgpls-srv6-ext

The routing directorate will, on request from the working group chair, perform an "early" review of a draft before it is submitted for publication to the IESG. The early review can be performed at any time during the draft's lifetime as a working group document. In this case, it appears that the review request was triggered by the request to the AD for publication. The purpose of this early review is to ensure as wide of a review as possible.

For more information about the Routing Directorate, please see http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Document: draft-ietf-idr-bgpls-srv6-ext-07.txt
Reviewer: Adrian Farrel
Review Date: 29-May-2021
Intended Status: Standards Track

== Summary ==

Some minor issues and lot of nits.

== Comments ==

I am one of the Designated Experts for the BGP-LS registries and I had previously considered the draft when an Early Allocation request was made. That request was approved.

The document is clear enough and I believe I could implement it if I had to. The approach described in this document is consistent with the body of BGP-LS work, but I think it is important to note that the mechanism set out here differs slightly from the mechanism defined to do similar function with SR-MPLS. This distinction is clearly made in Appendix A. However, the WG chairs and AD will want to be sure that the WG recognise this difference and are OK with having two slightly different mechanisms running side by side.

There are some Minor Issues that I think should be addressed before IETF Last Call in case they create any points that community review might object to.

There are also quite a lot of nits in the document. I have tried to capture them below, but the large number means I may have missed some.  
I recommend fixing these before IETF Last Call so that other reviewers has a cleaner document to look at.

== Minor Issues ==

You will need to reduce the number of front-page authors to five or fewer, or you will need to provide the document shepherd with a credible reason to diverge from this rule.

---

Section 1

   On the similar lines,
   introducing the SRv6 related information in BGP-LS allows consumer
   applications that require topological visibility to also receive the
   SRv6 SIDs from nodes across a domain or even across Autonomous
   Systems (AS), as required.

I caution you to be *very* careful with the word "domain". The IESG has recently been concerned by the definition contained in 8402. In that document, the "SR domain" (also just "domain") is the collection of all interconnected SR-capable nodes. Here (and in other parts of the
document) I think you are using "domain" in a more restricted sense. I don't know how you fix that, but I believe you should do something.
Perhaps you can call out the terminology issues here by noting the 8402 definition and specifically defining what you mean by your local definition of "domain".

---

Section 2

   The SRv6 SIDs associated with the node are advertised using the BGP-
   LS SRv6 SID NLRI introduced in this document.  This enables the BGP-
   LS encoding to scale to cover a potentially large set of SRv6 SIDs
   instantiated on a node with the granularity of individual SIDs and
   without affecting the size and scalability of the BGP-LS updates.

The claims of scalability are not expanded here or anywhere else in the document. Scalability of BGP-LS is important, so I'd prefer some explanation. But if that isn't available, it might be better to leave out these mentions.

---

4.1, 4.2, and 7.2

You should consider asking IANA to create a registry for the flags fields. This would make it easier to keep track of future assignments.

You should also state clearly that the three flags fields are aligned so that an addition to one is always also made to the other (if this is the case).

---

4.1 and 4.2

      Sub-TLVs : Used to advertise sub-TLVs that provide additional
      attributes for the specific SRv6 SID.

You need to say where these sub-TLVs are defined. If there are none defined yet, you should say so and explain why some might be defined in the future. 

You should also say where the sub-TLV type values are tracked.

Compare with 5.1

---

5.1

You should say where the sub-TLVs are tracked, and you should consider a registry for the flags.

---

7.1

   The SRv6 Endpoint Behavior TLV is a mandatory TLV that MUST be
   included in the BGP-LS Attribute associated with the BGP-LS SRv6 SID
   NLRI.  

What if it is missing?

---

8.

   The total of the locator block, locator node, function and argument
   lengths MUST be less than or equal to 128.

What if they add up to more than 128?

---

Well done for including Appendix A. I assume that the WG discussed the relative merits of having a slightly different approach for SR-MPLS and SRv6, and contrasted that with the benefits of a consistent approach. I personally think that making a difference is unfortunate, but if the WG was happy with this decision then I guess it's OK.

Without launching into a sales pitch, it might be nice if Appendix A was to very briefly explain why the difference.

== Nits ==

You have:
- "SR for [the] MPLS data-plane" (Abstract)
- SR/MPLS (Section 4.1)
- "SR with [the] MPLS data-plane" (Section 7.2)
- "SR-MPLS" (Sections 10 and 11, Appendix A)

8402 has "SR-MPLS"

---

Abstract

s/Segment Routing (SR) over IPv6 (SRv6)/Segment Routing over IPv6 (SRv6)/

s/by the various/by various/

OLD
   BGP Link-state (BGP-LS) address-family solution for SRv6 is similar
   to BGP-LS for SR for MPLS data-plane.  This draft defines extensions
   to the BGP-LS to advertise SRv6 Segments along with their behaviors
   and other attributes via BGP.
NEW
   This document defines extensions to BGP Link-state (BGP-LS) to
   advertise SRv6 segments along with their behaviors and other
   attributes via BGP.  The BGP-LS address-family solution for SRv6
   described in this document is similar to BGP-LS for SR for the MPLS
   data-plane defined in a separate document.
END

---

Section 1

s/A SRv6 Segment/An SRv6 segment/

OLD
   The IS-IS [I-D.ietf-lsr-isis-srv6-extensions] and OSPFv3
   [I-D.ietf-lsr-ospfv3-srv6-extensions] link-state routing protocols
   have been extended to advertise some of these SRv6 SIDs and
   SRv6-related information.
NEW
   The IS-IS and OSPFv3 link-state routing protocols have been extended
   to advertise some of these SRv6 SIDs and SRv6-related information
   [I-D.ietf-lsr-isis-srv6-extensions],
   [I-D.ietf-lsr-ospfv3-srv6-extensions].
END

s/Certain other/Other/

s/On the similar lines/On similar lines/

s/(e.g./(e.g.,/

---

Section 2

   o  SRv6 SID of the IGP Adjacency SID or the BGP EPE Peer Adjacency
      SID [RFC8402] is advertised via SRv6 End.X SID TLV introduced in
      this document (Section 4.1)

   o  SRv6 SID of the IGP Adjacency SID to a non-Designated Router (DR)
      or non-Designated Intermediate-System (DIS) [RFC8402] is
      advertised via SRv6 LAN End.X SID TLV introduced in this document
      (Section 4.2)

1. s/SRv6 SID/The SRv6 SID/ twice
2. It is confusing that the first bullet talks about advertising the
   IGP Adjacency with no qualification, but the second bullet talks
   about the IGP Adjacnecy to a non-DR or non-DIS. That appears to make
   the first bullet a superset of the second bullet. Sections 4.1 and
   4.2 make this a lot clearer: perhaps you could bring some of that 
   clarity to these bullets.

---

Section 2

   o  SRv6 Locator is advertised via SRv6 Locator TLV introduced in this
      document (Section 5.1)

s/SRv6/The SRv6/
s/SRv6/the SRv6/

---

2.

   The SRv6 SIDs associated with the node are advertised using the BGP-
   LS SRv6 SID NLRI introduced in this document.

Please include a forward pointer to Section 6.

---

2.

   o  The endpoint behavior of the SRv6 SID is advertised via SRv6
      Endpoint Behavior TLV (Section 7.1)

s/via SRv6/via the SRv6/

---

2.

   o  The BGP EPE Peer Node and Peer Set context for a Peer Node and
      Peer Set SID [RFC8402] respectively is advertised via SRv6 BGP EPE
      Peer Node SID TLV (Section 7.2)

s/via SRv6/via the SRv6/

The text is hard to read. Maybe...

   o  The BGP EPE Peer Node context for a Peer Node, and the Peer Set 
      context for a Peer Set SID [RFC8402] are advertised via the SRv6 
      BGP EPE Peer Node SID TLV (Section 7.2).

---

2.

s/(e.g./(e.g.,/

---

2.

   When the BGP-LS router is advertising topology information that it
   sources from the underlying link-state routing protocol (as described
   in [RFC7752]), then it maps the corresponding SRv6 information from
   the SRv6 extensions for IS-IS [I-D.ietf-lsr-isis-srv6-extensions] and
   OSPFv3 [I-D.ietf-lsr-ospfv3-srv6-extensions] protocols to their BGP-
   LS TLVs/sub-TLVs for all SRv6 capable nodes in that routing protocol
   domain.  When the BGP-LS router is advertising topology information
   from the BGP routing protocol (e.g. for EPE as described in
   [I-D.ietf-idr-bgpls-segment-routing-epe]), then it advertises the
   SRv6 information from the local node alone.

I'd suggest moving this paragraph to be the second in the section as it explains what is going on.

---

3.1 and 

   This TLV maps to
   the SRv6 Capabilities sub-TLV and the SRv6 Capabilities TLV of the
   IS-IS and OSPFv3 protocol SRv6 extensions respectively.
   :
   :
   o  Flags: 2 octet field.  The following flags are defined:

     0                   1
     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    | |O|       Reserved            |
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                Figure 2: SRv6 Capability TLV Flags Format

      *  O-flag: If set, then router is capable of supporting SRH O-bit
         Flags, as specified in [I-D.ietf-6man-spring-srv6-oam].

      *  The rest of the bits are reserved for future use and MUST be
         set to 0 and ignored on receipt.

Would be good to include references for the two capabilities TLVs.

It would also help to not include the bit definitions here. I think I am correct that if a new bit is defined for inclusion in the ISIS SRv6 Capabilities sub-TLV flags field, you expect to pick it up here automatically.

It looks like the mapping is in the wrong direction. I.e., this TLV maps from, not to, the IGP TLVs.

I think you would say... 

   This TLV maps from
   the SRv6 Capabilities sub-TLV of the IS-IS SRv6 extensions [ref] and
   the SRv6 Capabilities TLV of the OSPFv3 SRv6 extensions [ref].
   :
   :
   o  Flags: 2 octet field.  The contents of this field are copied from
      the IS-IS or OSPF SRv6 Capabilities TLVs and have the same
      meaning.

For what it's worth, it's a pitty that there is no registry for tracking the flags, but that's a problem with draft-ietf-lsr-isis-srv6-extensions
and not with this document.

---

4.1

s/End.X with USP and/End.X with USP, and/   (twice)
s/This TLV is also used by BGP/This TLV is also used by BGP-LS/

---

4.1 Figures 3 and 5

Convetion is that if you show all of the bytes, you close the start and end of the lines with a vertical pipe "|". Or you can leave out bytes and close the line ends with a tilda "~" or double slash "//" (see Figure 9).

---

4.1, 4.2, 5.1, and 6.1

Can you say what the Length field is. "The length of the value part of the TLV in bytes including any sub-TLVs that may be present." ??

---

4.1, 4.2, 5.1, and 7.1

      Algorithm: 1 octet field.  Algorithm associated with the SID.
      Algorithm values are defined in the IANA IGP Algorithm Type
      registry.

It would be good to add a reference to RFC 8665  or to https://www.iana.org/assignments/igp-parameters/igp-parameters.xhtml.

---

4.2

s/normally an IGP node only/an IGP node normally only/ s/to announce SRv6 SID/to announce the SRv6 SID/ s/(i.e./(i.e.,/
s/End.X with USP and/End.X with USP, and/   (twice)

---

4.2

   The IS-IS and OSPFv3 SRv6 LAN End.X SID TLVs have the following
   format:

Do you mean the BGP-LS TLVs?

---

5.1

OLD
   As specified in [RFC8986], an SRv6 SID comprises of Locator, Function
   and Argument parts.
NEW
   As specified in [RFC8986], an SRv6 SID comprises Locator, Function,
   and Argument parts.
END

---

6. Figure 9

There is a broken format line in the middle of the figure.

---

6.

The figure has
    |                        Identifier                             |
    |                        (64 bits)                              |
and the text has
   o  Identifier: 8 octet value as defined in [RFC7752].

Be consistent?

---

6.

   o  Protocol-ID: 1 octet field that specifies the protocol component
      through which BGP-LS learns the SRv6 SIDs of the node.  The
      Protocol-ID registry was created by [RFC7752] and then extended by
      other BGP-LS extensions.

- s/Protocol-ID registry/BGP-LS Protocol-ID registry/
- Probably, "...and then additional assignments were made for other
  BGP-LS extensions."

---

7.1

s/are bound with a SID/are bound to a SID,/

---

7.2

s/The similar/Similar/

---

7.2
   is an optional TLV for use in the BGP-LS Attribute for an SRv6 SID Is that "OPIONAL"?

But then...
   This TLV MUST be
   included along with SRv6 SIDs that are associated with the BGP Peer
   Node or Peer Set functionality.
... is confusing.

I suspect that the second sentence should be...
   If this TLV is present, it MUST be accompanied by the SRv6 SIDs that
   are associated with the BGP Peer Node or Peer Set functionality.

---

7.2

s/For a SRv6/For an SRv6/

---

8.

s/SRv6 SID Structure TLV is used/The SRv6 SID Structure TLV is used/

---

8.

   optional TLV

Is that "OPTIONAL"?

---

9.

OLD
   This document requests assigning code-points from the IANA "Border
   Gateway Protocol - Link State (BGP-LS) Parameters" registry as
   described in the sub-sections below.
NEW
   IANA has made early allocations of code points from the "Border
   Gateway Protocol - Link State (BGP-LS) Parameters" registry as
   described in the sub-sections below.  IANA is requested to updated
   the references to indicate this document when it is published as an
   RFC.
END

---

9.2

It would make sense to list the codepoints in numeric order. I.e., lift
518 to the top.

---

10.

s/(e.g./(e.g.,/

---

11.

s/(e.g./(e.g.,/

---

Appendix A

s/session(s)/session/
s/advertised as attribute/advertised as attributes/

---

From: "Ketan Talaulikar (ketant)" <[email-masked]>
Date: Tue, 8 Jun 2021 19:14:18 +0000

Hi Adrian,

Thanks for your detailed review and apologies for the delay in responding.

We have published an update that addresses the comments as mentioned inline below. We can continue discussions for open items, if any.

https://datatracker.ietf.org/doc/html/draft-ietf-idr-bgpls-srv6-ext-08

Please check inline for individual responses.

-----Original Message-----
From: Adrian Farrel via Datatracker <[email-masked]> 
Sent: 30 May 2021 02:50
To: [email-masked]
Cc: [email-masked]; [email-masked]
Subject: Rtgdir early review of draft-ietf-idr-bgpls-srv6-ext-07

Reviewer: Adrian Farrel

== Summary ==

== Comments ==

== Minor Issues ==

---

Section 1

   On the similar lines,

I caution you to be *very* careful with the word "domain". The IESG has recently been concerned by the definition contained in 8402. In that document, the "SR domain" (also just "domain") is the collection of all interconnected SR-capable nodes. Here (and in other parts of the
document) I think you are using "domain" in a more restricted sense. I don't know how you fix that, but I believe you should do something.
Perhaps you can call out the terminology issues here by noting the 8402 definition and specifically defining what you mean by your local definition of "domain".
[KT] The "domain" is "IGP domain" in this context and this is fixed.

---

Section 2

The claims of scalability are not expanded here or anywhere else in the document. Scalability of BGP-LS is important, so I'd prefer some explanation. But if that isn't available, it might be better to leave out these mentions.
[KT] Advertisement of large number of SRv6 SIDs as part of the BGP-LS Link Attribute associated with the existing Node NLRI would make that BGP-LS update rather large. Using the new separate SRv6 SID NLRI alleviates the problem. This is the scalability consideration for the protocol encoding design. I am not sure what further details are required.

---

4.1, 4.2, and 7.2

You should also state clearly that the three flags fields are aligned so that an addition to one is always also made to the other (if this is the case).
[KT] The flags fields follow the IGP encodings but I agree that we do need to define the flags registry in IANA for the BGP-EPE use-case. Please see further below.

---

4.1 and 4.2

Compare with 5.1
[KT] Added forward reference to sec 8 for both 4.1 and 4.2 that defines a sub-TLV.

---

5.1

[KT] ack

---

7.1

What if it is missing?
[KT] The consumer of the BGP-LS information would not be able to use this information. The BGP-LS speakers themselves do not perform semantic validation for the TLVs per RFC7752 and this is clarified further in the draft-ietf-idr-rfc7752bis.

---

8.

[KT] Same as above.

---

Without launching into a sales pitch, it might be nice if Appendix A was to very briefly explain why the difference.
[KT] I believe the current text does already and not sure what more is required.

== Nits ==

You have:

8402 has "SR-MPLS"
[KT] Ack - will use SR-MPLS.

---

Abstract

OLD
[KT] Ack to all

---

Section 1

OLD

s/Certain other/Other/

s/(e.g./(e.g.,/
[KT] ack to all

---

Section 2

[KT] Ack

[KT] This is just the intro and there is a pointer to the sections where things are described in further details.

---

Section 2

s/SRv6/The SRv6/
s/SRv6/the SRv6/
[KT] Ack

---

2.

[KT] ack

---

2.

s/via SRv6/via the SRv6/
[KT] ack

---

2.

s/via SRv6/via the SRv6/

[KT] Ack

---

2.

s/(e.g./(e.g.,/
[KT] ack

---

2.

[KT] ack

---

3.1 and 

   This TLV maps to

     0                   1

Would be good to include references for the two capabilities TLVs.
[KT] Ack - added references.

It would also help to not include the bit definitions here. I think I am correct that if a new bit is defined for inclusion in the ISIS SRv6 Capabilities sub-TLV flags field, you expect to pick it up here automatically.
[KT] Ack - removed those bit definitions.

[KT] Ack

I think you would say... 

   This TLV maps from

For what it's worth, it's a pitty that there is no registry for tracking the flags, but that's a problem with draft-ietf-lsr-isis-srv6-extensions
and not with this document.
[KT] The latest version of the ISIS SRv6 spec does introduce the registry for the flags and the equivalent OSPFv3 will be also updated on similar lines.

---

4.1

[KT] ack

---

4.1 Figures 3 and 5

[KT] Ack

---

4.1, 4.2, 5.1, and 6.1

Can you say what the Length field is. "The length of the value part of the TLV in bytes including any sub-TLVs that may be present." ??
[KT] This is base BGP-LS encoding. I don't think we need to repeat it for each TLV that is defined.

---

4.1, 4.2, 5.1, and 7.1

[KT] Ack

---

4.2

[KT] Ack

---

4.2

[KT] Yes - fixed.

---

5.1

OLD
[KT] Ack

---

6. Figure 9

[KT] Fixed

---

6.

The figure has
    |                        Identifier                             |
    |                        (64 bits)                              |
and the text has

Be consistent?
[KT] Not sure that I follow - 8 octets = 64 bits. We've tried to keep the figure aligned with RFC7752. While this document describes fields in terms of octets - so it is consistent.

---

6.

[KT] Ack

---

7.1

[KT] Ack

---

7.2

s/The similar/Similar/
[KT] Ack

---

7.2

But then...
   This TLV MUST be

I suspect that the second sentence should be...
   If this TLV is present, it MUST be accompanied by the SRv6 SIDs that
   are associated with the BGP Peer Node or Peer Set functionality.
[KT] This TLV is actually mandatory (as indicated by the MUST in the next sentence) for SRv6 SIDs related to BGP EPE functionality. Fixed in the text.

---

7.2

[KT] ack

---

8.

[KT] Ack

---

8.

   optional TLV

Is that "OPTIONAL"?
[KT] I haven’t seen OPTIONAL used for such TLVs in RFC7752. It is always been "optional". I believe the lower-case use is correct in this context as well.

---

9.

OLD
[KT] There is some change here due to the new registry sub-section.

---

9.2

[KT] Ack

---

10.

s/(e.g./(e.g.,/
[KT] Ack

---

11.

s/(e.g./(e.g.,/
[KT] Ack

---

Appendix A

s/session(s)/session/
[KT] Ack

---

From: "Adrian Farrel" <[email-masked]>
Date: Wed, 9 Jun 2021 09:46:05 +0100

Hi Ketan,

Thanks for the detailed response, it really helps cross-check the review.

There are a few residual comments/suggestions below with everything else snipped out.

None of these remaining points merits a response: either touch the document or ignore the email 😊

Best,
Adrian

== Minor Issues ==

[AF] I assume, from no change to the document, that you will be giving the shepherd some reasoning for them to use in the write-up.

---

Section 1

   On the similar lines,

[AF] I still see Section 11 "(e.g., between multiple AS/domains within a single provider network)". I think that use of "domain" needs qualification as well.

---

Section 2

[AF] Well, just a few words to help clarify using some of your words here.
Advertisement of large number of SRv6 SIDs as part of the BGP-LS Link Attribute associated with the existing Node NLRI could make that BGP-LS update rather large, causing scaling concerns as the number of SRv6 SIDs instantiated on a node. The SRv6 SID NLRI introduced in this document alleviates this problem by <doing what?>"

---

[snip]

---

7.1

What if it is missing?

[AF] A fine answer, and the text can be enhanced with a quick pointer to 7752 (or the bis, but you probably want a normative reference and 752 will do the job) saying "Processing and validation rules for BGP-LS TLVs are described in [RFC7752]." Do this in the right place and it covers all issues.

---

[snip]

== Nits ==

[snip]

4.1, 4.2, 5.1, and 6.1

[AF] Can you then add (probably in Section 2) to say this? Something like...
OLD
   BGP-LS Attribute TLVs for the SRv6 SID NLRI are
   introduced in this document as follows:
NEW
   BGP-LS Attribute TLVs for the SRv6 SID NLRI are
   introduced in this document as follows.  These new TLVs follow the
   formats and common field definitions provided in [RFC7752].
END

---

[snip]

---

6.

The figure has
    |                        Identifier                             |
    |                        (64 bits)                              |
and the text has

Be consistent?

[AF] Consistent would be to always say "8 octets" or always say "64 bits"

---

[snip]

---

From: "Adrian Farrel" <[email-masked]>
Date: Wed, 9 Jun 2021 09:56:06 +0100

Hi Ketan,

Best,
Adrian

== Minor Issues ==

---

Section 1

   On the similar lines,

---

Section 2

---

[snip]

---

7.1

What if it is missing?

---

[snip]

== Nits ==

[snip]

4.1, 4.2, 5.1, and 6.1

---

[snip]

---

6.

The figure has
    |                        Identifier                             |
    |                        (64 bits)                              |
and the text has

Be consistent?

---

[snip]

---

From: "Ketan Talaulikar (ketant)" <[email-masked]>
Date: Wed, 9 Jun 2021 14:37:26 +0000

Hi Adrian,

Thanks for your quick response. Please check inline below for clarifications with KT2. 

I will hold the changes indicated below in my edit buffer and push them out with the next update.

-----Original Message-----
From: Adrian Farrel <[email-masked]> 
Sent: 09 June 2021 14:26
To: Ketan Talaulikar (ketant) <[email-masked]>; [email-masked]
Cc: [email-masked]; [email-masked]
Subject: RE: Rtgdir early review of draft-ietf-idr-bgpls-srv6-ext-07

Hi Ketan,

Best,
Adrian

== Minor Issues ==

[KT2] Yes.

---

Section 1

   On the similar lines,

[AF] I still see Section 11 "(e.g., between multiple AS/domains within a single provider network)". I think that use of "domain" needs qualification as well.
[KT2] I missed that. Will correct that as "multiple AS or IGP domains" in the next update.

---

Section 2

[AF] Well, just a few words to help clarify using some of your words here.
Advertisement of large number of SRv6 SIDs as part of the BGP-LS Link Attribute associated with the existing Node NLRI could make that BGP-LS update rather large, causing scaling concerns as the number of SRv6 SIDs instantiated on a node. The SRv6 SID NLRI introduced in this document alleviates this problem by <doing what?>"
[KT2] I will add this text in that paragraph to clarify further. " Had the SRv6 SIDs been advertised within the BGP-LS Link Attribute associated with the existing Node NLRI, the BGP-LS update would have grown rather large with the increase in SRv6 SIDs on the node and would have also required a large update message to be generated for any change to even a single SRv6 SID." 

---

[snip]

---

7.1

What if it is missing?

[AF] A fine answer, and the text can be enhanced with a quick pointer to 7752 (or the bis, but you probably want a normative reference and 752 will do the job) saying "Processing and validation rules for BGP-LS TLVs are described in [RFC7752]." Do this in the right place and it covers all issues.
[KT2] I believe this is already discussed in Sec 10 of the document. 

---

[snip]

== Nits ==

[snip]

4.1, 4.2, 5.1, and 6.1

[KT2] Ack.  I will add that sentence at the end of Sec 2.

---

[snip]

---

6.

The figure has
    |                        Identifier                             |
    |                        (64 bits)                              |
and the text has

Be consistent?

[AF] Consistent would be to always say "8 octets" or always say "64 bits"
[KT2] OK. I will change the 64-bits to 8 octets. 

Thanks,
Ketan 

---

[snip]