]> git.proxmox.com Git - mirror_frr.git/blobdiff - zebra/zebra_nhg.c
zebra: can't improve efficiency for recursive depends
[mirror_frr.git] / zebra / zebra_nhg.c
index d33f3a432a05dc64ab34554283d5f3335c8552e8..2be9238ee87092d6b33685430c733eff3d062e17 100644 (file)
@@ -49,7 +49,8 @@ DEFINE_MTYPE_STATIC(ZEBRA, NHG_CTX, "Nexthop Group Context");
 /* id counter to keep in sync with kernel */
 uint32_t id_counter;
 
-static struct nhg_hash_entry *depends_find(struct nexthop *nh, afi_t afi);
+static struct nhg_hash_entry *depends_find(const struct nexthop *nh,
+                                          afi_t afi);
 static void depends_add(struct nhg_connected_tree_head *head,
                        struct nhg_hash_entry *depend);
 static struct nhg_hash_entry *
@@ -299,13 +300,22 @@ zebra_nhg_connect_depends(struct nhg_hash_entry *nhe,
        }
 }
 
-static struct nhg_hash_entry *zebra_nhg_copy(struct nhg_hash_entry *copy,
-                                            uint32_t id)
+struct nhg_hash_entry *zebra_nhg_alloc(void)
 {
        struct nhg_hash_entry *nhe;
 
        nhe = XCALLOC(MTYPE_NHG, sizeof(struct nhg_hash_entry));
 
+       return nhe;
+}
+
+static struct nhg_hash_entry *zebra_nhg_copy(const struct nhg_hash_entry *copy,
+                                            uint32_t id)
+{
+       struct nhg_hash_entry *nhe;
+
+       nhe = zebra_nhg_alloc();
+
        nhe->id = id;
 
        nhe->nhg = nexthop_group_new();
@@ -468,7 +478,7 @@ static void handle_recursive_depend(struct nhg_connected_tree_head *nhg_depends,
        struct nhg_hash_entry *depend = NULL;
        struct nexthop_group resolved_ng = {};
 
-       _nexthop_group_add_sorted(&resolved_ng, nh);
+       nexthop_group_add_sorted(&resolved_ng, nh);
 
        depend = zebra_nhg_rib_find(0, &resolved_ng, afi);
        depends_add(nhg_depends, depend);
@@ -582,27 +592,13 @@ zebra_nhg_find_nexthop(uint32_t id, struct nexthop *nh, afi_t afi, int type)
        struct nhg_hash_entry *nhe = NULL;
        struct nexthop_group nhg = {};
 
-       _nexthop_group_add_sorted(&nhg, nh);
+       nexthop_group_add_sorted(&nhg, nh);
 
-       zebra_nhg_find(&nhe, id, &nhg, NULL, nh->vrf_id, afi, 0);
+       zebra_nhg_find(&nhe, id, &nhg, NULL, nh->vrf_id, afi, type);
 
        return nhe;
 }
 
-static struct nhg_ctx *nhg_ctx_new()
-{
-       struct nhg_ctx *new = NULL;
-
-       new = XCALLOC(MTYPE_NHG_CTX, sizeof(struct nhg_ctx));
-
-       return new;
-}
-
-static void nhg_ctx_free(struct nhg_ctx *ctx)
-{
-       XFREE(MTYPE_NHG_CTX, ctx);
-}
-
 static uint32_t nhg_ctx_get_id(const struct nhg_ctx *ctx)
 {
        return ctx->id;
@@ -658,6 +654,36 @@ static struct nh_grp *nhg_ctx_get_grp(struct nhg_ctx *ctx)
        return ctx->u.grp;
 }
 
+static struct nhg_ctx *nhg_ctx_new()
+{
+       struct nhg_ctx *new = NULL;
+
+       new = XCALLOC(MTYPE_NHG_CTX, sizeof(struct nhg_ctx));
+
+       return new;
+}
+
+static void nhg_ctx_free(struct nhg_ctx **ctx)
+{
+       struct nexthop *nh;
+
+       if (ctx == NULL)
+               return;
+
+       assert((*ctx) != NULL);
+
+       if (nhg_ctx_get_count(*ctx))
+               goto done;
+
+       nh = nhg_ctx_get_nh(*ctx);
+
+       nexthop_del_labels(nh);
+
+done:
+       XFREE(MTYPE_NHG_CTX, *ctx);
+       *ctx = NULL;
+}
+
 static struct nhg_ctx *nhg_ctx_init(uint32_t id, struct nexthop *nh,
                                    struct nh_grp *grp, vrf_id_t vrf_id,
                                    afi_t afi, int type, uint8_t count)
@@ -906,23 +932,13 @@ static int nhg_ctx_process_del(struct nhg_ctx *ctx)
        return 0;
 }
 
-static void nhg_ctx_process_finish(struct nhg_ctx *ctx)
+static void nhg_ctx_fini(struct nhg_ctx **ctx)
 {
-       struct nexthop *nh;
-
        /*
         * Just freeing for now, maybe do something more in the future
         * based on flag.
         */
 
-       if (nhg_ctx_get_count(ctx))
-               goto done;
-
-       nh = nhg_ctx_get_nh(ctx);
-
-       nexthop_del_labels(nh);
-
-done:
        nhg_ctx_free(ctx);
 }
 
@@ -978,7 +994,7 @@ int nhg_ctx_process(struct nhg_ctx *ctx)
 
        nhg_ctx_set_status(ctx, (ret ? NHG_CTX_FAILURE : NHG_CTX_SUCCESS));
 
-       nhg_ctx_process_finish(ctx);
+       nhg_ctx_fini(&ctx);
 
        return ret;
 }
@@ -1007,7 +1023,7 @@ int zebra_nhg_kernel_find(uint32_t id, struct nexthop *nh, struct nh_grp *grp,
                return nhg_ctx_process(ctx);
 
        if (queue_add(ctx)) {
-               nhg_ctx_process_finish(ctx);
+               nhg_ctx_fini(&ctx);
                return -1;
        }
 
@@ -1024,7 +1040,7 @@ int zebra_nhg_kernel_del(uint32_t id)
        nhg_ctx_set_op(ctx, NHG_CTX_OP_DEL);
 
        if (queue_add(ctx)) {
-               nhg_ctx_process_finish(ctx);
+               nhg_ctx_fini(&ctx);
                return -1;
        }
 
@@ -1032,14 +1048,20 @@ int zebra_nhg_kernel_del(uint32_t id)
 }
 
 /* Some dependency helper functions */
-static struct nhg_hash_entry *depends_find(struct nexthop *nh, afi_t afi)
+static struct nhg_hash_entry *depends_find_recursive(const struct nexthop *nh,
+                                                    afi_t afi)
 {
+       struct nhg_hash_entry *nhe;
        struct nexthop *lookup = NULL;
-       struct nhg_hash_entry *nhe = 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);
 
-       /* Clear it, in case its a group */
+       /* Make it a single, recursive nexthop */
        nexthops_free(lookup->next);
        nexthops_free(lookup->prev);
        lookup->next = NULL;
@@ -1052,6 +1074,44 @@ static struct nhg_hash_entry *depends_find(struct nexthop *nh, afi_t afi)
        return nhe;
 }
 
+static struct nhg_hash_entry *depends_find_singleton(const struct nexthop *nh,
+                                                    afi_t afi)
+{
+       struct nhg_hash_entry *nhe;
+       struct nexthop lookup = {};
+
+       /* 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);
+
+       nhe = zebra_nhg_find_nexthop(0, &lookup, afi, 0);
+
+       /* The copy may have allocated labels; free them if necessary. */
+       nexthop_del_labels(&lookup);
+
+       return nhe;
+}
+
+static struct nhg_hash_entry *depends_find(const struct nexthop *nh, afi_t afi)
+{
+       struct nhg_hash_entry *nhe = NULL;
+
+       if (!nh)
+               goto done;
+
+       /* We are separating these functions out to increase handling speed
+        * in the non-recursive case (by not alloc/freeing)
+        */
+       if (CHECK_FLAG(nh->flags, NEXTHOP_FLAG_RECURSIVE))
+               nhe = depends_find_recursive(nh, afi);
+       else
+               nhe = depends_find_singleton(nh, afi);
+
+done:
+       return nhe;
+}
+
 static void depends_add(struct nhg_connected_tree_head *head,
                        struct nhg_hash_entry *depend)
 {
@@ -1118,12 +1178,8 @@ static void zebra_nhg_free_members(struct nhg_hash_entry *nhe)
        nhg_connected_tree_free(&nhe->nhg_dependents);
 }
 
-void zebra_nhg_free(void *arg)
+void zebra_nhg_free(struct nhg_hash_entry *nhe)
 {
-       struct nhg_hash_entry *nhe = NULL;
-
-       nhe = (struct nhg_hash_entry *)arg;
-
        if (nhe->refcnt)
                zlog_debug("nhe_id=%u hash refcnt=%d", nhe->id, nhe->refcnt);
 
@@ -1132,6 +1188,11 @@ void zebra_nhg_free(void *arg)
        XFREE(MTYPE_NHG, nhe);
 }
 
+void zebra_nhg_hash_free(void *p)
+{
+       zebra_nhg_free((struct nhg_hash_entry *)p);
+}
+
 void zebra_nhg_decrement_ref(struct nhg_hash_entry *nhe)
 {
        nhe->refcnt--;
@@ -1344,6 +1405,17 @@ static int nexthop_active(afi_t afi, struct route_entry *re,
                }
        }
 
+       if ((top->p.family == AF_INET && top->p.prefixlen == 32
+            && nexthop->gate.ipv4.s_addr == top->p.u.prefix4.s_addr)
+           || (top->p.family == AF_INET6 && top->p.prefixlen == 128
+               && memcmp(&nexthop->gate.ipv6, &top->p.u.prefix6, 16) == 0)) {
+               if (IS_ZEBRA_DEBUG_RIB_DETAILED)
+                       zlog_debug(
+                               "\t:%s: Attempting to install a max prefixlength route through itself",
+                               __PRETTY_FUNCTION__);
+               return 0;
+       }
+
        /* Make lookup prefix. */
        memset(&p, 0, sizeof(struct prefix));
        switch (afi) {
@@ -1426,7 +1498,7 @@ static int nexthop_active(afi_t afi, struct route_entry *re,
 
                if (match->type == ZEBRA_ROUTE_CONNECT) {
                        /* Directly point connected route. */
-                       newhop = match->ng->nexthop;
+                       newhop = match->nhe->nhg->nexthop;
                        if (newhop) {
                                if (nexthop->type == NEXTHOP_TYPE_IPV4
                                    || nexthop->type == NEXTHOP_TYPE_IPV6)
@@ -1435,7 +1507,7 @@ static int nexthop_active(afi_t afi, struct route_entry *re,
                        return 1;
                } else if (CHECK_FLAG(re->flags, ZEBRA_FLAG_ALLOW_RECURSION)) {
                        resolved = 0;
-                       for (ALL_NEXTHOPS_PTR(match->ng, newhop)) {
+                       for (ALL_NEXTHOPS_PTR(match->nhe->nhg, newhop)) {
                                if (!CHECK_FLAG(match->status,
                                                ROUTE_ENTRY_INSTALLED))
                                        continue;
@@ -1456,7 +1528,7 @@ static int nexthop_active(afi_t afi, struct route_entry *re,
                        return resolved;
                } else if (re->type == ZEBRA_ROUTE_STATIC) {
                        resolved = 0;
-                       for (ALL_NEXTHOPS_PTR(match->ng, newhop)) {
+                       for (ALL_NEXTHOPS_PTR(match->nhe->nhg, newhop)) {
                                if (!CHECK_FLAG(match->status,
                                                ROUTE_ENTRY_INSTALLED))
                                        continue;
@@ -1647,7 +1719,7 @@ int nexthop_active_update(struct route_node *rn, struct route_entry *re)
        UNSET_FLAG(re->status, ROUTE_ENTRY_CHANGED);
 
        /* Copy over the nexthops in current state */
-       nexthop_group_copy(&new_grp, re->ng);
+       nexthop_group_copy(&new_grp, re->nhe->nhg);
 
        for (nexthop = new_grp.nexthop; nexthop; nexthop = nexthop->next) {
 
@@ -1665,10 +1737,13 @@ int nexthop_active_update(struct route_node *rn, struct route_entry *re)
                new_active =
                        nexthop_active_check(rn, re, nexthop);
 
-               if (new_active
-                   && nexthop_group_active_nexthop_num(&new_grp)
-                              >= zrouter.multipath_num) {
-                       UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE);
+               if (new_active && curr_active >= zrouter.multipath_num) {
+                       struct nexthop *nh;
+
+                       /* Set it and its resolved nexthop as inactive. */
+                       for (nh = nexthop; nh; nh = nh->resolved)
+                               UNSET_FLAG(nh->flags, NEXTHOP_FLAG_ACTIVE);
+
                        new_active = 0;
                }
 
@@ -1694,7 +1769,7 @@ int nexthop_active_update(struct route_node *rn, struct route_entry *re)
 
                new_nhe = zebra_nhg_rib_find(0, &new_grp, rt_afi);
 
-               zebra_nhg_re_update_ref(re, new_nhe);
+               route_entry_update_nhe(re, new_nhe);
        }
 
        if (curr_active) {
@@ -1720,40 +1795,6 @@ int nexthop_active_update(struct route_node *rn, struct route_entry *re)
        return curr_active;
 }
 
-static void zebra_nhg_re_attach_ref(struct route_entry *re,
-                                   struct nhg_hash_entry *new)
-{
-       re->ng = new->nhg;
-       re->nhe_id = new->id;
-
-       zebra_nhg_increment_ref(new);
-}
-
-int zebra_nhg_re_update_ref(struct route_entry *re, struct nhg_hash_entry *new)
-{
-       struct nhg_hash_entry *old = NULL;
-       int ret = 0;
-
-       if (new == NULL) {
-               re->ng = NULL;
-               goto done;
-       }
-
-       if (re->nhe_id != new->id) {
-               old = zebra_nhg_lookup_id(re->nhe_id);
-
-               zebra_nhg_re_attach_ref(re, new);
-
-               if (old)
-                       zebra_nhg_decrement_ref(old);
-       } else if (!re->ng)
-               /* This is the first time it's being attached */
-               zebra_nhg_re_attach_ref(re, new);
-
-done:
-       return ret;
-}
-
 /* Convert a nhe into a group array */
 uint8_t zebra_nhg_nhe2grp(struct nh_grp *grp, struct nhg_hash_entry *nhe,
                          int max_num)
@@ -1790,7 +1831,7 @@ uint8_t zebra_nhg_nhe2grp(struct nh_grp *grp, struct nhg_hash_entry *nhe,
                if (!duplicate) {
                        grp[i].id = depend->id;
                        /* We aren't using weights for anything right now */
-                       grp[i].weight = 0;
+                       grp[i].weight = depend->nhg->nexthop->weight;
                        i++;
                }