bgpd: aggregate address suppress more specific

Add new aggregate-address option to selectively suppress routes based
on route map results.

Signed-off-by: Rafael Zalamena <rzalamena@opensourcerouting.org>

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 73f5526fe254da2b2122f69ed2a6988975695721..9f7c04423edb4f3996a99db72073e701b0c770b3 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -6177,11 +6177,46 @@ static struct bgp_aggregate *bgp_aggregate_new(void)
 
 static void bgp_aggregate_free(struct bgp_aggregate *aggregate)
 {
+       XFREE(MTYPE_ROUTE_MAP_NAME, aggregate->suppress_map_name);
+       route_map_counter_decrement(aggregate->suppress_map);
        XFREE(MTYPE_ROUTE_MAP_NAME, aggregate->rmap.name);
        route_map_counter_decrement(aggregate->rmap.map);
        XFREE(MTYPE_BGP_AGGREGATE, aggregate);
 }
 
+/**
+ * Helper function to avoid repeated code: prepare variables for a
+ * `route_map_apply` call.
+ *
+ * \returns `true` on route map match, otherwise `false`.
+ */
+static bool aggr_suppress_map_test(struct bgp *bgp,
+                                  struct bgp_aggregate *aggregate,
+                                  struct bgp_path_info *pi)
+{
+       const struct prefix *p = bgp_dest_get_prefix(pi->net);
+       route_map_result_t rmr = RMAP_DENYMATCH;
+       struct bgp_path_info rmap_path = {};
+       struct attr attr = {};
+
+       /* No route map entries created, just don't match. */
+       if (aggregate->suppress_map == NULL)
+               return false;
+
+       /* Call route map matching and return result. */
+       attr.aspath = aspath_empty();
+       rmap_path.peer = bgp->peer_self;
+       rmap_path.attr = &attr;
+
+       SET_FLAG(bgp->peer_self->rmap_type, PEER_RMAP_TYPE_AGGREGATE);
+       rmr = route_map_apply(aggregate->suppress_map, p, RMAP_BGP, &rmap_path);
+       bgp->peer_self->rmap_type = 0;
+
+       bgp_attr_flush(&attr);
+
+       return rmr == RMAP_PERMITMATCH;
+}
+
 static bool bgp_aggregate_info_same(struct bgp_path_info *pi, uint8_t origin,
                                    struct aspath *aspath,
                                    struct community *comm,
@@ -6565,6 +6600,24 @@ void bgp_aggregate_route(struct bgp *bgp, const struct prefix *p, afi_t afi,
                                match++;
                        }
 
+                       /*
+                        * Suppress more specific routes that match the route
+                        * map results.
+                        *
+                        * MED matching:
+                        * Don't suppress routes if MED matching is enabled and
+                        * it mismatched otherwise we might end up with no
+                        * routes for this path.
+                        */
+                       if (aggregate->suppress_map_name
+                           && AGGREGATE_MED_VALID(aggregate)
+                           && aggr_suppress_map_test(bgp, aggregate, pi)) {
+                               (bgp_path_info_extra_get(pi))->suppress++;
+                               bgp_path_info_set_flag(dest, pi,
+                                                      BGP_PATH_ATTR_CHANGED);
+                               match++;
+                       }
+
                        aggregate->count++;
 
                        /*
@@ -6711,6 +6764,32 @@ void bgp_aggregate_delete(struct bgp *bgp, const struct prefix *p, afi_t afi,
                                        match++;
                                }
                        }
+
+                       if (aggregate->suppress_map_name
+                           && AGGREGATE_MED_VALID(aggregate)
+                           && aggr_suppress_map_test(bgp, aggregate, pi)) {
+                               /*
+                                * We can only get here if we were suppressed
+                                * before: it is a failure to not have
+                                * `pi->extra`.
+                                */
+                               assert(pi->extra != NULL);
+                               /*
+                                * If we suppressed before then the value must
+                                * be greater than zero.
+                                */
+                               assert(pi->extra->suppress > 0);
+
+                               pi->extra->suppress--;
+                               /* Unsuppress route if we reached `0`. */
+                               if (pi->extra->suppress == 0) {
+                                       bgp_path_info_set_flag(
+                                               dest, pi,
+                                               BGP_PATH_ATTR_CHANGED);
+                                       match++;
+                               }
+                       }
+
                        aggregate->count--;
 
                        if (pi->attr->origin == BGP_ORIGIN_INCOMPLETE)
@@ -6803,6 +6882,10 @@ static void bgp_add_route_to_aggregate(struct bgp *bgp,
        if (aggregate->summary_only && AGGREGATE_MED_VALID(aggregate))
                (bgp_path_info_extra_get(pinew))->suppress++;
 
+       if (aggregate->suppress_map_name && AGGREGATE_MED_VALID(aggregate)
+           && aggr_suppress_map_test(bgp, aggregate, pinew))
+               (bgp_path_info_extra_get(pinew))->suppress++;
+
        switch (pinew->attr->origin) {
        case BGP_ORIGIN_INCOMPLETE:
                aggregate->incomplete_origin_count++;
@@ -6908,8 +6991,30 @@ static void bgp_remove_route_from_aggregate(struct bgp *bgp, afi_t afi,
                }
        }
 
+       if (aggregate->suppress_map_name && AGGREGATE_MED_VALID(aggregate)
+           && aggr_suppress_map_test(bgp, aggregate, pi)) {
+               /*
+                * We can only get here if we were suppressed before:
+                * it is a failure to not have `pi->extra`.
+                */
+               assert(pi->extra != NULL);
+               /*
+                * If we suppressed before then the value must be
+                * greater than zero.
+                */
+               assert(pi->extra->suppress > 0);
+
+               pi->extra->suppress--;
+               /* Unsuppress when we reached `0`. */
+               if (pi->extra->suppress == 0) {
+                       bgp_path_info_set_flag(pi->net, pi,
+                                              BGP_PATH_ATTR_CHANGED);
+                       match++;
+               }
+       }
+
        /*
-        * This must be called after `summary` check to avoid
+        * This must be called after `summary`, `suppress-map` check to avoid
         * "unsuppressing" twice.
         */
        if (aggregate->match_med)
@@ -7172,7 +7277,8 @@ static int bgp_aggregate_unset(struct vty *vty, const char *prefix_str,
 static int bgp_aggregate_set(struct vty *vty, const char *prefix_str, afi_t afi,
                             safi_t safi, const char *rmap,
                             uint8_t summary_only, uint8_t as_set,
-                            uint8_t origin, bool match_med)
+                            uint8_t origin, bool match_med,
+                            const char *suppress_map)
 {
        VTY_DECLVAR_CONTEXT(bgp, bgp);
        int ret;
@@ -7181,6 +7287,12 @@ static int bgp_aggregate_set(struct vty *vty, const char *prefix_str, afi_t afi,
        struct bgp_aggregate *aggregate;
        uint8_t as_set_new = as_set;
 
+       if (suppress_map && summary_only) {
+               vty_out(vty,
+                       "'summary-only' and 'suppress-map' can't be used at the same time\n");
+               return CMD_WARNING_CONFIG_FAILED;
+       }
+
        /* Convert string to prefix structure. */
        ret = str2prefix(prefix_str, &p);
        if (!ret) {
@@ -7252,6 +7364,18 @@ static int bgp_aggregate_set(struct vty *vty, const char *prefix_str, afi_t afi,
                aggregate->rmap.map = route_map_lookup_by_name(rmap);
                route_map_counter_increment(aggregate->rmap.map);
        }
+
+       if (suppress_map) {
+               XFREE(MTYPE_ROUTE_MAP_NAME, aggregate->suppress_map_name);
+               route_map_counter_decrement(aggregate->suppress_map);
+
+               aggregate->suppress_map_name =
+                       XSTRDUP(MTYPE_ROUTE_MAP_NAME, suppress_map);
+               aggregate->suppress_map =
+                       route_map_lookup_by_name(aggregate->suppress_map_name);
+               route_map_counter_increment(aggregate->suppress_map);
+       }
+
        bgp_dest_set_bgp_aggregate_info(dest, aggregate);
 
        /* Aggregate address insert into BGP routing table. */
@@ -7267,6 +7391,7 @@ DEFPY(aggregate_addressv4, aggregate_addressv4_cmd,
       "|route-map WORD$rmap_name"
       "|origin <egp|igp|incomplete>$origin_s"
       "|matching-MED-only$match_med"
+      "|suppress-map WORD$suppress_map"
       "}",
       NO_STR
       "Configure BGP aggregate entries\n"
@@ -7279,7 +7404,9 @@ DEFPY(aggregate_addressv4, aggregate_addressv4_cmd,
       "Remote EGP\n"
       "Local IGP\n"
       "Unknown heritage\n"
-      "Only aggregate routes with matching MED\n")
+      "Only aggregate routes with matching MED\n"
+      "Suppress the selected more specific routes\n"
+      "Route map with the route selectors\n")
 {
        const char *prefix_s = NULL;
        safi_t safi = bgp_node_safi(vty);
@@ -7315,7 +7442,7 @@ DEFPY(aggregate_addressv4, aggregate_addressv4_cmd,
 
        return bgp_aggregate_set(vty, prefix_s, AFI_IP, safi, rmap_name,
                                 summary_only != NULL, as_set, origin,
-                                match_med != NULL);
+                                match_med != NULL, suppress_map);
 }
 
 DEFPY(aggregate_addressv6, aggregate_addressv6_cmd,
@@ -7325,6 +7452,7 @@ DEFPY(aggregate_addressv6, aggregate_addressv6_cmd,
       "|route-map WORD$rmap_name"
       "|origin <egp|igp|incomplete>$origin_s"
       "|matching-MED-only$match_med"
+      "|suppress-map WORD$suppress_map"
       "}",
       NO_STR
       "Configure BGP aggregate entries\n"
@@ -7337,7 +7465,9 @@ DEFPY(aggregate_addressv6, aggregate_addressv6_cmd,
       "Remote EGP\n"
       "Local IGP\n"
       "Unknown heritage\n"
-      "Only aggregate routes with matching MED\n")
+      "Only aggregate routes with matching MED\n"
+      "Suppress the selected more specific routes\n"
+      "Route map with the route selectors\n")
 {
        uint8_t origin = BGP_ORIGIN_UNSPECIFIED;
        int as_set = AGGREGATE_AS_UNSET;
@@ -7361,7 +7491,7 @@ DEFPY(aggregate_addressv6, aggregate_addressv6_cmd,
 
        return bgp_aggregate_set(vty, prefix_str, AFI_IP6, SAFI_UNICAST,
                                 rmap_name, summary_only != NULL, as_set,
-                                origin, match_med != NULL);
+                                origin, match_med != NULL, suppress_map);
 }
 
 /* Redistribute route treatment. */
@@ -13949,6 +14079,10 @@ void bgp_config_write_network(struct vty *vty, struct bgp *bgp, afi_t afi,
                if (bgp_aggregate->match_med)
                        vty_out(vty, " matching-MED-only");
 
+               if (bgp_aggregate->suppress_map_name)
+                       vty_out(vty, " suppress-map %s",
+                               bgp_aggregate->suppress_map_name);
+
                vty_out(vty, "\n");
        }
 }

diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h
index 962a086081b3e26d4da6a198fb2345a2b75dfc57..40710ecf026b3977fefd89f1fc57f7cc64c83a18 100644 (file)
--- a/bgpd/bgp_route.h
+++ b/bgpd/bgp_route.h
@@ -398,6 +398,11 @@ struct bgp_aggregate {
 #define AGGREGATE_MED_VALID(aggregate)                                         \
        (((aggregate)->match_med && !(aggregate)->med_mismatched)              \
         || !(aggregate)->match_med)
+
+       /** Suppress map route map name (`NULL` when disabled). */
+       char *suppress_map_name;
+       /** Suppress map route map pointer. */
+       struct route_map *suppress_map;
 };
 
 #define BGP_NEXTHOP_AFI_FROM_NHLEN(nhlen)                                      \

diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index 968cda3f100261735a4acb053c4c16d1fb4487d8..dd22a47c89eccae1e52faf110fc0869c5c861a8b 100644 (file)
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -3845,6 +3845,16 @@ static void bgp_route_map_process_update(struct bgp *bgp, const char *rmap_name,
                        if (!aggregate)
                                continue;
 
+                       /* Update suppress map pointer. */
+                       if (aggregate->suppress_map_name
+                           && strmatch(aggregate->suppress_map_name,
+                                       rmap_name)) {
+                               if (!aggregate->rmap.map)
+                                       route_map_counter_increment(map);
+
+                               aggregate->suppress_map = map;
+                       }
+
                        if (!aggregate->rmap.name
                            || (strcmp(rmap_name, aggregate->rmap.name) != 0))
                                continue;