diff options
| -rw-r--r-- | lib/nexthop.h | 3 | ||||
| -rw-r--r-- | lib/nexthop_group.c | 24 | ||||
| -rw-r--r-- | lib/nexthop_group.h | 5 | ||||
| -rw-r--r-- | tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/adjacencies.py | 4 | ||||
| -rw-r--r-- | zebra/zebra_rib.c | 174 |
5 files changed, 104 insertions, 106 deletions
diff --git a/lib/nexthop.h b/lib/nexthop.h index 24b0953191..ad87c2e522 100644 --- a/lib/nexthop.h +++ b/lib/nexthop.h @@ -81,8 +81,7 @@ struct nexthop { #define NEXTHOP_FLAG_RECURSIVE (1 << 2) /* Recursive nexthop. */ #define NEXTHOP_FLAG_ONLINK (1 << 3) /* Nexthop should be installed onlink. */ #define NEXTHOP_FLAG_MATCHED (1 << 4) /* Already matched vs a nexthop */ -#define NEXTHOP_FLAG_FILTERED (1 << 5) /* rmap filtered, used by static only */ -#define NEXTHOP_FLAG_DUPLICATE (1 << 6) /* nexthop duplicates another active one */ +#define NEXTHOP_FLAG_DUPLICATE (1 << 5) /* nexthop duplicates another active one */ #define NEXTHOP_IS_ACTIVE(flags) \ (CHECK_FLAG(flags, NEXTHOP_FLAG_ACTIVE) \ && !CHECK_FLAG(flags, NEXTHOP_FLAG_DUPLICATE)) diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c index fa89b7708c..ed22f64494 100644 --- a/lib/nexthop_group.c +++ b/lib/nexthop_group.c @@ -59,6 +59,30 @@ nexthop_group_cmd_compare(const struct nexthop_group_cmd *nhgc1, return strcmp(nhgc1->name, nhgc2->name); } +uint8_t nexthop_group_nexthop_num(const struct nexthop_group *nhg) +{ + struct nexthop *nhop; + uint8_t num = 0; + + for (ALL_NEXTHOPS_PTR(nhg, nhop)) + num++; + + return num; +} + +uint8_t nexthop_group_active_nexthop_num(const struct nexthop_group *nhg) +{ + struct nexthop *nhop; + uint8_t num = 0; + + for (ALL_NEXTHOPS_PTR(nhg, nhop)) { + if (CHECK_FLAG(nhop->flags, NEXTHOP_FLAG_ACTIVE)) + num++; + } + + return num; +} + struct nexthop *nexthop_exists(struct nexthop_group *nhg, struct nexthop *nh) { struct nexthop *nexthop; diff --git a/lib/nexthop_group.h b/lib/nexthop_group.h index f68033c20c..5adf2db937 100644 --- a/lib/nexthop_group.h +++ b/lib/nexthop_group.h @@ -117,6 +117,11 @@ extern struct nexthop_group_cmd *nhgc_find(const char *name); extern void nexthop_group_write_nexthop(struct vty *vty, struct nexthop *nh); +/* Return the number of nexthops in this nhg */ +extern uint8_t nexthop_group_nexthop_num(const struct nexthop_group *nhg); +extern uint8_t +nexthop_group_active_nexthop_num(const struct nexthop_group *nhg); + #ifdef __cplusplus } #endif diff --git a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/adjacencies.py b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/adjacencies.py index 28ecfeec5a..5674120b9c 100644 --- a/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/adjacencies.py +++ b/tests/topotests/bgp_l3vpn_to_bgp_vrf/scripts/adjacencies.py @@ -1,7 +1,7 @@ from lutil import luCommand luCommand('ce1','vtysh -c "show bgp summary"',' 00:0','wait','Adjacencies up',180) -luCommand('ce2','vtysh -c "show bgp summary"',' 00:0','wait','Adjacencies up') -luCommand('ce3','vtysh -c "show bgp summary"',' 00:0','wait','Adjacencies up') +luCommand('ce2','vtysh -c "show bgp summary"',' 00:0','wait','Adjacencies up',180) +luCommand('ce3','vtysh -c "show bgp summary"',' 00:0','wait','Adjacencies up',180) luCommand('ce4','vtysh -c "show bgp vrf all summary"',' 00:0','wait','Adjacencies up',180) luCommand('r1','ping 2.2.2.2 -c 1',' 0. packet loss','wait','PE->P2 (loopback) ping',60) luCommand('r3','ping 2.2.2.2 -c 1',' 0. packet loss','wait','PE->P2 (loopback) ping',60) diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 013a1d9a70..0e1df1cc35 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -402,10 +402,13 @@ static void nexthop_set_resolved(afi_t afi, const struct nexthop *newhop, nexthop_add(&nexthop->resolved, resolved_hop); } -/* If force flag is not set, do not modify falgs at all for uninstall - the route from FIB. */ +/* + * Given a nexthop we need to properly recursively resolve + * the route. As such, do a table lookup to find and match + * if at all possible. Set the nexthop->ifindex as appropriate + */ static int nexthop_active(afi_t afi, struct route_entry *re, - struct nexthop *nexthop, bool set, + struct nexthop *nexthop, struct route_node *top) { struct prefix p; @@ -421,12 +424,10 @@ static int nexthop_active(afi_t afi, struct route_entry *re, || nexthop->type == NEXTHOP_TYPE_IPV6) nexthop->ifindex = 0; - if (set) { - UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE); - nexthops_free(nexthop->resolved); - nexthop->resolved = NULL; - re->nexthop_mtu = 0; - } + UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE); + nexthops_free(nexthop->resolved); + nexthop->resolved = NULL; + re->nexthop_mtu = 0; /* * If the kernel has sent us a route, then @@ -436,16 +437,6 @@ static int nexthop_active(afi_t afi, struct route_entry *re, re->type == ZEBRA_ROUTE_SYSTEM) return 1; - /* Skip nexthops that have been filtered out due to route-map */ - /* The nexthops are specific to this route and so the same */ - /* nexthop for a different route may not have this flag set */ - if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_FILTERED)) { - if (IS_ZEBRA_DEBUG_RIB_DETAILED) - zlog_debug("\t%s: Nexthop Filtered", - __PRETTY_FUNCTION__); - return 0; - } - /* * Check to see if we should trust the passed in information * for UNNUMBERED interfaces as that we won't find the GW @@ -580,17 +571,14 @@ static int nexthop_active(afi_t afi, struct route_entry *re, NEXTHOP_FLAG_RECURSIVE)) continue; - if (set) { - SET_FLAG(nexthop->flags, - NEXTHOP_FLAG_RECURSIVE); - SET_FLAG(re->status, - ROUTE_ENTRY_NEXTHOPS_CHANGED); - nexthop_set_resolved(afi, newhop, - nexthop); - } + SET_FLAG(nexthop->flags, + NEXTHOP_FLAG_RECURSIVE); + SET_FLAG(re->status, + ROUTE_ENTRY_NEXTHOPS_CHANGED); + nexthop_set_resolved(afi, newhop, nexthop); resolved = 1; } - if (resolved && set) + if (resolved) re->nexthop_mtu = match->mtu; if (!resolved && IS_ZEBRA_DEBUG_RIB_DETAILED) zlog_debug("\t%s: Recursion failed to find", @@ -606,15 +594,12 @@ static int nexthop_active(afi_t afi, struct route_entry *re, NEXTHOP_FLAG_RECURSIVE)) continue; - if (set) { - SET_FLAG(nexthop->flags, - NEXTHOP_FLAG_RECURSIVE); - nexthop_set_resolved(afi, newhop, - nexthop); - } + SET_FLAG(nexthop->flags, + NEXTHOP_FLAG_RECURSIVE); + nexthop_set_resolved(afi, newhop, nexthop); resolved = 1; } - if (resolved && set) + if (resolved) re->nexthop_mtu = match->mtu; if (!resolved && IS_ZEBRA_DEBUG_RIB_DETAILED) @@ -818,17 +803,15 @@ struct route_entry *rib_lookup_ipv4(struct prefix_ipv4 *p, vrf_id_t vrf_id) /* This function verifies reachability of one given nexthop, which can be * numbered or unnumbered, IPv4 or IPv6. The result is unconditionally stored - * in nexthop->flags field. If the 4th parameter, 'set', is non-zero, - * nexthop->ifindex will be updated appropriately as well. - * An existing route map can turn (otherwise active) nexthop into inactive, but - * not vice versa. + * in nexthop->flags field. The nexthop->ifindex will be updated + * appropriately as well. An existing route map can turn + * (otherwise active) nexthop into inactive, but not vice versa. * * The return value is the final value of 'ACTIVE' flag. */ - static unsigned nexthop_active_check(struct route_node *rn, struct route_entry *re, - struct nexthop *nexthop, bool set) + struct nexthop *nexthop) { struct interface *ifp; route_map_result_t ret = RMAP_MATCH; @@ -856,14 +839,14 @@ static unsigned nexthop_active_check(struct route_node *rn, case NEXTHOP_TYPE_IPV4: case NEXTHOP_TYPE_IPV4_IFINDEX: family = AFI_IP; - if (nexthop_active(AFI_IP, re, nexthop, set, rn)) + if (nexthop_active(AFI_IP, re, nexthop, rn)) SET_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE); else UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE); break; case NEXTHOP_TYPE_IPV6: family = AFI_IP6; - if (nexthop_active(AFI_IP6, re, nexthop, set, rn)) + if (nexthop_active(AFI_IP6, re, nexthop, rn)) SET_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE); else UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE); @@ -880,7 +863,7 @@ static unsigned nexthop_active_check(struct route_node *rn, else UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE); } else { - if (nexthop_active(AFI_IP6, re, nexthop, set, rn)) + if (nexthop_active(AFI_IP6, re, nexthop, rn)) SET_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE); else UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE); @@ -945,25 +928,21 @@ static unsigned nexthop_active_check(struct route_node *rn, return CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE); } -/* Iterate over all nexthops of the given RIB entry and refresh their +/* + * Iterate over all nexthops of the given RIB entry and refresh their * ACTIVE flag. re->nexthop_active_num is updated accordingly. If any * nexthop is found to toggle the ACTIVE flag, the whole re structure - * is flagged with ROUTE_ENTRY_CHANGED. The 4th 'set' argument is - * transparently passed to nexthop_active_check(). + * is flagged with ROUTE_ENTRY_CHANGED. * * Return value is the new number of active nexthops. */ - -static int nexthop_active_update(struct route_node *rn, struct route_entry *re, - bool set) +static int nexthop_active_update(struct route_node *rn, struct route_entry *re) { struct nexthop *nexthop; union g_addr prev_src; - unsigned int prev_active, new_active, old_num_nh; + unsigned int prev_active, new_active; ifindex_t prev_index; - old_num_nh = re->nexthop_active_num; - re->nexthop_active_num = 0; UNSET_FLAG(re->status, ROUTE_ENTRY_CHANGED); @@ -979,7 +958,7 @@ static int nexthop_active_update(struct route_node *rn, struct route_entry *re, * a multipath perpsective should not be a data plane * decision point. */ - new_active = nexthop_active_check(rn, re, nexthop, set); + new_active = nexthop_active_check(rn, re, nexthop); if (new_active && re->nexthop_active_num >= multipath_num) { UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE); new_active = 0; @@ -995,19 +974,13 @@ static int nexthop_active_update(struct route_node *rn, struct route_entry *re, || ((nexthop->type >= NEXTHOP_TYPE_IPV6 && nexthop->type < NEXTHOP_TYPE_BLACKHOLE) && !(IPV6_ADDR_SAME(&prev_src.ipv6, - &nexthop->rmap_src.ipv6)))) { + &nexthop->rmap_src.ipv6))) + || CHECK_FLAG(re->status, ROUTE_ENTRY_LABELS_CHANGED)) { SET_FLAG(re->status, ROUTE_ENTRY_CHANGED); SET_FLAG(re->status, ROUTE_ENTRY_NEXTHOPS_CHANGED); } } - if (old_num_nh != re->nexthop_active_num) - SET_FLAG(re->status, ROUTE_ENTRY_CHANGED); - - if (CHECK_FLAG(re->status, ROUTE_ENTRY_CHANGED)) { - SET_FLAG(re->status, ROUTE_ENTRY_NEXTHOPS_CHANGED); - } - return re->nexthop_active_num; } @@ -1353,7 +1326,7 @@ static void rib_process_add_fib(struct zebra_vrf *zvrf, struct route_node *rn, /* Update real nexthop. This may actually determine if nexthop is active * or not. */ - if (!nexthop_active_update(rn, new, true)) { + if (!nexthop_group_active_nexthop_num(&new->ng)) { UNSET_FLAG(new->status, ROUTE_ENTRY_CHANGED); return; } @@ -1400,8 +1373,7 @@ static void rib_process_del_fib(struct zebra_vrf *zvrf, struct route_node *rn, * down, causing the kernel to delete routes without sending DELROUTE * notifications */ - if (!nexthop_active_update(rn, old, true) && - (RIB_KERNEL_ROUTE(old))) + if (RIB_KERNEL_ROUTE(old)) SET_FLAG(old->status, ROUTE_ENTRY_REMOVED); else UNSET_FLAG(old->status, ROUTE_ENTRY_CHANGED); @@ -1423,7 +1395,7 @@ static void rib_process_update_fib(struct zebra_vrf *zvrf, /* Update the nexthop; we could determine here that nexthop is * inactive. */ - if (nexthop_active_update(rn, new, true)) + if (nexthop_group_active_nexthop_num(&new->ng)) nh_active = 1; /* If nexthop is active, install the selected route, if @@ -1508,11 +1480,8 @@ static void rib_process_update_fib(struct zebra_vrf *zvrf, } /* Update prior route. */ - if (new != old) { - /* Set real nexthop. */ - nexthop_active_update(rn, old, true); + if (new != old) UNSET_FLAG(old->status, ROUTE_ENTRY_CHANGED); - } /* Clear changed flag. */ UNSET_FLAG(new->status, ROUTE_ENTRY_CHANGED); @@ -1642,38 +1611,30 @@ static void rib_process(struct route_node *rn) /* 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, + * 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. + * 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 (!CHECK_FLAG(re->status, ROUTE_ENTRY_CHANGED) - && !nexthop_active_update(rn, re, false)) { + 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 + * 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 */ + * 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( @@ -1750,10 +1711,8 @@ static void rib_process(struct route_node *rn) /* Update SELECTED entry */ if (old_selected != new_selected || selected_changed) { - if (new_selected && new_selected != new_fib) { - nexthop_active_update(rn, new_selected, true); + if (new_selected && new_selected != new_fib) UNSET_FLAG(new_selected->status, ROUTE_ENTRY_CHANGED); - } if (new_selected) SET_FLAG(new_selected->flags, ZEBRA_FLAG_SELECTED); @@ -2612,8 +2571,8 @@ void _route_entry_dump(const char *func, union prefixconstptr pp, INET6_ADDRSTRLEN); break; } - zlog_debug("%s: %s %s[%u] vrf %s(%u) with flags %s%s%s", func, - (nexthop->rparent ? " NH" : "NH"), straddr, + zlog_debug("%s: %s %s[%u] vrf %s(%u) with flags %s%s%s%s%s%s", + func, (nexthop->rparent ? " NH" : "NH"), straddr, nexthop->ifindex, vrf ? vrf->name : "Unknown", nexthop->vrf_id, (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE) @@ -2623,7 +2582,16 @@ void _route_entry_dump(const char *func, union prefixconstptr pp, ? "FIB " : ""), (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_RECURSIVE) - ? "RECURSIVE" + ? "RECURSIVE " + : ""), + (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ONLINK) + ? "ONLINK " + : ""), + (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_MATCHED) + ? "MATCHED " + : ""), + (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_DUPLICATE) + ? "DUPLICATE " : "")); } zlog_debug("%s: dump complete", func); @@ -2813,6 +2781,8 @@ int rib_add_multipath(afi_t afi, safi_t safi, struct prefix *p, if (IS_ZEBRA_DEBUG_RIB_DETAILED) route_entry_dump(p, src_p, re); } + + SET_FLAG(re->status, ROUTE_ENTRY_CHANGED); rib_addnode(rn, re, 1); ret = 1; |