From nobody Fri Nov 27 07:14:53 2020 Return-Path: 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 13B8C3A0FF6; Fri, 27 Nov 2020 07:14:52 -0800 (PST) X-Virus-Scanned: amavisd-new at amsl.com X-Spam-Flag: NO X-Spam-Score: 2.903 X-Spam-Level: ** X-Spam-Status: No, score=2.903 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, GB_SUMOF=5, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, UNPARSEABLE_RELAY=0.001, URIBL_BLOCKED=0.001] autolearn=no autolearn_force=no Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7-KNakEKveXR; Fri, 27 Nov 2020 07:14:45 -0800 (PST) Received: from mail-ej1-x630.google.com (mail-ej1-x630.google.com [IPv6:2a00:1450:4864:20::630]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 891693A0FF5; Fri, 27 Nov 2020 07:14:41 -0800 (PST) Received: by mail-ej1-x630.google.com with SMTP id lt17so8088647ejb.3; Fri, 27 Nov 2020 07:14:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:mime-version:date:message-id:subject:to:cc :content-transfer-encoding; bh=7b9iB9tOlHKDrqvpZhVy8oPgdZ1vqwoH+RYtRK43Gsg=; b=s1cuywgdW10OSdL7xdXZQR8pBokzBIHzfTXTm54f/CF3ojCh8RDZ7AQMjzs2DGdxKc cpC9oyy6rvOBWK+jHGLD4iV4ezIyWq14E6pDedgTp/2XJPc0LIyV1SqHiZshpiAc1SQ/ VJeaG1QpAS2YYkdFXpExx+SPp5jITVSLHEv7ywUmoC8p/pBQn4Fxs9YRzs9XbD4ukDd8 P4bLSWFKAuVr2iOjgSBsnjBJeLqwzuIv/LCKMbwyWMcPFq8moEmk/y0v3jHOz+jpvKoV GxlRJOGCRMYeeuUvnl1ZwwAMmmUmF29zwfHhpyJCIxMXC0J+u73Am2EcWF2CXol3pi9t 0fpg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:mime-version:date:message-id:subject:to:cc :content-transfer-encoding; bh=7b9iB9tOlHKDrqvpZhVy8oPgdZ1vqwoH+RYtRK43Gsg=; b=TiQrca19HNc9w5OXPUHEsY4eDtirba6kw+Qq5GvwnmufWQUAa+lf6zeNl5QfGlMRHR UWzYdtdG7vtmfRTiwL5GBMK9cGPY3DTsZ3WQ7T1bv96MwC1QCAazX9YGN0YdXz/CLuTH 48M5aHFRPP9RpHeAVMWcbnms/IR3+CIJi/T4VPxbn1ll8OBMJvOip/40Guu08lseCJEi mK+Q9H8Ut5vgQzVstidxRtQbk1kMOSLoYrYVjmnEMazFsauPo9dqyyS/q2Q8qQS9zlUc TqTBXQNjZiHYTTyYCrVujSeIFRc+OeeWNTNfuzY8PUss6z1zjDFskjcG0BczU2IYzkAn u77w== X-Gm-Message-State: AOAM530Q2NjF3C06+06cabK2TX2xlIei0frmfe+v/45CeC1BzZ7XzP3Y ZsQOvgJcKSuejD14IcZs7YnPqUPh1/SrVx53QCxGUVuq X-Google-Smtp-Source: ABdhPJzRCx3M1tNjSFbYfWTC7GRuZ92jRYyJpN5QKGW1AR9ERhYy+G9Wu/eIVgOXK/avLAEb5iaxuJCnYTgMUYcEQpA= X-Received: by 2002:a17:906:16da:: with SMTP id t26mr8041770ejd.478.1606490076292; Fri, 27 Nov 2020 07:14:36 -0800 (PST) Received: from 1058052472880 named unknown by gmailapi.google.com with HTTPREST; Fri, 27 Nov 2020 07:14:34 -0800 From: Alvaro Retana MIME-Version: 1.0 Date: Fri, 27 Nov 2020 07:14:34 -0800 Message-ID: To: draft-ietf-lsvr-bgp-spf@ietf.org Cc: Victor Kuarsingh , "lsvr-chairs@ietf.org" , "lsvr@ietf.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Archived-At: Subject: [Lsvr] AD Review of draft-ietf-lsvr-bgp-spf-11 X-BeenThere: lsvr@ietf.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Link State Vector Routing List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Nov 2020 15:14:52 -0000 Dear authors: Thank you for the work you've put so far into this document! I believe that the protocol is underspecified, and there is a significant amount of work still needed, which have resulted in a longer than expected set of comments/questions [*]. I approached this document from the point of view of it defining a *new* link-state protocol, one that borrows from existing work. =C2=A0I am loosely defining "protocol" here as a set of rules for what happens inside a specific AFI/SAFI that doesn't carry "traditional" BGP routes, similar to BGP-LS, FlowSpec, etc. =C2=A0Specifically, I see BGP SPF as the sum of the BGP base as transport + BGP-LS encodings + SPF-based route calculation. =C2=A0When adding new components, something has to be taken out. =C2=A0I then expected a discussion about which parts of the BGP base are used/apply to BGP SPF, and which don't + a specification of how the BGP-LS encodings are used, and the definition of new TLVs + SPF details. Instead, the document is written as an extension to BGP-LS. =C2=A0BGP-LS also uses the BGP transport (as is!), but it has a set of assumptions that are not in line with a link-state protocol. =C2=A0For example, rfc7752 is clear about the consumer-based distribution model, including that "a BGP speaker MUST NOT accept updates from a consumer peer". =C2=A0More importantly, rfc7752 considers BGP-LS as transporting opaque data relevant only to the application, which is why there is no error checking (beyond the length): it is not BGP-LS's job to verify that the information is correct (or that it even makes sense). =C2=A0This document doesn't address either of these issues, but it presents a peering model (=C2=A72.3) where a controller (aka consumer) sends routes to other peers. =C2=A0IOW, by extending BGP-LS we inherit the good and the not-so-good. I believe I have a good understanding of what you are doing with BGP SPF. =C2=A0We need to make sure that the intent is reflected in the specification. =C2=A0To start, the document needs a clear explanation of what it is, a section(s) that explains what is being used from the rfc4271/rfc7752, and what isn't. =C2=A0There are many questions that need to be answered (see comments in-line and some more questions/comments below [**]). I haven't read draft-ietf-lsvr-applicability yet -- if some of the answers are there, please point me to it. =C2=A0When I finish that document I'll consider whether the documents need to be returned to the WG. For now, I'll rely on the Shepherd to help in moving this document forward. The mail archive contains early requests to both idr/lsr for review, but no answers. :-( =C2=A0 I would like both of those WGs to (again) be given the opportunity to comment -- we can do that as this document addresses some of my concerns. =C2=A0[I'm putting this note here so we don't forget.] Thank you!! Alvaro. [*] This is a long review, and some mail clients (Outlook?) don't seem to handle long messages well. =C2=A0Please take a look at the archive for the full review; I will put a link in the datatracker: https://datatracker.ietf.org/doc/draft-ietf-lsvr-bgp-spf/history/ [**] It would be ideal for the document to include a "Summary of Operation" section, similar to =C2=A73/rfc4271. =C2=A0This would also serve as a place= to point back at what applies and what doesn't. (1) Use of the BGP Base (rfc4271) - The well-known RIB structure from BGP (Adj-RIB-In, Loc-RIB, Adj-RIB-Out) =C2=A0 didn't quite make it into BGP SPF. =C2=A0It looks like the LSNDB may= correspond =C2=A0 to the Adj-RIB-In, and there is a Local RIB. =C2=A0The non-existence= of an =C2=A0 Adj-RIB-Out implies changes. =C2=A0[Note that it is obviously ok to = not follow =C2=A0 the same structure -- what needs to be explained are the differences= .] - Related to the Adj-RIB-Out not existing is the very unnatural and forced =C2=A0 way in which the text tries to retrofit the new SPF mechanism into t= he BGP =C2=A0 Decision Process. =C2=A0Why are you trying to do that? =C2=A0Isn't i= t easier to simply =C2=A0 point out that the SPF replaces the Decision Process (=C2=A79.1.*). = =C2=A0The =C2=A0 Update-Send Process (=C2=A79.2) is described in terms of the Adj-RIB= -Out, so it =C2=A0 doesn't apply...the text already talks about the setting of the rout= e =C2=A0 advertisement timers (from =C2=A79.2.1.1). =C2=A0The rest of =C2=A79= .2.2, =C2=A79.3 and =C2=A79.4 =C2=A0 don't seem to apply -- but do we need to replace them with anything?= =C2=A0Please =C2=A0 be explicit. - About the attributes... =C2=A0In =C2=A75.1 you mention that attributes th= at "would =C2=A0 influence the Decision process...are ignored". =C2=A0 What about other attributes, the ones that don't influence the Decis= ion =C2=A0 Process? =C2=A0If all the information is encoded in the NLRI or carr= ied in the =C2=A0 BGP-LS Attribute, does BGP SPF need any other attribute? =C2=A0Maybe= requiring =C2=A0 that other attributes not to be included (especially the mandatory o= nes) is =C2=A0 too much, but can it be recommended? =C2=A0Given that BGP (and rfc76= 06) require =C2=A0 specific actions if the attributes are malformed, it would be bad if= we =C2=A0 apply treat-as-withdraw because of an error in an attribute that is = not =C2=A0 relevant in BGP SPF. =C2=A0[Note: we need to add this risk to the Se= curity =C2=A0 Considerations.] =C2=A0 Note that if a recommendation to not include mandatory attributes is =C2=A0 present, then a note should be present related to this text in rfc76= 06: =C2=A0 =C2=A0d. =C2=A0If any of the well-known mandatory attributes are not= present in =C2=A0 =C2=A0 =C2=A0 =C2=A0an UPDATE message, then "treat-as-withdraw" MUST= be used. =C2=A0(Note =C2=A0 =C2=A0 =C2=A0 =C2=A0that [RFC4760] reclassifies NEXT_HOP as what is = effectively =C2=A0 =C2=A0 =C2=A0 =C2=A0discretionary.) - Are there any cases that would result in a BGP SPF-specific session reset= ? =C2=A0 Does the NOTIFICATION message need new error codes/sub-codes? - It seems that the FSM applies, are there any exceptions? - rfc8212 Updates rfc4271, so it is part of the BGP Base. =C2=A0Even though= that =C2=A0 RFC is intended at the use of BGP in the Internet, it doesn't specif= y any =C2=A0 exceptions. =C2=A0I would think that you don't want BGP SPF users to= have to =C2=A0 configure policies (for eBGP connections). =C2=A0It is also importan= t to clearly =C2=A0 define the concept of domain in the text. - It seems that there is no concept of multiple flooding domains in BGP SPF= . =C2=A0 Given the scenario of single-hop eBGP sessions, I'm assuming that if =C2=A0 multiple domains exist in the future they won't be determined by ASN= s. - Route-reflectors are mentioned several times and in one of the deployment =C2=A0 models, but there is no such thing as a RR in BGP SPF: all nodes alr= eady =C2=A0 "reflect" all the routes -- it is part of the synchronization requir= ement of =C2=A0 a link-state protocol. =C2=A0You might need to redefine the terminol= ogy. - Optimal Route Reflection is also mentioned, but it cannot be realized =C2=A0 without loosing synchronization. - Using a Confederation ID is mentioned, which seems ok. =C2=A0I don't see = an =C2=A0 advantage/reason for having Confederations in a BGP SPF network. =C2= =A0Is there =C2=A0 one? - =C2=A75 talks about outgoing policies. =C2=A0Is ORF a valid tool to set t= hose policies? (2) Use of the BGP-LS encodings. - This document is effectively defining a new AFI/SAFI combination -- yes, =C2=A0 related to BGP-LS, but not the same. =C2=A0IOW, you can define which= parts of =C2=A0 rfc7752 apply (if any), beyond the encodings. - Note that rfc7752 treats the data as opaque, not offering any validation. =C2=A0 This document has to specify validation procedures for the TLVs used= , along =C2=A0 with error correction actions, etc.. - There are many BGP-LS TLVs defined, but only some will apply to BGP SPF. = =C2=A0We =C2=A0 need the document to be very clear about that. =C2=A0Note that rfc77= 52 says that =C2=A0 "unrecognized types MUST be preserved and propagated". =C2=A0Please = make it clear =C2=A0 that any type not required in this document is considered unrecogniz= ed and =C2=A0 SHOULD NOT be included. - I'm assuming that this document uses the same format as in Figure 5/rfc77= 52 =C2=A0 for the Link-State NLRI (but with the new SAFI), right? =C2=A0Please= explicitly =C2=A0 point to it. - Which NLRI Types are supported? =C2=A0I guess only 1-4. =C2=A0What should= the receiver =C2=A0 to if a different type is received? - Which Protocol-ID should be used for the NLRI? =C2=A0 =C2=A74 suggests 7 = (BGP), but I =C2=A0 think that is not correct (see more there). =C2=A0What should the re= ceiver do =C2=A0 with a different value? - What should the Identifier (in the NLRI header) be set to? - In a "traditional" link-state protocol, there are a number of LSAs that =C2=A0 define a node: router LSA, network LSA, etc. =C2=A0Please describe h= ow a node is =C2=A0 fully described in BGP SPF -- and how the different NLRIs are associ= ated to =C2=A0 describe that one node. =C2=A0This description could serve as an int= roduction to =C2=A0 =C2=A74 (with forward references to where each part is discussed), o= r as a summary =C2=A0 to that section. =C2=A0 Examples would be very nice. - What must be included as a Node Descriptor? =C2=A0=C2=A74 talks about a p= ossible =C2=A0 combination of different things (see comments/questions there). =C2= =A0It looks =C2=A0 like a TLV with an ID is mandatory (see my note about TLV 515), what= else? - What must be included in a Link Descriptor? =C2=A0Which TLVs are mandator= y and =C2=A0 which ones are optional? =C2=A0There are some rules in =C2=A73.2.2/r= fc7752, do they =C2=A0 apply? =C2=A0How should errors be handled in these TLVs? - What must be included in a Prefix Descriptor? =C2=A0Is any TLV mandatory? - When using the BGP-LS Attribute, which TLVs are mandatory and which are =C2=A0 optional for each NLRI? - I haven't looked at the differences between rfc7752 and rfc7752bis and =C2=A0 whether they would have an impact of anything in this document. =C2= =A0Please do =C2=A0 so. (3) Link-State - How does BGP SPF guarantee that the nodes are synchronized? =C2=A0Other p= rotocols =C2=A0 have DBDs/CSNP/etc. =C2=A0It seems easier in BGP SPF to not exchange= all the =C2=A0 information with a peer than it is in traditional link-state protoco= ls. =C2=A0Being =C2=A0 able to easily break synchronization is a vulnerability that should = be =C2=A0 mentioned in the Security Considerations section. [Line numbers from idnits.] ... 12 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Shortest Path Routing Extensions for= BGP Protocol [major] So...the reason we have a separate WG is because this document is defining a new link state protocol -- yes, it reuses many parts of BGP, but it is not just an extension to BGP. =C2=A0Please use a title that reflects this. ... 15 Abstract 17 =C2=A0 Many Massively Scaled Data Centers (MSDCs) have converged on 18 =C2=A0 simplified layer 3 routing. =C2=A0Furthermore, requirements for 19 =C2=A0 operational simplicity have led many of these MSDCs to converge = on 20 =C2=A0 BGP as their single routing protocol for both their fabric routi= ng 21 =C2=A0 and their Data Center Interconnect (DCI) routing. =C2=A0This doc= ument 22 =C2=A0 describes a solution which leverages BGP Link-State distribution= and 23 =C2=A0 the Shortest Path First (SPF) algorithm similar to Internal Gate= way 24 =C2=A0 Protocols (IGPs) such as OSPF. [major] "This document describes a solution..." =C2=A0A solution to what? As with the title, let's please position this specification for what it is. ... 101 1. =C2=A0Introduction [] Too many claims without references. =C2=A0Are there a min set of BGP RFCs that are applicable, as is? Include a roadmap of the document. =C2=A0There are assumptions mentioned without proper justification. =C2=A0If I didn't know what you're trying to do I wouldn't know what you're talking about. 103 =C2=A0 Many Massively Scaled Data Centers (MSDCs) have converged on 104 =C2=A0 simplified layer 3 routing. =C2=A0Furthermore, requirements for 105 =C2=A0 operational simplicity have led many of these MSDCs to converge= on 106 =C2=A0 BGP [RFC4271] as their single routing protocol for both their f= abric 107 =C2=A0 routing and their Data Center Interconnect (DCI) routing. 108 =C2=A0 Requirements and procedures for using BGP are described in [RFC= 7938]. 109 =C2=A0 This document describes an alternative solution which leverages= BGP- 110 =C2=A0 LS [RFC7752] and the Shortest Path First algorithm similar to 111 =C2=A0 Internal Gateway Protocols (IGPs) such as OSPF [RFC2328]. [minor] The first part of the paragraph talks about using BGP and requirements that are applicable to the solution described in rfc7938. IOW, not applicable to this draft! =C2=A0Note that depending on rfc7938 would make it Normative -- I'm not sure what in that document, which is generic, would apply here. =C2=A0Am I missing something? 113 =C2=A0 [RFC4271] defines the Decision Process that is used to select r= outes 114 =C2=A0 for subsequent advertisement by applying the policies in the lo= cal 115 =C2=A0 Policy Information Base (PIB) to the routes stored in its Adj-R= IBs- 116 =C2=A0 In. =C2=A0The output of the Decision Process is the set of rout= es that are 117 =C2=A0 announced by a BGP speaker to its peers. =C2=A0These selected r= outes are 118 =C2=A0 stored by a BGP speaker in the speaker's Adj-RIBs-Out according= to 119 =C2=A0 policy. [] Direct copy from rfc4271... 121 =C2=A0 [RFC7752] describes a mechanism by which link-state and TE 122 =C2=A0 information can be collected from networks and shared with exte= rnal 123 =C2=A0 components using BGP. =C2=A0This is achieved by defining NLRI a= dvertised 124 =C2=A0 within the BGP-LS/BGP-LS-SPF AFI/SAFI. =C2=A0The BGP-LS extensi= ons defined 125 =C2=A0 in [RFC7752] makes use of the Decision Process defined in [RFC4= 271]. [major] s/ within the BGP-LS/BGP-LS-SPF AFI/SAFI./ using the BGP-LS AFI. 127 =C2=A0 This document augments [RFC7752] by replacing its use of the ex= isting 128 =C2=A0 Decision Process. =C2=A0Rather than reusing the BGP-LS SAFI, th= e BGP-LS- 129 =C2=A0 SPF SAFI is introduced to insure backward compatibility. =C2=A0= The Phase 1 130 =C2=A0 and 2 decision functions of the Decision Process are replaced w= ith 131 =C2=A0 the Shortest Path First (SPF) algorithm also known as the Dijks= tra 132 =C2=A0 algorithm. =C2=A0The Phase 3 decision function is also simplifi= ed since it 133 =C2=A0 is no longer dependent on the previous phases. =C2=A0This solut= ion avails 134 =C2=A0 the benefits of both BGP and SPF-based IGPs. =C2=A0These includ= e TCP based 135 =C2=A0 flow-control, no periodic link-state refresh, and completely 136 =C2=A0 incremental NLRI advertisement. =C2=A0These advantages can redu= ce the 137 =C2=A0 overhead in MSDCs where there is a high degree of Equal Cost Mu= lti- 138 =C2=A0 Path (ECMPs) and the topology is very stable. =C2=A0Additionall= y, using an 139 =C2=A0 SPF-based computation can support fast convergence and the 140 =C2=A0 computation of Loop-Free Alternatives (LFAs) [RFC5286] in the e= vent 141 =C2=A0 of link failures. =C2=A0Furthermore, a BGP based solution lends= itself to 142 =C2=A0 multiple peering models including those incorporating route- 143 =C2=A0 reflectors [RFC4456] or controllers. [minor] "augments [RFC7752] by replacing" =C2=A0 =C2=A0This sounds as if yo= u're extending the use of BGP-LS, when you're really defining a different use for what is specified there. =C2=A0 Maybe something along the lines of "uses the TLV/NLRI defined in rfc7752" would make it clearer. [minor] "BGP-LS-SPF SAFI is introduced to insure backward compatibility" =C2=A0What does the new BGP-LS-SPF SAFI insure backwards compatibility with? =C2=A0Again, you're taking what BGP-LS defined and using a new SAFI to transport it. =C2=A0IOW, there are no changes to rfc7752 and no reason to worry about backwards compatibility. [minor] "The Phase 1 and 2 decision functions..." =C2=A0The first sentence says that the Decision Process is replaced, so I fail to understand why you want to decompose it into phases vs. simply replacing the whole thing. =C2=A0 I'll read further... [nit] "This solution avails..." =C2=A0Maybe start a new paragraph there. [major] "support fast convergence and the computation of Loop-Free Alternatives (LFAs) [RFC5286] in the event of link failures" =C2=A0Nice claim, but there is no mention of LFAs anywhere in the specification part of this document. =C2=A0rfc5286 may apply as-is to the SPF defined here -- please include that information as part of the specification. [nit] "route-reflectors [RFC4456] or controllers." =C2=A0From the client point-of-view, is there a difference? 145 =C2=A0 Support for Multiple Topology Routing (MTR) as described in [RF= C4915] 146 =C2=A0 is an area for further study dependent on deployment requiremen= ts. [minor] rfc4915 is MTR for OSPF. =C2=A0How does it apply to a BGP transport? =C2=A0IOW, we don't seem to need this paragraph as is. =C2=A0It = is however important to mention which "base" link-state functionality is not supported. 148 1.1. =C2=A0BGP Shortest Path First (SPF) Motivation 150 =C2=A0 Given that [RFC7938] already describes how BGP could be used as= the 151 =C2=A0 sole routing protocol in an MSDC, one might question the motiva= tion 152 =C2=A0 for defining an alternate BGP deployment model when a mature so= lution 153 =C2=A0 exists. =C2=A0For both alternatives, BGP offers the operational= benefits 154 =C2=A0 of a single routing protocol. =C2=A0However, BGP SPF offers som= e unique 155 =C2=A0 advantages above and beyond standard BGP distance-vector routin= g. [major] This is another place where the solution here is contrasted against rfc7938. =C2=A0Please keep in mind that we're defining a new routing protocol -- one that could be used in place of "normal" BGP, but also in place of other things. =C2=A0If anything, comparison with other DC-oriented protocols/implementations might fit better in the applicability draft. =C2=A0However, comparison is almost never a good thing. [minor] "For both alternatives, BGP offers the operational benefits of a single routing protocol." =C2=A0Do you explain somewhere how to run both modes at the same time? =C2=A0Mentioning a "single routing protocol" sounds nice, but we really have two protocols potentially using the same peering. =C2=A0I have questions/concerns later about that. 157 =C2=A0 A primary advantage is that all BGP speakers in the BGP SPF rou= ting 158 =C2=A0 domain will have a complete view of the topology. =C2=A0This wi= ll allow 159 =C2=A0 support for ECMP, IP fast-reroute (e.g., Loop-Free Alternatives= ), 160 =C2=A0 Shared Risk Link Groups (SRLGs), and other routing enhancements 161 =C2=A0 without advertisement of addition BGP paths or other extensions= . =C2=A0In 162 =C2=A0 short, the advantages of an IGP such as OSPF [RFC2328] are avai= led in 163 =C2=A0 BGP. [] I think it would be a good idea to differentiate between a "BGP speaker" (rfc4271) and a "BGP SPF speaker" (this document). =C2=A0Please take a pass at normalizing the terminology. =C2=A0In this case s/BGP speakers in the BGP SPF routing domain/BGP SPF speakers in the routing domain [nit] s/addition BGP paths/additional BGP paths [minor] "addition BGP paths or other extensions" =C2=A0Did you mean to put a reference to rfc7911? =C2=A0Assuming that's what you meant. 165 =C2=A0 With the simplified BGP decision process as defined in Section = 5.1, 166 =C2=A0 NLRI changes can be disseminated throughout the BGP routing dom= ain 167 =C2=A0 much more rapidly (equivalent to IGPs with the proper 168 =C2=A0 implementation). [minor] "simplified BGP decision process as defined in Section 5.1" =C2=A75.1 seems to talk about Phase 1, but 3 phases are mentioned a few paragraphs above. =C2=A0As I mentioned then, you should consider talking about replacing the whole Decision Process, not just parts of it, or "simplifying" it. [major] What is a "BGP routing domain"? =C2=A0It is used in several places, but it is not defined anywhere. [minor] "much more rapidly (equivalent to IGPs with the proper implementation)." =C2=A0 Not only does this sound like a sales pitch ("Faster and Improved!"), but it caveats the claim to "IGPs with the proper implementation". =C2=A0I would ask which are those, etc...but I would prefer if you stick to the facts. 170 =C2=A0 Another primary advantage is a potential reduction in NLRI 171 =C2=A0 advertisement. =C2=A0With standard BGP distance-vector routing,= a single 172 =C2=A0 link failure may impact 100s or 1000s prefixes and result in th= e 173 =C2=A0 withdrawal or re-advertisement of the attendant NLRI. =C2=A0Wit= h BGP SPF, 174 =C2=A0 only the BGP speakers corresponding to the link NLRI need withd= raw 175 =C2=A0 the corresponding BGP-LS Link NLRI. =C2=A0This advantage will c= ontribute 176 =C2=A0 to both faster convergence and better scaling. [minor] "BGP-LS Link NLRI" =C2=A0 To avoid confusion, should we be calling this "BGP SPF Link NLRI"? [] "This advantage will contribute to both faster convergence and better scaling." =C2=A0Nice for a marketing brochure. 178 =C2=A0 With controller and route-reflector peering models, BGP SPF 179 =C2=A0 advertisement and distributed computation require a minimal num= ber of 180 =C2=A0 sessions and copies of the NLRI since only the latest version o= f the 181 =C2=A0 NLRI from the originator is required. =C2=A0Given that verifica= tion of the 182 =C2=A0 adjacencies is done outside of BGP (see Section 2), each BGP sp= eaker 183 =C2=A0 will only need as many sessions and copies of the NLRI as requi= red 184 =C2=A0 for redundancy (e.g., one for the SPF computation and another f= or 185 =C2=A0 backup). =C2=A0Functions such as Optimized Route Reflection (OR= R) are 186 =C2=A0 supported without extension by virtue of the primary advantages= . 187 =C2=A0 Additionally, a controller could inject topology that is learne= d 188 =C2=A0 outside the BGP routing domain. [major] "Given that verification of the adjacencies is done outside of BGP (see Section 2)..." =C2=A0=C2=A72 doesn't talk about "verification", bu= t it talks about "connection discovery and liveliness detection". =C2=A0Are you referring to both, or just one part? =C2=A0It would be clearer if you were explicit. [minor] "Given that verification of the adjacencies is done outside of BGP (see Section 2), each BGP speaker will only need as many sessions and copies of the NLRI as required for redundancy (e.g., one for the SPF computation and another for backup)." Hmmm -- are you saying that the number of sessions and updates depends on how the session is verified? =C2=A0I can't wait to get to =C2=A72! [major] "Functions such as Optimized Route Reflection (ORR) are supported without extension by virtue of the primary advantages." draft-ietf-idr-bgp-optimal-route-reflection talks about modification of the Decision Process, but you're replacing it in this document. =C2=A0I mean to say that ORR (as defined in the draft) cannot be supported with BGP SPF (because it doesn't support the Decision Process as described in rfc4271). =C2=A0If you want to talk about the functionality, please explain what it is and explicitly how BGP SPF could achieve a similar result. =C2=A0Note that ORR, from a BGP SPF point of view, relies on not advertising the full database to all the neighbors, resulting in not being synchronized on purpose...which would go against the "primary advantage...that all BGP speakers...will have a complete view of the topology". 190 =C2=A0 Given that controllers are already consuming BGP-LS NLRI [RFC77= 52], 191 =C2=A0 reusing for the BGP-LS SPF leverages the existing controller 192 =C2=A0 implementations. [major] At some point you need to be explicit about the relationship between rfc7752 and this document. =C2=A0Can all the BGP-LS information be reused? [minor] "reusing for the BGP-LS SPF" =C2=A0Reusing what? =C2=A0The "BGP-LS = SPF" what? =C2=A0Are we calling the new protocol "BGP SPF" or "BGP-LS SPF"..? 194 =C2=A0 Another potential advantage of BGP SPF is that both IPv6 and IP= v4 can 195 =C2=A0 be supported in the same address family using the same topology= . 196 =C2=A0 Although not described in this version of the document, multi- 197 =C2=A0 topology extensions can be used to support separate IPv4, IPv6, 198 =C2=A0 unicast, and multicast topologies while sharing the same NLRI. [minor] "IPv6 and IPv4 can be supported in the same address family" This sounds as if you're calling IPv4/IPv6 the same AF...when you're really referring to the transport of the information. =C2=A0Please put a forward reference to where this claim is specified. [minor] "Although not described in this version of the document..." If not described, please don't mention them without a reference. 200 =C2=A0 Finally, the BGP SPF topology can be used as an underlay for ot= her 201 =C2=A0 BGP address families (using the existing model) and realize all= the 202 =C2=A0 above advantages. =C2=A0A simplified peering model using IPv6 l= ink-local 203 =C2=A0 addresses as next-hops can be deployed similar to [RFC5549]. [major] "similar to [RFC5549]" =C2=A0Similar? =C2=A0Please put a forward reference to where this is specified/explained. ... 213 2. =C2=A0BGP Peering Models 215 =C2=A0 Depending on the requirements, scaling, and capabilities of the= BGP 216 =C2=A0 speakers, various peering models are supported. =C2=A0The only = requirement 217 =C2=A0 is that all BGP speakers in the BGP SPF routing domain receive = link- 218 =C2=A0 state NLRI on a timely basis, run an SPF calculation, and updat= e 219 =C2=A0 their data plane appropriately. =C2=A0The content of the Link N= LRI is 220 =C2=A0 described in Section 4.2. [major] "Depending on the requirements, scaling, and capabilities of the BGP speakers, various peering models are supported." =C2=A0I hope that the applicability draft talks about operational/deployment considerations when selecting the peering model. [nit] "only requirement" =C2=A0More than one is listed. [major] "receive link-state NLRI on a timely basis" =C2=A0Is "timely" specified somewhere else? 222 2.1. =C2=A0BGP Single-Hop Peering on Network Node Connections 224 =C2=A0 The simplest peering model is the one described in section 5.2.= 1 of 225 =C2=A0 [RFC7938]. =C2=A0In this model, EBGP single-hop sessions are es= tablished 226 =C2=A0 over direct point-to-point links interconnecting the SPF domain 227 =C2=A0 nodes. =C2=A0For the purposes of BGP SPF, Link NLRI is only adv= ertised if 228 =C2=A0 a single-hop BGP session has been established and the Link-Stat= e/SPF 229 =C2=A0 address family capability has been exchanged [RFC4790] on the 230 =C2=A0 corresponding session. =C2=A0If the session goes down, the corr= esponding 231 =C2=A0 Link NLRI will be withdrawn. =C2=A0Topologically, this would be= equivalent 232 =C2=A0 to the peering model in [RFC7938] where there is a BGP session = on 233 =C2=A0 every link in the data center switch fabric. [major] This reference to rfc7938 makes it Normative. =C2=A0Is that what you really want, to depend on the description of this peering model on rfc7938? =C2=A0I would prefer to see the paragraph reworded to make rfc7938 an example. For example> =C2=A0 =C2=A0In this model, EBGP single-hop sessions are established over d= irect =C2=A0 =C2=A0point-to-point links interconnecting the SPF domain nodes. =C2= =A0Link NLRI is =C2=A0 =C2=A0only advertised if... =C2=A0 Topologically, this would be simi= lar to the =C2=A0 =C2=A0peering model in section 5.2.1 of [RFC7938] where there is a B= GP session =C2=A0 =C2=A0on every link. [minor] "address family capability has been exchanged [RFC4790]" =C2=A0 A forward reference to Section 3 would be better. =C2=A0BTW, what is rfc4790? ;-) [nit] "For the purposes of BGP SPF..." =C2=A0Is redundant: this whole document is talking about it. 235 2.2. =C2=A0BGP Peering Between Directly Connected Network Nodes 237 =C2=A0 In this model, BGP speakers peer with all directly connected ne= twork 238 =C2=A0 nodes but the sessions may be multi-hop and the direct connecti= on 239 =C2=A0 discovery and liveliness detection for those connections are 240 =C2=A0 independent of the BGP protocol. =C2=A0How this is accomplished= is outside 241 =C2=A0 the scope of this document. =C2=A0Consequently, there will be a= single 242 =C2=A0 session even if there are multiple direct connections between B= GP 243 =C2=A0 speakers. =C2=A0For the purposes of BGP SPF, Link NLRI is adver= tised as 244 =C2=A0 long as a BGP session has been established, the Link-State/SPF 245 =C2=A0 address family capability has been exchanged [RFC4790] and the 246 =C2=A0 corresponding link is considered is up and considered operation= al. 247 =C2=A0 This is much like the previous peering model only peering is on= a 248 =C2=A0 single loopback address and the switch fabric links can be 249 =C2=A0 unnumbered. =C2=A0However, there will be the same number of ses= sions as 250 =C2=A0 with the previous peering model unless there are parallel links 251 =C2=A0 between switches in the fabric. [nit] "How this is accomplished is outside the scope of this document." =C2=A0This sentence is redundant... [minor] "Link NLRI is advertised as long as a BGP session has been established...and the corresponding link is considered is up and considered operational." =C2=A0 Is there criteria somewhere about considering the link operational? =C2=A0How is a BGP session paired with a "corresponding link" -- are there cases when the session would be up but the link won't? [major] The reference to rfc4790 is out of place. [minor] "unless there are parallel links" =C2=A0Related to the previous comment: it seems to me that the statement about a "corresponding link" being up/operational and the potential existence of multiple links is somewhat at odds. [nit] s/link is considered is up and considered operational/link is considered is up and operational 253 2.3. =C2=A0BGP Peering in Route-Reflector or Controller Topology 255 =C2=A0 In this model, BGP speakers peer solely with one or more Route 256 =C2=A0 Reflectors [RFC4456] or controllers. =C2=A0As in the previous m= odel, 257 =C2=A0 direct connection discovery and liveliness detection for those 258 =C2=A0 connections are done outside the BGP protocol. =C2=A0More speci= fically, 259 =C2=A0 the Liveliness detection is done using BFD protocol described i= n 260 =C2=A0 [RFC5880]. =C2=A0For the purposes of BGP SPF, Link NLRI is adve= rtised as 261 =C2=A0 long as the corresponding link is up and considered operational= . [minor] "direct connection discovery" =C2=A0I would assume that most of the connections to a RR are not direct. [major] "Liveliness detection is done using BFD" =C2=A0Is this a requirement? =C2=A0Just like any other BGP session, BFD can be used, but it doesn't have to, right? =C2=A0If not required, please word this sentence with BFD as an example. [nit] s/using BFD protocol/using the BFD protocol [nit] "...Link NLRI is advertised as long as the corresponding link is up and considered operational." =C2=A0The other sub-sections also mentioned that the BGP session had to be established and the capability exchanged, but not here. =C2=A0Why? =C2=A0To be honest, I think that mentio= ning all of it (session up, capability exchanged and link up) is obvious and not needed. 263 =C2=A0 This peering model, known as sparse peering, allows for many fe= wer 264 =C2=A0 BGP sessions and, consequently, instances of the same NLRI rece= ived 265 =C2=A0 from multiple peers. =C2=A0It is discussed in greater detail in 266 =C2=A0 [I-D.ietf-lsvr-applicability]. [nit] s/many fewer/fewer [major] Isn't there greater risk in delaying information? =C2=A0Maybe I need to go read the other document... 268 3. =C2=A0BGP-LS Shortest Path Routing (SPF) SAFI 270 =C2=A0 In order to replace the Phase 1 and 2 decision functions of the 271 =C2=A0 existing Decision Process with an SPF-based Decision Process an= d 272 =C2=A0 streamline the Phase 3 decision functions in a backward compati= ble 273 =C2=A0 manner, this draft introduces the BGP-LS-SFP SAFI for BGP-LS SP= F 274 =C2=A0 operation. =C2=A0The BGP-LS-SPF (AFI 16388 / SAFI TBD1) [RFC479= 0] is 275 =C2=A0 allocated by IANA as specified in the Section 6. =C2=A0A BGP sp= eaker using 276 =C2=A0 the BGP-LS SPF extensions described herein MUST exchange the AF= I/SAFI 277 =C2=A0 using Multiprotocol Extensions Capability Code [RFC4760] with o= ther 278 =C2=A0 BGP speakers in the SPF routing domain. [minor] "replace the Phase 1 and 2...and streamline the Phase 3" Again, aren't we replacing the whole Decision Process? [minor] "Phase 3 decision functions in a backward compatible manner" Again, with what? [major] "SAFI TBD1" =C2=A0Aren't there implementations already? =C2=A0What = SAFI value are they using, a private one? [major] The reference to rfc4790 is completely out of place. [] OLD> =C2=A0 =C2=A0A BGP speaker using the BGP-LS SPF extensions described herein= MUST =C2=A0 =C2=A0exchange the AFI/SAFI using Multiprotocol Extensions Capabilit= y Code =C2=A0 =C2=A0[RFC4760] with other BGP speakers in the SPF routing domain. NEW> =C2=A0 =C2=A0In order for two BGP speakers to exchange BGP SPF NLRI, they M= UST exchange =C2=A0 =C2=A0the Multiprotocol Extensions Capability [rfc5492] [rfc4760] to= ensure that =C2=A0 =C2=A0they are both capable of properly processing such NLRI. =C2=A0= This is done with =C2=A0 =C2=A0AFI 16388 / SAFI TBDx for BGP-LS-SPF. [major] Can the BGP SPF session also support other AFI/SAFI pairs? =C2=A0If so, what are the implications/limitations because of the peering models. =C2=A0Should this document (similar to rfc7752bis) recommend/require the isolation of BGP SPF in it's own BGP session? Why/why not? =C2=A0IMHO, it should be required. 280 4. =C2=A0Extensions to BGP-LS [] Are we extending BGP-LS or just using parts of it? 282 =C2=A0 [RFC7752] describes a mechanism by which link-state and TE 283 =C2=A0 information can be collected from networks and shared with exte= rnal 284 =C2=A0 components using BGP protocol. =C2=A0It describes both the defi= nition of 285 =C2=A0 BGP-LS NLRI that describes links, nodes, and prefixes comprisin= g IGP 286 =C2=A0 link-state information and the definition of a BGP path attribu= te 287 =C2=A0 (BGP-LS attribute) that carries link, node, and prefix properti= es and 288 =C2=A0 attributes, such as the link and prefix metric or auxiliary Rou= ter- 289 =C2=A0 IDs of nodes, etc. [nit] s/using BGP protocol/using the BGP protocol [major] This document reuses the BGP-LS Attribute. =C2=A0However, rfc7752 says that it "MUST be ignored for all other address families". =C2=A0My interpretation of that text is that it was not intended to be specific to the BGP-LS AFI, but the AFI/SAFI pairs defined there. =C2=A0This interpretation may be too strict -- please include a note related to why it is ok to reuse the BGP-LS Attribute here. 291 =C2=A0 The BGP protocol will be used in the Protocol-ID field specifie= d in 292 =C2=A0 table 1 of [I-D.ietf-idr-bgpls-segment-routing-epe]. =C2=A0The = local and 293 =C2=A0 remote node descriptors for all NLRI will be the BGP Router-ID = (TLV 294 =C2=A0 516) and either the AS Number (TLV 512) [RFC7752] or the BGP 295 =C2=A0 Confederation Member (TLV 517) [RFC8402]. =C2=A0However, if the= BGP 296 =C2=A0 Router-ID is known to be unique within the BGP Routing domain, = it can 297 =C2=A0 be used as the sole descriptor. [major] "The BGP protocol will be used in the Protocol-ID field specified in table 1 of [I-D.ietf-idr-bgpls-segment-routing-epe]." Assuming that you mean that the source Protocol-ID will be 7... =C2=A0Why? BGP SPF is propagating information about the local node, why is the source protocol not set to Direct? =C2=A0Note that the information advertised doesn't come from a different protocol. >From rfc7752: "direct access to interface information and wants to advertise a local link, then the Protocol-ID 'Direct' SHOULD be used." [minor] "The local and remote node descriptors for all NLRI..." =C2=A0 I'm assuming you're talking about the Local Node Descriptors and Remote Node Descriptors fields, right? =C2=A0Please be specific. [major] Incorrect references. =C2=A0s/BGP Router-ID (TLV 516)...[RFC8402]/BGP Router-ID (TLV 516) [I-D.ietf-idr-bgpls-segment-routing-epe], and either the AS Number (TLV 512) [RFC7752] or the BGP Confederation Member (TLV 517) [I-D.ietf-idr-bgpls-segment-routing-epe] [major] Even if it may be obvious to you, please specify when to use TLV 512 and when to use TLV 517. draft-ietf-idr-bgpls-segment-routing-epe requires the use of TLV 512 all the time (and include in it the Confederation ASN, if applicable) -- is there a reason not to follow the same recommendation? [minor] s/BGP Router ID/BGP Identifier That's what rfc6286 uses. Note that the TLV name is ok. [major] "if the BGP Router-ID is known to be unique within the BGP Routing domain" =C2=A0How? =C2=A0Is this a configuration/runtime decision? rfc6286 already required that the BGP Identifier be unique within an AS. [major] "...will be used...will be...can be used..." =C2=A0Should any of this be Normative language? =C2=A0I prefer to be explicit if these are requirements. [major] Related... Besides the language not being tight enough, there are too many options: 516 + ((512 OR 517) OR nothing). =C2=A0How does the receiver know if what it got (say only TLV 516, for example) is what is should have received? [major] rfc7752 classifies the IGP Router-ID TLV (515) as mandatory. Given that BGP SPF is a new IGP protocol, why isn't that used instead? =C2=A0You can specify the content as the BGP ID. [major] For everything specified here, what should a receiver do if the information received is not what is expected. 299 4.1. =C2=A0Node NLRI Usage 301 =C2=A0 The BGP Node NLRI will be advertised unconditionally by all rou= ters 302 =C2=A0 in the BGP SPF routing domain. [minor] "BGP Node NLRI" =C2=A0 rfc7752 uses "Node NLRI" -- are you using BGP to try and differentiate? [major] "will be" =C2=A0Is this a requirement? =C2=A0Should Normative langu= age be used? 304 4.1.1. =C2=A0Node NLRI Attribute SPF Capability TLV 306 =C2=A0 The SPF capability is a new Node Attribute TLV that will be add= ed to 307 =C2=A0 those defined in table 7 of [RFC7752]. =C2=A0The new attribute = TLV will 308 =C2=A0 only be applicable when BGP is specified in the Node NLRI Proto= col ID 309 =C2=A0 field. =C2=A0The TBD TLV type will be defined by IANA. =C2=A0Th= e new Node 310 =C2=A0 Attribute TLV will contain a single-octet SPF algorithm as defi= ned in 311 =C2=A0 [RFC8402]. [minor] s/new Node Attribute TLV that will be added to those defined in table 7 of [RFC7752]./new Node Attribute TLV. Note that we're not adding anything to the table in rfc7752 -- in the best case we're getting a new TLV code point from a registry. [minor] s/The new attribute TLV will only be applicable/The new attribute TLV is only applicable [nit] s/Protocol ID/Protocol-ID [] "BGP is specified in the Node NLRI Protocol ID" =C2=A0See my comments elsewhere about using BGP as the Protocol-ID. [major] "The TBD TLV type will be defined by IANA." =C2=A0I know IANA hasn't assigned one yet, use TBDx instead. =C2=A0Same question about implementation -- there is no private use here... [major] s/[RFC8402]/[RFC8665] =C2=A0 This should be a new Normative referen= ce. 313 =C2=A0 =C2=A0 =C2=A0 0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 1 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 2 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 3 314 =C2=A0 =C2=A0 =C2=A0 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 315 =C2=A0 =C2=A0 =C2=A0+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-= +-+-+-+-+-+-+-+ 316 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= Type =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 Length =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| 317 =C2=A0 =C2=A0 =C2=A0+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-= +-+-+-+-+-+-+-+ 318 =C2=A0 =C2=A0 =C2=A0| SPF Algorithm | 319 =C2=A0 =C2=A0 =C2=A0+-+-+-+-+-+-+-+-+ [minor] Please add Figure numbers everywhere. [minor] Please indicate the Type; use TBDx to identify the different values to be assigned. [major] Please indicate somewhere that the Length should be. =C2=A0In this = case 1. [] The length indication is not present in any of the other TLVs defined. =C2=A0Please indicate the value there too. 321 =C2=A0 The SPF Algorithm may take the following values: ... 335 =C2=A0 Note that usage of Strict Shortest Path First (SPF) algorithm i= s 336 =C2=A0 defined in the IGP algorithm registry but usage is restricted t= o 337 =C2=A0 [I-D.ietf-idr-bgpls-segment-routing-epe]. =C2=A0Hence, its usag= e for BGP- 338 =C2=A0 LS SPF is out of scope. [major] There's no need to repeat the definitions from rfc8665, especially given that only one value is supported. =C2=A0Just say that! [major] I don't know where any restriction on algorithm 1 is documented (certainly not in I-D.ietf-idr-bgpls-segment-routing-epe)... =C2=A0What about other values? If algorithm 1 is the only one supported, and maybe the only one for a while (?), why does it need to be signaled? 340 =C2=A0 When computing the SPF for a given BGP routing domain, only BGP= nodes 341 =C2=A0 advertising the SPF capability attribute will be included the 342 =C2=A0 Shortest Path Tree (SPT). [major] =C2=A75 talks about SPF running only for nodes that advertised the same SFP Algorithm as the local node...so just advertising this TLV is not enough. =C2=A0At least add a forward reference to =C2=A75. [major] Related... =C2=A0If this is a mandatory TLV, please say so. However, it seems that a default value (0) be assumed if not present? [minor] s/SPF capability attribute/SPF Capability TLV [major] Where is the SPF algorithm specification? =C2=A0How is the routing table calculated from the BGP SPF information? 344 4.1.2. =C2=A0BGP-LS Node NLRI Attribute SPF Status TLV 346 =C2=A0 A BGP-LS Attribute TLV to BGP-LS Node NLRI is defined to indica= te the 347 =C2=A0 status of the node with respect to the BGP SPF calculation. =C2= =A0This 348 =C2=A0 will be used to rapidly take a node out of service or to indica= te the 349 =C2=A0 node is not to be used for transit (i.e., non-local) traffic. = =C2=A0If the 350 =C2=A0 SPF Status TLV is not included with the Node NLRI, the node is 351 =C2=A0 considered to be up and is available for transit traffic. [?] "A BGP-LS Attribute TLV to BGP-LS Node NLRI..." =C2=A0What does this mean? =C2=A0My guess is that this TLV is to only be used in the BGP-LS Attribute with a Node NLRI. =C2=A0Is that it? [major] "TLV to BGP-LS Node NLRI is defined" =C2=A0By mentioning BGP-LS, are you implying that this new TLV can be used by an rfc7752 implementation? =C2=A0Or is the expectation that it will only be used by a BGP SPF SAFI is in use? =C2=A0Either way, please be explicit. =C2=A0This ty= pe of text is present in other sections. [] Using BGP-LS sometimes is getting confusing. 353 =C2=A0 =C2=A0 =C2=A0 0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 1 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 2 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 3 354 =C2=A0 =C2=A0 =C2=A0 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 355 =C2=A0 =C2=A0 =C2=A0+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-= +-+-+-+-+-+-+-+ 356 =C2=A0 =C2=A0 =C2=A0| =C2=A0 TBD Type =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Length =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| 357 =C2=A0 =C2=A0 =C2=A0+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-= +-+-+-+-+-+-+-+ 358 =C2=A0 =C2=A0 =C2=A0| SPF Status =C2=A0 =C2=A0| 359 =C2=A0 =C2=A0 =C2=A0+-+-+-+-+-+-+-+-+ [major] The TLV format defined in rfc7752 is: =C2=A0 =C2=A0 =C2=A0 0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 1 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= 2 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 3 =C2=A0 =C2=A0 =C2=A0 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 =C2=A0 =C2=A0 =C2=A0+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+= -+-+-+-+-+ =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Type = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 Length =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+= -+-+-+-+-+ =C2=A0 =C2=A0 =C2=A0// =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Value (variable) =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 // =C2=A0 =C2=A0 =C2=A0+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+= -+-+-+-+-+ [] There are other places where the TLV format is not followed either. 361 =C2=A0 =C2=A0 =C2=A0 BGP Status Values: 0 - Reserved 362 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A01 - Node Unreachable with respect to BGP SPF 363 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A02 - Node does not support transit with respect 364 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0to BGP SPF 365 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A03-254 - Undefined 366 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0255 - Reserved [major] What should the receiving node do with this information? =C2=A0I understand how the overload bit/max-metric work -- I'm asking for a specification in this new protocol document. =C2=A0A forward reference would be good. [minor] When does the status take effect? =C2=A0Immediately? =C2=A0Is there= a "graceful shutdown" concept? [major] What should the receiver do if an unknown value is received? 368 4.2. =C2=A0Link NLRI Usage ... 373 =C2=A0 Link NLRI is advertised with local and remote node descriptors = as 374 =C2=A0 described above and unique link identifiers dependent on the 375 =C2=A0 addressing. =C2=A0For IPv4 links, the links local IPv4 (TLV 259= ) and 376 =C2=A0 remote IPv4 (TLV 260) addresses will be used. =C2=A0For IPv6 li= nks, the 377 =C2=A0 local IPv6 (TLV 261) and remote IPv6 (TLV 262) addresses will b= e 378 =C2=A0 used. =C2=A0For unnumbered links, the link local/remote identif= iers (TLV 379 =C2=A0 258) will be used. =C2=A0For links supporting having both IPv4 = and IPv6 380 =C2=A0 addresses, both sets of descriptors may be included in the same= Link 381 =C2=A0 NLRI. =C2=A0The link identifiers are described in table 5 of [R= FC7752]. [major] "...links supporting having both IPv4 and IPv6 addresses, both sets of descriptors may be included in the same Link NLRI." =C2=A0Is it ok for one side of the link to advertise just one Link NLRI for a link (including both IPv4 and IPv6) while the other side advertises 2 Link NLRIs (one for IPv4 and the other for IPv6)? =C2=A0 If so, is there an expected default behavior? =C2=A0Is one Link NLRI an optional behavior, or the default? =C2=A0Why/when would a node choose to deviate from the default? 383 =C2=A0 The link IGP metric attribute TLV (TLV 1095) as well as any oth= ers 384 =C2=A0 required for non-SPF purposes SHOULD be advertised. =C2=A0The m= etric value 385 =C2=A0 in this TLV is variable length dependent on specific protocol u= sage 386 =C2=A0 (refer to section 3.3.2.4 in [RFC7752]). =C2=A0For simplicity, = the BGP-LS 387 =C2=A0 SPF metric length will be 4 octets. =C2=A0Algorithms such as se= tting the 388 =C2=A0 metric inversely to the link speed as done in the OSPF MIB [RFC= 4750] 389 =C2=A0 MAY be supported. =C2=A0However, this is beyond the scope of th= is 390 =C2=A0 document. [minor] s/link IGP metric attribute TLV/IGP Metric TLV That's what rfc7752 calls it. [major] "TLV 1095...SHOULD be advertised" =C2=A0When is it ok to not advertise the metric? =C2=A0IOW, why isn't it required? [nit] s/any others/any other [major] "others required for non-SPF purposes SHOULD be advertised" What other TLVs? =C2=A0Given that you're making this behavior Normative, you should provide more information. =C2=A0BTW, what other non-SPF purposes? =C2=A0This phrase is also used in =C2=A74.3, but there's no explanation as to why this new protocol would carry non-SPF stuff, or what what may be. =C2=A0Recommendation: leave the non-SPF part out. [minor] OLD> =C2=A0 =C2=A0The metric value in this TLV is variable length dependent on s= pecific =C2=A0 =C2=A0protocol usage (refer to section 3.3.2.4 in [RFC7752]). =C2=A0= For simplicity, =C2=A0 =C2=A0the BGP-LS SPF metric length will be 4 octets. NEW> =C2=A0 =C2=A0The BGP SPF metric length is 4 octets. [major] "Algorithms such as setting the metric inversely to the link speed as done in the OSPF MIB [RFC4750] MAY be supported. =C2=A0However, this is beyond the scope of this document." Wait, what?!? =C2=A0Setting, or even defining, a metric is out of scope? Ok, let's go with that for a second. =C2=A0Is there any guidance or default settings that can be mentioned? =C2=A0For example, I found this in rfc2328: =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 A cost is associated with the out= put side of each router =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 interface. =C2=A0This cost is con= figurable by the system =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 administrator. =C2=A0The lower th= e cost, the more likely the =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 interface is to be used to forwar= d data traffic. =C2=A0Costs are =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 also associated with the external= ly derived routing data =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (e.g., the BGP-learned routes). The text should at least say that the cost is configurable before offering optional (MAY) implementations. What about consistency? =C2=A0Does it matter for the SPF calculation if the nodes don't consistently calculate the local metrics the same way? =C2=A0Is it ok for some nodes to use rfc4750, while others are manually configured with the inverse to that, just as an example? =C2=A0Clearly there will be an impact and the manually configured links will be less preferred. =C2=A0Can you provide operational guidance on what to consider when selecting and deploying an algorithm? I would really, really like to see a default mechanism defined. =C2=A0While I'm assuming that most deployments will be form the same vendor, it would be nice for this new protocol to be interoperable out of the box. 392 4.2.1. =C2=A0BGP-LS Link NLRI Attribute Prefix-Length TLVs 394 =C2=A0 Two BGP-LS Attribute TLVs to BGP-LS Link NLRI are defined to 395 =C2=A0 advertise the prefix length associated with the IPv4 and IPv6 l= ink 396 =C2=A0 prefixes. =C2=A0The prefix length is used for the optional inst= allation of 397 =C2=A0 prefixes corresponding to Link NLRI as defined in Section 5.3. [?] Where do the prefixes come from? [major] rfc7752 defines the IP Reachability Information TLV, which is mandatory and also includes the prefix length (and the prefix itself). Why isn't that one reused? 399 =C2=A0 =C2=A0 =C2=A0 0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 1 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 2 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 3 400 =C2=A0 =C2=A0 =C2=A0 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 401 =C2=A0 =C2=A0 =C2=A0+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-= +-+-+-+-+-+-+-+ 402 =C2=A0 =C2=A0 =C2=A0| =C2=A0 =C2=A0 =C2=A0TBD IPv4 or IPv6 Type =C2=A0= =C2=A0| =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Length =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0| 403 =C2=A0 =C2=A0 =C2=A0+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-= +-+-+-+-+-+-+-+ 404 =C2=A0 =C2=A0 =C2=A0| Prefix-Length | 405 =C2=A0 =C2=A0 =C2=A0+-+-+-+-+-+-+-+-+ [minor] Indicate the different Types separately: TBD4 and TBD6, for example= . 407 =C2=A0 =C2=A0 =C2=A0 =C2=A0Prefix-length - A one-octet length restrict= ed to 1-32 for IPv4 408 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0Link NLRI endpoint prefixes and 1-128 for IPv6 409 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0Link NLRI endpoint prefixes. [major] What should the receiver do if the Prefix-length is invalid? For example, if 44 is received for IPv4. 411 4.2.2. =C2=A0BGP-LS Link NLRI Attribute SPF Status TLV 413 =C2=A0 A BGP-LS Attribute TLV to BGP-LS Link NLRI is defined to indica= te the 414 =C2=A0 status of the link with respect to the BGP SPF calculation. =C2= =A0This 415 =C2=A0 will be used to expedite convergence for link failures as discu= ssed 416 =C2=A0 in Section 5.6.1. =C2=A0If the SPF Status TLV is not included w= ith the 417 =C2=A0 Link NLRI, the link is considered up and available. [major] What is the relationship between this TLV and the (other) SPF Status TLV from =C2=A74.1.2? =C2=A0For example, if this TLV is not included ("the link is considered up and available"), and the (other) SPF Status TLV indicates something else, which one has priority? ... 427 =C2=A0 =C2=A0 =C2=A0 BGP Status Values: 0 - Reserved 428 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A01 - Link Unreachable with respect to BGP SPF 429 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A02-254 - Undefined 430 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0255 - Reserved [major] What should the receiver do if an unknown value is received? 432 4.3. =C2=A0Prefix NLRI Usage [minor] rfc7752 defines 2 Prefix NLRI types, the IPv4 Topology Prefix NLRI and the IPv6 Topology Prefix NLRI. =C2=A0I'm assuming that by "Prefix NLRI" you mean both. =C2=A0Please write that somewhere. 434 =C2=A0 Prefix NLRI is advertised with a local node descriptor as descr= ibed 435 =C2=A0 above and the prefix and length used as the descriptors (TLV 26= 5) as 436 =C2=A0 described in [RFC7752]. =C2=A0The prefix metric attribute TLV (= TLV 1155) 437 =C2=A0 as well as any others required for non-SPF purposes SHOULD be 438 =C2=A0 advertised. =C2=A0For loopback prefixes, the metric should be 0= . =C2=A0For non- 439 =C2=A0 loopback prefixes, the setting of the metric is a local matter = and 440 =C2=A0 beyond the scope of this document. [minor] s/local node descriptor/Local Node Descriptor That's the name of the field as defined in rfc7752. =C2=A0Please be consistent with all names. OLD> =C2=A0 =C2=A0Prefix NLRI is advertised with a local node descriptor as desc= ribed =C2=A0 =C2=A0above and the prefix and length used as the descriptors (TLV 2= 65) as =C2=A0 =C2=A0described in [RFC7752]. NEW> =C2=A0 =C2=A0The Prefix NLRI is advertised with the Local Node Descriptor a= s described =C2=A0 =C2=A0above (Section 4). =C2=A0The Prefix Descriptors field includes= the IP =C2=A0 =C2=A0Reachability Information TLV (TLV 265) as described in [RFC775= 2]. [minor] s/The prefix metric attribute TLV/The Prefix Metric TLV [major] "TLV 1155...SHOULD be advertised" =C2=A0When is it ok to not advertise the metric? =C2=A0IOW, why isn't it required? [nit] s/any others/any other 442 4.3.1. =C2=A0BGP-LS Prefix NLRI Attribute SPF Status TLV [] Wow! =C2=A0This is the third SPF Status TLV! 444 =C2=A0 A BGP-LS Attribute TLV to BGP-LS Prefix NLRI is defined to indi= cate 445 =C2=A0 the status of the prefix with respect to the BGP SPF calculatio= n. 446 =C2=A0 This will be used to expedite convergence for prefix unreachabi= lity 447 =C2=A0 as discussed in Section 5.6.1. =C2=A0If the SPF Status TLV is n= ot included 448 =C2=A0 with the Prefix NLRI, the prefix is considered reachable. [major] What is the relationship of this TLV with the other SPF Status TLVs? =C2=A0Which one has priority? ... 458 =C2=A0 =C2=A0 =C2=A0 BGP Status Values: 0 - Reserved 459 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A01 - Prefix down with respect to SPF 460 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A02-254 - Undefined 461 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0255 - Reserved [minor] The other SPF Status TLVs use "Unreachable" instead of "down" to describe value 1. =C2=A0Is there a difference in the meaning? =C2=A0It w= ould be nice to be consistent. 463 4.4. =C2=A0BGP-LS Attribute Sequence-Number TLV 465 =C2=A0 A new BGP-LS Attribute TLV to BGP-LS NLRI types is defined to a= ssure 466 =C2=A0 the most recent version of a given NLRI is used in the SPF 467 =C2=A0 computation. =C2=A0The TBD TLV type will be defined by IANA. = =C2=A0The new BGP- 468 =C2=A0 LS Attribute TLV will contain an 8-octet sequence number. =C2= =A0The usage 469 =C2=A0 of the Sequence Number TLV is described in Section 5.1. [major] I assume that this TLV is mandatory for BGP SPF. =C2=A0Please say s= o. ... 483 =C2=A0 The 64-bit strictly increasing sequence number is incremented f= or 484 =C2=A0 every version of BGP-LS NLRI originated. =C2=A0BGP speakers imp= lementing 485 =C2=A0 this specification MUST use available mechanisms to preserve th= e 486 =C2=A0 sequence number's strictly increasing property for the deployed= life 487 =C2=A0 of the BGP speaker (including cold restarts). =C2=A0One mechani= sm for 488 =C2=A0 accomplishing this would be to use the high-order 32 bits of th= e 489 =C2=A0 sequence number as a wrap/boot count that is incremented anytim= e the 490 =C2=A0 BGP router loses its sequence number state or the low-order 32 = bits 491 =C2=A0 wrap. [major] "sequence number is incremented" =C2=A0Should this behavior be Normative? =C2=A0s/is/MUST ? [major] "for every version of BGP-LS NLRI originated" =C2=A0 There are 3 NLRI Types. If only one NLRI type is originated, should the Sequence Number change for the others? =C2=A0Does this imply that the other NLRIs need to be updated too? 493 =C2=A0 When incrementing the sequence number for each self-originated = NLRI, 494 =C2=A0 the sequence number should be treated as an unsigned 64-bit val= ue. 495 =C2=A0 If the lower-order 32-bit value wraps, the higher-order 32-bit = value 496 =C2=A0 should be incremented and saved in non-volatile storage. =C2=A0= If by some 497 =C2=A0 chance the BGP Speaker is deployed long enough that there is a 498 =C2=A0 possibility that the 64-bit sequence number may wrap or a BGP S= peaker 499 =C2=A0 completely loses its sequence number state (e.g., the BGP speak= er 500 =C2=A0 hardware is replaced or experiences a cold-start), the phase 1 501 =C2=A0 decision function (see Section 5.1) rules will insure convergen= ce, 502 =C2=A0 albeit, not immediately. [] "If by some chance the BGP Speaker is deployed long enough..." =C2=A0I love positive thinking. :-( 504 5. =C2=A0Decision Process with SPF Algorithm ... 516 =C2=A0 The SPF based Decision process replaces the BGP best-path Decis= ion 517 =C2=A0 process described in [RFC4271]. =C2=A0This process starts with = selecting 518 =C2=A0 only those Node NLRI whose SPF capability TLV matches with the = local 519 =C2=A0 BGP speaker's SPF capability TLV value. =C2=A0Since Link-State = NLRI always 520 =C2=A0 contains the local descriptor [RFC7752], it will only be origin= ated 521 =C2=A0 by a single BGP speaker in the BGP routing domain. =C2=A0These = selected 522 =C2=A0 Node NLRI and their Link/Prefix NLRI are used to build a direct= ed 523 =C2=A0 graph during the SPF computation. =C2=A0The best paths for BGP = prefixes 524 =C2=A0 are installed as a result of the SPF process. [minor] s/best-path Decision process/Decision Process rfc4271 doesn't use the term best-path. [nit] "best-path" is used later to describe the BGP SPF process. =C2=A0That is ok, but you may want to consider "best route" too. [minor] "contains the local descriptor [RFC7752]" =C2=A0I think you're pointing at rfc7752 because that's where the formats are defined. =C2=A0You should (also) point at =C2=A74 where the contents of the descriptor are explained. [minor] "...it will only be originated by a single BGP speaker in the BGP routing domain" =C2=A0 I had to read this several times. =C2=A0It sound= s as if you're trying to say that each set of NLRI is only originated by one node, is that it? =C2=A0If so, that is not true: multiple nodes (maybe rogue ones) may originate information that doesn't belong to them, even an exact copy of a different node (with or without the same Local Node Descriptor). =C2=A0I think a better/true statement could be something like: =C2=A0Because the Link-State NLRIs always contain a Local node Descriptor [], it could be used to identify the originating BGP speaker in the domain. [minor] "selected Node NLRI and their Link/Prefix NLRI" =C2=A0I assume you'll explain later how they're selected...a forward reference would be nice. 526 =C2=A0 When BGP-LS-SPF NLRI is received, all that is required is to 527 =C2=A0 determine whether it is the best-path by examining the Node-ID = and 528 =C2=A0 sequence number as described in Section 5.1. =C2=A0If the recei= ved best- 529 =C2=A0 path NLRI had changed, it will be advertised to other BGP-LS-SP= F 530 =C2=A0 peers. =C2=A0If the attributes have changed (other than the seq= uence 531 =C2=A0 number), a BGP SPF calculation will be scheduled. =C2=A0However= , a changed 532 =C2=A0 NLRI MAY be advertised to other peers almost immediately and 533 =C2=A0 propagation of changes can approach IGP convergence times. =C2= =A0To 534 =C2=A0 accomplish this, the MinRouteAdvertisementIntervalTimer and 535 =C2=A0 MinASOriginationIntervalTimer [RFC4271] are not applicable to t= he 536 =C2=A0 BGP-LS-SPF SAFI. =C2=A0Rather, SPF calculations SHOULD be trigg= ered and 537 =C2=A0 dampened consistent with the SPF back-off algorithm specified i= n 538 =C2=A0 [RFC8405]. [minor] s/When BGP-LS-SPF NLRI is received/When a BGP SPF UPDATE is receive= d [] BGP SPF and BGP-LS-SPF are used throughout for apparently the same thing. =C2=A0Please be consistent. [major] This paragraph sounds out of place to me. =C2=A0It seems to describe what to do if an UPDATE containing the current best-path is received...but you haven't explained the details yet. =C2=A0Perhaps move this to some place later. [] "...determine whether it is the best-path by examining the Node-ID and sequence number..." =C2=A0This phrase sounds incomplete/wrong, I'm sure =C2=A75.1 will provide more details. [minor] s/If the received best-path NLRI had changed/If the best-path chang= es [major] "BGP SPF calculation will be scheduled" =C2=A0When? =C2=A0s/will/MU= ST ?? =C2=A0Should there be SPF timers (basic ones) defined somewhere? [major] "MAY be advertised to other peers almost immediately" =C2=A0It sounds as if the optional part is not the advertisement, but doing it "almost immediately"; how fast is that? =C2=A0Also, is the advertisement optional? =C2=A0The action is what should carry the Normative language. [minor] "propagation of changes can approach IGP convergence times" This phrase sounds out of place: this document is a specification, not the place to compare this protocol with others. =C2=A0BTW, I'm assmuming you consider BGP SPF an IGP, right? [major] Given that these are timers in rfc4271: s/MinRouteAdvertisementIntervalTimer and MinASOriginationIntervalTimer [RFC4271] are not applicable/MinRouteAdvertisementIntervalTimer and MinASOriginationIntervalTimer [RFC4271] MUST be set to 0 [minor] s/SHOULD be triggered and dampened consistent with the SPF back-off algorithm/SHOULD be triggered and dampened according to the SPF back-off algorithm [major] "SHOULD...SPF back-off algorithm" =C2=A0Is rfc8405 recommended (and not required) because it is optional in a BGP SPF implementation, or because there are cases when it should not be used? =C2=A0If the former, then you should use MAY instead; if the latter, then please mention when it wouldn't be used. [] In the description of the SPF algorithm, it would be nice to point out where rfc8405 would be applicable. 540 =C2=A0 The Phase 3 decision function of the Decision Process [RFC4271]= is 541 =C2=A0 also simplified since under normal SPF operation, a BGP speaker= would 542 =C2=A0 advertise the NLRI selected for the SPF to all BGP peers with t= he 543 =C2=A0 BGP-LS/BGP-LS-SPF AFI/SAFI. =C2=A0Application of policy would n= ot be 544 =C2=A0 prevented however its usage to best-path process would be limit= ed as 545 =C2=A0 the SPF relies solely on link metrics. [major] This seems to be where you (try to) specify Phase 3. =C2=A0Please make it clear that is what you're doing. =C2=A0Note that =C2=A79.1.3/rfc427= 1 talks about more than which routes are advertised. [major] "BGP speaker would advertise" =C2=A0MUST?? [major] "advertise the NLRI selected for the SPF" =C2=A0Only that? =C2=A0= =C2=A75.1 seems to potentially eliminate some NLRI (I have questions about that there) -- if all the valid received information is not propagated then there is no synchronization, which is needed for a link-state protocol. =C2=A0What am I missing? [minor] s/BGP peers with the BGP-LS/BGP-LS-SPF AFI/SAFI /BGP SPF peers [minor] I'm not sure what you're trying to say in the last sentence. When is policy ok? =C2=A0What is the relationship between policy (which I interpret as filtering routes) and the metrics (beyond obviously not propagating anything)? 547 5.1. =C2=A0Phase-1 BGP NLRI Selection 549 =C2=A0 The rules for NLRI selection are greatly simplified from [RFC42= 71]. [] Are you simplifying or replacing. I think making it clear that =C2=A79.1.1/rfc4271 doesn't apply anymore is important. [major] Does this set of rules apply per NLRI? =C2=A0IOW, do they apply to = all NLRIs? 551 =C2=A0 1. =C2=A0If the NLRI is received from the BGP speaker originati= ng the NLRI 552 =C2=A0 =C2=A0 =C2=A0 (as determined by the comparing BGP Router ID in = the NLRI Node 553 =C2=A0 =C2=A0 =C2=A0 identifiers with the BGP speaker Router ID), then= it is preferred 554 =C2=A0 =C2=A0 =C2=A0 over the same NLRI from non-originators. =C2=A0Th= is rule will assure 555 =C2=A0 =C2=A0 =C2=A0 that stale NLRI is updated even if a BGP-LS route= r loses its 556 =C2=A0 =C2=A0 =C2=A0 sequence number state due to a cold-start. [minor] s/BGP Router ID/BGP Identifier/g That's the name used in rfc6286 [] OLD> =C2=A0 =C2=A0 =C2=A0 =C2=A0If the NLRI is received from the BGP speaker ori= ginating the NLRI =C2=A0 =C2=A0 =C2=A0 =C2=A0(as determined by the comparing BGP Router ID in= the NLRI Node =C2=A0 =C2=A0 =C2=A0 =C2=A0identifiers with the BGP speaker Router ID), the= n it is preferred =C2=A0 =C2=A0 =C2=A0 =C2=A0over the same NLRI from non-originators. NEW> =C2=A0 =C2=A0 =C2=A0 =C2=A0Routes originated by directly connected BGP SPF = peers are preferred. =C2=A0 =C2=A0 =C2=A0 =C2=A0This condition can be determined by comparing th= e BGP Identifiers in =C2=A0 =C2=A0 =C2=A0 =C2=A0the received Local Node Descriptor and OPEN mess= age. [major] In the case where a prefix is multihomed to more than one BGP SPF router... =C2=A0Let's say that one of those routers is directly connected, but the other one isn't, does this first rule mean that the receiver wouldn't take the advertisement (of a Link/Prefix NLRI) from the non-connected speaker into account? =C2=A0Even if the topology is such that the metric is <=3D? 558 =C2=A0 2. =C2=A0If the Sequence-Number TLV is present in the BGP-LS At= tribute, 559 =C2=A0 =C2=A0 =C2=A0 then the NLRI with the most recent, i.e., highest= sequence number 560 =C2=A0 =C2=A0 =C2=A0 is selected. =C2=A0BGP-LS NLRI with a Sequence-Nu= mber TLV will be 561 =C2=A0 =C2=A0 =C2=A0 considered more recent than NLRI without a BGP-LS= Attribute or a 562 =C2=A0 =C2=A0 =C2=A0 BGP-LS Attribute that doesn't include the Sequenc= e-Number TLV. [major] How can a route without the Sequence-Number TLV even be valid? 564 =C2=A0 3. =C2=A0The final tie-breaker is the NLRI from the BGP Speaker= with the 565 =C2=A0 =C2=A0 =C2=A0 numerically largest BGP Router ID. [] NEW> =C2=A0 =C2=A0The route received from the BGP SPF speaker with the numerical= ly larger BGP =C2=A0 =C2=A0Identifier is preferred. [major] Is this the BGP ID of the directly connected peer, or the originato= r? 567 =C2=A0 When a BGP speaker completely loses its sequence number state, = i.e., 568 =C2=A0 due to a cold start, or in the unlikely possibility that that 569 =C2=A0 sequence number wraps, the BGP routing domain will still conver= ge. 570 =C2=A0 This is due to the fact that BGP speakers adjacent to the route= r will 571 =C2=A0 always accept self-originated NLRI from the associated speaker = as 572 =C2=A0 more recent (rule # 1). =C2=A0When BGP speaker reestablishes a = connection 573 =C2=A0 with its peers, any existing session will be taken down and sta= le 574 =C2=A0 NLRI will be replaced by the new NLRI and stale NLRI will be 575 =C2=A0 discarded independent of whether or not BGP graceful restart is 576 =C2=A0 deployed, [RFC4724]. =C2=A0The adjacent BGP speaker will update= their NLRI 577 =C2=A0 advertisements in turn until the BGP routing domain has converg= ed. [nit] s/When BGP speaker/When a BGP SPF speaker [minor] "stale NLRI will be replaced by the new NLRI and stale NLRI will be discarded" =C2=A0 You mentioned 2 different actions for stale routes. [major] "whether or not BGP graceful restart is deployed, [RFC4724]" Does this mean that GR is not supported in BGP SPF? =C2=A0It may be clearer to simply say that. 579 =C2=A0 The modified SPF Decision Process performs an SPF calculation r= ooted 580 =C2=A0 at the BGP speaker using the metrics from Link and Prefix NLRI 581 =C2=A0 Attribute TLVs [RFC7752]. =C2=A0As a result, any attributes tha= t would 582 =C2=A0 influence the Decision process defined in [RFC4271] like ORIGIN= , 583 =C2=A0 MULTI_EXIT_DISC, and LOCAL_PREF attributes are ignored by the S= PF 584 =C2=A0 algorithm. =C2=A0Furthermore, the NEXT_HOP attribute value is p= reserved 585 =C2=A0 but otherwise ignored during the SPF or best-path. [minor] "metrics from Link and Prefix NLRI Attribute TLVs [RFC7752]" Maybe be more specific and point to the IGP metric attribute TLV (TLV 1095) and the prefix metric attribute TLV (TLV 1155). [major] "ORIGIN, MULTI_EXIT_DISC, and LOCAL_PREF attributes are ignored" Given that these attributes are not necessary for BGP SPF, can it be specified that they can optionally be omitted from a BGP SPF UPDATE? Optionally omitting would allow implementations to reuse more of their BGP code (and send them); but it would also allow some optimizations. And it doesn't eliminate them completely in case there's some use for them in the future. [major] What about the AS_PATH? [major] What about other attributes? =C2=A0Not all influence the BGP decision process -- do we need them? =C2=A0The more unnecessary stuff that is carried the more risk. [] Please remove the sentence about the NEXT_HOP and focus all discussion about it in =C2=A75.4. [major] rfc4760 says that if only multi-protocol extensions are used then an "UPDATE...SHOULD NOT carry the NEXT_HOP attribute". =C2=A0If this is one of the exceptions, please be explicit about it. [minor] "the SPF or best-path" =C2=A0I guess you meant simply the SPF calcu= lation. 587 5.2. =C2=A0Dual Stack Support 589 =C2=A0 The SPF-based decision process operates on Node, Link, and Pref= ix 590 =C2=A0 NLRIs that support both IPv4 and IPv6 addresses. =C2=A0Whether = to run a 591 =C2=A0 single SPF instance or multiple SPF instances for separate AFs = is a 592 =C2=A0 matter of a local implementation. =C2=A0Normally, IPv4 next-hop= s are 593 =C2=A0 calculated for IPv4 prefixes and IPv6 next-hops are calculated = for 594 =C2=A0 IPv6 prefixes. =C2=A0However, an interesting use-case is deploy= ment of 595 =C2=A0 [RFC5549] where IPv6 next-hops are calculated for both IPv4 and= IPv6 596 =C2=A0 prefixes. =C2=A0As stated in Section 1, support for Multiple To= pology 597 =C2=A0 Routing (MTR) is an area for future study. [nit] s/is deployment/is the deployment [minor] "interesting use-case is deployment of [RFC5549]" =C2=A0Is this case covered in the specification of the SPF? =C2=A0If not, then it may be better to not include this mention here...unless it's obvious and I missed it. [minor] As mentioned in =C2=A71, we don't need to mention MTR again if it's not supported. 599 5.3. =C2=A0SPF Calculation based on BGP-LS NLRI 601 =C2=A0 This section details the BGP-LS SPF local routing information b= ase 602 =C2=A0 (RIB) calculation. =C2=A0The router will use BGP-LS Node, Link,= and Prefix 603 =C2=A0 NLRI to populate the local RIB using the following algorithm. = =C2=A0This 604 =C2=A0 calculation yields the set of intra-area routes associated with= the 605 =C2=A0 BGP-LS domain. =C2=A0A router calculates the shortest-path tree= using 606 =C2=A0 itself as the root. =C2=A0Variations and optimizations of the a= lgorithm 607 =C2=A0 are valid as long as it yields the same set of routes. =C2=A0Th= e algorithm 608 =C2=A0 below supports Equal Cost Multi-Path (ECMP) routes. =C2=A0Weigh= ted Unequal 609 =C2=A0 Cost Multi-Path are out of scope. =C2=A0The organization of thi= s section 610 =C2=A0 owes heavily to section 16 of [RFC2328]. [minor] s/BGP-LS domain/BGP SPF domain [major] "Variations and optimizations of the algorithm are valid as long as it yields the same set of routes." =C2=A0This sounds as if the algorithm described here is not normative, just an example. =C2=A0I expect to see a default algorithm -- or at least one who's output describes what MUST happen. ... 615 =C2=A0 o =C2=A0Local Route Information Base (RIB) - This is abstract c= ontains 616 =C2=A0 =C2=A0 =C2=A0reachability information (i.e., next hops) for all= prefixes (both 617 =C2=A0 =C2=A0 =C2=A0IPv4 and IPv6) as well as the Node NLRI reachabili= ty. 618 =C2=A0 =C2=A0 =C2=A0Implementations may choose to implement this as se= parate RIBs for 619 =C2=A0 =C2=A0 =C2=A0each address family and/or Node NLRI. [nit] s/This is abstract contains/This abstract structure contains =C2=A0(?= ) [major] Is this the same as the Loc-RIB from rfc4271? =C2=A0There is at least one place later where the term is used. =C2=A0If possible, reuse terminology concepts. =C2=A0It would be ideal if a comparison of the terms also existed; for example it looks like the LSNDB is equivalent to the Adj-RIB-In. 621 =C2=A0 o =C2=A0Link State NLRI Database (LSNDB) - Database of BGP-LS N= LRI that 622 =C2=A0 =C2=A0 =C2=A0facilitates access to all Node, Link, and Prefix N= LRI as well as 623 =C2=A0 =C2=A0 =C2=A0all the Link and Prefix NLRI corresponding to a gi= ven Node NLRI. 624 =C2=A0 =C2=A0 =C2=A0Other optimization, such as, resolving bi-directio= nal connectivity 625 =C2=A0 =C2=A0 =C2=A0associations between Link NLRI are possible but of= scope of this 626 =C2=A0 =C2=A0 =C2=A0document. [minor] This sounds redundant: "access to all Node, Link, and Prefix NLRI as well as all the Link and Prefix NLRI corresponding to a given Node NLRI" [minor] "bi-directional connectivity...possible but of scope of this document." =C2=A0this is specified later in this section. 628 =C2=A0 o =C2=A0Candidate List - This is a list of candidate Node NLRI = with the 629 =C2=A0 =C2=A0 =C2=A0lowest cost Node NLRI at the front of the list. = =C2=A0It is typically 630 =C2=A0 =C2=A0 =C2=A0implemented as a heap but other concrete data stru= ctures have also 631 =C2=A0 =C2=A0 =C2=A0been used. [major] "list of candidate Node NLRI" =C2=A0Where does this list come from? =C2=A0Is it the outcome of the rules in =C2=A75.1? [major] "lowest cost" =C2=A0How is this cost calculated? ... 635 =C2=A0 1. =C2=A0The current local RIB is invalidated. =C2=A0The local = RIB is rebuilt 636 =C2=A0 =C2=A0 =C2=A0 during the course of the SPF computation. =C2=A0T= he existing routing 637 =C2=A0 =C2=A0 =C2=A0 entries are preserved for comparison to determine= changes that 638 =C2=A0 =C2=A0 =C2=A0 need to be installed in the global RIB. [major] Please define the "global RIB". [minor] "determine changes that need to be installed in" =C2=A0...or remove= d from ... 643 =C2=A0 3. =C2=A0The Node NLRI with the lowest cost is removed from the= candidate 644 =C2=A0 =C2=A0 =C2=A0 list for processing. =C2=A0If the BGP-LS Node att= ribute includes an 645 =C2=A0 =C2=A0 =C2=A0 SPF Status TLV (Section 4.1.2) indicating the nod= e is 646 =C2=A0 =C2=A0 =C2=A0 unreachable, the Node NLRI is ignored and the nex= t lowest cost 647 =C2=A0 =C2=A0 =C2=A0 Node NLRI is selected from candidate list. =C2=A0= The Node 648 =C2=A0 =C2=A0 =C2=A0 corresponding to this NLRI will be referred to as= the Current 649 =C2=A0 =C2=A0 =C2=A0 Node. =C2=A0If the candidate list is empty, the S= PF calculation has 650 =C2=A0 =C2=A0 =C2=A0 completed and the algorithm proceeds to step 6. [minor] "removed from the candidate list" =C2=A0When was that list initiali= zed? ... 662 =C2=A0 =C2=A0 =C2=A0 * =C2=A0If the prefix is in the local RIB and the= cost is greater than 663 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0the Current route's metric, the Pref= ix NLRI does not 664 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0contribute to the route and is ignor= ed. [major] I'm not sure if I'm reading this text correctly. =C2=A0It sounds like this: if the cost of the prefix in the local RIB is > the route being considered, then ignore the route being considered. =C2=A0 But it should be the other way around, right? As I read this step, and the next one... =C2=A0What is the "current route"? =C2=A0Is that the route in the global RIB? =C2=A0 Also, does "consider for installation" mean that the prefix is installed in the local RIB (but not the global RIB, yet)? =C2=A0If so, then I guess the text makes sense. But it also makes sense if you're covering the case where there's a prefix from another node in the local RIB, and the route being considered has a higher metric than the one in the global RIB, so it is ignored. =C2=A0Of course in this case there is no comparison with the router in the local RIB, but I have to then assume that it is there because the metric is <=3D to the global RIB... I'm just guessing... =C2=A0Please clarify. =C2=A0We don't want other people= guessing too. 666 =C2=A0 =C2=A0 =C2=A0 * =C2=A0If the prefix is in the local RIB and the= cost is less than 667 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0the current route's metric, the Pref= ix is installed with the 668 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Current Node's next-hops replacing t= he local RIB route's next- 669 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0hops and the metric being updated. [major] see comment above... [major] Where do the "Current Node's next-hops" come from? =C2=A0The text is missing how that part is calculated/advertised... 671 =C2=A0 =C2=A0 =C2=A0 * =C2=A0If the prefix is in the local RIB and the= cost is same as the 672 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0current route's metric, the Prefix i= s installed with the 673 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Current Node's next-hops being merge= d with local RIB route's 674 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0next-hops. [] see comment above... 676 =C2=A0 5. =C2=A0All the Link NLRI with the same Node Identifiers as th= e Current 677 =C2=A0 =C2=A0 =C2=A0 Node will be considered for installation. =C2=A0E= ach link will be 678 =C2=A0 =C2=A0 =C2=A0 examined and will be referred to in the following= text as the 679 =C2=A0 =C2=A0 =C2=A0 Current Link. =C2=A0The cost of the Current Link = is the advertised 680 =C2=A0 =C2=A0 =C2=A0 metric in the Link NLRI added to the cost to reac= h the Current 681 =C2=A0 =C2=A0 =C2=A0 Node. [] This text clarifies the meaning of "Current Link" upfront. :-) [nit] Some of the text capitalizes "Current Link" (Node, etc.), but it is not written the same way in other places. =C2=A0Please be consistent. 683 =C2=A0 =C2=A0 =C2=A0 * =C2=A0Optionally, the prefix(es) associated wit= h the Current Link 684 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0are installed into the local RIB usi= ng the same rules as were 685 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0used for Prefix NLRI in the previous= steps. [major] "Optionally" =C2=A0When would they be installed? =C2=A0When wouldn'= t they be installed? ... 692 =C2=A0 =C2=A0 =C2=A0 * =C2=A0The Current Link's endpoint Node NLRI is = accessed (i.e., the 693 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Node NLRI with the same Node identif= iers as the Link 694 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0endpoint). =C2=A0If it exists, it wi= ll be referred to as the 695 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Endpoint Node NLRI and the algorithm= will proceed as follows: [minor] "If it exists..." =C2=A0It *has* to exist! =C2=A0The start of step = 5 says that we're working with "the Link NLRI with the same Node Identifiers as the Current Node". =C2=A0I guess there could be a race condition where the Node NLRI is not available anymore...but I hope that SPF is run with the available information, and completed before processing changes. =C2=A0Right? =C2=A0If so, where is that mentioned? [major] After I wrote the last comment I kept reading and realized you're talking about the Node NLRI of the other end of the link. =C2=A0Be clear about the fact that the Node NLRI we're looking for is the one with a Local Node Descriptor equal to the Remote Node Descriptor in the Current Link. 697 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0+ =C2=A0If the BGP-LS Link NLRI attr= ibute includes an SPF Status 698 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 TLV indicating the link is d= own, the BGP-LS Link NLRI is 699 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 considered down and the next= BGP-LS Link NLRI is examined. [minor] For the Current Link, this can be checked after the Node status. [major] This same check needs to be done for the Endpoint Node, at the link and node level. 701 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0+ =C2=A0All the Link NLRI correspond= ing the Endpoint Node NLRI will 702 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 be searched for a back-link = NLRI pointing to the current 703 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 node. =C2=A0Both the Node id= entifiers and the Link endpoint 704 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 identifiers in the Endpoint = Node's Link NLRI must match for 705 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 a match. =C2=A0If there is n= o corresponding Link NLRI 706 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 corresponding to the Endpoin= t Node NLRI, the Endpoint Node 707 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 NLRI fails the bi-directiona= l connectivity test and is not 708 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 processed further. [major] "back-link NLRI" =C2=A0Please specify this further: what fields from where need to match? Suggestion> =C2=A0 =C2=A0 =C2=A0Each Link NLRI corresponding the Endpoint Node NLRI is = examined for a =C2=A0 =C2=A0 =C2=A0back-link pointing to the Current Node. =C2=A0Specifica= lly, the following =C2=A0 =C2=A0 =C2=A0values MUST be the same: the Remote Node Descriptor in = the Endpoint Node's =C2=A0 =C2=A0 =C2=A0Link NLRI and the Local Node Descriptor in the Current = Node's Link NLRI, =C2=A0 =C2=A0 =C2=A0as well as the corresponding remote and local IPv4/IPv6= addresses in the =C2=A0 =C2=A0 =C2=A0Link NLRIs. =C2=A0 If there is no corresponding... This text may still be confusing (specially the part about the IP addresses= )... 710 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0+ =C2=A0If the Endpoint Node NLRI is= not on the candidate list, it 711 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 is inserted based on the lin= k cost and BGP Identifier (the 712 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 latter being used as a tie-b= reaker). [minor] I'm not sure what the "based on the link cost and BGP Identifier (the latter being used as a tie-breaker)" tries to say. Judging from the next two steps, it sounds as if the node is put in the candidate list. =C2=A0Why wouldn't this node be on the candidate list already? =C2=A0(see related question where the Candidate List is defined) 714 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0+ =C2=A0If the Endpoint Node NLRI is= already on the candidate list 715 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 with a lower cost, it need n= ot be inserted again. [minor] "with a lower cost" than what? 717 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0+ =C2=A0If the Endpoint Node NLRI is= already on the candidate list 718 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 with a higher cost, it must = be removed and reinserted with 719 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 a lower cost. [minor] "with a higher cost" than what? [minor] "removed and reinserted" =C2=A0I'm not sure why this would happen. See related questions when the Candidate List is defined. 721 =C2=A0 =C2=A0 =C2=A0 * =C2=A0Return to step 3 to process the next lowe= st cost Node NLRI on 722 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0the candidate list. [nit] Should this be its own step? 724 =C2=A0 6. =C2=A0The local RIB is examined and changes (adds, deletes, 725 =C2=A0 =C2=A0 =C2=A0 modifications) are installed into the global RIB. [major] The local RIB started empty (Step 1), so all the changes are adds. =C2=A0Please be more specific about what is being compared and how the adds, deletes, modifications are determined. 727 5.4. =C2=A0NEXT_HOP Manipulation [minor] This sub-section doesn't seem to belong in the Decision Process sec= tion. [major] rfc4760 says this: =C2=A0 =C2=A0An UPDATE message that carries no NLRI, other than the one enc= oded in =C2=A0 =C2=A0the MP_REACH_NLRI attribute, SHOULD NOT carry the NEXT_HOP att= ribute. =C2=A0 =C2=A0If such a message contains the NEXT_HOP attribute, the BGP spe= aker =C2=A0 =C2=A0that receives the message SHOULD ignore this attribute. My reading of the rest of this section is that the text is trying to specify rules that are not consistent with rfc4760. =C2=A0The use of the NEXT_HOP to export BGP SPF information should already be covered in rfc7752. =C2=A0It seems to me that this section is not needed. 729 =C2=A0 A BGP speaker that supports SPF extensions MAY interact with pe= ers 730 =C2=A0 that don't support SPF extensions. =C2=A0If the BGP-LS address = family is 731 =C2=A0 advertised to a peer not supporting the SPF extensions describe= d 732 =C2=A0 herein, then the BGP speaker MUST conform to the NEXT_HOP rules 733 =C2=A0 specified in [RFC4271] when announcing the Link-State address f= amily 734 =C2=A0 routes to those peers. [major] "If the BGP-LS address family is advertised to a peer not supporting the SPF extensions..." =C2=A0Do you mean to say that the BGP SPF information can be exported using BGP-LS? =C2=A0I can see that. =C2=A0Howev= er, there are other things that need to be specified. =C2=A0For example, what Protocol-ID should be used? [major] "MUST conform to the NEXT_HOP rules specified in [RFC4271]" Which rules? =C2=A0Are you talking about =C2=A75.1.3? =C2=A0In any case, no= te that we're now talking about using BGP-LS to export information; the Normative language doesn't belong here as this should already be covered in rfc7752. 736 =C2=A0 All BGP peers that support SPF extensions would locally compute= the 737 =C2=A0 Loc-RIB next-hops as a result of the SPF process. =C2=A0Consequ= ently, the 738 =C2=A0 NEXT_HOP attribute is always ignored on receipt. =C2=A0However,= BGP 739 =C2=A0 speakers SHOULD set the NEXT_HOP address according to the NEXT_= HOP 740 =C2=A0 attribute rules specified in [RFC4271]. [major] "locally compute the Loc-RIB next-hops as a result of the SPF process" =C2=A0That's what I expected, but I don't see that part specified in the SPF description. [major] "SHOULD set the NEXT_HOP address according to...[RFC4271]" Why isn't this action required? =C2=A0When would rfc4271 not be followed? [major] Note that the text in =C2=A75.1 says that the "NEXT_HOP attribute value is preserved but otherwise ignored", but the text here recommends/requires a potential change. =C2=A0Should the NEXT_HOP attribute be ignored or used? =C2=A0It seems to me that this section is unnecessary. 742 5.5. =C2=A0IPv4/IPv6 Unicast Address Family Interaction 744 =C2=A0 While the BGP-LS SPF address family and the IPv4/IPv6 unicast a= ddress 745 =C2=A0 families install routes into the same device routing tables, th= ey 746 =C2=A0 will operate independently much the same as OSPF and IS-IS woul= d 747 =C2=A0 operate today (i.e., "Ships-in-the-Night" mode). =C2=A0There wi= ll be no 748 =C2=A0 implicit route redistribution between the BGP address families. 749 =C2=A0 However, implementation specific redistribution mechanisms SHOU= LD be 750 =C2=A0 made available with the restriction that redistribution of BGP-= LS SPF 751 =C2=A0 routes into the IPv4 address family applies only to IPv4 routes= and 752 =C2=A0 redistribution of BGP-LS SPF route into the IPv6 address family 753 =C2=A0 applies only to IPv6 routes. [nit] s/install routes/may install routes [minor] s/There will be no/There is no [major] "implementation specific redistribution mechanisms SHOULD be made available with the restriction..." =C2=A0I'm not sure if the SHOULD applies to mechanisms "made available" or the restriction. =C2=A0In either case, do we need this text? =C2=A0Given that redistribution happens from the RIB, only IPvx routes will go into the IPvx receiver -- IOW, this seems to be more a RIB function than one dependent on where the information came from. [major] You don't say anything about redistribution into BGP SPF. =C2=A0Is that ok? =C2=A0I don't think I saw the concept of an "external" route anywhere... 755 =C2=A0 Given the fact that SPF algorithms are based on the assumption = that 756 =C2=A0 all routers in the routing domain calculate the precisely the s= ame 757 =C2=A0 SPF tree and install the same set of routes, it is RECOMMENDED = that 758 =C2=A0 BGP-LS SPF IPv4/IPv6 routes be given priority by default when 759 =C2=A0 installed into their respective RIBs. =C2=A0In common implement= ations the 760 =C2=A0 prioritization is governed by route preference or administrativ= e 761 =C2=A0 distance with lower being more preferred. [nit] s/calculate the precisely/calculate precisely [nit] "precisely the same SPF tree and install the same set of routes" =C2=A0Given that each node calculates SPF from its own perspective, the result is not precisely the same...it is consistent, but not the same. Just a nit. [minor] s/BGP-LS SPF IPv4/IPv6 routes/BGP SPF routes [major] "RECOMMENDED...routes be given priority" =C2=A0When is it ok to not give higher priority? =C2=A0IOW, why is this not required? [major] "routes be given priority" =C2=A0Priority over what? =C2=A0Over oth= er BGP-related routes? =C2=A0Over other IGPs? =C2=A0Wouldn't all IGPs want the= same thing? 763 5.6. =C2=A0NLRI Advertisement and Convergence [minor] This section only talks about convergence, not advertisement. [major] Related to advertisement, when are the different NLRIs advertised? =C2=A0For Link NLRI, for example, I assume that changes in cost/metric would trigger a new UPDATE, right? =C2=A0What about the other NLRI? 765 5.6.1. =C2=A0Link/Prefix Failure Convergence 767 =C2=A0 A local failure will prevent a link from being used in the SPF 768 =C2=A0 calculation due to the IGP bi-directional connectivity requirem= ent. 769 =C2=A0 Consequently, local link failures should always be given priori= ty 770 =C2=A0 over updates (e.g., withdrawing all routes learned on a session= ) in 771 =C2=A0 order to ensure the highest priority propagation and optimal 772 =C2=A0 convergence. [] How should this type of failure be signaled? =C2=A0Should the Link/Prefix NLRI be Withdrawn or should a metric of "infinity" be used? =C2=A0Is there even a concept of "infinity"? [minor] "local link failures should always be given priority over updates" =C2=A0Should there be Normative language here somewhere? =C2=A0Thi= s is a change in behavior with respect to rfc4271 -- what if there are other AFI/SAFIs negotiated over the same link/session? 774 =C2=A0 An IGP such as OSPF [RFC2328] will stop using the link as soon = as the 775 =C2=A0 Router-LSA for one side of the link is received. =C2=A0With nor= mal BGP 776 =C2=A0 advertisement, the link would continue to be used until the las= t copy 777 =C2=A0 of the BGP-LS Link NLRI is withdrawn. =C2=A0In order to avoid t= his delay, 778 =C2=A0 the originator of the Link NLRI will advertise a more recent ve= rsion 779 =C2=A0 of the BGP-LS Link NLRI including the SPF Status TLV Section 4.= 2.2 780 =C2=A0 indicating the link is down with respect to BGP SPF. =C2=A0Afte= r some 781 =C2=A0 configurable period of time, e.g., 2-3 seconds, the BGP-LS Link= NLRI 782 =C2=A0 can be withdrawn with no consequence. =C2=A0If the link becomes= available 783 =C2=A0 in that period, the originator of the BGP-LS LINK NLRI will sim= ply 784 =C2=A0 advertise a more recent version of the BGP-LS Link NLRI without= the 785 =C2=A0 SPF Status TLV in the BGP-LS Link Attributes. [minor] "normal BGP...BGP-LS Link NLRI is withdrawn" =C2=A0 BGP-LS is not "normal" BGP, and the transport is opaque; IOW, the information is not used for routing. [major] s/ originator...will advertise / originator...MUST/SHOULD (?) adver= tise [major] "a more recent version" =C2=A0MUST/SHOULD increase the Sequence Num= ber... [nit] s/SPF Status TLV Section 4.2.2/SPF Status TLV (Section 4.2.2) [major] "configurable period of time, e.g., 2-3 seconds" =C2=A0 Please define a timer and set a default. 787 =C2=A0 Similarly, when a prefix becomes unreachable, a more recent ver= sion 788 =C2=A0 of the BGP-LS Prefix NLRI will be advertised with the SPF Statu= s TLV 789 =C2=A0 Section 4.3.1 indicating the prefix is unreachable in the BGP-L= S 790 =C2=A0 Prefix Attributes and the prefix will be considered unreachable= with 791 =C2=A0 respect to BGP SPF. =C2=A0After some configurable period of tim= e, e.g., 792 =C2=A0 2-3 seconds, the BGP-LS Prefix NLRI can be withdrawn with no 793 =C2=A0 consequence. =C2=A0If the prefix becomes reachable in that peri= od, the 794 =C2=A0 originator of the BGP-LS Prefix NLRI will simply advertise a mo= re 795 =C2=A0 recent version of the BGP-LS Prefix NLRI without the SPF Status= TLV 796 =C2=A0 in the BGP-LS Prefix Attributes. [] Same comments... 798 5.6.2. =C2=A0Node Failure Convergence 800 =C2=A0 With BGP without graceful restart [RFC4724], all the NLRI adver= tised 801 =C2=A0 by node are implicitly withdrawn when a session failure is dete= cted. 802 =C2=A0 If fast failure detection such as BFD is utilized, and the node= is on 803 =C2=A0 the fastest converging path, the most recent versions of BGP-LS= NLRI 804 =C2=A0 may be withdrawn while these versions are in-flight on longer p= aths. 805 =C2=A0 This will result the older version of the NLRI being used until= the 806 =C2=A0 new versions arrive and, potentially, unnecessary route flaps. 807 =C2=A0 Therefore, BGP-LS SPF NLRI SHOULD always be retained before bei= ng 808 =C2=A0 implicitly withdrawn for a brief configurable interval, e.g., 2= -3 809 =C2=A0 seconds. =C2=A0This will not delay convergence since the adjace= nt nodes 810 =C2=A0 will detect the link failure and advertise a more recent NLRI 811 =C2=A0 indicating the link is down with respect to BGP SPF Section 5.6= .1 and 812 =C2=A0 the BGP-SPF calculation will failure the bi-directional connect= ivity 813 =C2=A0 check. [nit] s/by node/by a node [minor] "the most recent versions of BGP-LS NLRI may be withdrawn while these versions are in-flight on longer paths" =C2=A0 =C2=A0I don't understand what this means -- the "while these versions" part is throwing me off. [major] "SHOULD always be retained" =C2=A0s/SHOULD always/SHOULD =C2=A0 Whe= n would it be ok to not retain? =C2=A0IOW, why is this not a requirement? [minor] "BGP-LS SPF NLRI SHOULD always be retained" =C2=A0I'm assuming you exclusively mean Node NLRIs, and not all, right? =C2=A0Also, I guess you mean to say that when the NLRI changes it should be retained... [minor] "brief configurable interval, e.g., 2-3 seconds" =C2=A0Can we use the same timer as before? [nit] s/BGP SPF Section 5.6.1/BGP SPF (Section 5.6.1) [nit] s/will failure/will fail 815 5.7. =C2=A0Error Handling 817 =C2=A0 When a BGP speaker receives a BGP Update containing a malformed= SPF 818 =C2=A0 Capability TLV in the Node NLRI BGP-LS Attribute [RFC7752], it = MUST 819 =C2=A0 ignore the received TLV and the Node NLRI and not pass it to ot= her 820 =C2=A0 BGP peers as specified in [RFC7606]. =C2=A0When discarding a No= de NLRI 821 =C2=A0 with malformed TLV, a BGP speaker SHOULD log an error for furth= er 822 =C2=A0 analysis. [major] "MUST...as specified in [RFC7606]" =C2=A0Where? =C2=A0Are you refer= ring to one of the error handling strategies? [major] What about the other TLVs? =C2=A0rfc7752 only covers malformed TLVs as ones with a bad length...but doesn't say anything about other values. =C2=A0That is on purpose because BGP-LS just carries information around. =C2=A0But that is not the case with BGP SPF. =C2=A0IOW, you have to specify what to do if the different fields are not as expected, when is a TLV considered malformed, what to do about it, etc.. 824 6. =C2=A0IANA Considerations 826 =C2=A0 This document defines an AFI/SAFI for BGP-LS SPF operation and 827 =C2=A0 requests IANA to assign the BGP-LS/BGP-LS-SPF (AFI 16388 / SAFI= TBD1) 828 =C2=A0 as described in [RFC4760]. [major] rfc4760 sets up the Registry, but it doesn't specify what IANA should do unless you're more explicit with the request. Suggestion> =C2=A0 =C2=A0This document defines the use of a new SAFI (TBD1) for BGP SPF= operation =C2=A0 =C2=A0(Section 3), and requests IANA to assign a value from the xxx = range in the =C2=A0 =C2=A0Subsequent Address Family Identifiers (SAFI) Parameters regist= ry. 830 =C2=A0 This document also defines five attribute TLVs for BGP-LS NLRI.= =C2=A0We 831 =C2=A0 request IANA to assign types for the SPF capability TLV, Sequen= ce 832 =C2=A0 Number TLV, IPv4 Link Prefix-Length TLV, IPv6 Link Prefix-Lengt= h TLV, 833 =C2=A0 and SPF Status TLV from the "BGP-LS Node Descriptor, Link Descr= iptor, 834 =C2=A0 Prefix Descriptor, and Attribute TLVs" Registry. [major] The draft defines 3 different SPF Status TLVs (all with the same format), but only one value is requested? =C2=A0Is that the intent? = =C2=A0I can see how the one TLV can be came to work across all 3 NLRIs, even with a common set of SPF Status values, but that is not explained anywhere. [minor] It would be very nice if a table was included to show the TLV types and the name. 836 7. =C2=A0Security Considerations 838 =C2=A0 This extension to BGP does not change the underlying security i= ssues 839 =C2=A0 inherent in the existing [RFC4271], [RFC4724], and [RFC7752]. [] Wow! =C2=A0No Security Considerations. :-( [major] Instead of claiming no new security issues, you should say that the considerations in those RFCs apply to BGP SPF. =C2=A0The considerations of course include recommendations that you may not have to include here. [major] Can a BGP SPF session be used with other AFI/SAFI pairs? =C2=A0It is not clear anywhere whether it can be. =C2=A0If yes, then the interaction related to the deployment models may need to be explained/considered. If not, then the question about authentication will definitely come up. [major] Please add pointers to the BGP Security Analysis documents; rfc4272 and rfc6952. [major] It is not clear whether rfc4724 is a requirement or not... [major] I believe that the Security Considerations from rfc7752 don't apply here at all. =C2=A0They discuss aspects specific to the inclusion of consumers in the deployment model -- consumers don't exist in BGP SPF. Also, link-state information is not exported (except for the brief hint in =C2=A75.4) so the risks mentioned there are not applicable either. [major] Related to rfc7752, the more important fact is that it considers all the BGP-LS information to be opaque, which means that the security considerations don't include anything related to what a rogue node can do inside the system. =C2=A0Some examples, a rogue node can (off the top of my head): - advertise someone else's Local Node Descriptor (prompting the real node to "fight back") - advertise a whole set of nodes "behind" it -- maybe more useful during a partition event - advertise phantom routers resulting in a trigger of SPF, potentially delaying convergence - indicate the wrong remote link address or Node Descriptor which can disrupt the topology - advertise links present on remote router as stub links (prefixes in this case), resulting in others thinking that the destinations are also (maybe only, if the metrics are properly crafted) reachable in a different place - send an incorrect SPF Status, forcing a link (or even a node) out of SPF consideration - the metric (and other information) can be changed in-transit, and other nodes would not be able to detect the difference...there's no checksum or other protection in the LSAs (the group of Node/Link/Prefix NLRI related to a node). - arbitrarily/selectively stop the propagation of information [Some of these are clearly related to the link-state nature of BGP SPF.] Note that there may not be mitigation for some of these rogue router behaviors. =C2=A0The use of the protocol in a limited domain may be the only possible countermeasure. =C2=A0But claiming that there are no new vulnerabilities is just wrong. [minor] rfc4593 could also be a good Informative reference about generic threats, including some of what I mention above. [major] Route origin validation (and BGPSec) don't apply to BGP SPF. Whatever door those mechanisms were meant to close are still open in BGP SPF. =C2=A0Specifically, there's no verification about the origination of prefixes: anyone can advertise a prefix --- with the proper metric it can result in erroneous traffic delivery. ... 846 8.1. =C2=A0Configuration 848 =C2=A0 In addition to configuration of the BGP-LS SPF address family, 849 =C2=A0 implementations SHOULD support the configuration of the 850 =C2=A0 INITIAL_SPF_DELAY, SHORT_SPF_DELAY, LONG_SPF_DELAY, TIME_TO_LEA= RN, 851 =C2=A0 and HOLDDOWN_INTERVAL as documented in [RFC8405]. [major] "SHOULD support the configuration of...[RFC8405]" =C2=A0rfc8405 says that "All the parameters MUST be configurable", the "SHOULD" here is in conflict. =C2=A0It is not necessary to specify support for that again here. 853 8.2. =C2=A0Operational Data 855 =C2=A0 In order to troubleshoot SPF issues, implementations SHOULD sup= port 856 =C2=A0 an SPF log including entries for previous SPF computations, Eac= h SPF 857 =C2=A0 log entry would include the BGP-LS NLRI SPF triggering the SPF,= SPF 858 =C2=A0 scheduled time, SPF start time, SPF end time, and SPF type if 859 =C2=A0 different types of SPF are supported. =C2=A0Since the size of t= he log will 860 =C2=A0 be finite, implementations SHOULD also maintain counters for th= e 861 =C2=A0 total number of SPF computations of each type and the total num= ber of 862 =C2=A0 SPF triggering events. =C2=A0Additionally, to troubleshoot SPF = scheduling 863 =C2=A0 and back-off [RFC8405], the current SPF back-off state, remaini= ng 864 =C2=A0 time-to-learn, remaining holddown, last trigger event time, las= t SPF 865 =C2=A0 time, and next SPF time should be available. [] "SPF computations of each type" =C2=A0There are multiple types?? ... 933 12.1. =C2=A0Normative References ... 966 =C2=A0 [RFC8402] =C2=A0Filsfils, C., Ed., Previdi, S., Ed., Ginsberg, = L., 967 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Decraene, B., Litkowsk= i, S., and R. Shakir, "Segment 968 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Routing Architecture",= RFC 8402, DOI 10.17487/RFC8402, 969 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0July 2018, . [major] This reference is used in error -- it is not needed. ... 977 12.2. =C2=A0Information References ... 1005 =C2=A0 [RFC4750] =C2=A0Joyal, D., Ed., Galecki, P., Ed., Giacalone, S= ., Ed., 1006 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Coltun, R., and F. Ba= ker, "OSPF Version 2 Management 1007 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Information Base", RF= C 4750, DOI 10.17487/RFC4750, 1008 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0December 2006, . [major] There is Normative language referring to this RFC, so the reference should be Normative. 1010 =C2=A0 [RFC4760] =C2=A0Bates, T., Chandra, R., Katz, D., and Y. Rekht= er, 1011 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"Multiprotocol Extens= ions for BGP-4", RFC 4760, 1012 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0DOI 10.17487/RFC4760,= January 2007, 1013 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0. [major] This reference should be Normative. 1015 =C2=A0 [RFC4790] =C2=A0Newman, C., Duerst, M., and A. Gulbrandsen, "I= nternet 1016 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Application Protocol = Collation Registry", RFC 4790, 1017 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0DOI 10.17487/RFC4790,= March 2007, 1018 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0. [major] This reference is used in error; it is not needed.