Re: [Idr] AD Review of draft-ietf-idr-bgp-optimal-route-reflection-22

[Robert Raszuk <[e-mail]>]

Ack !

Work in progress ...

On Tue, Apr 20, 2021 at 9:29 PM Alvaro Retana <[e-mail]>
wrote:

> Ping!
>
> On March 18, 2021 at 3:32:01 PM, Alvaro Retana ([e-mail])
> wrote:
>
>
> Dear authors:
>
>
> Thank you for your work on this document.  I know that the draft and the
> related ideas have been around for a long time.
>
> I have a couple of significant issues that I want to highlight upfront,
> and many comments inline (see below).  In general, my focus is to make sure
> that the specification of this mechanism is clear, not just to the
> experienced reader, but to the casual one as well.  I believe that all the
> issues should be easy to resolve.
>
> (1) Terminology.  Please use the terminology already defined in
> rfc4271/rfc4456 instead of creating new one by indirection.  My first
> inline comment is about the use of "best path", which is not what rfc4271
> uses -- there are other occurrences later on.
>
> (2) Deployment Considerations.  §6 says that a compliant implementation is
> one that allows the operator to configure "a logical location from which
> the best path will be computed, on the basis of either a peer, a peer
> group, or an entire routing instance".  Please spend some time providing
> guidance so an operator can decide what best works for their network.  [I
> pointed below to other places there operational guidance would be ideal.]
>
> (3) I don't think that §5 (CPU and Memory Scalability) and Appendix A
> (alternative solutions with limited applicability) should be part of this
> document.  To start, comparisons are never good and the work has already
> been through the WG so there's no need to keep justifying it by comparing
> it to other work.  Both sections contain several claims (e.g. "extra cost
> in terms of CPU", "implemented efficiently", "expected to be higher but
> comparable", "large amount of BGP state and churn", "result in tens of
> paths for each prefix") that don't have references, and could be considered
> subjective because they are clearly relative and can change depending on
> the specific deployment and configuration of the network.  I am not saying
> that the claims are false, simply requesting to take these two sections
> out.
>
>
> Thanks!
>
> Alvaro.
>
>
> [Line numbers from idnits.]
>
> ...
> 17 Abstract
>
> 19   This document defines an extension to BGP route reflectors.  On route
> 20   reflectors, BGP route selection is modified in order to choose the
> 21   best path from the standpoint of their clients, rather than from the
> 22   standpoint of the route reflectors.  Multiple types of granularity
> 23   are proposed, from a per client BGP route selection or to a per peer
> 24   group, depending on the scaling and precision requirements on route
> 25   selection.  This solution is particularly applicable in deployments
> 26   using centralized route reflectors, where choosing the best route
> 27   based on the route reflector IGP location is suboptimal.  This
> 28   facilitates, for example, best exit point policy (hot potato
> 29   routing).
>
> [major] "best path"  rfc4271 doesn't use the term "best path".  The
> terminology used in this document should, at the very least, match what the
> base spec defines.  Let's please not create new terminology by indirection
> using the "definition of terms" section.
>
>
> [minor] "proposed"  This document is in line to be come an RFC; we're far
> beyond proposing things.  There are a couple of places later on where this
> same wording is used.  Please change it to specified or at least described
> (the latter probably fits best in this specific case).
>
>
> [nit] s/from a per client BGP route selection or to a per peer group/from
> per client BGP route selection or per peer group
>
>
> [nit] s/precision requirements on route selection./precision requirements.
>
>
> [nit] s/route reflector IGP location/route reflector's IGP location
>
>
> [major] "IGP location"  Before I forget -- please clearly define (not in
> the Abstract of course) what an "IGP location" is.
>
>
> ...
> 70   1.  Definitions of Terms Used in This Memo  . . . . . . . . . . .   2
> 71   2.  Introduction  . . . . . . . . . . . . . . . . . . . . . . . .   3
>
> [nit] It would be nice if the Introduction came up before the terminology.
>
>
> ...
> 91 1.  Definitions of Terms Used in This Memo
>
> [minor] To avoid repeating some terms, it is good practice to indicate
> which other RFCs the reader should be familiar with.  In this case rfc4271
> and rfc4456 come to mind.
>
>
> ...
> 132 2.  Introduction
> ...
> 153   Section 11 of [RFC4456] describes a deployment approach and a set of
> 154   constraints which, if satisfied, would result in the deployment of
> 155   route reflection yielding the same results as the IBGP full mesh
> 156   approach.  This deployment approach makes route reflection
> compatible
> 157   with the application of hot potato routing policy.  In accordance
> 158   with these design rules, route reflectors have traditionally often
> 159   been deployed in the forwarding path and carefully placed on the POP
> 160   to core boundaries.
>
> [nit] s/have traditionally often/have often
> Redundant...
>
>
> 162   The evolving model of intra-domain network design has enabled
> 163   deployments of route reflectors outside of the forwarding path.
> 164   Initially this model was only employed for new address families,
> e.g.
> 165   L3VPNs and L2VPNs, however it has been gradually extended to other
> 166   BGP address families including IPv4 and IPv6 Internet using either
> 167   native routing or 6PE.  In such environments, hot potato routing
> 168   policy remains desirable.
>
> [minor] "new address families, e.g. L3VPNs and L2VPNs"  These are not the
> name of the AFs.  Maybe call them new services.
>
>
> [minor] "IPv4 and IPv6 Internet"  The name of the AF is IP/IP6; again, you
> probably mean service/application.
>
>
> [nit] "native routing or 6PE"  Not 4PE applications?  ;-)   It seems to me
> that you can save some ink by ending the sentence after "Internet".
>
>
> ...
> 194 3.  Modifications to BGP Best Path selection
>
> 196   The core of this solution is the ability for an operator to specify
> 197   the IGP location for which the route reflector should calculate
> 198   routes.  This can be done on a per route reflector basis, per peer/
> 199   update group basis, or per peer basis.  This ability enables the
> 200   route reflector to send to a given set of clients routes with
> 201   shortest distance to the next hops from the position of the selected
> 202   IGP location.  This provides for freedom of route reflector physical
> 203   location, and allows transient or permanent migration of this
> network
> 204   control plane function to an arbitrary location.
>
> [major] "ability for an operator to specify the IGP location for which the
> route reflector should calculate routes.  This can be done on a per route
> reflector basis, per peer/update group basis, or per peer basis."
>
> When should an operator choose per RR vs per peer group vs per peer?
> Choice is good, but providing guidance for the deployment is much better.
> Please consider adding text to §6 about the considerations to chose one or
> the other.
>
>
> [minor] "peer/update group"  Where is this defined?  Is there a reference
> to a non-implementation-specific definition?  Can it be called "a group of
> peers"?  Maybe you also want to include it in the definitions in §1.
>
>
> ...
> 215   o  it can, and usually will, have a different position in the IGP
> 216      topology, and
>
> [] "usually will"   The position in the topology will *always* be
> different!
>
>
> ...
> 222   This document defines, on BGP Route Reflectors [RFC4456], two
> changes
> 223   to the BGP Best Path selection algorithm:
>
> [nit] s/defines, on BGP Route Reflectors/defines, for BGP Route Reflectors
>
>
> ...
> 235   A route reflector can implement either or both of the modifications
> 236   in order to allow it to choose the best path for its clients that
> the
> 237   clients themselves would have chosen given the same set of candidate
> 238   paths.
>
> [major] Please provide guidance for the operator to consider then one or
> both should be used in their network.
>
>
> 240   A significant advantage of these approaches is that the route
> 241   reflector clients do not need to run new software or hardware.
>
> [nit] s/do not need to run new software or hardware./do not need to be
> modified.
>
>
> 243 3.1.  Best Path Selection from a different IGP location
>
> 245   In this approach, optimal refers to the decision made during best
> 246   path selection at the IGP metric to BGP next hop comparison step.
> It
> 247   does not apply to path selection preference based on other policy
> 248   steps and provisions.
>
> [major] Please be specific about what is the "IGP metric to BGP next hop
> comparison step".  There is no step with that name, or even a string match
> in rfc4271.  Later you talk about the step e) tie-breaker -- don't wait to
> be specific!
>
>
> ...
> 263      e) Remove from consideration any routes with less-preferred
> 264      interior cost.  The interior cost of a route is determined by
> 265      calculating the metric from the selected IGP location to the
> 266      NEXT_HOP for the route using the shortest IGP path tree rooted at
> 267      the selected IGP location.
>
> [major] Note that the specification talks about "interior cost", but the
> descriptions in this document uses "IGP cost" and "IGP metric" to refer to
> the same thing.  Please be consistent with existing terminology!
>
>
> [major] Related to the definition of "IGP location" and its
> configuration.  How does the IGP location (and thus the calculation of the
> interior cost, as above) change when the configuration is done per
> peer/update groups instead than per peer?  This is another point where
> operators could benefit from guidance (for the peer/update-group and whole
> RR cases, of course).
>
>
> ...
> 275   The configuration of the IGP location is outside of the scope of
> this
> 276   document.  The operator may configure it manually, an implementation
> 277   may automate it based on heuristics, or it can be computed centrally
> 278   and configured by an external system.
>
> [nit] s/outside of the scope/outside the scope
>
> [major] There are several places later on that talk about the
> configuration, even making it a requirement for compliance (§6).  I think
> that you don't mean the configuration itself, but the way in which the
> configuration is instantiated in the router (which could be considered an
> implementation detail).  As written, the text is not clear.
>
>
> 280   This solution does not require any change (BGP or IGP) on the
> 281   clients, as all required changes are limited to the route reflector.
>
> [] This was also claimed before.
>
>
> 283   This solution applies to NLRIs of all address families that can be
> 284   route reflected.
>
> [] Are there AFs that cannot be reflected?  Just wondering if you really
> need to say this.
>
>
> 286 3.1.1.  Restriction when BGP next hop is BGP prefix
>
> 288   In situations where the BGP next hop is a BGP prefix itself, the IGP
> 289   metric of a route used for its resolution SHOULD be the final IGP
> 290   cost to reach such next hop.  Implementations which can not inform
> 291   BGP of the final IGP metric to a recursive next hop SHOULD treat
> such
> 292   paths as least preferred during next hop metric comparison.  However
> 293   such paths SHOULD still be considered valid for best path selection.
>
> [major] The recursive resolution is already covered in rfc4271
> (§5.1.3/§9.1.2.1).  Why isn't that enough?
>
> There seems to be a difference: if an implementation "can not inform BGP
> of the final IGP metric to a recursive next hop...SHOULD still be
> considered valid for best path selection."  rfc4271 treats these routes as
> unresolvable.  Do you want to consider unresolvable routes?
>
> Finally, there are a lot of SHOULDs in this paragraph.  Under which
> conditions is it ok to not perform the actions?  IOW, why are these actions
> recommended and not required?
>
>
> 295 3.2.  Multiple Best Path Selections
>
> 297   BGP Route Reflector as per [RFC4456] runs a single best path
> 298   selection.  Optimal route reflection may require calculation of
> 299   multiple best path selections or subsets of best path selection in
> 300   order to consider different IGP locations or BGP policies for
> 301   different sets of clients.
>
> [] It hasn't been mentioned before, but the talk about policy made me
> think about this:   Can a client be present on different sets?  How is that
> handled in terms of BGP sessions?  Do we need multiple sessions?  Or is
> add-path required?
>
>
> 303   If the required routing optimization is limited to the IGP cost to
> 304   the BGP Next-Hop, only step e) as defined [RFC4271] section 9.1.2.2,
> 305   needs to be duplicated.
>
> [major] I'm not sure if this paragraph is referencing §3.1 (where step e)
> was just modified), or or you're saying that for each subset (from the last
> paragraph) only step e) is considered, or something else.  ??
>
>
> 307   If the routing optimization requires the use of different BGP
> 308   policies for different sets of clients, a larger part of the
> decision
> 309   process needs to be duplicated, up to the whole decision process as
> 310   defined in section 9.1 of [RFC4271].  This is for example the case
> 311   when there is a need to use different policies to compute different
> 312   degree of preference during Phase 1.  This is needed for use cases
> 313   involving traffic engineering or dedicating certain exit points for
> 314   certain clients.  In the latter case, the user MAY specify and apply
> 315   a general policy on the route reflector for a set of clients.  For a
> 316   given set of clients, the policy SHOULD in that case allow the
> 317   operator to select different candidate exit points for different
> 318   address families.  Regular path selection, including IGP perspective
> 319   for a set of clients as per Section 3.1, is then applied to the
> 320   candidate paths to select the final paths to advertise to the
> 321   clients.
>
> [] Similar comment as before...   The use of "duplicated" is confusing me.
>
>
> [major] "the user MAY specify..."  s/MAY/may   It seems to me that you're
> simply stating a fact (the user can do this), and not specifying an
> optional behavior.
>
>
> [major] "...the policy SHOULD in that case allow the operator to select
> different candidate exit points for different address families."  There's
> no interoperable action that "the policy" can execute.  It sounds as if
> you're recommending that specific knobs ("allow the operator to...") be
> implemented.  Please reword.
>
>
> [] You use "IGP perspective" I guess as equivalent to "IGP location" -- is
> that right?
>
>
> 323 4.  Implementation considerations
>
> 325 4.1.  Likely Deployments and need for backup
>
> [nit] Do we really need a sub-section if all the text for §4 is in it?
>
>
> 327   With IGP based optimal route reflection, even though the IGP
> location
> 328   could be specified on a per route reflector basis or per peer/update
> 329   group basis or per peer basis, in reality, it's most likely to be
> 330   specified per peer/update group basis.  All clients with the same or
> 331   similar IGP location can be grouped into the same peer/update group.
> 332   An IGP location is then specified for the peer/update group.  The
> 333   location is usually specified as the location of one of the clients
> 334   from the peer group or an ABR to the area where clients are located.
> 335   Also, one or more backup locations SHOULD be allowed to be specified
> 336   for redundancy.  Implementations may wish to take advantage of peer
> 337   group mechanisms in order to provide for better scalability of
> 338   optimal route reflector client groups with similar properties.
>
> [major] "IGP based optimal route reflection"
>
> This is the first time you use "IGP based optimal route reflection"; I
> peeked forward and see that §5 also mentions "policy based optimal route
> reflection".  But you didn't define them anywhere -- in fact, the
> description before now focuses on the IGP location, which makes me think
> about "IGP based".
>
> I could guess that the "policy based" version is related to §3.2, but (1)
> no one should have to guess (!), and (2) the end of that section says that
> "IGP perspective...is then applied", making it also "IGP based".  ??
>
> Please either define these types of ORR earlier in the text, or (better
> yet) don't use the terms.  Instead, and given that the names are used just
> a couple of times, change "policy based" to "the case where a policy is
> applied"...
>
>
> [major] "one or more backup locations SHOULD be allowed to be specified
> for redundancy"
>
> (1) §3.1 says that the "configuration of the IGP location is outside of
> the scope of this document".  If that is true, then you can't recommend
> anything related to the configuration.
>
> (2) If there were "backup locations", how would they be used?
>
>
> [minor] This is the only time that "optimal route reflector client groups"
> is used.  I had been assuming that a group of clients would correspond to a
> specific peer/update group (as mentioned many times before).  Is there a
> difference between a "client group" and grouping clients to correspond to a
> peer/update group?
>
>
> [] "for better scalability of optimal route reflector client groups"  What
> exactly does this mean?  I asked before about references/definitions for
> peer/update groups; this point may be related.
>
>
> ...
> 373 6.  Advantages and Deployment Considerations
>
> 375   The solutions described provide a model for integrating the client
> 376   perspective into the best path computation for route reflectors.
> 377   More specifically, the choice of BGP path factors in either the IGP
> 378   cost between the client and the nexthop (rather than the IGP cost
> 379   from the route reflector to the nexthop) or other user configured
> 380   policies.
>
> [] "solutions"?   I only see one, where the "second" one simply adds
> policy, which is a pervasive tool in BGP.
>
>
> 382   The achievement of optimal routing relies upon all route reflectors
> 383   learning all paths that are eligible for consideration.  In order to
> 384   satisfy this requirement, path diversity enhancing mechanisms such
> as
> 385   BGP add-path [RFC7911] may need to be deployed between route
> 386   reflectors.
>
> [mayor] "path diversity enhancing mechanisms...may need to be deployed
> between route reflectors"  Given that this is the Deployment Considerations
> section, please provide guidance on when these mechanisms are needed, and
> when they're not.
>
>
> [minor] "path diversity enhancing mechanisms such as BGP add-path
> [RFC7911]"  Just out of curiosity, what other mechanisms are you thinking
> of?  Are there deployment considerations, when using ORR, to prefer one?
>
>
> 388   Implementations considered compliant with this document allow the
> 389   configuration of a logical location from which the best path will be
> 390   computed, on the basis of either a peer, a peer group, or an entire
> 391   routing instance.
>
> [] I guess that "logical location" is another name for IGP location...
>
>
> [major] §3.1 says that the "configuration of the IGP location is outside
> of the scope of this document".  If that is true, then the configuration
> cannot be a consideration for compliance.
>
> A better wording for compliance would be: "An implementation MUST allow
> the configuration..."
>
>
> 393   These solutions can be deployed in traditional hop-by-hop forwarding
> 394   networks as well as in end-to-end tunneled environments.  In
> networks
> 395   where there are multiple route reflectors and hop-by-hop forwarding
> 396   without encapsulation, such optimizations SHOULD be enabled in a
> 397   consistent way on all route reflectors.  Otherwise, clients may
> 398   receive an inconsistent view of the network, in turn leading to
> 399   intra-domain forwarding loops.
>
> [major] "optimizations SHOULD be enabled in a consistent way on all route
> reflectors"
>
> Is "a consistent way" different than "on all"?   s/.../optimizations
> SHOULD be enabled on all route reflectors
>
> When is it ok for all RRs not to be enabled with the optimizations?  IOW,
> why is it recommended and not required?  Avoiding "intra-domain forwarding
> loops" sounds like a good reason to require it.
>
>
> ...
> 406   As per above, these approaches reduce the amount of state which
> needs
> 407   to be pushed to the edge of the network in order to perform hot
> 408   potato routing.  The memory and CPU resources required at the edge
> of
> 409   the network to provide hot potato routing using these approaches is
> 410   lower than what would be required to achieve the same level of
> 411   optimality by pushing and retaining all available paths (potentially
> 412   10s) per each prefix at the edge.
>
> [] This paragraph sounds very speculative and doesn't seem necessary.  In
> line with §5.
>
>
> 414   The solutions above allow for a fast and safe transition to a BGP
> 415   control plane using centralized route reflection, without
> 416   compromising an operator's closest exit operational principle.  This
> 417   enables edge-to-edge LSP/IP encapsulation for traffic to IPv4 and
> 418   IPv6 prefixes.
>
> [] "allow for a fast and safe transition"  Besides this statement sounding
> like a marketing brochure, if you're going to talk about transition, please
> talk about it in detail.  Please take a look at §2/rfc5706 and include
> details about the transition and how it is "fast and safe".
>
>
> 420   Regarding Best Path Selection from a different IGP location, it
> 421   should be self evident that this solution does not interfere with
> 422   policies enforced above IGP tie-breaking in the BGP best path
> 423   algorithm.
>
> [minor] Don't assume anything is self evident to anyone.
>
> Suggestion (maybe more appropriate in §3.1)>
>    The modification specified in Section 3.1 does not impact any previous
>    considerations in the BGP Decision Process.
>
>
> 425 7.  Security Considerations
>
> 427   Similarly to [RFC4456], this extension to BGP does not change the
> 428   underlying security issues inherent in the existing IBGP [RFC4456].
>
> [minor] No need to reference rfc4456 twice in the same sentence.
>
>
> 430   It however enables the deployment of base BGP Route Reflection as
> 431   described in [RFC4456] to be possible using virtual compute
> 432   environments without any negative consequence on the BGP routing
> path
> 433   optimality.
>
> [] I'm not sure how this relates to the specification itself (platform
> independent), but I'm sure the Security ADs will have a lot of questions
> about the security of "virtual compute environments" -- and where the
> details are included in this document.   IOW, that sounds like an
> unnecessary assertion.
>
>
> [major] "without any negative consequence"  The new functionality
> specified in this document is having the RR run the client's selection for
> them (even specific policy) -- there are risks related to the duplication
> of the policy, the location of the RRs and the blind trust that the clients
> need to have.  Or should the clients rerun the policy locally?
>
> This behavior is not necessarily different from normal RR, but the
> instantiation is different.   It would be very easy for a rogue RR to
> propagate incorrect routing information to its clients -- specially if
> policy is offloaded to them.
>
> In the worst case the result would be a sub-optimal route (maybe even
> worse than what the clients get today), but probably not a "real" security
> issue.  It is probably a good idea to explain why it is not a security
> risk.
>
>
> 435   This document does not introduce requirements for any new protection
> 436   measures, but it also does not relax best operational practices for
> 437   keeping the IGP network stable or to pace rate of policy based IGP
> 438   cost to next hops such that it does not have any substantial effect
> 439   on BGP path changes and their propagation to route reflection
> 440   clients.
>
> [major] "best operational practices"  Like what?  It would be good to
> provide (at least) informational references.
>
>
> [major] "to pace rate of policy based IGP cost to next hops such that it
> does not have any substantial effect on BGP path changes"   You will have
> to explain this further because the operation of an IGP is not mentioned
> anywhere else.
>
> IMHO, there's no need to go into IGP operational details in this
> document.  There is nothing special/different about the operation of an
> IGP.
>
>
> [End of Review]
>
>
>
>