From 21fec674536495d5835cc4d27f29ac96bae111f0 Mon Sep 17 00:00:00 2001 From: vdhingra Date: Mon, 19 Aug 2019 00:47:50 -0700 Subject: [PATCH] bgpd : route agg. with comm attr is consuming lot of cycles. While configuring aggregate route prepare the hash table first, then prepare the aggregated standard comm value and then do the unique sort once for standard community. Signed-off-by: vishaldhingra --- bgpd/bgp_community.c | 89 ++++++++++++++++++++++++++++++++------------ bgpd/bgp_community.h | 8 ++++ bgpd/bgp_route.c | 11 ++++-- 3 files changed, 81 insertions(+), 27 deletions(-) diff --git a/bgpd/bgp_community.c b/bgpd/bgp_community.c index 22d61f702d..432c922ea5 100644 --- a/bgpd/bgp_community.c +++ b/bgpd/bgp_community.c @@ -910,15 +910,13 @@ static void *bgp_aggr_communty_hash_alloc(void *p) static void bgp_aggr_community_prepare(struct hash_backet *hb, void *arg) { - struct community *commerge = NULL; struct community *hb_community = hb->data; struct community **aggr_community = arg; - if (*aggr_community) { - commerge = community_merge(*aggr_community, hb_community); - *aggr_community = community_uniq_sort(commerge); - community_free(&commerge); - } else + if (*aggr_community) + *aggr_community = community_merge(*aggr_community, + hb_community); + else *aggr_community = community_dup(hb_community); } @@ -931,6 +929,14 @@ void bgp_aggr_community_remove(void *arg) void bgp_compute_aggregate_community(struct bgp_aggregate *aggregate, struct community *community) +{ + bgp_compute_aggregate_community_hash(aggregate, community); + bgp_compute_aggregate_community_val(aggregate); +} + + +void bgp_compute_aggregate_community_hash(struct bgp_aggregate *aggregate, + struct community *community) { struct community *aggr_community = NULL; @@ -951,32 +957,47 @@ void bgp_compute_aggregate_community(struct bgp_aggregate *aggregate, */ aggr_community = hash_get(aggregate->community_hash, community, bgp_aggr_communty_hash_alloc); + } - /* Re-compute aggregate's community. - */ - if (aggregate->community) - community_free(&aggregate->community); + /* Increment reference counter. + */ + aggr_community->refcnt++; +} +void bgp_compute_aggregate_community_val(struct bgp_aggregate *aggregate) +{ + struct community *commerge = NULL; + + if (aggregate == NULL) + return; + + /* Re-compute aggregate's community. + */ + if (aggregate->community) + community_free(&aggregate->community); + if (aggregate->community_hash && + aggregate->community_hash->count) { hash_iterate(aggregate->community_hash, bgp_aggr_community_prepare, &aggregate->community); + commerge = aggregate->community; + aggregate->community = community_uniq_sort(commerge); + if (commerge) + community_free(&commerge); } - - /* Increment refernce counter. - */ - aggr_community->refcnt++; } + + void bgp_remove_community_from_aggregate(struct bgp_aggregate *aggregate, struct community *community) { struct community *aggr_community = NULL; struct community *ret_comm = NULL; - if ((aggregate == NULL) || (community == NULL)) - return; - - if (aggregate->community_hash == NULL) + if ((!aggregate) + || (!aggregate->community_hash) + || (!community)) return; /* Look-up the community in the hash. @@ -990,13 +1011,33 @@ void bgp_remove_community_from_aggregate(struct bgp_aggregate *aggregate, aggr_community); community_free(&ret_comm); - community_free(&aggregate->community); + bgp_compute_aggregate_community_val(aggregate); + } + } +} + +void bgp_remove_comm_from_aggregate_hash(struct bgp_aggregate *aggregate, + struct community *community) +{ + + struct community *aggr_community = NULL; + struct community *ret_comm = NULL; - /* Compute aggregate's community. - */ - hash_iterate(aggregate->community_hash, - bgp_aggr_community_prepare, - &aggregate->community); + if ((!aggregate) + || (!aggregate->community_hash) + || (!community)) + return; + + /* Look-up the community in the hash. + */ + aggr_community = bgp_aggr_community_lookup(aggregate, community); + if (aggr_community) { + aggr_community->refcnt--; + + if (aggr_community->refcnt == 0) { + ret_comm = hash_release(aggregate->community_hash, + aggr_community); + community_free(&ret_comm); } } } diff --git a/bgpd/bgp_community.h b/bgpd/bgp_community.h index f761a8f5e0..74a3a6b507 100644 --- a/bgpd/bgp_community.h +++ b/bgpd/bgp_community.h @@ -92,8 +92,16 @@ extern struct hash *community_hash(void); extern uint32_t community_val_get(struct community *com, int i); extern void bgp_compute_aggregate_community(struct bgp_aggregate *aggregate, struct community *community); + +extern void bgp_compute_aggregate_community_val( + struct bgp_aggregate *aggregate); +extern void bgp_compute_aggregate_community_hash( + struct bgp_aggregate *aggregate, + struct community *community); extern void bgp_remove_community_from_aggregate(struct bgp_aggregate *aggregate, struct community *community); +extern void bgp_remove_comm_from_aggregate_hash(struct bgp_aggregate *aggregate, + struct community *community); extern void bgp_aggr_community_remove(void *arg); #endif /* _QUAGGA_BGP_COMMUNITY_H */ diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index be13d3bcad..4236c6158e 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -5933,7 +5933,7 @@ void bgp_aggregate_route(struct bgp *bgp, struct prefix *p, /* Compute aggregate route's community. */ if (pi->attr->community) - bgp_compute_aggregate_community( + bgp_compute_aggregate_community_hash( aggregate, pi->attr->community); @@ -5954,8 +5954,11 @@ void bgp_aggregate_route(struct bgp *bgp, struct prefix *p, if (match) bgp_process(bgp, rn, afi, safi); } - if (aggregate->as_set) + if (aggregate->as_set) { + bgp_compute_aggregate_community_val(aggregate); bgp_compute_aggregate_lcommunity_val(aggregate); + } + bgp_unlock_node(top); @@ -6044,7 +6047,7 @@ void bgp_aggregate_delete(struct bgp *bgp, struct prefix *p, afi_t afi, if (pi->attr->community) /* Remove community from aggregate. */ - bgp_remove_community_from_aggregate( + bgp_remove_comm_from_aggregate_hash( aggregate, pi->attr->community); @@ -6070,6 +6073,8 @@ void bgp_aggregate_delete(struct bgp *bgp, struct prefix *p, afi_t afi, bgp_process(bgp, rn, afi, safi); } if (aggregate->as_set) { + if (aggregate->community) + community_free(&aggregate->community); if (aggregate->lcommunity) lcommunity_free(&aggregate->lcommunity); } -- 2.39.5