Re: [Idr] AD Review of draft-ietf-idr-bgpls-srv6-ext-09

[Ketan Talaulikar <[e-mail]>]

Hi Alvaro,

Thanks for your detailed review and comments/suggestions. Please check
inline below for responses.

We have also posted an update on the lines of the discussion below:
https://datatracker.ietf.org/doc/html/draft-ietf-idr-bgpls-srv6-ext-10


On Mon, Sep 26, 2022 at 6:51 PM Alvaro Retana <[e-mail]>
wrote:

> [Resending to include the Shepherd, Chairs, and WG.]
>
> On September 26, 2022 at 8:00:05 AM, Alvaro Retana ([e-mail])
> wrote:
>
>
>
> Dear authors:
>
> Thank you for this work!  Please see my comments on it below.
>
> My main concern with this draft is that BGP-LS carries information from
> other routing protocols, but there are a significant number of cases where
> the place from where the information is derived is not specified.
>
>
> To the Chairs:  6 authors are listed on the front page.  Please provide a
> justification for having more than 5.
>
>
> Thanks!
>
> Alvaro.
>
>
> [Line numbers from idnits.]
>
> ...
> 98 1.  Introduction
> ...
> 108   An SRv6-capable node N maintains all the SRv6 segments explicitly
> 109   instantiated at node N.
>
> [minor] The mention of "node N" twice makes this sentence sound awkward.
>
> Suggestion>
>
>    An SRv6-capable node maintains all the SRv6 segments explicitly
>    instantiated locally.
>
> KT> Ack


>
>
>
>
> ...
> 147 2.  BGP-LS Extensions for SRv6
> ...
> 153   When the BGP-LS router is advertising topology information that it
> 154   sources from the underlying link-state routing protocol (as
> described
> 155   in [RFC7752]), then it maps the corresponding SRv6 information from
> 156   the SRv6 extensions for IS-IS [I-D.ietf-lsr-isis-srv6-extensions]
> and
> 157   OSPFv3 [I-D.ietf-lsr-ospfv3-srv6-extensions] protocols to their BGP-
> 158   LS TLVs/sub-TLVs for all SRv6 capable nodes in that routing protocol
> 159   domain.  When the BGP-LS router is advertising topology information
> 160   from the BGP routing protocol (e.g., for EPE as described in
> 161   [RFC9086]), then it advertises the SRv6 information from the local
> 162   node alone.
>
> [] "BGP-LS router is advertising topology information that it sources from
> the underlying link-state routing protocol (as described in [RFC7752])"
>
> Some text could be saved by referencing the rfc7752bis terminology and
> using BGP-LS Producer in this case.  I don't want to hold publication of
> this document on the publication of rfc7752bis, but if the base spec update
> is published first, we should come back and revise some of the language and
> the references.
>
> KT> Sure. We can update this and perhaps other BGP-LS documents that end
up getting published after RFC7752bis.


>
>
>
> [nit] s/When the BGP-LS router is advertising/When a BGP-LS router
> advertises/g
>
> KT> Ack


>
>
>
> [nit] "(e.g., for EPE as described in [RFC9086]), then it advertises the
> SRv6 information"
>
> rfc9086 (as explained in this document) is for SR-MPLS, so using it as an
> example for SRv6 seems confusing.
>
> KT> RFC9086 is the reference that we have for originating information from
the BGP protocol for the EPE use-case. Are you suggesting that we remove
that reference from this sentence?

>
>
>
>
> ...
> 173   o  Algorithm support for SRv6 is advertised via the SR Algorithm TLV
> 174      specified in [RFC9085].
>
> [major] s/SR Algorithm TLV/SR-Algorithm TLV
>
> KT> Ack

>
>
>
>
> ...
> 224 3.  SRv6 Node Attributes
>
> 226   SRv6 attributes of a node are advertised using the BGP-LS Attribute
> 227   TLVs defined in this section and associated with the BGP-LS Node
> 228   NLRI.
>
> [nit] s/SRv6 attributes/The SRv6 attributes
>
> KT> Ack

>
>
>
> 230 3.1.  SRv6 Capabilities TLV
>
> 232   This BGP-LS Attribute TLV is used to announce the SRv6 capabilities
> 233   of the node along with the BGP-LS Node NLRI and indicates the SRv6
> 234   support by the node.  A single instance of this TLV MUST be included
> 235   in the BGP-LS attribute for each SRv6 capable node.  This TLV maps
> 236   from the SRv6 Capabilities sub-TLV
> 237   [I-D.ietf-lsr-isis-srv6-extensions] and the SRv6 Capabilities TLV
> 238   [I-D.ietf-lsr-ospfv3-srv6-extensions] of the IS-IS and OSPFv3
> 239   protocol SRv6 extensions respectively.  Any sub-TLVs introduced by
> 240   future documents for IS-IS and OSPFv3 SRv6 Capabilities are REQUIRED
> 241   to be introduced as top-level TLVs in BGP-LS in the future.
>
> [minor] "This TLV maps from the SRv6 Capabilities sub-TLV
> [I-D.ietf-lsr-isis-srv6-extensions] and the SRv6 Capabilities TLV
> [I-D.ietf-lsr-ospfv3-srv6-extensions] of the IS-IS and OSPFv3 protocol SRv6
> extensions respectively."
>
> This sentence is redundant with the specification of the TLV below, where
> the source of the information is repeated.
>
> KT> Agree. Please also see the next response.

>
>
>
> [major] "Any sub-TLVs introduced by future documents for IS-IS and OSPFv3
> SRv6 Capabilities are REQUIRED to be introduced as top-level TLVs in BGP-LS
> in the future."
>
> I don't feel comfortable requiring this action since we don't know what
> the contents of such extensions might be.  Also, there's no background as
> to the reasoning.
>
> Suggestion (in a separate paragraph)>
>
>    The SRv6 Capabilities sub-TLV and the SRv6 Capabilities TLV in IS-IS
>    and OSPFv3, respectively, are specified with the ability to carry
>    optional sub-sub-TLVs/sub-TLVs.  However, no such extensions are
>    currently defined.  Moreover, the SRv6 Capabilities TLV defined below
>    is not extensible.  As a result, it is expected that any extensions
>    will be introduced as top-level TLVs in the BGP-LS Attribute.
>
> KT> Thanks for the suggestion and we've incorporated it with minor tweaks.

>
>
>
>
> ...
> 259   o  Flags: 2 octet field.  The flags are derived from the SRv6
> 260      Capabilities sub-TLV/TLV of IS-IS (section 2 of
> 261      [I-D.ietf-lsr-isis-srv6-extensions]) and OSPFv3 (section 2 of
> 262      [I-D.ietf-lsr-ospfv3-srv6-extensions]).
>
> [nit] Even though I doubt that the Section numbers will change, I find it
> less error-prone to simply mention the document name.
>
> KT> Given the dependencies, this document will either follow or get
published alongside the IGP specs and so we will take care to review and
update the section numbers (if necessary). I have received feedback in the
past that reference to section numbers would be helpful.

>
>
>
>
> ...
> 266 3.2.  SRv6 Node MSD Types
>
> [minor] s/MSD Types/MSD-Types/g
> That is the terminology from rfc8814.
>
> KT> Ack

>
>
>
> 268   The Node MSD TLV [RFC8814] of the BGP-LS Attribute of the Node NLRI
> 269   is also used to advertise the limits and the Segment Routing Header
> 270   (SRH) [RFC8754] operations supported by the SRv6 capable node.  The
> 271   SRv6 MSD Types specified in section 4 of
> 272   [I-D.ietf-lsr-isis-srv6-extensions] are also used with the BGP-LS
> 273   Node MSD TLV as these codepoints are shared between IS-IS, OSPF and
> 274   BGP-LS protocols.  The description and semantics of these new MSD
> 275   types for BGP-LS are identical as specified in
> 276   [I-D.ietf-lsr-isis-srv6-extensions].
>
> 278   Each MSD type is encoded as a one-octet type followed by a one-octet
> 279   value as specified in [RFC8814].
>
> [major] In the context of this document it doesn't matter what MSD is,
> what the MSD-Types are, or where they are defined.  What matters is where
> does the information to be put in the Node MSD TLV (rfc8814) come from.
>
> Instead of the text in this section, please be explicit about the origin
> of the information and the BGP-LS TLV used: "The Node MSD TLV (rfc8814) is
> used...the MSD-Type and MSD-Value fields are derived from ...".
>
> KT> This is already covered in RFC8814 and so we were just referring to it
as opposed to repeating the IGP TLV names. Clarified the text a bit to make
this more explicit.

>
>
>
>
> ...
> 287 4.1.  SRv6 End.X SID TLV
>
> 289   The SRv6 End.X SID TLV is used to advertise the SRv6 SIDs associated
> 290   with an IGP Adjacency SID behavior that correspond to a point-to-
> 291   point or point-to-multipoint link or adjacency of the node running
> 292   IS-IS and OSPFv3 protocols.  This TLV can also be used to advertise
> 293   the SRv6 SID corresponding to the underlying layer-2 member links
> for
> 294   a layer-3 bundle interface as a sub-TLV of the L2 Bundle Member
> 295   Attribute TLV [RFC9085].  The SRv6 SID for the IGP adjacency using
> 296   the End.X behaviors (viz.  End.X, End.X with Penultimate Segment Pop
> 297   of the SRH (PSP), End.X with Ultimate Segment Pop of the SRH (USP),
> 298   and End.X with PSP & USP) [RFC8986] are advertised using the SRv6
> 299   End.X SID TLV.
>
> [nit] s/running IS-IS and OSPFv3 protocols/running the IS-IS or OSPFv3
> protocol
>
> KT> Ack

>
>
>
> [nit] s/(viz.  End.X/(viz. End.X/g
>
> KT> The extra space seems to be introduced by the text rendering of the
xml2rfc ...

>
>
>
> 301   This TLV is also used by BGP-LS to advertise the BGP EPE Peer
> 302   Adjacency SID for SRv6 on the same lines as specified for SR-MPLS in
> 303   [RFC9086].  The SRv6 SID for the BGP Peer Adjacency using End.X
> 304   behaviors (viz.  End.X, End.X with PSP, End.X with USP, and End.X
> 305   with PSP & USP) [RFC8986] indicates the cross-connect to a specific
> 306   layer-3 link to the specific BGP session peer (neighbor).
>
> [?] "... on the same lines as specified for SR-MPLS in [RFC9086]."
>
> Is this statement intended to have normative (or other) value?  For
> example, rfc9086 says that the PeerAdj SID can be included more than once,
> but there's no mention of anything related here.
>
> KT> Yes, this can also be included more than once. Made that explicit.

>
>
>
> 308   The TLV has the following format:
>
> 310     0                   1                   2                   3
> 311     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> 312    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 313    |               Type            |          Length               |
> 314    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 315    |        Endpoint Behavior      |      Flags    |   Algorithm   |
> 316    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 317    |     Weight    |   Reserved    |  SID (16 octets) ...          |
> 318    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 319    |    SID (cont ...)                                             |
> 320    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 321    |    SID (cont ...)                                             |
> 322    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 323    |    SID (cont ...)                                             |
> 324    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 325    |    SID (cont ...)             | Sub-TLVs (variable) . . .
> 326    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
> 328                      Figure 2: SRv6 End.X TLV Format
>
> 330   Where:
>
> 332      Type: 1106
> 333      Length: variable
>
> 335      Endpoint Behavior: 2 octet field.  The Endpoint Behavior code
> 336      point for this SRv6 SID as defined in section 10.2 of [RFC8986].
>
> [major] All the information in this TLV comes from the corresponding TLVs
> in IS-IS/OSPF -- so please indicate where the information comes from.  That
> is more important than what the information is.  Also, despite the name and
> the explanation above, the "End.X behaviors (viz.  End.X, End.X with PSP,
> End.X with USP, and End.X with PSP & USP)" are not the only Endpoint
> Behaviors allowed in the IGP drafts.
>
> KT> Ack. Fixed.

>
>
>
> 338      Flags: 1 octet of flags.  The flags are derived from the SRv6
> 339      End.X SID sub-TLV/TLV of IS-IS (section 8.1 of
> 340      [I-D.ietf-lsr-isis-srv6-extensions]) and OSPFv3 (section 8.1 of
> 341      [I-D.ietf-lsr-ospfv3-srv6-extensions]).  In the case of BGP EPE
> 342      Peer Adjacency SID, the flags are as defined for the SRv6 BGP
> Peer
> 343      Node SID TLV (Section 7.2).
>
> [minor] "section 8.1 of [I-D.ietf-lsr-ospfv3-srv6-extensions]"
>
> That section doesn't exist.  See my comment above about section numbers.
>
> KT> Fixed.

>
>
>
> [major] "In the case of BGP EPE Peer Adjacency SID, the flags are as
> defined for the SRv6 BGP Peer Node SID TLV (Section 7.2)."
>
> I guess that this means that the flags are defined in §7.2 -- not just for
> the SRv6 BGP Peer Node SID TLV, but for this one too, right?
>
> KT> Correct.

>
>
>
> 345      Algorithm: 1 octet field.  Algorithm associated with the SID.
> 346      Algorithm values are defined in the IANA IGP Algorithm Type
> 347      registry (https://www.iana.org/assignments/igp-parameters/igp-
> 348      parameters.xhtml).
>
> [major] As above, the information comes from the IGP drafts...the registry
> doesn't matter.
>
> KT> Ack. Removed the reference to the registry.

>
>
>
> 350      Weight: 1 octet field.  The value represents the weight of the
> SID
> 351      for the purpose of load balancing.  The use of the weight is
> 352      defined in [RFC8402].
>
> [major] As above, the information comes from the IGP drafts...what it is
> or how it may be used is inherited from there...
>
> KT> True, but the weights are also applicable for BGP EPE and hence good
to put a reference here.

>
>
>
> 354      Reserved: 1 octet field that MUST be set to 0 and ignored on
> 355      receipt.
>
> 357      SID: 16 octet field.  This field encodes the advertised SRv6 SID
> 358      as 128 bit value.
>
> [major] ...and comes from the IGP drafts...
>
> KT> This is also used for BGP-EPE.

>
>
>
> 360      Sub-TLVs : They are allocated from the IANA "BGP-LS Node
> 361      Descriptor, Link Descriptor, Prefix Descriptor, and Attribute
> 362      TLVs" registry and are used to advertise sub-TLVs that provide
> 363      additional attributes for the specific SRv6 SID.  This document
> 364      defines one inSection 8.
>
> [major] Again, the information should come from corresponding IGP
> sub-TLVs, not just any TLV in the registry.  The TLV in §8 is "inherited"
> from the IGP drafts.
>
> KT> Updated the text.

>
>
>
> [nit] s/inSection 8/in Section 8
>
> KT> Ack.

>
>
>
> 366 4.2.  SRv6 LAN End.X SID TLV
>
> [] Most of the comments from the last section apply here too.
>
> KT> Ack

>
>
>
> ...
> 433   o  Neighbor ID : 6 octets of IS-IS System ID of the neighbor for the
> 434      IS-IS SRv6 LAN End.X SID TLV and 4 octets of OSPFv3 Router-id of
> 435      the neighbor for the OSPFv3 SRv6 LAN End.X SID TLV.
>
> [major] s/IS-IS System ID of the neighbor for/Neighbor System-ID in
>
> KT> Ack

>
>
>
> [major] s/OSPFv3 Router-id of the neighbor/
>
> It looks like draft-ietf-lsr-ospfv3-srv6-extensions-08 hasn't settled on a
> name yet.  Figure 8 says "OSPFv3 Router-ID of neighbor" while the
> description calls it "Neighbor ID".
>
> KT> I hold the pen on the OSPFv3 SRv6 draft and will change the field name
to Neighbor Router-ID to be consistent. Updated the text here accordingly.

>
>
>
>
> ...
> 446 4.3.  SRv6 Link MSD Types
>
> 448   The Link MSD TLV [RFC8814] of the BGP-LS Attribute of the Link NLRI
> 449   is also used to advertise the limits and the SRH operations
> supported
> 450   on the specific link by the SRv6 capable node.  The SRv6 MSD Types
> 451   specified in section 4 of[I-D.ietf-lsr-isis-srv6-extensions] are
> also
> 452   used with the BGP-LS Link MSD TLV as these codepoints are shared
> 453   between IS-IS, OSPF, and BGP-LS protocols.  The description and
> 454   semantics of these new MSD types for BGP-LS are identical as
> 455   specified in [I-D.ietf-lsr-isis-srv6-extensions].
>
> 457   Each MSD type is encoded as a one-octet type followed by a one-octet
> 458   value as specified in [RFC8814].
>
> [major] See the comment in §3.2.
>
> KT> Ack - fixed on the same lines as in section 3.2

>
>
>
>
> ...
> 466 5.1.  SRv6 Locator TLV
>
> [] Some of the comments from §4.1 about where the values are derived apply
> here as well.
>
> KT> Ack - Fixed.

>
>
>
>
> ...
> 471   A node is provisioned with one or more locators supported by that
> 472   node.  Locators are covering prefixes for the set of SIDs
> provisioned
> 473   on that node.  These Locators are advertised as BGP-LS Prefix NLRI
> 474   objects along with the SRv6 Locator TLV in its BGP-LS Attribute.
>
> [minor] s/locators/Locators/g
>
> Most of this document capitalize the term.
>
> KT> Ack

>
>
>
> 476   The IPv6 Prefix matching the Locator MAY be also advertised as a
> 477   prefix reachability by the underlying routing protocol.  In this
> 478   case, the Prefix NLRI would be also associated with the Prefix
> Metric
> 479   TLV that carries the routing metric for this prefix.  When the
> 480   Locator prefix is not being advertised as a prefix reachability,
> then
> 481   the Prefix NLRI would have the SRv6 Locator TLV associated with it
> 482   but no Prefix Metric TLV.  In the absence of Prefix Metric TLV, the
> 483   consumer of the BGP-LS topology information MUST NOT interpret the
> 484   Locator prefix as a prefix reachability routing advertisement in the
> 485   IGPs default SPF computation.
>
> [major] "IPv6 Prefix...MAY be also advertised...by the underlying routing
> protocol"
>
> This is not a Normative statement (on the IGPs!), just a statement of
> fact. s/MAY/may
>
> KT> Ack

>
>
>
> [major] Add a reference to where the Prefix Metric TLV is specified
> (rfc7752 ?).
>
> KT> Ack

>
>
>
> [nit] s/advertised as a prefix reachability/advertised as prefix
> reachability
>
> KT> Ack

>
>
>
> [major] "consumer...MUST NOT interpret"
>
> No instructions for the consumer.
>
> KT> Fixed the text.

>
>
>
>
> ...
> 522      Metric: 4 octet field.  The value of the metric for the Locator.
>
> [major] Where does this value come from?
>
> KT> Added reference to the underlying IGP specs.

>
>
>
>
> ...
> 530 6.  SRv6 SID NLRI
>
> 532   SRv6 SID information is advertised in BGP UPDATE messages using the
> 533   MP_REACH_NLRI and MP_UNREACH_NLRI attributes [RFC4760].  The "Link-
> 534   State NLRI" defined in [RFC7752] is extended to carry the SRv6 SID
> 535   information.
>
> [] The first sentence is not technically wrong, but it is confusing.  The
> new SRv6 SID NLRI is in fact carried in the MP_* attributes, but it is an
> extension of the specification in rfc7752.  IOW, it is not needed as the
> information is already present in rfc7752.
>
> KT> Ack. Removed.

>
>
>
> 537   A new "Link-State NLRI Type" is defined for SRv6 SID information as
> 538   following:
>
> [nit] s/as following/as follows
>
> KT> Ack

>
>
>
>
> ...
> 561   o  Protocol-ID: 1 octet field that specifies the protocol component
> 562      through which BGP-LS learns the SRv6 SIDs of the node.  The
> BGP-LS
> 563      Protocol-ID registry was created by [RFC7752] and then additional
> 564      assignments were made for other BGP-LS extensions.
>
> [minor] No need for the history:
>
> Suggestion>
>    Protocol-ID: 1 octet field that specifies the information source
> protocol
>    [RFC7752].
>
> KT> Ack

>
>
>
>
> ...
> 568   o  Local Node Descriptors TLV: as defined in [RFC7752] for IGPs,
> 569      local and static configuration and as defined in [RFC9086] for
> BGP
> 570      protocol.
>
> [minor] What is it?
>
> Suggestion>
>    Local Node Descriptors: set of Node Descriptor TLVs for the local
>    node, as defined in [RFC7752] and [RFC9086].
>
> KT> Ack

>
>
>
>
> 572   o  SRv6 SID Descriptors: MUST include a single SRv6 SID Information
> 573      TLV defined in Section 6.1 and optionally MAY include the Multi-
> 574      Topology Identifier TLV as defined in [RFC7752].
>
> [major] Ok, but what is it?  Also, "optionally MAY" is redundant.
>
> Suggestion>
>    SRv6 SID Descriptors: set of SRv6 SID Descriptor TLVs.  This field
>    MUST contain a single SRv6 SID Information TLV (Section 6.1) and
>    MAY contain the Multi-Topology Identifier TLV [RFC7752].
>
> KT> Ack

>
>
>
> [major] Are those the only 2 TLVs expected here?
>
> KT> Yes

>
>
>
> 576   New TLVs for advertisement within the BGP Link State Attribute
> 577   [RFC7752] are defined in Section 7 to carry the attributes of an
> SRv6
> 578   SID.
>
> [minor] s/BGP Link State Attribute/BGP-LS Attribute
>
> KT> Ack

>
>
>
> 580 6.1.  SRv6 SID Information TLV
> ...
> 608      SID: 16 octet field.  This field encodes the advertised SRv6 SID
> 609      as 128 bit value.
>
> [major] Where is this information derived from?
>
> KT> Updated text.

>
>
>
>
> ...
> 616 7.1.  SRv6 Endpoint Behavior TLV
> ...
> 643      Endpoint Behavior: 2 octet field.  The Endpoint Behavior code
> 644      point for this SRv6 SID as defined in section 10.2 of [RFC8986].
>
> [major] Where is this information derived from?
>
> KT> Updated the text.

>
>
>
>
> ...
> 649      Algorithm: 1 octet field.  Algorithm associated with the SID.
> 650      Algorithm values are defined in the IGP Algorithm Type registry
> 651      (https://www.iana.org/assignments/igp-parameters/igp-
> 652      parameters.xhtml).
>
> [major] Where is this information derived from?
>
> KT> Updated the text.

>
>
>
> 654 7.2.  SRv6 BGP Peer Node SID TLV
>
> 656   The BGP Peer Node SID and Peer Set SID for SR-MPLS are specified in
> 657   [RFC9086].  Similar Peer Node and Peer Set functionality can be
> 658   realized with SRv6 using SIDs with END.X behavior.  The SRv6 BGP
> Peer
> 659   Node SID TLV is a mandatory TLV for use in the BGP-LS Attribute for
> 660   an SRv6 SID NLRI advertised by BGP for the EPE functionality.  This
> 661   TLV MUST be included along with SRv6 SIDs that are associated with
> 662   the BGP Peer Node or Peer Set functionality.
>
> [major] s/BGP Peer Node SID and Peer Set SID/BGP PeerNode and PeerSet SIDs
>
> KT> Ack

>
>
>
> ...
> 686   o  Flags: 1 octet of flags with the following definition (also refer
> 687      to Section 9.3 for IANA registry):
>
> [minor] No need to refer to the registry here.
>
> KT> Ack - reference removed.

>
>
>
> 689     0 1 2 3 4 5 6 7
> 690    +-+-+-+-+-+-+-+-+
> 691    |B|S|P| Reserved|
> 692    +-+-+-+-+-+-+-+-+
>
> [minor] Remove "Reserved".
>
> KT> Ack

>
>
>
> 694                  Figure 9: SRv6 BGP EPE SID Flags Format
>
> 696      *  B-Flag: Backup Flag.  If set, the SID is eligible for
> 697         protection (e.g. using IPFRR) as described in [RFC8355].
>
> 699      *  S-Flag: Set Flag.  When set, the S-Flag indicates that the SID
> 700         refers to a set of BGP peering sessions (i.e.  BGP Peer Set
> SID
> 701         functionality) and therefore MAY be assigned to one or more
> 702         End.X SIDs associated with BGP peer sessions.
>
> [nit] s/(i.e.  BGP/(i.e. BGP
>
> KT> space introduced during text rendering from xml2rfc

>
>
>
> [major] "MAY be assigned"
>
> Who?  How?  Is this related to the End.X SID TLVs defined earlier?
>
> KT> This is related to the provisioning of BGP EPE PeerSet SID - the same
SID is shared across the peers in the set. The traffic sent to that SID
would therefore get load-balanced over to those peers. This is per RFC8402.

>
>
>
> 704      *  P-Flag: Persistent Flag: When set, the P-Flag indicates that
> 705         the SID is persistently allocated, i.e., the value remains
> 706         consistent across router restart and/or session flap.
>
> [major] Where are these flags derived from?
>
> KT> From the BGP EPE configuration. Same as in RFC9086.

>
>
>
>
> ...
> 711   o  Weight: 1 octet field.  The value represents the weight of the
> SID
> 712      for the purpose of load balancing.  The use of the weight is
> 713      defined in [RFC8402].
>
> [major] Where is this value derived from?
>
> KT>  From the BGP EPE configuration. Same as in RFC9086.

>
>
>
>
> ...
> 718   o  Peer AS Number : 4 octets of BGP AS number of the peer router.
>
> [major] Where is this value derived from?
>
> KT> From the BGP EPE configuration. Same as in RFC9086.

>
>
>
> 720   o  Peer BGP Identifier : 4 octets of the BGP Identifier (BGP Router-
> 721      ID) of the peer router.
>
> [major] Where is this value derived from?
>
> KT> From the BGP EPE configuration. Same as in RFC9086.

>
>
>
> 723   For an SRv6 BGP EPE Peer Node SID, one instance of this TLV is
> 724   associated with the SRv6 SID.  For SRv6 BGP EPE Peer Set SID,
> 725   multiple instances of this TLV (one for each peer in the "peer set")
> 726   are associated with the SRv6 SID and the S (set/group) flag is SET.
>
> [minor] s/S (set/group) flag is SET/S-Flag is set
>
> KT> Ack.

>
>
>
> [?] My take-away from this paragraph is that if multiple TLVs are present
> then they should all have the S-Flag set.  And if there's only one instance
> then it shouldn't.  ??
>
> KT> S-flag needs to be set when there are multiple instances of this TLV
(i.e. this SID is pointing to multiple peers). However, S-flag may also be
set when there is a single instance (i.e., a set of one).

>
>
>
> 728 8.  SRv6 SID Structure TLV
>
> 730   The SRv6 SID Structure TLV is used to advertise the length of each
> 731   individual part of the SRv6 SID as defined in [RFC8986].  It is an
> 732   optional TLV for use in the BGP-LS Attribute for an SRv6 SID NLRI
> and
> 733   as a sub-TLV of the SRv6 End.X, IS-IS SRv6 LAN End.X and OSPFv3 SRv6
> 734   LAN End.X TLVs.  The TLV has the following format:
>
> [minor] s/SRv6 End.X, IS-IS SRv6 LAN End.X and OSPFv3 SRv6 LAN End.X
> TLVs/SRv6 End.X SID, IS-IS SRv6 LAN End.X SID, and OSPFv3 SRv6 LAN End.X
> SID TLVs
>
> KT> Ack

>
>
>
> 736     0                   1                   2                   3
> 737     0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> 738    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 739    |               Type            |          Length               |
> 740    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> 741    |    LB Length  |  LN Length    | Fun. Length   |  Arg. Length  |
> 742    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> ...
> 752      Locator Block Length: 1 octet field.  SRv6 SID Locator Block
> 753      length in bits.
>
> 755      Locator Node Length: 1 octet field.  SRv6 SID Locator Node length
> 756      in bits.
>
> 758      Function Length: 1 octet field.  SRv6 SID Function length in
> bits.
>
> 760      Argument Length: 1 octet field.  SRv6 SID Argument length in
> bits.
>
> [major] The names of these fields (as shown in the figure) are different
> from the ones used in the description.
>
> KT> Fixed.

>
>
>
> [major] Where is this information derived from?
>
> KT> Updated text.

>
>
>
> 762   The total of the locator block, locator node, function, and argument
> 763   lengths MUST be less than or equal to 128.
>
> [minor] s/total of the locator block, locator node, function, and argument
> lengths/sum of the LB Length, LN Length, Fun. Length and Arg. Length
>
> KT> Ack

>
>
>
> 765 9.  IANA Considerations
>
> 767   This document requests assigning code-points from the IANA "Border
> 768   Gateway Protocol - Link State (BGP-LS) Parameters" registry as
> 769   described in the sub-sections below.
>
> [nit] This is the only occurrence of "code-point"; most of the text uses
> "codepoint", but it seems to me that the correct form is "code point".
> Please be consistent.
>
> KT> Fixed as code point.

>
>
>
> 771 9.1.  BGP-LS NLRI-Types
>
> 773   The following codepoints are assigned by IANA via the early
> 774   allocation process from within the sub-registry called "BGP-LS NLRI-
> 775   Types":
>
> [nit] s/are assigned/have been assigned/g
>
> KT> Ack

>
>
>
>
> ...
> 808 9.3.  SRv6 BGP EPE SID Flags
>
> 810   This document requests the creation of a new registry called "SRv6
> 811   BGP EPE SID Flags" under the "Border Gateway Protocol - Link State
> 812   (BGP-LS) Parameters" registry.  The allocation policy of this
> 813   registry is "Standards Action" according to [RFC8126].
>
> 815   The following flags are defined:
>
> 817             Bit     Description                   Reference
> 818            ---------------------------------------------------
> 819               0     Backup Flag (B-Flag)        This document
> 820               1     Set Flag (S-Flag)           This document
> 821               2     Persistent Flag (P-Flag)    This document
> 822             3-7     Unassigned
>
> [minor] Missing figure/table number.
>
> KT> Fixed

>
>
>
> 824 10.  Manageability Considerations
> ...
> 841   A consumer of the BGP-LS information retrieves this information over
> 842   a BGP-LS session (refer Section 1 and 2 of [RFC7752]).  The handling
> 843   of semantic or content errors by the consumer would be dictated by
> 844   the nature of its application usage and hence is beyond the scope of
> 845   this document.
>
> [major] While the reference is to rfc7752, rfc7752bis makes it clear that
> the interface to the consumer is not necessarily a BGP session.  This
> paragraph is not needed.
>
> KT> Ack. Removed.

>
>
>
>
> ...
> 866   The extensions, specified in this document, do not introduce any new
> 867   configuration or monitoring aspects in BGP or BGP-LS other than as
> 868   discussed in [RFC7752].  The manageability aspects of the underlying
> 869   SRv6 features are covered by [I-D.ietf-spring-srv6-yang].
>
> [minor] I-D.ietf-spring-srv6-yang:  Not only is this draft expired, but it
> doesn't address the manageability of BGP-LS (or BGP), which is what is
> specified in this document.  Recommendation: remove the reference.
>
> KT> Looks like it was just refreshed. It does cover provisioning of End.X
SID and SID owner as BGP. It does not explicitly mention BGP-EPE though,
which perhaps it should. But it should cover some of the questions you
asked above on how the info is derived for BGP-EPE SRv6 SIDs.

>
>
>
> 871 11.  Security Considerations
> ...
> 882   The extensions introduced in this document are used to propagate IGP
> 883   defined information ([I-D.ietf-lsr-isis-srv6-extensions] and
> 884   [I-D.ietf-lsr-ospfv3-srv6-extensions]).  These extensions represent
> 885   the advertisement of SRv6 information associated with the IGP node,
> 886   link, and prefix.  The IGP instances originating these TLVs are
> 887   assumed to support all the required security and authentication
> 888   mechanisms (as described in [I-D.ietf-lsr-isis-srv6-extensions] and
> 889   [I-D.ietf-lsr-ospfv3-srv6-extensions]) in order to prevent any
> 890   security issue when propagating the information into BGP-LS
>
> < the rest got snipped for me - getting it from the mailer archive >

[major] A recent IESG comment objected to the "prevent any security issue"
claim -- let's take that part out:

s/...in order to prevent any security issue when propagating the
information into BGP-LS./.

KT> Ack.



...
896   BGP-LS SRv6 extensions enable traffic engineering use-cases within
897   the Segment Routing domain.  SR operates within a trusted domain
898   [RFC8402] and its security considerations also apply to BGP-LS
899   sessions when carrying SR information.  The SR traffic engineering
900   policies using the SIDs advertised via BGP-LS are expected to be used
901   entirely within this trusted SR domain (e.g., between multiple AS or
902   IGP domains within a single provider network).  Therefore, precaution
903   is necessary to ensure that the link-state information (including
904   SRv6 information) advertised via BGP-LS sessions is limited to
905   consumers in a secure manner within this trusted SR domain.  BGP
906   peering sessions for address-families other than Link-State may be
907   set up to routers outside the SR domain.  The isolation of BGP-LS
908   peering sessions is recommended to ensure that BGP-LS topology
909   information (including the newly added SR information) is not
910   advertised to an external BGP peering session outside the SR domain.

[] It would be nice to point to the updated Operational and Security
considerations in rfc7752bis.  To not delay publication, should the
recommendation at the end of this paragraph be Normative?

s/recommended/RECOMMENDED

KT> Sure. Will update the text.



...
949 14.1.  Normative References
...
983   [RFC8754]  Filsfils, C., Ed., Dukes, D., Ed., Previdi, S., Leddy, J.,
984              Matsushima, S., and D. Voyer, "IPv6 Segment Routing Header
985              (SRH)", RFC 8754, DOI 10.17487/RFC8754, March 2020,
986              <https://www.rfc-editor.org/info/rfc8754>.

[minor] This reference can be Informative.

KT> Ack



...
1012 14.2.  Informative References
...
1031   [RFC8126]  Cotton, M., Leiba, B., and T. Narten, "Guidelines for
1032              Writing an IANA Considerations Section in RFCs", BCP 26,
1033              RFC 8126, DOI 10.17487/RFC8126, June 2017,
1034              <https://www.rfc-editor.org/info/rfc8126>.

[major] This reference should be Normative.

KT> Ack


1036   [RFC8355]  Filsfils, C., Ed., Previdi, S., Ed., Decraene, B., and R.
1037              Shakir, "Resiliency Use Cases in Source Packet Routing in
1038              Networking (SPRING) Networks", RFC 8355,
1039              DOI 10.17487/RFC8355, March 2018,
1040              <https://www.rfc-editor.org/info/rfc8355>.

[major] As used (§7.2) this reference should be Normative.  See my comment
there about the origin of the information.

KT> This is an informational RFC though ...


1042 Appendix A.  Differences with BGP-EPE for SR-MPLS

[minor] Please add a reference in the main text to this appendix so the
reader knows it is here.

KT> Ack

Thanks,

Ketan

[EoR -09]