lib: rework debug init

The debug library allows to register a `debug_set_all` callback which should enable all debugs in a daemon. This callback is implemented exactly the same in each daemon. Instead of duplicating the code, rework the lib to allow registration of each debug type, and implement the common code only once in the lib. Signed-off-by: Igor Ryzhov <[email protected]>
idryzhov · Apr 1, 2024 · b0e6f69 · b0e6f69
1 parent fe39a60
commit b0e6f69
Show file tree

Hide file tree

Showing 22 changed files with 93 additions and 212 deletions.
diff --git a/lib/debug.c b/lib/debug.c
@@ -9,42 +9,41 @@
 #include "debug.h"
 #include "command.h"
 
-static struct debug_cb_list_head cb_head;
+static struct debug_list_head debug_head;
 
-DECLARE_LIST(debug_cb_list, struct debug_callbacks, item);
+DECLARE_LIST(debug_list, struct debug, item);
 
 /* All code in this section should be reentrant and MT-safe */
 
 DEFUN_NOSH(debug_all, debug_all_cmd, "[no] debug all",
 	   NO_STR DEBUG_STR "Toggle all debugging output\n")
 {
-	struct debug_callbacks *cb;
-
+	struct debug *debug;
 	bool set = !strmatch(argv[0]->text, "no");
 	uint32_t mode = DEBUG_NODE2MODE(vty->node);
 
-	frr_each (debug_cb_list, &cb_head, cb)
-		cb->debug_set_all(mode, set);
+	frr_each (debug_list, &debug_head, debug) {
+		DEBUG_MODE_SET(debug, mode, set);
+
+		/* If all modes have been turned off, don't preserve options. */
+		if (!DEBUG_MODE_CHECK(debug, DEBUG_MODE_ALL))
+			DEBUG_CLEAR(debug);
+	}
 
 	return CMD_SUCCESS;
 }
 
 /* ------------------------------------------------------------------------- */
 
-void debug_init(struct debug_callbacks *cb)
+void debug_install(struct debug *debug)
 {
-	static bool inited = false;
-
-	if (!inited) {
-		inited = true;
-		debug_cb_list_init(&cb_head);
-	}
-
-	debug_cb_list_add_head(&cb_head, cb);
+	debug_list_add_tail(&debug_head, debug);
 }
 
-void debug_init_cli(void)
+void debug_init(void)
 {
+	debug_list_init(&debug_head);
+
 	install_element(ENABLE_NODE, &debug_all_cmd);
 	install_element(CONFIG_NODE, &debug_all_cmd);
 }
diff --git a/lib/debug.h b/lib/debug.h
@@ -34,6 +34,7 @@ extern "C" {
 #define DEBUG_OPT_NONE 0x00000000
 
 
+PREDECL_LIST(debug_list);
 /*
  * Debugging record.
  *
@@ -69,31 +70,8 @@ extern "C" {
 struct debug {
 	atomic_uint_fast32_t flags;
 	const char *desc;
-};
 
-PREDECL_LIST(debug_cb_list);
-/*
- * Callback set for debugging code.
- *
- * debug_set_all
- *    Function pointer to call when the user requests that all debugs have a
- *    mode set.
- */
-struct debug_callbacks {
-	/*
-	 * Linked list of Callbacks to call
-	 */
-	struct debug_cb_list_item item;
-
-	/*
-	 * flags
-	 *    flags to set on debug flag fields
-	 *
-	 * set
-	 *    true: set flags
-	 *    false: unset flags
-	 */
-	void (*debug_set_all)(uint32_t flags, bool set);
+	struct debug_list_item item;
 };
 
 /*
@@ -218,21 +196,15 @@ struct debug_callbacks {
 #define DEBUGD(name, fmt, ...) DEBUG(debug, name, fmt, ##__VA_ARGS__)
 
 /*
- * Optional initializer for debugging. Highly recommended.
- *
- * This function installs common debugging commands and allows the caller to
- * specify callbacks to take when these commands are issued, allowing the
- * caller to respond to events such as a request to turn off all debugs.
- *
- * MT-Safe
+ * Register a debug item.
  */
-void debug_init(struct debug_callbacks *cb);
+void debug_install(struct debug *debug);
 
 /*
- * Turn on the cli to turn on/off debugs.
- * Should only be called by libfrr
+ * Initialize debugging.
+ * Should only be called by libfrr.
  */
-void debug_init_cli(void);
+void debug_init(void);
 
 #ifdef __cplusplus
 }

diff --git a/lib/libfrr.c b/lib/libfrr.c
@@ -798,6 +798,7 @@ struct event_loop *frr_init(void)
 
 	vty_init(master, di->log_always);
 	lib_cmd_init();
+	debug_init();
 
 	frr_pthread_init();
 #ifdef HAVE_SCRIPTING
@@ -814,8 +815,6 @@ struct event_loop *frr_init(void)
 			  "%s: failed to initialize northbound database",
 			  __func__);
 
-	debug_init_cli();
-
 	return master;
 }
 

diff --git a/lib/mgmt_be_client.c b/lib/mgmt_be_client.c
@@ -1129,10 +1129,6 @@ void mgmt_debug_be_client_show_debug(struct vty *vty)
 		vty_out(vty, "debug mgmt client backend\n");
 }
 
-static struct debug_callbacks mgmt_dbg_be_client_cbs = {
-	.debug_set_all = mgmt_debug_client_be_set
-};
-
 static struct cmd_node mgmt_dbg_node = {
 	.name = "debug mgmt client backend",
 	.node = MGMT_BE_DEBUG_NODE,
@@ -1185,7 +1181,8 @@ struct mgmt_be_client *mgmt_be_client_create(const char *client_name,
 
 void mgmt_be_client_lib_vty_init(void)
 {
-	debug_init(&mgmt_dbg_be_client_cbs);
+	debug_install(&mgmt_dbg_be_client);
+
 	install_node(&mgmt_dbg_node);
 	install_element(ENABLE_NODE, &debug_mgmt_client_be_cmd);
 	install_element(CONFIG_NODE, &debug_mgmt_client_be_cmd);

diff --git a/lib/mgmt_fe_client.c b/lib/mgmt_fe_client.c
@@ -720,10 +720,6 @@ void mgmt_debug_fe_client_show_debug(struct vty *vty)
 		vty_out(vty, "debug mgmt client frontend\n");
 }
 
-static struct debug_callbacks mgmt_dbg_fe_client_cbs = {
-	.debug_set_all = mgmt_debug_client_fe_set
-};
-
 static struct cmd_node mgmt_dbg_node = {
 	.name = "debug mgmt client frontend",
 	.node = MGMT_FE_DEBUG_NODE,
@@ -771,7 +767,8 @@ struct mgmt_fe_client *mgmt_fe_client_create(const char *client_name,
 
 void mgmt_fe_client_lib_vty_init(void)
 {
-	debug_init(&mgmt_dbg_fe_client_cbs);
+	debug_install(&mgmt_dbg_fe_client);
+
 	install_node(&mgmt_dbg_node);
 	install_element(ENABLE_NODE, &debug_mgmt_client_fe_cmd);
 	install_element(CONFIG_NODE, &debug_mgmt_client_fe_cmd);

diff --git a/lib/northbound.h b/lib/northbound.h
@@ -806,7 +806,6 @@ DECLARE_HOOK(nb_notification_send, (const char *xpath, struct list *arguments),
 DECLARE_HOOK(nb_notification_tree_send,
 	     (const char *xpath, const struct lyd_node *tree), (xpath, tree));
 DECLARE_HOOK(nb_client_debug_config_write, (struct vty *vty), (vty));
-DECLARE_HOOK(nb_client_debug_set_all, (uint32_t flags, bool set), (flags, set));
 
 /* Northbound debugging records */
 extern struct debug nb_dbg_cbs_config;

diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c
@@ -1787,21 +1787,6 @@ static const char *const nb_debugs_conflines[] = {
 	"debug northbound libyang",
 };
 
-DEFINE_HOOK(nb_client_debug_set_all, (uint32_t flags, bool set), (flags, set));
-
-static void nb_debug_set_all(uint32_t flags, bool set)
-{
-	for (unsigned int i = 0; i < array_size(nb_debugs); i++) {
-		DEBUG_FLAGS_SET(nb_debugs[i], flags, set);
-
-		/* If all modes have been turned off, don't preserve options. */
-		if (!DEBUG_MODE_CHECK(nb_debugs[i], DEBUG_MODE_ALL))
-			DEBUG_CLEAR(nb_debugs[i]);
-	}
-
-	hook_call(nb_client_debug_set_all, flags, set);
-}
-
 DEFPY (debug_nb,
        debug_nb_cmd,
        "[no] debug northbound\
@@ -1824,8 +1809,13 @@ DEFPY (debug_nb,
        "libyang debugging\n")
 {
 	uint32_t mode = DEBUG_NODE2MODE(vty->node);
+	bool all = false;
 
-	if (cbs) {
+	/* no specific debug --> act on all of them */
+	if (strmatch(argv[argc - 1]->text, "northbound"))
+		all = true;
+
+	if (cbs || all) {
 		bool none = (!cbs_cfg && !cbs_state && !cbs_rpc && !cbs_notify);
 
 		if (none || cbs_cfg)
@@ -1837,21 +1827,15 @@ DEFPY (debug_nb,
 		if (none || cbs_notify)
 			DEBUG_MODE_SET(&nb_dbg_cbs_notify, mode, !no);
 	}
-	if (notifications)
+	if (notifications || all)
 		DEBUG_MODE_SET(&nb_dbg_notif, mode, !no);
-	if (events)
+	if (events || all)
 		DEBUG_MODE_SET(&nb_dbg_events, mode, !no);
-	if (libyang) {
+	if (libyang || all) {
 		DEBUG_MODE_SET(&nb_dbg_libyang, mode, !no);
 		yang_debugging_set(!no);
 	}
 
-	/* no specific debug --> act on all of them */
-	if (strmatch(argv[argc - 1]->text, "northbound")) {
-		nb_debug_set_all(mode, !no);
-		yang_debugging_set(!no);
-	}
-
 	return CMD_SUCCESS;
 }
 
@@ -1868,7 +1852,6 @@ static int nb_debug_config_write(struct vty *vty)
 	return 1;
 }
 
-static struct debug_callbacks nb_dbg_cbs = {.debug_set_all = nb_debug_set_all};
 static struct cmd_node nb_debug_node = {
 	.name = "northbound debug",
 	.node = NORTHBOUND_DEBUG_NODE,
@@ -1936,7 +1919,13 @@ void nb_cli_init(struct event_loop *tm)
 	/* Initialize the shared candidate configuration. */
 	vty_shared_candidate_config = nb_config_new(NULL);
 
-	debug_init(&nb_dbg_cbs);
+	debug_install(&nb_dbg_cbs_config);
+	debug_install(&nb_dbg_cbs_state);
+	debug_install(&nb_dbg_cbs_rpc);
+	debug_install(&nb_dbg_cbs_notify);
+	debug_install(&nb_dbg_notif);
+	debug_install(&nb_dbg_events);
+	debug_install(&nb_dbg_libyang);
 
 	install_node(&nb_debug_node);
 	install_element(ENABLE_NODE, &debug_nb_cmd);

diff --git a/lib/northbound_sysrepo.c b/lib/northbound_sysrepo.c
@@ -614,21 +614,11 @@ static int frr_sr_debug_config_write(struct vty *vty)
 	return 0;
 }
 
-static int frr_sr_debug_set_all(uint32_t flags, bool set)
-{
-	DEBUG_FLAGS_SET(&nb_dbg_client_sysrepo, flags, set);
-
-	/* If all modes have been turned off, don't preserve options. */
-	if (!DEBUG_MODE_CHECK(&nb_dbg_client_sysrepo, DEBUG_MODE_ALL))
-		DEBUG_CLEAR(&nb_dbg_client_sysrepo);
-
-	return 0;
-}
-
 static void frr_sr_cli_init(void)
 {
 	hook_register(nb_client_debug_config_write, frr_sr_debug_config_write);
-	hook_register(nb_client_debug_set_all, frr_sr_debug_set_all);
+
+	debug_install(&nb_dbg_client_sysrepo);
 
 	install_element(ENABLE_NODE, &debug_nb_sr_cmd);
 	install_element(CONFIG_NODE, &debug_nb_sr_cmd);

diff --git a/mgmtd/mgmt.c b/mgmtd/mgmt.c
@@ -39,6 +39,10 @@ void mgmt_master_init(struct event_loop *master, const int buffer_size)
 
 void mgmt_init(void)
 {
+	debug_install(&mgmt_debug_be);
+	debug_install(&mgmt_debug_ds);
+	debug_install(&mgmt_debug_fe);
+	debug_install(&mgmt_debug_txn);
 
 	/* Initialize datastores */
 	mgmt_ds_init(mm);

diff --git a/pathd/path_cli.c b/pathd/path_cli.c
@@ -1314,22 +1314,12 @@ static int path_policy_cli_debug_config_write(struct vty *vty)
 	return 0;
 }
 
-static int path_policy_cli_debug_set_all(uint32_t flags, bool set)
-{
-	DEBUG_FLAGS_SET(&path_policy_debug, flags, set);
-
-	/* If all modes have been turned off, don't preserve options. */
-	if (!DEBUG_MODE_CHECK(&path_policy_debug, DEBUG_MODE_ALL))
-		DEBUG_CLEAR(&path_policy_debug);
-
-	return 0;
-}
-
 void path_cli_init(void)
 {
 	hook_register(nb_client_debug_config_write,
 		      path_policy_cli_debug_config_write);
-	hook_register(nb_client_debug_set_all, path_policy_cli_debug_set_all);
+
+	debug_install(&path_policy_debug);
 
 	install_node(&segment_routing_node);
 	install_node(&sr_traffic_eng_node);

diff --git a/pathd/path_pcep_cli.c b/pathd/path_pcep_cli.c
@@ -47,7 +47,6 @@
 
 /* CLI Function declarations */
 static int pcep_cli_debug_config_write(struct vty *vty);
-static int pcep_cli_debug_set_all(uint32_t flags, bool set);
 static int pcep_cli_pcep_config_write(struct vty *vty);
 static int pcep_cli_pcc_config_write(struct vty *vty);
 static int pcep_cli_pce_config_write(struct vty *vty);
@@ -1710,17 +1709,6 @@ int pcep_cli_debug_config_write(struct vty *vty)
 	return 0;
 }
 
-int pcep_cli_debug_set_all(uint32_t flags, bool set)
-{
-	DEBUG_FLAGS_SET(&pcep_g->dbg, flags, set);
-
-	/* If all modes have been turned off, don't preserve options. */
-	if (!DEBUG_MODE_CHECK(&pcep_g->dbg, DEBUG_MODE_ALL))
-		DEBUG_CLEAR(&pcep_g->dbg);
-
-	return 0;
-}
-
 int pcep_cli_pcep_config_write(struct vty *vty)
 {
 	vty_out(vty, "  pcep\n");
@@ -2345,7 +2333,11 @@ void pcep_cli_init(void)
 	hook_register(pathd_srte_config_write, pcep_cli_pcep_config_write);
 	hook_register(nb_client_debug_config_write,
 		      pcep_cli_debug_config_write);
-	hook_register(nb_client_debug_set_all, pcep_cli_debug_set_all);
+
+	debug_install(&pcep_g->dbg_basic);
+	debug_install(&pcep_g->dbg_path);
+	debug_install(&pcep_g->dbg_msg);
+	debug_install(&pcep_g->dbg_lib);
 
 	memset(&pce_connections_g, 0, sizeof(pce_connections_g));