zebra: improve vty, simplify some primary/backup code

Improve vty output for routes and lsps with backups, including
json. Simplify or correct some code that uses both primary and
backup nexthops in dplane, nht.

Signed-off-by: Mark Stapp <mjs@voltanet.io>

diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
index 65ce8392cea30b4b515b75aa52d40b2f63032046..aabfcc84222b79f66a404e8b633032732dcc2d7f 100644 (file)
--- a/zebra/zebra_dplane.c
+++ b/zebra/zebra_dplane.c
@@ -2001,10 +2001,19 @@ int dplane_ctx_lsp_init(struct zebra_dplane_ctx *ctx, enum dplane_op_e op,
                        break;
                }
 
-               /* Need to copy flags too */
+               /* Need to copy flags and backup info too */
                new_nhlfe->flags = nhlfe->flags;
                new_nhlfe->nexthop->flags = nhlfe->nexthop->flags;
 
+               if (CHECK_FLAG(new_nhlfe->nexthop->flags,
+                              NEXTHOP_FLAG_HAS_BACKUP)) {
+                       new_nhlfe->nexthop->backup_num =
+                               nhlfe->nexthop->backup_num;
+                       memcpy(new_nhlfe->nexthop->backup_idx,
+                              nhlfe->nexthop->backup_idx,
+                              new_nhlfe->nexthop->backup_num);
+               }
+
                if (nhlfe == lsp->best_nhlfe)
                        ctx->u.lsp.best_nhlfe = new_nhlfe;
        }
@@ -2104,8 +2113,9 @@ static int dplane_ctx_pw_init(struct zebra_dplane_ctx *ctx,
 
                        if (re) {
                                nhg = rib_get_fib_nhg(re);
-                               copy_nexthops(&(ctx->u.pw.nhg.nexthop),
-                                             nhg->nexthop, NULL);
+                               if (nhg && nhg->nexthop)
+                                       copy_nexthops(&(ctx->u.pw.nhg.nexthop),
+                                                     nhg->nexthop, NULL);
                        }
                        route_unlock_node(rn);
                }
@@ -2461,6 +2471,7 @@ dplane_route_notif_update(struct route_node *rn,
        enum zebra_dplane_result ret = ZEBRA_DPLANE_REQUEST_FAILURE;
        struct zebra_dplane_ctx *new_ctx = NULL;
        struct nexthop *nexthop;
+       struct nexthop_group *nhg;
 
        if (rn == NULL || re == NULL)
                goto done;
@@ -2482,13 +2493,16 @@ dplane_route_notif_update(struct route_node *rn,
                nexthops_free(new_ctx->u.rinfo.zd_ng.nexthop);
                new_ctx->u.rinfo.zd_ng.nexthop = NULL;
 
-               copy_nexthops(&(new_ctx->u.rinfo.zd_ng.nexthop),
-                             (rib_get_fib_nhg(re))->nexthop, NULL);
+               nhg = rib_get_fib_nhg(re);
+               if (nhg && nhg->nexthop)
+                       copy_nexthops(&(new_ctx->u.rinfo.zd_ng.nexthop),
+                                     nhg->nexthop, NULL);
 
-               /* Check for backup nexthops also */
-               if (re->fib_backup_ng.nexthop) {
+               /* Check for installed backup nexthops also */
+               nhg = rib_get_fib_backup_nhg(re);
+               if (nhg && nhg->nexthop) {
                        copy_nexthops(&(new_ctx->u.rinfo.zd_ng.nexthop),
-                                     re->fib_backup_ng.nexthop, NULL);
+                                     nhg->nexthop, NULL);
                }
 
                for (ALL_NEXTHOPS(new_ctx->u.rinfo.zd_ng, nexthop))
@@ -4028,19 +4042,19 @@ bool dplane_is_in_shutdown(void)
  */
 void zebra_dplane_pre_finish(void)
 {
-       struct zebra_dplane_provider *dp;
+       struct zebra_dplane_provider *prov;
 
        if (IS_ZEBRA_DEBUG_DPLANE)
-               zlog_debug("Zebra dataplane pre-fini called");
+               zlog_debug("Zebra dataplane pre-finish called");
 
        zdplane_info.dg_is_shutdown = true;
 
        /* Notify provider(s) of pending shutdown. */
-       TAILQ_FOREACH(dp, &zdplane_info.dg_providers_q, dp_prov_link) {
-               if (dp->dp_fini == NULL)
+       TAILQ_FOREACH(prov, &zdplane_info.dg_providers_q, dp_prov_link) {
+               if (prov->dp_fini == NULL)
                        continue;
 
-               dp->dp_fini(dp, true);
+               prov->dp_fini(prov, true /* early */);
        }
 }
 
@@ -4362,7 +4376,10 @@ void zebra_dplane_shutdown(void)
        zdplane_info.dg_pthread = NULL;
        zdplane_info.dg_master = NULL;
 
-       /* Notify provider(s) of final shutdown. */
+       /* Notify provider(s) of final shutdown.
+        * Note that this call is in the main pthread, so providers must
+        * be prepared for that.
+        */
        TAILQ_FOREACH(dp, &zdplane_info.dg_providers_q, dp_prov_link) {
                if (dp->dp_fini == NULL)
                        continue;

diff --git a/zebra/zebra_mpls.c b/zebra/zebra_mpls.c
index f37552a542cb6dd819df572a65c01f8e4712334f..1e7aa41a63327c3ef0c39d0a4472ca8b94b0d35e 100644 (file)
--- a/zebra/zebra_mpls.c
+++ b/zebra/zebra_mpls.c
@@ -120,7 +120,8 @@ static int mpls_lsp_uninstall_all(struct hash *lsp_table, zebra_lsp_t *lsp,
                                  enum lsp_types_t type);
 static int mpls_static_lsp_uninstall_all(struct zebra_vrf *zvrf,
                                         mpls_label_t in_label);
-static void nhlfe_print(zebra_nhlfe_t *nhlfe, struct vty *vty);
+static void nhlfe_print(zebra_nhlfe_t *nhlfe, struct vty *vty,
+                       const char *indent);
 static void lsp_print(struct vty *vty, zebra_lsp_t *lsp);
 static void *slsp_alloc(void *p);
 static int snhlfe_match(zebra_snhlfe_t *snhlfe, enum nexthop_types_t gtype,
@@ -1506,7 +1507,9 @@ static json_object *nhlfe_json(zebra_nhlfe_t *nhlfe)
 {
        char buf[BUFSIZ];
        json_object *json_nhlfe = NULL;
+       json_object *json_backups = NULL;
        struct nexthop *nexthop = nhlfe->nexthop;
+       int i;
 
        json_nhlfe = json_object_new_object();
        json_object_string_add(json_nhlfe, "type", nhlfe_type2str(nhlfe->type));
@@ -1537,13 +1540,27 @@ static json_object *nhlfe_json(zebra_nhlfe_t *nhlfe)
        default:
                break;
        }
+
+       if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) {
+               json_backups = json_object_new_array();
+               for (i = 0; i < nexthop->backup_num; i++) {
+                       json_object_array_add(
+                               json_backups,
+                               json_object_new_int(nexthop->backup_idx[i]));
+               }
+
+               json_object_object_add(json_nhlfe, "backupIndex",
+                                      json_backups);
+       }
+
        return json_nhlfe;
 }
 
 /*
  * Print the NHLFE for a LSP forwarding entry.
  */
-static void nhlfe_print(zebra_nhlfe_t *nhlfe, struct vty *vty)
+static void nhlfe_print(zebra_nhlfe_t *nhlfe, struct vty *vty,
+                       const char *indent)
 {
        struct nexthop *nexthop;
        char buf[MPLS_LABEL_STRLEN];
@@ -1558,6 +1575,10 @@ static void nhlfe_print(zebra_nhlfe_t *nhlfe, struct vty *vty)
                               nexthop->nh_label->label,
                               buf, sizeof(buf), 0),
                nhlfe->distance);
+
+       if (indent)
+               vty_out(vty, "%s", indent);
+
        switch (nexthop->type) {
        case NEXTHOP_TYPE_IPV4:
        case NEXTHOP_TYPE_IPV4_IFINDEX:
@@ -1602,9 +1623,9 @@ static void lsp_print(struct vty *vty, zebra_lsp_t *lsp)
                                                           : "");
 
        frr_each(nhlfe_list, &lsp->nhlfe_list, nhlfe) {
-               nhlfe_print(nhlfe, vty);
+               nhlfe_print(nhlfe, vty, NULL);
 
-               if (nhlfe->nexthop &&
+               if (nhlfe->nexthop == NULL ||
                    !CHECK_FLAG(nhlfe->nexthop->flags,
                                NEXTHOP_FLAG_HAS_BACKUP))
                        continue;
@@ -1622,7 +1643,7 @@ static void lsp_print(struct vty *vty, zebra_lsp_t *lsp)
 
                        if (backup) {
                                vty_out(vty, "   [backup %d]", i);
-                               nhlfe_print(backup, vty);
+                               nhlfe_print(backup, vty, "   ");
                        }
                }
        }
@@ -1646,6 +1667,19 @@ static json_object *lsp_json(zebra_lsp_t *lsp)
                json_object_array_add(json_nhlfe_list, nhlfe_json(nhlfe));
 
        json_object_object_add(json, "nexthops", json_nhlfe_list);
+       json_nhlfe_list = NULL;
+
+
+       frr_each(nhlfe_list, &lsp->backup_nhlfe_list, nhlfe) {
+               if (json_nhlfe_list == NULL)
+                       json_nhlfe_list = json_object_new_array();
+
+               json_object_array_add(json_nhlfe_list, nhlfe_json(nhlfe));
+       }
+
+       if (json_nhlfe_list)
+               json_object_object_add(json, "backupNexthops", json_nhlfe_list);
+
        return json;
 }
 

diff --git a/zebra/zebra_nhg.c b/zebra/zebra_nhg.c
index 9bfd7aacb70ce06c21e8d47d2070d475f80263cc..c0580908444c7f38a83aba733e17620f9339f3fb 100644 (file)
--- a/zebra/zebra_nhg.c
+++ b/zebra/zebra_nhg.c
@@ -1951,8 +1951,11 @@ static int nexthop_active(afi_t afi, struct route_entry *re,
                                goto done_with_match;
                        }
 
-                       /* Examine installed nexthops */
-                       nhg = &match->nhe->nhg;
+                       /* Examine installed nexthops; note that there
+                        * may not be any installed primary nexthops if
+                        * only backups are installed.
+                        */
+                       nhg = rib_get_fib_nhg(match);
                        for (ALL_NEXTHOPS_PTR(nhg, newhop)) {
                                if (!nexthop_valid_resolve(nexthop, newhop))
                                        continue;
@@ -1973,8 +1976,7 @@ static int nexthop_active(afi_t afi, struct route_entry *re,
                         * dedicated fib list.
                         */
                        nhg = rib_get_fib_backup_nhg(match);
-                       if (nhg == NULL ||
-                           nhg == zebra_nhg_get_backup_nhg(match->nhe))
+                       if (nhg == NULL || nhg->nexthop == NULL)
                                goto done_with_match;
 
                        for (ALL_NEXTHOPS_PTR(nhg, newhop)) {

diff --git a/zebra/zebra_rnh.c b/zebra/zebra_rnh.c
index d1a5cf2a9d332fcdef28d7546b60301cde054f7a..9fcd9a33fdcedf701b3ba520d8501399d63ab6fb 100644 (file)
--- a/zebra/zebra_rnh.c
+++ b/zebra/zebra_rnh.c
@@ -1040,22 +1040,10 @@ static bool compare_valid_nexthops(struct route_entry *r1,
         * backups will be in the 'fib' list.
         */
        nhg1 = rib_get_fib_backup_nhg(r1);
-       if (nhg1 == zebra_nhg_get_backup_nhg(r1->nhe))
-               nhg1 = NULL;
-
        nhg2 = rib_get_fib_backup_nhg(r2);
-       if (nhg2 == zebra_nhg_get_backup_nhg(r2->nhe))
-               nhg2 = NULL;
-
-       if (nhg1)
-               nh1 = nhg1->nexthop;
-       else
-               nh1 = NULL;
 
-       if (nhg2)
-               nh2 = nhg2->nexthop;
-       else
-               nh2 = NULL;
+       nh1 = nhg1->nexthop;
+       nh2 = nhg2->nexthop;
 
        while (1) {
                /* Find each backup list's next valid nexthop */
@@ -1180,9 +1168,6 @@ static int send_client(struct rnh *rnh, struct zserv *client,
                        }
 
                nhg = rib_get_fib_backup_nhg(re);
-               if (nhg == zebra_nhg_get_backup_nhg(re->nhe))
-                       nhg = NULL;
-
                if (nhg) {
                        for (ALL_NEXTHOPS_PTR(nhg, nh))
                                if (rnh_nexthop_valid(re, nh)) {

diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c
index 9530d5cc776161ad693aea43de81e9ef6e628600..0005777006c24cf1b563567a8134723665a8b6ef 100644 (file)
--- a/zebra/zebra_vty.c
+++ b/zebra/zebra_vty.c
@@ -172,11 +172,24 @@ DEFUN (show_ip_rpf_addr,
 }
 
 static char re_status_output_char(const struct route_entry *re,
-                                 const struct nexthop *nhop)
+                                 const struct nexthop *nhop,
+                                 bool is_fib)
 {
        if (CHECK_FLAG(re->status, ROUTE_ENTRY_INSTALLED)) {
-               if (!CHECK_FLAG(nhop->flags, NEXTHOP_FLAG_DUPLICATE) &&
-                   !CHECK_FLAG(nhop->flags, NEXTHOP_FLAG_RECURSIVE))
+               bool star_p = false;
+
+               if (nhop &&
+                   !CHECK_FLAG(nhop->flags, NEXTHOP_FLAG_DUPLICATE) &&
+                   !CHECK_FLAG(nhop->flags, NEXTHOP_FLAG_RECURSIVE)) {
+                       /* More-specific test for 'fib' output */
+                       if (is_fib) {
+                               star_p = !!CHECK_FLAG(nhop->flags,
+                                                     NEXTHOP_FLAG_FIB);
+                       } else
+                               star_p = true;
+               }
+
+               if (star_p)
                        return '*';
                else
                        return ' ';
@@ -207,7 +220,8 @@ static void show_nh_backup_helper(struct vty *vty,
 
        /* Double-check that there _is_ a backup */
        if (!CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP) ||
-           re->nhe->backup_info == NULL || re->nhe->backup_info->nhe == NULL)
+           re->nhe->backup_info == NULL || re->nhe->backup_info->nhe == NULL ||
+           re->nhe->backup_info->nhe->nhg.nexthop == NULL)
                return;
 
        /* Locate the backup nexthop(s) */
@@ -221,6 +235,9 @@ static void show_nh_backup_helper(struct vty *vty,
                                break;
                }
 
+               /* It's possible for backups to be recursive too,
+                * so walk the recursive resolution list if present.
+                */
                temp = backup;
                while (backup) {
                        vty_out(vty, "  ");
@@ -258,7 +275,7 @@ static void show_nexthop_detail_helper(struct vty *vty,
                        nexthop->rparent ? "  " : "");
        else
                vty_out(vty, "  %c%s",
-                       re_status_output_char(re, nexthop),
+                       re_status_output_char(re, nexthop, false),
                        nexthop->rparent ? "  " : "");
 
        switch (nexthop->type) {
@@ -768,18 +785,19 @@ static void vty_show_ip_route(struct vty *vty, struct route_node *rn,
                              struct route_entry *re, json_object *json,
                              bool is_fib)
 {
-       struct nexthop *nexthop;
+       const struct nexthop *nexthop;
        int len = 0;
        char buf[SRCDEST2STR_BUFFER];
        json_object *json_nexthops = NULL;
        json_object *json_nexthop = NULL;
        json_object *json_route = NULL;
        time_t uptime;
-       struct vrf *vrf = NULL;
-       rib_dest_t *dest = rib_dest_from_rnode(rn);
-       struct nexthop_group *nhg;
+       const struct vrf *vrf = NULL;
+       const rib_dest_t *dest = rib_dest_from_rnode(rn);
+       const struct nexthop_group *nhg;
        char up_str[MONOTIME_STRLEN];
-       bool first_p;
+       bool first_p = true;
+       bool nhg_from_backup = false;
 
        uptime = monotime(NULL);
        uptime -= re->uptime;
@@ -854,9 +872,11 @@ static void vty_show_ip_route(struct vty *vty, struct route_node *rn,
 
                for (ALL_NEXTHOPS_PTR(nhg, nexthop)) {
                        json_nexthop = json_object_new_object();
+                       show_nexthop_json_helper(json_nexthop,
+                                                nexthop, re);
 
-                       show_nexthop_json_helper(json_nexthop, nexthop, re);
-                       json_object_array_add(json_nexthops, json_nexthop);
+                       json_object_array_add(json_nexthops,
+                                             json_nexthop);
                }
 
                json_object_object_add(json_route, "nexthops", json_nexthops);
@@ -867,7 +887,7 @@ static void vty_show_ip_route(struct vty *vty, struct route_node *rn,
                else
                        nhg = zebra_nhg_get_backup_nhg(re->nhe);
 
-               if (nhg) {
+               if (nhg && nhg->nexthop) {
                        json_nexthops = json_object_new_array();
 
                        for (ALL_NEXTHOPS_PTR(nhg, nexthop)) {
@@ -887,42 +907,62 @@ static void vty_show_ip_route(struct vty *vty, struct route_node *rn,
                return;
        }
 
+       /* Prefix information, and first nexthop. If we're showing 'fib',
+        * and there are no installed primary nexthops, see if there are any
+        * backup nexthops and start with those.
+        */
+       if (is_fib && nhg->nexthop == NULL) {
+               nhg = rib_get_fib_backup_nhg(re);
+               nhg_from_backup = true;
+       }
+
+       len = vty_out(vty, "%c", zebra_route_char(re->type));
+       if (re->instance)
+               len += vty_out(vty, "[%d]", re->instance);
+       if (nhg_from_backup && nhg->nexthop) {
+               len += vty_out(
+                       vty, "%cb%c %s",
+                       CHECK_FLAG(re->flags, ZEBRA_FLAG_SELECTED) ? '>' : ' ',
+                       re_status_output_char(re, nhg->nexthop, is_fib),
+                       srcdest_rnode2str(rn, buf, sizeof(buf)));
+       } else {
+               len += vty_out(
+                       vty, "%c%c %s",
+                       CHECK_FLAG(re->flags, ZEBRA_FLAG_SELECTED) ? '>' : ' ',
+                       re_status_output_char(re, nhg->nexthop, is_fib),
+                       srcdest_rnode2str(rn, buf, sizeof(buf)));
+       }
+
+       /* Distance and metric display. */
+       if (((re->type == ZEBRA_ROUTE_CONNECT) &&
+            (re->distance || re->metric)) ||
+           (re->type != ZEBRA_ROUTE_CONNECT))
+               len += vty_out(vty, " [%u/%u]", re->distance,
+                              re->metric);
+
        /* Nexthop information. */
-       first_p = true;
        for (ALL_NEXTHOPS_PTR(nhg, nexthop)) {
                if (first_p) {
                        first_p = false;
-
-                       /* Prefix information. */
-                       len = vty_out(vty, "%c", zebra_route_char(re->type));
-                       if (re->instance)
-                               len += vty_out(vty, "[%d]", re->instance);
-                       len += vty_out(
-                               vty, "%c%c %s",
-                               CHECK_FLAG(re->flags, ZEBRA_FLAG_SELECTED)
-                                       ? '>'
-                                       : ' ',
-                               re_status_output_char(re, nexthop),
-                               srcdest_rnode2str(rn, buf, sizeof(buf)));
-
-                       /* Distance and metric display. */
-                       if (((re->type == ZEBRA_ROUTE_CONNECT) &&
-                            (re->distance || re->metric)) ||
-                           (re->type != ZEBRA_ROUTE_CONNECT))
-                               len += vty_out(vty, " [%u/%u]", re->distance,
-                                              re->metric);
+               } else if (nhg_from_backup) {
+                       vty_out(vty, "  b%c%*c",
+                               re_status_output_char(re, nexthop, is_fib),
+                               len - 3 + (2 * nexthop_level(nexthop)), ' ');
                } else {
                        vty_out(vty, "  %c%*c",
-                               re_status_output_char(re, nexthop),
+                               re_status_output_char(re, nexthop, is_fib),
                                len - 3 + (2 * nexthop_level(nexthop)), ' ');
                }
 
                show_route_nexthop_helper(vty, re, nexthop);
-
                vty_out(vty, ", %s\n", up_str);
        }
 
-       /* Check for backup info if present */
+       /* If we only had backup nexthops, we're done */
+       if (nhg_from_backup)
+               return;
+
+       /* Check for backup nexthop info if present */
        if (is_fib)
                nhg = rib_get_fib_backup_nhg(re);
        else