Merge pull request #12287 from donaldsharp/bgp_rpki_takes_too_long

Bgp rpki takes too long

bgpd/bgp_mac.c

@@ -242,19 +242,18 @@ static void bgp_mac_rescan_evpn_table(struct bgp *bgp, struct ethaddr *macaddr)
 		if (!peer_established(peer))
 			continue;
 
-		if (CHECK_FLAG(peer->af_flags[afi][safi],
-			       PEER_FLAG_SOFT_RECONFIG)) {
 		if (bgp_debug_update(peer, NULL, NULL, 1))
-				zlog_debug("Processing EVPN MAC interface change on peer %s (inbound, soft-reconfig)",
+			zlog_debug(
-					   peer->host);
+				"Processing EVPN MAC interface change on peer %s %s",
+				peer->host,
+				CHECK_FLAG(peer->af_flags[afi][safi],
+					   PEER_FLAG_SOFT_RECONFIG)
+					? "(inbound, soft-reconfig)"
+					: "");
 
-			bgp_soft_reconfig_in(peer, afi, safi);
+		if (!bgp_soft_reconfig_in(peer, afi, safi)) {
-		} else {
 			struct bgp_table *table = bgp->rib[afi][safi];
 
-			if (bgp_debug_update(peer, NULL, NULL, 1))
-				zlog_debug("Processing EVPN MAC interface change on peer %s",
-					   peer->host);
 			bgp_process_mac_rescan_table(bgp, peer, table, macaddr);
 		}
 	}

bgpd/bgp_route.c

@@ -5342,7 +5342,10 @@ void bgp_soft_reconfig_table_task_cancel(const struct bgp *bgp,
 	}
 }
 
-void bgp_soft_reconfig_in(struct peer *peer, afi_t afi, safi_t safi)
+/*
+ * Returns false if the peer is not configured for soft reconfig in
+ */
+bool bgp_soft_reconfig_in(struct peer *peer, afi_t afi, safi_t safi)
 {
 	struct bgp_dest *dest;
 	struct bgp_table *table;
@@ -5350,14 +5353,14 @@ void bgp_soft_reconfig_in(struct peer *peer, afi_t afi, safi_t safi)
 	struct peer *npeer;
 	struct peer_af *paf;
 
-	if (!peer_established(peer))
+	if (!CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_SOFT_RECONFIG))
-		return;
+		return false;
 
 	if ((safi != SAFI_MPLS_VPN) && (safi != SAFI_ENCAP)
 	    && (safi != SAFI_EVPN)) {
 		table = peer->bgp->rib[afi][safi];
 		if (!table)
-			return;
+			return true;
 
 		table->soft_reconfig_init = true;
 
@@ -5417,6 +5420,8 @@ void bgp_soft_reconfig_in(struct peer *peer, afi_t afi, safi_t safi)
 
 			bgp_soft_reconfig_table(peer, afi, safi, table, &prd);
 		}
+
+	return true;
 }
 
 

bgpd/bgp_route.h

@@ -676,7 +676,12 @@ extern void bgp_default_originate(struct peer *, afi_t, safi_t, int);
 extern void bgp_soft_reconfig_table_task_cancel(const struct bgp *bgp,
 						const struct bgp_table *table,
 						const struct peer *peer);
-extern void bgp_soft_reconfig_in(struct peer *, afi_t, safi_t);
+
+/*
+ * If this peer is configured for soft reconfig in then do the work
+ * and return true.  If it is not return false; and do nothing
+ */
+extern bool bgp_soft_reconfig_in(struct peer *peer, afi_t afi, safi_t safi);
 extern void bgp_clear_route(struct peer *, afi_t, safi_t);
 extern void bgp_clear_route_all(struct peer *);
 extern void bgp_clear_adj_in(struct peer *, afi_t, safi_t);

bgpd/bgp_rpki.c

@@ -62,6 +62,7 @@
 DEFINE_MTYPE_STATIC(BGPD, BGP_RPKI_CACHE, "BGP RPKI Cache server");
 DEFINE_MTYPE_STATIC(BGPD, BGP_RPKI_CACHE_GROUP, "BGP RPKI Cache server group");
 DEFINE_MTYPE_STATIC(BGPD, BGP_RPKI_RTRLIB, "BGP RPKI RTRLib");
+DEFINE_MTYPE_STATIC(BGPD, BGP_RPKI_REVALIDATE, "BGP RPKI Revalidation");
 
 #define POLLING_PERIOD_DEFAULT 3600
 #define EXPIRE_INTERVAL_DEFAULT 7200
@@ -371,10 +372,9 @@ inline bool is_stopping(void)
 	return rtr_is_stopping;
 }
 
-static struct prefix *pfx_record_to_prefix(struct pfx_record *record)
+static void pfx_record_to_prefix(struct pfx_record *record,
+				 struct prefix *prefix)
 {
-	struct prefix *prefix = prefix_new();
-
 	prefix->prefixlen = record->min_len;
 
 	if (record->prefix.ver == LRTR_IPV4) {
@@ -385,15 +385,41 @@ static struct prefix *pfx_record_to_prefix(struct pfx_record *record)
 		ipv6_addr_to_network_byte_order(record->prefix.u.addr6.addr,
 						prefix->u.prefix6.s6_addr32);
 	}
+}
 
-	return prefix;
+struct rpki_revalidate_prefix {
+	struct bgp *bgp;
+	struct prefix prefix;
+	afi_t afi;
+	safi_t safi;
+};
+
+static void rpki_revalidate_prefix(struct thread *thread)
+{
+	struct rpki_revalidate_prefix *rrp = THREAD_ARG(thread);
+	struct bgp_dest *match, *node;
+
+	match = bgp_table_subtree_lookup(rrp->bgp->rib[rrp->afi][rrp->safi],
+					 &rrp->prefix);
+
+	node = match;
+
+	while (node) {
+		if (bgp_dest_has_bgp_path_info_data(node)) {
+			revalidate_bgp_node(node, rrp->afi, rrp->safi);
+		}
+
+		node = bgp_route_next_until(node, match);
+	}
+
+	XFREE(MTYPE_BGP_RPKI_REVALIDATE, rrp);
 }
 
 static void bgpd_sync_callback(struct thread *thread)
 {
 	struct bgp *bgp;
 	struct listnode *node;
-	struct prefix *prefix;
+	struct prefix prefix;
 	struct pfx_record rec;
 
 	thread_add_read(bm->master, bgpd_sync_callback, NULL,
@@ -416,7 +442,7 @@ static void bgpd_sync_callback(struct thread *thread)
 		RPKI_DEBUG("Could not read from rpki_sync_socket_bgpd");
 		return;
 	}
-	prefix = pfx_record_to_prefix(&rec);
+	pfx_record_to_prefix(&rec, &prefix);
 
 	afi_t afi = (rec.prefix.ver == LRTR_IPV4) ? AFI_IP : AFI_IP6;
 
@@ -425,30 +451,20 @@ static void bgpd_sync_callback(struct thread *thread)
 
 		for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++) {
 			struct bgp_table *table = bgp->rib[afi][safi];
+			struct rpki_revalidate_prefix *rrp;
 
 			if (!table)
 				continue;
 
-			struct bgp_dest *match;
+			rrp = XCALLOC(MTYPE_BGP_RPKI_REVALIDATE, sizeof(*rrp));
-			struct bgp_dest *node;
+			rrp->bgp = bgp;
-
+			rrp->prefix = prefix;
-			match = bgp_table_subtree_lookup(table, prefix);
+			rrp->afi = afi;
-			node = match;
+			rrp->safi = safi;
-
+			thread_add_event(bm->master, rpki_revalidate_prefix,
-			while (node) {
+					 rrp, 0, &bgp->t_revalidate[afi][safi]);
-				if (bgp_dest_has_bgp_path_info_data(node)) {
-					revalidate_bgp_node(node, afi, safi);
-				}
-
-				node = bgp_route_next_until(node, match);
-			}
-
-			if (match)
-				bgp_dest_unlock_node(match);
 		}
 	}
-
-	prefix_free(&prefix);
 }
 
 static void revalidate_bgp_node(struct bgp_dest *bgp_dest, afi_t afi,
@@ -473,6 +489,31 @@ static void revalidate_bgp_node(struct bgp_dest *bgp_dest, afi_t afi,
 	}
 }
 
+/*
+ * The act of a soft reconfig in revalidation is really expensive
+ * coupled with the fact that the download of a full rpki state
+ * from a rpki server can be expensive, let's break up the revalidation
+ * to a point in time in the future to allow other bgp events
+ * to take place too.
+ */
+struct rpki_revalidate_peer {
+	afi_t afi;
+	safi_t safi;
+	struct peer *peer;
+};
+
+static void bgp_rpki_revalidate_peer(struct thread *thread)
+{
+	struct rpki_revalidate_peer *rvp = THREAD_ARG(thread);
+
+	/*
+	 * Here's the expensive bit of gnomish deviousness
+	 */
+	bgp_soft_reconfig_in(rvp->peer, rvp->afi, rvp->safi);
+
+	XFREE(MTYPE_BGP_RPKI_REVALIDATE, rvp);
+}
+
 static void revalidate_all_routes(void)
 {
 	struct bgp *bgp;
@@ -483,18 +524,28 @@ static void revalidate_all_routes(void)
 		struct listnode *peer_listnode;
 
 		for (ALL_LIST_ELEMENTS_RO(bgp->peer, peer_listnode, peer)) {
-
+			afi_t afi;
-			for (size_t i = 0; i < 2; i++) {
 			safi_t safi;
-				afi_t afi = (i == 0) ? AFI_IP : AFI_IP6;
 
-				for (safi = SAFI_UNICAST; safi < SAFI_MAX;
+			FOREACH_AFI_SAFI (afi, safi) {
-				     safi++) {
+				struct rpki_revalidate_peer *rvp;
-					if (!peer->bgp->rib[afi][safi])
+
+				if (!bgp->rib[afi][safi])
 					continue;
 
-					bgp_soft_reconfig_in(peer, afi, safi);
+				if (!peer_established(peer))
-				}
+					continue;
+
+				rvp = XCALLOC(MTYPE_BGP_RPKI_REVALIDATE,
+					      sizeof(*rvp));
+				rvp->peer = peer;
+				rvp->afi = afi;
+				rvp->safi = safi;
+
+				thread_add_event(
+					bm->master, bgp_rpki_revalidate_peer,
+					rvp, 0,
+					&peer->t_revalidate_all[afi][safi]);
 			}
 		}
 	}

bgpd/bgpd.c

@@ -1127,6 +1127,8 @@ static void peer_free(struct peer *peer)
 	bgp_timer_set(peer);
 	bgp_reads_off(peer);
 	bgp_writes_off(peer);
+	FOREACH_AFI_SAFI (afi, safi)
+		THREAD_OFF(peer->t_revalidate_all[afi][safi]);
 	assert(!peer->t_write);
 	assert(!peer->t_read);
 	BGP_EVENT_FLUSH(peer);
@@ -2444,6 +2446,8 @@ int peer_delete(struct peer *peer)
 	bgp_keepalives_off(peer);
 	bgp_reads_off(peer);
 	bgp_writes_off(peer);
+	FOREACH_AFI_SAFI (afi, safi)
+		THREAD_OFF(peer->t_revalidate_all[afi][safi]);
 	assert(!CHECK_FLAG(peer->thread_flags, PEER_THREAD_WRITES_ON));
 	assert(!CHECK_FLAG(peer->thread_flags, PEER_THREAD_READS_ON));
 	assert(!CHECK_FLAG(peer->thread_flags, PEER_THREAD_KEEPALIVES_ON));
@@ -3641,6 +3645,9 @@ int bgp_delete(struct bgp *bgp)
 
 	hook_call(bgp_inst_delete, bgp);
 
+	FOREACH_AFI_SAFI (afi, safi)
+		THREAD_OFF(bgp->t_revalidate[afi][safi]);
+
 	THREAD_OFF(bgp->t_condition_check);
 	THREAD_OFF(bgp->t_startup);
 	THREAD_OFF(bgp->t_maxmed_onstartup);
@@ -5517,10 +5524,10 @@ void peer_on_policy_change(struct peer *peer, afi_t afi, safi_t safi,
 		if (!peer_established(peer))
 			return;
 
-		if (CHECK_FLAG(peer->af_flags[afi][safi],
+		if (bgp_soft_reconfig_in(peer, afi, safi))
-			       PEER_FLAG_SOFT_RECONFIG)) {
+			return;
-			bgp_soft_reconfig_in(peer, afi, safi);
+
-		} else if (CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_OLD_RCV) ||
+		if (CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_OLD_RCV) ||
 		    CHECK_FLAG(peer->cap, PEER_CAP_REFRESH_NEW_RCV)) {
 			if (CHECK_FLAG(peer->af_cap[afi][safi],
 				       PEER_CAP_ORF_PREFIX_SM_ADV) &&
@@ -7777,10 +7784,7 @@ int peer_clear_soft(struct peer *peer, afi_t afi, safi_t safi,
 	    || stype == BGP_CLEAR_SOFT_IN_ORF_PREFIX) {
 		/* If neighbor has soft reconfiguration inbound flag.
 		   Use Adj-RIB-In database. */
-		if (CHECK_FLAG(peer->af_flags[afi][safi],
+		if (!bgp_soft_reconfig_in(peer, afi, safi)) {
-			       PEER_FLAG_SOFT_RECONFIG))
-			bgp_soft_reconfig_in(peer, afi, safi);
-		else {
 			/* If neighbor has route refresh capability, send route
 			   refresh
 			   message to the peer. */

bgpd/bgpd.h

@@ -470,6 +470,8 @@ struct bgp {
 	/* BGP update delay on startup */
 	struct thread *t_update_delay;
 	struct thread *t_establish_wait;
+	struct thread *t_revalidate[AFI_MAX][SAFI_MAX];
+
 	uint8_t update_delay_over;
 	uint8_t main_zebra_update_hold;
 	uint8_t main_peers_update_hold;
@@ -1554,6 +1556,7 @@ struct peer {
 	struct thread *t_gr_restart;
 	struct thread *t_gr_stale;
 	struct thread *t_llgr_stale[AFI_MAX][SAFI_MAX];
+	struct thread *t_revalidate_all[AFI_MAX][SAFI_MAX];
 	struct thread *t_generate_updgrp_packets;
 	struct thread *t_process_packet;
 	struct thread *t_process_packet_error;