Thomas, thank you for your review and thank you all for the following discussion. I have entered a No Objection ballot for this document. Lars > On 2021-3-20, at 12:33, Thomas Fossati via Datatracker <noreply@xxxxxxxx> wrote: > > Reviewer: Thomas Fossati > Review result: Ready with Issues > > 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 > > <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. > > 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 > <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. > > 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? > > > > > > _______________________________________________ > Gen-art mailing list > Gen-art@xxxxxxxx > https://www.ietf.org/mailman/listinfo/gen-art
Attachment:
signature.asc
Description: Message signed with OpenPGP
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
