From: claudio <claudio@openbsd.org>
Date: Mon, 21 Mar 2022 13:33:20 +0000 (+0000)
Subject: Adjust how RIB are reloaded when their flags (esp. no evaluate) changes.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=2970de9e9ca1c861104fe3d9b730a62c0ee2cea6;p=openbsd

Adjust how RIB are reloaded when their flags (esp. no evaluate) changes.
First flush all affected Adj-RIB-Out and then in a second step re-evaluate
the RIB itself. The no evaluate case becomes simpler. Fix the way
prefixes are re-evaluated, the list remove needs to be explict and not
part of prefix_evaluate() as in most other cases since this list is not
part of the rib_entry.
OK tb@
---

diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c
index e178fc43668..e220e105624 100644
--- a/usr.sbin/bgpd/rde.c
+++ b/usr.sbin/bgpd/rde.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.c,v 1.541 2022/03/21 10:15:34 claudio Exp $ */
+/*	$OpenBSD: rde.c,v 1.542 2022/03/21 13:33:20 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -3402,7 +3402,7 @@ rde_reload_done(void)
 
 	/* check if filter changed */
 	LIST_FOREACH(peer, &peerlist, peer_l) {
-		if (peer->conf.id == 0)	/* ignore peerself*/
+		if (peer->conf.id == 0)	/* ignore peerself */
 			continue;
 		peer->reconf_out = 0;
 		peer->reconf_rib = 0;
@@ -3469,7 +3469,33 @@ rde_reload_done(void)
 			rib_free(rib);
 			break;
 		case RECONF_RELOAD:
-			rib_update(rib);
+			if (rib_update(rib)) {
+				LIST_FOREACH(peer, &peerlist, peer_l) {
+					/* ignore peerself */
+					if (peer->conf.id == 0)
+						continue;
+					/* skip peers using a different rib */
+					if (peer->loc_rib_id != rib->id)
+						continue;
+					/* peer rib is already being flushed */
+					if (peer->reconf_rib)
+						continue;
+
+					if (prefix_dump_new(peer, AID_UNSPEC,
+					    RDE_RUNNER_ROUNDS, NULL,
+					    rde_up_flush_upcall,
+					    rde_softreconfig_in_done,
+					    NULL) == -1)
+						fatal("%s: prefix_dump_new",
+						    __func__);
+
+					log_peer_info(&peer->conf,
+					    "flushing Adj-RIB-Out");
+					/* account for the running flush */
+					softreconfig++;
+				}
+			}
+
 			rib->state = RECONF_KEEP;
 			/* FALLTHROUGH */
 		case RECONF_KEEP:
@@ -3717,17 +3743,14 @@ rde_softreconfig_sync_reeval(struct rib_entry *re, void *arg)
 	if (rib->flags & F_RIB_NOEVALUATE) {
 		/*
 		 * evaluation process is turned off
-		 * so remove all prefixes from adj-rib-out
-		 * also unlink nexthop if it was linked
+		 * all dependent adj-rib-out were already flushed
+		 * unlink nexthop if it was linked
 		 */
 		LIST_FOREACH(p, &re->prefix_h, entry.list.rib) {
 			if (p->flags & PREFIX_NEXTHOP_LINKED)
 				nexthop_unlink(p);
 		}
-		if (re->active) {
-			rde_generate_updates(rib, NULL, re->active, 0);
-			re->active = NULL;
-		}
+		re->active = NULL;
 		return;
 	}
 
@@ -3736,11 +3759,18 @@ rde_softreconfig_sync_reeval(struct rib_entry *re, void *arg)
 	prefixes = re->prefix_h;
 	LIST_INIT(&re->prefix_h);
 
+	/*
+	 * TODO: this code works but is not optimal. prefix_evaluate()
+	 * does a lot of extra work in the worst case. Would be better
+	 * to resort the list once and then call rde_generate_updates()
+	 * and rde_send_kroute() once.
+	 */
 	LIST_FOREACH_SAFE(p, &prefixes, entry.list.rib, next) {
 		/* need to re-link the nexthop if not already linked */
+		LIST_REMOVE(p, entry.list.rib);
 		if ((p->flags & PREFIX_NEXTHOP_LINKED) == 0)
 			nexthop_link(p);
-		prefix_evaluate(re, p, p);
+		prefix_evaluate(re, p, NULL);
 	}
 }
 
diff --git a/usr.sbin/bgpd/rde.h b/usr.sbin/bgpd/rde.h
index 05569cac8e2..eebe96aeb32 100644
--- a/usr.sbin/bgpd/rde.h
+++ b/usr.sbin/bgpd/rde.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde.h,v 1.248 2022/03/15 16:50:29 claudio Exp $ */
+/*	$OpenBSD: rde.h,v 1.249 2022/03/21 13:33:20 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org> and
@@ -548,7 +548,7 @@ pt_unref(struct pt_entry *pt)
 extern uint16_t	rib_size;
 
 struct rib	*rib_new(char *, u_int, uint16_t);
-void		 rib_update(struct rib *);
+int		 rib_update(struct rib *);
 struct rib	*rib_byid(uint16_t);
 uint16_t	 rib_find(char *);
 void		 rib_free(struct rib *);
diff --git a/usr.sbin/bgpd/rde_rib.c b/usr.sbin/bgpd/rde_rib.c
index d8f61ade728..0be448ce6f5 100644
--- a/usr.sbin/bgpd/rde_rib.c
+++ b/usr.sbin/bgpd/rde_rib.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rde_rib.c,v 1.234 2022/03/15 16:50:29 claudio Exp $ */
+/*	$OpenBSD: rde_rib.c,v 1.235 2022/03/21 13:33:20 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Claudio Jeker <claudio@openbsd.org>
@@ -179,7 +179,7 @@ rib_new(char *name, u_int rtableid, uint16_t flags)
  * or RECONF_REINIT (rerun the route decision process for every element)
  * depending on the new flags.
  */
-void
+int
 rib_update(struct rib *rib)
 {
 	/* flush fib first if there was one */
@@ -198,6 +198,8 @@ rib_update(struct rib *rib)
 	if (rib->fibstate != RECONF_REINIT &&
 	    (rib->flags & (F_RIB_NOFIB | F_RIB_NOEVALUATE)) == 0)
 		rib->fibstate = RECONF_RELOAD;
+
+	return (rib->fibstate == RECONF_REINIT);
 }
 
 struct rib *