I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please treat these comments just like any other last call comments. For more information, please see the FAQ at . Document: draft-ietf-grow-bmp-local-rib-?? Reviewer: Thomas Fossati Review Date: 2021-03-20 IETF LC End Date: 2021-03-29 IESG Telechat date: Not scheduled for a telechat I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please treat these comments just like any other last call comments. For more information, please see the FAQ at . This draft is on the right track but has open issues, described in the review. Some high level notes: * A nice set of use cases that provide compelling reasons for this BMP extension are presented clearly and concisely at the beginning of the document. * It seems to me that this extension is in line with the original design goals of the protocol (i.e., simplicity, usefulness, ease of implementation, minimally service affecting). * This document updates BMP by providing a complete replacement for the monitoring of routes originated into BGP by the local router. This is made clear by the "update 7854" label, as well as the finer grained reference found in Section 1, which points to Section 8.2 of RFC7854. * Overall the document is very readable but there are a few places that need some editorial polishing to eliminate possible ambiguities. Nits: * Figure 1: * a couple of '+' are missing in the top-right and bottom-right corners of the Adj-RIB-In (Post) box; * "e.g." => "e.g.," in multiple places; * A few acronyms are not expanded: * IGP, BGP-LS, SPF, CSPF. VRF, ASN, BGP-ID, RD (route distinguisher?) * Section 1.1 * "directly effects" => "directly affects" * Section 3 * "an instance of an instance of BGP-4" => "an instance of BGP-4" * Section 5 * "post-policy" => "Post-Policy" * "ie." => "i.e.," * Section 5.1 * "in The Loc-RIB, expressed" => "in the Loc-RIB, expressed" * Section 5.2.1 * Consider using "Peer Up" instead of "Peer UP" for consistency with the capitalisation use in RFC7854 (also in Sections 5.3, 5.4.1, 6.1, 6.1.1, 6.1.3, and 8.3) * Section 5.3 * "peer Down" => "Peer Down" * Section 6.1 * "local router emulated peer." maybe "locally emulated peer." * Section 6.1.1 * "since it represents the same Loc-RIB instance" => "since they represents the same Loc-RIB instance" Editorial improvements: * Section 1.1 * I had some troubles parsing: "Complexities introduced by the lack of access to Loc-RIB in order to derive (e.g. correlate) peer to router Loc-RIB:", in particular the bit "in order to derive (e.g. correlate)". Is it "in order to derive (i.e., correlate)" or "in order to derive (or correlate)" or "in order to correlate"? * What does "suppresses more specifics" mean? Is there a term missing? * What does "derive a Loc-RIB to a router" mean? Is it "derive the Loc-RIB of a router" instead? * I find this "The BGP-IDs and session addresses to router correlation requires additional data" a bit hard to parse. Maybe re-flow it as: "Correlating BGP-IDs and session addresses to a router requires additional data" * Section 4.1 * I find "to distinguish that it represents Loc-RIB with or without RD and local instances" a bit hard to parse. I suggest rephrasing it to make it clearer. * Section 5 * Re: setting the F flag. It'd help if you put a forward ref to Section 6.1.2 here. (Before getting to 6.1.2 I got baffled by F; in particular, it was not clear to me from the surrounding text what is the monitoring station supposed to do with partial information without knowing exactly how much and what kind of info has been left out.) * Section 5.2 * Should add-paths be ADD-PATH instead? If so, maybe you could also add an informative reference to RFC7911 * In "The duplication allows the BMP receiver to use existing parsing" could you clarify what "existing parsing" mean? * Section 5.5 * Why would the receiver decide not to ignore a Route Mirror message? And what would happen if it decided so? I'm asking because I don't understand the reasons for a SHOULD rather than a MUST here. * Section 6.1.1 * In "There MUST be multiple emulated peers for each Loc-RIB instance" I am unsure whether what you want to say is that "there MUST be at least one emulated peer for each Loc-RIB instance" (which is what I thought) or that "each Loc-RIB instance *always* has multiple emulated peers" (which the current text seems to say)? * Section 8.2 * It is not clear to me if saying "and proposes that peer flags are specific to the peer type" you are asking IANA to modify the contents and/or structure of the BMP Peer Flags registry? If so, the request to IANA should be made more explicit. * Section 8.3 * Should "informational message TLV types" be "Initiation Message TLV type" instead?