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

Alvaro Retana <aretana.ietf@gmail.com> Mon, 26 September 2022 13:21 UTC

Return-Path: <aretana.ietf@gmail.com>
X-Original-To: idr@ietfa.amsl.com
Delivered-To: idr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 00668C1524A8; Mon, 26 Sep 2022 06:21:53 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.104
X-Spam-Level:
X-Spam-Status: No, score=-2.104 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01, URIBL_BLOCKED=0.001, URIBL_DBL_BLOCKED_OPENDNS=0.001, URIBL_ZEN_BLOCKED_OPENDNS=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([50.223.129.194]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id da1t16eIhwRj; Mon, 26 Sep 2022 06:21:48 -0700 (PDT)
Received: from mail-pl1-x634.google.com (mail-pl1-x634.google.com [IPv6:2607:f8b0:4864:20::634]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 9D4A4C1522CA; Mon, 26 Sep 2022 06:21:43 -0700 (PDT)
Received: by mail-pl1-x634.google.com with SMTP id t3so6243405ply.2; Mon, 26 Sep 2022 06:21:43 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:mime-version:from:from:to:cc:subject :date; bh=dTSR44s2HeyA0dzsOW6iVLWbsM5f6iqM9fCqqpBDxL0=; b=gPSlxLSsRcbAKxBugTf+G559nYVqjK0VOFF5a5VQT7bAxHePUeA2tTWvbBRtLKhwvk 5JjoBoZHx7ftuXj8kWhKwK/FmXxlYGyJGMqOOXesARj62ZTpy9iJBRRKREofim5WZ2AC a7cbRnK5oQXrnLBmoNH9CJx9sPEPF8ePGJio6L2ze3utZfVZ98h6simgE9IF9w+gdp/z YOdKeaSKVpevzI6ekMQoW1wD+ZkVMDFZFqGqqUcpMjnCiRp39dv9cNiSFvFZUKU9WK3f BIhjpQfb3yglYVOmRxtZUg9V4KtOwQQwZS0KSwBVUN/L8VLKU19IAfcPGL3MZuR6WBMj Nxqg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:mime-version:from:x-gm-message-state :from:to:cc:subject:date; bh=dTSR44s2HeyA0dzsOW6iVLWbsM5f6iqM9fCqqpBDxL0=; b=p4/jtJb98MPbvbsvWvjRJ4sPNFHjkaKJilJaXmXLS8meLDNTfMSKNOdGtnng0yQ1mN gnEJ8y+SqQHQykHZ902YmIsLgO17EfynnkMmwZ4Qid8JAisEFYtde57EZh+mZRt71Bnu 7IZzcDJUHikF0CQnOMCkrvuzin5cGdqj4NYOarPpgGeKK/STlf5K3XfyvNUlDMvYYpQl ND7Cko6vt6IXtri9LOgSaGhu1PhF6+rQUxqnaVseM+SvtcGFegPoJJM45npjyuRz3FlM bFMkNFk0qODrfQXSZZ1tT1iNbeoEUX6VDtgNXKGmrSe5NRtoYIurKEVgnD2DP9c8WccH oLCQ==
X-Gm-Message-State: ACrzQf1rt+MQZmZ+tgfZ/v7cvDirz/9eb7PldqsvJ8TZifHN1B8ZCIZl OdvDAkuW3hScNivSLhMw/K86SjlwcOJKcALJTZcTg/3v
X-Google-Smtp-Source: AMsMyM5oLAjq9QBGzcXsxG1NIAFjVamrF9/aM4mKGSLAV/ZQajm/CvS/38l39q3P2gpeJAJUYkrDcKY23DwWIni6HFY=
X-Received: by 2002:a17:90b:347:b0:203:c344:9e34 with SMTP id fh7-20020a17090b034700b00203c3449e34mr24893741pjb.115.1664198501755; Mon, 26 Sep 2022 06:21:41 -0700 (PDT)
Received: from 1058052472880 named unknown by gmailapi.google.com with HTTPREST; Mon, 26 Sep 2022 06:21:40 -0700
From: Alvaro Retana <aretana.ietf@gmail.com>
MIME-Version: 1.0
Date: Mon, 26 Sep 2022 06:21:40 -0700
Message-ID: <CAMMESsyFfYF9ZyUKuOM1DQmTPS=DnaN2-rymPvOkxG1OeA=oGQ@mail.gmail.com>
To: draft-ietf-idr-bgpls-srv6-ext@ietf.org
Cc: Susan Hares <shares@ndzh.com>, idr-chairs@ietf.org, idr@ietf.org
Content-Type: multipart/alternative; boundary="0000000000008871dc05e99468a7"
Archived-At: <https://mailarchive.ietf.org/arch/msg/idr/wlbbZAiw-8tqs-qYTd5wqag78q4>
Subject: [Idr] Fwd: AD Review of draft-ietf-idr-bgpls-srv6-ext-09
X-BeenThere: idr@ietf.org
X-Mailman-Version: 2.1.39
Precedence: list
List-Id: Inter-Domain Routing <idr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/idr>, <mailto:idr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/idr/>
List-Post: <mailto:idr@ietf.org>
List-Help: <mailto:idr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/idr>, <mailto:idr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 26 Sep 2022 13:21:53 -0000

[Resending to include the Shepherd, Chairs, and WG.]

On September 26, 2022 at 8:00:05 AM, Alvaro Retana (aretana.ietf@gmail.com)
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.



...
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.


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


[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.



...
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



...
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


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.


[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.



...
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.



...
266 3.2.  SRv6 Node MSD Types

[minor] s/MSD Types/MSD-Types/g
That is the terminology from rfc8814.


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 ...".



...
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


[nit] s/(viz.  End.X/(viz. End.X/g


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.


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.


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.


[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?


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.


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...


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...


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.


[nit] s/inSection 8/in Section 8


366 4.2.  SRv6 LAN End.X SID TLV

[] Most of the comments from the last section apply here too.


...
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


[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".



...
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.



...
466 5.1.  SRv6 Locator TLV

[] Some of the comments from §4.1 about where the values are derived apply
here as well.



...
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.


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


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


[nit] s/advertised as a prefix reachability/advertised as prefix
reachability


[major] "consumer...MUST NOT interpret"

No instructions for the consumer.



...
522      Metric: 4 octet field.  The value of the metric for the Locator.

[major] Where does this value come from?



...
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.


537   A new "Link-State NLRI Type" is defined for SRv6 SID information as
538   following:

[nit] s/as following/as follows



...
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].



...
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].



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].


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


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


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?



...
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?



...
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?


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


...
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.


689     0 1 2 3 4 5 6 7
690    +-+-+-+-+-+-+-+-+
691    |B|S|P| Reserved|
692    +-+-+-+-+-+-+-+-+

[minor] Remove "Reserved".


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


[major] "MAY be assigned"

Who?  How?  Is this related to the End.X SID TLVs defined earlier?


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?



...
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?



...
718   o  Peer AS Number : 4 octets of BGP AS number of the peer router.

[major] Where is this value derived from?


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?


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


[?] 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.  ??


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


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.


[major] Where is this information derived from?


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


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.


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



...
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.


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.



...
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.


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.

[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./.



...
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



...
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.



...
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.


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.


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.

[EoR -09]