Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bgpd: apply route-map for aggregate before attribute comparison (backport #17801) #17824

Closed
wants to merge 2 commits into from
Closed

bgpd: apply route-map for aggregate before attribute comparison (backport #17801) #17824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d8fd1d
Revert "bgpd: Reinstall aggregated routes if using route-maps and it …
enkechen-panw Jan 8, 2025
964421b
bgpd: apply route-map for aggregate before attribute comparison
enkechen-panw Jan 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 35 additions & 44 deletions bgpd/bgp_route.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7315,44 +7315,6 @@ static bool aggr_unsuppress_path(struct bgp_aggregate *aggregate,
return false;
}

static bool bgp_aggregate_info_same(struct bgp_path_info *pi, uint8_t origin,
struct aspath *aspath,
struct community *comm,
struct ecommunity *ecomm,
struct lcommunity *lcomm)
{
static struct aspath *ae = NULL;
enum asnotation_mode asnotation;

asnotation = bgp_get_asnotation(NULL);

if (!aspath)
ae = aspath_empty(asnotation);

if (!pi)
return false;

if (origin != pi->attr->origin)
return false;

if (!aspath_cmp(pi->attr->aspath, (aspath) ? aspath : ae))
return false;

if (!community_cmp(bgp_attr_get_community(pi->attr), comm))
return false;

if (!ecommunity_cmp(bgp_attr_get_ecommunity(pi->attr), ecomm))
return false;

if (!lcommunity_cmp(bgp_attr_get_lcommunity(pi->attr), lcomm))
return false;

if (!CHECK_FLAG(pi->flags, BGP_PATH_VALID))
return false;

return true;
}

static void bgp_aggregate_install(
struct bgp *bgp, afi_t afi, safi_t safi, const struct prefix *p,
uint8_t origin, struct aspath *aspath, struct community *community,
Expand All @@ -7361,14 +7323,14 @@ static void bgp_aggregate_install(
{
struct bgp_dest *dest;
struct bgp_table *table;
struct bgp_path_info *pi, *orig, *new;
struct bgp_path_info *pi, *new;
struct attr *attr;

table = bgp->rib[afi][safi];

dest = bgp_node_get(table, p);

for (orig = pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next)
for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next)
if (pi->peer == bgp->peer_self && pi->type == ZEBRA_ROUTE_BGP
&& pi->sub_type == BGP_ROUTE_AGGREGATE)
break;
Expand All @@ -7385,11 +7347,21 @@ static void bgp_aggregate_install(
* If the aggregate information has not changed
* no need to re-install it again.
*/
if (pi && (!aggregate->rmap.changed &&
bgp_aggregate_info_same(pi, origin, aspath, community,
ecommunity, lcommunity))) {
attr = bgp_attr_aggregate_intern(bgp, origin, aspath, community, ecommunity,
lcommunity, aggregate, atomic_aggregate, p);
if (!attr) {
aspath_free(aspath);
community_free(&community);
ecommunity_free(&ecommunity);
lcommunity_free(&lcommunity);
bgp_dest_unlock_node(dest);
bgp_aggregate_delete(bgp, p, afi, safi, aggregate);
if (debug)
zlog_debug("%s: %pFX null attribute", __func__, p);
return;
}

<<<<<<< HEAD
if (aspath)
aspath_free(aspath);
if (community)
Expand All @@ -7399,6 +7371,13 @@ static void bgp_aggregate_install(
if (lcommunity)
lcommunity_free(&lcommunity);

=======
if (pi && CHECK_FLAG(pi->flags, BGP_PATH_VALID) && attrhash_cmp(pi->attr, attr)) {
bgp_attr_unintern(&attr);
bgp_dest_unlock_node(dest);
if (debug)
zlog_debug(" aggregate %pFX: duplicate", p);
>>>>>>> 43c567014 (bgpd: apply route-map for aggregate before attribute comparison)
return;
}

Expand All @@ -7408,6 +7387,7 @@ static void bgp_aggregate_install(
if (pi)
bgp_path_info_delete(dest, pi);

<<<<<<< HEAD
attr = bgp_attr_aggregate_intern(
bgp, origin, aspath, community, ecommunity, lcommunity,
aggregate, atomic_aggregate, p);
Expand All @@ -7424,6 +7404,8 @@ static void bgp_aggregate_install(
p);
return;
}
=======
>>>>>>> 43c567014 (bgpd: apply route-map for aggregate before attribute comparison)

new = info_make(ZEBRA_ROUTE_BGP, BGP_ROUTE_AGGREGATE, 0,
bgp->peer_self, attr, dest);
Expand All @@ -7434,6 +7416,7 @@ static void bgp_aggregate_install(
bgp_process(bgp, dest, afi, safi);
} else {
uninstall_aggregate_route:
<<<<<<< HEAD
for (pi = orig; pi; pi = pi->next)
if (pi->peer == bgp->peer_self
&& pi->type == ZEBRA_ROUTE_BGP
Expand All @@ -7445,6 +7428,15 @@ static void bgp_aggregate_install(
bgp_path_info_delete(dest, pi);
bgp_process(bgp, dest, afi, safi);
}
=======
/* Withdraw the aggregate route from routing table. */
if (pi) {
bgp_path_info_delete(dest, pi);
bgp_process(bgp, dest, pi, afi, safi);
if (debug)
zlog_debug(" aggregate %pFX: uninstall", p);
}
>>>>>>> 43c567014 (bgpd: apply route-map for aggregate before attribute comparison)
}

bgp_dest_unlock_node(dest);
Expand Down Expand Up @@ -8397,7 +8389,6 @@ static int bgp_aggregate_set(struct vty *vty, const char *prefix_str, afi_t afi,
aggregate->rmap.name =
XSTRDUP(MTYPE_ROUTE_MAP_NAME, rmap);
aggregate->rmap.map = route_map_lookup_by_name(rmap);
aggregate->rmap.changed = true;
route_map_counter_increment(aggregate->rmap.map);
}

Expand Down
1 change: 0 additions & 1 deletion bgpd/bgp_route.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,6 @@ struct bgp_aggregate {
struct {
char *name;
struct route_map *map;
bool changed;
} rmap;

/* Suppress-count. */
Expand Down
1 change: 0 additions & 1 deletion bgpd/bgp_routemap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4604,7 +4604,6 @@ static void bgp_route_map_process_update(struct bgp *bgp, const char *rmap_name,
route_map_counter_increment(map);

aggregate->rmap.map = map;
aggregate->rmap.changed = true;

matched = true;
}
Expand Down
Loading