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);

Reply via email to