Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
bgpd: simplify up_generate_addpath_all
To:
tech@openbsd.org
Date:
Wed, 12 Nov 2025 17:11:50 +0100

Download raw body.

Thread
up_generate_addpath_all() should just call up_generate_addpath() for the
case where new and old are NULL.
Doing that removes a lot of unneeded complexity which is very helpful.

-- 
:wq Claudio

Index: rde_update.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
diff -u -p -r1.177 rde_update.c
--- rde_update.c	4 Nov 2025 15:01:09 -0000	1.177
+++ rde_update.c	12 Nov 2025 15:38:19 -0000
@@ -343,21 +343,15 @@ void
 up_generate_addpath_all(struct rde_peer *peer, struct rib_entry *re,
     struct prefix *new, struct prefix *old)
 {
-	struct prefix		*p, *head = NULL;
-	int			all = 0;
+	struct prefix		*p;
 
 	/*
-	 * if old and new are NULL then insert all prefixes from best,
-	 * clearing old routes in the process
+	 * If old and new are NULL then re-insert all prefixes from re,
+	 * use up_generate_addpath() for that.
 	 */
 	if (old == NULL && new == NULL) {
-		/* mark all paths as stale */
-		head = prefix_adjout_first(peer, re->prefix);
-		for (p = head; p != NULL; p = prefix_adjout_next(peer, p))
-			p->flags |= PREFIX_FLAG_STALE;
-
-		new = prefix_best(re);
-		all = 1;
+		up_generate_addpath(peer, re);
+		return;
 	}
 
 	if (new != NULL && !prefix_eligible(new)) {
@@ -366,39 +360,21 @@ up_generate_addpath_all(struct rde_peer 
 	}
 
 	if (old != NULL) {
-		/* withdraw stale paths */
+		/* withdraw old path */
 		p = prefix_adjout_get(peer, old->path_id_tx, old->pt);
 		if (p != NULL)
 			prefix_adjout_withdraw(p);
 	}
 
-	/* add new path (or multiple if all is set) */
-	while (new != NULL) {
-		switch (up_process_prefix(peer, new, (void *)-1)) {
-		case UP_OK:
-		case UP_FILTERED:
-		case UP_EXCLUDED:
-			break;
-		case UP_ERR_LIMIT:
-			/* just give up */
-			return;
-		}
-
-		if (!all)
-			break;
-
-		/* only allow valid prefixes */
-		new = TAILQ_NEXT(new, entry.list.rib);
-		if (new == NULL || !prefix_eligible(new))
-			break;
-	}
-
-	if (all) {
-		/* withdraw stale paths */
-		for (p = head; p != NULL; p = prefix_adjout_next(peer, p)) {
-			if (p->flags & PREFIX_FLAG_STALE)
-				prefix_adjout_withdraw(p);
-		}
+	/* add new path */
+	switch (up_process_prefix(peer, new, (void *)-1)) {
+	case UP_OK:
+	case UP_FILTERED:
+	case UP_EXCLUDED:
+		break;
+	case UP_ERR_LIMIT:
+		/* just give up */
+		return;
 	}
 }