From: Igor Ryzhov Date: Tue, 27 Apr 2021 22:57:21 +0000 (+0300) Subject: isisd: allow arbitrary order of area/interface configuration X-Git-Tag: base_8.0~78^2~8 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=bcf220815632500f9646cb0b4c13f498afa4edf6;p=matthieu%2Ffrr.git 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 --- 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; - } - - area->ip_circuits += ip_router - old_ipr; - area->ipv6_circuits += ipv6_router - old_ipv6r; + if (area) { + 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); + if (ip_router || ipv6_router) + 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"' )