zebra: set SELECTED before going into dplane code

There is a bit of an impedance mismatch in the sequence of events here.
Depending on the dplane behavior, the `ROUTE_ENTRY_SELECTED` bit will be
inconsistent for rib_process_result().

With an asynchronous dataplane:
0. rib_process() is called
1. rib_install_kernel() is called, dplane action is queued
2. rib_install_kernel() returns
3. rib_process() sets the SELECTED bit appropriately, returns
4. dplane is done, triggers rib_process_result()
5. SELECTED bit is seen in "after" state
(5a. NHT code looks at the SELECTED bit, works correctly.)

With a synchronous dataplane:
0. rib_process() is called
1. rib_install_kernel() is called, dplane action is executed
2. dplane (should) trigger rib_process_result()
3. SELECTED bit is seen in "before" state
(3a. NHT code looks at the SELECTED bit, fails.)
4. rib_install_kernel() returns
5. rib_process() sets the SELECTED bit appropriately, too late.

Essentially, poking the dataplane is a sequencing point where control is
handed over to the dplane.  Control may or may not return immediately.
Doing /anything/ after triggering the dataplane is a recipe for odd race
conditions.

(FWIW, I'm not sure rib_process_result() is called correctly in the
synchronous case, but that's a separate problem.)

Unfortunately, this change might have some unforeseen side effects.  I
haven't dug through the code to see if anything breaks.  There
/shouldn't/ be anything looking at the SELECTED bit here, but who knows.

Signed-off-by: David Lamparter <equinox@opensourcerouting.org>

diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index fcac3328c97b6d9bc833588a18ded9bb658ba32b..84c78a36923a801777745d7f7687183477212c0f 100644 (file)
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -1255,14 +1255,6 @@ static void rib_process(struct route_node *rn)
        bool selected_changed = new_selected && CHECK_FLAG(new_selected->status,
                                                           ROUTE_ENTRY_CHANGED);
 
-       /* Update fib according to selection results */
-       if (new_fib && old_fib)
-               rib_process_update_fib(zvrf, rn, old_fib, new_fib);
-       else if (new_fib)
-               rib_process_add_fib(zvrf, rn, new_fib);
-       else if (old_fib)
-               rib_process_del_fib(zvrf, rn, old_fib);
-
        /* Update SELECTED entry */
        if (old_selected != new_selected || selected_changed) {
 
@@ -1290,6 +1282,14 @@ static void rib_process(struct route_node *rn)
                }
        }
 
+       /* Update fib according to selection results */
+       if (new_fib && old_fib)
+               rib_process_update_fib(zvrf, rn, old_fib, new_fib);
+       else if (new_fib)
+               rib_process_add_fib(zvrf, rn, new_fib);
+       else if (old_fib)
+               rib_process_del_fib(zvrf, rn, old_fib);
+
        /* Remove all RE entries queued for removal */
        RNODE_FOREACH_RE_SAFE (rn, re, next) {
                if (CHECK_FLAG(re->status, ROUTE_ENTRY_REMOVED)) {