[bgpd] simplify peer refcounts, squash slow peer leak

2006-09-14 Paul Jakma <paul.jakma@sun.com>

	* (general) fix the peer refcount issue exposed by previous, by
	  just removing refcounting of peer threads, which is mostly
	  senseless as they're references leading from struct peer,
	  which peer_free cancels anyway. No need to muck around..
	* bgp_fsm.h: Just remove the refcounting from the various
	  TIMER/READ/WRITE/EVENT ON/OFF/ADD macros.
	* bgp_fsm.c: (bgp_stop) use BGP_EVENT_FLUSH, no refcounts attached
	  to events anymore.
	  (bgp_event) remove peer_unlock, events not refcounted.
	* bgpd.c: (peer_free) flush events before free.

diff --git a/bgpd/ChangeLog b/bgpd/ChangeLog
index 02aaf3abfc84a22b529d6281d0268c7837ec0f5a..393b00d4e965c5fd384e92a2a6b319173b4adcaf 100644 (file)
--- a/bgpd/ChangeLog
+++ b/bgpd/ChangeLog
@@ -1,3 +1,16 @@
+2006-09-14 Paul Jakma <paul.jakma@sun.com>
+
+       * (general) fix the peer refcount issue exposed by previous, by
+         just removing refcounting of peer threads, which is mostly
+         senseless as they're references leading from struct peer,
+         which peer_free cancels anyway. No need to muck around..
+       * bgp_fsm.h: Just remove the refcounting from the various
+         TIMER/READ/WRITE/EVENT ON/OFF/ADD macros.
+       * bgp_fsm.c: (bgp_stop) use BGP_EVENT_FLUSH, no refcounts attached
+         to events anymore.
+         (bgp_event) remove peer_unlock, events not refcounted.
+       * bgpd.c: (peer_free) flush events before free.
+
 2006-09-14 Paul Jakma <paul.jakma@sun.com>
 
        * (general) Fix some niggly issues around 'shutdown' and clearing

diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c
index bdb6517fdcc479af1cf30f32d3b6f3feff2c5617..cc2b2c3a8afd052050dc87423185c5a397131b38 100644 (file)
--- a/bgpd/bgp_fsm.c
+++ b/bgpd/bgp_fsm.c
@@ -426,7 +426,6 @@ bgp_stop (struct peer *peer)
 {
   afi_t afi;
   safi_t safi;
-  unsigned int i;
   char orf_name[BUFSIZ];
 
   /* Increment Dropped count. */
@@ -500,10 +499,8 @@ bgp_stop (struct peer *peer)
   BGP_TIMER_OFF (peer->t_asorig);
   BGP_TIMER_OFF (peer->t_routeadv);
 
-  /* Delete all existing events of the peer,
-     and corresponding peer ref-count */
-  for (i = thread_cancel_event (master, peer); i > 0; i--)
-    peer_unlock (peer); /* thread event reference */
+  /* Delete all existing events of the peer */
+  BGP_EVENT_FLUSH (peer);
   
   /* Stream reset. */
   peer->packet_size = 0;
@@ -1101,6 +1098,5 @@ bgp_event (struct thread *thread)
       bgp_timer_set (peer);
     }
   
-  peer_unlock (peer); /* bgp-event peer reference */
   return ret;
 }

diff --git a/bgpd/bgp_fsm.h b/bgpd/bgp_fsm.h
index 0a5d37157e441bdc9ecf0bb18a31227823a01804..c51bed37eb5ee257475847a517d0d4f879db22b7 100644 (file)
--- a/bgpd/bgp_fsm.h
+++ b/bgpd/bgp_fsm.h
@@ -25,77 +25,55 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
 /* Macro for BGP read, write and timer thread.  */
 #define BGP_READ_ON(T,F,V)                     \
   do {                                         \
-    if (!T)                                    \
-      {                                                \
-        peer_lock (peer);                      \
-        THREAD_READ_ON(master,T,F,peer,V);     \
-      }                                                \
+    if (!(T) && (peer->status != Deleted))     \
+      THREAD_READ_ON(master,T,F,peer,V);       \
   } while (0)
 
 #define BGP_READ_OFF(T)                                \
   do {                                         \
     if (T)                                     \
-      {                                                \
-        peer_unlock (peer);                    \
-        THREAD_READ_OFF(T);                    \
-      }                                                \
+      THREAD_READ_OFF(T);                      \
   } while (0)
 
 #define BGP_WRITE_ON(T,F,V)                    \
   do {                                         \
     if (!(T) && (peer->status != Deleted))     \
-      {                                                \
-        peer_lock (peer);                      \
-        THREAD_WRITE_ON(master,(T),(F),peer,(V)); \
-      }                                                \
+      THREAD_WRITE_ON(master,(T),(F),peer,(V)); \
   } while (0)
     
 #define BGP_WRITE_OFF(T)                       \
   do {                                         \
     if (T)                                     \
-      {                                                \
-        peer_unlock (peer);                    \
-        THREAD_WRITE_OFF(T);                   \
-      }                                                \
+      THREAD_WRITE_OFF(T);                     \
   } while (0)
 
 #define BGP_TIMER_ON(T,F,V)                    \
   do {                                         \
     if (!(T) && (peer->status != Deleted))     \
-      {                                                \
-        peer_lock (peer);                      \
-        THREAD_TIMER_ON(master,(T),(F),peer,(V)); \
-      }                                                \
+      THREAD_TIMER_ON(master,(T),(F),peer,(V)); \
   } while (0)
 
 #define BGP_TIMER_OFF(T)                       \
   do {                                         \
     if (T)                                     \
-      {                                        \
-        peer_unlock (peer);                    \
-        THREAD_TIMER_OFF(T);                   \
-      }                                                \
+      THREAD_TIMER_OFF(T);                     \
   } while (0)
 
 #define BGP_EVENT_ADD(P,E)                     \
   do {                                         \
     if ((P)->status != Deleted)                        \
-      {                                                \
-        peer_lock (peer); /* bgp event reference */ \
-        thread_add_event (master, bgp_event, (P), (E)); \
-      }                                                \
+      thread_add_event (master, bgp_event, (P), (E)); \
   } while (0)
 
-#define BGP_EVENT_DELETE(P)                    \
+#define BGP_EVENT_FLUSH(P)                     \
   do {                                                 \
-    peer_unlock (peer); /* bgp event peer reference */ \
     assert (peer);                             \
     thread_cancel_event (master, (P));                 \
   } while (0)
 
 #define BGP_EVENT_FLUSH_ADD(P,E)       \
   do {                                 \
-    BGP_EVENT_DELETE(P);               \
+    BGP_EVENT_FLUSH(P);                        \
     BGP_EVENT_ADD(P,E);                        \
   } while (0)
 

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 733b33a6aa42c0d7b6c9be4bfb82808d5ee0fb27..89f82628a30475972aa23df84b4631b0874860ff 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -695,6 +695,7 @@ peer_free (struct peer *peer)
   bgp_timer_set (peer);
   BGP_READ_OFF (peer->t_read);
   BGP_WRITE_OFF (peer->t_write);
+  BGP_EVENT_FLUSH (peer);
   
   if (peer->desc)
     XFREE (MTYPE_PEER_DESC, peer->desc);