lib, tests: major rework in the operational-data callbacks

The northbound infrastructure for operational data was subpar compared
to the infrastructure for configuration data. This commit addresses most
of the existing problems, making it possible to write operational-data
callbacks for more complex YANG models.

Summary of the changes:
* Add support for nested YANG lists.
* Add support for leaf-lists.
* Add support for leafs of type "empty".
* Introduce the "show yang operational-data XPATH" command, and write an
  unit test for it. The main purpose of this command is to make it
  easier to test the operational-data northbound callbacks.
* Introduce the nb_oper_data_iterate() function, that can be used
  to iterate over operational data. Make the CLI and sysrepo use this
  function.
* Since ConfD has a very peculiar API, it can't reuse the
  nb_oper_data_iterate() like the other northbound clients. In this
  case, adapt the existing ConfD callbacks to support the new features
  (and make some performance improvements in the process).

Signed-off-by: Renato Westphal <renato@opensourcerouting.org>

1 files changed, 167 insertions, 102 deletions

diff --git a/lib/northbound_confd.c b/lib/northbound_confd.c
index 886f17f81f..9d01541205 100644
--- a/lib/northbound_confd.c
+++ b/lib/northbound_confd.c
@@ -91,22 +91,61 @@ static int frr_confd_val2str(const char *xpath, const confd_value_t *value,
 	return 0;
 }
 
-/* Obtain list keys from ConfD hashed keypath. */
-static void frr_confd_hkeypath_get_keys(const confd_hkeypath_t *kp,
-					struct yang_list_keys *keys)
+/* Obtain list entry from ConfD hashed keypath. */
+static int frr_confd_hkeypath_get_list_entry(const confd_hkeypath_t *kp,
+					     struct nb_node *nb_node,
+					     const void **list_entry)
 {
-	memset(keys, 0, sizeof(*keys));
-	for (int i = 0; i < kp->len; i++) {
+	struct nb_node *nb_node_list;
+	int parent_lists = 0;
+	int curr_list = 0;
+
+	*list_entry = NULL;
+
+	/*
+	 * Count the number of YANG lists in the path, disconsidering the
+	 * last element.
+	 */
+	nb_node_list = nb_node;
+	while (nb_node_list->parent_list) {
+		nb_node_list = nb_node_list->parent_list;
+		parent_lists++;
+	}
+	if (nb_node->snode->nodetype != LYS_LIST && parent_lists == 0)
+		return 0;
+
+	/* Start from the beginning and move down the tree. */
+	for (int i = kp->len; i >= 0; i--) {
+		struct yang_list_keys keys;
+
+		/* Not a YANG list. */
 		if (kp->v[i][0].type != C_BUF)
 			continue;
 
+		/* Obtain list keys. */
+		memset(&keys, 0, sizeof(keys));
 		for (int j = 0; kp->v[i][j].type != C_NOEXISTS; j++) {
-			strlcpy(keys->key[keys->num],
+			strlcpy(keys.key[keys.num],
 				(char *)kp->v[i][j].val.buf.ptr,
-				sizeof(keys->key[keys->num]));
-			keys->num++;
+				sizeof(keys.key[keys.num]));
+			keys.num++;
 		}
+
+		/* Obtain northbound node associated to the YANG list. */
+		nb_node_list = nb_node;
+		for (int j = curr_list; j < parent_lists; j++)
+			nb_node_list = nb_node_list->parent_list;
+
+		/* Obtain list entry. */
+		*list_entry =
+			nb_node_list->cbs.lookup_entry(*list_entry, &keys);
+		if (*list_entry == NULL)
+			return -1;
+
+		curr_list++;
 	}
+
+	return 0;
 }
 
 /* Fill the current date and time into a confd_datetime structure. */
@@ -493,12 +532,13 @@ static int frr_confd_transaction_init(struct confd_trans_ctx *tctx)
 	return CONFD_OK;
 }
 
+#define CONFD_MAX_CHILD_NODES 32
+
 static int frr_confd_data_get_elem(struct confd_trans_ctx *tctx,
 				   confd_hkeypath_t *kp)
 {
-	struct nb_node *nb_node, *parent_list;
+	struct nb_node *nb_node;
 	char xpath[BUFSIZ];
-	struct yang_list_keys keys;
 	struct yang_data *data;
 	confd_value_t v;
 	const void *list_entry = NULL;
@@ -513,17 +553,9 @@ static int frr_confd_data_get_elem(struct confd_trans_ctx *tctx,
 		return CONFD_OK;
 	}
 
-	parent_list = nb_node->parent_list;
-	if (parent_list) {
-		frr_confd_hkeypath_get_keys(kp, &keys);
-		list_entry = parent_list->cbs.lookup_entry(&keys);
-		if (!list_entry) {
-			flog_warn(EC_LIB_NB_CB_STATE,
-				  "%s: list entry not found: %s", __func__,
-				  xpath);
-			confd_data_reply_not_found(tctx);
-			return CONFD_OK;
-		}
+	if (frr_confd_hkeypath_get_list_entry(kp, nb_node, &list_entry) != 0) {
+		confd_data_reply_not_found(tctx);
+		return CONFD_OK;
 	}
 
 	data = nb_node->cbs.get_elem(xpath, list_entry);
@@ -546,7 +578,8 @@ static int frr_confd_data_get_next(struct confd_trans_ctx *tctx,
 	struct nb_node *nb_node;
 	char xpath[BUFSIZ];
 	struct yang_list_keys keys;
-	const void *nb_next;
+	struct yang_data *data;
+	const void *parent_list_entry, *nb_next;
 	confd_value_t v[LIST_MAXKEYS];
 
 	frr_confd_get_xpath(kp, xpath, sizeof(xpath));
@@ -559,24 +592,51 @@ static int frr_confd_data_get_next(struct confd_trans_ctx *tctx,
 		return CONFD_OK;
 	}
 
-	nb_next = nb_node->cbs.get_next(xpath,
-					(next == -1) ? NULL : (void *)next);
-	if (!nb_next) {
-		/* End of the list. */
+	if (frr_confd_hkeypath_get_list_entry(kp, nb_node, &parent_list_entry)
+	    != 0) {
+		/* List entry doesn't exist anymore. */
 		confd_data_reply_next_key(tctx, NULL, -1, -1);
 		return CONFD_OK;
 	}
-	if (nb_node->cbs.get_keys(nb_next, &keys) != NB_OK) {
-		flog_warn(EC_LIB_NB_CB_STATE, "%s: failed to get list keys",
-			  __func__);
+
+	nb_next = nb_node->cbs.get_next(parent_list_entry,
+					(next == -1) ? NULL : (void *)next);
+	if (!nb_next) {
+		/* End of the list or leaf-list. */
 		confd_data_reply_next_key(tctx, NULL, -1, -1);
 		return CONFD_OK;
 	}
 
-	/* Feed keys to ConfD. */
-	for (size_t i = 0; i < keys.num; i++)
-		CONFD_SET_STR(&v[i], keys.key[i]);
-	confd_data_reply_next_key(tctx, v, keys.num, (long)nb_next);
+	switch (nb_node->snode->nodetype) {
+	case LYS_LIST:
+		memset(&keys, 0, sizeof(keys));
+		if (nb_node->cbs.get_keys(nb_next, &keys) != NB_OK) {
+			flog_warn(EC_LIB_NB_CB_STATE,
+				  "%s: failed to get list keys", __func__);
+			confd_data_reply_next_key(tctx, NULL, -1, -1);
+			return CONFD_OK;
+		}
+
+		/* Feed keys to ConfD. */
+		for (size_t i = 0; i < keys.num; i++)
+			CONFD_SET_STR(&v[i], keys.key[i]);
+		confd_data_reply_next_key(tctx, v, keys.num, (long)nb_next);
+		break;
+	case LYS_LEAFLIST:
+		data = nb_node->cbs.get_elem(xpath, nb_next);
+		if (data) {
+			if (data->value) {
+				CONFD_SET_STR(&v[0], data->value);
+				confd_data_reply_next_key(tctx, v, 1,
+							  (long)nb_next);
+			}
+			yang_data_free(data);
+		} else
+			confd_data_reply_next_key(tctx, NULL, -1, -1);
+		break;
+	default:
+		break;
+	}
 
 	return CONFD_OK;
 }
@@ -588,15 +648,14 @@ static int frr_confd_data_get_object(struct confd_trans_ctx *tctx,
 				     confd_hkeypath_t *kp)
 {
 	struct nb_node *nb_node;
+	const struct lys_node *child;
 	char xpath[BUFSIZ];
-	char xpath_children[XPATH_MAXLEN];
 	char xpath_child[XPATH_MAXLEN];
-	struct yang_list_keys keys;
 	struct list *elements;
 	struct yang_data *data;
 	const void *list_entry;
-	struct ly_set *set;
-	confd_value_t *values;
+	confd_value_t values[CONFD_MAX_CHILD_NODES];
+	size_t nvalues = 0;
 
 	frr_confd_get_xpath(kp, xpath, sizeof(xpath));
 
@@ -605,57 +664,53 @@ static int frr_confd_data_get_object(struct confd_trans_ctx *tctx,
 		flog_warn(EC_LIB_YANG_UNKNOWN_DATA_PATH,
 			  "%s: unknown data path: %s", __func__, xpath);
 		confd_data_reply_not_found(tctx);
-		return CONFD_OK;
+		return CONFD_ERR;
 	}
 
-	frr_confd_hkeypath_get_keys(kp, &keys);
-	list_entry = nb_node->cbs.lookup_entry(&keys);
-	if (!list_entry) {
-		flog_warn(EC_LIB_NB_CB_STATE, "%s: list entry not found: %s",
-			  __func__, xpath);
+	if (frr_confd_hkeypath_get_list_entry(kp, nb_node, &list_entry) != 0) {
 		confd_data_reply_not_found(tctx);
 		return CONFD_OK;
 	}
 
-	/* Find list child nodes. */
-	snprintf(xpath_children, sizeof(xpath_children), "%s/*", xpath);
-	set = lys_find_path(nb_node->snode->module, NULL, xpath_children);
-	if (!set) {
-		flog_warn(EC_LIB_LIBYANG, "%s: lys_find_path() failed",
-			  __func__);
-		return CONFD_ERR;
-	}
-
 	elements = yang_data_list_new();
-	values = XMALLOC(MTYPE_CONFD, set->number * sizeof(*values));
 
 	/* Loop through list child nodes. */
-	for (size_t i = 0; i < set->number; i++) {
-		struct lys_node *child;
-		struct nb_node *nb_node_child;
+	LY_TREE_FOR (nb_node->snode->child, child) {
+		struct nb_node *nb_node_child = child->priv;
+		confd_value_t *v;
+
+		if (nvalues > CONFD_MAX_CHILD_NODES)
+			break;
+
+		v = &values[nvalues++];
 
-		child = set->set.s[i];
-		nb_node_child = child->priv;
+		/* Non-presence containers, lists and leaf-lists. */
+		if (!nb_node_child->cbs.get_elem) {
+			CONFD_SET_NOEXISTS(v);
+			continue;
+		}
 
 		snprintf(xpath_child, sizeof(xpath_child), "%s/%s", xpath,
 			 child->name);
-
 		data = nb_node_child->cbs.get_elem(xpath_child, list_entry);
 		if (data) {
 			if (data->value)
-				CONFD_SET_STR(&values[i], data->value);
-			else
-				CONFD_SET_NOEXISTS(&values[i]);
+				CONFD_SET_STR(v, data->value);
+			else {
+				/* Presence containers and empty leafs. */
+				CONFD_SET_XMLTAG(
+					v, nb_node_child->confd_hash,
+					confd_str2hash(nb_node_child->snode
+							       ->module->ns));
+			}
 			listnode_add(elements, data);
 		} else
-			CONFD_SET_NOEXISTS(&values[i]);
+			CONFD_SET_NOEXISTS(v);
 	}
 
-	confd_data_reply_value_array(tctx, values, set->number);
+	confd_data_reply_value_array(tctx, values, nvalues);
 
 	/* Release memory. */
-	ly_set_free(set);
-	XFREE(MTYPE_CONFD, values);
 	list_delete(&elements);
 
 	return CONFD_OK;
@@ -668,10 +723,9 @@ static int frr_confd_data_get_next_object(struct confd_trans_ctx *tctx,
 					  confd_hkeypath_t *kp, long next)
 {
 	char xpath[BUFSIZ];
-	char xpath_children[XPATH_MAXLEN];
 	struct nb_node *nb_node;
-	struct ly_set *set;
 	struct list *elements;
+	const void *parent_list_entry;
 	const void *nb_next;
 #define CONFD_OBJECTS_PER_TIME 100
 	struct confd_next_object objects[CONFD_OBJECTS_PER_TIME + 1];
@@ -687,13 +741,10 @@ static int frr_confd_data_get_next_object(struct confd_trans_ctx *tctx,
 		return CONFD_OK;
 	}
 
-	/* Find list child nodes. */
-	snprintf(xpath_children, sizeof(xpath_children), "%s/*", xpath);
-	set = lys_find_path(nb_node->snode->module, NULL, xpath_children);
-	if (!set) {
-		flog_warn(EC_LIB_LIBYANG, "%s: lys_find_path() failed",
-			  __func__);
-		return CONFD_ERR;
+	if (frr_confd_hkeypath_get_list_entry(kp, nb_node, &parent_list_entry)
+	    != 0) {
+		confd_data_reply_next_object_array(tctx, NULL, 0, 0);
+		return CONFD_OK;
 	}
 
 	elements = yang_data_list_new();
@@ -702,62 +753,76 @@ static int frr_confd_data_get_next_object(struct confd_trans_ctx *tctx,
 	memset(objects, 0, sizeof(objects));
 	for (int j = 0; j < CONFD_OBJECTS_PER_TIME; j++) {
 		struct confd_next_object *object;
-		struct yang_list_keys keys;
+		struct lys_node *child;
 		struct yang_data *data;
-		const void *list_entry;
+		size_t nvalues = 0;
 
 		object = &objects[j];
 
-		nb_next = nb_node->cbs.get_next(xpath, nb_next);
+		nb_next = nb_node->cbs.get_next(parent_list_entry, nb_next);
 		if (!nb_next)
 			/* End of the list. */
 			break;
 
-		if (nb_node->cbs.get_keys(nb_next, &keys) != NB_OK) {
-			flog_warn(EC_LIB_NB_CB_STATE,
-				  "%s: failed to get list keys", __func__);
-			continue;
-		}
 		object->next = (long)nb_next;
 
-		list_entry = nb_node->cbs.lookup_entry(&keys);
-		if (!list_entry) {
-			flog_warn(EC_LIB_NB_CB_STATE,
-				  "%s: failed to lookup list entry", __func__);
-			continue;
+		/* Leaf-lists require special handling. */
+		if (nb_node->snode->nodetype == LYS_LEAFLIST) {
+			object->v = XMALLOC(MTYPE_CONFD, sizeof(confd_value_t));
+			data = nb_node->cbs.get_elem(xpath, nb_next);
+			assert(data && data->value);
+			CONFD_SET_STR(object->v, data->value);
+			nvalues++;
+			listnode_add(elements, data);
+			goto next;
 		}
 
-		object->v = XMALLOC(MTYPE_CONFD,
-				    set->number * sizeof(confd_value_t));
+		object->v =
+			XMALLOC(MTYPE_CONFD,
+				CONFD_MAX_CHILD_NODES * sizeof(confd_value_t));
 
 		/* Loop through list child nodes. */
-		for (unsigned int i = 0; i < set->number; i++) {
-			struct lys_node *child;
-			struct nb_node *nb_node_child;
+		LY_TREE_FOR (nb_node->snode->child, child) {
+			struct nb_node *nb_node_child = child->priv;
 			char xpath_child[XPATH_MAXLEN];
-			confd_value_t *v = &object->v[i];
+			confd_value_t *v;
+
+			if (nvalues > CONFD_MAX_CHILD_NODES)
+				break;
 
-			child = set->set.s[i];
-			nb_node_child = child->priv;
+			v = &object->v[nvalues++];
+
+			/* Non-presence containers, lists and leaf-lists. */
+			if (!nb_node_child->cbs.get_elem) {
+				CONFD_SET_NOEXISTS(v);
+				continue;
+			}
 
 			snprintf(xpath_child, sizeof(xpath_child), "%s/%s",
 				 xpath, child->name);
-
 			data = nb_node_child->cbs.get_elem(xpath_child,
-							   list_entry);
+							   nb_next);
 			if (data) {
 				if (data->value)
 					CONFD_SET_STR(v, data->value);
-				else
-					CONFD_SET_NOEXISTS(v);
+				else {
+					/*
+					 * Presence containers and empty leafs.
+					 */
+					CONFD_SET_XMLTAG(
+						v, nb_node_child->confd_hash,
+						confd_str2hash(
+							nb_node_child->snode
+								->module->ns));
+				}
 				listnode_add(elements, data);
 			} else
 				CONFD_SET_NOEXISTS(v);
 		}
-		object->n = set->number;
+	next:
+		object->n = nvalues;
 		nobjects++;
 	}
-	ly_set_free(set);
 
 	if (nobjects == 0) {
 		confd_data_reply_next_object_array(tctx, NULL, 0, 0);