ospfd: ospf nbr in full although mismatch in hello packet contents

Issue:
===================
OSPF neighbors are not going down even after 10 mins when
having a mismatch in hello and dead interval.
First neighbors are formed and then a mismatch in the interval
is created, it is observed that the neighbor is not going down.

Root Cause Analysis:
====================
The event HelloReceived defined in RFC 2328 was named as PacketReceived
and this event was scheduled whenever LS Update, LS Ack, LS Request,
DD description packet or Hello packet is received.
Although there is a mismatch in the Hello packet contents, the
event PacketReceived gets triggered due to LS Update received and the
dead timer gets reset and hence the neighbor was never going Down and
remains FULL.

Fix:
==================
As per RFC 2328, the HelloReceived needs to be triggered only when
valid OSPF Hello packet is received and not when other OSPF packets
are received. Modified the function name as well.

Signed-off-by: Mobashshera Rasool <mrasool@vmware.com>

ospfd/ospf_nsm.c

@@ -176,7 +176,7 @@ int nsm_should_adj(struct ospf_neighbor *nbr)
 }
 
 /* OSPF NSM functions. */
-static int nsm_packet_received(struct ospf_neighbor *nbr)
+static int nsm_hello_received(struct ospf_neighbor *nbr)
 {
 	/* Start or Restart Inactivity Timer. */
 	OSPF_NSM_TIMER_OFF(nbr->t_inactivity);
@@ -421,7 +421,7 @@ const struct {
 	{
 		/* DependUpon: dummy state. */
 		{NULL, NSM_DependUpon}, /* NoEvent           */
-		{NULL, NSM_DependUpon}, /* PacketReceived    */
+		{NULL, NSM_DependUpon}, /* HelloReceived     */
 		{NULL, NSM_DependUpon}, /* Start             */
 		{NULL, NSM_DependUpon}, /* 2-WayReceived     */
 		{NULL, NSM_DependUpon}, /* NegotiationDone   */
@@ -438,7 +438,7 @@ const struct {
 	{
 		/* Deleted: dummy state. */
 		{NULL, NSM_Deleted}, /* NoEvent           */
-		{NULL, NSM_Deleted}, /* PacketReceived    */
+		{NULL, NSM_Deleted}, /* HelloReceived     */
 		{NULL, NSM_Deleted}, /* Start             */
 		{NULL, NSM_Deleted}, /* 2-WayReceived     */
 		{NULL, NSM_Deleted}, /* NegotiationDone   */
@@ -455,8 +455,8 @@ const struct {
 	{
 		/* Down: */
 		{NULL, NSM_DependUpon},		 /* NoEvent           */
-		{nsm_packet_received, NSM_Init}, /* PacketReceived    */
+		{nsm_hello_received, NSM_Init},  /* HelloReceived     */
-		{nsm_start, NSM_Attempt},	/* Start             */
+		{nsm_start, NSM_Attempt},	 /* Start             */
 		{NULL, NSM_Down},		 /* 2-WayReceived     */
 		{NULL, NSM_Down},		 /* NegotiationDone   */
 		{NULL, NSM_Down},		 /* ExchangeDone      */
@@ -472,7 +472,7 @@ const struct {
 	{
 		/* Attempt: */
 		{NULL, NSM_DependUpon},		 /* NoEvent           */
-		{nsm_packet_received, NSM_Init}, /* PacketReceived    */
+		{nsm_hello_received, NSM_Init},  /* HelloReceived     */
 		{NULL, NSM_Attempt},		 /* Start             */
 		{NULL, NSM_Attempt},		 /* 2-WayReceived     */
 		{NULL, NSM_Attempt},		 /* NegotiationDone   */
@@ -489,7 +489,7 @@ const struct {
 	{
 		/* Init: */
 		{NULL, NSM_DependUpon},		       /* NoEvent           */
-		{nsm_packet_received, NSM_Init},       /* PacketReceived    */
+		{nsm_hello_received, NSM_Init},        /* HelloReceived     */
 		{NULL, NSM_Init},		       /* Start             */
 		{nsm_twoway_received, NSM_DependUpon}, /* 2-WayReceived     */
 		{NULL, NSM_Init},		       /* NegotiationDone   */
@@ -506,7 +506,7 @@ const struct {
 	{
 		/* 2-Way: */
 		{NULL, NSM_DependUpon},		   /* NoEvent           */
-		{nsm_packet_received, NSM_TwoWay}, /* HelloReceived     */
+		{nsm_hello_received, NSM_TwoWay},  /* HelloReceived     */
 		{NULL, NSM_TwoWay},		   /* Start             */
 		{NULL, NSM_TwoWay},		   /* 2-WayReceived     */
 		{NULL, NSM_TwoWay},		   /* NegotiationDone   */
@@ -523,7 +523,7 @@ const struct {
 	{
 		/* ExStart: */
 		{NULL, NSM_DependUpon},		      /* NoEvent           */
-		{nsm_packet_received, NSM_ExStart},   /* PacaketReceived   */
+		{nsm_hello_received, NSM_ExStart},    /* HelloReceived     */
 		{NULL, NSM_ExStart},		      /* Start             */
 		{NULL, NSM_ExStart},		      /* 2-WayReceived     */
 		{nsm_negotiation_done, NSM_Exchange}, /* NegotiationDone   */
@@ -540,7 +540,7 @@ const struct {
 	{
 		/* Exchange: */
 		{NULL, NSM_DependUpon},		     /* NoEvent           */
-		{nsm_packet_received, NSM_Exchange}, /* PacketReceived    */
+		{nsm_hello_received, NSM_Exchange},  /* HelloReceived     */
 		{NULL, NSM_Exchange},		     /* Start             */
 		{NULL, NSM_Exchange},		     /* 2-WayReceived     */
 		{NULL, NSM_Exchange},		     /* NegotiationDone   */
@@ -557,7 +557,7 @@ const struct {
 	{
 		/* Loading: */
 		{NULL, NSM_DependUpon},		    /* NoEvent           */
-		{nsm_packet_received, NSM_Loading}, /* PacketReceived    */
+		{nsm_hello_received, NSM_Loading},  /* HelloReceived     */
 		{NULL, NSM_Loading},		    /* Start             */
 		{NULL, NSM_Loading},		    /* 2-WayReceived     */
 		{NULL, NSM_Loading},		    /* NegotiationDone   */
@@ -574,7 +574,7 @@ const struct {
 	{
 		/* Full: */
 		{NULL, NSM_DependUpon},		 /* NoEvent           */
-		{nsm_packet_received, NSM_Full}, /* PacketReceived    */
+		{nsm_hello_received, NSM_Full},  /* HelloReceived     */
 		{NULL, NSM_Full},		 /* Start             */
 		{NULL, NSM_Full},		 /* 2-WayReceived     */
 		{NULL, NSM_Full},		 /* NegotiationDone   */
@@ -591,7 +591,7 @@ const struct {
 };
 
 static const char *const ospf_nsm_event_str[] = {
-	"NoEvent",	   "PacketReceived",  "Start",
+	"NoEvent",	   "HelloReceived",  "Start",
 	"2-WayReceived",     "NegotiationDone", "ExchangeDone",
 	"BadLSReq",	  "LoadingDone",     "AdjOK?",
 	"SeqNumberMismatch", "1-WayReceived",   "KillNbr",

ospfd/ospf_nsm.h

@@ -40,7 +40,7 @@
 
 /* OSPF Neighbor State Machine Event. */
 #define NSM_NoEvent	        0
-#define NSM_PacketReceived	1 /* HelloReceived in the protocol */
+#define NSM_HelloReceived	1 /* HelloReceived in the protocol */
 #define NSM_Start		2
 #define NSM_TwoWayReceived	3
 #define NSM_NegotiationDone	4

ospfd/ospf_packet.c

@@ -1031,7 +1031,7 @@ static void ospf_hello(struct ip *iph, struct ospf_header *ospfh,
 	old_state = nbr->state;
 
 	/* Add event to thread. */
-	OSPF_NSM_EVENT_SCHEDULE(nbr, NSM_PacketReceived);
+	OSPF_NSM_EVENT_SCHEDULE(nbr, NSM_HelloReceived);
 
 	/*  RFC2328  Section 9.5.1
 	    If the router is not eligible to become Designated Router,
@@ -1375,9 +1375,6 @@ static void ospf_db_desc(struct ip *iph, struct ospf_header *ospfh,
 		UNSET_FLAG(dd->options, OSPF_OPTION_O);
 	}
 
-	/* Add event to thread. */
-	OSPF_NSM_EVENT_SCHEDULE(nbr, NSM_PacketReceived);
-
 	if (CHECK_FLAG(oi->ospf->config, OSPF_LOG_ADJACENCY_DETAIL))
 		zlog_info(
 			"%s:Packet[DD]: Neighbor %pI4 state is %s, seq_num:0x%x, local:0x%x",
@@ -1620,9 +1617,6 @@ static void ospf_ls_req(struct ip *iph, struct ospf_header *ospfh,
 		return;
 	}
 
-	/* Add event to thread. */
-	OSPF_NSM_EVENT_SCHEDULE(nbr, NSM_PacketReceived);
-
 	/* Neighbor State should be Exchange or later. */
 	if (nbr->state != NSM_Exchange && nbr->state != NSM_Loading
 	    && nbr->state != NSM_Full) {
@@ -1867,9 +1861,6 @@ static void ospf_ls_upd(struct ospf *ospf, struct ip *iph,
 		return;
 	}
 
-	/* Add event to thread. */
-	OSPF_NSM_EVENT_SCHEDULE(nbr, NSM_PacketReceived);
-
 	/* Check neighbor state. */
 	if (nbr->state < NSM_Exchange) {
 		if (IS_DEBUG_OSPF(nsm, NSM_EVENTS))
@@ -2256,9 +2247,6 @@ static void ospf_ls_ack(struct ip *iph, struct ospf_header *ospfh,
 		return;
 	}
 
-	/* Add event to thread. */
-	OSPF_NSM_EVENT_SCHEDULE(nbr, NSM_PacketReceived);
-
 	if (nbr->state < NSM_Exchange) {
 		if (IS_DEBUG_OSPF(nsm, NSM_EVENTS))
 			zlog_debug(