Merge pull request #7239 from opensourcerouting/bgp-aggregate-med

bgpd: aggregate-address on matching MED only

bgpd/bgp_route.c

@@ -6240,6 +6240,13 @@ static void bgp_aggregate_install(
 		    && pi->sub_type == BGP_ROUTE_AGGREGATE)
 			break;
 
+	/*
+	 * If we have paths with different MEDs, then don't install
+	 * (or uninstall) the aggregate route.
+	 */
+	if (aggregate->match_med && aggregate->med_mismatched)
+		goto uninstall_aggregate_route;
+
 	if (aggregate->count > 0) {
 		/*
 		 * If the aggregate information has not changed
@@ -6284,6 +6291,7 @@ static void bgp_aggregate_install(
 		bgp_path_info_add(dest, new);
 		bgp_process(bgp, dest, afi, safi);
 	} else {
+	uninstall_aggregate_route:
 		for (pi = orig; pi; pi = pi->next)
 			if (pi->peer == bgp->peer_self
 			    && pi->type == ZEBRA_ROUTE_BGP
@@ -6300,6 +6308,189 @@ static void bgp_aggregate_install(
 	bgp_dest_unlock_node(dest);
 }
 
+/**
+ * Check if the current path has different MED than other known paths.
+ *
+ * \returns `true` if the MED matched the others else `false`.
+ */
+static bool bgp_aggregate_med_match(struct bgp_aggregate *aggregate,
+				    struct bgp *bgp, struct bgp_path_info *pi)
+{
+	uint32_t cur_med = bgp_med_value(pi->attr, bgp);
+
+	/* This is the first route being analyzed. */
+	if (!aggregate->med_initialized) {
+		aggregate->med_initialized = true;
+		aggregate->med_mismatched = false;
+		aggregate->med_matched_value = cur_med;
+	} else {
+		/* Check if routes with different MED showed up. */
+		if (cur_med != aggregate->med_matched_value)
+			aggregate->med_mismatched = true;
+	}
+
+	return !aggregate->med_mismatched;
+}
+
+/**
+ * Initializes and tests all routes in the aggregate address path for MED
+ * values.
+ *
+ * \returns `true` if all MEDs are the same otherwise `false`.
+ */
+static bool bgp_aggregate_test_all_med(struct bgp_aggregate *aggregate,
+				       struct bgp *bgp, const struct prefix *p,
+				       afi_t afi, safi_t safi)
+{
+	struct bgp_table *table = bgp->rib[afi][safi];
+	const struct prefix *dest_p;
+	struct bgp_dest *dest, *top;
+	struct bgp_path_info *pi;
+	bool med_matched = true;
+
+	aggregate->med_initialized = false;
+
+	top = bgp_node_get(table, p);
+	for (dest = bgp_node_get(table, p); dest;
+	     dest = bgp_route_next_until(dest, top)) {
+		dest_p = bgp_dest_get_prefix(dest);
+		if (dest_p->prefixlen <= p->prefixlen)
+			continue;
+
+		for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) {
+			if (BGP_PATH_HOLDDOWN(pi))
+				continue;
+			if (pi->sub_type == BGP_ROUTE_AGGREGATE)
+				continue;
+			if (!bgp_aggregate_med_match(aggregate, bgp, pi)) {
+				med_matched = false;
+				break;
+			}
+		}
+		if (!med_matched)
+			break;
+	}
+	bgp_dest_unlock_node(top);
+
+	return med_matched;
+}
+
+/**
+ * Toggles the route suppression status for this aggregate address
+ * configuration.
+ */
+static void bgp_aggregate_toggle_suppressed(struct bgp_aggregate *aggregate,
+					    struct bgp *bgp,
+					    const struct prefix *p, afi_t afi,
+					    safi_t safi, bool suppress)
+{
+	struct bgp_table *table = bgp->rib[afi][safi];
+	struct bgp_path_info_extra *pie;
+	const struct prefix *dest_p;
+	struct bgp_dest *dest, *top;
+	struct bgp_path_info *pi;
+	bool toggle_suppression;
+
+	/* We've found a different MED we must revert any suppressed routes. */
+	top = bgp_node_get(table, p);
+	for (dest = bgp_node_get(table, p); dest;
+	     dest = bgp_route_next_until(dest, top)) {
+		dest_p = bgp_dest_get_prefix(dest);
+		if (dest_p->prefixlen <= p->prefixlen)
+			continue;
+
+		toggle_suppression = false;
+		for (pi = bgp_dest_get_bgp_path_info(dest); pi; pi = pi->next) {
+			if (BGP_PATH_HOLDDOWN(pi))
+				continue;
+			if (pi->sub_type == BGP_ROUTE_AGGREGATE)
+				continue;
+
+			/*
+			 * On installation it is possible that pi->extra is
+			 * set to NULL, otherwise it must exists.
+			 */
+			assert(!suppress && pi->extra != NULL);
+
+			/* We are toggling suppression back. */
+			if (suppress) {
+				pie = bgp_path_info_extra_get(pi);
+				/* Suppress route if not suppressed already. */
+				pie->suppress++;
+				bgp_path_info_set_flag(dest, pi,
+						       BGP_PATH_ATTR_CHANGED);
+				toggle_suppression = true;
+				continue;
+			}
+
+			pie = pi->extra;
+			assert(pie->suppress > 0);
+			pie->suppress--;
+			/* Install route if there is no more suppression. */
+			if (pie->suppress == 0) {
+				bgp_path_info_set_flag(dest, pi,
+						       BGP_PATH_ATTR_CHANGED);
+				toggle_suppression = true;
+			}
+		}
+
+		if (toggle_suppression)
+			bgp_process(bgp, dest, afi, safi);
+	}
+	bgp_dest_unlock_node(top);
+}
+
+/**
+ * Aggregate address MED matching incremental test: this function is called
+ * when the initial aggregation occurred and we are only testing a single
+ * new path.
+ *
+ * In addition to testing and setting the MED validity it also installs back
+ * suppressed routes (if summary is configured).
+ *
+ * Must not be called in `bgp_aggregate_route`.
+ */
+static void bgp_aggregate_med_update(struct bgp_aggregate *aggregate,
+				     struct bgp *bgp, const struct prefix *p,
+				     afi_t afi, safi_t safi,
+				     struct bgp_path_info *pi, bool is_adding)
+{
+	/* MED matching disabled. */
+	if (!aggregate->match_med)
+		return;
+
+	/* Aggregation with different MED, nothing to do. */
+	if (aggregate->med_mismatched)
+		return;
+
+	/*
+	 * Test the current entry:
+	 *
+	 * is_adding == true: if the new entry doesn't match then we must
+	 * install all suppressed routes.
+	 *
+	 * is_adding == false: if the entry being removed was the last
+	 * unmatching entry then we can suppress all routes.
+	 */
+	if (!is_adding) {
+		if (bgp_aggregate_test_all_med(aggregate, bgp, p, afi, safi)
+		    && aggregate->summary_only)
+			bgp_aggregate_toggle_suppressed(aggregate, bgp, p, afi,
+							safi, true);
+	} else
+		bgp_aggregate_med_match(aggregate, bgp, pi);
+
+	/* No mismatches, just quit. */
+	if (!aggregate->med_mismatched)
+		return;
+
+	/* Route summarization is disabled. */
+	if (!aggregate->summary_only)
+		return;
+
+	bgp_aggregate_toggle_suppressed(aggregate, bgp, p, afi, safi, false);
+}
+
 /* Update an aggregate as routes are added/removed from the BGP table */
 void bgp_aggregate_route(struct bgp *bgp, const struct prefix *p, afi_t afi,
 			 safi_t safi, struct bgp_aggregate *aggregate)
@@ -6323,6 +6514,10 @@ void bgp_aggregate_route(struct bgp *bgp, const struct prefix *p, afi_t afi,
 	    || (bgp->peer_self == NULL))
 		return;
 
+	/* Initialize and test routes for MED difference. */
+	if (aggregate->match_med)
+		bgp_aggregate_test_all_med(aggregate, bgp, p, afi, safi);
+
 	/* ORIGIN attribute: If at least one route among routes that are
 	   aggregated has ORIGIN with the value INCOMPLETE, then the
 	   aggregated route must have the ORIGIN attribute with the value
@@ -6359,8 +6554,14 @@ void bgp_aggregate_route(struct bgp *bgp, const struct prefix *p, afi_t afi,
 			/*
 			 * summary-only aggregate route suppress
 			 * aggregated route announcements.
+			 *
+			 * MED matching:
+			 * Don't create summaries if MED didn't match
+			 * otherwise neither the specific routes and the
+			 * aggregation will be announced.
 			 */
-			if (aggregate->summary_only) {
+			if (aggregate->summary_only
+			    && AGGREGATE_MED_VALID(aggregate)) {
 				(bgp_path_info_extra_get(pi))->suppress++;
 				bgp_path_info_set_flag(dest, pi,
 						       BGP_PATH_ATTR_CHANGED);
@@ -6502,7 +6703,8 @@ void bgp_aggregate_delete(struct bgp *bgp, const struct prefix *p, afi_t afi,
 			if (pi->sub_type == BGP_ROUTE_AGGREGATE)
 				continue;
 
-			if (aggregate->summary_only && pi->extra) {
+			if (aggregate->summary_only && pi->extra
+			    && AGGREGATE_MED_VALID(aggregate)) {
 				pi->extra->suppress--;
 
 				if (pi->extra->suppress == 0) {
@@ -6593,7 +6795,15 @@ static void bgp_add_route_to_aggregate(struct bgp *bgp,
 
 	aggregate->count++;
 
-	if (aggregate->summary_only)
+	/*
+	 * This must be called before `summary` check to avoid
+	 * "suppressing" twice.
+	 */
+	if (aggregate->match_med)
+		bgp_aggregate_med_update(aggregate, bgp, aggr_p, afi, safi,
+					 pinew, true);
+
+	if (aggregate->summary_only && AGGREGATE_MED_VALID(aggregate))
 		(bgp_path_info_extra_get(pinew))->suppress++;
 
 	switch (pinew->attr->origin) {
@@ -6690,9 +6900,8 @@ static void bgp_remove_route_from_aggregate(struct bgp *bgp, afi_t afi,
 	if (pi->sub_type == BGP_ROUTE_AGGREGATE)
 		return;
 
-	if (aggregate->summary_only
+	if (aggregate->summary_only && pi->extra && pi->extra->suppress > 0
-		&& pi->extra
+	    && AGGREGATE_MED_VALID(aggregate)) {
-		&& pi->extra->suppress > 0) {
 		pi->extra->suppress--;
 
 		if (pi->extra->suppress == 0) {
@@ -6702,6 +6911,14 @@ static void bgp_remove_route_from_aggregate(struct bgp *bgp, afi_t afi,
 		}
 	}
 
+	/*
+	 * This must be called after `summary` check to avoid
+	 * "unsuppressing" twice.
+	 */
+	if (aggregate->match_med)
+		bgp_aggregate_med_update(aggregate, bgp, aggr_p, afi, safi, pi,
+					 true);
+
 	if (aggregate->count > 0)
 		aggregate->count--;
 
@@ -6958,7 +7175,7 @@ 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)
+			     uint8_t origin, bool match_med)
 {
 	VTY_DECLVAR_CONTEXT(bgp, bgp);
 	int ret;
@@ -7000,6 +7217,7 @@ static int bgp_aggregate_set(struct vty *vty, const char *prefix_str, afi_t afi,
 	/* Make aggregate address structure. */
 	aggregate = bgp_aggregate_new();
 	aggregate->summary_only = summary_only;
+	aggregate->match_med = match_med;
 
 	/* Network operators MUST NOT locally generate any new
 	 * announcements containing AS_SET or AS_CONFED_SET. If they have
@@ -7045,236 +7263,108 @@ static int bgp_aggregate_set(struct vty *vty, const char *prefix_str, afi_t afi,
 	return CMD_SUCCESS;
 }
 
-DEFUN (aggregate_address,
+DEFPY(aggregate_addressv4, aggregate_addressv4_cmd,
-       aggregate_address_cmd,
+      "[no] aggregate-address <A.B.C.D/M$prefix|A.B.C.D$addr A.B.C.D$mask> {"
-       "aggregate-address A.B.C.D/M [<as-set [summary-only]|summary-only [as-set]>] [route-map WORD] [origin <egp|igp|incomplete>]",
+      "as-set$as_set_s"
-       "Configure BGP aggregate entries\n"
+      "|summary-only$summary_only"
-       "Aggregate prefix\n"
+      "|route-map WORD$rmap_name"
-       "Generate AS set path information\n"
+      "|origin <egp|igp|incomplete>$origin_s"
-       "Filter more specific routes from updates\n"
+      "|matching-MED-only$match_med"
-       "Filter more specific routes from updates\n"
+      "}",
-       "Generate AS set path information\n"
+      NO_STR
-       "Apply route map to aggregate network\n"
+      "Configure BGP aggregate entries\n"
-       "Name of route map\n"
+      "Aggregate prefix\n" "Aggregate address\n" "Aggregate mask\n"
-       "BGP origin code\n"
+      "Generate AS set path information\n"
-       "Remote EGP\n"
+      "Filter more specific routes from updates\n"
-       "Local IGP\n"
+      "Apply route map to aggregate network\n"
-       "Unknown heritage\n")
+      "Route map name\n"
+      "BGP origin code\n"
+      "Remote EGP\n"
+      "Local IGP\n"
+      "Unknown heritage\n"
+      "Only aggregate routes with matching MED\n")
 {
-	int idx = 0;
+	const char *prefix_s = NULL;
-	argv_find(argv, argc, "A.B.C.D/M", &idx);
+	safi_t safi = bgp_node_safi(vty);
-	char *prefix = argv[idx]->arg;
-	char *rmap = NULL;
 	uint8_t origin = BGP_ORIGIN_UNSPECIFIED;
-	int as_set = argv_find(argv, argc, "as-set", &idx) ? AGGREGATE_AS_SET
+	int as_set = AGGREGATE_AS_UNSET;
-							   : AGGREGATE_AS_UNSET;
+	char prefix_buf[PREFIX2STR_BUFFER];
-	idx = 0;
-	int summary_only = argv_find(argv, argc, "summary-only", &idx)
-				   ? AGGREGATE_SUMMARY_ONLY
-				   : 0;
 
-	idx = 0;
+	if (addr_str) {
-	argv_find(argv, argc, "WORD", &idx);
+		if (netmask_str2prefix_str(addr_str, mask_str, prefix_buf)
-	if (idx)
+		    == 0) {
-		rmap = argv[idx]->arg;
+			vty_out(vty, "%% Inconsistent address and mask\n");
+			return CMD_WARNING_CONFIG_FAILED;
+		}
+		prefix_s = prefix_buf;
+	} else
+		prefix_s = prefix_str;
 
-	idx = 0;
+	if (origin_s) {
-	if (argv_find(argv, argc, "origin", &idx)) {
+		if (strcmp(origin_s, "egp") == 0)
-		if (strncmp(argv[idx + 1]->arg, "igp", 2) == 0)
-			origin = BGP_ORIGIN_IGP;
-		if (strncmp(argv[idx + 1]->arg, "egp", 1) == 0)
 			origin = BGP_ORIGIN_EGP;
-		if (strncmp(argv[idx + 1]->arg, "incomplete", 2) == 0)
+		else if (strcmp(origin_s, "igp") == 0)
+			origin = BGP_ORIGIN_IGP;
+		else if (strcmp(origin_s, "incomplete") == 0)
 			origin = BGP_ORIGIN_INCOMPLETE;
 	}
 
-	return bgp_aggregate_set(vty, prefix, AFI_IP, bgp_node_safi(vty), rmap,
+	if (as_set_s)
-				 summary_only, as_set, origin);
+		as_set = AGGREGATE_AS_SET;
+
+	/* Handle configuration removal, otherwise installation. */
+	if (no)
+		return bgp_aggregate_unset(vty, prefix_s, AFI_IP, safi);
+
+	return bgp_aggregate_set(vty, prefix_s, AFI_IP, safi, rmap_name,
+				 summary_only != NULL, as_set, origin,
+				 match_med != NULL);
 }
 
-DEFUN (aggregate_address_mask,
+DEFPY(aggregate_addressv6, aggregate_addressv6_cmd,
-       aggregate_address_mask_cmd,
+      "[no] aggregate-address X:X::X:X/M$prefix {"
-       "aggregate-address A.B.C.D A.B.C.D [<as-set [summary-only]|summary-only [as-set]>] [route-map WORD] [origin <egp|igp|incomplete>]",
+      "as-set$as_set_s"
-       "Configure BGP aggregate entries\n"
+      "|summary-only$summary_only"
-       "Aggregate address\n"
+      "|route-map WORD$rmap_name"
-       "Aggregate mask\n"
+      "|origin <egp|igp|incomplete>$origin_s"
-       "Generate AS set path information\n"
+      "|matching-MED-only$match_med"
-       "Filter more specific routes from updates\n"
+      "}",
-       "Filter more specific routes from updates\n"
+      NO_STR
-       "Generate AS set path information\n"
+      "Configure BGP aggregate entries\n"
-       "Apply route map to aggregate network\n"
+      "Aggregate prefix\n"
-       "Name of route map\n"
+      "Generate AS set path information\n"
-       "BGP origin code\n"
+      "Filter more specific routes from updates\n"
-       "Remote EGP\n"
+      "Apply route map to aggregate network\n"
-       "Local IGP\n"
+      "Route map name\n"
-       "Unknown heritage\n")
+      "BGP origin code\n"
+      "Remote EGP\n"
+      "Local IGP\n"
+      "Unknown heritage\n"
+      "Only aggregate routes with matching MED\n")
 {
-	int idx = 0;
-	argv_find(argv, argc, "A.B.C.D", &idx);
-	char *prefix = argv[idx]->arg;
-	char *mask = argv[idx + 1]->arg;
-	bool rmap_found;
-	char *rmap = NULL;
 	uint8_t origin = BGP_ORIGIN_UNSPECIFIED;
-	int as_set = argv_find(argv, argc, "as-set", &idx) ? AGGREGATE_AS_SET
+	int as_set = AGGREGATE_AS_UNSET;
-							   : AGGREGATE_AS_UNSET;
-	idx = 0;
-	int summary_only = argv_find(argv, argc, "summary-only", &idx)
-				   ? AGGREGATE_SUMMARY_ONLY
-				   : 0;
 
-	rmap_found = argv_find(argv, argc, "WORD", &idx);
+	if (origin_s) {
-	if (rmap_found)
+		if (strcmp(origin_s, "egp") == 0)
-		rmap = argv[idx]->arg;
-
-	char prefix_str[BUFSIZ];
-	int ret = netmask_str2prefix_str(prefix, mask, prefix_str);
-
-	if (!ret) {
-		vty_out(vty, "%% Inconsistent address and mask\n");
-		return CMD_WARNING_CONFIG_FAILED;
-	}
-
-	idx = 0;
-	if (argv_find(argv, argc, "origin", &idx)) {
-		if (strncmp(argv[idx + 1]->arg, "igp", 2) == 0)
-			origin = BGP_ORIGIN_IGP;
-		if (strncmp(argv[idx + 1]->arg, "egp", 1) == 0)
 			origin = BGP_ORIGIN_EGP;
-		if (strncmp(argv[idx + 1]->arg, "incomplete", 2) == 0)
+		else if (strcmp(origin_s, "igp") == 0)
+			origin = BGP_ORIGIN_IGP;
+		else if (strcmp(origin_s, "incomplete") == 0)
 			origin = BGP_ORIGIN_INCOMPLETE;
 	}
 
-	return bgp_aggregate_set(vty, prefix_str, AFI_IP, bgp_node_safi(vty),
+	if (as_set_s)
-				 rmap, summary_only, as_set, origin);
+		as_set = AGGREGATE_AS_SET;
-}
 
-DEFUN (no_aggregate_address,
+	/* Handle configuration removal, otherwise installation. */
-       no_aggregate_address_cmd,
+	if (no)
-       "no aggregate-address A.B.C.D/M [<as-set [summary-only]|summary-only [as-set]>] [route-map WORD] [origin <egp|igp|incomplete>]",
+		return bgp_aggregate_unset(vty, prefix_str, AFI_IP6,
-       NO_STR
+					   SAFI_UNICAST);
-       "Configure BGP aggregate entries\n"
-       "Aggregate prefix\n"
-       "Generate AS set path information\n"
-       "Filter more specific routes from updates\n"
-       "Filter more specific routes from updates\n"
-       "Generate AS set path information\n"
-       "Apply route map to aggregate network\n"
-       "Name of route map\n"
-       "BGP origin code\n"
-       "Remote EGP\n"
-       "Local IGP\n"
-       "Unknown heritage\n")
-{
-	int idx = 0;
-	argv_find(argv, argc, "A.B.C.D/M", &idx);
-	char *prefix = argv[idx]->arg;
-	return bgp_aggregate_unset(vty, prefix, AFI_IP, bgp_node_safi(vty));
-}
 
-DEFUN (no_aggregate_address_mask,
+	return bgp_aggregate_set(vty, prefix_str, AFI_IP6, SAFI_UNICAST,
-       no_aggregate_address_mask_cmd,
+				 rmap_name, summary_only != NULL, as_set,
-       "no aggregate-address A.B.C.D A.B.C.D [<as-set [summary-only]|summary-only [as-set]>] [route-map WORD] [origin <egp|igp|incomplete>]",
+				 origin, match_med != NULL);
-       NO_STR
-       "Configure BGP aggregate entries\n"
-       "Aggregate address\n"
-       "Aggregate mask\n"
-       "Generate AS set path information\n"
-       "Filter more specific routes from updates\n"
-       "Filter more specific routes from updates\n"
-       "Generate AS set path information\n"
-       "Apply route map to aggregate network\n"
-       "Name of route map\n"
-       "BGP origin code\n"
-       "Remote EGP\n"
-       "Local IGP\n"
-       "Unknown heritage\n")
-{
-	int idx = 0;
-	argv_find(argv, argc, "A.B.C.D", &idx);
-	char *prefix = argv[idx]->arg;
-	char *mask = argv[idx + 1]->arg;
-
-	char prefix_str[BUFSIZ];
-	int ret = netmask_str2prefix_str(prefix, mask, prefix_str);
-
-	if (!ret) {
-		vty_out(vty, "%% Inconsistent address and mask\n");
-		return CMD_WARNING_CONFIG_FAILED;
-	}
-
-	return bgp_aggregate_unset(vty, prefix_str, AFI_IP, bgp_node_safi(vty));
-}
-
-DEFUN (ipv6_aggregate_address,
-       ipv6_aggregate_address_cmd,
-       "aggregate-address X:X::X:X/M [<as-set [summary-only]|summary-only [as-set]>] [route-map WORD] [origin <egp|igp|incomplete>]",
-       "Configure BGP aggregate entries\n"
-       "Aggregate prefix\n"
-       "Generate AS set path information\n"
-       "Filter more specific routes from updates\n"
-       "Filter more specific routes from updates\n"
-       "Generate AS set path information\n"
-       "Apply route map to aggregate network\n"
-       "Name of route map\n"
-       "BGP origin code\n"
-       "Remote EGP\n"
-       "Local IGP\n"
-       "Unknown heritage\n")
-{
-	int idx = 0;
-	argv_find(argv, argc, "X:X::X:X/M", &idx);
-	char *prefix = argv[idx]->arg;
-	char *rmap = NULL;
-	bool rmap_found;
-	uint8_t origin = BGP_ORIGIN_UNSPECIFIED;
-	int as_set = argv_find(argv, argc, "as-set", &idx) ? AGGREGATE_AS_SET
-							   : AGGREGATE_AS_UNSET;
-
-	idx = 0;
-	int sum_only = argv_find(argv, argc, "summary-only", &idx)
-			       ? AGGREGATE_SUMMARY_ONLY
-			       : 0;
-
-	rmap_found = argv_find(argv, argc, "WORD", &idx);
-	if (rmap_found)
-		rmap = argv[idx]->arg;
-
-	idx = 0;
-	if (argv_find(argv, argc, "origin", &idx)) {
-		if (strncmp(argv[idx + 1]->arg, "igp", 2) == 0)
-			origin = BGP_ORIGIN_IGP;
-		if (strncmp(argv[idx + 1]->arg, "egp", 1) == 0)
-			origin = BGP_ORIGIN_EGP;
-		if (strncmp(argv[idx + 1]->arg, "incomplete", 2) == 0)
-			origin = BGP_ORIGIN_INCOMPLETE;
-	}
-
-	return bgp_aggregate_set(vty, prefix, AFI_IP6, SAFI_UNICAST, rmap,
-				 sum_only, as_set, origin);
-}
-
-DEFUN (no_ipv6_aggregate_address,
-       no_ipv6_aggregate_address_cmd,
-       "no aggregate-address X:X::X:X/M [<as-set [summary-only]|summary-only [as-set]>] [route-map WORD] [origin <egp|igp|incomplete>]",
-       NO_STR
-       "Configure BGP aggregate entries\n"
-       "Aggregate prefix\n"
-       "Generate AS set path information\n"
-       "Filter more specific routes from updates\n"
-       "Filter more specific routes from updates\n"
-       "Generate AS set path information\n"
-       "Apply route map to aggregate network\n"
-       "Name of route map\n"
-       "BGP origin code\n"
-       "Remote EGP\n"
-       "Local IGP\n"
-       "Unknown heritage\n")
-{
-	int idx = 0;
-	argv_find(argv, argc, "X:X::X:X/M", &idx);
-	char *prefix = argv[idx]->arg;
-	return bgp_aggregate_unset(vty, prefix, AFI_IP6, SAFI_UNICAST);
 }
 
 /* Redistribute route treatment. */
@@ -13880,6 +13970,9 @@ void bgp_config_write_network(struct vty *vty, struct bgp *bgp, afi_t afi,
 			vty_out(vty, " origin %s",
 				bgp_origin2str(bgp_aggregate->origin));
 
+		if (bgp_aggregate->match_med)
+			vty_out(vty, " matching-MED-only");
+
 		vty_out(vty, "\n");
 	}
 }
@@ -13929,36 +14022,24 @@ void bgp_route_init(void)
 	install_element(BGP_NODE, &bgp_network_cmd);
 	install_element(BGP_NODE, &no_bgp_table_map_cmd);
 
-	install_element(BGP_NODE, &aggregate_address_cmd);
+	install_element(BGP_NODE, &aggregate_addressv4_cmd);
-	install_element(BGP_NODE, &aggregate_address_mask_cmd);
-	install_element(BGP_NODE, &no_aggregate_address_cmd);
-	install_element(BGP_NODE, &no_aggregate_address_mask_cmd);
 
 	/* IPv4 unicast configuration.  */
 	install_element(BGP_IPV4_NODE, &bgp_table_map_cmd);
 	install_element(BGP_IPV4_NODE, &bgp_network_cmd);
 	install_element(BGP_IPV4_NODE, &no_bgp_table_map_cmd);
 
-	install_element(BGP_IPV4_NODE, &aggregate_address_cmd);
+	install_element(BGP_IPV4_NODE, &aggregate_addressv4_cmd);
-	install_element(BGP_IPV4_NODE, &aggregate_address_mask_cmd);
-	install_element(BGP_IPV4_NODE, &no_aggregate_address_cmd);
-	install_element(BGP_IPV4_NODE, &no_aggregate_address_mask_cmd);
 
 	/* IPv4 multicast configuration.  */
 	install_element(BGP_IPV4M_NODE, &bgp_table_map_cmd);
 	install_element(BGP_IPV4M_NODE, &bgp_network_cmd);
 	install_element(BGP_IPV4M_NODE, &no_bgp_table_map_cmd);
-	install_element(BGP_IPV4M_NODE, &aggregate_address_cmd);
+	install_element(BGP_IPV4M_NODE, &aggregate_addressv4_cmd);
-	install_element(BGP_IPV4M_NODE, &aggregate_address_mask_cmd);
-	install_element(BGP_IPV4M_NODE, &no_aggregate_address_cmd);
-	install_element(BGP_IPV4M_NODE, &no_aggregate_address_mask_cmd);
 
 	/* IPv4 labeled-unicast configuration.  */
 	install_element(BGP_IPV4L_NODE, &bgp_network_cmd);
-	install_element(BGP_IPV4L_NODE, &aggregate_address_cmd);
+	install_element(BGP_IPV4L_NODE, &aggregate_addressv4_cmd);
-	install_element(BGP_IPV4L_NODE, &aggregate_address_mask_cmd);
-	install_element(BGP_IPV4L_NODE, &no_aggregate_address_cmd);
-	install_element(BGP_IPV4L_NODE, &no_aggregate_address_mask_cmd);
 
 	install_element(VIEW_NODE, &show_ip_bgp_instance_all_cmd);
 	install_element(VIEW_NODE, &show_ip_bgp_cmd);
@@ -14003,15 +14084,13 @@ void bgp_route_init(void)
 	install_element(BGP_IPV6_NODE, &ipv6_bgp_network_cmd);
 	install_element(BGP_IPV6_NODE, &no_bgp_table_map_cmd);
 
-	install_element(BGP_IPV6_NODE, &ipv6_aggregate_address_cmd);
+	install_element(BGP_IPV6_NODE, &aggregate_addressv6_cmd);
-	install_element(BGP_IPV6_NODE, &no_ipv6_aggregate_address_cmd);
 
 	install_element(BGP_IPV6M_NODE, &ipv6_bgp_network_cmd);
 
 	/* IPv6 labeled unicast address family. */
 	install_element(BGP_IPV6L_NODE, &ipv6_bgp_network_cmd);
-	install_element(BGP_IPV6L_NODE, &ipv6_aggregate_address_cmd);
+	install_element(BGP_IPV6L_NODE, &aggregate_addressv6_cmd);
-	install_element(BGP_IPV6L_NODE, &no_ipv6_aggregate_address_cmd);
 
 	install_element(BGP_NODE, &bgp_distance_cmd);
 	install_element(BGP_NODE, &no_bgp_distance_cmd);

bgpd/bgp_route.h

@@ -379,6 +379,25 @@ struct bgp_aggregate {
 
 	/* SAFI configuration. */
 	safi_t safi;
+
+	/** Match only equal MED. */
+	bool match_med;
+	/* MED matching state. */
+	/** Did we get the first MED value? */
+	bool med_initialized;
+	/** Are there MED mismatches? */
+	bool med_mismatched;
+	/** MED value found in current group. */
+	uint32_t med_matched_value;
+
+	/**
+	 * Test if aggregated address MED of all route match, otherwise
+	 * returns `false`. This macro will also return `true` if MED
+	 * matching is disabled.
+	 */
+#define AGGREGATE_MED_VALID(aggregate)                                         \
+	(((aggregate)->match_med && !(aggregate)->med_mismatched)              \
+	 || !(aggregate)->match_med)
 };
 
 #define BGP_NEXTHOP_AFI_FROM_NHLEN(nhlen)                                      \

doc/user/bgp.rst

@@ -999,6 +999,12 @@ Route Aggregation-IPv4 Address Family
    This command specifies an aggregate address. Aggregated routes will
    not be announced.
 
+.. index:: aggregate-address A.B.C.D/M matching-MED-only
+.. clicmd:: aggregate-address A.B.C.D/M matching-MED-only
+
+   Configure the aggregated address to only be created when the routes MED
+   match, otherwise no aggregated route will be created.
+
 .. index:: no aggregate-address A.B.C.D/M
 .. clicmd:: no aggregate-address A.B.C.D/M
 
@@ -1051,6 +1057,13 @@ Route Aggregation-IPv6 Address Family
    This command specifies an aggregate address. Aggregated routes will
    not be announced.
 
+.. index:: aggregate-address X:X::X:X/M matching-MED-only
+.. clicmd:: aggregate-address X:X::X:X/M matching-MED-only
+
+   Configure the aggregated address to only be created when the routes MED
+   match, otherwise no aggregated route will be created.
+
+
 .. index:: no aggregate-address X:X::X:X/M
 .. clicmd:: no aggregate-address X:X::X:X/M
 

tests/topotests/bgp_aggregate_address_topo1/exabgp.env

@@ -0,0 +1,53 @@
+[exabgp.api]
+encoder = text
+highres = false
+respawn = false
+socket = ''
+
+[exabgp.bgp]
+openwait = 60
+
+[exabgp.cache]
+attributes = true
+nexthops = true
+
+[exabgp.daemon]
+daemonize = true
+pid = '/var/run/exabgp/exabgp.pid'
+user = 'exabgp'
+##daemonize = false
+
+[exabgp.log]
+all = false
+configuration = true
+daemon = true
+destination = '/var/log/exabgp.log'
+enable = true
+level = INFO
+message = false
+network = true
+packets = false
+parser = false
+processes = true
+reactor = true
+rib = false
+routes = false
+short = false
+timers = false
+
+[exabgp.pdb]
+enable = false
+
+[exabgp.profile]
+enable = false
+file = ''
+
+[exabgp.reactor]
+speed = 1.0
+
+[exabgp.tcp]
+acl = false
+bind = ''
+delay = 0
+once = false
+port = 179

tests/topotests/bgp_aggregate_address_topo1/peer1/exabgp.cfg

@@ -0,0 +1,17 @@
+neighbor 10.0.0.1 {
+  router-id 10.254.254.3;
+  local-address 10.0.0.2;
+  local-as 65001;
+  peer-as 65000;
+  static {
+    route 10.254.254.3/32 next-hop 10.0.0.2;
+
+    route 192.168.0.1/32 next-hop 10.0.0.2 med 10;
+    route 192.168.0.2/32 next-hop 10.0.0.2 med 10;
+    route 192.168.0.3/32 next-hop 10.0.0.2 med 10;
+
+    route 192.168.1.1/32 next-hop 10.0.0.2 med 10;
+    route 192.168.1.2/32 next-hop 10.0.0.2 med 10;
+    route 192.168.1.3/32 next-hop 10.0.0.2 med 20;
+  }
+}

tests/topotests/bgp_aggregate_address_topo1/r1/bgpd.conf

@@ -0,0 +1,12 @@
+router bgp 65000
+  no bgp ebgp-requires-policy
+  neighbor 10.0.0.2 remote-as 65001
+  neighbor 10.0.0.2 timers 3 10
+  neighbor 10.0.1.2 remote-as internal
+  neighbor 10.0.1.2 timers 3 10
+  address-family ipv4 unicast
+   redistribute connected
+   aggregate-address 192.168.0.0/24 matching-MED-only
+   aggregate-address 192.168.1.0/24 matching-MED-only
+  exit-address-family
+!

tests/topotests/bgp_aggregate_address_topo1/r1/zebra.conf

@@ -0,0 +1,13 @@
+!
+interface lo
+ ip address 10.254.254.1/32
+!
+interface r1-eth0
+ ip address 10.0.0.1/24
+!
+interface r1-eth1
+ ip address 10.0.1.1/24
+!
+ip forwarding
+ipv6 forwarding
+!

tests/topotests/bgp_aggregate_address_topo1/r2/bgpd.conf

@@ -0,0 +1,7 @@
+router bgp 65000
+  neighbor 10.0.1.1 remote-as internal
+  neighbor 10.0.1.1 timers 3 10
+  address-family ipv4 unicast
+    redistribute connected
+  exit-address-family
+!

tests/topotests/bgp_aggregate_address_topo1/r2/zebra.conf

@@ -0,0 +1,10 @@
+!
+interface lo
+ ip address 10.254.254.2/32
+!
+interface r2-eth0
+ ip address 10.0.1.2/24
+!
+ip forwarding
+ipv6 forwarding
+!

tests/topotests/bgp_aggregate_address_topo1/test_bgp_aggregate_address_topo1.py

@@ -0,0 +1,197 @@
+#!/usr/bin/env python
+
+#
+# test_bgp_aggregate_address_topo1.py
+# Part of NetDEF Topology Tests
+#
+# Copyright (c) 2020 by
+# Network Device Education Foundation, Inc. ("NetDEF")
+#
+# Permission to use, copy, modify, and/or distribute this software
+# for any purpose with or without fee is hereby granted, provided
+# that the above copyright notice and this permission notice appear
+# in all copies.
+#
+# THE SOFTWARE IS PROVIDED "AS IS" AND NETDEF DISCLAIMS ALL WARRANTIES
+# WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+# MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL NETDEF BE LIABLE FOR
+# ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY
+# DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS,
+# WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS
+# ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+# OF THIS SOFTWARE.
+#
+
+"""
+Test BGP aggregate address features.
+"""
+
+import os
+import sys
+import json
+import time
+import pytest
+import functools
+
+CWD = os.path.dirname(os.path.realpath(__file__))
+sys.path.append(os.path.join(CWD, "../"))
+
+# pylint: disable=C0413
+from lib import topotest
+from lib.topogen import Topogen, TopoRouter, get_topogen
+from lib.topolog import logger
+from mininet.topo import Topo
+
+
+class BgpAggregateAddressTopo1(Topo):
+    def build(self, *_args, **_opts):
+        tgen = get_topogen(self)
+
+        r1 = tgen.add_router('r1')
+        r2 = tgen.add_router('r2')
+        peer1 = tgen.add_exabgp_peer('peer1', ip='10.0.0.2',
+                                     defaultRoute='via 10.0.0.1')
+
+        switch = tgen.add_switch('s1')
+        switch.add_link(r1)
+        switch.add_link(peer1)
+
+        switch = tgen.add_switch('s2')
+        switch.add_link(r1)
+        switch.add_link(r2)
+
+
+def setup_module(mod):
+    tgen = Topogen(BgpAggregateAddressTopo1, mod.__name__)
+    tgen.start_topology()
+
+    router = tgen.gears['r1']
+    router.load_config(TopoRouter.RD_ZEBRA, os.path.join(CWD, "r1/zebra.conf"))
+    router.load_config(TopoRouter.RD_BGP, os.path.join(CWD, "r1/bgpd.conf"))
+    router.start()
+
+    router = tgen.gears['r2']
+    router.load_config(TopoRouter.RD_ZEBRA, os.path.join(CWD, "r2/zebra.conf"))
+    router.load_config(TopoRouter.RD_BGP, os.path.join(CWD, "r2/bgpd.conf"))
+    router.start()
+
+    peer = tgen.gears['peer1']
+    peer.start(os.path.join(CWD, "peer1"), os.path.join(CWD, "exabgp.env"))
+
+
+def teardown_module(mod):
+    tgen = get_topogen()
+    tgen.stop_topology()
+
+
+def test_expect_convergence():
+    "Test that BGP protocol converged."
+
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    logger.info("waiting for protocols to converge")
+    def expect_loopback_route(router, iptype, route, proto):
+        "Wait until route is present on RIB for protocol."
+        logger.info('waiting route {} in {}'.format(route, router))
+        test_func = functools.partial(
+            topotest.router_json_cmp,
+            tgen.gears[router],
+            'show {} route json'.format(iptype),
+            { route: [{ 'protocol': proto }] }
+        )
+        _, result = topotest.run_and_expect(test_func, None, count=130, wait=1)
+        assertmsg = '"{}" BGP convergence failure'.format(router)
+        assert result is None, assertmsg
+
+    expect_loopback_route('r2', 'ip', '10.254.254.1/32', 'bgp')
+    expect_loopback_route('r2', 'ip', '10.254.254.3/32', 'bgp')
+
+
+def test_bgp_aggregate_address_matching_med_only():
+    "Test that the command matching-MED-only works."
+
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    routes_expected = {
+        # All MED matches, aggregation must exist.
+        "192.168.0.0/24": [{"protocol": "bgp", "metric": 0}],
+        "192.168.0.1/32": [{"protocol": "bgp", "metric": 10}],
+        "192.168.0.2/32": [{"protocol": "bgp", "metric": 10}],
+        "192.168.0.3/32": [{"protocol": "bgp", "metric": 10}],
+
+        # Non matching MED: aggregation must not exist.
+        "192.168.1.0/24": None,
+        "192.168.1.1/32": [{"protocol": "bgp", "metric": 10}],
+        "192.168.1.2/32": [{"protocol": "bgp", "metric": 10}],
+        "192.168.1.3/32": [{"protocol": "bgp", "metric": 20}]
+    }
+
+    test_func = functools.partial(
+        topotest.router_json_cmp,
+        tgen.gears['r2'],
+        'show ip route json',
+        routes_expected
+    )
+    _, result = topotest.run_and_expect(test_func, None, count=20, wait=1)
+    assertmsg = '"r2" BGP convergence failure'
+    assert result is None, assertmsg
+
+
+def test_bgp_aggregate_address_match_and_supress():
+    "Test that the command matching-MED-only with suppression works."
+
+    tgen = get_topogen()
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    tgen.gears['r1'].vtysh_multicmd("""
+configure terminal
+router bgp 65000
+address-family ipv4 unicast
+no aggregate-address 192.168.0.0/24 matching-MED-only
+no aggregate-address 192.168.1.0/24 matching-MED-only
+aggregate-address 192.168.0.0/24 matching-MED-only summary-only
+aggregate-address 192.168.1.0/24 matching-MED-only summary-only
+""")
+
+    routes_expected = {
+        # All MED matches, aggregation must exist.
+        "192.168.0.0/24": [{"protocol": "bgp", "metric": 0}],
+        "192.168.0.1/32": None,
+        "192.168.0.2/32": None,
+        "192.168.0.3/32": None,
+
+        # Non matching MED: aggregation must not exist.
+        "192.168.1.0/24": None,
+        "192.168.1.1/32": [{"protocol": "bgp", "metric": 10}],
+        "192.168.1.2/32": [{"protocol": "bgp", "metric": 10}],
+        "192.168.1.3/32": [{"protocol": "bgp", "metric": 20}]
+    }
+
+    test_func = functools.partial(
+        topotest.router_json_cmp,
+        tgen.gears['r2'],
+        'show ip route json',
+        routes_expected
+    )
+    _, result = topotest.run_and_expect(test_func, None, count=120, wait=1)
+    assertmsg = '"r2" BGP convergence failure'
+    assert result is None, assertmsg
+
+
+def test_memory_leak():
+    "Run the memory leak test and report results."
+    tgen = get_topogen()
+    if not tgen.is_memleak_enabled():
+        pytest.skip("Memory leak test/report is disabled")
+
+    tgen.report_memory_leaks()
+
+
+if __name__ == "__main__":
+    args = ["-s"] + sys.argv[1:]
+    sys.exit(pytest.main(args))