Merge pull request #14936 from LabNConsulting/chopps/mgmt-dbg-improve

Chopps/mgmt dbg improve

lib/mgmt_be_client.c

@@ -112,7 +112,11 @@ struct mgmt_be_client {
 #define FOREACH_BE_TXN_IN_LIST(client_ctx, txn)                                \
 	frr_each_safe (mgmt_be_txns, &(client_ctx)->txn_head, (txn))
 
-struct debug mgmt_dbg_be_client = {0, "Management backend client operations"};
+struct debug mgmt_dbg_be_client = {
+	.desc = "Management backend client operations"
+};
+
+struct mgmt_be_client *mgmt_be_client;
 
 static int mgmt_be_client_send_msg(struct mgmt_be_client *client_ctx,
 				   Mgmtd__BeMessage *be_msg)
@@ -820,24 +824,28 @@ static int mgmt_be_client_notify_disconenct(struct msg_conn *conn)
  * Debug Flags
  */
 
+static void mgmt_debug_client_be_set(uint32_t flags, bool set)
+{
+	DEBUG_FLAGS_SET(&mgmt_dbg_be_client, flags, set);
+
+	if (!mgmt_be_client)
+		return;
+
+	mgmt_be_client->client.conn.debug =
+		DEBUG_MODE_CHECK(&mgmt_dbg_be_client, DEBUG_MODE_ALL);
+}
+
 DEFPY(debug_mgmt_client_be, debug_mgmt_client_be_cmd,
       "[no] debug mgmt client backend",
       NO_STR DEBUG_STR MGMTD_STR
       "client\n"
       "backend\n")
 {
-	uint32_t mode = DEBUG_NODE2MODE(vty->node);
+	mgmt_debug_client_be_set(DEBUG_NODE2MODE(vty->node), !no);
-
-	DEBUG_MODE_SET(&mgmt_dbg_be_client, mode, !no);
 
 	return CMD_SUCCESS;
 }
 
-static void mgmt_debug_client_be_set_all(uint32_t flags, bool set)
-{
-	DEBUG_FLAGS_SET(&mgmt_dbg_be_client, flags, set);
-}
-
 static int mgmt_debug_be_client_config_write(struct vty *vty)
 {
 	if (DEBUG_MODE_CHECK(&mgmt_dbg_be_client, DEBUG_MODE_CONF))
@@ -853,7 +861,8 @@ void mgmt_debug_be_client_show_debug(struct vty *vty)
 }
 
 static struct debug_callbacks mgmt_dbg_be_client_cbs = {
-	.debug_set_all = mgmt_debug_client_be_set_all};
+	.debug_set_all = mgmt_debug_client_be_set
+};
 
 static struct cmd_node mgmt_dbg_node = {
 	.name = "debug mgmt client backend",
@@ -867,8 +876,13 @@ struct mgmt_be_client *mgmt_be_client_create(const char *client_name,
 					     uintptr_t user_data,
 					     struct event_loop *event_loop)
 {
-	struct mgmt_be_client *client =
+	struct mgmt_be_client *client;
-		XCALLOC(MTYPE_MGMTD_BE_CLIENT, sizeof(*client));
+
+	if (mgmt_be_client)
+		return NULL;
+
+	client = XCALLOC(MTYPE_MGMTD_BE_CLIENT, sizeof(*client));
+	mgmt_be_client = client;
 
 	/* Only call after frr_init() */
 	assert(running_config);
@@ -902,6 +916,8 @@ void mgmt_be_client_lib_vty_init(void)
 
 void mgmt_be_client_destroy(struct mgmt_be_client *client)
 {
+	assert(client == mgmt_be_client);
+
 	MGMTD_BE_CLIENT_DBG("Destroying MGMTD Backend Client '%s'",
 			    client->name);
 
@@ -912,4 +928,6 @@ void mgmt_be_client_destroy(struct mgmt_be_client *client)
 
 	XFREE(MTYPE_MGMTD_BE_CLIENT_NAME, client->name);
 	XFREE(MTYPE_MGMTD_BE_CLIENT, client);
+
+	mgmt_be_client = NULL;
 }

lib/mgmt_fe_client.c

@@ -47,8 +47,11 @@ struct mgmt_fe_client {
 #define FOREACH_SESSION_IN_LIST(client, session)                               \
 	frr_each_safe (mgmt_sessions, &(client)->sessions, (session))
 
-struct debug mgmt_dbg_fe_client = {0, "Management frontend client operations"};
+struct debug mgmt_dbg_fe_client = {
+	.desc = "Management frontend client operations"
+};
 
+struct mgmt_fe_client *mgmt_fe_client;
 
 static inline const char *dsid2name(Mgmtd__DatastoreId id)
 {
@@ -543,6 +546,16 @@ static int mgmt_fe_client_notify_disconnect(struct msg_conn *conn)
 	return _notify_connect_disconnect(client, false);
 }
 
+static void mgmt_debug_client_fe_set(uint32_t mode, bool set)
+{
+	DEBUG_FLAGS_SET(&mgmt_dbg_fe_client, mode, set);
+
+	if (!mgmt_fe_client)
+		return;
+
+	mgmt_fe_client->client.conn.debug =
+		DEBUG_MODE_CHECK(&mgmt_dbg_fe_client, DEBUG_MODE_ALL);
+}
 
 DEFPY(debug_mgmt_client_fe, debug_mgmt_client_fe_cmd,
       "[no] debug mgmt client frontend",
@@ -550,18 +563,11 @@ DEFPY(debug_mgmt_client_fe, debug_mgmt_client_fe_cmd,
       "client\n"
       "frontend\n")
 {
-	uint32_t mode = DEBUG_NODE2MODE(vty->node);
+	mgmt_debug_client_fe_set(DEBUG_NODE2MODE(vty->node), !no);
-
-	DEBUG_MODE_SET(&mgmt_dbg_fe_client, mode, !no);
 
 	return CMD_SUCCESS;
 }
 
-static void mgmt_debug_client_fe_set_all(uint32_t flags, bool set)
-{
-	DEBUG_FLAGS_SET(&mgmt_dbg_fe_client, flags, set);
-}
-
 static int mgmt_debug_fe_client_config_write(struct vty *vty)
 {
 	if (DEBUG_MODE_CHECK(&mgmt_dbg_fe_client, DEBUG_MODE_CONF))
@@ -577,7 +583,8 @@ void mgmt_debug_fe_client_show_debug(struct vty *vty)
 }
 
 static struct debug_callbacks mgmt_dbg_fe_client_cbs = {
-	.debug_set_all = mgmt_debug_client_fe_set_all};
+	.debug_set_all = mgmt_debug_client_fe_set
+};
 
 static struct cmd_node mgmt_dbg_node = {
 	.name = "debug mgmt client frontend",
@@ -594,8 +601,13 @@ struct mgmt_fe_client *mgmt_fe_client_create(const char *client_name,
 					     uintptr_t user_data,
 					     struct event_loop *event_loop)
 {
-	struct mgmt_fe_client *client =
+	struct mgmt_fe_client *client;
-		XCALLOC(MTYPE_MGMTD_FE_CLIENT, sizeof(*client));
+
+	if (mgmt_fe_client)
+		return NULL;
+
+	client = XCALLOC(MTYPE_MGMTD_FE_CLIENT, sizeof(*client));
+	mgmt_fe_client = client;
 
 	client->name = XSTRDUP(MTYPE_MGMTD_FE_CLIENT_NAME, client_name);
 	client->user_data = user_data;
@@ -692,6 +704,8 @@ void mgmt_fe_client_destroy(struct mgmt_fe_client *client)
 {
 	struct mgmt_fe_client_session *session;
 
+	assert(client == mgmt_fe_client);
+
 	MGMTD_FE_CLIENT_DBG("Destroying MGMTD Frontend Client '%s'",
 			    client->name);
 
@@ -702,4 +716,6 @@ void mgmt_fe_client_destroy(struct mgmt_fe_client *client)
 
 	XFREE(MTYPE_MGMTD_FE_CLIENT_NAME, client->name);
 	XFREE(MTYPE_MGMTD_FE_CLIENT, client);
+
+	mgmt_fe_client = NULL;
 }

mgmtd/mgmt_be_adapter.c

@@ -668,6 +668,8 @@ struct msg_conn *mgmt_be_create_adapter(int conn_fd, union sockunion *from)
 		MGMTD_BE_MAX_NUM_MSG_WRITE, MGMTD_BE_MSG_MAX_LEN, adapter,
 		"BE-adapter");
 
+	adapter->conn->debug = DEBUG_MODE_CHECK(&mgmt_debug_be, DEBUG_MODE_ALL);
+
 	MGMTD_BE_ADAPTER_DBG("Added new MGMTD Backend adapter '%s'",
 			     adapter->name);
 
@@ -677,8 +679,7 @@ struct msg_conn *mgmt_be_create_adapter(int conn_fd, union sockunion *from)
 struct mgmt_be_client_adapter *
 mgmt_be_get_adapter_by_id(enum mgmt_be_client_id id)
 {
-	return (id < MGMTD_BE_CLIENT_ID_MAX ? mgmt_be_adapters_by_id[id]
+	return (id < MGMTD_BE_CLIENT_ID_MAX ? mgmt_be_adapters_by_id[id] : NULL);
-					       : NULL);
 }
 
 struct mgmt_be_client_adapter *
@@ -687,6 +688,14 @@ mgmt_be_get_adapter_by_name(const char *name)
 	return mgmt_be_find_adapter_by_name(name);
 }
 
+void mgmt_be_adapter_toggle_client_debug(bool set)
+{
+	struct mgmt_be_client_adapter *adapter;
+
+	FOREACH_ADAPTER_IN_LIST (adapter)
+		adapter->conn->debug = set;
+}
+
 /*
  * Get a full set of changes for all the config that an adapter is subscribed to
  * receive.

mgmtd/mgmt_be_adapter.h

@@ -149,6 +149,9 @@ mgmt_be_get_adapter_by_name(const char *name);
 extern struct mgmt_be_client_adapter *
 mgmt_be_get_adapter_by_id(enum mgmt_be_client_id id);
 
+/* Toggle debug on or off for connected clients. */
+extern void mgmt_be_adapter_toggle_client_debug(bool set);
+
 /* Fetch backend adapter config. */
 extern int mgmt_be_get_adapter_config(struct mgmt_be_client_adapter *adapter,
 				      struct nb_config_cbs **cfg_chgs);

mgmtd/mgmt_fe_adapter.c

@@ -246,6 +246,14 @@ mgmt_session_id2ctx(uint64_t session_id)
 	return session;
 }
 
+void mgmt_fe_adapter_toggle_client_debug(bool set)
+{
+	struct mgmt_fe_client_adapter *adapter;
+
+	FOREACH_ADAPTER_IN_LIST (adapter)
+		adapter->conn->debug = set;
+}
+
 static struct mgmt_fe_session_ctx *
 mgmt_fe_create_session(struct mgmt_fe_client_adapter *adapter,
 			   uint64_t client_id)
@@ -1132,6 +1140,9 @@ struct msg_conn *mgmt_fe_create_adapter(int conn_fd, union sockunion *from)
 			MGMTD_FE_MAX_NUM_MSG_WRITE, MGMTD_FE_MSG_MAX_LEN,
 			adapter, "FE-adapter");
 
+		adapter->conn->debug = DEBUG_MODE_CHECK(&mgmt_debug_fe,
+							DEBUG_MODE_ALL);
+
 		adapter->setcfg_stats.min_tm = ULONG_MAX;
 		adapter->cmt_stats.min_tm = ULONG_MAX;
 		MGMTD_FE_ADAPTER_DBG("Added new MGMTD Frontend adapter '%s'",

mgmtd/mgmt_fe_adapter.h

@@ -149,4 +149,8 @@ mgmt_fe_get_session_commit_stats(uint64_t session_id);
 extern void mgmt_fe_adapter_status_write(struct vty *vty, bool detail);
 extern void mgmt_fe_adapter_perf_measurement(struct vty *vty, bool config);
 extern void mgmt_fe_adapter_reset_perf_stats(struct vty *vty);
+
+/* Toggle debug on or off for connected clients. */
+extern void mgmt_fe_adapter_toggle_client_debug(bool set);
+
 #endif /* _FRR_MGMTD_FE_ADAPTER_H_ */

mgmtd/mgmt_vty.c

@@ -438,12 +438,18 @@ DEFPY(debug_mgmt, debug_mgmt_cmd,
 {
 	uint32_t mode = DEBUG_NODE2MODE(vty->node);
 
-	if (be)
+	if (be) {
 		DEBUG_MODE_SET(&mgmt_debug_be, mode, !no);
+		mgmt_be_adapter_toggle_client_debug(
+			DEBUG_MODE_CHECK(&mgmt_debug_be, DEBUG_MODE_ALL));
+	}
 	if (ds)
 		DEBUG_MODE_SET(&mgmt_debug_ds, mode, !no);
-	if (fe)
+	if (fe) {
 		DEBUG_MODE_SET(&mgmt_debug_fe, mode, !no);
+		mgmt_fe_adapter_toggle_client_debug(
+			DEBUG_MODE_CHECK(&mgmt_debug_fe, DEBUG_MODE_ALL));
+	}
 	if (txn)
 		DEBUG_MODE_SET(&mgmt_debug_txn, mode, !no);
 

tests/topotests/mgmt_debug_flags/r1/frr.conf

@@ -0,0 +1,11 @@
+log timestamp precision 6
+log file frr.log
+
+! debug mgmt backend datastore frontend transaction
+! debug mgmt client frontend
+! debug mgmt client backend
+
+interface r1-eth0
+  ip address 1.1.1.1/24
+exit
+ip route 11.11.11.11/32 1.1.1.2

tests/topotests/mgmt_debug_flags/test_debug.py

@@ -0,0 +1,129 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 eval: (blacken-mode 1) -*-
+# SPDX-License-Identifier: ISC
+#
+# Copyright (c) 2021, LabN Consulting, L.L.C.
+# Copyright (c) 2019-2020 by
+# Donatas Abraitis <donatas.abraitis@gmail.com>
+#
+# noqa: E501
+#
+"""
+Test static route functionality
+"""
+import pytest
+from lib.common_config import step
+from lib.topogen import Topogen
+from munet.watchlog import WatchLog
+
+pytestmark = [pytest.mark.staticd]
+
+
+@pytest.fixture(scope="module")
+def tgen(request):
+    "Setup/Teardown the environment and provide tgen argument to tests"
+
+    topodef = {"s1": ("r1",)}
+    tgen = Topogen(topodef, request.module.__name__)
+    tgen.start_topology()
+
+    for rname, router in tgen.routers().items():
+        router.load_frr_config("frr.conf")
+
+    tgen.start_router()
+    yield tgen
+    tgen.stop_topology()
+
+
+def test_client_debug_enable(tgen):
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    r1 = tgen.gears["r1"]
+
+    # Start watching log files
+    watch_mgmtd_log = WatchLog(r1.net.rundir / "mgmtd.log")
+    watch_staticd_log = WatchLog(r1.net.rundir / "staticd.log")
+
+    def __test_debug(r1, on):
+        watch_mgmtd_log.snapshot()
+        watch_staticd_log.snapshot()
+
+        # Add ip route and remove look for debug.
+        r1.vtysh_cmd("conf t\nip route 11.11.11.11/32 1.1.1.2")
+        r1.vtysh_cmd("conf t\nno ip route 11.11.11.11/32 1.1.1.2")
+
+        new_mgmt_logs = watch_mgmtd_log.snapshot()
+        new_be_logs = watch_staticd_log.snapshot()
+
+        fe_cl_msg = "Sending SET_CONFIG_REQ"
+        fe_ad_msg = "Got SETCFG_REQ"
+        be_ad_msg = "Sending CFGDATA_CREATE_REQ"
+        be_cl_msg = "Got CFG_APPLY_REQ"
+
+        if on:
+            assert fe_cl_msg in new_mgmt_logs
+            assert fe_ad_msg in new_mgmt_logs
+            assert be_ad_msg in new_mgmt_logs
+            assert be_cl_msg in new_be_logs
+        else:
+            assert fe_cl_msg not in new_mgmt_logs
+            assert fe_ad_msg not in new_mgmt_logs
+            assert be_ad_msg not in new_mgmt_logs
+            assert be_cl_msg not in new_be_logs
+
+    step("test debug off")
+    __test_debug(r1, False)
+
+    # Turn it on
+    step("tests debug on")
+    r1.vtysh_cmd("debug mgmt client frontend")
+    r1.vtysh_cmd("debug mgmt client backend")
+    r1.vtysh_cmd("debug mgmt backend frontend")
+    __test_debug(r1, True)
+
+    # Turn it off
+    step("tests debug off")
+    r1.vtysh_cmd("no debug mgmt client frontend")
+    r1.vtysh_cmd("no debug mgmt client backend")
+    r1.vtysh_cmd("no debug mgmt backend frontend")
+    __test_debug(r1, False)
+
+    # Configure it on
+    step("tests debug on")
+    r1.vtysh_cmd("conf t\ndebug mgmt client frontend")
+    r1.vtysh_cmd("conf t\ndebug mgmt client backend")
+    r1.vtysh_cmd("conf t\ndebug mgmt backend frontend")
+    __test_debug(r1, True)
+
+    # Configure it off
+    step("tests debug off")
+    r1.vtysh_cmd("conf t\nno debug mgmt client frontend")
+    r1.vtysh_cmd("conf t\nno debug mgmt client backend")
+    r1.vtysh_cmd("conf t\nno debug mgmt backend frontend")
+    __test_debug(r1, False)
+
+    # Turn it on
+    step("tests debug on")
+    r1.vtysh_cmd("debug mgmt client frontend")
+    r1.vtysh_cmd("debug mgmt client backend")
+    r1.vtysh_cmd("debug mgmt backend frontend")
+    __test_debug(r1, True)
+    # Configure it on
+    step("tests debug on")
+    r1.vtysh_cmd("conf t\ndebug mgmt client frontend")
+    r1.vtysh_cmd("conf t\ndebug mgmt client backend")
+    r1.vtysh_cmd("conf t\ndebug mgmt backend frontend")
+    __test_debug(r1, True)
+    # Turn it off
+    step("tests debug on")
+    r1.vtysh_cmd("no debug mgmt client frontend")
+    r1.vtysh_cmd("no debug mgmt client backend")
+    r1.vtysh_cmd("no debug mgmt backend frontend")
+    __test_debug(r1, True)
+    # Configure it off
+    step("tests debug off")
+    r1.vtysh_cmd("conf t\nno debug mgmt client frontend")
+    r1.vtysh_cmd("conf t\nno debug mgmt client backend")
+    r1.vtysh_cmd("conf t\nno debug mgmt backend frontend")
+    __test_debug(r1, False)