5 files changed, 159 insertions, 103 deletions

diff --git a/lib/mgmt_be_client.c b/lib/mgmt_be_client.c
index dc8005bf95..b0d55d2015 100644
--- a/lib/mgmt_be_client.c
+++ b/lib/mgmt_be_client.c
@@ -770,45 +770,6 @@ static int mgmt_be_client_handle_msg(struct mgmt_be_client *client_ctx,
 	return 0;
 }
 
-
-static int be_client_oper_data_cb(const struct lysc_node *snode,
-				  struct yang_translator *translator,
-				  struct yang_data *data, void *arg)
-{
-	struct be_oper_iter_arg *iarg = arg;
-	struct ly_ctx *ly_ctx = ly_native_ctx;
-	struct lyd_node *hint = iarg->hint;
-	struct lyd_node *dnode = NULL;
-	LY_ERR err;
-
-	if (hint &&
-	    (snode == hint->schema || snode->parent == hint->schema->parent)) {
-		/* This node and the previous node share the same parent, use
-		 * this fact to create the sibling node directly in the tree.
-		 */
-		err = lyd_new_term_canon(&hint->parent->node, snode->module,
-					 snode->name, data->value, true, &dnode);
-	} else if (hint && snode->parent == hint->schema) {
-		/* This node is a child of the previous added element (e.g., a list) */
-		err = lyd_new_term_canon(hint, snode->module, snode->name,
-					 data->value, true, &dnode);
-	} else {
-		/* Use the generic xpath parsing create function. This is
-		 * required for creating list entries (along with their child
-		 * key leafs) and other multiple node adding operations.
-		 */
-		err = lyd_new_path(iarg->root, ly_ctx, data->xpath,
-				   (void *)data->value, LYD_NEW_PATH_UPDATE,
-				   &dnode);
-	}
-	if (err)
-		flog_warn(EC_LIB_LIBYANG, "%s: failed creating node: %s: %s",
-			  __func__, data->xpath, ly_errmsg(ly_native_ctx));
-	iarg->hint = dnode;
-	yang_data_free(data);
-	return err ? NB_ERR : NB_OK;
-}
-
 /*
  * Process the get-tree request on our local oper state
  */
@@ -818,8 +779,7 @@ static void be_client_handle_get_tree(struct mgmt_be_client *client,
 {
 	struct mgmt_msg_get_tree *get_tree_msg = msgbuf;
 	struct mgmt_msg_tree_data *tree_msg = NULL;
-	struct be_oper_iter_arg iter_arg = {};
-	struct lyd_node *dnode;
+	struct lyd_node *dnode = NULL;
 	uint8_t *buf = NULL;
 	int ret;
 
@@ -832,13 +792,12 @@ static void be_client_handle_get_tree(struct mgmt_be_client *client,
 	 */
 
 	/* Obtain data. */
-	dnode = yang_dnode_new(ly_native_ctx, false);
-	iter_arg.root = dnode;
 	ret = nb_oper_data_iterate(get_tree_msg->xpath, NULL, 0,
-				   be_client_oper_data_cb, &iter_arg);
+				   NULL, NULL, &dnode);
 	if (ret != NB_OK) {
 fail:
-		yang_dnode_free(dnode);
+		if (dnode)
+			yang_dnode_free(dnode);
 		darr_free(buf);
 		be_client_send_error(client, get_tree_msg->txn_id,
 				     get_tree_msg->req_id, false, -EINVAL,
diff --git a/lib/northbound.c b/lib/northbound.c
index e70d05cea9..d55255a046 100644
--- a/lib/northbound.c
+++ b/lib/northbound.c
@@ -73,9 +73,9 @@ static void nb_transaction_apply_finish(struct nb_transaction *transaction,
 static int nb_oper_data_iter_node(const struct lysc_node *snode,
 				  const char *xpath, const void *list_entry,
 				  const struct yang_list_keys *list_keys,
-				  struct yang_translator *translator,
-				  bool first, uint32_t flags,
-				  nb_oper_data_cb cb, void *arg);
+				  struct yang_translator *translator, bool first,
+				  uint32_t flags, nb_oper_data_cb cb, void *arg,
+				  struct lyd_node *pdnode);
 
 static int nb_node_check_config_only(const struct lysc_node *snode, void *arg)
 {
@@ -1772,7 +1772,8 @@ static int nb_oper_data_iter_children(const struct lysc_node *snode,
 				      const struct yang_list_keys *list_keys,
 				      struct yang_translator *translator,
 				      bool first, uint32_t flags,
-				      nb_oper_data_cb cb, void *arg)
+				      nb_oper_data_cb cb, void *arg,
+				      struct lyd_node *pdnode)
 {
 	const struct lysc_node *child;
 
@@ -1781,7 +1782,7 @@ static int nb_oper_data_iter_children(const struct lysc_node *snode,
 
 		ret = nb_oper_data_iter_node(child, xpath, list_entry,
 					     list_keys, translator, false,
-					     flags, cb, arg);
+					     flags, cb, arg, pdnode);
 		if (ret != NB_OK)
 			return ret;
 	}
@@ -1793,15 +1794,19 @@ static int nb_oper_data_iter_leaf(const struct nb_node *nb_node,
 				  const char *xpath, const void *list_entry,
 				  const struct yang_list_keys *list_keys,
 				  struct yang_translator *translator,
-				  uint32_t flags, nb_oper_data_cb cb, void *arg)
+				  uint32_t flags, nb_oper_data_cb cb, void *arg,
+				  struct lyd_node *pdnode)
 {
+	const struct lysc_node *snode = nb_node->snode;
 	struct yang_data *data;
+	LY_ERR err = LY_SUCCESS;
 
-	if (CHECK_FLAG(nb_node->snode->flags, LYS_CONFIG_W))
+
+	if (CHECK_FLAG(snode->flags, LYS_CONFIG_W))
 		return NB_OK;
 
 	/* Ignore list keys. */
-	if (lysc_is_key(nb_node->snode))
+	if (lysc_is_key(snode))
 		return NB_OK;
 
 	data = nb_callback_get_elem(nb_node, xpath, list_entry);
@@ -1809,50 +1814,89 @@ static int nb_oper_data_iter_leaf(const struct nb_node *nb_node,
 		/* Leaf of type "empty" is not present. */
 		return NB_OK;
 
-	return (*cb)(nb_node->snode, translator, data, arg);
+	/*
+	 * Add a dnode to our tree
+	 */
+	err = lyd_new_term(pdnode, snode->module, snode->name, data->value,
+			   false, NULL);
+	if (err)
+		return NB_ERR_RESOURCE;
+
+	if (cb)
+		return (*cb)(nb_node->snode, translator, data, arg);
+	return NB_OK;
 }
 
 static int nb_oper_data_iter_container(const struct nb_node *nb_node,
-				       const char *xpath,
+				       const char *xpath, bool first,
 				       const void *list_entry,
 				       const struct yang_list_keys *list_keys,
 				       struct yang_translator *translator,
 				       uint32_t flags, nb_oper_data_cb cb,
-				       void *arg)
+				       void *arg, struct lyd_node *pdnode)
 {
 	const struct lysc_node *snode = nb_node->snode;
+	struct lyd_node *cnode = NULL;
+	bool presence = false;
+	LY_ERR err;
+	int ret;
 
 	if (CHECK_FLAG(nb_node->flags, F_NB_NODE_CONFIG_ONLY))
 		return NB_OK;
 
+	if (pdnode->schema == snode)
+		assert(first);
+	else
+		assert(!first);
+
 	/* Read-only presence containers. */
 	if (nb_node->cbs.get_elem) {
 		struct yang_data *data;
 		int ret;
 
+		presence = true;
 		data = nb_callback_get_elem(nb_node, xpath, list_entry);
 		if (data == NULL)
 			/* Presence container is not present. */
 			return NB_OK;
 
-		ret = (*cb)(snode, translator, data, arg);
-		if (ret != NB_OK)
-			return ret;
-	}
+		if (!first) {
+			err = lyd_new_inner(pdnode, snode->module, snode->name,
+					    false, &cnode);
+			if (err)
+				return NB_ERR_RESOURCE;
+		}
 
-	/* Read-write presence containers. */
-	if (CHECK_FLAG(snode->flags, LYS_CONFIG_W)) {
-		struct lysc_node_container *scontainer;
+		if (cb) {
+			ret = (*cb)(snode, translator, data, arg);
+			if (ret != NB_OK)
+				return ret;
+		}
+	}
 
-		scontainer = (struct lysc_node_container *)snode;
-		if (CHECK_FLAG(scontainer->flags, LYS_PRESENCE)
-		    && !yang_dnode_get(running_config->dnode, xpath))
-			return NB_OK;
+	if (first)
+		cnode = pdnode;
+	else if (!cnode) {
+		/* Add a node in for this container in-case we have children. */
+		err = lyd_new_inner(pdnode, snode->module, snode->name, false,
+				    &cnode);
+		if (err)
+			return NB_ERR_RESOURCE;
 	}
 
 	/* Iterate over the child nodes. */
-	return nb_oper_data_iter_children(snode, xpath, list_entry, list_keys,
-					  translator, false, flags, cb, arg);
+	ret = nb_oper_data_iter_children(snode, xpath, list_entry, list_keys,
+					 translator, false, flags, cb, arg,
+					 cnode);
+
+	/* TODO: here we are freeing only if we created; however, we may want to
+	 * also free if pdnode was cnode on entry to cleanup the data tree
+	 */
+	/* If we aren't presence container and we gained no children remove */
+	if (!presence && !first && !lyd_child(cnode))
+		lyd_free_tree(cnode);
+
+	return ret;
 }
 
 static int
@@ -1860,11 +1904,14 @@ nb_oper_data_iter_leaflist(const struct nb_node *nb_node, const char *xpath,
 			   const void *parent_list_entry,
 			   const struct yang_list_keys *parent_list_keys,
 			   struct yang_translator *translator, uint32_t flags,
-			   nb_oper_data_cb cb, void *arg)
+			   nb_oper_data_cb cb, void *arg,
+			   struct lyd_node *pdnode)
 {
+	const struct lysc_node *snode = nb_node->snode;
 	const void *list_entry = NULL;
+	LY_ERR err;
 
-	if (CHECK_FLAG(nb_node->snode->flags, LYS_CONFIG_W))
+	if (CHECK_FLAG(snode->flags, LYS_CONFIG_W))
 		return NB_OK;
 
 	do {
@@ -1881,9 +1928,19 @@ nb_oper_data_iter_leaflist(const struct nb_node *nb_node, const char *xpath,
 		if (data == NULL)
 			continue;
 
-		ret = (*cb)(nb_node->snode, translator, data, arg);
-		if (ret != NB_OK)
-			return ret;
+		/*
+		 * Add a dnode to our tree
+		 */
+		err = lyd_new_term(pdnode, snode->module, snode->name,
+				   data->value, false, NULL);
+		if (err)
+			return NB_ERR_RESOURCE;
+
+		if (cb) {
+			ret = (*cb)(nb_node->snode, translator, data, arg);
+			if (ret != NB_OK)
+				return ret;
+		}
 	} while (list_entry);
 
 	return NB_OK;
@@ -1894,11 +1951,16 @@ static int nb_oper_data_iter_list(const struct nb_node *nb_node,
 				  const void *parent_list_entry,
 				  const struct yang_list_keys *parent_list_keys,
 				  struct yang_translator *translator,
-				  uint32_t flags, nb_oper_data_cb cb, void *arg)
+				  uint32_t flags, nb_oper_data_cb cb, void *arg,
+				  struct lyd_node *pdnode)
 {
+	char xpath[XPATH_MAXLEN * 2];
 	const struct lysc_node *snode = nb_node->snode;
 	const void *list_entry = NULL;
+	struct lyd_node *list_node = NULL;
+	const char *key_preds = NULL;
 	uint32_t position = 1;
+	LY_ERR err;
 
 	if (CHECK_FLAG(nb_node->flags, F_NB_NODE_CONFIG_ONLY))
 		return NB_OK;
@@ -1907,8 +1969,7 @@ static int nb_oper_data_iter_list(const struct nb_node *nb_node,
 	do {
 		const struct lysc_node_leaf *skey;
 		struct yang_list_keys list_keys = {};
-		char xpath[XPATH_MAXLEN * 2];
-		int ret;
+		int len, len2, ret;
 
 		/* Obtain list entry. */
 		list_entry = nb_callback_get_next(nb_node, parent_list_entry,
@@ -1930,16 +1991,23 @@ static int nb_oper_data_iter_list(const struct nb_node *nb_node,
 
 			/* Build XPath of the list entry. */
 			strlcpy(xpath, xpath_list, sizeof(xpath));
+			len = strlen(xpath);
+			key_preds = &xpath[len];
+
 			unsigned int i = 0;
 			LY_FOR_KEYS (snode, skey) {
 				assert(i < list_keys.num);
-				snprintf(xpath + strlen(xpath),
-					 sizeof(xpath) - strlen(xpath),
-					 "[%s='%s']", skey->name,
-					 list_keys.key[i]);
+				len2 = snprintf(xpath + len, sizeof(xpath) - len,
+						"[%s='%s']", skey->name,
+						list_keys.key[i]);
+				if (len2 > (ssize_t)sizeof(xpath) - len)
+					len = sizeof(xpath);
+				else
+					len += len2;
 				i++;
 			}
 			assert(i == list_keys.num);
+
 		} else {
 			/*
 			 * Keyless list - build XPath using a positional index.
@@ -1949,10 +2017,20 @@ static int nb_oper_data_iter_list(const struct nb_node *nb_node,
 			position++;
 		}
 
+		/*
+		 * `pdnode` needs to point at lib - and it does for
+		 * "/frr-vrf:lib/vrf" need to test "/frr-vrf:lib" too though
+		 */
+		err = lyd_new_list2(pdnode, snode->module, snode->name,
+				    key_preds, false, &list_node);
+		if (err)
+			return NB_ERR_RESOURCE;
+
 		/* Iterate over the child nodes. */
-		ret = nb_oper_data_iter_children(
-			nb_node->snode, xpath, list_entry, &list_keys,
-			translator, false, flags, cb, arg);
+		ret = nb_oper_data_iter_children(nb_node->snode, xpath,
+						 list_entry, &list_keys,
+						 translator, false, flags, cb,
+						 arg, list_node);
 		if (ret != NB_OK)
 			return ret;
 	} while (list_entry);
@@ -1964,9 +2042,9 @@ static int nb_oper_data_iter_node(const struct lysc_node *snode,
 				  const char *xpath_parent,
 				  const void *list_entry,
 				  const struct yang_list_keys *list_keys,
-				  struct yang_translator *translator,
-				  bool first, uint32_t flags,
-				  nb_oper_data_cb cb, void *arg)
+				  struct yang_translator *translator, bool first,
+				  uint32_t flags, nb_oper_data_cb cb, void *arg,
+				  struct lyd_node *pdnode)
 {
 	struct nb_node *nb_node;
 	char xpath[XPATH_MAXLEN];
@@ -1976,6 +2054,10 @@ static int nb_oper_data_iter_node(const struct lysc_node *snode,
 	    && CHECK_FLAG(snode->nodetype, LYS_CONTAINER | LYS_LIST))
 		return NB_OK;
 
+	/*
+	 * would be nice to just be building a libyang data tree here as well
+	 */
+
 	/* Update XPath. */
 	strlcpy(xpath, xpath_parent, sizeof(xpath));
 	if (!first && snode->nodetype != LYS_USES) {
@@ -2001,29 +2083,31 @@ static int nb_oper_data_iter_node(const struct lysc_node *snode,
 	nb_node = snode->priv;
 	switch (snode->nodetype) {
 	case LYS_CONTAINER:
-		ret = nb_oper_data_iter_container(nb_node, xpath, list_entry,
-						  list_keys, translator, flags,
-						  cb, arg);
+		ret = nb_oper_data_iter_container(nb_node, xpath, first,
+						  list_entry, list_keys,
+						  translator, flags, cb, arg,
+						  pdnode);
+
 		break;
 	case LYS_LEAF:
 		ret = nb_oper_data_iter_leaf(nb_node, xpath, list_entry,
 					     list_keys, translator, flags, cb,
-					     arg);
+					     arg, pdnode);
 		break;
 	case LYS_LEAFLIST:
 		ret = nb_oper_data_iter_leaflist(nb_node, xpath, list_entry,
 						 list_keys, translator, flags,
-						 cb, arg);
+						 cb, arg, pdnode);
 		break;
 	case LYS_LIST:
 		ret = nb_oper_data_iter_list(nb_node, xpath, list_entry,
 					     list_keys, translator, flags, cb,
-					     arg);
+					     arg, pdnode);
 		break;
 	case LYS_USES:
 		ret = nb_oper_data_iter_children(snode, xpath, list_entry,
 						 list_keys, translator, false,
-						 flags, cb, arg);
+						 flags, cb, arg, pdnode);
 		break;
 	default:
 		break;
@@ -2089,7 +2173,8 @@ done:
 
 
 int nb_oper_data_iterate(const char *xpath, struct yang_translator *translator,
-			 uint32_t flags, nb_oper_data_cb cb, void *arg)
+			 uint32_t flags, nb_oper_data_cb cb, void *arg,
+			 struct lyd_node **tree)
 {
 	struct nb_node *nb_node;
 	const void *list_entry = NULL;
@@ -2198,16 +2283,24 @@ int nb_oper_data_iterate(const char *xpath, struct yang_translator *translator,
 	 */
 	if (dnode->schema->nodetype == LYS_LIST && lyd_child(dnode)
 	    && dnode->schema == nb_node->snode)
-		ret = nb_oper_data_iter_children(
-			nb_node->snode, xpath, list_entry, &list_keys,
-			translator, true, flags, cb, arg);
+		ret = nb_oper_data_iter_children(nb_node->snode, xpath,
+						 list_entry, &list_keys,
+						 translator, true, flags, cb,
+						 arg, dnode);
 	else
 		ret = nb_oper_data_iter_node(nb_node->snode, xpath, list_entry,
 					     &list_keys, translator, true,
-					     flags, cb, arg);
+					     flags, cb, arg, dnode);
 
 	list_delete(&list_dnodes);
-	yang_dnode_free(dnode);
+	if (dnode) {
+		while (lyd_parent(dnode))
+			dnode = lyd_parent(dnode);
+		if (!tree)
+			lyd_free_all(dnode);
+		else
+			*tree = dnode;
+	}
 
 	return ret;
 }
diff --git a/lib/northbound.h b/lib/northbound.h
index 9c0b4d16c3..2a338a082c 100644
--- a/lib/northbound.h
+++ b/lib/northbound.h
@@ -1268,12 +1268,16 @@ extern int nb_running_lock_check(enum nb_client client, const void *user);
  * arg
  *    Arbitrary argument passed as the fourth parameter in each call to 'cb'.
  *
+ * tree
+ *    If non-NULL will contain the data tree built from the walk.
+ *
  * Returns:
  *    NB_OK on success, NB_ERR otherwise.
  */
 extern int nb_oper_data_iterate(const char *xpath,
 				struct yang_translator *translator,
-				uint32_t flags, nb_oper_data_cb cb, void *arg);
+				uint32_t flags, nb_oper_data_cb cb, void *arg,
+				struct lyd_node **tree);
 
 /*
  * Validate if the northbound operation is valid for the given node.
diff --git a/lib/northbound_cli.c b/lib/northbound_cli.c
index f2415d3383..cba40035a3 100644
--- a/lib/northbound_cli.c
+++ b/lib/northbound_cli.c
@@ -1492,7 +1492,7 @@ DEFPY (show_yang_operational_data,
 	/* Obtain data. */
 	dnode = yang_dnode_new(ly_ctx, false);
 	ret = nb_oper_data_iterate(xpath, translator, 0, nb_cli_oper_data_cb,
-				   dnode);
+				   dnode, NULL);
 	if (ret != NB_OK) {
 		if (format == LYD_JSON)
 			vty_out(vty, "{}\n");
diff --git a/lib/northbound_sysrepo.c b/lib/northbound_sysrepo.c
index 7fd4af8356..ea5aa22b14 100644
--- a/lib/northbound_sysrepo.c
+++ b/lib/northbound_sysrepo.c
@@ -378,7 +378,7 @@ static int frr_sr_state_cb(sr_session_ctx_t *session, uint32_t sub_id,
 
 	dnode = *parent;
 	if (nb_oper_data_iterate(request_xpath, NULL, 0,
-				 frr_sr_state_data_iter_cb, dnode)
+				 frr_sr_state_data_iter_cb, dnode, NULL)
 	    != NB_OK) {
 		flog_warn(EC_LIB_NB_OPERATIONAL_DATA,
 			  "%s: failed to obtain operational data [xpath %s]",