staticd: handle when condition check in nb callbacks

At present, libyang validate api takes longer time to complete
for a transaction to complete if the same config is re-applied.
For instance if set of static routes are reapplied the config
completion takes longer than it took initial time.

One of the solution is to remove when statement from staticd nexthop yang OM.
When condition adds peformance toll on libyang's validate api.
The same when condition checks are done in frr northbound
validation phase (which are must faster).

With this change, if the same static routes are configured
agian and again, the time to completion does not go up and
perfomance does not degrade.

Ticket:CM-32530
Testing Done:

Configure 400 static routes across two vrfs and keep re-applying them.
The time to complete the config remains in few seconds.

Before:
root@bharat:~/stash/frr4# time vtysh -f static_route_cfg

real    0m19.877s
user    0m0.263s
sys 0m0.014s

After:
root@bharat:~/stash/frr4# time vtysh -f static_route_cfg

real    0m3.857s
user    0m0.239s
sys 0m0.034s

Co-developed-by: VishalDhingra <vdhingra@vmware.com>
Signed-off-by: Chirag Shah <chirag@nvidia.com>
This commit is contained in:
Chirag Shah 2021-01-13 09:02:32 -08:00
parent dc40d33703
commit 6dfc022ff2
3 changed files with 43 additions and 89 deletions

View File

@ -72,7 +72,6 @@ const struct frr_yang_module_info frr_staticd_info = {
.xpath = "/frr-routing:routing/control-plane-protocols/control-plane-protocol/frr-staticd:staticd/route-list/path-list/frr-nexthops/nexthop/onlink",
.cbs = {
.modify = routing_control_plane_protocols_control_plane_protocol_staticd_route_list_path_list_frr_nexthops_nexthop_onlink_modify,
.destroy = routing_control_plane_protocols_control_plane_protocol_staticd_route_list_path_list_frr_nexthops_nexthop_onlink_destroy,
}
},
{
@ -157,7 +156,6 @@ const struct frr_yang_module_info frr_staticd_info = {
.xpath = "/frr-routing:routing/control-plane-protocols/control-plane-protocol/frr-staticd:staticd/route-list/src-list/path-list/frr-nexthops/nexthop/onlink",
.cbs = {
.modify = routing_control_plane_protocols_control_plane_protocol_staticd_route_list_src_list_path_list_frr_nexthops_nexthop_onlink_modify,
.destroy = routing_control_plane_protocols_control_plane_protocol_staticd_route_list_src_list_path_list_frr_nexthops_nexthop_onlink_destroy,
}
},
{

View File

@ -43,8 +43,6 @@ int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_pa
struct nb_cb_destroy_args *args);
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_path_list_frr_nexthops_nexthop_onlink_modify(
struct nb_cb_modify_args *args);
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_path_list_frr_nexthops_nexthop_onlink_destroy(
struct nb_cb_destroy_args *args);
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_path_list_frr_nexthops_nexthop_color_modify(
struct nb_cb_modify_args *args);
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_path_list_frr_nexthops_nexthop_color_destroy(
@ -87,8 +85,6 @@ int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_sr
struct nb_cb_destroy_args *args);
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_src_list_path_list_frr_nexthops_nexthop_onlink_modify(
struct nb_cb_modify_args *args);
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_src_list_path_list_frr_nexthops_nexthop_onlink_destroy(
struct nb_cb_destroy_args *args);
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_src_list_path_list_frr_nexthops_nexthop_color_modify(
struct nb_cb_modify_args *args);
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_src_list_path_list_frr_nexthops_nexthop_color_destroy(

View File

@ -302,9 +302,27 @@ static int static_nexthop_mpls_label_modify(struct nb_cb_modify_args *args)
static int static_nexthop_onlink_modify(struct nb_cb_modify_args *args)
{
struct static_nexthop *nh;
static_types nh_type;
nh = nb_running_get_entry(args->dnode, NULL, true);
nh->onlink = yang_dnode_get_bool(args->dnode, NULL);
switch (args->event) {
case NB_EV_VALIDATE:
nh_type = yang_dnode_get_enum(args->dnode, "../nh-type");
if ((nh_type != STATIC_IPV4_GATEWAY_IFNAME)
&& (nh_type != STATIC_IPV6_GATEWAY_IFNAME)) {
snprintf(
args->errmsg, args->errmsg_len,
"nexthop type is not the ipv4 or ipv6 interface type");
return NB_ERR_VALIDATION;
}
break;
case NB_EV_PREPARE:
case NB_EV_ABORT:
break;
case NB_EV_APPLY:
nh = nb_running_get_entry(args->dnode, NULL, true);
nh->onlink = yang_dnode_get_bool(args->dnode, NULL);
break;
}
return NB_OK;
}
@ -332,9 +350,25 @@ static int static_nexthop_color_destroy(struct nb_cb_destroy_args *args)
static int static_nexthop_bh_type_modify(struct nb_cb_modify_args *args)
{
struct static_nexthop *nh;
static_types nh_type;
nh = nb_running_get_entry(args->dnode, NULL, true);
nh->bh_type = yang_dnode_get_enum(args->dnode, NULL);
switch (args->event) {
case NB_EV_VALIDATE:
nh_type = yang_dnode_get_enum(args->dnode, "../nh-type");
if (nh_type != STATIC_BLACKHOLE) {
snprintf(args->errmsg, args->errmsg_len,
"nexthop type is not the blackhole type");
return NB_ERR_VALIDATION;
}
break;
case NB_EV_PREPARE:
case NB_EV_ABORT:
break;
case NB_EV_APPLY:
nh = nb_running_get_entry(args->dnode, NULL, true);
nh->bh_type = yang_dnode_get_enum(args->dnode, NULL);
break;
}
return NB_OK;
}
@ -636,7 +670,7 @@ int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_pa
info = route_table_get_info(rn->table);
if (static_nexthop_create(args, rn_dnode, info) != NB_OK)
return NB_ERR_VALIDATION;
return NB_ERR_INCONSISTENCY;
break;
}
return NB_OK;
@ -673,17 +707,7 @@ int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_pa
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_path_list_frr_nexthops_nexthop_bh_type_modify(
struct nb_cb_modify_args *args)
{
switch (args->event) {
case NB_EV_VALIDATE:
case NB_EV_PREPARE:
case NB_EV_ABORT:
break;
case NB_EV_APPLY:
if (static_nexthop_bh_type_modify(args) != NB_OK)
return NB_ERR;
break;
}
return NB_OK;
return static_nexthop_bh_type_modify(args);
}
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_path_list_frr_nexthops_nexthop_bh_type_destroy(
@ -709,33 +733,7 @@ int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_pa
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_path_list_frr_nexthops_nexthop_onlink_modify(
struct nb_cb_modify_args *args)
{
switch (args->event) {
case NB_EV_VALIDATE:
case NB_EV_PREPARE:
case NB_EV_ABORT:
break;
case NB_EV_APPLY:
if (static_nexthop_onlink_modify(args) != NB_OK)
return NB_ERR;
break;
}
return NB_OK;
}
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_path_list_frr_nexthops_nexthop_onlink_destroy(
struct nb_cb_destroy_args *args)
{
/* onlink has a boolean type with default value,
* so no need to do any operations in destroy callback
*/
switch (args->event) {
case NB_EV_VALIDATE:
case NB_EV_PREPARE:
case NB_EV_ABORT:
case NB_EV_APPLY:
break;
}
return NB_OK;
return static_nexthop_onlink_modify(args);
}
/*
@ -1124,17 +1122,7 @@ int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_sr
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_src_list_path_list_frr_nexthops_nexthop_bh_type_modify(
struct nb_cb_modify_args *args)
{
switch (args->event) {
case NB_EV_VALIDATE:
case NB_EV_PREPARE:
case NB_EV_ABORT:
break;
case NB_EV_APPLY:
if (static_nexthop_bh_type_modify(args) != NB_OK)
return NB_ERR;
break;
}
return NB_OK;
return static_nexthop_bh_type_modify(args);
}
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_src_list_path_list_frr_nexthops_nexthop_bh_type_destroy(
@ -1161,35 +1149,7 @@ int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_sr
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_src_list_path_list_frr_nexthops_nexthop_onlink_modify(
struct nb_cb_modify_args *args)
{
switch (args->event) {
case NB_EV_VALIDATE:
case NB_EV_PREPARE:
case NB_EV_ABORT:
break;
case NB_EV_APPLY:
if (static_nexthop_onlink_modify(args) != NB_OK)
return NB_ERR;
break;
}
return NB_OK;
}
int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_src_list_path_list_frr_nexthops_nexthop_onlink_destroy(
struct nb_cb_destroy_args *args)
{
/* onlink has a boolean type with default value,
* so no need to do any operations in destroy callback
*/
switch (args->event) {
case NB_EV_VALIDATE:
case NB_EV_PREPARE:
case NB_EV_ABORT:
case NB_EV_APPLY:
break;
}
return NB_OK;
return static_nexthop_onlink_modify(args);
}
/*