Hello I have been selected to do a routing directorate “early” review of this draft. https://datatracker.ietf.org/doc/html/draft-ietf-idr-dynamic-cap-18 The routing directorate will, on request from the working group chair, perform an “early” review of a draft before it is submitted for publication to the IESG. The early review can be performed at any time during the draft’s lifetime as a working group document. The purpose of the early review depends on the stage that the document has reached. As this document is in working group last call, my focus for the review was to determine whether the document is ready to be published. Please consider my comments along with the other working group last call comments. For more information about the Routing Directorate, please see https://wiki.ietf.org/en/group/rtg/RtgDir Document: draft-ietf-idr-dynamic-cap-18 Reviewer: Bruno Decraene Review Date: 2026-02-09 Intended Status: Standards Track Summary: I have significant concerns about this document. It needs more work before being submitted to the IESG. Comments: ===== Major: - Introduction (§1) states that the goal of this protocol extension is to allow for changing BGP capabitiies in a non disrupting way. I'm curious as to why BGP Graceful Restart (RFC 4724) could not be used for this. (I'm aware that the authors are well aware of BGP Graceful Restart, so I'm certainly missing something. Thanks for taking the time to educate me) -§2.1 Is the "Dynamic Capability " a multi-instance capability or a single-instance capability? Could a priori be both. This seems important for dynamically updating the Dynamic capability (as per end of §3). Could this be clarified in the draft? -§2.1 Depending on the previous point, handling of multiple intances of Dynamic Capability in the OPEN does not seem specified, while RFC 5492 says "BGP speakers MAY include more than one instance of a capability (as identified by the Capability Code) with non-zero Capability Length field, but with different Capability Value and either the same or different Capability Length. Processing of these capability instances is specific to the Capability Code and MUST be described in the document introducing the new capability." - §3 "The Capability Length field, though, is larger than the one specified in [RFC5492]." To me, this seems to be calling for bugs. What's the reason for using a different length? - §4 "A BGP speaker SHOULD reset the HoldTimer upon receiving a DCAP message from its peer." This looks like an absolute requirement for interop. So probably :s/SHOULD/MUST Should this document formerly update RFC 4271? as 4271 says "The calculated value indicates the maximum number of seconds that may elapse between the receipt of successive KEEPALIVE and/or UPDATE messages from the sender." - §4.1 "If the Receiver is not capable of Dynamic Capability or if 'c' is not in the DCAP list, the Initiator should log and discard the capability revision." Do you mean "MUST discard the capability revision"? Do you mean "SHOULD log"? - §4.1 "During this phase, it must continue its usual operation of sending" "MUST" ? In subsequent sentences, there are two occurences of "it must not". Do you mean "MUST NOT"? - §4.1 "When CapabilityRevisionTimer times out and the capability revision is still in progress, the dynamic capability revision MUST be discarded." If the Receiver finally Ack the revised capability, the Initiator and the Receiver will be out of sync (Initiator cancelled while Receiver confirmed). To avoid this, I believe the the Iniator must send a new DCAP message "cancelling" its initial message. (e.g., removing a new capability, if the first message was adding it). Alternavily, there could be no time out (i.e. worst case, the state is kept on the initiator for the duration of the BGP session) "This document recommends 10 minute timeout value" while this seems large enough in normal BGP operation, this does not seem that large for any specific condition (e.g. TCP socket blocked on the sender, or on the receiver, large Route Refresh in operation...) - §4.2 "The Receiver should expect more than one capability tuple in the DCAP message and should process each capability revision independently." From §3, it was not clear to me that one DCAP message could carry more than one capability tuple. Could you clarify either §3 or §4.2 as to whether the DCAP can carry multiple tuples? Also, if multiple tuples, there would a priori be a single Sequence Number for all capability tuples. But multiple timers (one but capability tuple) since each capability revision is process independently. Is this correct? Do you mean :s/should/MUST ? (as this seems required for interop) - §4.2 "In the received DCAP message, if the Init/Ack bit is set to 1, it SHOULD silently discard the capability revision." Text is a bit too implicit (to me) on how the receiver assumes that it's a receiver (and not the initiator). In particular when both BGP peers udpdate a capability, not to mention the same capability code, at the same time (and hence both are receiver and initator). Could this be clarified/rephrased? - §4.2 "If the capability code is not listed in the Dynamic Capability (DCAP list) advertised by the Receiver itself, and the Receiver MUST send a NOTIFICATION message back to the Initiator as specified in the Error Handling section." This error habdling procedure seems quite impactful for the network and not in the spirit of RFC 7606. Could this be softened? e.g. log the error and silently discard the DCAP message. -§6 " Hence, the capabilities that have bi-directional capability dependency requiring 3-way handshake will not function accurately." Could the guidance be expressed in stronger terms? e.g. NEW: "Dynamic Capability MUST NOT be used to update a capability that change the format of BGP messages." -§8 This section seems to refer to interworking with previous version of this draft. If so, I don't think that we care for an RFC hence I would suggest to remove this paragraph. (if not, at minima "existing implementations" needs to be clarified) - IANA section (§9) -- Assuming that I'm correctly interpreting the text, draft is squatting on the BGP Message Types 6. (also in §3). I'll leave further actions to the chairs. Given that the codepoint is not random, I can't see how this can be an error. Is this deliberate codepoint squatting? In addition, this draft specifically had warnings on codepoint squatting: - the WG already raised this point more than a year ago https://mailarchive.ietf.org/arch/msg/idr/Bx5EFcJo_jfaR0Z8KS5YmRbXvFM/ - that draft seems to have squatted on another code point (Error Code), created a collision and the deprecation of the codepoint. Would you have suggestions on how we (IDR WG, IETF) can do better to avoid codepoint squatting? E.g., to better disincentivize codepoint squatting? e.g. what rules could have prevented authors from squatting on codepoints? [REVISED] I saw that -19 removed the codepoint from the draft but I don't think that this is enough to avoid collisions if there are shiping implementations. https://github.com/ietf/wiki.ietf.org/blob/main/group/idr/implementations/draft-ietf-idr-dynamic-cap.md We probably need to either allocate the codepoint (to DCAP or deprecation) such that the IANA registry is correct. -- IANA section does not seem to follow RFC 8126 (section 3). e.g. "Use the exact registry name as listed on the IANA web page, and cite the RFC where the registry is defined. When referring to an existing registry, providing a URL to precisely identify the registry is helpful (see Section 2.2)." -- Naming does not appear to be consistent. e.g., the message type is sometimes refered as "CAPABILITY" (§9), "Dynamic Capability message" (§2.1), "DYNAMIC CAPABILITY message" (§2.1, §3) - §12.3 "Appendix 3" defines normative requirement (MUST). I don't think that this appropriate in an appendix, especially when the section starts by saying "This section provides additional information useful for reviewers, and operators" - May I suggest authors to review and reply to comments from the IDR WG? In 15 years, I could only see 3 comments on this draft. One from Krishnaswamy Ananthamurthy has been replied but not acted upon. One from Jeffrey Haas has not been replied and at least not fully acted upon. - §7 Error Handling I'll admit that I'm quite sensitive to error handling, but I don't think that this section is aligned with the spirit of RFC 7606. (probably related to the fact that the text has been unchanged for more than 20 years). "If a BGP speaker detects an error while processing a CAPABILITY message, it MUST send a NOTIFICATION message with Error Code CAPABILITY Message Error." I think that this is way too broad, coarse and impactfull. I'd like to invite authors to fully reconsider the error handling. I haven't spent too much time thinking about this (since I'm not an author) but I would assume that we could probably do better. e.g. if the receiver correctly parse the CAPABILITY message type, but is not happy with its content (Capability code, value, length...) couldn't it simply discard the message? (and hence never acknowledge it so the that Initiator will time out as per current spec). If not, may be adding a flag for the receiver to be able to explicitly refuse a message (instead of acknowledging it) And if NOTIFICATION is mandatory, what about mandating the use of RFC8538? (since the whole purpose of this draft is to avoid reseting the session) -§7 "On the receipt of such a NOTIFICATION message, the BGP speaker should log for troubleshooting purposes. The ongoing capability revision MUST be discarded by both Initiator and Receiver. No new capability revisions can be initiated until administrator intervention." For the two last sentences, do you mean for all futures BGP sessions? If so, could this be indicated (as usually rules only refers to current BGP session). If not, I'm missing something a the BGP session is fully terminated by the NOTIFICATION -§7 "If a NOTIFICATION message with Error code Unknown Sequence Number is received, it MUST be logged for troubleshooting purposes before silently discarding it." I may be missing something but I don't see how one can ignore a NOTIFICATION since a priori the peer is closing the BGP connection. (RFC 4271 "A NOTIFICATION message is sent when an error condition is detected. The BGP connection is closed immediately after it is sent." ===== Minor: - The abstract uses the conditional (would). This seems unusual and it's not clear to me why. - I could not find the implementation of this document. https://github.com/ietf/wiki.ietf.org/blob/main/group/idr/implementations/draft-ietf-idr-dynamic-cap.md list one implementation of a very early version of the draft which does not correspond to latest specification. Content is also very light. It seems to support (only) dynamic cap for capability 6 (BGP Extended Message) which as per §6 seems to not be allowed (anymore). - §1 :s/[RFC2858]/[RFC4760] (hence reference to 2858 may be removed) -§2 "By announcing the capability via OPEN message a BGP speaker conveys that it is capable of receiving and properly handling messages related to that capability." :s/ messages/ CAPABILITY messages -§2.1 "Only individual capability-instance capabilities under multi-instance capabilities" is not crystal clear to me while it's an important restriction. Could you please rephrase/elaborate? - §3 "The Reserved bits should be set to zero by the sender and ignored by the receiver" :s/should/SHOULD (well, MUST if you ask me, but opinions vary.) Note that the WG already raised that point 4 years ago https://mailarchive.ietf.org/arch/msg/idr/EaUNOcfW5PqfWodBGevHDkmdXy8/ - §3 "also known as multi-instance capability" I'm not seeing this term defined in RFC 5492. If this is defined by this document, I'd rather have the term properly defined. (preferably before §2.1, as per my comment above) - §3 "Furthermore, if the "Action" is to remove a capability, then the Capability Length field SHOULD be set to zero by the sender" Why a "SHOULD" and not a "MUST"? Any reason to send a non zero length? - §2.1 "via existing BGP messages" What does "existing" means? Existing prior the publication of this document? Or any BGP message? If the later, may be "existing" could be deleted. If the former, may be clarifying. -§4 "Procedures for handling BGP Dynamic Capability Revisions" May be NEW: Procedures for handling BGP Capability Revisions (May be it's me, but I read that this section was about revising the Dynamic Capability (the one defined in §2.1) -§4.2 "The Receiver SHALL ignore such a capability revision that either results in no change to an existing capability, or removes a capability that was not advertised previously." "ignore" seems a bit strong to me as the DCAP message MUST be acknoledged. may be NEW: A capability revision that either results in no change to an existing capability, or removes a capability that was not advertised previously, MUST NOT be treated as an error and MUST NOT change the negociated capability. -§4.2 It's not crytal clear to me what the receiver must do when receiving a DCAP with Init/Ack sent to 1 and Ack Request set to 1. "if the Init/Ack bit is set to 1, it SHOULD silently discard the capability revision." seems to indicate to silently discard. "If the Ack Request bit is set to 1, the Receiver MUST send a DCAP message to acknowledge the receipt of the capability revision." seems to indicate MUST send a DCAP message. -§6 Please add the reference to the documents specifying the listed capabilities (e.g. BGP Role...) -§6 "The remaining capabilities may only be advertised via OPEN message during session establishment." It's not crystal clear which remaining capabilities you are refering to. Could be read as all other capabilities not mentioned in the list above. Hence all future capabilities. -§7 "This document revises the usage of Sequence Number." revised compared to what? A previous version of this draft? I don't think we care for an RFC: only the spec to be followed ias to be described. "The DCAP message fields are sufficient to correlate the message between the Initiator and the Receiver, hence the Sequence Number usage is limited to diagnostic purposes." The Sequence Number field is part of the DCAP message field, so this does not read well. You could indicate which fields are sufficient for this. -§10 "The extension proposed in this document does not change the underlying security or confidentiality issues inherent in the existing BGP [RFC4271]." I think that I agree, but it would seem to be more useful if you could explicit how/why you came to that conclusion. Otherwise it's difficult for a security reviewer to identify a flaw in the reasoning. -§12.2 "The Error Code is changed from 7 to TBD due to a conflict." How is that possible? Codepoints squatting? -§12.4. For consistency (and clarity) title should probably include "Appendix 4:" Nits: - §2 :s/. if an operator wishes/. If an operator wishes - §2 may be (I'm not a native speaker) :s/to enable new functionality /to enable a new functionality - §2.1 "The list of capabilities in the value field of Dynamic Capability TLV s hereby referred as DCAP list" Presumably :s/ s / is - §12.4.3 :s/habdling /handling - §4.2 "If the capability code is not listed in the Dynamic Capability (DCAP list) advertised by the Receiver itself, and the Receiver MUST send a NOTIFICATION message back to the Initiator as specified in the Error Handling section." IMO :s/, and the Receiver MUST/, the Receiver MUST