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

[Alvaro Retana <[e-mail]>]

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]