From 6403814caab46eef50dd1193593e0e9715ef6a43 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 19 May 2015 18:04:12 -0700 Subject: [PATCH] When unexpected events are received, do not silently transition to Idle state through bgp_ignore() as that may not do required cleanup. Instead, define a new event handler to handle such cases, which will go through bgp_stop(). A similar change is also done to handle the case where an event handler fails. Also add a couple of variables to keep track of events for a peer. --- bgpd/bgp_fsm.c | 123 ++++++++++++++++++++++++++++++++----------------- bgpd/bgpd.c | 1 + bgpd/bgpd.h | 7 +++ 3 files changed, 88 insertions(+), 43 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index e47cceb8c4..683d8da31a 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -48,6 +48,28 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA #endif /* HAVE_SNMP */ #include "bgpd/bgp_updgrp.h" +/* Definition of display strings corresponding to FSM events. This should be + * kept consistent with the events defined in bgpd.h + */ +static const char *bgp_event_str[] = +{ + NULL, + "BGP_Start", + "BGP_Stop", + "TCP_connection_open", + "TCP_connection_closed", + "TCP_connection_open_failed", + "TCP_fatal_error", + "ConnectRetry_timer_expired", + "Hold_Timer_expired", + "KeepAlive_timer_expired", + "Receive_OPEN_message", + "Receive_KEEPALIVE_message", + "Receive_UPDATE_message", + "Receive_NOTIFICATION_message", + "Clearing_Completed", +}; + /* BGP FSM (finite state machine) has three types of functions. Type one is thread functions. Type two is event functions. Type three is FSM functions. Timer functions are set by bgp_timer_set @@ -87,6 +109,7 @@ peer_xfer_conn(struct peer *from_peer) safi_t safi; int fd; int status, pstatus; + unsigned char last_evt, last_maj_evt; assert(from_peer != NULL); @@ -122,10 +145,16 @@ peer_xfer_conn(struct peer *from_peer) peer->cap = from_peer->cap; status = peer->status; pstatus = peer->ostatus; + last_evt = peer->last_event; + last_maj_evt = peer->last_major_event; peer->status = from_peer->status; peer->ostatus = from_peer->ostatus; + peer->last_event = from_peer->last_event; + peer->last_major_event = from_peer->last_major_event; from_peer->status = status; from_peer->ostatus = pstatus; + from_peer->last_event = last_evt; + from_peer->last_major_event = last_maj_evt; peer->remote_id = from_peer->remote_id; for (afi = AFI_IP; afi < AFI_MAX; afi++) @@ -888,6 +917,9 @@ bgp_fsm_change_status (struct peer *peer, int status) peer->ostatus = peer->status; peer->status = status; + /* Save event that caused status change. */ + peer->last_major_event = peer->cur_event; + if (status == Established) UNSET_FLAG(peer->sflags, PEER_STATUS_ACCEPT_PEER); @@ -1527,11 +1559,26 @@ bgp_fsm_update (struct peer *peer) static int bgp_ignore (struct peer *peer) { - if (bgp_debug_neighbor_events(peer)) - zlog_debug ("%s [FSM] bgp_ignore called", peer->host); + zlog_err ("%s [FSM] Ignoring event %s in state %s, prior events %s, %s", + peer->host, bgp_event_str[peer->cur_event], + LOOKUP (bgp_status_msg, peer->status), + bgp_event_str[peer->last_event], + bgp_event_str[peer->last_major_event]); return 0; } +/* This is to handle unexpected events.. */ +static int +bgp_fsm_exeption (struct peer *peer) +{ + zlog_err ("%s [FSM] Unexpected event %s in state %s, prior events %s, %s", + peer->host, bgp_event_str[peer->cur_event], + LOOKUP (bgp_status_msg, peer->status), + bgp_event_str[peer->last_event], + bgp_event_str[peer->last_major_event]); + return(bgp_stop (peer)); +} + void bgp_fsm_nht_update(struct peer *peer, int valid) { @@ -1607,13 +1654,13 @@ static const struct { {bgp_connect_fail, Active}, /* TCP_connection_open_failed */ {bgp_connect_fail, Idle}, /* TCP_fatal_error */ {bgp_reconnect, Connect}, /* ConnectRetry_timer_expired */ - {bgp_ignore, Idle}, /* Hold_Timer_expired */ - {bgp_ignore, Idle}, /* KeepAlive_timer_expired */ - {bgp_ignore, Idle}, /* Receive_OPEN_message */ - {bgp_ignore, Idle}, /* Receive_KEEPALIVE_message */ - {bgp_ignore, Idle}, /* Receive_UPDATE_message */ + {bgp_fsm_exeption, Idle}, /* Hold_Timer_expired */ + {bgp_fsm_exeption, Idle}, /* KeepAlive_timer_expired */ + {bgp_fsm_exeption, Idle}, /* Receive_OPEN_message */ + {bgp_fsm_exeption, Idle}, /* Receive_KEEPALIVE_message */ + {bgp_fsm_exeption, Idle}, /* Receive_UPDATE_message */ {bgp_stop, Idle}, /* Receive_NOTIFICATION_message */ - {bgp_ignore, Idle}, /* Clearing_Completed */ + {bgp_fsm_exeption, Idle}, /* Clearing_Completed */ }, { /* Active, */ @@ -1622,15 +1669,15 @@ static const struct { {bgp_connect_success, OpenSent}, /* TCP_connection_open */ {bgp_stop, Idle}, /* TCP_connection_closed */ {bgp_ignore, Active}, /* TCP_connection_open_failed */ - {bgp_ignore, Idle}, /* TCP_fatal_error */ + {bgp_fsm_exeption, Idle}, /* TCP_fatal_error */ {bgp_start, Connect}, /* ConnectRetry_timer_expired */ - {bgp_ignore, Idle}, /* Hold_Timer_expired */ - {bgp_ignore, Idle}, /* KeepAlive_timer_expired */ - {bgp_ignore, Idle}, /* Receive_OPEN_message */ - {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 */ + {bgp_fsm_exeption, Idle}, /* Hold_Timer_expired */ + {bgp_fsm_exeption, Idle}, /* KeepAlive_timer_expired */ + {bgp_fsm_exeption, Idle}, /* Receive_OPEN_message */ + {bgp_fsm_exeption, Idle}, /* Receive_KEEPALIVE_message */ + {bgp_fsm_exeption, Idle}, /* Receive_UPDATE_message */ + {bgp_fsm_exeption, Idle}, /* Receive_NOTIFICATION_message */ + {bgp_fsm_exeption, Idle}, /* Clearing_Completed */ }, { /* OpenSent, */ @@ -1640,14 +1687,14 @@ static const struct { {bgp_stop, Active}, /* TCP_connection_closed */ {bgp_stop, Active}, /* TCP_connection_open_failed */ {bgp_stop, Active}, /* TCP_fatal_error */ - {bgp_ignore, Idle}, /* ConnectRetry_timer_expired */ + {bgp_fsm_exeption, Idle}, /* ConnectRetry_timer_expired */ {bgp_fsm_holdtime_expire, Idle}, /* Hold_Timer_expired */ - {bgp_ignore, Idle}, /* KeepAlive_timer_expired */ + {bgp_fsm_exeption, Idle}, /* KeepAlive_timer_expired */ {bgp_fsm_open, OpenConfirm}, /* Receive_OPEN_message */ {bgp_fsm_event_error, Idle}, /* Receive_KEEPALIVE_message */ {bgp_fsm_event_error, Idle}, /* Receive_UPDATE_message */ {bgp_stop_with_error, Idle}, /* Receive_NOTIFICATION_message */ - {bgp_ignore, Idle}, /* Clearing_Completed */ + {bgp_fsm_exeption, Idle}, /* Clearing_Completed */ }, { /* OpenConfirm, */ @@ -1657,14 +1704,14 @@ static const struct { {bgp_stop, Idle}, /* TCP_connection_closed */ {bgp_stop, Idle}, /* TCP_connection_open_failed */ {bgp_stop, Idle}, /* TCP_fatal_error */ - {bgp_ignore, Idle}, /* ConnectRetry_timer_expired */ + {bgp_fsm_exeption, Idle}, /* ConnectRetry_timer_expired */ {bgp_fsm_holdtime_expire, Idle}, /* Hold_Timer_expired */ {bgp_ignore, OpenConfirm}, /* KeepAlive_timer_expired */ - {bgp_ignore, Idle}, /* Receive_OPEN_message */ + {bgp_fsm_exeption, Idle}, /* Receive_OPEN_message */ {bgp_establish, Established}, /* Receive_KEEPALIVE_message */ - {bgp_ignore, Idle}, /* Receive_UPDATE_message */ + {bgp_fsm_exeption, Idle}, /* Receive_UPDATE_message */ {bgp_stop_with_error, Idle}, /* Receive_NOTIFICATION_message */ - {bgp_ignore, Idle}, /* Clearing_Completed */ + {bgp_fsm_exeption, Idle}, /* Clearing_Completed */ }, { /* Established, */ @@ -1681,7 +1728,7 @@ static const struct { {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 */ + {bgp_fsm_exeption, Idle}, /* Clearing_Completed */ }, { /* Clearing, */ @@ -1719,25 +1766,6 @@ static const struct { }, }; -static const char *bgp_event_str[] = -{ - NULL, - "BGP_Start", - "BGP_Stop", - "TCP_connection_open", - "TCP_connection_closed", - "TCP_connection_open_failed", - "TCP_fatal_error", - "ConnectRetry_timer_expired", - "Hold_Timer_expired", - "KeepAlive_timer_expired", - "Receive_OPEN_message", - "Receive_KEEPALIVE_message", - "Receive_UPDATE_message", - "Receive_NOTIFICATION_message", - "Clearing_Completed", -}; - /* Execute event process. */ int bgp_event (struct thread *thread) @@ -1776,6 +1804,9 @@ bgp_event_update (struct peer *peer, int event) LOOKUP (bgp_status_msg, peer->status), LOOKUP (bgp_status_msg, next)); + peer->last_event = peer->cur_event; + peer->cur_event = event; + /* Call function. */ if (FSM [peer->status -1][event - 1].func) ret = (*(FSM [peer->status - 1][event - 1].func))(peer); @@ -1803,6 +1834,12 @@ bgp_event_update (struct peer *peer, int event) /* If we got a return value of -1, that means there was an error, restart * the FSM. If the peer structure was deleted */ + zlog_err ("%s [FSM] Failure handling event %s in state %s, prior events %s, %s", + peer->host, bgp_event_str[peer->cur_event], + LOOKUP (bgp_status_msg, peer->status), + bgp_event_str[peer->last_event], + bgp_event_str[peer->last_major_event]); + bgp_stop (peer); bgp_fsm_change_status(peer, Idle); bgp_timer_set(peer); } diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 8006054347..75656d249e 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -980,6 +980,7 @@ peer_new (struct bgp *bgp) peer->v_asorig = BGP_DEFAULT_ASORIGINATE; peer->status = Idle; peer->ostatus = Idle; + peer->cur_event = peer->last_event = peer->last_major_event = 0; peer->bgp = bgp; peer = peer_lock (peer); /* initial reference */ bgp_lock (bgp); diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 357b8389d5..ed555051b5 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -502,6 +502,13 @@ struct peer int status; int ostatus; + /* FSM events, stored for debug purposes. + * Note: uchar used for reduced memory usage. + */ + unsigned char cur_event; + unsigned char last_event; + unsigned char last_major_event; + /* Peer index, used for dumping TABLE_DUMP_V2 format */ uint16_t table_dump_index; -- 2.39.5