[Lsvr] Review of draft-ietf-lsvr-bgp-spf-26
Alvaro Retana <aretana.ietf@gmail.com> Thu, 13 July 2023 14:38 UTC
Return-Path: <aretana.ietf@gmail.com>
X-Original-To: lsvr@ietfa.amsl.com
Delivered-To: lsvr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 2186FC151B09; Thu, 13 Jul 2023 07:38:17 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: 2.902
X-Spam-Level: **
X-Spam-Status: No, score=2.902 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, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-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 ([50.223.129.194]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id cBhUsInhIH-2; Thu, 13 Jul 2023 07:38:12 -0700 (PDT)
Received: from mail-pf1-x42b.google.com (mail-pf1-x42b.google.com [IPv6:2607:f8b0:4864:20::42b]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 8FFA2C13738E; Thu, 13 Jul 2023 07:38:12 -0700 (PDT)
Received: by mail-pf1-x42b.google.com with SMTP id d2e1a72fcca58-6686c74183cso827103b3a.1; Thu, 13 Jul 2023 07:38:12 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1689259092; x=1691851092; h=content-transfer-encoding:cc:to:subject:message-id:date :mime-version:from:from:to:cc:subject:date:message-id:reply-to; bh=kj8cOfuzqBk2E58XKExF7yeDuvwkuxewNHjUVqK1xuI=; b=Vqb4KLPRMasuOfd5zB/qHA17lZrw6alpb7Zyp+Zdboyy7FUezOiRyVkiC37m1RmJuF zNqWMxpEjXEQZWIVt6kVW8stZtIXd0PSDPzj6Zi5S7D2TjblpvNUMUNu2BY4JhPAWfMf GmBBWMB/P7Cx7OfxGrhg0H6elW05KIVWihE56+6dwdL9HauB9Xi/m/sKXf1DocOVvFwK DmdXq9twOahiuSzePtJYC3A2ZvleMBqRQ9OSsDl3OyxTg4SNnNMD3SRmjfbHUKEXLE2a Togcrmn2YNC0V6I4o73Zt5D2cW4bqu82Ya0UgF1H3PmLRFStojmjvAThbcba6EJVJNMy qw3g==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689259092; x=1691851092; h=content-transfer-encoding:cc:to:subject:message-id:date :mime-version:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=kj8cOfuzqBk2E58XKExF7yeDuvwkuxewNHjUVqK1xuI=; b=gLn4iu9M3NJanieD395STirT9RPuQ0fwYWx6xvkomF2sAzarr5gOezlZMhsRea2mLn HXjw6m/87UORUF7XoDDB4ZLStl4pMbGI3BQuDay7JIASHLNkdFplBtHE8kva8fgyE27x 4n1c2dEBSraHpk98g7pHhUYw3U84rMydwqzsKWSJTCBPzh0uOhSHfQ/r5+FKDUVFzxxl lm3VTjCwM/doSe904fbVIgzW7kghgNp8NJyE8w7WG+dP+n6+n2iaRa7EfRX3n6CIh27x WP4A3F4i3Up7OAPe1S/BTQ4FOkCingL+FIbI9iy5kuYhD7n/hIs9AxoXRk/vwPnoG3mj V2cA==
X-Gm-Message-State: ABy/qLbhxMQauikuHhhpUln/Fy+f8o9d9UMKvYtrvo6aLu0Sba+8WyHD rmXJ11/MspoTEzeRaw3JODHEnCjK9MWdQjIRD7NMU5eaSmg=
X-Google-Smtp-Source: APBJJlHIYOoV6Jv6anJFKUesCTJsGSsdYQVhhzag2p48LtD0a6WfAs0c470xbv4EdlxC1I9/2Hhu/8u4WVAZudcv0cQ=
X-Received: by 2002:a17:903:2285:b0:1b8:5a49:a290 with SMTP id b5-20020a170903228500b001b85a49a290mr2215996plh.43.1689259091378; Thu, 13 Jul 2023 07:38:11 -0700 (PDT)
Received: from 1058052472880 named unknown by gmailapi.google.com with HTTPREST; Thu, 13 Jul 2023 07:38:10 -0700
From: Alvaro Retana <aretana.ietf@gmail.com>
MIME-Version: 1.0
Date: Thu, 13 Jul 2023 07:38:10 -0700
Message-ID: <CAMMESsxTCrsdd9sP6pWO_1mNugTsGUC2sPz3tSK_631zQYcR9g@mail.gmail.com>
To: Acee Lindem <acee.ietf@gmail.com>
Cc: draft-ietf-lsvr-bgp-spf@ietf.org, James Guichard <james.n.guichard@futurewei.com>, Victor Kuarsingh <victor@jvknet.com>, "lsvr@ietf.org" <lsvr@ietf.org>, lsvr-chairs@ietf.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://mailarchive.ietf.org/arch/msg/lsvr/53qx9ERNrHek1U4OphEHQJrhT_o>
Subject: [Lsvr] Review of draft-ietf-lsvr-bgp-spf-26
X-BeenThere: lsvr@ietf.org
X-Mailman-Version: 2.1.39
Precedence: list
List-Id: Link State Vector Routing <lsvr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/lsvr>, <mailto:lsvr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/lsvr/>
List-Post: <mailto:lsvr@ietf.org>
List-Help: <mailto:lsvr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/lsvr>, <mailto:lsvr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 13 Jul 2023 14:38:17 -0000
Acee: Hi! Jim asked me to look at the latest version because I had done the original AD review for this document. I looked at the diffs between -19 and -26. I have some minor comments and nits -- see them inline below (based on -26). I do want to highlight a couple of points: (1) Synchronization There are several errors that result in loosing the synchronization, and not being able to recover it. Most of them have to do with discarding the BGP-LS Attribute and using treat-as-withdraw (which I think are the correct actions). The problem can't be avoided, but it should be mentioned as a potential issue. I put a couple of suggestions in §5.1.2 (line 458). (2) Next Hop The next-hop-related information from BGP is not used because BGP SPF nodes "locally compute the Local-RIB Next-Hop as a result of the SPF process" (§5.3). However, text in §3 and §5.3 try to (confusingly) specify the behavior. In short, §5.3 is not needed, and §3 can simply point at rfc7752bis. See the comments in §5.3 (line 765). (3) Semantic Validation I think the "hack" to point at the original IGP documents works! ;-) There are some missing references (look for the comments around lines 1256 and 1265) as well as making sure that the non-IGP-originated TLVs (like the ones defined in rfc9086) are covered. I will leave it to Jim to decide if the document is ready for IETF LC. Thanks! Alvaro. [Line numbers from idnits.] ... 260 2. Base BGP Protocol Relationship 262 With the exception of the decision process, the BGP SPF extensions 263 leverage the BGP protocol [RFC4271] without change. This includes 264 the BGP protocol Finite State Machine, BGP messages and their 265 encodings, processing of BGP messages, BGP attributes and path 266 attributes, BGP NLRI encodings, and any error handling defined in the 267 [RFC4271] and [RFC7606]. [nit] s/defined in the/defined in 269 Due to the changes to the decision process, there are mechanisms and 270 encodings that are no longer applicable. While not necessarily 271 required for computation, the ORIGIN, AS_PATH, LOCAL_PREF (IBGP), and 272 NEXT_HOP path attributes are mandatory [RFC4271] and are validated. 273 Unless explicitly specified in the context of BGP SPF, all other 274 attributes SHOULD NOT be advertised. However, if they are 275 advertised, they will be accepted, validated, and propagated 276 consistent with the BGP protocol. [minor] Suggestion> OLD> While not necessarily required for computation, the ORIGIN, AS_PATH, LOCAL_PREF (IBGP), and NEXT_HOP path attributes are mandatory [RFC4271] and are validated. Unless explicitly specified in the context of BGP SPF, all other attributes SHOULD NOT be advertised. However, if they are advertised, they will be accepted, validated, and propagated consistent with the BGP protocol. NEW> Unless explicitly specified in the context of BGP SPF, all optional path attributes SHOULD NOT be advertised. If received, all path attributes MUST be accepted, validated, and propagated consistent with the BGP protocol [rfc4271], even if not needed by BGP SPF. ... 302 3. BGP Link-State (BGP-LS) Relationship ... 320 The rules for setting the NLRI next-hop path attribute for the BGP- 321 LS-SPF SAFI follow the BGP-LS SAFI as specified in section 3.4 of 322 [I-D.ietf-idr-rfc7752bis]. [major] §5.3 in this document also talks about the next hop, and the specification there is not the same as what it is here. Please specify things only once -- see my comments there (search for line 765). ... 421 5.1.1. BGP-LS-SPF NLRI TLVs ... 427 The NLRI and conprising TLVs MUST be processed as specified in 428 section 5.1 [I-D.ietf-idr-rfc7752bis]. TLVs specified as mandatory 429 in [I-D.ietf-idr-rfc7752bis] are considered mandatory for the BGP-LS- 430 SPF SAFI as well. If a mandatory TLV is not specified, the NLRI is 431 not used in the BGP SPF route calculation. All the other TLVs are 432 considered as an optional TLVs. [minor] "If a mandatory TLV is not specified, the NLRI is not used in the BGP SPF route calculation." Did you mean "not present" (instead of "not specified")? Can we make this behavior normative: MUST NOT be used in the BGP SPF route calculation ? 434 5.1.2. BGP-LS Attribute 436 The BGP-LS attribute of the BGP-LS-SPF SAFI uses exactly same format 437 of the BGP-LS AFI [I-D.ietf-idr-rfc7752bis]. In other words, all the 438 TLVs used in BGP-LS attribute of the BGP-LS AFI are applicable and 439 used for the BGP-LS attribute of the BGP-LS-SPF SAFI. This attribute 440 is an optional, non-transitive BGP attribute that is used to carry 441 link, node, and prefix properties and attributes. The BGP-LS 442 attribute is a set of TLVs. [minor] To be consistent with the previous section: NEW> All the TLVs defined for the BGP-LS Attribute [rfc7752bis] are applicable and can be used with the BGP-LS-SPF SAFI to carry link, node, and prefix properties and attributes. 444 The BGP-LS attribute may potentially grow large in size depending on 445 the amount of link-state information associated with a single Link- 446 State NLRI. The BGP specification [RFC4271] mandates a maximum BGP 447 message size of 4096 octets. It is RECOMMENDED that an 448 implementation support [RFC8654] in order to accommodate larger size 449 of information within the BGP-LS Attribute. BGP SPF speakers MUST 450 ensure that they limit the TLVs included in the BGP-LS Attribute to 451 ensure that a BGP update message for a single Link-State NLRI does 452 not cross the maximum limit for a BGP message. The determination of 453 the types of TLVs to be included by the BGP SPF speaker originating 454 the attribute is outside the scope of this document. When a BGP SPF 455 speaker finds that it is exceeding the maximum BGP message size due 456 to addition or update of some other BGP Attribute (e.g., AS_PATH), it 457 MUST consider the BGP-LS Attribute to be malformed and the attribute 458 discard handling of [RFC7606] applies. [major] I made these comments before: ----- [major] As above, most of this text is a copy from rfc7752bis. [major] "MUST consider the BGP-LS Attribute to be malformed and the attribute discard handling of [RFC7606] applies." In BGP SPF this is a serious problem because it results in loss of synchronization: after a router discards the attribute all the other routers won't have the same information as the ones before the discard. §7.3 has some text (also copied from rfc7752bis) that tries to say that it is ok to discard the BGP-LS Attribute because the remains can be used for troubleshooting. Even if that was true (see my comments there), the lack of synchronization remains. Can this issue be mitigated? ----- The reply was: "If the BGP-LS attribute is malformed, there really isn't anything that can be done." I agree. :-( Some comments: (1) The text above is still a copy from rfc7752bis. It's not needed unless it is specifying something different. (2) Is there an opportunity to consider "some other BGP Attribute" as malformed and drop that one instead? Obviously the AS_PATH is not it... Just thinking out loud... (3) Following the specification above/rfc7752bis results in the UPDATE being propagated, but without the BGP-LS Attribute. Among other things, the Sequence-Number TLV won't be received and §5.2.4 says: If the Sequence-Number TLV is not received, then the corresponding NLRI is considered as malformed and MUST be handled as 'Treat-as- withdraw'. An implementation MAY log an error for further analysis. This text addresses the issue of the downstream nodes trying to use the incomplete information. Can the optional logging be changed to a recommendation or a requirement (SHOULD/MUST)? But the loss of synchronization is still an issue as the view of the topology will be incomplete (in the downstream nodes). Please add some text related to the potential effect in the network -- maybe in the Management Considerations section. Note that this issues comes up whenever a node uses "treat-as-withdraw" (as in §7.1). (4) Support for rfc8654 is RECOMMENDED in rfc7752bis. Consider making it REQUIRED for BGP SPF. 460 5.2. Extensions to BGP-LS ... 473 The protocol identifier specified in the Protocol-ID field 474 [I-D.ietf-idr-rfc7752bis] represents the origin of the advertised 475 NLRI. For Node NLRI and Link NLRI, this MUST be the direct protocol 476 (4). Node or Link NLRI with a Protocol-ID other than the direct 477 protocol is considered malformed. For Prefix NLRI, the specified 478 Protocol-ID MUST be the origin of the prefix. The local and remote 479 node descriptors for all NLRI MUST include the BGP Identifier (TLV 480 516) and the AS Number (TLV 512) [I-D.ietf-idr-rfc7752bis]. The BGP 481 Confederation Member (TLV 517) [I-D.ietf-idr-rfc7752bis] is currently 482 not applicable. [minor] s/(TLV 516)/(TLV 516) [RFC9086] The reference should be normative. [minor] s/(TLV 517) [I-D.ietf-idr-rfc7752bis]/(TLV 517) [RFC9086] ... 520 5.2.1.2. BGP-LS-SPF Node NLRI Attribute SPF Status TLV ... 551 If a BGP SPF speaker received the Node NLRI but the SPF Status TLV is 552 not received, then any previously received information is considered 553 as implicitly withdrawn and the update is propagated to other BGP SPF 554 speakers. A BGP SPF speaker receiving a BGP Update containing a SPF 555 Status TLV in the BGP-LS attribute [I-D.ietf-idr-rfc7752bis] with a 556 value that is undefined values SHOULD be advertised to other BGP SPF 557 speakers. However, a BGP SPF speaker MUST NOT use the Status TLV in 558 its SPF computation. An implementation MAY log this condition for 559 further analysis. [nit] s/value that is undefined values/value that is undefined 561 If the BGP-LS-SPF Status TLV is advertised and the advertised value 562 is not defined, then the SPF Status TLV is ignored and not used in 563 SPF computation but is still advertised to other BGP SPF speakers. 564 An implementation MAY log an error for further analysis. [] This (new) paragraph (which is also in §5.2.2.2) repeats the normative statements in the previous one. It is not needed. 566 5.2.2. Link NLRI Usage ... 571 Link NLRI is advertised with unique local and remote node descriptors 572 dependent on the IP addressing. For IPv4 links, the link's local 573 IPv4 (TLV 259) and remote IPv4 (TLV 260) addresses are used. For 574 IPv6 links, the local IPv6 (TLV 261) and remote IPv6 (TLV 262) 575 addresses are used. For unnumbered links, the link local/remote 576 identifiers (TLV 258) are used. For links supporting having both 577 IPv4 and IPv6 addresses, both sets of descriptors MAY be included in 578 the same Link NLRI. The link descriptors are described in table 4 of 579 [I-D.ietf-idr-rfc7752bis]. [major] "For unnumbered links, the link local/remote identifiers (TLV 258) are used." §1.5/rfc5307 says this about the link identifiers in this TLV: Link Identifiers are exchanged in the Extended Local Circuit ID field of the "Point-to-Point Three-Way Adjacency" IS-IS Option type [ISIS-3way]. IS-IS uses Hellos to figure out the IDs. But there is no 3-way handshake, or Hellos in BGP SPF. How are these values learned? ... 601 5.2.2.1. BGP-LS-SPF Link NLRI Attribute Prefix-Length TLVs ... 626 The maximum prefix-length is 32 bits for an IPv4 Prefix-Length TLV 627 and 128 bits for an IPv6 Prefix-Length TLV. A prefix-length field 628 indicating a larger value is in error and the the corresponding Link 629 NLRI is considered as malformed and MUST be handled as 'Treat-as- 630 withdraw' [RFC7606]. An implementation MAY log. An implementation 631 MAY log an error for further analysis. [nit] s/An implementation MAY log. An/An ... 765 5.3. NEXT_HOP Attribute Manipulation [major] I still don't feel comfortable with this section, including this text from §3: The rules for setting the NLRI next-hop path attribute for the BGP- LS-SPF SAFI follow the BGP-LS SAFI as specified in section 3.4 of [I-D.ietf-idr-rfc7752bis]. Let's start here: the next hop information carried in BGP SPF exists in the "Network Address of Next Hop" field (aka "next hop information" [rfc4760], but "next hop address" is ok too [rfc7752bis]) of the MP_REACH_NLRI attribute [rfc4760], not in the NEXT_HOP attribute. IOW, the text in §3 is wrong because the NEXT_HOP attribute is not used...and ignored when using MP_REACH_NLRI [rfc4760]. §5.5 (not §3.4) of rfc7752bis ("BGP Next-Hop Information") does talk about how BGP-LS expects the value of the next hop information to be set. Suggestion> The rules for setting the next hop information for the BGP-LS-SPF SAFI follow the specification in section 5.5 of [I-D.ietf-idr-rfc7752bis]. 767 All BGP peers that support SPF extensions will locally compute the 768 Local-RIB Next-Hop as a result of the SPF process. Consequently, the 769 Next Hop Attribute is always ignored on receipt. The Next Hop 770 address MUST be encoded as described in [RFC4760]. BGP SPF speakers 771 MUST interpret the Next Hop address of MP_REACH_NLRI attribute as an 772 IPv4 address whenever the length of the Next Hop address is 4 octets, 773 and as a IPv6 address whenever the length of the Next Hop address is 774 16 octets. The first sentence is ok. The second one is not needed because the NEXT_HOP attribute is already ignored according to rfc4760. No need to respecify it here. The third and fourth are already covered in rfc7752bis. 776 [RFC4760] modifies the rules of NEXT_HOP attribute whenever the 777 multiprotocol extensions for BGP-4 are enabled. BGP SPF speakers 778 MUST set the NEXT_HOP attribute according to the rules specified in 779 [RFC4760] as the BGP-LS-SPF routing information is carried within the 780 multiprotocol extensions for BGP-4. First sentence: rfc4760 doesn't modify the rules of the NEXT_HOP attribute, it in fact uses them to set the next hop information in the MP_REACH_NLRI attribute. While the second sentence is true, it is the same thing that rfc7752bis says and is covered by the proposed statement in §3 (above). In summary, only the proposed sentence in §3 is needed (+ "All BGP peers that support SPF extensions will locally compute the Local-RIB Next-Hop as a result of the SPF process.", which can also go in §3). This section can be removed. ... 910 6.3. SPF Calculation based on BGP-LS-SPF NLRI ... 1006 * If the Current-Prefix's corresponding prefix is in the Local- 1007 RIB and the cost is the same as the Local-RIB route's metric, 1008 the Current-Node's next-hops are merged with Local-RIB route's 1009 next-hops. The algorithm below supports Equal Cost Multi-Path 1010 (ECMP) routes. Some platforms or implementations may have 1011 limits on the number of ECMP routes that can be supported. 1012 The setting or identification of any limitations is outside 1013 the scope if this document. Nonetheless, step 4 (below) 1014 includes a set of recommendations in case such as limit is 1015 encountered. Weighted Unequal Cost Multi-Path routes are out 1016 of scope as well. [nit] s/such as limit/such a limit ... 1200 7.1. Processing of BGP-LS-SPF TLVs ... 1244 When a BGP SPF speaker receives a BGP Update containing any malformed 1245 BGP-LS Attribute TE and IGP Metric TLV, then the spreaker MUST drop 1246 an entire BGP-LS Attribute. In such cases, a BGP SPF speaker MUST 1247 NOT pass the dropped BGP-LS Attribute to other BGP peers as specified 1248 in [RFC7606]. The corresponding NLRI is considered as a malformed 1249 and MUST be handled as 'Treat-as-withdraw'. An implementation SHOULD 1250 log an error (subject to rate-limiting) for further analysis. [major] "MUST drop an entire BGP-LS Attribute" The text also says that the "Link NLRI is considered as malformed and MUST be handled as 'Treat-as-withdraw'". This means that it doesn't matter what happens to the attribute. Is there a reason to keep that text? 1252 The BGP-LS Attribute consists of Node attribute TLVs, Link attribute 1253 TLVs, and the Prefix attribute TLVs. Node attribute TLVs and their 1254 error handling rules are either defined in [I-D.ietf-idr-rfc7752bis] 1255 or derived from xref target="RFC5305" format="default"/> and xref 1256 target="RFC6119" format="default"/>. [nit] You missed the opening "<" in "xref". [major] I think that what is missing is an explicit indication of the action to be taken. Let's say that a TLV is malformed according to rfc5305, what should BGP SPF do? I'm assuming the answer is the same as the other actions (treat-as-withdraw), but that should be explicit. [] The same comments apply to the next two paragraphs. 1258 Link Attribute TLVs and their error handling rules are either defined 1259 in [I-D.ietf-idr-rfc7752bis] or derived from xref target="RFC5305" 1260 format="default"/> and xref target="RFC6119" format="default"/>. 1262 Prefix Attribute TLVs and their error handling rules are either 1263 defined in [I-D.ietf-idr-rfc7752bis] or derived from xref 1264 target="RFC5130" format="default"/> and xref target="RFC2328" 1265 format="default"/>. [major] What about the validation of the Node/Link/Prefix Descriptor TLVs in the NLRIs? Please include that too. ... 1273 7.2. Processing of BGP-LS-SPF NLRIs 1275 A Link-State NLRI MUST NOT be considered as malformed or invalid 1276 based on the inclusion/exclusion of TLVs or contents of the TLV 1277 fields (i.e., semantic errors), as described in Section 5.1 and 1278 Section 5.1.1. [major] This first paragraph doesn't apply anymore: as specified in the last section, the TLVs are considered malformed... Also, there are requirements earlier about the inclusion of some TLVs. 1280 A BGP-LS-SPF Speaker MUST perform the following syntactic validation 1281 of the BGP-LS-SPF NLRI to determine if it is malformed. 1283 1. Does the sum of all TLVs found in the BGP MP_REACH_NLRI attribute 1284 correspond to the BGP MP_REACH_NLRI length? 1286 2. Does the sum of all TLVs found in the BGP MP_UNREACH_NLRI 1287 attribute correspond to the BGP MP_UNREACH_NLRI length? 1289 3. Does the sum of all TLVs found in a BGP-LS-SPF NLRI correspond to 1290 the Total NLRI Length field of all its Descriptors? 1292 4. When an NLRI TLV is recognized, is the length of the TLV and its 1293 sub-TLVs valid? 1295 5. Has the syntactic correctness of the NLRI fields been verified as 1296 per [RFC7606]? 1298 6. Has the rule regarding ordering of TLVs been followed as 1299 described in Section 5.1.1? [major] These checks were copied from (an old version of) rfc7752bis. The updated text in §8.2.2 is not phrased in the form of a question. Please reference that. Suggestion> A BGP-LS-SPF Speaker MUST perform the syntactic validation checks of the BGP-LS-SPF NLRI listed in Section 8.2.2 of [rfc7752bis] to determine if it is malformed. ... 1309 7.3. Processing of BGP-LS Attribute 1311 A BGP-LS Attribute MUST NOT be considered as malformed or invalid 1312 based on the inclusion/exclusion of TLVs or contents of the TLV 1313 fields (i.e., semantic errors), as described in Section 5.1 and 1314 Section 5.1.1. [major] Same comment as above. 1316 A BGP-LS-SPF Speaker MUST perform the following syntactic validation 1317 of the BGP-LS Attribute to determine if it is malformed. 1319 1. Does the sum of all TLVs found in the BGP-LS-SPF Attribute 1320 correspond to the BGP-LS Attribute length? 1322 2. Has the syntactic correctness of the Attributes (including BGP-LS 1323 Attribute) been verified as per [RFC7606]? 1325 3. Is the length of each TLV and, when the TLV is recognized then, 1326 its sub-TLVs in the BGP-LS Attribute valid? [] Same observation... Suggestion> A BGP-LS-SPF Speaker MUST perform the syntactic validation checks of the BGP-LS Attribute listed in Section 8.2.2 of [rfc7752bis] to determine if it is malformed. ... 1331 8. IANA Considerations ... 1340 8.2. BGP-LS-SPF Assignments to BGP-LS NLRI and Attribute TLV Registry 1342 IANA has assigned five TLVs for BGP-LS-SPF NLRI in the "BGP-LS NLRI 1343 and Attribute TLV" registry. These TLV types include the SPF 1344 capability TLV, Sequence Number TLV, IPv4 Link Prefix-Length TLV, 1345 IPv6 Link Prefix-Length TLV, and SPF Status TLV. 1347 +==========+===============+===================================+ 1348 | TLV Code | Description | Reference | 1349 | Point | | | 1350 +==========+===============+===================================+ 1351 | 1180 | SPF | Section 5.2.1.1 | 1352 | | Capability | | 1353 +----------+---------------+-----------------------------------+ 1354 | 1184 | SPF Status | Section 5.2.1.2, Section 5.2.2.2, | 1355 | | | and Section 5.2.3.1 | 1356 +----------+---------------+-----------------------------------+ 1357 | 1182 | IPv4 Link | Section 5.2.2.1 | 1358 | | Prefix Length | | 1359 +----------+---------------+-----------------------------------+ 1360 | 1183 | IPv6 Link | Section 5.2.2.1 | 1361 | | Prefix Length | | 1362 +----------+---------------+-----------------------------------+ 1363 | 1181 | Sequence | Section 5.2.4 | 1364 | | Number | | 1365 +----------+---------------+-----------------------------------+ 1367 Table 1: NLRI Attribute TLVs [major] The references should be to RFCXXX ([this document]), not the sections. The registry contains a mixture of references to RFCs and RFC+Section -- I'll leave the decision of which format to use to you. 1369 8.3. BGP-LS-SPF Node NLRI Attribute SPF Status TLV Status Registry 1371 IANA is requested to create the "BGP-LS-SPF Node NLRI Attribute SPF 1372 Status TLV Status" Registry for status values. The allocation of the 1373 unsigned 8-bit status are defined in the table below: [major] Where should this registry (and the ones created in §8.4 and §8.5) be located? IMO, a new group should be created for BGP SPF -- the alternative of locating them in the BGP-LS or BGP group doesn't seem right to me. Suggestion> IANA is requested to create the "BGP-LS-SPF Node NLRI Attribute SPF Status TLV Status" Registry for status values in a new BGP SPF group. Initial values for this registry are provided below. Future assignments are to be made using the IETF Review registration policy [rfc8126]. 1375 +=======+==========================================+ 1376 | Range | Assignment Policy | 1377 +=======+==========================================+ 1378 | 0 | Reserved (Not to be assigned) | 1379 +-------+------------------------------------------+ 1380 | 1 | Node unreachable with respect to BGP SPF | 1381 +-------+------------------------------------------+ 1382 | 2 | Node does not supprot transit traffic | 1383 | | with respect to BGP SPF | 1384 +-------+------------------------------------------+ 1385 | 3-254 | Unassigned (IETF Review) | 1386 +-------+------------------------------------------+ 1387 | 255 | Reserved (Not to be assigned) | 1388 +-------+------------------------------------------+ 1390 Table 2: BGP-LS-SPF Node NLRI Attribute SPF 1391 Status TLV Status Registry Assignments [major] This table mixes the registration policy with the assigned values. Given that only IETF Review is used for future assignments, there's no need to indicate the policy for unassigned values -- only the assignments made in this document are needed: Value Description 0 Reserved 1 Node unreachable with respect to BGP SPF 2 Node does not supprot transit traffic with respect to BGP SPF 3-255 Unassigned 255 Reserved [] Please apply the same suggestions to the other two sections. ... 1435 9. Security Considerations ... 1445 As the modifications described in this document for BGP SPF apply to 1446 IPv4 Unicast and IPv6 Unicast as underlay SAFIs in a single BGP SPF 1447 Routing Domain, the BGP security solutions described in [RFC6811] and 1448 [RFC8205] are somewhat out of scope as they are meant to apply for 1449 inter-domain BGP where multiple BGP Routing Domains are typically 1450 involved. The BGP-LS-SPF SAFI NLRI described in this document are 1451 typically advertised between EBGP or IBGP speakers under a single 1452 administrative domain. [nit] s/somewhat out of scope/out of scope ... 1486 10.2. SPF Algorithm Consistency 1488 Within a BGP SPF Routing Domain, all routers MUST use the same SPF 1489 algorithm (refer to Section 5.2.1.1). This is the responsibility of 1490 the administration for the routing domain. [minor] Please add something about the effect of not having the same SPF Algorithm. §5.2.1.1 already says that "only BGP nodes advertising the SPF capability TLV with same SPF algorithm are included in the SPF computation"; that is the effect from a management/operational point of view? I'm thinking about partitioned networks, incomplete topologies, etc.. [EoR -26]
- [Lsvr] Review of draft-ietf-lsvr-bgp-spf-26 Alvaro Retana