From 1bcf3a96de9923af1ab6c95d4f59f90374015336 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 9 Feb 2022 13:44:25 +0200 Subject: [PATCH] bgpd: Use get/set helpers for attr->lcommunity Signed-off-by: Donatas Abraitis --- bgpd/bgp_attr.c | 63 +++++++++++++++++++++++--------------- bgpd/bgp_attr.h | 12 ++++++++ bgpd/bgp_debug.c | 2 +- bgpd/bgp_evpn_vty.c | 18 +++++++---- bgpd/bgp_lcommunity.c | 3 ++ bgpd/bgp_mpath.c | 16 +++++----- bgpd/bgp_route.c | 71 ++++++++++++++++++++++++------------------- bgpd/bgp_routemap.c | 24 ++++++++------- bgpd/bgp_zebra.c | 3 +- 9 files changed, 130 insertions(+), 82 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 97ff4caca5..008998c8b4 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -682,8 +682,8 @@ unsigned int attrhash_key_make(const void *p) if (attr->community) MIX(community_hash_make(attr->community)); - if (attr->lcommunity) - MIX(lcommunity_hash_make(attr->lcommunity)); + if (bgp_attr_get_lcommunity(attr)) + MIX(lcommunity_hash_make(bgp_attr_get_lcommunity(attr))); if (bgp_attr_get_ecommunity(attr)) MIX(ecommunity_hash_make(bgp_attr_get_ecommunity(attr))); if (bgp_attr_get_ipv6_ecommunity(attr)) @@ -737,7 +737,8 @@ bool attrhash_cmp(const void *p1, const void *p2) == bgp_attr_get_ecommunity(attr2) && bgp_attr_get_ipv6_ecommunity(attr1) == bgp_attr_get_ipv6_ecommunity(attr2) - && attr1->lcommunity == attr2->lcommunity + && bgp_attr_get_lcommunity(attr1) + == bgp_attr_get_lcommunity(attr2) && bgp_attr_get_cluster(attr1) == bgp_attr_get_cluster(attr2) && bgp_attr_get_transit(attr1) @@ -853,6 +854,7 @@ struct attr *bgp_attr_intern(struct attr *attr) struct attr *find; struct ecommunity *ecomm = NULL; struct ecommunity *ipv6_ecomm = NULL; + struct lcommunity *lcomm = NULL; /* Intern referenced strucutre. */ if (attr->aspath) { @@ -885,11 +887,12 @@ struct attr *bgp_attr_intern(struct attr *attr) ipv6_ecomm->refcnt++; } - if (attr->lcommunity) { - if (!attr->lcommunity->refcnt) - attr->lcommunity = lcommunity_intern(attr->lcommunity); + lcomm = bgp_attr_get_lcommunity(attr); + if (lcomm) { + if (!lcomm->refcnt) + bgp_attr_set_lcommunity(attr, lcommunity_intern(lcomm)); else - attr->lcommunity->refcnt++; + lcomm->refcnt++; } struct cluster_list *cluster = bgp_attr_get_cluster(attr); @@ -1021,7 +1024,7 @@ struct attr *bgp_attr_aggregate_intern( } if (lcommunity) { - attr.lcommunity = lcommunity; + bgp_attr_set_lcommunity(&attr, lcommunity); attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_LARGE_COMMUNITIES); } @@ -1091,6 +1094,7 @@ void bgp_attr_unintern_sub(struct attr *attr) struct ecommunity *ecomm = NULL; struct ecommunity *ipv6_ecomm = NULL; struct cluster_list *cluster; + struct lcommunity *lcomm = NULL; /* aspath refcount shoud be decrement. */ if (attr->aspath) @@ -1111,9 +1115,10 @@ void bgp_attr_unintern_sub(struct attr *attr) UNSET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_IPV6_EXT_COMMUNITIES)); bgp_attr_set_ipv6_ecommunity(attr, NULL); - if (attr->lcommunity) - lcommunity_unintern(&attr->lcommunity); + lcomm = bgp_attr_get_lcommunity(attr); + lcommunity_unintern(&lcomm); UNSET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_LARGE_COMMUNITIES)); + bgp_attr_set_lcommunity(attr, NULL); cluster = bgp_attr_get_cluster(attr); if (cluster) { @@ -1177,6 +1182,7 @@ void bgp_attr_flush(struct attr *attr) struct ecommunity *ecomm; struct ecommunity *ipv6_ecomm; struct cluster_list *cluster; + struct lcommunity *lcomm; if (attr->aspath && !attr->aspath->refcnt) { aspath_free(attr->aspath); @@ -1192,8 +1198,10 @@ void bgp_attr_flush(struct attr *attr) if (ipv6_ecomm && !ipv6_ecomm->refcnt) ecommunity_free(&ipv6_ecomm); bgp_attr_set_ipv6_ecommunity(attr, NULL); - if (attr->lcommunity && !attr->lcommunity->refcnt) - lcommunity_free(&attr->lcommunity); + lcomm = bgp_attr_get_lcommunity(attr); + if (lcomm && !lcomm->refcnt) + lcommunity_free(&lcomm); + bgp_attr_set_lcommunity(attr, NULL); cluster = bgp_attr_get_cluster(attr); if (cluster && !cluster->refcnt) { @@ -2271,17 +2279,18 @@ bgp_attr_large_community(struct bgp_attr_parser_args *args) * Large community follows new attribute format. */ if (length == 0) { - attr->lcommunity = NULL; + bgp_attr_set_lcommunity(attr, NULL); /* Empty extcomm doesn't seem to be invalid per se */ return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR, args->total); } - attr->lcommunity = lcommunity_parse(stream_pnt(peer->curr), length); + bgp_attr_set_lcommunity( + attr, lcommunity_parse(stream_pnt(peer->curr), length)); /* XXX: fix ecommunity_parse to use stream API */ stream_forward_getp(peer->curr, length); - if (!attr->lcommunity) + if (!bgp_attr_get_lcommunity(attr)) return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR, args->total); @@ -4103,21 +4112,23 @@ bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer, if (CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_SEND_LARGE_COMMUNITY) && (attr->flag & ATTR_FLAG_BIT(BGP_ATTR_LARGE_COMMUNITIES))) { - if (lcom_length(attr->lcommunity) > 255) { + if (lcom_length(bgp_attr_get_lcommunity(attr)) > 255) { stream_putc(s, BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS | BGP_ATTR_FLAG_EXTLEN); stream_putc(s, BGP_ATTR_LARGE_COMMUNITIES); - stream_putw(s, lcom_length(attr->lcommunity)); + stream_putw(s, + lcom_length(bgp_attr_get_lcommunity(attr))); } else { stream_putc(s, BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS); stream_putc(s, BGP_ATTR_LARGE_COMMUNITIES); - stream_putc(s, lcom_length(attr->lcommunity)); + stream_putc(s, + lcom_length(bgp_attr_get_lcommunity(attr))); } - stream_put(s, attr->lcommunity->val, - lcom_length(attr->lcommunity)); + stream_put(s, bgp_attr_get_lcommunity(attr)->val, + lcom_length(bgp_attr_get_lcommunity(attr))); } /* Route Reflector. */ @@ -4547,22 +4558,24 @@ void bgp_dump_routes_attr(struct stream *s, struct attr *attr, /* Large Community attribute. */ if (attr->flag & ATTR_FLAG_BIT(BGP_ATTR_LARGE_COMMUNITIES)) { - if (lcom_length(attr->lcommunity) > 255) { + if (lcom_length(bgp_attr_get_lcommunity(attr)) > 255) { stream_putc(s, BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS | BGP_ATTR_FLAG_EXTLEN); stream_putc(s, BGP_ATTR_LARGE_COMMUNITIES); - stream_putw(s, lcom_length(attr->lcommunity)); + stream_putw(s, + lcom_length(bgp_attr_get_lcommunity(attr))); } else { stream_putc(s, BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS); stream_putc(s, BGP_ATTR_LARGE_COMMUNITIES); - stream_putc(s, lcom_length(attr->lcommunity)); + stream_putc(s, + lcom_length(bgp_attr_get_lcommunity(attr))); } - stream_put(s, attr->lcommunity->val, - lcom_length(attr->lcommunity)); + stream_put(s, bgp_attr_get_lcommunity(attr)->val, + lcom_length(bgp_attr_get_lcommunity(attr))); } /* Add a MP_NLRI attribute to dump the IPv6 next hop */ diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index b38cbf8ef8..9e94704262 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -531,6 +531,18 @@ static inline void bgp_attr_set_ecommunity(struct attr *attr, attr->ecommunity = ecomm; } +static inline struct lcommunity * +bgp_attr_get_lcommunity(const struct attr *attr) +{ + return attr->lcommunity; +} + +static inline void bgp_attr_set_lcommunity(struct attr *attr, + struct lcommunity *lcomm) +{ + attr->lcommunity = lcomm; +} + static inline struct ecommunity * bgp_attr_get_ipv6_ecommunity(const struct attr *attr) { diff --git a/bgpd/bgp_debug.c b/bgpd/bgp_debug.c index 453d53e876..82e05dc53a 100644 --- a/bgpd/bgp_debug.c +++ b/bgpd/bgp_debug.c @@ -416,7 +416,7 @@ bool bgp_dump_attr(struct attr *attr, char *buf, size_t size) if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_LARGE_COMMUNITIES))) snprintf(buf + strlen(buf), size - strlen(buf), ", large-community %s", - lcommunity_str(attr->lcommunity, false)); + lcommunity_str(bgp_attr_get_lcommunity(attr), false)); if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_EXT_COMMUNITIES))) snprintf(buf + strlen(buf), size - strlen(buf), diff --git a/bgpd/bgp_evpn_vty.c b/bgpd/bgp_evpn_vty.c index f377c8352b..caf0444850 100644 --- a/bgpd/bgp_evpn_vty.c +++ b/bgpd/bgp_evpn_vty.c @@ -1246,17 +1246,23 @@ static int bgp_show_ethernet_vpn(struct vty *vty, struct prefix_rd *prd, if (type == bgp_show_type_lcommunity_exact) { struct lcommunity *lcom = output_arg; - if (!pi->attr->lcommunity || - !lcommunity_cmp( - pi->attr->lcommunity, lcom)) + if (!bgp_attr_get_lcommunity( + pi->attr) || + !lcommunity_cmp( + bgp_attr_get_lcommunity( + pi->attr), + lcom)) continue; } if (type == bgp_show_type_lcommunity) { struct lcommunity *lcom = output_arg; - if (!pi->attr->lcommunity || - !lcommunity_match( - pi->attr->lcommunity, lcom)) + if (!bgp_attr_get_lcommunity( + pi->attr) || + !lcommunity_match( + bgp_attr_get_lcommunity( + pi->attr), + lcom)) continue; } if (type == bgp_show_type_community) { diff --git a/bgpd/bgp_lcommunity.c b/bgpd/bgp_lcommunity.c index 9d81968789..60ad75c73b 100644 --- a/bgpd/bgp_lcommunity.c +++ b/bgpd/bgp_lcommunity.c @@ -274,6 +274,9 @@ void lcommunity_unintern(struct lcommunity **lcom) { struct lcommunity *ret; + if (!*lcom) + return; + if ((*lcom)->refcnt) (*lcom)->refcnt--; diff --git a/bgpd/bgp_mpath.c b/bgpd/bgp_mpath.c index 3b92416c96..774953f6f8 100644 --- a/bgpd/bgp_mpath.c +++ b/bgpd/bgp_mpath.c @@ -846,8 +846,9 @@ void bgp_path_info_mpath_aggregate_update(struct bgp_path_info *new_best, ecomm = (bgp_attr_get_ecommunity(&attr)) ? ecommunity_dup(bgp_attr_get_ecommunity(&attr)) : NULL; - lcomm = (attr.lcommunity) ? lcommunity_dup(attr.lcommunity) - : NULL; + lcomm = (bgp_attr_get_lcommunity(&attr)) + ? lcommunity_dup(bgp_attr_get_lcommunity(&attr)) + : NULL; for (mpinfo = bgp_path_info_mpath_first(new_best); mpinfo; mpinfo = bgp_path_info_mpath_next(mpinfo)) { @@ -884,16 +885,17 @@ void bgp_path_info_mpath_aggregate_update(struct bgp_path_info *new_best, bgp_attr_get_ecommunity( mpinfo->attr)); } - if (mpinfo->attr->lcommunity) { + if (bgp_attr_get_lcommunity(mpinfo->attr)) { if (lcomm) { lcommerge = lcommunity_merge( - lcomm, - mpinfo->attr->lcommunity); + lcomm, bgp_attr_get_lcommunity( + mpinfo->attr)); lcomm = lcommunity_uniq_sort(lcommerge); lcommunity_free(&lcommerge); } else lcomm = lcommunity_dup( - mpinfo->attr->lcommunity); + bgp_attr_get_lcommunity( + mpinfo->attr)); } } @@ -908,7 +910,7 @@ void bgp_path_info_mpath_aggregate_update(struct bgp_path_info *new_best, attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_EXT_COMMUNITIES); } if (lcomm) { - attr.lcommunity = lcomm; + bgp_attr_set_lcommunity(&attr, lcomm); attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_LARGE_COMMUNITIES); } diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 792f0082a8..66c5d862a6 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -7046,7 +7046,7 @@ static bool bgp_aggregate_info_same(struct bgp_path_info *pi, uint8_t origin, if (!ecommunity_cmp(bgp_attr_get_ecommunity(pi->attr), ecomm)) return false; - if (!lcommunity_cmp(pi->attr->lcommunity, lcomm)) + if (!lcommunity_cmp(bgp_attr_get_lcommunity(pi->attr), lcomm)) return false; if (!CHECK_FLAG(pi->flags, BGP_PATH_VALID)) @@ -7473,10 +7473,10 @@ void bgp_aggregate_route(struct bgp *bgp, const struct prefix *p, afi_t afi, /* Compute aggregate route's large community. */ - if (pi->attr->lcommunity) + if (bgp_attr_get_lcommunity(pi->attr)) bgp_compute_aggregate_lcommunity_hash( - aggregate, - pi->attr->lcommunity); + aggregate, + bgp_attr_get_lcommunity(pi->attr)); } if (match) bgp_process(bgp, dest, afi, safi); @@ -7596,12 +7596,13 @@ void bgp_aggregate_delete(struct bgp *bgp, const struct prefix *p, afi_t afi, bgp_attr_get_ecommunity( pi->attr)); - if (pi->attr->lcommunity) + if (bgp_attr_get_lcommunity(pi->attr)) /* Remove lcommunity from aggregate. */ bgp_remove_lcomm_from_aggregate_hash( - aggregate, - pi->attr->lcommunity); + aggregate, + bgp_attr_get_lcommunity( + pi->attr)); } } @@ -7714,10 +7715,10 @@ static void bgp_add_route_to_aggregate(struct bgp *bgp, /* Compute aggregate route's large community. */ - if (pinew->attr->lcommunity) + if (bgp_attr_get_lcommunity(pinew->attr)) bgp_compute_aggregate_lcommunity( - aggregate, - pinew->attr->lcommunity); + aggregate, + bgp_attr_get_lcommunity(pinew->attr)); /* Retrieve aggregate route's as-path. */ @@ -7816,12 +7817,11 @@ static void bgp_remove_route_from_aggregate(struct bgp *bgp, afi_t afi, bgp_remove_ecommunity_from_aggregate( aggregate, bgp_attr_get_ecommunity(pi->attr)); - if (pi->attr->lcommunity) + if (bgp_attr_get_lcommunity(pi->attr)) /* Remove lcommunity from aggregate. */ bgp_remove_lcommunity_from_aggregate( - aggregate, - pi->attr->lcommunity); + aggregate, bgp_attr_get_lcommunity(pi->attr)); } /* If this node was suppressed, process the change. */ @@ -10517,14 +10517,16 @@ void route_vty_out_detail(struct vty *vty, struct bgp *bgp, struct bgp_dest *bn, /* Line 6 display Large community */ if (attr->flag & ATTR_FLAG_BIT(BGP_ATTR_LARGE_COMMUNITIES)) { if (json_paths) { - if (!attr->lcommunity->json) - lcommunity_str(attr->lcommunity, true); - json_object_lock(attr->lcommunity->json); - json_object_object_add(json_path, "largeCommunity", - attr->lcommunity->json); + if (!bgp_attr_get_lcommunity(attr)->json) + lcommunity_str(bgp_attr_get_lcommunity(attr), + true); + json_object_lock(bgp_attr_get_lcommunity(attr)->json); + json_object_object_add( + json_path, "largeCommunity", + bgp_attr_get_lcommunity(attr)->json); } else { vty_out(vty, " Large Community: %s\n", - attr->lcommunity->str); + bgp_attr_get_lcommunity(attr)->str); } } @@ -10909,8 +10911,11 @@ static int bgp_show_table(struct vty *vty, struct bgp *bgp, safi_t safi, XFREE(MTYPE_TMP, communities); } - if (!found && pi->attr->lcommunity) { - frrstr_split(pi->attr->lcommunity->str, + if (!found && + bgp_attr_get_lcommunity(pi->attr)) { + frrstr_split(bgp_attr_get_lcommunity( + pi->attr) + ->str, " ", &communities, &num); for (int i = 0; i < num; i++) { const char *com2alias = @@ -11050,25 +11055,28 @@ static int bgp_show_table(struct vty *vty, struct bgp *bgp, safi_t safi, if (type == bgp_show_type_lcommunity) { struct lcommunity *lcom = output_arg; - if (!pi->attr->lcommunity - || !lcommunity_match(pi->attr->lcommunity, - lcom)) + if (!bgp_attr_get_lcommunity(pi->attr) || + !lcommunity_match( + bgp_attr_get_lcommunity(pi->attr), + lcom)) continue; } if (type == bgp_show_type_lcommunity_exact) { struct lcommunity *lcom = output_arg; - if (!pi->attr->lcommunity - || !lcommunity_cmp(pi->attr->lcommunity, - lcom)) + if (!bgp_attr_get_lcommunity(pi->attr) || + !lcommunity_cmp( + bgp_attr_get_lcommunity(pi->attr), + lcom)) continue; } if (type == bgp_show_type_lcommunity_list) { struct community_list *list = output_arg; - if (!lcommunity_list_match(pi->attr->lcommunity, - list)) + if (!lcommunity_list_match( + bgp_attr_get_lcommunity(pi->attr), + list)) continue; } if (type @@ -11076,11 +11084,12 @@ static int bgp_show_table(struct vty *vty, struct bgp *bgp, safi_t safi, struct community_list *list = output_arg; if (!lcommunity_list_exact_match( - pi->attr->lcommunity, list)) + bgp_attr_get_lcommunity(pi->attr), + list)) continue; } if (type == bgp_show_type_lcommunity_all) { - if (!pi->attr->lcommunity) + if (!bgp_attr_get_lcommunity(pi->attr)) continue; } if (type == bgp_show_type_dampend_paths diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 89774875b6..fa03276f64 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -1258,10 +1258,10 @@ route_match_alias(void *rule, const struct prefix *prefix, void *object) return RMAP_MATCH; } - if (path->attr->lcommunity) { + if (bgp_attr_get_lcommunity(path->attr)) { found = false; - frrstr_split(path->attr->lcommunity->str, " ", &communities, - &num); + frrstr_split(bgp_attr_get_lcommunity(path->attr)->str, " ", + &communities, &num); for (int i = 0; i < num; i++) { const char *com2alias = bgp_community2alias(communities[i]); @@ -1521,10 +1521,12 @@ route_match_lcommunity(void *rule, const struct prefix *prefix, void *object) return RMAP_NOMATCH; if (rcom->exact) { - if (lcommunity_list_exact_match(path->attr->lcommunity, list)) + if (lcommunity_list_exact_match( + bgp_attr_get_lcommunity(path->attr), list)) return RMAP_MATCH; } else { - if (lcommunity_list_match(path->attr->lcommunity, list)) + if (lcommunity_list_match(bgp_attr_get_lcommunity(path->attr), + list)) return RMAP_MATCH; } @@ -2301,12 +2303,12 @@ route_set_lcommunity(void *rule, const struct prefix *prefix, void *object) rcs = rule; path = object; attr = path->attr; - old = attr->lcommunity; + old = bgp_attr_get_lcommunity(attr); /* "none" case. */ if (rcs->none) { attr->flag &= ~(ATTR_FLAG_BIT(BGP_ATTR_LARGE_COMMUNITIES)); - attr->lcommunity = NULL; + bgp_attr_set_lcommunity(attr, NULL); /* See the longer comment down below. */ if (old && old->refcnt == 0) @@ -2331,7 +2333,7 @@ route_set_lcommunity(void *rule, const struct prefix *prefix, void *object) lcommunity_free(&old); /* will be intern()'d or attr_flush()'d by bgp_update_main() */ - attr->lcommunity = new; + bgp_attr_set_lcommunity(attr, new); attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_LARGE_COMMUNITIES); @@ -2413,7 +2415,7 @@ route_set_lcommunity_delete(void *rule, const struct prefix *pfx, void *object) path = object; list = community_list_lookup(bgp_clist, rcom->name, rcom->name_hash, LARGE_COMMUNITY_LIST_MASTER); - old = path->attr->lcommunity; + old = bgp_attr_get_lcommunity(path->attr); if (list && old) { merge = lcommunity_list_match_delete(lcommunity_dup(old), list); @@ -2429,12 +2431,12 @@ route_set_lcommunity_delete(void *rule, const struct prefix *pfx, void *object) lcommunity_free(&old); if (new->size == 0) { - path->attr->lcommunity = NULL; + bgp_attr_set_lcommunity(path->attr, NULL); path->attr->flag &= ~ATTR_FLAG_BIT(BGP_ATTR_LARGE_COMMUNITIES); lcommunity_free(&new); } else { - path->attr->lcommunity = new; + bgp_attr_set_lcommunity(path->attr, new); path->attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_LARGE_COMMUNITIES); } diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index e028a7e8d6..6691dc56ca 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -1502,7 +1502,8 @@ void bgp_zebra_announce(struct bgp_dest *dest, const struct prefix *p, if (info->attr->flag & ATTR_FLAG_BIT(BGP_ATTR_LARGE_COMMUNITIES)) - strlcpy(bzo.lcommunity, info->attr->lcommunity->str, + strlcpy(bzo.lcommunity, + bgp_attr_get_lcommunity(info->attr)->str, sizeof(bzo.lcommunity)); strlcpy(bzo.selection_reason, reason, -- 2.39.5