From 220f0f4245e36cd170bea79e501de43ac46e428a Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 13 Dec 2018 09:21:26 -0500 Subject: [PATCH] zebra: Allow zebra to only mark up to multipath_num nexthops as ACTIVE NEXTHOP_FLAG_ACTIVE currently means that the nexthop is considered good enough to be installed. With current ecmp restrictions this translation from multipath_num is enforced in the data plane. The problem with this is of course that every data plane now becomes concerned about the multipath num and must enforce it independently. Currently *bsd does not honor multipath_num at all and linux marks all nexthops as being installed even when it honors a multipath_num that is less than the total. This code change moves the multipath_num enforcement from a dataplane decision to a zebra nexthop decision. Thus dataplanes now can just install those nexthops marked as NEXTHOP_FLAG_ACTIVE without having to worry about multipath_num. *BSD will now respect multipath_num and Linux now properly notes which routes are actually installed or not: sharpd@donna ~/f/t/topotests> ps -ef | grep frr frr 6261 1556 0 09:12 ? 00:00:00 /usr/lib/frr/zebra -e 2 --daemon -A 127.0.0.1 frr 6279 1556 0 09:12 ? 00:00:00 /usr/lib/frr/staticd --daemon -A 127.0.0.1 donna.cumulusnetworks.com(config)# do show ip route Codes: K - kernel route, C - connected, S - static, R - RIP, O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP, T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP, F - PBR, f - OpenFabric, > - selected route, * - FIB route K>* 0.0.0.0/0 [0/106] via 10.0.2.2, enp0s3, 00:00:45 S>* 4.4.4.4/32 [1/0] via 10.0.2.1, enp0s3, 00:00:02 * via 192.168.209.1, enp0s8, 00:00:02 via 192.168.210.1, enp0s9 inactive, 00:00:02 C>* 10.0.2.0/24 is directly connected, enp0s3, 00:00:45 C>* 192.168.209.0/24 is directly connected, enp0s8, 00:00:45 C>* 192.168.210.0/24 is directly connected, enp0s9, 00:00:45 donna.cumulusnetworks.com(config)# sharpd@donna ~/f/t/topotests> ip route show default via 10.0.2.2 dev enp0s3 proto dhcp metric 106 4.4.4.4 proto 196 metric 20 nexthop via 10.0.2.1 dev enp0s3 weight 1 nexthop via 192.168.209.1 dev enp0s8 weight 1 10.0.2.0/24 dev enp0s3 proto kernel scope link src 10.0.2.15 metric 106 172.17.0.0/16 dev docker0 proto kernel scope link src 172.17.0.1 linkdown 192.168.122.0/24 dev virbr0 proto kernel scope link src 192.168.122.1 linkdown 192.168.209.0/24 dev enp0s8 proto kernel scope link src 192.168.209.2 metric 105 192.168.210.0/24 dev enp0s9 proto kernel scope link src 192.168.210.2 metric 103 Signed-off-by: Donald Sharp --- zebra/rt_netlink.c | 16 ++-------------- zebra/zebra_rib.c | 13 ++++++++++++- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c index cb9ef8e36f..8ce963b37b 100644 --- a/zebra/rt_netlink.c +++ b/zebra/rt_netlink.c @@ -1584,7 +1584,7 @@ static int netlink_route_multipath(int cmd, struct zebra_dplane_ctx *ctx) } /* Singlepath case. */ - if (nexthop_num == 1 || multipath_num == 1) { + if (nexthop_num == 1) { nexthop_num = 0; for (ALL_NEXTHOPS_PTR(dplane_ctx_get_ng(ctx), nexthop)) { /* @@ -1676,9 +1676,6 @@ static int netlink_route_multipath(int cmd, struct zebra_dplane_ctx *ctx) nexthop_num = 0; for (ALL_NEXTHOPS_PTR(dplane_ctx_get_ng(ctx), nexthop)) { - if (nexthop_num >= multipath_num) - break; - if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE)) { /* This only works for IPv4 now */ @@ -1876,12 +1873,6 @@ enum zebra_dplane_result kernel_route_update(struct zebra_dplane_ctx *ctx) if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE)) { SET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB); - - /* If we're only allowed a single nh, don't - * continue. - */ - if (multipath_num == 1) - break; } } } @@ -2698,7 +2689,7 @@ int netlink_mpls_multipath(int cmd, zebra_lsp_t *lsp) /* Fill nexthops (paths) based on single-path or multipath. The paths * chosen depend on the operation. */ - if (nexthop_num == 1 || multipath_num == 1) { + if (nexthop_num == 1) { routedesc = "single-path"; _netlink_mpls_debug(cmd, lsp->ile.in_label, routedesc); @@ -2745,9 +2736,6 @@ int netlink_mpls_multipath(int cmd, zebra_lsp_t *lsp) if (!nexthop) continue; - if (nexthop_num >= multipath_num) - break; - if ((cmd == RTM_NEWROUTE && (CHECK_FLAG(nhlfe->flags, NHLFE_FLAG_SELECTED) && CHECK_FLAG(nexthop->flags, diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 0c8c777ea5..2dfeb26915 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -1049,7 +1049,18 @@ static int nexthop_active_update(struct route_node *rn, struct route_entry *re, prev_src = nexthop->rmap_src; prev_active = CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE); prev_index = nexthop->ifindex; - if ((new_active = nexthop_active_check(rn, re, nexthop, set))) + /* + * We need to respect the multipath_num here + * as that what we should be able to install from + * a multipath perpsective should not be a data plane + * decision point. + */ + new_active = nexthop_active_check(rn, re, nexthop, set); + if (new_active && re->nexthop_active_num >= multipath_num) { + UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE); + new_active = 0; + } + if (new_active) re->nexthop_active_num++; /* Don't allow src setting on IPv6 addr for now */ if (prev_active != new_active || prev_index != nexthop->ifindex