From 9d221fac7ef25e110b1d4bce82e2047b0e1e0c9c Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 30 Sep 2020 17:55:44 -0400 Subject: [PATCH] zebra: When processing route_entries ignore unusable routes When zebra is processing routes to determine what to send to the rib, suppose we have two routes (a) a route processed earlier that none of it's nexthops were active and (b) a route that has good nexthops but has a worse admin distance. rib_process, would not relook at (a)'s nexthops because the ROUTE_ENTRY_CHANGED flag was not true and it would win when compared to (b) because it's admin distance was better, leaving us with a state where we would attempt and fail to install route (a) because it was not valid. Modify the code to consider the number of nexthops we have as a determiner if we can use the route. Signed-off-by: Donald Sharp --- zebra/zebra_rib.c | 86 ++++++++++++++++++++++++++--------------------- 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 4e08c327cf..8231d26f5b 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -1098,46 +1098,56 @@ static void rib_process(struct route_node *rn) if (CHECK_FLAG(re->status, ROUTE_ENTRY_REMOVED)) continue; - /* Skip unreachable nexthop. */ - /* This first call to nexthop_active_update is merely to - * determine if there's any change to nexthops associated - * with this RIB entry. Now, rib_process() can be invoked due - * to an external event such as link down or due to - * next-hop-tracking evaluation. In the latter case, - * a decision has already been made that the NHs have changed. - * So, no need to invoke a potentially expensive call again. - * Further, since the change might be in a recursive NH which - * is not caught in the nexthop_active_update() code. Thus, we - * might miss changes to recursive NHs. + /* + * If the route entry has changed, verify/resolve + * the nexthops associated with the entry. + * + * In any event if we have nexthops that are not active + * then we cannot use this particular route entry so + * skip it. */ - if (CHECK_FLAG(re->status, ROUTE_ENTRY_CHANGED) - && !nexthop_active_update(rn, re)) { - if (re->type == ZEBRA_ROUTE_TABLE) { - /* XXX: HERE BE DRAGONS!!!!! - * In all honesty, I have not yet figured out - * what this part does or why the - * ROUTE_ENTRY_CHANGED test above is correct - * or why we need to delete a route here, and - * also not whether this concerns both selected - * and fib route, or only selected - * or only fib - * - * This entry was denied by the 'ip protocol - * table' route-map, we need to delete it */ - if (re != old_selected) { - if (IS_ZEBRA_DEBUG_RIB) - zlog_debug( - "%s: %s(%u):%s: imported via import-table but denied by the ip protocol table route-map", - __func__, - VRF_LOGNAME(vrf), - vrf_id, buf); - rib_unlink(rn, re); - } else - SET_FLAG(re->status, - ROUTE_ENTRY_REMOVED); - } + if (CHECK_FLAG(re->status, ROUTE_ENTRY_CHANGED)) { + if (!nexthop_active_update(rn, re)) { + if (re->type == ZEBRA_ROUTE_TABLE) { + /* XXX: HERE BE DRAGONS!!!!! + * In all honesty, I have not yet + * figured out what this part does or + * why the ROUTE_ENTRY_CHANGED test + * above is correct or why we need to + * delete a route here, and also not + * whether this concerns both selected + * and fib route, or only selected + * or only fib + * + * This entry was denied by the 'ip + * protocol + * table' route-map, we need to delete + * it */ + if (re != old_selected) { + if (IS_ZEBRA_DEBUG_RIB) + zlog_debug( + "%s: %s(%u):%s: imported via import-table but denied by the ip protocol table route-map", + __func__, + VRF_LOGNAME( + vrf), + vrf_id, buf); + rib_unlink(rn, re); + } else + SET_FLAG(re->status, + ROUTE_ENTRY_REMOVED); + } - continue; + continue; + } + } else { + /* + * If the re has not changed and the nhg we have is + * not usable, then we cannot use this route entry + * for consideration, as that the route will just + * not install if it is selected. + */ + if (!nexthop_group_active_nexthop_num(&re->nhe->nhg)) + continue; } /* Infinite distance. */ -- 2.39.5