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

[Yingzhen Qu <[e-mail]>]

Hi Ahmed,

Sorry for the late response and thank you for addressing my comments.

I’ve uploaded the “Shepherd writeup”, please let me know if you have any question.

One problem still remaining: only you replied to the IPR poll, would you please contact your coauthors about this?

Thanks,
Yingzhen

From: Ahmed Bashandy <[e-mail]>
Date: Monday, February 10, 2020 at 7:57 AM
To: Yingzhen Qu <[e-mail]>, "[e-mail]" <[e-mail]>, "[e-mail]" <[e-mail]>
Subject: Re: Shepherd review of draft-ietf-rtgwg-bgp-pic


I uploaded version 11 to IETF. See repond inline "#Ahmed"

Thanks again for the thorough review

Ahmed
On 1/3/20 12:00 PM, Yingzhen Qu 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
#Ahmed: Upadted both references as well as other outdated references


  *   There are links to references in the document are broken/not working, please go through and fix them.
#Ahmed: I do not create any links :). I just uploaded the text file using the submission tool. So it is the tool that has the problem:)


  *
  *   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<https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftrustee.ietf.org%2Flicense-info&data=02%7C01%7Cyingzhen.qu%40futurewei.com%7Cd2494bdb31a04de7f57008d7ae41f402%7C0fee8ff2a3b240189c753a1d5591fedc%7C1%7C0%7C637169470601832127&sdata=Auk3qAjbcDaTUYLOvwcjomngh7Lb0y%2FI3M1fjpR%2FQ8w%3D&reserved=0> for more information.)



#Ahmed: Removed that paragraph

  *   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.

#Ahmed:

Your understanding that an implementation would divide the list of next-hops into "primary" and "secondary" and would start to use "secondary" next-hops only after all "primary" next-hops become unreachable is most likely correct.

However I would rather leave the decision to whether divide next-hops into "primary" and "secondary" or just treat all of them as "primary" to implementations, rather than recommending such behavior.

The original BGP spec allows only for a single next-hop. The term "secondary next-hops" is explained in the 3rd paragraph of the same section where it refers to add-path [10] and best external [5],..., etc. So from the original BGP protocol point of view, there is only one path.

Basically the term "primary" and "secondary" in this section is referring to BGP, not to how an implementation chooses which paths are used when one of them becomes unreachable.





  *
  *   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.
#Ahmed: Section 7 details the properties of BGP-PIC behavior while Section 10 is just a summary. I can remove section 10 if it seems redundant


  *
  *   Throughout the document, lots of paragraphs are missing the ending “.”
#Ahmed: Corrected


  *

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”.
#Ahmed: Corrected



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”.
#Ahmed: Corrected



239        o  A shared hierarchical forwarding Chain: It is not uncommon to see

Should be “chain”.
#Ahmed: Corrected



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 “.”.
#Ahmed: Corrected



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”.
#Ahmed: corrected (Thanks for catching this one)






Thanks,

Yingzhen