zebra: support zebra shutdown and cleanup

Dplane support for zebra's route cleanup during shutdown (clean
shutdown via SIGINT, anyway.) The dplane has the opportunity to
process incoming updates, and then triggers final cleanup
in zebra's main thread.

Signed-off-by: Mark Stapp <mjs@voltanet.io>

5 files changed, 182 insertions, 27 deletions

diff --git a/zebra/main.c b/zebra/main.c
index 33730be92..955ab0445 100644
--- a/zebra/main.c
+++ b/zebra/main.c
@@ -147,6 +147,8 @@ static void sigint(void)
 
 	frr_early_fini();
 
+	zebra_dplane_pre_finish();
+
 	for (ALL_LIST_ELEMENTS(zebrad.client_list, ln, nn, client))
 		zserv_close_client(client);
 
@@ -171,7 +173,18 @@ static void sigint(void)
 	route_map_finish();
 
 	list_delete(&zebrad.client_list);
+
+	/* Indicate that all new dplane work has been enqueued */
 	zebra_dplane_finish();
+}
+
+/* TODO */
+int zebra_finalize(struct thread *dummy)
+{
+	zlog_info("Zebra final shutdown");
+
+	/* Stop dplane thread and finish any cleanup */
+	zebra_dplane_shutdown();
 
 	work_queue_free_and_null(&zebrad.ribq);
 	meta_queue_free(zebrad.mq);
@@ -420,7 +433,7 @@ int main(int argc, char **argv)
 
 	/* RNH init */
 	zebra_rnh_init();
-	
+
 	/* Error init */
 	zebra_error_init();
 
diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
index 2bc8e45eb..1e90fd15c 100644
--- a/zebra/zebra_dplane.c
+++ b/zebra/zebra_dplane.c
@@ -108,7 +108,7 @@ struct zebra_dplane_ctx_s {
 	/* Nexthops */
 	struct nexthop_group zd_ng;
 
-	/* "Previous" nexthops, used only in route update case without netlink */
+	/* "Previous" nexthops, used only in route updates without netlink */
 	struct nexthop_group zd_old_ng;
 
 	/* TODO -- use fixed array of nexthops, to avoid mallocs? */
@@ -155,7 +155,10 @@ static struct zebra_dplane_globals_s {
 	/* Results callback registered by zebra 'core' */
 	dplane_results_fp dg_results_cb;
 
-	/* Sentinel for shutdown */
+	/* Sentinel for beginning of shutdown */
+	volatile bool dg_is_shutdown;
+
+	/* Sentinel for end of shutdown */
 	volatile bool dg_run;
 
 	/* Route-update context queue inbound to the dataplane */
@@ -169,6 +172,7 @@ static struct zebra_dplane_globals_s {
 
 	_Atomic uint64_t dg_routes_in;
 	_Atomic uint32_t dg_routes_queued;
+	_Atomic uint32_t dg_routes_queued_max;
 	_Atomic uint64_t dg_route_errors;
 
 	/* Event-delivery context 'master' for the dplane */
@@ -177,6 +181,9 @@ static struct zebra_dplane_globals_s {
 	/* Event/'thread' pointer for queued updates */
 	struct thread *dg_t_update;
 
+	/* Event pointer for pending shutdown check loop */
+	struct thread *dg_t_shutdown_check;
+
 } zdplane_g;
 
 /*
@@ -229,6 +236,7 @@ static void dplane_ctx_free(dplane_ctx_h *pctx)
 		}
 
 		if ((*pctx)->zd_old_ng.nexthop) {
+			/* This deals with recursive nexthops too */
 			nexthops_free((*pctx)->zd_old_ng.nexthop);
 		}
 
@@ -579,9 +587,8 @@ static int dplane_ctx_route_init(dplane_ctx_h ctx,
 	/* TODO -- maybe use array of nexthops to avoid allocs? */
 
 	/* Ensure that the dplane's nexthop flag is clear. */
-	for (ALL_NEXTHOPS(ctx->zd_ng, nexthop)) {
+	for (ALL_NEXTHOPS(ctx->zd_ng, nexthop))
 		UNSET_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB);
-	}
 
 	/* Trying out the sequence number idea, so we can try to detect
 	 * when a result is stale.
@@ -697,8 +704,26 @@ done:
 				  memory_order_relaxed);
 
 	if (ret == AOK) {
-		atomic_fetch_add_explicit(&zdplane_g.dg_routes_queued, 1,
-					  memory_order_relaxed);
+		uint32_t high, curr;
+
+		curr = atomic_fetch_add_explicit(&zdplane_g.dg_routes_queued, 1,
+					  memory_order_seq_cst);
+
+		/* We don't have add_and_fetch - sigh */
+		curr++;
+
+		/* Maybe update high-water counter also */
+		high = atomic_load_explicit(&zdplane_g.dg_routes_queued_max,
+					    memory_order_seq_cst);
+		while (high < curr) {
+			if (atomic_compare_exchange_weak_explicit(
+				    &zdplane_g.dg_routes_queued_max,
+				    &high, curr,
+				    memory_order_seq_cst,
+				    memory_order_seq_cst))
+				break;
+		}
+
 		result = ZEBRA_DPLANE_REQUEST_QUEUED;
 	} else if (ctx) {
 		atomic_fetch_add_explicit(&zdplane_g.dg_route_errors, 1,
@@ -821,18 +846,24 @@ static int dplane_route_process(struct thread *event)
  */
 int dplane_show_helper(struct vty *vty, bool detailed)
 {
-	uint64_t queued, errs, incoming;
+	uint64_t queued, queue_max, errs, incoming;
 
+	/* Using atomics because counters are being changed in different
+	 * contexts.
+	 */
 	incoming = atomic_load_explicit(&zdplane_g.dg_routes_in,
 					memory_order_relaxed);
 	queued = atomic_load_explicit(&zdplane_g.dg_routes_queued,
 				      memory_order_relaxed);
+	queue_max = atomic_load_explicit(&zdplane_g.dg_routes_queued_max,
+					 memory_order_relaxed);
 	errs = atomic_load_explicit(&zdplane_g.dg_route_errors,
 				    memory_order_relaxed);
 
 	vty_out(vty, "Route updates:            %"PRIu64"\n", incoming);
 	vty_out(vty, "Route update errors:      %"PRIu64"\n", errs);
 	vty_out(vty, "Route update queue depth: %"PRIu64"\n", queued);
+	vty_out(vty, "Route update queue max:   %"PRIu64"\n", queue_max);
 
 	return CMD_SUCCESS;
 }
@@ -939,14 +970,108 @@ static void zebra_dplane_init_internal(struct zebra_t *zebra)
 	zdplane_g.dg_master = zebra->master;
 }
 
+/* Indicates zebra shutdown/exit is in progress. Some operations may be
+ * simplified or skipped during shutdown processing.
+ */
+bool dplane_is_in_shutdown(void)
+{
+	return zdplane_g.dg_is_shutdown;
+}
+
+/*
+ * Early or pre-shutdown, de-init notification api. This runs pretty
+ * early during zebra shutdown, as a signal to stop new work and prepare
+ * for updates generated by shutdown/cleanup activity, as zebra tries to
+ * remove everything it's responsible for.
+ * NB: This runs in the main zebra thread context.
+ */
+void zebra_dplane_pre_finish(void)
+{
+	if (IS_ZEBRA_DEBUG_DPLANE)
+		zlog_debug("Zebra dataplane pre-fini called");
+
+	zdplane_g.dg_is_shutdown = true;
+
+	/* Notify provider(s) of pending shutdown */
+}
+
+/*
+ * Utility to determine whether work remains enqueued within the dplane;
+ * used during system shutdown processing.
+ */
+static bool dplane_work_pending(void)
+{
+	dplane_ctx_h ctx;
+
+	/* TODO -- just checking incoming/pending work for now */
+	DPLANE_LOCK();
+	{
+		ctx = TAILQ_FIRST(&zdplane_g.dg_route_ctx_q);
+	}
+	DPLANE_UNLOCK();
+
+	return (ctx != NULL);
+}
+
+/*
+ * Shutdown-time intermediate callback, used to determine when all pending
+ * in-flight updates are done. If there's still work to do, reschedules itself.
+ * If all work is done, schedules an event to the main zebra thread for
+ * final zebra shutdown.
+ * This runs in the dplane pthread context.
+ */
+static int dplane_check_shutdown_status(struct thread *event)
+{
+	if (IS_ZEBRA_DEBUG_DPLANE)
+		zlog_debug("Zebra dataplane shutdown status check called");
+
+	if (dplane_work_pending()) {
+		/* Reschedule dplane check on a short timer */
+		thread_add_timer_msec(zdplane_g.dg_master,
+				      dplane_check_shutdown_status,
+				      NULL, 100,
+				      &zdplane_g.dg_t_shutdown_check);
+
+		/* TODO - give up and stop waiting after a short time? */
+
+	} else {
+		/* We appear to be done - schedule a final callback event
+		 * for the zebra main pthread.
+		 */
+		thread_add_event(zebrad.master, zebra_finalize, NULL, 0, NULL);
+	}
+
+	return 0;
+}
+
 /*
  * Shutdown, de-init api. This runs pretty late during shutdown,
- * because zebra tries to free/remove/uninstall all routes during shutdown.
+ * after zebra has tried to free/remove/uninstall all routes during shutdown.
+ * At this point, dplane work may still remain to be done, so we can't just
+ * blindly terminate. If there's still work to do, we'll periodically check
+ * and when done, we'll enqueue a task to the zebra main thread for final
+ * termination processing.
+ *
  * NB: This runs in the main zebra thread context.
  */
 void zebra_dplane_finish(void)
 {
-	/* Wait until all pending updates are processed */
+	if (IS_ZEBRA_DEBUG_DPLANE)
+		zlog_debug("Zebra dataplane fini called");
+
+	thread_add_event(zdplane_g.dg_master,
+			 dplane_check_shutdown_status, NULL, 0,
+			 &zdplane_g.dg_t_shutdown_check);
+}
+
+/*
+ * Final phase of shutdown, after all work enqueued to dplane has been
+ * processed. This is called from the zebra main pthread context.
+ */
+void zebra_dplane_shutdown(void)
+{
+	if (IS_ZEBRA_DEBUG_DPLANE)
+		zlog_debug("Zebra dataplane shutdown called");
 
 	/* Stop dplane thread, if it's running */
 
@@ -954,12 +1079,14 @@ void zebra_dplane_finish(void)
 
 	THREAD_OFF(zdplane_g.dg_t_update);
 
-	/* Notify provider(s) of shutdown */
+	/* TODO */
+	/* frr_pthread_stop(...) */
+
+	/* Notify provider(s) of final shutdown */
 
 	/* Clean-up provider objects */
 
 	/* Clean queue(s) */
-
 }
 
 /*
@@ -968,9 +1095,4 @@ void zebra_dplane_finish(void)
 void zebra_dplane_init(void)
 {
 	zebra_dplane_init_internal(&zebrad);
-
-	/* Finalize/cleanup code is called quite late during zebra shutdown;
-	 * zebra expects to try to clean up all vrfs and all routes during
-	 * shutdown, so the dplane must be available until very late.
-	 */
 }
diff --git a/zebra/zebra_dplane.h b/zebra/zebra_dplane.h
index 37cd2ca7f..096256313 100644
--- a/zebra/zebra_dplane.h
+++ b/zebra/zebra_dplane.h
@@ -170,6 +170,11 @@ const struct nexthop_group *dplane_ctx_get_ng(const dplane_ctx_h ctx);
 const struct zebra_dplane_info *dplane_ctx_get_ns(const dplane_ctx_h ctx);
 const struct nexthop_group *dplane_ctx_get_old_ng(const dplane_ctx_h ctx);
 
+/* Indicates zebra shutdown/exit is in progress. Some operations may be
+ * simplified or skipped during shutdown processing.
+ */
+bool dplane_is_in_shutdown(void);
+
 /*
  * Enqueue route change operations for the dataplane.
  */
@@ -241,10 +246,15 @@ int dplane_results_register(dplane_results_fp fp);
  */
 void zebra_dplane_init(void);
 
-/* Finalize/cleanup api, called quite late during zebra shutdown.
+/* Finalize/cleanup apis, one called early as shutdown is starting,
+ * one called late at the end of zebra shutdown, and then one called
+ * from the zebra main thread to stop the dplane thread free all resources.
+ *
  * Zebra expects to try to clean up all vrfs and all routes during
  * shutdown, so the dplane must be available until very late.
  */
+void zebra_dplane_pre_finish(void);
 void zebra_dplane_finish(void);
+void zebra_dplane_shutdown(void);
 
 #endif	/* _ZEBRA_DPLANE_H */
diff --git a/zebra/zebra_rnh.c b/zebra/zebra_rnh.c
index 838a623ef..bae596ec5 100644
--- a/zebra/zebra_rnh.c
+++ b/zebra/zebra_rnh.c
@@ -519,6 +519,18 @@ static void zebra_rnh_process_pbr_tables(int family,
 }
 
 /*
+ * Utility to determine whether a candidate nexthop is useable. We make this
+ * check in a couple of places, so this is a single home for the logic we
+ * use.
+ */
+static bool rnh_nexthop_valid(const struct nexthop *nh)
+{
+	return ((CHECK_FLAG(nh->flags, NEXTHOP_FLAG_FIB)
+		 || CHECK_FLAG(nh->flags, NEXTHOP_FLAG_RECURSIVE))
+		&& CHECK_FLAG(nh->flags, NEXTHOP_FLAG_ACTIVE));
+}
+
+/*
  * Determine appropriate route (route entry) resolving a tracked
  * nexthop.
  */
@@ -567,13 +579,10 @@ zebra_rnh_resolve_nexthop_entry(struct zebra_vrf *zvrf, int family,
 			 * have an installed nexthop to be useful.
 			 */
 			for (nexthop = re->ng.nexthop; nexthop;
-			     nexthop = nexthop->next)
-				if ((CHECK_FLAG(nexthop->flags, NEXTHOP_FLAG_FIB)
-				     || CHECK_FLAG(nexthop->flags,
-						   NEXTHOP_FLAG_RECURSIVE))
-				    && CHECK_FLAG(nexthop->flags,
-						  NEXTHOP_FLAG_ACTIVE))
+			     nexthop = nexthop->next) {
+				if (rnh_nexthop_valid(nexthop))
 					break;
+			}
 
 			if (nexthop == NULL)
 				continue;
@@ -904,9 +913,7 @@ static int send_client(struct rnh *rnh, struct zserv *client, rnh_type_t type,
 		nump = stream_get_endp(s);
 		stream_putc(s, 0);
 		for (nh = re->ng.nexthop; nh; nh = nh->next)
-			if ((CHECK_FLAG(nh->flags, NEXTHOP_FLAG_FIB)
-			     || CHECK_FLAG(nh->flags, NEXTHOP_FLAG_RECURSIVE))
-			    && CHECK_FLAG(nh->flags, NEXTHOP_FLAG_ACTIVE)) {
+			if (rnh_nexthop_valid(nh)) {
 				stream_putc(s, nh->type);
 				switch (nh->type) {
 				case NEXTHOP_TYPE_IPV4:
diff --git a/zebra/zserv.h b/zebra/zserv.h
index aaefd78ee..987c67635 100644
--- a/zebra/zserv.h
+++ b/zebra/zserv.h
@@ -251,4 +251,7 @@ extern void zserv_close_client(struct zserv *client);
 extern void zserv_read_file(char *input);
 #endif
 
+/* TODO */
+int zebra_finalize(struct thread *event);
+
 #endif /* _ZEBRA_ZEBRA_H */