From 8459128259f1db9258473f7e42558d984183ed5b Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Thu, 22 Oct 2020 18:09:44 -0400 Subject: [PATCH 1/2] Revert "Revert "zebra: fix NHE dependents backpointer relationship"" This reverts commit a682deea0fc1e2c2dc2de31a358126793f946371. --- zebra/zebra_nhg.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index f1f6e7e2bf..781a70da17 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -442,11 +442,8 @@ static void *zebra_nhg_hash_alloc(void *arg) /* Mark duplicate nexthops in a group at creation time. */ nexthop_group_mark_duplicates(&(nhe->nhg)); - zebra_nhg_connect_depends(nhe, &(copy->nhg_depends)); - /* Add the ifp now if it's not a group or recursive and has ifindex */ - if (zebra_nhg_depends_is_empty(nhe) && nhe->nhg.nexthop - && nhe->nhg.nexthop->ifindex) { + if (nhe->nhg.nexthop && nhe->nhg.nexthop->ifindex) { struct interface *ifp = NULL; ifp = if_lookup_by_index(nhe->nhg.nexthop->ifindex, @@ -461,7 +458,6 @@ static void *zebra_nhg_hash_alloc(void *arg) nhe->nhg.nexthop->vrf_id, nhe->id); } - return nhe; } @@ -754,7 +750,7 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */ * resolving nexthop; or a group of nexthops, where we need * relationships with the corresponding singletons. */ - zebra_nhg_depends_init(lookup); + zebra_nhg_depends_init(newnhe); nh = newnhe->nhg.nexthop; @@ -786,7 +782,14 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */ } if (recursive) - SET_FLAG((*nhe)->flags, NEXTHOP_GROUP_RECURSIVE); + SET_FLAG(newnhe->flags, NEXTHOP_GROUP_RECURSIVE); + + /* Attach dependent backpointers to singletons */ + zebra_nhg_connect_depends(newnhe, &newnhe->nhg_depends); + + /** + * Backup Nexthops + */ if (zebra_nhg_get_backup_nhg(newnhe) == NULL || zebra_nhg_get_backup_nhg(newnhe)->nexthop == NULL) @@ -829,6 +832,9 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */ if (recursive) SET_FLAG(backup_nhe->flags, NEXTHOP_GROUP_RECURSIVE); + /* Attach dependent backpointers to singletons */ + zebra_nhg_connect_depends(backup_nhe, &backup_nhe->nhg_depends); + done: return created; @@ -1588,6 +1594,7 @@ void zebra_nhg_free(struct nhg_hash_entry *nhe) void zebra_nhg_hash_free(void *p) { + zebra_nhg_release_all_deps((struct nhg_hash_entry *)p); zebra_nhg_free((struct nhg_hash_entry *)p); } @@ -2814,10 +2821,15 @@ struct nhg_hash_entry *zebra_nhg_proto_add(uint32_t id, int type, if (old) { /* * This is a replace, just release NHE from ID for now, The - * depends/dependents may still be used in the replacement. + * depends/dependents may still be used in the replacement so + * we don't touch them other than to remove their refs to their + * old parent. */ replace = true; hash_release(zrouter.nhgs_id, old); + + /* Free all the things */ + zebra_nhg_release_all_deps(old); } new = zebra_nhg_rib_find_nhe(&lookup, afi); @@ -2854,9 +2866,6 @@ struct nhg_hash_entry *zebra_nhg_proto_add(uint32_t id, int type, zebra_nhg_decrement_ref(rb_node_dep->nhe); } - /* Free all the things */ - zebra_nhg_release_all_deps(old); - /* Dont call the dec API, we dont want to uninstall the ID */ old->refcnt = 0; zebra_nhg_free(old); From 7fa239f165f47d4a9e5a77e337d5715f69aa609a Mon Sep 17 00:00:00 2001 From: Stephen Worley Date: Thu, 22 Oct 2020 18:10:44 -0400 Subject: [PATCH 2/2] zebra: disable dependent backpointers for backup nexthops Because the backup nexthop groups currently are more like pseudo-NHEs (they don't have IDs and are not inserted into the ID table or hashed), they can't really have this depends/dependents relationship yet in both directions. Some work needs to be done there to make them more like first class citizens like "normal" NHGs to enable this. Signed-off-by: Stephen Worley --- zebra/zebra_nhg.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index 781a70da17..94d4735d62 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -832,9 +832,6 @@ static bool zebra_nhe_find(struct nhg_hash_entry **nhe, /* return value */ if (recursive) SET_FLAG(backup_nhe->flags, NEXTHOP_GROUP_RECURSIVE); - /* Attach dependent backpointers to singletons */ - zebra_nhg_connect_depends(backup_nhe, &backup_nhe->nhg_depends); - done: return created;