Merge pull request #6459 from vivek-cumulus/bgp_nht_fix

Fix BGP NHT to skip inappropriate paths & only act upon change
This commit is contained in:
Russ White 2020-05-26 07:29:25 -04:00 committed by GitHub
commit a5e845f314
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 73 additions and 38 deletions

View File

@ -2634,17 +2634,17 @@ static int install_evpn_route_entry_in_vrf(struct bgp *bgp_vrf,
afi_t afi = 0; afi_t afi = 0;
safi_t safi = 0; safi_t safi = 0;
char buf[PREFIX_STRLEN]; char buf[PREFIX_STRLEN];
char buf1[PREFIX_STRLEN]; bool new_pi = false;
memset(pp, 0, sizeof(struct prefix)); memset(pp, 0, sizeof(struct prefix));
ip_prefix_from_evpn_prefix(evp, pp); ip_prefix_from_evpn_prefix(evp, pp);
if (bgp_debug_zebra(NULL)) { if (bgp_debug_zebra(NULL)) {
zlog_debug( zlog_debug(
"import evpn prefix %s as ip prefix %s in vrf %s", "vrf %s: import evpn prefix %s parent %p flags 0x%x",
vrf_id_to_name(bgp_vrf->vrf_id),
prefix2str(evp, buf, sizeof(buf)), prefix2str(evp, buf, sizeof(buf)),
prefix2str(pp, buf1, sizeof(buf)), parent_pi, parent_pi->flags);
vrf_id_to_name(bgp_vrf->vrf_id));
} }
/* Create (or fetch) route within the VRF. */ /* Create (or fetch) route within the VRF. */
@ -2678,9 +2678,10 @@ static int install_evpn_route_entry_in_vrf(struct bgp *bgp_vrf,
&& (struct bgp_path_info *)pi->extra->parent == parent_pi) && (struct bgp_path_info *)pi->extra->parent == parent_pi)
break; break;
if (!pi) if (!pi) {
pi = bgp_create_evpn_bgp_path_info(parent_pi, rn, &attr); pi = bgp_create_evpn_bgp_path_info(parent_pi, rn, &attr);
else { new_pi = true;
} else {
if (attrhash_cmp(pi->attr, &attr) if (attrhash_cmp(pi->attr, &attr)
&& !CHECK_FLAG(pi->flags, BGP_PATH_REMOVED)) { && !CHECK_FLAG(pi->flags, BGP_PATH_REMOVED)) {
bgp_unlock_node(rn); bgp_unlock_node(rn);
@ -2722,6 +2723,12 @@ static int install_evpn_route_entry_in_vrf(struct bgp *bgp_vrf,
bgp_unlock_node(rn); bgp_unlock_node(rn);
if (bgp_debug_zebra(NULL))
zlog_debug(
"... %s pi rn %p (l %d) pi %p (l %d, f 0x%x)",
new_pi ? "new" : "update",
rn, rn->lock, pi, pi->lock, pi->flags);
return ret; return ret;
} }
@ -2839,17 +2846,16 @@ static int uninstall_evpn_route_entry_in_vrf(struct bgp *bgp_vrf,
afi_t afi = 0; afi_t afi = 0;
safi_t safi = 0; safi_t safi = 0;
char buf[PREFIX_STRLEN]; char buf[PREFIX_STRLEN];
char buf1[PREFIX_STRLEN];
memset(pp, 0, sizeof(struct prefix)); memset(pp, 0, sizeof(struct prefix));
ip_prefix_from_evpn_prefix(evp, pp); ip_prefix_from_evpn_prefix(evp, pp);
if (bgp_debug_zebra(NULL)) { if (bgp_debug_zebra(NULL)) {
zlog_debug( zlog_debug(
"uninstalling evpn prefix %s as ip prefix %s in vrf %s", "vrf %s: unimport evpn prefix %s parent %p flags 0x%x",
vrf_id_to_name(bgp_vrf->vrf_id),
prefix2str(evp, buf, sizeof(buf)), prefix2str(evp, buf, sizeof(buf)),
prefix2str(pp, buf1, sizeof(buf)), parent_pi, parent_pi->flags);
vrf_id_to_name(bgp_vrf->vrf_id));
} }
/* Locate route within the VRF. */ /* Locate route within the VRF. */
@ -2876,6 +2882,11 @@ static int uninstall_evpn_route_entry_in_vrf(struct bgp *bgp_vrf,
if (!pi) if (!pi)
return 0; return 0;
if (bgp_debug_zebra(NULL))
zlog_debug(
"... delete rn %p (l %d) pi %p (l %d, f 0x%x)",
rn, rn->lock, pi, pi->lock, pi->flags);
/* Process for route leaking. */ /* Process for route leaking. */
vpn_leak_from_vrf_withdraw(bgp_get_default(), bgp_vrf, pi); vpn_leak_from_vrf_withdraw(bgp_get_default(), bgp_vrf, pi);

View File

@ -44,6 +44,7 @@
#include "bgpd/bgp_zebra.h" #include "bgpd/bgp_zebra.h"
#include "bgpd/bgp_flowspec_util.h" #include "bgpd/bgp_flowspec_util.h"
#include "bgpd/bgp_evpn.h" #include "bgpd/bgp_evpn.h"
#include "bgpd/bgp_rd.h"
extern struct zclient *zclient; extern struct zclient *zclient;
@ -700,7 +701,7 @@ static void evaluate_paths(struct bgp_nexthop_cache *bnc)
char buf[PREFIX2STR_BUFFER]; char buf[PREFIX2STR_BUFFER];
bnc_str(bnc, buf, PREFIX2STR_BUFFER); bnc_str(bnc, buf, PREFIX2STR_BUFFER);
zlog_debug( zlog_debug(
"NH update for %s(%s) - flags 0x%x chgflags 0x%x - evaluate paths", "NH update for %s %s flags 0x%x chgflags 0x%x - evaluate paths",
buf, bnc->bgp->name_pretty, bnc->flags, buf, bnc->bgp->name_pretty, bnc->flags,
bnc->change_flags); bnc->change_flags);
} }
@ -735,7 +736,8 @@ static void evaluate_paths(struct bgp_nexthop_cache *bnc)
* nexthops with labels * nexthops with labels
*/ */
int bnc_is_valid_nexthop = 0; bool bnc_is_valid_nexthop = false;
bool path_valid = false;
if (safi == SAFI_UNICAST && if (safi == SAFI_UNICAST &&
path->sub_type == BGP_ROUTE_IMPORTED && path->sub_type == BGP_ROUTE_IMPORTED &&
@ -743,7 +745,7 @@ static void evaluate_paths(struct bgp_nexthop_cache *bnc)
path->extra->num_labels) { path->extra->num_labels) {
bnc_is_valid_nexthop = bnc_is_valid_nexthop =
bgp_isvalid_labeled_nexthop(bnc) ? 1 : 0; bgp_isvalid_labeled_nexthop(bnc) ? true : false;
} else { } else {
if (bgp_update_martian_nexthop( if (bgp_update_martian_nexthop(
bnc->bgp, afi, safi, path->type, bnc->bgp, afi, safi, path->type,
@ -754,29 +756,31 @@ static void evaluate_paths(struct bgp_nexthop_cache *bnc)
__func__, rn, bgp_path->name); __func__, rn, bgp_path->name);
} else } else
bnc_is_valid_nexthop = bnc_is_valid_nexthop =
bgp_isvalid_nexthop(bnc) ? 1 : 0; bgp_isvalid_nexthop(bnc) ? true : false;
} }
if (BGP_DEBUG(nht, NHT)) if (BGP_DEBUG(nht, NHT)) {
zlog_debug("%s: prefix %pRN (vrf %s) %svalid", __func__, char buf1[RD_ADDRSTRLEN];
rn, bgp_path->name,
(bnc_is_valid_nexthop ? "" : "not "));
if ((CHECK_FLAG(path->flags, BGP_PATH_VALID) ? 1 : 0) if (rn->prn) {
!= bnc_is_valid_nexthop) { prefix_rd2str((struct prefix_rd *)&rn->prn->p,
if (CHECK_FLAG(path->flags, BGP_PATH_VALID)) { buf1, sizeof(buf1));
bgp_aggregate_decrement(bgp_path, p, path, afi, zlog_debug(
safi); "... eval path %d/%d %pRN RD %s %s flags 0x%x",
bgp_path_info_unset_flag(rn, path, afi, safi, rn, buf1,
BGP_PATH_VALID); bgp_path->name_pretty, path->flags);
} else { } else
bgp_path_info_set_flag(rn, path, zlog_debug(
BGP_PATH_VALID); "... eval path %d/%d %pRN %s flags 0x%x",
bgp_aggregate_increment(bgp_path, p, path, afi, afi, safi, rn, bgp_path->name_pretty,
safi); path->flags);
}
} }
/* Skip paths marked for removal or as history. */
if (CHECK_FLAG(path->flags, BGP_PATH_REMOVED)
|| CHECK_FLAG(path->flags, BGP_PATH_HISTORY))
continue;
/* Copy the metric to the path. Will be used for bestpath /* Copy the metric to the path. Will be used for bestpath
* computation */ * computation */
if (bgp_isvalid_nexthop(bnc) && bnc->metric) if (bgp_isvalid_nexthop(bnc) && bnc->metric)
@ -789,13 +793,33 @@ static void evaluate_paths(struct bgp_nexthop_cache *bnc)
|| CHECK_FLAG(bnc->change_flags, BGP_NEXTHOP_CHANGED)) || CHECK_FLAG(bnc->change_flags, BGP_NEXTHOP_CHANGED))
SET_FLAG(path->flags, BGP_PATH_IGP_CHANGED); SET_FLAG(path->flags, BGP_PATH_IGP_CHANGED);
if (safi == SAFI_EVPN && bgp_evpn_is_prefix_nht_supported(p)) { path_valid = !!CHECK_FLAG(path->flags, BGP_PATH_VALID);
if (CHECK_FLAG(path->flags, BGP_PATH_VALID)) if (path_valid != bnc_is_valid_nexthop) {
bgp_evpn_import_route(bgp_path, afi, safi, p, if (path_valid) {
path); /* No longer valid, clear flag; also for EVPN
else * routes, unimport from VRFs if needed.
bgp_evpn_unimport_route(bgp_path, afi, safi, p, */
path); bgp_aggregate_decrement(bgp_path, p, path, afi,
safi);
bgp_path_info_unset_flag(rn, path,
BGP_PATH_VALID);
if (safi == SAFI_EVPN &&
bgp_evpn_is_prefix_nht_supported(&rn->p))
bgp_evpn_unimport_route(bgp_path,
afi, safi, &rn->p, path);
} else {
/* Path becomes valid, set flag; also for EVPN
* routes, import from VRFs if needed.
*/
bgp_path_info_set_flag(rn, path,
BGP_PATH_VALID);
bgp_aggregate_increment(bgp_path, p, path, afi,
safi);
if (safi == SAFI_EVPN &&
bgp_evpn_is_prefix_nht_supported(&rn->p))
bgp_evpn_import_route(bgp_path,
afi, safi, &rn->p, path);
}
} }
bgp_process(bgp_path, rn, afi, safi); bgp_process(bgp_path, rn, afi, safi);