Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: fix processing of choices in northbound (backport #15423) #15425

Merged
merged 4 commits into from
Feb 26, 2024
Merged

lib: fix processing of choices in northbound (backport #15423) #15425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f9b3088
lib: fix order of northbound operations
idryzhov Feb 23, 2024
2ca3dc7
lib: fix nb callbacks for containers inside choice case
idryzhov Feb 23, 2024
5b499a6
tests: add test for northbound ordering fix
choppsv1 Feb 25, 2024
6a921b2
tests: add test for required choice/np-container callbacks
choppsv1 Feb 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions bgpd/bgp_routemap_nb.c
View file
Open in desktop
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
12 changes: 12 additions & 0 deletions bgpd/bgp_routemap_nb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
84 changes: 71 additions & 13 deletions bgpd/bgp_routemap_nb_config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}

/*
Expand All @@ -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;
}

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

/*
Expand Down
69 changes: 69 additions & 0 deletions lib/filter_nb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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
*/
Expand Down Expand Up @@ -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 = {
Expand All @@ -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 = {
Expand Down
27 changes: 23 additions & 4 deletions lib/northbound.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Loading
Loading