From 6dfc022ff2a16614541f801e8539c6f785eea1ff Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Wed, 13 Jan 2021 09:02:32 -0800 Subject: 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 Signed-off-by: Chirag Shah --- staticd/static_nb.c | 2 - staticd/static_nb.h | 4 -- staticd/static_nb_config.c | 126 ++++++++++++++++----------------------------- 3 files changed, 43 insertions(+), 89 deletions(-) (limited to 'staticd') diff --git a/staticd/static_nb.c b/staticd/static_nb.c index 51704426f..201002036 100644 --- a/staticd/static_nb.c +++ b/staticd/static_nb.c @@ -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, } }, { diff --git a/staticd/static_nb.h b/staticd/static_nb.h index d145c31f7..0a846ab6f 100644 --- a/staticd/static_nb.h +++ b/staticd/static_nb.h @@ -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( diff --git a/staticd/static_nb_config.c b/staticd/static_nb_config.c index 6e59f50a0..1360660bc 100644 --- a/staticd/static_nb_config.c +++ b/staticd/static_nb_config.c @@ -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); } /* -- cgit v1.2.3