Re: Shepherd review of draft-ietf-rtgwg-bgp-pic

[Ahmed Bashandy <[e-mail]>]

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 <[e-mail]> 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
>
>
>