From fdd834b8cc69b1b8c9bdfcfa1f033f35affb4c09 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 18 Jan 2024 23:27:56 +0200 Subject: [PATCH 1/8] lib: validate affinity-map reference using yang model Change the type of affinity leaf-list in frr-zebra to a leafref with "require-instance" property set to true. This change tells libyang to automatically check that affinity-map exists before usage and doesn't allow it to be deleted if it's referenced. It allows us to remove all the manual code that is doing the same thing. Signed-off-by: Igor Ryzhov --- isisd/isis_affinitymap.c | 30 ------------------------------ lib/affinitymap.c | 15 +-------------- lib/affinitymap.h | 3 --- lib/affinitymap_northbound.c | 5 ----- yang/frr-affinity-map.yang | 9 +++++++++ yang/frr-zebra.yang | 2 +- zebra/zebra_affinitymap.c | 25 ------------------------- 7 files changed, 11 insertions(+), 78 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..b748e74884 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, NULL}; static void affinity_map_free(struct affinity_map *map) { @@ -121,13 +121,6 @@ char *affinity_map_name_get(int pos) 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) @@ -153,12 +146,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)) { diff --git a/lib/affinitymap.h b/lib/affinitymap.h index 19edf5a269..5ce233404f 100644 --- a/lib/affinitymap.h +++ b/lib/affinitymap.h @@ -50,7 +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); @@ -66,11 +65,9 @@ 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, diff --git a/lib/affinitymap_northbound.c b/lib/affinitymap_northbound.c index 331075f5c1..bee2ebe861 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; diff --git a/yang/frr-affinity-map.yang b/yang/frr-affinity-map.yang index c4377e6246..992f5c7535 100644 --- a/yang/frr-affinity-map.yang +++ b/yang/frr-affinity-map.yang @@ -53,6 +53,15 @@ 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 diff --git a/yang/frr-zebra.yang b/yang/frr-zebra.yang index 3c6e45126a..68774a84fa 100644 --- a/yang/frr-zebra.yang +++ b/yang/frr-zebra.yang @@ -2011,7 +2011,7 @@ module frr-zebra { case affinity { container affinities { leaf-list affinity { - type string; + type frr-affinity-map:affinity-map-ref; max-elements "256"; description "Array of Attribute Names"; diff --git a/zebra/zebra_affinitymap.c b/zebra/zebra_affinitymap.c index ae0f9a8a35..fabd3e677c 100644 --- a/zebra/zebra_affinitymap.c +++ b/zebra/zebra_affinitymap.c @@ -26,30 +26,6 @@ #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) { @@ -138,7 +114,6 @@ 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); } From 26bd685a87cd95b0a864580c5d8696eee203d929 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 18 Jan 2024 23:39:32 +0200 Subject: [PATCH 2/8] lib: make affinity-map value unique in the yang model It allows us to remove the code that does the same thing manually. Signed-off-by: Igor Ryzhov --- lib/affinitymap.c | 15 --------------- lib/affinitymap.h | 1 - lib/affinitymap_northbound.c | 8 -------- yang/frr-affinity-map.yang | 1 + 4 files changed, 1 insertion(+), 24 deletions(-) diff --git a/lib/affinitymap.c b/lib/affinitymap.c index b748e74884..00f23c1ca3 100644 --- a/lib/affinitymap.c +++ b/lib/affinitymap.c @@ -106,21 +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_update_hook(const char *affmap_name, uint16_t new_pos) { if (affinity_map_master.check_update_hook) diff --git a/lib/affinitymap.h b/lib/affinitymap.h index 5ce233404f..8e0a798040 100644 --- a/lib/affinitymap.h +++ b/lib/affinitymap.h @@ -63,7 +63,6 @@ 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_update_hook(const char *affmap_name, uint16_t new_pos); void affinity_map_update_hook(const char *affmap_name, uint16_t new_pos); diff --git a/lib/affinitymap_northbound.c b/lib/affinitymap_northbound.c index bee2ebe861..1a1ff5f465 100644 --- a/lib/affinitymap_northbound.c +++ b/lib/affinitymap_northbound.c @@ -63,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( @@ -74,13 +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, diff --git a/yang/frr-affinity-map.yang b/yang/frr-affinity-map.yang index 992f5c7535..91b70ff22a 100644 --- a/yang/frr-affinity-map.yang +++ b/yang/frr-affinity-map.yang @@ -68,6 +68,7 @@ module frr-affinity-map { "Affinity Mapping Table"; list affinity-map { key "name"; + unique "value"; description "Affinity Mapping configuration"; leaf name { From 670e0c0737862e77f7baa042b2748345c13e7355 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Fri, 19 Jan 2024 01:40:21 +0200 Subject: [PATCH 3/8] lib: validate affinity-map bit position using the yang model When affinity mode is "standard", bit position cannot be greater than 31. Add a "must" statement to the YANG model to validate this, and remove our custom validation code that does the same. Signed-off-by: Igor Ryzhov --- lib/affinitymap.c | 16 +-------------- lib/affinitymap.h | 4 ---- lib/affinitymap_northbound.c | 7 ------- yang/frr-zebra.yang | 3 +++ zebra/zebra_affinitymap.c | 38 ------------------------------------ zebra/zebra_nb_config.c | 28 -------------------------- 6 files changed, 4 insertions(+), 92 deletions(-) diff --git a/lib/affinitymap.c b/lib/affinitymap.c index 00f23c1ca3..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}; +struct affinity_maps affinity_map_master = {NULL, NULL}; static void affinity_map_free(struct affinity_map *map) { @@ -106,14 +106,6 @@ struct affinity_map *affinity_map_get(const char *name) return NULL; } -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; @@ -131,12 +123,6 @@ void affinity_map_update_hook(const char *affmap_name, uint16_t new_pos) new_pos); } -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 8e0a798040..f5924ca3ef 100644 --- a/lib/affinitymap.h +++ b/lib/affinitymap.h @@ -50,7 +50,6 @@ DECLARE_QOBJ_TYPE(affinity_map); struct affinity_maps { struct list *maps; - 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,11 +63,8 @@ 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); -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_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 1a1ff5f465..30a9f373c2 100644 --- a/lib/affinitymap_northbound.c +++ b/lib/affinitymap_northbound.c @@ -73,13 +73,6 @@ static int lib_affinity_map_value_modify(struct nb_cb_modify_args *args) switch (args->event) { case NB_EV_VALIDATE: - 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; diff --git a/yang/frr-zebra.yang b/yang/frr-zebra.yang index 68774a84fa..1168f05f40 100644 --- a/yang/frr-zebra.yang +++ b/yang/frr-zebra.yang @@ -2013,6 +2013,9 @@ module frr-zebra { leaf-list affinity { 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/zebra_affinitymap.c b/zebra/zebra_affinitymap.c index fabd3e677c..dd06000038 100644 --- a/zebra/zebra_affinitymap.c +++ b/zebra/zebra_affinitymap.c @@ -26,43 +26,6 @@ #include "zebra/redistribute.h" #include "zebra/zebra_affinitymap.h" -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) { @@ -114,6 +77,5 @@ void zebra_affinity_map_init(void) { affinity_map_init(); - 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; From 3856ba2359f54b75276f19c54582b68d64146315 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Fri, 19 Jan 2024 01:52:41 +0200 Subject: [PATCH 4/8] lib: make affinity-map value mandatory There can't be an affinity map without a bit position. Signed-off-by: Igor Ryzhov --- lib/affinitymap_northbound.c | 6 ------ yang/frr-affinity-map.yang | 1 + 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/lib/affinitymap_northbound.c b/lib/affinitymap_northbound.c index 30a9f373c2..9daccc2800 100644 --- a/lib/affinitymap_northbound.c +++ b/lib/affinitymap_northbound.c @@ -85,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", @@ -106,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/yang/frr-affinity-map.yang b/yang/frr-affinity-map.yang index 91b70ff22a..f1d9e44738 100644 --- a/yang/frr-affinity-map.yang +++ b/yang/frr-affinity-map.yang @@ -79,6 +79,7 @@ module frr-affinity-map { "Affinity Name"; } leaf value { + mandatory true; type uint16 { range "0..1023"; } From a3bbe28e6d0e2f78bef8731002e76339320aacc9 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Fri, 19 Jan 2024 02:38:43 +0200 Subject: [PATCH 5/8] zebra: rework affinity-map update hook Don't use config tree when updating internal daemon state. Everything needed is already stored in internal structures. Signed-off-by: Igor Ryzhov --- zebra/zebra_affinitymap.c | 27 ++++++--------------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/zebra/zebra_affinitymap.c b/zebra/zebra_affinitymap.c index dd06000038..79bc78a7dc 100644 --- a/zebra/zebra_affinitymap.c +++ b/zebra/zebra_affinitymap.c @@ -30,37 +30,22 @@ 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; From f62e38cc364df4f41e6eb2f128366b90a3aa714a Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Fri, 19 Jan 2024 02:56:45 +0200 Subject: [PATCH 6/8] zebra: fix link-params admin-grp config output - it was not printed at all because of the incorrect `yang_dnode_exist` check - the intended output was "admin-group" instead of "admin-grp" used in the actual CLI command Signed-off-by: Igor Ryzhov --- zebra/interface.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/zebra/interface.c b/zebra/interface.c index f38e150d73..cf25fe0139 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -4801,11 +4801,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, From 733462a991ffa70a4b4bf04282833956a8925b43 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Fri, 19 Jan 2024 03:01:40 +0200 Subject: [PATCH 7/8] zebra: remove unnecessary checks from CLI First, any data tree validation in CLI handler is not correct, because this code won't be called when the change is done through any other frontend. Second, these checks are not necessary at all, because NB layer handles the change between admin-grp/affinity automatically. Signed-off-by: Igor Ryzhov --- zebra/interface.c | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/zebra/interface.c b/zebra/interface.c index cf25fe0139..cbc0eb6908 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); From 01be34fa3429e97fdcaf8299b74dc18ddb1c9629 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Fri, 19 Jan 2024 03:21:53 +0200 Subject: [PATCH 8/8] zebra: fix default value for affinity-mode - initialize the necessary bit when creating if_link_params - fix CLI description to mark extended as the default mode - correctly set mode to extended when using the "no" form of the command - handle the "show_defaults" parameter correctly in cli_show callback Signed-off-by: Igor Ryzhov --- lib/if.c | 2 +- zebra/interface.c | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) 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/zebra/interface.c b/zebra/interface.c index cbc0eb6908..5a246db4f2 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -4738,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 $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"; @@ -4753,13 +4753,13 @@ DEFPY_YANG(no_link_params_affinity_mode, no_link_params_affinity_mode_cmd, "no affinity-mode []", 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); } @@ -4788,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,