Merge pull request FRRouting#17733 from pguibert6WIND/bmp_event_changes

BMP handling of BGP configuration changes

bgpd/bgp_bmp.c

@@ -28,6 +28,7 @@
 #include "bgpd/bgp_table.h"
 #include "bgpd/bgpd.h"
 #include "bgpd/bgp_route.h"
+#include "bgpd/bgp_nht.h"
 #include "bgpd/bgp_attr.h"
 #include "bgpd/bgp_dump.h"
 #include "bgpd/bgp_errors.h"
@@ -1708,6 +1709,26 @@ static int bmp_process(struct bgp *bgp, afi_t afi, safi_t safi,
 	return 0;
 }
 
+static int bmp_nht_path_valid(struct bgp *bgp, struct bgp_path_info *path, bool valid)
+{
+	struct bgp_dest *dest = path->net;
+	struct bgp_table *table;
+
+	if (frrtrace_enabled(frr_bgp, bmp_nht_path_valid)) {
+		char pfxprint[PREFIX2STR_BUFFER];
+
+		prefix2str(&dest->rn->p, pfxprint, sizeof(pfxprint));
+		frrtrace(4, frr_bgp, bmp_nht_path_valid, bgp, pfxprint, path, valid);
+	}
+	if (bgp->peer_self == path->peer)
+		/* self declared networks or redistributed networks are not relevant for bmp */
+		return 0;
+
+	table = bgp_dest_table(dest);
+
+	return bmp_process(bgp, table->afi, table->safi, dest, path->peer, !valid);
+}
+
 static void bmp_stat_put_u32(struct stream *s, size_t *cnt, uint16_t type,
 		uint32_t value)
 {
@@ -2013,11 +2034,16 @@ static void bmp_bgp_peer_vrf(struct bmp_bgp_peer *bbpeer, struct bgp *bgp)
 	size_t open_len = stream_get_endp(s);
 
 	bbpeer->open_rx_len = open_len;
+	if (bbpeer->open_rx)
+		XFREE(MTYPE_BMP_OPEN, bbpeer->open_rx);
 	bbpeer->open_rx = XMALLOC(MTYPE_BMP_OPEN, open_len);
 	memcpy(bbpeer->open_rx, s->data, open_len);
 
 	bbpeer->open_tx_len = open_len;
-	bbpeer->open_tx = bbpeer->open_rx;
+	if (bbpeer->open_tx)
+		XFREE(MTYPE_BMP_OPEN, bbpeer->open_tx);
+	bbpeer->open_tx = XMALLOC(MTYPE_BMP_OPEN, open_len);
+	memcpy(bbpeer->open_tx, s->data, open_len);
 
 	stream_free(s);
 }
@@ -2057,6 +2083,7 @@ bool bmp_bgp_update_vrf_status(struct bmp_bgp *bmpbgp, enum bmp_vrf_state force)
 		} else {
 			bbpeer = bmp_bgp_peer_find(peer->qobj_node.nid);
 			if (bbpeer) {
+				XFREE(MTYPE_BMP_OPEN, bbpeer->open_tx);
 				XFREE(MTYPE_BMP_OPEN, bbpeer->open_rx);
 				bmp_peerh_del(&bmp_peerh, bbpeer);
 				XFREE(MTYPE_BMP_PEER, bbpeer);
@@ -3090,11 +3117,14 @@ static int bmp_route_update(struct bgp *bgp, afi_t afi, safi_t safi,
 		return 0;
 	}
 
-	struct bmp_bgp *bmpbgp = bmp_bgp_get(bgp);
+	struct bmp_bgp *bmpbgp = bmp_bgp_find(bgp);
 	struct peer *peer = updated_route->peer;
 	struct bmp_targets *bt;
 	struct bmp *bmp;
 
+	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;
@@ -3149,6 +3179,37 @@ static int bgp_bmp_early_fini(void)
 	return 0;
 }
 
+/* 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);
+
+	if (!bmpbgp)
+		return 0;
+
+	bmp_bgp_update_vrf_status(bmpbgp, 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;
+
+	/* vrf_state is up: trigger a peer event
+	 */
+	bmp_send_all_safe(bmpbgp, bmp_peerstate(bgp->peer_self, withdraw));
+	return 1;
+}
+
+static int bmp_routerid_update(struct bgp *bgp, bool withdraw)
+{
+	return bmp_bgp_attribute_updated(bgp, withdraw);
+}
+
+static int bmp_route_distinguisher_update(struct bgp *bgp, afi_t afi, bool preconfig)
+{
+	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
  */
@@ -3194,13 +3255,16 @@ static int bgp_bmp_module_init(void)
 	hook_register(peer_status_changed, bmp_peer_status_changed);
 	hook_register(peer_backward_transition, bmp_peer_backward);
 	hook_register(bgp_process, bmp_process);
+	hook_register(bgp_nht_path_update, bmp_nht_path_valid);
 	hook_register(bgp_inst_config_write, bmp_config_write);
 	hook_register(bgp_inst_delete, bmp_bgp_del);
 	hook_register(frr_late_init, bgp_bmp_init);
 	hook_register(bgp_route_update, bmp_route_update);
 	hook_register(frr_early_fini, bgp_bmp_early_fini);
 	hook_register(bgp_instance_state, bmp_vrf_state_changed);
 	hook_register(bgp_vrf_status_changed, bmp_vrf_itf_state_changed);
+	hook_register(bgp_routerid_update, bmp_routerid_update);
+	hook_register(bgp_route_distinguisher_update, bmp_route_distinguisher_update);
 	return 0;
 }
 

bgpd/bgp_io.c

@@ -199,7 +199,7 @@ static int read_ibuf_work(struct peer_connection *connection)
 	assert(ringbuf_get(ibw, pkt->data, pktsize) == pktsize);
 	stream_set_endp(pkt, pktsize);
 
-	frrtrace(2, frr_bgp, packet_read, connection->peer, pkt);
+	frrtrace(2, frr_bgp, packet_read, connection, pkt);
 	frr_with_mutex (&connection->io_mtx) {
 		stream_fifo_push(connection->ibuf, pkt);
 	}

bgpd/bgp_nht.c

@@ -41,6 +41,9 @@ static void unregister_zebra_rnh(struct bgp_nexthop_cache *bnc);
 static int make_prefix(int afi, struct bgp_path_info *pi, struct prefix *p);
 static void bgp_nht_ifp_initial(struct event *thread);
 
+DEFINE_HOOK(bgp_nht_path_update, (struct bgp *bgp, struct bgp_path_info *pi, bool valid),
+	    (bgp, pi, valid));
+
 static int bgp_isvalid_nexthop(struct bgp_nexthop_cache *bnc)
 {
 	return (bgp_zebra_num_connects() == 0
@@ -1449,6 +1452,9 @@ void evaluate_paths(struct bgp_nexthop_cache *bnc)
 			}
 		}
 
+		if (path_valid != bnc_is_valid_nexthop)
+			hook_call(bgp_nht_path_update, bgp_path, path, bnc_is_valid_nexthop);
+
 		bgp_process(bgp_path, dest, path, afi, safi);
 	}
 

bgpd/bgp_nht.h

@@ -83,4 +83,9 @@ extern void bgp_nht_ifp_up(struct interface *ifp);
 extern void bgp_nht_ifp_down(struct interface *ifp);
 
 extern void bgp_nht_interface_events(struct peer *peer);
+
+/* called when a path becomes valid or invalid, because of nexthop tracking */
+DECLARE_HOOK(bgp_nht_path_update, (struct bgp *bgp, struct bgp_path_info *pi, bool valid),
+	     (bgp, pi, valid));
+
 #endif /* _BGP_NHT_H */

bgpd/bgp_trace.h

@@ -210,6 +210,24 @@ TRACEPOINT_EVENT(
 
 TRACEPOINT_LOGLEVEL(frr_bgp, bmp_process, TRACE_DEBUG)
 
+/*
+ * BMP is hooked for a nexthop tracking event
+ */
+TRACEPOINT_EVENT(
+	frr_bgp,
+	bmp_nht_path_valid,
+	TP_ARGS(struct bgp *, bgp, char *, pfx, struct bgp_path_info *,
+		path, bool, valid),
+	TP_FIELDS(
+		ctf_string(bgp, bgp->name_pretty)
+		ctf_string(prefix, pfx)
+		ctf_string(path, PEER_HOSTNAME(path->peer))
+		ctf_integer(bool, valid, valid)
+	)
+)
+
+TRACEPOINT_LOGLEVEL(frr_bgp, bmp_nht_path_valid, TRACE_DEBUG)
+
 /*
  * bgp_dest_lock/bgp_dest_unlock
  */

bgpd/bgp_vty.c

@@ -141,6 +141,8 @@ DEFINE_HOOK(bgp_inst_config_write,
 DEFINE_HOOK(bgp_snmp_update_last_changed, (struct bgp *bgp), (bgp));
 DEFINE_HOOK(bgp_snmp_init_stats, (struct bgp *bgp), (bgp));
 DEFINE_HOOK(bgp_snmp_traps_config_write, (struct vty * vty), (vty));
+DEFINE_HOOK(bgp_route_distinguisher_update, (struct bgp *bgp, afi_t afi, bool preconfig),
+	    (bgp, afi, preconfig));
 
 static struct peer_group *listen_range_exists(struct bgp *bgp,
 					      struct prefix *range, int exact);
@@ -9805,6 +9807,14 @@ DEFPY (af_rd_vpn_export,
 	vpn_leak_prechange(BGP_VPN_POLICY_DIR_TOVPN, afi,
 			   bgp_get_default(), bgp);
 
+	if (!bgp->vpn_policy[afi].tovpn_rd_pretty && !rd_str)
+		return CMD_SUCCESS;
+
+	if (yes && bgp->vpn_policy[afi].tovpn_rd_pretty && rd_str &&
+	    strmatch(rd_str, bgp->vpn_policy[afi].tovpn_rd_pretty))
+		return CMD_SUCCESS;
+
+	hook_call(bgp_route_distinguisher_update, bgp, afi, true);
 	if (yes) {
 		if (bgp->vpn_policy[afi].tovpn_rd_pretty)
 			XFREE(MTYPE_BGP_NAME, bgp->vpn_policy[afi].tovpn_rd_pretty);
@@ -9815,9 +9825,11 @@ DEFPY (af_rd_vpn_export,
 			 BGP_VPN_POLICY_TOVPN_RD_SET);
 	} else {
 		XFREE(MTYPE_BGP_NAME, bgp->vpn_policy[afi].tovpn_rd_pretty);
+		bgp->vpn_policy[afi].tovpn_rd_pretty = NULL;
 		UNSET_FLAG(bgp->vpn_policy[afi].flags,
 			   BGP_VPN_POLICY_TOVPN_RD_SET);
 	}
+	hook_call(bgp_route_distinguisher_update, bgp, afi, false);
 
 	/* post-change: re-export vpn routes */
 	vpn_leak_postchange(BGP_VPN_POLICY_DIR_TOVPN, afi,

bgpd/bgpd.c

@@ -86,6 +86,7 @@ DEFINE_QOBJ_TYPE(bgp);
 DEFINE_QOBJ_TYPE(peer);
 DEFINE_HOOK(bgp_inst_delete, (struct bgp *bgp), (bgp));
 DEFINE_HOOK(bgp_instance_state, (struct bgp *bgp), (bgp));
+DEFINE_HOOK(bgp_routerid_update, (struct bgp *bgp, bool withdraw), (bgp, withdraw));
 
 /* BGP process wide configuration.  */
 static struct bgp_master bgp_master;
@@ -301,6 +302,8 @@ static int bgp_router_id_set(struct bgp *bgp, const struct in_addr *id,
 
 	vpn_handle_router_id_update(bgp, true, is_config);
 
+	hook_call(bgp_routerid_update, bgp, true);
+
 	IPV4_ADDR_COPY(&bgp->router_id, id);
 
 	/* Set all peer's local identifier with this value. */
@@ -318,6 +321,7 @@ static int bgp_router_id_set(struct bgp *bgp, const struct in_addr *id,
 
 	vpn_handle_router_id_update(bgp, false, is_config);
 
+	hook_call(bgp_routerid_update, bgp, false);
 	return 0;
 }
 

bgpd/bgpd.h

@@ -907,6 +907,9 @@ DECLARE_HOOK(bgp_config_end, (struct bgp *bgp), (bgp));
 DECLARE_HOOK(bgp_hook_vrf_update, (struct vrf *vrf, bool enabled),
 	     (vrf, enabled));
 DECLARE_HOOK(bgp_instance_state, (struct bgp *bgp), (bgp));
+DECLARE_HOOK(bgp_routerid_update, (struct bgp *bgp, bool withdraw), (bgp, withdraw));
+DECLARE_HOOK(bgp_route_distinguisher_update, (struct bgp *bgp, afi_t afi, bool preconfig),
+	     (bgp, afi, preconfig));
 
 /* Thread callback information */
 struct afi_safi_info {

tests/topotests/bgp_bmp/bgpbmp.py

@@ -187,12 +187,19 @@ def bmp_check_for_prefixes(
 
 
 def bmp_check_for_peer_message(
-    expected_peers, bmp_log_type, bmp_collector, bmp_log_file, is_rd_instance=False
+    expected_peers,
+    bmp_log_type,
+    bmp_collector,
+    bmp_log_file,
+    is_rd_instance=False,
+    peer_bgp_id=None,
+    peer_distinguisher=None,
 ):
     """
     Check for the presence of a peer up message for the peer
     """
     global SEQ
+    last_seq = SEQ
 
     # we care only about the new messages
     messages = [
@@ -208,23 +215,34 @@ def bmp_check_for_peer_message(
     for m in messages:
         if is_rd_instance and m["peer_distinguisher"] == "0:0":
             continue
+        if peer_distinguisher and m["peer_distinguisher"] != peer_distinguisher:
+            continue
+        if peer_bgp_id and m["peer_bgp_id"] != peer_bgp_id:
+            continue
         if (
             "peer_ip" in m.keys()
             and m["peer_ip"] != "0.0.0.0"
             and m["bmp_log_type"] == bmp_log_type
         ):
             if is_rd_instance and m["peer_type"] != "route distinguisher instance":
                 continue
-            peers.append(m["peer_ip"])
+            peers.append((m["peer_ip"], m["seq"]))
         elif m["policy"] == "loc-rib" and m["bmp_log_type"] == bmp_log_type:
-            peers.append("0.0.0.0")
+            peers.append(("0.0.0.0", m["seq"]))
 
     # check for prefixes
     for ep in expected_peers:
-        if ep not in peers:
+        for _ip, _seq in peers:
+            if ep == _ip:
+                msg = "The peer {} is present in the {} log messages."
+                logger.debug(msg.format(ep, bmp_log_type))
+                if _seq > last_seq:
+                    last_seq = _seq
+                break
+        else:
             msg = "The peer {} is not present in the {} log messages."
             logger.debug(msg.format(ep, bmp_log_type))
             return False
 
-    SEQ = messages[-1]["seq"]
+    SEQ = last_seq
     return True