From 6392aaa6547e665859ab5c648df30c1c04e26f6d Mon Sep 17 00:00:00 2001 From: Pascal Mathis Date: Wed, 16 May 2018 19:17:42 +0200 Subject: [PATCH 1/2] bgpd: Implement new adjacent route show commands This commit changes the behavior of `show bgp [afi] [safi] neighbor received-routes [json]` to return all received prefixes instead of filtering rejected/denied prefixes. Compared to Cisco and Juniper products, this is the usual way how this command is supposed to work, as `show bgp [afi] [safi] neighbor routes` will already return all accepted prefixes. Additionally, the new command `show bgp [afi] [safi] neighbor filtered-routes` has been added, which returns a list of all prefixes that got filtered away, so it can be roughly described as a subset of "received prefixes - accepted prefixes". As the already available `filtered_count` variable inside `show_adj_route` has not been used before, the last output line summarizing the amount of prefixes found was extended to also mention the amount of filtered prefixes if present. Signed-off-by: Pascal Mathis --- bgpd/bgp_route.c | 99 +++++++++++++++++++++++++++--------------------- bgpd/bgp_route.h | 6 +++ 2 files changed, 62 insertions(+), 43 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 7bfeee9669..31aca9edd6 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -10219,8 +10219,9 @@ DEFUN (show_ip_bgp_l2vpn_evpn_all_route_prefix, } static void show_adj_route(struct vty *vty, struct peer *peer, afi_t afi, - safi_t safi, int in, const char *rmap_name, - uint8_t use_json, json_object *json) + safi_t safi, enum bgp_show_adj_route_type type, + const char *rmap_name, uint8_t use_json, + json_object *json) { struct bgp_table *table; struct bgp_adj_in *ain; @@ -10277,7 +10278,7 @@ static void show_adj_route(struct vty *vty, struct peer *peer, afi_t afi, output_count = filtered_count = 0; subgrp = peer_subgroup(peer, afi, safi); - if (!in && subgrp + if (type == bgp_show_adj_route_advertised && subgrp && CHECK_FLAG(subgrp->sflags, SUBGRP_STATUS_DEFAULT_ORIGINATE)) { if (use_json) { json_object_int_add(json, "bgpTableVersion", @@ -10310,10 +10311,12 @@ static void show_adj_route(struct vty *vty, struct peer *peer, afi_t afi, } for (rn = bgp_table_top(table); rn; rn = bgp_route_next(rn)) { - if (in) { + if (type == bgp_show_adj_route_received + || type == bgp_show_adj_route_filtered) { for (ain = rn->adj_in; ain; ain = ain->next) { - if (ain->peer != peer) + if (ain->peer != peer || !ain->attr) continue; + if (header1) { if (use_json) { json_object_int_add( @@ -10356,22 +10359,24 @@ static void show_adj_route(struct vty *vty, struct peer *peer, afi_t afi, vty_out(vty, BGP_SHOW_HEADER); header2 = 0; } - if (ain->attr) { - bgp_attr_dup(&attr, ain->attr); - if (bgp_input_modifier(peer, &rn->p, - &attr, afi, safi, - rmap_name) - != RMAP_DENY) { - route_vty_out_tmp(vty, &rn->p, - &attr, safi, - use_json, - json_ar); - output_count++; - } else - filtered_count++; - } + + bgp_attr_dup(&attr, ain->attr); + ret = bgp_input_modifier(peer, &rn->p, &attr, + afi, safi, rmap_name); + + if (type == bgp_show_adj_route_filtered + && ret != RMAP_DENY) + continue; + + if (type == bgp_show_adj_route_received + && ret == RMAP_DENY) + filtered_count++; + + route_vty_out_tmp(vty, &rn->p, &attr, safi, + use_json, json_ar); + output_count++; } - } else { + } else if (type == bgp_show_adj_route_advertised) { for (adj = rn->adj_out; adj; adj = adj->next) SUBGRP_FOREACH_PEER (adj->subgroup, paf) { if (paf->peer != peer) @@ -10451,27 +10456,30 @@ static void show_adj_route(struct vty *vty, struct peer *peer, afi_t afi, } } } - if (use_json) - json_object_object_add(json, "advertisedRoutes", json_ar); - if (output_count != 0) { - if (use_json) - json_object_int_add(json, "totalPrefixCounter", - output_count); + if (use_json) { + json_object_object_add(json, "advertisedRoutes", json_ar); + json_object_int_add(json, "totalPrefixCounter", output_count); + json_object_int_add(json, "filteredPrefixCounter", + filtered_count); + + vty_out(vty, "%s\n", json_object_to_json_string_ext( + json, JSON_C_TO_STRING_PRETTY)); + json_object_free(json); + } else if (output_count > 0) { + if (filtered_count > 0) + vty_out(vty, + "\nTotal number of prefixes %ld (%ld filtered)\n", + output_count, filtered_count); else vty_out(vty, "\nTotal number of prefixes %ld\n", output_count); } - if (use_json) { - vty_out(vty, "%s\n", json_object_to_json_string_ext( - json, JSON_C_TO_STRING_PRETTY)); - json_object_free(json); - } } static int peer_adj_routes(struct vty *vty, struct peer *peer, afi_t afi, - safi_t safi, int in, const char *rmap_name, - uint8_t use_json) + safi_t safi, enum bgp_show_adj_route_type type, + const char *rmap_name, uint8_t use_json) { json_object *json = NULL; @@ -10495,7 +10503,8 @@ static int peer_adj_routes(struct vty *vty, struct peer *peer, afi_t afi, return CMD_WARNING; } - if (in + if ((type == bgp_show_adj_route_received + || type == bgp_show_adj_route_filtered) && !CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_SOFT_RECONFIG)) { if (use_json) { @@ -10511,7 +10520,7 @@ static int peer_adj_routes(struct vty *vty, struct peer *peer, afi_t afi, return CMD_WARNING; } - show_adj_route(vty, peer, afi, safi, in, rmap_name, use_json, json); + show_adj_route(vty, peer, afi, safi, type, rmap_name, use_json, json); return CMD_SUCCESS; } @@ -10519,7 +10528,7 @@ static int peer_adj_routes(struct vty *vty, struct peer *peer, afi_t afi, DEFUN (show_ip_bgp_instance_neighbor_advertised_route, show_ip_bgp_instance_neighbor_advertised_route_cmd, "show [ip] bgp [ VIEWVRFNAME] ["BGP_AFI_CMD_STR" ["BGP_SAFI_WITH_LABEL_CMD_STR"]] " - "neighbors [route-map WORD] [json]", + "neighbors [route-map WORD] [json]", SHOW_STR IP_STR BGP_STR @@ -10530,8 +10539,9 @@ DEFUN (show_ip_bgp_instance_neighbor_advertised_route, "Neighbor to display information about\n" "Neighbor to display information about\n" "Neighbor on BGP configured interface\n" - "Display the received routes from neighbor\n" "Display the routes advertised to a BGP neighbor\n" + "Display the received routes from neighbor\n" + "Display the filtered routes received from neighbor\n" "Route-map to modify the attributes\n" "Name of the route map\n" JSON_STR) @@ -10540,18 +10550,18 @@ DEFUN (show_ip_bgp_instance_neighbor_advertised_route, safi_t safi = SAFI_UNICAST; char *rmap_name = NULL; char *peerstr = NULL; - int rcvd = 0; struct bgp *bgp = NULL; struct peer *peer; + enum bgp_show_adj_route_type type = bgp_show_adj_route_advertised; int idx = 0; - bgp_vty_find_and_parse_afi_safi_bgp(vty, argv, argc, &idx, &afi, &safi, &bgp); if (!idx) return CMD_WARNING; int uj = use_json(argc, argv); + if (uj) argc--; @@ -10563,14 +10573,17 @@ DEFUN (show_ip_bgp_instance_neighbor_advertised_route, if (!peer) return CMD_WARNING; - if (argv_find(argv, argc, "received-routes", &idx)) - rcvd = 1; if (argv_find(argv, argc, "advertised-routes", &idx)) - rcvd = 0; + type = bgp_show_adj_route_advertised; + else if (argv_find(argv, argc, "received-routes", &idx)) + type = bgp_show_adj_route_received; + else if (argv_find(argv, argc, "filtered-routes", &idx)) + type = bgp_show_adj_route_filtered; + if (argv_find(argv, argc, "route-map", &idx)) rmap_name = argv[++idx]->arg; - return peer_adj_routes(vty, peer, afi, safi, rcvd, rmap_name, uj); + return peer_adj_routes(vty, peer, afi, safi, type, rmap_name, uj); } DEFUN (show_ip_bgp_neighbor_received_prefix_filter, diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index 00e5677fe0..288b66fb85 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -52,6 +52,12 @@ enum bgp_show_type { bgp_show_type_detail, }; +enum bgp_show_adj_route_type { + bgp_show_adj_route_advertised, + bgp_show_adj_route_received, + bgp_show_adj_route_filtered, +}; + #define BGP_SHOW_SCODE_HEADER \ "Status codes: s suppressed, d damped, " \ From b755861b95142446bac05f0f2506647bbca5d2d8 Mon Sep 17 00:00:00 2001 From: Pascal Mathis Date: Wed, 16 May 2018 21:55:55 +0200 Subject: [PATCH 2/2] bgpd: Fix memleak, adapt adv- to recv-routes code This commit tries to adapt a similar codeflow within the `show bgp [afi] [safi] neighbor advertised-routes` command compared to its `received-routes` and `filtered-routes` opponents. Some branching code has been restructured to achieve this. Additionally, this commit fixes a memory leak within `received-routes` (and `filtered-routes`, although the issue has been present before the previous commit!) where the previous implementation forgot to deduplicate the BGP attributes. When a user called `<...> received-routes route-map ` and that routemap changed any AS path or community parameters, the duplicated memory for these parameters was never freed. This has been fixed by ensuring to call `bgp_attr_undup()` accordingly. Signed-off-by: Pascal Mathis --- bgpd/bgp_route.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 31aca9edd6..84ea33b586 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -10365,8 +10365,10 @@ static void show_adj_route(struct vty *vty, struct peer *peer, afi_t afi, afi, safi, rmap_name); if (type == bgp_show_adj_route_filtered - && ret != RMAP_DENY) + && ret != RMAP_DENY) { + bgp_attr_undup(&attr, ain->attr); continue; + } if (type == bgp_show_adj_route_received && ret == RMAP_DENY) @@ -10374,12 +10376,13 @@ static void show_adj_route(struct vty *vty, struct peer *peer, afi_t afi, route_vty_out_tmp(vty, &rn->p, &attr, safi, use_json, json_ar); + bgp_attr_undup(&attr, ain->attr); output_count++; } } else if (type == bgp_show_adj_route_advertised) { for (adj = rn->adj_out; adj; adj = adj->next) SUBGRP_FOREACH_PEER (adj->subgroup, paf) { - if (paf->peer != peer) + if (paf->peer != peer || !adj->attr) continue; if (header1) { @@ -10427,7 +10430,6 @@ static void show_adj_route(struct vty *vty, struct peer *peer, afi_t afi, } header1 = 0; } - if (header2) { if (!use_json) vty_out(vty, @@ -10435,24 +10437,22 @@ static void show_adj_route(struct vty *vty, struct peer *peer, afi_t afi, header2 = 0; } - if (adj->attr) { - bgp_attr_dup(&attr, adj->attr); - ret = bgp_output_modifier( - peer, &rn->p, &attr, - afi, safi, rmap_name); - if (ret != RMAP_DENY) { - route_vty_out_tmp( - vty, &rn->p, - &attr, safi, - use_json, - json_ar); - output_count++; - } else - filtered_count++; + bgp_attr_dup(&attr, adj->attr); + ret = bgp_output_modifier( + peer, &rn->p, &attr, afi, safi, + rmap_name); - bgp_attr_undup(&attr, - adj->attr); + if (ret != RMAP_DENY) { + route_vty_out_tmp(vty, &rn->p, + &attr, safi, + use_json, + json_ar); + output_count++; + } else { + filtered_count++; } + + bgp_attr_undup(&attr, adj->attr); } } }