Hi Jeff, Thanks very much for these comments. I believe i did my best to process them, you can see the commit log here:
https://github.com/paololucente/draft-ietf-grow-bmp-loc-rib/commit/d5a042b3c5251d87bf8b66bb5cd3554bb5cee128 Inline: On 12 Jun 2019, at 23:29, Jeffrey Haas <jh...@pfrc.org<mailto:jh...@pfrc.org>> wrote: [ .. ] 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. Done as per your suggestion. "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. Done, added a “BGP Instance” in the Definitions section. Unfortunately by then the “BGP Instance” term is repeated already a few times in Abstract and Introduction. : 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. Done. : 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. Done, deleted. Section 2, as noted elsewhere, should likely use RFC 8174 these days. Done. Section 3: Loc-RIB definition - please point to 4271 section 9.4 Done. 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.” Done. : 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. Done, removed text and example and added text to section 4.2. Section 5.1: s/filed/filled/ Done. 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) Done. 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. Done, added section 6.1.3 about “Changes to existing BMP sessions” under Other Considerations. 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….) Done. 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. Done. This opened a further consideration at my end. The document lacks a statement as in https://tools.ietf.org/html/draft-lucente-bmp-tlv-00 about “TLVs can be sent in any order. Multiple TLVs of the same type can be repeated as part of the same message and it is left to the specific use-cases whether all, any, the first or the last TLV should be considered.”. In the specific case of VRF/Table Name one could have both a VRF id/name and a Table Name, hence the same TLV could be repeated twice (my take is that it’s a perfectly valid scenario). I’d tackle this case once i get green light from you that we are good about how your feedback was processed. Paolo
_______________________________________________ GROW mailing list GROW@ietf.org https://www.ietf.org/mailman/listinfo/grow
