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>

diff --git a/ospfd/ospf_nsm.c b/ospfd/ospf_nsm.c
index dee25275d69495fee364717007914b8e153f1607..5f84d04fedeea2bf26b6db09081784f76878a7c7 100644 (file)
--- a/ospfd/ospf_nsm.c
+++ b/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_start, NSM_Attempt},       /* Start             */
+               {nsm_hello_received, NSM_Init},  /* HelloReceived     */
+               {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",

diff --git a/ospfd/ospf_nsm.h b/ospfd/ospf_nsm.h
index e8573c6301a5b863ae129c4915b72b275c0002de..798325f79098f638ab8119bed0a1bb906b4c06a5 100644 (file)
--- a/ospfd/ospf_nsm.h
+++ b/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

diff --git a/ospfd/ospf_packet.c b/ospfd/ospf_packet.c
index 1efdfee3b4972cfddc15f961560ad183ea89e73c..1dcf93dcdef7c6302e967aac08323ae73c0f556f 100644 (file)
--- a/ospfd/ospf_packet.c
+++ b/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(