Merge pull request #8311 from idryzhov/nb-sorting

Sort route-maps/access-lists/prefix-lists by sequence number in running-config

lib/filter.h

@@ -240,10 +240,12 @@ bool plist_is_dup(const struct lyd_node *dnode, struct plist_dup_args *pda);
 struct lyd_node;
 struct vty;
 
+extern int access_list_cmp(struct lyd_node *dnode1, struct lyd_node *dnode2);
 extern void access_list_show(struct vty *vty, struct lyd_node *dnode,
 			     bool show_defaults);
 extern void access_list_remark_show(struct vty *vty, struct lyd_node *dnode,
 				    bool show_defaults);
+extern int prefix_list_cmp(struct lyd_node *dnode1, struct lyd_node *dnode2);
 extern void prefix_list_show(struct vty *vty, struct lyd_node *dnode,
 			     bool show_defaults);
 extern void prefix_list_remark_show(struct vty *vty, struct lyd_node *dnode,

lib/filter_cli.c

@@ -1077,6 +1077,14 @@ ALIAS(
 	ACCESS_LIST_REMARK_STR
 	ACCESS_LIST_REMARK_LINE_STR)
 
+int access_list_cmp(struct lyd_node *dnode1, struct lyd_node *dnode2)
+{
+	uint32_t seq1 = yang_dnode_get_uint32(dnode1, "./sequence");
+	uint32_t seq2 = yang_dnode_get_uint32(dnode2, "./sequence");
+
+	return seq1 - seq2;
+}
+
 void access_list_show(struct vty *vty, struct lyd_node *dnode,
 		      bool show_defaults)
 {
@@ -1700,6 +1708,14 @@ ALIAS(
 	ACCESS_LIST_REMARK_STR
 	ACCESS_LIST_REMARK_LINE_STR)
 
+int prefix_list_cmp(struct lyd_node *dnode1, struct lyd_node *dnode2)
+{
+	uint32_t seq1 = yang_dnode_get_uint32(dnode1, "./sequence");
+	uint32_t seq2 = yang_dnode_get_uint32(dnode2, "./sequence");
+
+	return seq1 - seq2;
+}
+
 void prefix_list_show(struct vty *vty, struct lyd_node *dnode,
 		      bool show_defaults)
 {

lib/filter_nb.c

@@ -1665,6 +1665,7 @@ const struct frr_yang_module_info frr_filter_info = {
 			.cbs = {
 				.create = lib_access_list_entry_create,
 				.destroy = lib_access_list_entry_destroy,
+				.cli_cmp = access_list_cmp,
 				.cli_show = access_list_show,
 			}
 		},
@@ -1788,6 +1789,7 @@ const struct frr_yang_module_info frr_filter_info = {
 			.cbs = {
 				.create = lib_prefix_list_entry_create,
 				.destroy = lib_prefix_list_entry_destroy,
+				.cli_cmp = prefix_list_cmp,
 				.cli_show = prefix_list_show,
 			}
 		},

lib/northbound.h

@@ -473,6 +473,23 @@ struct nb_callbacks {
 	 */
 	int (*rpc)(struct nb_cb_rpc_args *args);
 
+	/*
+	 * Optional callback to compare the data nodes when printing
+	 * the CLI commands associated with them.
+	 *
+	 * dnode1
+	 *    The first data node to compare.
+	 *
+	 * dnode2
+	 *    The second data node to compare.
+	 *
+	 * Returns:
+	 *    <0 when the CLI command for the dnode1 should be printed first
+	 *    >0 when the CLI command for the dnode2 should be printed first
+	 *     0 when there is no difference
+	 */
+	int (*cli_cmp)(struct lyd_node *dnode1, struct lyd_node *dnode2);
+
 	/*
 	 * Optional callback to show the CLI command associated to the given
 	 * YANG data node.

lib/northbound_cli.c

@@ -529,25 +529,6 @@ static int nb_cli_candidate_load_transaction(struct vty *vty,
 	return CMD_SUCCESS;
 }
 
-/*
- * ly_iter_next_is_up: detects when iterating up on the yang model.
- *
- * This function detects whether next node in the iteration is upwards,
- * then return the node otherwise return NULL.
- */
-static struct lyd_node *ly_iter_next_up(const struct lyd_node *elem)
-{
-	/* Are we going downwards? Is this still not a leaf? */
-	if (!(elem->schema->nodetype & (LYS_LEAF | LYS_LEAFLIST | LYS_ANYDATA)))
-		return NULL;
-
-	/* Are there still leaves in this branch? */
-	if (elem->next != NULL)
-		return NULL;
-
-	return elem->parent;
-}
-
 /* Prepare the configuration for display. */
 void nb_cli_show_config_prepare(struct nb_config *config, bool with_defaults)
 {
@@ -569,51 +550,78 @@ void nb_cli_show_config_prepare(struct nb_config *config, bool with_defaults)
 				   ly_native_ctx);
 }
 
-void nb_cli_show_dnode_cmds(struct vty *vty, struct lyd_node *root,
-			    bool with_defaults)
+static void show_dnode_children_cmds(struct vty *vty, struct lyd_node *root,
+				     bool with_defaults)
 {
-	struct lyd_node *next, *child, *parent;
-
-	LY_TREE_DFS_BEGIN (root, next, child) {
-		struct nb_node *nb_node;
+	struct nb_node *nb_node, *sort_node = NULL;
+	struct listnode *listnode;
+	struct lyd_node *child;
+	struct list *sort_list;
+	void *data;
 
+	LY_TREE_FOR (root->child, child) {
 		nb_node = child->schema->priv;
-		if (!nb_node || !nb_node->cbs.cli_show)
-			goto next;
 
-		/* Skip default values. */
-		if (!with_defaults && yang_dnode_is_default_recursive(child))
-			goto next;
-
-		(*nb_node->cbs.cli_show)(vty, child, with_defaults);
-	next:
 		/*
-		 * When transiting upwards in the yang model we should
-		 * give the previous container/list node a chance to
-		 * print its close vty output (e.g. "!" or "end-family"
-		 * etc...).
+		 * We finished processing current list,
+		 * it's time to print the config.
 		 */
-		parent = ly_iter_next_up(child);
-		if (parent != NULL) {
-			nb_node = parent->schema->priv;
-			if (nb_node && nb_node->cbs.cli_show_end)
-				(*nb_node->cbs.cli_show_end)(vty, parent);
+		if (sort_node && nb_node != sort_node) {
+			for (ALL_LIST_ELEMENTS_RO(sort_list, listnode, data))
+				nb_cli_show_dnode_cmds(vty, data,
+						       with_defaults);
+
+			list_delete(&sort_list);
+			sort_node = NULL;
 		}
 
 		/*
-		 * There is a possible path in this macro that ends up
-		 * dereferencing child->parent->parent. We just null checked
-		 * child->parent by checking (ly_iter_next_up(child) != NULL)
-		 * above.
-		 *
-		 * I am not sure whether it is possible for the other
-		 * conditions within this macro guarding the problem
-		 * dereference to be satisfied when child->parent == NULL.
+		 * If the config needs to be sorted,
+		 * then add the dnode to the sorting
+		 * list for later processing.
 		 */
-#ifndef __clang_analyzer__
-		LY_TREE_DFS_END(root, next, child);
-#endif
+		if (nb_node && nb_node->cbs.cli_cmp) {
+			if (!sort_node) {
+				sort_node = nb_node;
+				sort_list = list_new();
+				sort_list->cmp = (int (*)(void *, void *))
+						 nb_node->cbs.cli_cmp;
+			}
+
+			listnode_add_sort(sort_list, child);
+			continue;
+		}
+
+		nb_cli_show_dnode_cmds(vty, child, with_defaults);
 	}
+
+	if (sort_node) {
+		for (ALL_LIST_ELEMENTS_RO(sort_list, listnode, data))
+			nb_cli_show_dnode_cmds(vty, data, with_defaults);
+
+		list_delete(&sort_list);
+		sort_node = NULL;
+	}
+}
+
+void nb_cli_show_dnode_cmds(struct vty *vty, struct lyd_node *root,
+			    bool with_defaults)
+{
+	struct nb_node *nb_node;
+
+	if (!with_defaults && yang_dnode_is_default_recursive(root))
+		return;
+
+	nb_node = root->schema->priv;
+
+	if (nb_node && nb_node->cbs.cli_show)
+		(*nb_node->cbs.cli_show)(vty, root, with_defaults);
+
+	if (!(root->schema->nodetype & (LYS_LEAF | LYS_LEAFLIST | LYS_ANYDATA)))
+		show_dnode_children_cmds(vty, root, with_defaults);
+
+	if (nb_node && nb_node->cbs.cli_show_end)
+		(*nb_node->cbs.cli_show_end)(vty, root);
 }
 
 static void nb_cli_show_config_cmds(struct vty *vty, struct nb_config *config,

lib/routemap.h

@@ -694,6 +694,8 @@ void routemap_hook_context_free(struct routemap_hook_context *rhc);
 extern const struct frr_yang_module_info frr_route_map_info;
 
 /* routemap_cli.c */
+extern int route_map_instance_cmp(struct lyd_node *dnode1,
+				  struct lyd_node *dnode2);
 extern void route_map_instance_show(struct vty *vty, struct lyd_node *dnode,
 				    bool show_defaults);
 extern void route_map_instance_show_end(struct vty *vty,

lib/routemap_cli.c

@@ -113,6 +113,14 @@ DEFPY_YANG(
 	return nb_cli_apply_changes(vty, NULL);
 }
 
+int route_map_instance_cmp(struct lyd_node *dnode1, struct lyd_node *dnode2)
+{
+	uint16_t seq1 = yang_dnode_get_uint16(dnode1, "./sequence");
+	uint16_t seq2 = yang_dnode_get_uint16(dnode2, "./sequence");
+
+	return seq1 - seq2;
+}
+
 void route_map_instance_show(struct vty *vty, struct lyd_node *dnode,
 			     bool show_defaults)
 {

lib/routemap_northbound.c

@@ -1140,6 +1140,7 @@ const struct frr_yang_module_info frr_route_map_info = {
 			.cbs = {
 				.create = lib_route_map_entry_create,
 				.destroy = lib_route_map_entry_destroy,
+				.cli_cmp = route_map_instance_cmp,
 				.cli_show = route_map_instance_show,
 				.cli_show_end = route_map_instance_show_end,
 			}