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 <sharpd@cumulusnetworks.com>

diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c
index cb9ef8e36f8dc0ac0cb7c1782bc03bb233592a70..8ce963b37b52bce522547d38295d51fd148598cf 100644 (file)
--- 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 0c8c777ea56695e01d636af9945b15d1f2d54494..2dfeb269158460d205910dc1bb90f5cefbf07b00 100644 (file)
--- 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