From: Donatas Abraitis Date: Tue, 15 Feb 2022 16:08:32 +0000 (+0200) Subject: bgpd: Allow setting attributes over route-maps for conditional advertisements X-Git-Tag: frr-8.2.2~21^2 X-Git-Url: https://git.proxmox.com/?a=commitdiff_plain;h=cb290e57c773a3e3fbcce38a282ad4960ecd52dc;p=mirror_frr.git bgpd: Allow setting attributes over route-maps for conditional advertisements Signed-off-by: Donatas Abraitis --- diff --git a/bgpd/bgp_conditional_adv.c b/bgpd/bgp_conditional_adv.c index c0dd3d6f8..e5a4b0e9f 100644 --- a/bgpd/bgp_conditional_adv.c +++ b/bgpd/bgp_conditional_adv.c @@ -82,7 +82,7 @@ static void bgp_conditional_adv_routes(struct peer *peer, afi_t afi, struct peer_af *paf; const struct prefix *dest_p; struct update_subgroup *subgrp; - struct attr dummy_attr = {0}, attr = {0}; + struct attr advmap_attr = {0}, attr = {0}; struct bgp_path_info_extra path_extra = {0}; route_map_result_t ret; @@ -110,55 +110,53 @@ static void bgp_conditional_adv_routes(struct peer *peer, afi_t afi, assert(dest_p); for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) { - dummy_attr = *pi->attr; + advmap_attr = *pi->attr; /* Fill temp path_info */ prep_for_rmap_apply(&path, &path_extra, dest, pi, - pi->peer, &dummy_attr); + pi->peer, &advmap_attr); - RESET_FLAG(dummy_attr.rmap_change_flags); + RESET_FLAG(advmap_attr.rmap_change_flags); ret = route_map_apply(rmap, dest_p, &path); - bgp_attr_flush(&dummy_attr); - - if (ret != RMAP_PERMITMATCH) + if (ret != RMAP_PERMITMATCH || + !bgp_check_selected(pi, peer, addpath_capable, afi, + safi)) { + bgp_attr_flush(&advmap_attr); continue; + } - if (bgp_check_selected(pi, peer, addpath_capable, afi, - safi)) { - /* Skip route-map checks in - * subgroup_announce_check while executing from - * the conditional advertise scanner process. - * otherwise when route-map is also configured - * on same peer, routes in advertise-map may not - * be advertised as expected. + /* Skip route-map checks in + * subgroup_announce_check while executing from + * the conditional advertise scanner process. + * otherwise when route-map is also configured + * on same peer, routes in advertise-map may not + * be advertised as expected. + */ + if (update_type == ADVERTISE && + subgroup_announce_check(dest, pi, subgrp, dest_p, + &attr, &advmap_attr)) { + bgp_adj_out_set_subgroup(dest, subgrp, &attr, + pi); + } else { + /* If default originate is enabled for + * the peer, do not send explicit + * withdraw. This will prevent deletion + * of default route advertised through + * default originate. */ - if ((update_type == ADVERTISE) - && subgroup_announce_check(dest, pi, subgrp, - dest_p, &attr, - true)) - bgp_adj_out_set_subgroup(dest, subgrp, - &attr, pi); - else { - /* If default originate is enabled for - * the peer, do not send explicit - * withdraw. This will prevent deletion - * of default route advertised through - * default originate. - */ - if (CHECK_FLAG( - peer->af_flags[afi][safi], - PEER_FLAG_DEFAULT_ORIGINATE) - && is_default_prefix(dest_p)) - break; - - bgp_adj_out_unset_subgroup( - dest, subgrp, 1, - bgp_addpath_id_for_peer( - peer, afi, safi, - &pi->tx_addpath)); - } + if (CHECK_FLAG(peer->af_flags[afi][safi], + PEER_FLAG_DEFAULT_ORIGINATE) && + is_default_prefix(dest_p)) + break; + + bgp_adj_out_unset_subgroup( + dest, subgrp, 1, + bgp_addpath_id_for_peer( + peer, afi, safi, + &pi->tx_addpath)); } + bgp_attr_flush(&advmap_attr); } } UNSET_FLAG(subgrp->sflags, SUBGRP_STATUS_TABLE_REPARSING); diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 21685af95..b297ca006 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -1836,7 +1836,7 @@ void subgroup_announce_reset_nhop(uint8_t family, struct attr *attr) bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, struct update_subgroup *subgrp, const struct prefix *p, struct attr *attr, - bool skip_rmap_check) + struct attr *post_attr) { struct bgp_filter *filter; struct peer *from; @@ -2067,8 +2067,16 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, } } - /* For modify attribute, copy it to temporary structure. */ - *attr = *piattr; + /* For modify attribute, copy it to temporary structure. + * post_attr comes from BGP conditional advertisements, where + * attributes are already processed by advertise-map route-map, + * and this needs to be saved instead of overwriting from the + * path attributes. + */ + if (post_attr) + *attr = *post_attr; + else + *attr = *piattr; /* If local-preference is not set. */ if ((peer->sort == BGP_PEER_IBGP || peer->sort == BGP_PEER_CONFED) @@ -2162,8 +2170,8 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, bgp_peer_as_override(bgp, afi, safi, peer, attr); /* Route map & unsuppress-map apply. */ - if (!skip_rmap_check - && (ROUTE_MAP_OUT_NAME(filter) || bgp_path_suppressed(pi))) { + if (!post_attr && + (ROUTE_MAP_OUT_NAME(filter) || bgp_path_suppressed(pi))) { struct bgp_path_info rmap_path = {0}; struct bgp_path_info_extra dummy_rmap_path_extra = {0}; struct attr dummy_attr = {0}; @@ -2696,7 +2704,7 @@ void subgroup_process_announce_selected(struct update_subgroup *subgrp, if (selected) { if (subgroup_announce_check(dest, selected, subgrp, p, &attr, - false)) { + NULL)) { /* Route is selected, if the route is already installed * in FIB, then it is advertised */ diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index 317995594..a8ec2dc90 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -785,7 +785,7 @@ extern bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, struct update_subgroup *subgrp, const struct prefix *p, struct attr *attr, - bool skip_rmap_check); + struct attr *post_attr); extern void bgp_peer_clear_node_queue_drain_immediate(struct peer *peer); extern void bgp_process_queues_drain_immediate(void); diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c index 2110dcb8b..fc1a8bdf3 100644 --- a/bgpd/bgp_updgrp_adv.c +++ b/bgpd/bgp_updgrp_adv.c @@ -691,7 +691,7 @@ void subgroup_announce_table(struct update_subgroup *subgrp, safi)) { if (subgroup_announce_check(dest, ri, subgrp, dest_p, &attr, - false)) { + NULL)) { /* Check if route can be advertised */ if (advertise) { if (!bgp_check_withdrawal(bgp, @@ -910,7 +910,7 @@ void subgroup_default_originate(struct update_subgroup *subgrp, int withdraw) if (subgroup_announce_check( dest, pi, subgrp, bgp_dest_get_prefix(dest), - &attr, false)) + &attr, NULL)) bgp_adj_out_set_subgroup( dest, subgrp, &attr, pi); diff --git a/tests/topotests/bgp_conditional_advertisement/r2/bgpd.conf b/tests/topotests/bgp_conditional_advertisement/r2/bgpd.conf index 82525fac6..73f837c69 100644 --- a/tests/topotests/bgp_conditional_advertisement/r2/bgpd.conf +++ b/tests/topotests/bgp_conditional_advertisement/r2/bgpd.conf @@ -19,6 +19,7 @@ route-map ADV-MAP-1 permit 20 ! route-map ADV-MAP-2 permit 10 match ip address prefix-list IP2 + set metric 911 ! route-map EXIST-MAP permit 10 match community DEFAULT-ROUTE diff --git a/tests/topotests/bgp_conditional_advertisement/test_bgp_conditional_advertisement.py b/tests/topotests/bgp_conditional_advertisement/test_bgp_conditional_advertisement.py index e9b393ba7..6153aee41 100644 --- a/tests/topotests/bgp_conditional_advertisement/test_bgp_conditional_advertisement.py +++ b/tests/topotests/bgp_conditional_advertisement/test_bgp_conditional_advertisement.py @@ -334,7 +334,7 @@ def test_bgp_conditional_advertisement(): "192.0.2.1/32": None, "192.0.2.5/32": None, "10.139.224.0/20": None, - "203.0.113.1/32": [{"protocol": "bgp"}], + "203.0.113.1/32": [{"protocol": "bgp", "metric": 911}], } return topotest.json_cmp(output, expected)