Cache the Adj-RIB-Out for sessions that have not been down for more than

INTERVAL_SESSION_DOWN (3600) seconds.

Rebuilding the Adj-RIB-Out is a lot of work while keeping the RIB in sync
is reasonably trivial. So avoid the work for the case that a session
was just quickly reset. This only works if the same peer settings are
used in the old and new session.

For this introduce a IMSG_SESSION_DELETE that tells the RDE to remove the
peer and split peer_down into a part that takes the session down (and
clears the Adj-RIB-In) and a part the frees the peer (peer_delete).
The SE now sends an IMSG_SESSION_ADD command on first connect and skips
that imsg on later connects unless IMSG_SESSION_DELETE was called before.
During config reload the IMSG_SESSION_ADD calls only need to happen when
the RDE actually has that information.

OK tb@

usr.sbin/bgpd/bgpd.h

@@ -1,4 +1,4 @@
-/*	$OpenBSD: bgpd.h,v 1.504 2024/12/11 09:19:44 claudio Exp $ */
+/*	$OpenBSD: bgpd.h,v 1.505 2024/12/12 20:19:03 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -683,6 +683,7 @@ enum imsg_type {
 	IMSG_SESSION_ADD,
 	IMSG_SESSION_UP,
 	IMSG_SESSION_DOWN,
+	IMSG_SESSION_DELETE,
 	IMSG_SESSION_STALE,
 	IMSG_SESSION_NOGRACE,
 	IMSG_SESSION_FLUSH,

usr.sbin/bgpd/rde.c

@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.c,v 1.645 2024/12/11 09:19:44 claudio Exp $ */
+/*	$OpenBSD: rde.c,v 1.646 2024/12/12 20:19:03 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -433,6 +433,12 @@ rde_dispatch_imsg_session(struct imsgbuf *imsgbuf)
 			}
 			peer_down(peer);
 			break;
+		case IMSG_SESSION_DELETE:
+			/* silently ignore deletes for unknown peers */
+			if ((peer = peer_get(peerid)) == NULL)
+				break;
+			peer_delete(peer);
+			break;
 		case IMSG_SESSION_STALE:
 		case IMSG_SESSION_NOGRACE:
 		case IMSG_SESSION_FLUSH:

usr.sbin/bgpd/rde.h

@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.h,v 1.308 2024/12/11 09:19:44 claudio Exp $ */
+/*	$OpenBSD: rde.h,v 1.309 2024/12/12 20:19:03 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org> and
@@ -289,7 +289,7 @@ struct prefix {
 #define	PREFIX_FLAG_WITHDRAW	0x0001	/* enqueued on withdraw queue */
 #define	PREFIX_FLAG_UPDATE	0x0002	/* enqueued on update queue */
 #define	PREFIX_FLAG_DEAD	0x0004	/* locked but removed */
-#define	PREFIX_FLAG_STALE	0x0008	/* stale entry (graceful reload) */
+#define	PREFIX_FLAG_STALE	0x0008	/* stale entry (for addpath) */
 #define	PREFIX_FLAG_MASK	0x000f	/* mask for the prefix types */
 #define	PREFIX_FLAG_ADJOUT	0x0010	/* prefix is in the adj-out rib */
 #define	PREFIX_FLAG_EOR		0x0020	/* prefix is EoR */
@@ -369,6 +369,7 @@ void		 rde_generate_updates(struct rib_entry *, struct prefix *,
 
 void		 peer_up(struct rde_peer *, struct session_up *);
 void		 peer_down(struct rde_peer *);
+void		 peer_delete(struct rde_peer *);
 void		 peer_flush(struct rde_peer *, uint8_t, time_t);
 void		 peer_stale(struct rde_peer *, uint8_t, int);
 void		 peer_blast(struct rde_peer *, uint8_t);
@@ -603,6 +604,7 @@ void		 prefix_adjout_update(struct prefix *, struct rde_peer *,
 		    struct filterstate *, struct pt_entry *, uint32_t);
 void		 prefix_adjout_withdraw(struct prefix *);
 void		 prefix_adjout_destroy(struct prefix *);
+void		 prefix_adjout_flush_pending(struct rde_peer *);
 int		 prefix_adjout_reaper(struct rde_peer *);
 int		 prefix_dump_new(struct rde_peer *, uint8_t, unsigned int,
 		    void *, void (*)(struct prefix *, void *),

usr.sbin/bgpd/rde_peer.c

@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde_peer.c,v 1.41 2024/12/11 09:19:44 claudio Exp $ */
+/*	$OpenBSD: rde_peer.c,v 1.42 2024/12/12 20:19:03 claudio Exp $ */
 
 /*
  * Copyright (c) 2019 Claudio Jeker <claudio@openbsd.org>
@@ -82,7 +82,7 @@ peer_shutdown(void)
 	struct rde_peer *peer, *np;
 
 	RB_FOREACH_SAFE(peer, peer_tree, &peertable, np)
-		peer_down(peer);
+		peer_delete(peer);
 
 	while (!RB_EMPTY(&zombietable))
 		peer_reaper(NULL);
@@ -241,7 +241,8 @@ peer_generate_update(struct rde_peer *peer, struct rib_entry *re,
 	/* skip ourself */
 	if (peer == peerself)
 		return;
-	if (!peer_is_up(peer))
+	/* skip peers that never had a session open */
+	if (peer->state == PEER_NONE)
 		return;
 	/* skip peers using a different rib */
 	if (peer->loc_rib_id != re->rib_id)
@@ -286,28 +287,6 @@ rde_generate_updates(struct rib_entry *re, struct prefix *newpath,
 /*
  * Various RIB walker callbacks.
  */
-static void
-peer_adjout_clear_upcall(struct prefix *p, void *arg)
-{
-	prefix_adjout_destroy(p);
-}
-
-static void
-peer_adjout_stale_upcall(struct prefix *p, void *arg)
-{
-	if (p->flags & PREFIX_FLAG_DEAD) {
-		return;
-	} else if (p->flags & PREFIX_FLAG_WITHDRAW) {
-		/* no need to keep stale withdraws, they miss all attributes */
-		prefix_adjout_destroy(p);
-		return;
-	} else if (p->flags & PREFIX_FLAG_UPDATE) {
-		RB_REMOVE(prefix_tree, &prefix_peer(p)->updates[p->pt->aid], p);
-		p->flags &= ~PREFIX_FLAG_UPDATE;
-	}
-	p->flags |= PREFIX_FLAG_STALE;
-}
-
 struct peer_flush {
 	struct rde_peer *peer;
 	time_t		 staletime;
@@ -361,6 +340,7 @@ void
 peer_up(struct rde_peer *peer, struct session_up *sup)
 {
 	uint8_t	 i;
+	int force_sync = 1;
 
 	if (peer->state == PEER_ERR) {
 		/*
@@ -369,21 +349,33 @@ peer_up(struct rde_peer *peer, struct session_up *sup)
 		 */
 		rib_dump_terminate(peer);
 		peer_imsg_flush(peer);
-		if (prefix_dump_new(peer, AID_UNSPEC, 0, NULL,
-		    peer_adjout_clear_upcall, NULL, NULL) == -1)
-			fatal("%s: prefix_dump_new", __func__);
 		peer_flush(peer, AID_UNSPEC, 0);
 		peer->stats.prefix_cnt = 0;
-		peer->stats.prefix_out_cnt = 0;
 		peer->state = PEER_DOWN;
 	}
-	peer->remote_bgpid = sup->remote_bgpid;
+
-	peer->short_as = sup->short_as;
+	/*
+	 * Check if no value changed during flap to decide if the RIB
+	 * is in sync. The capa check is maybe too strict but it should
+	 * not matter for normal operation.
+	 */
+	if (memcmp(&peer->remote_addr, &sup->remote_addr,
+	    sizeof(sup->remote_addr)) == 0 &&
+	    memcmp(&peer->local_v4_addr, &sup->local_v4_addr,
+	    sizeof(sup->local_v4_addr)) == 0 &&
+	    memcmp(&peer->local_v6_addr, &sup->local_v6_addr,
+	    sizeof(sup->local_v6_addr)) == 0 &&
+	    memcmp(&peer->capa, &sup->capa, sizeof(sup->capa)) == 0)
+		force_sync = 0;
+
 	peer->remote_addr = sup->remote_addr;
 	peer->local_v4_addr = sup->local_v4_addr;
 	peer->local_v6_addr = sup->local_v6_addr;
+	memcpy(&peer->capa, &sup->capa, sizeof(sup->capa));
+	/* the Adj-RIB-Out does not depend on those */
+	peer->remote_bgpid = sup->remote_bgpid;
 	peer->local_if_scope = sup->if_scope;
-	memcpy(&peer->capa, &sup->capa, sizeof(peer->capa));
+	peer->short_as = sup->short_as;
 
 	/* clear eor markers depending on GR flags */
 	if (peer->capa.grestart.restart) {
@@ -396,9 +388,16 @@ peer_up(struct rde_peer *peer, struct session_up *sup)
 	}
 	peer->state = PEER_UP;
 
-	for (i = AID_MIN; i < AID_MAX; i++) {
+	if (!force_sync) {
-		if (peer->capa.mp[i])
+		for (i = AID_MIN; i < AID_MAX; i++) {
-			peer_dump(peer, i);
+			if (peer->capa.mp[i])
+				peer_blast(peer, i);
+		}
+	} else {
+		for (i = AID_MIN; i < AID_MAX; i++) {
+			if (peer->capa.mp[i])
+				peer_dump(peer, i);
+		}
 	}
 }
 
@@ -416,16 +415,24 @@ peer_down(struct rde_peer *peer)
 	 * and flush all pending imsg from the SE.
 	 */
 	rib_dump_terminate(peer);
+	prefix_adjout_flush_pending(peer);
 	peer_imsg_flush(peer);
 
 	/* flush Adj-RIB-In */
 	peer_flush(peer, AID_UNSPEC, 0);
 	peer->stats.prefix_cnt = 0;
+}
+
+void
+peer_delete(struct rde_peer *peer)
+{
+	if (peer->state != PEER_DOWN)
+		peer_down(peer);
 
 	/* free filters */
 	filterlist_free(peer->out_rules);
-	RB_REMOVE(peer_tree, &peertable, peer);
 
+	RB_REMOVE(peer_tree, &peertable, peer);
 	while (RB_INSERT(peer_tree, &zombietable, peer) != NULL) {
 		log_warnx("zombie peer conflict");
 		peer->conf.id = arc4random();
@@ -481,17 +488,12 @@ peer_stale(struct rde_peer *peer, uint8_t aid, int flushall)
 	 * and flush all pending imsg from the SE.
 	 */
 	rib_dump_terminate(peer);
+	prefix_adjout_flush_pending(peer);
 	peer_imsg_flush(peer);
 
 	if (flushall)
 		peer_flush(peer, aid, 0);
 
-	/* XXX this is not quite correct */
-	/* mark Adj-RIB-Out stale for this peer */
-	if (prefix_dump_new(peer, aid, 0, NULL,
-	    peer_adjout_stale_upcall, NULL, NULL) == -1)
-		fatal("%s: prefix_dump_new", __func__);
-
 	/* make sure new prefixes start on a higher timestamp */
 	while (now >= getmonotime())
 		sleep(1);
@@ -504,10 +506,7 @@ peer_stale(struct rde_peer *peer, uint8_t aid, int flushall)
 static void
 peer_blast_upcall(struct prefix *p, void *ptr)
 {
-	if (p->flags & PREFIX_FLAG_STALE) {
+	if (p->flags & PREFIX_FLAG_DEAD) {
-		/* remove stale entries */
-		prefix_adjout_destroy(p);
-	} else if (p->flags & PREFIX_FLAG_DEAD) {
 		/* ignore dead prefixes, they will go away soon */
 	} else if ((p->flags & PREFIX_FLAG_MASK) == 0) {
 		/* put entries on the update queue if not already on a queue */

usr.sbin/bgpd/rde_rib.c

@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde_rib.c,v 1.265 2024/12/11 09:19:44 claudio Exp $ */
+/*	$OpenBSD: rde_rib.c,v 1.266 2024/12/12 20:19:03 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org>
@@ -1273,7 +1273,6 @@ prefix_adjout_update(struct prefix *p, struct rde_peer *peer,
 			/* nothing changed */
 			p->validation_state = state->vstate;
 			p->lastchange = getmonotime();
-			p->flags &= ~PREFIX_FLAG_STALE;
 			return;
 		}
 
@@ -1344,7 +1343,6 @@ prefix_adjout_withdraw(struct prefix *p)
 	/* already a withdraw, shortcut */
 	if (p->flags & PREFIX_FLAG_WITHDRAW) {
 		p->lastchange = getmonotime();
-		p->flags &= ~PREFIX_FLAG_STALE;
 		return;
 	}
 	/* pending update just got withdrawn */
@@ -1417,6 +1415,24 @@ prefix_adjout_destroy(struct prefix *p)
 	}
 }
 
+void
+prefix_adjout_flush_pending(struct rde_peer *peer)
+{
+	struct prefix *p, *np;
+	uint8_t aid;
+
+	for (aid = AID_MIN; aid < AID_MAX; aid++) {
+		RB_FOREACH_SAFE(p, prefix_tree, &peer->withdraws[aid], np) {
+			prefix_adjout_destroy(p);
+		}
+		RB_FOREACH_SAFE(p, prefix_tree, &peer->updates[aid], np) {
+			p->flags &= ~PREFIX_FLAG_UPDATE;
+			RB_REMOVE(prefix_tree, &peer->updates[aid], p);
+			peer->stats.pending_update--;
+		}
+	}
+}
+
 int
 prefix_adjout_reaper(struct rde_peer *peer)
 {

usr.sbin/bgpd/session.c

@@ -1,4 +1,4 @@
-/*	$OpenBSD: session.c,v 1.502 2024/12/10 14:34:51 claudio Exp $ */
+/*	$OpenBSD: session.c,v 1.503 2024/12/12 20:19:03 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004, 2005 Henning Brauer <henning@openbsd.org>
@@ -270,6 +270,9 @@ session_main(int debug, int verbose)
 					    NULL);
 					timer_remove_all(&p->timers);
 					tcp_md5_del_listener(conf, p);
+					if (imsg_rde(IMSG_SESSION_DELETE,
+					    p->conf.id, NULL, 0) == -1)
+						fatalx("imsg_compose error");
 					msgbuf_free(p->wbuf);
 					RB_REMOVE(peer_head, &conf->peers, p);
 					log_peer_warnx(&p->conf, "removed");
@@ -405,6 +408,12 @@ session_main(int debug, int verbose)
 				case Timer_SessionDown:
 					timer_stop(&p->timers,
 					    Timer_SessionDown);
+
+					if (imsg_rde(IMSG_SESSION_DELETE,
+					    p->conf.id, NULL, 0) == -1)
+						fatalx("imsg_compose error");
+					p->rdesession = 0;
+
 					/* finally delete this cloned peer */
 					if (p->template)
 						p->reconf_action =
@@ -3462,9 +3471,13 @@ session_up(struct peer *p)
 
 	timer_stop(&p->timers, Timer_SessionDown);
 
-	if (imsg_rde(IMSG_SESSION_ADD, p->conf.id,
+	if (!p->rdesession) {
-	    &p->conf, sizeof(p->conf)) == -1)
+		/* inform rde about new peer */
-		fatalx("imsg_compose error");
+		if (imsg_rde(IMSG_SESSION_ADD, p->conf.id,
+		    &p->conf, sizeof(p->conf)) == -1)
+			fatalx("imsg_compose error");
+		p->rdesession = 1;
+	}
 
 	if (p->local.aid == AID_INET) {
 		sup.local_v4_addr = p->local;
@@ -3632,10 +3645,15 @@ merge_peers(struct bgpd_config *c, struct bgpd_config *nc)
 			imsg_compose(ibuf_main, IMSG_PFKEY_RELOAD,
 			    p->conf.id, 0, -1, NULL, 0);
 
-		/* sync the RDE in case we keep the peer */
+		/*
-		if (imsg_rde(IMSG_SESSION_ADD, p->conf.id,
+		 * If the session is established or the SessionDown timer is
-		    &p->conf, sizeof(struct peer_config)) == -1)
+		 * running sync with the RDE
-			fatalx("imsg_compose error");
+		 */
+		if (p->rdesession) {
+			if (imsg_rde(IMSG_SESSION_ADD, p->conf.id,
+			    &p->conf, sizeof(struct peer_config)) == -1)
+				fatalx("imsg_compose error");
+		}
 
 		/* apply the config to all clones of a template */
 		if (p->conf.template) {
@@ -3645,9 +3663,13 @@ merge_peers(struct bgpd_config *c, struct bgpd_config *nc)
 					continue;
 				session_template_clone(xp, NULL, xp->conf.id,
 				    xp->conf.remote_as);
-				if (imsg_rde(IMSG_SESSION_ADD, xp->conf.id,
+
-				    &xp->conf, sizeof(xp->conf)) == -1)
+				if (p->rdesession) {
-					fatalx("imsg_compose error");
+					if (imsg_rde(IMSG_SESSION_ADD,
+					    xp->conf.id, &xp->conf,
+					    sizeof(xp->conf)) == -1)
+						fatalx("imsg_compose error");
+				}
 			}
 		}
 	}

usr.sbin/bgpd/session.h

@@ -1,4 +1,4 @@
-/*	$OpenBSD: session.h,v 1.181 2024/12/10 14:34:51 claudio Exp $ */
+/*	$OpenBSD: session.h,v 1.182 2024/12/12 20:19:03 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -228,6 +228,7 @@ struct peer {
 	uint8_t			 passive;
 	uint8_t			 throttled;
 	uint8_t			 rpending;
+	uint8_t			 rdesession;
 };
 
 extern time_t		 pauseaccept;