From: David Lamparter <equinox@opensourcerouting.org>
Date: Thu, 7 Sep 2017 12:24:00 +0000 (+0200)
Subject: bgpd: kill bgp_attr_refcount()
X-Git-Tag: frr-4.0-dev~327^2~4
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=7c87afac92b94cd26c1a74df0e499a7a05603aa7;p=mirror%2Ffrr.git

bgpd: kill bgp_attr_refcount()

This attempt at optimization has cost us more than a week's worth of
time on several people hunting down the subtle bug that it was missing
an increment on attr->lcommunity.

This is absolutely not worth the maintenance cost.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
---

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 03cf3625b7..d3f1294751 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -771,45 +771,6 @@ struct attr *bgp_attr_intern(struct attr *attr)
 	return find;
 }
 
-/**
- * Increment the refcount on various structures that attr holds.
- * Note on usage: call _only_ when the 'attr' object has already
- * been 'intern'ed and exists in 'attrhash' table. The function
- * serves to hold a reference to that (real) object.
- * Note also that the caller can safely call bgp_attr_unintern()
- * after calling bgp_attr_refcount(). That would release the
- * reference and could result in a free() of the attr object.
- */
-struct attr *bgp_attr_refcount(struct attr *attr)
-{
-	/* Intern referenced strucutre. */
-	if (attr->aspath)
-		attr->aspath->refcnt++;
-
-	if (attr->community)
-		attr->community->refcnt++;
-
-	if (attr->ecommunity)
-		attr->ecommunity->refcnt++;
-
-	if (attr->cluster)
-		attr->cluster->refcnt++;
-
-	if (attr->transit)
-		attr->transit->refcnt++;
-
-	if (attr->encap_subtlvs)
-		attr->encap_subtlvs->refcnt++;
-
-#if ENABLE_BGP_VNC
-	if (attr->vnc_subtlvs)
-		attr->vnc_subtlvs->refcnt++;
-#endif
-
-	attr->refcnt++;
-	return attr;
-}
-
 /* Make network statement's attribute. */
 struct attr *bgp_attr_default_set(struct attr *attr, u_char origin)
 {
diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
index ea1e736520..d404c9046e 100644
--- a/bgpd/bgp_attr.h
+++ b/bgpd/bgp_attr.h
@@ -241,7 +241,6 @@ extern void bgp_attr_dup(struct attr *, struct attr *);
 extern void bgp_attr_deep_dup(struct attr *, struct attr *);
 extern void bgp_attr_deep_free(struct attr *);
 extern struct attr *bgp_attr_intern(struct attr *attr);
-extern struct attr *bgp_attr_refcount(struct attr *attr);
 extern void bgp_attr_unintern_sub(struct attr *);
 extern void bgp_attr_unintern(struct attr **);
 extern void bgp_attr_flush(struct attr *);
diff --git a/bgpd/bgp_updgrp.c b/bgpd/bgp_updgrp.c
index 9630afb717..05395a583a 100644
--- a/bgpd/bgp_updgrp.c
+++ b/bgpd/bgp_updgrp.c
@@ -1185,7 +1185,7 @@ static void update_subgroup_copy_adj_out(struct update_subgroup *source,
 		aout_copy =
 			bgp_adj_out_alloc(dest, aout->rn, aout->addpath_tx_id);
 		aout_copy->attr =
-			aout->attr ? bgp_attr_refcount(aout->attr) : NULL;
+			aout->attr ? bgp_attr_intern(aout->attr) : NULL;
 	}
 }