1 files changed, 108 insertions, 140 deletions

diff --git a/lib/northbound_sysrepo.c b/lib/northbound_sysrepo.c
index f645adede3..860c27edbd 100644
--- a/lib/northbound_sysrepo.c
+++ b/lib/northbound_sysrepo.c
@@ -37,6 +37,7 @@ static struct thread_master *master;
 static struct list *sysrepo_threads;
 static sr_session_ctx_t *session;
 static sr_conn_ctx_t *connection;
+static struct nb_transaction *transaction;
 
 static int frr_sr_read_cb(struct thread *thread);
 static int frr_sr_write_cb(struct thread *thread);
@@ -45,6 +46,7 @@ static int frr_sr_finish(void);
 /* Convert FRR YANG data value to sysrepo YANG data value. */
 static int yang_data_frr2sr(struct yang_data *frr_data, sr_val_t *sr_data)
 {
+	struct nb_node *nb_node;
 	const struct lys_node *snode;
 	struct lys_node_container *scontainer;
 	struct lys_node_leaf *sleaf;
@@ -53,7 +55,15 @@ static int yang_data_frr2sr(struct yang_data *frr_data, sr_val_t *sr_data)
 
 	sr_val_set_xpath(sr_data, frr_data->xpath);
 
-	snode = frr_data->snode;
+	nb_node = nb_node_find(frr_data->xpath);
+	if (!nb_node) {
+		flog_warn(EC_LIB_YANG_UNKNOWN_DATA_PATH,
+			  "%s: unknown data path: %s", __func__,
+			  frr_data->xpath);
+		return -1;
+	}
+
+	snode = nb_node->snode;
 	switch (snode->nodetype) {
 	case LYS_CONTAINER:
 		scontainer = (struct lys_node_container *)snode;
@@ -223,10 +233,9 @@ static int frr_sr_process_change(struct nb_config *candidate,
 	return NB_OK;
 }
 
-/* Callback for changes in the running configuration. */
-static int frr_sr_config_change_cb(sr_session_ctx_t *session,
-				   const char *module_name,
-				   sr_notif_event_t sr_ev, void *private_ctx)
+static int frr_sr_config_change_cb_verify(sr_session_ctx_t *session,
+					  const char *module_name,
+					  bool startup_config)
 {
 	sr_change_iter_t *it;
 	int ret;
@@ -235,14 +244,6 @@ static int frr_sr_config_change_cb(sr_session_ctx_t *session,
 	char xpath[XPATH_MAXLEN];
 	struct nb_config *candidate;
 
-	/*
-	 * Ignore SR_EV_ABORT and SR_EV_APPLY. We'll leverage the northbound
-	 * layer itself to abort or apply the configuration changes when a
-	 * transaction is created.
-	 */
-	if (sr_ev != SR_EV_ENABLED && sr_ev != SR_EV_VERIFY)
-		return SR_ERR_OK;
-
 	snprintf(xpath, sizeof(xpath), "/%s:*", module_name);
 	ret = sr_get_changes_iter(session, xpath, &it);
 	if (ret != SR_ERR_OK) {
@@ -271,15 +272,30 @@ static int frr_sr_config_change_cb(sr_session_ctx_t *session,
 		return SR_ERR_INTERNAL;
 	}
 
-	/* Commit changes. */
-	ret = nb_candidate_commit(candidate, NB_CLIENT_SYSREPO, true, NULL,
-				  NULL);
-	nb_config_free(candidate);
+	transaction = NULL;
+	if (startup_config) {
+		/*
+		 * sysrepod sends the entire startup configuration using a
+		 * single event (SR_EV_ENABLED). This means we need to perform
+		 * the full two-phase commit protocol in one go here.
+		 */
+		ret = nb_candidate_commit(candidate, NB_CLIENT_SYSREPO, true,
+					  NULL, NULL);
+	} else {
+		/*
+		 * Validate the configuration changes and allocate all resources
+		 * required to apply them.
+		 */
+		ret = nb_candidate_commit_prepare(candidate, NB_CLIENT_SYSREPO,
+						  NULL, &transaction);
+	}
 
 	/* Map northbound return code to sysrepo return code. */
 	switch (ret) {
 	case NB_OK:
+		return SR_ERR_OK;
 	case NB_ERR_NO_CHANGES:
+		nb_config_free(candidate);
 		return SR_ERR_OK;
 	case NB_ERR_LOCKED:
 		return SR_ERR_LOCKED;
@@ -290,111 +306,66 @@ static int frr_sr_config_change_cb(sr_session_ctx_t *session,
 	}
 }
 
-static void frr_sr_state_get_elem(struct list *elements,
-				  struct nb_node *nb_node,
-				  const void *list_entry, const char *xpath)
+static int frr_sr_config_change_cb_apply(sr_session_ctx_t *session,
+					 const char *module_name)
 {
-	struct yang_data *data;
+	/* Apply the transaction. */
+	if (transaction) {
+		struct nb_config *candidate = transaction->config;
 
-	data = nb_node->cbs.get_elem(xpath, list_entry);
-	if (data)
-		listnode_add(elements, data);
+		nb_candidate_commit_apply(transaction, true, NULL);
+		nb_config_free(candidate);
+	}
+
+	return SR_ERR_OK;
 }
 
-static void frr_sr_state_cb_container(struct list *elements, const char *xpath,
-				      const struct lys_node *snode)
+static int frr_sr_config_change_cb_abort(sr_session_ctx_t *session,
+					 const char *module_name)
 {
-	struct lys_node *child;
+	/* Abort the transaction. */
+	if (transaction) {
+		struct nb_config *candidate = transaction->config;
 
-	LY_TREE_FOR (snode->child, child) {
-		struct nb_node *nb_node = child->priv;
-		char xpath_child[XPATH_MAXLEN];
-
-		if (!nb_operation_is_valid(NB_OP_GET_ELEM, child))
-			continue;
-
-		snprintf(xpath_child, sizeof(xpath_child), "%s/%s", xpath,
-			 child->name);
-
-		frr_sr_state_get_elem(elements, nb_node, NULL, xpath_child);
+		nb_candidate_commit_abort(transaction);
+		nb_config_free(candidate);
 	}
+
+	return SR_ERR_OK;
 }
 
-static void frr_sr_state_cb_list_entry(struct list *elements,
-				       const char *xpath_list,
-				       const void *list_entry,
-				       struct lys_node *child)
+/* Callback for changes in the running configuration. */
+static int frr_sr_config_change_cb(sr_session_ctx_t *session,
+				   const char *module_name,
+				   sr_notif_event_t sr_ev, void *private_ctx)
 {
-	struct nb_node *nb_node = child->priv;
-	struct lys_node_leaf *sleaf;
-	char xpath_child[XPATH_MAXLEN];
-
-	/* Sysrepo doesn't want to know about list keys. */
-	switch (child->nodetype) {
-	case LYS_LEAF:
-		sleaf = (struct lys_node_leaf *)child;
-		if (lys_is_key(sleaf, NULL))
-			return;
-		break;
-	case LYS_LEAFLIST:
-		break;
+	switch (sr_ev) {
+	case SR_EV_ENABLED:
+		return frr_sr_config_change_cb_verify(session, module_name,
+						      true);
+	case SR_EV_VERIFY:
+		return frr_sr_config_change_cb_verify(session, module_name,
+						      false);
+	case SR_EV_APPLY:
+		return frr_sr_config_change_cb_apply(session, module_name);
+	case SR_EV_ABORT:
+		return frr_sr_config_change_cb_abort(session, module_name);
 	default:
-		return;
+		flog_err(EC_LIB_LIBSYSREPO, "%s: unknown sysrepo event: %u",
+			 __func__, sr_ev);
+		return SR_ERR_INTERNAL;
 	}
-
-	if (!nb_operation_is_valid(NB_OP_GET_ELEM, child))
-		return;
-
-	snprintf(xpath_child, sizeof(xpath_child), "%s/%s", xpath_list,
-		 child->name);
-
-	frr_sr_state_get_elem(elements, nb_node, list_entry, xpath_child);
 }
 
-static void frr_sr_state_cb_list(struct list *elements, const char *xpath,
-				 const struct lys_node *snode)
+static int frr_sr_state_data_iter_cb(const struct lys_node *snode,
+				     struct yang_translator *translator,
+				     struct yang_data *data, void *arg)
 {
-	struct nb_node *nb_node = snode->priv;
-	struct lys_node_list *slist = (struct lys_node_list *)snode;
-	const void *next;
-
-	for (next = nb_node->cbs.get_next(xpath, NULL); next;
-	     next = nb_node->cbs.get_next(xpath, next)) {
-		struct yang_list_keys keys;
-		const void *list_entry;
-		char xpath_list[XPATH_MAXLEN];
-		struct lys_node *child;
-
-		/* Get the list keys. */
-		if (nb_node->cbs.get_keys(next, &keys) != NB_OK) {
-			flog_warn(EC_LIB_NB_CB_STATE,
-				  "%s: failed to get list keys", __func__);
-			continue;
-		}
+	struct list *elements = arg;
 
-		/* Get list item. */
-		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;
-		}
-
-		/* Append list keys to the XPath. */
-		strlcpy(xpath_list, xpath, sizeof(xpath_list));
-		for (unsigned int i = 0; i < keys.num; i++) {
-			snprintf(xpath_list + strlen(xpath_list),
-				 sizeof(xpath_list) - strlen(xpath_list),
-				 "[%s='%s']", slist->keys[i]->name,
-				 keys.key[i].value);
-		}
+	listnode_add(elements, data);
 
-		/* Loop through list entries. */
-		LY_TREE_FOR (snode->child, child) {
-			frr_sr_state_cb_list_entry(elements, xpath_list,
-						   list_entry, child);
-		}
-	}
+	return NB_OK;
 }
 
 /* Callback for state retrieval. */
@@ -404,26 +375,20 @@ static int frr_sr_state_cb(const char *xpath, sr_val_t **values,
 {
 	struct list *elements;
 	struct yang_data *data;
-	const struct lys_node *snode;
 	struct listnode *node;
 	sr_val_t *v;
 	int ret, count, i = 0;
 
-	/* Find schema node. */
-	snode = ly_ctx_get_node(ly_native_ctx, NULL, xpath, 0);
-
 	elements = yang_data_list_new();
-
-	switch (snode->nodetype) {
-	case LYS_CONTAINER:
-		frr_sr_state_cb_container(elements, xpath, snode);
-		break;
-	case LYS_LIST:
-		frr_sr_state_cb_list(elements, xpath, snode);
-		break;
-	default:
-		break;
+	if (nb_oper_data_iterate(xpath, NULL, NB_OPER_DATA_ITER_NORECURSE,
+				 frr_sr_state_data_iter_cb, elements)
+	    != NB_OK) {
+		flog_warn(EC_LIB_NB_OPERATIONAL_DATA,
+			  "%s: failed to obtain operational data [xpath %s]",
+			  __func__, xpath);
+		goto exit;
 	}
+
 	if (list_isempty(elements))
 		goto exit;
 
@@ -723,18 +688,17 @@ static void frr_sr_subscribe_config(struct yang_module *module)
 			 sr_strerror(ret));
 }
 
-static void frr_sr_subscribe_state(const struct lys_node *snode, void *arg1,
-				   void *arg2)
+static int frr_sr_subscribe_state(const struct lys_node *snode, void *arg)
 {
-	struct yang_module *module = arg1;
+	struct yang_module *module = arg;
 	struct nb_node *nb_node;
 	int ret;
 
-	if (!(snode->flags & LYS_CONFIG_R))
-		return;
+	if (!CHECK_FLAG(snode->flags, LYS_CONFIG_R))
+		return YANG_ITER_CONTINUE;
 	/* We only need to subscribe to the root of the state subtrees. */
-	if (snode->parent && (snode->parent->flags & LYS_CONFIG_R))
-		return;
+	if (snode->parent && CHECK_FLAG(snode->parent->flags, LYS_CONFIG_R))
+		return YANG_ITER_CONTINUE;
 
 	nb_node = snode->priv;
 	if (debug_northbound)
@@ -747,17 +711,18 @@ static void frr_sr_subscribe_state(const struct lys_node *snode, void *arg1,
 	if (ret != SR_ERR_OK)
 		flog_err(EC_LIB_LIBSYSREPO, "sr_dp_get_items_subscribe(): %s",
 			 sr_strerror(ret));
+
+	return YANG_ITER_CONTINUE;
 }
 
-static void frr_sr_subscribe_rpc(const struct lys_node *snode, void *arg1,
-				 void *arg2)
+static int frr_sr_subscribe_rpc(const struct lys_node *snode, void *arg)
 {
-	struct yang_module *module = arg1;
+	struct yang_module *module = arg;
 	struct nb_node *nb_node;
 	int ret;
 
 	if (snode->nodetype != LYS_RPC)
-		return;
+		return YANG_ITER_CONTINUE;
 
 	nb_node = snode->priv;
 	if (debug_northbound)
@@ -770,17 +735,18 @@ static void frr_sr_subscribe_rpc(const struct lys_node *snode, void *arg1,
 	if (ret != SR_ERR_OK)
 		flog_err(EC_LIB_LIBSYSREPO, "sr_rpc_subscribe(): %s",
 			 sr_strerror(ret));
+
+	return YANG_ITER_CONTINUE;
 }
 
-static void frr_sr_subscribe_action(const struct lys_node *snode, void *arg1,
-				    void *arg2)
+static int frr_sr_subscribe_action(const struct lys_node *snode, void *arg)
 {
-	struct yang_module *module = arg1;
+	struct yang_module *module = arg;
 	struct nb_node *nb_node;
 	int ret;
 
 	if (snode->nodetype != LYS_ACTION)
-		return;
+		return YANG_ITER_CONTINUE;
 
 	nb_node = snode->priv;
 	if (debug_northbound)
@@ -793,6 +759,8 @@ static void frr_sr_subscribe_action(const struct lys_node *snode, void *arg1,
 	if (ret != SR_ERR_OK)
 		flog_err(EC_LIB_LIBSYSREPO, "sr_action_subscribe(): %s",
 			 sr_strerror(ret));
+
+	return YANG_ITER_CONTINUE;
 }
 
 /* FRR's Sysrepo initialization. */
@@ -830,12 +798,12 @@ static int frr_sr_init(const char *program_name)
 	/* Perform subscriptions. */
 	RB_FOREACH (module, yang_modules, &yang_modules) {
 		frr_sr_subscribe_config(module);
-		yang_module_snodes_iterate(module->info, frr_sr_subscribe_state,
-					   0, module, NULL);
-		yang_module_snodes_iterate(module->info, frr_sr_subscribe_rpc,
-					   0, module, NULL);
-		yang_module_snodes_iterate(
-			module->info, frr_sr_subscribe_action, 0, module, NULL);
+		yang_snodes_iterate_module(module->info, frr_sr_subscribe_state,
+					   0, module);
+		yang_snodes_iterate_module(module->info, frr_sr_subscribe_rpc,
+					   0, module);
+		yang_snodes_iterate_module(module->info,
+					   frr_sr_subscribe_action, 0, module);
 	}
 
 	hook_register(nb_notification_send, frr_sr_notification_send);