diff --git a/lib/nexthop.c b/lib/nexthop.c index 65c12c1e6931..ac22e7ec8451 100644 --- a/lib/nexthop.c +++ b/lib/nexthop.c @@ -139,7 +139,7 @@ static int _nexthop_source_cmp(const struct nexthop *nh1, } static int _nexthop_cmp_no_labels(const struct nexthop *next1, - const struct nexthop *next2) + const struct nexthop *next2, bool use_weight) { int ret = 0; @@ -155,11 +155,13 @@ static int _nexthop_cmp_no_labels(const struct nexthop *next1, if (next1->type > next2->type) return 1; - if (next1->weight < next2->weight) - return -1; + if (use_weight) { + if (next1->weight < next2->weight) + return -1; - if (next1->weight > next2->weight) - return 1; + if (next1->weight > next2->weight) + return 1; + } switch (next1->type) { case NEXTHOP_TYPE_IPV4: @@ -227,11 +229,12 @@ static int _nexthop_cmp_no_labels(const struct nexthop *next1, return ret; } -int nexthop_cmp(const struct nexthop *next1, const struct nexthop *next2) +static int nexthop_cmp_internal(const struct nexthop *next1, + const struct nexthop *next2, bool use_weight) { int ret = 0; - ret = _nexthop_cmp_no_labels(next1, next2); + ret = _nexthop_cmp_no_labels(next1, next2, use_weight); if (ret != 0) return ret; @@ -244,6 +247,17 @@ int nexthop_cmp(const struct nexthop *next1, const struct nexthop *next2) return ret; } +int nexthop_cmp(const struct nexthop *next1, const struct nexthop *next2) +{ + return nexthop_cmp_internal(next1, next2, true); +} + +int nexthop_cmp_no_weight(const struct nexthop *next1, + const struct nexthop *next2) +{ + return nexthop_cmp_internal(next1, next2, false); +} + /* * More-limited comparison function used to detect duplicate * nexthops. This is used in places where we don't need the full @@ -441,7 +455,7 @@ bool nexthop_same_no_labels(const struct nexthop *nh1, if (nh1 == nh2) return true; - if (_nexthop_cmp_no_labels(nh1, nh2) != 0) + if (_nexthop_cmp_no_labels(nh1, nh2, true) != 0) return false; return true; diff --git a/lib/nexthop.h b/lib/nexthop.h index 27073b948dea..15cfb26d8206 100644 --- a/lib/nexthop.h +++ b/lib/nexthop.h @@ -207,6 +207,8 @@ extern bool nexthop_same(const struct nexthop *nh1, const struct nexthop *nh2); extern bool nexthop_same_no_labels(const struct nexthop *nh1, const struct nexthop *nh2); extern int nexthop_cmp(const struct nexthop *nh1, const struct nexthop *nh2); +extern int nexthop_cmp_no_weight(const struct nexthop *nh1, + const struct nexthop *nh2); extern int nexthop_g_addr_cmp(enum nexthop_types_t type, const union g_addr *addr1, const union g_addr *addr2); diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index 8164e2a56fae..96807356d622 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -1417,6 +1417,11 @@ static struct nhg_hash_entry *depends_find_singleton(const struct nexthop *nh, */ nexthop_copy_no_recurse(&lookup, nh, NULL); + /* + * So this is to intentionally cause the singleton nexthop + * to be created with a weight of 1. + */ + lookup.weight = 1; nhe = zebra_nhg_find_nexthop(0, &lookup, afi, type, from_dplane); /* The copy may have allocated labels; free them if necessary. */ @@ -3010,13 +3015,14 @@ int nexthop_active_update(struct route_node *rn, struct route_entry *re) * I'm pretty sure we only allow ONE level of group within group currently. * But making this recursive just in case that ever changes. */ -static uint8_t zebra_nhg_nhe2grp_internal(struct nh_grp *grp, - uint8_t curr_index, +static uint8_t zebra_nhg_nhe2grp_internal(struct nh_grp *grp, uint8_t curr_index, struct nhg_hash_entry *nhe, + struct nhg_hash_entry *original, int max_num) { struct nhg_connected *rb_node_dep = NULL; struct nhg_hash_entry *depend = NULL; + struct nexthop *nexthop; uint8_t i = curr_index; frr_each(nhg_connected_tree, &nhe->nhg_depends, rb_node_dep) { @@ -3043,8 +3049,11 @@ static uint8_t zebra_nhg_nhe2grp_internal(struct nh_grp *grp, if (!zebra_nhg_depends_is_empty(depend)) { /* This is a group within a group */ - i = zebra_nhg_nhe2grp_internal(grp, i, depend, max_num); + i = zebra_nhg_nhe2grp_internal(grp, i, depend, nhe, + max_num); } else { + bool found; + if (!CHECK_FLAG(depend->flags, NEXTHOP_GROUP_VALID)) { if (IS_ZEBRA_DEBUG_RIB_DETAILED || IS_ZEBRA_DEBUG_NHG) @@ -3085,8 +3094,37 @@ static uint8_t zebra_nhg_nhe2grp_internal(struct nh_grp *grp, continue; } + /* + * So we need to create the nexthop group with + * the appropriate weights. The nexthops weights + * are stored in the fully resolved nexthops for + * the nhg so we need to find the appropriate + * nexthop associated with this and set the weight + * appropriately + */ + found = false; + for (ALL_NEXTHOPS_PTR(&original->nhg, nexthop)) { + if (CHECK_FLAG(nexthop->flags, + NEXTHOP_FLAG_RECURSIVE)) + continue; + + if (nexthop_cmp_no_weight(depend->nhg.nexthop, + nexthop) != 0) + continue; + + found = true; + break; + } + + if (!found) { + if (IS_ZEBRA_DEBUG_RIB_DETAILED || + IS_ZEBRA_DEBUG_NHG) + zlog_debug("%s: Nexthop ID (%u) unable to find nexthop in Nexthop Gropu Entry, something is terribly wrong", + __func__, depend->id); + continue; + } grp[i].id = depend->id; - grp[i].weight = depend->nhg.nexthop->weight; + grp[i].weight = nexthop->weight; i++; } } @@ -3110,7 +3148,7 @@ uint8_t zebra_nhg_nhe2grp(struct nh_grp *grp, struct nhg_hash_entry *nhe, int max_num) { /* Call into the recursive function */ - return zebra_nhg_nhe2grp_internal(grp, 0, nhe, max_num); + return zebra_nhg_nhe2grp_internal(grp, 0, nhe, nhe, max_num); } void zebra_nhg_install_kernel(struct nhg_hash_entry *nhe)