Skip to content

Commit

Permalink
Don't configure "next hop self"
Browse files Browse the repository at this point in the history
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);
  else
    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
wrong.

Net: unless/until we can identify any scenarios where "next hop self" is
definitively needed, we should omit it.
  • Loading branch information
Neil Jerram committed Sep 17, 2018
1 parent cbce279 commit c6177d3
Showing 1 changed file with 0 additions and 8 deletions.
8 changes: 0 additions & 8 deletions filesystem/etc/calico/confd/templates/bird.cfg.template
Expand Up @@ -73,8 +73,6 @@ template bgp bgp_template {
{{if eq $onode_ip ($node_ip) }}# Skipping ourselves ({{$node_ip}})
{{else if ne "" $onode_ip}}protocol bgp Mesh_{{$id}} from bgp_template {
neighbor {{$onode_ip}} as {{if exists $onode_as_key}}{{getv $onode_as_key}}{{else}}{{getv "/global/as_num"}}{{end}};
next hop self; # Disable next hop processing and always advertise our
# local address as nexthop
}{{end}}{{end}}{{end}}
{{else}}
# Node-to-node mesh disabled
Expand All @@ -95,9 +93,6 @@ protocol bgp Global_{{$id}} from bgp_template {
{{- if and (ne "" ($node_cluster_id)) (ne $data.rr_cluster_id ($node_cluster_id))}}
rr client;
rr cluster id {{$node_cluster_id}};
{{- else}}
next hop self; # Disable next hop processing and always advertise our
# local address as nexthop
{{- end}}
}
{{- end}}
Expand All @@ -119,9 +114,6 @@ protocol bgp Node_{{$id}} from bgp_template {
{{- if and (ne "" ($node_cluster_id)) (ne $data.rr_cluster_id ($node_cluster_id))}}
rr client;
rr cluster id {{$node_cluster_id}};
{{- else}}
next hop self; # Disable next hop processing and always advertise our
# local address as nexthop
{{- end}}
}
{{- end}}
Expand Down

0 comments on commit c6177d3

Please sign in to comment.