From a687b6b27bb2b5c785cdf5d897cd450412b1642d Mon Sep 17 00:00:00 2001 From: vdhingra Date: Fri, 15 Jan 2021 10:43:28 -0800 Subject: [PATCH] staticd: Backend cofiguration code to fix table-id problem problem: table-id gets overwritten for a given route. RCA: table-id was getting overwritten from the NB layer, So route was getting installed with the latest table-id. Fix: make the table-id as the key in the NB layer. This will program the route in zebra correctly. - Removed the table-id modify callbacks. - Moved the validate and apply table-id changes to path-list creation issue #7347 Signed-off-by: vishaldhingra --- staticd/static_nb.h | 6 +- staticd/static_nb_config.c | 130 +++++++------------------------------ staticd/static_routes.c | 4 +- staticd/static_routes.h | 2 +- staticd/static_vty.c | 19 ++---- 5 files changed, 36 insertions(+), 125 deletions(-) diff --git a/staticd/static_nb.h b/staticd/static_nb.h index 8bfb64fec7..b293224dd1 100644 --- a/staticd/static_nb.h +++ b/staticd/static_nb.h @@ -130,13 +130,11 @@ int routing_control_plane_protocols_name_validate( "/frr-routing:routing/control-plane-protocols/" \ "control-plane-protocol[type='%s'][name='%s'][vrf='%s']/" \ "frr-staticd:staticd/route-list[prefix='%s'][afi-safi='%s']/" \ - "path-list[distance='%u']" + "path-list[table-id='%u'][distance='%u']" #define FRR_STATIC_ROUTE_PATH_TAG_XPATH "/tag" -#define FRR_STATIC_ROUTE_PATH_TABLEID_XPATH "/table-id" - /* route-list/frr-nexthops */ #define FRR_STATIC_ROUTE_NH_KEY_XPATH \ "/frr-nexthops/" \ @@ -157,7 +155,7 @@ int routing_control_plane_protocols_name_validate( "/frr-routing:routing/control-plane-protocols/" \ "control-plane-protocol[type='%s'][name='%s'][vrf='%s']/" \ "frr-staticd:staticd/route-list[prefix='%s'][afi-safi='%s']/" \ - "src-list[src-prefix='%s']/path-list[distance='%u']" + "src-list[src-prefix='%s']/path-list[table-id='%u'][distance='%u']" /* route-list/frr-nexthops */ #define FRR_DEL_S_ROUTE_NH_KEY_XPATH \ diff --git a/staticd/static_nb_config.c b/staticd/static_nb_config.c index e89832069c..2cdf854244 100644 --- a/staticd/static_nb_config.c +++ b/staticd/static_nb_config.c @@ -35,17 +35,40 @@ static int static_path_list_create(struct nb_cb_create_args *args) { struct route_node *rn; struct static_path *pn; + const struct lyd_node *vrf_dnode; + const char *vrf; uint8_t distance; + uint32_t table_id; switch (args->event) { case NB_EV_VALIDATE: + vrf_dnode = yang_dnode_get_parent(args->dnode, + "control-plane-protocol"); + vrf = yang_dnode_get_string(vrf_dnode, "./vrf"); + table_id = yang_dnode_get_uint32(args->dnode, "./table-id"); + + /* + * TableId is not applicable for VRF. Consider the case of + * l3mdev, there is one uint32_t space to work with. + * A l3mdev device points at a specific table that it + * relates to and a set of interfaces it belongs to. + */ + if (table_id && (strcmp(vrf, vrf_get_default_name()) != 0) + && !vrf_is_backend_netns()) { + snprintf( + args->errmsg, args->errmsg_len, + "%% table param only available when running on netns-based vrfs"); + return NB_ERR_VALIDATION; + } + break; case NB_EV_ABORT: case NB_EV_PREPARE: break; case NB_EV_APPLY: rn = nb_running_get_entry(args->dnode, NULL, true); distance = yang_dnode_get_uint8(args->dnode, "./distance"); - pn = static_add_path(rn, distance); + table_id = yang_dnode_get_uint32(args->dnode, "./table-id"); + pn = static_add_path(rn, table_id, distance); nb_running_set_entry(args->dnode, pn); } @@ -80,44 +103,6 @@ static void static_path_list_tag_modify(struct nb_cb_modify_args *args, static_install_path(rn, pn, info->safi, info->svrf); } -static int static_path_list_tableid_modify(struct nb_cb_modify_args *args, - const struct lyd_node *rn_dnode, - struct stable_info *info) -{ - struct static_path *pn; - struct route_node *rn; - uint32_t table_id; - const struct lyd_node *vrf_dnode; - const char *vrf; - - switch (args->event) { - case NB_EV_VALIDATE: - vrf_dnode = yang_dnode_get_parent(args->dnode, - "control-plane-protocol"); - vrf = yang_dnode_get_string(vrf_dnode, "./vrf"); - table_id = yang_dnode_get_uint32(args->dnode, NULL); - if (table_id && (strcmp(vrf, vrf_get_default_name()) != 0) - && !vrf_is_backend_netns()) { - snprintf(args->errmsg, args->errmsg_len, - "%% table param only available when running on netns-based vrfs"); - return NB_ERR_VALIDATION; - } - break; - case NB_EV_PREPARE: - case NB_EV_ABORT: - break; - case NB_EV_APPLY: - table_id = yang_dnode_get_uint32(args->dnode, NULL); - pn = nb_running_get_entry(args->dnode, NULL, true); - pn->table_id = table_id; - rn = nb_running_get_entry(rn_dnode, NULL, true); - static_install_path(rn, pn, info->safi, info->svrf); - break; - } - - return NB_OK; -} - static bool static_nexthop_create(struct nb_cb_create_args *args, const struct lyd_node *rn_dnode, struct stable_info *info) @@ -566,38 +551,6 @@ int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_pa return NB_OK; } -/* - * XPath: - * /frr-routing:routing/control-plane-protocols/control-plane-protocol/frr-staticd:staticd/route-list/path-list/table-id - */ -int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_path_list_table_id_modify( - struct nb_cb_modify_args *args) -{ - struct route_node *rn; - const struct lyd_node *rn_dnode; - struct stable_info *info; - - switch (args->event) { - case NB_EV_VALIDATE: - if (static_path_list_tableid_modify(args, NULL, NULL) != NB_OK) - return NB_ERR_VALIDATION; - break; - case NB_EV_PREPARE: - case NB_EV_ABORT: - break; - case NB_EV_APPLY: - rn_dnode = yang_dnode_get_parent(args->dnode, "route-list"); - rn = nb_running_get_entry(rn_dnode, NULL, true); - info = route_table_get_info(rn->table); - - if (static_path_list_tableid_modify(args, rn_dnode, info) - != NB_OK) - return NB_ERR_VALIDATION; - break; - } - return NB_OK; -} - /* * XPath: * /frr-routing:routing/control-plane-protocols/control-plane-protocol/frr-staticd:staticd/route-list/path-list/frr-nexthops/nexthop @@ -1009,41 +962,6 @@ int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_sr return NB_OK; } -/* - * XPath: - * /frr-routing:routing/control-plane-protocols/control-plane-protocol/frr-staticd:staticd/route-list/src-list/path-list/table-id - */ -int routing_control_plane_protocols_control_plane_protocol_staticd_route_list_src_list_path_list_table_id_modify( - struct nb_cb_modify_args *args) -{ - struct route_node *rn; - const struct lyd_node *rn_dnode; - const struct lyd_node *src_dnode; - struct stable_info *info; - - switch (args->event) { - case NB_EV_VALIDATE: - if (static_path_list_tableid_modify(args, NULL, NULL) != NB_OK) - return NB_ERR_VALIDATION; - break; - case NB_EV_PREPARE: - case NB_EV_ABORT: - break; - case NB_EV_APPLY: - src_dnode = yang_dnode_get_parent(args->dnode, "src-list"); - rn_dnode = yang_dnode_get_parent(src_dnode, "route-list"); - rn = nb_running_get_entry(rn_dnode, NULL, true); - info = route_table_get_info(rn->table); - - if (static_path_list_tableid_modify(args, src_dnode, info) - != NB_OK) - return NB_ERR_VALIDATION; - - break; - } - return NB_OK; -} - /* * XPath: * /frr-routing:routing/control-plane-protocols/control-plane-protocol/frr-staticd:staticd/route-list/src-list/path-list/frr-nexthops/nexthop diff --git a/staticd/static_routes.c b/staticd/static_routes.c index 22a93d7a26..18a6785616 100644 --- a/staticd/static_routes.c +++ b/staticd/static_routes.c @@ -158,7 +158,8 @@ bool static_add_nexthop_validate(struct static_vrf *svrf, static_types type, return true; } -struct static_path *static_add_path(struct route_node *rn, uint8_t distance) +struct static_path *static_add_path(struct route_node *rn, uint32_t table_id, + uint8_t distance) { struct static_path *pn; struct static_route_info *si; @@ -169,6 +170,7 @@ struct static_path *static_add_path(struct route_node *rn, uint8_t distance) pn = XCALLOC(MTYPE_STATIC_PATH, sizeof(struct static_path)); pn->distance = distance; + pn->table_id = table_id; static_nexthop_list_init(&(pn->nexthop_list)); si = rn->info; diff --git a/staticd/static_routes.h b/staticd/static_routes.h index e5c10d18a4..268af7c00e 100644 --- a/staticd/static_routes.h +++ b/staticd/static_routes.h @@ -187,7 +187,7 @@ extern void static_del_route(struct route_node *rn, safi_t safi, struct static_vrf *svrf); extern struct static_path *static_add_path(struct route_node *rn, - uint8_t distance); + uint32_t table_id, uint8_t distance); extern void static_del_path(struct route_node *rn, struct static_path *pn, safi_t safi, struct static_vrf *svrf); diff --git a/staticd/static_vty.c b/staticd/static_vty.c index 8cccecfed7..0ff6f7e790 100644 --- a/staticd/static_vty.c +++ b/staticd/static_vty.c @@ -68,7 +68,6 @@ static int static_route_leak(struct vty *vty, const char *svrf, char buf_src_prefix[PREFIX_STRLEN]; char buf_nh_type[PREFIX_STRLEN]; char buf_tag[PREFIX_STRLEN]; - char buf_tableid[PREFIX_STRLEN]; uint8_t label_stack_id = 0; const char *buf_gate_str; uint8_t distance = ZEBRA_STATIC_DISTANCE_DEFAULT; @@ -162,14 +161,14 @@ static int static_route_leak(struct vty *vty, const char *svrf, "frr-staticd:staticd", "staticd", svrf, buf_prefix, yang_afi_safi_value2identity(afi, safi), - buf_src_prefix, distance); + buf_src_prefix, table_id, distance); else snprintf(xpath_prefix, sizeof(xpath_prefix), FRR_STATIC_ROUTE_INFO_KEY_XPATH, "frr-staticd:staticd", "staticd", svrf, buf_prefix, yang_afi_safi_value2identity(afi, safi), - distance); + table_id, distance); nb_cli_enqueue_change(vty, xpath_prefix, NB_OP_CREATE, NULL); @@ -180,12 +179,6 @@ static int static_route_leak(struct vty *vty, const char *svrf, sizeof(ab_xpath)); nb_cli_enqueue_change(vty, ab_xpath, NB_OP_MODIFY, buf_tag); - /* Table-Id processing */ - snprintf(buf_tableid, sizeof(buf_tableid), "%u", table_id); - strlcpy(ab_xpath, xpath_prefix, sizeof(ab_xpath)); - strlcat(ab_xpath, FRR_STATIC_ROUTE_PATH_TABLEID_XPATH, - sizeof(ab_xpath)); - nb_cli_enqueue_change(vty, ab_xpath, NB_OP_MODIFY, buf_tableid); /* nexthop processing */ snprintf(ab_xpath, sizeof(ab_xpath), @@ -289,16 +282,16 @@ static int static_route_leak(struct vty *vty, const char *svrf, "frr-staticd:staticd", "staticd", svrf, buf_prefix, yang_afi_safi_value2identity(afi, safi), - buf_src_prefix, distance, buf_nh_type, nh_svrf, - buf_gate_str, ifname); + buf_src_prefix, table_id, distance, + buf_nh_type, nh_svrf, buf_gate_str, ifname); else snprintf(ab_xpath, sizeof(ab_xpath), FRR_DEL_S_ROUTE_NH_KEY_XPATH, "frr-staticd:staticd", "staticd", svrf, buf_prefix, yang_afi_safi_value2identity(afi, safi), - distance, buf_nh_type, nh_svrf, buf_gate_str, - ifname); + table_id, distance, buf_nh_type, nh_svrf, + buf_gate_str, ifname); dnode = yang_dnode_get(vty->candidate_config->dnode, ab_xpath); if (!dnode) -- 2.39.5