bgpd: Store aspath count after aspath has changed

When running bestpath on a very large number of ecmp.
BGP ends up calling aspath_count a very very large number
of times, which results in ~15% cpu runtime in aspath_count_hops.
Modify the aspath to keep track of it's own count.  This results
in the function now taking up ~1.5% of the cpu runtime.  Enough
for the moment to be ignored.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>

diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
index 4c1615a5c625a6ebe01863155771e64715667606..a86b42e250152495b09d18ec72fc7ea590c2099c 100644 (file)
--- a/bgpd/bgp_aspath.c
+++ b/bgpd/bgp_aspath.c
@@ -297,6 +297,8 @@ static struct aspath *aspath_new(enum asnotation_mode asnotation)
 
        as = XCALLOC(MTYPE_AS_PATH, sizeof(struct aspath));
        as->asnotation = asnotation;
+       as->count = 0;
+
        return as;
 }
 
@@ -399,6 +401,11 @@ unsigned int aspath_count_confeds(struct aspath *aspath)
 }
 
 unsigned int aspath_count_hops(const struct aspath *aspath)
+{
+       return aspath->count;
+}
+
+static unsigned int aspath_count_hops_internal(const struct aspath *aspath)
 {
        int count = 0;
        struct assegment *seg = aspath->segments;
@@ -708,6 +715,7 @@ struct aspath *aspath_dup(struct aspath *aspath)
        else
                new->str[0] = '\0';
 
+       new->count = aspath->count;
        return new;
 }
 
@@ -729,6 +737,7 @@ static void *aspath_hash_alloc(void *arg)
        new->str_len = aspath->str_len;
        new->json = aspath->json;
        new->asnotation = aspath->asnotation;
+       new->count = aspath->count;
 
        return new;
 }
@@ -856,6 +865,8 @@ struct aspath *aspath_parse(struct stream *s, size_t length, int use32bit,
        if (assegments_parse(s, length, &as.segments, use32bit) < 0)
                return NULL;
 
+       as.count = aspath_count_hops_internal(&as);
+
        /* If already same aspath exist then return it. */
        find = hash_get(ashash, &as, aspath_hash_alloc);
 
@@ -1032,7 +1043,7 @@ static struct assegment *aspath_aggregate_as_set_add(struct aspath *aspath,
                asset->as[asset->length - 1] = as;
        }
 
-
+       aspath->count = aspath_count_hops_internal(aspath);
        return asset;
 }
 
@@ -1113,6 +1124,8 @@ struct aspath *aspath_aggregate(struct aspath *as1, struct aspath *as2)
 
        assegment_normalise(aspath->segments);
        aspath_str_update(aspath, false);
+       aspath->count = aspath_count_hops_internal(aspath);
+
        return aspath;
 }
 
@@ -1268,6 +1281,7 @@ struct aspath *aspath_replace_regex_asn(struct aspath *aspath,
        }
 
        aspath_str_update(new, false);
+       new->count = aspath_count_hops_internal(new);
        return new;
 }
 
@@ -1293,6 +1307,8 @@ struct aspath *aspath_replace_specific_asn(struct aspath *aspath,
        }
 
        aspath_str_update(new, false);
+       new->count = aspath_count_hops_internal(new);
+
        return new;
 }
 
@@ -1315,6 +1331,8 @@ struct aspath *aspath_replace_all_asn(struct aspath *aspath, as_t our_asn)
        }
 
        aspath_str_update(new, false);
+       new->count = aspath_count_hops_internal(new);
+
        return new;
 }
 
@@ -1341,6 +1359,8 @@ struct aspath *aspath_replace_private_asns(struct aspath *aspath, as_t asn,
        }
 
        aspath_str_update(new, false);
+       new->count = aspath_count_hops_internal(new);
+
        return new;
 }
 
@@ -1413,6 +1433,7 @@ struct aspath *aspath_remove_private_asns(struct aspath *aspath, as_t peer_asn)
        if (!aspath->refcnt)
                aspath_free(aspath);
        aspath_str_update(new, false);
+       new->count = aspath_count_hops_internal(new);
        return new;
 }
 
@@ -1469,6 +1490,7 @@ static struct aspath *aspath_merge(struct aspath *as1, struct aspath *as2)
                last->next = as2->segments;
        as2->segments = new;
        aspath_str_update(as2, false);
+       as2->count = aspath_count_hops_internal(as2);
        return as2;
 }
 
@@ -1486,6 +1508,7 @@ struct aspath *aspath_prepend(struct aspath *as1, struct aspath *as2)
        if (as2->segments == NULL) {
                as2->segments = assegment_dup_all(as1->segments);
                aspath_str_update(as2, false);
+               as2->count = aspath_count_hops_internal(as2);
                return as2;
        }
 
@@ -1506,6 +1529,7 @@ struct aspath *aspath_prepend(struct aspath *as1, struct aspath *as2)
        if (!as2->segments) {
                as2->segments = assegment_dup_all(as1->segments);
                aspath_str_update(as2, false);
+               as2->count = aspath_count_hops_internal(as2);
                return as2;
        }
 
@@ -1551,6 +1575,7 @@ struct aspath *aspath_prepend(struct aspath *as1, struct aspath *as2)
                 * the inbetween AS_SEQUENCE of seg2 in the process
                 */
                aspath_str_update(as2, false);
+               as2->count = aspath_count_hops_internal(as2);
                return as2;
        } else {
                /* AS_SET merge code is needed at here. */
@@ -1662,6 +1687,7 @@ struct aspath *aspath_filter_exclude(struct aspath *source,
                lastseg = newseg;
        }
        aspath_str_update(newpath, false);
+       newpath->count = aspath_count_hops_internal(newpath);
        /* We are happy returning even an empty AS_PATH, because the
         * administrator
         * might expect this very behaviour. There's a mean to avoid this, if
@@ -1680,6 +1706,7 @@ struct aspath *aspath_filter_exclude_all(struct aspath *source)
        newpath = aspath_new(source->asnotation);
 
        aspath_str_update(newpath, false);
+       newpath->count = aspath_count_hops_internal(newpath);
        /* We are happy returning even an empty AS_PATH, because the
         * administrator
         * might expect this very behaviour. There's a mean to avoid this, if
@@ -1767,6 +1794,7 @@ struct aspath *aspath_filter_exclude_acl(struct aspath *source,
 
 
        aspath_str_update(source, false);
+       source->count = aspath_count_hops_internal(source);
        /* We are happy returning even an empty AS_PATH, because the
         * administrator
         * might expect this very behaviour. There's a mean to avoid this, if
@@ -1805,6 +1833,7 @@ static struct aspath *aspath_add_asns(struct aspath *aspath, as_t asno,
        }
 
        aspath_str_update(aspath, false);
+       aspath->count = aspath_count_hops_internal(aspath);
        return aspath;
 }
 
@@ -1896,6 +1925,7 @@ struct aspath *aspath_reconcile_as4(struct aspath *aspath,
        if (!hops) {
                newpath = aspath_dup(as4path);
                aspath_str_update(newpath, false);
+               /* dup sets the count properly */
                return newpath;
        }
 
@@ -1957,6 +1987,7 @@ struct aspath *aspath_reconcile_as4(struct aspath *aspath,
        aspath_free(newpath);
        mergedpath->segments = assegment_normalise(mergedpath->segments);
        aspath_str_update(mergedpath, false);
+       mergedpath->count = aspath_count_hops_internal(mergedpath);
 
        if (BGP_DEBUG(as4, AS4))
                zlog_debug("[AS4] result of synthesizing is %s",
@@ -2027,8 +2058,10 @@ struct aspath *aspath_delete_confed_seq(struct aspath *aspath)
                seg = next;
        }
 
-       if (removed_confed_segment)
+       if (removed_confed_segment) {
                aspath_str_update(aspath, false);
+               aspath->count = aspath_count_hops_internal(aspath);
+       }
 
        return aspath;
 }

diff --git a/bgpd/bgp_aspath.h b/bgpd/bgp_aspath.h
index f7e57fd66ddafbc04612919aaca5cc07dde4d5b6..46202fd34afc33b7307c9aa85ff4bc20205f4df7 100644 (file)
--- a/bgpd/bgp_aspath.h
+++ b/bgpd/bgp_aspath.h
@@ -59,6 +59,7 @@ struct aspath {
           and AS path regular expression match.  */
        char *str;
        unsigned short str_len;
+       uint32_t count;
 
        /* AS notation used by string expression of AS path */
        enum asnotation_mode asnotation;