From 9d219585e672fa7afe8801e18fb8ba750e369749 Mon Sep 17 00:00:00 2001 From: Maxence Younsi Date: Fri, 22 Jul 2022 12:12:35 +0200 Subject: [PATCH 01/23] bgpd: basic loc rib monitoring (no syncing yet, not rfc compliant encoding) bmp loc rib monitoring rfc 9069 debuts, loc-rib monitoring draft/poc Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 304 +++++++++++++++++++++++++++++++++++++++++------ bgpd/bgp_bmp.h | 7 ++ bgpd/bgp_route.c | 16 +++ bgpd/bgp_route.h | 6 + 4 files changed, 297 insertions(+), 36 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 7270802915e6..d1edba30f187 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -1051,6 +1051,7 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) prefix_copy(&bmp->syncpos, bgp_dest_get_prefix(bn)); } + // TODO BMP add BMP_MON_LOC_RIB case if (bmp->targets->afimon[afi][safi] & BMP_MON_POSTPOLICY) { for (bpiter = bgp_dest_get_bgp_path_info(bn); bpiter; bpiter = bpiter->next) { @@ -1116,24 +1117,123 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) return true; } -static struct bmp_queue_entry *bmp_pull(struct bmp *bmp) +static struct bmp_queue_entry * +bmp_pull_from_queue(struct bmp_qlist_head *list, struct bmp_qhash_head *hash, + struct bmp_queue_entry **queuepos_ptr) { struct bmp_queue_entry *bqe; - bqe = bmp->queuepos; + bqe = *queuepos_ptr; if (!bqe) return NULL; - bmp->queuepos = bmp_qlist_next(&bmp->targets->updlist, bqe); + *queuepos_ptr = bmp_qlist_next(list, bqe); bqe->refcount--; if (!bqe->refcount) { - bmp_qhash_del(&bmp->targets->updhash, bqe); - bmp_qlist_del(&bmp->targets->updlist, bqe); + bmp_qhash_del(hash, bqe); + bmp_qlist_del(list, bqe); } return bqe; } +static inline struct bmp_queue_entry *bmp_pull(struct bmp *bmp) +{ + return bmp_pull_from_queue(&bmp->targets->updlist, + &bmp->targets->updhash, &bmp->queuepos); +} + +static inline struct bmp_queue_entry *bmp_pull_locrib(struct bmp *bmp) +{ + return bmp_pull_from_queue(&bmp->targets->locupdlist, + &bmp->targets->locupdhash, + &bmp->locrib_queuepos); +} + +// TODO BMP_MON_LOCRIB find a way to merge properly this function with +// bmp_wrqueue or abstract it if possible +static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) +{ + + zlog_info("bmp: wrqueue_locrib!"); + struct bmp_queue_entry *bqe; + struct peer *peer; + struct bgp_dest *bn; + bool written = false; + + bqe = bmp_pull_locrib(bmp); + if (!bqe) + return false; + + zlog_info("bmp: got update from queue"); + + afi_t afi = bqe->afi; + safi_t safi = bqe->safi; + + switch (bmp->afistate[afi][safi]) { + case BMP_AFI_INACTIVE: + case BMP_AFI_NEEDSYNC: + goto out; + case BMP_AFI_SYNC: + if (prefix_cmp(&bqe->p, &bmp->syncpos) <= 0) + /* currently syncing but have already passed this + * prefix => send it. */ + break; + + // TODO BMP_MON_LOCRIB check if this is true after implenting + // LOCRIB syncing + /* currently syncing & haven't reached this prefix yet + * => it'll be sent as part of the table sync, no need here */ + zlog_info( + "bmp: skipping direct monitor msg bc will be sent with sync :)"); + goto out; + case BMP_AFI_LIVE: + break; + } + + zlog_info("passed afi state check"); + + peer = QOBJ_GET_TYPESAFE(bqe->peerid, peer); + if (!peer) { + zlog_info("bmp: skipping queued item for deleted peer"); + goto out; + } + if (!peer_established(peer)) { + zlog_info("bmp: not established peer"); + goto out; + } + + bn = bgp_node_lookup(bmp->targets->bgp->rib[afi][safi], &bqe->p); + struct prefix_rd *prd = NULL; + if ((bqe->afi == AFI_L2VPN && bqe->safi == SAFI_EVPN) || + (bqe->safi == SAFI_MPLS_VPN)) + prd = &bqe->rd; + + if (bmp->targets->afimon[afi][safi] & BMP_MON_LOC_RIB) { + zlog_info("bmp: loc rib monitoring on"); + struct bgp_path_info *bpi; + + // TODO BMP_MON_LOC_RIB understand this part more, why iterate + // through the table ? + for (bpi = bn ? bgp_dest_get_bgp_path_info(bn) : NULL; bpi; + bpi = bpi->next) { + // if (!CHECK_FLAG(bpi->flags, + //BGP_PATH_VALID)) continue; + if (bpi->peer == peer) + break; + } + + bmp_monitor(bmp, peer, 5, &bqe->p, prd, bpi ? bpi->attr : NULL, + afi, safi, bpi ? bpi->uptime : monotime(NULL)); + written = true; + } + +out: + if (!bqe->refcount) + XFREE(MTYPE_BMP_QUEUE, bqe); + return written; +} + static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) { struct bmp_queue_entry *bqe; @@ -1181,6 +1281,7 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) &bqe->p, prd); + // TODO BMP add MON_BGP_LOC_RIB case if (bmp->targets->afimon[afi][safi] & BMP_MON_POSTPOLICY) { struct bgp_path_info *bpi; @@ -1235,6 +1336,8 @@ static void bmp_wrfill(struct bmp *bmp, struct pullwr *pullwr) break; if (bmp_wrqueue(bmp, pullwr)) break; + if (bmp_wrqueue_locrib(bmp, pullwr)) + break; if (bmp_wrsync(bmp, pullwr)) break; break; @@ -1633,6 +1736,8 @@ static struct bmp_targets *bmp_targets_get(struct bgp *bgp, const char *name) bmp_session_init(&bt->sessions); bmp_qhash_init(&bt->updhash); bmp_qlist_init(&bt->updlist); + bmp_qhash_init(&bt->locupdhash); + bmp_qlist_init(&bt->locupdlist); bmp_actives_init(&bt->actives); bmp_listeners_init(&bt->listeners); @@ -1663,6 +1768,8 @@ static void bmp_targets_put(struct bmp_targets *bt) bmp_actives_fini(&bt->actives); bmp_qhash_fini(&bt->updhash); bmp_qlist_fini(&bt->updlist); + bmp_qhash_fini(&bt->locupdhash); + bmp_qlist_fini(&bt->locupdlist); XFREE(MTYPE_BMP_ACLNAME, bt->acl_name); XFREE(MTYPE_BMP_ACLNAME, bt->acl6_name); @@ -2206,21 +2313,15 @@ DEFPY(bmp_stats_cfg, return CMD_SUCCESS; } -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 - BGP_AF_STR - BGP_AF_STR - BGP_AF_STR - BGP_AF_STR +DEFPY(bmp_monitor_cfg, bmp_monitor_cmd, + // TODO BMP add loc-rib option + "[no] bmp monitor $policy", + NO_STR BMP_STR + "Send BMP route monitoring messages\n" BGP_AF_STR BGP_AF_STR BGP_AF_STR + BGP_AF_STR BGP_AF_STR BGP_AF_STR BGP_AF_STR "Send state before policy and filter processing\n" - "Send state with policy and filters applied\n") + "Send state with policy and filters applied\n" + "Send state after decision process is applied\n") { int index = 0; uint8_t flag, prev; @@ -2233,7 +2334,12 @@ DEFPY(bmp_monitor_cfg, argv_find_and_parse_afi(argv, argc, &index, &afi); argv_find_and_parse_safi(argv, argc, &index, &safi); - if (policy[1] == 'r') + // TODO BMP set right flag + if (policy[0] == 'l') { + flag = BMP_MON_LOC_RIB; + vty_out(vty, + "changing loc rib monitoring config for this target\n"); + } else if (policy[1] == 'r') flag = BMP_MON_PREPOLICY; else flag = BMP_MON_POSTPOLICY; @@ -2364,23 +2470,29 @@ DEFPY(show_bmp, safi_t safi; FOREACH_AFI_SAFI (afi, safi) { - const char *str = NULL; - - switch (bt->afimon[afi][safi]) { - case BMP_MON_PREPOLICY: - str = "pre-policy"; - break; - case BMP_MON_POSTPOLICY: - str = "post-policy"; - break; - case BMP_MON_PREPOLICY | BMP_MON_POSTPOLICY: - str = "pre-policy and post-policy"; - break; - } - if (!str) + + uint8_t afimon_flag = bt->afimon[afi][safi]; + + if (!afimon_flag) continue; - vty_out(vty, " Route Monitoring %s %s %s\n", - afi2str(afi), safi2str(safi), str); + + const char *pre_str = + afimon_flag & BMP_MON_PREPOLICY + ? "pre-policy " + : ""; + const char *post_str = + afimon_flag & BMP_MON_POSTPOLICY + ? "post-policy " + : ""; + const char *locrib_str = + afimon_flag & BMP_MON_LOC_RIB + ? "loc-rib" + : ""; + + vty_out(vty, + " Route Monitoring %s %s %s%s%s\n", + afi2str(afi), safi2str(safi), pre_str, + post_str, locrib_str); } vty_out(vty, " Listeners:\n"); @@ -2507,6 +2619,9 @@ static int bmp_config_write(struct bgp *bgp, struct vty *vty) vty_out(vty, " bmp monitor %s %s post-policy\n", afi2str_lower(afi), safi2str(safi)); + if (bt->afimon[afi][safi] & BMP_MON_LOC_RIB) + vty_out(vty, " bmp monitor %s %s loc-rib\n", + afi2str(afi), safi2str(safi)); } frr_each (bmp_listeners, &bt->listeners, bl) vty_out(vty, " \n bmp listener %pSU port %d\n", @@ -2555,6 +2670,122 @@ static int bgp_bmp_init(struct event_loop *tm) return 0; } + +// TODO remove "bn", redundant with updated_route->net ? +static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, + struct bgp_dest *bn, + struct bgp_path_info *updated_route, bool withdraw) +{ + + zlog_info( + "bmp: got route update (%s) ! vrf_id=%d, afi/safi=%s, dest=%pRN, bpi=%pRN", + withdraw ? "withdraw" : "update", bgp->vrf_id, + get_afi_safi_str(afi, safi, false), bn, updated_route->net); + + 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); + + zlog_info("bmp: peer %pBP", peer); + + struct bmp_targets *bt; + struct bmp *bmp; + + afi_t afi_iter; + safi_t safi_iter; + + frr_each (bmp_targets, &bmpbgp->targets, bt) { + zlog_info("bmp: targets name=%s", bt->name); + + FOREACH_AFI_SAFI (afi_iter, safi_iter) { + if (bt->afimon[afi_iter][safi_iter]) + zlog_info("bmp: afi/safi=%s, flag=%d", + get_afi_safi_str(afi_iter, safi_iter, + false), + bt->afimon[afi_iter][safi_iter]); + }; + + + uint8_t flags = 5; + zlog_info("bmp wanna monitor : peer=%pBP", updated_route->peer); + zlog_info("flags=%d", flags); + zlog_info("prefix=%pFX", prefix); + zlog_info("attr=%p", updated_route->attr); + zlog_info("afi=%d safi=%d", afi, safi); + zlog_info("uptime=%ld", updated_route->uptime); + + if (bt->afimon[afi][safi] & BMP_MON_LOC_RIB) { + zlog_info("has LOC RIB monitoring on!"); + + struct bmp_queue_entry *bqe, bqeref; + size_t refcount; + + refcount = bmp_session_count(&bt->sessions); + if (refcount == 0) + return 0; + + memset(&bqeref, 0, sizeof(bqeref)); + prefix_copy(&bqeref.p, prefix); + bqeref.peerid = peer->qobj_node.nid; + bqeref.afi = afi; + bqeref.safi = safi; + + zlog_info("before afi/safi check"); + if ((afi == AFI_L2VPN && safi == SAFI_EVPN && + bn->pdest) || + (safi == SAFI_MPLS_VPN)) + prefix_copy( + &bqeref.rd, + (struct prefix_rd *)bgp_dest_get_prefix( + bn->pdest)); + + zlog_info("bmp: before hash check"); + bqe = bmp_qhash_find(&bt->locupdhash, &bqeref); + uint32_t key = bmp_qhash_hkey(&bqeref); + zlog_info("bmp: key = %lu", key); + if (bqe) { + zlog_info("bmp: prefix already registered"); + if (bqe->refcount >= refcount) + /* nothing to do here */ + return 0; + + zlog_info("bmp: removing prefix"); + bmp_qlist_del(&bt->locupdlist, bqe); + zlog_info("bmp: removed prefix"); + } else { + zlog_info( + "bmp: prefix not registered in queue yet"); + bqe = XMALLOC(MTYPE_BMP_QUEUE, sizeof(*bqe)); + zlog_info("bmp: got malloc %p", bqe); + memcpy(bqe, &bqeref, sizeof(*bqe)); + zlog_info("bmp: copied bqeref into bqe"); + + bmp_qhash_add(&bt->locupdhash, bqe); + zlog_info("bmp: added hash"); + } + + zlog_info("bmp: before list add tail"); + bqe->refcount = refcount; + bmp_qlist_add_tail(&bt->locupdlist, bqe); + + zlog_info("bmp: before queuepos update"); + frr_each (bmp_session, &bt->sessions, bmp) { + zlog_info("bmp: session host=%s", bmp->remote); + + if (!bmp->locrib_queuepos) + bmp->locrib_queuepos = bqe; + + pullwr_bump(bmp->pullwr); + }; + + zlog_info("bmp: end"); + } + }; + + return 0; +} + + static int bgp_bmp_module_init(void) { hook_register(bgp_packet_dump, bmp_mirror_packet); @@ -2565,6 +2796,7 @@ static int bgp_bmp_module_init(void) hook_register(bgp_inst_config_write, bmp_config_write); hook_register(bgp_inst_delete, bmp_bgp_del); hook_register(frr_late_init, bgp_bmp_init); + hook_register(bgp_route_update, bmp_route_update); return 0; } diff --git a/bgpd/bgp_bmp.h b/bgpd/bgp_bmp.h index ab7463fadcad..be952b591a96 100644 --- a/bgpd/bgp_bmp.h +++ b/bgpd/bgp_bmp.h @@ -124,6 +124,7 @@ struct bmp { * ahead we need to make sure that refcount is decremented. Also, on * disconnects we need to walk the queue and drop our reference. */ + struct bmp_queue_entry *locrib_queuepos; struct bmp_queue_entry *queuepos; struct bmp_mirrorq *mirrorpos; bool mirror_lost; @@ -221,6 +222,9 @@ struct bmp_targets { */ #define BMP_MON_PREPOLICY (1 << 0) #define BMP_MON_POSTPOLICY (1 << 1) + +// TODO define BMP_MON_LOC_RIB flag +#define BMP_MON_LOC_RIB (1 << 2) uint8_t afimon[AFI_MAX][SAFI_MAX]; bool mirror; @@ -232,6 +236,9 @@ struct bmp_targets { struct bmp_qhash_head updhash; struct bmp_qlist_head updlist; + struct bmp_qhash_head locupdhash; + struct bmp_qlist_head locupdlist; + uint64_t cnt_accept, cnt_aclrefused; QOBJ_FIELDS; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index b627487e7c20..b206f96f41e3 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -85,6 +85,11 @@ DEFINE_HOOK(bgp_rpki_prefix_status, const struct prefix *prefix), (peer, attr, prefix)); +DEFINE_HOOK(bgp_route_update, + (struct bgp * bgp, afi_t afi, safi_t safi, struct bgp_dest *bn, + struct bgp_path_info *updated_route, bool withdraw), + (bgp, afi, safi, bn, updated_route, withdraw)); + /* Extern from bgp_dump.c */ extern const char *bgp_origin_str[]; extern const char *bgp_origin_long_str[]; @@ -3435,6 +3440,7 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, &bgp->t_rmap_def_originate_eval); } + // TODO BMP insert rib update hook if (old_select) bgp_path_info_unset_flag(dest, old_select, BGP_PATH_SELECTED); if (new_select) { @@ -3447,6 +3453,16 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, UNSET_FLAG(new_select->flags, BGP_PATH_LINK_BW_CHG); } + if (old_select || new_select) { + zlog_info("old_select==NULL %s | new_select==NULL %s", + old_select == NULL ? "YES" : "NO", + new_select == NULL ? "YES" : "NO"); + bool is_withdraw = old_select && !new_select; + hook_call(bgp_route_update, bgp, afi, safi, dest, + is_withdraw ? old_select : new_select, is_withdraw); + } + + #ifdef ENABLE_BGP_VNC if ((afi == AFI_IP || afi == AFI_IP6) && (safi == SAFI_UNICAST)) { if (old_select != new_select) { diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index e9f48ea64778..b340327d3be8 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -674,6 +674,12 @@ DECLARE_HOOK(bgp_process, struct peer *peer, bool withdraw), (bgp, afi, safi, bn, peer, withdraw)); +/* called when a route is updated in the rib */ +DECLARE_HOOK(bgp_route_update, + (struct bgp * bgp, afi_t afi, safi_t safi, struct bgp_dest *bn, + struct bgp_path_info *updated_route, bool withdraw), + (bgp, afi, safi, bn, updated_route, withdraw)); + /* BGP show options */ #define BGP_SHOW_OPT_JSON (1 << 0) #define BGP_SHOW_OPT_WIDE (1 << 1) From 66e0c6f82645af8649dce6b2b7542d919c567fbe Mon Sep 17 00:00:00 2001 From: mxyns Date: Fri, 22 Jul 2022 12:13:36 +0200 Subject: [PATCH 02/23] bgpd: peer flag set for loc-rib monitoring (left set to 0 in other cases) set peer type flag to 3 for loc rib monitoring leave to 0 in other cases like before, even though RFC7854 tells us to set it to 0 1 or 2 depending on the case global/rd/local instance Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 78 ++++++++++++++++++++++++++---------------------- bgpd/bgp_bmp.h | 2 +- bgpd/bgp_route.c | 5 ++-- bgpd/bgp_route.h | 2 +- 4 files changed, 47 insertions(+), 40 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index d1edba30f187..ed35e93d755a 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -243,25 +243,26 @@ static void bmp_free(struct bmp *bmp) static void bmp_common_hdr(struct stream *s, uint8_t ver, uint8_t type) { stream_putc(s, ver); - stream_putl(s, 0); //dummy message length. will be set later. + stream_putl(s, 0); /*dummy message length. will be set later. */ stream_putc(s, type); } static void bmp_per_peer_hdr(struct stream *s, struct peer *peer, - uint8_t flags, const struct timeval *tv) + uint8_t flags, uint8_t peer_type_flag, const struct timeval *tv) { char peer_distinguisher[8]; #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_LOC_RIB_INSTANCE 3 #define BMP_PEER_FLAG_V (1 << 7) #define BMP_PEER_FLAG_L (1 << 6) #define BMP_PEER_FLAG_A (1 << 5) /* Peer Type */ - stream_putc(s, BMP_PEER_TYPE_GLOBAL_INSTANCE); + stream_putc(s, peer_type_flag); /* Peer Flags */ if (peer->connection->su.sa.sa_family == AF_INET6) @@ -328,7 +329,7 @@ static int bmp_send_initiation(struct bmp *bmp) bmp_put_info_tlv(s, BMP_INFO_TYPE_SYSNAME, cmd_hostname_get()); len = stream_get_endp(s); - stream_putl_at(s, BMP_LENGTH_POS, len); //message length is set. + stream_putl_at(s, BMP_LENGTH_POS, len); /*message length is set. */ pullwr_write_stream(bmp->pullwr, s); stream_free(s); @@ -375,7 +376,7 @@ 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, 0, &uptime_real); + bmp_per_peer_hdr(s, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, &uptime_real); /* Local Address (16 bytes) */ if (peer->su_local->sa.sa_family == AF_INET6) @@ -428,7 +429,7 @@ 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, 0, &uptime_real); + bmp_per_peer_hdr(s, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, &uptime_real); type_pos = stream_get_endp(s); stream_putc(s, 0); /* placeholder for down reason */ @@ -460,7 +461,7 @@ static struct stream *bmp_peerstate(struct peer *peer, bool down) } len = stream_get_endp(s); - stream_putl_at(s, BMP_LENGTH_POS, len); //message length is set. + stream_putl_at(s, BMP_LENGTH_POS, len); /*message length is set. */ return s; } @@ -618,7 +619,7 @@ static void bmp_wrmirror_lost(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_self, 0, &tv); + bmp_per_peer_hdr(s, bmp->targets->bgp->peer_self, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, &tv); stream_putw(s, BMP_MIRROR_TLV_TYPE_INFO); stream_putw(s, 2); @@ -656,7 +657,7 @@ 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, peer, 0, &bmq->tv); + bmp_per_peer_hdr(s, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, &bmq->tv); /* BMP Mirror TLV. */ stream_putw(s, BMP_MIRROR_TLV_TYPE_BGP_MESSAGE); @@ -807,7 +808,7 @@ static void bmp_eor(struct bmp *bmp, afi_t afi, safi_t safi, uint8_t flags) bmp_common_hdr(s2, BMP_VERSION_3, BMP_TYPE_ROUTE_MONITORING); - bmp_per_peer_hdr(s2, peer, flags, NULL); + bmp_per_peer_hdr(s2, peer, flags, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL); stream_putl_at(s2, BMP_LENGTH_POS, stream_get_endp(s) + stream_get_endp(s2)); @@ -912,9 +913,9 @@ static struct stream *bmp_withdraw(const struct prefix *p, } static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags, - const struct prefix *p, struct prefix_rd *prd, - struct attr *attr, afi_t afi, safi_t safi, - time_t uptime) + uint8_t peer_type_flags, 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 }; @@ -928,7 +929,7 @@ 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, peer, flags, &uptime_real); + bmp_per_peer_hdr(hdr, peer, flags, peer_type_flags, &uptime_real); stream_putl_at(hdr, BMP_LENGTH_POS, stream_get_endp(hdr) + stream_get_endp(msg)); @@ -1051,7 +1052,7 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) prefix_copy(&bmp->syncpos, bgp_dest_get_prefix(bn)); } - // TODO BMP add BMP_MON_LOC_RIB case + /* TODO BMP add BMP_MON_LOC_RIB case */ if (bmp->targets->afimon[afi][safi] & BMP_MON_POSTPOLICY) { for (bpiter = bgp_dest_get_bgp_path_info(bn); bpiter; bpiter = bpiter->next) { @@ -1105,10 +1106,10 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) prd = (struct prefix_rd *)bgp_dest_get_prefix(bmp->syncrdpos); if (bpi) - bmp_monitor(bmp, bpi->peer, BMP_PEER_FLAG_L, bn_p, prd, + bmp_monitor(bmp, bpi->peer, BMP_PEER_FLAG_L, BMP_PEER_TYPE_GLOBAL_INSTANCE, bn_p, prd, bpi->attr, afi, safi, bpi->uptime); if (adjin) - bmp_monitor(bmp, adjin->peer, 0, bn_p, prd, adjin->attr, afi, + bmp_monitor(bmp, adjin->peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, bn_p, prd, adjin->attr, afi, safi, adjin->uptime); if (bn) @@ -1150,8 +1151,9 @@ static inline struct bmp_queue_entry *bmp_pull_locrib(struct bmp *bmp) &bmp->locrib_queuepos); } -// TODO BMP_MON_LOCRIB find a way to merge properly this function with -// bmp_wrqueue or abstract it if possible +/* TODO BMP_MON_LOCRIB find a way to merge properly this function with + * bmp_wrqueue or abstract it if possible + */ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) { @@ -1177,13 +1179,16 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) case BMP_AFI_SYNC: if (prefix_cmp(&bqe->p, &bmp->syncpos) <= 0) /* currently syncing but have already passed this - * prefix => send it. */ + * prefix => send it. + */ break; - // TODO BMP_MON_LOCRIB check if this is true after implenting - // LOCRIB syncing + /* TODO BMP_MON_LOCRIB check if this is true after implenting + * LOCRIB syncing + */ /* currently syncing & haven't reached this prefix yet - * => it'll be sent as part of the table sync, no need here */ + * => it'll be sent as part of the table sync, no need here + */ zlog_info( "bmp: skipping direct monitor msg bc will be sent with sync :)"); goto out; @@ -1205,6 +1210,7 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) bn = bgp_node_lookup(bmp->targets->bgp->rib[afi][safi], &bqe->p); struct prefix_rd *prd = NULL; + if ((bqe->afi == AFI_L2VPN && bqe->safi == SAFI_EVPN) || (bqe->safi == SAFI_MPLS_VPN)) prd = &bqe->rd; @@ -1213,17 +1219,16 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) zlog_info("bmp: loc rib monitoring on"); struct bgp_path_info *bpi; - // TODO BMP_MON_LOC_RIB understand this part more, why iterate - // through the table ? + /* TODO BMP_MON_LOC_RIB understand this part more, why iterate + * through the table ? + */ for (bpi = bn ? bgp_dest_get_bgp_path_info(bn) : NULL; bpi; bpi = bpi->next) { - // if (!CHECK_FLAG(bpi->flags, - //BGP_PATH_VALID)) continue; if (bpi->peer == peer) break; } - bmp_monitor(bmp, peer, 5, &bqe->p, prd, bpi ? bpi->attr : NULL, + bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, &bqe->p, prd, bpi ? bpi->attr : NULL, afi, safi, bpi ? bpi->uptime : monotime(NULL)); written = true; } @@ -1281,7 +1286,7 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) &bqe->p, prd); - // TODO BMP add MON_BGP_LOC_RIB case + /* TODO BMP add MON_BGP_LOC_RIB case */ if (bmp->targets->afimon[afi][safi] & BMP_MON_POSTPOLICY) { struct bgp_path_info *bpi; @@ -1293,7 +1298,7 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) break; } - bmp_monitor(bmp, peer, BMP_PEER_FLAG_L, &bqe->p, prd, + bmp_monitor(bmp, peer, BMP_PEER_FLAG_L, BMP_PEER_TYPE_GLOBAL_INSTANCE, &bqe->p, prd, bpi ? bpi->attr : NULL, afi, safi, bpi ? bpi->uptime : monotime(NULL)); written = true; @@ -1307,7 +1312,7 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) if (adjin->peer == peer) break; } - bmp_monitor(bmp, peer, 0, &bqe->p, prd, + bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, &bqe->p, prd, adjin ? adjin->attr : NULL, afi, safi, adjin ? adjin->uptime : monotime(NULL)); written = true; @@ -1463,7 +1468,7 @@ 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, peer, 0, &tv); + bmp_per_peer_hdr(s, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, &tv); count_pos = stream_get_endp(s); stream_putl(s, 0); @@ -2314,7 +2319,7 @@ DEFPY(bmp_stats_cfg, } DEFPY(bmp_monitor_cfg, bmp_monitor_cmd, - // TODO BMP add loc-rib option + /* TODO BMP add loc-rib option */ "[no] bmp monitor $policy", NO_STR BMP_STR "Send BMP route monitoring messages\n" BGP_AF_STR BGP_AF_STR BGP_AF_STR @@ -2334,7 +2339,7 @@ DEFPY(bmp_monitor_cfg, bmp_monitor_cmd, argv_find_and_parse_afi(argv, argc, &index, &afi); argv_find_and_parse_safi(argv, argc, &index, &safi); - // TODO BMP set right flag + /* TODO BMP set right flag */ if (policy[0] == 'l') { flag = BMP_MON_LOC_RIB; vty_out(vty, @@ -2670,8 +2675,7 @@ static int bgp_bmp_init(struct event_loop *tm) return 0; } - -// TODO remove "bn", redundant with updated_route->net ? +/* TODO remove "bn", redundant with updated_route->net ? */ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, struct bgp_dest *bn, struct bgp_path_info *updated_route, bool withdraw) @@ -2707,6 +2711,7 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, uint8_t flags = 5; + zlog_info("bmp wanna monitor : peer=%pBP", updated_route->peer); zlog_info("flags=%d", flags); zlog_info("prefix=%pFX", prefix); @@ -2742,6 +2747,7 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, zlog_info("bmp: before hash check"); bqe = bmp_qhash_find(&bt->locupdhash, &bqeref); uint32_t key = bmp_qhash_hkey(&bqeref); + zlog_info("bmp: key = %lu", key); if (bqe) { zlog_info("bmp: prefix already registered"); diff --git a/bgpd/bgp_bmp.h b/bgpd/bgp_bmp.h index be952b591a96..152e7eb0e2ef 100644 --- a/bgpd/bgp_bmp.h +++ b/bgpd/bgp_bmp.h @@ -223,7 +223,7 @@ struct bmp_targets { #define BMP_MON_PREPOLICY (1 << 0) #define BMP_MON_POSTPOLICY (1 << 1) -// TODO define BMP_MON_LOC_RIB flag +/* TODO define BMP_MON_LOC_RIB flag */ #define BMP_MON_LOC_RIB (1 << 2) uint8_t afimon[AFI_MAX][SAFI_MAX]; bool mirror; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index b206f96f41e3..7333d5a432f2 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -86,7 +86,7 @@ DEFINE_HOOK(bgp_rpki_prefix_status, (peer, attr, prefix)); DEFINE_HOOK(bgp_route_update, - (struct bgp * bgp, afi_t afi, safi_t safi, struct bgp_dest *bn, + (struct bgp *bgp, afi_t afi, safi_t safi, struct bgp_dest *bn, struct bgp_path_info *updated_route, bool withdraw), (bgp, afi, safi, bn, updated_route, withdraw)); @@ -3440,7 +3440,7 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, &bgp->t_rmap_def_originate_eval); } - // TODO BMP insert rib update hook + /* TODO BMP insert rib update hook */ if (old_select) bgp_path_info_unset_flag(dest, old_select, BGP_PATH_SELECTED); if (new_select) { @@ -3458,6 +3458,7 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, old_select == NULL ? "YES" : "NO", new_select == NULL ? "YES" : "NO"); bool is_withdraw = old_select && !new_select; + hook_call(bgp_route_update, bgp, afi, safi, dest, is_withdraw ? old_select : new_select, is_withdraw); } diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index b340327d3be8..f83453974282 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -676,7 +676,7 @@ DECLARE_HOOK(bgp_process, /* called when a route is updated in the rib */ DECLARE_HOOK(bgp_route_update, - (struct bgp * bgp, afi_t afi, safi_t safi, struct bgp_dest *bn, + (struct bgp *bgp, afi_t afi, safi_t safi, struct bgp_dest *bn, struct bgp_path_info *updated_route, bool withdraw), (bgp, afi, safi, bn, updated_route, withdraw)); From 2c1900e0d11007e43715723d0e33900d59d0cb9b Mon Sep 17 00:00:00 2001 From: mxyns Date: Fri, 22 Jul 2022 12:16:29 +0200 Subject: [PATCH 03/23] bgpd: correct loc rib update queue cleanup empties out and free the locrib specific queue's memory on bmp_close call Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index ed35e93d755a..dfed77ccf066 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -1638,6 +1638,9 @@ static void bmp_close(struct bmp *bmp) while ((bqe = bmp_pull(bmp))) if (!bqe->refcount) XFREE(MTYPE_BMP_QUEUE, bqe); + while ((bqe = bmp_pull_locrib(bmp))) + if (!bqe->refcount) + XFREE(MTYPE_BMP_QUEUE, bqe); EVENT_OFF(bmp->t_read); pullwr_del(bmp->pullwr); From 8f0a017e4f776dba51bae9deb3554c2834535420 Mon Sep 17 00:00:00 2001 From: mxyns Date: Mon, 25 Jul 2022 11:13:52 +0200 Subject: [PATCH 04/23] bgpd: peer distinguisher set to vrf id set peer distinguisher to vrf id temporarily until i find out how to use the rd set for export on the vrf instance associated to this bgp instance Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 58 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 23 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index dfed77ccf066..efdda3bf7c4c 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -248,9 +248,8 @@ static void bmp_common_hdr(struct stream *s, uint8_t ver, uint8_t type) } static void bmp_per_peer_hdr(struct stream *s, struct peer *peer, - uint8_t flags, uint8_t peer_type_flag, const struct timeval *tv) + uint8_t flags, uint8_t peer_type_flag, uint8_t *peer_distinguisher, const struct timeval *tv) { - char peer_distinguisher[8]; #define BMP_PEER_TYPE_GLOBAL_INSTANCE 0 #define BMP_PEER_TYPE_RD_INSTANCE 1 @@ -272,8 +271,9 @@ static void bmp_per_peer_hdr(struct stream *s, struct peer *peer, stream_putc(s, flags); /* Peer Distinguisher */ - memset (&peer_distinguisher[0], 0, 8); - stream_put(s, &peer_distinguisher[0], 8); + uint8_t empty_peer_distinguisher[8] = {0}; + uint64_t peerd = *(uint64_t*)(peer_distinguisher ? peer_distinguisher : empty_peer_distinguisher); + stream_putq(s, peerd); /* Peer Address */ if (peer->connection->su.sa.sa_family == AF_INET6) @@ -376,7 +376,7 @@ 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, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, &uptime_real); + bmp_per_peer_hdr(s, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, &uptime_real); /* Local Address (16 bytes) */ if (peer->su_local->sa.sa_family == AF_INET6) @@ -429,7 +429,7 @@ 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, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, &uptime_real); + bmp_per_peer_hdr(s, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, &uptime_real); type_pos = stream_get_endp(s); stream_putc(s, 0); /* placeholder for down reason */ @@ -619,7 +619,7 @@ static void bmp_wrmirror_lost(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_self, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, &tv); + bmp_per_peer_hdr(s, bmp->targets->bgp->peer_self, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, &tv); stream_putw(s, BMP_MIRROR_TLV_TYPE_INFO); stream_putw(s, 2); @@ -657,7 +657,7 @@ 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, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, &bmq->tv); + bmp_per_peer_hdr(s, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, &bmq->tv); /* BMP Mirror TLV. */ stream_putw(s, BMP_MIRROR_TLV_TYPE_BGP_MESSAGE); @@ -808,7 +808,7 @@ static void bmp_eor(struct bmp *bmp, afi_t afi, safi_t safi, uint8_t flags) bmp_common_hdr(s2, BMP_VERSION_3, BMP_TYPE_ROUTE_MONITORING); - bmp_per_peer_hdr(s2, peer, flags, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL); + bmp_per_peer_hdr(s2, peer, flags, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, NULL); stream_putl_at(s2, BMP_LENGTH_POS, stream_get_endp(s) + stream_get_endp(s2)); @@ -913,9 +913,11 @@ 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_flags, const struct prefix *p, - struct prefix_rd *prd, struct attr *attr, - afi_t afi, safi_t safi, time_t uptime) + uint8_t peer_type_flags, + uint8_t *peer_distinguisher_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 }; @@ -929,7 +931,7 @@ 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, peer, flags, peer_type_flags, &uptime_real); + bmp_per_peer_hdr(hdr, peer, flags, peer_type_flags, peer_distinguisher_flag, &uptime_real); stream_putl_at(hdr, BMP_LENGTH_POS, stream_get_endp(hdr) + stream_get_endp(msg)); @@ -1106,11 +1108,13 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) prd = (struct prefix_rd *)bgp_dest_get_prefix(bmp->syncrdpos); if (bpi) - bmp_monitor(bmp, bpi->peer, BMP_PEER_FLAG_L, BMP_PEER_TYPE_GLOBAL_INSTANCE, bn_p, prd, + bmp_monitor(bmp, bpi->peer, BMP_PEER_FLAG_L, + BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, bn_p, prd, 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); + bmp_monitor(bmp, adjin->peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, + NULL, bn_p, prd, adjin->attr, afi, safi, + adjin->uptime); if (bn) bgp_dest_unlock_node(bn); @@ -1203,8 +1207,8 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) zlog_info("bmp: skipping queued item for deleted peer"); goto out; } - if (!peer_established(peer)) { - zlog_info("bmp: not established peer"); + if (peer != bmp->targets->bgp->peer_self && !peer_established(peer->connection)) { + zlog_info("bmp: peer neither self and nor established"); goto out; } @@ -1224,11 +1228,18 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) */ for (bpi = bn ? bgp_dest_get_bgp_path_info(bn) : NULL; bpi; bpi = bpi->next) { + if (!CHECK_FLAG(bpi->flags, BGP_PATH_SELECTED)) + continue; if (bpi->peer == peer) break; } - bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, &bqe->p, prd, bpi ? bpi->attr : NULL, + uint8_t peer_distinguisher[8] = {0}; + if (bmp->targets->bgp->inst_type != VRF_DEFAULT) { + memcpy(peer_distinguisher, &bmp->targets->bgp->vrf_id, sizeof(vrf_id_t)); + } + bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, peer_distinguisher, + &bqe->p, prd, bpi ? bpi->attr : NULL, afi, safi, bpi ? bpi->uptime : monotime(NULL)); written = true; } @@ -1298,7 +1309,8 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) break; } - bmp_monitor(bmp, peer, BMP_PEER_FLAG_L, BMP_PEER_TYPE_GLOBAL_INSTANCE, &bqe->p, prd, + bmp_monitor(bmp, peer, BMP_PEER_FLAG_L, + BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, &bqe->p, prd, bpi ? bpi->attr : NULL, afi, safi, bpi ? bpi->uptime : monotime(NULL)); written = true; @@ -1312,8 +1324,8 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) if (adjin->peer == peer) break; } - bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, &bqe->p, prd, - adjin ? adjin->attr : NULL, afi, safi, + bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, + &bqe->p, prd, adjin ? adjin->attr : NULL, afi, safi, adjin ? adjin->uptime : monotime(NULL)); written = true; } @@ -1468,7 +1480,7 @@ 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, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, &tv); + bmp_per_peer_hdr(s, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, &tv); count_pos = stream_get_endp(s); stream_putl(s, 0); From 3d07f70f950a2c049c564edc842e2758c6083a22 Mon Sep 17 00:00:00 2001 From: mxyns Date: Tue, 26 Jul 2022 18:15:17 +0200 Subject: [PATCH 05/23] bgpd: fixed bmp vpnv4 monitoring withdraws instead of updates vpnv4 monitoring always sends withdraws bc of wrong lookup call, fixes this Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 74 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index efdda3bf7c4c..7be6b0929d46 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -1176,6 +1176,12 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) afi_t afi = bqe->afi; safi_t safi = bqe->safi; + if (!CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_LOC_RIB)) { + zlog_info("bmp: loc rib monitoring not enabled, ignoring"); + goto out; + } + + switch (bmp->afistate[afi][safi]) { case BMP_AFI_INACTIVE: case BMP_AFI_NEEDSYNC: @@ -1212,38 +1218,54 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) goto out; } + zlog_info("bmp: for prefix=%pFX in vrf=%d afi/safi is %s", &bqe->p, bmp->targets->bgp->vrf_id, get_afi_safi_str(afi, safi, false)); bn = bgp_node_lookup(bmp->targets->bgp->rib[afi][safi], &bqe->p); struct prefix_rd *prd = NULL; - if ((bqe->afi == AFI_L2VPN && bqe->safi == SAFI_EVPN) || - (bqe->safi == SAFI_MPLS_VPN)) + bool is_vpn = (bqe->afi == AFI_L2VPN && bqe->safi == SAFI_EVPN) || (bqe->safi == SAFI_MPLS_VPN); + if (is_vpn) { prd = &bqe->rd; + bn = bgp_safi_node_lookup(bmp->targets->bgp->rib[afi][safi], afi, safi, &bqe->p, &bqe->rd); + } - if (bmp->targets->afimon[afi][safi] & BMP_MON_LOC_RIB) { - zlog_info("bmp: loc rib monitoring on"); - struct bgp_path_info *bpi; - - /* TODO BMP_MON_LOC_RIB understand this part more, why iterate - * through the table ? - */ - for (bpi = bn ? bgp_dest_get_bgp_path_info(bn) : NULL; bpi; - bpi = bpi->next) { - if (!CHECK_FLAG(bpi->flags, BGP_PATH_SELECTED)) - continue; - if (bpi->peer == peer) - break; + zlog_info("bmp: loc rib monitoring on"); + struct bgp_path_info *bpi; + + struct bgp_path_info *pathinfo = NULL; + pathinfo = bgp_dest_get_bgp_path_info(bn); + if (!pathinfo) + zlog_info("bmp: no info on path %pRN", bn); + + + for (bpi = bn ? bgp_dest_get_bgp_path_info(bn) : NULL; bpi; + bpi = bpi->next) { + zlog_info("bmp: path info for dest(bn)=%pRN", bn); + zlog_info("bmp: is null? %s", bpi == NULL ? "yes" : "no"); + if (bpi) { + zlog_info("bmp: type=%d, subtype=%d", bpi->type, bpi->sub_type); + if (bpi->peer) + zlog_info("bmp: peer=%pBP", bpi->peer); + if (bpi->attr) { + zlog_info("bmp: has attr"); + zlog_info("bmp: nh=%pI4", &bpi->attr->nexthop); + } } + if (!CHECK_FLAG(bpi->flags, BGP_PATH_SELECTED)) + continue; + if (bpi->peer == peer) + break; + } - uint8_t peer_distinguisher[8] = {0}; - if (bmp->targets->bgp->inst_type != VRF_DEFAULT) { - memcpy(peer_distinguisher, &bmp->targets->bgp->vrf_id, sizeof(vrf_id_t)); - } - bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, peer_distinguisher, - &bqe->p, prd, bpi ? bpi->attr : NULL, - afi, safi, bpi ? bpi->uptime : monotime(NULL)); - written = true; + uint8_t peer_distinguisher[8] = {0}; + if (bmp->targets->bgp->inst_type != VRF_DEFAULT) { + memcpy(peer_distinguisher, &bmp->targets->bgp->vrf_id, sizeof(vrf_id_t)); } + bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, peer_distinguisher, + &bqe->p, prd, bpi ? bpi->attr : NULL, + afi, safi, bpi ? bpi->uptime : monotime(NULL)); + written = true; + out: if (!bqe->refcount) XFREE(MTYPE_BMP_QUEUE, bqe); @@ -1289,6 +1311,7 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) if (!peer_established(peer->connection)) goto out; + bool is_vpn = (bqe->afi == AFI_L2VPN && bqe->safi == SAFI_EVPN) || (bqe->safi == SAFI_MPLS_VPN); @@ -1296,7 +1319,6 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) bn = bgp_safi_node_lookup(bmp->targets->bgp->rib[afi][safi], safi, &bqe->p, prd); - /* TODO BMP add MON_BGP_LOC_RIB case */ if (bmp->targets->afimon[afi][safi] & BMP_MON_POSTPOLICY) { struct bgp_path_info *bpi; @@ -2753,11 +2775,13 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, zlog_info("before afi/safi check"); if ((afi == AFI_L2VPN && safi == SAFI_EVPN && bn->pdest) || - (safi == SAFI_MPLS_VPN)) + (safi == SAFI_MPLS_VPN)) { + zlog_info("bmp: added prefix rd info to bqe"); prefix_copy( &bqeref.rd, (struct prefix_rd *)bgp_dest_get_prefix( bn->pdest)); + } zlog_info("bmp: before hash check"); bqe = bmp_qhash_find(&bt->locupdhash, &bqeref); From c5512950f5f21e52ca199fa4dd37cd22bf2385a6 Mon Sep 17 00:00:00 2001 From: mxyns Date: Wed, 27 Jul 2022 17:56:00 +0200 Subject: [PATCH 06/23] bgpd: bmp afi/safi sync for loc-rib added afi/safi monitoring synchronisation for loc-rib added peer_type_flag to bmp_eor signature, only set to BMP_PEER_TYPE_LOC_RIB and to 0 in other cases like it was before updated tracelog to include peer_type_flag value Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 45 ++++++++++++++++++++++++++++++++++----------- bgpd/bgp_trace.h | 3 ++- 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 7be6b0929d46..33cf2793f459 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -764,7 +764,7 @@ 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) +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; @@ -772,7 +772,7 @@ static void bmp_eor(struct bmp *bmp, afi_t afi, safi_t safi, uint8_t flags) iana_afi_t pkt_afi = IANA_AFI_IPV4; iana_safi_t pkt_safi = IANA_SAFI_UNICAST; - frrtrace(3, frr_bgp, bmp_eor, afi, safi, flags); + frrtrace(3, frr_bgp, bmp_eor, afi, safi, flags, peer_type_flag); s = stream_new(BGP_MAX_PACKET_SIZE); @@ -808,7 +808,7 @@ static void bmp_eor(struct bmp *bmp, afi_t afi, safi_t safi, uint8_t flags) bmp_common_hdr(s2, BMP_VERSION_3, BMP_TYPE_ROUTE_MONITORING); - bmp_per_peer_hdr(s2, peer, flags, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, NULL); + bmp_per_peer_hdr(s2, peer, flags, peer_type_flag, NULL, NULL); stream_putl_at(s2, BMP_LENGTH_POS, stream_get_endp(s) + stream_get_endp(s2)); @@ -945,6 +945,7 @@ static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags, static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) { + zlog_info("bmp: sync ran!"); afi_t afi; safi_t safi; @@ -1042,8 +1043,10 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) zlog_info("bmp[%s] %s %s table completed (EoR)", bmp->remote, afi2str(afi), safi2str(safi)); - bmp_eor(bmp, afi, safi, BMP_PEER_FLAG_L); - bmp_eor(bmp, afi, safi, 0); + + 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; @@ -1055,10 +1058,12 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) } /* TODO BMP add BMP_MON_LOC_RIB case */ - if (bmp->targets->afimon[afi][safi] & BMP_MON_POSTPOLICY) { + 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)) + if (!CHECK_FLAG(bpiter->flags, BGP_PATH_VALID) + && !CHECK_FLAG(bpiter->flags, BGP_PATH_SELECTED)) continue; if (bpiter->peer->qobj_node.nid <= bmp->syncpeerid) @@ -1107,10 +1112,26 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) (safi == SAFI_MPLS_VPN)) prd = (struct prefix_rd *)bgp_dest_get_prefix(bmp->syncrdpos); - if (bpi) + if (bpi && CHECK_FLAG(bpi->flags, BGP_PATH_SELECTED) + && CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_LOC_RIB)) { + uint8_t peer_distinguisher[8] = {0}; + if (bmp->targets->bgp->inst_type != VRF_DEFAULT) { + memcpy(peer_distinguisher, + &bmp->targets->bgp->vrf_id, + sizeof(vrf_id_t)); + } + + bmp_monitor(bmp, bpi->peer, 0, + BMP_PEER_TYPE_LOC_RIB_INSTANCE, peer_distinguisher, bn_p, + prd, bpi->attr, afi, safi, bpi->uptime); + } + + 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, NULL, bn_p, prd, - bpi->attr, afi, safi, bpi->uptime); + BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, bn_p, + prd, bpi->attr, afi, safi, bpi->uptime); + if (adjin) bmp_monitor(bmp, adjin->peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, bn_p, prd, adjin->attr, afi, safi, @@ -2395,6 +2416,7 @@ DEFPY(bmp_monitor_cfg, bmp_monitor_cmd, if (prev == bt->afimon[afi][safi]) return CMD_SUCCESS; + vty_out(vty, "setting sync states\n"); frr_each (bmp_session, &bt->sessions, bmp) { if (bmp->syncafi == afi && bmp->syncsafi == safi) { bmp->syncafi = AFI_MAX; @@ -2405,7 +2427,8 @@ DEFPY(bmp_monitor_cfg, bmp_monitor_cmd, bmp->afistate[afi][safi] = BMP_AFI_INACTIVE; continue; } - + vty_out(vty, "%s needs sync now\n", get_afi_safi_str(afi, safi, false)); + zlog_info("bmp: %s needs sync now\n", get_afi_safi_str(afi, safi, false)); bmp->afistate[afi][safi] = BMP_AFI_NEEDSYNC; } diff --git a/bgpd/bgp_trace.h b/bgpd/bgp_trace.h index 964393a9f5d1..0980073e2bee 100644 --- a/bgpd/bgp_trace.h +++ b/bgpd/bgp_trace.h @@ -135,11 +135,12 @@ TRACEPOINT_LOGLEVEL(frr_bgp, bmp_mirror_packet, TRACE_INFO) TRACEPOINT_EVENT( frr_bgp, bmp_eor, - TP_ARGS(afi_t, afi, safi_t, safi, uint8_t, flags), + TP_ARGS(afi_t, afi, safi_t, safi, uint8_t, flags, peer_type_flag), TP_FIELDS( ctf_integer(afi_t, afi, afi) ctf_integer(safi_t, safi, safi) ctf_integer(uint8_t, flags, flags) + ctf_integer(uint8_t, peer_type_flag, peer_type_flag) ) ) From f9af3476db4734e6994b24d67269993e7d659ef2 Mon Sep 17 00:00:00 2001 From: mxyns Date: Wed, 27 Jul 2022 20:40:13 +0200 Subject: [PATCH 07/23] bgpd: bmp set peer distinguisher with RD peer distinguisher set to vrf RD if there is one or to vrf_id if in a vrf set to 0 if in default vrf Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 87 +++++++++++++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 37 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 33cf2793f459..2cce7f84f5ce 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -240,6 +240,25 @@ static void bmp_free(struct bmp *bmp) XFREE(MTYPE_BMP_CONN, bmp); } +static uint64_t bmp_get_peer_distinguisher(struct bmp *bmp, afi_t afi) +{ + + // legacy : TODO should be turned into an option at some point + // return bmp->targets->bgp->vrf_id; + 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? + * if yes => use RD value + * 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); +} + static void bmp_common_hdr(struct stream *s, uint8_t ver, uint8_t type) { stream_putc(s, ver); @@ -247,8 +266,10 @@ 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 peer *peer, - uint8_t flags, uint8_t peer_type_flag, uint8_t *peer_distinguisher, const struct timeval *tv) +static void bmp_per_peer_hdr(struct stream *s, struct peer *peer, uint8_t flags, + uint8_t peer_type_flag, + uint64_t peer_distinguisher, + const struct timeval *tv) { #define BMP_PEER_TYPE_GLOBAL_INSTANCE 0 @@ -271,9 +292,7 @@ static void bmp_per_peer_hdr(struct stream *s, struct peer *peer, stream_putc(s, flags); /* Peer Distinguisher */ - uint8_t empty_peer_distinguisher[8] = {0}; - uint64_t peerd = *(uint64_t*)(peer_distinguisher ? peer_distinguisher : empty_peer_distinguisher); - stream_putq(s, peerd); + stream_put(s, (uint8_t *)&peer_distinguisher, 8); /* Peer Address */ if (peer->connection->su.sa.sa_family == AF_INET6) @@ -376,7 +395,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, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, &uptime_real); + bmp_per_peer_hdr(s, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, + &uptime_real); /* Local Address (16 bytes) */ if (peer->su_local->sa.sa_family == AF_INET6) @@ -429,7 +449,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, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, &uptime_real); + bmp_per_peer_hdr(s, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, + &uptime_real); type_pos = stream_get_endp(s); stream_putc(s, 0); /* placeholder for down reason */ @@ -619,7 +640,8 @@ static void bmp_wrmirror_lost(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_self, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, &tv); + bmp_per_peer_hdr(s, bmp->targets->bgp->peer_self, 0, + BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, &tv); stream_putw(s, BMP_MIRROR_TLV_TYPE_INFO); stream_putw(s, 2); @@ -657,7 +679,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, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, &bmq->tv); + bmp_per_peer_hdr(s, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, + &bmq->tv); /* BMP Mirror TLV. */ stream_putw(s, BMP_MIRROR_TLV_TYPE_BGP_MESSAGE); @@ -808,7 +831,7 @@ 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, peer, flags, peer_type_flag, NULL, NULL); + bmp_per_peer_hdr(s2, peer, flags, peer_type_flag, 0, NULL); stream_putl_at(s2, BMP_LENGTH_POS, stream_get_endp(s) + stream_get_endp(s2)); @@ -914,7 +937,7 @@ 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_flags, - uint8_t *peer_distinguisher_flag, + uint64_t peer_distinguisher_flag, const struct prefix *p, struct prefix_rd *prd, struct attr *attr, afi_t afi, safi_t safi, time_t uptime) @@ -1112,29 +1135,22 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) (safi == SAFI_MPLS_VPN)) prd = (struct prefix_rd *)bgp_dest_get_prefix(bmp->syncrdpos); - if (bpi && CHECK_FLAG(bpi->flags, BGP_PATH_SELECTED) - && CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_LOC_RIB)) { - uint8_t peer_distinguisher[8] = {0}; - if (bmp->targets->bgp->inst_type != VRF_DEFAULT) { - memcpy(peer_distinguisher, - &bmp->targets->bgp->vrf_id, - sizeof(vrf_id_t)); - } - - bmp_monitor(bmp, bpi->peer, 0, - BMP_PEER_TYPE_LOC_RIB_INSTANCE, peer_distinguisher, bn_p, - prd, bpi->attr, afi, safi, bpi->uptime); + 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, + bmp_get_peer_distinguisher(bmp, afi), bn_p, prd, + bpi->attr, afi, safi, bpi->uptime); } 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, NULL, bn_p, - prd, bpi->attr, afi, safi, bpi->uptime); + BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, bn_p, prd, + bpi->attr, afi, safi, bpi->uptime); if (adjin) bmp_monitor(bmp, adjin->peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, - NULL, bn_p, prd, adjin->attr, afi, safi, + 0, bn_p, prd, adjin->attr, afi, safi, adjin->uptime); if (bn) @@ -1277,14 +1293,10 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) break; } - uint8_t peer_distinguisher[8] = {0}; - if (bmp->targets->bgp->inst_type != VRF_DEFAULT) { - memcpy(peer_distinguisher, &bmp->targets->bgp->vrf_id, sizeof(vrf_id_t)); - } - - bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, peer_distinguisher, - &bqe->p, prd, bpi ? bpi->attr : NULL, - afi, safi, bpi ? bpi->uptime : monotime(NULL)); + bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, + bmp_get_peer_distinguisher(bmp, afi), &bqe->p, prd, + bpi ? bpi->attr : NULL, afi, safi, + bpi ? bpi->uptime : monotime(NULL)); written = true; out: @@ -1353,7 +1365,7 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) } bmp_monitor(bmp, peer, BMP_PEER_FLAG_L, - BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, &bqe->p, prd, + BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, &bqe->p, prd, bpi ? bpi->attr : NULL, afi, safi, bpi ? bpi->uptime : monotime(NULL)); written = true; @@ -1367,7 +1379,7 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) if (adjin->peer == peer) break; } - bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, + bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, &bqe->p, prd, adjin ? adjin->attr : NULL, afi, safi, adjin ? adjin->uptime : monotime(NULL)); written = true; @@ -1523,7 +1535,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, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, NULL, &tv); + bmp_per_peer_hdr(s, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, + &tv); count_pos = stream_get_endp(s); stream_putl(s, 0); From f83857832f6083b27ea40fda1bd32a2095fbfc27 Mon Sep 17 00:00:00 2001 From: mxyns Date: Thu, 28 Jul 2022 16:11:47 +0200 Subject: [PATCH 08/23] bgpd: bmp loc-rib RFC9069 compliant monitoring messages set field peer bgp id to the peer's remote id in every case except loc-rib (RFC9069 case) in which we put the bgp instance's router-id if available or 0-filled if not available set field peer asn to local primary bgp asn in case of loc-rib instance (RFC9069) else it's set to the peer's asn set field peer address to 0 in loc-rib instance (RFC9069 case) and to the peer's address in other cases had to pass struct bgp reference to bmp_per_peer_hdr to access router-id and such, but it's always safely accessed when used Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 48 +++++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 2cce7f84f5ce..e75f08eaff0f 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -243,15 +243,16 @@ static void bmp_free(struct bmp *bmp) static uint64_t bmp_get_peer_distinguisher(struct bmp *bmp, afi_t afi) { - // legacy : TODO should be turned into an option at some point - // return bmp->targets->bgp->vrf_id; + /* legacy : TODO should be turned into an option at some point + * return bmp->targets->bgp->vrf_id; + */ 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? - * if yes => use RD value - * else => use vrf_id and convert it so that + * if yes => use RD value + * else => use vrf_id and convert it so that * peer_distinguisher is 0::vrf_id */ return bgp->inst_type == VRF_DEFAULT ? 0 @@ -266,7 +267,7 @@ 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 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) @@ -281,6 +282,8 @@ static void bmp_per_peer_hdr(struct stream *s, struct peer *peer, uint8_t flags, #define BMP_PEER_FLAG_L (1 << 6) #define BMP_PEER_FLAG_A (1 << 5) + bool is_locrib = peer_type_flag == BMP_PEER_TYPE_LOC_RIB_INSTANCE; + /* Peer Type */ stream_putc(s, peer_type_flag); @@ -295,25 +298,32 @@ static void bmp_per_peer_hdr(struct stream *s, struct peer *peer, uint8_t flags, stream_put(s, (uint8_t *)&peer_distinguisher, 8); /* Peer Address */ - if (peer->connection->su.sa.sa_family == AF_INET6) - stream_put(s, &peer->connection->su.sin6.sin6_addr, 16); - else if (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); - stream_put_in_addr(s, &peer->connection->su.sin.sin_addr); - } else { stream_putl(s, 0); + } else if (peer->connection->su.sa.sa_family == AF_INET6) + stream_put(s, &peer->connection->su.sin6.sin6_addr, 16); + else if (peer->connection->su.sa.sa_family == AF_INET) { stream_putl(s, 0); stream_putl(s, 0); stream_putl(s, 0); + stream_put_in_addr(s, &peer->connection->su.sin.sin_addr); } /* Peer AS */ - stream_putl(s, peer->as); + /* 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 */ - stream_put_in_addr(s, &peer->remote_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; + zlog_info("bmp: per peer header is_locrib=%d, peer->remote_id=%pI4, bgp->router_id=%pI4, selected=%pI4", is_locrib, &peer->remote_id, &bgp->router_id, bgp_id); + stream_put_in_addr(s, bgp_id); /* Timestamp */ if (tv) { @@ -395,7 +405,7 @@ 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, 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) */ @@ -449,7 +459,7 @@ 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, 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); @@ -640,7 +650,7 @@ static void bmp_wrmirror_lost(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_self, 0, + bmp_per_peer_hdr(s, bmp->targets->bgp, bmp->targets->bgp->peer_self, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, &tv); stream_putw(s, BMP_MIRROR_TLV_TYPE_INFO); @@ -679,7 +689,7 @@ 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, 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. */ @@ -831,7 +841,7 @@ 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, peer, flags, peer_type_flag, 0, NULL); + bmp_per_peer_hdr(s2, bmp->targets->bgp, peer, flags, peer_type_flag, 0, NULL); stream_putl_at(s2, BMP_LENGTH_POS, stream_get_endp(s) + stream_get_endp(s2)); @@ -954,7 +964,7 @@ 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, peer, flags, peer_type_flags, peer_distinguisher_flag, &uptime_real); + bmp_per_peer_hdr(hdr, bmp->targets->bgp, peer, flags, peer_type_flags, peer_distinguisher_flag, &uptime_real); stream_putl_at(hdr, BMP_LENGTH_POS, stream_get_endp(hdr) + stream_get_endp(msg)); @@ -1535,7 +1545,7 @@ 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, 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); From 24f3d9ff1a8680df1e2c11c457bec6dca88b82c0 Mon Sep 17 00:00:00 2001 From: mxyns Date: Thu, 28 Jul 2022 18:23:14 +0200 Subject: [PATCH 09/23] bgpd: safer vrf/table name (RFC9069) info tlv vrf_id_to_name is used for display values only and returns "Unknown" when the vrf is not found doing a manual lookup and not providing any tlv when the vrf is not found should be better Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index e75f08eaff0f..78528b080ac6 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -344,6 +344,19 @@ 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) +{ + +#define BMP_INFO_TYPE_VRFTABLENAME 3 + 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; + } + if (vrftablename != NULL) + bmp_put_info_tlv(s, BMP_INFO_TYPE_VRFTABLENAME, vrftablename); +} + static int bmp_send_initiation(struct bmp *bmp) { int len; From bbf6cb68678099b006e29e768359fcf252e0d809 Mon Sep 17 00:00:00 2001 From: mxyns Date: Thu, 28 Jul 2022 18:54:10 +0200 Subject: [PATCH 10/23] bgpd: bmp loc-rib end-of-rib message add peer distinguisher added peer distinguisher for BMP_PEER_TYPE_LOC_RIB_INSTANCE in bmp_eor Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 78528b080ac6..15b9998e3cbe 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -854,7 +854,9 @@ 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, 0, NULL); + + uint64_t peerd = peer_type_flag == BMP_PEER_TYPE_LOC_RIB_INSTANCE ? bmp_get_peer_distinguisher(bmp, afi) : 0; + bmp_per_peer_hdr(s2, bmp->targets->bgp, peer, flags, peer_type_flag, peerd, NULL); stream_putl_at(s2, BMP_LENGTH_POS, stream_get_endp(s) + stream_get_endp(s2)); From 6691e1dc61636598ed8ae8792f32e802365e03af Mon Sep 17 00:00:00 2001 From: mxyns Date: Fri, 29 Jul 2022 17:41:30 +0200 Subject: [PATCH 11/23] bgpd: temporary set timestamp to 0 for loc rib monitoring messages set timestamp to 0 for loc rib monitoring messages as path selection time is not available atm this is temporary and tv is meant to be set to the path selection/install time at some point Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 15b9998e3cbe..6a48cb3e5ca4 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -326,7 +326,8 @@ static void bmp_per_peer_hdr(struct stream *s, struct bgp* bgp, struct peer *pee stream_put_in_addr(s, bgp_id); /* Timestamp */ - if (tv) { + /* set to 0 for LOC-RIB INSTANCE as install uptime is not saved atm */ + if (!is_locrib && tv) { stream_putl(s, tv->tv_sec); stream_putl(s, tv->tv_usec); } else { From 90ffa97e3817115a3076787dff1b98698c9d4034 Mon Sep 17 00:00:00 2001 From: mxyns Date: Fri, 29 Jul 2022 20:06:08 +0200 Subject: [PATCH 12/23] bgpd: beginning to add rib_uptime field for loc-rib timestamp added time_t field to bgp_path_info set value before bgp dp hook is called value not set in the msg yet, testing and double checking is needed before Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 4 ++-- bgpd/bgp_route.c | 6 ++++++ bgpd/bgp_route.h | 1 + 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 6a48cb3e5ca4..f74647f09437 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -1165,7 +1165,7 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_LOC_RIB)) { bmp_monitor(bmp, bpi->peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, bmp_get_peer_distinguisher(bmp, afi), bn_p, prd, - bpi->attr, afi, safi, bpi->uptime); + bpi->attr, afi, safi, bpi->rib_uptime); } if (bpi && CHECK_FLAG(bpi->flags, BGP_PATH_VALID) @@ -1322,7 +1322,7 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, bmp_get_peer_distinguisher(bmp, afi), &bqe->p, prd, bpi ? bpi->attr : NULL, afi, safi, - bpi ? bpi->uptime : monotime(NULL)); + bpi ? bpi->rib_uptime : monotime(NULL)); written = true; out: diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 7333d5a432f2..9353b64f305b 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -3457,6 +3457,11 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, zlog_info("old_select==NULL %s | new_select==NULL %s", old_select == NULL ? "YES" : "NO", new_select == NULL ? "YES" : "NO"); + + if (old_select) /* route is not installed in locrib anymore */ + old_select->rib_uptime = (time_t)(-1L); + if (new_select) /* route is installed in locrib from now on */ + new_select->rib_uptime = bgp_clock(); bool is_withdraw = old_select && !new_select; hook_call(bgp_route_update, bgp, afi, safi, dest, @@ -3989,6 +3994,7 @@ struct bgp_path_info *info_make(int type, int sub_type, unsigned short instance, new->peer = peer; new->attr = attr; new->uptime = monotime(NULL); + new->rib_uptime = (time_t)(-1L); new->net = dest; return new; } diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index f83453974282..232d680188e5 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -295,6 +295,7 @@ struct bgp_path_info { /* Uptime. */ time_t uptime; + time_t rib_uptime; /* reference count */ int lock; From 257ca34e94c7d637f7df9438585288516ea04318 Mon Sep 17 00:00:00 2001 From: Maxou Date: Mon, 8 Aug 2022 13:39:17 +0200 Subject: [PATCH 13/23] bgpd: cleanup bmp_get_peer_distinguisher function cleaner implementation and use of the new get peer distinguisher function can be now used for other cases of RFC7854 that are not supported atm Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 53 ++++++++++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index f74647f09437..b7f35d811589 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -240,14 +240,20 @@ static void bmp_free(struct bmp *bmp) XFREE(MTYPE_BMP_CONN, bmp); } -static uint64_t bmp_get_peer_distinguisher(struct bmp *bmp, afi_t afi) -{ +#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_LOC_RIB_INSTANCE 3 - /* legacy : TODO should be turned into an option at some point - * return bmp->targets->bgp->vrf_id; - */ - struct bgp *bgp = bmp->targets->bgp; - struct prefix_rd *prd = &bgp->vpn_policy[afi].tovpn_rd; +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. */ + 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; /* * default vrf => can't have RD => 0 * vrf => has RD? @@ -272,12 +278,6 @@ static void bmp_per_peer_hdr(struct stream *s, struct bgp* bgp, struct peer *pee uint64_t peer_distinguisher, const struct timeval *tv) { - -#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_LOC_RIB_INSTANCE 3 - #define BMP_PEER_FLAG_V (1 << 7) #define BMP_PEER_FLAG_L (1 << 6) #define BMP_PEER_FLAG_A (1 << 5) @@ -856,8 +856,7 @@ 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); - uint64_t peerd = peer_type_flag == BMP_PEER_TYPE_LOC_RIB_INSTANCE ? bmp_get_peer_distinguisher(bmp, afi) : 0; - bmp_per_peer_hdr(s2, bmp->targets->bgp, peer, flags, peer_type_flag, peerd, 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)); @@ -962,8 +961,7 @@ 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_flags, - uint64_t peer_distinguisher_flag, + 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) @@ -980,7 +978,7 @@ 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_flags, peer_distinguisher_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)); @@ -1162,21 +1160,21 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) prd = (struct prefix_rd *)bgp_dest_get_prefix(bmp->syncrdpos); 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, - bmp_get_peer_distinguisher(bmp, afi), bn_p, prd, + 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); } 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, 0, bn_p, prd, + BMP_PEER_TYPE_GLOBAL_INSTANCE, bn_p, prd, bpi->attr, afi, safi, bpi->uptime); if (adjin) bmp_monitor(bmp, adjin->peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, - 0, bn_p, prd, adjin->attr, afi, safi, + bn_p, prd, adjin->attr, afi, safi, adjin->uptime); if (bn) @@ -1320,9 +1318,8 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) } bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, - bmp_get_peer_distinguisher(bmp, afi), &bqe->p, prd, - bpi ? bpi->attr : NULL, afi, safi, - bpi ? bpi->rib_uptime : monotime(NULL)); + &bqe->p, prd, bpi ? bpi->attr : NULL, + afi, safi, bpi ? bpi->rib_uptime : monotime(NULL)); written = true; out: @@ -1391,7 +1388,7 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) } bmp_monitor(bmp, peer, BMP_PEER_FLAG_L, - BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, &bqe->p, prd, + BMP_PEER_TYPE_GLOBAL_INSTANCE, &bqe->p, prd, bpi ? bpi->attr : NULL, afi, safi, bpi ? bpi->uptime : monotime(NULL)); written = true; @@ -1405,7 +1402,7 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) if (adjin->peer == peer) break; } - bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, + bmp_monitor(bmp, peer, 0, BMP_PEER_TYPE_GLOBAL_INSTANCE, &bqe->p, prd, adjin ? adjin->attr : NULL, afi, safi, adjin ? adjin->uptime : monotime(NULL)); written = true; From 0a09b4905e3e4e9e4de68975b81012410e5fa196 Mon Sep 17 00:00:00 2001 From: Maxou Date: Mon, 8 Aug 2022 13:44:24 +0200 Subject: [PATCH 14/23] bgpd: removed temporary dev logs dev logs cleanup Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 88 ++++---------------------------------------------- 1 file changed, 6 insertions(+), 82 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index b7f35d811589..8ac17e22e886 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -322,7 +322,6 @@ static void bmp_per_peer_hdr(struct stream *s, struct bgp* bgp, struct peer *pee /* 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; - zlog_info("bmp: per peer header is_locrib=%d, peer->remote_id=%pI4, bgp->router_id=%pI4, selected=%pI4", is_locrib, &peer->remote_id, &bgp->router_id, bgp_id); stream_put_in_addr(s, bgp_id); /* Timestamp */ @@ -992,7 +991,6 @@ static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags, static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) { - zlog_info("bmp: sync ran!"); afi_t afi; safi_t safi; @@ -1222,7 +1220,6 @@ static inline struct bmp_queue_entry *bmp_pull_locrib(struct bmp *bmp) static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) { - zlog_info("bmp: wrqueue_locrib!"); struct bmp_queue_entry *bqe; struct peer *peer; struct bgp_dest *bn; @@ -1232,13 +1229,10 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) if (!bqe) return false; - zlog_info("bmp: got update from queue"); - afi_t afi = bqe->afi; safi_t safi = bqe->safi; if (!CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_LOC_RIB)) { - zlog_info("bmp: loc rib monitoring not enabled, ignoring"); goto out; } @@ -1267,19 +1261,16 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) break; } - zlog_info("passed afi state check"); - peer = QOBJ_GET_TYPESAFE(bqe->peerid, peer); if (!peer) { - zlog_info("bmp: skipping queued item for deleted peer"); + // skipping queued item for deleted peer goto out; } if (peer != bmp->targets->bgp->peer_self && !peer_established(peer->connection)) { - zlog_info("bmp: peer neither self and nor established"); + // peer is neither self, nor established goto out; } - zlog_info("bmp: for prefix=%pFX in vrf=%d afi/safi is %s", &bqe->p, bmp->targets->bgp->vrf_id, get_afi_safi_str(afi, safi, false)); bn = bgp_node_lookup(bmp->targets->bgp->rib[afi][safi], &bqe->p); struct prefix_rd *prd = NULL; @@ -1289,28 +1280,13 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) bn = bgp_safi_node_lookup(bmp->targets->bgp->rib[afi][safi], afi, safi, &bqe->p, &bqe->rd); } - zlog_info("bmp: loc rib monitoring on"); struct bgp_path_info *bpi; struct bgp_path_info *pathinfo = NULL; pathinfo = bgp_dest_get_bgp_path_info(bn); - if (!pathinfo) - zlog_info("bmp: no info on path %pRN", bn); - for (bpi = bn ? bgp_dest_get_bgp_path_info(bn) : NULL; bpi; bpi = bpi->next) { - zlog_info("bmp: path info for dest(bn)=%pRN", bn); - zlog_info("bmp: is null? %s", bpi == NULL ? "yes" : "no"); - if (bpi) { - zlog_info("bmp: type=%d, subtype=%d", bpi->type, bpi->sub_type); - if (bpi->peer) - zlog_info("bmp: peer=%pBP", bpi->peer); - if (bpi->attr) { - zlog_info("bmp: has attr"); - zlog_info("bmp: nh=%pI4", &bpi->attr->nexthop); - } - } if (!CHECK_FLAG(bpi->flags, BGP_PATH_SELECTED)) continue; if (bpi->peer == peer) @@ -2452,7 +2428,6 @@ DEFPY(bmp_monitor_cfg, bmp_monitor_cmd, if (prev == bt->afimon[afi][safi]) return CMD_SUCCESS; - vty_out(vty, "setting sync states\n"); frr_each (bmp_session, &bt->sessions, bmp) { if (bmp->syncafi == afi && bmp->syncsafi == safi) { bmp->syncafi = AFI_MAX; @@ -2463,8 +2438,7 @@ DEFPY(bmp_monitor_cfg, bmp_monitor_cmd, bmp->afistate[afi][safi] = BMP_AFI_INACTIVE; continue; } - vty_out(vty, "%s needs sync now\n", get_afi_safi_str(afi, safi, false)); - zlog_info("bmp: %s needs sync now\n", get_afi_safi_str(afi, safi, false)); + bmp->afistate[afi][safi] = BMP_AFI_NEEDSYNC; } @@ -2777,46 +2751,16 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, struct bgp_path_info *updated_route, bool withdraw) { - zlog_info( - "bmp: got route update (%s) ! vrf_id=%d, afi/safi=%s, dest=%pRN, bpi=%pRN", - withdraw ? "withdraw" : "update", bgp->vrf_id, - get_afi_safi_str(afi, safi, false), bn, updated_route->net); - - 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); - - zlog_info("bmp: peer %pBP", peer); - struct bmp_targets *bt; struct bmp *bmp; afi_t afi_iter; safi_t safi_iter; - - frr_each (bmp_targets, &bmpbgp->targets, bt) { - zlog_info("bmp: targets name=%s", bt->name); - - FOREACH_AFI_SAFI (afi_iter, safi_iter) { - if (bt->afimon[afi_iter][safi_iter]) - zlog_info("bmp: afi/safi=%s, flag=%d", - get_afi_safi_str(afi_iter, safi_iter, - false), - bt->afimon[afi_iter][safi_iter]); - }; - - - uint8_t flags = 5; - - zlog_info("bmp wanna monitor : peer=%pBP", updated_route->peer); - zlog_info("flags=%d", flags); - zlog_info("prefix=%pFX", prefix); - zlog_info("attr=%p", updated_route->attr); - zlog_info("afi=%d safi=%d", afi, safi); - zlog_info("uptime=%ld", updated_route->uptime); - + frr_each(bmp_targets, &bmpbgp->targets, bt) { if (bt->afimon[afi][safi] & BMP_MON_LOC_RIB) { - zlog_info("has LOC RIB monitoring on!"); struct bmp_queue_entry *bqe, bqeref; size_t refcount; @@ -2831,58 +2775,38 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, bqeref.afi = afi; bqeref.safi = safi; - zlog_info("before afi/safi check"); if ((afi == AFI_L2VPN && safi == SAFI_EVPN && bn->pdest) || - (safi == SAFI_MPLS_VPN)) { - zlog_info("bmp: added prefix rd info to bqe"); + (safi == SAFI_MPLS_VPN)) prefix_copy( &bqeref.rd, (struct prefix_rd *)bgp_dest_get_prefix( bn->pdest)); - } - zlog_info("bmp: before hash check"); bqe = bmp_qhash_find(&bt->locupdhash, &bqeref); uint32_t key = bmp_qhash_hkey(&bqeref); - zlog_info("bmp: key = %lu", key); if (bqe) { - zlog_info("bmp: prefix already registered"); if (bqe->refcount >= refcount) /* nothing to do here */ return 0; - zlog_info("bmp: removing prefix"); bmp_qlist_del(&bt->locupdlist, bqe); - zlog_info("bmp: removed prefix"); } else { - zlog_info( - "bmp: prefix not registered in queue yet"); bqe = XMALLOC(MTYPE_BMP_QUEUE, sizeof(*bqe)); - zlog_info("bmp: got malloc %p", bqe); memcpy(bqe, &bqeref, sizeof(*bqe)); - zlog_info("bmp: copied bqeref into bqe"); - bmp_qhash_add(&bt->locupdhash, bqe); - zlog_info("bmp: added hash"); } - zlog_info("bmp: before list add tail"); bqe->refcount = refcount; bmp_qlist_add_tail(&bt->locupdlist, bqe); - zlog_info("bmp: before queuepos update"); frr_each (bmp_session, &bt->sessions, bmp) { - zlog_info("bmp: session host=%s", bmp->remote); - if (!bmp->locrib_queuepos) bmp->locrib_queuepos = bqe; pullwr_bump(bmp->pullwr); }; - - zlog_info("bmp: end"); } }; From 6da477b3957a28b5b09a5ad35afeaf795ebf7714 Mon Sep 17 00:00:00 2001 From: Maxou Date: Mon, 8 Aug 2022 14:32:38 +0200 Subject: [PATCH 15/23] bgpd: refactored bmp_route_update & cleanup TODOs TODOs that are done/un-necessary now deleted refactored bmp_route_update to use a modified bmp_process_one function call instead of duplicating similar code Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 90 +++++++++++++----------------------------------- bgpd/bgp_route.c | 4 +-- 2 files changed, 24 insertions(+), 70 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 8ac17e22e886..820720df55f8 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -1102,7 +1102,6 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) prefix_copy(&bmp->syncpos, bgp_dest_get_prefix(bn)); } - /* TODO BMP add BMP_MON_LOC_RIB case */ 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; @@ -1236,7 +1235,6 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) goto out; } - switch (bmp->afistate[afi][safi]) { case BMP_AFI_INACTIVE: case BMP_AFI_NEEDSYNC: @@ -1248,9 +1246,6 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) */ break; - /* TODO BMP_MON_LOCRIB check if this is true after implenting - * LOCRIB syncing - */ /* currently syncing & haven't reached this prefix yet * => it'll be sent as part of the table sync, no need here */ @@ -1271,14 +1266,12 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) goto out; } - bn = bgp_node_lookup(bmp->targets->bgp->rib[afi][safi], &bqe->p); - struct prefix_rd *prd = NULL; + bool is_vpn = (bqe->afi == AFI_L2VPN && bqe->safi == SAFI_EVPN) || + (bqe->safi == SAFI_MPLS_VPN); - bool is_vpn = (bqe->afi == AFI_L2VPN && bqe->safi == SAFI_EVPN) || (bqe->safi == SAFI_MPLS_VPN); - if (is_vpn) { - prd = &bqe->rd; - bn = bgp_safi_node_lookup(bmp->targets->bgp->rib[afi][safi], afi, safi, &bqe->p, &bqe->rd); - } + struct prefix_rd *prd = is_vpn ? &bqe->rd : NULL; + bn = bgp_safi_node_lookup(bmp->targets->bgp->rib[afi][safi], safi, + &bqe->p, prd); struct bgp_path_info *bpi; @@ -1351,7 +1344,6 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) bn = bgp_safi_node_lookup(bmp->targets->bgp->rib[afi][safi], safi, &bqe->p, prd); - /* TODO BMP add MON_BGP_LOC_RIB case */ if (bmp->targets->afimon[afi][safi] & BMP_MON_POSTPOLICY) { struct bgp_path_info *bpi; @@ -1427,7 +1419,7 @@ static void bmp_wrerr(struct bmp *bmp, struct pullwr *pullwr, bool eof) bmp_free(bmp); } -static void bmp_process_one(struct bmp_targets *bt, struct bgp *bgp, afi_t afi, +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; @@ -1449,26 +1441,26 @@ static void bmp_process_one(struct bmp_targets *bt, struct bgp *bgp, afi_t afi, prefix_copy(&bqeref.rd, (struct prefix_rd *)bgp_dest_get_prefix(bn->pdest)); - bqe = bmp_qhash_find(&bt->updhash, &bqeref); + bqe = bmp_qhash_find(updhash, &bqeref); if (bqe) { if (bqe->refcount >= refcount) /* nothing to do here */ return; - bmp_qlist_del(&bt->updlist, bqe); + bmp_qlist_del(updlist, bqe); } else { bqe = XMALLOC(MTYPE_BMP_QUEUE, sizeof(*bqe)); memcpy(bqe, &bqeref, sizeof(*bqe)); - bmp_qhash_add(&bt->updhash, bqe); + bmp_qhash_add(updhash, bqe); } bqe->refcount = refcount; - bmp_qlist_add_tail(&bt->updlist, bqe); + bmp_qlist_add_tail(updlist, bqe); + + return bqe; - frr_each (bmp_session, &bt->sessions, bmp) - if (!bmp->queuepos) - bmp->queuepos = bqe; + /* 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, @@ -1490,12 +1482,16 @@ static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi, return 0; frr_each(bmp_targets, &bmpbgp->targets, bt) { - if (!bt->afimon[afi][safi]) + /* 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; - bmp_process_one(bt, bgp, afi, safi, bn, peer); + struct bmp_queue_entry* last_item = bmp_process_one(bt, &bt->updhash, &bt->updlist, bgp, afi, safi, bn, peer); frr_each(bmp_session, &bt->sessions, bmp) { + if (!bmp->queuepos) + bmp->queuepos = last_item; + pullwr_bump(bmp->pullwr); } } @@ -2388,8 +2384,8 @@ DEFPY(bmp_stats_cfg, return CMD_SUCCESS; } -DEFPY(bmp_monitor_cfg, bmp_monitor_cmd, - /* TODO BMP add loc-rib option */ +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 @@ -2409,11 +2405,8 @@ DEFPY(bmp_monitor_cfg, bmp_monitor_cmd, argv_find_and_parse_afi(argv, argc, &index, &afi); argv_find_and_parse_safi(argv, argc, &index, &safi); - /* TODO BMP set right flag */ if (policy[0] == 'l') { flag = BMP_MON_LOC_RIB; - vty_out(vty, - "changing loc rib monitoring config for this target\n"); } else if (policy[1] == 'r') flag = BMP_MON_PREPOLICY; else @@ -2762,48 +2755,11 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, frr_each(bmp_targets, &bmpbgp->targets, bt) { if (bt->afimon[afi][safi] & BMP_MON_LOC_RIB) { - struct bmp_queue_entry *bqe, bqeref; - size_t refcount; - - refcount = bmp_session_count(&bt->sessions); - if (refcount == 0) - return 0; - - memset(&bqeref, 0, sizeof(bqeref)); - prefix_copy(&bqeref.p, prefix); - bqeref.peerid = peer->qobj_node.nid; - bqeref.afi = afi; - bqeref.safi = safi; - - if ((afi == AFI_L2VPN && safi == SAFI_EVPN && - bn->pdest) || - (safi == SAFI_MPLS_VPN)) - prefix_copy( - &bqeref.rd, - (struct prefix_rd *)bgp_dest_get_prefix( - bn->pdest)); - - bqe = bmp_qhash_find(&bt->locupdhash, &bqeref); - uint32_t key = bmp_qhash_hkey(&bqeref); - - if (bqe) { - if (bqe->refcount >= refcount) - /* nothing to do here */ - return 0; - - bmp_qlist_del(&bt->locupdlist, bqe); - } else { - bqe = XMALLOC(MTYPE_BMP_QUEUE, sizeof(*bqe)); - memcpy(bqe, &bqeref, sizeof(*bqe)); - bmp_qhash_add(&bt->locupdhash, bqe); - } - - bqe->refcount = refcount; - bmp_qlist_add_tail(&bt->locupdlist, bqe); + struct bmp_queue_entry *last_item = bmp_process_one(bt, &bt->locupdhash, &bt->locupdlist, bgp, afi, safi, bn, peer); frr_each (bmp_session, &bt->sessions, bmp) { if (!bmp->locrib_queuepos) - bmp->locrib_queuepos = bqe; + bmp->locrib_queuepos = last_item; pullwr_bump(bmp->pullwr); }; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 9353b64f305b..2dd84d457ab3 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -3453,10 +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 */ if (old_select || new_select) { - zlog_info("old_select==NULL %s | new_select==NULL %s", - old_select == NULL ? "YES" : "NO", - new_select == NULL ? "YES" : "NO"); if (old_select) /* route is not installed in locrib anymore */ old_select->rib_uptime = (time_t)(-1L); From 8557a1368968c8b967d11b583a2bfcfa42536c2d Mon Sep 17 00:00:00 2001 From: Maxou <5208681+mxyns@users.noreply.github.com> Date: Mon, 8 Aug 2022 20:34:45 +0200 Subject: [PATCH 16/23] bgpd: Update bmp.rst documentation added documentation for the current state of the bmp implementation Signed-off-by: Maxence Younsi --- doc/user/bmp.rst | 51 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/doc/user/bmp.rst b/doc/user/bmp.rst index 1983995c1fdb..e02c24c688af 100644 --- a/doc/user/bmp.rst +++ b/doc/user/bmp.rst @@ -36,8 +36,8 @@ The `BMP` implementation in FRR has the following properties: successfully. OPEN messages for failed sessions cannot currently be mirrored. -- **route monitoring** is available for IPv4 and IPv6 AFIs, unicast and - multicast SAFIs. Other SAFIs (VPN, Labeled-Unicast, Flowspec, etc.) are not +- **route monitoring** is available for IPv4 and IPv6 AFIs, unicast, multicast + EVPN and VPN SAFIs. Other SAFIs (VPN, Labeled-Unicast, Flowspec, etc.) are not currently supported. - monitoring peers that have BGP **add-path** enabled on the session will @@ -57,6 +57,51 @@ The `BMP` implementation in FRR has the following properties: - monitoring peers with :rfc:`5549` extended next-hops has not been tested. +RFC 7854 +======== +Unsupported features (non exhaustive): + - Per-Peer Header + + - Peer Type Flag + - Peer Distingsher + + - Peer Up + + - Reason codes (according to TODO comments in code) + +Peer Type Flag and Peer Distinguisher can be implemented easily using RFC 9069's base code. + +RFC 9069 +======== +Everything that isn't listed here is implemented and should be working. +Unsupported features (should be exhaustive): + +- Per-Peer Header + + - Timestamp + + - set to 0 + - value is now saved `struct bgp_path_info -> locrib_uptime` + - needs testing + +- Peer Up/Down + + - VRF/Table Name TLV + + - code for TLV exists + - need better RFC understanding + +- Peer Down Only + + - Reason code (bc not supported in RFC 7854 either) + +- Statistics Report + + - Stat Type = 8: (64-bit Gauge) Number of routes in Loc-RIB. + - Stat Type = 10: Number of routes in per-AFI/SAFI Loc-RIB. The value is + structured as: 2-byte AFI, 1-byte SAFI, followed by a 64-bit Gauge. + + Starting BMP ============ @@ -146,7 +191,7 @@ associated with a particular ``bmp targets``: Send BMP Statistics (counter) messages at the specified interval (in milliseconds.) -.. clicmd:: bmp monitor AFI SAFI +.. clicmd:: bmp monitor AFI SAFI Perform Route Monitoring for the specified AFI and SAFI. Only IPv4 and IPv6 are currently valid for AFI. SAFI valid values are currently From 19b3ead3f0397bc00725145c2cb4a5bea74e5a74 Mon Sep 17 00:00:00 2001 From: Maxou Date: Fri, 12 Aug 2022 16:45:05 +0200 Subject: [PATCH 17/23] bgpd: bmp locrib monitoring unlock node after lookup bgp_afi_node_lookup calls bgp_node_lookup which locks the node, unlocking it safely after function is finished Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 820720df55f8..f52c069f3cdb 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -1213,7 +1213,7 @@ static inline struct bmp_queue_entry *bmp_pull_locrib(struct bmp *bmp) &bmp->locrib_queuepos); } -/* TODO BMP_MON_LOCRIB find a way to merge properly this function with +/* TODO BMP_MON_LOCRIB find a way to merge properly this function with * bmp_wrqueue or abstract it if possible */ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) @@ -1221,7 +1221,7 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) struct bmp_queue_entry *bqe; struct peer *peer; - struct bgp_dest *bn; + struct bgp_dest *bn = NULL; bool written = false; bqe = bmp_pull_locrib(bmp); @@ -1274,10 +1274,6 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) &bqe->p, prd); struct bgp_path_info *bpi; - - struct bgp_path_info *pathinfo = NULL; - pathinfo = bgp_dest_get_bgp_path_info(bn); - for (bpi = bn ? bgp_dest_get_bgp_path_info(bn) : NULL; bpi; bpi = bpi->next) { if (!CHECK_FLAG(bpi->flags, BGP_PATH_SELECTED)) @@ -1294,6 +1290,10 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) out: if (!bqe->refcount) XFREE(MTYPE_BMP_QUEUE, bqe); + + if (bn) + bgp_dest_unlock_node(bn); + return written; } From 1cca53e5c645ee011910e16a387c47abde163508 Mon Sep 17 00:00:00 2001 From: mxyns Date: Fri, 12 Aug 2022 18:00:38 +0200 Subject: [PATCH 18/23] 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 --- bgpd/bgp_bmp.c | 160 ++++++++++++++++++++++++++++------------------- bgpd/bgp_route.c | 3 +- 2 files changed, 99 insertions(+), 64 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index f52c069f3cdb..68057bcbafca 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 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) 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 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) 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 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) 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 2dd84d457ab3..d8a87c1fa67c 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 */ From 66d564a60be23ae7b1181ed908fd8ecce8db8434 Mon Sep 17 00:00:00 2001 From: mxyns Date: Wed, 28 Sep 2022 08:05:25 +0200 Subject: [PATCH 19/23] bgpd: loc-rib uptime moved to bgp_path_info_extra and set in header moved loc-rib uptime field "bgp_rib_uptime" to struct bgp_path_info_extra for memory concerns moved logic into bgp_route_update's callback bmp_route_update written timestamp in per peer header Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 134 +++++++++++++++++++++++++++++------------- bgpd/bgp_bmp.h | 7 +-- bgpd/bgp_route.c | 19 ++---- bgpd/bgp_route.h | 8 ++- doc/developer/bmp.rst | 49 +++++++++++++++ doc/user/bmp.rst | 49 +-------------- 6 files changed, 158 insertions(+), 108 deletions(-) create mode 100644 doc/developer/bmp.rst diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 68057bcbafca..a6cd915ab11a 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -250,24 +250,40 @@ static inline uint64_t bmp_get_peer_distinguisher(struct bmp *bmp, afi_t afi, { /* remove this check when the other peer types get correct peer dist. - // (RFC7854) impl. */ + *(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; - /* - * default vrf => can't have RD => 0 + + /* default vrf => can't have RD => 0 * vrf => has RD? * if yes => use RD value - * else => use vrf_id and convert it so that - * peer_distinguisher is 0::vrf_id + * else => use vrf_id + * vrf_id == VRF_UNKNOWN ? + * if yes => 0 + * else => 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); + if (bgp->inst_type == VRF_DEFAULT) + return 0; + + struct prefix_rd *prd = &bgp->vpn_policy[afi].tovpn_rd; + + if (CHECK_FLAG(bgp->vpn_policy[afi].flags, + BGP_VPN_POLICY_TOVPN_RD_SET)) { + uint64_t peerd = 0; + + memcpy(&peerd, prd->val, sizeof(prd->val)); + return peerd; + } + + if (bgp->vrf_id == VRF_UNKNOWN) + return 0; + + return ((uint64_t)htonl(bgp->vrf_id)) << 32; } static void bmp_common_hdr(struct stream *s, uint8_t ver, uint8_t type) @@ -304,7 +320,8 @@ static void bmp_per_peer_hdr(struct stream *s, struct bgp *bgp, /* Peer Address */ /* Set to 0 if it's a LOC-RIB INSTANCE (RFC 9069) or if it's not an - * IPv4/6 address */ + * 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); @@ -322,20 +339,23 @@ static void bmp_per_peer_hdr(struct stream *s, struct bgp *bgp, /* Peer AS */ /* set peer ASN but for LOC-RIB INSTANCE (RFC 9069) put the local bgp - * ASN if available or 0 */ + * 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 */ + * 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 */ - /* set to 0 for LOC-RIB INSTANCE as install uptime is not saved atm */ - if (!is_locrib && tv) { + if (tv) { stream_putl(s, tv->tv_sec); stream_putl(s, tv->tv_usec); } else { @@ -361,6 +381,7 @@ bmp_put_vrftablename_info_tlv(struct stream *s, struct bmp *bmp) 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; } if (vrftablename != NULL) @@ -715,8 +736,7 @@ static bool bmp_wrmirror(struct bmp *bmp, struct pullwr *pullwr) 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, - &bmq->tv); + BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, &bmq->tv); /* BMP Mirror TLV. */ stream_putw(s, BMP_MIRROR_TLV_TYPE_BGP_MESSAGE); @@ -995,7 +1015,7 @@ static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags, 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); + uptime == (time_t)(-1L) ? NULL : &uptime_real); stream_putl_at(hdr, BMP_LENGTH_POS, stream_get_endp(hdr) + stream_get_endp(msg)); @@ -1180,16 +1200,17 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) prd = (struct prefix_rd *)bgp_dest_get_prefix(bmp->syncrdpos); if (bpi && CHECK_FLAG(bpi->flags, BGP_PATH_SELECTED) && - CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_LOC_RIB)) { + 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); + bn_p, prd, bpi->attr, afi, safi, + bpi && bpi->extra ? bpi->extra->bgp_rib_uptime + : (time_t)(-1L)); } 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, + BMP_PEER_TYPE_GLOBAL_INSTANCE, bn_p, prd, bpi->attr, afi, safi, bpi->uptime); if (adjin) @@ -1253,9 +1274,8 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) afi_t afi = bqe->afi; safi_t safi = bqe->safi; - if (!CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_LOC_RIB)) { + if (!CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_LOC_RIB)) goto out; - } switch (bmp->afistate[afi][safi]) { case BMP_AFI_INACTIVE: @@ -1280,11 +1300,13 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) peer = QOBJ_GET_TYPESAFE(bqe->peerid, peer); if (!peer) { - // skipping queued item for deleted peer + /* skipping queued item for deleted peer + */ goto out; } if (peer != bmp->targets->bgp->peer_self && !peer_established(peer->connection)) { - // peer is neither self, nor established + /* peer is neither self, nor established + */ goto out; } @@ -1292,10 +1314,12 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) (bqe->safi == SAFI_MPLS_VPN); struct prefix_rd *prd = is_vpn ? &bqe->rd : NULL; + bn = bgp_safi_node_lookup(bmp->targets->bgp->rib[afi][safi], safi, - &bqe->p, prd); + &bqe->p, prd); struct bgp_path_info *bpi; + for (bpi = bn ? bgp_dest_get_bgp_path_info(bn) : NULL; bpi; bpi = bpi->next) { if (!CHECK_FLAG(bpi->flags, BGP_PATH_SELECTED)) @@ -1306,7 +1330,8 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) 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)); + bpi && bpi->extra ? bpi->extra->bgp_rib_uptime + : (time_t)(-1L)); written = true; out: @@ -1358,7 +1383,6 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) if (!peer_established(peer->connection)) goto out; - bool is_vpn = (bqe->afi == AFI_L2VPN && bqe->safi == SAFI_EVPN) || (bqe->safi == SAFI_MPLS_VPN); @@ -1484,7 +1508,8 @@ bmp_process_one(struct bmp_targets *bt, struct bmp_qhash_head *updhash, return bqe; /* need to update correct queue pos for all sessions of the target after - * a call to this function */ + * a call to this function + */ } static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi, @@ -1507,7 +1532,8 @@ static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi, frr_each(bmp_targets, &bmpbgp->targets, bt) { /* check if any monitoring is enabled (ignoring loc-rib since it - * uses another hook & queue */ + * uses another hook & queue + */ if (!(bt->afimon[afi][safi] & ~BMP_MON_LOC_RIB)) continue; @@ -1515,8 +1541,9 @@ static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi, 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 + // if bmp_process_one returns NULL + // we don't have anything to do next + if (!last_item) continue; frr_each(bmp_session, &bt->sessions, bmp) { @@ -1562,8 +1589,7 @@ 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, - &tv); + BMP_PEER_TYPE_GLOBAL_INSTANCE, 0, &tv); count_pos = stream_get_endp(s); stream_putl(s, 0); @@ -2436,9 +2462,9 @@ DEFPY(bmp_monitor_cfg, bmp_monitor_cmd, argv_find_and_parse_afi(argv, argc, &index, &afi); argv_find_and_parse_safi(argv, argc, &index, &safi); - if (policy[0] == 'l') { + if (policy[0] == 'l') flag = BMP_MON_LOC_RIB; - } else if (policy[1] == 'r') + else if (policy[1] == 'r') flag = BMP_MON_PREPOLICY; else flag = BMP_MON_POSTPOLICY; @@ -2772,14 +2798,39 @@ static int bgp_bmp_init(struct event_loop *tm) /* TODO remove "bn", redundant with updated_route->net ? */ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, struct bgp_dest *bn, - struct bgp_path_info *updated_route, bool withdraw) + struct bgp_path_info *old_route, + struct bgp_path_info *new_route) { - + bool is_locribmon_enabled = false; + bool is_withdraw = old_route && !new_route; + struct bgp_path_info *updated_route = + is_withdraw ? old_route : new_route; struct bmp_bgp *bmpbgp = bmp_bgp_get(bgp); struct peer *peer = updated_route->peer; struct bmp_targets *bt; struct bmp *bmp; + frr_each (bmp_targets, &bmpbgp->targets, bt) { + if ((is_locribmon_enabled |= + (bt->afimon[afi][safi] & BMP_MON_LOC_RIB))) + break; + } + + if (!is_locribmon_enabled) + return 0; + + /* route is not installed in locrib anymore and rib uptime was saved */ + if (old_route && old_route->extra) + bgp_path_info_extra_get(old_route)->bgp_rib_uptime = + (time_t)(-1L); + + /* route is installed in locrib from now on so + * save rib uptime in bgp_path_info_extra + */ + if (new_route) + bgp_path_info_extra_get(new_route)->bgp_rib_uptime = + monotime(NULL); + frr_each (bmp_targets, &bmpbgp->targets, bt) { if (bt->afimon[afi][safi] & BMP_MON_LOC_RIB) { @@ -2787,8 +2838,9 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, 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 + // if bmp_process_one returns NULL + // we don't have anything to do next + if (!last_item) continue; frr_each (bmp_session, &bt->sessions, bmp) { diff --git a/bgpd/bgp_bmp.h b/bgpd/bgp_bmp.h index 152e7eb0e2ef..dadd99eb6d0a 100644 --- a/bgpd/bgp_bmp.h +++ b/bgpd/bgp_bmp.h @@ -216,15 +216,14 @@ struct bmp_targets { int stat_msec; /* only supporting: - * - IPv4 / unicast & multicast - * - IPv6 / unicast & multicast + * - IPv4 / unicast & multicast & VPN + * - IPv6 / unicast & multicast & VPN * - L2VPN / EVPN */ #define BMP_MON_PREPOLICY (1 << 0) #define BMP_MON_POSTPOLICY (1 << 1) - -/* TODO define BMP_MON_LOC_RIB flag */ #define BMP_MON_LOC_RIB (1 << 2) + uint8_t afimon[AFI_MAX][SAFI_MAX]; bool mirror; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index d8a87c1fa67c..560da893f765 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -87,8 +87,8 @@ DEFINE_HOOK(bgp_rpki_prefix_status, DEFINE_HOOK(bgp_route_update, (struct bgp *bgp, afi_t afi, safi_t safi, struct bgp_dest *bn, - struct bgp_path_info *updated_route, bool withdraw), - (bgp, afi, safi, bn, updated_route, withdraw)); + struct bgp_path_info *old_route, struct bgp_path_info *new_route), + (bgp, afi, safi, bn, old_route, new_route)); /* Extern from bgp_dump.c */ extern const char *bgp_origin_str[]; @@ -3454,17 +3454,11 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, } /* call bmp hook for loc-rib route update / withdraw after flags were - * set */ + * set + */ if (old_select || new_select) { - - if (old_select) /* route is not installed in locrib anymore */ - old_select->rib_uptime = (time_t)(-1L); - if (new_select) /* route is installed in locrib from now on */ - new_select->rib_uptime = bgp_clock(); - bool is_withdraw = old_select && !new_select; - - hook_call(bgp_route_update, bgp, afi, safi, dest, - is_withdraw ? old_select : new_select, is_withdraw); + hook_call(bgp_route_update, bgp, afi, safi, dest, old_select, + new_select); } @@ -3993,7 +3987,6 @@ struct bgp_path_info *info_make(int type, int sub_type, unsigned short instance, new->peer = peer; new->attr = attr; new->uptime = monotime(NULL); - new->rib_uptime = (time_t)(-1L); new->net = dest; return new; } diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index 232d680188e5..3057a4259abe 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -211,6 +211,9 @@ struct bgp_path_info_extra { mpls_label_t label[BGP_MAX_LABELS]; uint32_t num_labels; + /* timestamp of the rib installation */ + time_t bgp_rib_uptime; + /*For EVPN*/ struct bgp_path_info_extra_evpn *evpn; @@ -295,7 +298,6 @@ struct bgp_path_info { /* Uptime. */ time_t uptime; - time_t rib_uptime; /* reference count */ int lock; @@ -678,8 +680,8 @@ DECLARE_HOOK(bgp_process, /* called when a route is updated in the rib */ DECLARE_HOOK(bgp_route_update, (struct bgp *bgp, afi_t afi, safi_t safi, struct bgp_dest *bn, - struct bgp_path_info *updated_route, bool withdraw), - (bgp, afi, safi, bn, updated_route, withdraw)); + struct bgp_path_info *old_route, struct bgp_path_info *new_route), + (bgp, afi, safi, bn, old_route, new_route)); /* BGP show options */ #define BGP_SHOW_OPT_JSON (1 << 0) diff --git a/doc/developer/bmp.rst b/doc/developer/bmp.rst new file mode 100644 index 000000000000..1c0e4b045426 --- /dev/null +++ b/doc/developer/bmp.rst @@ -0,0 +1,49 @@ +.. _bmp: + +*** +BMP +*** + +RFC 7854 +======== +Missing features (non exhaustive): + - Per-Peer Header + + - Peer Type Flag + - Peer Distingsher + + - Peer Up + + - Reason codes (according to TODO comments in code) + +Peer Type Flag and Peer Distinguisher can be implemented easily using RFC 9069's base code. + +RFC 9069 +======== +Everything that isn't listed here is implemented and should be working. +Missing features (should be exhaustive): + +- Per-Peer Header + + - Timestamp + + - set to 0 + - value is now saved `struct bgp_path_info -> locrib_uptime` + - needs testing + +- Peer Up/Down + + - VRF/Table Name TLV + + - code for TLV exists + - need better RFC understanding + +- Peer Down Only + + - Reason code (bc not supported in RFC 7854 either) + +- Statistics Report + + - Stat Type = 8: (64-bit Gauge) Number of routes in Loc-RIB. + - Stat Type = 10: Number of routes in per-AFI/SAFI Loc-RIB. The value is + structured as: 2-byte AFI, 1-byte SAFI, followed by a 64-bit Gauge. diff --git a/doc/user/bmp.rst b/doc/user/bmp.rst index e02c24c688af..6cd7e19cac3a 100644 --- a/doc/user/bmp.rst +++ b/doc/user/bmp.rst @@ -36,7 +36,7 @@ The `BMP` implementation in FRR has the following properties: successfully. OPEN messages for failed sessions cannot currently be mirrored. -- **route monitoring** is available for IPv4 and IPv6 AFIs, unicast, multicast +- **route monitoring** is available for IPv4 and IPv6 AFIs, unicast, multicast EVPN and VPN SAFIs. Other SAFIs (VPN, Labeled-Unicast, Flowspec, etc.) are not currently supported. @@ -57,51 +57,6 @@ The `BMP` implementation in FRR has the following properties: - monitoring peers with :rfc:`5549` extended next-hops has not been tested. -RFC 7854 -======== -Unsupported features (non exhaustive): - - Per-Peer Header - - - Peer Type Flag - - Peer Distingsher - - - Peer Up - - - Reason codes (according to TODO comments in code) - -Peer Type Flag and Peer Distinguisher can be implemented easily using RFC 9069's base code. - -RFC 9069 -======== -Everything that isn't listed here is implemented and should be working. -Unsupported features (should be exhaustive): - -- Per-Peer Header - - - Timestamp - - - set to 0 - - value is now saved `struct bgp_path_info -> locrib_uptime` - - needs testing - -- Peer Up/Down - - - VRF/Table Name TLV - - - code for TLV exists - - need better RFC understanding - -- Peer Down Only - - - Reason code (bc not supported in RFC 7854 either) - -- Statistics Report - - - Stat Type = 8: (64-bit Gauge) Number of routes in Loc-RIB. - - Stat Type = 10: Number of routes in per-AFI/SAFI Loc-RIB. The value is - structured as: 2-byte AFI, 1-byte SAFI, followed by a 64-bit Gauge. - - Starting BMP ============ @@ -194,7 +149,7 @@ associated with a particular ``bmp targets``: .. clicmd:: bmp monitor AFI SAFI Perform Route Monitoring for the specified AFI and SAFI. Only IPv4 and - IPv6 are currently valid for AFI. SAFI valid values are currently + IPv6 are currently valid for AFI. SAFI valid values are currently unicast, multicast, evpn and vpn. Other AFI/SAFI combinations may be added in the future. From fa17129752d39bd2c0d6f9faa08c219f4c56a50a Mon Sep 17 00:00:00 2001 From: Maxence Younsi Date: Mon, 6 Feb 2023 17:30:28 +0100 Subject: [PATCH 20/23] bgpd: skip bmp messages when vrf id is unknown changed result type of bmp_get_peer_distinguisher to int added result pointer parameter to bmp_get_peer_distinguisher bmp_get_peer_distinguisher returns 0 means the result is valid else error occured do not use result Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 80 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 48 insertions(+), 32 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index a6cd915ab11a..629bc4209abf 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -245,45 +245,41 @@ static void bmp_free(struct bmp *bmp) #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 int bmp_get_peer_distinguisher(struct bmp *bmp, afi_t afi, + uint8_t peer_type, + uint64_t *result_ref) { /* remove this check when the other peer types get correct peer dist. *(RFC7854) impl. + * for now, always return no error and 0 peer distinguisher as before */ if (peer_type != BMP_PEER_TYPE_LOC_RIB_INSTANCE) - return 0; + return (*result_ref = 0); /* sending vrf_id or rd could be turned into an option at some point */ struct bgp *bgp = bmp->targets->bgp; - /* default vrf => can't have RD => 0 - * vrf => has RD? - * if yes => use RD value - * else => use vrf_id - * vrf_id == VRF_UNKNOWN ? - * if yes => 0 - * else => convert it so that - * peer_distinguisher is 0::vrf_id - */ + /* vrf default => ok, distinguisher 0 */ if (bgp->inst_type == VRF_DEFAULT) - return 0; + return (*result_ref = 0); + /* use RD if set in VRF config for this AFI */ struct prefix_rd *prd = &bgp->vpn_policy[afi].tovpn_rd; if (CHECK_FLAG(bgp->vpn_policy[afi].flags, BGP_VPN_POLICY_TOVPN_RD_SET)) { - uint64_t peerd = 0; - - memcpy(&peerd, prd->val, sizeof(prd->val)); - return peerd; + memcpy(result_ref, prd->val, sizeof(prd->val)); + return 0; } + /* VRF has no id => error => message should be skipped */ if (bgp->vrf_id == VRF_UNKNOWN) - return 0; + return 1; - return ((uint64_t)htonl(bgp->vrf_id)) << 32; + /* use VRF id converted to ::vrf_id 64bits format */ + *result_ref = ((uint64_t)htonl(bgp->vrf_id)) << 32; + return 0; } static void bmp_common_hdr(struct stream *s, uint8_t ver, uint8_t type) @@ -391,8 +387,8 @@ bmp_put_vrftablename_info_tlv(struct stream *s, struct bmp *bmp) static int bmp_send_initiation(struct bmp *bmp) { int len; - struct stream *s; - s = stream_new(BGP_MAX_PACKET_SIZE); + struct stream *s = stream_new(BGP_MAX_PACKET_SIZE); + bmp_common_hdr(s, BMP_VERSION_3, BMP_TYPE_INITIATION); #define BMP_INFO_TYPE_SYSDESCR 1 @@ -884,15 +880,22 @@ static void bmp_eor(struct bmp *bmp, afi_t afi, safi_t safi, uint8_t flags, if (!peer->afc_nego[afi][safi]) continue; + uint64_t peer_distinguisher = 0; + /* skip this message if peer distinguisher is not available */ + if (bmp_get_peer_distinguisher(bmp, afi, peer_type_flag, + &peer_distinguisher)) { + zlog_debug( + "skipping bmp message for reason: can't get peer distinguisher"); + continue; + } + s2 = stream_new(BGP_MAX_PACKET_SIZE); 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, peer_distinguisher, NULL); stream_putl_at(s2, BMP_LENGTH_POS, stream_get_endp(s) + stream_get_endp(s2)); @@ -1005,6 +1008,15 @@ static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags, struct timeval tv = { .tv_sec = uptime, .tv_usec = 0 }; struct timeval uptime_real; + uint64_t peer_distinguisher = 0; + /* skip this message if peer distinguisher is not available */ + if (bmp_get_peer_distinguisher(bmp, afi, peer_type_flag, + &peer_distinguisher)) { + zlog_debug( + "skipping bmp message for reason: can't get peer distinguisher"); + return; + } + monotime_to_realtime(&tv, &uptime_real); if (attr) msg = bmp_update(p, prd, peer, attr, afi, safi); @@ -1014,7 +1026,7 @@ 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), + peer_distinguisher, uptime == (time_t)(-1L) ? NULL : &uptime_real); stream_putl_at(hdr, BMP_LENGTH_POS, @@ -1541,8 +1553,9 @@ static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi, bmp_process_one(bt, &bt->updhash, &bt->updlist, bgp, afi, safi, bn, peer); - // if bmp_process_one returns NULL - // we don't have anything to do next + /* if bmp_process_one returns NULL + * we don't have anything to do next + */ if (!last_item) continue; @@ -2811,8 +2824,10 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, struct bmp *bmp; frr_each (bmp_targets, &bmpbgp->targets, bt) { - if ((is_locribmon_enabled |= - (bt->afimon[afi][safi] & BMP_MON_LOC_RIB))) + is_locribmon_enabled |= + (bt->afimon[afi][safi] & BMP_MON_LOC_RIB); + + if (is_locribmon_enabled) break; } @@ -2838,8 +2853,9 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, bt, &bt->locupdhash, &bt->locupdlist, bgp, afi, safi, bn, peer); - // if bmp_process_one returns NULL - // we don't have anything to do next + /* if bmp_process_one returns NULL + * we don't have anything to do next + */ if (!last_item) continue; From 9607070acd3ca542f52ab34d14e0e56f0cd3eeaf Mon Sep 17 00:00:00 2001 From: Maxence Younsi Date: Mon, 11 Sep 2023 14:51:45 +0200 Subject: [PATCH 21/23] bgpd: bmp unset v6 flag + address PR#14188 comments use CHECK_FLAG fix comment spaces change zlog_debug to zlog_warn safeguard on updated_route added doc/developer/bmp.rst to subdir.am other qol changes Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 77 ++++++++++++++++++++++++----------------- doc/developer/subdir.am | 1 + doc/user/bmp.rst | 2 +- 3 files changed, 47 insertions(+), 33 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 629bc4209abf..e9f912cb18cb 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -285,7 +285,7 @@ static inline int bmp_get_peer_distinguisher(struct bmp *bmp, afi_t afi, static void bmp_common_hdr(struct stream *s, uint8_t ver, uint8_t type) { stream_putc(s, ver); - stream_putl(s, 0); /*dummy message length. will be set later. */ + stream_putl(s, 0); /* dummy message length. will be set later. */ stream_putc(s, type); } @@ -305,7 +305,7 @@ static void bmp_per_peer_hdr(struct stream *s, struct bgp *bgp, stream_putc(s, peer_type_flag); /* Peer Flags */ - if (peer->connection->su.sa.sa_family == AF_INET6) + if (!is_locrib && peer->connection->su.sa.sa_family == AF_INET6) SET_FLAG(flags, BMP_PEER_FLAG_V); else UNSET_FLAG(flags, BMP_PEER_FLAG_V); @@ -325,7 +325,7 @@ static void bmp_per_peer_hdr(struct stream *s, struct bgp *bgp, stream_putl(s, 0); stream_putl(s, 0); } else if (peer->connection->su.sa.sa_family == AF_INET6) - stream_put(s, &peer->connection->su.sin6.sin6_addr, 16); + stream_put(s, &peer->connection->su.sin6.sin6_addr, IPV6_MAX_BYTELEN); else if (peer->connection->su.sa.sa_family == AF_INET) { stream_putl(s, 0); stream_putl(s, 0); @@ -398,7 +398,7 @@ static int bmp_send_initiation(struct bmp *bmp) bmp_put_info_tlv(s, BMP_INFO_TYPE_SYSNAME, cmd_hostname_get()); len = stream_get_endp(s); - stream_putl_at(s, BMP_LENGTH_POS, len); /*message length is set. */ + stream_putl_at(s, BMP_LENGTH_POS, len); /* message length is set. */ pullwr_write_stream(bmp->pullwr, s); stream_free(s); @@ -534,7 +534,7 @@ static struct stream *bmp_peerstate(struct peer *peer, bool down) } len = stream_get_endp(s); - stream_putl_at(s, BMP_LENGTH_POS, len); /*message length is set. */ + stream_putl_at(s, BMP_LENGTH_POS, len); /* message length is set. */ return s; } @@ -884,7 +884,7 @@ static void bmp_eor(struct bmp *bmp, afi_t afi, safi_t safi, uint8_t flags, /* skip this message if peer distinguisher is not available */ if (bmp_get_peer_distinguisher(bmp, afi, peer_type_flag, &peer_distinguisher)) { - zlog_debug( + zlog_warn( "skipping bmp message for reason: can't get peer distinguisher"); continue; } @@ -1012,7 +1012,7 @@ static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags, /* skip this message if peer distinguisher is not available */ if (bmp_get_peer_distinguisher(bmp, afi, peer_type_flag, &peer_distinguisher)) { - zlog_debug( + zlog_warn( "skipping bmp message for reason: can't get peer distinguisher"); return; } @@ -1155,8 +1155,10 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) 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 (CHECK_FLAG(bmp->targets->afimon[afi][safi], + BMP_MON_POSTPOLICY) || + CHECK_FLAG(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, @@ -1173,7 +1175,8 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) bpi = bpiter; } } - if (bmp->targets->afimon[afi][safi] & BMP_MON_PREPOLICY) { + if (CHECK_FLAG(bmp->targets->afimon[afi][safi], + BMP_MON_PREPOLICY)) { for (adjiter = bn->adj_in; adjiter; adjiter = adjiter->next) { if (adjiter->peer->qobj_node.nid @@ -1303,8 +1306,6 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) /* currently syncing & haven't reached this prefix yet * => it'll be sent as part of the table sync, no need here */ - zlog_info( - "bmp: skipping direct monitor msg bc will be sent with sync :)"); goto out; case BMP_AFI_LIVE: break; @@ -1332,8 +1333,7 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) struct bgp_path_info *bpi; - for (bpi = bn ? bgp_dest_get_bgp_path_info(bn) : NULL; bpi; - bpi = bpi->next) { + for (bpi = bgp_dest_get_bgp_path_info(bn); bpi; bpi = bpi->next) { if (!CHECK_FLAG(bpi->flags, BGP_PATH_SELECTED)) continue; if (bpi->peer == peer) @@ -1402,10 +1402,10 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) bn = bgp_safi_node_lookup(bmp->targets->bgp->rib[afi][safi], safi, &bqe->p, prd); - if (bmp->targets->afimon[afi][safi] & BMP_MON_POSTPOLICY) { + if (CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_POSTPOLICY)) { struct bgp_path_info *bpi; - for (bpi = bn ? bgp_dest_get_bgp_path_info(bn) : NULL; bpi; + for (bpi = bgp_dest_get_bgp_path_info(bn); bpi; bpi = bpi->next) { if (!CHECK_FLAG(bpi->flags, BGP_PATH_VALID)) continue; @@ -1420,7 +1420,7 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) written = true; } - if (bmp->targets->afimon[afi][safi] & BMP_MON_PREPOLICY) { + if (CHECK_FLAG(bmp->targets->afimon[afi][safi], BMP_MON_PREPOLICY)) { struct bgp_adj_in *adjin; for (adjin = bn ? bn->adj_in : NULL; adjin; @@ -1546,7 +1546,7 @@ static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi, /* check if any monitoring is enabled (ignoring loc-rib since it * uses another hook & queue */ - if (!(bt->afimon[afi][safi] & ~BMP_MON_LOC_RIB)) + if (!CHECK_FLAG(bt->afimon[afi][safi], ~BMP_MON_LOC_RIB)) continue; struct bmp_queue_entry *last_item = @@ -2455,6 +2455,9 @@ DEFPY(bmp_stats_cfg, return CMD_SUCCESS; } +#define BMP_POLICY_IS_LOCRIB(str) ((str)[0] == 'l') /* __l__oc-rib */ +#define BMP_POLICY_IS_PRE(str) ((str)[1] == 'r') /* p__r__e-policy */ + DEFPY(bmp_monitor_cfg, bmp_monitor_cmd, "[no] bmp monitor $policy", NO_STR BMP_STR @@ -2475,9 +2478,9 @@ DEFPY(bmp_monitor_cfg, bmp_monitor_cmd, argv_find_and_parse_afi(argv, argc, &index, &afi); argv_find_and_parse_safi(argv, argc, &index, &safi); - if (policy[0] == 'l') + if (BMP_POLICY_IS_LOCRIB(policy)) flag = BMP_MON_LOC_RIB; - else if (policy[1] == 'r') + else if (BMP_POLICY_IS_PRE(policy)) flag = BMP_MON_PREPOLICY; else flag = BMP_MON_POSTPOLICY; @@ -2615,15 +2618,17 @@ DEFPY(show_bmp, continue; const char *pre_str = - afimon_flag & BMP_MON_PREPOLICY + CHECK_FLAG(afimon_flag, + BMP_MON_PREPOLICY) ? "pre-policy " : ""; const char *post_str = - afimon_flag & BMP_MON_POSTPOLICY + CHECK_FLAG(afimon_flag, + BMP_MON_POSTPOLICY) ? "post-policy " : ""; const char *locrib_str = - afimon_flag & BMP_MON_LOC_RIB + CHECK_FLAG(afimon_flag, BMP_MON_LOC_RIB) ? "loc-rib" : ""; @@ -2750,14 +2755,16 @@ static int bmp_config_write(struct bgp *bgp, struct vty *vty) vty_out(vty, " bmp mirror\n"); FOREACH_AFI_SAFI (afi, safi) { - if (bt->afimon[afi][safi] & BMP_MON_PREPOLICY) + if (CHECK_FLAG(bt->afimon[afi][safi], + BMP_MON_PREPOLICY)) vty_out(vty, " bmp monitor %s %s pre-policy\n", afi2str_lower(afi), safi2str(safi)); - if (bt->afimon[afi][safi] & BMP_MON_POSTPOLICY) + if (CHECK_FLAG(bt->afimon[afi][safi], + BMP_MON_POSTPOLICY)) vty_out(vty, " bmp monitor %s %s post-policy\n", afi2str_lower(afi), safi2str(safi)); - if (bt->afimon[afi][safi] & BMP_MON_LOC_RIB) + if (CHECK_FLAG(bt->afimon[afi][safi], BMP_MON_LOC_RIB)) vty_out(vty, " bmp monitor %s %s loc-rib\n", afi2str(afi), safi2str(safi)); } @@ -2808,7 +2815,6 @@ static int bgp_bmp_init(struct event_loop *tm) return 0; } -/* TODO remove "bn", redundant with updated_route->net ? */ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, struct bgp_dest *bn, struct bgp_path_info *old_route, @@ -2818,17 +2824,24 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, bool is_withdraw = old_route && !new_route; struct bgp_path_info *updated_route = is_withdraw ? old_route : new_route; + + + /* this should never happen */ + if (!updated_route) { + zlog_warn("%s: no updated route found!", __func__); + return 0; + } + struct bmp_bgp *bmpbgp = bmp_bgp_get(bgp); struct peer *peer = updated_route->peer; struct bmp_targets *bt; struct bmp *bmp; frr_each (bmp_targets, &bmpbgp->targets, bt) { - is_locribmon_enabled |= - (bt->afimon[afi][safi] & BMP_MON_LOC_RIB); - - if (is_locribmon_enabled) + if (CHECK_FLAG(bt->afimon[afi][safi], BMP_MON_LOC_RIB)) { + is_locribmon_enabled = true; break; + } } if (!is_locribmon_enabled) @@ -2847,7 +2860,7 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, monotime(NULL); frr_each (bmp_targets, &bmpbgp->targets, bt) { - if (bt->afimon[afi][safi] & BMP_MON_LOC_RIB) { + if (CHECK_FLAG(bt->afimon[afi][safi], BMP_MON_LOC_RIB)) { struct bmp_queue_entry *last_item = bmp_process_one( bt, &bt->locupdhash, &bt->locupdlist, bgp, afi, diff --git a/doc/developer/subdir.am b/doc/developer/subdir.am index 0deb0f5da089..652ee4e1af6a 100644 --- a/doc/developer/subdir.am +++ b/doc/developer/subdir.am @@ -5,6 +5,7 @@ dev_RSTFILES = \ doc/developer/bgp-typecodes.rst \ doc/developer/bgpd.rst \ + doc/developer/bmp.rst \ doc/developer/building-frr-for-alpine.rst \ doc/developer/building-frr-for-archlinux.rst \ doc/developer/building-frr-for-centos6.rst \ diff --git a/doc/user/bmp.rst b/doc/user/bmp.rst index 6cd7e19cac3a..0f4683205978 100644 --- a/doc/user/bmp.rst +++ b/doc/user/bmp.rst @@ -36,7 +36,7 @@ The `BMP` implementation in FRR has the following properties: successfully. OPEN messages for failed sessions cannot currently be mirrored. -- **route monitoring** is available for IPv4 and IPv6 AFIs, unicast, multicast +- **route monitoring** is available for IPv4 and IPv6 AFIs, unicast, multicast, EVPN and VPN SAFIs. Other SAFIs (VPN, Labeled-Unicast, Flowspec, etc.) are not currently supported. From e65db90567747fa51e06de5cfe244edf5b19b5b0 Mon Sep 17 00:00:00 2001 From: Farid MIHOUB Date: Fri, 11 Aug 2023 16:11:03 +0200 Subject: [PATCH 22/23] tests: rework bmp policy message logging Add "policy" field to the logged bmp messages so policy checks within topotests would be easier and clearer. Signed-off-by: Farid MIHOUB --- tests/topotests/bgp_bmp/test_bgp_bmp.py | 8 ++++---- tests/topotests/lib/bmp_collector/bmp.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/topotests/bgp_bmp/test_bgp_bmp.py b/tests/topotests/bgp_bmp/test_bgp_bmp.py index 65f191b33aa7..1aeb2d9c1993 100644 --- a/tests/topotests/bgp_bmp/test_bgp_bmp.py +++ b/tests/topotests/bgp_bmp/test_bgp_bmp.py @@ -120,7 +120,7 @@ def get_bmp_messages(): return messages -def check_for_prefixes(expected_prefixes, bmp_log_type, post_policy): +def check_for_prefixes(expected_prefixes, bmp_log_type, policy): """ Check for the presence of the given prefixes in the BMP server logs with the given message type and the set policy. @@ -138,7 +138,7 @@ def check_for_prefixes(expected_prefixes, bmp_log_type, post_policy): if "ip_prefix" in m.keys() and "bmp_log_type" in m.keys() and m["bmp_log_type"] == bmp_log_type - and m["post_policy"] == post_policy + and m["policy"] == policy ] # check for prefixes @@ -202,7 +202,7 @@ def unicast_prefixes(policy): logger.info("checking for updated prefixes") # check - test_func = partial(check_for_prefixes, prefixes, "update", policy == POST_POLICY) + test_func = partial(check_for_prefixes, prefixes, "update", policy) success, _ = topotest.run_and_expect(test_func, True, wait=0.5) assert success, "Checking the updated prefixes has been failed !." @@ -210,7 +210,7 @@ def unicast_prefixes(policy): configure_prefixes(tgen, "r2", 65502, "unicast", prefixes, update=False) logger.info("checking for withdrawed prefxies") # check - test_func = partial(check_for_prefixes, prefixes, "withdraw", policy == POST_POLICY) + test_func = partial(check_for_prefixes, prefixes, "withdraw", policy) success, _ = topotest.run_and_expect(test_func, True, wait=0.5) assert success, "Checking the withdrawed prefixes has been failed !." diff --git a/tests/topotests/lib/bmp_collector/bmp.py b/tests/topotests/lib/bmp_collector/bmp.py index b07329cd523f..4ac4fdb0a930 100644 --- a/tests/topotests/lib/bmp_collector/bmp.py +++ b/tests/topotests/lib/bmp_collector/bmp.py @@ -259,7 +259,7 @@ def dissect(cls, data): is_as_path = bool(peer_flags & IS_AS_PATH) is_post_policy = bool(peer_flags & IS_POST_POLICY) is_ipv6 = bool(peer_flags & IS_IPV6) - msg['post_policy'] = is_post_policy + msg['policy'] = 'post-policy' if is_post_policy else 'pre-policy' msg['ipv6'] = is_ipv6 msg['peer_ip'] = bin2str_ipaddress(peer_address, is_ipv6) From e7adf2762fcb334219e26ce2971255525006a2a3 Mon Sep 17 00:00:00 2001 From: Farid MIHOUB Date: Fri, 11 Aug 2023 16:18:17 +0200 Subject: [PATCH 23/23] tests: extend the bmp test to support bmp loc-rib Configure the bmp monitor unicast loc-rib. Check the logging messages for the updated/withdrawn prefixes with the presence of the loc-rib peer-type. Signed-off-by: Farid MIHOUB --- tests/topotests/bgp_bmp/test_bgp_bmp.py | 3 +++ tests/topotests/lib/bmp_collector/bmp.py | 1 + 2 files changed, 4 insertions(+) diff --git a/tests/topotests/bgp_bmp/test_bgp_bmp.py b/tests/topotests/bgp_bmp/test_bgp_bmp.py index 1aeb2d9c1993..250f1cb90d0e 100644 --- a/tests/topotests/bgp_bmp/test_bgp_bmp.py +++ b/tests/topotests/bgp_bmp/test_bgp_bmp.py @@ -49,6 +49,7 @@ PRE_POLICY = "pre-policy" POST_POLICY = "post-policy" +LOC_RIB = "loc-rib" def build_topo(tgen): @@ -239,6 +240,8 @@ def test_bmp_bgp_unicast(): unicast_prefixes(PRE_POLICY) logger.info("*** Unicast prefixes post-policy logging ***") unicast_prefixes(POST_POLICY) + logger.info("*** Unicast prefixes loc-rib logging ***") + unicast_prefixes(LOC_RIB) if __name__ == "__main__": diff --git a/tests/topotests/lib/bmp_collector/bmp.py b/tests/topotests/lib/bmp_collector/bmp.py index 4ac4fdb0a930..57f642aa0ef4 100644 --- a/tests/topotests/lib/bmp_collector/bmp.py +++ b/tests/topotests/lib/bmp_collector/bmp.py @@ -252,6 +252,7 @@ def dissect(cls, data): if peer_type == 0x03: msg['is_filtered'] = bool(peer_flags & IS_FILTERED) + msg['policy'] = 'loc-rib' else: # peer_flags = 0x0000 0000 # ipv6, post-policy, as-path, adj-rib-out, reserverdx4