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 <vdhingra@vmware.com>

diff --git a/staticd/static_nb.h b/staticd/static_nb.h
index 8bfb64fec7ec66839a6be8bbabdeb20381823abe..b293224dd1212a0c7c21254d0147ccc7c5eeda3a 100644 (file)
--- 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 6e59f50a0020c8feb2d26eff94ffb66ff14d33b5..3778960cd1510ddad5036ceca80212d17151df1c 100644 (file)
--- 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)
@@ -578,38 +563,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
@@ -1021,41 +974,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 05355c48fe800ba66617b5d9f05c1c31c758db70..1c436a66b079782beb0d333e2c2dc83838d5d268 100644 (file)
--- a/staticd/static_routes.c
+++ b/staticd/static_routes.c
@@ -161,7 +161,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;
@@ -172,6 +173,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 bd2cd78fd9338894cc1a5047d0e7286cfdb12015..470afb605d0a843ac6be5b8f5df5102a2ccd4fa4 100644 (file)
--- 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 c3c453f42d524cc516290658226196eb28153ffc..1488cc1775378452569c030f68450413c600e6c2 100644 (file)
--- 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)