Adjustments to scheduling of Routing Engine work.

These changes mean that Quagga copes a little better when there are
very large numbers of updates/withrawal messages arriving all at
once...

...it is not possible to cure the problem of overloading Quagga by
throwing too much at it.  However, these changes at least mean that
when BGP sessions drop, the Routing Engine will notice that in a
reasonable time, and can clear up all routes associated with
the session (throwing away any updates/withdraws already received,
but not yet dealt with.)

Amonst these changes are Chris Caputo's patches for bgp_node locking
issues -- see quagga-dev 7960 mailing list message.

10 files changed, 114 insertions, 99 deletions

diff --git a/bgpd/bgp_engine.h b/bgpd/bgp_engine.h
index 2d36b260..ceec1b2f 100644
--- a/bgpd/bgp_engine.h
+++ b/bgpd/bgp_engine.h
@@ -116,22 +116,12 @@ bgp_queue_logging(const char* name, mqueue_queue mq, struct queue_stats* stats)
   stats->xon    = 0 ;
 } ;
 
-/* Send given message to the BGP Engine -- ordinary
+/* Send given message to the BGP Engine -- priority/ordinary
  */
 Inline void
-bgp_to_bgp_engine(mqueue_block mqb)
+bgp_to_bgp_engine(mqueue_block mqb, enum mqb_rank priority)
 {
-  mqueue_enqueue(bgp_nexus->queue, mqb, 0) ;
-  if (qdebug)
-    bgp_queue_logging("BGP Engine", bgp_nexus->queue, &bgp_engine_queue_stats) ;
-} ;
-
-/* Send given message to the BGP Engine -- priority
- */
-Inline void
-bgp_to_bgp_engine_priority(mqueue_block mqb)
-{
-  mqueue_enqueue(bgp_nexus->queue, mqb, 1) ;
+  mqueue_enqueue(bgp_nexus->queue, mqb, priority) ;
   if (qdebug)
     bgp_queue_logging("BGP Engine", bgp_nexus->queue, &bgp_engine_queue_stats) ;
 } ;
@@ -140,26 +130,15 @@ bgp_to_bgp_engine_priority(mqueue_block mqb)
  *
  */
 
-/* Send given message to the Routing Engine -- ordinary
+/* Send given message to the Routing Engine -- priority/ordinary
  */
 Inline void
-bgp_to_routing_engine(mqueue_block mqb)
+bgp_to_routing_engine(mqueue_block mqb, enum mqb_rank priority)
 {
-  mqueue_enqueue(routing_nexus->queue, mqb, 0) ;
+  mqueue_enqueue(routing_nexus->queue, mqb, priority) ;
   if (qdebug)
     bgp_queue_logging("Routing Engine", routing_nexus->queue,
                                                  &routing_engine_queue_stats) ;
 } ;
 
-/* Send given message to the Routing Engine -- priority
- */
-Inline void
-bgp_to_routing_engine_priority(mqueue_block mqb)
-{
-  mqueue_enqueue(routing_nexus->queue, mqb, 1) ;
-  if (qdebug)
-    bgp_queue_logging("Routing Engine", routing_nexus->queue,
-                                                  &routing_engine_queue_stats) ;
-} ;
-
 #endif /* QUAGGA_BGP_ENGINE_H */
diff --git a/bgpd/bgp_main.c b/bgpd/bgp_main.c
index 434cfcfb..0de28b9b 100644
--- a/bgpd/bgp_main.c
+++ b/bgpd/bgp_main.c
@@ -299,7 +299,15 @@ bgp_exit (int status)
   if (retain_mode)
     if_add_hook (IF_DELETE_HOOK, NULL);
   for (ALL_LIST_ELEMENTS (iflist, node, nnode, ifp))
-    if_delete (ifp);
+    {
+      struct listnode *c_node, *c_nnode;
+      struct connected *c;
+
+      for (ALL_LIST_ELEMENTS (ifp->connected, c_node, c_nnode, c))
+        bgp_connected_delete (c);
+
+      if_delete (ifp);
+    }
   list_free (iflist);
 
   /* curtains                                   */
diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c
index 7e8c82ee..c72ca09f 100644
--- a/bgpd/bgp_nexthop.c
+++ b/bgpd/bgp_nexthop.c
@@ -276,6 +276,8 @@ bgp_nexthop_lookup_ipv6 (struct peer *peer, struct bgp_info *ri, int *changed,
 
 		  if (bnc->metric != oldbnc->metric)
 		    bnc->metricchanged = 1;
+
+		  bgp_unlock_node (oldrn);
 		}
 	    }
 	}
@@ -365,6 +367,8 @@ bgp_nexthop_lookup (afi_t afi, struct peer *peer, struct bgp_info *ri,
 
 		  if (bnc->metric != oldbnc->metric)
 		    bnc->metricchanged = 1;
+
+                  bgp_unlock_node (oldrn);
 		}
 	    }
 	}
@@ -571,7 +575,7 @@ bgp_connected_add (struct connected *ifc)
 	}
       else
 	{
-	  bc = XCALLOC (0, sizeof (struct bgp_connected_ref));
+	  bc = XCALLOC (MTYPE_BGP_CONN, sizeof (struct bgp_connected_ref));
 	  bc->refcnt = 1;
 	  rn->info = bc;
 	}
@@ -596,7 +600,7 @@ bgp_connected_add (struct connected *ifc)
 	}
       else
 	{
-	  bc = XCALLOC (0, sizeof (struct bgp_connected_ref));
+	  bc = XCALLOC (MTYPE_BGP_CONN, sizeof (struct bgp_connected_ref));
 	  bc->refcnt = 1;
 	  rn->info = bc;
 	}
@@ -636,7 +640,7 @@ bgp_connected_delete (struct connected *ifc)
       bc->refcnt--;
       if (bc->refcnt == 0)
 	{
-	  XFREE (0, bc);
+	  XFREE (MTYPE_BGP_CONN, bc);
 	  rn->info = NULL;
 	}
       bgp_unlock_node (rn);
@@ -662,7 +666,7 @@ bgp_connected_delete (struct connected *ifc)
       bc->refcnt--;
       if (bc->refcnt == 0)
 	{
-	  XFREE (0, bc);
+	  XFREE (MTYPE_BGP_CONN, bc);
 	  rn->info = NULL;
 	}
       bgp_unlock_node (rn);
@@ -1113,11 +1117,15 @@ bgp_multiaccess_check_v4 (struct in_addr nexthop, char *peer)
   rn1 = bgp_node_match (bgp_connected_table[AFI_IP], &p1);
   if (! rn1)
     return 0;
+  bgp_unlock_node (rn1);
 
   rn2 = bgp_node_match (bgp_connected_table[AFI_IP], &p2);
   if (! rn2)
     return 0;
+  bgp_unlock_node (rn2);
 
+  /* This is safe, even with above unlocks, since we are just
+     comparing pointers to the objects, not the objects themselves. */
   if (rn1 == rn2)
     return 1;
 
@@ -1296,6 +1304,9 @@ bgp_scan_init (void)
 void
 bgp_scan_finish (void)
 {
+  /* Only the current one needs to be reset. */
+  bgp_nexthop_cache_reset (bgp_nexthop_cache_table[AFI_IP]);
+
   bgp_table_unlock (cache1_table[AFI_IP]);
   cache1_table[AFI_IP] = NULL;
 
@@ -1306,6 +1317,9 @@ bgp_scan_finish (void)
   bgp_connected_table[AFI_IP] = NULL;
 
 #ifdef HAVE_IPV6
+  /* Only the current one needs to be reset. */
+  bgp_nexthop_cache_reset (bgp_nexthop_cache_table[AFI_IP6]);
+
   bgp_table_unlock (cache1_table[AFI_IP6]);
   cache1_table[AFI_IP6] = NULL;
 
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 6553b3ff..ff7dada3 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -6860,7 +6860,10 @@ bgp_show_route_in_table (struct vty *vty, struct bgp *bgp,
               if ((rm = bgp_node_match (table, &match)) != NULL)
                 {
                   if (prefix_check && rm->p.prefixlen != match.prefixlen)
-                    continue;
+                    {
+                      bgp_unlock_node (rm);
+                      continue;
+                    }
 
                   for (ri = rm->info; ri; ri = ri->info_next)
                     {
@@ -6874,6 +6877,8 @@ bgp_show_route_in_table (struct vty *vty, struct bgp *bgp,
                       display++;
                       route_vty_out_detail (vty, bgp, &rm->p, ri, AFI_IP, SAFI_MPLS_VPN);
                     }
+
+                  bgp_unlock_node (rm);
                 }
             }
         }
@@ -11655,41 +11660,47 @@ bgp_clear_damp_route (struct vty *vty, const char *view_name,
 
 	  if ((table = rn->info) != NULL)
 	    if ((rm = bgp_node_match (table, &match)) != NULL)
-	      if (! prefix_check || rm->p.prefixlen == match.prefixlen)
-		{
-		  ri = rm->info;
-		  while (ri)
-		    {
-		      if (ri->extra && ri->extra->damp_info)
-			{
-			  ri_temp = ri->info_next;
-			  bgp_damp_info_free (ri->extra->damp_info, 1);
-			  ri = ri_temp;
-			}
-		      else
-			ri = ri->info_next;
-		    }
-		}
+	      {
+                if (! prefix_check || rm->p.prefixlen == match.prefixlen)
+                  {
+                    ri = rm->info;
+                    while (ri)
+                      {
+                        if (ri->extra && ri->extra->damp_info)
+                          {
+                            ri_temp = ri->info_next;
+                            bgp_damp_info_free (ri->extra->damp_info, 1);
+                            ri = ri_temp;
+                          }
+                        else
+                          ri = ri->info_next;
+                      }
+                  }
+                bgp_unlock_node (rm);
+	      }
         }
     }
   else
     {
       if ((rn = bgp_node_match (bgp->rib[afi][safi], &match)) != NULL)
-	if (! prefix_check || rn->p.prefixlen == match.prefixlen)
-	  {
-	    ri = rn->info;
-	    while (ri)
-	      {
-		if (ri->extra && ri->extra->damp_info)
-		  {
-		    ri_temp = ri->info_next;
-		    bgp_damp_info_free (ri->extra->damp_info, 1);
-		    ri = ri_temp;
-		  }
-		else
-		  ri = ri->info_next;
-	      }
-	  }
+        {
+          if (! prefix_check || rn->p.prefixlen == match.prefixlen)
+            {
+              ri = rn->info;
+              while (ri)
+                {
+                  if (ri->extra && ri->extra->damp_info)
+                    {
+                      ri_temp = ri->info_next;
+                      bgp_damp_info_free (ri->extra->damp_info, 1);
+                      ri = ri_temp;
+                    }
+                  else
+                    ri = ri->info_next;
+                }
+            }
+          bgp_unlock_node (rn);
+        }
     }
 
   return CMD_SUCCESS;
diff --git a/bgpd/bgp_session.c b/bgpd/bgp_session.c
index 1334298e..8d8bfea2 100644
--- a/bgpd/bgp_session.c
+++ b/bgpd/bgp_session.c
@@ -325,7 +325,7 @@ bgp_session_enable(bgp_peer peer)
 
   ++bgp_engine_queue_stats.event ;
 
-  bgp_to_bgp_engine(mqb) ;
+  bgp_to_bgp_engine(mqb, mqb_ordinary) ;
 } ;
 
 /*------------------------------------------------------------------------------
@@ -426,7 +426,7 @@ bgp_session_disable(bgp_peer peer, bgp_notify notification)
 
   ++bgp_engine_queue_stats.event ;
 
-  bgp_to_bgp_engine_priority(mqb) ;
+  bgp_to_bgp_engine(mqb, mqb_priority) ;
 } ;
 
 /*------------------------------------------------------------------------------
@@ -464,7 +464,7 @@ bgp_session_event(bgp_session session, bgp_session_event_t  event,
                                        bgp_notify           notification,
                                        int                  err,
                                        bgp_connection_ord_t ordinal,
-                                       int                  stopped)
+                                       bool                 stopped)
 {
   struct bgp_session_event_args* args ;
   mqueue_block   mqb ;
@@ -484,7 +484,7 @@ bgp_session_event(bgp_session session, bgp_session_event_t  event,
 
   ++routing_engine_queue_stats.event ;
 
-  bgp_to_routing_engine(mqb) ;
+  bgp_to_routing_engine(mqb, stopped ? mqb_priority : mqb_ordinary) ;
 } ;
 
 /*==============================================================================
@@ -509,7 +509,7 @@ bgp_session_update_send(bgp_session session, struct stream_fifo* fifo)
 
   ++bgp_engine_queue_stats.update ;
 
-  bgp_to_bgp_engine(mqb) ;
+  bgp_to_bgp_engine(mqb, mqb_ordinary) ;
 
   stream_fifo_reset(fifo) ;
 } ;
@@ -637,7 +637,7 @@ bgp_session_route_refresh_send(bgp_session session, bgp_route_refresh rr)
 
   ++bgp_engine_queue_stats.event ;
 
-  bgp_to_bgp_engine(mqb) ;
+  bgp_to_bgp_engine(mqb, mqb_ordinary) ;
 } ;
 
 /*------------------------------------------------------------------------------
@@ -702,7 +702,7 @@ bgp_session_end_of_rib_send(bgp_session session, qAFI_t afi, qSAFI_t safi)
 
   ++bgp_engine_queue_stats.xon ;
 
-  bgp_to_bgp_engine(mqb) ;
+  bgp_to_bgp_engine(mqb, mqb_ordinary) ;
 } ;
 
 /*------------------------------------------------------------------------------
@@ -769,7 +769,7 @@ bgp_session_update_recv(bgp_session session, struct stream* buf, bgp_size_t size
 
   ++routing_engine_queue_stats.update ;
 
-  bgp_to_routing_engine(mqb) ;
+  bgp_to_routing_engine(mqb, mqb_ordinary) ;
 }
 
 /*------------------------------------------------------------------------------
@@ -813,7 +813,7 @@ bgp_session_route_refresh_recv(bgp_session session, bgp_route_refresh rr)
   args->rr         = rr ;
   args->is_pending = NULL ;
 
-  bgp_to_routing_engine(mqb) ;
+  bgp_to_routing_engine(mqb, mqb_ordinary) ;
 } ;
 
 /*------------------------------------------------------------------------------
@@ -849,7 +849,7 @@ bgp_session_XON(bgp_session session)
 
   ++routing_engine_queue_stats.xon ;
 
-  bgp_to_routing_engine(mqb) ;
+  bgp_to_routing_engine(mqb, mqb_ordinary) ;
 }
 
 /*------------------------------------------------------------------------------
@@ -888,7 +888,7 @@ bgp_session_set_ttl(bgp_session session, int ttl)
 
   ++bgp_engine_queue_stats.event ;
 
-  bgp_to_bgp_engine(mqb) ;
+  bgp_to_bgp_engine(mqb, mqb_ordinary) ;
 }
 
 /*------------------------------------------------------------------------------
diff --git a/bgpd/bgp_session.h b/bgpd/bgp_session.h
index 615c6829..6f449bec 100644
--- a/bgpd/bgp_session.h
+++ b/bgpd/bgp_session.h
@@ -22,6 +22,7 @@
 #ifndef _QUAGGA_BGP_SESSION_H
 #define _QUAGGA_BGP_SESSION_H
 
+#include <stdbool.h>
 #include <zebra.h>
 
 #include "bgpd/bgp_common.h"
@@ -324,7 +325,7 @@ bgp_session_event(bgp_session session, bgp_session_event_t  event,
                                        bgp_notify           notification,
                                        int                  err,
                                        bgp_connection_ord_t ordinal,
-                                       int                  stopped) ;
+                                       bool                 stopped) ;
 
 extern void
 bgp_session_update_send(bgp_session session, struct stream_fifo* fifo) ;
diff --git a/lib/memtypes.c b/lib/memtypes.c
index d0cb1eee..98b53209 100644
--- a/lib/memtypes.c
+++ b/lib/memtypes.c
@@ -143,6 +143,7 @@ struct memory_list memory_list_bgp[] =
   { MTYPE_BGP_NODE,		"BGP node"			},
   { MTYPE_BGP_ROUTE,		"BGP route"			},
   { MTYPE_BGP_ROUTE_EXTRA,	"BGP ancillary route info"	},
+  { MTYPE_BGP_CONN,             "BGP connected"                 },
   { MTYPE_BGP_STATIC,		"BGP static"			},
   { MTYPE_BGP_ADVERTISE_ATTR,	"BGP adv attr"			},
   { MTYPE_BGP_ADVERTISE,	"BGP adv"			},
diff --git a/lib/mqueue.c b/lib/mqueue.c
index 90e1616c..8fa9fbd5 100644
--- a/lib/mqueue.c
+++ b/lib/mqueue.c
@@ -509,8 +509,8 @@ static void mqueue_dequeue_signal(mqueue_queue mq, mqueue_thread_signal mtsig) ;
 /*------------------------------------------------------------------------------
  * Enqueue message.
  *
- * If priority != 0, will enqueue after any previously enqueued priority
- * messages.
+ * If priority, will enqueue after any previously enqueued priority
+ * messages.  (See enum: mqb_priority and mqb_ordinary.)
  *
  * If there are any waiters, then we kick one or all of them.
  *
@@ -533,7 +533,7 @@ static void mqueue_dequeue_signal(mqueue_queue mq, mqueue_thread_signal mtsig) ;
  *     never be any waiters... so no kicking is ever done.
  */
 extern void
-mqueue_enqueue(mqueue_queue mq, mqueue_block mqb, int priority)
+mqueue_enqueue(mqueue_queue mq, mqueue_block mqb, enum mqb_rank priority)
 {
   if (mq == NULL)
     return mqb_dispatch_destroy(mqb) ;
diff --git a/lib/mqueue.h b/lib/mqueue.h
index f22ea022..a28b6606 100644
--- a/lib/mqueue.h
+++ b/lib/mqueue.h
@@ -23,6 +23,7 @@
 #define _ZEBRA_MQUEUE_H
 
 #include <stddef.h>
+#include <stdbool.h>
 
 #include "qpthreads.h"
 #include "qtime.h"
@@ -243,8 +244,14 @@ mqb_re_init(mqueue_block mqb, mqueue_action action, void* arg0) ;
 extern void
 mqb_free(mqueue_block mqb) ;
 
+enum mqb_rank
+{
+  mqb_priority  = true,
+  mqb_ordinary  = false
+} ;
+
 extern void
-mqueue_enqueue(mqueue_queue mq, mqueue_block mqb, int priority) ;
+mqueue_enqueue(mqueue_queue mq, mqueue_block mqb, enum mqb_rank priority) ;
 
 extern mqueue_block
 mqueue_dequeue(mqueue_queue mq, int wait, void* arg) ;
diff --git a/lib/qpnexus.c b/lib/qpnexus.c
index e568e7d8..59dcc535 100644
--- a/lib/qpnexus.c
+++ b/lib/qpnexus.c
@@ -191,29 +191,23 @@ qpn_start(void* arg)
   done = 1 ;
   while (!qpn->terminate)
     {
-      wait = (done == 0) ;      /* may wait this time only if nothing
-                                   found to do on the last pass         */
-
-      /* Signals are highest priority -- only execute for main thread
-       *
-       * Restarts "done" for this pass.
-       */
+      /* Signals are highest priority -- only execute for main thread   */
       if (qpn->main_thread)
-        done = quagga_sigevent_process() ;
-      else
-        done = 0 ;
+        done |= quagga_sigevent_process() ;
 
       /* Foreground hooks, if any.                                      */
       for (i = 0; i < qpn->foreground.count ;)
         done |= ((qpn_hook_function*)(qpn->foreground.hooks[i++]))() ;
 
-      /* drain the message queue, will be in waiting for signal state
-       * when it's empty */
-
-      if (done != 0)
-        wait = 0 ;              /* turn off wait if found something     */
-
-      while (1)
+      /* take stuff from the message queue
+       *
+       * If nothing done the last time around the loop then may wait this
+       * time if the queue is empty first time through.
+       */
+      wait = (done == 0) ;      /* may wait this time only if nothing
+                                   found to do on the last pass         */
+      done = 0 ;
+      do
         {
           mqb = mqueue_dequeue(qpn->queue, wait, qpn->mts) ;
           if (mqb == NULL)
@@ -221,9 +215,9 @@ qpn_start(void* arg)
 
           mqb_dispatch(mqb, mqb_action);
 
-          done = 1 ;            /* done something                       */
-          wait = 0 ;            /* turn off wait                        */
-        } ;
+          ++done ;              /* done another                         */
+          wait = 0 ;            /* done something, so turn off wait     */
+        } while (done < 200) ;
 
       /* block for some input, output, signal or timeout
        *