diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 15c1df8473..43ba4ceef6 100644 --- a/bgpd/bgp_route.c +++ b/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 - && pi->extra - && pi->extra->suppress > 0) { + if (aggregate->summary_only && pi->extra && pi->extra->suppress > 0 + && AGGREGATE_MED_VALID(aggregate)) { 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, - aggregate_address_cmd, - "aggregate-address A.B.C.D/M [] [route-map WORD] [origin ]", - "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") +DEFPY(aggregate_addressv4, aggregate_addressv4_cmd, + "[no] aggregate-address {" + "as-set$as_set_s" + "|summary-only$summary_only" + "|route-map WORD$rmap_name" + "|origin $origin_s" + "|matching-MED-only$match_med" + "}", + NO_STR + "Configure BGP aggregate entries\n" + "Aggregate prefix\n" "Aggregate address\n" "Aggregate mask\n" + "Generate AS set path information\n" + "Filter more specific routes from updates\n" + "Apply route map to aggregate network\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; - argv_find(argv, argc, "A.B.C.D/M", &idx); - char *prefix = argv[idx]->arg; - char *rmap = NULL; + const char *prefix_s = NULL; + safi_t safi = bgp_node_safi(vty); 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 summary_only = argv_find(argv, argc, "summary-only", &idx) - ? AGGREGATE_SUMMARY_ONLY - : 0; + int as_set = AGGREGATE_AS_UNSET; + char prefix_buf[PREFIX2STR_BUFFER]; - idx = 0; - argv_find(argv, argc, "WORD", &idx); - if (idx) - rmap = argv[idx]->arg; + if (addr_str) { + if (netmask_str2prefix_str(addr_str, mask_str, prefix_buf) + == 0) { + 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 (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) + if (origin_s) { + if (strcmp(origin_s, "egp") == 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, - summary_only, as_set, origin); + if (as_set_s) + 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, - aggregate_address_mask_cmd, - "aggregate-address A.B.C.D A.B.C.D [] [route-map WORD] [origin ]", - "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") +DEFPY(aggregate_addressv6, aggregate_addressv6_cmd, + "[no] aggregate-address X:X::X:X/M$prefix {" + "as-set$as_set_s" + "|summary-only$summary_only" + "|route-map WORD$rmap_name" + "|origin $origin_s" + "|matching-MED-only$match_med" + "}", + NO_STR + "Configure BGP aggregate entries\n" + "Aggregate prefix\n" + "Generate AS set path information\n" + "Filter more specific routes from updates\n" + "Apply route map to aggregate network\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; - 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 - : AGGREGATE_AS_UNSET; - idx = 0; - int summary_only = argv_find(argv, argc, "summary-only", &idx) - ? AGGREGATE_SUMMARY_ONLY - : 0; + int as_set = AGGREGATE_AS_UNSET; - rmap_found = argv_find(argv, argc, "WORD", &idx); - if (rmap_found) - 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) + if (origin_s) { + if (strcmp(origin_s, "egp") == 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), - rmap, summary_only, as_set, origin); -} + if (as_set_s) + as_set = AGGREGATE_AS_SET; -DEFUN (no_aggregate_address, - no_aggregate_address_cmd, - "no aggregate-address A.B.C.D/M [] [route-map WORD] [origin ]", - 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, "A.B.C.D/M", &idx); - char *prefix = argv[idx]->arg; - return bgp_aggregate_unset(vty, prefix, AFI_IP, bgp_node_safi(vty)); -} + /* Handle configuration removal, otherwise installation. */ + if (no) + return bgp_aggregate_unset(vty, prefix_str, AFI_IP6, + SAFI_UNICAST); -DEFUN (no_aggregate_address_mask, - no_aggregate_address_mask_cmd, - "no aggregate-address A.B.C.D A.B.C.D [] [route-map WORD] [origin ]", - 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 [] [route-map WORD] [origin ]", - "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 [] [route-map WORD] [origin ]", - 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); + return bgp_aggregate_set(vty, prefix_str, AFI_IP6, SAFI_UNICAST, + rmap_name, summary_only != NULL, as_set, + origin, match_med != NULL); } /* 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_address_mask_cmd); - install_element(BGP_NODE, &no_aggregate_address_cmd); - install_element(BGP_NODE, &no_aggregate_address_mask_cmd); + install_element(BGP_NODE, &aggregate_addressv4_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_address_mask_cmd); - install_element(BGP_IPV4_NODE, &no_aggregate_address_cmd); - install_element(BGP_IPV4_NODE, &no_aggregate_address_mask_cmd); + install_element(BGP_IPV4_NODE, &aggregate_addressv4_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_address_mask_cmd); - install_element(BGP_IPV4M_NODE, &no_aggregate_address_cmd); - install_element(BGP_IPV4M_NODE, &no_aggregate_address_mask_cmd); + install_element(BGP_IPV4M_NODE, &aggregate_addressv4_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_address_mask_cmd); - install_element(BGP_IPV4L_NODE, &no_aggregate_address_cmd); - install_element(BGP_IPV4L_NODE, &no_aggregate_address_mask_cmd); + install_element(BGP_IPV4L_NODE, &aggregate_addressv4_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, &no_ipv6_aggregate_address_cmd); + install_element(BGP_IPV6_NODE, &aggregate_addressv6_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, &no_ipv6_aggregate_address_cmd); + install_element(BGP_IPV6L_NODE, &aggregate_addressv6_cmd); install_element(BGP_NODE, &bgp_distance_cmd); install_element(BGP_NODE, &no_bgp_distance_cmd); diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index 3407884897..962a086081 100644 --- a/bgpd/bgp_route.h +++ b/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) \ diff --git a/doc/user/bgp.rst b/doc/user/bgp.rst index 10eaaee9fb..fb0ca10da4 100644 --- a/doc/user/bgp.rst +++ b/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 diff --git a/tests/topotests/bgp_aggregate_address_topo1/__init__.py b/tests/topotests/bgp_aggregate_address_topo1/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/topotests/bgp_aggregate_address_topo1/exabgp.env b/tests/topotests/bgp_aggregate_address_topo1/exabgp.env new file mode 100644 index 0000000000..28e642360a --- /dev/null +++ b/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 diff --git a/tests/topotests/bgp_aggregate_address_topo1/peer1/exabgp.cfg b/tests/topotests/bgp_aggregate_address_topo1/peer1/exabgp.cfg new file mode 100644 index 0000000000..277c6603ad --- /dev/null +++ b/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; + } +} diff --git a/tests/topotests/bgp_aggregate_address_topo1/r1/bgpd.conf b/tests/topotests/bgp_aggregate_address_topo1/r1/bgpd.conf new file mode 100644 index 0000000000..4e1406177d --- /dev/null +++ b/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 +! diff --git a/tests/topotests/bgp_aggregate_address_topo1/r1/zebra.conf b/tests/topotests/bgp_aggregate_address_topo1/r1/zebra.conf new file mode 100644 index 0000000000..931c73d8fa --- /dev/null +++ b/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 +! diff --git a/tests/topotests/bgp_aggregate_address_topo1/r2/bgpd.conf b/tests/topotests/bgp_aggregate_address_topo1/r2/bgpd.conf new file mode 100644 index 0000000000..acacd86526 --- /dev/null +++ b/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 +! diff --git a/tests/topotests/bgp_aggregate_address_topo1/r2/zebra.conf b/tests/topotests/bgp_aggregate_address_topo1/r2/zebra.conf new file mode 100644 index 0000000000..38e0c44299 --- /dev/null +++ b/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 +! diff --git a/tests/topotests/bgp_aggregate_address_topo1/test_bgp_aggregate_address_topo1.py b/tests/topotests/bgp_aggregate_address_topo1/test_bgp_aggregate_address_topo1.py new file mode 100644 index 0000000000..25760501b4 --- /dev/null +++ b/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))