diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index ab80aff81b..331c0ece2d 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -421,14 +421,15 @@ static struct transit *transit_intern(struct transit *transit) return find; } -void transit_unintern(struct transit *transit) +static void transit_unintern(struct transit **transit) { - if (transit->refcnt) - transit->refcnt--; + if ((*transit)->refcnt) + (*transit)->refcnt--; - if (transit->refcnt == 0) { - hash_release(transit_hash, transit); - transit_free(transit); + if ((*transit)->refcnt == 0) { + hash_release(transit_hash, *transit); + transit_free(*transit); + *transit = NULL; } } @@ -851,7 +852,7 @@ void bgp_attr_unintern_sub(struct attr *attr) UNSET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_CLUSTER_LIST)); if (attr->transit) - transit_unintern(attr->transit); + transit_unintern(&attr->transit); if (attr->encap_subtlvs) encap_unintern(&attr->encap_subtlvs, ENCAP_SUBTLV_TYPE); @@ -2483,7 +2484,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr, bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); - return BGP_ATTR_PARSE_ERROR; + ret = BGP_ATTR_PARSE_ERROR; + goto done; } /* Fetch attribute flag and type. */ @@ -2506,7 +2508,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr, bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); - return BGP_ATTR_PARSE_ERROR; + ret = BGP_ATTR_PARSE_ERROR; + goto done; } /* Check extended attribue length bit. */ @@ -2527,7 +2530,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr, bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_MAL_ATTR); - return BGP_ATTR_PARSE_ERROR; + ret = BGP_ATTR_PARSE_ERROR; + goto done; } /* Set type to bitmap to check duplicate attribute. `type' is @@ -2584,7 +2588,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, ndata, ndl + lfl + 1); - return BGP_ATTR_PARSE_ERROR; + ret = BGP_ATTR_PARSE_ERROR; + goto done; } struct bgp_attr_parser_args attr_args = { @@ -2609,7 +2614,7 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr, attr_args.total); if (ret == BGP_ATTR_PARSE_PROCEED) continue; - return ret; + goto done; } /* OK check attribute and store it's value. */ @@ -2687,32 +2692,25 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr, bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_MAL_ATTR); ret = BGP_ATTR_PARSE_ERROR; + goto done; } if (ret == BGP_ATTR_PARSE_EOR) { - if (as4_path) - aspath_unintern(&as4_path); - return ret; + goto done; } - /* If hard error occurred immediately return to the caller. */ if (ret == BGP_ATTR_PARSE_ERROR) { flog_warn(EC_BGP_ATTRIBUTE_PARSE_ERROR, "%s: Attribute %s, parse error", peer->host, lookup_msg(attr_str, type, NULL)); - if (as4_path) - aspath_unintern(&as4_path); - return ret; + goto done; } if (ret == BGP_ATTR_PARSE_WITHDRAW) { - flog_warn( EC_BGP_ATTRIBUTE_PARSE_WITHDRAW, "%s: Attribute %s, parse error - treating as withdrawal", peer->host, lookup_msg(attr_str, type, NULL)); - if (as4_path) - aspath_unintern(&as4_path); - return ret; + goto done; } /* Check the fetched length. */ @@ -2722,9 +2720,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr, peer->host, lookup_msg(attr_str, type, NULL)); bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); - if (as4_path) - aspath_unintern(&as4_path); - return BGP_ATTR_PARSE_ERROR; + ret = BGP_ATTR_PARSE_ERROR; + goto done; } } @@ -2735,9 +2732,9 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr, lookup_msg(attr_str, type, NULL)); bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); - if (as4_path) - aspath_unintern(&as4_path); - return BGP_ATTR_PARSE_ERROR; + + ret = BGP_ATTR_PARSE_ERROR; + goto done; } /* @@ -2756,16 +2753,14 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr, if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP)) && !CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_MP_REACH_NLRI))) { if (bgp_attr_nexthop_valid(peer, attr) < 0) { - return BGP_ATTR_PARSE_ERROR; + ret = BGP_ATTR_PARSE_ERROR; + goto done; } } /* Check all mandatory well-known attributes are present */ - if ((ret = bgp_attr_check(peer, attr)) < 0) { - if (as4_path) - aspath_unintern(&as4_path); - return ret; - } + if ((ret = bgp_attr_check(peer, attr)) < 0) + goto done; /* * At this place we can see whether we got AS4_PATH and/or @@ -2788,28 +2783,10 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr, &as4_aggregator_addr)) { bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_MAL_ATTR); - if (as4_path) - aspath_unintern(&as4_path); - return BGP_ATTR_PARSE_ERROR; + ret = BGP_ATTR_PARSE_ERROR; + goto done; } - /* At this stage, we have done all fiddling with as4, and the - * resulting info is in attr->aggregator resp. attr->aspath - * so we can chuck as4_aggregator and as4_path alltogether in - * order to save memory - */ - if (as4_path) { - aspath_unintern(&as4_path); /* unintern - it is in the hash */ - /* The flag that we got this is still there, but that does not - * do any trouble - */ - } - /* - * The "rest" of the code does nothing with as4_aggregator. - * there is no memory attached specifically which is not part - * of the attr. - * so ignoring just means do nothing. - */ /* * Finally do the checks on the aspath we did not do yet * because we waited for a potentially synthesized aspath. @@ -2817,21 +2794,59 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr, if (attr->flag & (ATTR_FLAG_BIT(BGP_ATTR_AS_PATH))) { ret = bgp_attr_aspath_check(peer, attr); if (ret != BGP_ATTR_PARSE_PROCEED) - return ret; + goto done; } - /* Finally intern unknown attribute. */ + + ret = BGP_ATTR_PARSE_PROCEED; +done: + + /* + * At this stage, we have done all fiddling with as4, and the + * resulting info is in attr->aggregator resp. attr->aspath so + * we can chuck as4_aggregator and as4_path alltogether in order + * to save memory + */ + if (as4_path) { + /* + * unintern - it is in the hash + * The flag that we got this is still there, but that + * does not do any trouble + */ + aspath_unintern(&as4_path); + } + + if (ret != BGP_ATTR_PARSE_ERROR) { + /* Finally intern unknown attribute. */ + if (attr->transit) + attr->transit = transit_intern(attr->transit); + if (attr->encap_subtlvs) + attr->encap_subtlvs = encap_intern(attr->encap_subtlvs, + ENCAP_SUBTLV_TYPE); +#if ENABLE_BGP_VNC + if (attr->vnc_subtlvs) + attr->vnc_subtlvs = encap_intern(attr->vnc_subtlvs, + VNC_SUBTLV_TYPE); +#endif + } else { + if (attr->transit) { + transit_free(attr->transit); + attr->transit = NULL; + } + + bgp_attr_flush_encap(attr); + }; + + /* Sanity checks */ if (attr->transit) - attr->transit = transit_intern(attr->transit); + assert(attr->transit->refcnt > 0); if (attr->encap_subtlvs) - attr->encap_subtlvs = - encap_intern(attr->encap_subtlvs, ENCAP_SUBTLV_TYPE); + assert(attr->encap_subtlvs->refcnt > 0); #if ENABLE_BGP_VNC if (attr->vnc_subtlvs) - attr->vnc_subtlvs = - encap_intern(attr->vnc_subtlvs, VNC_SUBTLV_TYPE); + assert(attr->vnc_subtlvs->refcnt > 0); #endif - return BGP_ATTR_PARSE_PROCEED; + return ret; } /* diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index 13ed3e1ee3..40e87e190a 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -304,9 +304,6 @@ extern unsigned long int attr_unknown_count(void); extern int cluster_loop_check(struct cluster_list *, struct in_addr); extern void cluster_unintern(struct cluster_list *); -/* Transit attribute prototypes. */ -void transit_unintern(struct transit *); - /* Below exported for unit-test purposes only */ struct bgp_attr_parser_args { struct peer *peer;