mgmtd: remove heavy duplication in mgmtd config read

Previously each container created all it's decendents before descending into the children and repeating the process. Signed-off-by: Christian Hopps <[email protected]>
FRRouting · Jan 19, 2024 · 8a0b5d9 · 8a0b5d9
1 parent ee3ce27
commit 8a0b5d9
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 50 deletions.
diff --git a/lib/northbound.c b/lib/northbound.c
@@ -408,10 +408,9 @@ static inline int nb_config_cb_compare(const struct nb_config_cb *a,
 }
 RB_GENERATE(nb_config_cbs, nb_config_cb, entry, nb_config_cb_compare);
 
-static void nb_config_diff_add_change(struct nb_config_cbs *changes,
-				      enum nb_cb_operation operation,
-				      uint32_t *seq,
-				      const struct lyd_node *dnode)
+void nb_config_diff_add_change(struct nb_config_cbs *changes,
+			       enum nb_cb_operation operation, uint32_t *seq,
+			       const struct lyd_node *dnode)
 {
 	struct nb_config_change *change;
 
@@ -690,9 +689,10 @@ int nb_candidate_edit(struct nb_config *candidate, const struct nb_node *nb_node
 		      bool in_backend, const struct yang_data *previous,
 		      const struct yang_data *data)
 {
-	struct lyd_node *dnode, *dep_dnode, *old_dnode, *parent;
+	struct lyd_node *dnode, *dep_dnode, *old_dnode;
 	char xpath_edit[XPATH_MAXLEN];
 	char dep_xpath[XPATH_MAXLEN];
+	struct lyd_node *parent = NULL;
 	uint32_t options = 0;
 	LY_ERR err;
 
@@ -867,10 +867,17 @@ void nb_candidate_edit_config_changes(struct nb_config *candidate_config,
 		/* Find the northbound node associated to the data path. */
 		nb_node = nb_node_find(xpath);
 		if (!nb_node) {
-			flog_warn(EC_LIB_YANG_UNKNOWN_DATA_PATH,
-				  "%s: unknown data path: %s", __func__, xpath);
-			if (error)
-				*error = true;
+			if (in_backend)
+				DEBUGD(&nb_dbg_cbs_config,
+				       "%s: ignoring non-handled path: %s",
+				       __func__, xpath);
+			else {
+				flog_warn(EC_LIB_YANG_UNKNOWN_DATA_PATH,
+					  "%s: unknown data path: %s", __func__,
+					  xpath);
+				if (error)
+					*error = true;
+			}
 			continue;
 		}
 		/* Find if the node to be edited is not a key node */

diff --git a/lib/northbound.h b/lib/northbound.h
@@ -1031,6 +1031,11 @@ extern void nb_candidate_edit_config_changes(struct nb_config *candidate_config,
 					     bool in_backend, char *err_buf,
 					     int err_bufsize, bool *error);
 
+
+extern void nb_config_diff_add_change(struct nb_config_cbs *changes,
+				      enum nb_cb_operation operation,
+				      uint32_t *seq,
+				      const struct lyd_node *dnode);
 /*
  * Delete candidate configuration changes.
  *

diff --git a/mgmtd/mgmt_be_adapter.c b/mgmtd/mgmt_be_adapter.c
@@ -597,20 +597,6 @@ struct mgmt_be_get_adapter_config_params {
 	uint32_t seq;
 };
 
-/*
- * Callback to store the change a node in the datastore if it should be sync'd
- * to the adapter (i.e., if the adapter is subscribed to it).
- */
-static void mgmt_be_iter_and_get_cfg(const char *xpath, struct lyd_node *node,
-				     struct nb_node *nb_node, void *ctx)
-{
-	struct mgmt_be_get_adapter_config_params *parms = ctx;
-	struct mgmt_be_client_adapter *adapter = parms->adapter;
-
-	if (be_is_client_interested(xpath, adapter->id, true))
-		nb_config_diff_created(node, &parms->seq, parms->cfg_chgs);
-}
-
 /*
  * Initialize a BE client over a new connection
  */
@@ -769,32 +755,32 @@ void mgmt_be_adapter_toggle_client_debug(bool set)
  * Get a full set of changes for all the config that an adapter is subscribed to
  * receive.
  */
-int mgmt_be_get_adapter_config(struct mgmt_be_client_adapter *adapter,
-			       struct nb_config_cbs **cfg_chgs)
-{
-	struct mgmt_be_get_adapter_config_params parms;
-	struct nb_config *cfg_root = mgmt_ds_get_nb_config(mm->running_ds);
-
-	assert(cfg_chgs);
-
-	/*
-	 * TODO: we should consider making this an assertable condition and
-	 * guaranteeing it be true when this function is called. B/c what is
-	 * going to happen if there are some changes being sent, and we don't
-	 * gather a new snapshot, what new changes that came after the previous
-	 * snapshot will then be lost?
-	 */
-	if (RB_EMPTY(nb_config_cbs, &adapter->cfg_chgs)) {
-		parms.adapter = adapter;
-		parms.cfg_chgs = &adapter->cfg_chgs;
-		parms.seq = 0;
-
-		mgmt_ds_iter_data(MGMTD_DS_RUNNING, cfg_root, "",
-				  mgmt_be_iter_and_get_cfg, (void *)&parms);
+void mgmt_be_get_adapter_config(struct mgmt_be_client_adapter *adapter,
+			       struct nb_config_cbs **changes)
+{
+	const struct lyd_node *root, *dnode;
+	uint32_t seq = 0;
+	char *xpath;
+
+	/* We can't be in the middle of sending other chgs when here. */
+	assert(RB_EMPTY(nb_config_cbs, &adapter->cfg_chgs));
+
+	*changes = &adapter->cfg_chgs;
+	LY_LIST_FOR (running_config->dnode, root) {
+		LYD_TREE_DFS_BEGIN (root, dnode) {
+			if (lysc_is_key(dnode->schema))
+				goto walk_cont;
+
+			xpath = lyd_path(dnode, LYD_PATH_STD, NULL, 0);
+			if (be_is_client_interested(xpath, adapter->id, true))
+				nb_config_diff_add_change(*changes, NB_CB_CREATE, &seq, dnode);
+			else
+				LYD_TREE_DFS_continue = 1; /* skip any subtree */
+			free(xpath);
+		walk_cont:
+			LYD_TREE_DFS_END(root, dnode);
+		}
 	}
-
-	*cfg_chgs = &adapter->cfg_chgs;
-	return 0;
 }
 
 uint64_t mgmt_be_interested_clients(const char *xpath, bool config)

diff --git a/mgmtd/mgmt_be_adapter.h b/mgmtd/mgmt_be_adapter.h
@@ -157,8 +157,8 @@ extern const char *mgmt_be_client_id2name(enum mgmt_be_client_id id);
 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);
+extern void mgmt_be_get_adapter_config(struct mgmt_be_client_adapter *adapter,
+				       struct nb_config_cbs **changes);
 
 /* Create/destroy a transaction. */
 extern int mgmt_be_send_txn_req(struct mgmt_be_client_adapter *adapter,