[quagga-dev 7041] [RESEND] [PATCH] 4/7 Fixes for circuit state machine

Fritz Reichmann fritz at reichmann.nl
Thu Aug 6 20:33:52 BST 2009


From http://lists.quagga.net/pipermail/quagga-dev/2009-April/006512.html.

Author: Peter Szilagyi <peszilagyi at gmail.com>
Date:   Apr 13 13:31:14 IST 2009

    [isisd] isisd has a so-called circuit state machine that takes care about the interface state changes, such as initializing, down, up. When an interface was brought down by a link failure, the interface information was deleted and set to NULL. When the link was restored later, the interface was looked up by the old pointer, but since it was cleared, it was never found again, resulting in an interface never entering the up state again. Also, the program regularly crashed because of a deleted pointer in the same context which was later accessed without any further checking.

Index: isisd/isis_adjacency.c
===================================================================
--- isisd/isis_adjacency.c      (revision 401)
+++ isisd/isis_adjacency.c      (working copy)
@@ -172,7 +172,7 @@
        circuit->upadjcount[level - 1]++;
       if (state == ISIS_ADJ_DOWN)
        {
-         isis_delete_adj (adj, adj->circuit->u.bc.adjdb[level - 1]);
+         listnode_delete (adj->circuit->u.bc.adjdb[level - 1], adj);
          circuit->upadjcount[level - 1]--;
        }

Index: isisd/isis_pdu.c
===================================================================
--- isisd/isis_pdu.c    (revision 401)
+++ isisd/isis_pdu.c    (working copy)
@@ -1913,6 +1911,9 @@
   unsigned long len_pointer, length;
   int retval;

+  if (circuit->state != C_STATE_UP || circuit->interface == NULL)
+    return ISIS_WARNING;
+
   if (circuit->interface->mtu == 0)
     {
       zlog_warn ("circuit has zero MTU");
@@ -2192,6 +2193,9 @@
   struct listnode *node;
   struct isis_lsp *lsp;

+  if (circuit->state != C_STATE_UP || circuit->interface == NULL)
+    return ISIS_WARNING;
+
   memset (start, 0x00, ISIS_SYS_ID_LEN + 2);
   memset (stop, 0xff, ISIS_SYS_ID_LEN + 2);

@@ -2357,6 +2361,9 @@
   struct list *list = NULL;
   struct listnode *node;

+  if (circuit->state != C_STATE_UP || circuit->interface == NULL)
+    return ISIS_WARNING;
+
   if ((circuit->circ_type == CIRCUIT_T_BROADCAST &&
        !circuit->u.bc.is_dr[level - 1]) ||
       circuit->circ_type != CIRCUIT_T_BROADCAST)
@@ -2463,85 +2470,85 @@
   circuit = THREAD_ARG (thread);
   assert (circuit);

-  if (circuit->state == C_STATE_UP)
+  if (circuit->state != C_STATE_UP || circuit->interface == NULL)
+    return ISIS_WARNING;
+
+  lsp = listgetdata ((node = listhead (circuit->lsp_queue)));
+
+  /*
+   * Do not send if levels do not match
+   */
+  if (!(lsp->level & circuit->circuit_is_type))
+    goto dontsend;
+
+  /*
+   * Do not send if we do not have adjacencies in state up on the circuit
+   */
+  if (circuit->upadjcount[lsp->level - 1] == 0)
+    goto dontsend;
+  /* only send if it needs sending */
+  if ((time (NULL) - lsp->last_sent) >=
+      circuit->area->lsp_gen_interval[lsp->level - 1])
     {
-      lsp = listgetdata ((node = listhead (circuit->lsp_queue)));

-      /*
-       * Do not send if levels do not match
-       */
-      if (!(lsp->level & circuit->circuit_is_type))
-       goto dontsend;
+      if (isis->debugs & DEBUG_UPDATE_PACKETS)
+        {
+          zlog_debug
+            ("ISIS-Upd (%s): Sent L%d LSP %s, seq 0x%08x, cksum 0x%04x,"
+             " lifetime %us on %s", circuit->area->area_tag, lsp->level,
+             rawlspid_print (lsp->lsp_header->lsp_id),
+             ntohl (lsp->lsp_header->seq_num),
+             ntohs (lsp->lsp_header->checksum),
+             ntohs (lsp->lsp_header->rem_lifetime),
+             circuit->interface->name);
+        }
+      /* copy our lsp to the send buffer */
+      stream_copy (circuit->snd_stream, lsp->pdu);

+      retval = circuit->tx (circuit, lsp->level);
+
       /*
-       * Do not send if we do not have adjacencies in state up on the circuit
+       * If the sending succeeded, we can del the lsp from circuits
+       * lsp_queue
        */
-      if (circuit->upadjcount[lsp->level - 1] == 0)
-       goto dontsend;
-      /* only send if it needs sending */
-      if ((time (NULL) - lsp->last_sent) >=
-         circuit->area->lsp_gen_interval[lsp->level - 1])
-       {
+      if (retval == ISIS_OK)
+        {
+          list_delete_node (circuit->lsp_queue, node);

-         if (isis->debugs & DEBUG_UPDATE_PACKETS)
-           {
-             zlog_debug
-               ("ISIS-Upd (%s): Sent L%d LSP %s, seq 0x%08x, cksum 0x%04x,"
-                " lifetime %us on %s", circuit->area->area_tag, lsp->level,
-                rawlspid_print (lsp->lsp_header->lsp_id),
-                ntohl (lsp->lsp_header->seq_num),
-                ntohs (lsp->lsp_header->checksum),
-                ntohs (lsp->lsp_header->rem_lifetime),
-                circuit->interface->name);
-           }
-         /* copy our lsp to the send buffer */
-         stream_copy (circuit->snd_stream, lsp->pdu);
+          /*
+           * On broadcast circuits also the SRMflag can be cleared
+           */
+          if (circuit->circ_type == CIRCUIT_T_BROADCAST)
+            ISIS_CLEAR_FLAG (lsp->SRMflags, circuit);

-         retval = circuit->tx (circuit, lsp->level);
-
-         /*
-          * If the sending succeeded, we can del the lsp from circuits
-          * lsp_queue
-          */
-         if (retval == ISIS_OK)
-           {
-             list_delete_node (circuit->lsp_queue, node);
-
-             /*
-              * On broadcast circuits also the SRMflag can be cleared
-              */
-             if (circuit->circ_type == CIRCUIT_T_BROADCAST)
-               ISIS_CLEAR_FLAG (lsp->SRMflags, circuit);
-
-             if (flags_any_set (lsp->SRMflags) == 0)
-               {
-                 /*
-                  * need to remember when we were last sent
-                  */
-                 lsp->last_sent = time (NULL);
-               }
-           }
-         else
-           {
-             zlog_debug ("sending of level %d link state failed", lsp->level);
-           }
-       }
+          if (flags_any_set (lsp->SRMflags) == 0)
+            {
+              /*
+               * need to remember when we were last sent
+               */
+              lsp->last_sent = time (NULL);
+            }
+        }
       else
-       {
-         /* my belief is that if it wasn't his time, the lsp can be removed
-          * from the queue
-          */
-       dontsend:
-         list_delete_node (circuit->lsp_queue, node);
-       }
+        {
+          zlog_debug ("sending of level %d link state failed", lsp->level);
+        }
+    }
+  else
+    {
+      /* my belief is that if it wasn't his time, the lsp can be removed
+       * from the queue
+       */
+    dontsend:
+      list_delete_node (circuit->lsp_queue, node);
+    }
 #if 0
-      /*
-       * If there are still LSPs send next one after lsp-interval (33 msecs)
-       */
-      if (listcount (circuit->lsp_queue) > 0)
-       thread_add_timer (master, send_lsp, circuit, 1);
+  /*
+   * If there are still LSPs send next one after lsp-interval (33 msecs)
+   */
+  if (listcount (circuit->lsp_queue) > 0)
+    thread_add_timer (master, send_lsp, circuit, 1);
 #endif
-    }

   return retval;
 }
Index: isisd/isis_circuit.h
===================================================================
--- isisd/isis_circuit.h        (revision 401)
+++ isisd/isis_circuit.h        (working copy)
@@ -65,6 +65,7 @@
 struct isis_circuit
 {
   int state;
+  int connected;
   u_char circuit_id;           /* l1/l2 p2p/bcast CircuitID */
   struct isis_area *area;      /* back pointer to the area */
   struct interface *interface; /* interface info from z */
Index: isisd/isis_csm.c
===================================================================
--- isisd/isis_csm.c    (revision 401)
+++ isisd/isis_csm.c    (working copy)
@@ -112,6 +112,7 @@
          isis_circuit_configure (circuit, (struct isis_area *) arg);
          isis_circuit_up (circuit);
          circuit->state = C_STATE_UP;
+          circuit->connected = 1;
          isis_event_circuit_state_change (circuit, 1);
          listnode_delete (isis->init_circ_list, circuit);
          break;
@@ -136,9 +137,12 @@
          zlog_warn ("circuit already enabled");
          break;
        case IF_UP_FROM_Z:
-         isis_circuit_if_add (circuit, (struct interface *) arg);
-         isis_circuit_up (circuit);
+          if (!circuit->connected) {
+            isis_circuit_if_add (circuit, (struct interface *) arg);
+            isis_circuit_up (circuit);
+          }
          circuit->state = C_STATE_UP;
+          circuit->connected = 1;
          isis_event_circuit_state_change (circuit, 1);
          break;
        case ISIS_DISABLE:
@@ -167,7 +171,6 @@
          isis_event_circuit_state_change (circuit, 0);
          break;
        case IF_DOWN_FROM_Z:
-         isis_circuit_if_del (circuit);
          circuit->state = C_STATE_CONF;
          isis_event_circuit_state_change (circuit, 0);
          break;
Index: isisd/isis_lsp.c
===================================================================
--- isisd/isis_lsp.c    (revision 401)
+++ isisd/isis_lsp.c    (working copy)
@@ -2037,6 +2037,8 @@
            {
               for (ALL_LIST_ELEMENTS_RO (area->circuit_list, cnode, circuit))
                {
+                  if (circuit->state != C_STATE_UP)
+                    continue;
                   for (ALL_LIST_ELEMENTS_RO (lsp_list, lspnode, lsp))
                    {
                      if (ISIS_CHECK_FLAG (lsp->SRMflags, circuit))



More information about the Quagga-dev mailing list