From 1f2c2743ac188b909114a1bf054a9a41a0cd5635 Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Mon, 10 Jul 2006 07:45:13 +0000 Subject: [ospfd] cleanup NSM neighbour delete through a new Deleted NSM state 2006-07-07 Paul Jakma * ospf_nsm.h: Add a NSM_Deleted neighbour state, to act as dummy state indicating the neighbour is to be deleted. * ospf_nsm.c: (general) Use the NSM_Deleted state to delete neighbours, thus allowing code to be slightly more obvious in its flow. (nsm_timer_set) Add NSM_Deleted. Add another timer the code missed. (nsm_kill_nbr) No need for special case call to nsm_change_state anymore. Make the assert and error-handling for same case more readable (Andrew Schorr) Remove the call to ospf_nbr_delete, nsm_change_state can do this generally now via NSM_Deleted. (struct ... NSM) Add the dummy NSM_Deleted state, the 3 events that can lead to nsm_kill_nbr all now transition the NBR to NSM_Deleted and the general change_state function can be left to do the work. (ospf_nsm_event) Special casing of events and early-return can be removed now. On transition into Deleted, delete the nbr. * ospf_dump.c: (ospf_nsm_state_msg) Add Deleted. --- ospfd/ChangeLog | 24 ++++++++++ ospfd/ospf_dump.c | 1 + ospfd/ospf_nsm.c | 134 ++++++++++++++++++++++++++++-------------------------- ospfd/ospf_nsm.h | 19 ++++---- 4 files changed, 105 insertions(+), 73 deletions(-) diff --git a/ospfd/ChangeLog b/ospfd/ChangeLog index b4f7d3e7..7c374fb8 100644 --- a/ospfd/ChangeLog +++ b/ospfd/ChangeLog @@ -1,3 +1,27 @@ +2006-07-07 Paul Jakma + + * ospf_nsm.h: Add a NSM_Deleted neighbour state, to act as dummy + state indicating the neighbour is to be deleted. + * ospf_nsm.c: (general) Use the NSM_Deleted state to delete + neighbours, thus allowing code to be slightly more obvious + in its flow. + (nsm_timer_set) Add NSM_Deleted. Add another timer the code + missed. + (nsm_kill_nbr) No need for special case call to nsm_change_state + anymore. + Make the assert and error-handling for same case more readable + (Andrew Schorr) + Remove the call to ospf_nbr_delete, nsm_change_state can do + this generally now via NSM_Deleted. + (struct ... NSM) Add the dummy NSM_Deleted state, the 3 events + that can lead to nsm_kill_nbr all now transition the NBR to + NSM_Deleted and the general change_state function can be left + to do the work. + (ospf_nsm_event) Special casing of events and early-return can + be removed now. + On transition into Deleted, delete the nbr. + * ospf_dump.c: (ospf_nsm_state_msg) Add Deleted. + 2006-07-06 Paul Jakma * ospf_nsm.c: (ospf_nsm_event) LLDown event also results in nbr diff --git a/ospfd/ospf_dump.c b/ospfd/ospf_dump.c index 47b76fcb..b8dc7951 100644 --- a/ospfd/ospf_dump.c +++ b/ospfd/ospf_dump.c @@ -58,6 +58,7 @@ int ospf_ism_state_msg_max = OSPF_ISM_STATE_MAX; struct message ospf_nsm_state_msg[] = { { NSM_DependUpon, "DependUpon" }, + { NSM_Deleted, "Deleted" }, { NSM_Down, "Down" }, { NSM_Attempt, "Attempt" }, { NSM_Init, "Init" }, diff --git a/ospfd/ospf_nsm.c b/ospfd/ospf_nsm.c index fb736eba..56f81865 100644 --- a/ospfd/ospf_nsm.c +++ b/ospfd/ospf_nsm.c @@ -95,18 +95,24 @@ ospf_db_desc_timer (struct thread *thread) return 0; } -/* Hook function called after ospf NSM event is occured. */ - +/* Hook function called after ospf NSM event is occured. + * + * Set/clear any timers whose condition is implicit to the neighbour + * state. There may be other timers which are set/unset according to other + * state. + * + * We rely on this function to properly clear timers in lower states, + * particularly before deleting a neighbour. + */ static void nsm_timer_set (struct ospf_neighbor *nbr) { switch (nbr->state) { + case NSM_Deleted: case NSM_Down: - /* This is here for documentation purposes, don't actually get here - * as Down neighbours are deleted typically, see nsm_kill_nbr - */ OSPF_NSM_TIMER_OFF (nbr->t_inactivity); + OSPF_NSM_TIMER_OFF (nbr->t_hello_reply); case NSM_Attempt: case NSM_Init: case NSM_TwoWay: @@ -332,9 +338,6 @@ nsm_exchange_done (struct ospf_neighbor *nbr) if (ospf_ls_request_isempty (nbr)) return NSM_Full; - /* Cancel dd retransmit timer. */ - /* OSPF_NSM_TIMER_OFF (nbr->t_db_desc); */ - /* Send Link State Request. */ ospf_ls_req_send (nbr); @@ -422,13 +425,12 @@ nsm_reset_nbr (struct ospf_neighbor *nbr) static int nsm_kill_nbr (struct ospf_neighbor *nbr) { - /* call it here because we cannot call it from ospf_nsm_event */ - nsm_change_state (nbr, NSM_Down); - /* killing nbr_self is invalid */ - assert (nbr != nbr->oi->nbr_self); if (nbr == nbr->oi->nbr_self) - return 0; + { + assert (nbr != nbr->oi->nbr_self); + return 0; + } /* Reset neighbor. */ nsm_reset_nbr (nbr); @@ -450,9 +452,6 @@ nsm_kill_nbr (struct ospf_neighbor *nbr) IF_NAME (nbr->oi), inet_ntoa (nbr->address.u.prefix4)); } - /* Delete neighbor from interface. */ - ospf_nbr_delete (nbr); - return 0; } @@ -468,9 +467,6 @@ nsm_inactivity_timer (struct ospf_neighbor *nbr) static int nsm_ll_down (struct ospf_neighbor *nbr) { - /* Reset neighbor. */ - /*nsm_reset_nbr (nbr);*/ - /* Kill neighbor. */ nsm_kill_nbr (nbr); @@ -500,6 +496,23 @@ struct { { nsm_ignore, NSM_DependUpon }, /* InactivityTimer */ { nsm_ignore, NSM_DependUpon }, /* LLDown */ }, + { + /* Deleted: dummy state. */ + { nsm_ignore, NSM_Deleted }, /* NoEvent */ + { nsm_ignore, NSM_Deleted }, /* HelloReceived */ + { nsm_ignore, NSM_Deleted }, /* Start */ + { nsm_ignore, NSM_Deleted }, /* 2-WayReceived */ + { nsm_ignore, NSM_Deleted }, /* NegotiationDone */ + { nsm_ignore, NSM_Deleted }, /* ExchangeDone */ + { nsm_ignore, NSM_Deleted }, /* BadLSReq */ + { nsm_ignore, NSM_Deleted }, /* LoadingDone */ + { nsm_ignore, NSM_Deleted }, /* AdjOK? */ + { nsm_ignore, NSM_Deleted }, /* SeqNumberMismatch */ + { nsm_ignore, NSM_Deleted }, /* 1-WayReceived */ + { nsm_ignore, NSM_Deleted }, /* KillNbr */ + { nsm_ignore, NSM_Deleted }, /* InactivityTimer */ + { nsm_ignore, NSM_Deleted }, /* LLDown */ + }, { /* Down: */ { nsm_ignore, NSM_DependUpon }, /* NoEvent */ @@ -513,9 +526,9 @@ struct { { nsm_ignore, NSM_Down }, /* AdjOK? */ { nsm_ignore, NSM_Down }, /* SeqNumberMismatch */ { nsm_ignore, NSM_Down }, /* 1-WayReceived */ - { nsm_kill_nbr, NSM_Down }, /* KillNbr */ - { nsm_inactivity_timer, NSM_Down }, /* InactivityTimer */ - { nsm_ll_down, NSM_Down }, /* LLDown */ + { nsm_kill_nbr, NSM_Deleted }, /* KillNbr */ + { nsm_inactivity_timer, NSM_Deleted }, /* InactivityTimer */ + { nsm_ll_down, NSM_Deleted }, /* LLDown */ }, { /* Attempt: */ @@ -530,9 +543,9 @@ struct { { nsm_ignore, NSM_Attempt }, /* AdjOK? */ { nsm_ignore, NSM_Attempt }, /* SeqNumberMismatch */ { nsm_ignore, NSM_Attempt }, /* 1-WayReceived */ - { nsm_kill_nbr, NSM_Down }, /* KillNbr */ - { nsm_inactivity_timer, NSM_Down }, /* InactivityTimer */ - { nsm_ll_down, NSM_Down }, /* LLDown */ + { nsm_kill_nbr, NSM_Deleted }, /* KillNbr */ + { nsm_inactivity_timer, NSM_Deleted }, /* InactivityTimer */ + { nsm_ll_down, NSM_Deleted }, /* LLDown */ }, { /* Init: */ @@ -547,9 +560,9 @@ struct { { nsm_ignore, NSM_Init }, /* AdjOK? */ { nsm_ignore, NSM_Init }, /* SeqNumberMismatch */ { nsm_ignore, NSM_Init }, /* 1-WayReceived */ - { nsm_kill_nbr, NSM_Down }, /* KillNbr */ - { nsm_inactivity_timer, NSM_Down }, /* InactivityTimer */ - { nsm_ll_down, NSM_Down }, /* LLDown */ + { nsm_kill_nbr, NSM_Deleted }, /* KillNbr */ + { nsm_inactivity_timer, NSM_Deleted }, /* InactivityTimer */ + { nsm_ll_down, NSM_Deleted }, /* LLDown */ }, { /* 2-Way: */ @@ -564,9 +577,9 @@ struct { { nsm_adj_ok, NSM_DependUpon }, /* AdjOK? */ { nsm_ignore, NSM_TwoWay }, /* SeqNumberMismatch */ { nsm_oneway_received, NSM_Init }, /* 1-WayReceived */ - { nsm_kill_nbr, NSM_Down }, /* KillNbr */ - { nsm_inactivity_timer, NSM_Down }, /* InactivityTimer */ - { nsm_ll_down, NSM_Down }, /* LLDown */ + { nsm_kill_nbr, NSM_Deleted }, /* KillNbr */ + { nsm_inactivity_timer, NSM_Deleted }, /* InactivityTimer */ + { nsm_ll_down, NSM_Deleted }, /* LLDown */ }, { /* ExStart: */ @@ -581,9 +594,9 @@ struct { { nsm_adj_ok, NSM_DependUpon }, /* AdjOK? */ { nsm_ignore, NSM_ExStart }, /* SeqNumberMismatch */ { nsm_oneway_received, NSM_Init }, /* 1-WayReceived */ - { nsm_kill_nbr, NSM_Down }, /* KillNbr */ - { nsm_inactivity_timer, NSM_Down }, /* InactivityTimer */ - { nsm_ll_down, NSM_Down }, /* LLDown */ + { nsm_kill_nbr, NSM_Deleted }, /* KillNbr */ + { nsm_inactivity_timer, NSM_Deleted }, /* InactivityTimer */ + { nsm_ll_down, NSM_Deleted }, /* LLDown */ }, { /* Exchange: */ @@ -598,9 +611,9 @@ struct { { nsm_adj_ok, NSM_DependUpon }, /* AdjOK? */ { nsm_seq_number_mismatch, NSM_ExStart }, /* SeqNumberMismatch */ { nsm_oneway_received, NSM_Init }, /* 1-WayReceived */ - { nsm_kill_nbr, NSM_Down }, /* KillNbr */ - { nsm_inactivity_timer, NSM_Down }, /* InactivityTimer */ - { nsm_ll_down, NSM_Down }, /* LLDown */ + { nsm_kill_nbr, NSM_Deleted }, /* KillNbr */ + { nsm_inactivity_timer, NSM_Deleted }, /* InactivityTimer */ + { nsm_ll_down, NSM_Deleted }, /* LLDown */ }, { /* Loading: */ @@ -615,9 +628,9 @@ struct { { nsm_adj_ok, NSM_DependUpon }, /* AdjOK? */ { nsm_seq_number_mismatch, NSM_ExStart }, /* SeqNumberMismatch */ { nsm_oneway_received, NSM_Init }, /* 1-WayReceived */ - { nsm_kill_nbr, NSM_Down }, /* KillNbr */ - { nsm_inactivity_timer, NSM_Down }, /* InactivityTimer */ - { nsm_ll_down, NSM_Down }, /* LLDown */ + { nsm_kill_nbr, NSM_Deleted }, /* KillNbr */ + { nsm_inactivity_timer, NSM_Deleted }, /* InactivityTimer */ + { nsm_ll_down, NSM_Deleted }, /* LLDown */ }, { /* Full: */ { nsm_ignore, NSM_DependUpon }, /* NoEvent */ @@ -631,9 +644,9 @@ struct { { nsm_adj_ok, NSM_DependUpon }, /* AdjOK? */ { nsm_seq_number_mismatch, NSM_ExStart }, /* SeqNumberMismatch */ { nsm_oneway_received, NSM_Init }, /* 1-WayReceived */ - { nsm_kill_nbr, NSM_Down }, /* KillNbr */ - { nsm_inactivity_timer, NSM_Down }, /* InactivityTimer */ - { nsm_ll_down, NSM_Down }, /* LLDown */ + { nsm_kill_nbr, NSM_Deleted }, /* KillNbr */ + { nsm_inactivity_timer, NSM_Deleted }, /* InactivityTimer */ + { nsm_ll_down, NSM_Deleted }, /* LLDown */ }, }; @@ -689,9 +702,9 @@ nsm_change_state (struct ospf_neighbor *nbr, int state) (CHECK_FLAG(oi->ospf->config, OSPF_LOG_ADJACENCY_DETAIL) || (state == NSM_Full) || (state < old_state))) zlog_notice("AdjChg: Nbr %s on %s: %s -> %s", - inet_ntoa (nbr->router_id), IF_NAME (nbr->oi), - LOOKUP (ospf_nsm_state_msg, old_state), - LOOKUP (ospf_nsm_state_msg, state)); + inet_ntoa (nbr->router_id), IF_NAME (nbr->oi), + LOOKUP (ospf_nsm_state_msg, old_state), + LOOKUP (ospf_nsm_state_msg, state)); #ifdef HAVE_SNMP /* Terminal state or regression */ @@ -701,7 +714,7 @@ nsm_change_state (struct ospf_neighbor *nbr, int state) if (oi->type == OSPF_IFTYPE_VIRTUALLINK) ospfTrapVirtNbrStateChange(nbr); /* ospfNbrStateChange trap */ - else + else /* To/From FULL, only managed by DR */ if (((state != NSM_Full) && (old_state != NSM_Full)) || (oi->state == ISM_DR)) @@ -855,23 +868,6 @@ ospf_nsm_event (struct thread *thread) /* Call function. */ next_state = (*(NSM [nbr->state][event].func))(nbr); - /* When event is NSM_KillNbr or InactivityTimer, the neighbor is - deleted. */ - if (event == NSM_KillNbr - || event == NSM_InactivityTimer - || event == NSM_LLDown) - { - if (IS_DEBUG_OSPF (nsm, NSM_EVENTS)) - zlog_debug ("NSM[%s:%s]: neighbor deleted", - IF_NAME (oi), inet_ntoa (router_id)); - - /* Timers are canceled in ospf_nbr_free, moreover we cannot call - nsm_timer_set here because nbr is freed already!!!*/ - /*nsm_timer_set (nbr);*/ - - return 0; - } - if (! next_state) next_state = NSM [nbr->state][event].next_state; else if (NSM [nbr->state][event].next_state != NSM_DependUpon) @@ -904,6 +900,16 @@ ospf_nsm_event (struct thread *thread) /* Make sure timer is set. */ nsm_timer_set (nbr); + /* When event is NSM_KillNbr, InactivityTimer or LLDown, the neighbor + * is deleted. + * + * Rather than encode knowledge here of which events lead to NBR + * delete, we take our cue from the NSM table, via the dummy + * 'Deleted' neighbour state. + */ + if (nbr->state == NSM_Deleted) + ospf_nbr_delete (nbr); + return 0; } diff --git a/ospfd/ospf_nsm.h b/ospfd/ospf_nsm.h index fe42f7a0..1121dae6 100644 --- a/ospfd/ospf_nsm.h +++ b/ospfd/ospf_nsm.h @@ -26,15 +26,16 @@ /* OSPF Neighbor State Machine State. */ #define NSM_DependUpon 0 -#define NSM_Down 1 -#define NSM_Attempt 2 -#define NSM_Init 3 -#define NSM_TwoWay 4 -#define NSM_ExStart 5 -#define NSM_Exchange 6 -#define NSM_Loading 7 -#define NSM_Full 8 -#define OSPF_NSM_STATE_MAX 9 +#define NSM_Deleted 1 +#define NSM_Down 2 +#define NSM_Attempt 3 +#define NSM_Init 4 +#define NSM_TwoWay 5 +#define NSM_ExStart 6 +#define NSM_Exchange 7 +#define NSM_Loading 8 +#define NSM_Full 9 +#define OSPF_NSM_STATE_MAX 10 /* OSPF Neighbor State Machine Event. */ #define NSM_NoEvent 0 -- cgit v1.2.1