summaryrefslogtreecommitdiff
path: root/bgpd
diff options
context:
space:
mode:
Diffstat (limited to 'bgpd')
-rw-r--r--bgpd/ChangeLog52
-rw-r--r--bgpd/bgp_debug.c2
-rw-r--r--bgpd/bgp_fsm.c104
-rw-r--r--bgpd/bgp_fsm.h17
-rw-r--r--bgpd/bgp_packet.c27
-rw-r--r--bgpd/bgp_route.c47
-rw-r--r--bgpd/bgp_vty.c2
-rw-r--r--bgpd/bgpd.c85
-rw-r--r--bgpd/bgpd.h8
9 files changed, 251 insertions, 93 deletions
diff --git a/bgpd/ChangeLog b/bgpd/ChangeLog
index 482beeda..02aaf3ab 100644
--- a/bgpd/ChangeLog
+++ b/bgpd/ChangeLog
@@ -1,3 +1,55 @@
+2006-09-14 Paul Jakma <paul.jakma@sun.com>
+
+ * (general) Fix some niggly issues around 'shutdown' and clearing
+ by adding a Clearing FSM wait-state and a hidden 'Deleted'
+ FSM state, to allow deleted peers to 'cool off' and hit 0
+ references. This introduces a slow memory leak of struct peer,
+ however that's more a testament to the fragility of the
+ reference counting than a bug in this patch, cleanup of
+ reference counting to fix this is to follow.
+ * bgpd.h: Add Clearing, Deleted states and Clearing_Completed
+ and event.
+ * bgp_debug.c: (bgp_status_msg[]) Add strings for Clearing and
+ Deleted.
+ * bgp_fsm.h: Don't allow timer/event threads to set anything
+ for Deleted peers.
+ * bgp_fsm.c: (bgp_timer_set) Add Clearing and Deleted. Deleted
+ needs to stop everything.
+ (bgp_stop) Remove explicit fsm_change_status call, the
+ general framework handles the transition.
+ (bgp_start) Log a warning if a start is attempted on a peer
+ that should stay down, trying to start a peer.
+ (struct .. FSM) Add Clearing_Completed
+ events, has little influence except when in state
+ Clearing to signal wait-state can end.
+ Add Clearing and Deleted states, former is a wait-state,
+ latter is a placeholder state to allow peers to disappear
+ quietly once refcounts settle.
+ (bgp_event) Try reduce verbosity of FSM state-change debug,
+ changes to same state are not interesting (Established->Established)
+ Allow NULL action functions in FSM.
+ * bgp_packet.c: (bgp_write) Use FSM events, rather than trying
+ to twiddle directly with FSM state behind the back of FSM.
+ (bgp_write_notify) ditto.
+ (bgp_read) Remove the vague ACCEPT_PEER peer_unlock, or else
+ this patch crashes, now it leaks instead.
+ * bgp_route.c: (bgp_clear_node_complete) Clearing_Completed
+ event, to end clearing.
+ (bgp_clear_route) See extensive comments.
+ * bgpd.c: (peer_free) should only be called while in Deleted,
+ peer refcounting controls when peer_free is called.
+ bgp_sync_delete should be here, not in peer_delete.
+ (peer_delete) Initiate delete.
+ Transition to Deleted state manually.
+ When removing peer from indices that provide visibility of it,
+ take great care to be idempotent wrt the reference counting
+ of struct peer through those indices.
+ Use bgp_timer_set, rather than replicating.
+ Call to bgp_sync_delete isn't appropriate here, sync can be
+ referenced while shutting down and finishing deletion.
+ (peer_group_bind) Take care to be idempotent wrt list references
+ indexing peers.
+
2006-09-13 Paul Jakma <paul.jakma@sun.com>
* bgp_aspath.c: (aspath_highest) new, return highest ASN in an
diff --git a/bgpd/bgp_debug.c b/bgpd/bgp_debug.c
index 1b398ee8..1e0fcd1f 100644
--- a/bgpd/bgp_debug.c
+++ b/bgpd/bgp_debug.c
@@ -62,6 +62,8 @@ struct message bgp_status_msg[] =
{ OpenSent, "OpenSent" },
{ OpenConfirm, "OpenConfirm" },
{ Established, "Established" },
+ { Clearing, "Clearing" },
+ { Deleted, "Deleted" },
};
int bgp_status_msg_max = BGP_STATUS_MAX;
diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c
index 770a7911..bdb6517f 100644
--- a/bgpd/bgp_fsm.c
+++ b/bgpd/bgp_fsm.c
@@ -68,6 +68,11 @@ bgp_start_jitter (int time)
return ((rand () % (time + 1)) - (time / 2));
}
+/* Check if suppress start/restart of sessions to peer. */
+#define BGP_PEER_START_SUPPRESSED(P) \
+ (CHECK_FLAG ((P)->flags, PEER_FLAG_SHUTDOWN) \
+ || CHECK_FLAG ((P)->sflags, PEER_STATUS_PREFIX_OVERFLOW))
+
/* Hook function called after bgp event is occered. And vty's
neighbor command invoke this function after making neighbor
structure. */
@@ -82,10 +87,7 @@ bgp_timer_set (struct peer *peer)
/* First entry point of peer's finite state machine. In Idle
status start timer is on unless peer is shutdown or peer is
inactive. All other timer must be turned off */
- if (CHECK_FLAG (peer->flags, PEER_FLAG_SHUTDOWN)
- || CHECK_FLAG (peer->sflags, PEER_STATUS_PREFIX_OVERFLOW)
- || CHECK_FLAG (peer->sflags, PEER_STATUS_CLEARING)
- || ! peer_active (peer))
+ if (BGP_PEER_START_SUPPRESSED (peer) || ! peer_active (peer))
{
BGP_TIMER_OFF (peer->t_start);
}
@@ -197,6 +199,17 @@ bgp_timer_set (struct peer *peer)
}
BGP_TIMER_OFF (peer->t_asorig);
break;
+ case Deleted:
+ BGP_TIMER_OFF (peer->t_gr_restart);
+ BGP_TIMER_OFF (peer->t_gr_stale);
+ BGP_TIMER_OFF (peer->t_pmax_restart);
+ case Clearing:
+ BGP_TIMER_OFF (peer->t_start);
+ BGP_TIMER_OFF (peer->t_connect);
+ BGP_TIMER_OFF (peer->t_holdtime);
+ BGP_TIMER_OFF (peer->t_keepalive);
+ BGP_TIMER_OFF (peer->t_asorig);
+ BGP_TIMER_OFF (peer->t_routeadv);
}
}
@@ -420,7 +433,6 @@ bgp_stop (struct peer *peer)
if (peer->status == Established)
{
peer->dropped++;
- bgp_fsm_change_status (peer, Idle);
/* bgp log-neighbor-changes of neighbor Down */
if (bgp_flag_check (peer->bgp, BGP_FLAG_LOG_NEIGHBOR_CHANGES))
@@ -625,6 +637,14 @@ bgp_start (struct peer *peer)
{
int status;
+ if (BGP_PEER_START_SUPPRESSED (peer))
+ {
+ if (BGP_DEBUG (fsm, FSM))
+ plog_err (peer->log, "%s [FSM] Trying to start suppressed peer"
+ " - this is never supposed to happen!", peer->host);
+ return -1;
+ }
+
/* Scrub some information that might be left over from a previous,
* session
*/
@@ -903,6 +923,7 @@ struct {
{bgp_ignore, Idle}, /* Receive_KEEPALIVE_message */
{bgp_ignore, Idle}, /* Receive_UPDATE_message */
{bgp_ignore, Idle}, /* Receive_NOTIFICATION_message */
+ {bgp_ignore, Idle}, /* Clearing_Completed */
},
{
/* Connect */
@@ -919,6 +940,7 @@ struct {
{bgp_ignore, Idle}, /* Receive_KEEPALIVE_message */
{bgp_ignore, Idle}, /* Receive_UPDATE_message */
{bgp_stop, Idle}, /* Receive_NOTIFICATION_message */
+ {bgp_ignore, Idle}, /* Clearing_Completed */
},
{
/* Active, */
@@ -935,6 +957,7 @@ struct {
{bgp_ignore, Idle}, /* Receive_KEEPALIVE_message */
{bgp_ignore, Idle}, /* Receive_UPDATE_message */
{bgp_stop_with_error, Idle}, /* Receive_NOTIFICATION_message */
+ {bgp_ignore, Idle}, /* Clearing_Completed */
},
{
/* OpenSent, */
@@ -951,6 +974,7 @@ struct {
{bgp_ignore, Idle}, /* Receive_KEEPALIVE_message */
{bgp_ignore, Idle}, /* Receive_UPDATE_message */
{bgp_stop_with_error, Idle}, /* Receive_NOTIFICATION_message */
+ {bgp_ignore, Idle}, /* Clearing_Completed */
},
{
/* OpenConfirm, */
@@ -967,22 +991,58 @@ struct {
{bgp_establish, Established}, /* Receive_KEEPALIVE_message */
{bgp_ignore, Idle}, /* Receive_UPDATE_message */
{bgp_stop_with_error, Idle}, /* Receive_NOTIFICATION_message */
+ {bgp_ignore, Idle}, /* Clearing_Completed */
},
{
/* Established, */
- {bgp_ignore, Established}, /* BGP_Start */
- {bgp_stop, Idle}, /* BGP_Stop */
- {bgp_stop, Idle}, /* TCP_connection_open */
- {bgp_stop, Idle}, /* TCP_connection_closed */
- {bgp_ignore, Idle}, /* TCP_connection_open_failed */
- {bgp_stop, Idle}, /* TCP_fatal_error */
- {bgp_ignore, Idle}, /* ConnectRetry_timer_expired */
- {bgp_fsm_holdtime_expire, Idle}, /* Hold_Timer_expired */
+ {bgp_ignore, Established}, /* BGP_Start */
+ {bgp_stop, Clearing}, /* BGP_Stop */
+ {bgp_stop, Clearing}, /* TCP_connection_open */
+ {bgp_stop, Clearing}, /* TCP_connection_closed */
+ {bgp_ignore, Clearing}, /* TCP_connection_open_failed */
+ {bgp_stop, Clearing}, /* TCP_fatal_error */
+ {bgp_ignore, Clearing}, /* ConnectRetry_timer_expired */
+ {bgp_fsm_holdtime_expire, Clearing}, /* Hold_Timer_expired */
{bgp_fsm_keepalive_expire, Established}, /* KeepAlive_timer_expired */
- {bgp_stop, Idle}, /* Receive_OPEN_message */
- {bgp_fsm_keepalive, Established}, /* Receive_KEEPALIVE_message */
- {bgp_fsm_update, Established}, /* Receive_UPDATE_message */
- {bgp_stop_with_error, Idle}, /* Receive_NOTIFICATION_message */
+ {bgp_stop, Clearing}, /* Receive_OPEN_message */
+ {bgp_fsm_keepalive, Established}, /* Receive_KEEPALIVE_message */
+ {bgp_fsm_update, Established}, /* Receive_UPDATE_message */
+ {bgp_stop_with_error, Clearing}, /* Receive_NOTIFICATION_message */
+ {bgp_ignore, Idle}, /* Clearing_Completed */
+ },
+ {
+ /* Clearing, */
+ {bgp_ignore, Clearing}, /* BGP_Start */
+ {bgp_ignore, Clearing}, /* BGP_Stop */
+ {bgp_ignore, Clearing}, /* TCP_connection_open */
+ {bgp_ignore, Clearing}, /* TCP_connection_closed */
+ {bgp_ignore, Clearing}, /* TCP_connection_open_failed */
+ {bgp_ignore, Clearing}, /* TCP_fatal_error */
+ {bgp_ignore, Clearing}, /* ConnectRetry_timer_expired */
+ {bgp_ignore, Clearing}, /* Hold_Timer_expired */
+ {bgp_ignore, Clearing}, /* KeepAlive_timer_expired */
+ {bgp_ignore, Clearing}, /* Receive_OPEN_message */
+ {bgp_ignore, Clearing}, /* Receive_KEEPALIVE_message */
+ {bgp_ignore, Clearing}, /* Receive_UPDATE_message */
+ {bgp_ignore, Clearing}, /* Receive_NOTIFICATION_message */
+ {bgp_ignore, Idle }, /* Clearing_Completed */
+ },
+ {
+ /* Deleted, */
+ {bgp_ignore, Deleted}, /* BGP_Start */
+ {bgp_ignore, Deleted}, /* BGP_Stop */
+ {bgp_ignore, Deleted}, /* TCP_connection_open */
+ {bgp_ignore, Deleted}, /* TCP_connection_closed */
+ {bgp_ignore, Deleted}, /* TCP_connection_open_failed */
+ {bgp_ignore, Deleted}, /* TCP_fatal_error */
+ {bgp_ignore, Deleted}, /* ConnectRetry_timer_expired */
+ {bgp_ignore, Deleted}, /* Hold_Timer_expired */
+ {bgp_ignore, Deleted}, /* KeepAlive_timer_expired */
+ {bgp_ignore, Deleted}, /* Receive_OPEN_message */
+ {bgp_ignore, Deleted}, /* Receive_KEEPALIVE_message */
+ {bgp_ignore, Deleted}, /* Receive_UPDATE_message */
+ {bgp_ignore, Deleted}, /* Receive_NOTIFICATION_message */
+ {bgp_ignore, Deleted}, /* Clearing_Completed */
},
};
@@ -1001,14 +1061,15 @@ static const char *bgp_event_str[] =
"Receive_OPEN_message",
"Receive_KEEPALIVE_message",
"Receive_UPDATE_message",
- "Receive_NOTIFICATION_message"
+ "Receive_NOTIFICATION_message",
+ "Clearing_Completed",
};
/* Execute event process. */
int
bgp_event (struct thread *thread)
{
- int ret;
+ int ret = 0;
int event;
int next;
struct peer *peer;
@@ -1019,14 +1080,15 @@ bgp_event (struct thread *thread)
/* Logging this event. */
next = FSM [peer->status -1][event - 1].next_state;
- if (BGP_DEBUG (fsm, FSM))
+ if (BGP_DEBUG (fsm, FSM) && peer->status != next)
plog_debug (peer->log, "%s [FSM] %s (%s->%s)", peer->host,
bgp_event_str[event],
LOOKUP (bgp_status_msg, peer->status),
LOOKUP (bgp_status_msg, next));
/* Call function. */
- ret = (*(FSM [peer->status - 1][event - 1].func))(peer);
+ if (FSM [peer->status -1][event - 1].func)
+ ret = (*(FSM [peer->status - 1][event - 1].func))(peer);
/* When function do not want proceed next job return -1. */
if (ret >= 0)
diff --git a/bgpd/bgp_fsm.h b/bgpd/bgp_fsm.h
index e90f3b43..0a5d3715 100644
--- a/bgpd/bgp_fsm.h
+++ b/bgpd/bgp_fsm.h
@@ -43,7 +43,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
#define BGP_WRITE_ON(T,F,V) \
do { \
- if (!T) \
+ if (!(T) && (peer->status != Deleted)) \
{ \
peer_lock (peer); \
THREAD_WRITE_ON(master,(T),(F),peer,(V)); \
@@ -61,7 +61,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
#define BGP_TIMER_ON(T,F,V) \
do { \
- if (!T) \
+ if (!(T) && (peer->status != Deleted)) \
{ \
peer_lock (peer); \
THREAD_TIMER_ON(master,(T),(F),peer,(V)); \
@@ -79,8 +79,11 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
#define BGP_EVENT_ADD(P,E) \
do { \
- peer_lock (peer); /* bgp event reference */ \
- thread_add_event (master, bgp_event, (P), (E)); \
+ if ((P)->status != Deleted) \
+ { \
+ peer_lock (peer); /* bgp event reference */ \
+ thread_add_event (master, bgp_event, (P), (E)); \
+ } \
} while (0)
#define BGP_EVENT_DELETE(P) \
@@ -90,6 +93,12 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
thread_cancel_event (master, (P)); \
} while (0)
+#define BGP_EVENT_FLUSH_ADD(P,E) \
+ do { \
+ BGP_EVENT_DELETE(P); \
+ BGP_EVENT_ADD(P,E); \
+ } while (0)
+
/* Prototypes. */
extern int bgp_event (struct thread *);
extern int bgp_stop (struct peer *peer);
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 8b024a1c..da59d329 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -637,9 +637,7 @@ bgp_write (struct thread *thread)
if (write_errno == EWOULDBLOCK || write_errno == EAGAIN)
break;
- BGP_EVENT_ADD (peer, BGP_Stop);
- peer->status = Idle;
- bgp_timer_set (peer);
+ BGP_EVENT_FLUSH_ADD (peer, TCP_fatal_error);
return 0;
}
if (num != writenum)
@@ -673,10 +671,8 @@ bgp_write (struct thread *thread)
if (peer->v_start >= (60 * 2))
peer->v_start = (60 * 2);
- BGP_EVENT_ADD (peer, BGP_Stop);
- /*bgp_stop (peer);*/
- peer->status = Idle;
- bgp_timer_set (peer);
+ /* Flush any existing events */
+ BGP_EVENT_FLUSH_ADD (peer, BGP_Stop);
return 0;
case BGP_MSG_KEEPALIVE:
peer->keepalive_out++;
@@ -721,9 +717,7 @@ bgp_write_notify (struct peer *peer)
ret = writen (peer->fd, STREAM_DATA (s), stream_get_endp (s));
if (ret <= 0)
{
- BGP_EVENT_ADD (peer, BGP_Stop);
- peer->status = Idle;
- bgp_timer_set (peer);
+ BGP_EVENT_FLUSH_ADD (peer, TCP_fatal_error);
return 0;
}
@@ -743,10 +737,7 @@ bgp_write_notify (struct peer *peer)
if (peer->v_start >= (60 * 2))
peer->v_start = (60 * 2);
- /* We don't call event manager at here for avoiding other events. */
- bgp_stop (peer);
- peer->status = Idle;
- bgp_timer_set (peer);
+ BGP_EVENT_FLUSH_ADD (peer, BGP_Stop);
return 0;
}
@@ -2375,14 +2366,6 @@ bgp_read (struct thread *thread)
if (BGP_DEBUG (events, EVENTS))
zlog_debug ("%s [Event] Accepting BGP peer delete", peer->host);
peer_delete (peer);
- /* we've lost track of a reference to ACCEPT_PEER somehow. It doesnt
- * _seem_ to be the 'update realpeer with accept peer' hack, yet it
- * *must* be.. Very very odd, but I give up trying to
- * root cause this - ACCEPT_PEER is a dirty hack, it should be fixed
- * instead, which would make root-causing this a moot point..
- * A hack because of a hack, appropriate.
- */
- peer_unlock (peer); /* god knows what reference... ACCEPT_PEER sucks */
}
return 0;
}
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 2ce2ef4d..5dde41de 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -2527,11 +2527,10 @@ bgp_clear_node_complete (struct work_queue *wq)
{
struct peer *peer = wq->spec.data;
- UNSET_FLAG (peer->sflags, PEER_STATUS_CLEARING);
peer_unlock (peer); /* bgp_clear_node_complete */
/* Tickle FSM to start moving again */
- BGP_EVENT_ADD (peer, BGP_Start);
+ BGP_EVENT_ADD (peer, Clearing_Completed);
}
static void
@@ -2593,9 +2592,10 @@ bgp_clear_route (struct peer *peer, afi_t afi, safi_t safi)
if (peer->clear_node_queue == NULL)
bgp_clear_node_queue_init (peer);
- /* bgp_fsm.c will not bring CLEARING sessions out of Idle this
- * protects against peers which flap faster than we can we clear,
- * which could lead to:
+ /* bgp_fsm.c keeps sessions in state Clearing, not transitioning to
+ * Idle until it receives a Clearing_Completed event. This protects
+ * against peers which flap faster than we can we clear, which could
+ * lead to:
*
* a) race with routes from the new session being installed before
* clear_route_node visits the node (to delete the route of that
@@ -2604,11 +2604,8 @@ bgp_clear_route (struct peer *peer, afi_t afi, safi_t safi)
* on the process_main queue. Fast-flapping could cause that queue
* to grow and grow.
*/
- if (!CHECK_FLAG (peer->sflags, PEER_STATUS_CLEARING))
- {
- SET_FLAG (peer->sflags, PEER_STATUS_CLEARING);
- peer_lock (peer); /* bgp_clear_node_complete */
- }
+ if (!peer->clear_node_queue->thread)
+ peer_lock (peer); /* bgp_clear_node_complete */
if (safi != SAFI_MPLS_VPN)
bgp_clear_route_table (peer, afi, safi, NULL, NULL);
@@ -2624,11 +2621,37 @@ bgp_clear_route (struct peer *peer, afi_t afi, safi_t safi)
bgp_clear_route_table (peer, afi, safi, NULL, rsclient);
}
- /* If no routes were cleared, nothing was added to workqueue, run the
- * completion function now.
+ /* If no routes were cleared, nothing was added to workqueue, the
+ * completion function won't be run by workqueue code - call it here.
+ *
+ * Additionally, there is a presumption in FSM that clearing is only
+ * needed if peer state is Established - peers in pre-Established states
+ * shouldn't have any route-update state associated with them (in or out).
+ *
+ * We still get here from FSM through bgp_stop->clear_route_all in
+ * pre-Established though, so this is a useful sanity check to ensure
+ * the assumption above holds.
+ *
+ * At some future point, this check could be move to the top of the
+ * function, and do a quick early-return when state is
+ * pre-Established, avoiding above list and table scans. Once we're
+ * sure it is safe..
*/
if (!peer->clear_node_queue->thread)
bgp_clear_node_complete (peer->clear_node_queue);
+ else
+ {
+ /* clearing queue scheduled. Normal if in Established state
+ * (and about to transition out of it), but otherwise...
+ */
+ if (peer->status != Established)
+ {
+ plog_err (peer->log, "%s [Error] State %s is not Established,"
+ " but routes were cleared - bug!",
+ peer->host, LOOKUP (bgp_status_msg, peer->status));
+ assert (peer->status == Established);
+ }
+ }
}
void
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index ec4b6c22..b108164f 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -6693,8 +6693,6 @@ bgp_show_summary (struct vty *vty, struct bgp *bgp, int afi, int safi)
vty_out (vty, " Idle (Admin)");
else if (CHECK_FLAG (peer->sflags, PEER_STATUS_PREFIX_OVERFLOW))
vty_out (vty, " Idle (PfxCt)");
- else if (CHECK_FLAG (peer->sflags, PEER_STATUS_CLEARING))
- vty_out (vty, " Idle (Clrng)");
else
vty_out (vty, " %-11s", LOOKUP(bgp_status_msg, peer->status));
}
diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 8ed598d2..733b33a6 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -687,6 +687,15 @@ peer_sort (struct peer *peer)
static inline void
peer_free (struct peer *peer)
{
+ assert (peer->status == Deleted);
+
+ /* this /ought/ to have been done already through bgp_stop earlier,
+ * but just to be sure..
+ */
+ bgp_timer_set (peer);
+ BGP_READ_OFF (peer->t_read);
+ BGP_WRITE_OFF (peer->t_write);
+
if (peer->desc)
XFREE (MTYPE_PEER_DESC, peer->desc);
@@ -704,6 +713,7 @@ peer_free (struct peer *peer)
if (peer->clear_node_queue)
work_queue_free (peer->clear_node_queue);
+ bgp_sync_delete (peer);
memset (peer, 0, sizeof (struct peer));
XFREE (MTYPE_BGP_PEER, peer);
@@ -714,7 +724,8 @@ struct peer *
peer_lock (struct peer *peer)
{
assert (peer && (peer->lock >= 0));
-
+ assert (peer->status != Deleted);
+
peer->lock++;
return peer;
@@ -761,18 +772,17 @@ peer_new ()
struct servent *sp;
/* Allocate new peer. */
- peer = XMALLOC (MTYPE_BGP_PEER, sizeof (struct peer));
- memset (peer, 0, sizeof (struct peer));
+ peer = XCALLOC (MTYPE_BGP_PEER, sizeof (struct peer));
/* Set default value. */
peer->fd = -1;
- peer->lock = 1;
peer->v_start = BGP_INIT_START_TIMER;
peer->v_connect = BGP_DEFAULT_CONNECT_RETRY;
peer->v_asorig = BGP_DEFAULT_ASORIGINATE;
peer->status = Idle;
peer->ostatus = Idle;
peer->weight = 0;
+ peer = peer_lock (peer); /* initial reference */
/* Set default flags. */
for (afi = AFI_IP; afi < AFI_MAX; afi++)
@@ -1139,7 +1149,17 @@ peer_nsf_stop (struct peer *peer)
bgp_clear_route_all (peer);
}
-/* Delete peer from confguration. */
+/* Delete peer from confguration.
+ *
+ * The peer is moved to a dead-end "Deleted" neighbour-state, to allow
+ * it to "cool off" and refcounts to hit 0, at which state it is freed.
+ *
+ * This function /should/ take care to be idempotent, to guard against
+ * it being called multiple times through stray events that come in
+ * that happen to result in this function being called again. That
+ * said, getting here for a "Deleted" peer is a bug in the neighbour
+ * FSM.
+ */
int
peer_delete (struct peer *peer)
{
@@ -1149,6 +1169,8 @@ peer_delete (struct peer *peer)
struct bgp *bgp;
struct bgp_filter *filter;
+ assert (peer->status != Deleted);
+
bgp = peer->bgp;
if (CHECK_FLAG (peer->sflags, PEER_STATUS_NSF_WAIT))
@@ -1158,8 +1180,13 @@ peer_delete (struct peer *peer)
relationship. */
if (peer->group)
{
- peer = peer_unlock (peer); /* peer-group reference */
- listnode_delete (peer->group->peer, peer);
+ struct listnode *pn;
+
+ if ((pn = listnode_lookup (peer->group->peer, peer)))
+ {
+ peer = peer_unlock (peer); /* group->peer list reference */
+ list_delete_node (peer->group->peer, pn);
+ }
peer->group = NULL;
}
@@ -1169,29 +1196,25 @@ peer_delete (struct peer *peer)
*/
peer->last_reset = PEER_DOWN_NEIGHBOR_DELETE;
bgp_stop (peer);
- bgp_fsm_change_status (peer, Idle); /* stops all timers */
+ bgp_fsm_change_status (peer, Deleted);
+ bgp_timer_set (peer); /* stops all timers for Deleted */
- /* Stop all timers - should already have been done in bgp_stop */
- BGP_TIMER_OFF (peer->t_start);
- BGP_TIMER_OFF (peer->t_connect);
- BGP_TIMER_OFF (peer->t_holdtime);
- BGP_TIMER_OFF (peer->t_keepalive);
- BGP_TIMER_OFF (peer->t_asorig);
- BGP_TIMER_OFF (peer->t_routeadv);
- BGP_TIMER_OFF (peer->t_pmax_restart);
- BGP_TIMER_OFF (peer->t_gr_restart);
- BGP_TIMER_OFF (peer->t_gr_stale);
-
/* Delete from all peer list. */
if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP))
{
- peer_unlock (peer); /* bgp peer list reference */
- listnode_delete (bgp->peer, peer);
+ struct listnode *pn;
+
+ if ((pn = listnode_lookup (bgp->peer, peer)))
+ {
+ peer_unlock (peer); /* bgp peer list reference */
+ list_delete_node (bgp->peer, pn);
+ }
- if (peer_rsclient_active (peer))
+ if (peer_rsclient_active (peer)
+ && (pn = listnode_lookup (bgp->rsclient, peer)))
{
peer_unlock (peer); /* rsclient list reference */
- listnode_delete (bgp->rsclient, peer);
+ list_delete_node (bgp->rsclient, pn);
}
}
@@ -1219,8 +1242,6 @@ peer_delete (struct peer *peer)
sockunion_free (peer->su_remote);
peer->su_local = peer->su_remote = NULL;
- bgp_sync_delete (peer);
-
/* Free filter related memory. */
for (afi = AFI_IP; afi < AFI_MAX; afi++)
for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++)
@@ -1692,7 +1713,7 @@ peer_group_bind (struct bgp *bgp, union sockunion *su,
peer->group = group;
peer->af_group[afi][safi] = 1;
- peer = peer_lock (peer); /* peer-group group->peer reference */
+ peer = peer_lock (peer); /* group->peer list reference */
listnode_add (group->peer, peer);
peer_group2peer_config_copy (group, peer, afi, safi);
@@ -1733,9 +1754,11 @@ peer_group_bind (struct bgp *bgp, union sockunion *su,
{
peer->group = group;
- peer = peer_lock (peer); /* peer-group group->peer reference */
+ peer = peer_lock (peer); /* group->peer list reference */
listnode_add (group->peer, peer);
}
+ else
+ assert (group && peer->group == group);
if (first_member)
{
@@ -1759,13 +1782,16 @@ peer_group_bind (struct bgp *bgp, union sockunion *su,
if (CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_RSERVER_CLIENT))
{
+ struct listnode *pn;
+
/* If it's not configured as RSERVER_CLIENT in any other address
family, without being member of a peer_group, remove it from
list bgp->rsclient.*/
- if (! peer_rsclient_active (peer))
+ if (! peer_rsclient_active (peer)
+ && (pn = listnode_lookup (bgp->rsclient, peer)))
{
peer_unlock (peer); /* peer rsclient reference */
- listnode_delete (bgp->rsclient, peer);
+ list_delete_node (bgp->rsclient, pn);
}
bgp_table_finish (peer->rib[afi][safi]);
@@ -1821,6 +1847,7 @@ peer_group_unbind (struct bgp *bgp, struct peer *peer,
if (! peer_group_active (peer))
{
+ assert (listnode_lookup (group->peer, peer));
peer_unlock (peer); /* peer group list reference */
listnode_delete (group->peer, peer);
peer->group = NULL;
diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index b8ae30ae..8b180a43 100644
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -386,7 +386,6 @@ struct peer
#define PEER_STATUS_GROUP (1 << 4) /* peer-group conf */
#define PEER_STATUS_NSF_MODE (1 << 5) /* NSF aware peer */
#define PEER_STATUS_NSF_WAIT (1 << 6) /* wait comeback peer */
-#define PEER_STATUS_CLEARING (1 << 7) /* peers table being cleared */
/* Peer status af flags (reset in bgp_stop) */
u_int16_t af_sflags[AFI_MAX][SAFI_MAX];
@@ -662,7 +661,9 @@ struct bgp_nlri
#define OpenSent 4
#define OpenConfirm 5
#define Established 6
-#define BGP_STATUS_MAX 7
+#define Clearing 7
+#define Deleted 8
+#define BGP_STATUS_MAX 9
/* BGP finite state machine events. */
#define BGP_Start 1
@@ -678,7 +679,8 @@ struct bgp_nlri
#define Receive_KEEPALIVE_message 11
#define Receive_UPDATE_message 12
#define Receive_NOTIFICATION_message 13
-#define BGP_EVENTS_MAX 14
+#define Clearing_Completed 14
+#define BGP_EVENTS_MAX 15
/* BGP timers default value. */
#define BGP_INIT_START_TIMER 5