bgpd : route agg. with aspath attribute is consuming lot of cycles.

While configuring aggregate route prepare the hash table first,
then prepare the aggregated aspath value just like lcomm,
ecomm and standard community.

Signed-off-by: vishaldhingra<vdhingra@vmware.com>

diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
index f5652b07c5e46cb66ef3a1ea2c5fbd26dd4b6411..6caeb7ca1f71defb61ac18b52297540bbd2b56b8 100644 (file)
--- a/bgpd/bgp_aspath.c
+++ b/bgpd/bgp_aspath.c
@@ -2114,15 +2114,12 @@ static void *bgp_aggr_aspath_hash_alloc(void *p)
 
 static void bgp_aggr_aspath_prepare(struct hash_backet *hb, void *arg)
 {
-       struct aspath *asmerge = NULL;
        struct aspath *hb_aspath = hb->data;
        struct aspath **aggr_aspath = arg;
 
-       if (*aggr_aspath) {
-               asmerge = aspath_aggregate(*aggr_aspath, hb_aspath);
-               aspath_free(*aggr_aspath);
-               *aggr_aspath = asmerge;
-       } else
+       if (*aggr_aspath)
+               *aggr_aspath = aspath_aggregate(*aggr_aspath, hb_aspath);
+       else
                *aggr_aspath = aspath_dup(hb_aspath);
 }
 
@@ -2135,6 +2132,15 @@ void bgp_aggr_aspath_remove(void *arg)
 
 void bgp_compute_aggregate_aspath(struct bgp_aggregate *aggregate,
                                  struct aspath *aspath)
+{
+       bgp_compute_aggregate_aspath_hash(aggregate, aspath);
+
+       bgp_compute_aggregate_aspath_val(aggregate);
+
+}
+
+void bgp_compute_aggregate_aspath_hash(struct bgp_aggregate *aggregate,
+                                      struct aspath *aspath)
 {
        struct aspath *aggr_aspath = NULL;
 
@@ -2154,17 +2160,29 @@ void bgp_compute_aggregate_aspath(struct bgp_aggregate *aggregate,
                 */
                aggr_aspath = hash_get(aggregate->aspath_hash, aspath,
                                       bgp_aggr_aspath_hash_alloc);
+       }
 
-               /* Compute aggregate's as-path.
-                */
+       /* Increment reference counter.
+        */
+       aggr_aspath->refcnt++;
+}
+
+void bgp_compute_aggregate_aspath_val(struct bgp_aggregate *aggregate)
+{
+       if (aggregate == NULL)
+               return;
+       /* Re-compute aggregate's as-path.
+        */
+       if (aggregate->aspath) {
+               aspath_free(aggregate->aspath);
+               aggregate->aspath = NULL;
+       }
+       if (aggregate->aspath_hash
+           && aggregate->aspath_hash->count) {
                hash_iterate(aggregate->aspath_hash,
                             bgp_aggr_aspath_prepare,
                             &aggregate->aspath);
        }
-
-       /* Increment refernce counter.
-        */
-       aggr_aspath->refcnt++;
 }
 
 void bgp_remove_aspath_from_aggregate(struct bgp_aggregate *aggregate,
@@ -2173,10 +2191,9 @@ void bgp_remove_aspath_from_aggregate(struct bgp_aggregate *aggregate,
        struct aspath *aggr_aspath = NULL;
        struct aspath *ret_aspath = NULL;
 
-       if ((aggregate == NULL) || (aspath == NULL))
-               return;
-
-       if (aggregate->aspath_hash == NULL)
+       if ((!aggregate)
+           || (!aggregate->aspath_hash)
+           || (!aspath))
                return;
 
        /* Look-up the aspath in the hash.
@@ -2189,17 +2206,41 @@ void bgp_remove_aspath_from_aggregate(struct bgp_aggregate *aggregate,
                        ret_aspath = hash_release(aggregate->aspath_hash,
                                                  aggr_aspath);
                        aspath_free(ret_aspath);
+                       ret_aspath = NULL;
 
                        /* Remove aggregate's old as-path.
                         */
                        aspath_free(aggregate->aspath);
                        aggregate->aspath = NULL;
 
-                       /* Compute aggregate's as-path.
-                        */
-                       hash_iterate(aggregate->aspath_hash,
-                                    bgp_aggr_aspath_prepare,
-                                    &aggregate->aspath);
+                       bgp_compute_aggregate_aspath_val(aggregate);
                }
        }
 }
+
+void bgp_remove_aspath_from_aggregate_hash(struct bgp_aggregate *aggregate,
+                                          struct aspath *aspath)
+{
+       struct aspath *aggr_aspath = NULL;
+       struct aspath *ret_aspath = NULL;
+
+       if ((!aggregate)
+           || (!aggregate->aspath_hash)
+           || (!aspath))
+               return;
+
+       /* Look-up the aspath in the hash.
+        */
+       aggr_aspath = bgp_aggr_aspath_lookup(aggregate, aspath);
+       if (aggr_aspath) {
+               aggr_aspath->refcnt--;
+
+               if (aggr_aspath->refcnt == 0) {
+                       ret_aspath = hash_release(aggregate->aspath_hash,
+                                                 aggr_aspath);
+                       aspath_free(ret_aspath);
+                       ret_aspath = NULL;
+               }
+       }
+}
+

diff --git a/bgpd/bgp_aspath.h b/bgpd/bgp_aspath.h
index f84b3740c96d3a850de6b4221f0e0060de363620..10f6ee28219d6e64bbe0a344ea113da87235c74b 100644 (file)
--- a/bgpd/bgp_aspath.h
+++ b/bgpd/bgp_aspath.h
@@ -134,8 +134,16 @@ extern uint8_t *aspath_snmp_pathseg(struct aspath *, size_t *);
 
 extern void bgp_compute_aggregate_aspath(struct bgp_aggregate *aggregate,
                                         struct aspath *aspath);
+
+extern void bgp_compute_aggregate_aspath_hash(struct bgp_aggregate *aggregate,
+                                             struct aspath *aspath);
+extern void bgp_compute_aggregate_aspath_val(struct bgp_aggregate *aggregate);
 extern void bgp_remove_aspath_from_aggregate(struct bgp_aggregate *aggregate,
                                             struct aspath *aspath);
+extern void bgp_remove_aspath_from_aggregate_hash(
+                                               struct bgp_aggregate *aggregate,
+                                               struct aspath *aspath);
+
 extern void bgp_aggr_aspath_remove(void *arg);
 
 #endif /* _QUAGGA_BGP_ASPATH_H */

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index e286beecfb4fb00f48c23cf78bd07398daf6da45..37360a559a41296ff93ca61e11c924c40dd1da2a 100644 (file)
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -5927,8 +5927,8 @@ void bgp_aggregate_route(struct bgp *bgp, struct prefix *p,
                         */
                        /* Compute aggregate route's as-path.
                         */
-                       bgp_compute_aggregate_aspath(aggregate,
-                                                    pi->attr->aspath);
+                       bgp_compute_aggregate_aspath_hash(aggregate,
+                                                         pi->attr->aspath);
 
                        /* Compute aggregate route's community.
                         */
@@ -5955,6 +5955,7 @@ void bgp_aggregate_route(struct bgp *bgp, struct prefix *p,
                        bgp_process(bgp, rn, afi, safi);
        }
        if (aggregate->as_set) {
+               bgp_compute_aggregate_aspath_val(aggregate);
                bgp_compute_aggregate_community_val(aggregate);
                bgp_compute_aggregate_ecommunity_val(aggregate);
                bgp_compute_aggregate_lcommunity_val(aggregate);
@@ -6041,7 +6042,7 @@ void bgp_aggregate_delete(struct bgp *bgp, struct prefix *p, afi_t afi,
                        if (aggregate->as_set) {
                                /* Remove as-path from aggregate.
                                 */
-                               bgp_remove_aspath_from_aggregate(
+                               bgp_remove_aspath_from_aggregate_hash(
                                                        aggregate,
                                                        pi->attr->aspath);
 
@@ -6074,6 +6075,8 @@ void bgp_aggregate_delete(struct bgp *bgp, struct prefix *p, afi_t afi,
                        bgp_process(bgp, rn, afi, safi);
        }
        if (aggregate->as_set) {
+               aspath_free(aggregate->aspath);
+               aggregate->aspath = NULL;
                if (aggregate->community)
                        community_free(&aggregate->community);
                if (aggregate->ecommunity)