Skip to content

Commit

Permalink
lib: fix nb callbacks for containers inside choice case
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
idryzhov committed Feb 23, 2024
1 parent 9210771 commit 8517891
Show file tree
Hide file tree
Showing 8 changed files with 265 additions and 16 deletions.
6 changes: 6 additions & 0 deletions bgpd/bgp_routemap_nb.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
},
Expand Down Expand Up @@ -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,
}
},
Expand Down Expand Up @@ -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,
}
},
Expand Down
14 changes: 14 additions & 0 deletions bgpd/bgp_routemap_nb.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ 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);
Expand Down Expand Up @@ -127,6 +129,12 @@ 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);
Expand All @@ -138,6 +146,12 @@ 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);
Expand Down
89 changes: 76 additions & 13 deletions bgpd/bgp_routemap_nb_config.c
Original file line number Diff line number Diff line change
Expand Up @@ -1121,6 +1121,28 @@ 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)
Expand Down Expand Up @@ -1219,9 +1241,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;
Expand Down Expand Up @@ -1253,9 +1274,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;
Expand Down Expand Up @@ -1287,9 +1307,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;
Expand Down Expand Up @@ -2735,6 +2754,29 @@ 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)
{
Expand Down Expand Up @@ -2807,9 +2849,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;
Expand Down Expand Up @@ -2842,9 +2883,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;
Expand Down Expand Up @@ -2918,6 +2958,29 @@ 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)
Expand Down Expand Up @@ -2977,7 +3040,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;
}

/*
Expand All @@ -2995,7 +3058,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;
}

/*
Expand Down Expand Up @@ -3065,7 +3128,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;
}

/*
Expand Down
71 changes: 71 additions & 0 deletions lib/filter_nb.c
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,34 @@ 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
*/
Expand Down Expand Up @@ -927,6 +955,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
*/
Expand Down Expand Up @@ -1628,6 +1685,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 = {
Expand All @@ -1654,6 +1718,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 = {
Expand Down
4 changes: 4 additions & 0 deletions lib/northbound.c
Original file line number Diff line number Diff line change
Expand Up @@ -1938,6 +1938,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;
Expand Down Expand Up @@ -1992,6 +1994,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;
Expand Down
16 changes: 16 additions & 0 deletions zebra/zebra_nb.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
{
Expand Down Expand Up @@ -530,13 +532,27 @@ 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 = {
.modify = lib_interface_zebra_evpn_mh_type_0_esi_modify,
.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 = {
Expand Down
Loading

0 comments on commit 8517891

Please sign in to comment.