lib,zebra: tighten up the nexthop_copy/nexthop_dup APIs

Make the nexthop_copy/nexthop_dup APIs more consistent by
adding a secondary, non-recursive, version of them. Before,
it was inconsistent whether the APIs were expected to copy
recursive info or not. Make it clear now that the default is
recursive info is copied unless the _no_recurse() version is
called. These APIs are not heavily used so it is fine to
change them for now.

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

diff --git a/lib/nexthop.c b/lib/nexthop.c
index d2ab70e209276f109f5933d71468a4dd821d1d4c..e23f8b0792b032e67ade5b290e5bfb1f98d47b5e 100644 (file)
--- a/lib/nexthop.c
+++ b/lib/nexthop.c
@@ -34,6 +34,7 @@
 #include "jhash.h"
 #include "printfrr.h"
 #include "vrf.h"
+#include "nexthop_group.h"
 
 DEFINE_MTYPE_STATIC(LIB, NEXTHOP, "Nexthop")
 DEFINE_MTYPE_STATIC(LIB, NH_LABEL, "Nexthop label")
@@ -565,8 +566,9 @@ uint32_t nexthop_hash(const struct nexthop *nexthop)
        return key;
 }
 
-void nexthop_copy(struct nexthop *copy, const struct nexthop *nexthop,
-                 struct nexthop *rparent)
+void nexthop_copy_no_recurse(struct nexthop *copy,
+                            const struct nexthop *nexthop,
+                            struct nexthop *rparent)
 {
        copy->vrf_id = nexthop->vrf_id;
        copy->ifindex = nexthop->ifindex;
@@ -583,6 +585,28 @@ void nexthop_copy(struct nexthop *copy, const struct nexthop *nexthop,
                                   &nexthop->nh_label->label[0]);
 }
 
+void nexthop_copy(struct nexthop *copy, const struct nexthop *nexthop,
+                 struct nexthop *rparent)
+{
+       nexthop_copy_no_recurse(copy, nexthop, rparent);
+
+       /* Bit of a special case here, we need to handle the case
+        * of a nexthop resolving to agroup. Hence, we need to
+        * use a nexthop_group API.
+        */
+       if (CHECK_FLAG(copy->flags, NEXTHOP_FLAG_RECURSIVE))
+               copy_nexthops(&copy->resolved, nexthop->resolved, copy);
+}
+
+struct nexthop *nexthop_dup_no_recurse(const struct nexthop *nexthop,
+                                      struct nexthop *rparent)
+{
+       struct nexthop *new = nexthop_new();
+
+       nexthop_copy_no_recurse(new, nexthop, rparent);
+       return new;
+}
+
 struct nexthop *nexthop_dup(const struct nexthop *nexthop,
                            struct nexthop *rparent)
 {

diff --git a/lib/nexthop.h b/lib/nexthop.h
index 040b643a84ac5eae30692aeda14ae7036864bf1a..cb5efe00ccbc7c50be2ab02dadbd7b4ab74d2945 100644 (file)
--- a/lib/nexthop.h
+++ b/lib/nexthop.h
@@ -187,9 +187,16 @@ extern unsigned int nexthop_level(struct nexthop *nexthop);
 /* Copies to an already allocated nexthop struct */
 extern void nexthop_copy(struct nexthop *copy, const struct nexthop *nexthop,
                         struct nexthop *rparent);
+/* Copies to an already allocated nexthop struct, not including recurse info */
+extern void nexthop_copy_no_recurse(struct nexthop *copy,
+                                   const struct nexthop *nexthop,
+                                   struct nexthop *rparent);
 /* Duplicates a nexthop and returns the newly allocated nexthop */
 extern struct nexthop *nexthop_dup(const struct nexthop *nexthop,
                                   struct nexthop *rparent);
+/* Duplicates a nexthop and returns the newly allocated nexthop */
+extern struct nexthop *nexthop_dup_no_recurse(const struct nexthop *nexthop,
+                                             struct nexthop *rparent);
 
 #ifdef __cplusplus
 }

diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c
index 0051cba62572f6bc9d6e45afb985b6339ae6ce94..0f1a87cf6e13b80a3e906da533e693c188f6f3bf 100644 (file)
--- a/lib/nexthop_group.c
+++ b/lib/nexthop_group.c
@@ -363,10 +363,6 @@ void copy_nexthops(struct nexthop **tnh, const struct nexthop *nh,
        for (nh1 = nh; nh1; nh1 = nh1->next) {
                nexthop = nexthop_dup(nh1, rparent);
                _nexthop_add(tnh, nexthop);
-
-               if (CHECK_FLAG(nh1->flags, NEXTHOP_FLAG_RECURSIVE))
-                       copy_nexthops(&nexthop->resolved, nh1->resolved,
-                                     nexthop);
        }
 }
 

diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c
index 2be9238ee87092d6b33685430c733eff3d062e17..2a76fa61e90326888d886fac22449b86abc743cb 100644 (file)
--- a/zebra/zebra_nhg.c
+++ b/zebra/zebra_nhg.c
@@ -1054,18 +1054,7 @@ static struct nhg_hash_entry *depends_find_recursive(const struct nexthop *nh,
        struct nhg_hash_entry *nhe;
        struct nexthop *lookup = NULL;
 
-       /*
-        * We need to copy its resolved nexthop if its recursively
-        * resolved so that has to be handled with allocs/frees since
-        * it could resolve to a group of unknown size.
-        */
-       copy_nexthops(&lookup, nh, NULL);
-
-       /* Make it a single, recursive nexthop */
-       nexthops_free(lookup->next);
-       nexthops_free(lookup->prev);
-       lookup->next = NULL;
-       lookup->prev = NULL;
+       lookup = nexthop_dup(nh, NULL);
 
        nhe = zebra_nhg_find_nexthop(0, lookup, afi, 0);
 
@@ -1083,7 +1072,7 @@ static struct nhg_hash_entry *depends_find_singleton(const struct nexthop *nh,
        /* Capture a snapshot of this single nh; it might be part of a list,
         * so we need to make a standalone copy.
         */
-       nexthop_copy(&lookup, nh, NULL);
+       nexthop_copy_no_recurse(&lookup, nh, NULL);
 
        nhe = zebra_nhg_find_nexthop(0, &lookup, afi, 0);