zebra: Cleanup zebra_nhg APIs

Add a private header file for functions that are internal/special
case like how we do it for `lib/nexthop_group_private.h`.

Remove a bunch of functions from the header file only being used
statically and add some comments for those remaining to indicate
better what their use is.

Signed-off-by: Stephen Worley <sworley@cumulusnetworks.com>

diff --git a/zebra/interface.h b/zebra/interface.h
index c6f0e465fcf1fb274835651e9e3c8c033d6bb81d..78ccbae6235d96e1320873454bfbc945d0746af7 100644 (file)
--- a/zebra/interface.h
+++ b/zebra/interface.h
@@ -27,7 +27,7 @@
 #include "hook.h"
 
 #include "zebra/zebra_l2.h"
-#include "zebra/zebra_nhg.h"
+#include "zebra/zebra_nhg_private.h"
 
 #ifdef __cplusplus
 extern "C" {

diff --git a/zebra/subdir.am b/zebra/subdir.am
index 25040a2717dd2a15e818a9d4fdf44db6a984965b..28847ce09b8fe832883ee23f667a42bdcb5676b4 100644 (file)
--- a/zebra/subdir.am
+++ b/zebra/subdir.am
@@ -137,6 +137,7 @@ noinst_HEADERS += \
        zebra/zebra_mpls.h \
        zebra/zebra_mroute.h \
        zebra/zebra_nhg.h \
+       zebra/zebra_nhg_private.h \
        zebra/zebra_ns.h \
        zebra/zebra_pbr.h \
        zebra/zebra_ptm.h \

diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c
index 3ec764c5d390403435d3b0367321f30c2a6304d2..2bb117b2729d886ab2740adc5f3845d226cb5bab 100644 (file)
--- a/zebra/zebra_nhg.c
+++ b/zebra/zebra_nhg.c
@@ -32,7 +32,7 @@
 #include "zebra/connected.h"
 #include "zebra/debug.h"
 #include "zebra/zebra_router.h"
-#include "zebra/zebra_nhg.h"
+#include "zebra/zebra_nhg_private.h"
 #include "zebra/zebra_rnh.h"
 #include "zebra/zebra_routemap.h"
 #include "zebra/zebra_memory.h"
@@ -60,12 +60,12 @@ depends_find_id_add(struct nhg_connected_tree_head *head, uint32_t id);
 static void depends_decrement_free(struct nhg_connected_tree_head *head);
 
 
-void nhg_connected_free(struct nhg_connected *dep)
+static void nhg_connected_free(struct nhg_connected *dep)
 {
        XFREE(MTYPE_NHG_CONNECTED, dep);
 }
 
-struct nhg_connected *nhg_connected_new(struct nhg_hash_entry *nhe)
+static struct nhg_connected *nhg_connected_new(struct nhg_hash_entry *nhe)
 {
        struct nhg_connected *new = NULL;
 
@@ -156,26 +156,6 @@ struct nhg_hash_entry *zebra_nhg_resolve(struct nhg_hash_entry *nhe)
        return nhe;
 }
 
-uint32_t zebra_nhg_get_resolved_id(uint32_t id)
-{
-       struct nhg_hash_entry *nhe = NULL;
-
-       nhe = zebra_nhg_lookup_id(id);
-
-       if (!nhe) {
-               flog_err(
-                       EC_ZEBRA_TABLE_LOOKUP_FAILED,
-                       "Zebra failed to lookup a resolved nexthop hash entry id=%u",
-                       id);
-               return id;
-       }
-
-       if (CHECK_FLAG(nhe->flags, NEXTHOP_GROUP_RECURSIVE))
-               nhe = zebra_nhg_resolve(nhe);
-
-       return nhe->id;
-}
-
 unsigned int zebra_nhg_depends_count(const struct nhg_hash_entry *nhe)
 {
        return nhg_connected_tree_count(&nhe->nhg_depends);
@@ -186,59 +166,41 @@ bool zebra_nhg_depends_is_empty(const struct nhg_hash_entry *nhe)
        return nhg_connected_tree_is_empty(&nhe->nhg_depends);
 }
 
-void zebra_nhg_depends_del(struct nhg_hash_entry *from,
-                          struct nhg_hash_entry *depend)
+static void zebra_nhg_depends_del(struct nhg_hash_entry *from,
+                                 struct nhg_hash_entry *depend)
 {
        nhg_connected_tree_del_nhe(&from->nhg_depends, depend);
 }
 
-void zebra_nhg_depends_add(struct nhg_hash_entry *to,
-                          struct nhg_hash_entry *depend)
-{
-       nhg_connected_tree_add_nhe(&to->nhg_depends, depend);
-}
-
-void zebra_nhg_depends_init(struct nhg_hash_entry *nhe)
+static void zebra_nhg_depends_init(struct nhg_hash_entry *nhe)
 {
        nhg_connected_tree_init(&nhe->nhg_depends);
 }
 
-/* Release this nhe from anything that it depends on */
-static void zebra_nhg_depends_release(struct nhg_hash_entry *nhe)
-{
-       if (!zebra_nhg_depends_is_empty(nhe)) {
-               struct nhg_connected *rb_node_dep = NULL;
-
-               frr_each_safe(nhg_connected_tree, &nhe->nhg_depends,
-                              rb_node_dep) {
-                       zebra_nhg_dependents_del(rb_node_dep->nhe, nhe);
-               }
-       }
-}
-
 unsigned int zebra_nhg_dependents_count(const struct nhg_hash_entry *nhe)
 {
        return nhg_connected_tree_count(&nhe->nhg_dependents);
 }
 
+
 bool zebra_nhg_dependents_is_empty(const struct nhg_hash_entry *nhe)
 {
        return nhg_connected_tree_is_empty(&nhe->nhg_dependents);
 }
 
-void zebra_nhg_dependents_del(struct nhg_hash_entry *from,
-                             struct nhg_hash_entry *dependent)
+static void zebra_nhg_dependents_del(struct nhg_hash_entry *from,
+                                    struct nhg_hash_entry *dependent)
 {
        nhg_connected_tree_del_nhe(&from->nhg_dependents, dependent);
 }
 
-void zebra_nhg_dependents_add(struct nhg_hash_entry *to,
-                             struct nhg_hash_entry *dependent)
+static void zebra_nhg_dependents_add(struct nhg_hash_entry *to,
+                                    struct nhg_hash_entry *dependent)
 {
        nhg_connected_tree_add_nhe(&to->nhg_dependents, dependent);
 }
 
-void zebra_nhg_dependents_init(struct nhg_hash_entry *nhe)
+static void zebra_nhg_dependents_init(struct nhg_hash_entry *nhe)
 {
        nhg_connected_tree_init(&nhe->nhg_dependents);
 }
@@ -255,6 +217,20 @@ static void zebra_nhg_dependents_release(struct nhg_hash_entry *nhe)
        }
 }
 
+/* Release this nhe from anything that it depends on */
+static void zebra_nhg_depends_release(struct nhg_hash_entry *nhe)
+{
+       if (!zebra_nhg_depends_is_empty(nhe)) {
+               struct nhg_connected *rb_node_dep = NULL;
+
+               frr_each_safe(nhg_connected_tree, &nhe->nhg_depends,
+                              rb_node_dep) {
+                       zebra_nhg_dependents_del(rb_node_dep->nhe, nhe);
+               }
+       }
+}
+
+
 struct nhg_hash_entry *zebra_nhg_lookup_id(uint32_t id)
 {
        struct nhg_hash_entry lookup = {};
@@ -263,7 +239,7 @@ struct nhg_hash_entry *zebra_nhg_lookup_id(uint32_t id)
        return hash_lookup(zrouter.nhgs_id, &lookup);
 }
 
-int zebra_nhg_insert_id(struct nhg_hash_entry *nhe)
+static int zebra_nhg_insert_id(struct nhg_hash_entry *nhe)
 {
        if (hash_lookup(zrouter.nhgs_id, nhe)) {
                flog_err(
@@ -278,6 +254,12 @@ int zebra_nhg_insert_id(struct nhg_hash_entry *nhe)
        return 0;
 }
 
+static void zebra_nhg_set_if(struct nhg_hash_entry *nhe, struct interface *ifp)
+{
+       nhe->ifp = ifp;
+       if_nhg_dependents_add(ifp, nhe);
+}
+
 static void
 zebra_nhg_connect_depends(struct nhg_hash_entry *nhe,
                          struct nhg_connected_tree_head nhg_depends)
@@ -529,7 +511,7 @@ static bool zebra_nhg_find(struct nhg_hash_entry **nhe, uint32_t id,
                        lookup.nhg_depends = *nhg_depends;
                else {
                        if (nhg->nexthop->next) {
-                               nhg_connected_tree_init(&lookup.nhg_depends);
+                               zebra_nhg_depends_init(&lookup);
 
                                /* If its a group, create a dependency tree */
                                struct nexthop *nh = NULL;
@@ -539,7 +521,7 @@ static bool zebra_nhg_find(struct nhg_hash_entry **nhe, uint32_t id,
                                                         nh, afi);
                        } else if (CHECK_FLAG(nhg->nexthop->flags,
                                              NEXTHOP_FLAG_RECURSIVE)) {
-                               nhg_connected_tree_init(&lookup.nhg_depends);
+                               zebra_nhg_depends_init(&lookup);
                                handle_recursive_depend(&lookup.nhg_depends,
                                                        nhg->nexthop->resolved,
                                                        afi);
@@ -1091,7 +1073,7 @@ zebra_nhg_rib_find(uint32_t id, struct nexthop_group *nhg, afi_t rt_afi)
        return nhe;
 }
 
-void zebra_nhg_free_members(struct nhg_hash_entry *nhe)
+static void zebra_nhg_free_members(struct nhg_hash_entry *nhe)
 {
        nexthop_group_delete(&nhe->nhg);
        /* Decrement to remove connection ref */
@@ -1133,12 +1115,6 @@ void zebra_nhg_increment_ref(struct nhg_hash_entry *nhe)
                nhg_connected_tree_increment_ref(&nhe->nhg_depends);
 }
 
-void zebra_nhg_set_if(struct nhg_hash_entry *nhe, struct interface *ifp)
-{
-       nhe->ifp = ifp;
-       if_nhg_dependents_add(ifp, nhe);
-}
-
 static void nexthop_set_resolved(afi_t afi, const struct nexthop *newhop,
                                 struct nexthop *nexthop)
 {

diff --git a/zebra/zebra_nhg.h b/zebra/zebra_nhg.h
index e636c0f2d2ea4ace34098f2974ee87db4cdeb979..1f695433c9689dc2733261511ba724ac6ab1df1d 100644 (file)
--- a/zebra/zebra_nhg.h
+++ b/zebra/zebra_nhg.h
@@ -41,7 +41,6 @@ struct nh_grp {
 
 PREDECL_RBTREE_UNIQ(nhg_connected_tree);
 
-
 /*
  * Hashtables contiaining entries found in `zebra_router`.
  */
@@ -111,21 +110,6 @@ struct nhg_hash_entry {
 /* Was this one we created, either this session or previously? */
 #define ZEBRA_NHG_CREATED(NHE) ((NHE->type) == ZEBRA_ROUTE_NHG)
 
-/* Abstraction for connected trees */
-struct nhg_connected {
-       struct nhg_connected_tree_item tree_item;
-       struct nhg_hash_entry *nhe;
-};
-
-static int nhg_connected_cmp(const struct nhg_connected *con1,
-                            const struct nhg_connected *con2)
-{
-       return (con1->nhe->id - con2->nhe->id);
-}
-
-DECLARE_RBTREE_UNIQ(nhg_connected_tree, struct nhg_connected, tree_item,
-                   nhg_connected_cmp);
-
 
 enum nhg_ctx_op_e {
        NHG_CTX_OP_NONE = 0,
@@ -172,52 +156,23 @@ struct nhg_ctx {
 };
 
 
-extern void nhg_connected_free(struct nhg_connected *dep);
-extern struct nhg_connected *nhg_connected_new(struct nhg_hash_entry *nhe);
-
-/* nhg connected tree direct access functions */
-extern void nhg_connected_tree_init(struct nhg_connected_tree_head *head);
-extern void nhg_connected_tree_free(struct nhg_connected_tree_head *head);
-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);
-
 /**
  * NHE abstracted tree functions.
  * Use these where possible instead of the direct ones access ones.
  */
-
-
 extern struct nhg_hash_entry *zebra_nhg_resolve(struct nhg_hash_entry *nhe);
-extern uint32_t zebra_nhg_get_resolved_id(uint32_t id);
 
-/* Depends */
 extern unsigned int zebra_nhg_depends_count(const struct nhg_hash_entry *nhe);
 extern bool zebra_nhg_depends_is_empty(const struct nhg_hash_entry *nhe);
-extern void zebra_nhg_depends_del(struct nhg_hash_entry *from,
-                                 struct nhg_hash_entry *depend);
-extern void zebra_nhg_depends_add(struct nhg_hash_entry *to,
-                                 struct nhg_hash_entry *depend);
-extern void zebra_nhg_depends_init(struct nhg_hash_entry *nhe);
-/* Dependents */
+
 extern unsigned int
 zebra_nhg_dependents_count(const struct nhg_hash_entry *nhe);
 extern bool zebra_nhg_dependents_is_empty(const struct nhg_hash_entry *nhe);
-extern void zebra_nhg_dependents_del(struct nhg_hash_entry *from,
-                                    struct nhg_hash_entry *dependent);
-extern void zebra_nhg_dependents_add(struct nhg_hash_entry *to,
-                                    struct nhg_hash_entry *dependent);
-extern void zebra_nhg_dependents_init(struct nhg_hash_entry *nhe);
-
 
+/* Lookup ID, doesn't create */
 extern struct nhg_hash_entry *zebra_nhg_lookup_id(uint32_t id);
-extern int zebra_nhg_insert_id(struct nhg_hash_entry *nhe);
 
+/* Hash functions */
 extern uint32_t zebra_nhg_hash_key(const void *arg);
 extern uint32_t zebra_nhg_id_key(const void *arg);
 
@@ -243,29 +198,31 @@ extern int zebra_nhg_kernel_del(uint32_t id);
 extern struct nhg_hash_entry *
 zebra_nhg_rib_find(uint32_t id, struct nexthop_group *nhg, afi_t rt_afi);
 
-
-void zebra_nhg_free_members(struct nhg_hash_entry *nhe);
-void zebra_nhg_free(void *arg);
-void zebra_nhg_decrement_ref(struct nhg_hash_entry *nhe);
-void zebra_nhg_increment_ref(struct nhg_hash_entry *nhe);
-
-void zebra_nhg_check_valid(struct nhg_hash_entry *nhe);
-void zebra_nhg_set_if(struct nhg_hash_entry *nhe, struct interface *ifp);
-
-extern int nexthop_active_update(struct route_node *rn, struct route_entry *re);
-
+/* Reference counter functions */
+extern void zebra_nhg_decrement_ref(struct nhg_hash_entry *nhe);
+extern void zebra_nhg_increment_ref(struct nhg_hash_entry *nhe);
 extern int zebra_nhg_re_update_ref(struct route_entry *re,
                                   struct nhg_hash_entry *nhe);
 
+/* Check validity of nhe, if invalid will update dependents as well */
+extern void zebra_nhg_check_valid(struct nhg_hash_entry *nhe);
+
+/* Convert nhe depends to a grp context that can be passed around safely */
 extern uint8_t zebra_nhg_nhe2grp(struct nh_grp *grp, struct nhg_hash_entry *nhe,
                                 int size);
 
-void zebra_nhg_install_kernel(struct nhg_hash_entry *nhe);
-void zebra_nhg_uninstall_kernel(struct nhg_hash_entry *nhe);
+/* Dataplane install/uninstall */
+extern void zebra_nhg_install_kernel(struct nhg_hash_entry *nhe);
+extern void zebra_nhg_uninstall_kernel(struct nhg_hash_entry *nhe);
 
 /* Forward ref of dplane update context type */
 struct zebra_dplane_ctx;
-void zebra_nhg_dplane_result(struct zebra_dplane_ctx *ctx);
+extern void zebra_nhg_dplane_result(struct zebra_dplane_ctx *ctx);
+
 
-void zebra_nhg_sweep_table(struct hash *hash);
+/* Sweet the nhg hash tables for old entries on restart */
+extern void zebra_nhg_sweep_table(struct hash *hash);
+
+/* Nexthop resolution processing */
+extern int nexthop_active_update(struct route_node *rn, struct route_entry *re);
 #endif

diff --git a/zebra/zebra_nhg_private.h b/zebra/zebra_nhg_private.h
new file mode 100644 (file)
index 0000000..170e235
--- /dev/null
+++ b/zebra/zebra_nhg_private.h
@@ -0,0 +1,62 @@
+/*
+ * Nexthop Group Private Functions.
+ * Copyright (C) 2019 Cumulus Networks, Inc.
+ *                    Stephen Worley
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; see the file COPYING; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * These functions should only be used internally for nhg_hash_entry
+ * manipulation and in certain special cases.
+ *
+ * Please use `zebra/zebra_nhg.h` for any general nhg_hash_entry api needs.
+ */
+
+#ifndef __ZEBRA_NHG_PRIVATE_H__
+#define __ZEBRA_NHG_PRIVATE_H__
+
+#include "zebra/zebra_nhg.h"
+
+/* Abstraction for connected trees */
+struct nhg_connected {
+       struct nhg_connected_tree_item tree_item;
+       struct nhg_hash_entry *nhe;
+};
+
+static int nhg_connected_cmp(const struct nhg_connected *con1,
+                            const struct nhg_connected *con2)
+{
+       return (con1->nhe->id - con2->nhe->id);
+}
+
+DECLARE_RBTREE_UNIQ(nhg_connected_tree, struct nhg_connected, tree_item,
+                   nhg_connected_cmp);
+
+/* nhg connected tree direct access functions */
+extern void nhg_connected_tree_init(struct nhg_connected_tree_head *head);
+extern void nhg_connected_tree_free(struct nhg_connected_tree_head *head);
+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);
+
+extern void zebra_nhg_free(void *arg);
+
+#endif /* __ZEBRA_NHG_PRIVATE_H__ */

diff --git a/zebra/zebra_router.c b/zebra/zebra_router.c
index f52249ad8b2b2069c3799d7c7f080b3792b7e6e9..e5319c64af031452f90c21d782f8143a1037b7b5 100644 (file)
--- a/zebra/zebra_router.c
+++ b/zebra/zebra_router.c
@@ -29,7 +29,7 @@
 #include "zebra_pbr.h"
 #include "zebra_vxlan.h"
 #include "zebra_mlag.h"
-#include "zebra_nhg.h"
+#include "zebra_nhg_private.h"
 #include "debug.h"
 
 DEFINE_MTYPE_STATIC(ZEBRA, RIB_TABLE_INFO, "RIB table info")