From f87d51201245695ad680916c8f95dae8014393fa Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Thu, 23 Nov 2023 20:23:52 -0300 Subject: [PATCH] ospfd: fix deferred shutdown handling The ospfd cleanup code is relatively complicated given the need to appropriately handle the "max-metric router-lsa on-shutdown (5-100)" command. When that command is configured and an OSPF instance is unconfigured, the removal of the instance should be deferred to allow other routers sufficient time to find alternate paths before the local Router-LSAs are flushed. When ospfd is killed, however, deferred shutdown shouldn't take place and all instances should be cleared immediately. This commit fixes a problem where ospf_deferred_shutdown_finish() was prematurely exiting the daemon when no instances were left, inadvertently preventing ospf_terminate() from clearing the ospfd global variables. Additionally, the commit includes code refactoring to enhance readability and maintainability. Fixes #14855. Signed-off-by: Renato Westphal --- ospfd/ospfd.c | 56 +++++++++++---------------------------------------- 1 file changed, 12 insertions(+), 44 deletions(-) diff --git a/ospfd/ospfd.c b/ospfd/ospfd.c index 2e936ba7aeaa..e304cdddfcff 100644 --- a/ospfd/ospfd.c +++ b/ospfd/ospfd.c @@ -575,40 +575,12 @@ static struct ospf *ospf_lookup_by_name(const char *vrf_name) return NULL; } -/* Handle the second half of deferred shutdown. This is called either - * from the deferred-shutdown timer thread, or directly through - * ospf_deferred_shutdown_check. - * - * Function is to cleanup G-R state, if required then call ospf_finish_final - * to complete shutdown of this ospf instance. Possibly exit if the - * whole process is being shutdown and this was the last OSPF instance. - */ -static void ospf_deferred_shutdown_finish(struct ospf *ospf) -{ - ospf->stub_router_shutdown_time = OSPF_STUB_ROUTER_UNCONFIGURED; - EVENT_OFF(ospf->t_deferred_shutdown); - - ospf_finish_final(ospf); - - /* *ospf is now invalid */ - - /* ospfd being shut-down? If so, was this the last ospf instance? */ - if (CHECK_FLAG(om->options, OSPF_MASTER_SHUTDOWN) - && (listcount(om->ospf) == 0)) { - route_map_finish(); - frr_fini(); - exit(0); - } - - return; -} - -/* Timer thread for G-R */ +/* Timer thread for deferred shutdown */ static void ospf_deferred_shutdown_timer(struct event *t) { struct ospf *ospf = EVENT_ARG(t); - ospf_deferred_shutdown_finish(ospf); + ospf_finish_final(ospf); } /* Check whether deferred-shutdown must be scheduled, otherwise call @@ -635,15 +607,12 @@ static void ospf_deferred_shutdown_check(struct ospf *ospf) ospf_router_lsa_update_area(area); } timeout = ospf->stub_router_shutdown_time; + OSPF_TIMER_ON(ospf->t_deferred_shutdown, + ospf_deferred_shutdown_timer, timeout); } else { /* No timer needed */ - ospf_deferred_shutdown_finish(ospf); - return; + ospf_finish_final(ospf); } - - OSPF_TIMER_ON(ospf->t_deferred_shutdown, ospf_deferred_shutdown_timer, - timeout); - return; } /* Shut down the entire process */ @@ -692,14 +661,12 @@ void ospf_terminate(void) void ospf_finish(struct ospf *ospf) { - /* let deferred shutdown decide */ - ospf_deferred_shutdown_check(ospf); - - /* if ospf_deferred_shutdown returns, then ospf_finish_final is - * deferred to expiry of G-S timer thread. Return back up, hopefully - * to thread scheduler. - */ - return; + if (CHECK_FLAG(om->options, OSPF_MASTER_SHUTDOWN)) + ospf_finish_final(ospf); + else { + /* let deferred shutdown decide */ + ospf_deferred_shutdown_check(ospf); + } } /* Final cleanup of ospf instance */ @@ -899,6 +866,7 @@ static void ospf_finish_final(struct ospf *ospf) EVENT_OFF(ospf->t_ase_calc); EVENT_OFF(ospf->t_maxage); EVENT_OFF(ospf->t_maxage_walker); + EVENT_OFF(ospf->t_deferred_shutdown); EVENT_OFF(ospf->t_abr_task); EVENT_OFF(ospf->t_abr_fr); EVENT_OFF(ospf->t_asbr_check);