Document: draft-ietf-idr-next-next-hop-nodes-01 Title: BGP Next-next Hop Nodes Attributes Reviewer: Bruno Decraene Review result: Has issues Hello, I have been selected as the BGP Directorate reviewer for this draft. The purpose of this review is to provide assistance to the Routing ADs. For more information about the BGP Directorate, please see https://datatracker.ietf.org/group/bgpdir/about/ Although these comments are primarily for the use of the chairs, it would be helpful if you could consider them along with any other WG comments that you receive, and strive to resolve them through discussion or by updating the draft. Document: draft-ietf-idr-next-next-hop-nodes-01 Attributes Reviewer: Bruno Decraene Review Date: 2026-07-09 Intended Status: Standards Track • This document has issues. Major issues: ============ §2.1 "Next-hop BGP ID: 32-bit BGP Identifier" As per rfc6286, the BGP Identifier is not globally unique, but only unique within an AS. Given that when a BGP Next-Hop is changed, typically an AS border is crossed, NNHN cannot rely on the BGP Identifier in the typical cases. To address this, I guess that AS number could be added in addition to the BGP IP (just like for the BGP ID NHC defined in draft-ietf-idr-nhc). But this would represent a non-backward compatible change for the two existing implementations. On the other hand, this WG document is only 9 months old, so could still be considered modifiable by the WG. So the WG should probably balance technical accuraccy vs friendlyness with pre-standard implementations. I see that §3 is moving this specification/implementation issue to the network operator: "we need to make sure they are unique across the network where NNHN characteristic is sent.". As a network operator, I would have prefered the specification to be general enough. At minimun, the text "are unique across the network where NNHN characteristic is sent" seems unclear to me. What do you mean by "the network"? May be better "all Autonomous Systems"? (but this place a coordination requirement between ASes which are defined as "autonomous"). May be "Administrative Domain" as per draft-uttaro-idr-bgp-oad terminology? Still, this does seem to contradict with RFC6286. And an administrative domain deployed as per RFC6286 would not be able to use this specification. This could be seen as a non-backward compatible change. Minor issues: ============ §2.1 "Next-hop BGP ID: 32-bit BGP Identifier of the next hop node attaching this NHC characteristic." - field does not seem to be used by the receiver (as per §2.3). Why do you need it and how do you use it? - trying to guess, this field seems to duplicate the BGP ID NHC already defined in draft-ietf-idr-nhc. Why defining the same thing again? Especially since the existing BGP ID NHC carries the AS number in addition, which seems better from a technical standpoint (cf comment above) - it's not clear to me what "next hop node" means. (because you are always the next-hop node of someone; and next-hop compared to who?). Probably "the BGP node changing the BGP Next Hop" (to self, since this spec seems restricted to NHS) --- §2.1 "One or more 32-bit BGP Identifiers, each representing a next-next hop node used by the next hop node for ECMP forwarding" That's a lot of "next". Probably this could be simplified. In particular, one could read this as the list of "next-next hop nodes" used by the next hop node. (which is not the list of next-hops used by the next hope node). Suggestion: each representing a next-hop node used by the node referenced in the "Next-hop BGP ID" for ECMP forwarding. ---- §2.1 "For all the ECMP paths of route R which are used for forwarding, the BGP Identifiers of those BGP peers MUST be encoded as the "Next-next-hop BGP IDs"." If you learnt the path from IBGP, and a BGP RR is used, "the BGP Identifiers of those BGP peers" is the BGP ID of the BGP RR, not the BGP ID of the Next-next-hops. In such case, do you mean to use the ORIGINATOR_ID of the received routes? Please clarify. And if you mean to use this specification only in a very specific scenario, please specify this scenario, the restrictions, and the need to never send/delete NNHN in all others scenarios. Comment equally applicable to the next sentence " When more than one paths are from the same BGP peer, the characteristic MUST have only one BGP Identifier of that peer." --- §2.2 There is no mention of anycast next hop. draft-ietf-idr-nhc prohibits the use of NHC with anycast NH unless all anycast node support this caracteristic. I don't feel that NNHN requires this limitation (plus I'm not sure to see what "support this caracteristic" means for NNHN). May be explicitly covering this case, could be useful. --- §2.2 draft-ietf-idr-nhc touches on some special cases such as when "forwarding routes can be derived from a BGP route without regard to its next hop. One example is when the Tunnel Encapsulation Attribute [RFC9012] Tunnel Egress Endpoint Sub-TLV is used to point to a remote router." This draft does not cover this case. In particular, do you intend to use the BGP IP of the BGP NH or the BGP ID of the Tunnel Egress Endpoint? May be to some extend, same question for the SRv6 SID vs BGP NH. (I don't think that RFC 9252 requires that the BGP NH and SRv6 SID must belong to the same BGP ID. e.g. for inter-AS cases) --- §2.3 I'm not seeing the large last paragraph as a specification, but as use cases. I would rather move it to the introduction (§1). I'm not seing the first sentence as specifying anything (new). (clearly draft-ietf-idr-nhc is the one defining the reception of NHC). --- §2.4 "When a BGP speaker wishes to enforce hop-by-hop eBGP propagation of the NNHN, if the received NNHN characteristic's Next-hop BGP Identifier does not match the BGP Identifier of the BGP speaker the UPDATE was received from, it MUST be discarded and SHOULD be logged." - It's not completely clear to me what is meant by "hop-by-hop eBGP propagation". Probably not "BGP next-hop to BGP next-hop". Could you elaborate or point to a reference? - Probably this rely to a configuration knob that need to be present? - Not sure whether this is error-handling (§2.4) versus receiving specification (§2.3). To me the sender was correct, the receiver sees no error, but does not want to use NNHN in this scenario. --- §6 "Insertion of a syntactically valid but bogus NNHN characteristic by an attacker could potentially make the forwarding behavior of the route non-optimal." Where does this come from? (Given that essentially this specification does not define any behavior on the receiving/using side.) And if you want to cover the security considerations of the applications/examples, why not cover the security considerations of all applications/examples? Nits: ==== §1 "link quality": it's not clear to me which quality you are refering to. Do you mean "link congestion"? "the previous-previous layer nodes": it may be just me, but I have difficulty visualizing which are those nodes. A figure would probably help. ---- §2.1 "The NHC characteristic length specifies the remaining number of octets in the NNHN TLV." Stricto census, this does not belong in this specification, as this is specified by draft-ietf-idr-nhc. I'm not a fan of sentences redefining existing specifications; especially when the terms are not exactly the same. (NHC says "a two-octet unsigned integer that indicates the length, in octets, of the Characteristic Value field.") (because eventually, someone will see a difference causing interop issues) I would rather have this sentence removed. --- §2.4 "When duplicate "Next-next-hop BGP IDs" exist in an NNHN, only the first one is accepted and the remaining MUST be discarded and SHOULD be logged." "remaining ones" or "remaining of the NNHN"? --- §2.4 "Malformed NNHN characteristics MUST be discarded and SHOULD be logged." draft-ietf-idr-nhc specifies "If a characteristic TLV is malformed, that characteristic TLV SHOULD be ignored and removed." While draft-ietf-idr-nhc allows for a different error handling behavior, I'm wondering: - the difference that you are making between "discarded" and "ignored and removed". - the motivation change "SHOULD" to "MUST". (because if no difference is intended, there is no need to re-specify with a different choice of words) --- Appendix "The next hop carried in the existing NHC attribute can help to validate that the next-next hop nodes are indeed for the next hop of the NLRI." This does not seem specified/used in §2.3 "Carrying next-next hop nodes via a seperate path attribute will cost an additional attribute code, which is supposed to be allocated for more generally used attributes." Seems a bit weak argument. I would have expected an argument along "attribute escape". Such as the existing NHC infrastructure provides an existing filtering facility at AS/NH change boundaries.