IP-in-IP does not compose with ECMP routes

[nelljerram]

In a dual ToR setup (attempt), with ECMP routes to some node loopback IPs and pod /26 blocks, we tried enabling IP-in-IP (to address a separate bootstrapping issue), and observed that most of the routes did not change to IP-in-IP routes:

ubuntu@lance-ipv6:~$ docker exec kind-worker ip r
default via 172.31.12.2 dev eth1 src 172.31.12.3
172.31.10.0/23 src 172.31.10.4
        nexthop dev eth0 weight 1
        nexthop dev eth1 weight 1
172.31.11.0/24 dev eth0 proto kernel scope link src 172.31.11.3
172.31.12.0/24 dev eth1 proto kernel scope link src 172.31.12.3
172.31.20.0/23 src 172.31.10.4
        nexthop via 172.31.11.1 dev eth0 weight 1
        nexthop via 172.31.12.1 dev eth1 weight 1
172.31.21.0/24 via 172.31.11.1 dev eth0
172.31.22.0/24 via 172.31.12.1 dev eth1
192.168.82.0/26 via 172.31.10.3 dev tunl0 proto bird onlink
192.168.110.128/26 proto bird
        nexthop via 172.31.11.1 dev eth0 weight 1
        nexthop via 172.31.12.1 dev eth1 weight 1
blackhole 192.168.162.128/26 proto bird
192.168.162.129 dev calif2ae67c76cf scope link
192.168.162.130 dev calic1faebfe4cf scope link
192.168.162.131 dev calib66283a5cca scope link
192.168.162.133 dev calid730704be43 scope link
192.168.195.192/26 proto bird
        nexthop via 172.31.11.1 dev eth0 weight 1
        nexthop via 172.31.12.1 dev eth1 weight 1

In fact, only the non-ECMP routes change to go via the IP-in-IP tunnel device tunl0.

On reviewing the BIRD code, it's clear this is because of lack of support in the BIRD code. There are two places where the code handles EA_KRT_TUNNEL, and in both places the handling is also conditional on RTD_ROUTER - which means "a route with a single path". In both places we would need to add corresponding code for RTD_MULTIPATH.

Does this matter?

Well, it depends if there are any use cases for ECMP routes while IP-in-IP is in use. The dual ToR work unconditionally added "merge paths on" to the BIRD config, which means that any TSEE deployment would program an ECMP route if it received more than one possible path for a given prefix. We should perhaps make that conditional somehow.

[nelljerram]

I’ve spent some time looking at the work that would be needed, and it's quite hard. I think it would be better for us to spend equivalent time and effort on rethinking and eliminating the BIRD code hacks that we’ve made for IP-in-IP, replacing them with

• a new design for how we program IP-in-IP tunnel routes
• upstreaming any minimal missing BIRD code pieces that we need for that.

I will record the relevant history, and my analysis, in following comments here.

[nelljerram]

I began a discussion on the BIRD ML in 2016, about upstreaming our BIRD patches for IP-in-IP: https://bird.network.cz/pipermail/bird-users/2016-September/010629.html

The maintainer (Ondrej)’s response to that was that we should only need to change BIRD to allow setting “onlink” and “iface” from a filter, and then we could achieve everything else we need in config. I now think that is probably true, and the right way to go. In fact, it might only be “onlink” that we need a code change for, because I think there is already a writable “ifname” property that a filter can set.

I began the changes, for extending our current hacks for ECMP, as you can see at nelljerram@fdf04ac.

However, the thing that is hard, about that, is that we don’t have storage equivalent to orig_gw for the possible paths of an ECMP route. We could probably add that, but it would need understanding when and how the route merging happens in BIRD, and from a quick look that did not seem straightforward.

As I researched this, it started looking like we might actually not need orig_gw at all, because that information is probably already available as the BA_NEXT_HOP eattr. That led me down the path of reviewing our overall design here, and of whether we need all our current hacks.

[nelljerram]

Here is a review of our current BIRD code changes, and the filter config that we use in conjunction with those:

  +--------+  [1] [2]  +---------+    [4]    +----------+  [5]
  | Local  |<--export--| BIRD    |--export-->| BGP      |<-------
  | Kernel |           | routing |           | protocol |         BGP peers
  |        |--import-->| table   |<--import--|          |------->
  +--------+    [3]    +---------+     all   +----------+

[1] calico_kernel_programming filter:

    reject if net ~ /rejectcidrs
    reject IPv4 IP pool CIDRs with VXLAN mode
    set EA_KRT_TUNNEL and accept if in IP pool with IP-IP
    accept everything else

    Instead of setting EA_KRT_TUNNEL:

        onlink = true
        gw = nexthop
        ifname = tunl0

[2] when programming route into local kernel

    netlink.c: nl_send_route: if EA_KRT_TUNNEL provided, use it, with orig_gw
    i.e. set gw and oif

        From nl_add_rte, from krt_replace_rte

    This is designed for when BIRD has received a route from a BGP
    peer (which might not be directly connected).  After receipt by
    the local BGP, and import into the routing table, the route will
    be like

        10.65.128.192/26 via <resolved next hop> dev eth0

    When BGP peers are directly connected, <resolved next hop> will be
    the peer IP.  For GCP, <resolved next hop> will be the GCP gateway
    10.240.0.1.  For cases like GCP, where we use IP-IP to get through
    the gateway, we want to program that as

        10.65.128.192/26 via <IP of host for that block> dev tunl0 onlink

    If that route came to us directly from BIRD on the relevant host,
    we could get <IP of host for that block> as the route's "from"
    attribute.  But that would be wrong if it came via an RR.  In both
    cases the right IP is whatever was in the NEXTHOP attribute of the
    BGP UPDATE message, so that's what we should use, and that's what
    our "orig_gw" change is achieving.

[3] when importing route from local kernel to BIRD routing table

    netlink.c: nl_parse_multipath: Always NEF_ONLINK in neigh_find2 call
    netlink.c: nl_parse_route: Always NEF_ONLINK in neigh_find2 call

    krt.c: krt_same_dest: Tunnel flapping change
    krt.c: krt_got_route: Tunnel flapping change

        From nl_announce_route, from nl_parse_route/nl_parse_end

[4] bgp_import_control
      bgp_update_attrs if route came from BGP
        Set next hop to self if:
          No existing next hop ||
          "next hop self" ||
          (!"next hop keep" && more conditions...)
      bgp_create_attrs otherwise
        Set next hop to self if:
          "next hop self" ||
          rta->dest != RTD_ROUTER ||
          rta->gw is none or is link local ||
          (!"next hop keep" && more conditions...)
        Otherwise set next hop to rta->gw

    filter calico_export_to_bgp_peers:

    	accept block CIDRs
    	reject more specific routes inside block CIDRs
    	accept if net ~ /staticroutes
    	accept if net ~ IPv4 IP pool CIDRs
    	reject

[5] On receipt of UPDATE message from a BGP peer

bgp_rx_update
  bgp_do_rx_update
    bgp_decode_attrs
      Maps BGP UPDATE attrs to struct rta
    bgp_set_next_hop
      direct vs recursive algorithm here
      Calico: set orig_gw to BGP nexthop, before possibly changing the latter
      rta_set_recursive_next_hop
    bgp_rte_update
      rte_update2
        Run import filter
        Add to BIRD routing table

    packets.c: bgp_set_next_hop: 'a->orig_gw = *nexthop' just before rta_set_recursive_next_hop call

       This is called from bgp_do_rx_update (two versions, IPv4 and IPv6),
       from bgp_rx_update, so on receipt of a BGP UPDATE message from a
       peer.

(Another view of the same thing is at #37 (comment), but I believe the ASCII art above is a better representation of the BIRD internal architecture, and it also shows where the relevant import and export filters are involved.)

[nelljerram]

(In addition to the points above where code is actually doing things, there are these infrastructure changes, between upstream and our fork, to facilitate the above:

- filter.c: Add EAF_TYPE_STRING
- filter.c: Earlier declaration of two variables used for EAF_TYPE_IP_ADDRESS
- route.h: Add 'orig_gw' field next to 'gw'
- route.h: Add EAF_TYPE_STRING
- krt-sys.h: Add EA_KRT_TUNNEL
- netlink.Y: Add KRT_TUNNEL (x2)
- netlink.c: krt_sys_get_addr: Add EA_KRT_TUNNEL
- config.h: "v1.6.3" -> "v0.3.2+birdv1.6.3"

)

[nelljerram]

Other useful input is from the PR at #21. The contributor was proposing to update the iface for a tunnel route in the BIRD routing table, instead of only when exporting it to the local kernel. At the time I favoured a less dramatic change, but I now think the contributor’s approach may have been along better lines.

[nelljerram]

Putting all of that together, it now seems to me that - maybe -

• we don’t need the orig_gw field, because the same information is available as the BA_NEXT_HOP attribute, and as “next_hop” in filters

• the only code change we need is to allow the BGP import filter to set “onlink”

• then, for tunnel routes, we simply define a BGP import filter that does

        onlink = true
        gw = nexthop
        ifname = tunl0

Ideally, then, we’d upstream the “onlink” setting capability, and then we’d no longer need our own BIRD fork.

However, there are these things I’m not yet sure about, and that need more review or testing:

• Need to check that that solution corrects non-tunnel routes that are present when BIRD starts up. (https://github.com/projectcalico/calicoctl/issues/947)

• Need to check that that solution does not lead to flapping of the tunnel routes. (Fix change detection logic for tunnel routes. #37)

Finally, after all that, we still need to work out how to do the equivalent of that import filter code for each path of an ECMP route. But I think it would be much easier to ask about that - and to upstream any further code changes that might be needed for it - if we were using the upstream BIRD code.

[nelljerram]

The first step of the above plan - removing the orig_gw field and using BA_NEXT_HOP instead - has been done by #75 and projectcalico/node#380.

Tigera themselves have no current schedule for pursuing other pieces of the plan above, so contributions would be very welcome.

[rcarpa]

Hi team. This message is just to inform you that latest bird release (2.0.12) implemented setting the onlink attribute on routes https://gitlab.nic.cz/labs/bird/-/commit/7144c9ca46f092da33a4e051bbce8f973a3bd8c4

[nelljerram]

Thanks @rcarpa . It would take a bit of work for us to update from our current code based on 1.6.8, but we certainly might do that in future, and it's useful to know that a version of this particular tweak is now upstream.

[jjqq2013]

I have confirmed that the following version still has the issue.

BIRD version v0.3.3+birdv1.6.8

Is there any version that have fixed this issue?

[rcarpa]

@jjqq2013 , you probably miss-understood my comment. It's about vanila upstream bird 2.0.12 supporting a feature which would allow the calico project to switch from their old fork of bird to upstream bird. When this is done, ECMP support should (tm) improve. I think it would even be possible to advertise multiple routes to the same node via bgp add-path and solve the issue of imbalanced traffic to services having externalTrafficPolicy = Local. However, as of today, there was no work done to perform this migration on the calico side. This issue is not fixed yet. Also, I have no links to the calico development team, but I would assume nobody is even working on it yet ;)

[jjqq2013]

@rcarpa thank you for your explanation.

I am new to this project.

To be sure that the issue is same, please let me explain the issue I am facing. If it is a different issue, I will create a new ticket.

There is a server with has following ip routes, i.e., ECMP

100.z.z.z/m proto bgp src 100.117.134.89 metric 20
        nexthop via inet6 fe80::920a:84ff:fe7e:f16c dev eth0 weight 1
        nexthop via inet6 fe80::920a:84ff:fe7e:db6c dev eth1 weight 1

What I expect is the BIRD create ip routes like:

...
192.168.x.x/y via 100.z.z.z dev tunl0 proto bird onlink
...

according to actual peer status.

However, the actual result is like:

...
unreachable 192.168.x.x/y proto bird
...

A notable thing is that: if I don't use ECMP, then BIRD works.

i.e., if I change the route from

100.z.z.z/m proto bgp src 100.117.134.89 metric 20
        nexthop via inet6 fe80::920a:84ff:fe7e:f16c dev ens3f1.1037 weight 1
        nexthop via inet6 fe80::920a:84ff:fe7e:db6c dev ens3f0.1037 weight 1

to

100.z.z.z/m nhid 14 via inet6 fe80::920a:84ff:fe7e:f16c dev eth0 proto bgp src 100.117.134.89 metric 20

then BIRD will produce

192.168.x.x/y via 100.z.z.z dev tunl0 proto bird onlink

PS, the BIRD config actually does not matter that much, because it can produce expected above result when not using ECMP.

Just in case, let me paste part of BIRD config:

...
filter calico_kernel_programming {

  if ( net ~ 192.168.0.0/16 ) then {
    krt_tunnel = "tunl0";                     ################## this is the famous hacking
    accept;
  }

  accept;
}

router id 100.117.134.89;

# Configure synchronization between routing tables and kernel.
protocol kernel {
  learn;             # Learn all alien routes from the kernel
  persist;           # Don't remove routes on bird shutdown
  scan time 2;       # Scan kernel routing table every 2 seconds
  import all;
  export filter calico_kernel_programming; # Default is export none
  graceful restart;  # Turn on graceful restart to reduce potential flaps in
                     # routes when reloading BIRD configuration.  With a full
                     # automatic mesh, there is no way to prevent BGP from
                     # flapping since multiple nodes update their BGP
                     # configuration at the same time, GR is not guaranteed to
                     # work correctly in this scenario.
}

...

# Template for all BGP clients
template bgp bgp_template {
  debug { states };
  description "Connection to BGP peer";
  local as 64512;
  multihop;
  gateway recursive; # This should be the default, but just in case.
  import all;        # Import all routes, since we don't know what the upstream
                     # topology is and therefore have to trust the ToR/RR.
  export filter calico_export_to_bgp_peers;  # Only want to export routes for workloads.
  source address 100.117.134.89;  # The local address we use for the TCP connection
  add paths on;
  graceful restart;  # See comment in kernel section about graceful restart.
  connect delay time 2;
  connect retry time 5;
  error wait time 5,30;
}

protocol bgp Mesh_100_89_27_72 from bgp_template {
  neighbor 100.89.27.72 as 64512;
}

... many bgp peers ..., omitted here

[jjqq2013]

I think the issue I mentioned happens like this:

• BIRD got BGP peer info: host 100.z.z.z has an 192.168.x.x behind dev tunl0.
• BIRD prepare an ip route of how to reach 192.168.x.x/y
• BIRD knows it should use dev tunl0, and should use via 100.z.z.z
• However, BIRD want to make sure that the 100.z.z.z is reachable, so BIRD check the kernel ip route table
• BIRD could not find proper route to 100.z.z.z, because it can not understand the ECMP route

100.z.z.z/m proto bgp src 100.117.134.89 metric 20
        nexthop via inet6 fe80::920a:84ff:fe7e:f16c dev ens3f1.1037 weight 1
        nexthop via inet6 fe80::920a:84ff:fe7e:db6c dev ens3f0.1037 weight 1

• BIRD think 100.z.z.z is unreachable
• BIRD produces ip route unreachable 192.168.x.x/y proto bird

Am I correct?