diff options
27 files changed, 736 insertions, 281 deletions
diff --git a/bgpd/bgp_nb_config.c b/bgpd/bgp_nb_config.c index 8330ff57c4..ba7c5714bb 100644 --- a/bgpd/bgp_nb_config.c +++ b/bgpd/bgp_nb_config.c @@ -288,9 +288,6 @@ int bgp_global_confederation_identifier_modify(struct nb_cb_modify_args *args) break; } - as = yang_dnode_get_uint32(args->dnode, NULL); - - return NB_OK; } @@ -1381,7 +1378,6 @@ int bgp_global_global_config_timers_hold_time_modify( switch (args->event) { case NB_EV_VALIDATE: - keepalive = yang_dnode_get_uint16(args->dnode, "../keepalive"); holdtime = yang_dnode_get_uint16(args->dnode, NULL); /* Holdtime value check. */ if (holdtime < 3 && holdtime != 0) { @@ -1423,7 +1419,6 @@ int bgp_global_global_config_timers_keepalive_modify( switch (args->event) { case NB_EV_VALIDATE: - keepalive = yang_dnode_get_uint16(args->dnode, NULL); holdtime = yang_dnode_get_uint16(args->dnode, "../hold-time"); /* Holdtime value check. */ if (holdtime < 3 && holdtime != 0) { 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 [<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, "A.B.C.D/M", &idx); - char *prefix = argv[idx]->arg; - char *rmap = NULL; +DEFPY(aggregate_addressv4, aggregate_addressv4_cmd, + "[no] aggregate-address <A.B.C.D/M$prefix|A.B.C.D$addr A.B.C.D$mask> {" + "as-set$as_set_s" + "|summary-only$summary_only" + "|route-map WORD$rmap_name" + "|origin <egp|igp|incomplete>$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") +{ + 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; - - idx = 0; - argv_find(argv, argc, "WORD", &idx); - if (idx) - 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_IP, bgp_node_safi(vty), rmap, - summary_only, as_set, origin); -} + int as_set = AGGREGATE_AS_UNSET; + char prefix_buf[PREFIX2STR_BUFFER]; -DEFUN (aggregate_address_mask, - aggregate_address_mask_cmd, - "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>]", - "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; - 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; - - 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; - } + 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_str, AFI_IP, bgp_node_safi(vty), - rmap, summary_only, as_set, origin); -} - -DEFUN (no_aggregate_address, - no_aggregate_address_cmd, - "no aggregate-address A.B.C.D/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, "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, - no_aggregate_address_mask_cmd, - "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>]", - 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") + 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); +} + +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 <egp|igp|incomplete>$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; - - 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; + int as_set = AGGREGATE_AS_UNSET; - 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) + 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_IP6, SAFI_UNICAST, rmap, - sum_only, as_set, origin); -} + if (as_set_s) + as_set = AGGREGATE_AS_SET; -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); + /* Handle configuration removal, otherwise installation. */ + if (no) + return bgp_aggregate_unset(vty, prefix_str, 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/lib/northbound.c b/lib/northbound.c index 5816c162d8..c99f993ea5 100644 --- a/lib/northbound.c +++ b/lib/northbound.c @@ -1142,7 +1142,8 @@ const void *nb_callback_lookup_entry(const struct nb_node *nb_node, } int nb_callback_rpc(const struct nb_node *nb_node, const char *xpath, - const struct list *input, struct list *output) + const struct list *input, struct list *output, char *errmsg, + size_t errmsg_len) { struct nb_cb_rpc_args args = {}; @@ -1151,6 +1152,8 @@ int nb_callback_rpc(const struct nb_node *nb_node, const char *xpath, args.xpath = xpath; args.input = input; args.output = output; + args.errmsg = errmsg; + args.errmsg_len = errmsg_len; return nb_node->cbs.rpc(&args); } diff --git a/lib/northbound.h b/lib/northbound.h index 01dc528aa4..16f19b3d5b 100644 --- a/lib/northbound.h +++ b/lib/northbound.h @@ -258,6 +258,12 @@ struct nb_cb_rpc_args { /* List of output parameters to be populated by the callback. */ struct list *output; + + /* Buffer to store human-readable error message in case of error. */ + char *errmsg; + + /* Size of errmsg. */ + size_t errmsg_len; }; /* @@ -689,7 +695,8 @@ extern const void *nb_callback_lookup_entry(const struct nb_node *nb_node, const void *parent_list_entry, const struct yang_list_keys *keys); extern int nb_callback_rpc(const struct nb_node *nb_node, const char *xpath, - const struct list *input, struct list *output); + const struct list *input, struct list *output, + char *errmsg, size_t errmsg_len); /* * Create a northbound node for all YANG schema nodes. diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c index 6ce520149a..a7f3a1b305 100644 --- a/lib/northbound_cli.c +++ b/lib/northbound_cli.c @@ -284,10 +284,12 @@ int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, ...) return CMD_SUCCESS; } -int nb_cli_rpc(const char *xpath, struct list *input, struct list *output) +int nb_cli_rpc(struct vty *vty, const char *xpath, struct list *input, + struct list *output) { struct nb_node *nb_node; int ret; + char errmsg[BUFSIZ] = {0}; nb_node = nb_node_find(xpath); if (!nb_node) { @@ -296,11 +298,14 @@ int nb_cli_rpc(const char *xpath, struct list *input, struct list *output) return CMD_WARNING; } - ret = nb_callback_rpc(nb_node, xpath, input, output); + ret = nb_callback_rpc(nb_node, xpath, input, output, errmsg, + sizeof(errmsg)); switch (ret) { case NB_OK: return CMD_SUCCESS; default: + if (strlen(errmsg)) + vty_show_nb_errors(vty, ret, errmsg); return CMD_WARNING; } } diff --git a/lib/northbound_cli.h b/lib/northbound_cli.h index 112d62efda..2290a76b8d 100644 --- a/lib/northbound_cli.h +++ b/lib/northbound_cli.h @@ -75,6 +75,9 @@ extern int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, /* * Execute a YANG RPC or Action. * + * vty + * The vty terminal to dump any error. + * * xpath * XPath of the YANG RPC or Action node. * @@ -90,7 +93,7 @@ extern int nb_cli_apply_changes(struct vty *vty, const char *xpath_base_fmt, * Returns: * CMD_SUCCESS on success, CMD_WARNING otherwise. */ -extern int nb_cli_rpc(const char *xpath, struct list *input, +extern int nb_cli_rpc(struct vty *vty, const char *xpath, struct list *input, struct list *output); /* diff --git a/lib/northbound_confd.c b/lib/northbound_confd.c index 1f480f3d02..c1cb0fc11d 100644 --- a/lib/northbound_confd.c +++ b/lib/northbound_confd.c @@ -1068,6 +1068,7 @@ static int frr_confd_action_execute(struct confd_user_info *uinfo, struct yang_data *data; confd_tag_value_t *reply; int ret = CONFD_OK; + char errmsg[BUFSIZ] = {0}; /* Getting the XPath is tricky. */ if (kp) { @@ -1115,7 +1116,9 @@ static int frr_confd_action_execute(struct confd_user_info *uinfo, } /* Execute callback registered for this XPath. */ - if (nb_callback_rpc(nb_node, xpath, input, output) != NB_OK) { + if (nb_callback_rpc(nb_node, xpath, input, output, errmsg, + sizeof(errmsg)) + != NB_OK) { flog_warn(EC_LIB_NB_CB_RPC, "%s: rpc callback failed: %s", __func__, xpath); ret = CONFD_ERR; diff --git a/lib/northbound_grpc.cpp b/lib/northbound_grpc.cpp index f35b4bb31b..abdae993b1 100644 --- a/lib/northbound_grpc.cpp +++ b/lib/northbound_grpc.cpp @@ -962,6 +962,7 @@ class NorthboundImpl struct listnode *node; struct yang_data *data; const char *xpath; + char errmsg[BUFSIZ] = {0}; switch (tag->state) { case CREATE: @@ -1012,7 +1013,7 @@ class NorthboundImpl // Execute callback registered for this XPath. if (nb_callback_rpc(nb_node, xpath, input_list, - output_list) + output_list, errmsg, sizeof(errmsg)) != NB_OK) { flog_warn(EC_LIB_NB_CB_RPC, "%s: rpc callback failed: %s", diff --git a/lib/northbound_sysrepo.c b/lib/northbound_sysrepo.c index b5ef040a3f..3cd310c5a7 100644 --- a/lib/northbound_sysrepo.c +++ b/lib/northbound_sysrepo.c @@ -414,6 +414,7 @@ static int frr_sr_config_rpc_cb(sr_session_ctx_t *session, const char *xpath, struct yang_data *data; size_t cb_output_cnt; int ret = SR_ERR_OK; + char errmsg[BUFSIZ] = {0}; nb_node = nb_node_find(xpath); if (!nb_node) { @@ -436,7 +437,9 @@ static int frr_sr_config_rpc_cb(sr_session_ctx_t *session, const char *xpath, } /* Execute callback registered for this XPath. */ - if (nb_callback_rpc(nb_node, xpath, input, output) != NB_OK) { + if (nb_callback_rpc(nb_node, xpath, input, output, errmsg, + sizeof(errmsg)) + != NB_OK) { flog_warn(EC_LIB_NB_CB_RPC, "%s: rpc callback failed: %s", __func__, xpath); ret = SR_ERR_OPERATION_FAILED; diff --git a/ospfd/ospf_flood.c b/ospfd/ospf_flood.c index 58afb2b392..e96b31e3f4 100644 --- a/ospfd/ospf_flood.c +++ b/ospfd/ospf_flood.c @@ -383,7 +383,7 @@ static int ospf_flood_through_interface(struct ospf_interface *oi, if (!ospf_if_is_enable(oi)) return 0; - /* Remember if new LSA is aded to a retransmit list. */ + /* Remember if new LSA is added to a retransmit list. */ retx_flag = 0; /* Each of the neighbors attached to this interface are examined, @@ -936,7 +936,7 @@ void ospf_ls_retransmit_delete_nbr_as(struct ospf *ospf, struct ospf_lsa *lsa) /* Sets ls_age to MaxAge and floods throu the area. - When we implement ASE routing, there will be anothe function + When we implement ASE routing, there will be another function flushing an LSA from the whole domain. */ void ospf_lsa_flush_area(struct ospf_lsa *lsa, struct ospf_area *area) { diff --git a/ospfd/ospf_packet.h b/ospfd/ospf_packet.h index 5a3e029f2e..72b87edafb 100644 --- a/ospfd/ospf_packet.h +++ b/ospfd/ospf_packet.h @@ -33,11 +33,11 @@ #define OSPF_LS_UPD_MIN_SIZE 4U #define OSPF_LS_ACK_MIN_SIZE 0U -#define OSPF_MSG_HELLO 1 /* OSPF Hello Message. */ -#define OSPF_MSG_DB_DESC 2 /* OSPF Database Descriptoin Message. */ -#define OSPF_MSG_LS_REQ 3 /* OSPF Link State Request Message. */ -#define OSPF_MSG_LS_UPD 4 /* OSPF Link State Update Message. */ -#define OSPF_MSG_LS_ACK 5 /* OSPF Link State Acknoledgement Message. */ +#define OSPF_MSG_HELLO 1 /* OSPF Hello Message. */ +#define OSPF_MSG_DB_DESC 2 /* OSPF Database Description Message. */ +#define OSPF_MSG_LS_REQ 3 /* OSPF Link State Request Message. */ +#define OSPF_MSG_LS_UPD 4 /* OSPF Link State Update Message. */ +#define OSPF_MSG_LS_ACK 5 /* OSPF Link State Acknowledgement Message. */ #define OSPF_SEND_PACKET_DIRECT 1 #define OSPF_SEND_PACKET_INDIRECT 2 diff --git a/ripd/rip_cli.c b/ripd/rip_cli.c index 5e64b7afdb..87098ece64 100644 --- a/ripd/rip_cli.c +++ b/ripd/rip_cli.c @@ -1012,7 +1012,7 @@ DEFPY_YANG (clear_ip_rip, listnode_add(input, yang_vrf); } - ret = nb_cli_rpc("/frr-ripd:clear-rip-route", input, NULL); + ret = nb_cli_rpc(vty, "/frr-ripd:clear-rip-route", input, NULL); list_delete(&input); diff --git a/ripngd/ripng_cli.c b/ripngd/ripng_cli.c index f66de175fa..365082f806 100644 --- a/ripngd/ripng_cli.c +++ b/ripngd/ripng_cli.c @@ -496,7 +496,7 @@ DEFPY_YANG (clear_ipv6_rip, listnode_add(input, yang_vrf); } - ret = nb_cli_rpc("/frr-ripngd:clear-ripng-route", input, NULL); + ret = nb_cli_rpc(vty, "/frr-ripngd:clear-ripng-route", input, NULL); list_delete(&input); 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 --- /dev/null +++ b/tests/topotests/bgp_aggregate_address_topo1/__init__.py 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)) diff --git a/zebra/zebra_nb_rpcs.c b/zebra/zebra_nb_rpcs.c index 204cc5da6d..e7d438b1af 100644 --- a/zebra/zebra_nb_rpcs.c +++ b/zebra/zebra_nb_rpcs.c @@ -65,19 +65,22 @@ int clear_evpn_dup_addr_rpc(struct nb_cb_rpc_args *args) if (yang_dup_mac) { yang_str2mac(yang_dup_mac->value, &mac); ret = zebra_vxlan_clear_dup_detect_vni_mac( - zvrf, vni, &mac); + zvrf, vni, &mac, args->errmsg, + args->errmsg_len); } else if (yang_dup_ip) { yang_str2ip(yang_dup_ip->value, &host_ip); ret = zebra_vxlan_clear_dup_detect_vni_ip( - zvrf, vni, &host_ip); + zvrf, vni, &host_ip, args->errmsg, + args->errmsg_len); } else ret = zebra_vxlan_clear_dup_detect_vni(zvrf, vni); } } - ret = (ret != CMD_SUCCESS) ? NB_ERR : NB_OK; + if (ret < 0) + return NB_ERR; - return ret; + return NB_OK; } /* diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c index 6785151705..920fdd6baa 100644 --- a/zebra/zebra_vty.c +++ b/zebra/zebra_vty.c @@ -3264,7 +3264,8 @@ DEFPY (clear_evpn_dup_addr, if (yang_dup) { listnode_add(input, yang_dup); - ret = nb_cli_rpc("/frr-zebra:clear-evpn-dup-addr", input, NULL); + ret = nb_cli_rpc(vty, "/frr-zebra:clear-evpn-dup-addr", input, + NULL); } list_delete(&input); diff --git a/zebra/zebra_vxlan.c b/zebra/zebra_vxlan.c index d8ed58edef..82e871801a 100644 --- a/zebra/zebra_vxlan.c +++ b/zebra/zebra_vxlan.c @@ -36,6 +36,7 @@ #ifdef GNU_LINUX #include <linux/neighbour.h> #endif +#include "lib/printfrr.h" #include "zebra/zebra_router.h" #include "zebra/debug.h" @@ -2994,7 +2995,8 @@ void zebra_vxlan_print_macs_vni_dad(struct vty *vty, } int zebra_vxlan_clear_dup_detect_vni_mac(struct zebra_vrf *zvrf, vni_t vni, - struct ethaddr *macaddr) + struct ethaddr *macaddr, char *errmsg, + size_t errmsg_len) { zebra_evpn_t *zevpn; zebra_mac_t *mac; @@ -3006,18 +3008,20 @@ int zebra_vxlan_clear_dup_detect_vni_mac(struct zebra_vrf *zvrf, vni_t vni, zevpn = zebra_evpn_lookup(vni); if (!zevpn) { - zlog_warn("VNI %u does not exist\n", vni); + snprintfrr(errmsg, errmsg_len, "VNI %u does not exist", vni); return -1; } mac = zebra_evpn_mac_lookup(zevpn, macaddr); if (!mac) { - zlog_warn("Requested MAC does not exist in VNI %u\n", vni); + snprintf(errmsg, errmsg_len, + "Requested MAC does not exist in VNI %u\n", vni); return -1; } if (!CHECK_FLAG(mac->flags, ZEBRA_MAC_DUPLICATE)) { - zlog_warn("Requested MAC is not duplicate detected\n"); + snprintfrr(errmsg, errmsg_len, + "Requested MAC is not duplicate detected\n"); return -1; } @@ -3079,7 +3083,8 @@ int zebra_vxlan_clear_dup_detect_vni_mac(struct zebra_vrf *zvrf, vni_t vni, } int zebra_vxlan_clear_dup_detect_vni_ip(struct zebra_vrf *zvrf, vni_t vni, - struct ipaddr *ip) + struct ipaddr *ip, char *errmsg, + size_t errmsg_len) { zebra_evpn_t *zevpn; zebra_neigh_t *nbr; @@ -3092,28 +3097,31 @@ int zebra_vxlan_clear_dup_detect_vni_ip(struct zebra_vrf *zvrf, vni_t vni, zevpn = zebra_evpn_lookup(vni); if (!zevpn) { - zlog_debug("VNI %u does not exist\n", vni); + snprintfrr(errmsg, errmsg_len, "VNI %u does not exist\n", vni); return -1; } nbr = zebra_evpn_neigh_lookup(zevpn, ip); if (!nbr) { - zlog_warn("Requested host IP does not exist in VNI %u\n", vni); + snprintfrr(errmsg, errmsg_len, + "Requested host IP does not exist in VNI %u\n", vni); return -1; } ipaddr2str(&nbr->ip, buf, sizeof(buf)); if (!CHECK_FLAG(nbr->flags, ZEBRA_NEIGH_DUPLICATE)) { - zlog_warn("Requested host IP %s is not duplicate detected\n", - buf); + snprintfrr(errmsg, errmsg_len, + "Requested host IP %s is not duplicate detected\n", + buf); return -1; } mac = zebra_evpn_mac_lookup(zevpn, &nbr->emac); if (CHECK_FLAG(mac->flags, ZEBRA_MAC_DUPLICATE)) { - zlog_warn( + snprintfrr( + errmsg, errmsg_len, "Requested IP's associated MAC %s is still in duplicate state\n", prefix_mac2str(&nbr->emac, buf2, sizeof(buf2))); return -1; diff --git a/zebra/zebra_vxlan.h b/zebra/zebra_vxlan.h index 9c8af9d1fc..534e29936d 100644 --- a/zebra/zebra_vxlan.h +++ b/zebra/zebra_vxlan.h @@ -208,9 +208,12 @@ extern void zebra_vxlan_evpn_vrf_route_del(vrf_id_t vrf_id, struct prefix *host_prefix); extern int zebra_vxlan_clear_dup_detect_vni_mac(struct zebra_vrf *zvrf, vni_t vni, - struct ethaddr *macaddr); + struct ethaddr *macaddr, + char *errmsg, + size_t errmsg_len); extern int zebra_vxlan_clear_dup_detect_vni_ip(struct zebra_vrf *zvrf, - vni_t vni, struct ipaddr *ip); + vni_t vni, struct ipaddr *ip, + char *errmsg, size_t errmsg_len); extern int zebra_vxlan_clear_dup_detect_vni_all(struct zebra_vrf *zvrf); extern int zebra_vxlan_clear_dup_detect_vni(struct zebra_vrf *zvrf, vni_t vni); extern void zebra_vxlan_handle_result(struct zebra_dplane_ctx *ctx); |