zebra: Handle out of order kernel nexthop groups

Add a mechanism to requeue groups we receive from the
kernel if the IDs are in a weird order (Group ID is lower
than individual nexthop IDs for example).

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

diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c
index c22bf349b75c793f97aded3f728d6ec514a22128..4130365f2d0a371dce27ed34f07c524fcb7a184d 100644 (file)
--- a/zebra/zebra_nhg.c
+++ b/zebra/zebra_nhg.c
@@ -408,9 +408,9 @@ bool zebra_nhg_hash_id_equal(const void *arg1, const void *arg2)
        return nhe1->id == nhe2->id;
 }
 
-static void zebra_nhg_process_grp(struct nexthop_group *nhg,
-                                 struct nhg_connected_tree_head *depends,
-                                 struct nh_grp *grp, uint8_t count)
+static int zebra_nhg_process_grp(struct nexthop_group *nhg,
+                                struct nhg_connected_tree_head *depends,
+                                struct nh_grp *grp, uint8_t count)
 {
        nhg_connected_tree_init(depends);
 
@@ -428,7 +428,7 @@ static void zebra_nhg_process_grp(struct nexthop_group *nhg,
                                EC_ZEBRA_NHG_SYNC,
                                "Received Nexthop Group from the kernel with a dependent Nexthop ID (%u) which we do not have in our table",
                                grp[i].id);
-                       return;
+                       return -1;
                }
 
                /*
@@ -440,6 +440,8 @@ static void zebra_nhg_process_grp(struct nexthop_group *nhg,
 
                copy_nexthops(&nhg->nexthop, depend->nhg->nexthop, NULL);
        }
+
+       return 0;
 }
 
 static void handle_recursive_depend(struct nhg_connected_tree_head *nhg_depends,
@@ -566,12 +568,12 @@ static uint32_t nhg_ctx_get_id(const struct nhg_ctx *ctx)
        return ctx->id;
 }
 
-static void nhg_ctx_set_status(struct nhg_ctx *ctx, enum nhg_ctx_result status)
+static void nhg_ctx_set_status(struct nhg_ctx *ctx, enum nhg_ctx_status status)
 {
        ctx->status = status;
 }
 
-static enum nhg_ctx_result nhg_ctx_get_status(const struct nhg_ctx *ctx)
+static enum nhg_ctx_status nhg_ctx_get_status(const struct nhg_ctx *ctx)
 {
        return ctx->status;
 }
@@ -742,8 +744,13 @@ static int nhg_ctx_process_new(struct nhg_ctx *ctx)
 
        if (nhg_ctx_get_count(ctx)) {
                nhg = nexthop_group_new();
-               zebra_nhg_process_grp(nhg, &nhg_depends, nhg_ctx_get_grp(ctx),
-                                     count);
+               if (zebra_nhg_process_grp(nhg, &nhg_depends,
+                                         nhg_ctx_get_grp(ctx), count)) {
+                       depends_decrement_free(&nhg_depends);
+                       nexthop_group_free_delete(&nhg);
+                       return ENOENT;
+               }
+
                if (!zebra_nhg_find(&nhe, id, nhg, &nhg_depends, vrf_id, type,
                                    afi))
                        depends_decrement_free(&nhg_depends);
@@ -839,6 +846,22 @@ done:
                nhg_ctx_free(ctx);
 }
 
+static int queue_add(struct nhg_ctx *ctx)
+{
+       /* If its queued or already processed do nothing */
+       if (nhg_ctx_get_status(ctx) == NHG_CTX_QUEUED)
+               return 0;
+
+       if (rib_queue_nhg_add(ctx)) {
+               nhg_ctx_set_status(ctx, NHG_CTX_FAILURE);
+               return -1;
+       }
+
+       nhg_ctx_set_status(ctx, NHG_CTX_QUEUED);
+
+       return 0;
+}
+
 int nhg_ctx_process(struct nhg_ctx *ctx)
 {
        int ret = 0;
@@ -846,6 +869,19 @@ int nhg_ctx_process(struct nhg_ctx *ctx)
        switch (nhg_ctx_get_op(ctx)) {
        case NHG_CTX_OP_NEW:
                ret = nhg_ctx_process_new(ctx);
+               if (nhg_ctx_get_count(ctx) && ret == ENOENT
+                   && nhg_ctx_get_status(ctx) != NHG_CTX_REQUEUED) {
+                       /* Depends probably came before group, re-queue.
+                        *
+                        * Only going to retry once, hence just using status
+                        * flag rather than counter.
+                        */
+                       nhg_ctx_set_status(ctx, NHG_CTX_NONE);
+                       if (queue_add(ctx) == 0) {
+                               nhg_ctx_set_status(ctx, NHG_CTX_REQUEUED);
+                               return 0;
+                       }
+               }
                break;
        case NHG_CTX_OP_DEL:
                ret = nhg_ctx_process_del(ctx);
@@ -860,22 +896,6 @@ int nhg_ctx_process(struct nhg_ctx *ctx)
        return ret;
 }
 
-static int queue_add(struct nhg_ctx *ctx)
-{
-       /* If its queued or already processed do nothing */
-       if (nhg_ctx_get_status(ctx))
-               return 0;
-
-       if (rib_queue_nhg_add(ctx)) {
-               nhg_ctx_set_status(ctx, NHG_CTX_FAILURE);
-               return -1;
-       }
-
-       nhg_ctx_set_status(ctx, NHG_CTX_QUEUED);
-
-       return 0;
-}
-
 /* Kernel-side, you either get a single new nexthop or a array of ID's */
 int zebra_nhg_kernel_find(uint32_t id, struct nexthop *nh, struct nh_grp *grp,
                          uint8_t count, vrf_id_t vrf_id, afi_t afi, int type,
@@ -959,7 +979,9 @@ depends_find_add(struct nhg_connected_tree_head *head, struct nexthop *nh,
        struct nhg_hash_entry *depend = NULL;
 
        depend = depends_find(nh, afi);
-       depends_add(head, depend);
+
+       if (depend)
+               depends_add(head, depend);
 
        return depend;
 }
@@ -970,7 +992,9 @@ depends_find_id_add(struct nhg_connected_tree_head *head, uint32_t id)
        struct nhg_hash_entry *depend = NULL;
 
        depend = zebra_nhg_lookup_id(id);
-       depends_add(head, depend);
+
+       if (depend)
+               depends_add(head, depend);
 
        return depend;
 }

diff --git a/zebra/zebra_nhg.h b/zebra/zebra_nhg.h
index eb9173457c5dd4caa64720ab1c407ba692d53b7b..812d9a1a90e8eacc031a6e66f759c2d31504ce7c 100644 (file)
--- a/zebra/zebra_nhg.h
+++ b/zebra/zebra_nhg.h
@@ -125,9 +125,10 @@ enum nhg_ctx_op_e {
        NHG_CTX_OP_DEL,
 };
 
-enum nhg_ctx_result {
+enum nhg_ctx_status {
        NHG_CTX_NONE = 0,
        NHG_CTX_QUEUED,
+       NHG_CTX_REQUEUED,
        NHG_CTX_SUCCESS,
        NHG_CTX_FAILURE,
 };
@@ -159,7 +160,7 @@ struct nhg_ctx {
        } u;
 
        enum nhg_ctx_op_e op;
-       enum nhg_ctx_result status;
+       enum nhg_ctx_status status;
 };