From 924c3f6ae9ec7bb18f923f64d63ef7a3bcfbf9e9 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 24 Oct 2017 20:21:09 -0400 Subject: [PATCH 1/4] bgpd: Cleanup indentation a tiny bit Signed-off-by: Donald Sharp --- bgpd/bgp_route.c | 124 ++++++++++++++++++++++++----------------------- 1 file changed, 63 insertions(+), 61 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index a5f9c5f2ab..cc7b80df82 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -10180,70 +10180,72 @@ static void show_adj_route(struct vty *vty, struct peer *peer, afi_t afi, } } else { for (adj = rn->adj_out; adj; adj = adj->next) - SUBGRP_FOREACH_PEER (adj->subgroup, paf) - if (paf->peer == peer) { - if (header1) { - if (use_json) { - json_object_int_add( - json, - "bgpTableVersion", - table->version); - json_object_string_add( - json, - "bgpLocalRouterId", - inet_ntoa( - bgp->router_id)); - json_object_object_add( - json, - "bgpStatusCodes", - json_scode); - json_object_object_add( - json, - "bgpOriginCodes", - json_ocode); - } else { - vty_out(vty, - "BGP table version is %" PRIu64 - ", local router ID is %s\n", - table->version, - inet_ntoa( - bgp->router_id)); - vty_out(vty, - BGP_SHOW_SCODE_HEADER); - vty_out(vty, - BGP_SHOW_OCODE_HEADER); - } - header1 = 0; - } + SUBGRP_FOREACH_PEER (adj->subgroup, paf) { + if (paf->peer != peer) + continue; - if (header2) { - if (!use_json) - vty_out(vty, - BGP_SHOW_HEADER); - 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++; + if (header1) { + if (use_json) { + json_object_int_add( + json, + "bgpTableVersion", + table->version); + json_object_string_add( + json, + "bgpLocalRouterId", + inet_ntoa( + bgp->router_id)); + json_object_object_add( + json, + "bgpStatusCodes", + json_scode); + json_object_object_add( + json, + "bgpOriginCodes", + json_ocode); + } else { + vty_out(vty, + "BGP table version is %" PRIu64 + ", local router ID is %s\n", + table->version, + inet_ntoa( + bgp->router_id)); + vty_out(vty, + BGP_SHOW_SCODE_HEADER); + vty_out(vty, + BGP_SHOW_OCODE_HEADER); } + header1 = 0; } + + if (header2) { + if (!use_json) + vty_out(vty, + BGP_SHOW_HEADER); + 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++; + } + } } } if (use_json) From b787157a528c5730b4d82c06f17de8eccf0b3508 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 24 Oct 2017 20:23:06 -0400 Subject: [PATCH 2/4] bgpd: When we don't have a route-map to apply don't apply the original When we display the advertised-routes and we don't have a route-map to apply do not re-apply the route-map. This does two things: 1) Fixes a display issue with the show command. 2) More importantly stops leaking memory like a sieve for when you have a full bgp table. Fixes: #1345 Signed-off-by: Donald Sharp --- bgpd/bgp_route.c | 55 ++++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index cc7b80df82..3a7cfc4f2c 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -1157,49 +1157,50 @@ static int bgp_output_modifier(struct peer *peer, struct prefix *p, struct attr *attr, afi_t afi, safi_t safi, const char *rmap_name) { - struct bgp_filter *filter; struct bgp_info info; route_map_result_t ret; struct route_map *rmap = NULL; - filter = &peer->filter[afi][safi]; + /* + * So if we get to this point and have no rmap_name + * we want to just show the output as it currently + * exists. + */ + if (!rmap_name) + return RMAP_PERMIT; /* Apply default weight value. */ if (peer->weight[afi][safi]) attr->weight = peer->weight[afi][safi]; - if (rmap_name) { - rmap = route_map_lookup_by_name(rmap_name); + rmap = route_map_lookup_by_name(rmap_name); - if (rmap == NULL) - return RMAP_DENY; - } else { - if (ROUTE_MAP_OUT_NAME(filter)) { - rmap = ROUTE_MAP_OUT(filter); - - if (rmap == NULL) - return RMAP_DENY; - } - } + /* + * If we have a route map name and we do not find + * the routemap that means we have an implicit + * deny. + */ + if (rmap == NULL) + return RMAP_DENY; /* Route map apply. */ - if (rmap) { - /* Duplicate current value to new strucutre for modification. */ - info.peer = peer; - info.attr = attr; + /* Duplicate current value to new strucutre for modification. */ + info.peer = peer; + info.attr = attr; - SET_FLAG(peer->rmap_type, PEER_RMAP_TYPE_OUT); + SET_FLAG(peer->rmap_type, PEER_RMAP_TYPE_OUT); - /* Apply BGP route map to the attribute. */ - ret = route_map_apply(rmap, p, RMAP_BGP, &info); + /* Apply BGP route map to the attribute. */ + ret = route_map_apply(rmap, p, RMAP_BGP, &info); - peer->rmap_type = 0; + peer->rmap_type = 0; + + if (ret == RMAP_DENYMATCH) + /* + * caller has multiple error paths with bgp_attr_flush() + */ + return RMAP_DENY; - if (ret == RMAP_DENYMATCH) - /* caller has multiple error paths with bgp_attr_flush() - */ - return RMAP_DENY; - } return RMAP_PERMIT; } From 0f672529d86b4c55f11c761f171f635ebb4bdb10 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 24 Oct 2017 20:41:59 -0400 Subject: [PATCH 3/4] bgpd: Store original route-map type for the peer When we are applying an experimental route-map type to a peer( as part of a show command ) save and restore the peer's ->rmap_type. Signed-off-by: Donald Sharp --- bgpd/bgp_route.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 3a7cfc4f2c..1d267f259f 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -1160,6 +1160,7 @@ static int bgp_output_modifier(struct peer *peer, struct prefix *p, struct bgp_info info; route_map_result_t ret; struct route_map *rmap = NULL; + u_char rmap_type; /* * So if we get to this point and have no rmap_name @@ -1188,12 +1189,13 @@ static int bgp_output_modifier(struct peer *peer, struct prefix *p, info.peer = peer; info.attr = attr; + rmap_type = peer->rmap_type; SET_FLAG(peer->rmap_type, PEER_RMAP_TYPE_OUT); /* Apply BGP route map to the attribute. */ ret = route_map_apply(rmap, p, RMAP_BGP, &info); - peer->rmap_type = 0; + peer->rmap_type = rmap_type; if (ret == RMAP_DENYMATCH) /* From f46d8e1ed038cead4ab0b86c1e5701afee711be3 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 24 Oct 2017 20:57:00 -0400 Subject: [PATCH 4/4] bgpd: Free up changes to attr that the speculative route-map applied So we have the ability to apply speculative route-maps to neighbor display to see what the changes would look like via some show commands. When we do this we make a shallow copy of the attr data structure and then pass it around for applying the routemap. After we've applied this route-map and displayed it we really need to clean up memory that the route-map application applied. Signed-off-by: Donald Sharp --- bgpd/bgp_attr.c | 26 ++++++++++++++++++++++++++ bgpd/bgp_attr.h | 1 + bgpd/bgp_route.c | 3 +++ 3 files changed, 30 insertions(+) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 76fc8f968e..6ddb2ec8a7 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -822,6 +822,32 @@ void bgp_attr_unintern_sub(struct attr *attr) #endif } +/* + * We have some show commands that let you experimentally + * apply a route-map. When we apply the route-map + * we are reseting values but not saving them for + * posterity via intern'ing( because route-maps don't + * do that) but at this point in time we need + * to compare the new attr to the old and if the + * routemap has changed it we need to, as Snoop Dog says, + * Drop it like it's hot + */ +void bgp_attr_undup(struct attr *new, struct attr *old) +{ + if (new->aspath != old->aspath) + aspath_free(new->aspath); + + if (new->community != old->community) + community_free(new->community); + + if (new->ecommunity != old->ecommunity) + ecommunity_free(&new->ecommunity); + + if (new->lcommunity != old->lcommunity) + lcommunity_free(&new->lcommunity); + +} + /* Free bgp attribute and aspath. */ void bgp_attr_unintern(struct attr **pattr) { diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index 80ff36b59f..f694f01adb 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -239,6 +239,7 @@ extern bgp_attr_parse_ret_t bgp_attr_parse(struct peer *, struct attr *, bgp_size_t, struct bgp_nlri *, struct bgp_nlri *); extern void bgp_attr_dup(struct attr *, struct attr *); +extern void bgp_attr_undup(struct attr *new, struct attr *old); extern struct attr *bgp_attr_intern(struct attr *attr); extern void bgp_attr_unintern_sub(struct attr *); extern void bgp_attr_unintern(struct attr **); diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 1d267f259f..a655bd0b6f 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -10247,6 +10247,9 @@ static void show_adj_route(struct vty *vty, struct peer *peer, afi_t afi, output_count++; } else filtered_count++; + + bgp_attr_undup(&attr, + adj->attr); } } }