From e2b5b7d6d7fac5507f7bfb4dcf9d66f7c79e7fbb Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Tue, 27 Apr 2021 17:28:44 +0300 Subject: [PATCH 01/11] isisd: fix incorrect snmp-id gen/free Necessary structures for snmp-id generation are currently stored in `struct isis`. When we generate the new circuit ID, we always use the instance from the default VRF. When we free the circuit ID, we use the instance from the circuit VRF. This causes the following problems: 1. If there is no instance in the default VRF, this code doesn't work. 2. When circuit in non-default VRF is deleted, the ID is not actually freed. This is fixed by using global structures instead. The code itself is moved to isis_snmp.c and linked to the main code using hooks. We should not call SNMP-related code when the SNMP module is not loaded at all. More than that, we don't allow to activate the circuit if we failed to generate the SNMP ID. Even if SNMP support is completely disabled! This check is removed. Signed-off-by: Igor Ryzhov --- isisd/isis_circuit.c | 64 +++----------------------------------- isisd/isis_circuit.h | 3 ++ isisd/isis_snmp.c | 74 ++++++++++++++++++++++++++++++++------------ isisd/isisd.h | 4 --- 4 files changed, 62 insertions(+), 83 deletions(-) diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c index a637429e84..ed6490266c 100644 --- a/isisd/isis_circuit.c +++ b/isisd/isis_circuit.c @@ -73,47 +73,8 @@ DEFINE_HOOK(isis_if_new_hook, (struct interface *ifp), (ifp)); int isis_if_new_hook(struct interface *); int isis_if_delete_hook(struct interface *); -static int isis_circuit_smmp_id_gen(struct isis_circuit *circuit) -{ - struct vrf *vrf = vrf_lookup_by_id(VRF_DEFAULT); - struct isis *isis = NULL; - uint32_t id; - uint32_t i; - - isis = isis_lookup_by_vrfid(vrf->vrf_id); - if (isis == NULL) - return 0; - - id = isis->snmp_circuit_id_last; - id++; - - /* find next unused entry */ - for (i = 0; i < SNMP_CIRCUITS_MAX; i++) { - if (id >= SNMP_CIRCUITS_MAX) { - id = 0; - continue; - } - - if (id == 0) - continue; - - if (isis->snmp_circuits[id] == NULL) - break; - - id++; - } - - if (i == SNMP_CIRCUITS_MAX) { - zlog_warn("Could not allocate a smmp-circuit-id"); - return 0; - } - - isis->snmp_circuits[id] = circuit; - isis->snmp_circuit_id_last = id; - circuit->snmp_id = id; - - return 1; -} +DEFINE_HOOK(isis_circuit_new_hook, (struct isis_circuit *circuit), (circuit)); +DEFINE_HOOK(isis_circuit_del_hook, (struct isis_circuit *circuit), (circuit)); struct isis_circuit *isis_circuit_new(struct isis *isis) { @@ -123,11 +84,6 @@ struct isis_circuit *isis_circuit_new(struct isis *isis) circuit = XCALLOC(MTYPE_ISIS_CIRCUIT, sizeof(struct isis_circuit)); circuit->isis = isis; - /* - * Note: if snmp-id generation failed circuit will fail - * up operation - */ - isis_circuit_smmp_id_gen(circuit); /* * Default values @@ -193,6 +149,8 @@ struct isis_circuit *isis_circuit_new(struct isis *isis) isis_lfa_excluded_ifaces_init(circuit, ISIS_LEVEL1); isis_lfa_excluded_ifaces_init(circuit, ISIS_LEVEL2); + hook_call(isis_circuit_new_hook, circuit); + QOBJ_REG(circuit, isis_circuit); return circuit; @@ -200,17 +158,12 @@ struct isis_circuit *isis_circuit_new(struct isis *isis) void isis_circuit_del(struct isis_circuit *circuit) { - struct isis *isis = NULL; - if (!circuit) return; QOBJ_UNREG(circuit); - if (circuit->interface) { - isis = isis_lookup_by_vrfid(circuit->interface->vrf_id); - isis->snmp_circuits[circuit->snmp_id] = NULL; - } + hook_call(isis_circuit_del_hook, circuit); isis_circuit_if_unbind(circuit, circuit->interface); @@ -681,13 +634,6 @@ int isis_circuit_up(struct isis_circuit *circuit) return ISIS_OK; } - if (circuit->snmp_id == 0) { - /* We cannot bring circuit up if does not have snmp-id */ - flog_err(EC_ISIS_CONFIG, - "No snnmp-id: there are too many circuits:"); - return ISIS_ERROR; - } - if (circuit->area->lsp_mtu > isis_circuit_pdu_size(circuit)) { flog_err( EC_ISIS_CONFIG, diff --git a/isisd/isis_circuit.h b/isisd/isis_circuit.h index d4c7baea1a..72722d8217 100644 --- a/isisd/isis_circuit.h +++ b/isisd/isis_circuit.h @@ -238,4 +238,7 @@ DECLARE_HOOK(isis_circuit_config_write, DECLARE_HOOK(isis_circuit_add_addr_hook, (struct isis_circuit *circuit), (circuit)); +DECLARE_HOOK(isis_circuit_new_hook, (struct isis_circuit *circuit), (circuit)); +DECLARE_HOOK(isis_circuit_del_hook, (struct isis_circuit *circuit), (circuit)); + #endif /* _ZEBRA_ISIS_CIRCUIT_H */ diff --git a/isisd/isis_snmp.c b/isisd/isis_snmp.c index 1efe9f3bfb..6337faab1c 100644 --- a/isisd/isis_snmp.c +++ b/isisd/isis_snmp.c @@ -629,6 +629,54 @@ static uint8_t isis_null_sysid[ISIS_SYS_ID_LEN]; /* Protocols supported value */ static uint8_t isis_snmp_protocols_supported = 0x7; /* All: iso, ipv4, ipv6 */ +#define SNMP_CIRCUITS_MAX (512) + +static struct isis_circuit *snmp_circuits[SNMP_CIRCUITS_MAX]; +static uint32_t snmp_circuit_id_last; + +static int isis_circuit_snmp_id_gen(struct isis_circuit *circuit) +{ + uint32_t id; + uint32_t i; + + id = snmp_circuit_id_last; + id++; + + /* find next unused entry */ + for (i = 0; i < SNMP_CIRCUITS_MAX; i++) { + if (id >= SNMP_CIRCUITS_MAX) { + id = 0; + continue; + } + + if (id == 0) + continue; + + if (snmp_circuits[id] == NULL) + break; + + id++; + } + + if (i == SNMP_CIRCUITS_MAX) { + zlog_warn("Could not allocate a smmp-circuit-id"); + return 0; + } + + snmp_circuits[id] = circuit; + snmp_circuit_id_last = id; + circuit->snmp_id = id; + + return 0; +} + +static int isis_circuit_snmp_id_free(struct isis_circuit *circuit) +{ + snmp_circuits[circuit->snmp_id] = NULL; + circuit->snmp_id = 0; + return 0; +} + /* * Convenience function to move to the next circuit, */ @@ -636,10 +684,6 @@ static struct isis_circuit *isis_snmp_circuit_next(struct isis_circuit *circuit) { uint32_t start; uint32_t off; - struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT); - - if (isis == NULL) - return NULL; start = 1; @@ -647,7 +691,7 @@ static struct isis_circuit *isis_snmp_circuit_next(struct isis_circuit *circuit) start = circuit->snmp_id + 1; for (off = start; off < SNMP_CIRCUITS_MAX; off++) { - circuit = isis->snmp_circuits[off]; + circuit = snmp_circuits[off]; if (circuit != NULL) return circuit; @@ -912,16 +956,12 @@ static int isis_snmp_circuit_lookup_exact(oid *oid_idx, size_t oid_idx_len, struct isis_circuit **ret_circuit) { struct isis_circuit *circuit; - struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT); - - if (isis == NULL) - return 0; if (oid_idx == NULL || oid_idx_len < 1 || oid_idx[0] > SNMP_CIRCUITS_MAX) return 0; - circuit = isis->snmp_circuits[oid_idx[0]]; + circuit = snmp_circuits[oid_idx[0]]; if (circuit == NULL) return 0; @@ -937,10 +977,6 @@ static int isis_snmp_circuit_lookup_next(oid *oid_idx, size_t oid_idx_len, oid off; oid start; struct isis_circuit *circuit; - struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT); - - if (isis == NULL) - return 0; start = 0; @@ -952,7 +988,7 @@ static int isis_snmp_circuit_lookup_next(oid *oid_idx, size_t oid_idx_len, } for (off = start; off < SNMP_CIRCUITS_MAX; ++off) { - circuit = isis->snmp_circuits[off]; + circuit = snmp_circuits[off]; if (circuit != NULL && off > start) { if (ret_circuit != NULL) @@ -1011,10 +1047,6 @@ static int isis_snmp_circuit_level_lookup_next( oid start; struct isis_circuit *circuit; int level; - struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT); - - if (isis == NULL) - return 0; start = 0; @@ -1026,7 +1058,7 @@ static int isis_snmp_circuit_level_lookup_next( } for (off = start; off < SNMP_CIRCUITS_MAX; off++) { - circuit = isis->snmp_circuits[off]; + circuit = snmp_circuits[off]; if (circuit == NULL) continue; @@ -3449,6 +3481,8 @@ static int isis_snmp_module_init(void) hook_register(isis_hook_adj_state_change, isis_snmp_adj_state_change_update); hook_register(isis_hook_lsp_error, isis_snmp_lsp_error_update); + hook_register(isis_circuit_new_hook, isis_circuit_snmp_id_gen); + hook_register(isis_circuit_del_hook, isis_circuit_snmp_id_free); hook_register(frr_late_init, isis_snmp_init); return 0; diff --git a/isisd/isisd.h b/isisd/isisd.h index a2e821ad2f..fd0e73eb39 100644 --- a/isisd/isisd.h +++ b/isisd/isisd.h @@ -65,8 +65,6 @@ extern void isis_cli_init(void); all_vrf = strmatch(vrf_name, "all"); \ } -#define SNMP_CIRCUITS_MAX (512) - extern struct zebra_privs_t isisd_privs; /* uncomment if you are a developer in bug hunt */ @@ -97,8 +95,6 @@ struct isis { time_t uptime; /* when did we start */ struct thread *t_dync_clean; /* dynamic hostname cache cleanup thread */ uint32_t circuit_ids_used[8]; /* 256 bits to track circuit ids 1 through 255 */ - struct isis_circuit *snmp_circuits[SNMP_CIRCUITS_MAX]; - uint32_t snmp_circuit_id_last; int snmp_notifications; struct route_table *ext_info[REDIST_PROTOCOL_COUNT]; From 0fdd8b2b115d1aaa73f483aa8bdec619c036338b Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Tue, 27 Apr 2021 15:06:37 +0300 Subject: [PATCH 02/11] isisd: update link params after circuit is up Call from isis_circuit_create works only if we enable isis on an already existing interface. If we configure isis on a pseudo interface and then actually create it - this call doesn't work. Signed-off-by: Igor Ryzhov --- isisd/isis_circuit.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c index ed6490266c..80659a4e74 100644 --- a/isisd/isis_circuit.c +++ b/isisd/isis_circuit.c @@ -735,6 +735,9 @@ int isis_circuit_up(struct isis_circuit *circuit) circuit->last_uptime = time(NULL); + if (circuit->area->mta && circuit->area->mta->status) + isis_link_params_update(circuit, circuit->interface); + #ifndef FABRICD /* send northbound notification */ isis_notif_if_state_change(circuit, false); @@ -1302,8 +1305,6 @@ struct isis_circuit *isis_circuit_create(struct isis_area *area, if (circuit->state != C_STATE_CONF && circuit->state != C_STATE_UP) return circuit; isis_circuit_if_bind(circuit, ifp); - if (circuit->area->mta && circuit->area->mta->status) - isis_link_params_update(circuit, ifp); return circuit; } From bcf220815632500f9646cb0b4c13f498afa4edf6 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Wed, 28 Apr 2021 01:57:21 +0300 Subject: [PATCH 03/11] isisd: allow arbitrary order of area/interface configuration Currently we don't allow to configure the interface before the area is configured. This approach has the following issues: 1. The area config can be deleted even when we have an interface config relying on it. The code is not ready for that - we'll have a whole bunch of stale pointers if user does that. 2. The code doesn't correctly process the event of changing the VRF for an interface. There is no mechanism to ensure that the area exists in the new VRF so currently the circuit still stays in the old VRF. This commit allows an arbitrary order of area/interface configuration. There is no more need to configure the area before configuring the interface. This change fixes both the issues. Signed-off-by: Igor Ryzhov --- isisd/isis_circuit.c | 165 +++++++--------- isisd/isis_circuit.h | 5 +- isisd/isis_csm.c | 94 ++++------ isisd/isis_nb_config.c | 177 +++++++++--------- isisd/isis_vty_fabricd.c | 12 +- isisd/isisd.c | 157 ++++++++-------- isisd/isisd.h | 9 +- tests/isisd/test_isis_spf.c | 1 - tests/topotests/isis-snmp/test_isis_snmp.py | 33 ++-- .../isis-sr-topo1/test_isis_sr_topo1.py | 6 + 10 files changed, 314 insertions(+), 345 deletions(-) diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c index 80659a4e74..efc7fec541 100644 --- a/isisd/isis_circuit.c +++ b/isisd/isis_circuit.c @@ -76,14 +76,39 @@ int isis_if_delete_hook(struct interface *); DEFINE_HOOK(isis_circuit_new_hook, (struct isis_circuit *circuit), (circuit)); DEFINE_HOOK(isis_circuit_del_hook, (struct isis_circuit *circuit), (circuit)); -struct isis_circuit *isis_circuit_new(struct isis *isis) +static void isis_circuit_enable(struct isis_circuit *circuit) +{ + struct isis_area *area; + struct interface *ifp = circuit->interface; + + area = isis_area_lookup(circuit->tag, ifp->vrf_id); + if (area) + isis_area_add_circuit(area, circuit); + + if (if_is_operative(ifp)) + isis_csm_state_change(IF_UP_FROM_Z, circuit, ifp); +} + +static void isis_circuit_disable(struct isis_circuit *circuit) +{ + struct isis_area *area = circuit->area; + struct interface *ifp = circuit->interface; + + if (if_is_operative(ifp)) + isis_csm_state_change(IF_DOWN_FROM_Z, circuit, ifp); + + if (area) + isis_area_del_circuit(area, circuit); +} + +struct isis_circuit *isis_circuit_new(struct interface *ifp, const char *tag) { struct isis_circuit *circuit; int i; circuit = XCALLOC(MTYPE_ISIS_CIRCUIT, sizeof(struct isis_circuit)); - circuit->isis = isis; + circuit->tag = XSTRDUP(MTYPE_ISIS_CIRCUIT, tag); /* * Default values @@ -149,10 +174,13 @@ struct isis_circuit *isis_circuit_new(struct isis *isis) isis_lfa_excluded_ifaces_init(circuit, ISIS_LEVEL1); isis_lfa_excluded_ifaces_init(circuit, ISIS_LEVEL2); - hook_call(isis_circuit_new_hook, circuit); - QOBJ_REG(circuit, isis_circuit); + isis_circuit_if_bind(circuit, ifp); + + if (ifp->ifindex != IFINDEX_INTERNAL) + isis_circuit_enable(circuit); + return circuit; } @@ -161,16 +189,19 @@ void isis_circuit_del(struct isis_circuit *circuit) if (!circuit) return; - QOBJ_UNREG(circuit); - - hook_call(isis_circuit_del_hook, circuit); + if (circuit->interface->ifindex != IFINDEX_INTERNAL) + isis_circuit_disable(circuit); isis_circuit_if_unbind(circuit, circuit->interface); + QOBJ_UNREG(circuit); + circuit_mt_finish(circuit); isis_lfa_excluded_ifaces_clear(circuit, ISIS_LEVEL1); isis_lfa_excluded_ifaces_clear(circuit, ISIS_LEVEL2); + XFREE(MTYPE_ISIS_CIRCUIT, circuit->tag); + /* and lastly the circuit itself */ XFREE(MTYPE_ISIS_CIRCUIT, circuit); @@ -181,6 +212,7 @@ void isis_circuit_configure(struct isis_circuit *circuit, struct isis_area *area) { assert(area); + circuit->isis = area->isis; circuit->area = area; /* @@ -200,12 +232,16 @@ void isis_circuit_configure(struct isis_circuit *circuit, circuit->idx = flags_get_index(&area->flags); + hook_call(isis_circuit_new_hook, circuit); + return; } void isis_circuit_deconfigure(struct isis_circuit *circuit, struct isis_area *area) { + hook_call(isis_circuit_del_hook, circuit); + /* Free the index of SRM and SSN flags */ flags_free_index(&area->flags, circuit->idx); circuit->idx = 0; @@ -213,6 +249,7 @@ void isis_circuit_deconfigure(struct isis_circuit *circuit, assert(circuit->area == area); listnode_delete(area->circuit_list, circuit); circuit->area = NULL; + circuit->isis = NULL; return; } @@ -237,29 +274,7 @@ struct isis_circuit *circuit_lookup_by_ifp(struct interface *ifp, struct isis_circuit *circuit_scan_by_ifp(struct interface *ifp) { - struct isis_area *area; - struct listnode *node; - struct isis_circuit *circuit; - struct isis *isis = NULL; - - if (ifp->info) - return (struct isis_circuit *)ifp->info; - - isis = isis_lookup_by_vrfid(ifp->vrf_id); - if (isis == NULL) { - zlog_warn(" %s : ISIS routing instance not found", __func__); - return NULL; - } - - if (isis->area_list) { - for (ALL_LIST_ELEMENTS_RO(isis->area_list, node, area)) { - circuit = - circuit_lookup_by_ifp(ifp, area->circuit_list); - if (circuit) - return circuit; - } - } - return circuit_lookup_by_ifp(ifp, isis->init_circ_list); + return (struct isis_circuit *)ifp->info; } DEFINE_HOOK(isis_circuit_add_addr_hook, (struct isis_circuit *circuit), @@ -469,8 +484,6 @@ void isis_circuit_if_add(struct isis_circuit *circuit, struct interface *ifp) struct listnode *node, *nnode; struct connected *conn; - isis_circuit_if_bind(circuit, ifp); - if (if_is_broadcast(ifp)) { if (fabricd || circuit->circ_type_config == CIRCUIT_T_P2P) circuit->circ_type = CIRCUIT_T_P2P; @@ -1294,21 +1307,6 @@ static int isis_interface_config_write(struct vty *vty) } #endif /* ifdef FABRICD */ -struct isis_circuit *isis_circuit_create(struct isis_area *area, - struct interface *ifp) -{ - struct isis_circuit *circuit = circuit_scan_by_ifp(ifp); - - if (circuit && circuit->area) - return NULL; - circuit = isis_csm_state_change(ISIS_ENABLE, circuit, area); - if (circuit->state != C_STATE_CONF && circuit->state != C_STATE_UP) - return circuit; - isis_circuit_if_bind(circuit, ifp); - - return circuit; -} - void isis_circuit_af_set(struct isis_circuit *circuit, bool ip_router, bool ipv6_router) { @@ -1324,24 +1322,13 @@ void isis_circuit_af_set(struct isis_circuit *circuit, bool ip_router, circuit->ipv6_router = ipv6_router; circuit_update_nlpids(circuit); - /* the area should always be there if we get here, but in the past - * there were corner cases where the area was NULL (e.g. because the - * circuit was deconfigured following a validation error). Do not - * segfault if this happens again. - */ - if (!area) { - zlog_err("%s: NULL area for circuit %u", __func__, - circuit->circuit_id); - return; + if (area) { + area->ip_circuits += ip_router - old_ipr; + area->ipv6_circuits += ipv6_router - old_ipv6r; + + if (ip_router || ipv6_router) + lsp_regenerate_schedule(area, circuit->is_type, 0); } - - area->ip_circuits += ip_router - old_ipr; - area->ipv6_circuits += ipv6_router - old_ipv6r; - - if (!ip_router && !ipv6_router) - isis_csm_state_change(ISIS_DISABLE, circuit, area); - else - lsp_regenerate_schedule(area, circuit->is_type, 0); } ferr_r isis_circuit_passive_set(struct isis_circuit *circuit, bool passive) @@ -1463,8 +1450,9 @@ int isis_circuit_mt_enabled_set(struct isis_circuit *circuit, uint16_t mtid, setting = circuit_get_mt_setting(circuit, mtid); if (setting->enabled != enabled) { setting->enabled = enabled; - lsp_regenerate_schedule(circuit->area, IS_LEVEL_1 | IS_LEVEL_2, - 0); + if (circuit->area) + lsp_regenerate_schedule(circuit->area, + IS_LEVEL_1 | IS_LEVEL_2, 0); } return CMD_SUCCESS; @@ -1477,27 +1465,19 @@ int isis_if_new_hook(struct interface *ifp) int isis_if_delete_hook(struct interface *ifp) { - struct isis_circuit *circuit; - /* Clean up the circuit data */ - if (ifp && ifp->info) { - circuit = ifp->info; - isis_csm_state_change(IF_DOWN_FROM_Z, circuit, ifp); - } + if (ifp->info) + isis_circuit_del(ifp->info); return 0; } static int isis_ifp_create(struct interface *ifp) { - struct vrf *vrf = NULL; + struct isis_circuit *circuit = ifp->info; + + if (circuit) + isis_circuit_enable(circuit); - if (if_is_operative(ifp)) { - vrf = vrf_lookup_by_id(ifp->vrf_id); - if (vrf) - isis_global_instance_create(vrf->name); - isis_csm_state_change(IF_UP_FROM_Z, circuit_scan_by_ifp(ifp), - ifp); - } hook_call(isis_if_new_hook, ifp); return 0; @@ -1505,34 +1485,33 @@ static int isis_ifp_create(struct interface *ifp) static int isis_ifp_up(struct interface *ifp) { - isis_csm_state_change(IF_UP_FROM_Z, circuit_scan_by_ifp(ifp), ifp); + struct isis_circuit *circuit = ifp->info; + + if (circuit) + isis_csm_state_change(IF_UP_FROM_Z, circuit, ifp); return 0; } static int isis_ifp_down(struct interface *ifp) { - struct isis_circuit *circuit; + struct isis_circuit *circuit = ifp->info; + + if (circuit) { + isis_csm_state_change(IF_DOWN_FROM_Z, circuit, ifp); - circuit = isis_csm_state_change(IF_DOWN_FROM_Z, - circuit_scan_by_ifp(ifp), ifp); - if (circuit) SET_FLAG(circuit->flags, ISIS_CIRCUIT_FLAPPED_AFTER_SPF); + } return 0; } static int isis_ifp_destroy(struct interface *ifp) { - if (if_is_operative(ifp)) - zlog_warn("Zebra: got delete of %s, but interface is still up", - ifp->name); + struct isis_circuit *circuit = ifp->info; - isis_csm_state_change(IF_DOWN_FROM_Z, circuit_scan_by_ifp(ifp), ifp); - - /* Cannot call if_delete because we should retain the pseudo interface - in case there is configuration info attached to it. */ - if_delete_retain(ifp); + if (circuit) + isis_circuit_disable(circuit); return 0; } diff --git a/isisd/isis_circuit.h b/isisd/isis_circuit.h index 72722d8217..45c0a7e0ed 100644 --- a/isisd/isis_circuit.h +++ b/isisd/isis_circuit.h @@ -122,6 +122,7 @@ struct isis_circuit { /* * Configurables */ + char *tag; /* area tag */ struct isis_passwd passwd; /* Circuit rx/tx password */ int is_type; /* circuit is type == level of circuit * differentiated from circuit type (media) */ @@ -180,7 +181,7 @@ struct isis_circuit { DECLARE_QOBJ_TYPE(isis_circuit); void isis_circuit_init(void); -struct isis_circuit *isis_circuit_new(struct isis *isis); +struct isis_circuit *isis_circuit_new(struct interface *ifp, const char *tag); void isis_circuit_del(struct isis_circuit *circuit); struct isis_circuit *circuit_lookup_by_ifp(struct interface *ifp, struct list *list); @@ -207,8 +208,6 @@ void isis_circuit_print_vty(struct isis_circuit *circuit, struct vty *vty, size_t isis_circuit_pdu_size(struct isis_circuit *circuit); void isis_circuit_stream(struct isis_circuit *circuit, struct stream **stream); -struct isis_circuit *isis_circuit_create(struct isis_area *area, - struct interface *ifp); void isis_circuit_af_set(struct isis_circuit *circuit, bool ip_router, bool ipv6_router); ferr_r isis_circuit_passive_set(struct isis_circuit *circuit, bool passive); diff --git a/isisd/isis_csm.c b/isisd/isis_csm.c index ebb68ba3f0..0a29dcd09a 100644 --- a/isisd/isis_csm.c +++ b/isisd/isis_csm.c @@ -65,70 +65,56 @@ struct isis_circuit *isis_csm_state_change(enum isis_circuit_event event, void *arg) { enum isis_circuit_state old_state; - struct isis *isis = NULL; struct isis_area *area = NULL; struct interface *ifp; - old_state = circuit ? circuit->state : C_STATE_NA; + assert(circuit); + + old_state = circuit->state; if (IS_DEBUG_EVENTS) - zlog_debug("CSM_EVENT: %s", EVENT2STR(event)); + zlog_debug("CSM_EVENT for %s: %s", circuit->interface->name, + EVENT2STR(event)); switch (old_state) { case C_STATE_NA: - if (circuit) - zlog_warn("Non-null circuit while state C_STATE_NA"); - assert(circuit == NULL); switch (event) { case ISIS_ENABLE: area = arg; - circuit = isis_circuit_new(area->isis); isis_circuit_configure(circuit, area); circuit->state = C_STATE_CONF; break; case IF_UP_FROM_Z: ifp = arg; - isis = isis_lookup_by_vrfid(ifp->vrf_id); - if (isis == NULL) { - if (IS_DEBUG_EVENTS) - zlog_debug( - " %s : ISIS routing instance not found when attempting to apply against interface %s", - __func__, ifp->name); - break; - } - circuit = isis_circuit_new(isis); + isis_circuit_if_add(circuit, ifp); - listnode_add(isis->init_circ_list, circuit); circuit->state = C_STATE_INIT; break; case ISIS_DISABLE: if (IS_DEBUG_EVENTS) - zlog_debug( - "circuit disable event passed for a non existent circuit"); + zlog_debug("circuit %s already disabled", + circuit->interface->name); break; case IF_DOWN_FROM_Z: if (IS_DEBUG_EVENTS) - zlog_debug( - "circuit disconnect event passed for a non existent circuit"); + zlog_debug("circuit %s already disconnected", + circuit->interface->name); break; } break; case C_STATE_INIT: - assert(circuit); switch (event) { case ISIS_ENABLE: - isis_circuit_configure(circuit, - (struct isis_area *)arg); + area = arg; + + isis_circuit_configure(circuit, area); if (isis_circuit_up(circuit) != ISIS_OK) { - isis_circuit_deconfigure( - circuit, (struct isis_area *)arg); + isis_circuit_deconfigure(circuit, area); break; } circuit->state = C_STATE_UP; isis_event_circuit_state_change(circuit, circuit->area, 1); - listnode_delete(circuit->isis->init_circ_list, - circuit); break; case IF_UP_FROM_Z: if (IS_DEBUG_EVENTS) @@ -141,26 +127,26 @@ struct isis_circuit *isis_csm_state_change(enum isis_circuit_event event, circuit->interface->name); break; case IF_DOWN_FROM_Z: - isis_circuit_if_del(circuit, (struct interface *)arg); - listnode_delete(circuit->isis->init_circ_list, - circuit); - isis_circuit_del(circuit); - circuit = NULL; + ifp = arg; + + isis_circuit_if_del(circuit, ifp); + circuit->state = C_STATE_NA; break; } break; case C_STATE_CONF: - assert(circuit); switch (event) { case ISIS_ENABLE: if (IS_DEBUG_EVENTS) - zlog_debug("circuit %p is already enabled", - circuit); + zlog_debug("circuit %s is already enabled", + circuit->interface->name); break; case IF_UP_FROM_Z: - isis_circuit_if_add(circuit, (struct interface *)arg); + ifp = arg; + + isis_circuit_if_add(circuit, ifp); if (isis_circuit_up(circuit) != ISIS_OK) { - isis_circuit_if_del(circuit, (struct interface *)arg); + isis_circuit_if_del(circuit, ifp); flog_err( EC_ISIS_CONFIG, "Could not bring up %s because of invalid config.", @@ -172,24 +158,23 @@ struct isis_circuit *isis_csm_state_change(enum isis_circuit_event event, 1); break; case ISIS_DISABLE: - isis_circuit_deconfigure(circuit, - (struct isis_area *)arg); - isis_circuit_del(circuit); - circuit = NULL; + area = arg; + + isis_circuit_deconfigure(circuit, area); + circuit->state = C_STATE_NA; break; case IF_DOWN_FROM_Z: if (IS_DEBUG_EVENTS) - zlog_debug("circuit %p already disconnected", - circuit); + zlog_debug("circuit %s already disconnected", + circuit->interface->name); break; } break; case C_STATE_UP: - assert(circuit); switch (event) { case ISIS_ENABLE: if (IS_DEBUG_EVENTS) - zlog_debug("circuit %s already configured", + zlog_debug("circuit %s already enabled", circuit->interface->name); break; case IF_UP_FROM_Z: @@ -198,18 +183,18 @@ struct isis_circuit *isis_csm_state_change(enum isis_circuit_event event, circuit->interface->name); break; case ISIS_DISABLE: - isis = circuit->isis; + area = arg; + isis_circuit_down(circuit); - isis_circuit_deconfigure(circuit, - (struct isis_area *)arg); + isis_circuit_deconfigure(circuit, area); circuit->state = C_STATE_INIT; - isis_event_circuit_state_change( - circuit, (struct isis_area *)arg, 0); - listnode_add(isis->init_circ_list, circuit); + isis_event_circuit_state_change(circuit, area, 0); break; case IF_DOWN_FROM_Z: + ifp = arg; + isis_circuit_down(circuit); - isis_circuit_if_del(circuit, (struct interface *)arg); + isis_circuit_if_del(circuit, ifp); circuit->state = C_STATE_CONF; isis_event_circuit_state_change(circuit, circuit->area, 0); @@ -220,8 +205,7 @@ struct isis_circuit *isis_csm_state_change(enum isis_circuit_event event, if (IS_DEBUG_EVENTS) zlog_debug("CSM_STATE_CHANGE: %s -> %s ", STATE2STR(old_state), - circuit ? STATE2STR(circuit->state) - : STATE2STR(C_STATE_NA)); + STATE2STR(circuit->state)); return circuit; } diff --git a/isisd/isis_nb_config.c b/isisd/isis_nb_config.c index c8ad816b58..4b68cd3bed 100644 --- a/isisd/isis_nb_config.c +++ b/isisd/isis_nb_config.c @@ -2544,24 +2544,8 @@ int lib_interface_isis_create(struct nb_cb_create_args *args) } break; case NB_EV_APPLY: - area = isis_area_lookup_by_vrf(area_tag, vrf_name); - /* The area should have already be created. We are - * setting the priority of the global isis area creation - * slightly lower, so it should be executed first, but I - * cannot rely on that so here I have to check. - */ - if (!area) { - flog_err( - EC_LIB_NB_CB_CONFIG_APPLY, - "%s: attempt to create circuit for area %s before the area has been created", - __func__, area_tag); - abort(); - } ifp = nb_running_get_entry(args->dnode, NULL, true); - circuit = isis_circuit_create(area, ifp); - assert(circuit - && (circuit->state == C_STATE_CONF - || circuit->state == C_STATE_UP)); + circuit = isis_circuit_new(ifp, area_tag); nb_running_set_entry(args->dnode, circuit); break; } @@ -2577,18 +2561,12 @@ int lib_interface_isis_destroy(struct nb_cb_destroy_args *args) return NB_OK; circuit = nb_running_unset_entry(args->dnode); - if (!circuit) - return NB_ERR_INCONSISTENCY; /* remove ldp-sync config */ isis_ldp_sync_if_remove(circuit, true); - /* disable both AFs for this circuit. this will also update the - * CSM state by sending an ISIS_DISABLED signal. If there is no - * area associated to the circuit there is nothing to do - */ - if (circuit->area) - isis_circuit_af_set(circuit, false, false); + isis_circuit_del(circuit); + return NB_OK; } @@ -2678,7 +2656,6 @@ int lib_interface_isis_circuit_type_modify(struct nb_cb_modify_args *args) struct interface *ifp; struct vrf *vrf; const char *ifname, *vrfname; - struct isis *isis = NULL; switch (args->event) { case NB_EV_VALIDATE: @@ -2694,11 +2671,7 @@ int lib_interface_isis_circuit_type_modify(struct nb_cb_modify_args *args) if (!ifp) break; - isis = isis_lookup_by_vrfid(ifp->vrf_id); - if (isis == NULL) - return NB_ERR_VALIDATION; - - circuit = circuit_lookup_by_ifp(ifp, isis->init_circ_list); + circuit = circuit_scan_by_ifp(ifp); if (circuit && circuit->state == C_STATE_UP && circuit->area->is_type != IS_LEVEL_1_AND_2 && circuit->area->is_type != circ_type) { @@ -3085,7 +3058,6 @@ int lib_interface_isis_network_type_modify(struct nb_cb_modify_args *args) int lib_interface_isis_passive_modify(struct nb_cb_modify_args *args) { struct isis_circuit *circuit; - struct isis_area *area; struct interface *ifp; bool passive = yang_dnode_get_bool(args->dnode, NULL); @@ -3108,14 +3080,7 @@ int lib_interface_isis_passive_modify(struct nb_cb_modify_args *args) return NB_OK; circuit = nb_running_get_entry(args->dnode, NULL, true); - if (circuit->state != C_STATE_UP) { - circuit->is_passive = passive; - } else { - area = circuit->area; - isis_csm_state_change(ISIS_DISABLE, circuit, area); - circuit->is_passive = passive; - isis_csm_state_change(ISIS_ENABLE, circuit, area); - } + isis_circuit_passive_set(circuit, passive); return NB_OK; } @@ -3473,15 +3438,18 @@ int lib_interface_isis_fast_reroute_level_1_lfa_enable_modify( circuit = nb_running_get_entry(args->dnode, NULL, true); circuit->lfa_protection[0] = yang_dnode_get_bool(args->dnode, NULL); - if (circuit->lfa_protection[0]) - circuit->area->lfa_protected_links[0]++; - else { - assert(circuit->area->lfa_protected_links[0] > 0); - circuit->area->lfa_protected_links[0]--; - } area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) { + if (circuit->lfa_protection[0]) + area->lfa_protected_links[0]++; + else { + assert(area->lfa_protected_links[0] > 0); + area->lfa_protected_links[0]--; + } + + lsp_regenerate_schedule(area, area->is_type, 0); + } return NB_OK; } @@ -3505,7 +3473,8 @@ int lib_interface_isis_fast_reroute_level_1_lfa_exclude_interface_create( isis_lfa_excluded_iface_add(circuit, ISIS_LEVEL1, exclude_ifname); area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) + lsp_regenerate_schedule(area, area->is_type, 0); return NB_OK; } @@ -3525,7 +3494,8 @@ int lib_interface_isis_fast_reroute_level_1_lfa_exclude_interface_destroy( isis_lfa_excluded_iface_delete(circuit, ISIS_LEVEL1, exclude_ifname); area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) + lsp_regenerate_schedule(area, area->is_type, 0); return NB_OK; } @@ -3545,15 +3515,18 @@ int lib_interface_isis_fast_reroute_level_1_remote_lfa_enable_modify( circuit = nb_running_get_entry(args->dnode, NULL, true); circuit->rlfa_protection[0] = yang_dnode_get_bool(args->dnode, NULL); - if (circuit->rlfa_protection[0]) - circuit->area->rlfa_protected_links[0]++; - else { - assert(circuit->area->rlfa_protected_links[0] > 0); - circuit->area->rlfa_protected_links[0]--; - } area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) { + if (circuit->rlfa_protection[0]) + area->rlfa_protected_links[0]++; + else { + assert(area->rlfa_protected_links[0] > 0); + area->rlfa_protected_links[0]--; + } + + lsp_regenerate_schedule(area, area->is_type, 0); + } return NB_OK; } @@ -3575,7 +3548,8 @@ int lib_interface_isis_fast_reroute_level_1_remote_lfa_maximum_metric_modify( circuit->rlfa_max_metric[0] = yang_dnode_get_uint32(args->dnode, NULL); area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) + lsp_regenerate_schedule(area, area->is_type, 0); return NB_OK; } @@ -3593,7 +3567,8 @@ int lib_interface_isis_fast_reroute_level_1_remote_lfa_maximum_metric_destroy( circuit->rlfa_max_metric[0] = 0; area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) + lsp_regenerate_schedule(area, area->is_type, 0); return NB_OK; } @@ -3613,15 +3588,18 @@ int lib_interface_isis_fast_reroute_level_1_ti_lfa_enable_modify( circuit = nb_running_get_entry(args->dnode, NULL, true); circuit->tilfa_protection[0] = yang_dnode_get_bool(args->dnode, NULL); - if (circuit->tilfa_protection[0]) - circuit->area->tilfa_protected_links[0]++; - else { - assert(circuit->area->tilfa_protected_links[0] > 0); - circuit->area->tilfa_protected_links[0]--; - } area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) { + if (circuit->tilfa_protection[0]) + area->tilfa_protected_links[0]++; + else { + assert(area->tilfa_protected_links[0] > 0); + area->tilfa_protected_links[0]--; + } + + lsp_regenerate_schedule(area, area->is_type, 0); + } return NB_OK; } @@ -3644,7 +3622,8 @@ int lib_interface_isis_fast_reroute_level_1_ti_lfa_node_protection_modify( yang_dnode_get_bool(args->dnode, NULL); area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) + lsp_regenerate_schedule(area, area->is_type, 0); return NB_OK; } @@ -3664,15 +3643,18 @@ int lib_interface_isis_fast_reroute_level_2_lfa_enable_modify( circuit = nb_running_get_entry(args->dnode, NULL, true); circuit->lfa_protection[1] = yang_dnode_get_bool(args->dnode, NULL); - if (circuit->lfa_protection[1]) - circuit->area->lfa_protected_links[1]++; - else { - assert(circuit->area->lfa_protected_links[1] > 0); - circuit->area->lfa_protected_links[1]--; - } area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) { + if (circuit->lfa_protection[1]) + area->lfa_protected_links[1]++; + else { + assert(area->lfa_protected_links[1] > 0); + area->lfa_protected_links[1]--; + } + + lsp_regenerate_schedule(area, area->is_type, 0); + } return NB_OK; } @@ -3696,7 +3678,8 @@ int lib_interface_isis_fast_reroute_level_2_lfa_exclude_interface_create( isis_lfa_excluded_iface_add(circuit, ISIS_LEVEL2, exclude_ifname); area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) + lsp_regenerate_schedule(area, area->is_type, 0); return NB_OK; } @@ -3716,7 +3699,8 @@ int lib_interface_isis_fast_reroute_level_2_lfa_exclude_interface_destroy( isis_lfa_excluded_iface_delete(circuit, ISIS_LEVEL2, exclude_ifname); area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) + lsp_regenerate_schedule(area, area->is_type, 0); return NB_OK; } @@ -3736,15 +3720,18 @@ int lib_interface_isis_fast_reroute_level_2_remote_lfa_enable_modify( circuit = nb_running_get_entry(args->dnode, NULL, true); circuit->rlfa_protection[1] = yang_dnode_get_bool(args->dnode, NULL); - if (circuit->rlfa_protection[1]) - circuit->area->rlfa_protected_links[1]++; - else { - assert(circuit->area->rlfa_protected_links[1] > 0); - circuit->area->rlfa_protected_links[1]--; - } area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) { + if (circuit->rlfa_protection[1]) + area->rlfa_protected_links[1]++; + else { + assert(area->rlfa_protected_links[1] > 0); + area->rlfa_protected_links[1]--; + } + + lsp_regenerate_schedule(area, area->is_type, 0); + } return NB_OK; } @@ -3766,7 +3753,8 @@ int lib_interface_isis_fast_reroute_level_2_remote_lfa_maximum_metric_modify( circuit->rlfa_max_metric[1] = yang_dnode_get_uint32(args->dnode, NULL); area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) + lsp_regenerate_schedule(area, area->is_type, 0); return NB_OK; } @@ -3784,7 +3772,8 @@ int lib_interface_isis_fast_reroute_level_2_remote_lfa_maximum_metric_destroy( circuit->rlfa_max_metric[1] = 0; area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) + lsp_regenerate_schedule(area, area->is_type, 0); return NB_OK; } @@ -3804,15 +3793,18 @@ int lib_interface_isis_fast_reroute_level_2_ti_lfa_enable_modify( circuit = nb_running_get_entry(args->dnode, NULL, true); circuit->tilfa_protection[1] = yang_dnode_get_bool(args->dnode, NULL); - if (circuit->tilfa_protection[1]) - circuit->area->tilfa_protected_links[1]++; - else { - assert(circuit->area->tilfa_protected_links[1] > 0); - circuit->area->tilfa_protected_links[1]--; - } area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) { + if (circuit->tilfa_protection[1]) + area->tilfa_protected_links[1]++; + else { + assert(area->tilfa_protected_links[1] > 0); + area->tilfa_protected_links[1]--; + } + + lsp_regenerate_schedule(area, area->is_type, 0); + } return NB_OK; } @@ -3835,7 +3827,8 @@ int lib_interface_isis_fast_reroute_level_2_ti_lfa_node_protection_modify( yang_dnode_get_bool(args->dnode, NULL); area = circuit->area; - lsp_regenerate_schedule(area, area->is_type, 0); + if (area) + lsp_regenerate_schedule(area, area->is_type, 0); return NB_OK; } diff --git a/isisd/isis_vty_fabricd.c b/isisd/isis_vty_fabricd.c index 6055984195..7020b6efeb 100644 --- a/isisd/isis_vty_fabricd.c +++ b/isisd/isis_vty_fabricd.c @@ -229,10 +229,10 @@ DEFUN (ip_router_isis, area = isis_area_lookup(area_tag, VRF_DEFAULT); if (!area) - area = isis_area_create(area_tag, VRF_DEFAULT_NAME); + isis_area_create(area_tag, VRF_DEFAULT_NAME); - if (!circuit || !circuit->area) { - circuit = isis_circuit_create(area, ifp); + if (!circuit) { + circuit = isis_circuit_new(ifp, area_tag); if (circuit->state != C_STATE_CONF && circuit->state != C_STATE_UP) { @@ -288,7 +288,7 @@ DEFUN (no_ip_router_isis, return CMD_ERR_NO_MATCH; } - circuit = circuit_lookup_by_ifp(ifp, area->circuit_list); + circuit = circuit_scan_by_ifp(ifp); if (!circuit) { vty_out(vty, "ISIS is not enabled on circuit %s\n", ifp->name); return CMD_ERR_NO_MATCH; @@ -301,6 +301,10 @@ DEFUN (no_ip_router_isis, ip = false; isis_circuit_af_set(circuit, ip, ipv6); + + if (!ip && !ipv6) + isis_circuit_del(circuit); + return CMD_SUCCESS; } diff --git a/isisd/isisd.c b/isisd/isisd.c index 714961c177..9730680de9 100644 --- a/isisd/isisd.c +++ b/isisd/isisd.c @@ -114,16 +114,6 @@ int show_isis_neighbor_common(struct vty *, const char *id, char, int clear_isis_neighbor_common(struct vty *, const char *id, const char *vrf_name, bool all_vrf); -static void isis_add(struct isis *isis) -{ - listnode_add(im->isis, isis); -} - -static void isis_delete(struct isis *isis) -{ - listnode_delete(im->isis, isis); -} - /* Link ISIS instance to VRF. */ void isis_vrf_link(struct isis *isis, struct vrf *vrf) { @@ -189,10 +179,8 @@ void isis_global_instance_create(const char *vrf_name) struct isis *isis; isis = isis_lookup_by_vrfname(vrf_name); - if (isis == NULL) { - isis = isis_new(vrf_name); - isis_add(isis); - } + if (isis == NULL) + isis_new(vrf_name); } struct isis *isis_new(const char *vrf_name) @@ -201,16 +189,15 @@ struct isis *isis_new(const char *vrf_name) struct isis *isis; isis = XCALLOC(MTYPE_ISIS, sizeof(struct isis)); + + isis->name = XSTRDUP(MTYPE_ISIS_NAME, vrf_name); + vrf = vrf_lookup_by_name(vrf_name); - if (vrf) { - isis->vrf_id = vrf->vrf_id; + if (vrf) isis_vrf_link(isis, vrf); - isis->name = XSTRDUP(MTYPE_ISIS_NAME, vrf->name); - } else { + else isis->vrf_id = VRF_UNKNOWN; - isis->name = XSTRDUP(MTYPE_ISIS_NAME, vrf_name); - } if (IS_DEBUG_EVENTS) zlog_debug( @@ -224,43 +211,81 @@ struct isis *isis_new(const char *vrf_name) isis->process_id = getpid(); isis->router_id = 0; isis->area_list = list_new(); - isis->init_circ_list = list_new(); isis->uptime = time(NULL); isis->snmp_notifications = 1; dyn_cache_init(isis); + listnode_add(im->isis, isis); + return isis; } +void isis_finish(struct isis *isis) +{ + struct vrf *vrf = NULL; + + listnode_delete(im->isis, isis); + + vrf = vrf_lookup_by_name(isis->name); + if (vrf) + isis_vrf_unlink(isis, vrf); + XFREE(MTYPE_ISIS_NAME, isis->name); + + isis_redist_free(isis); + list_delete(&isis->area_list); + XFREE(MTYPE_ISIS, isis); +} + +void isis_area_add_circuit(struct isis_area *area, struct isis_circuit *circuit) +{ + isis_csm_state_change(ISIS_ENABLE, circuit, area); + + area->ip_circuits += circuit->ip_router; + area->ipv6_circuits += circuit->ipv6_router; + + area->lfa_protected_links[0] += circuit->lfa_protection[0]; + area->rlfa_protected_links[0] += circuit->rlfa_protection[0]; + area->tilfa_protected_links[0] += circuit->tilfa_protection[0]; + + area->lfa_protected_links[1] += circuit->lfa_protection[1]; + area->rlfa_protected_links[1] += circuit->rlfa_protection[1]; + area->tilfa_protected_links[1] += circuit->tilfa_protection[1]; +} + +void isis_area_del_circuit(struct isis_area *area, struct isis_circuit *circuit) +{ + area->ip_circuits -= circuit->ip_router; + area->ipv6_circuits -= circuit->ipv6_router; + + area->lfa_protected_links[0] -= circuit->lfa_protection[0]; + area->rlfa_protected_links[0] -= circuit->rlfa_protection[0]; + area->tilfa_protected_links[0] -= circuit->tilfa_protection[0]; + + area->lfa_protected_links[1] -= circuit->lfa_protection[1]; + area->rlfa_protected_links[1] -= circuit->rlfa_protection[1]; + area->tilfa_protected_links[1] -= circuit->tilfa_protection[1]; + + isis_csm_state_change(ISIS_DISABLE, circuit, area); +} + struct isis_area *isis_area_create(const char *area_tag, const char *vrf_name) { struct isis_area *area; struct isis *isis = NULL; struct vrf *vrf = NULL; + struct interface *ifp; + struct isis_circuit *circuit; + area = XCALLOC(MTYPE_ISIS_AREA, sizeof(struct isis_area)); - if (vrf_name) { - vrf = vrf_lookup_by_name(vrf_name); - if (vrf) { - isis = isis_lookup_by_vrfid(vrf->vrf_id); - if (isis == NULL) { - isis = isis_new(vrf_name); - isis_add(isis); - } - } else { - isis = isis_lookup_by_vrfid(VRF_UNKNOWN); - if (isis == NULL) { - isis = isis_new(vrf_name); - isis_add(isis); - } - } - } else { - isis = isis_lookup_by_vrfid(VRF_DEFAULT); - if (isis == NULL) { - isis = isis_new(VRF_DEFAULT_NAME); - isis_add(isis); - } - } + if (!vrf_name) + vrf_name = VRF_DEFAULT_NAME; + + vrf = vrf_lookup_by_name(vrf_name); + isis = isis_lookup_by_vrfname(vrf_name); + + if (isis == NULL) + isis = isis_new(vrf_name); listnode_add(isis->area_list, area); area->isis = isis; @@ -373,9 +398,19 @@ struct isis_area *isis_area_create(const char *area_tag, const char *vrf_name) area->bfd_signalled_down = false; area->bfd_force_spf_refresh = false; - QOBJ_REG(area, isis_area); + if (vrf) { + FOR_ALL_INTERFACES (vrf, ifp) { + if (ifp->ifindex == IFINDEX_INTERNAL) + continue; + + circuit = ifp->info; + if (circuit) + isis_area_add_circuit(area, circuit); + } + } + return area; } @@ -470,11 +505,9 @@ void isis_area_destroy(struct isis_area *area) if (area->circuit_list) { for (ALL_LIST_ELEMENTS(area->circuit_list, node, nnode, - circuit)) { - circuit->ip_router = 0; - circuit->ipv6_router = 0; - isis_csm_state_change(ISIS_DISABLE, circuit, area); - } + circuit)) + isis_area_del_circuit(area, circuit); + list_delete(&area->circuit_list); } list_delete(&area->adjacency_list); @@ -572,10 +605,6 @@ static int isis_vrf_enable(struct vrf *vrf) isis = isis_lookup_by_vrfname(vrf->name); if (isis) { - if (isis->name && strmatch(vrf->name, VRF_DEFAULT_NAME)) { - XFREE(MTYPE_ISIS_NAME, isis->name); - isis->name = NULL; - } old_vrf_id = isis->vrf_id; /* We have instance configured, link to VRF and make it "up". */ isis_vrf_link(isis, vrf); @@ -630,28 +659,6 @@ void isis_vrf_init(void) isis_vrf_delete, isis_vrf_enable); } -void isis_finish(struct isis *isis) -{ - struct vrf *vrf = NULL; - - isis_delete(isis); - if (isis->name) { - vrf = vrf_lookup_by_name(isis->name); - if (vrf) - isis_vrf_unlink(isis, vrf); - XFREE(MTYPE_ISIS_NAME, isis->name); - } else { - vrf = vrf_lookup_by_id(VRF_DEFAULT); - if (vrf) - isis_vrf_unlink(isis, vrf); - } - - isis_redist_free(isis); - list_delete(&isis->area_list); - list_delete(&isis->init_circ_list); - XFREE(MTYPE_ISIS, isis); -} - void isis_terminate() { struct isis *isis; diff --git a/isisd/isisd.h b/isisd/isisd.h index fd0e73eb39..79717b0cbb 100644 --- a/isisd/isisd.h +++ b/isisd/isisd.h @@ -89,7 +89,6 @@ struct isis { uint8_t sysid[ISIS_SYS_ID_LEN]; /* SystemID for this IS */ uint32_t router_id; /* Router ID from zebra */ struct list *area_list; /* list of IS-IS areas */ - struct list *init_circ_list; uint8_t max_area_addrs; /* maximumAreaAdresses */ struct area_addr *man_area_addrs; /* manualAreaAddresses */ time_t uptime; /* when did we start */ @@ -244,7 +243,6 @@ DECLARE_MTYPE(ISIS_AREA_ADDR); /* isis_area->area_addrs */ DECLARE_HOOK(isis_area_overload_bit_update, (struct isis_area * area), (area)); void isis_terminate(void); -void isis_finish(struct isis *isis); void isis_master_init(struct thread_master *master); void isis_vrf_link(struct isis *isis, struct vrf *vrf); void isis_vrf_unlink(struct isis *isis, struct vrf *vrf); @@ -257,6 +255,13 @@ void isis_init(void); void isis_vrf_init(void); struct isis *isis_new(const char *vrf_name); +void isis_finish(struct isis *isis); + +void isis_area_add_circuit(struct isis_area *area, + struct isis_circuit *circuit); +void isis_area_del_circuit(struct isis_area *area, + struct isis_circuit *circuit); + struct isis_area *isis_area_create(const char *, const char *); struct isis_area *isis_area_lookup(const char *, vrf_id_t vrf_id); struct isis_area *isis_area_lookup_by_vrf(const char *area_tag, diff --git a/tests/isisd/test_isis_spf.c b/tests/isisd/test_isis_spf.c index e06944a037..8fe1ad0b8a 100644 --- a/tests/isisd/test_isis_spf.c +++ b/tests/isisd/test_isis_spf.c @@ -556,7 +556,6 @@ int main(int argc, char **argv) /* IS-IS inits. */ yang_module_load("frr-isisd"); isis = isis_new(VRF_DEFAULT_NAME); - listnode_add(im->isis, isis); SET_FLAG(im->options, F_ISIS_UNIT_TEST); debug_spf_events |= DEBUG_SPF_EVENTS; debug_lfa |= DEBUG_LFA; diff --git a/tests/topotests/isis-snmp/test_isis_snmp.py b/tests/topotests/isis-snmp/test_isis_snmp.py index 07f3335e23..04e043847d 100755 --- a/tests/topotests/isis-snmp/test_isis_snmp.py +++ b/tests/topotests/isis-snmp/test_isis_snmp.py @@ -243,15 +243,15 @@ def test_r1_scalar_snmp(): circtable_test = { - "isisCircAdminState": ["on(1)", "on(1)", "on(1)"], - "isisCircExistState": ["active(1)", "active(1)", "active(1)"], - "isisCircType": ["broadcast(1)", "ptToPt(2)", "staticIn(3)"], - "isisCircExtDomain": ["false(2)", "false(2)", "false(2)"], - "isisCircLevelType": ["level1(1)", "level1(1)", "level1and2(3)"], - "isisCircPassiveCircuit": ["false(2)", "false(2)", "true(1)"], - "isisCircMeshGroupEnabled": ["inactive(1)", "inactive(1)", "inactive(1)"], - "isisCircSmallHellos": ["false(2)", "false(2)", "false(2)"], - "isisCirc3WayEnabled": ["false(2)", "false(2)", "false(2)"], + "isisCircAdminState": ["on(1)", "on(1)"], + "isisCircExistState": ["active(1)", "active(1)"], + "isisCircType": ["broadcast(1)", "ptToPt(2)"], + "isisCircExtDomain": ["false(2)", "false(2)"], + "isisCircLevelType": ["level1(1)", "level1(1)"], + "isisCircPassiveCircuit": ["false(2)", "false(2)"], + "isisCircMeshGroupEnabled": ["inactive(1)", "inactive(1)"], + "isisCircSmallHellos": ["false(2)", "false(2)"], + "isisCirc3WayEnabled": ["false(2)", "false(2)"], } @@ -266,7 +266,6 @@ def test_r1_isisCircTable(): oids = [] oids.append(generate_oid(1, 1, 0)) oids.append(generate_oid(1, 2, 0)) - oids.append(generate_oid(1, 3, 0)) # check items for item in circtable_test.keys(): @@ -277,21 +276,17 @@ def test_r1_isisCircTable(): circleveltable_test = { - "isisCircLevelMetric": ["10", "10", "10", "10"], - "isisCircLevelWideMetric": ["10", "10", "0", "0"], - "isisCircLevelISPriority": ["64", "64", "64", "64"], - "isisCircLevelHelloMultiplier": ["10", "10", "10", "10"], + "isisCircLevelMetric": ["10", "10"], + "isisCircLevelWideMetric": ["10", "10"], + "isisCircLevelISPriority": ["64", "64"], + "isisCircLevelHelloMultiplier": ["10", "10"], "isisCircLevelHelloTimer": [ "3000 milliseconds", "3000 milliseconds", - "3000 milliseconds", - "3000 milliseconds", ], "isisCircLevelMinLSPRetransInt": [ "1 seconds", "1 seconds", - "0 seconds", - "0 seconds", ], } @@ -307,8 +302,6 @@ def test_r1_isislevelCircTable(): oids = [] oids.append(generate_oid(2, 1, "area")) oids.append(generate_oid(2, 2, "area")) - oids.append(generate_oid(2, 3, "area")) - oids.append(generate_oid(2, 3, "domain")) # check items for item in circleveltable_test.keys(): diff --git a/tests/topotests/isis-sr-topo1/test_isis_sr_topo1.py b/tests/topotests/isis-sr-topo1/test_isis_sr_topo1.py index 148a89474e..c22bd65d2d 100644 --- a/tests/topotests/isis-sr-topo1/test_isis_sr_topo1.py +++ b/tests/topotests/isis-sr-topo1/test_isis_sr_topo1.py @@ -1002,6 +1002,12 @@ def test_isis_adjacencies_step12(): tgen.net["rt4"].cmd( 'vtysh -c "conf t" -c "interface eth-rt5" -c "ipv6 router isis 1"' ) + tgen.net["rt4"].cmd( + 'vtysh -c "conf t" -c "interface eth-rt5" -c "isis network point-to-point"' + ) + tgen.net["rt4"].cmd( + 'vtysh -c "conf t" -c "interface eth-rt5" -c "isis hello-multiplier 3"' + ) tgen.net["rt6"].cmd( 'vtysh -c "conf t" -c "router isis 1" -c "segment-routing global-block 16000 23999"' ) From ec62fbaa07998df71a0bfce4a0383556bb604521 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 29 Apr 2021 01:59:56 +0300 Subject: [PATCH 04/11] isisd: fix ldp-sync configuration YANG model and CLI commands allow user to configure LDP-sync per area. But the actual implementation is incorrect - all commands are changing the config for the whole VRF instead of a single area. This commit fixes this issue by actually implementing per area configuration. Fixes #8578. Signed-off-by: Igor Ryzhov --- isisd/isis_circuit.c | 9 + isisd/isis_ldp_sync.c | 315 +++++++----------- isisd/isis_ldp_sync.h | 11 +- isisd/isis_nb_config.c | 141 ++------ isisd/isis_zebra.c | 4 + isisd/isisd.h | 3 +- .../r3/show_isis_ldp_sync.ref | 2 +- .../show_isis_ldp_sync_r1_eth1_shutdown.ref | 2 +- .../show_isis_ldp_sync_r2_eth1_shutdown.ref | 2 +- yang/frr-isisd.yang | 8 +- 10 files changed, 176 insertions(+), 321 deletions(-) diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c index efc7fec541..7fd9c07ed2 100644 --- a/isisd/isis_circuit.c +++ b/isisd/isis_circuit.c @@ -174,6 +174,9 @@ struct isis_circuit *isis_circuit_new(struct interface *ifp, const char *tag) isis_lfa_excluded_ifaces_init(circuit, ISIS_LEVEL1); isis_lfa_excluded_ifaces_init(circuit, ISIS_LEVEL2); + circuit->ldp_sync_info = ldp_sync_info_create(); + circuit->ldp_sync_info->enabled = LDP_IGP_SYNC_ENABLED; + QOBJ_REG(circuit, isis_circuit); isis_circuit_if_bind(circuit, ifp); @@ -196,6 +199,8 @@ void isis_circuit_del(struct isis_circuit *circuit) QOBJ_UNREG(circuit); + ldp_sync_info_free(&circuit->ldp_sync_info); + circuit_mt_finish(circuit); isis_lfa_excluded_ifaces_clear(circuit, ISIS_LEVEL1); isis_lfa_excluded_ifaces_clear(circuit, ISIS_LEVEL2); @@ -751,6 +756,8 @@ int isis_circuit_up(struct isis_circuit *circuit) if (circuit->area->mta && circuit->area->mta->status) isis_link_params_update(circuit, circuit->interface); + isis_if_ldp_sync_enable(circuit); + #ifndef FABRICD /* send northbound notification */ isis_notif_if_state_change(circuit, false); @@ -766,6 +773,8 @@ void isis_circuit_down(struct isis_circuit *circuit) isis_notif_if_state_change(circuit, true); #endif /* ifndef FABRICD */ + isis_if_ldp_sync_disable(circuit); + /* log adjacency changes if configured to do so */ if (circuit->area->log_adj_changes) { struct isis_adjacency *adj = NULL; diff --git a/isisd/isis_ldp_sync.c b/isisd/isis_ldp_sync.c index 585f769806..62d8b8334a 100644 --- a/isisd/isis_ldp_sync.c +++ b/isisd/isis_ldp_sync.c @@ -65,28 +65,20 @@ int isis_ldp_sync_state_update(struct ldp_igp_sync_if_state state) struct interface *ifp; struct isis_circuit *circuit = NULL; struct isis_area *area; - struct listnode *node; - struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT); - - /* if isis is not enabled or LDP-SYNC is not configured ignore */ - if (!isis || - !CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) - return 0; /* lookup circuit */ ifp = if_lookup_by_index(state.ifindex, VRF_DEFAULT); if (ifp == NULL) return 0; - for (ALL_LIST_ELEMENTS_RO(isis->area_list, node, area)) { - circuit = circuit_lookup_by_ifp(ifp, area->circuit_list); - if (circuit != NULL) - break; - } + circuit = ifp->info; + if (circuit == NULL) + return 0; /* if isis is not enabled or LDP-SYNC is not configured ignore */ - if (circuit == NULL || - !CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) + area = circuit->area; + if (area == NULL + || !CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) return 0; /* received ldp-sync interface state from LDP */ @@ -103,15 +95,12 @@ int isis_ldp_sync_state_update(struct ldp_igp_sync_if_state state) int isis_ldp_sync_announce_update(struct ldp_igp_sync_announce announce) { struct isis_area *area; - struct listnode *node; - struct vrf *vrf; - struct interface *ifp; + struct listnode *anode, *cnode; struct isis_circuit *circuit; struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT); - /* if isis is not enabled or LDP-SYNC is not configured ignore */ - if (!isis || - !CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) + /* if isis is not enabled ignore */ + if (!isis) return 0; if (announce.proto != ZEBRA_ROUTE_LDP) @@ -123,15 +112,12 @@ int isis_ldp_sync_announce_update(struct ldp_igp_sync_announce announce) * set cost to LSInfinity * send request to LDP for LDP-SYNC state for each interface */ - vrf = vrf_lookup_by_id(VRF_DEFAULT); - FOR_ALL_INTERFACES (vrf, ifp) { - for (ALL_LIST_ELEMENTS_RO(isis->area_list, node, area)) { - circuit = circuit_lookup_by_ifp(ifp, - area->circuit_list); - if (circuit == NULL) - continue; + for (ALL_LIST_ELEMENTS_RO(isis->area_list, anode, area)) { + if (!CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) + continue; + + for (ALL_LIST_ELEMENTS_RO(area->circuit_list, cnode, circuit)) isis_ldp_sync_if_start(circuit, true); - } } return 0; @@ -157,32 +143,6 @@ void isis_ldp_sync_state_req_msg(struct isis_circuit *circuit) /* * LDP-SYNC general interface routines */ -void isis_ldp_sync_if_init(struct isis_circuit *circuit, struct isis *isis) -{ - struct ldp_sync_info *ldp_sync_info; - struct interface *ifp = circuit->interface; - - /* called when ISIS is configured on an interface - * if LDP-IGP Sync is configured globally set state - * and if ptop interface LDP LDP-SYNC is enabled - */ - ils_debug("ldp_sync: init if %s ", ifp->name); - if (circuit->ldp_sync_info == NULL) - circuit->ldp_sync_info = ldp_sync_info_create(); - ldp_sync_info = circuit->ldp_sync_info; - - /* specifed on interface overrides global config. */ - if (!CHECK_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_HOLDDOWN)) - ldp_sync_info->holddown = isis->ldp_sync_cmd.holddown; - - if (!CHECK_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_IF_CONFIG)) - ldp_sync_info->enabled = LDP_IGP_SYNC_ENABLED; - - if ((circuit->circ_type == CIRCUIT_T_P2P || if_is_pointopoint(ifp)) && - ldp_sync_info->enabled == LDP_IGP_SYNC_ENABLED) - ldp_sync_info->state = LDP_IGP_SYNC_STATE_REQUIRED_NOT_UP; -} - void isis_ldp_sync_if_start(struct isis_circuit *circuit, bool send_state_req) { @@ -251,49 +211,17 @@ void isis_ldp_sync_ldp_fail(struct isis_circuit *circuit) } } -void isis_ldp_sync_if_remove(struct isis_circuit *circuit, bool remove) -{ - struct ldp_sync_info *ldp_sync_info; - - if (circuit->ldp_sync_info == NULL) - return; - - ldp_sync_info = circuit->ldp_sync_info; - - /* Stop LDP-SYNC on this interface: - * if holddown timer is running stop it - * delete ldp instance on interface - * restore metric - */ - ils_debug("ldp_sync: remove if %s", circuit->interface - ? circuit->interface->name : ""); - - THREAD_OFF(ldp_sync_info->t_holddown); - ldp_sync_info->state = LDP_IGP_SYNC_STATE_NOT_REQUIRED; - isis_ldp_sync_set_if_metric(circuit, true); - if (remove) { - /* ISIS instance being removed free ldp-sync info */ - ldp_sync_info_free((struct ldp_sync_info **)&(ldp_sync_info)); - circuit->ldp_sync_info = NULL; - } -} - static int isis_ldp_sync_adj_state_change(struct isis_adjacency *adj) { struct isis_circuit *circuit = adj->circuit; - struct ldp_sync_info *ldp_sync_info; - struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT); + struct ldp_sync_info *ldp_sync_info = circuit->ldp_sync_info; + struct isis_area *area = circuit->area; - if (!isis || - !CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE) || - circuit->interface->vrf_id != VRF_DEFAULT || - if_is_loopback(circuit->interface)) + if (!CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE) + || circuit->interface->vrf_id != VRF_DEFAULT + || if_is_loopback(circuit->interface)) return 0; - if (circuit->ldp_sync_info == NULL) - isis_ldp_sync_if_init(circuit, isis); - ldp_sync_info = circuit->ldp_sync_info; - if (ldp_sync_info->enabled != LDP_IGP_SYNC_ENABLED) return 0; @@ -329,16 +257,15 @@ bool isis_ldp_sync_if_metric_config(struct isis_circuit *circuit, int level, int metric) { struct ldp_sync_info *ldp_sync_info = circuit->ldp_sync_info; - struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT); + struct isis_area *area = circuit->area; /* configured interface metric has been changed: * if LDP-IGP Sync is running and metric has been set to LSInfinity * change saved value so when ldp-sync completes proper metric is * restored */ - if (isis && - CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE) && - ldp_sync_info != NULL) { + if (area && CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE) + && ldp_sync_info != NULL) { if (CHECK_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_SET_METRIC)) { @@ -471,15 +398,12 @@ void isis_ldp_sync_holddown_timer_add(struct isis_circuit *circuit) void isis_ldp_sync_handle_client_close(struct zapi_client_close_info *info) { struct isis_area *area; - struct listnode *node; + struct listnode *anode, *cnode; struct isis_circuit *circuit; - struct interface *ifp; - struct vrf *vrf = vrf_lookup_by_id(VRF_DEFAULT); struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT); - /* if isis is not enabled or LDP-SYNC is not configured ignore */ - if (!isis - || !CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) + /* if isis is not enabled ignore */ + if (!isis) return; /* Check if the LDP main client session closed */ @@ -492,14 +416,12 @@ void isis_ldp_sync_handle_client_close(struct zapi_client_close_info *info) */ zlog_err("ldp_sync: LDP down"); - FOR_ALL_INTERFACES (vrf, ifp) { - for (ALL_LIST_ELEMENTS_RO(isis->area_list, node, area)) { - circuit = - circuit_lookup_by_ifp(ifp, area->circuit_list); - if (circuit == NULL) - continue; + for (ALL_LIST_ELEMENTS_RO(isis->area_list, anode, area)) { + if (!CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) + continue; + + for (ALL_LIST_ELEMENTS_RO(area->circuit_list, cnode, circuit)) isis_ldp_sync_ldp_fail(circuit); - } } } @@ -507,110 +429,130 @@ void isis_ldp_sync_handle_client_close(struct zapi_client_close_info *info) * LDP-SYNC routes used by set commands. */ -void isis_if_set_ldp_sync_enable(struct isis_circuit *circuit) +void isis_area_ldp_sync_enable(struct isis_area *area) { - struct ldp_sync_info *ldp_sync_info; - struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT); + struct isis_circuit *circuit; + struct listnode *node; + + if (!CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) { + SET_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE); + + for (ALL_LIST_ELEMENTS_RO(area->circuit_list, node, circuit)) + isis_if_ldp_sync_enable(circuit); + } +} + +void isis_area_ldp_sync_disable(struct isis_area *area) +{ + struct isis_circuit *circuit; + struct listnode *node; + + if (CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) { + for (ALL_LIST_ELEMENTS_RO(area->circuit_list, node, circuit)) + isis_if_ldp_sync_disable(circuit); + + UNSET_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE); + + UNSET_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_HOLDDOWN); + area->ldp_sync_cmd.holddown = LDP_IGP_SYNC_HOLDDOWN_DEFAULT; + } +} + +void isis_area_ldp_sync_set_holddown(struct isis_area *area, uint16_t holddown) +{ + struct isis_circuit *circuit; + struct listnode *node; + + if (holddown == LDP_IGP_SYNC_HOLDDOWN_DEFAULT) + UNSET_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_HOLDDOWN); + else + SET_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_HOLDDOWN); + + area->ldp_sync_cmd.holddown = holddown; + + for (ALL_LIST_ELEMENTS_RO(area->circuit_list, node, circuit)) + isis_if_set_ldp_sync_holddown(circuit); +} + +void isis_if_ldp_sync_enable(struct isis_circuit *circuit) +{ + struct ldp_sync_info *ldp_sync_info = circuit->ldp_sync_info; + struct isis_area *area = circuit->area; /* called when setting LDP-SYNC at the global level: * specifed on interface overrides global config * if ptop link send msg to LDP indicating ldp-sync enabled */ - if (!isis || if_is_loopback(circuit->interface)) + if (if_is_loopback(circuit->interface)) return; - if (CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) { - if (circuit->ldp_sync_info == NULL) - isis_ldp_sync_if_init(circuit, isis); - ldp_sync_info = circuit->ldp_sync_info; + ils_debug("ldp_sync: enable if %s", circuit->interface->name); - /* config on interface, overrides global config. */ - if (CHECK_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_IF_CONFIG)) - if (ldp_sync_info->enabled != LDP_IGP_SYNC_ENABLED) - return; + if (!CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) + return; - ldp_sync_info->enabled = LDP_IGP_SYNC_ENABLED; - ils_debug("ldp_sync: enable if %s", circuit->interface->name); + /* config on interface, overrides global config. */ + if (CHECK_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_IF_CONFIG)) + if (ldp_sync_info->enabled != LDP_IGP_SYNC_ENABLED) + return; - /* send message to LDP if ptop link */ - if (circuit->circ_type == CIRCUIT_T_P2P || - if_is_pointopoint(circuit->interface)) { - ldp_sync_info->state = - LDP_IGP_SYNC_STATE_REQUIRED_NOT_UP; - isis_ldp_sync_state_req_msg(circuit); - } else { - ldp_sync_info->state = LDP_IGP_SYNC_STATE_NOT_REQUIRED; - zlog_debug("ldp_sync: Sync only runs on P2P links %s", - circuit->interface->name); - } - } else - /* delete LDP sync even if configured on an interface */ - isis_ldp_sync_if_remove(circuit, false); + if (!CHECK_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_HOLDDOWN)) + ldp_sync_info->holddown = area->ldp_sync_cmd.holddown; + + if (circuit->circ_type == CIRCUIT_T_P2P + || if_is_pointopoint(circuit->interface)) { + ldp_sync_info->state = LDP_IGP_SYNC_STATE_REQUIRED_NOT_UP; + isis_ldp_sync_state_req_msg(circuit); + } else { + ldp_sync_info->state = LDP_IGP_SYNC_STATE_NOT_REQUIRED; + ils_debug("ldp_sync: Sync only runs on P2P links %s", + circuit->interface->name); + } +} + +void isis_if_ldp_sync_disable(struct isis_circuit *circuit) +{ + struct ldp_sync_info *ldp_sync_info = circuit->ldp_sync_info; + struct isis_area *area = circuit->area; + + /* Stop LDP-SYNC on this interface: + * if holddown timer is running stop it + * delete ldp instance on interface + * restore metric + */ + if (if_is_loopback(circuit->interface)) + return; + + ils_debug("ldp_sync: remove if %s", circuit->interface->name); + + if (!CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) + return; + + THREAD_OFF(ldp_sync_info->t_holddown); + ldp_sync_info->state = LDP_IGP_SYNC_STATE_NOT_REQUIRED; + isis_ldp_sync_set_if_metric(circuit, true); } void isis_if_set_ldp_sync_holddown(struct isis_circuit *circuit) { - struct ldp_sync_info *ldp_sync_info; - struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT); + struct ldp_sync_info *ldp_sync_info = circuit->ldp_sync_info; + struct isis_area *area = circuit->area; /* called when setting LDP-SYNC at the global level: * specifed on interface overrides global config. */ - if (!isis || if_is_loopback(circuit->interface)) + if (if_is_loopback(circuit->interface)) return; - if (circuit->ldp_sync_info == NULL) - isis_ldp_sync_if_init(circuit, isis); - ldp_sync_info = circuit->ldp_sync_info; - /* config on interface, overrides global config. */ if (CHECK_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_HOLDDOWN)) return; - if (CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_HOLDDOWN)) - ldp_sync_info->holddown = isis->ldp_sync_cmd.holddown; + if (CHECK_FLAG(area->ldp_sync_cmd.flags, LDP_SYNC_FLAG_HOLDDOWN)) + ldp_sync_info->holddown = area->ldp_sync_cmd.holddown; else ldp_sync_info->holddown = LDP_IGP_SYNC_HOLDDOWN_DEFAULT; } -void isis_ldp_sync_gbl_exit(bool remove) -{ - struct isis_area *area; - struct listnode *node; - struct isis_circuit *circuit; - struct interface *ifp; - struct vrf *vrf = vrf_lookup_by_id(VRF_DEFAULT); - struct isis *isis = isis_lookup_by_vrfid(VRF_DEFAULT); - - /* if you delete LDP-SYNC at a gobal level is clears all LDP-SYNC - * configuration, even interface configuration - */ - if (isis && - CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) { - /* register with opaque client to recv LDP-IGP Sync msgs */ - zclient_unregister_opaque(zclient, - LDP_IGP_SYNC_IF_STATE_UPDATE); - zclient_unregister_opaque(zclient, - LDP_IGP_SYNC_ANNOUNCE_UPDATE); - - /* disable LDP-SYNC globally */ - UNSET_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE); - UNSET_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_HOLDDOWN); - isis->ldp_sync_cmd.holddown = LDP_IGP_SYNC_HOLDDOWN_DEFAULT; - - /* remove LDP-SYNC on all ISIS interfaces */ - FOR_ALL_INTERFACES (vrf, ifp) { - for (ALL_LIST_ELEMENTS_RO(isis->area_list, node, - area)) { - circuit = circuit_lookup_by_ifp(ifp, - area->circuit_list); - if (circuit == NULL) - continue; - isis_ldp_sync_if_remove(circuit, remove); - } - } - } -} - /* * LDP-SYNC routines used by show commands. */ @@ -693,11 +635,6 @@ DEFUN (show_isis_mpls_ldp_interface, return CMD_SUCCESS; } - if (!CHECK_FLAG(isis->ldp_sync_cmd.flags, LDP_SYNC_FLAG_ENABLE)) { - vty_out(vty, "LDP-sync is disabled\n"); - return CMD_SUCCESS; - } - if (argv_find(argv, argc, "INTERFACE", &idx_intf)) ifname = argv[idx_intf]->arg; diff --git a/isisd/isis_ldp_sync.h b/isisd/isis_ldp_sync.h index 977c5ba0de..69a1800007 100644 --- a/isisd/isis_ldp_sync.h +++ b/isisd/isis_ldp_sync.h @@ -29,13 +29,15 @@ zlog_debug(__VA_ARGS__); \ } while (0) -extern void isis_if_set_ldp_sync_enable(struct isis_circuit *circuit); +extern void isis_area_ldp_sync_enable(struct isis_area *area); +extern void isis_area_ldp_sync_disable(struct isis_area *area); +extern void isis_area_ldp_sync_set_holddown(struct isis_area *area, + uint16_t holddown); +extern void isis_if_ldp_sync_enable(struct isis_circuit *circuit); +extern void isis_if_ldp_sync_disable(struct isis_circuit *circuit); extern void isis_if_set_ldp_sync_holddown(struct isis_circuit *circuit); -extern void isis_ldp_sync_if_init(struct isis_circuit *circuit, - struct isis *isis); extern void isis_ldp_sync_if_start(struct isis_circuit *circuit, bool send_state_req); -extern void isis_ldp_sync_if_remove(struct isis_circuit *circuit, bool remove); extern void isis_ldp_sync_if_complete(struct isis_circuit *circuit); extern void isis_ldp_sync_holddown_timer_add(struct isis_circuit *circuit); extern void @@ -49,5 +51,4 @@ extern void isis_ldp_sync_set_if_metric(struct isis_circuit *circuit, extern bool isis_ldp_sync_if_metric_config(struct isis_circuit *circuit, int level, int metric); extern void isis_ldp_sync_init(void); -extern void isis_ldp_sync_gbl_exit(bool remove); #endif /* _ZEBRA_ISIS_LDP_SYNC_H */ diff --git a/isisd/isis_nb_config.c b/isisd/isis_nb_config.c index 4b68cd3bed..5cad9fcfcb 100644 --- a/isisd/isis_nb_config.c +++ b/isisd/isis_nb_config.c @@ -32,7 +32,6 @@ #include "spf_backoff.h" #include "lib_errors.h" #include "vrf.h" -#include "zclient.h" #include "ldp_sync.h" #include "isisd/isisd.h" @@ -56,8 +55,6 @@ DEFINE_MTYPE_STATIC(ISISD, ISIS_MPLS_TE, "ISIS MPLS_TE parameters"); DEFINE_MTYPE_STATIC(ISISD, ISIS_PLIST_NAME, "ISIS prefix-list name"); -extern struct zclient *zclient; - /* * XPath: /frr-isisd:isis/instance */ @@ -87,16 +84,10 @@ int isis_instance_create(struct nb_cb_create_args *args) int isis_instance_destroy(struct nb_cb_destroy_args *args) { struct isis_area *area; - vrf_id_t vrf_id; if (args->event != NB_EV_APPLY) return NB_OK; area = nb_running_unset_entry(args->dnode); - vrf_id = area->isis->vrf_id; - - /* remove ldp-sync config */ - if (vrf_id == VRF_DEFAULT) - isis_ldp_sync_gbl_exit(true); isis_area_destroy(area); return NB_OK; @@ -2369,11 +2360,6 @@ 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; - struct listnode *node; - struct isis_circuit *circuit; - struct interface *ifp; - struct vrf *vrf; - struct isis *isis; switch (args->event) { case NB_EV_VALIDATE: @@ -2392,30 +2378,7 @@ int isis_instance_mpls_ldp_sync_create(struct nb_cb_create_args *args) break; case NB_EV_APPLY: area = nb_running_get_entry(args->dnode, NULL, true); - isis = area->isis; - vrf = vrf_lookup_by_id(isis->vrf_id); - - /* register with opaque client to recv LDP-IGP Sync msgs */ - zclient_register_opaque(zclient, LDP_IGP_SYNC_IF_STATE_UPDATE); - zclient_register_opaque(zclient, LDP_IGP_SYNC_ANNOUNCE_UPDATE); - - if (!CHECK_FLAG(isis->ldp_sync_cmd.flags, - LDP_SYNC_FLAG_ENABLE)) { - SET_FLAG(isis->ldp_sync_cmd.flags, - LDP_SYNC_FLAG_ENABLE); - - /* turn on LDP-IGP Sync on all ptop ISIS interfaces */ - FOR_ALL_INTERFACES (vrf, ifp) { - for (ALL_LIST_ELEMENTS_RO(isis->area_list, node, - area)) { - circuit = circuit_lookup_by_ifp( - ifp, area->circuit_list); - if (circuit == NULL) - continue; - isis_if_set_ldp_sync_enable(circuit); - } - } - } + isis_area_ldp_sync_enable(area); break; } return NB_OK; @@ -2423,11 +2386,13 @@ int isis_instance_mpls_ldp_sync_create(struct nb_cb_create_args *args) int isis_instance_mpls_ldp_sync_destroy(struct nb_cb_destroy_args *args) { + struct isis_area *area; + if (args->event != NB_EV_APPLY) return NB_OK; - /* remove ldp-sync config */ - isis_ldp_sync_gbl_exit(false); + area = nb_running_get_entry(args->dnode, NULL, true); + isis_area_ldp_sync_disable(area); return NB_OK; } @@ -2438,12 +2403,7 @@ int isis_instance_mpls_ldp_sync_destroy(struct nb_cb_destroy_args *args) int isis_instance_mpls_ldp_sync_holddown_modify(struct nb_cb_modify_args *args) { struct isis_area *area; - struct listnode *node; - struct isis_circuit *circuit; - struct interface *ifp; - struct vrf *vrf; - uint16_t holddown = LDP_IGP_SYNC_HOLDDOWN_DEFAULT; - struct isis *isis; + uint16_t holddown; switch (args->event) { case NB_EV_VALIDATE: @@ -2462,29 +2422,8 @@ int isis_instance_mpls_ldp_sync_holddown_modify(struct nb_cb_modify_args *args) break; case NB_EV_APPLY: area = nb_running_get_entry(args->dnode, NULL, true); - isis = area->isis; - vrf = vrf_lookup_by_id(isis->vrf_id); holddown = yang_dnode_get_uint16(args->dnode, NULL); - - if (holddown == LDP_IGP_SYNC_HOLDDOWN_DEFAULT) - UNSET_FLAG(isis->ldp_sync_cmd.flags, - LDP_SYNC_FLAG_HOLDDOWN); - else - SET_FLAG(isis->ldp_sync_cmd.flags, - LDP_SYNC_FLAG_HOLDDOWN); - isis->ldp_sync_cmd.holddown = holddown; - - /* set holddown time on all ISIS interfaces */ - FOR_ALL_INTERFACES (vrf, ifp) { - for (ALL_LIST_ELEMENTS_RO(isis->area_list, node, - area)) { - circuit = circuit_lookup_by_ifp(ifp, - area->circuit_list); - if (circuit == NULL) - continue; - isis_if_set_ldp_sync_holddown(circuit); - } - } + isis_area_ldp_sync_set_holddown(area, holddown); break; } return NB_OK; @@ -2562,9 +2501,6 @@ int lib_interface_isis_destroy(struct nb_cb_destroy_args *args) circuit = nb_running_unset_entry(args->dnode); - /* remove ldp-sync config */ - isis_ldp_sync_if_remove(circuit, true); - isis_circuit_del(circuit); return NB_OK; @@ -3283,13 +3219,12 @@ 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 isis *isis; switch (args->event) { case NB_EV_VALIDATE: circuit = nb_running_get_entry(args->dnode, NULL, false); if (circuit == NULL || circuit->area == NULL) - return NB_ERR_VALIDATION; + break; if (circuit->isis->vrf_id != VRF_DEFAULT) { snprintf(args->errmsg, args->errmsg_len, @@ -3303,39 +3238,17 @@ int lib_interface_isis_mpls_ldp_sync_modify(struct nb_cb_modify_args *args) case NB_EV_APPLY: circuit = nb_running_get_entry(args->dnode, NULL, true); ldp_sync_enable = yang_dnode_get_bool(args->dnode, NULL); - isis = circuit->isis; - if (circuit->ldp_sync_info == NULL) - isis_ldp_sync_if_init(circuit, isis); - assert(circuit->ldp_sync_info != NULL); ldp_sync_info = circuit->ldp_sync_info; - if (ldp_sync_enable) { - /* enable LDP-SYNC on an interface - * if ptop interface send message to LDP to get state - */ - SET_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_IF_CONFIG); - ldp_sync_info->enabled = LDP_IGP_SYNC_ENABLED; - if (circuit->circ_type == CIRCUIT_T_P2P) { - ldp_sync_info->state = - LDP_IGP_SYNC_STATE_REQUIRED_NOT_UP; - isis_ldp_sync_state_req_msg(circuit); - } else { - zlog_debug("ldp_sync: only runs on P2P links %s", - circuit->interface->name); - ldp_sync_info->state = - LDP_IGP_SYNC_STATE_NOT_REQUIRED; - } - } else { - /* disable LDP-SYNC on an interface - * stop holddown timer if running - * restore isis metric - */ - SET_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_IF_CONFIG); - ldp_sync_info->enabled = LDP_IGP_SYNC_DEFAULT; - ldp_sync_info->state = LDP_IGP_SYNC_STATE_NOT_REQUIRED; - THREAD_OFF(ldp_sync_info->t_holddown); - isis_ldp_sync_set_if_metric(circuit, true); + SET_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_IF_CONFIG); + ldp_sync_info->enabled = ldp_sync_enable; + + if (circuit->area) { + if (ldp_sync_enable) + isis_if_ldp_sync_enable(circuit); + else + isis_if_ldp_sync_disable(circuit); } break; } @@ -3350,13 +3263,12 @@ 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 isis *isis; switch (args->event) { case NB_EV_VALIDATE: circuit = nb_running_get_entry(args->dnode, NULL, false); if (circuit == NULL || circuit->area == NULL) - return NB_ERR_VALIDATION; + break; if (circuit->isis->vrf_id != VRF_DEFAULT) { snprintf(args->errmsg, args->errmsg_len, @@ -3370,11 +3282,7 @@ int lib_interface_isis_mpls_holddown_modify(struct nb_cb_modify_args *args) case NB_EV_APPLY: circuit = nb_running_get_entry(args->dnode, NULL, true); holddown = yang_dnode_get_uint16(args->dnode, NULL); - isis = circuit->isis; - if (circuit->ldp_sync_info == NULL) - isis_ldp_sync_if_init(circuit, isis); - assert(circuit->ldp_sync_info != NULL); ldp_sync_info = circuit->ldp_sync_info; SET_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_HOLDDOWN); @@ -3388,14 +3296,12 @@ 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 isis *isis; switch (args->event) { case NB_EV_VALIDATE: circuit = nb_running_get_entry(args->dnode, NULL, false); - if (circuit == NULL || circuit->ldp_sync_info == NULL - || circuit->area == NULL) - return NB_ERR_VALIDATION; + if (circuit == NULL || circuit->area == NULL) + break; if (circuit->isis->vrf_id != VRF_DEFAULT) { snprintf(args->errmsg, args->errmsg_len, @@ -3408,15 +3314,12 @@ int lib_interface_isis_mpls_holddown_destroy(struct nb_cb_destroy_args *args) break; case NB_EV_APPLY: circuit = nb_running_get_entry(args->dnode, NULL, true); - isis = circuit->isis; ldp_sync_info = circuit->ldp_sync_info; + UNSET_FLAG(ldp_sync_info->flags, LDP_SYNC_FLAG_HOLDDOWN); - if (CHECK_FLAG(isis->ldp_sync_cmd.flags, - LDP_SYNC_FLAG_HOLDDOWN)) - ldp_sync_info->holddown = isis->ldp_sync_cmd.holddown; - else - ldp_sync_info->holddown = LDP_IGP_SYNC_HOLDDOWN_DEFAULT; + if (circuit->area) + isis_if_set_ldp_sync_holddown(circuit); break; } diff --git a/isisd/isis_zebra.c b/isisd/isis_zebra.c index cb4dd2569d..90959eb98c 100644 --- a/isisd/isis_zebra.c +++ b/isisd/isis_zebra.c @@ -728,6 +728,8 @@ static void isis_zebra_connected(struct zclient *zclient) { zclient_send_reg_requests(zclient, VRF_DEFAULT); zclient_register_opaque(zclient, LDP_RLFA_LABELS); + zclient_register_opaque(zclient, LDP_IGP_SYNC_IF_STATE_UPDATE); + zclient_register_opaque(zclient, LDP_IGP_SYNC_ANNOUNCE_UPDATE); } /* @@ -818,6 +820,8 @@ void isis_zebra_init(struct thread_master *master, int instance) void isis_zebra_stop(void) { zclient_unregister_opaque(zclient, LDP_RLFA_LABELS); + zclient_unregister_opaque(zclient, LDP_IGP_SYNC_IF_STATE_UPDATE); + zclient_unregister_opaque(zclient, LDP_IGP_SYNC_ANNOUNCE_UPDATE); zclient_stop(zclient_sync); zclient_free(zclient_sync); zclient_stop(zclient); diff --git a/isisd/isisd.h b/isisd/isisd.h index 79717b0cbb..9d0b57e9f6 100644 --- a/isisd/isisd.h +++ b/isisd/isisd.h @@ -97,7 +97,6 @@ struct isis { int snmp_notifications; struct route_table *ext_info[REDIST_PROTOCOL_COUNT]; - struct ldp_sync_info_cmd ldp_sync_cmd; /* MPLS LDP-IGP Sync */ }; extern struct isis_master *im; @@ -209,6 +208,8 @@ struct isis_area { struct prefix_list *rlfa_plist[ISIS_LEVELS]; size_t rlfa_protected_links[ISIS_LEVELS]; size_t tilfa_protected_links[ISIS_LEVELS]; + /* MPLS LDP-IGP Sync */ + struct ldp_sync_info_cmd ldp_sync_cmd; /* Counters */ uint32_t circuit_state_changes; struct isis_redist redist_settings[REDIST_PROTOCOL_COUNT] diff --git a/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync.ref b/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync.ref index 9cb70a4758..7180f84d1a 100644 --- a/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync.ref +++ b/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync.ref @@ -1,7 +1,7 @@ { "r3-eth1":{ "ldpIgpSyncEnabled":false, - "holdDownTimeInSec":50, + "holdDownTimeInSec":0, "ldpIgpSyncState":"Sync not required" }, "r3-eth2":{ diff --git a/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r1_eth1_shutdown.ref b/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r1_eth1_shutdown.ref index 9cb70a4758..7180f84d1a 100644 --- a/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r1_eth1_shutdown.ref +++ b/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r1_eth1_shutdown.ref @@ -1,7 +1,7 @@ { "r3-eth1":{ "ldpIgpSyncEnabled":false, - "holdDownTimeInSec":50, + "holdDownTimeInSec":0, "ldpIgpSyncState":"Sync not required" }, "r3-eth2":{ diff --git a/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r2_eth1_shutdown.ref b/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r2_eth1_shutdown.ref index 9cb70a4758..7180f84d1a 100644 --- a/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r2_eth1_shutdown.ref +++ b/tests/topotests/ldp-sync-isis-topo1/r3/show_isis_ldp_sync_r2_eth1_shutdown.ref @@ -1,7 +1,7 @@ { "r3-eth1":{ "ldpIgpSyncEnabled":false, - "holdDownTimeInSec":50, + "holdDownTimeInSec":0, "ldpIgpSyncState":"Sync not required" }, "r3-eth2":{ diff --git a/yang/frr-isisd.yang b/yang/frr-isisd.yang index 7c820c9611..be7426957e 100644 --- a/yang/frr-isisd.yang +++ b/yang/frr-isisd.yang @@ -791,10 +791,10 @@ module frr-isisd { leaf holddown { type uint16 { range "0..10000"; - } - units "seconds"; - description - "Time to wait for LDP-Sync to occur before restoring interface metric."; + } + units "seconds"; + description + "Time to wait for LDP-Sync to occur before restoring interface metric."; } } From 25fe5b0fe86ae5d07671ebbfea38750f29b6ca6a Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Mon, 26 Apr 2021 16:48:11 +0300 Subject: [PATCH 05/11] isisd: remove useless checks from cli is-type defaults to level-1-2 for more than a year already. Signed-off-by: Igor Ryzhov --- isisd/isis_cli.c | 32 -------------------------------- 1 file changed, 32 deletions(-) diff --git a/isisd/isis_cli.c b/isisd/isis_cli.c index b108210686..27d2492bcf 100644 --- a/isisd/isis_cli.c +++ b/isisd/isis_cli.c @@ -62,19 +62,7 @@ DEFPY_YANG_NOSH(router_isis, router_isis_cmd, "/frr-isisd:isis/instance[area-tag='%s'][vrf='%s']", tag, vrf_name); nb_cli_enqueue_change(vty, ".", NB_OP_CREATE, NULL); - /* default value in yang for is-type is level-1, but in FRR - * the first instance is assigned is-type level-1-2. We - * need to make sure to set it in the yang model so that it - * is consistent with what FRR sees. - */ - if (!im) { - return CMD_SUCCESS; - } - - if (listcount(im->isis) == 0) - nb_cli_enqueue_change(vty, "./is-type", NB_OP_MODIFY, - "level-1-2"); ret = nb_cli_apply_changes(vty, base_xpath); if (ret == CMD_SUCCESS) VTY_PUSH_XPATH(ISIS_NODE, base_xpath); @@ -190,13 +178,6 @@ DEFPY_YANG(ip_router_isis, ip_router_isis_cmd, "/frr-isisd:isis/instance[area-tag='%s'][vrf='%s']", tag, vrf_name); nb_cli_enqueue_change(vty, temp_xpath, NB_OP_CREATE, tag); - snprintf( - temp_xpath, XPATH_MAXLEN, - "/frr-isisd:isis/instance[area-tag='%s'][vrf='%s']/is-type", - tag, vrf_name); - nb_cli_enqueue_change(vty, temp_xpath, NB_OP_MODIFY, - listcount(im->isis) == 0 ? "level-1-2" - : NULL); nb_cli_enqueue_change(vty, "./frr-isisd:isis", NB_OP_CREATE, NULL); nb_cli_enqueue_change(vty, "./frr-isisd:isis/area-tag", @@ -206,9 +187,6 @@ DEFPY_YANG(ip_router_isis, ip_router_isis_cmd, vrf_name); nb_cli_enqueue_change(vty, "./frr-isisd:isis/ipv4-routing", NB_OP_MODIFY, "true"); - nb_cli_enqueue_change( - vty, "./frr-isisd:isis/circuit-type", NB_OP_MODIFY, - listcount(im->isis) == 0 ? "level-1-2" : "level-1"); } else { /* area exists, circuit type defaults to its area's is_type */ switch (area->is_type) { @@ -287,13 +265,6 @@ DEFPY_YANG(ip6_router_isis, ip6_router_isis_cmd, "/frr-isisd:isis/instance[area-tag='%s'][vrf='%s']", tag, vrf_name); nb_cli_enqueue_change(vty, temp_xpath, NB_OP_CREATE, tag); - snprintf( - temp_xpath, XPATH_MAXLEN, - "/frr-isisd:isis/instance[area-tag='%s'][vrf='%s']/is-type", - tag, vrf_name); - nb_cli_enqueue_change(vty, temp_xpath, NB_OP_MODIFY, - listcount(im->isis) == 0 ? "level-1-2" - : NULL); nb_cli_enqueue_change(vty, "./frr-isisd:isis", NB_OP_CREATE, NULL); nb_cli_enqueue_change(vty, "./frr-isisd:isis/area-tag", @@ -303,9 +274,6 @@ DEFPY_YANG(ip6_router_isis, ip6_router_isis_cmd, nb_cli_enqueue_change(vty, "./frr-isisd:isis/ipv6-routing", NB_OP_MODIFY, "true"); - nb_cli_enqueue_change( - vty, "./frr-isisd:isis/circuit-type", NB_OP_MODIFY, - listcount(im->isis) == 0 ? "level-1-2" : "level-1"); } else { /* area exists, circuit type defaults to its area's is_type */ switch (area->is_type) { From f2c170ce95fe9f64f00554211b3d914886c2b20a Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Mon, 26 Apr 2021 17:47:19 +0300 Subject: [PATCH 06/11] isisd: don't use operational data in "no router isis" We need to delete isis config from interfaces when we delete the isis router instance. This should be done using only config data. Signed-off-by: Igor Ryzhov --- isisd/isis_cli.c | 57 ++++++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/isisd/isis_cli.c b/isisd/isis_cli.c index 27d2492bcf..d5f86a6f98 100644 --- a/isisd/isis_cli.c +++ b/isisd/isis_cli.c @@ -70,16 +70,42 @@ DEFPY_YANG_NOSH(router_isis, router_isis_cmd, return ret; } +struct if_iter { + struct vty *vty; + const char *tag; +}; + +static int if_iter_cb(const struct lyd_node *dnode, void *arg) +{ + struct if_iter *iter = arg; + const char *tag; + + if (!yang_dnode_exists(dnode, "frr-isisd:isis/area-tag")) + return YANG_ITER_CONTINUE; + + tag = yang_dnode_get_string(dnode, "frr-isisd:isis/area-tag"); + if (strmatch(tag, iter->tag)) { + char xpath[XPATH_MAXLEN]; + const char *name = yang_dnode_get_string(dnode, "name"); + const char *vrf = yang_dnode_get_string(dnode, "vrf"); + + snprintf( + xpath, XPATH_MAXLEN, + "/frr-interface:lib/interface[name='%s'][vrf='%s']/frr-isisd:isis", + name, vrf); + nb_cli_enqueue_change(iter->vty, xpath, NB_OP_DESTROY, NULL); + } + + return YANG_ITER_CONTINUE; +} + DEFPY_YANG(no_router_isis, no_router_isis_cmd, "no router isis WORD$tag [vrf NAME$vrf_name]", NO_STR ROUTER_STR "ISO IS-IS\n" "ISO Routing area tag\n" VRF_CMD_HELP_STR) { - char temp_xpath[XPATH_MAXLEN]; - struct listnode *node, *nnode; - struct isis_circuit *circuit = NULL; - struct isis_area *area = NULL; + struct if_iter iter; if (!vrf_name) vrf_name = VRF_DEFAULT_NAME; @@ -92,24 +118,13 @@ DEFPY_YANG(no_router_isis, no_router_isis_cmd, return CMD_ERR_NOTHING_TODO; } + iter.vty = vty; + iter.tag = tag; + + yang_dnode_iterate(if_iter_cb, &iter, vty->candidate_config->dnode, + "/frr-interface:lib/interface[vrf='%s']", vrf_name); + nb_cli_enqueue_change(vty, ".", NB_OP_DESTROY, NULL); - area = isis_area_lookup_by_vrf(tag, vrf_name); - if (area && area->circuit_list && listcount(area->circuit_list)) { - for (ALL_LIST_ELEMENTS(area->circuit_list, node, nnode, - circuit)) { - /* add callbacks to delete each of the circuits listed - */ - const char *vrf_name = - vrf_lookup_by_id(circuit->interface->vrf_id) - ->name; - snprintf( - temp_xpath, XPATH_MAXLEN, - "/frr-interface:lib/interface[name='%s'][vrf='%s']/frr-isisd:isis", - circuit->interface->name, vrf_name); - nb_cli_enqueue_change(vty, temp_xpath, NB_OP_DESTROY, - NULL); - } - } return nb_cli_apply_changes( vty, "/frr-isisd:isis/instance[area-tag='%s'][vrf='%s']", tag, From 95018cdaa5c6a2fe106d7be1769d89c00dc62778 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Mon, 26 Apr 2021 18:09:19 +0300 Subject: [PATCH 07/11] isisd: don't create instances directly from cli This must be done only through NB code. The necessary change is enqueued right on the next two lines. Signed-off-by: Igor Ryzhov --- isisd/isis_cli.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/isisd/isis_cli.c b/isisd/isis_cli.c index d5f86a6f98..9112dc9c9a 100644 --- a/isisd/isis_cli.c +++ b/isisd/isis_cli.c @@ -188,7 +188,6 @@ DEFPY_YANG(ip_router_isis, ip_router_isis_cmd, area = isis_area_lookup_by_vrf(tag, vrf_name); if (!area) { - isis_global_instance_create(vrf_name); snprintf(temp_xpath, XPATH_MAXLEN, "/frr-isisd:isis/instance[area-tag='%s'][vrf='%s']", tag, vrf_name); @@ -275,7 +274,6 @@ DEFPY_YANG(ip6_router_isis, ip6_router_isis_cmd, area = isis_area_lookup_by_vrf(tag, vrf_name); if (!area) { - isis_global_instance_create(vrf_name); snprintf(temp_xpath, XPATH_MAXLEN, "/frr-isisd:isis/instance[area-tag='%s'][vrf='%s']", tag, vrf_name); From aaf8e80994d431b66b4930d221f2743f0b57da6f Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Mon, 26 Apr 2021 18:30:53 +0300 Subject: [PATCH 08/11] isisd: don't use operational data in "ip/ipv6 router isis" Currently the operational data is used for two things: - to inherit the is-type from the isis instance - to set passive flag for loopback interfaces This commit implements the first one using only the config data. Signed-off-by: Igor Ryzhov --- isisd/isis_cli.c | 179 +++++++++++++++-------------------------------- 1 file changed, 55 insertions(+), 124 deletions(-) diff --git a/isisd/isis_cli.c b/isisd/isis_cli.c index 9112dc9c9a..5afce94b33 100644 --- a/isisd/isis_cli.c +++ b/isisd/isis_cli.c @@ -159,78 +159,43 @@ DEFPY_YANG(ip_router_isis, ip_router_isis_cmd, "IS-IS routing protocol\n" "Routing process tag\n" VRF_CMD_HELP_STR) { - char temp_xpath[XPATH_MAXLEN]; - const char *circ_type; - struct isis_area *area = NULL; + char inst_xpath[XPATH_MAXLEN]; + struct lyd_node *if_dnode, *inst_dnode; + const char *circ_type = NULL; struct interface *ifp; - struct vrf *vrf; - /* area will be created if it is not present. make sure the yang model - * is synced with FRR and call the appropriate NB cb. - */ - - if (!im) { - return CMD_SUCCESS; - } - ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false); - if (!vrf_name) { - if (ifp) { - if (ifp->vrf_id == VRF_DEFAULT) - vrf_name = VRF_DEFAULT_NAME; - else { - vrf = vrf_lookup_by_id(ifp->vrf_id); - if (vrf && !vrf_name) - vrf_name = vrf->name; - } - } else - vrf_name = VRF_DEFAULT_NAME; + if_dnode = yang_dnode_get(vty->candidate_config->dnode, VTY_CURR_XPATH); + if (!if_dnode) { + vty_out(vty, "%% Failed to get iface dnode in candidate DB\n"); + return CMD_WARNING_CONFIG_FAILED; } - area = isis_area_lookup_by_vrf(tag, vrf_name); - if (!area) { - snprintf(temp_xpath, XPATH_MAXLEN, - "/frr-isisd:isis/instance[area-tag='%s'][vrf='%s']", - tag, vrf_name); - nb_cli_enqueue_change(vty, temp_xpath, NB_OP_CREATE, tag); - nb_cli_enqueue_change(vty, "./frr-isisd:isis", NB_OP_CREATE, - NULL); - nb_cli_enqueue_change(vty, "./frr-isisd:isis/area-tag", - NB_OP_MODIFY, tag); + vrf_name = yang_dnode_get_string(if_dnode, "vrf"); - nb_cli_enqueue_change(vty, "./frr-isisd:isis/vrf", NB_OP_MODIFY, - vrf_name); - nb_cli_enqueue_change(vty, "./frr-isisd:isis/ipv4-routing", - NB_OP_MODIFY, "true"); - } else { - /* area exists, circuit type defaults to its area's is_type */ - switch (area->is_type) { - case IS_LEVEL_1: - circ_type = "level-1"; - break; - case IS_LEVEL_2: - circ_type = "level-2"; - break; - case IS_LEVEL_1_AND_2: - circ_type = "level-1-2"; - break; - default: - /* just to silence compiler warnings */ - return CMD_WARNING_CONFIG_FAILED; - } - nb_cli_enqueue_change(vty, "./frr-isisd:isis", NB_OP_CREATE, - NULL); - nb_cli_enqueue_change(vty, "./frr-isisd:isis/area-tag", - NB_OP_MODIFY, tag); - nb_cli_enqueue_change(vty, "./frr-isisd:isis/vrf", NB_OP_MODIFY, - vrf_name); + snprintf(inst_xpath, XPATH_MAXLEN, + "/frr-isisd:isis/instance[area-tag='%s'][vrf='%s']", tag, + vrf_name); - nb_cli_enqueue_change(vty, "./frr-isisd:isis/ipv4-routing", - NB_OP_MODIFY, "true"); + /* if instance exists then inherit its type, create it otherwise */ + inst_dnode = yang_dnode_get(vty->candidate_config->dnode, inst_xpath); + if (inst_dnode) + circ_type = yang_dnode_get_string(inst_dnode, "is-type"); + else + nb_cli_enqueue_change(vty, inst_xpath, NB_OP_CREATE, NULL); + + nb_cli_enqueue_change(vty, "./frr-isisd:isis", NB_OP_CREATE, NULL); + nb_cli_enqueue_change(vty, "./frr-isisd:isis/area-tag", NB_OP_MODIFY, + tag); + nb_cli_enqueue_change(vty, "./frr-isisd:isis/vrf", NB_OP_MODIFY, + vrf_name); + nb_cli_enqueue_change(vty, "./frr-isisd:isis/ipv4-routing", + NB_OP_MODIFY, "true"); + if (circ_type) nb_cli_enqueue_change(vty, "./frr-isisd:isis/circuit-type", NB_OP_MODIFY, circ_type); - } /* check if the interface is a loopback and if so set it as passive */ + ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false); if (ifp && if_is_loopback(ifp)) nb_cli_enqueue_change(vty, "./frr-isisd:isis/passive", NB_OP_MODIFY, "true"); @@ -245,77 +210,43 @@ DEFPY_YANG(ip6_router_isis, ip6_router_isis_cmd, "IS-IS routing protocol\n" "Routing process tag\n" VRF_CMD_HELP_STR) { - char temp_xpath[XPATH_MAXLEN]; - const char *circ_type; + char inst_xpath[XPATH_MAXLEN]; + struct lyd_node *if_dnode, *inst_dnode; + const char *circ_type = NULL; struct interface *ifp; - struct isis_area *area; - struct vrf *vrf; - /* area will be created if it is not present. make sure the yang model - * is synced with FRR and call the appropriate NB cb. - */ - - if (!im) - return CMD_SUCCESS; - - ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false); - if (!vrf_name) { - if (ifp) { - if (ifp->vrf_id == VRF_DEFAULT) - vrf_name = VRF_DEFAULT_NAME; - else { - vrf = vrf_lookup_by_id(ifp->vrf_id); - if (vrf && !vrf_name) - vrf_name = vrf->name; - } - } else - vrf_name = VRF_DEFAULT_NAME; + if_dnode = yang_dnode_get(vty->candidate_config->dnode, VTY_CURR_XPATH); + if (!if_dnode) { + vty_out(vty, "%% Failed to get iface dnode in candidate DB\n"); + return CMD_WARNING_CONFIG_FAILED; } - area = isis_area_lookup_by_vrf(tag, vrf_name); - if (!area) { - snprintf(temp_xpath, XPATH_MAXLEN, - "/frr-isisd:isis/instance[area-tag='%s'][vrf='%s']", - tag, vrf_name); - nb_cli_enqueue_change(vty, temp_xpath, NB_OP_CREATE, tag); - nb_cli_enqueue_change(vty, "./frr-isisd:isis", NB_OP_CREATE, - NULL); - nb_cli_enqueue_change(vty, "./frr-isisd:isis/area-tag", - NB_OP_MODIFY, tag); - nb_cli_enqueue_change(vty, "./frr-isisd:isis/vrf", NB_OP_MODIFY, - vrf_name); + vrf_name = yang_dnode_get_string(if_dnode, "vrf"); - nb_cli_enqueue_change(vty, "./frr-isisd:isis/ipv6-routing", - NB_OP_MODIFY, "true"); - } else { - /* area exists, circuit type defaults to its area's is_type */ - switch (area->is_type) { - case IS_LEVEL_1: - circ_type = "level-1"; - break; - case IS_LEVEL_2: - circ_type = "level-2"; - break; - case IS_LEVEL_1_AND_2: - circ_type = "level-1-2"; - break; - default: - /* just to silence compiler warnings */ - return CMD_WARNING_CONFIG_FAILED; - } - nb_cli_enqueue_change(vty, "./frr-isisd:isis", NB_OP_CREATE, - NULL); - nb_cli_enqueue_change(vty, "./frr-isisd:isis/area-tag", - NB_OP_MODIFY, tag); - nb_cli_enqueue_change(vty, "./frr-isisd:isis/vrf", NB_OP_MODIFY, - vrf_name); - nb_cli_enqueue_change(vty, "./frr-isisd:isis/ipv6-routing", - NB_OP_MODIFY, "true"); + snprintf(inst_xpath, XPATH_MAXLEN, + "/frr-isisd:isis/instance[area-tag='%s'][vrf='%s']", tag, + vrf_name); + + /* if instance exists then inherit its type, create it otherwise */ + inst_dnode = yang_dnode_get(vty->candidate_config->dnode, inst_xpath); + if (inst_dnode) + circ_type = yang_dnode_get_string(inst_dnode, "is-type"); + else + nb_cli_enqueue_change(vty, inst_xpath, NB_OP_CREATE, NULL); + + nb_cli_enqueue_change(vty, "./frr-isisd:isis", NB_OP_CREATE, NULL); + nb_cli_enqueue_change(vty, "./frr-isisd:isis/area-tag", NB_OP_MODIFY, + tag); + nb_cli_enqueue_change(vty, "./frr-isisd:isis/vrf", NB_OP_MODIFY, + vrf_name); + nb_cli_enqueue_change(vty, "./frr-isisd:isis/ipv6-routing", + NB_OP_MODIFY, "true"); + if (circ_type) nb_cli_enqueue_change(vty, "./frr-isisd:isis/circuit-type", NB_OP_MODIFY, circ_type); - } /* check if the interface is a loopback and if so set it as passive */ + ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false); if (ifp && if_is_loopback(ifp)) nb_cli_enqueue_change(vty, "./frr-isisd:isis/passive", NB_OP_MODIFY, "true"); From 1457b1d5dfe8e762487641667ee448b287e9c813 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Mon, 26 Apr 2021 21:13:08 +0300 Subject: [PATCH 09/11] isisd, yang: remove vrf leaf from isis interface node This is very confusing and incorrect. We can and should use vrf leaf of the interface itself instead. Signed-off-by: Igor Ryzhov --- doc/user/isisd.rst | 2 +- isisd/isis_cli.c | 64 ++++++++++++++++++++++++------------------ isisd/isis_nb.c | 7 ----- isisd/isis_nb.h | 1 - isisd/isis_nb_config.c | 48 ------------------------------- yang/frr-isisd.yang | 7 ----- 6 files changed, 38 insertions(+), 91 deletions(-) diff --git a/doc/user/isisd.rst b/doc/user/isisd.rst index ef2bf16166..c1bdd9a694 100644 --- a/doc/user/isisd.rst +++ b/doc/user/isisd.rst @@ -172,7 +172,7 @@ ISIS interface .. _ip-router-isis-word: -.. clicmd:: router isis WORD [vrf NAME] +.. clicmd:: router isis WORD Activate ISIS adjacency on this interface. Note that the name of ISIS instance must be the same as the one used to configure the ISIS process (see diff --git a/isisd/isis_cli.c b/isisd/isis_cli.c index 5afce94b33..9f42a82a1a 100644 --- a/isisd/isis_cli.c +++ b/isisd/isis_cli.c @@ -153,15 +153,16 @@ void cli_show_router_isis(struct vty *vty, struct lyd_node *dnode, * XPath: /frr-isisd:isis/instance */ DEFPY_YANG(ip_router_isis, ip_router_isis_cmd, - "ip router isis WORD$tag [vrf NAME$vrf_name]", + "ip router isis WORD$tag", "Interface Internet Protocol config commands\n" "IP router interface commands\n" "IS-IS routing protocol\n" - "Routing process tag\n" VRF_CMD_HELP_STR) + "Routing process tag\n") { char inst_xpath[XPATH_MAXLEN]; struct lyd_node *if_dnode, *inst_dnode; const char *circ_type = NULL; + const char *vrf_name; struct interface *ifp; if_dnode = yang_dnode_get(vty->candidate_config->dnode, VTY_CURR_XPATH); @@ -186,8 +187,6 @@ DEFPY_YANG(ip_router_isis, ip_router_isis_cmd, nb_cli_enqueue_change(vty, "./frr-isisd:isis", NB_OP_CREATE, NULL); nb_cli_enqueue_change(vty, "./frr-isisd:isis/area-tag", NB_OP_MODIFY, tag); - nb_cli_enqueue_change(vty, "./frr-isisd:isis/vrf", NB_OP_MODIFY, - vrf_name); nb_cli_enqueue_change(vty, "./frr-isisd:isis/ipv4-routing", NB_OP_MODIFY, "true"); if (circ_type) @@ -203,16 +202,24 @@ DEFPY_YANG(ip_router_isis, ip_router_isis_cmd, return nb_cli_apply_changes(vty, NULL); } +ALIAS_HIDDEN(ip_router_isis, ip_router_isis_vrf_cmd, + "ip router isis WORD$tag vrf NAME$vrf_name", + "Interface Internet Protocol config commands\n" + "IP router interface commands\n" + "IS-IS routing protocol\n" + "Routing process tag\n" VRF_CMD_HELP_STR) + DEFPY_YANG(ip6_router_isis, ip6_router_isis_cmd, - "ipv6 router isis WORD$tag [vrf NAME$vrf_name]", + "ipv6 router isis WORD$tag", "Interface Internet Protocol config commands\n" "IP router interface commands\n" "IS-IS routing protocol\n" - "Routing process tag\n" VRF_CMD_HELP_STR) + "Routing process tag\n") { char inst_xpath[XPATH_MAXLEN]; struct lyd_node *if_dnode, *inst_dnode; const char *circ_type = NULL; + const char *vrf_name; struct interface *ifp; if_dnode = yang_dnode_get(vty->candidate_config->dnode, VTY_CURR_XPATH); @@ -237,8 +244,6 @@ DEFPY_YANG(ip6_router_isis, ip6_router_isis_cmd, nb_cli_enqueue_change(vty, "./frr-isisd:isis", NB_OP_CREATE, NULL); nb_cli_enqueue_change(vty, "./frr-isisd:isis/area-tag", NB_OP_MODIFY, tag); - nb_cli_enqueue_change(vty, "./frr-isisd:isis/vrf", NB_OP_MODIFY, - vrf_name); nb_cli_enqueue_change(vty, "./frr-isisd:isis/ipv6-routing", NB_OP_MODIFY, "true"); if (circ_type) @@ -254,15 +259,21 @@ DEFPY_YANG(ip6_router_isis, ip6_router_isis_cmd, return nb_cli_apply_changes(vty, NULL); } +ALIAS_HIDDEN(ip6_router_isis, ip6_router_isis_vrf_cmd, + "ipv6 router isis WORD$tag vrf NAME$vrf_name", + "Interface Internet Protocol config commands\n" + "IP router interface commands\n" + "IS-IS routing protocol\n" + "Routing process tag\n" VRF_CMD_HELP_STR) + DEFPY_YANG(no_ip_router_isis, no_ip_router_isis_cmd, - "no $ip router isis [WORD]$tag [vrf NAME$vrf_name]", + "no $ip router isis [WORD]$tag", NO_STR "Interface Internet Protocol config commands\n" "IP router interface commands\n" "IP router interface commands\n" "IS-IS routing protocol\n" - "Routing process tag\n" - VRF_CMD_HELP_STR) + "Routing process tag\n") { const struct lyd_node *dnode; @@ -295,36 +306,32 @@ DEFPY_YANG(no_ip_router_isis, no_ip_router_isis_cmd, return nb_cli_apply_changes(vty, NULL); } +ALIAS_HIDDEN(no_ip_router_isis, no_ip_router_isis_vrf_cmd, + "no $ip router isis WORD$tag vrf NAME$vrf_name", + NO_STR + "Interface Internet Protocol config commands\n" + "IP router interface commands\n" + "IP router interface commands\n" + "IS-IS routing protocol\n" + "Routing process tag\n" + VRF_CMD_HELP_STR) + void cli_show_ip_isis_ipv4(struct vty *vty, struct lyd_node *dnode, bool show_defaults) { - const char *vrf; - - vrf = yang_dnode_get_string(dnode, "../vrf"); - if (!yang_dnode_get_bool(dnode, NULL)) vty_out(vty, " no"); - vty_out(vty, " ip router isis %s", + vty_out(vty, " ip router isis %s\n", yang_dnode_get_string(dnode, "../area-tag")); - if (!strmatch(vrf, VRF_DEFAULT_NAME)) - vty_out(vty, " vrf %s", vrf); - vty_out(vty, "\n"); } void cli_show_ip_isis_ipv6(struct vty *vty, struct lyd_node *dnode, bool show_defaults) { - const char *vrf; - - vrf = yang_dnode_get_string(dnode, "../vrf"); - if (!yang_dnode_get_bool(dnode, NULL)) vty_out(vty, " no"); - vty_out(vty, " ipv6 router isis %s", + vty_out(vty, " ipv6 router isis %s\n", yang_dnode_get_string(dnode, "../area-tag")); - if (!strmatch(vrf, VRF_DEFAULT_NAME)) - vty_out(vty, " vrf %s", vrf); - vty_out(vty, "\n"); } /* @@ -3174,8 +3181,11 @@ void isis_cli_init(void) install_element(CONFIG_NODE, &no_router_isis_cmd); install_element(INTERFACE_NODE, &ip_router_isis_cmd); + install_element(INTERFACE_NODE, &ip_router_isis_vrf_cmd); install_element(INTERFACE_NODE, &ip6_router_isis_cmd); + install_element(INTERFACE_NODE, &ip6_router_isis_vrf_cmd); install_element(INTERFACE_NODE, &no_ip_router_isis_cmd); + install_element(INTERFACE_NODE, &no_ip_router_isis_vrf_cmd); install_element(INTERFACE_NODE, &isis_bfd_cmd); install_element(INTERFACE_NODE, &isis_bfd_profile_cmd); diff --git a/isisd/isis_nb.c b/isisd/isis_nb.c index 227724934b..4eac8de2cb 100644 --- a/isisd/isis_nb.c +++ b/isisd/isis_nb.c @@ -685,13 +685,6 @@ const struct frr_yang_module_info frr_isisd_info = { .modify = lib_interface_isis_area_tag_modify, }, }, - { - .xpath = "/frr-interface:lib/interface/frr-isisd:isis/vrf", - .cbs = { - .modify = lib_interface_isis_vrf_modify, - }, - }, - { .xpath = "/frr-interface:lib/interface/frr-isisd:isis/circuit-type", .cbs = { diff --git a/isisd/isis_nb.h b/isisd/isis_nb.h index a6841b9fd4..94aa00c975 100644 --- a/isisd/isis_nb.h +++ b/isisd/isis_nb.h @@ -214,7 +214,6 @@ int isis_instance_mpls_te_router_address_destroy( int lib_interface_isis_create(struct nb_cb_create_args *args); int lib_interface_isis_destroy(struct nb_cb_destroy_args *args); int lib_interface_isis_area_tag_modify(struct nb_cb_modify_args *args); -int lib_interface_isis_vrf_modify(struct nb_cb_modify_args *args); int lib_interface_isis_ipv4_routing_modify(struct nb_cb_modify_args *args); int lib_interface_isis_ipv6_routing_modify(struct nb_cb_modify_args *args); int lib_interface_isis_circuit_type_modify(struct nb_cb_modify_args *args); diff --git a/isisd/isis_nb_config.c b/isisd/isis_nb_config.c index 5cad9fcfcb..58a3304629 100644 --- a/isisd/isis_nb_config.c +++ b/isisd/isis_nb_config.c @@ -2437,9 +2437,7 @@ int lib_interface_isis_create(struct nb_cb_create_args *args) struct isis_area *area = NULL; struct interface *ifp; struct isis_circuit *circuit = NULL; - struct vrf *vrf; const char *area_tag = yang_dnode_get_string(args->dnode, "./area-tag"); - const char *vrf_name = yang_dnode_get_string(args->dnode, "./vrf"); uint32_t min_mtu, actual_mtu; switch (args->event) { @@ -2454,14 +2452,6 @@ int lib_interface_isis_create(struct nb_cb_create_args *args) /* zebra might not know yet about the MTU - nothing we can do */ if (!ifp || ifp->mtu == 0) break; - vrf = vrf_lookup_by_id(ifp->vrf_id); - if (ifp->vrf_id != VRF_DEFAULT && vrf - && strcmp(vrf->name, vrf_name) != 0) { - snprintf(args->errmsg, args->errmsg_len, - "interface %s not in vrf %s\n", ifp->name, - vrf_name); - return NB_ERR_VALIDATION; - } actual_mtu = if_is_broadcast(ifp) ? ifp->mtu - LLC_LEN : ifp->mtu; @@ -2544,44 +2534,6 @@ int lib_interface_isis_area_tag_modify(struct nb_cb_modify_args *args) return NB_OK; } -/* - * XPath: /frr-interface:lib/interface/frr-isisd:isis/vrf - */ -int lib_interface_isis_vrf_modify(struct nb_cb_modify_args *args) -{ - struct interface *ifp; - struct vrf *vrf; - const char *ifname, *vrfname, *vrf_name; - struct isis_circuit *circuit; - - if (args->event == NB_EV_VALIDATE) { - /* libyang doesn't like relative paths across module boundaries - */ - ifname = yang_dnode_get_string(args->dnode->parent->parent, - "./name"); - vrfname = yang_dnode_get_string(args->dnode->parent->parent, - "./vrf"); - vrf = vrf_lookup_by_name(vrfname); - assert(vrf); - ifp = if_lookup_by_name(ifname, vrf->vrf_id); - - if (!ifp) - return NB_OK; - - vrf_name = yang_dnode_get_string(args->dnode, NULL); - circuit = circuit_scan_by_ifp(ifp); - if (circuit && circuit->area && circuit->isis - && strcmp(circuit->isis->name, vrf_name)) { - snprintf(args->errmsg, args->errmsg_len, - "ISIS circuit is already defined on vrf %s", - circuit->isis->name); - return NB_ERR_VALIDATION; - } - } - - return NB_OK; -} - /* * XPath: /frr-interface:lib/interface/frr-isisd:isis/circuit-type */ diff --git a/yang/frr-isisd.yang b/yang/frr-isisd.yang index be7426957e..46ad8d3971 100644 --- a/yang/frr-isisd.yang +++ b/yang/frr-isisd.yang @@ -506,13 +506,6 @@ module frr-isisd { "Area-tag associated to this circuit."; } - leaf vrf { - type frr-vrf:vrf-ref; - default "default"; - description - "VRF NAME."; - } - leaf ipv4-routing { type boolean; default "false"; From b5c0a71b56b17a737462c61e32fccef7172b81a8 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Mon, 26 Apr 2021 21:34:39 +0300 Subject: [PATCH 10/11] isisd: don't use operational data in "no isis circuit-type" Use the config data instead. Signed-off-by: Igor Ryzhov --- isisd/isis_cli.c | 61 +++++++++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 35 deletions(-) diff --git a/isisd/isis_cli.c b/isisd/isis_cli.c index 9f42a82a1a..f29d6e3412 100644 --- a/isisd/isis_cli.c +++ b/isisd/isis_cli.c @@ -2503,52 +2503,43 @@ DEFPY_YANG(no_isis_circuit_type, no_isis_circuit_type_cmd, "Level-1-2 adjacencies are formed\n" "Level-2 only adjacencies are formed\n") { - struct interface *ifp; - struct isis_circuit *circuit; - int is_type; - const char *circ_type; + char inst_xpath[XPATH_MAXLEN]; + struct lyd_node *if_dnode, *inst_dnode; + const char *vrf_name; + const char *tag; + const char *circ_type = NULL; /* * Default value depends on whether the circuit is part of an area, * and the is-type of the area if there is one. So we need to do this * here. */ - ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false); - if (!ifp) - goto def_val; - - circuit = circuit_scan_by_ifp(ifp); - if (!circuit) - goto def_val; - - if (circuit->state == C_STATE_UP) - is_type = circuit->area->is_type; - else - goto def_val; - - switch (is_type) { - case IS_LEVEL_1: - circ_type = "level-1"; - break; - case IS_LEVEL_2: - circ_type = "level-2"; - break; - case IS_LEVEL_1_AND_2: - circ_type = "level-1-2"; - break; - default: - return CMD_ERR_NO_MATCH; + if_dnode = yang_dnode_get(vty->candidate_config->dnode, VTY_CURR_XPATH); + if (!if_dnode) { + vty_out(vty, "%% Failed to get iface dnode in candidate DB\n"); + return CMD_WARNING_CONFIG_FAILED; } + + if (!yang_dnode_exists(if_dnode, "frr-isisd:isis/area-tag")) { + vty_out(vty, "%% ISIS is not configured on the interface\n"); + return CMD_WARNING_CONFIG_FAILED; + } + + vrf_name = yang_dnode_get_string(if_dnode, "vrf"); + tag = yang_dnode_get_string(if_dnode, "frr-isisd:isis/area-tag"); + + snprintf(inst_xpath, XPATH_MAXLEN, + "/frr-isisd:isis/instance[area-tag='%s'][vrf='%s']", tag, + vrf_name); + + inst_dnode = yang_dnode_get(vty->candidate_config->dnode, inst_xpath); + if (inst_dnode) + circ_type = yang_dnode_get_string(inst_dnode, "is-type"); + nb_cli_enqueue_change(vty, "./frr-isisd:isis/circuit-type", NB_OP_MODIFY, circ_type); return nb_cli_apply_changes(vty, NULL); - -def_val: - nb_cli_enqueue_change(vty, "./frr-isisd:isis/circuit-type", - NB_OP_MODIFY, NULL); - - return nb_cli_apply_changes(vty, NULL); } void cli_show_ip_isis_circ_type(struct vty *vty, struct lyd_node *dnode, From f07572c3c748f5ee5a0277ad4210d76001423512 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Mon, 26 Apr 2021 22:01:43 +0300 Subject: [PATCH 11/11] isisd: move ldp-sync checks from cli to nb callbacks Signed-off-by: Igor Ryzhov --- isisd/isis_cli.c | 36 ------------------------------------ isisd/isis_nb_config.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 36 deletions(-) diff --git a/isisd/isis_cli.c b/isisd/isis_cli.c index f29d6e3412..3e719df4bb 100644 --- a/isisd/isis_cli.c +++ b/isisd/isis_cli.c @@ -3057,7 +3057,6 @@ DEFPY(isis_mpls_if_ldp_sync, isis_mpls_if_ldp_sync_cmd, NO_STR "IS-IS routing protocol\n" MPLS_STR MPLS_LDP_SYNC_STR) { const struct lyd_node *dnode; - struct interface *ifp; dnode = yang_dnode_get(vty->candidate_config->dnode, "%s/frr-isisd:isis", VTY_CURR_XPATH); @@ -3066,17 +3065,6 @@ DEFPY(isis_mpls_if_ldp_sync, isis_mpls_if_ldp_sync_cmd, return CMD_SUCCESS; } - ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false); - if (if_is_loopback(ifp)) { - vty_out(vty, "ldp-sync does not run on loopback interface\n"); - return CMD_SUCCESS; - } - - if (ifp->vrf_id != VRF_DEFAULT) { - vty_out(vty, "ldp-sync only runs on DEFAULT VRF\n"); - return CMD_SUCCESS; - } - nb_cli_enqueue_change(vty, "./frr-isisd:isis/mpls/ldp-sync", NB_OP_MODIFY, no ? "false" : "true"); @@ -3100,7 +3088,6 @@ DEFPY(isis_mpls_if_ldp_sync_holddown, isis_mpls_if_ldp_sync_holddown_cmd, "Time in seconds\n") { const struct lyd_node *dnode; - struct interface *ifp; dnode = yang_dnode_get(vty->candidate_config->dnode, "%s/frr-isisd:isis", VTY_CURR_XPATH); @@ -3109,17 +3096,6 @@ DEFPY(isis_mpls_if_ldp_sync_holddown, isis_mpls_if_ldp_sync_holddown_cmd, return CMD_SUCCESS; } - ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false); - if (if_is_loopback(ifp)) { - vty_out(vty, "ldp-sync does not run on loopback interface\n"); - return CMD_SUCCESS; - } - - if (ifp->vrf_id != VRF_DEFAULT) { - vty_out(vty, "ldp-sync only runs on DEFAULT VRF\n"); - return CMD_SUCCESS; - } - nb_cli_enqueue_change(vty, "./frr-isisd:isis/mpls/holddown", NB_OP_MODIFY, holddown_str); @@ -3132,7 +3108,6 @@ DEFPY(no_isis_mpls_if_ldp_sync_holddown, no_isis_mpls_if_ldp_sync_holddown_cmd, NO_MPLS_LDP_SYNC_HOLDDOWN_STR "Time in seconds\n") { const struct lyd_node *dnode; - struct interface *ifp; dnode = yang_dnode_get(vty->candidate_config->dnode, "%s/frr-isisd:isis", VTY_CURR_XPATH); @@ -3141,17 +3116,6 @@ DEFPY(no_isis_mpls_if_ldp_sync_holddown, no_isis_mpls_if_ldp_sync_holddown_cmd, return CMD_SUCCESS; } - ifp = nb_running_get_entry(NULL, VTY_CURR_XPATH, false); - if (if_is_loopback(ifp)) { - vty_out(vty, "ldp-sync does not run on loopback interface\n"); - return CMD_SUCCESS; - } - - if (ifp->vrf_id != VRF_DEFAULT) { - vty_out(vty, "ldp-sync only runs on DEFAULT VRF\n"); - return CMD_SUCCESS; - } - nb_cli_enqueue_change(vty, "./frr-isisd:isis/mpls/holddown", NB_OP_DESTROY, NULL); diff --git a/isisd/isis_nb_config.c b/isisd/isis_nb_config.c index 58a3304629..f9028bdd9f 100644 --- a/isisd/isis_nb_config.c +++ b/isisd/isis_nb_config.c @@ -3171,9 +3171,20 @@ 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; switch (args->event) { case NB_EV_VALIDATE: + ifp = nb_running_get_entry(args->dnode->parent->parent->parent, + 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; @@ -3215,9 +3226,20 @@ 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; switch (args->event) { case NB_EV_VALIDATE: + ifp = nb_running_get_entry(args->dnode->parent->parent->parent, + 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; @@ -3248,9 +3270,20 @@ 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; switch (args->event) { case NB_EV_VALIDATE: + ifp = nb_running_get_entry(args->dnode->parent->parent->parent, + 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;