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 FRRouting#7347

Signed-off-by: vishaldhingra <vdhingra@vmware.com>

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                                           \

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

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;

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);
 

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)