Re: [GROW] [Gen-art] Genart last call review of draft-ietf-grow-bmp-local-rib-10

[Lars Eggert <[e-mail]>]

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 <[e-mail]> 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
> [e-mail]
> https://www.ietf.org/mailman/listinfo/gen-art