From 61cd5761a38b20a0e0ae44071f46bc36fbdbe611 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 22 Apr 2021 14:46:53 -0400 Subject: [PATCH 1/3] isisd: use an enum for circuit states Signed-off-by: Donald Sharp --- isisd/isis_circuit.h | 3 ++- isisd/isis_csm.c | 5 +---- isisd/isis_csm.h | 10 ++++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/isisd/isis_circuit.h b/isisd/isis_circuit.h index cbe4040b64..d4c7baea1a 100644 --- a/isisd/isis_circuit.h +++ b/isisd/isis_circuit.h @@ -31,6 +31,7 @@ #include "isis_constants.h" #include "isis_common.h" +#include "isis_csm.h" DECLARE_HOOK(isis_if_new_hook, (struct interface *ifp), (ifp)); @@ -77,7 +78,7 @@ struct isis_circuit_arg { }; struct isis_circuit { - int state; + enum isis_circuit_state state; uint8_t circuit_id; /* l1/l2 bcast CircuitID */ time_t last_uptime; struct isis *isis; diff --git a/isisd/isis_csm.c b/isisd/isis_csm.c index 736d8d63f9..6214a6b8cc 100644 --- a/isisd/isis_csm.c +++ b/isisd/isis_csm.c @@ -63,7 +63,7 @@ static const char *const csm_eventstr[] = { struct isis_circuit * isis_csm_state_change(int event, struct isis_circuit *circuit, void *arg) { - int old_state; + enum isis_circuit_state old_state; struct isis *isis = NULL; struct isis_area *area = NULL; @@ -197,9 +197,6 @@ isis_csm_state_change(int event, struct isis_circuit *circuit, void *arg) break; } break; - - default: - zlog_warn("Invalid circuit state %d", old_state); } if (IS_DEBUG_EVENTS) diff --git a/isisd/isis_csm.h b/isisd/isis_csm.h index 53a5f9d5d0..9da06fb1a6 100644 --- a/isisd/isis_csm.h +++ b/isisd/isis_csm.h @@ -27,10 +27,12 @@ /* * Circuit states */ -#define C_STATE_NA 0 -#define C_STATE_INIT 1 /* Connected to interface */ -#define C_STATE_CONF 2 /* Configured for ISIS */ -#define C_STATE_UP 3 /* CONN | CONF */ +enum isis_circuit_state { + C_STATE_NA, + C_STATE_INIT, /* Connected to interface */ + C_STATE_CONF, /* Configured for ISIS */ + C_STATE_UP, /* CONN | CONF */ +}; /* * Circuit events From 9d454ad27f154e664861be25da6863ffe5866b48 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 22 Apr 2021 14:52:40 -0400 Subject: [PATCH 2/3] isisd: Use enum for circuit state Signed-off-by: Donald Sharp --- isisd/isis_csm.c | 5 +++-- isisd/isis_csm.h | 15 +++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/isisd/isis_csm.c b/isisd/isis_csm.c index 6214a6b8cc..0ac9aa0950 100644 --- a/isisd/isis_csm.c +++ b/isisd/isis_csm.c @@ -60,8 +60,9 @@ static const char *const csm_eventstr[] = { #define EVENT2STR(E) csm_eventstr[E] -struct isis_circuit * -isis_csm_state_change(int event, struct isis_circuit *circuit, void *arg) +struct isis_circuit *isis_csm_state_change(enum isis_circuit_event event, + struct isis_circuit *circuit, + void *arg) { enum isis_circuit_state old_state; struct isis *isis = NULL; diff --git a/isisd/isis_csm.h b/isisd/isis_csm.h index 9da06fb1a6..ad72ff5113 100644 --- a/isisd/isis_csm.h +++ b/isisd/isis_csm.h @@ -37,12 +37,15 @@ enum isis_circuit_state { /* * Circuit events */ -#define ISIS_ENABLE 1 -#define IF_UP_FROM_Z 2 -#define ISIS_DISABLE 3 -#define IF_DOWN_FROM_Z 4 +enum isis_circuit_event { + ISIS_ENABLE = 1, + IF_UP_FROM_Z, + ISIS_DISABLE, + IF_DOWN_FROM_Z, +}; -struct isis_circuit * -isis_csm_state_change(int event, struct isis_circuit *circuit, void *arg); +struct isis_circuit *isis_csm_state_change(enum isis_circuit_event event, + struct isis_circuit *circuit, + void *arg); #endif /* _ZEBRA_ISIS_CSM_H */ From c5b8ce0620a9b9558569224ee28b5dfc95e71836 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 22 Apr 2021 15:04:15 -0400 Subject: [PATCH 3/3] isisd: Remove warnings and add some data to debugs for isis_csm.c When running isis and not running isis on all interfaces results in a bunch of warn messages to the log about circuit state changes. These warn messages also didn't bother to inform the end user what interface was causing the fun. Since the end operator cannot do anything with these warn messages and nor should they in the vast array of normal operations modify the code to use event debugging and turn the warns to debugs. Additionally add some information to clue the operator in on to what actual interface we are talking about. Signed-off-by: Donald Sharp --- isisd/isis_csm.c | 46 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/isisd/isis_csm.c b/isisd/isis_csm.c index 0ac9aa0950..ebb68ba3f0 100644 --- a/isisd/isis_csm.c +++ b/isisd/isis_csm.c @@ -67,6 +67,7 @@ struct isis_circuit *isis_csm_state_change(enum isis_circuit_event event, 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; if (IS_DEBUG_EVENTS) @@ -86,23 +87,29 @@ struct isis_circuit *isis_csm_state_change(enum isis_circuit_event event, circuit->state = C_STATE_CONF; break; case IF_UP_FROM_Z: - isis = isis_lookup_by_vrfid(((struct interface *)arg)->vrf_id); + ifp = arg; + isis = isis_lookup_by_vrfid(ifp->vrf_id); if (isis == NULL) { - zlog_warn( - " %s : ISIS routing instance not found", - __func__); + 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, (struct interface *)arg); + isis_circuit_if_add(circuit, ifp); listnode_add(isis->init_circ_list, circuit); circuit->state = C_STATE_INIT; break; case ISIS_DISABLE: - zlog_warn("circuit already disabled"); + if (IS_DEBUG_EVENTS) + zlog_debug( + "circuit disable event passed for a non existent circuit"); break; case IF_DOWN_FROM_Z: - zlog_warn("circuit already disconnected"); + if (IS_DEBUG_EVENTS) + zlog_debug( + "circuit disconnect event passed for a non existent circuit"); break; } break; @@ -124,11 +131,14 @@ struct isis_circuit *isis_csm_state_change(enum isis_circuit_event event, circuit); break; case IF_UP_FROM_Z: - assert(circuit); - zlog_warn("circuit already connected"); + if (IS_DEBUG_EVENTS) + zlog_debug("circuit %s already connected", + circuit->interface->name); break; case ISIS_DISABLE: - zlog_warn("circuit already disabled"); + if (IS_DEBUG_EVENTS) + zlog_debug("circuit %s already disabled", + circuit->interface->name); break; case IF_DOWN_FROM_Z: isis_circuit_if_del(circuit, (struct interface *)arg); @@ -143,7 +153,9 @@ struct isis_circuit *isis_csm_state_change(enum isis_circuit_event event, assert(circuit); switch (event) { case ISIS_ENABLE: - zlog_warn("circuit already enabled"); + if (IS_DEBUG_EVENTS) + zlog_debug("circuit %p is already enabled", + circuit); break; case IF_UP_FROM_Z: isis_circuit_if_add(circuit, (struct interface *)arg); @@ -166,7 +178,9 @@ struct isis_circuit *isis_csm_state_change(enum isis_circuit_event event, circuit = NULL; break; case IF_DOWN_FROM_Z: - zlog_warn("circuit already disconnected"); + if (IS_DEBUG_EVENTS) + zlog_debug("circuit %p already disconnected", + circuit); break; } break; @@ -174,10 +188,14 @@ struct isis_circuit *isis_csm_state_change(enum isis_circuit_event event, assert(circuit); switch (event) { case ISIS_ENABLE: - zlog_warn("circuit already configured"); + if (IS_DEBUG_EVENTS) + zlog_debug("circuit %s already configured", + circuit->interface->name); break; case IF_UP_FROM_Z: - zlog_warn("circuit already connected"); + if (IS_DEBUG_EVENTS) + zlog_debug("circuit %s already connected", + circuit->interface->name); break; case ISIS_DISABLE: isis = circuit->isis;