From: Donald Sharp Date: Mon, 26 Jul 2021 11:32:35 +0000 (-0400) Subject: Revert "Backport of PR 9001 and PR 8174 for 8.0" X-Git-Tag: frr-8.0.1~51^2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=cf4a6469ab18254b513d986c2e1fa6d19e95fd7e;p=matthieu%2Ffrr.git Revert "Backport of PR 9001 and PR 8174 for 8.0" --- diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index 223a8ca438..544bb07fbe 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -681,8 +681,6 @@ static int zsend_ipv4_nexthop_lookup_mrib(struct zserv *client, stream_put_in_addr(s, &addr); if (re) { - struct nexthop_group *nhg; - stream_putc(s, re->distance); stream_putl(s, re->metric); num = 0; @@ -690,11 +688,15 @@ static int zsend_ipv4_nexthop_lookup_mrib(struct zserv *client, nump = stream_get_endp(s); /* reserve room for nexthop_num */ stream_putc(s, 0); - nhg = rib_get_fib_nhg(re); - for (ALL_NEXTHOPS_PTR(nhg, nexthop)) { - if (rnh_nexthop_valid(re, nexthop)) + /* + * Only non-recursive routes are elegible to resolve the + * nexthop we are looking up. Therefore, we will just iterate + * over the top chain of nexthops. + */ + for (nexthop = re->nhe->nhg.nexthop; nexthop; + nexthop = nexthop->next) + if (CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_ACTIVE)) num += zserv_encode_nexthop(s, nexthop); - } /* store nexthop_num */ stream_putc_at(s, nump, num); diff --git a/zebra/zebra_rnh.c b/zebra/zebra_rnh.c index 804ce733bd..3b0ef71987 100644 --- a/zebra/zebra_rnh.c +++ b/zebra/zebra_rnh.c @@ -52,16 +52,11 @@ DEFINE_MTYPE_STATIC(ZEBRA, RNH, "Nexthop tracking object"); -/* UI controls whether to notify about changes that only involve backup - * nexthops. Default is to notify all changes. - */ -static bool rnh_hide_backups; - static void free_state(vrf_id_t vrf_id, struct route_entry *re, struct route_node *rn); static void copy_state(struct rnh *rnh, const struct route_entry *re, struct route_node *rn); -static bool compare_state(struct route_entry *r1, struct route_entry *r2); +static int compare_state(struct route_entry *r1, struct route_entry *r2); static void print_rnh(struct route_node *rn, struct vty *vty); static int zebra_client_cleanup_rnh(struct zserv *client); @@ -593,72 +588,14 @@ static void zebra_rnh_notify_protocol_clients(struct zebra_vrf *zvrf, afi_t afi, * check in a couple of places, so this is a single home for the logic we * use. */ - -static const int RNH_INVALID_NH_FLAGS = (NEXTHOP_FLAG_RECURSIVE | - NEXTHOP_FLAG_DUPLICATE | - NEXTHOP_FLAG_RNH_FILTERED); - -bool rnh_nexthop_valid(const struct route_entry *re, const struct nexthop *nh) +static bool rnh_nexthop_valid(const struct route_entry *re, + const struct nexthop *nh) { return (CHECK_FLAG(re->status, ROUTE_ENTRY_INSTALLED) && CHECK_FLAG(nh->flags, NEXTHOP_FLAG_ACTIVE) - && !CHECK_FLAG(nh->flags, RNH_INVALID_NH_FLAGS)); -} - -/* - * Determine whether an re's nexthops are valid for tracking. - */ -static bool rnh_check_re_nexthops(const struct route_entry *re, - const struct rnh *rnh) -{ - bool ret = false; - const struct nexthop *nexthop = NULL; - - /* Check route's nexthops */ - for (ALL_NEXTHOPS(re->nhe->nhg, nexthop)) { - if (rnh_nexthop_valid(re, nexthop)) - break; - } - - /* Check backup nexthops, if any. */ - if (nexthop == NULL && re->nhe->backup_info && - re->nhe->backup_info->nhe) { - for (ALL_NEXTHOPS(re->nhe->backup_info->nhe->nhg, nexthop)) { - if (rnh_nexthop_valid(re, nexthop)) - break; - } - } - - if (nexthop == NULL) { - if (IS_ZEBRA_DEBUG_NHT_DETAILED) - zlog_debug( - " Route Entry %s no nexthops", - zebra_route_string(re->type)); - - goto done; - } - - /* Some special checks if registration asked for them. */ - if (CHECK_FLAG(rnh->flags, ZEBRA_NHT_CONNECTED)) { - if ((re->type == ZEBRA_ROUTE_CONNECT) - || (re->type == ZEBRA_ROUTE_STATIC)) - ret = true; - if (re->type == ZEBRA_ROUTE_NHRP) { - - for (nexthop = re->nhe->nhg.nexthop; - nexthop; - nexthop = nexthop->next) - if (nexthop->type == NEXTHOP_TYPE_IFINDEX) - break; - if (nexthop) - ret = true; - } - } else { - ret = true; - } - -done: - return ret; + && !CHECK_FLAG(nh->flags, NEXTHOP_FLAG_RECURSIVE) + && !CHECK_FLAG(nh->flags, NEXTHOP_FLAG_DUPLICATE) + && !CHECK_FLAG(nh->flags, NEXTHOP_FLAG_RNH_FILTERED)); } /* @@ -667,12 +604,13 @@ done: */ static struct route_entry * zebra_rnh_resolve_nexthop_entry(struct zebra_vrf *zvrf, afi_t afi, - struct route_node *nrn, const struct rnh *rnh, + struct route_node *nrn, struct rnh *rnh, struct route_node **prn) { struct route_table *route_table; struct route_node *rn; struct route_entry *re; + struct nexthop *nexthop; *prn = NULL; @@ -736,7 +674,35 @@ zebra_rnh_resolve_nexthop_entry(struct zebra_vrf *zvrf, afi_t afi, /* Just being SELECTED isn't quite enough - must * have an installed nexthop to be useful. */ - if (rnh_check_re_nexthops(re, rnh)) + for (ALL_NEXTHOPS(re->nhe->nhg, nexthop)) { + if (rnh_nexthop_valid(re, nexthop)) + break; + } + + if (nexthop == NULL) { + if (IS_ZEBRA_DEBUG_NHT_DETAILED) + zlog_debug( + " Route Entry %s no nexthops", + zebra_route_string(re->type)); + continue; + } + + if (CHECK_FLAG(rnh->flags, ZEBRA_NHT_CONNECTED)) { + if ((re->type == ZEBRA_ROUTE_CONNECT) + || (re->type == ZEBRA_ROUTE_STATIC)) + break; + if (re->type == ZEBRA_ROUTE_NHRP) { + + for (nexthop = re->nhe->nhg.nexthop; + nexthop; + nexthop = nexthop->next) + if (nexthop->type + == NEXTHOP_TYPE_IFINDEX) + break; + if (nexthop) + break; + } + } else break; } @@ -998,135 +964,7 @@ static void copy_state(struct rnh *rnh, const struct route_entry *re, } /* - * Locate the next primary nexthop, used when comparing current rnh info with - * an updated route. - */ -static struct nexthop *next_valid_primary_nh(struct route_entry *re, - struct nexthop *nh) -{ - struct nexthop_group *nhg; - struct nexthop *bnh; - int i, idx; - bool default_path = true; - - /* Fib backup ng present: some backups are installed, - * and we're configured for special handling if there are backups. - */ - if (rnh_hide_backups && (re->fib_backup_ng.nexthop != NULL)) - default_path = false; - - /* Default path: no special handling, just using the 'installed' - * primary nexthops and the common validity test. - */ - if (default_path) { - if (nh == NULL) { - nhg = rib_get_fib_nhg(re); - nh = nhg->nexthop; - } else - nh = nexthop_next(nh); - - while (nh) { - if (rnh_nexthop_valid(re, nh)) - break; - else - nh = nexthop_next(nh); - } - - return nh; - } - - /* Hide backup activation/switchover events. - * - * If we've had a switchover, an inactive primary won't be in - * the fib list at all - the 'fib' list could even be empty - * in the case where no primary is installed. But we want to consider - * those primaries "valid" if they have an activated backup nh. - * - * The logic is something like: - * if (!fib_nhg) - * // then all primaries are installed - * else - * for each primary in re nhg - * if in fib_nhg - * primary is installed - * else if a backup is installed - * primary counts as installed - * else - * primary !installed - */ - - /* Start with the first primary */ - if (nh == NULL) - nh = re->nhe->nhg.nexthop; - else - nh = nexthop_next(nh); - - while (nh) { - - if (IS_ZEBRA_DEBUG_NHT_DETAILED) - zlog_debug("%s: checking primary NH %pNHv", - __func__, nh); - - /* If this nexthop is in the fib list, it's installed */ - nhg = rib_get_fib_nhg(re); - - for (bnh = nhg->nexthop; bnh; bnh = nexthop_next(bnh)) { - if (nexthop_cmp(nh, bnh) == 0) - break; - } - - if (bnh != NULL) { - /* Found the match */ - if (IS_ZEBRA_DEBUG_NHT_DETAILED) - zlog_debug("%s: NH in fib list", __func__); - break; - } - - /* Else if this nexthop's backup is installed, it counts */ - nhg = rib_get_fib_backup_nhg(re); - bnh = nhg->nexthop; - - for (idx = 0; bnh != NULL; idx++) { - /* If we find an active backup nh for this - * primary, we're done; - */ - if (IS_ZEBRA_DEBUG_NHT_DETAILED) - zlog_debug("%s: checking backup %pNHv [%d]", - __func__, bnh, idx); - - if (!CHECK_FLAG(bnh->flags, NEXTHOP_FLAG_ACTIVE)) - continue; - - for (i = 0; i < nh->backup_num; i++) { - /* Found a matching activated backup nh */ - if (nh->backup_idx[i] == idx) { - if (IS_ZEBRA_DEBUG_NHT_DETAILED) - zlog_debug("%s: backup %d activated", - __func__, i); - - goto done; - } - } - - /* Note that we're not recursing here if the - * backups are recursive: the primary's index is - * only valid in the top-level backup list. - */ - bnh = bnh->next; - } - - /* Try the next primary nexthop */ - nh = nexthop_next(nh); - } - -done: - - return nh; -} - -/* - * Compare two route_entries' nexthops. Account for backup nexthops - * and for the 'fib' nexthop lists, if present. + * Compare two route_entries' nexthops. */ static bool compare_valid_nexthops(struct route_entry *r1, struct route_entry *r2) @@ -1135,23 +973,34 @@ static bool compare_valid_nexthops(struct route_entry *r1, struct nexthop_group *nhg1, *nhg2; struct nexthop *nh1, *nh2; - /* Start with the primary nexthops */ + /* Account for backup nexthops and for the 'fib' nexthop lists, + * if present. + */ + nhg1 = rib_get_fib_nhg(r1); + nhg2 = rib_get_fib_nhg(r2); - nh1 = next_valid_primary_nh(r1, NULL); - nh2 = next_valid_primary_nh(r2, NULL); + nh1 = nhg1->nexthop; + nh2 = nhg2->nexthop; while (1) { - /* Find any differences in the nexthop lists */ + /* Find each list's next valid nexthop */ + while ((nh1 != NULL) && !rnh_nexthop_valid(r1, nh1)) + nh1 = nexthop_next(nh1); + + while ((nh2 != NULL) && !rnh_nexthop_valid(r2, nh2)) + nh2 = nexthop_next(nh2); if (nh1 && nh2) { /* Any difference is a no-match */ if (nexthop_cmp(nh1, nh2) != 0) { if (IS_ZEBRA_DEBUG_NHT_DETAILED) - zlog_debug("%s: nh1: %pNHv, nh2: %pNHv differ", - __func__, nh1, nh2); + zlog_debug("%s: nh1, nh2 differ", + __func__); goto done; } + nh1 = nexthop_next(nh1); + nh2 = nexthop_next(nh2); } else if (nh1 || nh2) { /* One list has more valid nexthops than the other */ if (IS_ZEBRA_DEBUG_NHT_DETAILED) @@ -1161,37 +1010,6 @@ static bool compare_valid_nexthops(struct route_entry *r1, goto done; } else break; /* Done with both lists */ - - nh1 = next_valid_primary_nh(r1, nh1); - nh2 = next_valid_primary_nh(r2, nh2); - } - - /* If configured, don't compare installed backup state - we've - * accounted for that with the primaries above. - * - * But we do want to compare the routes' backup info, - * in case the owning route has changed the backups - - * that change we do want to report. - */ - if (rnh_hide_backups) { - uint32_t hash1 = 0, hash2 = 0; - - if (r1->nhe->backup_info) - hash1 = nexthop_group_hash( - &r1->nhe->backup_info->nhe->nhg); - - if (r2->nhe->backup_info) - hash2 = nexthop_group_hash( - &r2->nhe->backup_info->nhe->nhg); - - if (IS_ZEBRA_DEBUG_NHT_DETAILED) - zlog_debug("%s: backup hash1 %#x, hash2 %#x", - __func__, hash1, hash2); - - if (hash1 != hash2) - goto done; - else - goto finished; } /* The test for the backups is slightly different: the only installed @@ -1215,8 +1033,8 @@ static bool compare_valid_nexthops(struct route_entry *r1, /* Any difference is a no-match */ if (nexthop_cmp(nh1, nh2) != 0) { if (IS_ZEBRA_DEBUG_NHT_DETAILED) - zlog_debug("%s: backup nh1: %pNHv, nh2: %pNHv differ", - __func__, nh1, nh2); + zlog_debug("%s: backup nh1, nh2 differ", + __func__); goto done; } @@ -1234,40 +1052,35 @@ static bool compare_valid_nexthops(struct route_entry *r1, break; /* Done with both lists */ } -finished: - /* Well, it's a match */ + if (IS_ZEBRA_DEBUG_NHT_DETAILED) + zlog_debug("%s: matched", __func__); + matched_p = true; done: - if (IS_ZEBRA_DEBUG_NHT_DETAILED) - zlog_debug("%s: %smatched", - __func__, (matched_p ? "" : "NOT ")); - return matched_p; } -/* Returns 'false' if no difference. */ -static bool compare_state(struct route_entry *r1, - struct route_entry *r2) +static int compare_state(struct route_entry *r1, struct route_entry *r2) { if (!r1 && !r2) - return false; + return 0; if ((!r1 && r2) || (r1 && !r2)) - return true; + return 1; if (r1->distance != r2->distance) - return true; + return 1; if (r1->metric != r2->metric) - return true; + return 1; if (!compare_valid_nexthops(r1, r2)) - return true; + return 1; - return false; + return 0; } int zebra_send_rnh_update(struct rnh *rnh, struct zserv *client, @@ -1508,16 +1321,3 @@ int rnh_resolve_via_default(struct zebra_vrf *zvrf, int family) else return 0; } - -/* - * UI control to avoid notifications if backup nexthop status changes - */ -void rnh_set_hide_backups(bool hide_p) -{ - rnh_hide_backups = hide_p; -} - -bool rnh_get_hide_backups(void) -{ - return rnh_hide_backups; -} diff --git a/zebra/zebra_rnh.h b/zebra/zebra_rnh.h index 24ee6d0bd9..c71a2b9cce 100644 --- a/zebra/zebra_rnh.h +++ b/zebra/zebra_rnh.h @@ -64,13 +64,6 @@ extern void zebra_print_rnh_table(vrf_id_t vrfid, afi_t afi, struct vty *vty, extern int rnh_resolve_via_default(struct zebra_vrf *zvrf, int family); -extern bool rnh_nexthop_valid(const struct route_entry *re, - const struct nexthop *nh); - -/* UI control to avoid notifications if backup nexthop status changes */ -void rnh_set_hide_backups(bool hide_p); -bool rnh_get_hide_backups(void); - #ifdef __cplusplus } #endif diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c index 7859460e61..8061f34d2b 100644 --- a/zebra/zebra_vty.c +++ b/zebra/zebra_vty.c @@ -1701,6 +1701,7 @@ DEFUN (no_ipv6_nht_default_route, "Filter Next Hop tracking route resolution\n" "Resolve via default route\n") { + ZEBRA_DECLVAR_CONTEXT(vrf, zvrf); if (!zvrf) @@ -1714,17 +1715,6 @@ DEFUN (no_ipv6_nht_default_route, return CMD_SUCCESS; } -DEFPY_HIDDEN(rnh_hide_backups, rnh_hide_backups_cmd, - "[no] ip nht hide-backup-events", - NO_STR - IP_STR - "Nexthop-tracking configuration\n" - "Hide notification about backup nexthops\n") -{ - rnh_set_hide_backups(!no); - return CMD_SUCCESS; -} - DEFPY (show_route, show_route_cmd, "show\ @@ -3671,9 +3661,6 @@ static int config_write_protocol(struct vty *vty) if (!zebra_nhg_recursive_use_backups()) vty_out(vty, "no zebra nexthop resolve-via-backup\n"); - if (rnh_get_hide_backups()) - vty_out(vty, "ip nht hide-backup-events\n"); - #ifdef HAVE_NETLINK /* Include netlink info */ netlink_config_write_helper(vty); @@ -4133,8 +4120,6 @@ void zebra_vty_init(void) install_element(VRF_NODE, &no_ip_nht_default_route_cmd); install_element(VRF_NODE, &ipv6_nht_default_route_cmd); install_element(VRF_NODE, &no_ipv6_nht_default_route_cmd); - install_element(CONFIG_NODE, &rnh_hide_backups_cmd); - install_element(VIEW_NODE, &show_ipv6_mroute_cmd); /* Commands for VRF */