Merge pull request #15423 from idryzhov/fix-nb-choice

lib: fix processing of choices in northbound

bgpd/bgp_routemap_nb.c

@@ -147,6 +147,8 @@ const struct frr_yang_module_info frr_bgp_route_map_info = {
 		{
 			.xpath = "/frr-route-map:lib/route-map/entry/match-condition/rmap-match-condition/frr-bgp-route-map:comm-list",
 			.cbs = {
+				.create = lib_route_map_entry_match_condition_rmap_match_condition_comm_list_create,
+				.destroy = lib_route_map_entry_match_condition_rmap_match_condition_comm_list_destroy,
 				.apply_finish = lib_route_map_entry_match_condition_rmap_match_condition_comm_list_finish,
 			}
 		},
@@ -356,6 +358,8 @@ const struct frr_yang_module_info frr_bgp_route_map_info = {
 		{
 			.xpath = "/frr-route-map:lib/route-map/entry/set-action/rmap-set-action/frr-bgp-route-map:aggregator",
 			.cbs = {
+				.create = lib_route_map_entry_set_action_rmap_set_action_aggregator_create,
+				.destroy = lib_route_map_entry_set_action_rmap_set_action_aggregator_destroy,
 				.apply_finish = lib_route_map_entry_set_action_rmap_set_action_aggregator_finish,
 			}
 		},
@@ -390,6 +394,8 @@ const struct frr_yang_module_info frr_bgp_route_map_info = {
 		{
 			.xpath = "/frr-route-map:lib/route-map/entry/set-action/rmap-set-action/frr-bgp-route-map:extcommunity-lb",
 			.cbs = {
+				.create = lib_route_map_entry_set_action_rmap_set_action_extcommunity_lb_create,
+				.destroy = lib_route_map_entry_set_action_rmap_set_action_extcommunity_lb_destroy,
 				.apply_finish = lib_route_map_entry_set_action_rmap_set_action_extcommunity_lb_finish,
 			}
 		},

bgpd/bgp_routemap_nb.h

@@ -60,6 +60,10 @@ int lib_route_map_entry_match_condition_rmap_match_condition_evpn_route_type_mod
 int lib_route_map_entry_match_condition_rmap_match_condition_evpn_route_type_destroy(struct nb_cb_destroy_args *args);
 int lib_route_map_entry_match_condition_rmap_match_condition_route_distinguisher_modify(struct nb_cb_modify_args *args);
 int lib_route_map_entry_match_condition_rmap_match_condition_route_distinguisher_destroy(struct nb_cb_destroy_args *args);
+int lib_route_map_entry_match_condition_rmap_match_condition_comm_list_create(
+	struct nb_cb_create_args *args);
+int lib_route_map_entry_match_condition_rmap_match_condition_comm_list_destroy(
+	struct nb_cb_destroy_args *args);
 void lib_route_map_entry_match_condition_rmap_match_condition_comm_list_finish(struct nb_cb_apply_finish_args *args);
 int lib_route_map_entry_match_condition_rmap_match_condition_comm_list_comm_list_name_modify(struct nb_cb_modify_args *args);
 int lib_route_map_entry_match_condition_rmap_match_condition_comm_list_comm_list_name_destroy(struct nb_cb_destroy_args *args);
@@ -127,6 +131,10 @@ int lib_route_map_entry_set_action_rmap_set_action_large_community_none_modify(s
 int lib_route_map_entry_set_action_rmap_set_action_large_community_none_destroy(struct nb_cb_destroy_args *args);
 int lib_route_map_entry_set_action_rmap_set_action_large_community_string_modify(struct nb_cb_modify_args *args);
 int lib_route_map_entry_set_action_rmap_set_action_large_community_string_destroy(struct nb_cb_destroy_args *args);
+int lib_route_map_entry_set_action_rmap_set_action_aggregator_create(
+	struct nb_cb_create_args *args);
+int lib_route_map_entry_set_action_rmap_set_action_aggregator_destroy(
+	struct nb_cb_destroy_args *args);
 void lib_route_map_entry_set_action_rmap_set_action_aggregator_finish(struct nb_cb_apply_finish_args *args);
 int lib_route_map_entry_set_action_rmap_set_action_aggregator_aggregator_asn_modify(struct nb_cb_modify_args *args);
 int lib_route_map_entry_set_action_rmap_set_action_aggregator_aggregator_asn_destroy(struct nb_cb_destroy_args *args);
@@ -138,6 +146,10 @@ int lib_route_map_entry_set_action_rmap_set_action_comm_list_num_extended_modify
 int lib_route_map_entry_set_action_rmap_set_action_comm_list_num_extended_destroy(struct nb_cb_destroy_args *args);
 int lib_route_map_entry_set_action_rmap_set_action_comm_list_name_modify(struct nb_cb_modify_args *args);
 int lib_route_map_entry_set_action_rmap_set_action_comm_list_name_destroy(struct nb_cb_destroy_args *args);
+int lib_route_map_entry_set_action_rmap_set_action_extcommunity_lb_create(
+	struct nb_cb_create_args *args);
+int lib_route_map_entry_set_action_rmap_set_action_extcommunity_lb_destroy(
+	struct nb_cb_destroy_args *args);
 void lib_route_map_entry_set_action_rmap_set_action_extcommunity_lb_finish(struct nb_cb_apply_finish_args *args);
 int lib_route_map_entry_set_action_rmap_set_action_extcommunity_lb_lb_type_modify(struct nb_cb_modify_args *args);
 int lib_route_map_entry_set_action_rmap_set_action_extcommunity_lb_lb_type_destroy(struct nb_cb_destroy_args *args);

bgpd/bgp_routemap_nb_config.c

@@ -1121,6 +1121,27 @@ lib_route_map_entry_match_condition_rmap_match_condition_route_distinguisher_des
 /*
  * XPath = /frr-route-map:lib/route-map/entry/match-condition/rmap-match-condition/frr-bgp-route-map:comm-list
  */
+int lib_route_map_entry_match_condition_rmap_match_condition_comm_list_create(
+	struct nb_cb_create_args *args)
+{
+	return NB_OK;
+}
+
+int lib_route_map_entry_match_condition_rmap_match_condition_comm_list_destroy(
+	struct nb_cb_destroy_args *args)
+{
+	switch (args->event) {
+	case NB_EV_VALIDATE:
+	case NB_EV_PREPARE:
+	case NB_EV_ABORT:
+		break;
+	case NB_EV_APPLY:
+		return lib_route_map_entry_match_destroy(args);
+	}
+
+	return NB_OK;
+}
+
 void
 lib_route_map_entry_match_condition_rmap_match_condition_comm_list_finish(
 	struct nb_cb_apply_finish_args *args)
@@ -1219,9 +1240,8 @@ lib_route_map_entry_match_condition_rmap_match_condition_comm_list_comm_list_nam
 	case NB_EV_VALIDATE:
 	case NB_EV_PREPARE:
 	case NB_EV_ABORT:
-		break;
 	case NB_EV_APPLY:
-		return lib_route_map_entry_match_destroy(args);
+		break;
 	}
 
 	return NB_OK;
@@ -1253,9 +1273,8 @@ int lib_route_map_entry_match_condition_rmap_match_condition_comm_list_comm_list
 	case NB_EV_VALIDATE:
 	case NB_EV_PREPARE:
 	case NB_EV_ABORT:
-		break;
 	case NB_EV_APPLY:
-		return lib_route_map_entry_match_destroy(args);
+		break;
 	}
 
 	return NB_OK;
@@ -1287,9 +1306,8 @@ lib_route_map_entry_match_condition_rmap_match_condition_comm_list_comm_list_nam
 	case NB_EV_VALIDATE:
 	case NB_EV_PREPARE:
 	case NB_EV_ABORT:
-		break;
 	case NB_EV_APPLY:
-		return lib_route_map_entry_match_destroy(args);
+		break;
 	}
 
 	return NB_OK;
@@ -2735,6 +2753,27 @@ lib_route_map_entry_set_action_rmap_set_action_large_community_string_destroy(
  * xpath =
  * /frr-route-map:lib/route-map/entry/set-action/rmap-set-action/frr-bgp-route-map:aggregator
  */
+int lib_route_map_entry_set_action_rmap_set_action_aggregator_create(
+	struct nb_cb_create_args *args)
+{
+	return NB_OK;
+}
+
+int lib_route_map_entry_set_action_rmap_set_action_aggregator_destroy(
+	struct nb_cb_destroy_args *args)
+{
+	switch (args->event) {
+	case NB_EV_VALIDATE:
+	case NB_EV_PREPARE:
+	case NB_EV_ABORT:
+		break;
+	case NB_EV_APPLY:
+		return lib_route_map_entry_set_destroy(args);
+	}
+
+	return NB_OK;
+}
+
 void lib_route_map_entry_set_action_rmap_set_action_aggregator_finish(
 	struct nb_cb_apply_finish_args *args)
 {
@@ -2807,9 +2846,8 @@ lib_route_map_entry_set_action_rmap_set_action_aggregator_aggregator_asn_destroy
 	case NB_EV_VALIDATE:
 	case NB_EV_PREPARE:
 	case NB_EV_ABORT:
-		break;
 	case NB_EV_APPLY:
-		return lib_route_map_entry_set_destroy(args);
+		break;
 	}
 
 	return NB_OK;
@@ -2842,9 +2880,8 @@ lib_route_map_entry_set_action_rmap_set_action_aggregator_aggregator_address_des
 	case NB_EV_VALIDATE:
 	case NB_EV_PREPARE:
 	case NB_EV_ABORT:
-		break;
 	case NB_EV_APPLY:
-		return lib_route_map_entry_set_destroy(args);
+		break;
 	}
 
 	return NB_OK;
@@ -2918,6 +2955,27 @@ lib_route_map_entry_set_action_rmap_set_action_comm_list_name_destroy(
  * XPath:
  * /frr-route-map:lib/route-map/entry/set-action/rmap-set-action/frr-bgp-route-map:extcommunity-lb
  */
+int lib_route_map_entry_set_action_rmap_set_action_extcommunity_lb_create(
+	struct nb_cb_create_args *args)
+{
+	return NB_OK;
+}
+
+int lib_route_map_entry_set_action_rmap_set_action_extcommunity_lb_destroy(
+	struct nb_cb_destroy_args *args)
+{
+	switch (args->event) {
+	case NB_EV_VALIDATE:
+	case NB_EV_PREPARE:
+	case NB_EV_ABORT:
+		break;
+	case NB_EV_APPLY:
+		return lib_route_map_entry_set_destroy(args);
+	}
+
+	return NB_OK;
+}
+
 void
 lib_route_map_entry_set_action_rmap_set_action_extcommunity_lb_finish(
 	struct nb_cb_apply_finish_args *args)
@@ -2977,7 +3035,7 @@ int
 lib_route_map_entry_set_action_rmap_set_action_extcommunity_lb_lb_type_destroy(
 		struct nb_cb_destroy_args *args)
 {
-	return lib_route_map_entry_set_destroy(args);
+	return NB_OK;
 }
 
 /*
@@ -2995,7 +3053,7 @@ int
 lib_route_map_entry_set_action_rmap_set_action_extcommunity_lb_bandwidth_destroy(
 		struct nb_cb_destroy_args *args)
 {
-	return lib_route_map_entry_set_destroy(args);
+	return NB_OK;
 }
 
 /*
@@ -3065,7 +3123,7 @@ int
 lib_route_map_entry_set_action_rmap_set_action_extcommunity_lb_two_octet_as_specific_destroy(
 	struct nb_cb_destroy_args *args)
 {
-	return lib_route_map_entry_set_destroy(args);
+	return NB_OK;
 }
 
 /*

lib/filter_nb.c

@@ -756,6 +756,32 @@ lib_access_list_entry_host_destroy(struct nb_cb_destroy_args *args)
 	return NB_OK;
 }
 
+/*
+ * XPath: /frr-filter:lib/access-list/entry/network
+ */
+static int lib_access_list_entry_network_create(struct nb_cb_create_args *args)
+{
+	/* Nothing to do here, everything is done in children callbacks */
+	return NB_OK;
+}
+
+static int lib_access_list_entry_network_destroy(struct nb_cb_destroy_args *args)
+{
+	struct filter_cisco *fc;
+	struct filter *f;
+
+	if (args->event != NB_EV_APPLY)
+		return NB_OK;
+
+	f = nb_running_get_entry(args->dnode, NULL, true);
+	fc = &f->u.cfilter;
+	cisco_unset_addr_mask(&fc->addr, &fc->addr_mask);
+
+	acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_DELETED);
+
+	return NB_OK;
+}
+
 /*
  * XPath: /frr-filter:lib/access-list/entry/network/address
  */
@@ -927,6 +953,35 @@ static int lib_access_list_entry_destination_host_destroy(
 	return NB_OK;
 }
 
+/*
+ * XPath: /frr-filter:lib/access-list/entry/destination-network
+ */
+static int
+lib_access_list_entry_destination_network_create(struct nb_cb_create_args *args)
+{
+	/* Nothing to do here, everything is done in children callbacks */
+	return NB_OK;
+}
+
+static int lib_access_list_entry_destination_network_destroy(
+	struct nb_cb_destroy_args *args)
+{
+	struct filter_cisco *fc;
+	struct filter *f;
+
+	if (args->event != NB_EV_APPLY)
+		return NB_OK;
+
+	f = nb_running_get_entry(args->dnode, NULL, true);
+	fc = &f->u.cfilter;
+	fc->extended = 0;
+	cisco_unset_addr_mask(&fc->mask, &fc->mask_mask);
+
+	acl_notify_route_map(f->acl, RMAP_EVENT_FILTER_DELETED);
+
+	return NB_OK;
+}
+
 /*
  * XPath: /frr-filter:lib/access-list/entry/destination-network/address
  */
@@ -1628,6 +1683,13 @@ const struct frr_yang_module_info frr_filter_info = {
 				.destroy = lib_access_list_entry_host_destroy,
 			}
 		},
+		{
+			.xpath = "/frr-filter:lib/access-list/entry/network",
+			.cbs = {
+				.create = lib_access_list_entry_network_create,
+				.destroy = lib_access_list_entry_network_destroy,
+			}
+		},
 		{
 			.xpath = "/frr-filter:lib/access-list/entry/network/address",
 			.cbs = {
@@ -1654,6 +1716,13 @@ const struct frr_yang_module_info frr_filter_info = {
 				.destroy = lib_access_list_entry_destination_host_destroy,
 			}
 		},
+		{
+			.xpath = "/frr-filter:lib/access-list/entry/destination-network",
+			.cbs = {
+				.create = lib_access_list_entry_destination_network_create,
+				.destroy = lib_access_list_entry_destination_network_destroy,
+			}
+		},
 		{
 			.xpath = "/frr-filter:lib/access-list/entry/destination-network/address",
 			.cbs = {

lib/northbound.c

@@ -391,14 +391,27 @@ void nb_config_replace(struct nb_config *config_dst,
 static inline int nb_config_cb_compare(const struct nb_config_cb *a,
 				       const struct nb_config_cb *b)
 {
+	bool a_destroy = a->operation == NB_CB_DESTROY;
+	bool b_destroy = b->operation == NB_CB_DESTROY;
+
+	/*
+	 * Sort by operation first. All "destroys" must come first, to correctly
+	 * process the change of a "case" inside a "choice". The old "case" must
+	 * be deleted before the new "case" is created.
+	 */
+	if (a_destroy && !b_destroy)
+		return -1;
+	if (!a_destroy && b_destroy)
+		return 1;
+
 	/*
-	 * Sort by priority first. If the operation is "destroy", reverse the
-	 * order, so that the dependencies are destroyed before the dependants.
+	 * Then sort by priority. If the operation is "destroy", reverse the
+	 * order, so that the dependants are deleted before the dependencies.
 	 */
 	if (a->nb_node->priority < b->nb_node->priority)
-		return a->operation != NB_CB_DESTROY ? -1 : 1;
+		return !a_destroy ? -1 : 1;
 	if (a->nb_node->priority > b->nb_node->priority)
-		return a->operation != NB_CB_DESTROY ? 1 : -1;
+		return !a_destroy ? 1 : -1;
 
 	/*
 	 * Preserve the order of the configuration changes as told by libyang.
@@ -1814,6 +1827,7 @@ nb_apply_finish_cb_new(struct nb_config_cbs *cbs, const struct nb_node *nb_node,
 	struct nb_config_cb *cb;
 
 	cb = XCALLOC(MTYPE_TMP, sizeof(*cb));
+	cb->operation = NB_CB_APPLY_FINISH;
 	cb->nb_node = nb_node;
 	cb->dnode = dnode;
 	RB_INSERT(nb_config_cbs, cbs, cb);
@@ -1828,6 +1842,7 @@ nb_apply_finish_cb_find(struct nb_config_cbs *cbs,
 {
 	struct nb_config_cb s;
 
+	s.operation = NB_CB_APPLY_FINISH;
 	s.seq = 0;
 	s.nb_node = nb_node;
 	s.dnode = dnode;
@@ -1925,6 +1940,8 @@ bool nb_cb_operation_is_valid(enum nb_cb_operation operation,
 				return false;
 			break;
 		case LYS_CONTAINER:
+			if (snode->parent && snode->parent->nodetype == LYS_CASE)
+				return true;
 			scontainer = (struct lysc_node_container *)snode;
 			if (!CHECK_FLAG(scontainer->flags, LYS_PRESENCE))
 				return false;
@@ -1979,6 +1996,8 @@ bool nb_cb_operation_is_valid(enum nb_cb_operation operation,
 				return false;
 			break;
 		case LYS_CONTAINER:
+			if (snode->parent && snode->parent->nodetype == LYS_CASE)
+				return true;
 			scontainer = (struct lysc_node_container *)snode;
 			if (!CHECK_FLAG(scontainer->flags, LYS_PRESENCE))
 				return false;