zebra: Refactor nexthop reading from zapi messages

Take the zebra code that reads nexthops and combine it
into one function so that when we add zapi messages
to send/receive nexthops we can take advantage of this function.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c
index 6d7e84235412dbaf668f7dbe2df221174e5b7782..1301a577f2989ac2b8ab04a69b8309555eabeefc 100644 (file)
--- a/zebra/zapi_msg.c
+++ b/zebra/zapi_msg.c
@@ -1578,99 +1578,72 @@ done:
        return nexthop;
 }
 
-static void zread_route_add(ZAPI_HANDLER_ARGS)
+static bool zapi_read_nexthops(struct zserv *client, struct zapi_route *api,
+                              struct route_entry *re,
+                              struct nexthop_group **png,
+                              struct nhg_backup_info **pbnhg)
 {
-       struct stream *s;
-       struct zapi_route api;
-       struct zapi_nexthop *api_nh;
-       afi_t afi;
-       struct prefix_ipv6 *src_p = NULL;
-       struct route_entry *re;
-       struct nexthop *nexthop = NULL, *last_nh;
        struct nexthop_group *ng = NULL;
        struct nhg_backup_info *bnhg = NULL;
-       int i, ret;
-       vrf_id_t vrf_id;
-       struct nhg_hash_entry nhe;
-       enum lsp_types_t label_type;
-       char nhbuf[NEXTHOP_STRLEN];
-       char labelbuf[MPLS_LABEL_STRLEN];
-
-       s = msg;
-       if (zapi_route_decode(s, &api) < 0) {
-               if (IS_ZEBRA_DEBUG_RECV)
-                       zlog_debug("%s: Unable to decode zapi_route sent",
-                                  __func__);
-               return;
-       }
+       uint16_t nexthop_num, i;;
+       struct zapi_nexthop *nhops;
+       struct nexthop *last_nh = NULL;
 
-       vrf_id = zvrf_id(zvrf);
+       assert(!(png && pbnhg));
 
-       if (IS_ZEBRA_DEBUG_RECV) {
-               char buf_prefix[PREFIX_STRLEN];
-
-               prefix2str(&api.prefix, buf_prefix, sizeof(buf_prefix));
-               zlog_debug("%s: p=(%u:%u)%s, msg flags=0x%x, flags=0x%x",
-                          __func__, vrf_id, api.tableid, buf_prefix, (int)api.message, api.flags);
+       if (png) {
+               *png = ng = nexthop_group_new();
+               nexthop_num = api->nexthop_num;
+               nhops = api->nexthops;
        }
 
-       /* Allocate new route. */
-       re = XCALLOC(MTYPE_RE, sizeof(struct route_entry));
-       re->type = api.type;
-       re->instance = api.instance;
-       re->flags = api.flags;
-       re->uptime = monotime(NULL);
-       re->vrf_id = vrf_id;
-
-       if (api.tableid)
-               re->table = api.tableid;
-       else
-               re->table = zvrf->table_id;
-
-       if (!CHECK_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP)
-           || api.nexthop_num == 0) {
-               flog_warn(EC_ZEBRA_RX_ROUTE_NO_NEXTHOPS,
-                         "%s: received a route without nexthops for prefix %pFX from client %s",
-                         __func__, &api.prefix,
-                         zebra_route_string(client->proto));
-
-               XFREE(MTYPE_RE, re);
-               return;
-       }
+       if (pbnhg && api->backup_nexthop_num > 0) {
+               if (IS_ZEBRA_DEBUG_RECV)
+                       zlog_debug("%s: adding %d backup nexthops",
+                                  __func__, api->backup_nexthop_num);
 
-       /* Report misuse of the backup flag */
-       if (CHECK_FLAG(api.message, ZAPI_MESSAGE_BACKUP_NEXTHOPS) &&
-           api.backup_nexthop_num == 0) {
-               if (IS_ZEBRA_DEBUG_RECV || IS_ZEBRA_DEBUG_EVENT)
-                       zlog_debug("%s: client %s: BACKUP flag set but no backup nexthops, prefix %pFX",
-                               __func__,
-                               zebra_route_string(client->proto), &api.prefix);
+               nexthop_num = api->backup_nexthop_num;
+               *pbnhg = bnhg = zebra_nhg_backup_alloc();
+               nhops = api->backup_nexthops;
        }
 
-       /* Use temporary list of nexthops */
-       ng = nexthop_group_new();
-
        /*
         * TBD should _all_ of the nexthop add operations use
         * api_nh->vrf_id instead of re->vrf_id ? I only changed
         * for cases NEXTHOP_TYPE_IPV4 and NEXTHOP_TYPE_IPV6.
         */
-       for (i = 0; i < api.nexthop_num; i++) {
-               api_nh = &api.nexthops[i];
+       for (i = 0; i < nexthop_num; i++) {
+               struct nexthop *nexthop;
+               enum lsp_types_t label_type;
+               char nhbuf[NEXTHOP_STRLEN];
+               char labelbuf[MPLS_LABEL_STRLEN];
+               struct zapi_nexthop *api_nh = &nhops[i];
 
                /* Convert zapi nexthop */
-               nexthop = nexthop_from_zapi(re, api_nh, &api);
+               nexthop = nexthop_from_zapi(re, api_nh, api);
                if (!nexthop) {
                        flog_warn(
                                EC_ZEBRA_NEXTHOP_CREATION_FAILED,
-                               "%s: Nexthops Specified: %d but we failed to properly create one",
-                               __func__, api.nexthop_num);
-                       nexthop_group_delete(&ng);
-                       XFREE(MTYPE_RE, re);
-                       return;
+                               "%s: Nexthops Specified: %u(%u) but we failed to properly create one",
+                               __func__, nexthop_num, i);
+                       if (ng)
+                               nexthop_group_delete(&ng);
+                       if (bnhg)
+                               zebra_nhg_backup_free(&bnhg);
+                       return false;
                }
 
-               if (CHECK_FLAG(api.message, ZAPI_MESSAGE_SRTE)) {
+               if (bnhg && CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) {
+                       if (IS_ZEBRA_DEBUG_RECV) {
+                               nexthop2str(nexthop, nhbuf, sizeof(nhbuf));
+                               zlog_debug("%s: backup nh %s with BACKUP flag!",
+                                          __func__, nhbuf);
+                       }
+                       UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP);
+                       nexthop->backup_num = 0;
+               }
+
+               if (CHECK_FLAG(api->message, ZAPI_MESSAGE_SRTE)) {
                        SET_FLAG(nexthop->flags, NEXTHOP_FLAG_SRTE);
                        nexthop->srte_color = api_nh->srte_color;
                }
@@ -1705,100 +1678,102 @@ static void zread_route_add(ZAPI_HANDLER_ARGS)
                                   __func__, nhbuf, api_nh->vrf_id, labelbuf);
                }
 
-               /* Add new nexthop to temporary list. This list is
-                * canonicalized - sorted - so that it can be hashed later
-                * in route processing. We expect that the sender has sent
-                * the list sorted, and the zapi client api attempts to enforce
-                * that, so this should be inexpensive - but it is necessary
-                * to support shared nexthop-groups.
-                */
-               nexthop_group_add_sorted(ng, nexthop);
-       }
-
-       /* Allocate temporary list of backup nexthops, if necessary */
-       if (api.backup_nexthop_num > 0) {
-               if (IS_ZEBRA_DEBUG_RECV)
-                       zlog_debug("%s: adding %d backup nexthops",
-                                  __func__, api.backup_nexthop_num);
-
-               bnhg = zebra_nhg_backup_alloc();
-               nexthop = NULL;
-               last_nh = NULL;
+               if (png) {
+                       /* Add new nexthop to temporary list. This list is
+                        * canonicalized - sorted - so that it can be hashed later
+                        * in route processing. We expect that the sender has sent
+                        * the list sorted, and the zapi client api attempts to enforce
+                        * that, so this should be inexpensive - but it is necessary
+                        * to support shared nexthop-groups.
+                        */
+                       nexthop_group_add_sorted(ng, nexthop);
+               }
+               if (bnhg) {
+                       /* Note that the order of the backup nexthops is significant,
+                        * so we don't sort this list as we do the primary nexthops,
+                        * we just append.
+                        */
+                       if (last_nh)
+                               NEXTHOP_APPEND(last_nh, nexthop);
+                       else
+                               bnhg->nhe->nhg.nexthop = nexthop;
+
+                       last_nh = nexthop;
+               }
        }
 
-       /* Copy backup nexthops also, if present */
-       for (i = 0; i < api.backup_nexthop_num; i++) {
-               api_nh = &api.backup_nexthops[i];
+       return true;
+}
 
-               /* Convert zapi backup nexthop */
-               nexthop = nexthop_from_zapi(re, api_nh, &api);
-               if (!nexthop) {
-                       flog_warn(
-                               EC_ZEBRA_NEXTHOP_CREATION_FAILED,
-                               "%s: Backup Nexthops Specified: %d but we failed to properly create one",
-                               __func__, api.backup_nexthop_num);
-                       nexthop_group_delete(&ng);
-                       zebra_nhg_backup_free(&bnhg);
-                       XFREE(MTYPE_RE, re);
-                       return;
-               }
+static void zread_route_add(ZAPI_HANDLER_ARGS)
+{
+       struct stream *s;
+       struct zapi_route api;
+       afi_t afi;
+       struct prefix_ipv6 *src_p = NULL;
+       struct route_entry *re;
+       struct nexthop_group *ng = NULL;
+       struct nhg_backup_info *bnhg = NULL;
+       int ret;
+       vrf_id_t vrf_id;
+       struct nhg_hash_entry nhe;
 
-               /* Backup nexthops can't have backups; that's not valid. */
-               if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) {
-                       if (IS_ZEBRA_DEBUG_RECV) {
-                               nexthop2str(nexthop, nhbuf, sizeof(nhbuf));
-                               zlog_debug("%s: backup nh %s with BACKUP flag!",
-                                          __func__, nhbuf);
-                       }
-                       UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP);
-                       nexthop->backup_num = 0;
-               }
+       s = msg;
+       if (zapi_route_decode(s, &api) < 0) {
+               if (IS_ZEBRA_DEBUG_RECV)
+                       zlog_debug("%s: Unable to decode zapi_route sent",
+                                  __func__);
+               return;
+       }
 
-               if (CHECK_FLAG(api.message, ZAPI_MESSAGE_SRTE)) {
-                       SET_FLAG(nexthop->flags, NEXTHOP_FLAG_SRTE);
-                       nexthop->srte_color = api_nh->srte_color;
-               }
+       vrf_id = zvrf_id(zvrf);
 
-               /* MPLS labels for BGP-LU or Segment Routing */
-               if (CHECK_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_LABEL)
-                   && api_nh->type != NEXTHOP_TYPE_IFINDEX
-                   && api_nh->type != NEXTHOP_TYPE_BLACKHOLE
-                   && api_nh->label_num > 0) {
+       if (IS_ZEBRA_DEBUG_RECV) {
+               char buf_prefix[PREFIX_STRLEN];
 
-                       label_type = lsp_type_from_re_type(client->proto);
-                       nexthop_add_labels(nexthop, label_type,
-                                          api_nh->label_num,
-                                          &api_nh->labels[0]);
-               }
+               prefix2str(&api.prefix, buf_prefix, sizeof(buf_prefix));
+               zlog_debug("%s: p=(%u:%u)%s, msg flags=0x%x, flags=0x%x",
+                          __func__, vrf_id, api.tableid, buf_prefix,
+                          (int)api.message, api.flags);
+       }
 
-               if (IS_ZEBRA_DEBUG_RECV) {
-                       labelbuf[0] = '\0';
-                       nhbuf[0] = '\0';
+       /* Allocate new route. */
+       re = XCALLOC(MTYPE_RE, sizeof(struct route_entry));
+       re->type = api.type;
+       re->instance = api.instance;
+       re->flags = api.flags;
+       re->uptime = monotime(NULL);
+       re->vrf_id = vrf_id;
 
-                       nexthop2str(nexthop, nhbuf, sizeof(nhbuf));
+       if (api.tableid)
+               re->table = api.tableid;
+       else
+               re->table = zvrf->table_id;
 
-                       if (nexthop->nh_label &&
-                           nexthop->nh_label->num_labels > 0) {
-                               mpls_label2str(nexthop->nh_label->num_labels,
-                                              nexthop->nh_label->label,
-                                              labelbuf, sizeof(labelbuf),
-                                              false);
-                       }
+       if (!CHECK_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP)
+           || api.nexthop_num == 0) {
+               flog_warn(EC_ZEBRA_RX_ROUTE_NO_NEXTHOPS,
+                         "%s: received a route without nexthops for prefix %pFX from client %s",
+                         __func__, &api.prefix,
+                         zebra_route_string(client->proto));
 
-                       zlog_debug("%s: backup nh=%s, vrf_id=%d %s",
-                                  __func__, nhbuf, api_nh->vrf_id, labelbuf);
-               }
+               XFREE(MTYPE_RE, re);
+               return;
+       }
 
-               /* Note that the order of the backup nexthops is significant,
-                * so we don't sort this list as we do the primary nexthops,
-                * we just append.
-                */
-               if (last_nh)
-                       NEXTHOP_APPEND(last_nh, nexthop);
-               else
-                       bnhg->nhe->nhg.nexthop = nexthop;
+       /* Report misuse of the backup flag */
+       if (CHECK_FLAG(api.message, ZAPI_MESSAGE_BACKUP_NEXTHOPS) &&
+           api.backup_nexthop_num == 0) {
+               if (IS_ZEBRA_DEBUG_RECV || IS_ZEBRA_DEBUG_EVENT)
+                       zlog_debug("%s: client %s: BACKUP flag set but no backup nexthops, prefix %pFX",
+                               __func__,
+                               zebra_route_string(client->proto), &api.prefix);
+       }
 
-               last_nh = nexthop;
+       if (!zapi_read_nexthops(client, &api, re, &ng, NULL) ||
+           !zapi_read_nexthops(client, &api, re, NULL, &bnhg)) {
+               XFREE(MTYPE_RE, re);
+               return;
        }
 
        if (CHECK_FLAG(api.message, ZAPI_MESSAGE_DISTANCE))