Merge pull request #15175 from idryzhov/affinity-map-fixes

10 files changed, 32 insertions, 257 deletions

diff --git a/isisd/isis_affinitymap.c b/isisd/isis_affinitymap.c
index 41bad0a7d9..595091db27 100644
--- a/isisd/isis_affinitymap.c
+++ b/isisd/isis_affinitymap.c
@@ -11,35 +11,6 @@
 
 #ifndef FABRICD
 
-static bool isis_affinity_map_check_use(const char *affmap_name)
-{
-	struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT);
-	struct isis_area *area;
-	struct listnode *area_node, *fa_node;
-	struct flex_algo *fa;
-	struct affinity_map *map;
-	uint16_t pos;
-
-	if (!isis)
-		return false;
-
-	map = affinity_map_get(affmap_name);
-	pos = map->bit_position;
-
-	for (ALL_LIST_ELEMENTS_RO(isis->area_list, area_node, area)) {
-		for (ALL_LIST_ELEMENTS_RO(area->flex_algos->flex_algos, fa_node,
-					  fa)) {
-			if (admin_group_get(&fa->admin_group_exclude_any,
-					    pos) ||
-			    admin_group_get(&fa->admin_group_include_any,
-					    pos) ||
-			    admin_group_get(&fa->admin_group_include_all, pos))
-				return true;
-		}
-	}
-	return false;
-}
-
 static void isis_affinity_map_update(const char *affmap_name, uint16_t old_pos,
 				     uint16_t new_pos)
 {
@@ -90,7 +61,6 @@ void isis_affinity_map_init(void)
 {
 	affinity_map_init();
 
-	affinity_map_set_check_use_hook(isis_affinity_map_check_use);
 	affinity_map_set_update_hook(isis_affinity_map_update);
 }
 
diff --git a/lib/affinitymap.c b/lib/affinitymap.c
index 17e1b2cc01..e53d54a443 100644
--- a/lib/affinitymap.c
+++ b/lib/affinitymap.c
@@ -47,7 +47,7 @@ DEFINE_MTYPE_STATIC(LIB, AFFINITY_MAP_INDEX, "Affinity map index");
 DEFINE_QOBJ_TYPE(affinity_maps);
 DEFINE_QOBJ_TYPE(affinity_map);
 
-struct affinity_maps affinity_map_master = {NULL, NULL, NULL, NULL};
+struct affinity_maps affinity_map_master = {NULL, NULL};
 
 static void affinity_map_free(struct affinity_map *map)
 {
@@ -106,36 +106,6 @@ struct affinity_map *affinity_map_get(const char *name)
 	return NULL;
 }
 
-
-char *affinity_map_name_get(int pos)
-{
-	struct listnode *node;
-	struct affinity_map *map;
-
-	if (!affinity_map_master.maps)
-		return NULL;
-
-	for (ALL_LIST_ELEMENTS_RO(affinity_map_master.maps, node, map))
-		if (map->bit_position == pos)
-			return map->name;
-	return NULL;
-}
-
-bool affinity_map_check_use_hook(const char *affmap_name)
-{
-	if (affinity_map_master.check_use_hook)
-		return (*affinity_map_master.check_use_hook)(affmap_name);
-	return false;
-}
-
-bool affinity_map_check_update_hook(const char *affmap_name, uint16_t new_pos)
-{
-	if (affinity_map_master.check_update_hook)
-		return (*affinity_map_master.check_update_hook)(affmap_name,
-								new_pos);
-	return true;
-}
-
 void affinity_map_update_hook(const char *affmap_name, uint16_t new_pos)
 {
 	struct affinity_map *map;
@@ -153,18 +123,6 @@ void affinity_map_update_hook(const char *affmap_name, uint16_t new_pos)
 					   new_pos);
 }
 
-
-void affinity_map_set_check_use_hook(bool (*func)(const char *affmap_name))
-{
-	affinity_map_master.check_use_hook = func;
-}
-
-void affinity_map_set_check_update_hook(bool (*func)(const char *affmap_name,
-						     uint16_t new_pos))
-{
-	affinity_map_master.check_update_hook = func;
-}
-
 void affinity_map_set_update_hook(void (*func)(const char *affmap_name,
 					       uint16_t old_pos,
 					       uint16_t new_pos))
diff --git a/lib/affinitymap.h b/lib/affinitymap.h
index 19edf5a269..f5924ca3ef 100644
--- a/lib/affinitymap.h
+++ b/lib/affinitymap.h
@@ -50,8 +50,6 @@ DECLARE_QOBJ_TYPE(affinity_map);
 struct affinity_maps {
 	struct list *maps;
 
-	bool (*check_use_hook)(const char *affmap_name);
-	bool (*check_update_hook)(const char *affmap_name, uint16_t new_pos);
 	void (*update_hook)(const char *affmap_name, uint16_t old_pos,
 			    uint16_t new_pos);
 
@@ -64,15 +62,9 @@ extern const struct frr_yang_module_info frr_affinity_map_info;
 void affinity_map_set(const char *name, int pos);
 void affinity_map_unset(const char *name);
 struct affinity_map *affinity_map_get(const char *name);
-char *affinity_map_name_get(const int pos);
 
-bool affinity_map_check_use_hook(const char *affmap_name);
-bool affinity_map_check_update_hook(const char *affmap_name, uint16_t new_pos);
 void affinity_map_update_hook(const char *affmap_name, uint16_t new_pos);
 
-void affinity_map_set_check_use_hook(bool (*func)(const char *affmap_name));
-void affinity_map_set_check_update_hook(bool (*func)(const char *affmap_name,
-						     uint16_t new_pos));
 void affinity_map_set_update_hook(void (*func)(const char *affmap_name,
 					       uint16_t old_pos,
 					       uint16_t new_pos));
diff --git a/lib/affinitymap_northbound.c b/lib/affinitymap_northbound.c
index 331075f5c1..9daccc2800 100644
--- a/lib/affinitymap_northbound.c
+++ b/lib/affinitymap_northbound.c
@@ -47,11 +47,6 @@ static int lib_affinity_map_destroy(struct nb_cb_destroy_args *args)
 
 	switch (args->event) {
 	case NB_EV_VALIDATE:
-		if (!affinity_map_check_use_hook(name))
-			break;
-		snprintf(args->errmsg, args->errmsg_len,
-			 "affinity-map %s is used", name);
-		return NB_ERR_VALIDATION;
 	case NB_EV_PREPARE:
 	case NB_EV_ABORT:
 		break;
@@ -68,7 +63,6 @@ static int lib_affinity_map_destroy(struct nb_cb_destroy_args *args)
 static int lib_affinity_map_value_modify(struct nb_cb_modify_args *args)
 {
 	const char *name;
-	char *map_name;
 	uint16_t pos;
 
 	name = yang_dnode_get_string(
@@ -79,20 +73,6 @@ static int lib_affinity_map_value_modify(struct nb_cb_modify_args *args)
 
 	switch (args->event) {
 	case NB_EV_VALIDATE:
-		map_name = affinity_map_name_get(pos);
-		if (map_name &&
-		    strncmp(map_name, name, AFFINITY_NAME_SIZE) != 0) {
-			snprintf(args->errmsg, args->errmsg_len,
-				 "bit-position is used by %s.", map_name);
-			return NB_ERR_VALIDATION;
-		}
-		if (!affinity_map_check_update_hook(name, pos)) {
-			snprintf(
-				args->errmsg, args->errmsg_len,
-				"affinity-map new bit-position > 31 but is used with standard admin-groups");
-			return NB_ERR_VALIDATION;
-		}
-		break;
 	case NB_EV_PREPARE:
 	case NB_EV_ABORT:
 		break;
@@ -105,11 +85,6 @@ static int lib_affinity_map_value_modify(struct nb_cb_modify_args *args)
 	return NB_OK;
 }
 
-static int lib_affinity_map_value_destroy(struct nb_cb_destroy_args *args)
-{
-	return NB_OK;
-}
-
 /* clang-format off */
 const struct frr_yang_module_info frr_affinity_map_info = {
 	.name = "frr-affinity-map",
@@ -126,7 +101,6 @@ const struct frr_yang_module_info frr_affinity_map_info = {
 			.xpath = "/frr-affinity-map:lib/affinity-maps/affinity-map/value",
 			.cbs = {
 				.modify = lib_affinity_map_value_modify,
-				.destroy = lib_affinity_map_value_destroy,
 			}
 		},
 		{
diff --git a/lib/if.c b/lib/if.c
index 1328e21874..1a8195de67 100644
--- a/lib/if.c
+++ b/lib/if.c
@@ -1135,7 +1135,7 @@ struct if_link_params *if_link_params_enable(struct interface *ifp)
 		iflp->unrsv_bw[i] = iflp->default_bw;
 
 	/* Update Link parameters status */
-	iflp->lp_status = LP_MAX_BW | LP_MAX_RSV_BW | LP_UNRSV_BW;
+	iflp->lp_status = LP_MAX_BW | LP_MAX_RSV_BW | LP_UNRSV_BW | LP_EXTEND_ADM_GRP;
 
 	/* Set TE metric equal to standard metric only if it is set */
 	if (ifp->metric != 0) {
diff --git a/yang/frr-affinity-map.yang b/yang/frr-affinity-map.yang
index c4377e6246..f1d9e44738 100644
--- a/yang/frr-affinity-map.yang
+++ b/yang/frr-affinity-map.yang
@@ -53,12 +53,22 @@ module frr-affinity-map {
       "Initial revision";
   }
 
+  typedef affinity-map-ref {
+    type leafref {
+      path "/frr-affinity-map:lib/frr-affinity-map:affinity-maps/frr-affinity-map:affinity-map/frr-affinity-map:name";
+      require-instance true;
+    }
+    description
+      "Reference to an affinity map";
+  }
+
   container lib {
     container affinity-maps {
       description
         "Affinity Mapping Table";
       list affinity-map {
         key "name";
+        unique "value";
         description
           "Affinity Mapping configuration";
         leaf name {
@@ -69,6 +79,7 @@ module frr-affinity-map {
             "Affinity Name";
         }
         leaf value {
+          mandatory true;
           type uint16 {
             range "0..1023";
           }
diff --git a/yang/frr-zebra.yang b/yang/frr-zebra.yang
index 3c6e45126a..1168f05f40 100644
--- a/yang/frr-zebra.yang
+++ b/yang/frr-zebra.yang
@@ -2011,8 +2011,11 @@ module frr-zebra {
           case affinity {
             container affinities {
               leaf-list affinity {
-                type string;
+                type frr-affinity-map:affinity-map-ref;
                 max-elements "256";
+                must '../../affinity-mode != "standard" or /frr-affinity-map:lib/frr-affinity-map:affinity-maps/frr-affinity-map:affinity-map[frr-affinity-map:name=current()]/frr-affinity-map:value < 32' {
+                  error-message "Affinity bit-position must be less than 32 when used with standard affinity mode";
+                }
                 description
                   "Array of Attribute Names";
               }
diff --git a/zebra/interface.c b/zebra/interface.c
index f38e150d73..5a246db4f2 100644
--- a/zebra/interface.c
+++ b/zebra/interface.c
@@ -4256,23 +4256,10 @@ DEFPY_YANG(link_params_admin_grp, link_params_admin_grp_cmd,
 	   "Administrative group membership\n"
 	   "32-bit Hexadecimal value (e.g. 0xa1)\n")
 {
-	char xpath[XPATH_MAXLEN];
 	int idx_bitpattern = 1;
 	unsigned long value;
 	char value_str[11];
 
-	VTY_DECLVAR_CONTEXT(interface, ifp);
-
-	snprintf(
-		xpath, sizeof(xpath),
-		"/frr-interface:lib/interface[name='%s']/frr-zebra:zebra/link-params/affinities",
-		ifp->name);
-	if (yang_dnode_exists(running_config->dnode, xpath)) {
-		vty_out(vty,
-			"cannot use the admin-grp command when affinity is set\n");
-		return CMD_WARNING_CONFIG_FAILED;
-	}
-
 	if (sscanf(argv[idx_bitpattern]->arg, "0x%lx", &value) != 1) {
 		vty_out(vty, "link_params_admin_grp: fscanf: %s\n",
 			safe_strerror(errno));
@@ -4738,19 +4725,6 @@ DEFPY_YANG(link_params_affinity, link_params_affinity_cmd,
 	   "Interface affinities\n"
 	   "Affinity names\n")
 {
-	VTY_DECLVAR_CONTEXT(interface, ifp);
-	char xpath[XPATH_MAXLEN];
-
-	snprintf(
-		xpath, sizeof(xpath),
-		"/frr-interface:lib/interface[name='%s']/frr-zebra:zebra/link-params/legacy-admin-group",
-		ifp->name);
-	if (yang_dnode_exists(running_config->dnode, xpath)) {
-		vty_out(vty,
-			"cannot use the affinity command when admin-grp is set\n");
-		return CMD_WARNING_CONFIG_FAILED;
-	}
-
 	return ag_change(vty, argc, argv,
 			 "./frr-zebra:zebra/link-params/affinities/affinity",
 			 no, no ? 2 : 1);
@@ -4764,8 +4738,8 @@ DEFPY_YANG(link_params_affinity, link_params_affinity_cmd,
 DEFPY_YANG(link_params_affinity_mode, link_params_affinity_mode_cmd,
 	   "affinity-mode <standard|extended|both>$affmode",
 	   "Interface affinity mode\n"
-	   "Standard Admin-Group only RFC3630,5305,5329 (default)\n"
-	   "Extended Admin-Group only RFC7308\n"
+	   "Standard Admin-Group only RFC3630,5305,5329\n"
+	   "Extended Admin-Group only RFC7308 (default)\n"
 	   "Standard and extended Admin-Group format\n")
 {
 	const char *xpath = "./frr-zebra:zebra/link-params/affinity-mode";
@@ -4779,13 +4753,13 @@ DEFPY_YANG(no_link_params_affinity_mode, no_link_params_affinity_mode_cmd,
 	   "no affinity-mode [<standard|extended|both>]",
 	   NO_STR
 	   "Interface affinity mode\n"
-	   "Standard Admin-Group only RFC3630,5305,5329 (default)\n"
-	   "Extended Admin-Group only RFC7308\n"
+	   "Standard Admin-Group only RFC3630,5305,5329\n"
+	   "Extended Admin-Group only RFC7308 (default)\n"
 	   "Standard and extended Admin-Group format\n")
 {
 	const char *xpath = "./frr-zebra:zebra/link-params/affinity-mode";
 
-	nb_cli_enqueue_change(vty, xpath, NB_OP_MODIFY, "standard");
+	nb_cli_enqueue_change(vty, xpath, NB_OP_MODIFY, "extended");
 
 	return nb_cli_apply_changes(vty, NULL);
 }
@@ -4801,11 +4775,8 @@ static int ag_iter_cb(const struct lyd_node *dnode, void *arg)
 void cli_show_legacy_admin_group(struct vty *vty, const struct lyd_node *dnode,
 				 bool show_defaults)
 {
-	if (!yang_dnode_exists(dnode, "legacy-admin-group"))
-		return;
-
-	vty_out(vty, "  admin-group 0x%x\n",
-		yang_dnode_get_uint32(dnode, "legacy-admin-group"));
+	vty_out(vty, "  admin-grp 0x%x\n",
+		yang_dnode_get_uint32(dnode, NULL));
 }
 
 void cli_show_affinity_mode(struct vty *vty, const struct lyd_node *dnode,
@@ -4817,6 +4788,8 @@ void cli_show_affinity_mode(struct vty *vty, const struct lyd_node *dnode,
 		vty_out(vty, "  affinity-mode standard\n");
 	else if (affinity_mode == AFFINITY_MODE_BOTH)
 		vty_out(vty, "  affinity-mode both\n");
+	else if (affinity_mode == AFFINITY_MODE_EXTENDED && show_defaults)
+		vty_out(vty, "  affinity-mode extended\n");
 }
 
 void cli_show_affinity(struct vty *vty, const struct lyd_node *dnode,
diff --git a/zebra/zebra_affinitymap.c b/zebra/zebra_affinitymap.c
index ae0f9a8a35..79bc78a7dc 100644
--- a/zebra/zebra_affinitymap.c
+++ b/zebra/zebra_affinitymap.c
@@ -26,102 +26,26 @@
 #include "zebra/redistribute.h"
 #include "zebra/zebra_affinitymap.h"
 
-static bool zebra_affinity_map_check_use(const char *affmap_name)
-{
-	char xpath[XPATH_MAXLEN];
-	struct interface *ifp;
-	struct vrf *vrf;
-
-	RB_FOREACH (vrf, vrf_id_head, &vrfs_by_id) {
-		FOR_ALL_INTERFACES (vrf, ifp) {
-			snprintf(xpath, sizeof(xpath),
-				 "/frr-interface:lib/interface[name='%s']",
-				 ifp->name);
-			if (!yang_dnode_exists(running_config->dnode, xpath))
-				continue;
-			snprintf(
-				xpath, sizeof(xpath),
-				"/frr-interface:lib/interface[name='%s']/frr-zebra:zebra/link-params/affinities[affinity='%s']",
-				ifp->name, affmap_name);
-			if (yang_dnode_exists(running_config->dnode, xpath))
-				return true;
-		}
-	}
-	return false;
-}
-
-static bool zebra_affinity_map_check_update(const char *affmap_name,
-					    uint16_t new_pos)
-{
-	char xpath[XPATH_MAXLEN];
-	struct interface *ifp;
-	struct vrf *vrf;
-
-	/* check whether the affinity-map new bit position is upper than 31
-	 * but is used on an interface on which affinity-mode is standard.
-	 * Return false if the change is not possible.
-	 */
-	if (new_pos < 32)
-		return true;
-
-	RB_FOREACH (vrf, vrf_id_head, &vrfs_by_id) {
-		FOR_ALL_INTERFACES (vrf, ifp) {
-			snprintf(xpath, sizeof(xpath),
-				 "/frr-interface:lib/interface[name='%s']",
-				 ifp->name);
-			if (!yang_dnode_exists(running_config->dnode, xpath))
-				continue;
-			snprintf(
-				xpath, sizeof(xpath),
-				"/frr-interface:lib/interface[name='%s']/frr-zebra:zebra/link-params/affinities[affinity='%s']",
-				ifp->name, affmap_name);
-			if (!yang_dnode_exists(running_config->dnode, xpath))
-				continue;
-			if (yang_dnode_get_enum(
-				    running_config->dnode,
-				    "/frr-interface:lib/interface[name='%s']/frr-zebra:zebra/link-params/affinity-mode",
-				    ifp->name) == AFFINITY_MODE_STANDARD)
-				return false;
-		}
-	}
-	return true;
-}
-
 static void zebra_affinity_map_update(const char *affmap_name, uint16_t old_pos,
 				      uint16_t new_pos)
 {
 	struct if_link_params *iflp;
-	enum affinity_mode aff_mode;
-	char xpath[XPATH_MAXLEN];
 	struct interface *ifp;
 	struct vrf *vrf;
 
 	RB_FOREACH (vrf, vrf_id_head, &vrfs_by_id) {
 		FOR_ALL_INTERFACES (vrf, ifp) {
-			snprintf(xpath, sizeof(xpath),
-				 "/frr-interface:lib/interface[name='%s']",
-				 ifp->name);
-			if (!yang_dnode_exists(running_config->dnode, xpath))
-				continue;
-			snprintf(
-				xpath, sizeof(xpath),
-				"/frr-interface:lib/interface[name='%s']/frr-zebra:zebra/link-params/affinities[affinity='%s']",
-				ifp->name, affmap_name);
-			if (!yang_dnode_exists(running_config->dnode, xpath))
-				continue;
-			aff_mode = yang_dnode_get_enum(
-				running_config->dnode,
-				"/frr-interface:lib/interface[name='%s']/frr-zebra:zebra/link-params/affinity-mode",
-				ifp->name);
 			iflp = if_link_params_get(ifp);
-			if (aff_mode == AFFINITY_MODE_EXTENDED ||
-			    aff_mode == AFFINITY_MODE_BOTH) {
+			if (!iflp)
+				continue;
+			if (IS_PARAM_SET(iflp, LP_EXTEND_ADM_GRP) &&
+			    admin_group_get(&iflp->ext_admin_grp, old_pos)) {
 				admin_group_unset(&iflp->ext_admin_grp,
 						  old_pos);
 				admin_group_set(&iflp->ext_admin_grp, new_pos);
 			}
-			if (aff_mode == AFFINITY_MODE_STANDARD ||
-			    aff_mode == AFFINITY_MODE_BOTH) {
+			if (IS_PARAM_SET(iflp, LP_ADM_GRP) &&
+			    (iflp->admin_grp & (1 << old_pos))) {
 				iflp->admin_grp &= ~(1 << old_pos);
 				if (new_pos < 32)
 					iflp->admin_grp |= 1 << new_pos;
@@ -138,7 +62,5 @@ void zebra_affinity_map_init(void)
 {
 	affinity_map_init();
 
-	affinity_map_set_check_use_hook(zebra_affinity_map_check_use);
-	affinity_map_set_check_update_hook(zebra_affinity_map_check_update);
 	affinity_map_set_update_hook(zebra_affinity_map_update);
 }
diff --git a/zebra/zebra_nb_config.c b/zebra/zebra_nb_config.c
index 50caaa819e..98c241b2a1 100644
--- a/zebra/zebra_nb_config.c
+++ b/zebra/zebra_nb_config.c
@@ -1270,20 +1270,6 @@ int lib_interface_zebra_affinity_create(struct nb_cb_create_args *args)
 
 	switch (args->event) {
 	case NB_EV_VALIDATE:
-		if (!affmap) {
-			snprintf(args->errmsg, args->errmsg_len,
-				 "affinity-map %s not found.", affname);
-			return NB_ERR_VALIDATION;
-		}
-		if (affinity_mode == AFFINITY_MODE_STANDARD &&
-		    affmap->bit_position > 31) {
-			snprintf(
-				args->errmsg, args->errmsg_len,
-				"affinity %s bit-position %d is not compatible with affinity-mode standard (bit-position > 31).",
-				affname, affmap->bit_position);
-			return NB_ERR_VALIDATION;
-		}
-		break;
 	case NB_EV_PREPARE:
 	case NB_EV_ABORT:
 		break;
@@ -1331,12 +1317,6 @@ int lib_interface_zebra_affinity_destroy(struct nb_cb_destroy_args *args)
 
 	switch (args->event) {
 	case NB_EV_VALIDATE:
-		if (!affmap) {
-			snprintf(args->errmsg, args->errmsg_len,
-				 "affinity-map %s not found.", affname);
-			return NB_ERR_VALIDATION;
-		}
-		break;
 	case NB_EV_PREPARE:
 	case NB_EV_ABORT:
 		break;
@@ -1386,14 +1366,6 @@ int lib_interface_zebra_affinity_mode_modify(struct nb_cb_modify_args *args)
 
 	switch (args->event) {
 	case NB_EV_VALIDATE:
-		if (affinity_mode == AFFINITY_MODE_STANDARD &&
-		    admin_group_nb_words(&iflp->ext_admin_grp) > 1) {
-			snprintf(
-				args->errmsg, args->errmsg_len,
-				"affinity-mode standard cannot be set when a bit-position > 31 is set.");
-			return NB_ERR_VALIDATION;
-		}
-		break;
 	case NB_EV_PREPARE:
 	case NB_EV_ABORT:
 		break;