From e4229afd5fdb7bb0921683c877226b41d657b1cf Mon Sep 17 00:00:00 2001 From: Emanuele Di Pascale Date: Wed, 23 Sep 2020 16:37:21 +0200 Subject: [PATCH 1/2] isisd: simplify adj_change hook call There is no need to call isis_adj_state_change_hook once per level in isis_adj_state_change, we can just do it once at the end. Signed-off-by: Emanuele Di Pascale --- isisd/isis_adjacency.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/isisd/isis_adjacency.c b/isisd/isis_adjacency.c index f7cdd58f72..f0cf93ac4b 100644 --- a/isisd/isis_adjacency.c +++ b/isisd/isis_adjacency.c @@ -304,7 +304,6 @@ void isis_adj_state_change(struct isis_adjacency **padj, continue; if (new_state == ISIS_ADJ_UP) { circuit->upadjcount[level - 1]++; - hook_call(isis_adj_state_change_hook, adj); /* update counter & timers for debugging * purposes */ adj->last_flap = time(NULL); @@ -317,7 +316,6 @@ void isis_adj_state_change(struct isis_adjacency **padj, if (circuit->upadjcount[level - 1] == 0) isis_tx_queue_clean(circuit->tx_queue); - hook_call(isis_adj_state_change_hook, adj); if (new_state == ISIS_ADJ_DOWN) del = true; } @@ -342,7 +340,6 @@ void isis_adj_state_change(struct isis_adjacency **padj, continue; if (new_state == ISIS_ADJ_UP) { circuit->upadjcount[level - 1]++; - hook_call(isis_adj_state_change_hook, adj); /* update counter & timers for debugging * purposes */ @@ -365,13 +362,14 @@ void isis_adj_state_change(struct isis_adjacency **padj, if (circuit->upadjcount[level - 1] == 0) isis_tx_queue_clean(circuit->tx_queue); - hook_call(isis_adj_state_change_hook, adj); if (new_state == ISIS_ADJ_DOWN) del = true; } } } + hook_call(isis_adj_state_change_hook, adj); + if (del) { isis_delete_adj(adj); *padj = NULL; From 3cbe31c7980723e120404db3d8bb4f886118f1f2 Mon Sep 17 00:00:00 2001 From: Emanuele Di Pascale Date: Wed, 23 Sep 2020 16:46:44 +0200 Subject: [PATCH 2/2] isisd: guard against adj timer display overflow An adjacency should be removed when the holdtimer expires, but if the system is overloaded we may end up doing it late. In the meanwhile vtysh will display an incorrect value in the show isis neighbor output, due to an overflow of the unsigned variable used to display the Holdtime, e.g.: pe1# show isis neighbor Area test: System Id Interface L state Holdtime SNPA Spirent-1 2.201 1 Down 26 2020.2020.2020 Spirent-1 2.203 1 Up 21 2020.2020.2020 Spirent-1 2.204 1 Up 18446744073709551615 2020.2020.2020 Spirent-1 2.207 1 Up 18446744073709551615 2020.2020.2020 Spirent-1 2.208 1 Up 18446744073709551615 2020.2020.2020 Spirent-1 2.209 1 Up 0 2020.2020.2020 Spirent-1 2.210 1 Up 18446744073709551615 2020.2020.2020 pe2 12.200 1 Up 30 2020.2020.2020 Guard against that by printing an "Expiring" message instead. Signed-off-by: Emanuele Di Pascale --- isisd/isis_adjacency.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/isisd/isis_adjacency.c b/isisd/isis_adjacency.c index f0cf93ac4b..5bfbb2cf7e 100644 --- a/isisd/isis_adjacency.c +++ b/isisd/isis_adjacency.c @@ -465,11 +465,15 @@ void isis_adj_print_vty(struct isis_adjacency *adj, struct vty *vty, vty_out(vty, "%-3u", adj->level); /* level */ vty_out(vty, "%-13s", adj_state2string(adj->adj_state)); now = time(NULL); - if (adj->last_upd) - vty_out(vty, "%-9llu", - (unsigned long long)adj->last_upd - + adj->hold_time - now); - else + if (adj->last_upd) { + if (adj->last_upd + adj->hold_time + < (unsigned long long)now) + vty_out(vty, " Expiring"); + else + vty_out(vty, " %-9llu", + (unsigned long long)adj->last_upd + + adj->hold_time - now); + } else vty_out(vty, "- "); vty_out(vty, "%-10s", snpa_print(adj->snpa)); vty_out(vty, "\n"); @@ -489,11 +493,15 @@ void isis_adj_print_vty(struct isis_adjacency *adj, struct vty *vty, vty_out(vty, ", Level: %u", adj->level); /* level */ vty_out(vty, ", State: %s", adj_state2string(adj->adj_state)); now = time(NULL); - if (adj->last_upd) - vty_out(vty, ", Expires in %s", - time2string(adj->last_upd + adj->hold_time - - now)); - else + if (adj->last_upd) { + if (adj->last_upd + adj->hold_time + < (unsigned long long)now) + vty_out(vty, " Expiring"); + else + vty_out(vty, ", Expires in %s", + time2string(adj->last_upd + + adj->hold_time - now)); + } else vty_out(vty, ", Expires in %s", time2string(adj->hold_time)); vty_out(vty, "\n");