diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index c21d00bbe6..4fa7a3c164 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -1475,6 +1475,8 @@ static void zread_route_add(ZAPI_HANDLER_ARGS) api_nh = &api.nexthops[i]; ifindex_t ifindex = 0; + nexthop = NULL; + if (IS_ZEBRA_DEBUG_RECV) zlog_debug("nh type %d", api_nh->type); diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c index 7430307320..a3b414975e 100644 --- a/zebra/zebra_nhg.c +++ b/zebra/zebra_nhg.c @@ -99,31 +99,60 @@ nhg_connected_tree_root(struct nhg_connected_tree_head *head) return nhg_connected_tree_first(head); } -void nhg_connected_tree_del_nhe(struct nhg_connected_tree_head *head, - struct nhg_hash_entry *depend) +struct nhg_hash_entry * +nhg_connected_tree_del_nhe(struct nhg_connected_tree_head *head, + struct nhg_hash_entry *depend) { struct nhg_connected lookup = {}; struct nhg_connected *remove = NULL; + struct nhg_hash_entry *removed_nhe; lookup.nhe = depend; /* Lookup to find the element, then remove it */ remove = nhg_connected_tree_find(head, &lookup); - remove = nhg_connected_tree_del(head, remove); - if (remove) + /* Re-returning here just in case this API changes.. + * the _del list api's are a bit undefined at the moment. + * + * So hopefully returning here will make it fail if the api + * changes to something different than currently expected. + */ + remove = nhg_connected_tree_del(head, remove); + + /* If the entry was sucessfully removed, free the 'connected` struct */ + if (remove) { + removed_nhe = remove->nhe; nhg_connected_free(remove); + return removed_nhe; + } + + return NULL; } -void nhg_connected_tree_add_nhe(struct nhg_connected_tree_head *head, - struct nhg_hash_entry *depend) +/* Assuming UNIQUE RB tree. If this changes, assumptions here about + * insertion need to change. + */ +struct nhg_hash_entry * +nhg_connected_tree_add_nhe(struct nhg_connected_tree_head *head, + struct nhg_hash_entry *depend) { struct nhg_connected *new = NULL; new = nhg_connected_new(depend); - if (new) - nhg_connected_tree_add(head, new); + /* On success, NULL will be returned from the + * RB code. + */ + if (new && (nhg_connected_tree_add(head, new) == NULL)) + return NULL; + + /* If it wasn't successful, it must be a duplicate. We enforce the + * unique property for the `nhg_connected` tree. + */ + nhg_connected_free(new); + + return depend; } static void @@ -481,7 +510,9 @@ static void handle_recursive_depend(struct nhg_connected_tree_head *nhg_depends, resolved_ng.nexthop = nh; depend = zebra_nhg_rib_find(0, &resolved_ng, afi); - depends_add(nhg_depends, depend); + + if (depend) + depends_add(nhg_depends, depend); } static bool zebra_nhg_find(struct nhg_hash_entry **nhe, uint32_t id, @@ -1104,8 +1135,14 @@ done: static void depends_add(struct nhg_connected_tree_head *head, struct nhg_hash_entry *depend) { - nhg_connected_tree_add_nhe(head, depend); - zebra_nhg_increment_ref(depend); + /* If NULL is returned, it was successfully added and + * needs to have its refcnt incremented. + * + * Else the NHE is already present in the tree and doesn't + * need to increment the refcnt. + */ + if (nhg_connected_tree_add_nhe(head, depend) == NULL) + zebra_nhg_increment_ref(depend); } static struct nhg_hash_entry * diff --git a/zebra/zebra_nhg_private.h b/zebra/zebra_nhg_private.h index 79107b047c..92f438fcec 100644 --- a/zebra/zebra_nhg_private.h +++ b/zebra/zebra_nhg_private.h @@ -52,9 +52,22 @@ extern bool nhg_connected_tree_is_empty(const struct nhg_connected_tree_head *head); extern struct nhg_connected * nhg_connected_tree_root(struct nhg_connected_tree_head *head); -extern void nhg_connected_tree_del_nhe(struct nhg_connected_tree_head *head, - struct nhg_hash_entry *nhe); -extern void nhg_connected_tree_add_nhe(struct nhg_connected_tree_head *head, - struct nhg_hash_entry *nhe); + +/* I realize _add/_del returns are backwords. + * + * Currently the list APIs are not standardized for what happens in + * the _del() function when the item isn't present. + * + * We are choosing to return NULL if not found in the _del case for now. + */ + +/* Delete NHE from the tree. On success, return the NHE, otherwise NULL. */ +extern struct nhg_hash_entry * +nhg_connected_tree_del_nhe(struct nhg_connected_tree_head *head, + struct nhg_hash_entry *nhe); +/* ADD NHE to the tree. On success, return NULL, otherwise return the NHE. */ +extern struct nhg_hash_entry * +nhg_connected_tree_add_nhe(struct nhg_connected_tree_head *head, + struct nhg_hash_entry *nhe); #endif /* __ZEBRA_NHG_PRIVATE_H__ */