[Lsvr] Shepherd review of draft-ietf-lsvr-bgp-spf-28

Ketan Talaulikar <ketant.ietf@gmail.com> Fri, 22 September 2023 07:19 UTC

Return-Path: <ketant.ietf@gmail.com>
X-Original-To: lsvr@ietfa.amsl.com
Delivered-To: lsvr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 59A9FC153CA0; Fri, 22 Sep 2023 00:19:13 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.107
X-Spam-Level:
X-Spam-Status: No, score=-2.107 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_DNSWL_NONE=-0.0001, RCVD_IN_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01] 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 kswZj14rd-ay; Fri, 22 Sep 2023 00:19:10 -0700 (PDT)
Received: from mail-ej1-x62a.google.com (mail-ej1-x62a.google.com [IPv6:2a00:1450:4864:20::62a]) (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 D22BDC153CBB; Fri, 22 Sep 2023 00:19:00 -0700 (PDT)
Received: by mail-ej1-x62a.google.com with SMTP id a640c23a62f3a-9aa0495f9cfso700252666b.1; Fri, 22 Sep 2023 00:19:00 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1695367138; x=1695971938; darn=ietf.org; h=to:subject:message-id:date:from:mime-version:from:to:cc:subject :date:message-id:reply-to; bh=mXJE4f0jkENPRMFA6ygvupfuucxBLdICKe7qxSU3XtA=; b=H67E65HLLRcL1Cms084N6PWyv/BzXS8vTpPFpGX7MCxUPUEzlrE/1yGcpEOHwXe5AR jKRC4azofhc0Q/D+XLPrFCDOgIepAIqMu0Q+2OKApNsDzhC4tB2mdnHHuyN1H0DyUmXK 6PhWTXHfh74Aus8VvilUKmoOIMP++QlpsXRVZR0Al+e6BSqUSwdhb+c10Asy072zPvpN fx4s94pDvjt5ORooz+BfzC3TTsxQbOOQLN9YYXAUOEXJCahJnNdHXTt77vlXlnP6b95U 3IiFJcueXkSslFWlPmqNCs8Tlf4yzpv/QJlQgIqDkTsPLrglx9/3amMyVBi9SkPKxnSX 4cbA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695367138; x=1695971938; h=to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=mXJE4f0jkENPRMFA6ygvupfuucxBLdICKe7qxSU3XtA=; b=DcPSBib/eogVeUla7DNxYc9Dl2nuYTj5DKglLgxf3gmVY/Yy2bFKaGmeddDpq5apL0 2jYWgfEa51ZllCfb92FKJhMNrh//GBnARnbPIIiXuaGu/EsXhPP+2LH/AOhrgqv0hAC7 DLe5G/SnmZSMkGABB+Hu79VL9lXkHVsp2C7oo76MLVIfOpbYu/fOShgWI3RkLoJB8+qP 7GKKExnAqh6YY1pILI86/EK2cPu4RI4eSHQ1+S09vHHD5grDu+O/WYEe4TsVH69NAOS5 7xRlOlnlhwVk+PqGNvkVprvho8+L0964d/zTAbiLVA40qgZ3QrcVbAvEM9uPCAkNZ28Z ed4A==
X-Gm-Message-State: AOJu0YylP/lx28sr4AgNjSo9c+pTztxTg1btH7X3xU+bMkmIK+nec8/X GbqZVkHkRQo+zTo+ZzkDN7a8SdN77WpMBt69tHqtVjxRjAU=
X-Google-Smtp-Source: AGHT+IHyMLU6J3Q+/ADVzORuMTpHGUq8H0T7LoAQ5oxVdpYYlrxXd0Pp21Vyy9FBz6kIihqHOzaaGZA61I5h5Clqkco=
X-Received: by 2002:a17:906:7492:b0:9a1:f1b2:9f2e with SMTP id e18-20020a170906749200b009a1f1b29f2emr2467072ejl.2.1695367136728; Fri, 22 Sep 2023 00:18:56 -0700 (PDT)
MIME-Version: 1.0
From: Ketan Talaulikar <ketant.ietf@gmail.com>
Date: Fri, 22 Sep 2023 12:48:44 +0530
Message-ID: <CAH6gdPwaWWmMk2KDTQ7Lw2Ld8Oq_h-hcgLzxu-MCD1D1ALAPcA@mail.gmail.com>
To: lsvr@ietf.org, draft-ietf-lsvr-bgp-spf@ietf.org
Content-Type: multipart/alternative; boundary="000000000000f2dcaf0605ed6be7"
Archived-At: <https://mailarchive.ietf.org/arch/msg/lsvr/j9_N7KaSn-1_BSfZ8YH0mvYLMl4>
Subject: [Lsvr] Shepherd review of draft-ietf-lsvr-bgp-spf-28
X-BeenThere: lsvr@ietf.org
X-Mailman-Version: 2.1.39
Precedence: list
List-Id: Link State Vector Routing <lsvr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/lsvr>, <mailto:lsvr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/lsvr/>
List-Post: <mailto:lsvr@ietf.org>
List-Help: <mailto:lsvr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/lsvr>, <mailto:lsvr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 22 Sep 2023 07:19:13 -0000

*Summary:*
This shepherd review has been focussed from an implementer's
perspective and therefore brings up the need for a few more updates in the
form of clarifications, editorial changes in the form of consolidation of
certain procedures, adding certain specification details and some
suggestions
for fixing certain technical issues.

I expect that some of these comments/suggestions will likely require
active discussions and WG members should follow/review them closely.


*Suggested editorial changes for consolidation of protocol procedures:*
The flooding procedures (that is the term that I am using in this review)
is the most important change introduced in BGP after the SPF computation.
While an existing BGP implementation can "outsource" certain portions
related
to SPF, the flooding related changes are very close to the core BGP code
flows.

One way to look at it (the way I did), is to see that almost the entire BGP
decision process as related mainly to flooding. There is only the route
computation part which is replaced by the SPF.

How about the following?
- Sec 5: Here focus only on the encodings and their semantic aspects. Do not
  keep text related to procedures/processing.
- A new section 6 that covers the origination and flooding (i.e. propagation
  & update in the LSDB) of the NRLIs. This includes the special handling
when
  self-originated NLRIs are received. It includes the procedures for
  origination due to link/prefix/node failures. Most importantly it should
  include the LSDB synchronization on peering up.
- Then most of the existing section 6 that covers the SPF and route
computation.

IMHO the document will improve vastly for implementers if text were
consolidated in
this manner and it would make a huge impact on the clarity of this protocol
specification.

This is just a suggestion.


*Key Technical Issues:*
a) This spec (as it stands today) is entirely dependent on some mechanism
for
adjacency formation and liveness. I would say normatively dependent. Without
that aspect being covered, I am concerned that the spec may not get to RFC.
This is especially blocking for the RR/controller peering model. Perhaps
for the other two peering models, we can address this with configuration of
the
directly connected interface neighbors and then using an existing liveness
mechanism such as BFD (which is mentioned in the document).

b) EOR being optional there are some issues with the initial database sync
during a session bringup between two routers. I've suggested some solutions
for this.

c) The use of "algo" and participation of nodes using SPF Capability TLV is
problematic. The purpose or goal for this is not clarified by the spec - if
this similar to IGP algo (as in flexalgo), then the encoding does not seem
proper.

d) The document keeps some encoding aspects loose when it comes to the
descriptor TLVs in the NLRI part. This would be problematic for interop and
robustness of the protocol. Please consider tightening things in this area.

e) Issues with the advertisement of prefix mask as a link attribute and then
using the link identifier is a broken method of advertisement of prefix
reachability when there is a Prefix NLRI for this purpose. There are also
issues with the encoding.

f) More clarity is required about topology and prefix computation -
especially
in the context of dual stack the determination of IPv4/IPv6 enablement on
the
links is not signalled explicitly.

g) The document includes the SPF under the BGP decision process currently,
however it is perhaps better to introduce it as a separate functional block
that happens after the decision process. The reason is that the decision
process seems to be intricately associated with the flooding. Some clarity
on
this would be helpful.

I expect that these issues and their solutions are discussed within the WG.


*Detailed review using the idnits rendering of the draft:*
148   This solution avails the benefits of both BGP and SPF-based IGPs.
149   These include TCP based flow-control, no periodic link-state refresh,
150   and completely incremental NLRI advertisement.  These advantages can
151   reduce the overhead in MSDCs where there is a high degree of Equal
152   Cost Multi-Path (ECMPs) and the topology is very stable.
153   Additionally, using an SPF-based computation can support fast
154   convergence and the computation of Loop-Free Alternatives (LFAs).
155   The SPF LFA extensions defined in [RFC5286] can be similarly applied
156   to BGP SPF calculations.  However, the details are a matter of
157   implementation detail.  Furthermore, a BGP-based solution lends

[minor] Perhaps you meant to say "However, the details are implementation
specific and outside the scope of this document." ?

158   itself to multiple peering models including those incorporating
159   route-reflectors [RFC4456] or controllers.

[major] There are several aspects or features that are available in the
link-state IGPs which are referenced in passing all through the document in
various sections - e.g., LFA, MT, SRLG, etc. It would be good to have a
separate section towards the end of the document to list them all -
potentially they all form future work? That way this document stays
focussed on the base protocol spec.

[major] There are some aspects or features in IGPs that are not covered by
this document and will make readers wonder if they apply - e.g, LANs, P2MP
circuits, multi-area/level, advertisement of external routes into the
BGP-SPF
domain, etc. If they are future work, they can be called out as such or
there
is also the option to entirely leave them out of the protocol given the
focus on DCs.

180 1.2.  BGP Shortest Path First (SPF) Motivation

182   Given that [RFC7938] already describes how BGP could be used as the
183   sole routing protocol in an MSDC, one might question the motivation
184   for defining an alternate BGP deployment model when a mature solution
185   exists.  For both alternatives, BGP offers the operational benefits
186   of a single routing protocol as opposed to the combination of an IGP
187   for the underlay and BGP as an overlay.  However, BGP SPF offers some
188   unique advantages above and beyond standard BGP distance-vector
189   routing.  With BGP SPF, the standard hop-by-hop peering model is
190   relaxed.

[minor] Did you mean to say ... hop-by-hop route propagation is relaxed?

192   A primary advantage is that all BGP SPF speakers in the BGP SPF
193   routing domain have a complete view of the topology.  This allows
194   support for ECMP, IP fast-reroute (e.g., Loop-Free Alternatives),
195   Shared Risk Link Groups (SRLGs), and other routing enhancements
196   without advertisement of additional BGP paths [RFC7911] or other
197   extensions.

[minor] Please add reference to RFC5286 (LFA) and RFC4202 section-2.3 (SRLG)

205   Another primary advantage is a potential reduction in NLRI
206   advertisement.  With standard BGP distance-vector routing, a single
207   link failure may impact 100s or 1000s prefixes and result in the
208   withdrawal or re-advertisement of the attendant NLRI.  With BGP SPF,
209   only the BGP SPF speakers corresponding to the link NLRI need to
210   withdraw the corresponding BGP-LS-SPF Link NLRI.  Additionally, the

[minor] Perhaps you meant to say "only the BGP SPF speakers that originated
the link NLRI need ..." ?

211   changed NLRI is advertised immediately as opposed to normal BGP where
212   it is only advertised after the best route selection.  These
213   advantages provide NLRI dissemination throughout the BGP SPF routing
214   domain with efficiencies similar to link-state protocols.

216   With controller and route-reflector peering models, BGP SPF

[minor] Please provide a forward reference to section 4 where these peering
models are specified.

217   advertisement and distributed computation require a minimal number of
218   sessions and copies of the NLRI since only the latest version of the
219   NLRI from the originator is required.  Given that verification of the
220   adjacencies is done outside of BGP (see Section 4), each BGP SPF
221   speaker only needs as many sessions and copies of the NLRI as
222   required for redundancy.

[minor] I think what you want to convey is that a sparse peering approach
also
works unlike traditional BGP. And also unlike IGPs there is no need for a
BGP
peering with every connected router since adjacency verification is done by
some other entity. This is not obvious. Please clarify with the help of
suitable references.

Additionally, a controller could inject
223   topology that is learned outside the BGP SPF routing domain.

[major] Please elaborate what is meant by this injection from a controller,
and how this works. Did you mean about advertising external routes that are
redistributed into the domain? Or something else? Please clarify. Or perhaps
just remove this sentence.

225   Given BGP-LS NLRI is already consumed [I-D.ietf-idr-rfc7752bis], this
226   functionality can be reused for BGP-LS-SPF NLRI.

[minor] What is meant by "consumed" here? Please rephrase for clarity.

228   Another advantage of BGP SPF is that both IPv6 and IPv4 can be
229   supported using the BGP-LS-SPF SAFI with the same BGP-LS-SPF NLRIs.
230   In many MSDC fabrics, the IPv4 and IPv6 topologies are congruent
231   (refer to Section 5.2.2 and Section 5.2.3).  Although beyond the
232   scope of this document, multi-topology extensions could be used to
233   support separate IPv4, IPv6, unicast, and multicast topologies while
234   sharing the same NLRI.

[minor] Perhaps you meant "... while sharing the same AFI/SAFI session" or
perhaps "... while sharing the same BGP-LS NLRI encoding" or both? Please
clarify since the same NLRI is not used for different MTs. There will be
different NLRIs for a prefix for different MTs.

261 2.  Base BGP Protocol Relationship

263   With the exception of the decision process, the BGP SPF extensions
264   leverage the BGP protocol [RFC4271] without change.  This includes
265   the BGP protocol Finite State Machine, BGP messages and their
266   encodings, processing of BGP messages, BGP attributes and path
267   attributes, BGP NLRI encodings, and any error handling defined in
268   [RFC4271] and [RFC7606].

[minor] Reference to RFC4760 is also required here.

270   Due to the changes to the decision process, there are mechanisms and
271   encodings that are no longer applicable.  Unless explicitly specified
272   in the context of BGP SPF, all optional path attributes SHOULD NOT be
273   advertised.  If received, all path attributes MUST be accepted,
274   validated, and propagated consistent with the BGP protocol [RFC4271],
275   even if not needed by BGP SPF.

277   Section 9 of [RFC4271] defines the decision process that is used to
278   select routes for subsequent advertisement by applying the policies
279   in the local Policy Information Base (PIB) to the routes stored in
280   its Adj-RIBs-In.  The output of the Decision Process is the set of
281   routes that are announced by a BGP speaker to its peers.  These
282   selected routes are stored by a BGP speaker in the speaker's Adj-
283   RIBs-Out according to policy.

[major] Link-state protocols do not apply any policy to filter or alter the
information being flooded as this can lead to routing loops or broken
forwarding paths. An exception is at area/domain boundaries where prefixes
are
filtered. Why does it need to be any different here?

285   The BGP SPF extension fundamentally changes the decision process, as
286   described herein.  Specifically:

[minor] Consider severely trimming the text in the 3 bullets below - i.e.
don't
say "how" but only say "what aspect is changed" and then point to the
section.
A reader may get confused when reading the document top down.

288   1.  BGP advertisements are readvertised to neighbors immediately

[minor] I suggest introducing and using the term "flooding" in this document
for this behavior. This way it is immediately clear to readers that this is
flooding as in IGPs and not propagation as in classic BGP.

289       without waiting or dependence on the route computation as
290       specified in phase 3 of the base BGP decision process.  Multiple
291       peering models are supported as specified in Section 4.

[major] How are flooding loops avoided? Basically, if we have learnt the
same
NLRI instance from a peer, we should not advertise it back to that peer.
Right? Does this not need to be specified? Is AS-PATH used? If so, please
clarify since it will have impact in how this is provisioned (perhaps a
suitable reference from RFC7938?).

319 4.  BGP SPF Peering Models

321   Depending on the topology, scaling, capabilities of the BGP SPF
322   speakers, and redundancy requirements, various peering models are
323   supported.

The only requirement is that all BGP SPF speakers in the
324   BGP SPF routing domain adhere to this specification.

[minor] The above sentence is confusing. Isn't it a given? Or was it
something else that needs to be conveyed?

326 4.1.  BGP Single-Hop Peering on Network Node Connections

328   The simplest peering model is the one where EBGP single-hop sessions
329   are established over direct point-to-point links interconnecting the
330   nodes in the BGP SPF routing domain.  Once the single-hop BGP session
331   has been established and the Multi-Protocol Extensions Capability
332   with the BGP-LS-SPF AFI/SAFI has been exchanged [RFC4760] for the
333   corresponding session, then the link is considered up from a BGP SPF
334   perspective and the corresponding BGP-LS-SPF Link NLRI is advertised.

336   An End-of-RIB (EoR) Marker [RFC4724] for the BGP-LS-SPF SAFI MAY be
337   expected prior to advertising the BGP-LS Link NLRI for to peer.

339   A failure to consistently configure the use of the EoR marker can
340   result in transient micro-loops and dropped traffic due to incomplete
341   forwarding state.

[major] Shouldn't EOR be mandatory and form the core requirement of the spec
to ensure database sync at the start of the peering.

343   If the session goes down, the corresponding Link NLRI are withdrawn.
344   Topologically, this would be equivalent to the peering model in
345   [RFC7938] where there is a BGP session on every link in the data
346   center switch fabric.  The content of the Link NLRI is described in
347   Section 5.2.2.

349 4.2.  BGP Peering Between Directly-Connected Nodes

351   In this model, BGP SPF speakers peer with all directly-connected
352   nodes but the sessions may be between loopback addresses (i.e., two-
353   hop sessions) and the direct connection discovery and liveliness
354   detection for the interconnecting links are independent of the BGP
355   protocol.  For example, liveliness detection could be done using the
356   BFD protocol [RFC5880].  Precisely how discovery and liveliness
357   detection is accomplished is outside the scope of this document.

[major] There is a normative dependence of this protocol spec on this
"adjacency" maintenance mechanism. I would consider it to blocking
progressing
this document to RFC unless we include a solution that consists of a static
config + something like BFD as an option to unblock.

358   Consequently, there is a single BGP session even if there are
359   multiple direct connections between BGP SPF speakers.  The BGP-LS-SPF
360   Link NLRI is advertised as long as a BGP session has been
361   established, the BGP-LS-SPF AFI/SAFI capability has been exchanged
362   [RFC4760], the link is operational as determined using liveliness
363   detection mechanisms, and, optionally, the EoR Marker has been
364   received as described in the Section 4.1.  This is much like the
365   previous peering model only peering is between loopback addresses and
366   the interconnecting links can be unnumbered.  However, since there
367   are BGP sessions between every directly-connected node in the BGP SPF
368   routing domain, there is a reduction in BGP sessions when there are
369   parallel links between nodes.  Hence, this peering model is
370   RECOMMENDED over the single-hop peering model Section 4.1.

[minor] I would think that the per-link BGP session in 4.1 also takes care
of
adjacency/liveness and hence is somewhat better than 4.2. The protocol spec
can already specify that in the case of multiple sessions with the same
neighbor router (identified by BGP Router-ID), the updates need not be sent
over all the sessions. IMO, this would therefore be the recommended model -
at
least while there isn't a good adjacency discovery mechanism spec.

372   An End-of-RIB (EoR) Marker [RFC4724] for the BGP-LS-SPF SAFI MAY also
373   be expected prior to advertising the BGP-LS Link NLRI for the link(s)
374   to this peer.

376 4.3.  BGP Peering in Route-Reflector or Controller Topology

378   In this model, BGP SPF speakers peer solely with one or more Route
379   Reflectors [RFC4456] or controllers.  As in the previous model,
380   direct connection discovery and liveliness detection for those links
381   in the BGP SPF routing domain are done outside of the BGP protocol.
382   BGP-LS-SPF Link NLRI is advertised as long as the corresponding link
383   is considered up as per the chosen liveness detection mechanism.

385   This peering model, known as sparse peering, allows for fewer BGP
386   sessions and, consequently, fewer instances of the same NLRI received
387   from multiple peers.  Normally, the route-reflectors or controller
388   BGP sessions would be on directly-connected links to avoid dependence
389   on another routing protocol for session connectivity.  However,
390   multi-hop peering is not precluded.  The number of BGP sessions is
391   dependent on the redundancy requirements and the stability of the BGP
392   sessions.  This is discussed in greater detail in
393   [I-D.ietf-lsvr-applicability].

395   The controller may use constraints to determine when to advertise
396   BGP-LS-SPF NLRI for BGP-LS peers.  For example, a controller may
397   defer advertisement until the EoR marker has been received from both
398   BGP peers and both have received each other's NLRI.  These
399   constraints are outside the scope of this document and, since they
400   are internal to the controller, need not be standardized.

[major] This is problematic. The reliability of flooding and consistency is
paramount for any distributed link-state computation. Please also see
comments
related to the use of EOR. If the controller or RR is involving in flooding
then it just needs to behave just like any other BGP-SPF router when it
comes
to flooding.

402 5.  BGP Shortest Path Routing (SPF) Protocol Extensions

404 5.1.  BGP-LS Shortest Path Routing (SPF) SAFI

406   This document introduces the BGP-LS-SPF SAFI with a value of 80.  The
407   SPF-based decision process (Section 6) applies only to the BGP-LS-SPF
408   SAFI and MUST NOT be used with other combinations of the BGP-LS AFI
409   (16388).  In order for two BGP SPF speakers to exchange BGP-LS-SPF
410   NLRI, they MUST exchange the Multiprotocol Extensions Capability
411   [RFC4760] to ensure that they are both capable of properly processing
412   such NLRI.  This is done with AFI 16388 / SAFI 80.  The BGP-LS-SPF
413   SAFI is used to advertise IPv4 and IPv6 prefix information in a
414   format facilitating an SPF-based decision process.

[minor] What is advertised is not prefixes but the link-state database.
Please
consider removing the last sentence above and instead introduce a new
section
before the current 5.1.1 that introduces the concept of LSDB for
BGP-SPF and what it comprises. The LSDB is what is advertised using the
NLRIs. In the same section, describe that each router will advertise its own
Node NLRI and its half-link (rfc7752bis sec 5.2.2) using the link NLRI and
its
local prefixes (plus any externals, if that is to be covered). This would
set
the context for the sections that follow.

416 5.1.1.  BGP-LS-SPF NLRI TLVs

418   All the TLVs defined for BGP-LS [I-D.ietf-idr-rfc7752bis] are
419   applicable and can be used with the BGP-LS-SPF SAFI to describe
420   links, nodes, and prefixes comprising IGP link-state information.

[minor] Please replace "IGP link-state" with "BGP-SPF LSDB"

422   The NLRI and comprising TLVs MUST be processed as specified in
423   section 5.1 [I-D.ietf-idr-rfc7752bis].

[minor] Perhaps you meant to say that the encoding follows the rules as in
sec
5.1 of rfc7752bis?

TLVs specified as mandatory
424   in [I-D.ietf-idr-rfc7752bis] are considered mandatory for the BGP-LS-
425   SPF SAFI as well.  If a mandatory TLV is not present, the NLRI MUST

[major] Please remove the above line and instead actually specify exactly
what is mandatory for BGP-SPF in the respective sections below. There is no
point in referencing the rfc7752bis for this since a reader will trip over
the
text there as it focuses almost entirely on the IGPs.

426   NOT be used in the BGP SPF route calculation.  All the other TLVs are
427   considered as an optional TLVs.

[major] Please consider if we should tighten the NLRI TLVs for BGP-SPF since
they are so critical to consistent LSDB and computation. Unknown TLVs are ok
in the attribute but in NLRI, they would be problematic. At least a very
strong
warning for any future protocol extensions to not introduce any other TLV in
the NLRI and if there is anything to introduce then the backward
compatibility
aspects are covered.

429 5.1.2.  BGP-LS Attribute

431   The BGP-LS attribute of the BGP-LS-SPF SAFI uses exactly same format
432   of the BGP-LS AFI [I-D.ietf-idr-rfc7752bis].

[minor] Same reference to sec 5.1 of rfc7752bis can be repeated here or
perhaps in some common section like section 3?

In other words, all the
433   TLVs used in BGP-LS attribute of the BGP-LS AFI are applicable and
434   used for the BGP-LS attribute of the BGP-LS-SPF SAFI.  This attribute
435   is an optional, non-transitive BGP attribute that is used to carry
436   link, node, and prefix properties and attributes.  The BGP-LS
437   attribute is a set of TLVs.

[major] Doesn't this need to become a mandatory attribute for BGP-SPF?

439   All the TLVs defined for the BGP-LS Attribute
440   [I-D.ietf-idr-rfc7752bis] are applicable and can be used with the
441   BGP-LS-SPF SAFI to carry link, node, and prefix properties and
442   attributes.

444   The BGP-LS attribute may potentially grow large in size depending on
445   the amount of link-state information associated with a single Link-
446   State NLRI.  The BGP specification [RFC4271] mandates a maximum BGP
447   message size of 4096 octets.  It is RECOMMENDED that an
448   implementation support [RFC8654] in order to accommodate larger size
449   of information within the BGP-LS Attribute.  BGP SPF speakers MUST
450   ensure that they limit the TLVs included in the BGP-LS Attribute to
451   ensure that a BGP update message for a single Link-State NLRI does
452   not cross the maximum limit for a BGP message.  The determination of
453   the types of TLVs to be included by the BGP SPF speaker originating
454   the attribute is outside the scope of this document.  When a BGP SPF
455   speaker finds that it is exceeding the maximum BGP message size due
456   to addition or update of some other BGP Attribute (e.g., AS_PATH), it
457   MUST consider the BGP-LS Attribute to be malformed and the attribute
458   discard handling of [RFC7606] applies.

[minor] Please consider removing the entire paragraph above and instead
putting
a reference to the section in rfc7752bis which says exactly the same thing.

460 5.2.  Extensions to BGP-LS

462   [I-D.ietf-idr-rfc7752bis] describes a mechanism by which link-state
463   and TE information can be collected from IGPs and shared with
464   external components using the BGP protocol.  It describes both the
465   definition of the BGP-LS NLRI that advertise links, nodes, and
466   prefixes comprising IGP link-state information and the definition of
467   a BGP path attribute (BGP-LS attribute) that carries link, node, and
468   prefix properties and attributes, such as the link and prefix metric
469   or auxiliary Router-IDs of nodes, etc.  This document extends the
470   usage of BGP-LS NLRI for the purpose of BGP SPF calculation via
471   advertisement in the BGP-LS-SPF SAFI.

473   The protocol identifier specified in the Protocol-ID field
474   [I-D.ietf-idr-rfc7752bis] represents the origin of the advertised
475   NLRI.  For Node NLRI and Link NLRI, this MUST be the direct protocol
476   (4).  Node or Link NLRI with a Protocol-ID other than the direct
477   protocol is considered malformed.

[major] Why not BGP (7) as in RFC9086? The descriptors would align with
RFC9086. If for any use-case, this information needs to be advertised out
via
BGP-LS to a multi-domain controller, it would make it easier to identify
that
the domain was actually running BGP protocol.

 For Prefix NLRI, the specified
478   Protocol-ID MUST be the origin of the prefix.

[major] Why not BGP again? At least for the local IP addresses of the node.
In any case, this does not seem to have any impact on route computation?

 The local and remote
479   node descriptors for all NLRI MUST include the BGP Identifier (TLV
480   516) [RFC9086] and the AS Number (TLV 512) [I-D.ietf-idr-rfc7752bis].

[major] The name of the TLV is BGP Router-ID and not BGP Identifier - it
might
be confused with BGP-LS Identifier (TLV 513)

481   The BGP Confederation Member (TLV 517) [RFC9086] is currently not
482   applicable.

[major] Why "currently"? Why not just make it not applicable? Please see
previous comment about keeping things "loose" in the NLRI part.

484 5.2.1.  Node NLRI Usage

486   The Node NLRI MUST be advertised unconditionally by all routers in
487   the BGP SPF routing domain.

489 5.2.1.1.  BGP-LS-SPF Node NLRI Attribute SPF Capability TLV

491   The SPF capability is an additional Node Attribute TLV.  This
492   attribute TLV MUST be included with the BGP-LS-SPF SAFI and SHOULD
493   NOT be used for other SAFIs.  The TLV type is 1180.  The Node
494   Attribute TLV contains a single-octet SPF algorithm as defined in
495   [RFC8665].

[major] Is the reference to RFC8665 the SR algorithm TLV? I am not sure why
the existing SR Algorithm TLV (1035) is not used to indicated the algorithm
if
that was the goal. Then again, there is probably no need to advertise algo 0
since that is considered as the default. If it was to explicitly advertise
BGP-SPF capability as the name suggests then something like the SRv6
Capabilities TLV (1038) would have been more appropriate - it also gives the
ability to signal some actual "capabilities" down the line. Also, if down
the
line if the router is participating/supporting multiple algos will it
advertise the same TLV multiple times? - here too using SR Algorithm TLV
would
have been better.

497    0                   1                   2                   3
498    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
499   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
500   |              Type (1180)      |     Length - (1 Octet)        |
501   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
502   | SPF Algorithm |
503   +-+-+-+-+-+-+-+-+

505   The SPF Algorithm field is used to advertise the algorithm used by
506   the router to calculate the paths to other routers in the BGP SPF
507   routing domain.  The SPF algorithm inherits the values from the IGP
508   Algorithm Types registry [RFC8665].  Algorithm 0, (Shortest Path
509   Algorithm (SPF) based on link metric, is supported and described in
510   Section 6.3.  Support for other algorithm types is beyond the scope
511   of this specification.

513   When computing the SPF for a given BGP routing domain, only BGP nodes
514   advertising the SPF capability TLV with same SPF algorithm are
515   included in the SPF computation Section 6.3.  An implementation MAY
516   optionally log detection of a BGP node that has either not advertised
517   the SPF capability TLV or is advertising the SPF capability TLV with
518   an algorithm type other than 0.

[major] What about multiple instances of this TLV being advertised?


520 5.2.1.2.  BGP-LS-SPF Node NLRI Attribute SPF Status TLV

522   A BGP-LS Attribute TLV of the BGP-LS-SPF Node NLRI is defined to
523   indicate the status of the node with respect to the BGP SPF
524   calculation.  This is used to rapidly take a node out of service
525   (refer to Section 6.5.2) or to indicate the node is not to be used
526   for transit (i.e., non-local) traffic (refer to Section 6.3).  If the
527   SPF Status TLV is not included with the Node NLRI, the node is
528   considered to be up and is available for transit traffic.  The SPF
529   status is acted upon with the execution of the next SPF calculation
530   (refer to Section 6.3).

[minor] What is the need for this clarification "... the next SPF .."
required
for this and other TLVs? Is that not the norm for all NLRIs?

532    0                   1                   2                   3
533    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
534   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
535   |   Type (1184)                 |       Length (1 Octet)        |
536   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
537   | SPF Status    |
538   +-+-+-+-+-+-+-+-+

540   SPF Status Values: 0 - Reserved
541                      1 - Node unreachable with respect to BGP SPF
542                      2 - Node does not support transit with respect
543                          to BGP SPF
544                      3-254 - Undefined
545                      255 - Reserved

547   The BGP-LS-SPF Node Attribute SPF Status TLV, Link Attribute SPF
548   Status TLV, and Prefix Attribute SPF Status TLV use the same TLV Type
549   (1184).

551   If a BGP SPF speaker received the Node NLRI but the SPF Status TLV is
552   not received, then any previously received information is considered
553   as implicitly withdrawn and the update is propagated to other BGP SPF
554   speakers.  A BGP SPF speaker receiving a BGP Update containing a SPF

[minor] Why is this special handling required only for this TLV? Should it
not
be a generic behavior to flood a newer copy of the NLRI regardless of the
TLVs
carried within it?

555   Status TLV in the BGP-LS attribute [I-D.ietf-idr-rfc7752bis] with a
556   value that is undefined SHOULD be advertised to other BGP SPF
557   speakers.  However, a BGP SPF speaker MUST NOT use the Status TLV in

[minor] I think you meant "unknown" instead of "undefined"? Also, please
qualify the MUST NOT for clarity - i.e. Status TLVs with unknown values MUST
NOT be used in the SPF.

[major] This makes it very difficult to extend the protocol for new status
since it would break the consistency of the distributed SPF computation.
Not sure if this is a good idea when the protocol is not providing some
actual "flags" to indicate the Router's BGP SPF capability for such things.
At a minimum, there needs to be some guidance in extending this status. If
we
had capability flags, then the routers could verify/check support across the
domain before actually using new(er) extensions.

558   its SPF computation.  An implementation MAY log this condition for
559   further analysis.

561 5.2.2.  Link NLRI Usage

563   The criteria for advertisement of Link NLRI are discussed in
564   Section 4.

566   Link NLRI is advertised with unique local and remote node descriptors
567   dependent on the IP addressing.  For IPv4 links, the link's local
568   IPv4 (TLV 259) and remote IPv4 (TLV 260) addresses are used.  For
569   IPv6 links, the local IPv6 (TLV 261) and remote IPv6 (TLV 262)

[minor] It is clarified in rfc7752bis but you may wish to indicate that the
reference is to global IPv6 addresses here and not link-locals. Link-locals
are not advertised.

570   addresses are used.  For links supporting having both IPv4 and IPv6
571   addresses, both sets of descriptors MAY be included in the same Link
572   NLRI.

[major] I would recommend a tighter spec - leaving things open to MAY in the
NLRI will complicate interop. Perhaps mandate both. For multiple addresses,
perhaps mandate the highest (or lowest) in the NLRI and rest in the
attribute?

574   For unnumbered links, the Link Local/Remote Identifiers (TLV 258) are

[major] Same applies for IPv6 link-local only links.

575   used.  The Link Remote Identifier isn't normally exchanged in BGP and

[major] There is no way anyway for BGP to determine this - so "normally"
does
not make sense to me here. I would recommend that a config option be
provided to
provision the remote identifier. This way it is not entirely dependent on
the
underlying mechanism that is used for adjacency verification.

576   discovering the Link Remote Identifier is beyond the scope of this
577   document.  If the Link Remote Identifier is unknown, a Link Remote
578   Identifier of 0 MUST be advertised.  When 0 is advertised and there

[major] These link identifiers are the only always available and
deterministic
way to correlated links. It would be good to mandate that they are always
there and used. Then we don't need to use IP addresses in the descriptor and
things become simpler and more deterministic (like in ISIS and OSPFv3, as
opposed to OSPFv2). The link identifiers being AF agnostic will greatly help
implementations and deployments.

579   parallel unnumbered links between a pair of BGP SPF speakers, there
580   may be transient intervals where the BGP SPF speakers don't agree on
581   which of the parallel unnumbered links are operational.  For this
582   reason, it is RECOMMENDED that the Link Remote Identifiers be known
583   (e.g., discovered using alternate mechanisms or configured) in the
584   presence of parallel unnumbered links.

586   The link descriptors are described in table 4 of
587   [I-D.ietf-idr-rfc7752bis].

589   For a link to be used in SPF computation for a given address family,
590   i.e., IPv4 or IPv6, both routers connecting the link MUST have an
591   address in the same subnet for that address family.  However, an IPv4
592   or IPv6 prefix associated with the link MAY be installed without the
593   corresponding address on the other side of link.

[major] Again, the spec should be tighter. There is no reason to install
any address from the link (or node) NLRI into the forwarding. If some
prefix
needs to be installed in the forwarding, then it MUST be advertised in the
Prefix NLRI.

595   The IGP metric attribute TLV (TLV 1095) MUST be advertised.  If a BGP

[minor] RFC7752bis indicates that the IGP metric value is variable. It
specifies the length for ISIS and OSPF. Suggest to follow this immediately
with the sentence further below about the length being 4 octets for
BGP-SPF. The
spec should also include guidance to ensure that during SPF calculations the
value is capped at a suitable maximum value to ensure there is no overflow -
refer RFC5305 sec 3.7 as an example.

596   SPF speaker receives a Link NLRI without an IGP metric attribute TLV,
597   then it MUST consider the received NLRI as a malformed and the
598   receiving BGP SPF speaker MUST handle such malformed NLRI as 'Treat-
599   as-withdraw' [RFC7606].

[minor] I think it would be sufficient to just say when any TLV would be
malformed in its respective section and then leave the handling in the Error
Handling section. Perhaps only the exceptions (if any - I don't see them),
need to be covered in the respective section.

The BGP SPF metric length is 4 octets.  A
600   metric is associated with the output side of each router interface.
601   This metric is configurable by the system administrator.  The lower
602   the metric, the more likely the interface is to be used to forward
603   data traffic.  One possible default for metric would be to give each
604   interface a metric of 1 making it effectively a hop count.

[minor] Since we are using the link and prefix metric TLVs that were
originally defined for IGPs, it is good to clarify here and in the Prefix
Metric section that those metrics do not convey any "max-metric" or
"unreachability" notion and that is conveyed by the new TLVs introduced for
BGP-SPF.

606   The usage of other link attribute TLVs is beyond the scope of this
607   document.

609 5.2.2.1.  BGP-LS-SPF Link NLRI Attribute Prefix-Length TLVs

611   Two BGP-LS Attribute TLVs of the BGP-LS-SPF Link NLRI are defined to
612   advertise the prefix length associated with the IPv4 and IPv6 link
613   prefixes derived from the link descriptor addresses.  The prefix
614   length is used for the optional installation of prefixes
615   corresponding to Link NLRI as defined in Section 6.3.

617      0                   1                   2                   3
618      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
619     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
620     |IPv4 (1182) or IPv6 Type (1183)|          Length (1 Octet)     |
621     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
622     | Prefix-Length |
623     +-+-+-+-+-+-+-+-+

625     Prefix-length - A one-octet length restricted to 1-32 for IPv4
626                     Link NLRI endpoint prefixes and 1-128 for IPv6
627                     Link NLRI endpoint prefixes.

629   The Prefix-Length TLV is only relevant to Link NLRIs.  When received
630   with any NLRIs other than Link NRLIs, the corresponding Link NLRI is
631   considered as malformed and MUST be handled as 'Treat-as-withdraw'
632   [RFC7606].  An implementation MAY log an error for further analysis.

[major] Why not use the Prefix NLRI for this purpose? This seems to follow
OSPFv2 while we should be following ISIS or OSPFv3.

634   The maximum prefix-length is 32 bits for an IPv4 Prefix-Length TLV
635   and 128 bits for an IPv6 Prefix-Length TLV.  A prefix-length field
636   indicating a larger value is in error and the corresponding Link NLRI
637   is considered as malformed and MUST be handled as 'Treat-as-withdraw'
638   [RFC7606].  An implementation MAY log an error for further analysis.

[major] When the link has multiple IP addresses, how does one correlate
which
prefix has what prefix length?

640 5.2.2.2.  BGP-LS-SPF Link NLRI Attribute SPF Status TLV

642   This BGP-LS-SPF Attribute TLV of the BGP-LS-SPF Link NLRI is defined
643   to indicate the status of the link with respect to the BGP SPF
644   calculation.  This is used to expedite convergence for link failures
645   as discussed in Section 6.5.1.  If the SPF Status TLV is not included
646   with the Link NLRI, the link is considered up and available.  The SPF
647   status is acted upon with the execution of the next SPF calculation
648   Section 6.3.  A single TLV type is shared by the Node, Link, and
649   Prefix NLRI.  The TLV type is 1184.

651      0                   1                   2                   3
652      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
653     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
654     |   Type (1184)                 |      Length (1 Octet)         |
655     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
656     | SPF Status    |
657     +-+-+-+-+-+-+-+-+

659     BGP Status Values: 0 - Reserved
660                        1 - Link Unreachable with respect to BGP SPF
661                        2-254 - Undefined
662                        255 - Reserved

664   The BGP-LS-SPF Node Attribute SPF Status TLV, Link Attribute SPF
665   Status TLV, and Prefix Attribute SPF Status TLV use the same TLV Type
666   (1184).  This implies that a BGP Update cannot contain multiple NLRI.

[major] Not sure why this "cannot" is needed. Consider a case where multiple
links are going down, the originator can pack an update with multiple link
NLRIs with MP_UNREACH and then have a BGP-LS Attribute for them only with
this
SPF attribute set to 1. The metric and other things probably don't matter.

668   If a BGP SPF speaker received the Link NLRI but the SPF Status TLV is
669   not received, then any previously received information is considered
670   as implicitly withdrawn and the update is propagated to other BGP SPF
671   speakers.  A BGP SPF speaker receiving a BGP Update containing an SPF

[minor] When you say "update" is propagated, I believe you mean the BGP
decision process is followed and if necessary the withdraw is propagated. It
might be easier to just stop after saying considered malformed - which
results
in treat-as-withdraw?

672   Status TLV in the BGP-LS attribute [I-D.ietf-idr-rfc7752bis] with a
673   value that is undefined SHOULD be advertised to other BGP SPF
674   speakers.  However, a BGP SPF speaker MUST NOT use the Status TLV in
675   its SPF computation.  An implementation MAY log this information for
676   further analysis.

678 5.2.3.  IPv4/IPv6 Prefix NLRI Usage

680   IPv4/IPv6 Prefix NLRI is advertised with a Local Node Descriptor and
681   the prefix and length.  The Prefix Descriptors field includes the IP
682   Reachability Information TLV (TLV 265) as described in
683   [I-D.ietf-idr-rfc7752bis].  The Prefix Metric TLV (TLV 1155) MUST be
684   advertised.  The IGP Route Tag TLV (TLV 1153) MAY be advertised.  The

[major] I would suggest to qualify this MUST. Perhaps " ... MUST be
advertised
to be considered for route calculation". This would allow for future
extensibility and use of the Prefix NLRI for conveying other prefix
information. Otherwise, such NLRIs would be considered malformed - and we
don't
want them to be considered malformed.

685   usage of other BGP-LS attribute TLVs is beyond the scope of this
686   document.

688 5.2.3.1.  BGP-LS-SPF Prefix NLRI Attribute SPF Status TLV

690   A BGP-LS Attribute TLV to BGP-LS-SPF Prefix NLRI is defined to
691   indicate the status of the prefix with respect to the BGP SPF
692   calculation.  This is used to expedite convergence for prefix
693   unreachability as discussed in Section 6.5.1.  If the SPF Status TLV
694   is not included with the Prefix NLRI, the prefix is considered
695   reachable.  A single TLV type is shared by the Node, Link, and Prefix
696   NLRI.  The TLV type is 1184.

698        0                   1                   2                   3
699        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
700       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
701       |   Type (1184)                 |      Length (1 Octet)         |
702       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
703       | SPF Status    |
704       +-+-+-+-+-+-+-+-+

706       BGP Status Values: 0 - Reserved
707                          1 - Prefix Unreachable with respect to SPF
708                          2-254 - Undefined
709                          255 - Reserved

711   The BGP-LS-SPF Node Attribute SPF Status TLV, Link Attribute SPF
712   Status TLV, and Prefix Attribute SPF Status TLV use the same TLV Type
713   (1184).  This implies that a BGP Update cannot contain multiple NLRI.

715   If a BGP SPF speaker received the Prefix NLRI but the SPF Status TLV
716   is not received, then any previously received information is
717   considered as implicitly withdrawn and the update is propagated to
718   other BGP SPF speakers.  A BGP SPF speaker receiving a BGP Update
719   containing an SPF Status TLV in the BGP-LS attribute
720   [I-D.ietf-idr-rfc7752bis] with a value that is undefined SHOULD be
721   advertised to other BGP SPF speakers.  However, a BGP SPF speaker
722   MUST NOT use the Status TLV in its SPF computation.  An
723   implementation MAY log this information for further analysis.

725 5.2.4.  BGP-LS Attribute Sequence-Number TLV

727   A BGP-LS Attribute TLV of the BGP-LS-SPF NLRI types is defined to
728   assure the most recent version of a given NLRI is used in the SPF
729   computation.  The Sequence-Number TLV is mandatory for BGP-LS-SPF
730   NLRI.  The TLV type 1181 has been assigned by IANA.  The BGP-LS

[minor] Perhaps there should be a mandate that this TLV is the first one in
the attribute when used for BGP-SPF to ensure efficient flooding?

731   Attribute TLV contains an 8-octet sequence number.  The usage of the
732   Sequence Number TLV is described in Section 6.1.

734      0                   1                   2                   3
735      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
736     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
737     |   Type (1181)                 |      Length (8 Octets)        |
738     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
739     |                Sequence Number (High-Order 32 Bits)           |
740     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
741     |                Sequence Number (Low-Order 32 Bits)            |
742     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

744   Sequence Number: The 64-bit strictly-increasing sequence number MUST
745   be incremented for every self-originated version of a BGP-LS-SPF
746   NLRI.  BGP SPF speakers implementing this specification MUST use
747   available mechanisms to preserve the sequence number's strictly
748   increasing property for the deployed life of the BGP SPF speaker
749   (including cold restarts).  One mechanism for accomplishing this
750   would be to use the high-order 32 bits of the sequence number as a
751   wrap/boot count that is incremented any time the BGP router loses its
752   sequence number state or the low-order 32 bits wrap.

754   When incrementing the sequence number for each self-originated NLRI,
755   the sequence number should be treated as an unsigned 64-bit value.
756   If the lower-order 32-bit value wraps, the higher-order 32-bit value
757   should be incremented and saved in non-volatile storage.  If a BGP
758   SPF speaker completely loses its sequence number state (e.g., the BGP
759   SPF speaker hardware is replaced or experiences a cold-start), the
760   BGP NLRI selection rules (see Section 6.1) insure convergence, albeit
761   not immediately.

763   If the Sequence-Number TLV is not received, then the corresponding
764   NLRI is considered as malformed and MUST be handled as 'Treat-as-
765   withdraw'.  An implementation MAY log an error for further analysis.

767 5.3.  NEXT_HOP Attribute Manipulation

769   The rules for setting the next hop information for the BGP-LS-SPF
770   SAFI follow the specification in section 5.5 of
771   [I-D.ietf-idr-rfc7752bis].  All BGP peers that support SPF extensions
772   will locally compute the Local-RIB Next-Hop as a result of the SPF
773   process.

[major] It is important to specify what impact the NH reachability has on
the
flooding process. In classic BGP, the NH unreachability results in the path
being considered ineligible for best path computation. For BGP-SPF, that is
not the case. However, the question is whether the NLRIs would be withdrawn
when the NH becomes unreachable. There is no need for doing this since we
are
only using the session for flooding and the actual NH used are going to be
determined by the SPF. Also for each of the peering models, would be good to
provide guidance on what to use as an NH by default.

775 6.  Decision Process with SPF Algorithm

777   The Decision Process described in [RFC4271] takes place in three
778   distinct phases.  The Phase 1 decision function of the Decision
779   Process is responsible for calculating the degree of preference for
780   each route received from a BGP SPF speaker's peer.  The Phase 2
781   decision function is invoked on completion of the Phase 1 decision
782   function and is responsible for choosing the best route out of all
783   those available for each distinct destination, and for installing
784   each chosen route into the Local-RIB.  The combination of the Phase 1
785   and 2 decision functions is characterized as a Path Vector algorithm.

787   The SPF based Decision process replaces the BGP Decision process
788   described in [RFC4271].  This process starts with selecting only
789   those Node NLRI whose SPF capability TLV matches with the local BGP
790   SPF speaker's SPF capability TLV value.  Since Link-State NLRI always
791   contains the local node descriptor as described in Section 5.2, each
792   NLRI is uniquely originated by a single BGP SPF speaker in the BGP
793   SPF routing domain (the BGP node matching the NLRI's Node
794   Descriptors).  Instances of the same NLRI originated by multiple BGP
795   SPF speakers would be indicative of a configuration error or a
796   masquerading attack (refer to Section 9).  These selected Node NLRI
797   and their Link/Prefix NLRI are used to build a directed graph during
798   the SPF computation as described below.  The best routes for BGP
799   prefixes are installed in the RIB as a result of the SPF process.

801   When BGP-LS-SPF NLRI is received, all that is required is to
802   determine whether it is the most recent by examining the Node-ID and
803   sequence number as described in Section 6.1.  If the received NLRI
804   has changed, it is advertised to other BGP-LS-SPF peers.  If the
805   attributes have changed (other than the sequence number), a BGP SPF
806   calculation is triggered.  However, a changed NLRI MAY be advertised
807   immediately to other peers and prior to any SPF calculation.  Note

[minor] The above is really the processing of received BGP-LS-SPF NLRIs.
In fact, this is the flooding procedure that perhaps deserves it own
section
where the handling of reachable as well as unreachable is described, There
some
bits and pieces of information that is spread in different sections that
pertain
 to the flooding procedure/function - it would help if they were
consolidated.

808   that the BGP MinRouteAdvertisementIntervalTimer and
809   MinASOriginationIntervalTimer [RFC4271] timers are not applicable to
810   the BGP-LS-SPF SAFI.  The scheduling of the SPF calculation, as

[major] Actually, MRAI would be useful with some exponetial backoff
component
for BGP-SPF as well. It enables throttling of continuous flapping/churning
NLRIs. Please consider MinLSArrival and MinLSInterval of OSPF.

811   described in Section 6.3, is an implementation issue.  Scheduling MAY
812   be dampened consistent with the SPF back-off algorithm specified in
813   [RFC8405].

[major] We cannot leave SPF scheduling to implementation since the use of
inconsistent values or designs in implementations aggravate microloops. Why
not mandate behavior with existing references - either RFC8405 or something
basic that involves specific parameters (initial wait, max wait and the expo
backoff behavior) - as indicated in Sec 10.5 that implementations MUST
provide for configuration. This is required for interoperability and smooth
deployments.

815   The Phase 3 decision function of the Decision Process [RFC4271] is
816   also simplified since under normal SPF operation, a BGP SPF speaker
817   MUST advertise the changed NLRIs to all BGP peers with the BGP-LS-SPF
818   AFI/SAFI and install the changed routes in the GLOBAL-RIB.  The only
819   exception are unchanged NLRIs or stale NLRIs, i.e., NLRI received
820   with a less recent (numerically smaller) sequence number.

822 6.1.  BGP SPF NLRI Selection

824   The rules for all BGP-LS-SPF NLRIs selection for phase 1 of the BGP
825   decision process, section 9.1.1 [RFC4271], no longer apply.

827   1.  NLRI originated by directly connected BGP SPF peers are
828       preferred.  This condition can be determined by comparing the BGP
829       Identifiers in the received Local Node Descriptor and the BGP
830       OPEN message.  This rule assures that stale NLRI is updated even
831       if a BGP-LS router loses its sequence number state due to a cold-
832       start.

[major] Is the above special case needed when we have the procedures in
section
6.1.1? It seems somewhat contradictory?

834   2.  The NLRI with the most recent Sequence Number TLV, i.e., highest
835       sequence number is selected.

[minor] Could the router delete all instances of the NLRI that it has
received
from its neighbors that have lesser sequence number than a recently
received
NLRI instance? If this were done when processing the received updated, then
it
would removes the above step and perhaps also help simplify matters?

837   3.  The NLRI received from the BGP SPF speaker with the numerically
838       larger BGP Identifier is preferred.

840   When a BGP SPF speaker completely loses its sequence number state,
841   i.e., due to a cold start, or in the unlikely possibility that 64-bit
842   sequence number wraps, the BGP routing domain will still converge.
843   This is due to the fact that BGP SPF speakers adjacent to the router
844   always accept self-originated NLRI from the associated speaker as
845   more recent (rule # 1).  When a BGP SPF speaker reestablishes a
846   connection with its peers, any existing sessions are taken down and
847   stale NLRI are replaced.  The adjacent BGP SPF speakers update their
848   NLRI advertisements and advertise to their neighbors until the BGP
849   routing domain has converged.

851   The modified SPF Decision Process performs an SPF calculation rooted
852   at the local BGP SPF speaker using the metrics from the Link
853   Attribute IGP Metric TLV (1095) and the Prefix Attribute Prefix
854   Metric TLV (1155) [I-D.ietf-idr-rfc7752bis].  As a result, any other
855   BGP attributes that would influence the BGP decision process defined
856   in [RFC4271] including ORIGIN, MULTI_EXIT_DISC, and LOCAL_PREF
857   attributes are ignored by the SPF algorithm.  The NEXT_HOP attribute
858   is discussed in Section 5.3.  The AS_PATH and AS4_PATH [RFC6793]
859   attributes are preserved and used for loop detection [RFC4271].  They
860   are ignored during the SPF computation for BGP-LS-SPF NRLIs.

[major] I note that there are no "route types" in this specification. Does
this imply that all prefixes are treated as equal and considered purely on
the
basis of the link and prefix metric values? If so, please specify the same.

862 6.1.1.  BGP Self-Originated NLRI

864   Node, Link, or Prefix NLRI with Node Descriptors matching the local
865   BGP SPF speaker are considered self-originated.  When self-originated
866   NLRI is received and it doesn't match the local node's NLRI content
867   (including sequence number), special processing is required.

869   *  If self-originated NLRI is received and the sequence number is
870      more recent (i.e., greater than the local node's sequence number
871      for the NLRI), the NLRI sequence number is advanced to one greater
872      than the received sequence number and the NLRI is readvertised to
873      all peers.

875   *  If self-originated NLRI is received and the sequence number is the
876      same as the local node's sequence number but the attributes
877      differ, the NLRI sequence number is advanced to one greater than
878      the received sequence number and the NLRI is readvertised to all
879      peers.

881   *  If self-originated Link or Prefix NLRI is received and the Link or
882      Prefix NLRI is no longer being advertised by the local node, the
883      NLRI is withdrawn.

885   The above actions are performed immediately when the first instance
886   of a newer self-originated NLRI is received.  In this case, the newer
887   instance is considered to be a stale instance that was advertised by
888   the local node prior to a restart where the NLRI state is lost.
889   However, if subsequent newer self-originated NLRI is received for the
890   same Node, Link, or Prefix NLRI, the readvertisement or withdrawal is
891   delayed by 5 seconds since it is likely being advertised by a
892   misconfigured or rogue BGP SPF speaker (refer to Section 9).

[major] Suggest to introduce a configurable parameter here - this should
normally be the same as the interval between 2 originations of an NLRI and
there should be an initial wait, max wait and expo backoff between them.

894 6.2.  Dual Stack Support

896   The SPF-based decision process operates on Node, Link, and Prefix
897   NLRIs that support both IPv4 and IPv6 addresses.  Whether to run a
898   single SPF computation or multiple SPF computations for separate AFs
899   is an implementation matter.  Normally, IPv4 next-hops are calculated
900   for IPv4 prefixes and IPv6 next-hops are calculated for IPv6
901   prefixes.

[major] There is the topology computation and then the prefix calculation
that
is done post the topology computation. The spec needs to clarify whether
IPv4 and IPv6 topologies are to be computed separately (this requires the
indication of AFI enablement per link) or if we are considering a single
(congruent) topology. I assume we want the former. In that case, the use of
addresses as link identifiers is not a clear enough indication - consider
IPv6
link-local only scenario. No matter how we may approach - this needs to be
specified in detail. It cannot be an implementation matter.


1128 6.5.  NLRI Advertisement

1130 6.5.1.  Link/Prefix Failure Convergence

1132   A local failure prevents a link from being used in the SPF
1133   calculation due to the IGP bi-directional connectivity requirement.
1134   Consequently, local link failures SHOULD always be given priority
1135   over updates (e.g., withdrawing all routes learned on a session) in
1136   order to ensure the highest priority propagation and optimal
1137   convergence.

[minor] Why would all the NLRIs received over a session be withdrawn from
other peers if the session went down? This needs to happen only if there is
no
other instance of the same NLRI with the same sequence num received from
another peer, right? This may not be obvious since we are not following the
classic BGP and this is perhaps part of the flooding procedure that does not
need to wait for the Route Calculation (i.e. SPF) - or rather should be
running
parallel or independent of the SPF.

1139   With a BGP advertisement, the link would continue to be used until
1140   the last copy of the BGP-LS-SPF Link NLRI is withdrawn.  In order to
1141   avoid this delay, the originator of the Link NLRI SHOULD advertise a
1142   more recent version with an increased Sequence Number TLV for the
1143   BGP-LS-SPF Link NLRI including the SPF Status TLV (refer to
1144   Section 5.2.2.2) indicating the link is down with respect to BGP SPF.
1145   The configurable LinkStatusDownAdvertise timer controls the interval
1146   that the BGP-LS-LINK NLRI is advertised with SPF Status indicating
1147   the link is down prior to withdrawal.  If BGP-LS-SPF Link NLRI has
1148   been advertised with the SPF Status TLV and the link becomes
1149   available in that period, the originator of the BGP-LS-SPF LINK NLRI
1150   MUST advertise a more recent version of the BGP-LS-SPF Link NLRI
1151   without the SPF Status TLV in the BGP-LS Link Attributes.  The
1152   suggested default value for the LinkStatusDownAdvertise timer is 2
1153   seconds.

1155   Similarly, when a prefix becomes unreachable, a more recent version
1156   of the BGP-LS-SPF Prefix NLRI SHOULD be advertised with the SPF
1157   Status TLV (refer to Section 5.2.3.1) indicating the prefix is
1158   unreachable in the BGP-LS Prefix Attributes and the prefix will be
1159   considered unreachable with respect to BGP SPF.  The configurable
1160   PrefixStatusDownAdvertise timer controls the interval that the BGP-
1161   LS-Prefix NLRI is advertised with SPF Status indicating the prefix is
1162   unreachable prior to withdrawal.  If the BGP-LS-SPF Prefix has been
1163   advertised with the SPF Status TLV and the prefix becomes reachable
1164   in that period, the originator of the BGP-LS-SPF Prefix NLRI MUST
1165   advertise a more recent version of the BGP-LS-SPF Prefix NLRI without
1166   the SPF Status TLV in the BGP-LS Prefix Attributes.  The suggested
1167   default value for the PrefixStatusDownAdvertise timer is 2 seconds.

1169 6.5.2.  Node Failure Convergence

1171   By default [RFC4271], all the NLRI advertised by a node are withdrawn
1172   when a session failure is detected.  If fast failure detection such
1173   as BFD is utilized, and the node is on the fastest converging path,
1174   the most recent versions of BGP-LS-SPF NLRI may be withdrawn.  This
1175   results in an older version of the NLRI received on a different path
1176   being used until the new versions arrive and, potentially,
1177   unnecessary route flaps.  For the BGP-LS-SPF SAFI, NLRI received from
1178   the failing node SHOULD NOT be implicitly withdrawn immediately to
1179   prevent such unnecessary route flaps.  The configurable

[major] Why is this special case for Node NLRI alone? It should be the same
for all NLRIs - do not withdraw as long as we have the same instance
received
from another neighbor. The flooding procedure for BGP-SPF entirely replace
the
propagation procedures for classic BGP, right?

1180   NLRIImplicitWithdrawalDelay timer controls the interval that NLRI
1181   from the failed node is retained prior to implicit withdrawal after a
1182   BGP SPF speaker has transitioned out of Established state.  This does
1183   delay convergence since the adjacent nodes detect the link failure

[minor] Perhaps you meant "This does not delay convergence ..."?

1184   and advertise a more recent NLRI indicating the link is down with
1185   respect to BGP SPF (refer to Section 6.5.1) and the bi-directional
1186   connectivity check fails during the BGP SPF calculation (refer to
1187   Section 6.3).  The suggested default value for the
1188   NLRIImplicitWithdrawalDelay timer is 2 seconds.

1190 7.  Error Handling

1192   This section describes the Error Handling actions, as described in
1193   [RFC7606], that are specific to SAFI BGP-LS-SPF BGP Update message
1194   processing.

1196 7.1.  Processing of BGP-LS-SPF TLVs

1198   When a BGP SPF speaker receives a BGP Update containing a malformed
1199   Node NLRI SPF Status TLV in the BGP-LS Attribute
1200   [I-D.ietf-idr-rfc7752bis], the corresponding Node NLRI is considered
1201   as malformed and MUST be handled as 'Treat-as-withdraw'.  An
1202   implementation SHOULD log an error (subject to rate-limiting) for
1203   further analysis.

1205   When a BGP SPF speaker receives a BGP Update containing a malformed
1206   Link NLRI SPF Status TLV in the BGP-LS Attribute
1207   [I-D.ietf-idr-rfc7752bis], the corresponding Link NLRI is considered
1208   as malformed and MUST be handled as 'Treat-as-withdraw'.  An
1209   implementation SHOULD log an error (subject to rate-limiting) for
1210   further analysis.

1212   When a BGP SPF speaker receives a BGP Update containing a malformed
1213   Prefix NLRI SPF Status TLV in the BGP-LS Attribute
1214   [I-D.ietf-idr-rfc7752bis], the corresponding Prefix NLRI is
1215   considered as malformed and MUST be handled as 'Treat-as-withdraw'.
1216   An implementation SHOULD log an error (subject to rate-limiting) for
1217   further analysis.

1219   When a BGP SPF speaker receives a BGP Update containing a malformed
1220   SPF Capability TLV in the Node NLRI BGP-LS Attribute
1221   [I-D.ietf-idr-rfc7752bis], the corresponding Node NLRI is considered
1222   as malformed and MUST be handled as 'Treat-as-withdraw'.  An
1223   implementation SHOULD log an error (subject to rate-limiting) for
1224   further analysis.

1226   When a BGP SPF speaker receives a BGP Update containing a malformed
1227   IPv4 Prefix-Length TLV in the Link NLRI BGP-LS Attribute
1228   [I-D.ietf-idr-rfc7752bis], the corresponding Link NLRI is considered
1229   as malformed and MUST be handled as 'Treat-as-withdraw'.  An
1230   implementation SHOULD log an error (subject to rate-limiting) for
1231   further analysis.

1233   When a BGP SPF speaker receives a BGP Update containing a malformed
1234   IPv6 Prefix-Length TLV in the Link NLRI BGP-LS Attribute
1235   [I-D.ietf-idr-rfc7752bis], the corresponding Link NLRI is considered
1236   as malformed and MUST be handled as 'Treat-as-withdraw'.  An
1237   implementation SHOULD log an error (subject to rate-limiting) for
1238   further analysis.

1240   When a BGP SPF speaker receives a BGP Update containing any malformed
1241   BGP-LS Attribute TE and IGP Metric TLV, the corresponding NLRI is
1242   considered as a malformed and MUST be handled as 'Treat-as-withdraw'
1243   [RFC7606].  An implementation SHOULD log an error (subject to rate-
1244   limiting) for further analysis.

[minor] There is a lot of repetition above. Basically, missing mandatory
TLVs
result in the NLRI being considered malformed and the same handling. If what
is mandatory is covered in section 5.2 then this text can be greatly
condensed.

1246   The BGP-LS Attribute consists of Node attribute TLVs, Link attribute
1247   TLVs, and the Prefix attribute TLVs.  Node attribute TLVs and their
1248   error handling rules are either defined in [I-D.ietf-idr-rfc7752bis]
1249   or derived from [RFC5305] and [RFC6119].  If a BGP SPF speaker
1250   receives a BGP-LS Attribute which is considered malformed based on
1251   these error handling rules, then it MUST consider the received NLRI
1252   as a malformed and the receiving BGP SPF speaker MUST handle such
1253   malformed NLRI as 'Treat-as-withdraw' [RFC7606].

1255   Node Descriptor TLVs and their error handling rules are either
1256   defined in section 5.2.1 of [I-D.ietf-idr-rfc7752bis].  Node
1257   Attribute TLVs and their error handling rules are either defined in
1258   [I-D.ietf-idr-rfc7752bis] or derived from [RFC5305] and [RFC6119].

1260   Link Descriptor TLVs and their error handling rules are either
1261   defined in section 5.2.2 of [I-D.ietf-idr-rfc7752bis].  Link
1262   Attribute TLVs and their error handling rules are either defined in
1263   [I-D.ietf-idr-rfc7752bis] or derived from [RFC5305] and [RFC6119].

1265   Prefix Descriptor TLVs and their error handling rules are either
1266   defined in section 5.2.3 of [I-D.ietf-idr-rfc7752bis].  Prefix
1267   Attribute TLVs and their error handling rules are either defined in
1268   [I-D.ietf-idr-rfc7752bis] or derived from [RFC5130] and [RFC2328].

[minor] Same as previous comment.

1270   If a BGP SPF speaker receives NLRI with a Node Descriptor TLV, Link
1271   Descriptor TLV, or Prefix Descriptor TLV that is considered malformed
1272   based on error handling rules defined in the above references, then
1273   it MUST consider the received NLRI as a malformed and the receiving
1274   BGP SPF speaker MUST handle such malformed NLRI as 'Treat-as-
1275   withdraw' [RFC7606].

1277   When a BGP SPF speaker receives a BGP Update that does not contain
1278   any BGP-LS Attribute, then a BGP SPF speaker MUST consider the
1279   corresponding NLRI as a malformed and MUST handle it as 'Treat-as-
1280   withdraw' [RFC7606].  An implementation SHOULD log and error (subject
1281   to rate-limiting) for further analysis.

1283 7.2.  Processing of BGP-LS-SPF NLRIs

1285   A BGP-LS-SPF Speaker MUST perform the syntactic validation checks of
1286   the BGP-LS-SPF NLRI listed in Section 8.2.2 of
1287   [I-D.ietf-idr-rfc7752bis] to determine if it is malformed.

1289   In common deployment scenarios, the unicast routes installed during
1290   BGP-LS-SPF AFI/SAFI SPF computation serve as the underlay for other
1291   BGP AFI/SAFIs.  To avoid errors encountered in other AFI/SAFIs from
1292   impacting the BGP-LS-SPF AFI/SAFI or vice-versa, isolation mechanisms
1293   such as separate BGP instances or separate BGP sessions (e.g., using
1294   different addresses for peering) for BGP SPF Link-State information
1295   distribution SHOULD be used.

[minor] The above paragraph seems more appropriate as an operational
consideration.

1297 7.3.  Processing of BGP-LS Attribute

1299   A BGP-LS-SPF Speaker MUST perform the syntactic validation checks of
1300   the BGP-LS Attribute listed in Section 8.2.2 of
1301   [I-D.ietf-idr-rfc7752bis] to determine if it is malformed.

1303   An implementation SHOULD log an error for further analysis for
1304   problems detected during syntax validation.

[minor] The text in 7.2 and 7.3 would ideally be covered in the procedures
for
handling reception of updates.

1306 7.4.  BGP-LS-SPF Link State NLRI Database Synchronization

[major] There is no discussion of the need for the initial LSDB sync when a
peering is brought up. The EOR is termed as optional while it should be
mandatory. The EOR should be sent after the router has sent over all the
NLRIs
in its LSDB to its neighbor. If there are multiple sessions, then given we
are
using TCP, the LSDB can be sent over any of the sessions (without
duplication). If there is an existing session between the two peers and a
new
(parallel) one comes up, an EOR can be sent immediately. Unless a router has
received an EOR from its peer, it should not advertise the link NLRI
corresponding to any of its links to that peer. This dependency on holding
back the Link NLRI only applies for directly connected routers and not
peerings to remote controllers/RR. Once we have EOR for any session between
two routers, there is no dependency on the Link NLRI for subsequent (say
parallel) link NLRIs. Basically, the EOR can be used to ensure at least the
initial LSDB sync between over a peering - then we rely on TCP to keep
things
in sync.

1308   While uncommon, there may be situations where the LSNDBs of two BGP-
1309   LS-SPF speakers lose synchronization.  In these situations, the BGP
1310   session MUST be reset.  The mechanisms to detect loss of
1311   synchronization are beyond the scope of this document.

[major] How does one determine that the synchronization is lost in the first
place?

[minor] Could route refresh help or play a role in any of these procedures?

1313 8.  IANA Considerations

1315 8.1.  BGP-LS-SPF Allocation in SAFI Parameters Registry

1317   IANA has assigned value 80 for BGP-LS-SPF from the First Come First
1318   Served range in the "Subsequent Address Family Identifiers (SAFI)
1319   Parameters" registry.  IANA is requested to update the registration
1320   to reference only to this document.

1322 8.2.  BGP-LS-SPF Assignments to BGP-LS NLRI and Attribute TLV Registry

1324   IANA has assigned five TLVs for BGP-LS-SPF NLRI in the "BGP-LS NLRI
1325   and Attribute TLV" registry.  These TLV types include the SPF
1326   capability TLV, Sequence Number TLV, IPv4 Link Prefix-Length TLV,
1327   IPv6 Link Prefix-Length TLV, and SPF Status TLV.

1329       +==========+=============+=================================+
1330       | TLV Code | Description | Reference                       |
1331       | Point    |             |                                 |
1332       +==========+=============+=================================+
1333       | 1180     | SPF         | RFCXXXX ([this document]),      |
1334       |          | Capability  | Section 5.2.1.1                 |
1335       +----------+-------------+---------------------------------+
1336       | 1184     | SPF Status  | Section 5.2.1.2, RFCXXXX ([this |
1337       |          |             | document]), Section 5.2.2.2 and |
1338       |          |             | Section 5.2.3.1                 |

[major] A single SPF Status TLV (1184) is defined here but it is called
different names depending on the NLRI type along with the BGP-LS Attribute.
In fact, it would be really great if many different types of NLRIs could be
packed together in a single update with the BGP-LS Attribute simply
containing
the SPF status "unreachable" and nothing else to cause rapid/efficient
propagation of "down" events. In any case, it just seems semantically wrong
to overload in this manner. I would suggest that we still retain a single
SPF
 Status TLV and which will indicate the status for the object (i.e. NLRI)
which it is associated with. In addition, harmonize the status values in
sec 8.3/4/5 into a single registry and simply clarify in section 5 which
status values are relevant for which object and others can be ignored.

1339       +----------+-------------+---------------------------------+
1340       | 1182     | IPv4 Link   | RFCXXXX ([this document]),      |
1341       |          | Prefix      | Section 5.2.2.1                 |
1342       |          | Length      |                                 |
1343       +----------+-------------+---------------------------------+
1344       | 1183     | IPv6 Link   | RFCXXXX ([this document]),      |
1345       |          | Prefix      | Section 5.2.2.1                 |
1346       |          | Length      |                                 |
1347       +----------+-------------+---------------------------------+
1348       | 1181     | Sequence    | RFCXXXX ([this document]),      |
1349       |          | Number      | Section 5.2.4                   |
1350       +----------+-------------+---------------------------------+

1352                       Table 1: NLRI Attribute TLVs

1354 8.3.  BGP-LS-SPF Node NLRI Attribute SPF Status TLV Status Registry

1356   IANA is requested to create the "BGP-LS-SPF Node NLRI Attribute SPF
1357   Status TLV Status" Registry for status values in a new BGP SPF group.
1358   Initial values for this registry are provided below.  Future
1359   assignments are to be made using the IETF Review registration policy
1360   [RFC8126].

1362           +========+==========================================+
1363           | Values | Description                              |
1364           +========+==========================================+
1365           | 0      | Reserved                                 |
1366           +--------+------------------------------------------+
1367           | 1      | Node unreachable with respect to BGP SPF |
1368           +--------+------------------------------------------+
1369           | 2      | Node does not support transit traffic    |
1370           |        | with respect to BGP SPF                  |
1371           +--------+------------------------------------------+
1372           | 3-254  | Unassigned                               |
1373           +--------+------------------------------------------+
1374           | 255    | Reserved                                 |
1375           +--------+------------------------------------------+

1377                Table 2: BGP-LS-SPF Node NLRI Attribute SPF
1378                   Status TLV Status Registry Assignments

1380 8.4.  BGP-LS-SPF Link NLRI Attribute SPF Status TLV Status Registry

1382   IANA is requested to create the "BGP-LS-SPF Link NLRI Attribute SPF
1383   Status TLV Status" Registry for status values in a new BGP SPF group.
1384   Initial values for this registry are provided below.  Future
1385   assignments are to be made using the IETF Review registration policy
1386   [RFC8126].

1388           +=======+==========================================+
1389           | Value | Description                              |
1390           +=======+==========================================+
1391           | 0     | Reserved                                 |
1392           +-------+------------------------------------------+
1393           | 1     | Link unreachable with respect to BGP SPF |
1394           +-------+------------------------------------------+
1395           | 3-254 | Unassigned                               |
1396           +-------+------------------------------------------+
1397           | 255   | Reserved                                 |
1398           +-------+------------------------------------------+

1400               Table 3: BGP-LS-SPF Link NLRI Attribute SPF
1401                  Status TLV Status Registry Assignments

1403 8.5.  BGP-LS-SPF Prefix NLRI Attribute SPF Status TLV Status Registry

1405   IANA is requested to create the "BGP-LS-SPF Prefix NLRI Attribute SPF
1406   Status TLV Status" Registry for status values in a new BGP SPF group.
1407   Initial values for this registry are provided below.  Future
1408   assignments are to be made using the IETF Review registration policy
1409   [RFC8126].

1411          +=======+============================================+
1412          | Value | Description                                |
1413          +=======+============================================+
1414          | 0     | Reserved                                   |
1415          +-------+--------------------------------------------+
1416          | 1     | Prefix unreachable with respect to BGP SPF |
1417          +-------+--------------------------------------------+
1418          | 3-254 | Unassigned                                 |
1419          +-------+--------------------------------------------+
1420          | 255   | Reserved                                   |
1421          +-------+--------------------------------------------+

1423              Table 4: BGP-LS-SPF Prefix NLRI Attribute SPF
1424                  Status TLV Status Registry Assignments

1467 10.  Management Considerations

1469   This section includes unique management considerations for the BGP-
1470   LS-SPF address family.

1472 10.1.  Configuration

1474   All routers in BGP SPF Routing Domain are under a single
1475   administrative domain allowing for consistent configuration.

[major] AS numbering is not discussed.

1507 10.4.  Adjacency End-of-RIB (EOR) Marker Requirement

1509   Depending on the peering model, topology, and convergence
1510   requirements, an End-of-RIB (EoR) Marker [RFC4724] for the BGP-LS-SPF
1511   SAFI MAY be required from the peer prior to advertising a BGP-LS Link
1512   NLRI for the peer.  If configuration is supported, this SHOULD be
1513   configurable at the BGP SPF instance level and SHOULD be configured
1514   consistently throughout the BGP SPF routing domain.

[major] I think the EOR is required. It is about end-of-sync over a peering
rather than each adjacency/link. This ensures that a router coming up after
say a
reboot does not start attracting transit traffic until it has recovered its
LSDB. For peering with remote RR/controller, it is not possible to have this
dependency check between Link NLRI update and EOR - this should be captured
as
a caveat/limitation of that peering model in section 4.3


1516 10.5.  backoff-config

1518   In addition to configuration of the BGP-LS-SPF address family,
1519   implementations SHOULD support the "Shortest Path First (SPF) Back-
1520   Off Delay Algorithm for Link-State IGPs" [RFC8405].  If supported,
1521   configuration of the INITIAL_SPF_DELAY, SHORT_SPF_DELAY,
1522   LONG_SPF_DELAY, TIME_TO_LEARN, and HOLDDOWN_INTERVAL MUST be
1523   supported [RFC8405].  Section 6 of [RFC8405] recommends consistent
1524   configuration of these values throughout the IGP routing domain and
1525   this also applies to the BGP SPF Routing Domain.

[major] As shared in previous comments, these should be included as
architectural constructs in the spec formally with a default value specified
and which MUST be modifiable via config. Also, the usual guidance for
consistency of values across the domain.


1527 10.6.  Operational Data

[minor] The document is missing operational consideration. Perhaps check
what
could be leveraged from rfc7752bis and there is also some operational
aspects
that are there in other sections.

1529   In order to troubleshoot SPF issues, implementations SHOULD support
1530   an SPF log including entries for previous SPF computations.  Each SPF
1531   log entry would include the BGP-LS-SPF NLRI SPF triggering the SPF,
1532   SPF scheduled time, SPF start time, SPF end time, and SPF type if
1533   different types of SPF are supported.  Since the size of the log is
1534   finite, implementations SHOULD also maintain counters for the total
1535   number of SPF computations and the total number of SPF triggering
1536   events.  Additionally, to troubleshoot SPF scheduling and back-off
1537   [RFC8405], the current SPF back-off state, remaining time-to-learn,
1538   remaining hold-down interval, last trigger event time, last SPF time,
1539   and next SPF time should be available.

[minor] Any guidance for mechanisms for checking and verifying that the
flooding procedure is working well? Oper commands for verifying LSDB and
what
NLRIs are received from which peers - normally we would expect to receive
and
retain a copy of the same NLRI instance from all peers. If this is not
happening, then perhaps it indicates an issue with the flooding?

[EOR - end of review]