From 2f9a06f0600b6d1c6f28117bb2846ce3675fc129 Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Sat, 30 Oct 2021 03:15:24 +0300 Subject: [PATCH] isisd: fix circuit is-type configuration Currently, we have a lot of checks in CLI and NB layer to prevent incompatible IS-types of circuits and areas. All these checks become completely meaningless when the interface is moved between VRFs. If the area IS-type is different in the new VRF, previously done checks mean nothing and we still end up with incorrect circuit IS type. To actually prevent incorrect IS type, all checks must be done in the processing code. Signed-off-by: Igor Ryzhov --- isisd/isis_circuit.c | 10 ++- isisd/isis_circuit.h | 1 + isisd/isis_cli.c | 89 +------------------ isisd/isis_events.c | 7 +- isisd/isis_nb_config.c | 26 +----- isisd/isisd.c | 8 +- .../isis_topo1_vrf/r3/r3_topology.json | 12 +-- 7 files changed, 30 insertions(+), 123 deletions(-) diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c index c09bdf4f4..8810d6107 100644 --- a/isisd/isis_circuit.c +++ b/isisd/isis_circuit.c @@ -116,7 +116,7 @@ struct isis_circuit *isis_circuit_new(struct interface *ifp, const char *tag) * Default values */ #ifndef FABRICD - circuit->is_type = yang_get_default_enum( + circuit->is_type_config = yang_get_default_enum( "/frr-interface:lib/interface/frr-isisd:isis/circuit-type"); circuit->flags = 0; @@ -156,7 +156,7 @@ struct isis_circuit *isis_circuit_new(struct interface *ifp, const char *tag) circuit->level_arg[i].circuit = circuit; } #else - circuit->is_type = IS_LEVEL_1_AND_2; + circuit->is_type_config = IS_LEVEL_1_AND_2; circuit->flags = 0; circuit->pad_hellos = 1; for (i = 0; i < 2; i++) { @@ -172,6 +172,8 @@ struct isis_circuit *isis_circuit_new(struct interface *ifp, const char *tag) } #endif /* ifndef FABRICD */ + circuit->is_type = circuit->is_type_config; + circuit_mt_init(circuit); isis_lfa_excluded_ifaces_init(circuit, ISIS_LEVEL1); isis_lfa_excluded_ifaces_init(circuit, ISIS_LEVEL2); @@ -253,6 +255,10 @@ void isis_circuit_deconfigure(struct isis_circuit *circuit, /* Free the index of SRM and SSN flags */ flags_free_index(&area->flags, circuit->idx); circuit->idx = 0; + + /* Reset IS type to configured */ + circuit->is_type = circuit->is_type_config; + /* Remove circuit from area */ assert(circuit->area == area); listnode_delete(area->circuit_list, circuit); diff --git a/isisd/isis_circuit.h b/isisd/isis_circuit.h index e7b7a2434..e286194d3 100644 --- a/isisd/isis_circuit.h +++ b/isisd/isis_circuit.h @@ -122,6 +122,7 @@ struct isis_circuit { */ char *tag; /* area tag */ struct isis_passwd passwd; /* Circuit rx/tx password */ + int is_type_config; /* configured circuit is type */ int is_type; /* circuit is type == level of circuit * differentiated from circuit type (media) */ uint32_t hello_interval[ISIS_LEVELS]; /* hello-interval in seconds */ diff --git a/isisd/isis_cli.c b/isisd/isis_cli.c index c0727a669..b543c2d3e 100644 --- a/isisd/isis_cli.c +++ b/isisd/isis_cli.c @@ -127,38 +127,11 @@ DEFPY_YANG(ip_router_isis, ip_router_isis_cmd, "IS-IS routing protocol\n" "Routing process tag\n") { - char inst_xpath[XPATH_MAXLEN]; - const struct lyd_node *if_dnode, *inst_dnode; - const char *circ_type = NULL; - const char *vrf_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; - } - - vrf_name = yang_dnode_get_string(if_dnode, "vrf"); - - 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/ipv4-routing", NB_OP_MODIFY, "true"); - if (circ_type) - nb_cli_enqueue_change(vty, "./frr-isisd:isis/circuit-type", - NB_OP_MODIFY, circ_type); return nb_cli_apply_changes(vty, NULL); } @@ -177,38 +150,11 @@ DEFPY_YANG(ip6_router_isis, ip6_router_isis_cmd, "IS-IS routing protocol\n" "Routing process tag\n") { - char inst_xpath[XPATH_MAXLEN]; - const struct lyd_node *if_dnode, *inst_dnode; - const char *circ_type = NULL; - const char *vrf_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; - } - - vrf_name = yang_dnode_get_string(if_dnode, "vrf"); - - 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/ipv6-routing", NB_OP_MODIFY, "true"); - if (circ_type) - nb_cli_enqueue_change(vty, "./frr-isisd:isis/circuit-type", - NB_OP_MODIFY, circ_type); return nb_cli_apply_changes(vty, NULL); } @@ -2494,41 +2440,8 @@ 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") { - char inst_xpath[XPATH_MAXLEN]; - const 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. - */ - 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); + NB_OP_MODIFY, NULL); return nb_cli_apply_changes(vty, NULL); } diff --git a/isisd/isis_events.c b/isisd/isis_events.c index 0b987fc5c..26c68db76 100644 --- a/isisd/isis_events.c +++ b/isisd/isis_events.c @@ -128,7 +128,7 @@ static void circuit_resign_level(struct isis_circuit *circuit, int level) void isis_circuit_is_type_set(struct isis_circuit *circuit, int newtype) { - if (circuit->state != C_STATE_UP) { + if (!circuit->area) { circuit->is_type = newtype; return; } @@ -151,6 +151,11 @@ void isis_circuit_is_type_set(struct isis_circuit *circuit, int newtype) return; } + if (circuit->state != C_STATE_UP) { + circuit->is_type = newtype; + return; + } + if (!circuit->is_passive) { switch (circuit->is_type) { case IS_LEVEL_1: diff --git a/isisd/isis_nb_config.c b/isisd/isis_nb_config.c index 6148cd7bf..07af46c04 100644 --- a/isisd/isis_nb_config.c +++ b/isisd/isis_nb_config.c @@ -2546,39 +2546,15 @@ int lib_interface_isis_circuit_type_modify(struct nb_cb_modify_args *args) { int circ_type = yang_dnode_get_enum(args->dnode, NULL); struct isis_circuit *circuit; - struct interface *ifp; - struct vrf *vrf; - const char *ifname, *vrfname; switch (args->event) { case NB_EV_VALIDATE: - /* libyang doesn't like relative paths across module boundaries - */ - ifname = yang_dnode_get_string( - lyd_parent(lyd_parent(args->dnode)), "./name"); - vrfname = yang_dnode_get_string( - lyd_parent(lyd_parent(args->dnode)), "./vrf"); - vrf = vrf_lookup_by_name(vrfname); - assert(vrf); - ifp = if_lookup_by_name(ifname, vrf->vrf_id); - if (!ifp) - break; - - 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) { - snprintf(args->errmsg, args->errmsg_len, - "Invalid circuit level for area %s", - circuit->area->area_tag); - return NB_ERR_VALIDATION; - } - break; case NB_EV_PREPARE: case NB_EV_ABORT: break; case NB_EV_APPLY: circuit = nb_running_get_entry(args->dnode, NULL, true); + circuit->is_type_config = circ_type; isis_circuit_is_type_set(circuit, circ_type); break; } diff --git a/isisd/isisd.c b/isisd/isisd.c index 8c26a57be..b33f272f5 100644 --- a/isisd/isisd.c +++ b/isisd/isisd.c @@ -2656,10 +2656,16 @@ void isis_area_is_type_set(struct isis_area *area, int is_type) area->is_type = is_type; - /* override circuit's is_type */ + /* + * If area's IS type is strict Level-1 or Level-2, override circuit's + * IS type. Otherwise use circuit's configured IS type. + */ if (area->is_type != IS_LEVEL_1_AND_2) { for (ALL_LIST_ELEMENTS_RO(area->circuit_list, node, circuit)) isis_circuit_is_type_set(circuit, is_type); + } else { + for (ALL_LIST_ELEMENTS_RO(area->circuit_list, node, circuit)) + isis_circuit_is_type_set(circuit, circuit->is_type_config); } spftree_area_init(area); diff --git a/tests/topotests/isis_topo1_vrf/r3/r3_topology.json b/tests/topotests/isis_topo1_vrf/r3/r3_topology.json index 34892d4a3..044a6c043 100644 --- a/tests/topotests/isis_topo1_vrf/r3/r3_topology.json +++ b/tests/topotests/isis_topo1_vrf/r3/r3_topology.json @@ -94,16 +94,16 @@ { "interface": "r3-eth0", "metric": "10", - "next-hop": "r3", + "next-hop": "r1", "parent": "r3(4)", "type": "TE-IS", - "vertex": "r3" + "vertex": "r1" }, { "interface": "r3-eth0", "metric": "10", - "next-hop": "r3", - "parent": "r3(4)", + "next-hop": "r1", + "parent": "r1(4)", "type": "IP TE", "vertex": "10.0.20.0/24" } @@ -121,10 +121,10 @@ { "interface": "r3-eth0", "metric": "10", - "next-hop": "r3", + "next-hop": "r1", "parent": "r3(4)", "type": "TE-IS", - "vertex": "r3" + "vertex": "r1" } ] } -- 2.39.2