I optimized "announce add-path send all" to not always re-evaluate all
possible prefixes. While doing that I introduced a small bug. The problem
is that the new prefix passed to up_generate_addpath_all() could be not
eligible. This causes up_process_prefix() -> up_test_update() to error
out.
The below fix should solve this twice. The rde_update.c diff ensures that
up_generate_addpath_all() checks the new path to be eligible and is the
minimal fix. The rde_decide.c diff fixes the problem at the root.
I came to the conclusion that I want both.
Btw. the other up_generate_* functions don't have this issue since they
use prefix_best() to get new and that already calls prefix_eligible().
--
:wq Claudio
Index: rde_decide.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_decide.c,v
retrieving revision 1.101
diff -u -p -r1.101 rde_decide.c
--- rde_decide.c 13 Mar 2023 16:52:42 -0000 1.101
+++ rde_decide.c 12 Oct 2023 12:56:59 -0000
@@ -568,9 +568,12 @@ prefix_evaluate(struct rib_entry *re, st
* to be passed on (not only a change of the best prefix).
* rde_generate_updates() will then take care of distribution.
*/
- if (rde_evaluate_all())
- if ((new != NULL && prefix_eligible(new)) || old != NULL)
+ if (rde_evaluate_all()) {
+ if (new != NULL && !prefix_eligible(new))
+ new = NULL;
+ if (new != NULL || old != NULL)
rde_generate_updates(re, new, old, EVAL_ALL);
+ }
}
void
@@ -578,7 +581,7 @@ prefix_evaluate_nexthop(struct prefix *p
enum nexthop_state oldstate)
{
struct rib_entry *re = prefix_re(p);
- struct prefix *newbest, *oldbest;
+ struct prefix *newbest, *oldbest, *new, *old;
struct rib *rib;
/* Skip non local-RIBs or RIBs that are flagged as noeval. */
@@ -608,6 +611,7 @@ prefix_evaluate_nexthop(struct prefix *p
* Re-evaluate the prefix by removing the prefix then updating the
* nexthop state and reinserting the prefix again.
*/
+ old = p;
oldbest = prefix_best(re);
prefix_remove(p, re);
@@ -618,6 +622,9 @@ prefix_evaluate_nexthop(struct prefix *p
prefix_insert(p, NULL, re);
newbest = prefix_best(re);
+ new = p;
+ if (!prefix_eligible(new))
+ new = NULL;
/*
* If the active prefix changed or the active prefix was removed
@@ -631,7 +638,7 @@ prefix_evaluate_nexthop(struct prefix *p
*/
if ((rib->flags & F_RIB_NOFIB) == 0)
rde_send_kroute(rib, newbest, oldbest);
- rde_generate_updates(re, p, p, EVAL_DEFAULT);
+ rde_generate_updates(re, new, old, EVAL_DEFAULT);
return;
}
@@ -641,5 +648,5 @@ prefix_evaluate_nexthop(struct prefix *p
* rde_generate_updates() will then take care of distribution.
*/
if (rde_evaluate_all())
- rde_generate_updates(re, p, p, EVAL_ALL);
+ rde_generate_updates(re, new, old, EVAL_ALL);
}
Index: rde_update.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_update.c,v
retrieving revision 1.163
diff -u -p -r1.163 rde_update.c
--- rde_update.c 12 Jul 2023 14:45:43 -0000 1.163
+++ rde_update.c 12 Oct 2023 12:54:05 -0000
@@ -352,6 +352,11 @@ up_generate_addpath_all(struct rde_peer
all = 1;
}
+ if (new != NULL && !prefix_eligible(new)) {
+ /* only allow valid prefixes */
+ new = NULL;
+ }
+
if (old != NULL) {
/* withdraw stale paths */
p = prefix_adjout_get(peer, old->path_id_tx, old->pt);
