From: Mark Stapp Date: Tue, 30 Jun 2020 19:52:37 +0000 (-0400) Subject: lib,sharpd,zebra: initial support for multiple backup nexthops X-Git-Tag: base_7.5~153^2~8 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=474aebd9391588d4cd5169187fd0e9abc812de42;p=mirror%2Ffrr.git lib,sharpd,zebra: initial support for multiple backup nexthops Initial changes to support a nexthop with multiple backups. Lib changes to hold a small array in each primary, zapi message changes to support sending multiple backups, and daemon changes to show commands to support multiple backups. The config input for multiple backup indices is not present here. Signed-off-by: Mark Stapp --- diff --git a/lib/nexthop.c b/lib/nexthop.c index 3496081d47..3f5308bc27 100644 --- a/lib/nexthop.c +++ b/lib/nexthop.c @@ -156,6 +156,10 @@ static int _nexthop_cmp_no_labels(const struct nexthop *next1, if (ret != 0) goto done; + if (!CHECK_FLAG(next1->flags, NEXTHOP_FLAG_HAS_BACKUP) && + !CHECK_FLAG(next2->flags, NEXTHOP_FLAG_HAS_BACKUP)) + return 0; + if (!CHECK_FLAG(next1->flags, NEXTHOP_FLAG_HAS_BACKUP) && CHECK_FLAG(next2->flags, NEXTHOP_FLAG_HAS_BACKUP)) return -1; @@ -164,12 +168,18 @@ static int _nexthop_cmp_no_labels(const struct nexthop *next1, !CHECK_FLAG(next2->flags, NEXTHOP_FLAG_HAS_BACKUP)) return 1; - if (next1->backup_idx < next2->backup_idx) + if (next1->backup_num == 0 && next2->backup_num == 0) + goto done; + + if (next1->backup_num < next2->backup_num) return -1; - if (next1->backup_idx > next2->backup_idx) + if (next1->backup_num > next2->backup_num) return 1; + ret = memcmp(next1->backup_idx, + next2->backup_idx, next1->backup_num); + done: return ret; } @@ -529,14 +539,15 @@ unsigned int nexthop_level(struct nexthop *nexthop) uint32_t nexthop_hash_quick(const struct nexthop *nexthop) { uint32_t key = 0x45afe398; - uint32_t val; + int i; key = jhash_3words(nexthop->type, nexthop->vrf_id, nexthop->nh_label_type, key); if (nexthop->nh_label) { int labels = nexthop->nh_label->num_labels; - int i = 0; + + i = 0; while (labels >= 3) { key = jhash_3words(nexthop->nh_label->label[i], @@ -559,14 +570,35 @@ uint32_t nexthop_hash_quick(const struct nexthop *nexthop) key = jhash_1word(nexthop->nh_label->label[i], key); } - val = 0; - if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) - val = (uint32_t)nexthop->backup_idx; - - key = jhash_3words(nexthop->ifindex, - CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ONLINK), val, + key = jhash_2words(nexthop->ifindex, + CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ONLINK), key); + /* Include backup nexthops, if present */ + if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) { + int backups = nexthop->backup_num; + + i = 0; + + while (backups >= 3) { + key = jhash_3words(nexthop->backup_idx[i], + nexthop->backup_idx[i + 1], + nexthop->backup_idx[i + 2], key); + backups -= 3; + i += 3; + } + + while (backups >= 2) { + key = jhash_2words(nexthop->backup_idx[i], + nexthop->backup_idx[i + 1], key); + backups -= 2; + i += 2; + } + + if (backups >= 1) + key = jhash_1word(nexthop->backup_idx[i], key); + } + return key; } @@ -604,7 +636,12 @@ void nexthop_copy_no_recurse(struct nexthop *copy, copy->type = nexthop->type; copy->flags = nexthop->flags; copy->weight = nexthop->weight; - copy->backup_idx = nexthop->backup_idx; + + assert(nexthop->backup_num < NEXTHOP_MAX_BACKUPS); + copy->backup_num = nexthop->backup_num; + if (copy->backup_num > 0) + memcpy(copy->backup_idx, nexthop->backup_idx, copy->backup_num); + memcpy(©->gate, &nexthop->gate, sizeof(nexthop->gate)); memcpy(©->src, &nexthop->src, sizeof(nexthop->src)); memcpy(©->rmap_src, &nexthop->rmap_src, sizeof(nexthop->rmap_src)); diff --git a/lib/nexthop.h b/lib/nexthop.h index eda88efc08..e40c27d873 100644 --- a/lib/nexthop.h +++ b/lib/nexthop.h @@ -65,6 +65,12 @@ enum nh_encap_type { NET_VXLAN = 100, /* value copied from FPM_NH_ENCAP_VXLAN. */ }; +/* Fixed limit on the number of backup nexthops per primary nexthop */ +#define NEXTHOP_MAX_BACKUPS 8 + +/* Backup index value is limited */ +#define NEXTHOP_BACKUP_IDX_MAX 255 + /* Nexthop structure. */ struct nexthop { struct nexthop *next; @@ -124,10 +130,11 @@ struct nexthop { /* Weight of the nexthop ( for unequal cost ECMP ) */ uint8_t weight; - /* Index of a corresponding backup nexthop in a backup list; + /* Count and index of corresponding backup nexthop(s) in a backup list; * only meaningful if the HAS_BACKUP flag is set. */ - uint8_t backup_idx; + uint8_t backup_num; + uint8_t backup_idx[NEXTHOP_MAX_BACKUPS]; /* Encapsulation information. */ enum nh_encap_type nh_encap_type; @@ -136,9 +143,6 @@ struct nexthop { } nh_encap; }; -/* Backup index value is limited */ -#define NEXTHOP_BACKUP_IDX_MAX 255 - /* Utility to append one nexthop to another. */ #define NEXTHOP_APPEND(to, new) \ do { \ diff --git a/lib/nexthop_group.c b/lib/nexthop_group.c index 4f0c72af27..318f9f6161 100644 --- a/lib/nexthop_group.c +++ b/lib/nexthop_group.c @@ -806,7 +806,8 @@ static bool nexthop_group_parse_nexthop(struct nexthop *nhop, return false; SET_FLAG(nhop->flags, NEXTHOP_FLAG_HAS_BACKUP); - nhop->backup_idx = backup_idx; + nhop->backup_num = 1; + nhop->backup_idx[0] = backup_idx; } return true; @@ -992,7 +993,7 @@ void nexthop_group_write_nexthop(struct vty *vty, struct nexthop *nh) vty_out(vty, " weight %u", nh->weight); if (CHECK_FLAG(nh->flags, NEXTHOP_FLAG_HAS_BACKUP)) - vty_out(vty, " backup-idx %d", nh->backup_idx); + vty_out(vty, " backup-idx %d", nh->backup_idx[0]); vty_out(vty, "\n"); } @@ -1048,7 +1049,7 @@ void nexthop_group_json_nexthop(json_object *j, struct nexthop *nh) json_object_int_add(j, "weight", nh->weight); if (CHECK_FLAG(nh->flags, NEXTHOP_FLAG_HAS_BACKUP)) - json_object_int_add(j, "backupIdx", nh->backup_idx); + json_object_int_add(j, "backupIdx", nh->backup_idx[0]); } static void nexthop_group_write_nexthop_internal(struct vty *vty, diff --git a/lib/zclient.c b/lib/zclient.c index 793864243c..6a511ccab5 100644 --- a/lib/zclient.c +++ b/lib/zclient.c @@ -888,7 +888,7 @@ static void zapi_nexthop_group_sort(struct zapi_nexthop *nh_grp, int zapi_nexthop_encode(struct stream *s, const struct zapi_nexthop *api_nh, uint32_t api_flags) { - int ret = 0; + int i, ret = 0; int nh_flags = api_nh->flags; stream_putl(s, api_nh->vrf_id); @@ -951,8 +951,17 @@ int zapi_nexthop_encode(struct stream *s, const struct zapi_nexthop *api_nh, sizeof(struct ethaddr)); /* Index of backup nexthop */ - if (CHECK_FLAG(nh_flags, ZAPI_NEXTHOP_FLAG_HAS_BACKUP)) - stream_putc(s, api_nh->backup_idx); + if (CHECK_FLAG(nh_flags, ZAPI_NEXTHOP_FLAG_HAS_BACKUP)) { + /* Validate backup count */ + if (api_nh->backup_num > NEXTHOP_MAX_BACKUPS) { + ret = -1; + goto done; + } + + stream_putc(s, api_nh->backup_num); + for (i = 0; i < api_nh->backup_num; i++) + stream_putc(s, api_nh->backup_idx[i]); + } done: return ret; @@ -1111,7 +1120,7 @@ int zapi_route_encode(uint8_t cmd, struct stream *s, struct zapi_route *api) static int zapi_nexthop_decode(struct stream *s, struct zapi_nexthop *api_nh, uint32_t api_flags) { - int ret = -1; + int i, ret = -1; STREAM_GETL(s, api_nh->vrf_id); STREAM_GETC(s, api_nh->type); @@ -1163,8 +1172,15 @@ static int zapi_nexthop_decode(struct stream *s, struct zapi_nexthop *api_nh, sizeof(struct ethaddr)); /* Backup nexthop index */ - if (CHECK_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_HAS_BACKUP)) - STREAM_GETC(s, api_nh->backup_idx); + if (CHECK_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_HAS_BACKUP)) { + STREAM_GETC(s, api_nh->backup_num); + + if (api_nh->backup_num > NEXTHOP_MAX_BACKUPS) + return -1; + + for (i = 0; i < api_nh->backup_num; i++) + STREAM_GETC(s, api_nh->backup_idx[i]); + } /* Success */ ret = 0; @@ -1483,7 +1499,8 @@ struct nexthop *nexthop_from_zapi_nexthop(const struct zapi_nexthop *znh) if (CHECK_FLAG(znh->flags, ZAPI_NEXTHOP_FLAG_HAS_BACKUP)) { SET_FLAG(n->flags, NEXTHOP_FLAG_HAS_BACKUP); - n->backup_idx = znh->backup_idx; + n->backup_num = znh->backup_num; + memcpy(n->backup_idx, znh->backup_idx, n->backup_num); } return n; @@ -1519,8 +1536,12 @@ int zapi_nexthop_from_nexthop(struct zapi_nexthop *znh, } if (CHECK_FLAG(nh->flags, NEXTHOP_FLAG_HAS_BACKUP)) { + if (nh->backup_num > NEXTHOP_MAX_BACKUPS) + return -1; + SET_FLAG(znh->flags, ZAPI_NEXTHOP_FLAG_HAS_BACKUP); - znh->backup_idx = nh->backup_idx; + znh->backup_num = nh->backup_num; + memcpy(znh->backup_idx, nh->backup_idx, znh->backup_num); } return 0; diff --git a/lib/zclient.h b/lib/zclient.h index 3ded2f55d7..250824e612 100644 --- a/lib/zclient.h +++ b/lib/zclient.h @@ -394,8 +394,9 @@ struct zapi_nexthop { uint32_t weight; - /* Index of backup nexthop */ - uint8_t backup_idx; + /* Backup nexthops, for IP-FRR, TI-LFA, etc */ + uint8_t backup_num; + uint8_t backup_idx[NEXTHOP_MAX_BACKUPS]; }; /* diff --git a/sharpd/sharp_vty.c b/sharpd/sharp_vty.c index 48220d1c9b..1d2b87b9ba 100644 --- a/sharpd/sharp_vty.c +++ b/sharpd/sharp_vty.c @@ -278,7 +278,8 @@ DEFPY (install_routes, if (backup) { /* Set flag and index in primary nexthop */ SET_FLAG(sg.r.nhop.flags, NEXTHOP_FLAG_HAS_BACKUP); - sg.r.nhop.backup_idx = 0; + sg.r.nhop.backup_num = 1; + sg.r.nhop.backup_idx[0] = 0; if (backup_nexthop4.s_addr != INADDR_ANY) { sg.r.backup_nhop.gate.ipv4 = backup_nexthop4; diff --git a/sharpd/sharp_zebra.c b/sharpd/sharp_zebra.c index 7ab2d6ec22..74e44014a9 100644 --- a/sharpd/sharp_zebra.c +++ b/sharpd/sharp_zebra.c @@ -155,6 +155,8 @@ int sharp_install_lsps_helper(bool install_p, const struct prefix *p, return -1; i++; + if (i >= MULTIPATH_NUM) + break; } } @@ -188,6 +190,8 @@ int sharp_install_lsps_helper(bool install_p, const struct prefix *p, return -1; i++; + if (i >= MULTIPATH_NUM) + break; } if (i > 0) diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index dc7c595d26..632ae08f51 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -1416,6 +1416,7 @@ static struct nexthop *nexthop_from_zapi(struct route_entry *re, struct nexthop *nexthop = NULL; struct ipaddr vtep_ip; struct interface *ifp; + int i; char nhbuf[INET6_ADDRSTRLEN] = ""; switch (api_nh->type) { @@ -1521,17 +1522,36 @@ static struct nexthop *nexthop_from_zapi(struct route_entry *re, nexthop->weight = api_nh->weight; if (CHECK_FLAG(api_nh->flags, ZAPI_NEXTHOP_FLAG_HAS_BACKUP)) { - if (api_nh->backup_idx < api->backup_nexthop_num) { - /* Capture backup info */ - SET_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP); - nexthop->backup_idx = api_nh->backup_idx; - } else { - /* Warn about invalid backup index */ + /* Validate count */ + if (api_nh->backup_num > NEXTHOP_MAX_BACKUPS) { if (IS_ZEBRA_DEBUG_RECV || IS_ZEBRA_DEBUG_EVENT) - zlog_debug("%s: invalid backup nh idx %d", - __func__, api_nh->backup_idx); + zlog_debug("%s: invalid backup nh count %d", + __func__, api_nh->backup_num); + nexthop_free(nexthop); + nexthop = NULL; + goto done; + } + + /* Copy backup info */ + SET_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP); + nexthop->backup_num = api_nh->backup_num; + + for (i = 0; i < api_nh->backup_num; i++) { + /* Validate backup index */ + if (api_nh->backup_idx[i] < api->backup_nexthop_num) { + nexthop->backup_idx[i] = api_nh->backup_idx[i]; + } else { + if (IS_ZEBRA_DEBUG_RECV || IS_ZEBRA_DEBUG_EVENT) + zlog_debug("%s: invalid backup nh idx %d", + __func__, + api_nh->backup_idx[i]); + nexthop_free(nexthop); + nexthop = NULL; + goto done; + } } } + done: return nexthop; } @@ -1703,7 +1723,7 @@ static void zread_route_add(ZAPI_HANDLER_ARGS) __func__, nhbuf); } UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP); - nexthop->backup_idx = 0; + nexthop->backup_num = 0; } /* MPLS labels for BGP-LU or Segment Routing */ diff --git a/zebra/zebra_mpls.c b/zebra/zebra_mpls.c index 290273f664..f37552a542 100644 --- a/zebra/zebra_mpls.c +++ b/zebra/zebra_mpls.c @@ -1595,7 +1595,7 @@ static void nhlfe_print(zebra_nhlfe_t *nhlfe, struct vty *vty) static void lsp_print(struct vty *vty, zebra_lsp_t *lsp) { zebra_nhlfe_t *nhlfe, *backup; - int i; + int i, j; vty_out(vty, "Local label: %u%s\n", lsp->ile.in_label, CHECK_FLAG(lsp->flags, LSP_FLAG_INSTALLED) ? " (installed)" @@ -1605,14 +1605,17 @@ static void lsp_print(struct vty *vty, zebra_lsp_t *lsp) nhlfe_print(nhlfe, vty); if (nhlfe->nexthop && - CHECK_FLAG(nhlfe->nexthop->flags, - NEXTHOP_FLAG_HAS_BACKUP)) { - /* Find backup in backup list */ + !CHECK_FLAG(nhlfe->nexthop->flags, + NEXTHOP_FLAG_HAS_BACKUP)) + continue; + + /* Backup nhlfes: find backups in backup list */ + for (j = 0; j < nhlfe->nexthop->backup_num; j++) { i = 0; backup = NULL; frr_each(nhlfe_list, &lsp->backup_nhlfe_list, backup) { - if (i == nhlfe->nexthop->backup_idx) + if (i == nhlfe->nexthop->backup_idx[j]) break; i++; } @@ -3338,7 +3341,14 @@ static int lsp_znh_install(zebra_lsp_t *lsp, enum lsp_types_t type, /* Update backup info if present */ if (CHECK_FLAG(znh->flags, ZAPI_NEXTHOP_FLAG_HAS_BACKUP)) { - nhlfe->nexthop->backup_idx = znh->backup_idx; + if (znh->backup_num > NEXTHOP_MAX_BACKUPS) { + nhlfe_del(nhlfe); + return -1; + } + + nhlfe->nexthop->backup_num = znh->backup_num; + memcpy(nhlfe->nexthop->backup_idx, znh->backup_idx, + znh->backup_num); SET_FLAG(nhlfe->nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP); } diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 67b3812ed3..c36bb622f4 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -2630,6 +2630,8 @@ static void _route_entry_dump_nh(const struct route_entry *re, char nhname[PREFIX_STRLEN]; char backup_str[50]; char wgt_str[50]; + char temp_str[10]; + int i; struct interface *ifp; struct vrf *vrf = vrf_lookup_by_id(nexthop->vrf_id); @@ -2655,8 +2657,12 @@ static void _route_entry_dump_nh(const struct route_entry *re, backup_str[0] = '\0'; if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) { - snprintf(backup_str, sizeof(backup_str), "backup %d,", - (int)nexthop->backup_idx); + snprintf(backup_str, sizeof(backup_str), "backup "); + for (i = 0; i < nexthop->backup_num; i++) { + snprintf(temp_str, sizeof(temp_str), "%d, ", + nexthop->backup_idx[i]); + strlcat(backup_str, temp_str, sizeof(backup_str)); + } } wgt_str[0] = '\0'; diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c index 1da2660509..2242709238 100644 --- a/zebra/zebra_vty.c +++ b/zebra/zebra_vty.c @@ -422,6 +422,7 @@ static void show_route_nexthop_helper(struct vty *vty, const struct nexthop *nexthop) { char buf[MPLS_LABEL_STRLEN]; + int i; switch (nexthop->type) { case NEXTHOP_TYPE_IPV4: @@ -519,8 +520,12 @@ static void show_route_nexthop_helper(struct vty *vty, if (nexthop->weight) vty_out(vty, ", weight %u", nexthop->weight); - if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) - vty_out(vty, ", backup %d", nexthop->backup_idx); + if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) { + vty_out(vty, ", backup %d", nexthop->backup_idx[0]); + + for (i = 1; i < nexthop->backup_num; i++) + vty_out(vty, ", %d", nexthop->backup_idx[i]); + } } /* @@ -534,6 +539,8 @@ static void show_nexthop_json_helper(json_object *json_nexthop, char buf[SRCDEST2STR_BUFFER]; struct vrf *vrf = NULL; json_object *json_labels = NULL; + json_object *json_backups = NULL; + int i; json_object_int_add(json_nexthop, "flags", nexthop->flags); @@ -645,9 +652,17 @@ static void show_nexthop_json_helper(json_object *json_nexthop, json_object_boolean_true_add(json_nexthop, "recursive"); - if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) - json_object_int_add(json_nexthop, "backupIndex", - nexthop->backup_idx); + 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_nexthop, "backupIndex", + json_backups); + } switch (nexthop->type) { case NEXTHOP_TYPE_IPV4: @@ -1206,7 +1221,7 @@ static void show_nexthop_group_out(struct vty *vty, struct nhg_hash_entry *nhe) if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) vty_out(vty, " [backup %d]", - nexthop->backup_idx); + nexthop->backup_idx[0]); vty_out(vty, "\n"); continue; @@ -1214,22 +1229,13 @@ static void show_nexthop_group_out(struct vty *vty, struct nhg_hash_entry *nhe) /* TODO -- print more useful backup info */ if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_HAS_BACKUP)) { - struct nexthop *backup; int i; - i = 0; - for (ALL_NEXTHOPS(nhe->backup_info->nhe->nhg, backup)) { - if (i == nexthop->backup_idx) - break; - i++; - } + vty_out(vty, "[backup"); + for (i = 0; i < nexthop->backup_num; i++) + vty_out(vty, " %d", nexthop->backup_idx[i]); - /* TODO */ - if (backup) - vty_out(vty, " [backup %d]", - nexthop->backup_idx); - else - vty_out(vty, " [backup INVALID]"); + vty_out(vty, "]"); } vty_out(vty, "\n");