Hi authors and WG, Thank you for this document, I believe that allowing BMP to share Loc-RIB is clearly a good thing.
I do have a few comments/nits that addressing now should help the IETF LC and IESG eval go more smoothly. Please SHOUT loudly once you've had a chance to address these. AD Review of draft-ietf-grow-bmp-local-rib -------------------------------------------- 1: "As shown in Figure 2, Locally originated section 9.4 of [RFC4271]" I'm unable to parse this - changing "As shown in Figure 2, Locally originated" into "As shown in Figure 2, Locally Originated into Loc-RIB, ..." doesn't fix it, because the figure doesn't really "show what Sec 9.4 of RFC4271" says. Perhaps something like: "Figure 2 (Locally Originated into Loc-RIB) illustrates how redistributed or otherwise originated routes get installed into the Loc-RIB based on the decision process selection in [RFC4271]" 2: In Section 1.1 the document says things like: "The current method introduces the need..." Once the document is published, the phrase "the current method" seems incorrect, but I don't have a better suggestion... 3: "Locally sourced routes MUST be conveyed using the Loc-RIB instance peer type." Should this be "locally sourced BGP routes"? It would be silly to think that this might carry e.g OSPF only routes, but you have a MUST, so important to be explicit. This also seems to conflict with "The F flag indicates that the Loc-RIB is filtered". Perhaps that above is better worded something like: "If locally sourced routes are communicated using BMP, they MUST be conveyed using the Loc-RIB instance peer type." ? 4: " The Loc-RIB contains all routes selected by the BGP protocol Decision Process section 9.1 of [RFC4271]." Similar to #1 - perhaps this is just missing a "in section of..."? Still needs rewording. 5: "These routes include those learned from BGP peers via its Adj-RIBs-In post-policy, as well as routes learned by other means section 9.4 of [RFC4271]." Similar -- I suspect that there was an errant search and replace which clobbered some text? 6: "Peer AS: Set to the BGP instance global or default ASN value." Erm, what's this default ASN value? 7: "5.1. Per-Peer Header" I think that this section needs a pointer to RFC7854 Sec 4.2. 8: "Capabilities MUST include 4-octet ASN" s/include 4/include the 4/ 9: "For example, prefix 10.0.0.0/8 is updated " Please use RFC5737 examples instead. Nit: 1: "This is overly complex for such a simple application that only needed to have access to the Loc-RIB." s/needed/needs/ 2: It can greatly reduce time to troubleshoot and resolve issues if operators had the history of Loc-RIB changes. s/had/have/ 3: "BGP Instance: it refers to an" s/it// -- Perhaps they really do strive for incomprehensibility in their specs. After all, when the liturgy was in Latin, the laity knew their place. -- Michael Padlipsky
_______________________________________________ GROW mailing list [email protected] https://www.ietf.org/mailman/listinfo/grow
