Merge pull request #13690 from LabNConsulting/chopps/mgmtd-cleanup

Chopps/mgmtd cleanup

9 files changed, 62 insertions, 127 deletions

diff --git a/lib/northbound.c b/lib/northbound.c
index 775f6ff92..ef2344ee1 100644
--- a/lib/northbound.c
+++ b/lib/northbound.c
@@ -792,18 +792,19 @@ static void nb_update_candidate_changes(struct nb_config *candidate,
 	LYD_TREE_DFS_BEGIN (root, dnode) {
 		op = nb_lyd_diff_get_op(dnode);
 		switch (op) {
-		case 'c':
+		case 'c': /* create */
 			nb_config_diff_created(dnode, seq, cfg_chgs);
 			LYD_TREE_DFS_continue = 1;
 			break;
-		case 'd':
+		case 'd': /* delete */
 			nb_config_diff_deleted(dnode, seq, cfg_chgs);
 			LYD_TREE_DFS_continue = 1;
 			break;
-		case 'r':
+		case 'r': /* replace */
 			nb_config_diff_add_change(cfg_chgs, NB_OP_MODIFY, seq,
 						  dnode);
 			break;
+		case 'n': /* none */
 		default:
 			break;
 		}
diff --git a/lib/vty.c b/lib/vty.c
index b701f3bc0..50c4f80aa 100644
--- a/lib/vty.c
+++ b/lib/vty.c
@@ -2420,6 +2420,7 @@ void vty_close(struct vty *vty)
 	vty->status = VTY_CLOSE;
 
 	if (mgmt_fe_client && vty->mgmt_session_id) {
+		MGMTD_FE_CLIENT_DBG("closing vty session");
 		mgmt_fe_destroy_client_session(mgmt_fe_client,
 					       vty->mgmt_client_id);
 		vty->mgmt_session_id = 0;
diff --git a/mgmtd/mgmt_be_adapter.c b/mgmtd/mgmt_be_adapter.c
index 2d01f8eca..e4a62951d 100644
--- a/mgmtd/mgmt_be_adapter.c
+++ b/mgmtd/mgmt_be_adapter.c
@@ -564,8 +564,8 @@ mgmt_be_adapter_handle_msg(struct mgmt_be_client_adapter *adapter,
 	return 0;
 }
 
-static int mgmt_be_send_txn_req(struct mgmt_be_client_adapter *adapter,
-				    uint64_t txn_id, bool create)
+int mgmt_be_send_txn_req(struct mgmt_be_client_adapter *adapter,
+			 uint64_t txn_id, bool create)
 {
 	Mgmtd__BeMessage be_msg;
 	Mgmtd__BeTxnReq txn_req;
@@ -584,11 +584,10 @@ static int mgmt_be_send_txn_req(struct mgmt_be_client_adapter *adapter,
 	return mgmt_be_adapter_send_msg(adapter, &be_msg);
 }
 
-static int
-mgmt_be_send_cfgdata_create_req(struct mgmt_be_client_adapter *adapter,
-				   uint64_t txn_id, uint64_t batch_id,
-				   Mgmtd__YangCfgDataReq **cfgdata_reqs,
-				   size_t num_reqs, bool end_of_data)
+int mgmt_be_send_cfgdata_req(struct mgmt_be_client_adapter *adapter,
+			     uint64_t txn_id, uint64_t batch_id,
+			     Mgmtd__YangCfgDataReq **cfgdata_reqs,
+			     size_t num_reqs, bool end_of_data)
 {
 	Mgmtd__BeMessage be_msg;
 	Mgmtd__BeCfgDataCreateReq cfgdata_req;
@@ -612,8 +611,8 @@ mgmt_be_send_cfgdata_create_req(struct mgmt_be_client_adapter *adapter,
 	return mgmt_be_adapter_send_msg(adapter, &be_msg);
 }
 
-static int mgmt_be_send_cfgapply_req(struct mgmt_be_client_adapter *adapter,
-					uint64_t txn_id)
+int mgmt_be_send_cfgapply_req(struct mgmt_be_client_adapter *adapter,
+			      uint64_t txn_id)
 {
 	Mgmtd__BeMessage be_msg;
 	Mgmtd__BeCfgDataApplyReq apply_req;
@@ -834,35 +833,6 @@ int mgmt_be_get_adapter_config(struct mgmt_be_client_adapter *adapter,
 	return 0;
 }
 
-int mgmt_be_create_txn(struct mgmt_be_client_adapter *adapter,
-			   uint64_t txn_id)
-{
-	return mgmt_be_send_txn_req(adapter, txn_id, true);
-}
-
-int mgmt_be_destroy_txn(struct mgmt_be_client_adapter *adapter,
-			    uint64_t txn_id)
-{
-	return mgmt_be_send_txn_req(adapter, txn_id, false);
-}
-
-int mgmt_be_send_cfg_data_create_req(struct mgmt_be_client_adapter *adapter,
-					uint64_t txn_id, uint64_t batch_id,
-					struct mgmt_be_cfgreq *cfg_req,
-					bool end_of_data)
-{
-	return mgmt_be_send_cfgdata_create_req(
-		adapter, txn_id, batch_id, cfg_req->cfgdata_reqs,
-		cfg_req->num_reqs, end_of_data);
-}
-
-extern int
-mgmt_be_send_cfg_apply_req(struct mgmt_be_client_adapter *adapter,
-			      uint64_t txn_id)
-{
-	return mgmt_be_send_cfgapply_req(adapter, txn_id);
-}
-
 void mgmt_be_get_subscr_info_for_xpath(
 	const char *xpath, struct mgmt_be_client_subscr_info *subscr_info)
 {
diff --git a/mgmtd/mgmt_be_adapter.h b/mgmtd/mgmt_be_adapter.h
index 8f4eef5fb..e1676e63a 100644
--- a/mgmtd/mgmt_be_adapter.h
+++ b/mgmtd/mgmt_be_adapter.h
@@ -115,13 +115,9 @@ mgmt_be_get_adapter_config(struct mgmt_be_client_adapter *adapter,
 			      struct mgmt_ds_ctx *ds_ctx,
 			      struct nb_config_cbs **cfg_chgs);
 
-/* Create a transaction. */
-extern int mgmt_be_create_txn(struct mgmt_be_client_adapter *adapter,
-				  uint64_t txn_id);
-
-/* Destroy a transaction. */
-extern int mgmt_be_destroy_txn(struct mgmt_be_client_adapter *adapter,
-				   uint64_t txn_id);
+/* Create/destroy a transaction. */
+extern int mgmt_be_send_txn_req(struct mgmt_be_client_adapter *adapter,
+				uint64_t txn_id, bool create);
 
 /*
  * Send config data create request to backend client.
@@ -135,8 +131,11 @@ extern int mgmt_be_destroy_txn(struct mgmt_be_client_adapter *adapter,
  * batch_id
  *    Request batch ID.
  *
- * cfg_req
- *    Config data request.
+ * cfgdata_reqs
+ *    An array of pointer to Mgmtd__YangCfgDataReq.
+ *
+ * num_reqs
+ *    Length of the cfgdata_reqs array.
  *
  * end_of_data
  *    TRUE if the data from last batch, FALSE otherwise.
@@ -144,37 +143,15 @@ extern int mgmt_be_destroy_txn(struct mgmt_be_client_adapter *adapter,
  * Returns:
  *    0 on success, -1 on failure.
  */
-extern int mgmt_be_send_cfg_data_create_req(
-	struct mgmt_be_client_adapter *adapter, uint64_t txn_id,
-	uint64_t batch_id, struct mgmt_be_cfgreq *cfg_req, bool end_of_data);
-
-/*
- * Send config validate request to backend client.
- *
- * adaptr
- *    Backend adapter information.
- *
- * txn_id
- *    Unique transaction identifier.
- *
- * batch_ids
- *    List of request batch IDs.
- *
- * num_batch_ids
- *    Number of batch ids.
- *
- * Returns:
- *    0 on success, -1 on failure.
- */
-extern int
-mgmt_be_send_cfg_validate_req(struct mgmt_be_client_adapter *adapter,
-				 uint64_t txn_id, uint64_t batch_ids[],
-				 size_t num_batch_ids);
+extern int mgmt_be_send_cfgdata_req(struct mgmt_be_client_adapter *adapter,
+				    uint64_t txn_id, uint64_t batch_id,
+				    Mgmtd__YangCfgDataReq **cfgdata_reqs,
+				    size_t num_reqs, bool end_of_data);
 
 /*
  * Send config apply request to backend client.
  *
- * adaptr
+ * adapter
  *    Backend adapter information.
  *
  * txn_id
@@ -183,9 +160,8 @@ mgmt_be_send_cfg_validate_req(struct mgmt_be_client_adapter *adapter,
  * Returns:
  *    0 on success, -1 on failure.
  */
-extern int
-mgmt_be_send_cfg_apply_req(struct mgmt_be_client_adapter *adapter,
-			      uint64_t txn_id);
+extern int mgmt_be_send_cfgapply_req(struct mgmt_be_client_adapter *adapter,
+				     uint64_t txn_id);
 
 /*
  * Dump backend adapter status to vty.
diff --git a/mgmtd/mgmt_txn.c b/mgmtd/mgmt_txn.c
index a666422b7..588693b7e 100644
--- a/mgmtd/mgmt_txn.c
+++ b/mgmtd/mgmt_txn.c
@@ -635,7 +635,6 @@ static void mgmt_txn_process_set_cfg(struct event *thread)
 		      txn->session_id);
 
 	FOREACH_TXN_REQ_IN_LIST (&txn->set_cfg_reqs, txn_req) {
-		error = false;
 		assert(txn_req->req_event == MGMTD_TXN_PROC_SETCFG);
 		ds_ctx = txn_req->req.set_cfg->ds_ctx;
 		if (!ds_ctx) {
@@ -644,7 +643,6 @@ static void mgmt_txn_process_set_cfg(struct event *thread)
 				txn_req->req.set_cfg->ds_id, txn_req->req_id,
 				MGMTD_INTERNAL_ERROR, "No such datastore!",
 				txn_req->req.set_cfg->implicit_commit);
-			error = true;
 			goto mgmt_txn_process_set_cfg_done;
 		}
 
@@ -656,7 +654,6 @@ static void mgmt_txn_process_set_cfg(struct event *thread)
 				MGMTD_INTERNAL_ERROR,
 				"Unable to retrieve DS Config Tree!",
 				txn_req->req.set_cfg->implicit_commit);
-			error = true;
 			goto mgmt_txn_process_set_cfg_done;
 		}
 
@@ -713,7 +710,6 @@ static void mgmt_txn_process_set_cfg(struct event *thread)
 				"Failed to send SET_CONFIG_REPLY txn-id %" PRIu64
 				" session-id: %" PRIu64,
 				txn->txn_id, txn->session_id);
-			error = true;
 		}
 
 	mgmt_txn_process_set_cfg_done:
@@ -1337,8 +1333,7 @@ static int mgmt_txn_send_be_txn_create(struct mgmt_txn_ctx *txn)
 	FOREACH_MGMTD_BE_CLIENT_ID (id) {
 		if (cmtcfg_req->subscr_info.xpath_subscr[id]) {
 			adapter = mgmt_be_get_adapter_by_id(id);
-			if (mgmt_be_create_txn(adapter, txn->txn_id)
-			    != 0) {
+			if (mgmt_be_send_txn_req(adapter, txn->txn_id, true)) {
 				(void)mgmt_txn_send_commit_cfg_reply(
 					txn, MGMTD_INTERNAL_ERROR,
 					"Could not send TXN_CREATE to backend adapter");
@@ -1371,9 +1366,8 @@ static int mgmt_txn_send_be_txn_create(struct mgmt_txn_ctx *txn)
 	return 0;
 }
 
-static int
-mgmt_txn_send_be_cfg_data(struct mgmt_txn_ctx *txn,
-			      struct mgmt_be_client_adapter *adapter)
+static int mgmt_txn_send_be_cfg_data(struct mgmt_txn_ctx *txn,
+				     struct mgmt_be_client_adapter *adapter)
 {
 	struct mgmt_commit_cfg_req *cmtcfg_req;
 	struct mgmt_txn_be_cfg_batch *cfg_btch;
@@ -1395,10 +1389,10 @@ mgmt_txn_send_be_cfg_data(struct mgmt_txn_ctx *txn,
 		cfg_req.cfgdata_reqs = cfg_btch->cfg_datap;
 		cfg_req.num_reqs = cfg_btch->num_cfg_data;
 		indx++;
-		if (mgmt_be_send_cfg_data_create_req(
-			    adapter, txn->txn_id, cfg_btch->batch_id, &cfg_req,
-			    indx == num_batches ? true : false)
-		    != 0) {
+		if (mgmt_be_send_cfgdata_req(
+			    adapter, txn->txn_id, cfg_btch->batch_id,
+			    cfg_req.cfgdata_reqs, cfg_req.num_reqs,
+			    indx == num_batches ? true : false)) {
 			(void)mgmt_txn_send_commit_cfg_reply(
 				txn, MGMTD_INTERNAL_ERROR,
 				"Internal Error! Could not send config data to backend!");
@@ -1418,7 +1412,7 @@ mgmt_txn_send_be_cfg_data(struct mgmt_txn_ctx *txn,
 	}
 
 	/*
-	 * This could ne the last Backend Client to send CFGDATA_CREATE_REQ to.
+	 * This could be the last Backend Client to send CFGDATA_CREATE_REQ to.
 	 * Try moving the commit to next phase.
 	 */
 	mgmt_try_move_commit_to_next_phase(txn, cmtcfg_req);
@@ -1438,7 +1432,7 @@ mgmt_txn_send_be_txn_delete(struct mgmt_txn_ctx *txn,
 	cmtcfg_req = &txn->commit_cfg_req->req.commit_cfg;
 	if (cmtcfg_req->subscr_info.xpath_subscr[adapter->id]) {
 		adapter = mgmt_be_get_adapter_by_id(adapter->id);
-		(void)mgmt_be_destroy_txn(adapter, txn->txn_id);
+		(void)mgmt_be_send_txn_req(adapter, txn->txn_id, false);
 
 		FOREACH_TXN_CFG_BATCH_IN_LIST (
 			&txn->commit_cfg_req->req.commit_cfg
@@ -1511,8 +1505,7 @@ static int mgmt_txn_send_be_cfg_apply(struct mgmt_txn_ctx *txn)
 				return -1;
 
 			btch_list = &cmtcfg_req->curr_batches[id];
-			if (mgmt_be_send_cfg_apply_req(adapter, txn->txn_id)
-			    != 0) {
+			if (mgmt_be_send_cfgapply_req(adapter, txn->txn_id)) {
 				(void)mgmt_txn_send_commit_cfg_reply(
 					txn, MGMTD_INTERNAL_ERROR,
 					"Could not send CFG_APPLY_REQ to backend adapter");
@@ -2260,11 +2253,6 @@ uint64_t mgmt_create_txn(uint64_t session_id, enum mgmt_txn_type type)
 	return txn ? txn->txn_id : MGMTD_TXN_ID_NONE;
 }
 
-bool mgmt_txn_id_is_valid(uint64_t txn_id)
-{
-	return mgmt_txn_id2ctx(txn_id) ? true : false;
-}
-
 void mgmt_destroy_txn(uint64_t *txn_id)
 {
 	struct mgmt_txn_ctx *txn;
@@ -2277,17 +2265,6 @@ void mgmt_destroy_txn(uint64_t *txn_id)
 	*txn_id = MGMTD_TXN_ID_NONE;
 }
 
-enum mgmt_txn_type mgmt_get_txn_type(uint64_t txn_id)
-{
-	struct mgmt_txn_ctx *txn;
-
-	txn = mgmt_txn_id2ctx(txn_id);
-	if (!txn)
-		return MGMTD_TXN_TYPE_NONE;
-
-	return txn->type;
-}
-
 int mgmt_txn_send_set_config_req(uint64_t txn_id, uint64_t req_id,
 				  Mgmtd__DatastoreId ds_id,
 				  struct mgmt_ds_ctx *ds_ctx,
diff --git a/mgmtd/mgmt_txn.h b/mgmtd/mgmt_txn.h
index be781ab95..071839713 100644
--- a/mgmtd/mgmt_txn.h
+++ b/mgmtd/mgmt_txn.h
@@ -101,16 +101,6 @@ extern uint64_t mgmt_create_txn(uint64_t session_id, enum mgmt_txn_type type);
 extern void mgmt_destroy_txn(uint64_t *txn_id);
 
 /*
- * Check if transaction is valid given an ID.
- */
-extern bool mgmt_txn_id_is_valid(uint64_t txn_id);
-
-/*
- * Returns the type of transaction given an ID.
- */
-extern enum mgmt_txn_type mgmt_get_txn_type(uint64_t txn_id);
-
-/*
  * Send set-config request to be processed later in transaction.
  *
  * txn_id
diff --git a/tests/topotests/mgmt_startup/test_bigconf.py b/tests/topotests/mgmt_startup/test_bigconf.py
index 3b13229af..4f46c8fab 100644
--- a/tests/topotests/mgmt_startup/test_bigconf.py
+++ b/tests/topotests/mgmt_startup/test_bigconf.py
@@ -42,8 +42,10 @@ def tgen(request):
     tgen = Topogen(topodef, request.module.__name__)
     tgen.start_topology()
 
+    prologue = open(f"{CWD}/r1/mgmtd.conf").read()
+
     confpath = f"{tgen.gears['r1'].gearlogdir}/r1-late-big.conf"
-    start, end = write_big_route_conf("10.0.0.0/8", ROUTE_COUNT, confpath)
+    start, end = write_big_route_conf("10.0.0.0/8", ROUTE_COUNT, confpath, prologue)
     ROUTE_RANGE[0] = start
     ROUTE_RANGE[1] = end
 
diff --git a/tests/topotests/mgmt_startup/test_late_bigconf.py b/tests/topotests/mgmt_startup/test_late_bigconf.py
index 5e594aba6..0b5bf38d1 100644
--- a/tests/topotests/mgmt_startup/test_late_bigconf.py
+++ b/tests/topotests/mgmt_startup/test_late_bigconf.py
@@ -42,8 +42,10 @@ def tgen(request):
     tgen = Topogen(topodef, request.module.__name__)
     tgen.start_topology()
 
+    prologue = open(f"{CWD}/r1/mgmtd.conf").read()
+
     confpath = f"{tgen.gears['r1'].gearlogdir}/r1-late-big.conf"
-    start, end = write_big_route_conf("10.0.0.0/8", ROUTE_COUNT, confpath)
+    start, end = write_big_route_conf("10.0.0.0/8", ROUTE_COUNT, confpath, prologue)
     ROUTE_RANGE[0] = start
     ROUTE_RANGE[1] = end
 
@@ -74,9 +76,23 @@ def test_staticd_latestart(tgen):
     assert result is not None, "last route present and should not be"
 
     step("Starting staticd")
+    t2 = Timeout(0)
     r1.startDaemons(["staticd"])
 
     result = check_kernel(r1, ROUTE_RANGE[0], retry_timeout=60)
     assert result is None, "first route not present and should be"
-    result = check_kernel(r1, ROUTE_RANGE[1], retry_timeout=60)
+    logging.info("r1: elapsed time for first route %ss", t2.elapsed())
+
+    count = 0
+    ocount = 0
+    while count < ROUTE_COUNT:
+        rc, o, e = r1.net.cmd_status("ip -o route | wc -l")
+        if not rc:
+            if count > ocount + 100:
+                ocount = count
+                logging.info("r1: elapsed time for %d routes %s", count, t2.elapsed())
+            count = int(o)
+
+    result = check_kernel(r1, ROUTE_RANGE[1], retry_timeout=1200)
     assert result is None, "last route not present and should be"
+    logging.info("r1: elapsed time for last route %ss", t2.elapsed())
diff --git a/tests/topotests/mgmt_startup/util.py b/tests/topotests/mgmt_startup/util.py
index 87a2ad442..e36635132 100644
--- a/tests/topotests/mgmt_startup/util.py
+++ b/tests/topotests/mgmt_startup/util.py
@@ -50,11 +50,13 @@ def get_ip_networks(super_prefix, count):
     return tuple(network.subnets(count_log2))[0:count]
 
 
-def write_big_route_conf(super_prefix, count, confpath):
+def write_big_route_conf(super_prefix, count, confpath, prologue=""):
     start = None
     end = None
 
     with open(confpath, "w+", encoding="ascii") as f:
+        if prologue:
+            f.write(prologue + "\n")
         for net in get_ip_networks(super_prefix, count):
             end = net
             if not start: