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 <rabhinay@vmware.com>
Signed-off-by: Iqra Siddiqui <imujeebsiddi@vmware.com>

diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c
index 52f903f4fa414b5df6d747a47f5e0fc8568802d8..0f7f2f4c022ed61436d3575bcab17e20c6f0d8af 100644 (file)
--- 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 7fd5a96e5b5f596ac854b4279f6c8ede2f8f4485..9529b79419407e09010aa28be7d957a56312fbcc 100644 (file)
--- 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 13dafe6849edb6d1b645b65a1ed2b6c69ed7ea13..ad391981e0de5a0fa9f53ae37044ebcd00bf569d 100644 (file)
--- 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 *));