From 38b85e0c2bc555b8827dbd2cb6515b6febf548b4 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Fri, 23 Feb 2024 21:14:26 +0200 Subject: [PATCH 1/4] lib: fix order of northbound operations When ordering operations, destroys must always come before other operations, to correctly cover the change of a "case" in a "choice". The problem can be reproduced with the following commands: ``` access-list test seq 1 permit 10.0.0.0/8 access-list test seq 1 permit host 10.0.0.1 access-list test seq 1 permit 10.0.0.0/8 ``` Before this commit, the order of changes would be the following: - `access-list test seq 1 permit 10.0.0.0/8` - `modify` for `ipv4-prefix` - `access-list test seq 1 permit host 10.0.0.1` - `destroy` for `ipv4-prefix` - `modify` for `host` - `access-list test seq 1 permit 10.0.0.0/8` - `modify` for `ipv4-prefix` - `destroy` for `host` As `destroy` for `host` is called last, it rewrites the fields that were filled by `modify` callback of `ipv4-prefix`. This commit fixes this problem by always calling `destroy` callbacks first. Signed-off-by: Igor Ryzhov --- lib/northbound.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/lib/northbound.c b/lib/northbound.c index 6b31b818c515..8d9302a816e4 100644 --- a/lib/northbound.c +++ b/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; From 0b905f740459291bca12e552aea4975fb46e9d98 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Sat, 24 Feb 2024 00:06:41 +0200 Subject: [PATCH 2/4] lib: fix nb callbacks for containers inside choice case Containers inside a choice's case must be treated as presence containers as they can be explicitly created and deleted. They must have `create` and `destroy` callbacks, otherwise the internal data they represent may never be deleted. The issue can be reproduced with the following steps: - create an access-list with destination-network params ``` # access-list test seq 1 permit ip any 10.10.10.0 0.0.0.255 ``` - delete the `destination-network` container ``` # mgmt delete-config /frr-filter:lib/access-list[name='test'][type='ipv4']/entry[sequence='1']/destination-network # mgmt commit apply MGMTD: No changes found to be committed! ``` As the `destination-network` container is non-presence, and all its leafs are mandatory, mgmtd doesn't see any changes to be commited and simply updates its YANG data tree without passing any updates to backend daemons. This commit fixes the issue by requiring `create` and `destroy` callbacks for containers inside choice's cases. Signed-off-by: Igor Ryzhov --- bgpd/bgp_routemap_nb.c | 6 +++ bgpd/bgp_routemap_nb.h | 12 +++++ bgpd/bgp_routemap_nb_config.c | 84 +++++++++++++++++++++++++++++------ lib/filter_nb.c | 69 ++++++++++++++++++++++++++++ lib/northbound.c | 4 ++ zebra/zebra_nb.c | 16 +++++++ zebra/zebra_nb.h | 6 +++ zebra/zebra_nb_config.c | 75 +++++++++++++++++++++++++++++-- 8 files changed, 256 insertions(+), 16 deletions(-) diff --git a/bgpd/bgp_routemap_nb.c b/bgpd/bgp_routemap_nb.c index abebfe5155c1..2320d2f9084e 100644 --- a/bgpd/bgp_routemap_nb.c +++ b/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, } }, diff --git a/bgpd/bgp_routemap_nb.h b/bgpd/bgp_routemap_nb.h index 28e4188026c4..00c1bde62ab4 100644 --- a/bgpd/bgp_routemap_nb.h +++ b/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); diff --git a/bgpd/bgp_routemap_nb_config.c b/bgpd/bgp_routemap_nb_config.c index d6fcb6b8dcec..e7b4fa6a7ebf 100644 --- a/bgpd/bgp_routemap_nb_config.c +++ b/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; } /* diff --git a/lib/filter_nb.c b/lib/filter_nb.c index eba4e421c050..ec31f6d39558 100644 --- a/lib/filter_nb.c +++ b/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 = { diff --git a/lib/northbound.c b/lib/northbound.c index 8d9302a816e4..f9f9449ea66d 100644 --- a/lib/northbound.c +++ b/lib/northbound.c @@ -1940,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; @@ -1994,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; diff --git a/zebra/zebra_nb.c b/zebra/zebra_nb.c index 7e1c43c204d7..e1ca5ec19b5b 100644 --- a/zebra/zebra_nb.c +++ b/zebra/zebra_nb.c @@ -456,6 +456,8 @@ const struct frr_yang_module_info frr_zebra_info = { { .xpath = "/frr-interface:lib/interface/frr-zebra:zebra/link-params/affinities", .cbs = { + .create = lib_interface_zebra_affinities_create, + .destroy = lib_interface_zebra_affinities_destroy, }, }, { @@ -530,6 +532,13 @@ const struct frr_yang_module_info frr_zebra_info = { .destroy = lib_interface_zebra_link_params_packet_loss_destroy, } }, + { + .xpath = "/frr-interface:lib/interface/frr-zebra:zebra/evpn-mh/type-0", + .cbs = { + .create = lib_interface_zebra_evpn_mh_type_0_create, + .destroy = lib_interface_zebra_evpn_mh_type_0_destroy, + } + }, { .xpath = "/frr-interface:lib/interface/frr-zebra:zebra/evpn-mh/type-0/esi", .cbs = { @@ -537,6 +546,13 @@ const struct frr_yang_module_info frr_zebra_info = { .destroy = lib_interface_zebra_evpn_mh_type_0_esi_destroy, } }, + { + .xpath = "/frr-interface:lib/interface/frr-zebra:zebra/evpn-mh/type-3", + .cbs = { + .create = lib_interface_zebra_evpn_mh_type_3_create, + .destroy = lib_interface_zebra_evpn_mh_type_3_destroy, + } + }, { .xpath = "/frr-interface:lib/interface/frr-zebra:zebra/evpn-mh/type-3/system-mac", .cbs = { diff --git a/zebra/zebra_nb.h b/zebra/zebra_nb.h index 97979ef9623b..d7cf5f404025 100644 --- a/zebra/zebra_nb.h +++ b/zebra/zebra_nb.h @@ -145,6 +145,8 @@ int lib_interface_zebra_link_params_utilized_bandwidth_destroy( int lib_interface_zebra_legacy_admin_group_modify(struct nb_cb_modify_args *args); int lib_interface_zebra_legacy_admin_group_destroy( struct nb_cb_destroy_args *args); +int lib_interface_zebra_affinities_create(struct nb_cb_create_args *args); +int lib_interface_zebra_affinities_destroy(struct nb_cb_destroy_args *args); int lib_interface_zebra_affinity_create(struct nb_cb_create_args *args); int lib_interface_zebra_affinity_destroy(struct nb_cb_destroy_args *args); int lib_interface_zebra_affinity_mode_modify(struct nb_cb_modify_args *args); @@ -175,9 +177,13 @@ int lib_interface_zebra_link_params_packet_loss_modify( struct nb_cb_modify_args *args); int lib_interface_zebra_link_params_packet_loss_destroy( struct nb_cb_destroy_args *args); +int lib_interface_zebra_evpn_mh_type_0_create(struct nb_cb_create_args *args); +int lib_interface_zebra_evpn_mh_type_0_destroy(struct nb_cb_destroy_args *args); int lib_interface_zebra_evpn_mh_type_0_esi_modify(struct nb_cb_modify_args *args); int lib_interface_zebra_evpn_mh_type_0_esi_destroy( struct nb_cb_destroy_args *args); +int lib_interface_zebra_evpn_mh_type_3_create(struct nb_cb_create_args *args); +int lib_interface_zebra_evpn_mh_type_3_destroy(struct nb_cb_destroy_args *args); int lib_interface_zebra_evpn_mh_type_3_system_mac_modify( struct nb_cb_modify_args *args); int lib_interface_zebra_evpn_mh_type_3_system_mac_destroy( diff --git a/zebra/zebra_nb_config.c b/zebra/zebra_nb_config.c index 46c95e6c0f3f..5cb9985ee4b5 100644 --- a/zebra/zebra_nb_config.c +++ b/zebra/zebra_nb_config.c @@ -1746,9 +1746,6 @@ int lib_interface_zebra_legacy_admin_group_modify( iflp->admin_grp = admin_group_value; SET_PARAM(iflp, LP_ADM_GRP); - - admin_group_clear(&iflp->ext_admin_grp); - UNSET_PARAM(iflp, LP_EXTEND_ADM_GRP); break; } return NB_OK; @@ -1776,6 +1773,35 @@ int lib_interface_zebra_legacy_admin_group_destroy( return NB_OK; } +/* + * XPath: + * /frr-interface:lib/interface/frr-zebra:zebra/link-params/affinities + */ +int lib_interface_zebra_affinities_create(struct nb_cb_create_args *args) +{ + return NB_OK; +} + +int lib_interface_zebra_affinities_destroy(struct nb_cb_destroy_args *args) +{ + struct interface *ifp; + struct if_link_params *iflp; + + if (args->event != NB_EV_APPLY) + return NB_OK; + + ifp = nb_running_get_entry(args->dnode, NULL, true); + iflp = if_link_params_get(ifp); + + iflp->admin_grp = 0; + UNSET_PARAM(iflp, LP_ADM_GRP); + + admin_group_clear(&iflp->ext_admin_grp); + UNSET_PARAM(iflp, LP_EXTEND_ADM_GRP); + + return NB_OK; +} + /* * XPath: * /frr-interface:lib/interface/frr-zebra:zebra/link-params/affinities/affinity @@ -2281,6 +2307,27 @@ static bool esi_unique(struct lyd_node *dnode) return true; } +/* + * XPath: /frr-interface:lib/interface/frr-zebra:zebra/evpn-mh/type-0 + */ +int lib_interface_zebra_evpn_mh_type_0_create(struct nb_cb_create_args *args) +{ + return NB_OK; +} + +int lib_interface_zebra_evpn_mh_type_0_destroy(struct nb_cb_destroy_args *args) +{ + struct interface *ifp; + + if (args->event != NB_EV_APPLY) + return NB_OK; + + ifp = nb_running_get_entry(args->dnode, NULL, true); + zebra_evpn_es_type0_esi_update(ifp->info, NULL); + + return NB_OK; +} + /* * XPath: /frr-interface:lib/interface/frr-zebra:zebra/evpn-mh/type-0/esi */ @@ -2324,6 +2371,28 @@ int lib_interface_zebra_evpn_mh_type_0_esi_destroy(struct nb_cb_destroy_args *ar return NB_OK; } +/* + * XPath: /frr-interface:lib/interface/frr-zebra:zebra/evpn-mh/type-3 + */ +int lib_interface_zebra_evpn_mh_type_3_create(struct nb_cb_create_args *args) +{ + return NB_OK; +} + +int lib_interface_zebra_evpn_mh_type_3_destroy(struct nb_cb_destroy_args *args) +{ + struct interface *ifp; + + if (args->event != NB_EV_APPLY) + return NB_OK; + + ifp = nb_running_get_entry(args->dnode, NULL, true); + zebra_evpn_es_sys_mac_update(ifp->info, NULL); + zebra_evpn_es_lid_update(ifp->info, 0); + + return NB_OK; +} + /* * XPath: /frr-interface:lib/interface/frr-zebra:zebra/evpn-mh/type-3/system-mac */ From a37f5f9153b0652f0f551d2e36fbfea8dab8c020 Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Sat, 24 Feb 2024 21:39:30 -0500 Subject: [PATCH 3/4] tests: add test for northbound ordering fix Signed-off-by: Christian Hopps --- tests/topotests/nb_config/r1/frr.conf | 6 ++ tests/topotests/nb_config/test_nb_config.py | 69 +++++++++++++++++++++ 2 files changed, 75 insertions(+) create mode 100644 tests/topotests/nb_config/r1/frr.conf create mode 100644 tests/topotests/nb_config/test_nb_config.py diff --git a/tests/topotests/nb_config/r1/frr.conf b/tests/topotests/nb_config/r1/frr.conf new file mode 100644 index 000000000000..677ec0b86d55 --- /dev/null +++ b/tests/topotests/nb_config/r1/frr.conf @@ -0,0 +1,6 @@ +log timestamp precision 6 +log file frr.log + +interface r1-eth0 + ip address 1.1.1.1/24 +exit diff --git a/tests/topotests/nb_config/test_nb_config.py b/tests/topotests/nb_config/test_nb_config.py new file mode 100644 index 000000000000..f699a4e20e1b --- /dev/null +++ b/tests/topotests/nb_config/test_nb_config.py @@ -0,0 +1,69 @@ +# -*- coding: utf-8 eval: (blacken-mode 1) -*- +# SPDX-License-Identifier: ISC +# +# February 24 2024, Christian Hopps +# +# Copyright (c) 2024, LabN Consulting, L.L.C. +# +""" +Test Northbound Config Operations +""" +import json +import os + +import pytest +from lib.topogen import Topogen +from lib.topotest import json_cmp + +pytestmark = [pytest.mark.mgmtd] + +CWD = os.path.dirname(os.path.realpath(__file__)) + + +@pytest.fixture(scope="module") +def tgen(request): + "Setup/Teardown the environment and provide tgen argument to tests" + + topodef = { + "s1": ("r1",) + } + + tgen = Topogen(topodef, request.module.__name__) + tgen.start_topology() + + router_list = tgen.routers() + for rname, router in router_list.items(): + router.load_frr_config("frr.conf") + + tgen.start_router() + yield tgen + tgen.stop_topology() + + +def test_access_list_config_ordering(tgen): + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + + output = r1.vtysh_multicmd([ + "conf t", + "access-list test seq 1 permit host 10.0.0.1"]) + output = r1.vtysh_cmd("show ip access-list test json") + got = json.loads(output) + expected = json.loads('{"ZEBRA":{"test":{"type":"Standard", "addressFamily":"IPv4", "rules":[{"sequenceNumber":1, "filterType":"permit", "address":"10.0.0.1", "mask":"0.0.0.0"}]}}}') + result = json_cmp(got, expected) + assert result is None + + # + # If the northbound mis-orders the create/delete then this test fails. + # https://github.com/FRRouting/frr/pull/15423/commits/38b85e0c2bc555b8827dbd2cb6515b6febf548b4 + # + output = r1.vtysh_multicmd([ + "conf t", + "access-list test seq 1 permit 10.0.0.0/8"]) + output = r1.vtysh_cmd("show ip access-list test json") + got = json.loads(output) + expected = json.loads('{"ZEBRA":{"test":{"type":"Zebra", "addressFamily":"IPv4", "rules":[{"sequenceNumber":1, "filterType":"permit", "prefix":"10.0.0.0/8", "exact-match":false}]}}}') + result = json_cmp(got, expected) + assert result is None From e5acf6797cbeb3c68946c3a7f65d4d8b433d85f5 Mon Sep 17 00:00:00 2001 From: Christian Hopps Date: Sat, 24 Feb 2024 23:24:05 -0500 Subject: [PATCH 4/4] tests: add test for required choice/np-container callbacks Signed-off-by: Christian Hopps --- .../topotests/mgmt_config/test_regression.py | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/topotests/mgmt_config/test_regression.py b/tests/topotests/mgmt_config/test_regression.py index 70f38d2ec765..928151a23afa 100644 --- a/tests/topotests/mgmt_config/test_regression.py +++ b/tests/topotests/mgmt_config/test_regression.py @@ -51,3 +51,23 @@ def test_regression_issue_13920(tgen): ) output = r1.net.checkRouterCores() assert not output.strip() + + +def test_regression_pullreq_15423(tgen): + r1 = tgen.gears["r1"] + r1.vtysh_multicmd( + """ + conf t + access-list test seq 1 permit ip any 10.10.10.0 0.0.0.255 + """ + ) + + output = r1.vtysh_multicmd( + """ + conf terminal file-lock + mgmt delete-config /frr-filter:lib/access-list[name='test'][type='ipv4']/entry[sequence='1']/destination-network + mgmt commit apply + end + """ + ) + assert "No changes found" not in output