Grow WG, Here are my comments on a fresh re-read of the bmp-local-rib draft. I'll make the following leading statements:
- Portions of the issues we're tripping across are biases coming from working on a given implementation. Those happen. :-) - I strongly suggest you get a review pass of this document in IDR so you can make sure that terminology is consistent vs. IETF BGP RFC documents. These comments are vs. -04. Section 1 - Introduction: The introduction provides a description about the problems with how one gathers data today, including RFC 7854 local view and compares issues (beginning in the text "The current method") that impede standard RFC 4271 BGP (plus extensions) from being used to monitor a router. The introduction really needs a few sentences at the top giving a positive statement of what this draft is trying to accomplish. An example of this would be: "This document defines a mechanism to monitor the BGP Loc-Rib state for multiple BGP instances without the need for one or more unneeded BGP peering sessions." The rest of the introduction provides the justification. "the BGP instance" doesn't have any RFC context associated with it. RFC 7854 makes some discussion about multi-instancing of BGP in section 8.1. I'd suggest you define this somewhere in a terminology section as "an instance of BGP-4 (RFC 4271)" or similar, with the related pointer to the 7854 multi-instance comment. : As shown in Figure 2, Locally originated follows a similar flow where : the redistributed or otherwise originated routes get installed into : the Loc-RIB based on the decision process selection. I'd suggest a pointer to RFC 4271, section 9.4 here. : BGP instance Loc-RIB usually provides a similar, if not exact, : forwarding information base (FIB) view of the routes from BGP that : the router will use. The BGP spec doesn't mention a FIB at all. The interaction with the rest of the system is via the Routing Table. (RFC 4271, section 3.2) I'd either normalize the text to use that, or delete references to the FIB. Section 2, as noted elsewhere, should likely use RFC 8174 these days. Section 3: Loc-RIB definition - please point to 4271 section 9.4 Section 5: : Loc-RIB contains all routes from BGP peers as well as any and all : routes redistributed or otherwise locally originated. In this : context, only the BGP instance Loc-RIB is included. Routes from : other routing protocols that have not been redistributed, originated : by or into BGP, or received via Adj-RIB-In are not considered. I'd suggest the following: "The Loc-RIB contains all routes selected by BGP's Decision Process (RFC 4271, section 9.1). These routes include those learned from BGP peers via its Adj-RIBs-In post-policy, as well as routes learned by other means. (RFC 4271, section 9.4) Examples of these include redistribution of routes from other protocols into BGP, or otherwise locally originated; e.g. aggregate routes." The purpose of the examples sentence is not to be proscriptive about how things got into the decision process. If the examples are found to be contentious, I'd simply suggest deleting the sentence rather than try to enumerate all valid examples. : Loc-RIB in this context does not attempt to maintain a pre-policy and : post-policy representation. Loc-RIB is the selected and used routes, : which is equivalent to post-policy. : : For example, VRF "Blue" imports several targets but filters out : specific routes. The end result of VRF "Blue" Loc-RIB is conveyed. : Even though the import is filtered, the result is complete for VRF : "Blue" Loc-RIB. The F flag is not set in this case since the Loc-RIB : is complete and not filtered to the BMP receiver. The above feels awkward. I've tried to remove the need for this text by noting "post-policy" in the suggested text above. The second paragraph above is structured to try to discuss when the F-bit is used. I'd suggest instead that section 4.2 add a new sentence describing that the F-bit is only set when a subset of the Loc-Rib is sent on the BMP session and not when BGP routes are excluded from the Adj-Ribs-In based on policy. Section 5.1: s/filed/filled/ Peer BGP ID: It's fine for this document to shorten this to BGP ID, but a reference should be made to "BGP Identifier". (RFC 4271, section 1.1; RFC 6826) Section 5.2/5.3: Per prior discussion in other threads, it should be noted that changes that result in an alteration of behavior need a PeerDown/PeerUp bounce along with re-sync of routes. I believe this may only include a change to the F-bit in the current specification. Section 5.5: It might be worth mentioning that route mirroring messages MAY/SHOULD be ignored. However, it might not be worth mentioning. It's not like we'll bounce the session as long as things parse? (But we've seen protocol pedantry where that would be a valid behavior....) Section 6.1.1/6.1.2: 6.1.2 clearly talks about filtering. 6.1.1 talks about multiple peers, perhaps split on things like address family. For clarity, this section may want to discuss options for multiple views for filtering. For example, consider two distinct views: ebgp peers, ibgp peers. Or "customers" and "infrastructure". I believe the text as written suggest it'd be fine to have multiple views, each filtered, each using the same peer distinguisher and BGP ID. However, they may wish to be distinguished using a Table Name; e.g. -- Jeff _______________________________________________ GROW mailing list GROW@ietf.org https://www.ietf.org/mailman/listinfo/grow
