[quagga-dev 4218] Re: possible ospfd bug: in state ExStart with
routers that are neither DR nor BDR
Paul Jakma
paul at clubi.ie
Thu Jul 6 03:44:54 IST 2006
On Thu, 6 Jul 2006, Paul Jakma wrote:
> It's a bit hacky though, I wonder could we add an additional
> NSM_Deleted dummy-state so that it'd be more obvious from the nsm
> table (or the result of an NSM_DependUpon action) when the
> early-return is required.
E.g. something like the completely untested patch (for illustrative
purposes only ;) ).
regards,
--
Paul Jakma paul at clubi.ie paul at jakma.org Key ID: 64A2FF6A
Fortune:
interest, n.:
What borrowers pay, lenders receive, stockholders own, and
burned out employees must feign.
-------------- next part --------------
diff --git a/ospfd/ospf_nsm.c b/ospfd/ospf_nsm.c
index 11da503..3b1287f 100644
--- a/ospfd/ospf_nsm.c
+++ b/ospfd/ospf_nsm.c
@@ -103,9 +103,6 @@ nsm_timer_set (struct ospf_neighbor *nbr
switch (nbr->state)
{
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);
case NSM_Attempt:
case NSM_Init:
@@ -422,9 +419,6 @@ #endif /* HAVE_OPAQUE_LSA */
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)
@@ -450,9 +444,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 +459,6 @@ nsm_inactivity_timer (struct ospf_neighb
static int
nsm_ll_down (struct ospf_neighbor *nbr)
{
- /* Reset neighbor. */
- /*nsm_reset_nbr (nbr);*/
-
/* Kill neighbor. */
nsm_kill_nbr (nbr);
@@ -501,6 +489,23 @@ struct {
{ 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 */
{ nsm_hello_received, NSM_Init }, /* HelloReceived */
@@ -513,9 +518,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 +535,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 +552,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 +569,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 +586,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 +603,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 +620,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 +636,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 */
},
};
@@ -855,20 +860,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)
- {
- 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;
@@ -899,8 +890,25 @@ ospf_nsm_event (struct thread *thread)
if (next_state != nbr->state)
nsm_change_state (nbr, next_state);
- /* 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 (next_state = NSM_Deleted)
+ {
+ if (IS_DEBUG_OSPF (nsm, NSM_EVENTS))
+ zlog_debug ("NSM[%s:%s]: neighbor deleted",
+ IF_NAME (oi), inet_ntoa (router_id));
+
+ /* Delete neighbor from interface. */
+ ospf_nbr_delete (nbr);
+ }
+ else
+ /* Make sure timer is set. */
+ nsm_timer_set (nbr);
return 0;
}
diff --git a/ospfd/ospf_nsm.h b/ospfd/ospf_nsm.h
index fe42f7a..1121dae 100644
--- a/ospfd/ospf_nsm.h
+++ b/ospfd/ospf_nsm.h
@@ -26,15 +26,16 @@ #define _ZEBRA_OSPF_NSM_H
/* 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
More information about the Quagga-dev
mailing list