From: mxyns Date: Fri, 12 Aug 2022 16:00:38 +0000 (+0200) Subject: bgpd: applied styling and fixed warnings X-Git-Tag: base_10.0~282^2~5 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=1cca53e5c645ee011910e16a387c47abde163508;p=matthieu%2Ffrr.git bgpd: applied styling and fixed warnings frrbot found style &/| linter errors fixed bmp_process_one return value warnings and added safety checks fixed const modifier warning in bmp_put_vrftablename_info_tlv added unused attribute to bmp_put_vrftablename_info_tlv remove unused variables in bmp_process_one and bmp_route_update Signed-off-by: Maxence Younsi --- diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index f52c069f3c..68057bcbaf 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -241,19 +241,22 @@ static void bmp_free(struct bmp *bmp) } #define BMP_PEER_TYPE_GLOBAL_INSTANCE 0 -#define BMP_PEER_TYPE_RD_INSTANCE 1 -#define BMP_PEER_TYPE_LOCAL_INSTANCE 2 +#define BMP_PEER_TYPE_RD_INSTANCE 1 +#define BMP_PEER_TYPE_LOCAL_INSTANCE 2 #define BMP_PEER_TYPE_LOC_RIB_INSTANCE 3 -static inline uint64_t bmp_get_peer_distinguisher(struct bmp* bmp, afi_t afi, uint8_t peer_type) { +static inline uint64_t bmp_get_peer_distinguisher(struct bmp *bmp, afi_t afi, + uint8_t peer_type) +{ - /* remove this check when the other peer types get correct peer dist. (RFC7854) impl. */ + /* remove this check when the other peer types get correct peer dist. + // (RFC7854) impl. */ if (peer_type != BMP_PEER_TYPE_LOC_RIB_INSTANCE) return 0; /* sending vrf_id or rd could be turned into an option at some point */ - struct bgp* bgp = bmp->targets->bgp; - struct prefix_rd* prd = &bgp->vpn_policy[afi].tovpn_rd; + struct bgp *bgp = bmp->targets->bgp; + struct prefix_rd *prd = &bgp->vpn_policy[afi].tovpn_rd; /* * default vrf => can't have RD => 0 * vrf => has RD? @@ -261,9 +264,10 @@ static inline uint64_t bmp_get_peer_distinguisher(struct bmp* bmp, afi_t afi, ui * else => use vrf_id and convert it so that * peer_distinguisher is 0::vrf_id */ - return bgp->inst_type == VRF_DEFAULT ? 0 - : prd ? *(uint64_t *)prd->val - : (((uint64_t)htonl(bgp->vrf_id)) << 32); + return bgp->inst_type == VRF_DEFAULT + ? 0 + : prd ? *(uint64_t *)prd->val + : (((uint64_t)htonl(bgp->vrf_id)) << 32); } static void bmp_common_hdr(struct stream *s, uint8_t ver, uint8_t type) @@ -273,7 +277,8 @@ static void bmp_common_hdr(struct stream *s, uint8_t ver, uint8_t type) stream_putc(s, type); } -static void bmp_per_peer_hdr(struct stream *s, struct bgp* bgp, struct peer *peer, uint8_t flags, +static void bmp_per_peer_hdr(struct stream *s, struct bgp *bgp, + struct peer *peer, uint8_t flags, uint8_t peer_type_flag, uint64_t peer_distinguisher, const struct timeval *tv) @@ -298,9 +303,10 @@ static void bmp_per_peer_hdr(struct stream *s, struct bgp* bgp, struct peer *pee stream_put(s, (uint8_t *)&peer_distinguisher, 8); /* Peer Address */ - /* Set to 0 if it's a LOC-RIB INSTANCE (RFC 9069) or if it's not an IPv4/6 address */ - if (is_locrib - || (peer->connection->su.sa.sa_family != AF_INET6 && peer->connection->su.sa.sa_family != AF_INET)) { + /* Set to 0 if it's a LOC-RIB INSTANCE (RFC 9069) or if it's not an + * IPv4/6 address */ + if (is_locrib || (peer->connection->su.sa.sa_family != AF_INET6 && + peer->connection->su.sa.sa_family != AF_INET)) { stream_putl(s, 0); stream_putl(s, 0); stream_putl(s, 0); @@ -315,13 +321,16 @@ static void bmp_per_peer_hdr(struct stream *s, struct bgp* bgp, struct peer *pee } /* Peer AS */ - /* set peer ASN but for LOC-RIB INSTANCE (RFC 9069) put the local bgp ASN if available or 0 */ + /* set peer ASN but for LOC-RIB INSTANCE (RFC 9069) put the local bgp + * ASN if available or 0 */ as_t asn = !is_locrib ? peer->as : bgp ? bgp->as : 0L; stream_putl(s, asn); /* Peer BGP ID */ - /* set router-id but for LOC-RIB INSTANCE (RFC 9069) put the instance router-id if available or 0 */ - struct in_addr* bgp_id = !is_locrib ? &peer->remote_id : bgp ? &bgp->router_id : NULL; + /* set router-id but for LOC-RIB INSTANCE (RFC 9069) put the instance + * router-id if available or 0 */ + struct in_addr *bgp_id = + !is_locrib ? &peer->remote_id : bgp ? &bgp->router_id : NULL; stream_put_in_addr(s, bgp_id); /* Timestamp */ @@ -344,11 +353,12 @@ static void bmp_put_info_tlv(struct stream *s, uint16_t type, stream_put(s, string, len); } -static void bmp_put_vrftablename_info_tlv(struct stream *s, struct bmp *bmp) +static void __attribute__((unused)) +bmp_put_vrftablename_info_tlv(struct stream *s, struct bmp *bmp) { #define BMP_INFO_TYPE_VRFTABLENAME 3 - char *vrftablename = "global"; + const char *vrftablename = "global"; if (bmp->targets->bgp->inst_type != BGP_INSTANCE_TYPE_DEFAULT) { struct vrf *vrf = vrf_lookup_by_id(bmp->targets->bgp->vrf_id); vrftablename = vrf ? vrf->name : NULL; @@ -418,7 +428,8 @@ static struct stream *bmp_peerstate(struct peer *peer, bool down) bmp_common_hdr(s, BMP_VERSION_3, BMP_TYPE_PEER_UP_NOTIFICATION); - bmp_per_peer_hdr(s, peer->bgp, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, + bmp_per_peer_hdr(s, peer->bgp, peer, 0, + BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, &uptime_real); /* Local Address (16 bytes) */ @@ -472,7 +483,8 @@ static struct stream *bmp_peerstate(struct peer *peer, bool down) bmp_common_hdr(s, BMP_VERSION_3, BMP_TYPE_PEER_DOWN_NOTIFICATION); - bmp_per_peer_hdr(s, peer->bgp, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, + bmp_per_peer_hdr(s, peer->bgp, peer, 0, + BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, &uptime_real); type_pos = stream_get_endp(s); @@ -702,7 +714,8 @@ static bool bmp_wrmirror(struct bmp *bmp, struct pullwr *pullwr) s = stream_new(BGP_MAX_PACKET_SIZE); bmp_common_hdr(s, BMP_VERSION_3, BMP_TYPE_ROUTE_MIRRORING); - bmp_per_peer_hdr(s, bmp->targets->bgp, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, + bmp_per_peer_hdr(s, bmp->targets->bgp, peer, 0, + BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, &bmq->tv); /* BMP Mirror TLV. */ @@ -810,7 +823,8 @@ static int bmp_peer_backward(struct peer *peer) return 0; } -static void bmp_eor(struct bmp *bmp, afi_t afi, safi_t safi, uint8_t flags, uint8_t peer_type_flag) +static void bmp_eor(struct bmp *bmp, afi_t afi, safi_t safi, uint8_t flags, + uint8_t peer_type_flag) { struct peer *peer; struct listnode *node; @@ -855,7 +869,10 @@ static void bmp_eor(struct bmp *bmp, afi_t afi, safi_t safi, uint8_t flags, uint bmp_common_hdr(s2, BMP_VERSION_3, BMP_TYPE_ROUTE_MONITORING); - bmp_per_peer_hdr(s2, bmp->targets->bgp, peer, flags, peer_type_flag, bmp_get_peer_distinguisher(bmp, afi, peer_type_flag), NULL); + bmp_per_peer_hdr( + s2, bmp->targets->bgp, peer, flags, peer_type_flag, + bmp_get_peer_distinguisher(bmp, afi, peer_type_flag), + NULL); stream_putl_at(s2, BMP_LENGTH_POS, stream_get_endp(s) + stream_get_endp(s2)); @@ -960,10 +977,9 @@ static struct stream *bmp_withdraw(const struct prefix *p, } static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags, - uint8_t peer_type_flag, - const struct prefix *p, struct prefix_rd *prd, - struct attr *attr, afi_t afi, safi_t safi, - time_t uptime) + uint8_t peer_type_flag, const struct prefix *p, + struct prefix_rd *prd, struct attr *attr, afi_t afi, + safi_t safi, time_t uptime) { struct stream *hdr, *msg; struct timeval tv = { .tv_sec = uptime, .tv_usec = 0 }; @@ -977,7 +993,9 @@ static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags, hdr = stream_new(BGP_MAX_PACKET_SIZE); bmp_common_hdr(hdr, BMP_VERSION_3, BMP_TYPE_ROUTE_MONITORING); - bmp_per_peer_hdr(hdr, bmp->targets->bgp, peer, flags, peer_type_flag, bmp_get_peer_distinguisher(bmp, afi, peer_type_flag), &uptime_real); + bmp_per_peer_hdr(hdr, bmp->targets->bgp, peer, flags, peer_type_flag, + bmp_get_peer_distinguisher(bmp, afi, peer_type_flag), + &uptime_real); stream_putl_at(hdr, BMP_LENGTH_POS, stream_get_endp(hdr) + stream_get_endp(msg)); @@ -1089,9 +1107,12 @@ afibreak: bmp->remote, afi2str(afi), safi2str(safi)); - bmp_eor(bmp, afi, safi, BMP_PEER_FLAG_L, BMP_PEER_TYPE_GLOBAL_INSTANCE); - bmp_eor(bmp, afi, safi, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE); - bmp_eor(bmp, afi, safi, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE); + bmp_eor(bmp, afi, safi, BMP_PEER_FLAG_L, + BMP_PEER_TYPE_GLOBAL_INSTANCE); + bmp_eor(bmp, afi, safi, 0, + BMP_PEER_TYPE_GLOBAL_INSTANCE); + bmp_eor(bmp, afi, safi, 0, + BMP_PEER_TYPE_LOC_RIB_INSTANCE); bmp->afistate[afi][safi] = BMP_AFI_LIVE; bmp->syncafi = AFI_MAX; @@ -1102,12 +1123,14 @@ afibreak: prefix_copy(&bmp->syncpos, bgp_dest_get_prefix(bn)); } - if (bmp->targets->afimon[afi][safi] & BMP_MON_POSTPOLICY - || bmp->targets->afimon[afi][safi] & BMP_MON_LOC_RIB) { + if (bmp->targets->afimon[afi][safi] & BMP_MON_POSTPOLICY || + bmp->targets->afimon[afi][safi] & BMP_MON_LOC_RIB) { for (bpiter = bgp_dest_get_bgp_path_info(bn); bpiter; bpiter = bpiter->next) { - if (!CHECK_FLAG(bpiter->flags, BGP_PATH_VALID) - && !CHECK_FLAG(bpiter->flags, BGP_PATH_SELECTED)) + if (!CHECK_FLAG(bpiter->flags, + BGP_PATH_VALID) && + !CHECK_FLAG(bpiter->flags, + BGP_PATH_SELECTED)) continue; if (bpiter->peer->qobj_node.nid <= bmp->syncpeerid) @@ -1158,21 +1181,20 @@ afibreak: if (bpi && CHECK_FLAG(bpi->flags, BGP_PATH_SELECTED) && CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_LOC_RIB)) { - bmp_monitor(bmp, bpi->peer, 0, - BMP_PEER_TYPE_LOC_RIB_INSTANCE, bn_p, prd, - bpi->attr, afi, safi, bpi->rib_uptime); + bmp_monitor(bmp, bpi->peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, + bn_p, prd, bpi->attr, afi, safi, bpi->rib_uptime); } - if (bpi && CHECK_FLAG(bpi->flags, BGP_PATH_VALID) - && CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_POSTPOLICY)) + if (bpi && CHECK_FLAG(bpi->flags, BGP_PATH_VALID) && + CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_POSTPOLICY)) bmp_monitor(bmp, bpi->peer, BMP_PEER_FLAG_L, BMP_PEER_TYPE_GLOBAL_INSTANCE, bn_p, prd, - bpi->attr, afi, safi, bpi->uptime); + bpi->attr, + afi, safi, bpi->uptime); if (adjin) bmp_monitor(bmp, adjin->peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, - bn_p, prd, adjin->attr, afi, safi, - adjin->uptime); + bn_p, prd, adjin->attr, afi, safi, adjin->uptime); if (bn) bgp_dest_unlock_node(bn); @@ -1282,9 +1304,9 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) break; } - bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, - &bqe->p, prd, bpi ? bpi->attr : NULL, - afi, safi, bpi ? bpi->rib_uptime : monotime(NULL)); + bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, &bqe->p, prd, + bpi ? bpi->attr : NULL, afi, safi, + bpi ? bpi->rib_uptime : monotime(NULL)); written = true; out: @@ -1419,16 +1441,17 @@ static void bmp_wrerr(struct bmp *bmp, struct pullwr *pullwr, bool eof) bmp_free(bmp); } -static struct bmp_queue_entry* bmp_process_one(struct bmp_targets *bt, struct bmp_qhash_head* updhash, struct bmp_qlist_head* updlist, struct bgp *bgp, afi_t afi, - safi_t safi, struct bgp_dest *bn, struct peer *peer) +static struct bmp_queue_entry * +bmp_process_one(struct bmp_targets *bt, struct bmp_qhash_head *updhash, + struct bmp_qlist_head *updlist, struct bgp *bgp, afi_t afi, + safi_t safi, struct bgp_dest *bn, struct peer *peer) { - struct bmp *bmp; struct bmp_queue_entry *bqe, bqeref; size_t refcount; refcount = bmp_session_count(&bt->sessions); if (refcount == 0) - return; + return NULL; memset(&bqeref, 0, sizeof(bqeref)); prefix_copy(&bqeref.p, bgp_dest_get_prefix(bn)); @@ -1445,7 +1468,7 @@ static struct bmp_queue_entry* bmp_process_one(struct bmp_targets *bt, struct bm if (bqe) { if (bqe->refcount >= refcount) /* nothing to do here */ - return; + return NULL; bmp_qlist_del(updlist, bqe); } else { @@ -1460,7 +1483,8 @@ static struct bmp_queue_entry* bmp_process_one(struct bmp_targets *bt, struct bm return bqe; - /* need to update correct queue pos for all sessions of the target after a call to this function */ + /* need to update correct queue pos for all sessions of the target after + * a call to this function */ } static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi, @@ -1482,11 +1506,18 @@ static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi, return 0; frr_each(bmp_targets, &bmpbgp->targets, bt) { - /* check if any monitoring is enabled (ignoring loc-rib since it uses another hook & queue */ + /* check if any monitoring is enabled (ignoring loc-rib since it + * uses another hook & queue */ if (!(bt->afimon[afi][safi] & ~BMP_MON_LOC_RIB)) continue; - struct bmp_queue_entry* last_item = bmp_process_one(bt, &bt->updhash, &bt->updlist, bgp, afi, safi, bn, peer); + struct bmp_queue_entry *last_item = + bmp_process_one(bt, &bt->updhash, &bt->updlist, bgp, + afi, safi, bn, peer); + + if (!last_item) // if bmp_process_one returns NULL we don't have + // anything to do next + continue; frr_each(bmp_session, &bt->sessions, bmp) { if (!bmp->queuepos) @@ -1530,7 +1561,8 @@ static void bmp_stats(struct event *thread) s = stream_new(BGP_MAX_PACKET_SIZE); bmp_common_hdr(s, BMP_VERSION_3, BMP_TYPE_STATISTICS_REPORT); - bmp_per_peer_hdr(s, bt->bgp, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, + bmp_per_peer_hdr(s, bt->bgp, peer, 0, + BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, &tv); count_pos = stream_get_endp(s); @@ -2384,8 +2416,7 @@ DEFPY(bmp_stats_cfg, return CMD_SUCCESS; } -DEFPY(bmp_monitor_cfg, - bmp_monitor_cmd, +DEFPY(bmp_monitor_cfg, bmp_monitor_cmd, "[no] bmp monitor $policy", NO_STR BMP_STR "Send BMP route monitoring messages\n" BGP_AF_STR BGP_AF_STR BGP_AF_STR @@ -2744,18 +2775,21 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, struct bgp_path_info *updated_route, bool withdraw) { - struct bmp_bgp* bmpbgp = bmp_bgp_get(bgp); + struct bmp_bgp *bmpbgp = bmp_bgp_get(bgp); struct peer *peer = updated_route->peer; - const struct prefix *prefix = bgp_dest_get_prefix(updated_route->net); struct bmp_targets *bt; struct bmp *bmp; - afi_t afi_iter; - safi_t safi_iter; - frr_each(bmp_targets, &bmpbgp->targets, bt) { + frr_each (bmp_targets, &bmpbgp->targets, bt) { if (bt->afimon[afi][safi] & BMP_MON_LOC_RIB) { - struct bmp_queue_entry *last_item = bmp_process_one(bt, &bt->locupdhash, &bt->locupdlist, bgp, afi, safi, bn, peer); + struct bmp_queue_entry *last_item = bmp_process_one( + bt, &bt->locupdhash, &bt->locupdlist, bgp, afi, + safi, bn, peer); + + if (!last_item) // if bmp_process_one returns NULL we + // don't have anything to do next + continue; frr_each (bmp_session, &bt->sessions, bmp) { if (!bmp->locrib_queuepos) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 2dd84d457a..d8a87c1fa6 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -3453,7 +3453,8 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, UNSET_FLAG(new_select->flags, BGP_PATH_LINK_BW_CHG); } - /* call bmp hook for loc-rib route update / withdraw after flags were set */ + /* call bmp hook for loc-rib route update / withdraw after flags were + * set */ if (old_select || new_select) { if (old_select) /* route is not installed in locrib anymore */