Hello, I have been selected as the Routing Directorate reviewer for this draft. The Routing Directorate seeks to review all routing or routing-related drafts as they pass through IETF last call and IESG review, and sometimes on special request. The purpose of the review is to provide assistance to the Routing ADs. For more information about the Routing Directorate, please see https://wiki.ietf.org/en/group/rtg/RtgDir Although these comments are primarily for the use of the Routing ADs, it would be helpful if you could consider them along with any other IETF Last Call comments that you receive, and strive to resolve them through discussion or by updating the draft. Document: draft-ietf-lsvr-bgp-spf-39 Reviewer: Alvaro Retana Review Date: December 5, 2024 IETF LC End Date: December 10, 2024 Intended Status: Standards Track Summary: This document is basically ready for publication, but I have some concerns that I think should be resolved before publication. Comments: I was the Responsible AD for this document until -21, and reviewed -27. I also participated in the WG's discussion of the last RtgDir review (-31). I based my review mainly on the diffs between -28 and -39. In general, this is a well-written document. Its revisions have improved the specification. Any remaining issues (see details below) should be easy to address. Major Issues: While I identified several issues as "major" (below -- mostly because of their Normative impact), I only want to highlight 2 here: (1) In §5.2.2.1 (BGP-LS Link NLRI Address Family Link Descriptor TLV), the case where multiple AFs are supported on the link is not described. See comments around line 616. (2) The IANA Considerations subsection 8.2 (BGP-LS-SPF Assignments to BGP-LS NLRI and Attribute TLV Registry) addresses the fact that some early-allocated values are not required anymore, but rfc9552 doesn't specify a process to remove unrequired code points from the registry. The code points should be marked as "Deprecated". See comments around line 1324. Minor Issues and Nits: Please see details below. Thanks! Alvaro. [Line numbers from idnits.] ... 182 1.2. BGP Shortest Path First (SPF) Motivation ... 219 With controller and route-reflector peering models, BGP SPF 220 advertisement and distributed computation require a minimal number of 221 sessions and copies of the NLRI since only the latest version of the 222 NLRI from the originator is required (sed Section 4). Given that [nit] s/sed/see ... 304 3. BGP Link-State (BGP-LS) Relationship ... 315 The "BGP-LS NLRI and Attribute TLVs" registry [RFC9552] is shared 316 between the BGP-LS SAFI and the BGP-LS-SPF SAFI. However, the TLVs 317 defined in this document are not applicable to the BGP-LS SAFI and 318 BGP-LS SAFI consumers [RFC9552] MUST NOT result in the NLRI or the 319 BGP-LS Attribute being considered malformed (section 5.2 of 320 [RFC9552]). The list of BGP-LS TLVs applicable to the BGP-LS-SPF 321 SAFI are described in Section 5.2. By default, the usage of other 322 BGP-LS TLVs or extensions are ignored for the BGP-LS-SPF SAFI. 323 However, this doesn't preclude the usage specification of these TLVs 324 for the BGP-LS-SPF SAFI in future documents. [major] "...the TLVs defined in this document are not applicable to the BGP-LS SAFI and BGP-LS SAFI consumers [RFC9552] MUST NOT result in the NLRI or the BGP-LS Attribute being considered malformed (section 5.2 of [RFC9552])." This document cannot require any action from BGP-LS nodes. The text may have been an attempt to copy the specification in §5.1/rfc9552 (not §5.2): "The presence of unknown or unexpected TLVs MUST NOT result in the NLRI or the BGP-LS Attribute being considered malformed." Suggestion> OLD> However, the TLVs defined in this document are not applicable to the BGP-LS SAFI and BGP-LS SAFI consumers [RFC9552] MUST NOT result in the NLRI or the BGP-LS Attribute being considered malformed (section 5.2 of [RFC9552]). NEW> However, the TLVs defined in this document may not be applicable to the BGP-LS SAFI. As specified in Section 5.1 of RFC9552, the presence of unknown or unexpected TLVs is required to not result in the NLRI or the BGP-LS Attribute being considered malformed. ... 337 4.1. BGP Single-Hop Peering on Network Node Connections ... 347 An End-of-RIB (EoR) Marker Section 5.3 for the BGP-LS-SPF SAFI MAY be 348 required from a peer prior to advertising the BGP-LS Link NLRI for 349 the corresponding link to that peer. [nit] s/Section 5.3/(Section 5.3)/g [major, but it may just be me] "MAY be required from a peer" The Normative text sounds confusing even if "required" is in lowercase. The previous text (-28, for example) said "MAY be expected". I agree that adding "from a peer" helps with clarity. s/MAY be required from a peer/MAY be expected from a peer/g ... 498 5.2.1.1. BGP-LS-SPF Node NLRI Attribute SPF Status TLV ... 528 If a BGP SPF speaker received the Node NLRI but the SPF Status TLV is 529 not received, then any previously received SPF status information is 530 considered as implicitly withdrawn and the update is propagated to 531 other BGP SPF speakers. A BGP SPF speaker receiving a BGP Update 532 containing a SPF Status TLV in the BGP-LS attribute [RFC9552] with a 533 non-reserved value (3-254) that is unknown SHOULD be advertised to 534 other BGP SPF speakers. However, a BGP SPF speaker MUST NOT use the 535 Status TLV in its SPF computation. An implementation MAY log this 536 condition for further analysis. If the SPF Status TLV contains a 537 reserved value (0 or 255) the TLV is considered malformed and is 538 handled as described in Section 7.1. [major] "non-reserved value (3-254) that is unknown" If we're being strict, values 1 and 2 are also "non-reserved" (because they are assigned). Instead of attaching a normative behavior to values that may change in the future (and require future documents to Update this one), use "unknown". s/a non-reserved value (3-254) that is unknown/an unknown value/g 540 5.2.2. Link NLRI Usage ... 565 The link descriptors are described in table 4 of [RFC9552]. 566 Additionally, an address family link descriptor is defined to 567 determine whether an unnumbered link can be used in the IPv4 SPF, the 568 IPv6, or both (refer to Section 5.2.2.1). [nit] s/an address family link descriptor/the Address Family Link Descriptor TLV 570 For a link to be used in SPF computation for a given address family, 571 i.e., IPv4 or IPv6, both routers connecting the link MUST have 572 matching addresses (i.e., interface addresses must match the neighbor 573 addresses). [] Maybe it's just me, but "matching addresses" doesn't tell me what the characteristics of the addresses should be. It sounds as if they are required to be the same. FWIW, the previous description was more explicit: "both routers connecting the link MUST have an address in the same subnet for that address family." ... 588 5.2.2.1. BGP-LS Link NLRI Address Family Link Descriptor TLV 590 For unnumbered links, the address family cannot be ascertained from 591 the endpoint link descriptors. Hence, the Address Family (AF) Link 592 Descriptor SHOULD be included with the Link Local/Remote Identifiers 593 TLV for unnumbered links, so that the link can be used in the 594 respective address family SPF. If the Address Family Link Descriptor 595 is not present for an unnumbered link, the link will not be used in 596 the SPF computation for either address family. If the Address Family 597 Link Descriptor is present for a numbered link, the link descriptor 598 will be ignored. If the Address Family Link Descriptor TLV contains 599 an undefined value (3-254), the link descriptor will be ignored. If 600 the Address Family Link Descriptor TLV contains a reserved value (0 601 or 255) the TLV is considered malformed and is handled as described 602 in Section 7.1. 604 0 1 2 3 605 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 606 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 607 | Type (TBD) | Length (1 Octet) | 608 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 609 | Address Family| 610 +-+-+-+-+-+-+-+-+ 612 Address Family Values: 0 - Reserved 613 1 - IPv4 SPF Computation 614 2 - IPv6 SPF Computation 615 3-254 - Undefined 616 255 - Reserved [minor] Why are the AFs called "IPvx SPF Computation" and not simply "IPvx"? [major] What about the case where multiple AFs are supported on the link? Should multiple TLVs be advertised, or should the length (which is not specified above) reflect the inclusion of more than one AF? ... 752 5.3. BGP-LS-SPF End of RIB (EoR) Marker 754 The usage of the End-of-RIB (EoR) Marker [RFC4724] with the BGP-LS- 755 SPF SAFI is somewhat different than the other BGP SAFIs. Optionally, 756 reception of the EoR marker MAY be required prior to advertising an 757 LINK-NLRI for a given peer. This is similar to OSPF's [RFC2328] 758 requirement to not advertise an adjacency until database 759 synchronization has completed. [major] "Optionally, reception of the EoR marker MAY be required..." This sentence is confusing because, at first, it sounds redundant ("Optionally" and "MAY" basically mean the same thing), but it is then mixed with a (possibly) *required* behavior (even if used in lowercase). In §4.1 I suggested using "MAY be expected from a peer". That wording should be worked in here as well. Also, the behavior is already specified in §4.1/§4.2, so there's no need to specify it here again. Suggestion> Reception of the EoR marker is optionally expected from a peer prior to advertising a Link NLRI for that peer. [major] "This is similar to OSPF's..." This is the one remaining sentence that compares BGP SPF to other protocols. Not needed, please remove it. ... 813 6.1. BGP SPF NLRI Selection ... 818 1. NLRI self-originated from directly-connected BGP SPF peers are 819 preferred. This condition can be determined by comparing the BGP 820 Identifiers in the received Local Node Descriptor and the BGP 821 OPEN message for an active BGP session. This rule assures that 822 stale NLRI is updated even if a BGP-LS router loses its sequence 823 number state due to a cold-start. Note that once the BGP session 824 goes down, the NLRI received is no longer considered as being 825 from a directly connected BGP SPF peer. [minor] s/BGP-LS router/BGP SPF router ... 899 6.3. SPF Calculation based on BGP-LS-SPF NLRI ... 1054 - For unnumbered links to match during the IPv4 or IPv6 1055 SPF computation, Current-Link and Remote-Link's Address 1056 Family link descriptor must match address family of the 1057 IPv4 or IPv6 SPF computation, the Current--Link's Local 1058 Identifier MUST match the Remote-Link's Remote 1059 Identifier, and the Current-Link's Remote Identifier 1060 MUST the Remote-Link's Local Identifier. Since the 1061 Link's Remote Identifier may not be known, a value of 0 1062 is considered a wildcard and will match any Current or 1063 Remote Link's Local Identifier (see TLV 258 [RFC9552]). [nit] s/Current-Link and Remote-Link's Address Family link descriptor/the Current-Link and Remote-Link's Address Family Link Descriptor TLV value [minor] s/the Current-Link's Remote Identifier MUST the Remote-Link's Local Identifier/the Current-Link's Remote Identifier MUST match the Remote-Link's Local Identifier ... 1298 8.2. BGP-LS-SPF Assignments to BGP-LS NLRI and Attribute TLV Registry 1300 IANA has assigned five TLVs for BGP-LS-SPF NLRI in the "BGP-LS NLRI 1301 and Attribute TLV" registry. These TLV types include the SPF Status 1302 TLV types, Address Famliy Link Descriptor TLV type, and Sequence 1303 Number TLV type. 1305 +==========+=================+=================================+ 1306 | TLV Code | Description | Reference | 1307 | Point | | | 1308 +==========+=================+=================================+ 1309 | TBD | Address Family | Section 5.2.2.1, RFCXXXX ([this | 1310 | | Link Descriptor | document]). | 1311 +----------+-----------------+---------------------------------+ 1312 | 1181 | Sequence Number | RFCXXXX ([this document]), | 1313 | | | Section 5.2.4 | 1314 +----------+-----------------+---------------------------------+ 1315 | 1184 | SPF Status | Section 5.2.1.1, RFCXXXX ([this | 1316 | | | document]), Section 5.2.2.2 and | 1317 | | | Section 5.2.3.1 | 1318 +----------+-----------------+---------------------------------+ 1320 Table 1: NLRI Attribute TLVs 1322 The early allocation assignments for the TLV types SPF Capability 1323 (1180), IPv4 Prefix Length (1182), and IPv6 Prefix Length (1183) are 1324 no longer required and can be removed. [major] RFC9552 doesn’t specify a process for removing unrequired code points from the registry. Therefore, I believe the correct action in this document is to request that the code points be marked as “Deprecated." Note that §7.2/rfc9552 says that the "WG chairs may inform IANA that a deprecated code point can be completely deallocated (i.e., made available for new allocations) at any time after it has been deprecated if there is a shortage of unallocated code points in the registry." I believe the "WG chairs" referred to here are the idr chairs -- and there's no shortage. [EoR -39]