Thanks a lot for the review and the detailed comments I will work on addressing them
Ahmed On Fri, Jan 3, 2020, 10:00 PM Yingzhen Qu <[email protected]> wrote: > Hi authors, > > > > Happy New Year! > > > > I did a review of draft-ietf-rtgwg-bgp-pic-10 for shepherd write-up. > Thanks for working on this informational document, and it’s very useful to > improve routing convergence. > > > > I have the following comments and would like you to consider. > > > > General: > > - Throughout the document, both BGP PIC and BGP-PIC are used. I’m ok > with either one, please keep it consistent. > - Regarding references, idnits is giving the following warnings: > > > > == Outdated reference: draft-ietf-spring-segment-routing-mpls has been > > published as RFC 8660 > > > > == Outdated reference: A later version (-05) exists of > > draft-bashandy-rtgwg-segment-routing-ti-lfa-02 > > - There are links to references in the document are broken/not > working, please go through and fix them. > - Other idnits warnings: > > > > == The copyright year in the IETF Trust and authors Copyright Line does > not > > match the current year > > > > == The document seems to contain a disclaimer for pre-RFC5378 work, but > was > > first submitted on or after 10 November 2008. The disclaimer is > usually > > necessary only for documents that revise or obsolete older RFCs, and > that > > take significant amounts of text from those RFCs. If you can contact > all > > authors of the source material and they are willing to grant the BCP78 > > rights to the IETF Trust, you can and should remove the disclaimer. > > Otherwise, the disclaimer is needed and you can ignore this comment. > > (See the Legal Provisions document at > > https://trustee.ietf.org/license-info for more information.) > > > > - Section 2.1.2: some clarification needed here. When the primary > next-hop fails, my understanding is that BGP PIC will first use other > primary next-hops if available, e.g ECMP before using the pre-computed > backup paths. Also “The existence of a secondary next-hop is clear for the > following reason:”, this needs some explanations, and this is different > from for example pre-computed backup paths using IP FRR. > - Section 7 title is “Properties”, and it seems to me this section is > more like a summary. I’d suggest combining section 7 and 10, then change > the title to summary or something. No strong opinion on this one though. > - Throughout the document, lots of paragraphs are missing the ending > “.” > > > > Nits: > > - The following are editorial nits, please consider fixing them. I’m > using the line number from idnits. > > > > 136 techniques, multiple techniques have been proposed to allow for > > 137 BGP to advertise more than one path for a given prefix > > I’m not sure it should be “allow” or “allow for”. > > > > 169 o Ingress PE, "iPE": A BGP speaker that learns about a prefix > > 170 through a IBGP peer and chooses an egress PE as the next-hop > for > > 171 the prefix. > > Should be “an iBGP peer”. Also this definition is not clear to me. I’d > also suggestion add one for “ePE”. > > > > 239 o A shared hierarchical forwarding Chain: It is not uncommon > to see > > Should be “chain”. > > > > 270 This section describes the required functionality in the > forwarding > > 271 and control planes to support BGP-PIC described in this document > > “functionalities”, also missing ending “.”. > > > > 334 VPN-IP2, respectively. Suppose that BGP-NH1 and BGP-NH2 are > resolved > > 335 via the IGP prefixes IGP-IP1 and IGP-P2, where each happen to > have 2 > > 336 ECMP paths with IGP-NH1 and IGP-NH2 reachable via the > interfaces I1 > > 337 and I2, respectively. Suppose that local labels (whether LDP > [4] or > > 338 segment routing [13]) on the downstream LSRs for IGP-IP1 are > IGP-L11 > > 339 and IGP-L12 while for IGP-P2 are IGP-L21 and IGP-L22. As such, > the > > 340 routing table at iPE is as follows: > > I think you meant “IGP-IP2”, instead of “IGP-P2”. > > > > > > Thanks, > > Yingzhen > > >
_______________________________________________ rtgwg mailing list [email protected] https://www.ietf.org/mailman/listinfo/rtgwg
