New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Streamline templating for 'rr client' vs 'next hop self' #55

merged 2 commits into from Sep 17, 2018


None yet
2 participants

neiljerram commented Sep 13, 2018

calico/node no longer configures next hop self on all BGP peer configurations. 

neiljerram added some commits Sep 13, 2018

Don't configure "next hop self"
As far as we currently understand, there are no routes / scenarios for
which "next hop self" is actually needed, and it is actively harmful to
have "next hop self" for routes that are being reflected.  So the
practical solution is not to configure "next hop self" at all.

Here is the relevant BIRD code for what "next hop self" does:

  if (p->cf->next_hop_self ||
      rta->dest != RTD_ROUTER ||
      ipa_equal(rta->gw, IPA_NONE) ||
      ipa_is_link_local(rta->gw) ||
      (!p->is_internal && !p->cf->next_hop_keep &&
       (!p->neigh || (rta->iface != p->neigh->iface))))
    set_next_hop(z, p->source_addr);
    set_next_hop(z, rta->gw);

In other words,

(1) If "next hop self" is set, the behaviour is to set the route's next
hop to the peering's local address.

(2) That also happens, without "next hop self" being set, for
non-gateway routes - which include the local Calico endpoint routes that
we are primarily interested in.  When originating those routes, we do
want the next hop that "next hop self" would give us, but the code above
makes clear that we will get that anyway.

On the other hand, if "next hop self" accidentally applies to a route
that is being reflected, the effect is to insert the route reflector
into the data path for traffic to that route - which is absolutely

Net: unless/until we can identify any scenarios where "next hop self" is
definitively needed, we should omit it.

@neiljerram neiljerram force-pushed the neiljerram:dedupe-peer-templating branch from 531fb00 to c6177d3 Sep 17, 2018

@neiljerram neiljerram requested a review from caseydavenport Sep 17, 2018

@caseydavenport caseydavenport added this to the Calico v3.3.0 milestone Sep 17, 2018

@neiljerram neiljerram merged commit 8eb57a2 into projectcalico:master Sep 17, 2018

2 checks passed

license/cla Contributor License Agreement is signed.
semaphoreci The build passed on Semaphore.

@neiljerram neiljerram deleted the neiljerram:dedupe-peer-templating branch Sep 17, 2018

fasaxc added a commit that referenced this pull request Nov 30, 2018

Merge pull request #55 from tigera/calsoft-DNS-netconf-entry
Add DNS entries in cni.conf.template.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment