From 0a156eecf2d4db42e3f21b35fbc09e2516898c53 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Wed, 23 Jun 2021 00:21:24 +0300 Subject: [PATCH 1/3] isisd: fix NET NB configuration Don't rely on operational data to check for system ID consistency. This is purely configurational data thing. Signed-off-by: Igor Ryzhov --- isisd/isis_nb_config.c | 47 +++++++++++++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/isisd/isis_nb_config.c b/isisd/isis_nb_config.c index 2622c5b51b..515ceadea4 100644 --- a/isisd/isis_nb_config.c +++ b/isisd/isis_nb_config.c @@ -111,6 +111,28 @@ int isis_instance_is_type_modify(struct nb_cb_modify_args *args) return NB_OK; } +struct sysid_iter { + struct area_addr *addr; + bool same; +}; + +static int sysid_iter_cb(const struct lyd_node *dnode, void *arg) +{ + struct sysid_iter *iter = arg; + struct area_addr addr; + const char *net; + + net = yang_dnode_get_string(dnode, NULL); + addr.addr_len = dotformat2buff(addr.area_addr, net); + + if (memcmp(GETSYSID(iter->addr), GETSYSID((&addr)), ISIS_SYS_ID_LEN)) { + iter->same = false; + return YANG_ITER_STOP; + } + + return YANG_ITER_CONTINUE; +} + /* * XPath: /frr-isisd:isis/instance/area-address */ @@ -119,14 +141,12 @@ int isis_instance_area_address_create(struct nb_cb_create_args *args) struct isis_area *area; struct area_addr addr, *addrr = NULL, *addrp = NULL; struct listnode *node; + struct sysid_iter iter; uint8_t buff[255]; const char *net_title = yang_dnode_get_string(args->dnode, NULL); switch (args->event) { case NB_EV_VALIDATE: - area = nb_running_get_entry(args->dnode, NULL, false); - if (area == NULL) - return NB_ERR_VALIDATION; addr.addr_len = dotformat2buff(buff, net_title); memcpy(addr.area_addr, buff, addr.addr_len); if (addr.area_addr[addr.addr_len - 1] != 0) { @@ -135,15 +155,18 @@ int isis_instance_area_address_create(struct nb_cb_create_args *args) "nsel byte (last byte) in area address must be 0"); return NB_ERR_VALIDATION; } - if (area->isis->sysid_set) { - /* Check that the SystemID portions match */ - if (memcmp(area->isis->sysid, GETSYSID((&addr)), - ISIS_SYS_ID_LEN)) { - snprintf( - args->errmsg, args->errmsg_len, - "System ID must not change when defining additional area addresses"); - return NB_ERR_VALIDATION; - } + + iter.addr = &addr; + iter.same = true; + + yang_dnode_iterate(sysid_iter_cb, &iter, args->dnode, + "../area-address"); + + if (!iter.same) { + snprintf( + args->errmsg, args->errmsg_len, + "System ID must not change when defining additional area addresses"); + return NB_ERR_VALIDATION; } break; case NB_EV_PREPARE: From 80ab95b134b00a115175adc46b82a3f69f58ec87 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Wed, 23 Jun 2021 00:23:18 +0300 Subject: [PATCH 2/3] isisd: fix instance ldp-sync configuration Don't rely on operational data to validate that configuration is applied to the default VRF. The VRF name is stored in the config - use it instead. Signed-off-by: Igor Ryzhov --- isisd/isis_nb_config.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/isisd/isis_nb_config.c b/isisd/isis_nb_config.c index 515ceadea4..a0da19fce8 100644 --- a/isisd/isis_nb_config.c +++ b/isisd/isis_nb_config.c @@ -2383,14 +2383,14 @@ int isis_instance_segment_routing_prefix_sid_map_prefix_sid_n_flag_clear_modify( int isis_instance_mpls_ldp_sync_create(struct nb_cb_create_args *args) { struct isis_area *area; + const char *vrfname; switch (args->event) { case NB_EV_VALIDATE: - area = nb_running_get_entry(args->dnode, NULL, false); - if (area == NULL || area->isis == NULL) - return NB_ERR_VALIDATION; + vrfname = yang_dnode_get_string( + lyd_parent(lyd_parent(args->dnode)), "./vrf"); - if (area->isis->vrf_id != VRF_DEFAULT) { + if (strcmp(vrfname, VRF_DEFAULT_NAME)) { snprintf(args->errmsg, args->errmsg_len, "LDP-Sync only runs on Default VRF"); return NB_ERR_VALIDATION; @@ -2427,14 +2427,15 @@ int isis_instance_mpls_ldp_sync_holddown_modify(struct nb_cb_modify_args *args) { struct isis_area *area; uint16_t holddown; + const char *vrfname; switch (args->event) { case NB_EV_VALIDATE: - area = nb_running_get_entry(args->dnode, NULL, false); - if (area == NULL || area->isis == NULL) - return NB_ERR_VALIDATION; + vrfname = yang_dnode_get_string( + lyd_parent(lyd_parent(lyd_parent(args->dnode))), + "./vrf"); - if (area->isis->vrf_id != VRF_DEFAULT) { + if (strcmp(vrfname, VRF_DEFAULT_NAME)) { snprintf(args->errmsg, args->errmsg_len, "LDP-Sync only runs on Default VRF"); return NB_ERR_VALIDATION; From e432649280b47b55723ca3ce6b39bacc039b6e18 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Wed, 23 Jun 2021 00:27:55 +0300 Subject: [PATCH 3/3] isisd: fix interface ldp-sync configuration There are two checks done when configuring ldp-sync on an interface: - interface is not a loopback - interface is in the default VRF Both checks are incorrectly done using the operational data. The second check can be done using only config data - do that. The first check can't be done using only configurational data, but it's not necessary. LDP sync code doesn't operate on loopback interfaces already. There's no harm in allowing this to be configured. Signed-off-by: Igor Ryzhov --- isisd/isis_nb_config.c | 67 ++++++++++-------------------------------- 1 file changed, 15 insertions(+), 52 deletions(-) diff --git a/isisd/isis_nb_config.c b/isisd/isis_nb_config.c index a0da19fce8..4a01c728f0 100644 --- a/isisd/isis_nb_config.c +++ b/isisd/isis_nb_config.c @@ -3204,26 +3204,14 @@ int lib_interface_isis_mpls_ldp_sync_modify(struct nb_cb_modify_args *args) struct isis_circuit *circuit; struct ldp_sync_info *ldp_sync_info; bool ldp_sync_enable; - struct interface *ifp; + const char *vrfname; switch (args->event) { case NB_EV_VALIDATE: - ifp = nb_running_get_entry( - lyd_parent(lyd_parent(lyd_parent(args->dnode))), NULL, - false); - if (ifp == NULL) - return NB_ERR_VALIDATION; - if (if_is_loopback(ifp)) { - snprintf(args->errmsg, args->errmsg_len, - "LDP-Sync does not run on loopback interface"); - return NB_ERR_VALIDATION; - } - - circuit = nb_running_get_entry(args->dnode, NULL, false); - if (circuit == NULL || circuit->area == NULL) - break; - - if (circuit->isis->vrf_id != VRF_DEFAULT) { + vrfname = yang_dnode_get_string( + lyd_parent(lyd_parent(lyd_parent(args->dnode))), + "./vrf"); + if (strcmp(vrfname, VRF_DEFAULT_NAME)) { snprintf(args->errmsg, args->errmsg_len, "LDP-Sync only runs on Default VRF"); return NB_ERR_VALIDATION; @@ -3260,27 +3248,14 @@ int lib_interface_isis_mpls_holddown_modify(struct nb_cb_modify_args *args) struct isis_circuit *circuit; struct ldp_sync_info *ldp_sync_info; uint16_t holddown; - struct interface *ifp; + const char *vrfname; switch (args->event) { case NB_EV_VALIDATE: - - ifp = nb_running_get_entry( - lyd_parent(lyd_parent(lyd_parent(args->dnode))), NULL, - false); - if (ifp == NULL) - return NB_ERR_VALIDATION; - if (if_is_loopback(ifp)) { - snprintf(args->errmsg, args->errmsg_len, - "LDP-Sync does not run on loopback interface"); - return NB_ERR_VALIDATION; - } - - circuit = nb_running_get_entry(args->dnode, NULL, false); - if (circuit == NULL || circuit->area == NULL) - break; - - if (circuit->isis->vrf_id != VRF_DEFAULT) { + vrfname = yang_dnode_get_string( + lyd_parent(lyd_parent(lyd_parent(args->dnode))), + "./vrf"); + if (strcmp(vrfname, VRF_DEFAULT_NAME)) { snprintf(args->errmsg, args->errmsg_len, "LDP-Sync only runs on Default VRF"); return NB_ERR_VALIDATION; @@ -3306,26 +3281,14 @@ int lib_interface_isis_mpls_holddown_destroy(struct nb_cb_destroy_args *args) { struct isis_circuit *circuit; struct ldp_sync_info *ldp_sync_info; - struct interface *ifp; + const char *vrfname; switch (args->event) { case NB_EV_VALIDATE: - ifp = nb_running_get_entry( - lyd_parent(lyd_parent(lyd_parent(args->dnode))), NULL, - false); - if (ifp == NULL) - return NB_ERR_VALIDATION; - if (if_is_loopback(ifp)) { - snprintf(args->errmsg, args->errmsg_len, - "LDP-Sync does not run on loopback interface"); - return NB_ERR_VALIDATION; - } - - circuit = nb_running_get_entry(args->dnode, NULL, false); - if (circuit == NULL || circuit->area == NULL) - break; - - if (circuit->isis->vrf_id != VRF_DEFAULT) { + vrfname = yang_dnode_get_string( + lyd_parent(lyd_parent(lyd_parent(args->dnode))), + "./vrf"); + if (strcmp(vrfname, VRF_DEFAULT_NAME)) { snprintf(args->errmsg, args->errmsg_len, "LDP-Sync only runs on Default VRF"); return NB_ERR_VALIDATION;