summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Lamparter <equinox@opensourcerouting.org>2021-10-21 17:54:42 +0200
committerDavid Lamparter <equinox@opensourcerouting.org>2021-10-22 12:13:46 +0200
commitf68ce9762785c0d33a4a983e6aa7e64c1107f198 (patch)
tree1e77c30e40063955de237171e6654316a5e2de29
parent3c8161eaa88f6ae181637b8ad7d3a39399470cee (diff)
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>
-rw-r--r--zebra/zebra_rib.c16
1 files changed, 8 insertions, 8 deletions
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index fcac3328c9..84c78a3692 100644
--- 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)) {