diff options
| author | David Lamparter <equinox@opensourcerouting.org> | 2021-10-21 17:54:42 +0200 | 
|---|---|---|
| committer | David Lamparter <equinox@opensourcerouting.org> | 2021-10-22 12:13:46 +0200 | 
| commit | f68ce9762785c0d33a4a983e6aa7e64c1107f198 (patch) | |
| tree | 1e77c30e40063955de237171e6654316a5e2de29 /zebra/zebra_rib.c | |
| parent | 3c8161eaa88f6ae181637b8ad7d3a39399470cee (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>
Diffstat (limited to 'zebra/zebra_rib.c')
| -rw-r--r-- | zebra/zebra_rib.c | 16 | 
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)) {  | 
