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 <iryzhov@nfware.com>

diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c
index c09bdf4f4ff6d78f55baa8e6277f337ce60a98a1..8810d6107d77cbfb6086e9fce59a389fe2c472f5 100644 (file)
--- 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 e7b7a2434da0ea049fd646758fab8cd1fdf110fe..e286194d3e60ea42ac66cfbf1340205a0bd57a40 100644 (file)
--- 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 c0727a66959e20a5b9922158250c785177b3d6dd..b543c2d3ecbd834640357f8637055c251855c812 100644 (file)
--- 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 0b987fc5cf50e2886c40ed94b371869f41ebbeee..26c68db762f3d1f5d4a852e4d209cc34e7d41559 100644 (file)
--- 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 6148cd7bf943c2f9da11dbc9c2fbd37bebc6d8b6..07af46c04a34cf69d0ff89d58d6668eb89b08a37 100644 (file)
--- 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 8c26a57bef4ffedc11a5ba0e614279d00c325f72..b33f272f56078b61d0f6d95cb72ff3def4a4ac7f 100644 (file)
--- 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 34892d4a3a2bc59116682e39a362678eb213dbf5..044a6c0438aa3fd26ce3a5d12fde690e714f5b75 100644 (file)
--- 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"
         }
       ]
     }