From 139c1f36d45f7ab34ea3d1f343b63ac3487b3415 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 11 Aug 2016 16:59:08 +0200 Subject: [PATCH 1/6] isisd: fix isis_circuit_create() Between the awkwardly managed CSM and the tacked-on IPv6 support, the simplified logic to setup a circuit wasn't quite right. Note that the API essentially allows creating a circuit without enabling either IPv4 or IPv6. This wasn't possible before and probably breaks isisd in 'interesting' ways. The CLI won't do this, so it's only an issue when adding on other configuration mechanisms. Reported-by: Martin Winter Signed-off-by: David Lamparter --- isisd/isis_circuit.c | 6 ++++-- isisd/isis_vty.c | 16 ++++++---------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c index d8ca694d59..b9ebf52505 100644 --- a/isisd/isis_circuit.c +++ b/isisd/isis_circuit.c @@ -1226,8 +1226,10 @@ isis_interface_config_write (struct vty *vty) struct isis_circuit * isis_circuit_create (struct isis_area *area, struct interface *ifp) { - struct isis_circuit *circuit; - circuit = isis_csm_state_change (ISIS_ENABLE, NULL, area); + struct isis_circuit *circuit = circuit_scan_by_ifp (ifp); + if (circuit && circuit->area) + return NULL; + circuit = isis_csm_state_change (ISIS_ENABLE, circuit, area); assert (circuit->state == C_STATE_CONF || circuit->state == C_STATE_UP); isis_circuit_if_bind (circuit, ifp); return circuit; diff --git a/isisd/isis_vty.c b/isisd/isis_vty.c index 3f218561cc..3ce06b83da 100644 --- a/isisd/isis_vty.c +++ b/isisd/isis_vty.c @@ -72,17 +72,13 @@ DEFUN (ip_router_isis, /* Prevent more than one area per circuit */ circuit = circuit_scan_by_ifp (ifp); - if (circuit) + if (circuit && circuit->area) { - if (circuit->ip_router == 1) + if (strcmp (circuit->area->area_tag, area_tag)) { - if (strcmp (circuit->area->area_tag, area_tag)) - { - vty_out (vty, "ISIS circuit is already defined on %s%s", - circuit->area->area_tag, VTY_NEWLINE); - return CMD_ERR_NOTHING_TODO; - } - return CMD_SUCCESS; + vty_out (vty, "ISIS circuit is already defined on %s%s", + circuit->area->area_tag, VTY_NEWLINE); + return CMD_ERR_NOTHING_TODO; } } @@ -90,7 +86,7 @@ DEFUN (ip_router_isis, if (!area) area = isis_area_create (area_tag); - if (!circuit) + if (!circuit || !circuit->area) circuit = isis_circuit_create (area, ifp); bool ip = circuit->ip_router, ipv6 = circuit->ipv6_router; From aa11f85082d8d0272bfd7e06742739afb4783e07 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 11 Aug 2016 17:02:50 +0200 Subject: [PATCH 2/6] isisd: fix isis_circuit_af_set() on fresh circuit A newly-created circuit will be in enabled state but have neither IPv4 nor IPv6 configured. The logic in isis_circuit_af_set assumed that "enabled" is equivalent to "ip || ipv6". This is the only place where this distinction is currently relevant, as the CLI won't allow enabling an interface without enabling either IPv4 or IPv6; and it will also disable a circuit when both are deconfigured. Reported-by: Martin Winter Signed-off-by: David Lamparter --- isisd/isis_circuit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c index b9ebf52505..ad1eca89e3 100644 --- a/isisd/isis_circuit.c +++ b/isisd/isis_circuit.c @@ -1240,7 +1240,7 @@ isis_circuit_af_set (struct isis_circuit *circuit, bool ip_router, bool ipv6_rou { struct isis_area *area = circuit->area; bool change = circuit->ip_router != ip_router || circuit->ipv6_router != ipv6_router; - bool was_enabled = circuit->ip_router || circuit->ipv6_router; + bool was_enabled = !!circuit->area; area->ip_circuits += ip_router - circuit->ip_router; area->ipv6_circuits += ipv6_router - circuit->ipv6_router; From 6f32c890cb549b6ba58d5aae5eb6fbb50abfe063 Mon Sep 17 00:00:00 2001 From: Christian Franke Date: Thu, 11 Aug 2016 16:08:05 +0200 Subject: [PATCH 3/6] isisd: fix network-type configuration Reported-by: Martin Winter Signed-off-by: David Lamparter --- isisd/isis_circuit.c | 1 - isisd/isis_vty.c | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c index ad1eca89e3..ac41b55b87 100644 --- a/isisd/isis_circuit.c +++ b/isisd/isis_circuit.c @@ -1358,7 +1358,6 @@ isis_circuit_circ_type_set(struct isis_circuit *circuit, int circ_type) * is not supported. */ if (circ_type == CIRCUIT_T_UNKNOWN || circ_type == CIRCUIT_T_LOOPBACK - || circuit->circ_type == CIRCUIT_T_UNKNOWN || circuit->circ_type == CIRCUIT_T_LOOPBACK) { if (circuit->circ_type != circ_type) diff --git a/isisd/isis_vty.c b/isisd/isis_vty.c index 3ce06b83da..f9b96a42d8 100644 --- a/isisd/isis_vty.c +++ b/isisd/isis_vty.c @@ -255,7 +255,7 @@ DEFUN (isis_network, if (!circuit) return CMD_ERR_NO_MATCH; - if (!isis_circuit_circ_type_set(circuit, CIRCUIT_T_P2P)) + if (isis_circuit_circ_type_set(circuit, CIRCUIT_T_P2P)) { vty_out (vty, "isis network point-to-point " "is valid only on broadcast interfaces%s", @@ -278,7 +278,7 @@ DEFUN (no_isis_network, if (!circuit) return CMD_ERR_NO_MATCH; - if (!isis_circuit_circ_type_set(circuit, CIRCUIT_T_BROADCAST)) + if (isis_circuit_circ_type_set(circuit, CIRCUIT_T_BROADCAST)) { vty_out (vty, "isis network point-to-point " "is valid only on broadcast interfaces%s", From 791ffe38a5e87236fef7943b28d4db7caca768b4 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sat, 13 Aug 2016 01:20:20 +0200 Subject: [PATCH 4/6] isisd: fix is_type_set Code's "is_type" is "circuit-type" in CLI, "circuit_type" is "network" (type) in CLI, and the function to change is_type is isis_event_circuit_type_change()... *headdesk* Reported-by: Martin Winter Signed-off-by: David Lamparter --- isisd/isis_circuit.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c index ac41b55b87..300c2cd85e 100644 --- a/isisd/isis_circuit.c +++ b/isisd/isis_circuit.c @@ -1285,10 +1285,10 @@ isis_circuit_passive_set (struct isis_circuit *circuit, bool passive) } void -isis_circuit_is_type_set (struct isis_circuit *circuit, int circ_type) +isis_circuit_is_type_set (struct isis_circuit *circuit, int is_type) { - if (circuit->circ_type != circ_type) - isis_event_circuit_type_change (circuit, circ_type); + if (circuit->is_type != is_type) + isis_event_circuit_type_change (circuit, is_type); } int From f1d489ea29240037c6579d2ee5134dce11b7d511 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sat, 13 Aug 2016 01:32:52 +0200 Subject: [PATCH 5/6] isisd: fold up isis_circuit_is_type_set() see previous commit. Signed-off-by: David Lamparter --- isisd/isis_circuit.c | 7 ------- isisd/isis_events.c | 2 +- isisd/isisd.c | 2 +- 3 files changed, 2 insertions(+), 9 deletions(-) diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c index 300c2cd85e..efed420161 100644 --- a/isisd/isis_circuit.c +++ b/isisd/isis_circuit.c @@ -1284,13 +1284,6 @@ isis_circuit_passive_set (struct isis_circuit *circuit, bool passive) return 0; } -void -isis_circuit_is_type_set (struct isis_circuit *circuit, int is_type) -{ - if (circuit->is_type != is_type) - isis_event_circuit_type_change (circuit, is_type); -} - int isis_circuit_metric_set (struct isis_circuit *circuit, int level, int metric) { diff --git a/isisd/isis_events.c b/isisd/isis_events.c index 8fb92fdd39..460b1d25ba 100644 --- a/isisd/isis_events.c +++ b/isisd/isis_events.c @@ -145,7 +145,7 @@ circuit_resign_level (struct isis_circuit *circuit, int level) } void -isis_event_circuit_type_change (struct isis_circuit *circuit, int newtype) +isis_circuit_is_type_set (struct isis_circuit *circuit, int newtype) { if (circuit->state != C_STATE_UP) { diff --git a/isisd/isisd.c b/isisd/isisd.c index 62c5e26538..0d0b805a20 100644 --- a/isisd/isisd.c +++ b/isisd/isisd.c @@ -1775,7 +1775,7 @@ isis_area_is_type_set(struct isis_area *area, int is_type) if (area->is_type != IS_LEVEL_1_AND_2) { for (ALL_LIST_ELEMENTS_RO (area->circuit_list, node, circuit)) - isis_event_circuit_type_change (circuit, is_type); + isis_circuit_is_type_set (circuit, is_type); } spftree_area_init (area); From 83cd1f1aab571b479000a50d0d322a8c44ab409f Mon Sep 17 00:00:00 2001 From: Christian Franke Date: Mon, 15 Aug 2016 13:36:59 +0200 Subject: [PATCH 6/6] isisd: warn if there is an MTU issue on circuits Instead of later tripping over an assert, add a proper warning for interfaces whose MTU is too low. Signed-off-by: David Lamparter --- isisd/isis_circuit.c | 3 ++- isisd/isis_vty.c | 9 ++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/isisd/isis_circuit.c b/isisd/isis_circuit.c index efed420161..728cbdc67d 100644 --- a/isisd/isis_circuit.c +++ b/isisd/isis_circuit.c @@ -1230,7 +1230,8 @@ isis_circuit_create (struct isis_area *area, struct interface *ifp) if (circuit && circuit->area) return NULL; circuit = isis_csm_state_change (ISIS_ENABLE, circuit, area); - assert (circuit->state == C_STATE_CONF || circuit->state == C_STATE_UP); + if (circuit->state != C_STATE_CONF && circuit->state != C_STATE_UP) + return circuit; isis_circuit_if_bind (circuit, ifp); return circuit; } diff --git a/isisd/isis_vty.c b/isisd/isis_vty.c index f9b96a42d8..4148eb55b9 100644 --- a/isisd/isis_vty.c +++ b/isisd/isis_vty.c @@ -86,9 +86,16 @@ DEFUN (ip_router_isis, if (!area) area = isis_area_create (area_tag); - if (!circuit || !circuit->area) + if (!circuit || !circuit->area) { circuit = isis_circuit_create (area, ifp); + if (circuit->state != C_STATE_CONF && circuit->state != C_STATE_UP) + { + vty_out(vty, "Couldn't bring up interface, please check log.%s", VTY_NEWLINE); + return CMD_WARNING; + } + } + bool ip = circuit->ip_router, ipv6 = circuit->ipv6_router; if (af[2] != '\0') ipv6 = true;