From a633fb579e377fb745d3d59b6158dba9dc784875 Mon Sep 17 00:00:00 2001 From: Iqra Siddiqui Date: Fri, 27 May 2022 08:07:36 -0700 Subject: [PATCH] bgpd: Fix insonsistencies with default-originate route-map Description: - When there are multiple policies configured with route-map then the first matching policy is not getting applied on default route originated with default-originate. - In BGP we first run through the BGP RIB and then pass it to the route-map to find if its permit or deny. Due to this behaviour the first route in BGP RIB that passes the route-map will be applied. Fix: - Passing extra parameter to routemap_apply so that we can get the preference of the matching policy, keep comparing it with the old preference and finally consider the policy with less preference. Co-authored-by: Abhinay Ramesh Signed-off-by: Iqra Siddiqui --- bgpd/bgp_updgrp_adv.c | 34 ++++++++++++++++++---------------- lib/routemap.c | 12 ++++++++++-- lib/routemap.h | 4 ++-- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c index 52f903f4fa..0f7f2f4c02 100644 --- a/bgpd/bgp_updgrp_adv.c +++ b/bgpd/bgp_updgrp_adv.c @@ -796,8 +796,11 @@ void subgroup_default_originate(struct update_subgroup *subgrp, int withdraw) struct peer *peer; struct bgp_adj_out *adj; route_map_result_t ret = RMAP_DENYMATCH; + route_map_result_t new_ret = RMAP_DENYMATCH; afi_t afi; safi_t safi; + int pref = 65536; + int new_pref = 0; if (!subgrp) return; @@ -853,28 +856,27 @@ void subgroup_default_originate(struct update_subgroup *subgrp, int withdraw) tmp_pi.attr = &tmp_attr; - ret = route_map_apply_ext( + new_ret = route_map_apply_ext( peer->default_rmap[afi][safi].map, - bgp_dest_get_prefix(dest), pi, &tmp_pi); - - if (ret == RMAP_DENYMATCH) { - bgp_attr_flush(&tmp_attr); - continue; - } else { - new_attr = bgp_attr_intern(&tmp_attr); - + bgp_dest_get_prefix(dest), pi, &tmp_pi, + &new_pref); + + if (new_ret == RMAP_PERMITMATCH) { + if (new_pref < pref) { + pref = new_pref; + bgp_attr_flush(new_attr); + new_attr = bgp_attr_intern( + tmp_pi.attr); + bgp_attr_flush(tmp_pi.attr); + } subgroup_announce_reset_nhop( (peer_cap_enhe(peer, afi, safi) ? AF_INET6 : AF_INET), new_attr); - - break; - } - } - if (ret == RMAP_PERMITMATCH) { - bgp_dest_unlock_node(dest); - break; + ret = new_ret; + } else + bgp_attr_flush(&tmp_attr); } } bgp->peer_self->rmap_type = 0; diff --git a/lib/routemap.c b/lib/routemap.c index 7fd5a96e5b..9529b79419 100644 --- a/lib/routemap.c +++ b/lib/routemap.c @@ -2540,7 +2540,8 @@ void route_map_notify_pentry_dependencies(const char *affected_name, */ route_map_result_t route_map_apply_ext(struct route_map *map, const struct prefix *prefix, - void *match_object, void *set_object) + void *match_object, void *set_object, + int *pref) { static int recursion = 0; enum route_map_cmd_result_t match_ret = RMAP_NOMATCH; @@ -2676,7 +2677,7 @@ route_map_result_t route_map_apply_ext(struct route_map *map, ret = route_map_apply_ext( nextrm, prefix, match_object, - set_object); + set_object, NULL); recursion--; } @@ -2721,6 +2722,13 @@ route_map_apply_end: (map ? map->name : "null"), prefix, route_map_result_str(ret)); + if (pref) { + if (index != NULL && ret == RMAP_PERMITMATCH) + *pref = index->pref; + else + *pref = 65536; + } + return (ret); } diff --git a/lib/routemap.h b/lib/routemap.h index 13dafe6849..ad391981e0 100644 --- a/lib/routemap.h +++ b/lib/routemap.h @@ -482,9 +482,9 @@ struct route_map *route_map_lookup_warn_noexist(struct vty *vty, const char *nam extern route_map_result_t route_map_apply_ext(struct route_map *map, const struct prefix *prefix, void *match_object, - void *set_object); + void *set_object, int *pref); #define route_map_apply(map, prefix, object) \ - route_map_apply_ext(map, prefix, object, object) + route_map_apply_ext(map, prefix, object, object, NULL) extern void route_map_add_hook(void (*func)(const char *)); extern void route_map_delete_hook(void (*func)(const char *)); -- 2.39.5