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 03cf3625b7546ba4e1fb13da14dc537a4c45b682..d3f129475137ab6be4ea1e3a52e411f701aa1bc7 100644 (file)
--- 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 ea1e736520046baa3440dfca7720f6df5a8dce9f..d404c9046e59b1e704cced00f8533b05c7b8a03e 100644 (file)
--- 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 9630afb717209232ccd16497a201fd3dbba62dda..05395a583af346359f2ceed24841256080883991 100644 (file)
--- 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;
        }
 }