From c5ce29a64f0b77c3f98abea994a45db7946a1895 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 23 Sep 2024 13:23:34 +0200 Subject: [PATCH 01/23] bgpd: bmp, rename bmp->targets->bgp with peer->bgp Some BMP actions require to get the bgp instance of the given peer. Instead of considering that the BGP BMP instance is the BGP instance of the peer, let us use directly the peer->bgp pointer. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 036bece35970..b881057d05ab 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -399,14 +399,17 @@ static void bmp_put_info_tlv(struct stream *s, uint16_t type, /* put the vrf table name of the bgp instance bmp is bound to in a tlv on the * stream */ -static void bmp_put_vrftablename_info_tlv(struct stream *s, struct bgp *bgp) +static void bmp_put_vrftablename_info_tlv(struct stream *s, struct peer *peer) { const char *vrftablename = "global"; + struct vrf *vrf; #define BMP_INFO_TYPE_VRFTABLENAME 3 - if (bgp->inst_type != BGP_INSTANCE_TYPE_DEFAULT) - vrftablename = bgp->name; + if (peer->bgp->inst_type != BGP_INSTANCE_TYPE_DEFAULT) { + vrf = vrf_lookup_by_id(peer->bgp->vrf_id); + vrftablename = vrf ? vrf->name : NULL; + } if (vrftablename != NULL) bmp_put_info_tlv(s, BMP_INFO_TYPE_VRFTABLENAME, vrftablename); @@ -590,7 +593,7 @@ static struct stream *bmp_peerstate(struct peer *peer, bool down) } if (is_locrib) - bmp_put_vrftablename_info_tlv(s, peer->bgp); + bmp_put_vrftablename_info_tlv(s, peer); len = stream_get_endp(s); stream_putl_at(s, BMP_LENGTH_POS, len); /* message length is set. */ @@ -847,8 +850,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, bmp->targets->bgp, peer, 0, peer_type_flag, peer_distinguisher, - &bmq->tv); + bmp_per_peer_hdr(s, peer->bgp, peer, 0, peer_type_flag, peer_distinguisher, &bmq->tv); /* BMP Mirror TLV. */ stream_putw(s, BMP_MIRROR_TLV_TYPE_BGP_MESSAGE); @@ -1141,8 +1143,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, - peer_distinguisher, + bmp_per_peer_hdr(hdr, peer->bgp, peer, flags, peer_type_flag, peer_distinguisher, uptime == (time_t)(-1L) ? NULL : &uptime_real); stream_putl_at(hdr, BMP_LENGTH_POS, @@ -1446,7 +1447,7 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) */ goto out; } - if (peer != bmp->targets->bgp->peer_self && !peer_established(peer->connection)) { + if (peer != peer->bgp->peer_self && !peer_established(peer->connection)) { /* peer is neither self, nor established */ goto out; @@ -1457,8 +1458,7 @@ static bool bmp_wrqueue_locrib(struct bmp *bmp, struct pullwr *pullwr) struct prefix_rd *prd = is_vpn ? &bqe->rd : NULL; - bn = bgp_safi_node_lookup(bmp->targets->bgp->rib[afi][safi], safi, - &bqe->p, prd); + bn = bgp_safi_node_lookup(peer->bgp->rib[afi][safi], safi, &bqe->p, prd); struct bgp_path_info *bpi; @@ -1534,8 +1534,7 @@ static bool bmp_wrqueue(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); + bn = bgp_safi_node_lookup(peer->bgp->rib[afi][safi], safi, &bqe->p, prd); peer_type_flag = bmp_get_peer_type(peer); From 2a609860c8a3e2418d6f951ea2467683ff2a7ac2 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 19 Sep 2024 18:04:02 +0200 Subject: [PATCH 02/23] bgpd: add 'bmp import-vrf-view' command Add a configuration command to import BGP information from another BGP instance. Specifically, it should be possible for a user to have a BMP instance configured on the default VRF, and be able to import the VRF information from the other BGP VRF instances. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++ bgpd/bgp_bmp.h | 11 ++++++ 2 files changed, 114 insertions(+) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index b881057d05ab..c67935c52c7c 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -64,6 +64,7 @@ DEFINE_MTYPE_STATIC(BMP, BMP, "BMP instance state"); DEFINE_MTYPE_STATIC(BMP, BMP_MIRRORQ, "BMP route mirroring buffer"); DEFINE_MTYPE_STATIC(BMP, BMP_PEER, "BMP per BGP peer data"); DEFINE_MTYPE_STATIC(BMP, BMP_OPEN, "BMP stored BGP OPEN message"); +DEFINE_MTYPE_STATIC(BMP, BMP_IMPORTED_BGP, "BMP imported BGP instance"); DEFINE_QOBJ_TYPE(bmp_targets); @@ -141,6 +142,17 @@ static int bmp_targets_cmp(const struct bmp_targets *a, DECLARE_SORTLIST_UNIQ(bmp_targets, struct bmp_targets, bti, bmp_targets_cmp); +static int bmp_imported_bgps_cmp(const struct bmp_imported_bgp *a, const struct bmp_imported_bgp *b) +{ + if (a->name == NULL && b->name == NULL) + return 0; + if (a->name == NULL || b->name == NULL) + return 1; + return strcmp(a->name, b->name); +} + +DECLARE_SORTLIST_UNIQ(bmp_imported_bgps, struct bmp_imported_bgp, bib, bmp_imported_bgps_cmp); + DECLARE_LIST(bmp_session, struct bmp, bsi); DECLARE_DLIST(bmp_qlist, struct bmp_queue_entry, bli); @@ -2145,16 +2157,25 @@ static struct bmp_targets *bmp_targets_get(struct bgp *bgp, const char *name) bmp_qlist_init(&bt->locupdlist); bmp_actives_init(&bt->actives); bmp_listeners_init(&bt->listeners); + bmp_imported_bgps_init(&bt->imported_bgps); QOBJ_REG(bt, bmp_targets); bmp_targets_add(&bt->bmpbgp->targets, bt); return bt; } +static void bmp_imported_bgp_free(struct bmp_imported_bgp *bib) +{ + if (bib->name) + XFREE(MTYPE_BMP_IMPORTED_BGP, bib->name); + XFREE(MTYPE_BMP_IMPORTED_BGP, bib); +} + static void bmp_targets_put(struct bmp_targets *bt) { struct bmp *bmp; struct bmp_active *ba; + struct bmp_imported_bgp *bib; EVENT_OFF(bt->t_stats); @@ -2169,6 +2190,10 @@ static void bmp_targets_put(struct bmp_targets *bt) bmp_targets_del(&bt->bmpbgp->targets, bt); QOBJ_UNREG(bt); + frr_each_safe (bmp_imported_bgps, &bt->imported_bgps, bib) + bmp_imported_bgp_free(bib); + + bmp_imported_bgps_fini(&bt->imported_bgps); bmp_listeners_fini(&bt->listeners); bmp_actives_fini(&bt->actives); bmp_qhash_fini(&bt->updhash); @@ -2213,6 +2238,36 @@ static struct bmp_listener *bmp_listener_get(struct bmp_targets *bt, return bl; } +static struct bmp_imported_bgp *bmp_imported_bgp_find(struct bmp_targets *bt, char *name) +{ + struct bmp_imported_bgp dummy; + + dummy.name = name; + return bmp_imported_bgps_find(&bt->imported_bgps, &dummy); +} + +static void bmp_imported_bgp_put(struct bmp_targets *bt, struct bmp_imported_bgp *bib) +{ + bmp_imported_bgps_del(&bt->imported_bgps, bib); + bmp_imported_bgp_free(bib); +} + +static struct bmp_imported_bgp *bmp_imported_bgp_get(struct bmp_targets *bt, char *name) +{ + struct bmp_imported_bgp *bib = bmp_imported_bgp_find(bt, name); + + if (bib) + return bib; + + bib = XCALLOC(MTYPE_BMP_IMPORTED_BGP, sizeof(*bib)); + if (name) + bib->name = XSTRDUP(MTYPE_BMP_IMPORTED_BGP, name); + bib->targets = bt; + bmp_imported_bgps_add(&bt->imported_bgps, bib); + + return bib; +} + static void bmp_listener_start(struct bmp_listener *bl) { int sock, ret; @@ -2573,6 +2628,47 @@ DEFPY(no_bmp_targets_main, return CMD_SUCCESS; } +DEFPY(bmp_import_vrf, + bmp_import_vrf_cmd, + "[no] bmp import-vrf-view VRFNAME$vrfname", + NO_STR + BMP_STR + "Import BMP information from another VRF\n" + "Specify the VRF or view instance name\n") +{ + VTY_DECLVAR_CONTEXT_SUB(bmp_targets, bt); + struct bmp_imported_bgp *bib; + + if (!bt->bgp) { + vty_out(vty, "%% BMP target, BGP instance not found\n"); + return CMD_WARNING; + } + if ((bt->bgp->name == NULL && vrfname == NULL) || + (bt->bgp->name && vrfname && strmatch(vrfname, bt->bgp->name))) { + vty_out(vty, "%% BMP target, can not import our own BGP instance\n"); + return CMD_WARNING; + } + if (no) { + bib = bmp_imported_bgp_find(bt, (char *)vrfname); + if (!bib) { + vty_out(vty, "%% BMP imported BGP instance not found\n"); + return CMD_WARNING; + } + /* TODO: handle loc-rib peer down change */ + bmp_imported_bgp_put(bt, bib); + return CMD_SUCCESS; + } + bib = bmp_imported_bgp_find(bt, (char *)vrfname); + if (bib) + return CMD_SUCCESS; + + bmp_imported_bgp_get(bt, (char *)vrfname); + /* TODO: handle loc-rib peer up and other peers state changes + * TODO: Start the syncronisation + */ + return CMD_SUCCESS; +} + DEFPY(bmp_listener_main, bmp_listener_cmd, "bmp listener port (1-65535)", @@ -3010,6 +3106,7 @@ static int bmp_config_write(struct bgp *bgp, struct vty *vty) struct bmp_targets *bt; struct bmp_listener *bl; struct bmp_active *ba; + struct bmp_imported_bgp *bib; afi_t afi; safi_t safi; @@ -3052,6 +3149,11 @@ static int bmp_config_write(struct bgp *bgp, struct vty *vty) vty_out(vty, " bmp monitor %s %s loc-rib\n", afi2str_lower(afi), safi2str(safi)); } + + frr_each (bmp_imported_bgps, &bt->imported_bgps, bib) + vty_out(vty, " bmp import-vrf-view %s\n", + bib->name ? bib->name : VRF_DEFAULT_NAME); + frr_each (bmp_listeners, &bt->listeners, bl) vty_out(vty, " bmp listener %pSU port %d\n", &bl->addr, bl->port); @@ -3089,6 +3191,7 @@ static int bgp_bmp_init(struct event_loop *tm) install_element(BMP_NODE, &bmp_stats_cmd); install_element(BMP_NODE, &bmp_monitor_cmd); install_element(BMP_NODE, &bmp_mirror_cmd); + install_element(BMP_NODE, &bmp_import_vrf_cmd); install_element(BGP_NODE, &bmp_mirror_limit_cmd); install_element(BGP_NODE, &no_bmp_mirror_limit_cmd); diff --git a/bgpd/bgp_bmp.h b/bgpd/bgp_bmp.h index d45a4278f69e..bde65f6bdd7c 100644 --- a/bgpd/bgp_bmp.h +++ b/bgpd/bgp_bmp.h @@ -195,6 +195,9 @@ struct bmp_listener { int sock; }; +/* config for imported bgp instances */ +PREDECL_SORTLIST_UNIQ(bmp_imported_bgps); + /* bmp_targets - plural since it may contain multiple bmp_listener & * bmp_active items. If they have the same config, BMP session should be * put in the same targets since that's a bit more effective. @@ -238,6 +241,8 @@ struct bmp_targets { struct bmp_qhash_head locupdhash; struct bmp_qlist_head locupdlist; + struct bmp_imported_bgps_head imported_bgps; + uint64_t cnt_accept, cnt_aclrefused; bool stats_send_experimental; @@ -274,6 +279,12 @@ enum bmp_vrf_state { vrf_state_up = 1, }; +struct bmp_imported_bgp { + struct bmp_imported_bgps_item bib; + struct bmp_targets *targets; + char *name; +}; + struct bmp_bgp { struct bmp_bgph_item bbi; From c2c95b29ba27574a92626242740e61cb7ffc5ca8 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 19 Sep 2024 18:35:50 +0200 Subject: [PATCH 03/23] bgpd: bmp, rework the bmp_route_update() function Separate the bmp_route_update() function in two, by passing the bgpbmp structure to the internal function instead of the bgp instance. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 64 +++++++++++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index c67935c52c7c..d1f2083f745c 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -50,6 +50,9 @@ static struct bmp_bgp_peer *bmp_bgp_peer_find(uint64_t peerid); static struct bmp_bgp_peer *bmp_bgp_peer_get(struct peer *peer); static void bmp_active_disconnected(struct bmp_active *ba); static void bmp_active_put(struct bmp_active *ba); +static int bmp_route_update_bgpbmp(struct bmp_targets *bt, afi_t afi, safi_t safi, + struct bgp_dest *bn, struct bgp_path_info *old_route, + struct bgp_path_info *new_route); DEFINE_MGROUP(BMP, "BMP (BGP Monitoring Protocol)"); @@ -3207,11 +3210,12 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, 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; + struct bmp_targets *bt; + int ret = 0; /* this should never happen */ if (!updated_route) { @@ -3219,23 +3223,25 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, return 0; } - struct bmp_bgp *bmpbgp = bmp_bgp_find(bgp); - struct peer *peer = updated_route->peer; - struct bmp_targets *bt; - struct bmp *bmp; - + bmpbgp = bmp_bgp_find(bgp); if (!bmpbgp) return 0; - frr_each (bmp_targets, &bmpbgp->targets, bt) { - if (CHECK_FLAG(bt->afimon[afi][safi], BMP_MON_LOC_RIB)) { - is_locribmon_enabled = true; - break; - } - } + frr_each (bmp_targets, &bmpbgp->targets, bt) + if (CHECK_FLAG(bt->afimon[afi][safi], BMP_MON_LOC_RIB)) + ret = bmp_route_update_bgpbmp(bt, afi, safi, bn, old_route, new_route); + return ret; +} - if (!is_locribmon_enabled) - return 0; +static int bmp_route_update_bgpbmp(struct bmp_targets *bt, afi_t afi, safi_t safi, + struct bgp_dest *bn, struct bgp_path_info *old_route, + struct bgp_path_info *new_route) +{ + bool is_withdraw = old_route && !new_route; + struct bgp_path_info *updated_route = is_withdraw ? old_route : new_route; + struct peer *peer = updated_route->peer; + struct bmp *bmp; + struct bmp_queue_entry *last_item; /* route is not installed in locrib anymore and rib uptime was saved */ if (old_route && old_route->extra) @@ -3249,26 +3255,20 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, bgp_path_info_extra_get(new_route)->bgp_rib_uptime = monotime(NULL); - frr_each (bmp_targets, &bmpbgp->targets, bt) { - 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, - safi, bn, peer); + last_item = bmp_process_one(bt, &bt->locupdhash, &bt->locupdlist, bt->bgp, afi, safi, bn, + peer); - /* if bmp_process_one returns NULL - * we don't have anything to do next - */ - if (!last_item) - continue; + /* if bmp_process_one returns NULL + * we don't have anything to do next + */ + if (!last_item) + return 0; - frr_each (bmp_session, &bt->sessions, bmp) { - if (!bmp->locrib_queuepos) - bmp->locrib_queuepos = last_item; + frr_each (bmp_session, &bt->sessions, bmp) { + if (!bmp->locrib_queuepos) + bmp->locrib_queuepos = last_item; - pullwr_bump(bmp->pullwr); - }; - } + pullwr_bump(bmp->pullwr); }; return 0; From c032d532556ab4e1cbef9862f7eb9100dbd1126e Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Fri, 20 Sep 2024 08:47:56 +0200 Subject: [PATCH 04/23] bgpd: bmp, handle imported bgp instances for route updates Upon route update, the list of available BGP instances that import the BGP instance where this updates comes from, is checked. For each eligible BGP instance, the route update is sent. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index d1f2083f745c..91d566f3e57f 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -3216,6 +3216,8 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, struct bmp_bgp *bmpbgp; struct bmp_targets *bt; int ret = 0; + struct bgp *bgp_vrf; + struct listnode *node; /* this should never happen */ if (!updated_route) { @@ -3223,13 +3225,18 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi, return 0; } - bmpbgp = bmp_bgp_find(bgp); - if (!bmpbgp) - return 0; - - frr_each (bmp_targets, &bmpbgp->targets, bt) - if (CHECK_FLAG(bt->afimon[afi][safi], BMP_MON_LOC_RIB)) + for (ALL_LIST_ELEMENTS_RO(bm->bgp, node, bgp_vrf)) { + bmpbgp = bmp_bgp_find(bgp_vrf); + if (!bmpbgp) + continue; + frr_each (bmp_targets, &bmpbgp->targets, bt) { + if (!CHECK_FLAG(bt->afimon[afi][safi], BMP_MON_LOC_RIB)) + continue; + if (bgp_vrf != bgp && !bmp_imported_bgp_find(bt, bgp->name)) + continue; ret = bmp_route_update_bgpbmp(bt, afi, safi, bn, old_route, new_route); + } + } return ret; } From f8a693311145559fe4072db7cf1e04e0a0681957 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 7 Nov 2024 14:28:39 +0100 Subject: [PATCH 05/23] bgpd: bmp, handle imported bgp instances for peer up/down events Only the peer transition events of the local BGP instance where BMP is configured, were handled. Add the support for peers from imported BGP instances: - Add an internal API bmp_send_bt() function to handle stream emission per bmp target - Modify the BMP peer transition code to export the peer status notifications to all BMP instances importing it. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 55 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 91d566f3e57f..99f9206fb2c0 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -53,6 +53,7 @@ static void bmp_active_put(struct bmp_active *ba); static int bmp_route_update_bgpbmp(struct bmp_targets *bt, afi_t afi, safi_t safi, struct bgp_dest *bn, struct bgp_path_info *old_route, struct bgp_path_info *new_route); +static void bmp_send_all_bgp(struct peer *peer, bool down); DEFINE_MGROUP(BMP, "BMP (BGP Monitoring Protocol)"); @@ -652,19 +653,26 @@ static int bmp_send_peerup_vrf(struct bmp *bmp) return 0; } +static void bmp_send_bt(struct bmp_targets *bt, struct stream *s) +{ + struct bmp *bmp; + + frr_each (bmp_session, &bt->sessions, bmp) + pullwr_write_stream(bmp->pullwr, s); +} + /* send a stream to all bmp sessions configured in a bgp instance */ /* XXX: kludge - filling the pullwr's buffer */ static void bmp_send_all(struct bmp_bgp *bmpbgp, struct stream *s) { struct bmp_targets *bt; - struct bmp *bmp; if (!s) return; frr_each(bmp_targets, &bmpbgp->targets, bt) - frr_each(bmp_session, &bt->sessions, bmp) - pullwr_write_stream(bmp->pullwr, s); + bmp_send_bt(bt, s); + stream_free(s); } @@ -904,14 +912,10 @@ static int bmp_outgoing_packet(struct peer *peer, uint8_t type, bgp_size_t size, static int bmp_peer_status_changed(struct peer *peer) { - struct bmp_bgp *bmpbgp = bmp_bgp_find(peer->bgp); struct bmp_bgp_peer *bbpeer, *bbdopp; frrtrace(1, frr_bgp, bmp_peer_status_changed, peer); - if (!bmpbgp) - return 0; - if (peer->connection->status == Deleted) { bbpeer = bmp_bgp_peer_find(peer->qobj_node.nid); if (bbpeer) { @@ -946,20 +950,16 @@ static int bmp_peer_status_changed(struct peer *peer) } } - bmp_send_all_safe(bmpbgp, bmp_peerstate(peer, false)); + bmp_send_all_bgp(peer, false); return 0; } static int bmp_peer_backward(struct peer *peer) { - struct bmp_bgp *bmpbgp = bmp_bgp_find(peer->bgp); struct bmp_bgp_peer *bbpeer; frrtrace(1, frr_bgp, bmp_peer_backward_transition, peer); - if (!bmpbgp) - return 0; - bbpeer = bmp_bgp_peer_find(peer->qobj_node.nid); if (bbpeer) { XFREE(MTYPE_BMP_OPEN, bbpeer->open_tx); @@ -968,7 +968,7 @@ static int bmp_peer_backward(struct peer *peer) bbpeer->open_rx_len = 0; } - bmp_send_all_safe(bmpbgp, bmp_peerstate(peer, true)); + bmp_send_all_bgp(peer, true); return 0; } @@ -2249,6 +2249,35 @@ static struct bmp_imported_bgp *bmp_imported_bgp_find(struct bmp_targets *bt, ch return bmp_imported_bgps_find(&bt->imported_bgps, &dummy); } +static void bmp_send_all_bgp(struct peer *peer, bool down) +{ + struct bmp_bgp *bmpbgp = bmp_bgp_find(peer->bgp); + struct bgp *bgp_vrf; + struct listnode *node; + struct stream *s = NULL; + struct bmp_targets *bt; + + s = bmp_peerstate(peer, down); + if (!s) + return; + + if (bmpbgp) { + frr_each (bmp_targets, &bmpbgp->targets, bt) + bmp_send_bt(bt, s); + } + for (ALL_LIST_ELEMENTS_RO(bm->bgp, node, bgp_vrf)) { + bmpbgp = bmp_bgp_find(bgp_vrf); + if (!bmpbgp) + continue; + frr_each (bmp_targets, &bmpbgp->targets, bt) { + if (bgp_vrf != peer->bgp && !bmp_imported_bgp_find(bt, peer->bgp->name)) + continue; + bmp_send_bt(bt, s); + } + } + stream_free(s); +} + static void bmp_imported_bgp_put(struct bmp_targets *bt, struct bmp_imported_bgp *bib) { bmp_imported_bgps_del(&bt->imported_bgps, bib); From ee605d80ab5bbf4f7f090f214a50ed3374ac98f8 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Fri, 20 Sep 2024 13:47:17 +0200 Subject: [PATCH 06/23] bgpd: bmp, handle imported bgp instances in bmp_process Modify the bmp_process() function to export the route update information to all BMP instances importing it. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 49 ++++++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 99f9206fb2c0..4156a74b867c 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -54,6 +54,7 @@ static int bmp_route_update_bgpbmp(struct bmp_targets *bt, afi_t afi, safi_t saf struct bgp_dest *bn, struct bgp_path_info *old_route, struct bgp_path_info *new_route); static void bmp_send_all_bgp(struct peer *peer, bool down); +static struct bmp_imported_bgp *bmp_imported_bgp_find(struct bmp_targets *bt, char *name); DEFINE_MGROUP(BMP, "BMP (BGP Monitoring Protocol)"); @@ -1681,9 +1682,12 @@ bmp_process_one(struct bmp_targets *bt, struct bmp_qhash_head *updhash, static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi, struct bgp_dest *bn, struct peer *peer, bool withdraw) { - struct bmp_bgp *bmpbgp = bmp_bgp_find(peer->bgp); + struct bmp_bgp *bmpbgp; struct bmp_targets *bt; struct bmp *bmp; + struct bgp *bgp_vrf; + struct listnode *node; + struct bmp_queue_entry *last_item; if (frrtrace_enabled(frr_bgp, bmp_process)) { char pfxprint[PREFIX2STR_BUFFER]; @@ -1693,31 +1697,34 @@ static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi, withdraw); } - if (!bmpbgp) - return 0; - - frr_each(bmp_targets, &bmpbgp->targets, bt) { - /* check if any monitoring is enabled (ignoring loc-rib since it - * uses another hook & queue - */ - if (!CHECK_FLAG(bt->afimon[afi][safi], ~BMP_MON_LOC_RIB)) + for (ALL_LIST_ELEMENTS_RO(bm->bgp, node, bgp_vrf)) { + bmpbgp = bmp_bgp_find(bgp_vrf); + if (!bmpbgp) continue; + frr_each (bmp_targets, &bmpbgp->targets, bt) { + /* check if any monitoring is enabled (ignoring loc-rib since it + * uses another hook & queue + */ + if (!CHECK_FLAG(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); + if (bgp_vrf != peer->bgp && !bmp_imported_bgp_find(bt, peer->bgp->name)) + continue; - /* if bmp_process_one returns NULL - * we don't have anything to do next - */ - if (!last_item) - continue; + last_item = 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 (!last_item) + continue; - frr_each(bmp_session, &bt->sessions, bmp) { - if (!bmp->queuepos) - bmp->queuepos = last_item; + frr_each (bmp_session, &bt->sessions, bmp) { + if (!bmp->queuepos) + bmp->queuepos = last_item; - pullwr_bump(bmp->pullwr); + pullwr_bump(bmp->pullwr); + } } } return 0; From b1ebe54b29524ed3df5c92cac65f06ab374eb236 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Fri, 20 Sep 2024 16:50:07 +0200 Subject: [PATCH 07/23] bgpd: bmp, handle imported bgp instances in bmp_mirror Modify the bmp_mirror() function to export the route update information to all BMP instances importing it. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 56 +++++++++++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 21 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 4156a74b867c..3bac6a8f1d17 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -752,11 +752,13 @@ static void bmp_mirror_cull(struct bmp_bgp *bmpbgp) static int bmp_mirror_packet(struct peer *peer, uint8_t type, bgp_size_t size, struct stream *packet) { - struct bmp_bgp *bmpbgp = bmp_bgp_find(peer->bgp); + struct bmp_bgp *bmpbgp; struct timeval tv; - struct bmp_mirrorq *qitem; + struct bmp_mirrorq *qitem = NULL; struct bmp_targets *bt; struct bmp *bmp; + struct bgp *bgp_vrf; + struct listnode *node; frrtrace(3, frr_bgp, bmp_mirror_packet, peer, type, packet); @@ -772,8 +774,6 @@ static int bmp_mirror_packet(struct peer *peer, uint8_t type, bgp_size_t size, memcpy(bbpeer->open_rx, packet->data, size); } - if (!bmpbgp) - return 0; qitem = XCALLOC(MTYPE_BMP_MIRRORQ, sizeof(*qitem) + size); qitem->peerid = peer->qobj_node.nid; @@ -781,27 +781,41 @@ static int bmp_mirror_packet(struct peer *peer, uint8_t type, bgp_size_t size, qitem->len = size; memcpy(qitem->data, packet->data, size); - frr_each(bmp_targets, &bmpbgp->targets, bt) { - if (!bt->mirror) + for (ALL_LIST_ELEMENTS_RO(bm->bgp, node, bgp_vrf)) { + bmpbgp = bmp_bgp_find(bgp_vrf); + if (!bmpbgp) continue; - frr_each(bmp_session, &bt->sessions, bmp) { - qitem->refcount++; - if (!bmp->mirrorpos) - bmp->mirrorpos = qitem; - pullwr_bump(bmp->pullwr); - } - } - if (qitem->refcount == 0) - XFREE(MTYPE_BMP_MIRRORQ, qitem); - else { - bmpbgp->mirror_qsize += sizeof(*qitem) + size; - bmp_mirrorq_add_tail(&bmpbgp->mirrorq, qitem); + frr_each (bmp_targets, &bmpbgp->targets, bt) { + if (!bt->mirror) + continue; + + if (bgp_vrf != peer->bgp && !bmp_imported_bgp_find(bt, peer->bgp->name)) + continue; + + frr_each (bmp_session, &bt->sessions, bmp) { + if (!qitem) { + qitem = XCALLOC(MTYPE_BMP_MIRRORQ, sizeof(*qitem) + size); + qitem->peerid = peer->qobj_node.nid; + qitem->tv = tv; + qitem->len = size; + memcpy(qitem->data, packet->data, size); + } - bmp_mirror_cull(bmpbgp); + qitem->refcount++; + if (!bmp->mirrorpos) + bmp->mirrorpos = qitem; + pullwr_bump(bmp->pullwr); + } + bmpbgp->mirror_qsize += sizeof(*qitem) + size; + bmp_mirrorq_add_tail(&bmpbgp->mirrorq, qitem); - bmpbgp->mirror_qsizemax = MAX(bmpbgp->mirror_qsizemax, - bmpbgp->mirror_qsize); + bmp_mirror_cull(bmpbgp); + + bmpbgp->mirror_qsizemax = MAX(bmpbgp->mirror_qsizemax, bmpbgp->mirror_qsize); + } } + if (qitem && qitem->refcount == 0) + XFREE(MTYPE_BMP_MIRRORQ, qitem); return 0; } From 841ae62a898d12d29bd8f6e721de423870650439 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 23 Sep 2024 12:01:20 +0200 Subject: [PATCH 08/23] bgpd: bmp, handle imported bgp instances in bmp_send_peerup() When a BMP target comes up, only the peer up events of the current BGP instance are sent. - Apply the peer up event for external peers that are imported by the BMP target. - handle the peer up event when an imported vrf is configured in a target. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 3bac6a8f1d17..f98361f0ab25 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -617,21 +617,34 @@ static struct stream *bmp_peerstate(struct peer *peer, bool down) return s; } - -static int bmp_send_peerup(struct bmp *bmp) +static int bmp_send_peerup_per_instance(struct bmp *bmp, struct bgp *bgp) { struct peer *peer; struct listnode *node; struct stream *s; /* Walk down all peers */ - for (ALL_LIST_ELEMENTS_RO(bmp->targets->bgp->peer, node, peer)) { + for (ALL_LIST_ELEMENTS_RO(bgp->peer, node, peer)) { s = bmp_peerstate(peer, false); if (s) { pullwr_write_stream(bmp->pullwr, s); stream_free(s); } } + return 0; +} + +static int bmp_send_peerup(struct bmp *bmp) +{ + struct bmp_imported_bgp *bib; + struct bgp *bgp; + + bmp_send_peerup_per_instance(bmp, bmp->targets->bgp); + frr_each (bmp_imported_bgps, &bmp->targets->imported_bgps, bib) { + bgp = bgp_lookup_by_name(bib->name); + if (bgp) + bmp_send_peerup_per_instance(bmp, bgp); + } return 0; } @@ -2691,6 +2704,8 @@ DEFPY(bmp_import_vrf, { VTY_DECLVAR_CONTEXT_SUB(bmp_targets, bt); struct bmp_imported_bgp *bib; + struct bgp *bgp; + struct bmp *bmp; if (!bt->bgp) { vty_out(vty, "%% BMP target, BGP instance not found\n"); @@ -2715,10 +2730,18 @@ DEFPY(bmp_import_vrf, if (bib) return CMD_SUCCESS; - bmp_imported_bgp_get(bt, (char *)vrfname); - /* TODO: handle loc-rib peer up and other peers state changes + bib = bmp_imported_bgp_get(bt, (char *)vrfname); + bgp = bgp_lookup_by_name(bib->name); + if (!bgp) + return CMD_SUCCESS; + /* TODO: handle loc-rib peer up changes * TODO: Start the syncronisation */ + frr_each (bmp_session, &bt->sessions, bmp) { + if (bmp->state != BMP_PeerUp && bmp->state != BMP_Run) + continue; + bmp_send_peerup_per_instance(bmp, bgp); + } return CMD_SUCCESS; } From 5fda10f4b8a8c4d3bb6431a7e629eab84d276def Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 23 Sep 2024 15:03:15 +0200 Subject: [PATCH 09/23] bgpd: bmp, handle imported bgp instances for bmp statistics Only the BMP statistics of the current BGP instance are handled. Extend the transmission of BMP statistics for imported BGP instances. - Separate the bmp_stats() function in two, and pass the bgp instance to process its bgp peers, as a separate parameter. - Pass the BGP peers from imported instances as parameter Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index f98361f0ab25..8c5a72146dc7 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -55,6 +55,7 @@ static int bmp_route_update_bgpbmp(struct bmp_targets *bt, afi_t afi, safi_t saf struct bgp_path_info *new_route); static void bmp_send_all_bgp(struct peer *peer, bool down); static struct bmp_imported_bgp *bmp_imported_bgp_find(struct bmp_targets *bt, char *name); +static void bmp_stats_per_instance(struct bgp *bgp, struct bmp_targets *bt); DEFINE_MGROUP(BMP, "BMP (BGP Monitoring Protocol)"); @@ -1798,6 +1799,22 @@ static void bmp_stat_put_u64(struct stream *s, size_t *cnt, uint16_t type, static void bmp_stats(struct event *thread) { struct bmp_targets *bt = EVENT_ARG(thread); + struct bmp_imported_bgp *bib; + struct bgp *bgp; + + if (bt->stat_msec) + event_add_timer_msec(bm->master, bmp_stats, bt, bt->stat_msec, &bt->t_stats); + + bmp_stats_per_instance(bt->bgp, bt); + frr_each (bmp_imported_bgps, &bt->imported_bgps, bib) { + bgp = bgp_lookup_by_name(bib->name); + if (bgp) + bmp_stats_per_instance(bgp, bt); + } +} + +static void bmp_stats_per_instance(struct bgp *bgp, struct bmp_targets *bt) +{ struct stream *s; struct peer *peer; struct listnode *node; @@ -1805,14 +1822,10 @@ static void bmp_stats(struct event *thread) uint8_t peer_type_flag; uint64_t peer_distinguisher = 0; - if (bt->stat_msec) - event_add_timer_msec(bm->master, bmp_stats, bt, bt->stat_msec, - &bt->t_stats); - gettimeofday(&tv, NULL); /* Walk down all peers */ - for (ALL_LIST_ELEMENTS_RO(bt->bgp->peer, node, peer)) { + for (ALL_LIST_ELEMENTS_RO(bgp->peer, node, peer)) { size_t count = 0, count_pos, len; if (!peer_established(peer->connection)) From f52e963b98a11056745e73e94a20dd0fe001b415 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 28 Oct 2024 18:52:58 +0100 Subject: [PATCH 10/23] topotests: bgp_bmp, add test for import-vrf-view service Add a test with a new peer defined in a VRF, and where the BGP updates are imported in the BMP instance of the default BGP instance. Signed-off-by: Philippe Guibert --- .../bmp1import/bmp-update-loc-rib-step1.json | 34 +++ .../bmp-update-post-policy-step1.json | 36 +++ .../bmp-update-pre-policy-step1.json | 36 +++ .../bmp-withdraw-loc-rib-step1.json | 28 ++ .../bmp-withdraw-loc-rib-step2.json | 34 +++ .../bmp-withdraw-post-policy-step1.json | 30 ++ .../bmp-withdraw-post-policy-step2.json | 36 +++ .../bmp-withdraw-pre-policy-step1.json | 30 ++ tests/topotests/bgp_bmp/r1import/frr.conf | 73 +++++ .../show-bgp-vrf1-ipv4-update-step1.json | 21 ++ .../show-bgp-vrf1-ipv4-withdraw-step1.json | 6 + .../show-bgp-vrf1-ipv6-update-step1.json | 27 ++ .../show-bgp-vrf1-ipv6-withdraw-step1.json | 6 + tests/topotests/bgp_bmp/r3/frr.conf | 18 ++ tests/topotests/bgp_bmp/test_bgp_bmp_3.py | 267 ++++++++++++++++++ 15 files changed, 682 insertions(+) create mode 100644 tests/topotests/bgp_bmp/bmp1import/bmp-update-loc-rib-step1.json create mode 100644 tests/topotests/bgp_bmp/bmp1import/bmp-update-post-policy-step1.json create mode 100644 tests/topotests/bgp_bmp/bmp1import/bmp-update-pre-policy-step1.json create mode 100644 tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-loc-rib-step1.json create mode 100644 tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-loc-rib-step2.json create mode 100644 tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-post-policy-step1.json create mode 100644 tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-post-policy-step2.json create mode 100644 tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-pre-policy-step1.json create mode 100644 tests/topotests/bgp_bmp/r1import/frr.conf create mode 100644 tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv4-update-step1.json create mode 100644 tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv4-withdraw-step1.json create mode 100644 tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv6-update-step1.json create mode 100644 tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv6-withdraw-step1.json create mode 100644 tests/topotests/bgp_bmp/r3/frr.conf create mode 100644 tests/topotests/bgp_bmp/test_bgp_bmp_3.py diff --git a/tests/topotests/bgp_bmp/bmp1import/bmp-update-loc-rib-step1.json b/tests/topotests/bgp_bmp/bmp1import/bmp-update-loc-rib-step1.json new file mode 100644 index 000000000000..3542f4e49513 --- /dev/null +++ b/tests/topotests/bgp_bmp/bmp1import/bmp-update-loc-rib-step1.json @@ -0,0 +1,34 @@ +{ + "loc-rib": { + "update": { + "172.31.0.77/32": { + "as_path": "", + "bgp_nexthop": "192.168.1.3", + "bmp_log_type": "update", + "ip_prefix": "172.31.0.77/32", + "is_filtered": false, + "origin": "IGP", + "peer_asn": 65501, + "peer_bgp_id": "192.168.0.1", + "peer_distinguisher": "444:1", + "peer_type": "loc-rib instance", + "policy": "loc-rib" + }, + "2001::1125/128": { + "afi": 2, + "as_path": "", + "bmp_log_type": "update", + "ip_prefix": "2001::1125/128", + "is_filtered": false, + "nxhp_ip": "192:167::3", + "origin": "IGP", + "peer_asn": 65501, + "peer_bgp_id": "192.168.0.1", + "peer_distinguisher": "555:1", + "peer_type": "loc-rib instance", + "policy": "loc-rib", + "safi": 1 + } + } + } +} diff --git a/tests/topotests/bgp_bmp/bmp1import/bmp-update-post-policy-step1.json b/tests/topotests/bgp_bmp/bmp1import/bmp-update-post-policy-step1.json new file mode 100644 index 000000000000..cf71f2048509 --- /dev/null +++ b/tests/topotests/bgp_bmp/bmp1import/bmp-update-post-policy-step1.json @@ -0,0 +1,36 @@ +{ + "post-policy": { + "update": { + "172.31.0.77/32": { + "as_path": "", + "bgp_nexthop": "192.168.1.3", + "bmp_log_type": "update", + "ip_prefix": "172.31.0.77/32", + "ipv6": false, + "origin": "IGP", + "peer_asn": 65501, + "peer_bgp_id": "192.168.1.3", + "peer_distinguisher": "444:1", + "peer_ip": "192.168.1.3", + "peer_type": "route distinguisher instance", + "policy": "post-policy" + }, + "2001::1125/128": { + "afi": 2, + "as_path": "", + "bmp_log_type": "update", + "ip_prefix": "2001::1125/128", + "ipv6": true, + "nxhp_ip": "192:167::3", + "origin": "IGP", + "peer_asn": 65501, + "peer_bgp_id": "192.168.1.3", + "peer_distinguisher": "555:1", + "peer_ip": "192:167::3", + "peer_type": "route distinguisher instance", + "policy": "post-policy", + "safi": 1 + } + } + } +} diff --git a/tests/topotests/bgp_bmp/bmp1import/bmp-update-pre-policy-step1.json b/tests/topotests/bgp_bmp/bmp1import/bmp-update-pre-policy-step1.json new file mode 100644 index 000000000000..43273cc93a41 --- /dev/null +++ b/tests/topotests/bgp_bmp/bmp1import/bmp-update-pre-policy-step1.json @@ -0,0 +1,36 @@ +{ + "pre-policy": { + "update": { + "172.31.0.77/32": { + "as_path": "", + "bgp_nexthop": "192.168.1.3", + "bmp_log_type": "update", + "ip_prefix": "172.31.0.77/32", + "ipv6": false, + "origin": "IGP", + "peer_asn": 65501, + "peer_bgp_id": "192.168.1.3", + "peer_distinguisher": "444:1", + "peer_ip": "192.168.1.3", + "peer_type": "route distinguisher instance", + "policy": "pre-policy" + }, + "2001::1125/128": { + "afi": 2, + "as_path": "", + "bmp_log_type": "update", + "ip_prefix": "2001::1125/128", + "ipv6": true, + "nxhp_ip": "192:167::3", + "origin": "IGP", + "peer_asn": 65501, + "peer_bgp_id": "192.168.1.3", + "peer_distinguisher": "555:1", + "peer_ip": "192:167::3", + "peer_type": "route distinguisher instance", + "policy": "pre-policy", + "safi": 1 + } + } + } +} diff --git a/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-loc-rib-step1.json b/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-loc-rib-step1.json new file mode 100644 index 000000000000..fcf518390d71 --- /dev/null +++ b/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-loc-rib-step1.json @@ -0,0 +1,28 @@ +{ + "loc-rib": { + "withdraw": { + "172.31.0.77/32": { + "bmp_log_type": "withdraw", + "ip_prefix": "172.31.0.77/32", + "is_filtered": false, + "peer_asn": 65501, + "peer_bgp_id": "192.168.0.1", + "peer_distinguisher": "444:1", + "peer_type": "loc-rib instance", + "policy": "loc-rib" + }, + "2001::1125/128": { + "afi": 2, + "bmp_log_type": "withdraw", + "ip_prefix": "2001::1125/128", + "is_filtered": false, + "peer_asn": 65501, + "peer_bgp_id": "192.168.0.1", + "peer_distinguisher": "555:1", + "peer_type": "loc-rib instance", + "policy": "loc-rib", + "safi": 1 + } + } + } +} diff --git a/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-loc-rib-step2.json b/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-loc-rib-step2.json new file mode 100644 index 000000000000..1e5040ba60b2 --- /dev/null +++ b/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-loc-rib-step2.json @@ -0,0 +1,34 @@ +{ + "loc-rib": { + "withdraw": { + "172.31.0.15/32": { + "afi": 1, + "bmp_log_type": "withdraw", + "ip_prefix": "172.31.0.15/32", + "is_filtered": false, + "label": 0, + "peer_asn": 65501, + "peer_bgp_id": "192.168.0.1", + "peer_distinguisher": "0:0", + "peer_type": "loc-rib instance", + "policy": "loc-rib", + "rd": "444:2", + "safi": 128 + }, + "2001::1111/128": { + "afi": 2, + "bmp_log_type": "withdraw", + "ip_prefix": "2001::1111/128", + "is_filtered": false, + "label": 0, + "peer_asn": 65501, + "peer_bgp_id": "192.168.0.1", + "peer_distinguisher": "0:0", + "peer_type": "loc-rib instance", + "policy": "loc-rib", + "rd": "555:2", + "safi": 128 + } + } + } +} diff --git a/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-post-policy-step1.json b/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-post-policy-step1.json new file mode 100644 index 000000000000..6626e913618f --- /dev/null +++ b/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-post-policy-step1.json @@ -0,0 +1,30 @@ +{ + "post-policy": { + "withdraw": { + "172.31.0.77/32": { + "bmp_log_type": "withdraw", + "ip_prefix": "172.31.0.77/32", + "ipv6": false, + "peer_asn": 65501, + "peer_bgp_id": "192.168.1.3", + "peer_distinguisher": "444:1", + "peer_ip": "192.168.1.3", + "peer_type": "route distinguisher instance", + "policy": "post-policy" + }, + "2001::1125/128": { + "afi": 2, + "bmp_log_type": "withdraw", + "ip_prefix": "2001::1125/128", + "ipv6": true, + "peer_asn": 65501, + "peer_bgp_id": "192.168.1.3", + "peer_distinguisher": "555:1", + "peer_ip": "192:167::3", + "peer_type": "route distinguisher instance", + "policy": "post-policy", + "safi": 1 + } + } + } +} diff --git a/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-post-policy-step2.json b/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-post-policy-step2.json new file mode 100644 index 000000000000..9eb221d4d0a2 --- /dev/null +++ b/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-post-policy-step2.json @@ -0,0 +1,36 @@ +{ + "post-policy": { + "withdraw": { + "2001::1111/128": { + "afi": 2, + "bmp_log_type": "withdraw", + "ip_prefix": "2001::1111/128", + "ipv6": true, + "label": 0, + "peer_asn": 65502, + "peer_bgp_id": "192.168.0.2", + "peer_distinguisher": "0:0", + "peer_ip": "192:168::2", + "peer_type": "global instance", + "policy": "post-policy", + "rd": "555:2", + "safi": 128 + }, + "172.31.0.15/32": { + "afi": 1, + "bmp_log_type": "withdraw", + "ip_prefix": "172.31.0.15/32", + "ipv6": false, + "label": 0, + "peer_asn": 65502, + "peer_bgp_id": "192.168.0.2", + "peer_distinguisher": "0:0", + "peer_ip": "192.168.0.2", + "peer_type": "global instance", + "policy": "post-policy", + "rd": "444:2", + "safi": 128 + } + } + } +} diff --git a/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-pre-policy-step1.json b/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-pre-policy-step1.json new file mode 100644 index 000000000000..d3fb1b7ba1f2 --- /dev/null +++ b/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-pre-policy-step1.json @@ -0,0 +1,30 @@ +{ + "pre-policy": { + "withdraw": { + "172.31.0.77/32": { + "bmp_log_type": "withdraw", + "ip_prefix": "172.31.0.77/32", + "ipv6": false, + "peer_asn": 65501, + "peer_bgp_id": "192.168.1.3", + "peer_distinguisher": "444:1", + "peer_ip": "192.168.1.3", + "peer_type": "route distinguisher instance", + "policy": "pre-policy" + }, + "2001::1125/128": { + "afi": 2, + "bmp_log_type": "withdraw", + "ip_prefix": "2001::1125/128", + "ipv6": true, + "peer_asn": 65501, + "peer_bgp_id": "192.168.1.3", + "peer_distinguisher": "555:1", + "peer_ip": "192:167::3", + "peer_type": "route distinguisher instance", + "policy": "pre-policy", + "safi": 1 + } + } + } +} diff --git a/tests/topotests/bgp_bmp/r1import/frr.conf b/tests/topotests/bgp_bmp/r1import/frr.conf new file mode 100644 index 000000000000..bec4eb01c78b --- /dev/null +++ b/tests/topotests/bgp_bmp/r1import/frr.conf @@ -0,0 +1,73 @@ +interface r1import-eth0 + ip address 192.0.2.1/24 +! +interface r1import-eth1 + ip address 192.168.0.1/24 + ipv6 address 192:168::1/64 +! +interface r1import-eth2 + ip address 192.168.1.1/24 + ipv6 address 192:167::1/64 +! +router bgp 65501 + bgp router-id 192.168.0.1 + bgp log-neighbor-changes + no bgp ebgp-requires-policy + neighbor 192.168.0.2 remote-as 65502 + neighbor 192:168::2 remote-as 65502 +! + bmp targets bmp1 + bmp connect 192.0.2.10 port 1789 min-retry 100 max-retry 10000 + bmp monitor ipv4 unicast pre-policy + bmp monitor ipv6 unicast pre-policy + bmp monitor ipv4 unicast post-policy + bmp monitor ipv6 unicast post-policy + bmp monitor ipv4 unicast loc-rib + bmp monitor ipv6 unicast loc-rib + bmp import-vrf-view vrf1 + exit +! + address-family ipv4 vpn + neighbor 192.168.0.2 activate + neighbor 192.168.0.2 soft-reconfiguration inbound + exit-address-family + address-family ipv6 vpn + neighbor 192:168::2 activate + neighbor 192:168::2 soft-reconfiguration inbound + exit-address-family + address-family ipv4 unicast + neighbor 192.168.0.2 activate + neighbor 192.168.0.2 soft-reconfiguration inbound + no neighbor 192:168::2 activate + exit-address-family +! + address-family ipv6 unicast + neighbor 192:168::2 activate + neighbor 192:168::2 soft-reconfiguration inbound + exit-address-family +! +router bgp 65501 vrf vrf1 + bgp router-id 192.168.0.1 + bgp log-neighbor-changes + neighbor 192.168.1.3 remote-as 65501 + neighbor 192:167::3 remote-as 65501 + address-family ipv4 unicast + neighbor 192.168.1.3 activate + neighbor 192.168.1.3 soft-reconfiguration inbound + no neighbor 192:167::3 activate + label vpn export 101 + rd vpn export 444:1 + rt vpn both 52:100 + export vpn + import vpn + exit-address-family + address-family ipv6 unicast + neighbor 192:167::3 activate + neighbor 192:167::3 soft-reconfiguration inbound + label vpn export 103 + rd vpn export 555:1 + rt vpn both 54:200 + export vpn + import vpn + exit-address-family +exit diff --git a/tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv4-update-step1.json b/tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv4-update-step1.json new file mode 100644 index 000000000000..c21a586c3b27 --- /dev/null +++ b/tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv4-update-step1.json @@ -0,0 +1,21 @@ +{ + "routes": { + "172.31.0.77/32": [ + { + "bestpath": true, + "pathFrom": "internal", + "path": "", + "origin": "IGP", + "nexthops": [ + { + "ip": "192.168.1.3", + "hostname": "r3", + "afi": "ipv4", + "used": true + } + ] + } + ] + } +} + diff --git a/tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv4-withdraw-step1.json b/tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv4-withdraw-step1.json new file mode 100644 index 000000000000..154bef7995fa --- /dev/null +++ b/tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv4-withdraw-step1.json @@ -0,0 +1,6 @@ +{ + "routes": { + "172.31.0.77/32": null + } +} + diff --git a/tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv6-update-step1.json b/tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv6-update-step1.json new file mode 100644 index 000000000000..14df5ec93126 --- /dev/null +++ b/tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv6-update-step1.json @@ -0,0 +1,27 @@ +{ + "routes": { + "2001::1125/128": [ + { + "bestpath": true, + "pathFrom": "internal", + "path": "", + "origin": "IGP", + "nexthops": [ + { + "ip": "192:167::3", + "hostname": "r3", + "afi": "ipv6", + "scope": "global" + }, + { + "hostname": "r3", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ] + } +} + diff --git a/tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv6-withdraw-step1.json b/tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv6-withdraw-step1.json new file mode 100644 index 000000000000..7c7a95e33ee6 --- /dev/null +++ b/tests/topotests/bgp_bmp/r1import/show-bgp-vrf1-ipv6-withdraw-step1.json @@ -0,0 +1,6 @@ +{ + "routes": { + "2001::1125/128": null + } +} + diff --git a/tests/topotests/bgp_bmp/r3/frr.conf b/tests/topotests/bgp_bmp/r3/frr.conf new file mode 100644 index 000000000000..145e156b11d4 --- /dev/null +++ b/tests/topotests/bgp_bmp/r3/frr.conf @@ -0,0 +1,18 @@ +interface r3-eth0 + ip address 192.168.1.3/24 + ipv6 address 192:167::3/64 +! +router bgp 65501 + bgp router-id 192.168.1.3 + bgp log-neighbor-changes + no bgp network import-check + neighbor 192.168.1.1 remote-as 65501 + neighbor 192:167::1 remote-as 65501 + address-family ipv4 unicast + neighbor 192.168.1.1 activate + no neighbor 192:167::1 activate + exit-address-family + address-family ipv6 unicast + neighbor 192:167::1 activate + exit-address-family +exit diff --git a/tests/topotests/bgp_bmp/test_bgp_bmp_3.py b/tests/topotests/bgp_bmp/test_bgp_bmp_3.py new file mode 100644 index 000000000000..0d2bf181bd96 --- /dev/null +++ b/tests/topotests/bgp_bmp/test_bgp_bmp_3.py @@ -0,0 +1,267 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# Copyright 2024 6WIND S.A. +# + +""" +test_bgp_bmp.py_3: Test BGP BMP functionalities + + +------+ +------+ +------+ + | | | | | | + | BMP1 |------------| R1 |---------------| R2 | + | | | | | | + +------+ +--+---+ +------+ + | + +--+---+ + | | + | R3 | + | | + +------+ + +Setup two routers R1 and R2 with one link configured with IPv4 and +IPv6 addresses. +Configure BGP in R1 and R2 to exchange prefixes from +the latter to the first router. +Setup a link between R1 and the BMP server, activate the BMP feature in R1 +and ensure the monitored BGP sessions logs are well present on the BMP server. +""" + +from functools import partial +import json +import os +import pytest +import sys + +# Save the Current Working Directory to find configuration files. +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join("../")) +sys.path.append(os.path.join("../lib/")) + +# pylint: disable=C0413 +# Import topogen and topotest helpers +from lib import topotest +from lib.bgp import verify_bgp_convergence_from_running_config +from lib.bgp import bgp_configure_prefixes +from .bgpbmp import ( + bmp_check_for_prefixes, + bmp_check_for_peer_message, + bmp_update_seq, + bmp_reset_seq, +) +from lib.topogen import Topogen, TopoRouter, get_topogen +from lib.topolog import logger + +pytestmark = [pytest.mark.bgpd] + +PRE_POLICY = "pre-policy" +POST_POLICY = "post-policy" +LOC_RIB = "loc-rib" + +UPDATE_EXPECTED_JSON = False +DEBUG_PCAP = False + + +def build_topo(tgen): + tgen.add_router("r1import") + tgen.add_router("r2") + tgen.add_router("r3") # CPE behind r1 + + tgen.add_bmp_server("bmp1import", ip="192.0.2.10", defaultRoute="via 192.0.2.1") + + switch = tgen.add_switch("s1") + switch.add_link(tgen.gears["r1import"]) + switch.add_link(tgen.gears["bmp1import"]) + + tgen.add_link(tgen.gears["r1import"], tgen.gears["r2"], "r1import-eth1", "r2-eth0") + tgen.add_link(tgen.gears["r1import"], tgen.gears["r3"], "r1import-eth2", "r3-eth0") + + +def setup_module(mod): + tgen = Topogen(build_topo, mod.__name__) + tgen.start_topology() + + tgen.net["r1import"].cmd( + """ +ip link add vrf1 type vrf table 10 +ip link set vrf1 up +ip link set r1import-eth2 master vrf1 + """ + ) + + bmp_reset_seq() + if DEBUG_PCAP: + tgen.gears["r1import"].run("rm /tmp/bmp.pcap") + tgen.gears["r1import"].run( + "tcpdump -nni r1import-eth0 -s 0 -w /tmp/bmp.pcap &", stdout=None + ) + + for rname, router in tgen.routers().items(): + logger.info("Loading router %s" % rname) + router.load_frr_config( + os.path.join(CWD, "{}/frr.conf".format(rname)), + [(TopoRouter.RD_ZEBRA, None), (TopoRouter.RD_BGP, "-M bmp")], + ) + + tgen.start_router() + + logger.info("starting BMP servers") + for bmp_name, server in tgen.get_bmp_servers().items(): + server.start(log_file=os.path.join(tgen.logdir, bmp_name, "bmp.log")) + + +def teardown_module(_mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_bgp_convergence(): + tgen = get_topogen() + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + result = verify_bgp_convergence_from_running_config(tgen, dut="r1import") + assert result is True, "BGP is not converging" + + +def _test_prefixes(policy, vrf=None, step=0): + """ + Setup the BMP monitor policy, Add and withdraw ipv4/v6 prefixes. + Check if the previous actions are logged in the BMP server with the right + message type and the right policy. + """ + tgen = get_topogen() + + safi = "vpn" if vrf else "unicast" + + prefixes = ["172.31.0.77/32", "2001::1125/128"] + + for type in ("update", "withdraw"): + bmp_update_seq( + tgen.gears["bmp1import"], os.path.join(tgen.logdir, "bmp1import", "bmp.log") + ) + + bgp_configure_prefixes( + tgen.gears["r3"], + 65501, + "unicast", + prefixes, + vrf=None, + update=(type == "update"), + ) + + logger.info(f"checking for prefixes {type}") + + for ipver in [4, 6]: + if UPDATE_EXPECTED_JSON: + continue + ref_file = "{}/r1import/show-bgp-{}-ipv{}-{}-step{}.json".format( + CWD, vrf, ipver, type, step + ) + expected = json.loads(open(ref_file).read()) + + test_func = partial( + topotest.router_json_cmp, + tgen.gears["r1import"], + f"show bgp vrf {vrf} ipv{ipver} json", + expected, + ) + _, res = topotest.run_and_expect(test_func, None, count=30, wait=1) + assertmsg = f"r1: BGP IPv{ipver} convergence failed" + assert res is None, assertmsg + + # check + test_func = partial( + bmp_check_for_prefixes, + prefixes, + type, + policy, + step, + tgen.gears["bmp1import"], + os.path.join(tgen.logdir, "bmp1import"), + tgen.gears["r1import"], + f"{CWD}/bmp1import", + UPDATE_EXPECTED_JSON, + LOC_RIB, + ) + success, res = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert success, "Checking the updated prefixes has failed ! %s" % res + + +def test_bmp_server_logging(): + """ + Assert the logging of the bmp server. + """ + + def check_for_log_file(): + tgen = get_topogen() + output = tgen.gears["bmp1import"].run( + "ls {}".format(os.path.join(tgen.logdir, "bmp1import")) + ) + if "bmp.log" not in output: + return False + return True + + success, _ = topotest.run_and_expect(check_for_log_file, True, count=30, wait=1) + assert success, "The BMP server is not logging" + + +def test_peer_up(): + """ + Checking for BMP peers up messages + """ + + tgen = get_topogen() + peers = ["0.0.0.0", "192.168.1.3", "192:167::3"] + + logger.info("checking for BMP peers up messages") + + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer up", + tgen.gears["bmp1import"], + os.path.join(tgen.logdir, "bmp1import", "bmp.log"), + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert success, "Checking the updated prefixes has been failed !." + + +def test_bmp_bgp_unicast(): + """ + Add/withdraw bgp unicast prefixes and check the bmp logs. + """ + logger.info("*** Unicast prefixes pre-policy logging ***") + _test_prefixes(PRE_POLICY, vrf="vrf1", step=1) + logger.info("*** Unicast prefixes post-policy logging ***") + _test_prefixes(POST_POLICY, vrf="vrf1", step=1) + logger.info("*** Unicast prefixes loc-rib logging ***") + _test_prefixes(LOC_RIB, vrf="vrf1", step=1) + + +def test_peer_down(): + """ + Checking for BMP peers down messages + """ + tgen = get_topogen() + + tgen.gears["r3"].vtysh_cmd("clear bgp *") + + peers = ["192.168.1.3", "192:167::3"] + + logger.info("checking for BMP peers down messages") + + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer down", + tgen.gears["bmp1import"], + os.path.join(tgen.logdir, "bmp1import", "bmp.log"), + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert success, "Checking the updated prefixes has been failed !." + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) From 3e63abfc959dfa9884506123e1f655fcaef27192 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Tue, 17 Dec 2024 18:13:59 +0100 Subject: [PATCH 11/23] bgpd: bmp, fix memory leak in peer messages The following memory leak is observed when running bgp_bmp test. > ==614841==ERROR: LeakSanitizer: detected memory leaks > > Direct leak of 81 byte(s) in 1 object(s) allocated from: > #0 0x7f0e9f2b4887 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145 > #1 0x7f0e9ec771f8 in qmalloc lib/memory.c:101 > #2 0x7f0e9e5a2f89 in bmp_bgp_peer_vrf bgpd/bgp_bmp.c:2211 > #3 0x7f0e9e5a31a8 in bmp_bgp_update_vrf_status bgpd/bgp_bmp.c:2247 > #4 0x7f0e9e5b0325 in bmp_bgp_attribute_updated_instance bgpd/bgp_bmp.c:3476 > #5 0x7f0e9e5b0661 in bmp_bgp_attribute_updated bgpd/bgp_bmp.c:3526 > #6 0x7f0e9e5b08ae in bmp_routerid_update bgpd/bgp_bmp.c:3547 > #7 0x55cdc4bcbd88 in hook_call_bgp_routerid_update bgpd/bgpd.c:89 > #8 0x55cdc4bccf0b in bgp_router_id_set bgpd/bgpd.c:305 > #9 0x55cdc4bcd87d in bgp_router_id_zebra_bump bgpd/bgpd.c:393 > #10 0x55cdc4ba87d5 in bgp_router_id_update bgpd/bgp_zebra.c:99 > #11 0x7f0e9ede3f0b in zclient_read lib/zclient.c:4626 > #12 0x7f0e9ed8074d in event_call lib/event.c:1996 > #13 0x7f0e9ec48933 in frr_run lib/libfrr.c:1232 > #14 0x55cdc48a9a27 in main bgpd/bgp_main.c:555 > #15 0x7f0e9e629d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 > > Direct leak of 81 byte(s) in 1 object(s) allocated from: > #0 0x7f0e9f2b4887 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:145 > #1 0x7f0e9ec771f8 in qmalloc lib/memory.c:101 > #2 0x7f0e9e5a2ed8 in bmp_bgp_peer_vrf bgpd/bgp_bmp.c:2207 > #3 0x7f0e9e5a31a8 in bmp_bgp_update_vrf_status bgpd/bgp_bmp.c:2247 > #4 0x7f0e9e5b0325 in bmp_bgp_attribute_updated_instance bgpd/bgp_bmp.c:3476 > #5 0x7f0e9e5b0661 in bmp_bgp_attribute_updated bgpd/bgp_bmp.c:3526 > #6 0x7f0e9e5b08ae in bmp_routerid_update bgpd/bgp_bmp.c:3547 > #7 0x55cdc4bcbd88 in hook_call_bgp_routerid_update bgpd/bgpd.c:89 > #8 0x55cdc4bccf0b in bgp_router_id_set bgpd/bgpd.c:305 > #9 0x55cdc4bcd87d in bgp_router_id_zebra_bump bgpd/bgpd.c:393 > #10 0x55cdc4ba87d5 in bgp_router_id_update bgpd/bgp_zebra.c:99 > #11 0x7f0e9ede3f0b in zclient_read lib/zclient.c:4626 > #12 0x7f0e9ed8074d in event_call lib/event.c:1996 > #13 0x7f0e9ec48933 in frr_run lib/libfrr.c:1232 > #14 0x55cdc48a9a27 in main bgpd/bgp_main.c:555 > #15 0x7f0e9e629d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 > > Direct leak of 64 byte(s) in 1 object(s) allocated from: > #0 0x7f0e9f2b4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154 > #1 0x7f0e9ec77235 in qcalloc lib/memory.c:106 > #2 0x7f0e9e5a498d in bmp_imported_bgp_get bgpd/bgp_bmp.c:2441 > #3 0x7f0e9e5acbed in bmp_import_vrf_magic bgpd/bgp_bmp.c:2855 > #4 0x7f0e9e5a7f97 in bmp_import_vrf bgpd/bgp_bmp_clippy.c:147 > #5 0x7f0e9ebb1178 in cmd_execute_command_real lib/command.c:1003 > #6 0x7f0e9ebb1505 in cmd_execute_command lib/command.c:1062 > #7 0x7f0e9ebb21d7 in cmd_execute lib/command.c:1228 > #8 0x7f0e9ed90bf0 in vty_command lib/vty.c:626 > #9 0x7f0e9ed95ad5 in vty_execute lib/vty.c:1389 > #10 0x7f0e9ed9c01e in vtysh_read lib/vty.c:2408 > #11 0x7f0e9ed8074d in event_call lib/event.c:1996 > #12 0x7f0e9ec48933 in frr_run lib/libfrr.c:1232 > #13 0x55cdc48a9a27 in main bgpd/bgp_main.c:555 > #14 0x7f0e9e629d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 > > Direct leak of 6 byte(s) in 1 object(s) allocated from: > #0 0x7f0e9f25b9a7 in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:454 > #1 0x7f0e9ec772fa in qstrdup lib/memory.c:118 > #2 0x55cdc4b57d54 in af_rd_vpn_export_magic bgpd/bgp_vty.c:9814 > #3 0x55cdc4b288d7 in af_rd_vpn_export bgpd/bgp_vty_clippy.c:3493 > #4 0x7f0e9ebb1178 in cmd_execute_command_real lib/command.c:1003 > #5 0x7f0e9ebb1505 in cmd_execute_command lib/command.c:1062 > #6 0x7f0e9ebb21d7 in cmd_execute lib/command.c:1228 > #7 0x7f0e9ed90bf0 in vty_command lib/vty.c:626 > #8 0x7f0e9ed95ad5 in vty_execute lib/vty.c:1389 > #9 0x7f0e9ed9c01e in vtysh_read lib/vty.c:2408 > #10 0x7f0e9ed8074d in event_call lib/event.c:1996 > #11 0x7f0e9ec48933 in frr_run lib/libfrr.c:1232 > #12 0x55cdc48a9a27 in main bgpd/bgp_main.c:555 > #13 0x7f0e9e629d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 > > Indirect leak of 5 byte(s) in 1 object(s) allocated from: > #0 0x7f0e9f25b9a7 in __interceptor_strdup ../../../../src/libsanitizer/asan/asan_interceptors.cpp:454 > #1 0x7f0e9ec772fa in qstrdup lib/memory.c:118 > #2 0x7f0e9e5a49ae in bmp_imported_bgp_get bgpd/bgp_bmp.c:2443 > #3 0x7f0e9e5acbed in bmp_import_vrf_magic bgpd/bgp_bmp.c:2855 > #4 0x7f0e9e5a7f97 in bmp_import_vrf bgpd/bgp_bmp_clippy.c:147 > #5 0x7f0e9ebb1178 in cmd_execute_command_real lib/command.c:1003 > #6 0x7f0e9ebb1505 in cmd_execute_command lib/command.c:1062 > #7 0x7f0e9ebb21d7 in cmd_execute lib/command.c:1228 > #8 0x7f0e9ed90bf0 in vty_command lib/vty.c:626 > #9 0x7f0e9ed95ad5 in vty_execute lib/vty.c:1389 > #10 0x7f0e9ed9c01e in vtysh_read lib/vty.c:2408 > #11 0x7f0e9ed8074d in event_call lib/event.c:1996 > #12 0x7f0e9ec48933 in frr_run lib/libfrr.c:1232 > #13 0x55cdc48a9a27 in main bgpd/bgp_main.c:555 > #14 0x7f0e9e629d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 > > SUMMARY: AddressSanitizer: 237 byte(s) leaked in 5 allocation(s). Fix this by freeing the missing memory block that helps building the open message to send to remote bmp collector. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 8c5a72146dc7..a4f99f902099 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -2146,6 +2146,7 @@ bool bmp_bgp_update_vrf_status(struct bmp_bgp *bmpbgp, enum bmp_vrf_state force) if (bbpeer) { XFREE(MTYPE_BMP_OPEN, bbpeer->open_tx); XFREE(MTYPE_BMP_OPEN, bbpeer->open_rx); + XFREE(MTYPE_BMP_OPEN, bbpeer->open_tx); bmp_peerh_del(&bmp_peerh, bbpeer); XFREE(MTYPE_BMP_PEER, bbpeer); } From b68b4aa70e3e434dee4ef31325e7601ef4c02c9f Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 7 Nov 2024 10:30:03 +0100 Subject: [PATCH 12/23] bgpd: modify bmp_bgp_update_vrf_status() API The bgpbmp parameter is not used. Instead the bgp pointer, and the vrf state are used. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 32 +++++++++++++++++--------------- bgpd/bgp_bmp.h | 3 ++- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index a4f99f902099..b1f8c3f10a25 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -658,7 +658,7 @@ static int bmp_send_peerup_vrf(struct bmp *bmp) /* send unconditionally because state may has been set before the * session was up. and in this case the peer up has not been sent. */ - bmp_bgp_update_vrf_status(bmpbgp, vrf_state_unknown); + bmp_bgp_update_vrf_status(&bmpbgp->vrf_state, bmpbgp->bgp, vrf_state_unknown); s = bmp_peerstate(bmpbgp->bgp->peer_self, bmpbgp->vrf_state == vrf_state_down); if (s) { @@ -2115,30 +2115,29 @@ static void bmp_bgp_peer_vrf(struct bmp_bgp_peer *bbpeer, struct bgp *bgp) * * returns true if state has changed */ -bool bmp_bgp_update_vrf_status(struct bmp_bgp *bmpbgp, enum bmp_vrf_state force) +bool bmp_bgp_update_vrf_status(enum bmp_vrf_state *vrf_state, struct bgp *bgp, + enum bmp_vrf_state force) { enum bmp_vrf_state old_state; struct bmp_bgp_peer *bbpeer; struct peer *peer; struct vrf *vrf; - struct bgp *bgp; bool changed; - if (!bmpbgp || !bmpbgp->bgp) + if (!vrf_state || !bgp) return false; - bgp = bmpbgp->bgp; - old_state = bmpbgp->vrf_state; + old_state = *vrf_state; vrf = bgp_vrf_lookup_by_instance_type(bgp); - bmpbgp->vrf_state = force != vrf_state_unknown ? force - : vrf_is_enabled(vrf) ? vrf_state_up - : vrf_state_down; + *vrf_state = force != vrf_state_unknown ? force + : vrf_is_enabled(vrf) ? vrf_state_up + : vrf_state_down; - changed = old_state != bmpbgp->vrf_state; + changed = old_state != *vrf_state; if (changed) { - peer = bmpbgp->bgp->peer_self; - if (bmpbgp->vrf_state == vrf_state_up) { + peer = bgp->peer_self; + if (*vrf_state == vrf_state_up) { bbpeer = bmp_bgp_peer_get(peer); bmp_bgp_peer_vrf(bbpeer, bgp); } else { @@ -3383,7 +3382,7 @@ static int bmp_bgp_attribute_updated(struct bgp *bgp, bool withdraw) if (!bmpbgp) return 0; - bmp_bgp_update_vrf_status(bmpbgp, vrf_state_unknown); + bmp_bgp_update_vrf_status(&bmpbgp->vrf_state, bgp, vrf_state_unknown); if (bmpbgp->vrf_state == vrf_state_down) /* do not send peer events, router id will not be enough to set state to up @@ -3413,7 +3412,10 @@ static int bmp_vrf_state_changed(struct bgp *bgp) { struct bmp_bgp *bmpbgp = bmp_bgp_find(bgp); - if (!bmp_bgp_update_vrf_status(bmpbgp, vrf_state_unknown)) + if (!bmpbgp) + return 0; + + if (!bmp_bgp_update_vrf_status(&bmpbgp->vrf_state, bgp, vrf_state_unknown)) return 1; bmp_send_all_safe(bmpbgp, @@ -3438,7 +3440,7 @@ static int bmp_vrf_itf_state_changed(struct bgp *bgp, struct interface *itf) bmpbgp = bmp_bgp_find(bgp); new_state = if_is_up(itf) ? vrf_state_up : vrf_state_down; - if (bmp_bgp_update_vrf_status(bmpbgp, new_state)) + if (bmpbgp && bmp_bgp_update_vrf_status(&bmpbgp->vrf_state, bgp, new_state)) bmp_send_all(bmpbgp, bmp_peerstate(bgp->peer_self, new_state == vrf_state_down)); return 0; diff --git a/bgpd/bgp_bmp.h b/bgpd/bgp_bmp.h index bde65f6bdd7c..f5d981d632e5 100644 --- a/bgpd/bgp_bmp.h +++ b/bgpd/bgp_bmp.h @@ -300,7 +300,8 @@ struct bmp_bgp { size_t mirror_qsizelimit; }; -extern bool bmp_bgp_update_vrf_status(struct bmp_bgp *bmpbgp, enum bmp_vrf_state force); +extern bool bmp_bgp_update_vrf_status(enum bmp_vrf_state *vrf_state, struct bgp *bgp, + enum bmp_vrf_state force); enum { /* RFC7854 - 10.8 */ From ab31e964e467c9905f9e9475cd36c4c25b633149 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 19 Dec 2024 08:36:12 +0100 Subject: [PATCH 13/23] bgpd, topotests: bmp, add loc-rib peer up event for imported bgp Add the emission of a loc-rib peer up event for an imported bgp instance at import-vrf configuration. Add a test to control in the BMP collector that the peer up message is the one from that BGP instance. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 32 ++++++++++++++++++----- bgpd/bgp_bmp.h | 1 + tests/topotests/bgp_bmp/test_bgp_bmp_3.py | 1 + 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index b1f8c3f10a25..880cb2c8f4fb 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -650,21 +650,40 @@ static int bmp_send_peerup(struct bmp *bmp) return 0; } -static int bmp_send_peerup_vrf(struct bmp *bmp) +static void bmp_send_peerup_vrf_per_instance(struct bmp *bmp, enum bmp_vrf_state *vrf_state, + struct bgp *bgp) { - struct bmp_bgp *bmpbgp = bmp->targets->bmpbgp; struct stream *s; /* send unconditionally because state may has been set before the * session was up. and in this case the peer up has not been sent. */ - bmp_bgp_update_vrf_status(&bmpbgp->vrf_state, bmpbgp->bgp, vrf_state_unknown); + bmp_bgp_update_vrf_status(vrf_state, bgp, vrf_state_unknown); - s = bmp_peerstate(bmpbgp->bgp->peer_self, bmpbgp->vrf_state == vrf_state_down); + s = bmp_peerstate(bgp->peer_self, *vrf_state == vrf_state_down); if (s) { pullwr_write_stream(bmp->pullwr, s); stream_free(s); } +} + +static int bmp_send_peerup_vrf(struct bmp *bmp) +{ + struct bgp *bgp; + struct bmp_imported_bgp *bib; + struct bmp_bgp *bmpbgp = bmp->targets->bmpbgp; + struct bmp_targets *bt; + + bmp_send_peerup_vrf_per_instance(bmp, &bmpbgp->vrf_state, bmpbgp->bgp); + + frr_each (bmp_targets, &bmpbgp->targets, bt) { + frr_each (bmp_imported_bgps, &bt->imported_bgps, bib) { + bgp = bgp_lookup_by_name(bib->name); + if (!bgp) + continue; + bmp_send_peerup_vrf_per_instance(bmp, &bib->vrf_state, bgp); + } + } return 0; } @@ -2341,6 +2360,7 @@ static struct bmp_imported_bgp *bmp_imported_bgp_get(struct bmp_targets *bt, cha bib = XCALLOC(MTYPE_BMP_IMPORTED_BGP, sizeof(*bib)); if (name) bib->name = XSTRDUP(MTYPE_BMP_IMPORTED_BGP, name); + bib->vrf_state = vrf_state_unknown; bib->targets = bt; bmp_imported_bgps_add(&bt->imported_bgps, bib); @@ -2747,13 +2767,13 @@ DEFPY(bmp_import_vrf, bgp = bgp_lookup_by_name(bib->name); if (!bgp) return CMD_SUCCESS; - /* TODO: handle loc-rib peer up changes - * TODO: Start the syncronisation + /* TODO: Start the syncronisation */ frr_each (bmp_session, &bt->sessions, bmp) { if (bmp->state != BMP_PeerUp && bmp->state != BMP_Run) continue; bmp_send_peerup_per_instance(bmp, bgp); + bmp_send_peerup_vrf_per_instance(bmp, &bib->vrf_state, bgp); } return CMD_SUCCESS; } diff --git a/bgpd/bgp_bmp.h b/bgpd/bgp_bmp.h index f5d981d632e5..6eb3eacb562e 100644 --- a/bgpd/bgp_bmp.h +++ b/bgpd/bgp_bmp.h @@ -283,6 +283,7 @@ struct bmp_imported_bgp { struct bmp_imported_bgps_item bib; struct bmp_targets *targets; char *name; + enum bmp_vrf_state vrf_state; }; struct bmp_bgp { diff --git a/tests/topotests/bgp_bmp/test_bgp_bmp_3.py b/tests/topotests/bgp_bmp/test_bgp_bmp_3.py index 0d2bf181bd96..088f4cb4538e 100644 --- a/tests/topotests/bgp_bmp/test_bgp_bmp_3.py +++ b/tests/topotests/bgp_bmp/test_bgp_bmp_3.py @@ -222,6 +222,7 @@ def test_peer_up(): "peer up", tgen.gears["bmp1import"], os.path.join(tgen.logdir, "bmp1import", "bmp.log"), + is_rd_instance=True, ) success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) assert success, "Checking the updated prefixes has been failed !." From 2bd5cd1b81a91dc6f38547f75b74e97f4e9ea5e9 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 19 Dec 2024 08:58:02 +0100 Subject: [PATCH 14/23] bgpd, topotests: bmp imported bgp, add loc-rib peer up support when router-id changes Add the emission of a loc-rib peer up event for an imported bgp instance at route-id reconfiguration. Add a test to control in the BMP collector that the peer up message is the one from that BGP instance. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 54 ++++++++++++++++++----- tests/topotests/bgp_bmp/test_bgp_bmp_3.py | 51 +++++++++++++++++++++ 2 files changed, 95 insertions(+), 10 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 880cb2c8f4fb..313834c023e8 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -3394,24 +3394,58 @@ static int bgp_bmp_early_fini(void) return 0; } +static int bmp_bgp_attribute_updated_instance(struct bmp_targets *bt, enum bmp_vrf_state *vrf_state, + struct bgp *bgp, bool withdraw, struct stream *s) +{ + bmp_bgp_update_vrf_status(vrf_state, bgp, vrf_state_unknown); + if (*vrf_state == vrf_state_down) + /* do not send peer events, router id will not be enough to set state to up + */ + return 0; + + /* vrf_state is up: trigger a peer event + */ + bmp_send_bt(bt, s); + return 1; +} + /* called when the routerid of an instance changes */ static int bmp_bgp_attribute_updated(struct bgp *bgp, bool withdraw) { struct bmp_bgp *bmpbgp = bmp_bgp_find(bgp); + struct bgp *bgp_vrf; + struct bmp_targets *bt; + struct listnode *node; + struct bmp_imported_bgp *bib; + int ret = 0; + struct stream *s = bmp_peerstate(bgp->peer_self, withdraw); - if (!bmpbgp) + if (!s) return 0; - bmp_bgp_update_vrf_status(&bmpbgp->vrf_state, bgp, vrf_state_unknown); - - if (bmpbgp->vrf_state == vrf_state_down) - /* do not send peer events, router id will not be enough to set state to up - */ - return 0; + if (bmpbgp) { + frr_each (bmp_targets, &bmpbgp->targets, bt) { + ret = bmp_bgp_attribute_updated_instance(bt, &bmpbgp->vrf_state, bgp, + withdraw, s); + } + } - /* vrf_state is up: trigger a peer event - */ - bmp_send_all_safe(bmpbgp, bmp_peerstate(bgp->peer_self, withdraw)); + for (ALL_LIST_ELEMENTS_RO(bm->bgp, node, bgp_vrf)) { + if (bgp == bgp_vrf) + continue; + bmpbgp = bmp_bgp_find(bgp_vrf); + if (!bmpbgp) + continue; + frr_each (bmp_targets, &bmpbgp->targets, bt) { + frr_each (bmp_imported_bgps, &bt->imported_bgps, bib) { + if (bgp_lookup_by_name(bib->name) != bgp) + continue; + ret += bmp_bgp_attribute_updated_instance(bt, &bib->vrf_state, bgp, + withdraw, s); + } + } + } + stream_free(s); return 1; } diff --git a/tests/topotests/bgp_bmp/test_bgp_bmp_3.py b/tests/topotests/bgp_bmp/test_bgp_bmp_3.py index 088f4cb4538e..06d9f1f9e612 100644 --- a/tests/topotests/bgp_bmp/test_bgp_bmp_3.py +++ b/tests/topotests/bgp_bmp/test_bgp_bmp_3.py @@ -263,6 +263,57 @@ def test_peer_down(): assert success, "Checking the updated prefixes has been failed !." +def test_bgp_routerid_changed(): + """ + Checking for BGP loc-rib up messages with new router-id + """ + tgen = get_topogen() + + tgen.gears["r1import"].vtysh_cmd( + """ + configure terminal + router bgp 65501 vrf vrf1 + bgp router-id 192.168.1.77 + """ + ) + + peers = ["0.0.0.0"] + + logger.info( + "checking for BMP peer down LOC-RIB message with router-id set to 192.168.0.1." + ) + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer down", + tgen.gears["bmp1import"], + os.path.join(tgen.logdir, "bmp1import", "bmp.log"), + is_rd_instance=True, + peer_bgp_id="192.168.0.1", + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert ( + success + ), "Checking the BMP peer down LOC-RIB message with router-id set to 192.168.0.1 failed !." + + logger.info( + "checking for BMP peer up LOC-RIB message with router-id set to 192.168.1.77." + ) + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer up", + tgen.gears["bmp1import"], + os.path.join(tgen.logdir, "bmp1import", "bmp.log"), + is_rd_instance=True, + peer_bgp_id="192.168.1.77", + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert ( + success + ), "Checking the BMP peer up LOC-RIB message with router-id set to 192.168.1.77 failed !." + + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] sys.exit(pytest.main(args)) From 62891717e3f625a364026bbf6b5f7d6a6df9a468 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 19 Dec 2024 09:19:12 +0100 Subject: [PATCH 15/23] bgpd, topotests: bmp imported bgp, add loc-rib peer up support when vrf flaps Add the emission of a loc-rib peer up event for an imported bgp instance when vrf state changes. Add a test to control in the BMP collector that the peer up message is the one from that BGP instance. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 60 +++++++++++++++++------ tests/topotests/bgp_bmp/test_bgp_bmp_3.py | 39 +++++++++++++++ 2 files changed, 84 insertions(+), 15 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 313834c023e8..c1eaa55c4a05 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -695,6 +695,16 @@ static void bmp_send_bt(struct bmp_targets *bt, struct stream *s) pullwr_write_stream(bmp->pullwr, s); } +static void bmp_send_bt_safe(struct bmp_targets *bt, struct stream *s) +{ + if (!s) + return; + + bmp_send_bt(bt, s); + + stream_free(s); +} + /* send a stream to all bmp sessions configured in a bgp instance */ /* XXX: kludge - filling the pullwr's buffer */ static void bmp_send_all(struct bmp_bgp *bmpbgp, struct stream *s) @@ -3459,22 +3469,45 @@ static int bmp_route_distinguisher_update(struct bgp *bgp, afi_t afi, bool preco return bmp_bgp_attribute_updated(bgp, preconfig); } -/* called when a bgp instance goes up/down, implying that the underlying VRF - * has been created or deleted in zebra - */ -static int bmp_vrf_state_changed(struct bgp *bgp) +static void _bmp_vrf_state_changed_internal(struct bgp *bgp, enum bmp_vrf_state vrf_state) { struct bmp_bgp *bmpbgp = bmp_bgp_find(bgp); + struct bgp *bgp_vrf; + struct bmp_targets *bt; + struct listnode *node; + struct bmp_imported_bgp *bib; - if (!bmpbgp) - return 0; - - if (!bmp_bgp_update_vrf_status(&bmpbgp->vrf_state, bgp, vrf_state_unknown)) - return 1; + if (bmpbgp && bmp_bgp_update_vrf_status(&bmpbgp->vrf_state, bgp, vrf_state)) + bmp_send_all_safe(bmpbgp, bmp_peerstate(bgp->peer_self, + bmpbgp->vrf_state == vrf_state_down)); - bmp_send_all_safe(bmpbgp, - bmp_peerstate(bgp->peer_self, bmpbgp->vrf_state == vrf_state_down)); + for (ALL_LIST_ELEMENTS_RO(bm->bgp, node, bgp_vrf)) { + bmpbgp = bmp_bgp_find(bgp_vrf); + if (!bmpbgp) + continue; + if (bgp_vrf == bgp) + continue; + frr_each (bmp_targets, &bmpbgp->targets, bt) { + frr_each (bmp_imported_bgps, &bt->imported_bgps, bib) { + if (bgp_lookup_by_name(bib->name) != bgp) + continue; + if (bmp_bgp_update_vrf_status(&bib->vrf_state, bgp, vrf_state)) { + bmp_send_bt_safe(bt, bmp_peerstate(bgp->peer_self, + bib->vrf_state == + vrf_state_down)); + break; + } + } + } + } +} +/* called when a bgp instance goes up/down, implying that the underlying VRF + * has been created or deleted in zebra + */ +static int bmp_vrf_state_changed(struct bgp *bgp) +{ + _bmp_vrf_state_changed_internal(bgp, vrf_state_unknown); return 0; } @@ -3483,7 +3516,6 @@ static int bmp_vrf_state_changed(struct bgp *bgp) */ static int bmp_vrf_itf_state_changed(struct bgp *bgp, struct interface *itf) { - struct bmp_bgp *bmpbgp; enum bmp_vrf_state new_state; /* if the update is not about the vrf device double-check @@ -3492,10 +3524,8 @@ static int bmp_vrf_itf_state_changed(struct bgp *bgp, struct interface *itf) if (!itf || !if_is_vrf(itf)) return bmp_vrf_state_changed(bgp); - bmpbgp = bmp_bgp_find(bgp); new_state = if_is_up(itf) ? vrf_state_up : vrf_state_down; - if (bmpbgp && bmp_bgp_update_vrf_status(&bmpbgp->vrf_state, bgp, new_state)) - bmp_send_all(bmpbgp, bmp_peerstate(bgp->peer_self, new_state == vrf_state_down)); + _bmp_vrf_state_changed_internal(bgp, new_state); return 0; } diff --git a/tests/topotests/bgp_bmp/test_bgp_bmp_3.py b/tests/topotests/bgp_bmp/test_bgp_bmp_3.py index 06d9f1f9e612..70037a5257e9 100644 --- a/tests/topotests/bgp_bmp/test_bgp_bmp_3.py +++ b/tests/topotests/bgp_bmp/test_bgp_bmp_3.py @@ -314,6 +314,45 @@ def test_bgp_routerid_changed(): ), "Checking the BMP peer up LOC-RIB message with router-id set to 192.168.1.77 failed !." +def test_bgp_instance_flapping(): + """ + Checking for BGP loc-rib up messages + """ + tgen = get_topogen() + + # create flapping at BMP + # note: only peer up are handled at BMP level today + tgen.net["r1import"].cmd("ip link set dev vrf1 down") + + peers = ["0.0.0.0"] + + logger.info("checking for BMP peer down LOC-RIB message.") + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer down", + tgen.gears["bmp1import"], + os.path.join(tgen.logdir, "bmp1import", "bmp.log"), + is_rd_instance=True, + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert success, "Checking the BMP peer down LOC-RIB message failed !." + + tgen.net["r1import"].cmd("ip link set dev vrf1 up") + + logger.info("checking for BMP peer up LOC-RIB message.") + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer up", + tgen.gears["bmp1import"], + os.path.join(tgen.logdir, "bmp1import", "bmp.log"), + is_rd_instance=True, + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert success, "Checking the BMP peer up LOC-RIB message failed !." + + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] sys.exit(pytest.main(args)) From ca7699f4aea34c12757ae9d469248f577b080dc1 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Tue, 12 Nov 2024 08:46:24 +0100 Subject: [PATCH 16/23] bgpd, topotests: bmp, send peer down when unconfiguring imported vrf When unconfiguring an imported BGP instance, a peer down should be sent to notify BMP collector that the BGP instance is leaving. Add a test that controls the presence of the peer down loc-rib message. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 16 ++++- tests/topotests/bgp_bmp/test_bgp_bmp_3.py | 84 +++++++++++++++++------ 2 files changed, 79 insertions(+), 21 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index c1eaa55c4a05..c8baf161def1 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -705,6 +705,17 @@ static void bmp_send_bt_safe(struct bmp_targets *bt, struct stream *s) stream_free(s); } +static void bmp_send_peerdown_vrf_per_instance(struct bmp_targets *bt, struct bgp *bgp) +{ + struct stream *s; + + s = bmp_peerstate(bgp->peer_self, true); + if (!s) + return; + bmp_send_bt(bt, s); + stream_free(s); +} + /* send a stream to all bmp sessions configured in a bgp instance */ /* XXX: kludge - filling the pullwr's buffer */ static void bmp_send_all(struct bmp_bgp *bmpbgp, struct stream *s) @@ -2765,7 +2776,10 @@ DEFPY(bmp_import_vrf, vty_out(vty, "%% BMP imported BGP instance not found\n"); return CMD_WARNING; } - /* TODO: handle loc-rib peer down change */ + bgp = bgp_lookup_by_name(bib->name); + if (!bgp) + return CMD_WARNING; + bmp_send_peerdown_vrf_per_instance(bt, bgp); bmp_imported_bgp_put(bt, bib); return CMD_SUCCESS; } diff --git a/tests/topotests/bgp_bmp/test_bgp_bmp_3.py b/tests/topotests/bgp_bmp/test_bgp_bmp_3.py index 70037a5257e9..7a68dcde801e 100644 --- a/tests/topotests/bgp_bmp/test_bgp_bmp_3.py +++ b/tests/topotests/bgp_bmp/test_bgp_bmp_3.py @@ -188,31 +188,16 @@ def _test_prefixes(policy, vrf=None, step=0): assert success, "Checking the updated prefixes has failed ! %s" % res -def test_bmp_server_logging(): - """ - Assert the logging of the bmp server. - """ - - def check_for_log_file(): - tgen = get_topogen() - output = tgen.gears["bmp1import"].run( - "ls {}".format(os.path.join(tgen.logdir, "bmp1import")) - ) - if "bmp.log" not in output: - return False - return True - - success, _ = topotest.run_and_expect(check_for_log_file, True, count=30, wait=1) - assert success, "The BMP server is not logging" - - -def test_peer_up(): +def _test_peer_up(check_locrib=True): """ Checking for BMP peers up messages """ tgen = get_topogen() - peers = ["0.0.0.0", "192.168.1.3", "192:167::3"] + if check_locrib: + peers = ["0.0.0.0", "192.168.1.3", "192:167::3"] + else: + peers = ["192.168.1.3", "192:167::3"] logger.info("checking for BMP peers up messages") @@ -228,6 +213,28 @@ def test_peer_up(): assert success, "Checking the updated prefixes has been failed !." +def test_bmp_server_logging(): + """ + Assert the logging of the bmp server. + """ + + def check_for_log_file(): + tgen = get_topogen() + output = tgen.gears["bmp1import"].run( + "ls {}".format(os.path.join(tgen.logdir, "bmp1import")) + ) + if "bmp.log" not in output: + return False + return True + + success, _ = topotest.run_and_expect(check_for_log_file, True, count=30, wait=1) + assert success, "The BMP server is not logging" + + +def test_bmp_peer_up_start(): + _test_peer_up() + + def test_bmp_bgp_unicast(): """ Add/withdraw bgp unicast prefixes and check the bmp logs. @@ -353,6 +360,43 @@ def test_bgp_instance_flapping(): assert success, "Checking the BMP peer up LOC-RIB message failed !." +def test_peer_up_after_flush(): + """ + Checking for BMP peers down messages + """ + _test_peer_up(check_locrib=False) + + +def test_peer_down_locrib(): + """ + Checking for BMP peers down loc-rib messages + """ + tgen = get_topogen() + + tgen.gears["r1import"].vtysh_cmd( + """ + configure terminal + router bgp 65501 + bmp targets bmp1 + no bmp import-vrf-view vrf1 + """ + ) + + peers = ["0.0.0.0"] + + logger.info("checking for BMP peers down messages") + + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer down", + tgen.gears["bmp1import"], + os.path.join(tgen.logdir, "bmp1import", "bmp.log"), + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert success, "Checking the BMP peer down message has failed !." + + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] sys.exit(pytest.main(args)) From 6c7b2abc0122a5d881a5242abb20f86f026b5f92 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 2 Dec 2024 15:23:56 +0100 Subject: [PATCH 17/23] bgpd, topotests: bmp imported bgp, send peer up events when config param changed When a BGP instance is created or becomes valid, and when a parameter is updated (router-id, route distinguisher), the peer up messages other than loc rib peer up messages, are sent. Add a test that controls if peer down and peer up messages are sent accordingly with correct route distinguisher values. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 24 +++++++- tests/topotests/bgp_bmp/test_bgp_bmp_3.py | 75 +++++++++++++++++++++++ 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index c8baf161def1..b04ba9841717 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -3443,6 +3443,7 @@ static int bmp_bgp_attribute_updated(struct bgp *bgp, bool withdraw) struct bmp_imported_bgp *bib; int ret = 0; struct stream *s = bmp_peerstate(bgp->peer_self, withdraw); + struct bmp *bmp; if (!s) return 0; @@ -3451,6 +3452,10 @@ static int bmp_bgp_attribute_updated(struct bgp *bgp, bool withdraw) frr_each (bmp_targets, &bmpbgp->targets, bt) { ret = bmp_bgp_attribute_updated_instance(bt, &bmpbgp->vrf_state, bgp, withdraw, s); + if (withdraw) + continue; + frr_each (bmp_session, &bt->sessions, bmp) + bmp_send_peerup_per_instance(bmp, bgp); } } @@ -3466,6 +3471,10 @@ static int bmp_bgp_attribute_updated(struct bgp *bgp, bool withdraw) continue; ret += bmp_bgp_attribute_updated_instance(bt, &bib->vrf_state, bgp, withdraw, s); + if (withdraw) + continue; + frr_each (bmp_session, &bt->sessions, bmp) + bmp_send_peerup_per_instance(bmp, bgp); } } } @@ -3490,10 +3499,18 @@ static void _bmp_vrf_state_changed_internal(struct bgp *bgp, enum bmp_vrf_state struct bmp_targets *bt; struct listnode *node; struct bmp_imported_bgp *bib; + struct bmp *bmp; - if (bmpbgp && bmp_bgp_update_vrf_status(&bmpbgp->vrf_state, bgp, vrf_state)) + if (bmpbgp && bmp_bgp_update_vrf_status(&bmpbgp->vrf_state, bgp, vrf_state)) { bmp_send_all_safe(bmpbgp, bmp_peerstate(bgp->peer_self, bmpbgp->vrf_state == vrf_state_down)); + if (vrf_state == vrf_state_up && bmpbgp->vrf_state == vrf_state_up) { + frr_each (bmp_targets, &bmpbgp->targets, bt) { + frr_each (bmp_session, &bt->sessions, bmp) + bmp_send_peerup_per_instance(bmp, bgp); + } + } + } for (ALL_LIST_ELEMENTS_RO(bm->bgp, node, bgp_vrf)) { bmpbgp = bmp_bgp_find(bgp_vrf); @@ -3509,6 +3526,11 @@ static void _bmp_vrf_state_changed_internal(struct bgp *bgp, enum bmp_vrf_state bmp_send_bt_safe(bt, bmp_peerstate(bgp->peer_self, bib->vrf_state == vrf_state_down)); + if (vrf_state == vrf_state_up && + bib->vrf_state == vrf_state_up) { + frr_each (bmp_session, &bt->sessions, bmp) + bmp_send_peerup_per_instance(bmp, bgp); + } break; } } diff --git a/tests/topotests/bgp_bmp/test_bgp_bmp_3.py b/tests/topotests/bgp_bmp/test_bgp_bmp_3.py index 7a68dcde801e..a108774821aa 100644 --- a/tests/topotests/bgp_bmp/test_bgp_bmp_3.py +++ b/tests/topotests/bgp_bmp/test_bgp_bmp_3.py @@ -270,6 +270,81 @@ def test_peer_down(): assert success, "Checking the updated prefixes has been failed !." +def test_reconfigure_route_distinguisher_vrf1(): + """ + Checking for BMP peers down messages + """ + tgen = get_topogen() + + bmp_update_seq( + tgen.gears["bmp1import"], os.path.join(tgen.logdir, "bmp1import", "bmp.log") + ) + peers = ["0.0.0.0"] + + tgen.gears["r1import"].vtysh_cmd( + """ + configure terminal + router bgp 65501 vrf vrf1 + address-family ipv4 unicast + rd vpn export 666:22 + exit-address-family + address-family ipv6 unicast + rd vpn export 666:22 + """ + ) + logger.info( + "checking for BMP peer down LOC-RIB message with route-distinguisher set to 444:1" + ) + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer down", + tgen.gears["bmp1import"], + os.path.join(tgen.logdir, "bmp1import", "bmp.log"), + is_rd_instance=True, + peer_distinguisher="444:1", + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert ( + success + ), "Checking the BMP peer down LOC-RIB message with route-distinguisher set to 444:1 failed !." + + logger.info( + "checking for BMP peer up LOC-RIB messages with route-distinguisher set to 666:22" + ) + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer up", + tgen.gears["bmp1import"], + os.path.join(tgen.logdir, "bmp1import", "bmp.log"), + is_rd_instance=True, + peer_distinguisher="666:22", + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert ( + success + ), "Checking the BMP peer up LOC-RIB message with route-distinguisher set to 666:22 failed !." + + logger.info( + "checking for BMP peer up messages with route-distinguisher set to 666:22" + ) + peers = ["192.168.1.3", "192:167::3"] + test_func = partial( + bmp_check_for_peer_message, + peers, + "peer up", + tgen.gears["bmp1import"], + os.path.join(tgen.logdir, "bmp1import", "bmp.log"), + is_rd_instance=True, + peer_distinguisher="666:22", + ) + success, _ = topotest.run_and_expect(test_func, True, count=30, wait=1) + assert ( + success + ), "Checking the BMP peer up messages with route-distinguisher set to 666:22 failed !." + + def test_bgp_routerid_changed(): """ Checking for BGP loc-rib up messages with new router-id From 3e05ba06e662dc12d31ee7042a4157aa3d74928c Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Thu, 19 Dec 2024 14:23:40 +0100 Subject: [PATCH 18/23] bgpd: fix access to invalid memory zone > ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f73891cb146 bp 0x7ffca86584c0 sp 0x7ffca8658490 T0) > ==837617==The signal is caused by a READ memory access. > ==837617==Hint: address points to the zero page. > #0 0x7f73891cb146 in bmp_targets_const_next bgpd/bgp_bmp.c:149 > #1 0x7f73891cb1a5 in bmp_targets_next bgpd/bgp_bmp.c:149 > #2 0x7f73891e875a in _bmp_vrf_state_changed_internal bgpd/bgp_bmp.c:3520 > #3 0x7f73891e8922 in bmp_vrf_itf_state_changed bgpd/bgp_bmp.c:3566 > #4 0x55e511af8d1b in hook_call_bgp_vrf_status_changed bgpd/bgp_zebra.c:64 > #5 0x55e511afa304 in bgp_ifp_up bgpd/bgp_zebra.c:234 > #6 0x7f738981c193 in hook_call_if_up lib/if.c:57 > #7 0x7f738981d09a in if_up_via_zapi lib/if.c:203 > #8 0x7f73899d6f54 in zclient_interface_up lib/zclient.c:2671 > #9 0x7f73899e3e5a in zclient_read lib/zclient.c:4624 > #10 0x7f738998078d in event_call lib/event.c:1996 > #11 0x7f7389848933 in frr_run lib/libfrr.c:1232 > #12 0x55e5117f7ae1 in main bgpd/bgp_main.c:557 > #13 0x7f7389229d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 > #14 0x7f7389229e3f in __libc_start_main_impl ../csu/libc-start.c:392 > #15 0x55e5117f4234 in _start (/usr/lib/frr/bgpd+0x2ec234) Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 1 + 1 file changed, 1 insertion(+) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index b04ba9841717..164f591943a3 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -2081,6 +2081,7 @@ static struct bmp_bgp *bmp_bgp_get(struct bgp *bgp) bmpbgp->bgp = bgp; bmpbgp->vrf_state = vrf_state_unknown; bmpbgp->mirror_qsizelimit = ~0UL; + bmp_targets_init(&bmpbgp->targets); bmp_mirrorq_init(&bmpbgp->mirrorq); bmp_bgph_add(&bmp_bgph, bmpbgp); From d34fe66a5d346d47b4d9bd02d085fb2856ef1d54 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 16 Dec 2024 12:12:53 +0100 Subject: [PATCH 19/23] bgpd: rework bmp end of rib processing Move the end of rib processing code of a given BGP instance in a separate function. This code prepares the next commit, it avoids having the following warning: > WARNING: Too many leading tabs - consider code refactoring Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 164f591943a3..cdaeb2527eec 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -1240,6 +1240,19 @@ static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags, stream_free(msg); } +static void bmp_eor_afi_safi(struct bmp *bmp, afi_t afi, safi_t safi, uint8_t peer_type_flag) +{ + zlog_info("bmp[%s] %s %s table completed (EoR)", bmp->remote, afi2str(afi), safi2str(safi)); + + bmp_eor(bmp, afi, safi, BMP_PEER_FLAG_L, peer_type_flag); + bmp_eor(bmp, afi, safi, 0, peer_type_flag); + bmp_eor(bmp, afi, safi, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE); + + bmp->afistate[afi][safi] = BMP_AFI_LIVE; + bmp->syncafi = AFI_MAX; + bmp->syncsafi = SAFI_MAX; +} + static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) { uint8_t bpi_num_labels, adjin_num_labels; @@ -1340,19 +1353,9 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) return true; } eor: - zlog_info("bmp[%s] %s %s table completed (EoR)", - bmp->remote, afi2str(afi), - safi2str(safi)); - - bmp_eor(bmp, afi, safi, BMP_PEER_FLAG_L, peer_type_flag); - bmp_eor(bmp, afi, safi, 0, peer_type_flag); - bmp_eor(bmp, afi, safi, 0, - BMP_PEER_TYPE_LOC_RIB_INSTANCE); - - bmp->afistate[afi][safi] = BMP_AFI_LIVE; - bmp->syncafi = AFI_MAX; - bmp->syncsafi = SAFI_MAX; - return true; + bmp_eor_afi_safi(bmp, afi, safi, + peer_type_flag); + return true; } bmp->syncpeerid = 0; prefix_copy(&bmp->syncpos, bgp_dest_get_prefix(bn)); From b79574b5c6ad2ff68f863b9baf420c76c2970725 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 11 Nov 2024 22:24:57 +0100 Subject: [PATCH 20/23] bgpd: add sync for monitored afi/safi of imported bgps If an imported BGP is configured after BGP updates have been received, then BMP will not detect those updates in the monitor messages. Syncronisation is also needed for separate instances. For each imported bgp instance, syncronisation is re-done for monitored afi/safis for ALL available instances. - upon configuring an afi/safi (as previously) - when configuring an imported view Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 170 +++++++++++++++++++++++++++++++++++++++---------- bgpd/bgp_bmp.h | 5 +- 2 files changed, 142 insertions(+), 33 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index cdaeb2527eec..c398cf46fd81 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -246,6 +246,7 @@ static struct bmp *bmp_new(struct bmp_targets *bt, int bmp_sock) new->targets = bt; new->socket = bmp_sock; new->syncafi = AFI_MAX; + new->sync_bgp = NULL; FOREACH_AFI_SAFI (afi, safi) { new->afistate[afi][safi] = bt->afimon[afi][safi] @@ -1240,17 +1241,91 @@ static void bmp_monitor(struct bmp *bmp, struct peer *peer, uint8_t flags, stream_free(msg); } +static struct bgp *bmp_get_next_bgp(struct bmp_targets *bt, struct bgp *bgp, afi_t afi, safi_t safi) +{ + struct bmp_imported_bgp *bib; + struct bgp *bgp_inst; + bool get_first = false; + + if (bgp == NULL && bt->bgp_request_sync[afi][safi]) + return bt->bgp; + if (bgp == NULL) + get_first = true; + frr_each (bmp_imported_bgps, &bt->imported_bgps, bib) { + bgp_inst = bgp_lookup_by_name(bib->name); + if (get_first && bgp_inst && bib->bgp_request_sync[afi][safi]) + return bgp_inst; + if (bgp_inst == bgp) + get_first = true; + } + return NULL; +} + +static void bmp_update_syncro(struct bmp *bmp, afi_t afi, safi_t safi, struct bgp *bgp) +{ + struct bmp_imported_bgp *bib; + + if (bmp->syncafi == afi && bmp->syncsafi == safi) { + bmp->syncafi = AFI_MAX; + bmp->syncsafi = SAFI_MAX; + bmp->sync_bgp = NULL; + } + + if (!bmp->targets->afimon[afi][safi]) { + bmp->afistate[afi][safi] = BMP_AFI_INACTIVE; + return; + } + + bmp->afistate[afi][safi] = BMP_AFI_NEEDSYNC; + + if (bgp == NULL || bmp->targets->bgp == bgp) + bmp->targets->bgp_request_sync[afi][safi] = true; + + frr_each (bmp_imported_bgps, &bmp->targets->imported_bgps, bib) { + if (bgp != NULL && bgp_lookup_by_name(bib->name) != bgp) + continue; + bib->bgp_request_sync[afi][safi] = true; + } +} + +static void bmp_update_syncro_set(struct bmp *bmp, afi_t afi, safi_t safi, struct bgp *bgp, + enum bmp_afi_state state) +{ + struct bmp_imported_bgp *bib; + + bmp->afistate[afi][safi] = state; + bmp->syncafi = AFI_MAX; + bmp->syncsafi = SAFI_MAX; + if (bgp == NULL || bmp->targets->bgp == bmp->sync_bgp) + bmp->targets->bgp_request_sync[afi][safi] = false; + + frr_each (bmp_imported_bgps, &bmp->targets->imported_bgps, bib) { + if (bgp == NULL || bgp_lookup_by_name(bib->name) != bmp->sync_bgp) + continue; + bib->bgp_request_sync[afi][safi] = false; + } +} + static void bmp_eor_afi_safi(struct bmp *bmp, afi_t afi, safi_t safi, uint8_t peer_type_flag) { - zlog_info("bmp[%s] %s %s table completed (EoR)", bmp->remote, afi2str(afi), safi2str(safi)); + struct bgp *sync_bgp; + + zlog_info("bmp[%s] %s %s table completed (EoR) (BGP %s)", bmp->remote, afi2str(afi), + safi2str(safi), bmp->sync_bgp->name_pretty); bmp_eor(bmp, afi, safi, BMP_PEER_FLAG_L, peer_type_flag); bmp_eor(bmp, afi, safi, 0, peer_type_flag); bmp_eor(bmp, afi, safi, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE); - bmp->afistate[afi][safi] = BMP_AFI_LIVE; - bmp->syncafi = AFI_MAX; - bmp->syncsafi = SAFI_MAX; + sync_bgp = bmp_get_next_bgp(bmp->targets, bmp->sync_bgp, afi, safi); + if (sync_bgp) { + memset(&bmp->syncpos, 0, sizeof(bmp->syncpos)); + bmp->syncpos.family = afi2family(afi); + bmp->syncrdpos = NULL; + bmp->syncpeerid = 0; + } else + bmp_update_syncro_set(bmp, afi, safi, bmp->sync_bgp, BMP_AFI_LIVE); + bmp->sync_bgp = sync_bgp; } static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) @@ -1273,10 +1348,13 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) memset(&bmp->syncpos, 0, sizeof(bmp->syncpos)); bmp->syncpos.family = afi2family(afi); bmp->syncrdpos = NULL; - zlog_info("bmp[%s] %s %s sending table", - bmp->remote, - afi2str(bmp->syncafi), - safi2str(bmp->syncsafi)); + bmp->sync_bgp = bmp_get_next_bgp(bmp->targets, NULL, afi, safi); + if (bmp->sync_bgp == NULL) + /* all BGP instances already synced*/ + return true; + zlog_info("bmp[%s] %s %s sending table (BGP %s)", bmp->remote, + afi2str(bmp->syncafi), safi2str(bmp->syncsafi), + bmp->sync_bgp->name_pretty); /* break does not work here, 2 loops... */ goto afibreak; } @@ -1290,18 +1368,22 @@ static bool bmp_wrsync(struct bmp *bmp, struct pullwr *pullwr) if (!bmp->targets->afimon[afi][safi]) { /* shouldn't happen */ - bmp->afistate[afi][safi] = BMP_AFI_INACTIVE; - bmp->syncafi = AFI_MAX; - bmp->syncsafi = SAFI_MAX; + bmp_update_syncro_set(bmp, afi, safi, bmp->sync_bgp, BMP_AFI_INACTIVE); + bmp->sync_bgp = NULL; return true; } + if (bmp->sync_bgp == NULL) { + bmp->sync_bgp = bmp_get_next_bgp(bmp->targets, NULL, afi, safi); + if (bmp->sync_bgp == NULL) + return true; + } - struct bgp_table *table = bmp->targets->bgp->rib[afi][safi]; + struct bgp_table *table = bmp->sync_bgp->rib[afi][safi]; struct bgp_dest *bn = NULL; struct bgp_path_info *bpi = NULL, *bpiter; struct bgp_adj_in *adjin = NULL, *adjiter; - peer_type_flag = bmp_get_peer_type_vrf(bmp->targets->bgp->vrf_id); + peer_type_flag = bmp_get_peer_type_vrf(bmp->sync_bgp->vrf_id); if ((afi == AFI_L2VPN && safi == SAFI_EVPN) || (safi == SAFI_MPLS_VPN)) { @@ -1671,11 +1753,16 @@ static bool bmp_wrqueue(struct bmp *bmp, struct pullwr *pullwr) static void bmp_wrfill(struct bmp *bmp, struct pullwr *pullwr) { + afi_t afi; + safi_t safi; + switch(bmp->state) { case BMP_PeerUp: bmp_send_peerup_vrf(bmp); bmp_send_peerup(bmp); bmp->state = BMP_Run; + FOREACH_AFI_SAFI (afi, safi) + bmp_update_syncro(bmp, afi, safi, NULL); break; case BMP_Run: @@ -2234,6 +2321,8 @@ static struct bmp_targets *bmp_targets_find1(struct bgp *bgp, const char *name) static struct bmp_targets *bmp_targets_get(struct bgp *bgp, const char *name) { struct bmp_targets *bt; + afi_t afi; + safi_t safi; bt = bmp_targets_find1(bgp, name); if (bt) @@ -2244,6 +2333,8 @@ static struct bmp_targets *bmp_targets_get(struct bgp *bgp, const char *name) bt->bgp = bgp; bt->bmpbgp = bmp_bgp_get(bgp); bt->stats_send_experimental = true; + FOREACH_AFI_SAFI (afi, safi) + bt->bgp_request_sync[afi][safi] = false; bmp_session_init(&bt->sessions); bmp_qhash_init(&bt->updhash); bmp_qlist_init(&bt->updlist); @@ -2378,6 +2469,8 @@ static void bmp_imported_bgp_put(struct bmp_targets *bt, struct bmp_imported_bgp static struct bmp_imported_bgp *bmp_imported_bgp_get(struct bmp_targets *bt, char *name) { struct bmp_imported_bgp *bib = bmp_imported_bgp_find(bt, name); + afi_t afi; + safi_t safi; if (bib) return bib; @@ -2386,6 +2479,9 @@ static struct bmp_imported_bgp *bmp_imported_bgp_get(struct bmp_targets *bt, cha if (name) bib->name = XSTRDUP(MTYPE_BMP_IMPORTED_BGP, name); bib->vrf_state = vrf_state_unknown; + FOREACH_AFI_SAFI (afi, safi) + bib->bgp_request_sync[afi][safi] = false; + bib->targets = bt; bmp_imported_bgps_add(&bt->imported_bgps, bib); @@ -2764,6 +2860,8 @@ DEFPY(bmp_import_vrf, struct bmp_imported_bgp *bib; struct bgp *bgp; struct bmp *bmp; + afi_t afi; + safi_t safi; if (!bt->bgp) { vty_out(vty, "%% BMP target, BGP instance not found\n"); @@ -2795,13 +2893,14 @@ DEFPY(bmp_import_vrf, bgp = bgp_lookup_by_name(bib->name); if (!bgp) return CMD_SUCCESS; - /* TODO: Start the syncronisation - */ + frr_each (bmp_session, &bt->sessions, bmp) { if (bmp->state != BMP_PeerUp && bmp->state != BMP_Run) continue; bmp_send_peerup_per_instance(bmp, bgp); bmp_send_peerup_vrf_per_instance(bmp, &bib->vrf_state, bgp); + FOREACH_AFI_SAFI (afi, safi) + bmp_update_syncro(bmp, afi, safi, bgp); } return CMD_SUCCESS; } @@ -3008,19 +3107,8 @@ DEFPY(bmp_monitor_cfg, bmp_monitor_cmd, if (prev == bt->afimon[afi][safi]) return CMD_SUCCESS; - frr_each (bmp_session, &bt->sessions, bmp) { - if (bmp->syncafi == afi && bmp->syncsafi == safi) { - bmp->syncafi = AFI_MAX; - bmp->syncsafi = SAFI_MAX; - } - - if (!bt->afimon[afi][safi]) { - bmp->afistate[afi][safi] = BMP_AFI_INACTIVE; - continue; - } - - bmp->afistate[afi][safi] = BMP_AFI_NEEDSYNC; - } + frr_each (bmp_session, &bt->sessions, bmp) + bmp_update_syncro(bmp, afi, safi, NULL); return CMD_SUCCESS; } @@ -3448,6 +3536,8 @@ static int bmp_bgp_attribute_updated(struct bgp *bgp, bool withdraw) int ret = 0; struct stream *s = bmp_peerstate(bgp->peer_self, withdraw); struct bmp *bmp; + afi_t afi; + safi_t safi; if (!s) return 0; @@ -3458,8 +3548,11 @@ static int bmp_bgp_attribute_updated(struct bgp *bgp, bool withdraw) withdraw, s); if (withdraw) continue; - frr_each (bmp_session, &bt->sessions, bmp) + frr_each (bmp_session, &bt->sessions, bmp) { bmp_send_peerup_per_instance(bmp, bgp); + FOREACH_AFI_SAFI (afi, safi) + bmp_update_syncro(bmp, afi, safi, bgp); + } } } @@ -3477,8 +3570,12 @@ static int bmp_bgp_attribute_updated(struct bgp *bgp, bool withdraw) withdraw, s); if (withdraw) continue; - frr_each (bmp_session, &bt->sessions, bmp) + frr_each (bmp_session, &bt->sessions, bmp) { bmp_send_peerup_per_instance(bmp, bgp); + FOREACH_AFI_SAFI (afi, safi) { + bmp_update_syncro(bmp, afi, safi, bgp); + } + } } } } @@ -3504,14 +3601,19 @@ static void _bmp_vrf_state_changed_internal(struct bgp *bgp, enum bmp_vrf_state struct listnode *node; struct bmp_imported_bgp *bib; struct bmp *bmp; + afi_t afi; + safi_t safi; if (bmpbgp && bmp_bgp_update_vrf_status(&bmpbgp->vrf_state, bgp, vrf_state)) { bmp_send_all_safe(bmpbgp, bmp_peerstate(bgp->peer_self, bmpbgp->vrf_state == vrf_state_down)); if (vrf_state == vrf_state_up && bmpbgp->vrf_state == vrf_state_up) { frr_each (bmp_targets, &bmpbgp->targets, bt) { - frr_each (bmp_session, &bt->sessions, bmp) + frr_each (bmp_session, &bt->sessions, bmp) { bmp_send_peerup_per_instance(bmp, bgp); + FOREACH_AFI_SAFI (afi, safi) + bmp_update_syncro(bmp, afi, safi, bgp); + } } } } @@ -3532,8 +3634,12 @@ static void _bmp_vrf_state_changed_internal(struct bgp *bgp, enum bmp_vrf_state vrf_state_down)); if (vrf_state == vrf_state_up && bib->vrf_state == vrf_state_up) { - frr_each (bmp_session, &bt->sessions, bmp) + frr_each (bmp_session, &bt->sessions, bmp) { bmp_send_peerup_per_instance(bmp, bgp); + FOREACH_AFI_SAFI (afi, safi) + bmp_update_syncro(bmp, afi, safi, + bgp); + } } break; } diff --git a/bgpd/bgp_bmp.h b/bgpd/bgp_bmp.h index 6eb3eacb562e..d81b8f9b052e 100644 --- a/bgpd/bgp_bmp.h +++ b/bgpd/bgp_bmp.h @@ -92,7 +92,7 @@ struct bmp_mirrorq { uint8_t data[0]; }; -enum { +enum bmp_afi_state { BMP_AFI_INACTIVE = 0, BMP_AFI_NEEDSYNC, BMP_AFI_SYNC, @@ -148,6 +148,7 @@ struct bmp { uint64_t syncpeerid; afi_t syncafi; safi_t syncsafi; + struct bgp *sync_bgp; }; /* config & state for an active outbound connection. When the connection @@ -209,6 +210,7 @@ struct bmp_targets { struct bmp_bgp *bmpbgp; struct bgp *bgp; + bool bgp_request_sync[AFI_MAX][SAFI_MAX]; char *name; struct bmp_listeners_head listeners; @@ -284,6 +286,7 @@ struct bmp_imported_bgp { struct bmp_targets *targets; char *name; enum bmp_vrf_state vrf_state; + bool bgp_request_sync[AFI_MAX][SAFI_MAX]; }; struct bmp_bgp { From ffdb4cd0f589273e879c6acd71761f79f5638306 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Tue, 3 Dec 2024 09:05:45 +0100 Subject: [PATCH 21/23] bgpd: bmp, handle imported bgp instances in bmp end of rib Modify the bmp_eor() function to send end of rib messages for each peer of the current BGP instance. Signed-off-by: Philippe Guibert --- bgpd/bgp_bmp.c | 17 ++++++++--------- bgpd/bgp_trace.h | 3 ++- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index c398cf46fd81..680eaae47a9a 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -1042,8 +1042,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 bgp *bgp) { struct peer *peer; struct listnode *node; @@ -1051,7 +1051,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(4, frr_bgp, bmp_eor, afi, safi, flags, peer_type_flag); + frrtrace(5, frr_bgp, bmp_eor, afi, safi, flags, peer_type_flag, bgp); s = stream_new(BGP_MAX_PACKET_SIZE); @@ -1079,7 +1079,7 @@ static void bmp_eor(struct bmp *bmp, afi_t afi, safi_t safi, uint8_t flags, bgp_packet_set_size(s); - for (ALL_LIST_ELEMENTS_RO(bmp->targets->bgp->peer, node, peer)) { + for (ALL_LIST_ELEMENTS_RO(bgp->peer, node, peer)) { if (!peer->afc_nego[afi][safi]) continue; @@ -1096,8 +1096,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, bmp->targets->bgp, peer, flags, - peer_type_flag, peer_distinguisher, NULL); + bmp_per_peer_hdr(s2, bgp, peer, flags, peer_type_flag, peer_distinguisher, NULL); stream_putl_at(s2, BMP_LENGTH_POS, stream_get_endp(s) + stream_get_endp(s2)); @@ -1313,9 +1312,9 @@ static void bmp_eor_afi_safi(struct bmp *bmp, afi_t afi, safi_t safi, uint8_t pe zlog_info("bmp[%s] %s %s table completed (EoR) (BGP %s)", bmp->remote, afi2str(afi), safi2str(safi), bmp->sync_bgp->name_pretty); - bmp_eor(bmp, afi, safi, BMP_PEER_FLAG_L, peer_type_flag); - bmp_eor(bmp, afi, safi, 0, peer_type_flag); - bmp_eor(bmp, afi, safi, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE); + bmp_eor(bmp, afi, safi, BMP_PEER_FLAG_L, peer_type_flag, bmp->sync_bgp); + bmp_eor(bmp, afi, safi, 0, peer_type_flag, bmp->sync_bgp); + bmp_eor(bmp, afi, safi, 0, BMP_PEER_TYPE_LOC_RIB_INSTANCE, bmp->sync_bgp); sync_bgp = bmp_get_next_bgp(bmp->targets, bmp->sync_bgp, afi, safi); if (sync_bgp) { diff --git a/bgpd/bgp_trace.h b/bgpd/bgp_trace.h index 43bc7a5a1738..ce869206349f 100644 --- a/bgpd/bgp_trace.h +++ b/bgpd/bgp_trace.h @@ -135,12 +135,13 @@ 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, uint8_t, peer_type_flag), + TP_ARGS(afi_t, afi, safi_t, safi, uint8_t, flags, uint8_t, peer_type_flag, bgp), 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) + ctf_string(bgp, bgp->name_pretty) ) ) From 51cedaa30cd5da7705e421d04b67b9b8ac5f9ead Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Tue, 12 Nov 2024 09:41:46 +0100 Subject: [PATCH 22/23] topotests: bmp, test syncro for pre/post-policy of imported bgps Add a test that controls that the configuration of an imported BGP instance triggers a re-syncronisation. Ensure that changing an attribute like route distinguisher triggers also a re-syncronisation. Signed-off-by: Philippe Guibert --- .../bmp1import/bmp-update-loc-rib-step2.json | 34 +++++++ .../bmp-update-post-policy-step2.json | 36 +++++++ .../bmp-update-pre-policy-step2.json | 36 +++++++ .../bmp-withdraw-post-policy-step2.json | 36 ------- tests/topotests/bgp_bmp/test_bgp_bmp_3.py | 96 ++++++++++++++++++- 5 files changed, 199 insertions(+), 39 deletions(-) create mode 100644 tests/topotests/bgp_bmp/bmp1import/bmp-update-loc-rib-step2.json create mode 100644 tests/topotests/bgp_bmp/bmp1import/bmp-update-post-policy-step2.json create mode 100644 tests/topotests/bgp_bmp/bmp1import/bmp-update-pre-policy-step2.json delete mode 100644 tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-post-policy-step2.json diff --git a/tests/topotests/bgp_bmp/bmp1import/bmp-update-loc-rib-step2.json b/tests/topotests/bgp_bmp/bmp1import/bmp-update-loc-rib-step2.json new file mode 100644 index 000000000000..60066d502cc7 --- /dev/null +++ b/tests/topotests/bgp_bmp/bmp1import/bmp-update-loc-rib-step2.json @@ -0,0 +1,34 @@ +{ + "loc-rib": { + "update": { + "172.31.0.77/32": { + "as_path": "", + "bgp_nexthop": "192.168.1.3", + "bmp_log_type": "update", + "ip_prefix": "172.31.0.77/32", + "is_filtered": false, + "origin": "IGP", + "peer_asn": 65501, + "peer_bgp_id": "192.168.0.1", + "peer_distinguisher": "666:22", + "peer_type": "loc-rib instance", + "policy": "loc-rib" + }, + "2001::1125/128": { + "afi": 2, + "as_path": "", + "bmp_log_type": "update", + "ip_prefix": "2001::1125/128", + "is_filtered": false, + "nxhp_ip": "192:167::3", + "origin": "IGP", + "peer_asn": 65501, + "peer_bgp_id": "192.168.0.1", + "peer_distinguisher": "666:22", + "peer_type": "loc-rib instance", + "policy": "loc-rib", + "safi": 1 + } + } + } +} diff --git a/tests/topotests/bgp_bmp/bmp1import/bmp-update-post-policy-step2.json b/tests/topotests/bgp_bmp/bmp1import/bmp-update-post-policy-step2.json new file mode 100644 index 000000000000..b555c2a371e3 --- /dev/null +++ b/tests/topotests/bgp_bmp/bmp1import/bmp-update-post-policy-step2.json @@ -0,0 +1,36 @@ +{ + "post-policy": { + "update": { + "172.31.0.77/32": { + "as_path": "", + "bgp_nexthop": "192.168.1.3", + "bmp_log_type": "update", + "ip_prefix": "172.31.0.77/32", + "ipv6": false, + "origin": "IGP", + "peer_asn": 65501, + "peer_bgp_id": "192.168.1.3", + "peer_distinguisher": "666:22", + "peer_ip": "192.168.1.3", + "peer_type": "route distinguisher instance", + "policy": "post-policy" + }, + "2001::1125/128": { + "afi": 2, + "as_path": "", + "bmp_log_type": "update", + "ip_prefix": "2001::1125/128", + "ipv6": true, + "nxhp_ip": "192:167::3", + "origin": "IGP", + "peer_asn": 65501, + "peer_bgp_id": "192.168.1.3", + "peer_distinguisher": "666:22", + "peer_ip": "192:167::3", + "peer_type": "route distinguisher instance", + "policy": "post-policy", + "safi": 1 + } + } + } +} diff --git a/tests/topotests/bgp_bmp/bmp1import/bmp-update-pre-policy-step2.json b/tests/topotests/bgp_bmp/bmp1import/bmp-update-pre-policy-step2.json new file mode 100644 index 000000000000..20549926d58b --- /dev/null +++ b/tests/topotests/bgp_bmp/bmp1import/bmp-update-pre-policy-step2.json @@ -0,0 +1,36 @@ +{ + "pre-policy": { + "update": { + "172.31.0.77/32": { + "as_path": "", + "bgp_nexthop": "192.168.1.3", + "bmp_log_type": "update", + "ip_prefix": "172.31.0.77/32", + "ipv6": false, + "origin": "IGP", + "peer_asn": 65501, + "peer_bgp_id": "192.168.1.3", + "peer_distinguisher": "666:22", + "peer_ip": "192.168.1.3", + "peer_type": "route distinguisher instance", + "policy": "pre-policy" + }, + "2001::1125/128": { + "afi": 2, + "as_path": "", + "bmp_log_type": "update", + "ip_prefix": "2001::1125/128", + "ipv6": true, + "nxhp_ip": "192:167::3", + "origin": "IGP", + "peer_asn": 65501, + "peer_bgp_id": "192.168.1.3", + "peer_distinguisher": "666:22", + "peer_ip": "192:167::3", + "peer_type": "route distinguisher instance", + "policy": "pre-policy", + "safi": 1 + } + } + } +} diff --git a/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-post-policy-step2.json b/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-post-policy-step2.json deleted file mode 100644 index 9eb221d4d0a2..000000000000 --- a/tests/topotests/bgp_bmp/bmp1import/bmp-withdraw-post-policy-step2.json +++ /dev/null @@ -1,36 +0,0 @@ -{ - "post-policy": { - "withdraw": { - "2001::1111/128": { - "afi": 2, - "bmp_log_type": "withdraw", - "ip_prefix": "2001::1111/128", - "ipv6": true, - "label": 0, - "peer_asn": 65502, - "peer_bgp_id": "192.168.0.2", - "peer_distinguisher": "0:0", - "peer_ip": "192:168::2", - "peer_type": "global instance", - "policy": "post-policy", - "rd": "555:2", - "safi": 128 - }, - "172.31.0.15/32": { - "afi": 1, - "bmp_log_type": "withdraw", - "ip_prefix": "172.31.0.15/32", - "ipv6": false, - "label": 0, - "peer_asn": 65502, - "peer_bgp_id": "192.168.0.2", - "peer_distinguisher": "0:0", - "peer_ip": "192.168.0.2", - "peer_type": "global instance", - "policy": "post-policy", - "rd": "444:2", - "safi": 128 - } - } - } -} diff --git a/tests/topotests/bgp_bmp/test_bgp_bmp_3.py b/tests/topotests/bgp_bmp/test_bgp_bmp_3.py index a108774821aa..212cf9e69675 100644 --- a/tests/topotests/bgp_bmp/test_bgp_bmp_3.py +++ b/tests/topotests/bgp_bmp/test_bgp_bmp_3.py @@ -124,6 +124,32 @@ def test_bgp_convergence(): assert result is True, "BGP is not converging" +def _test_prefixes_syncro(policy, vrf=None, step=1): + """ + Check that the given policy has syncronised the previously received BGP + updates. + """ + tgen = get_topogen() + + prefixes = ["172.31.0.77/32", "2001::1125/128"] + # check + test_func = partial( + bmp_check_for_prefixes, + prefixes, + "update", + policy, + step, + tgen.gears["bmp1import"], + os.path.join(tgen.logdir, "bmp1import"), + tgen.gears["r1import"], + f"{CWD}/bmp1import", + UPDATE_EXPECTED_JSON, + LOC_RIB, + ) + success, res = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert success, "Checking the updated prefixes has failed ! %s" % res + + def _test_prefixes(policy, vrf=None, step=0): """ Setup the BMP monitor policy, Add and withdraw ipv4/v6 prefixes. @@ -270,6 +296,63 @@ def test_peer_down(): assert success, "Checking the updated prefixes has been failed !." +def test_reconfigure_prefixes(): + """ + Reconfigured BGP networks from R3. Check for BGP VRF update messages + """ + + tgen = get_topogen() + + prefixes = ["172.31.0.77/32", "2001::1125/128"] + bgp_configure_prefixes( + tgen.gears["r3"], + 65501, + "unicast", + prefixes, + vrf=None, + update=True, + ) + + for ipver in [4, 6]: + ref_file = "{}/r1import/show-bgp-{}-ipv{}-{}-step{}.json".format( + CWD, "vrf1", ipver, "update", 1 + ) + expected = json.loads(open(ref_file).read()) + + test_func = partial( + topotest.router_json_cmp, + tgen.gears["r1import"], + f"show bgp vrf vrf1 ipv{ipver} json", + expected, + ) + _, res = topotest.run_and_expect(test_func, None, count=30, wait=1) + assertmsg = f"r1: BGP IPv{ipver} convergence failed" + assert res is None, assertmsg + + +def test_monitor_syncro(): + """ + Checking for BMP peers down messages + """ + tgen = get_topogen() + + tgen.gears["r1import"].vtysh_cmd( + """ + configure terminal + router bgp 65501 + bmp targets bmp1 + bmp import-vrf-view vrf1 + """ + ) + + logger.info("*** Unicast prefixes pre-policy logging ***") + _test_prefixes_syncro(PRE_POLICY, vrf="vrf1") + logger.info("*** Unicast prefixes post-policy logging ***") + _test_prefixes_syncro(POST_POLICY, vrf="vrf1") + logger.info("*** Unicast prefixes loc-rib logging ***") + _test_prefixes_syncro(LOC_RIB, vrf="vrf1") + + def test_reconfigure_route_distinguisher_vrf1(): """ Checking for BMP peers down messages @@ -293,7 +376,7 @@ def test_reconfigure_route_distinguisher_vrf1(): """ ) logger.info( - "checking for BMP peer down LOC-RIB message with route-distinguisher set to 444:1" + "Checking for BMP peer down LOC-RIB message with route-distinguisher set to 444:1" ) test_func = partial( bmp_check_for_peer_message, @@ -310,7 +393,7 @@ def test_reconfigure_route_distinguisher_vrf1(): ), "Checking the BMP peer down LOC-RIB message with route-distinguisher set to 444:1 failed !." logger.info( - "checking for BMP peer up LOC-RIB messages with route-distinguisher set to 666:22" + "Checking for BMP peer up LOC-RIB messages with route-distinguisher set to 666:22" ) test_func = partial( bmp_check_for_peer_message, @@ -327,7 +410,7 @@ def test_reconfigure_route_distinguisher_vrf1(): ), "Checking the BMP peer up LOC-RIB message with route-distinguisher set to 666:22 failed !." logger.info( - "checking for BMP peer up messages with route-distinguisher set to 666:22" + "Checking for BMP peer up messages with route-distinguisher set to 666:22" ) peers = ["192.168.1.3", "192:167::3"] test_func = partial( @@ -344,6 +427,13 @@ def test_reconfigure_route_distinguisher_vrf1(): success ), "Checking the BMP peer up messages with route-distinguisher set to 666:22 failed !." + logger.info("*** Unicast prefixes pre-policy logging ***") + _test_prefixes_syncro(PRE_POLICY, vrf="vrf1", step=2) + logger.info("*** Unicast prefixes post-policy logging ***") + _test_prefixes_syncro(POST_POLICY, vrf="vrf1", step=2) + logger.info("*** Unicast prefixes loc-rib logging ***") + _test_prefixes_syncro(LOC_RIB, vrf="vrf1", step=2) + def test_bgp_routerid_changed(): """ From 9f2932d106c2cb04b5510cc905122516cc397cfd Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Wed, 11 Dec 2024 21:12:07 +0100 Subject: [PATCH 23/23] doc: add bmp import-vrf-view on the bmp user guide Add the bmp import-vrf-view command on the bmp user guide. Signed-off-by: Philippe Guibert --- doc/user/bmp.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/user/bmp.rst b/doc/user/bmp.rst index 14d0849b3410..07c3c1c8bd6b 100644 --- a/doc/user/bmp.rst +++ b/doc/user/bmp.rst @@ -171,3 +171,8 @@ associated with a particular ``bmp targets``: All BGP neighbors are included in Route Mirroring. Options to select a subset of BGP sessions may be added in the future. + +.. clicmd:: bmp import-vrf-view VRF_OR_VIEW_NAME + + Perform Route Mirroring and Route Monitoring from an other BGP + instance.