From 1ff9a340588a8890c3036378ca5e754a0a941dab Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 19 May 2015 17:40:37 -0700 Subject: [PATCH] bgpd: bgpd-fsm-fix.patch BGP: Fix FSM to handle active/passive connections better The existing code didn't work well when dual connections resulted between peers during session bringup. This patch fixes that. Signed-off-by: Dinesh G Dutt --- bgpd/bgp_fsm.c | 255 ++++++++++++++++++++++++----- bgpd/bgp_fsm.h | 1 + bgpd/bgp_main.c | 10 +- bgpd/bgp_network.c | 105 ++++++++---- bgpd/bgp_network.h | 2 +- bgpd/bgp_open.c | 1 - bgpd/bgp_packet.c | 256 ++++++++--------------------- bgpd/bgp_route.c | 2 +- bgpd/bgp_vty.c | 28 +++- bgpd/bgpd.c | 390 ++++++++++++++++++++++++++++++++------------- bgpd/bgpd.h | 12 +- 11 files changed, 680 insertions(+), 382 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 9dbe904e88..062392166b 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -70,6 +70,104 @@ bgp_start_jitter (int time) return ((rand () % (time + 1)) - (time / 2)); } +static void +peer_xfer_stats (struct peer *peer_dst, struct peer *peer_src) +{ + /* Copy stats over. These are only the pre-established state stats */ + peer_dst->open_in += peer_src->open_in; + peer_dst->open_out += peer_src->open_out; + peer_dst->keepalive_in += peer_src->keepalive_in; + peer_dst->keepalive_out += peer_src->keepalive_out; + peer_dst->notify_in += peer_src->notify_in; + peer_dst->notify_out += peer_src->notify_out; + peer_dst->dynamic_cap_in += peer_src->dynamic_cap_in; + peer_dst->dynamic_cap_out += peer_src->dynamic_cap_out; +} + +static struct peer * +peer_xfer_conn(struct peer *from_peer) +{ + struct peer *peer; + afi_t afi; + safi_t safi; + int fd; + int status, pstatus; + + assert(from_peer != NULL); + + peer = from_peer->doppelganger; + + if (!peer || !CHECK_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE)) + return from_peer; + + BGP_WRITE_OFF(peer->t_write); + BGP_READ_OFF(peer->t_read); + BGP_WRITE_OFF(from_peer->t_write); + BGP_READ_OFF(from_peer->t_read); + + fd = peer->fd; + peer->fd = from_peer->fd; + from_peer->fd = fd; + stream_reset(peer->ibuf); + stream_fifo_clean(peer->obuf); + stream_fifo_clean(from_peer->obuf); + + peer->v_holdtime = from_peer->v_holdtime; + peer->v_keepalive = from_peer->v_keepalive; + peer->v_asorig = from_peer->v_asorig; + peer->routeadv = from_peer->routeadv; + peer->v_routeadv = from_peer->v_routeadv; + peer->v_gr_restart = from_peer->v_gr_restart; + peer->cap = from_peer->cap; + status = peer->status; + pstatus = peer->ostatus; + peer->status = from_peer->status; + peer->ostatus = from_peer->ostatus; + from_peer->status = status; + from_peer->ostatus = pstatus; + peer->remote_id = from_peer->remote_id; + + for (afi = AFI_IP; afi < AFI_MAX; afi++) + for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++) + { + peer->af_flags[afi][safi] = from_peer->af_flags[afi][safi]; + peer->af_sflags[afi][safi] = from_peer->af_sflags[afi][safi]; + peer->af_cap[afi][safi] = from_peer->af_cap[afi][safi]; + peer->afc_nego[afi][safi] = from_peer->afc_nego[afi][safi]; + peer->afc_adv[afi][safi] = from_peer->afc_adv[afi][safi]; + peer->afc_recv[afi][safi] = from_peer->afc_recv[afi][safi]; + } + + if (bgp_getsockname(peer) < 0) + { + zlog_err ("%%bgp_getsockname() failed for %s peer %s fd %d (from_peer fd %d)", + (CHECK_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER) ? "accept" : ""), + peer->host, peer->fd, from_peer->fd); + bgp_stop(peer); + bgp_stop(from_peer); + return NULL; + } + if (from_peer->status > Active) + { + if (bgp_getsockname(from_peer) < 0) + { + zlog_err ("%%bgp_getsockname() failed for %s from_peer %s fd %d (peer fd %d)", + (CHECK_FLAG (from_peer->sflags, PEER_STATUS_ACCEPT_PEER) ? "accept" : ""), + from_peer->host, from_peer->fd, peer->fd); + bgp_stop(from_peer); + from_peer = NULL; + } + } + + BGP_READ_ON(peer->t_read, bgp_read, peer->fd); + BGP_WRITE_ON(peer->t_write, bgp_write, peer->fd); + + if (from_peer) + peer_xfer_stats(peer, from_peer); + + return(peer); +} + /* Check if suppress start/restart of sessions to peer. */ #define BGP_PEER_START_SUPPRESSED(P) \ (CHECK_FLAG ((P)->flags, PEER_FLAG_SHUTDOWN) \ @@ -212,6 +310,7 @@ bgp_timer_set (struct peer *peer) BGP_TIMER_OFF (peer->t_keepalive); BGP_TIMER_OFF (peer->t_asorig); BGP_TIMER_OFF (peer->t_routeadv); + break; } } @@ -240,6 +339,7 @@ static int bgp_connect_timer (struct thread *thread) { struct peer *peer; + int ret; peer = THREAD_ARG (thread); peer->t_connect = NULL; @@ -248,10 +348,19 @@ bgp_connect_timer (struct thread *thread) zlog (peer->log, LOG_DEBUG, "%s [FSM] Timer (connect timer expire)", peer->host); - THREAD_VAL (thread) = ConnectRetry_timer_expired; - bgp_event (thread); /* bgp_event unlocks peer */ + if (CHECK_FLAG(peer->sflags, PEER_STATUS_ACCEPT_PEER)) + { + bgp_stop(peer); + ret = -1; + } + else + { + THREAD_VAL (thread) = ConnectRetry_timer_expired; + bgp_event (thread); /* bgp_event unlocks peer */ + ret = 0; + } - return 0; + return ret; } /* BGP holdtime timer. */ @@ -668,6 +777,7 @@ bgp_update_delay_process_status_change(struct peer *peer) void bgp_fsm_change_status (struct peer *peer, int status) { + bgp_dump_state (peer, peer->status, status); /* Transition into Clearing or Deleted must /always/ clear all routes.. @@ -680,6 +790,9 @@ bgp_fsm_change_status (struct peer *peer, int status) peer->ostatus = peer->status; peer->status = status; + if (status == Established) + UNSET_FLAG(peer->sflags, PEER_STATUS_ACCEPT_PEER); + /* If update-delay processing is applicable, do the necessary. */ if (bgp_update_delay_configured(peer->bgp) && bgp_update_delay_applicable(peer->bgp)) @@ -697,7 +810,9 @@ static int bgp_clearing_completed (struct peer *peer) { int rc = bgp_stop(peer); - BGP_EVENT_FLUSH (peer); + + if (rc >= 0) + BGP_EVENT_FLUSH (peer); return rc; } @@ -710,6 +825,7 @@ bgp_stop (struct peer *peer) afi_t afi; safi_t safi; char orf_name[BUFSIZ]; + int ret = 0; /* Can't do this in Clearing; events are used for state transitions */ if (peer->status != Clearing) @@ -816,9 +932,11 @@ bgp_stop (struct peer *peer) /* Received ORF prefix-filter */ peer->orf_plist[afi][safi] = NULL; - /* ORF received prefix-filter pnt */ - sprintf (orf_name, "%s.%d.%d", peer->host, afi, safi); - prefix_bgp_orf_remove_all (orf_name); + if ((peer->status == OpenConfirm) || (peer->status == Established)) { + /* ORF received prefix-filter pnt */ + sprintf (orf_name, "%s.%d.%d", peer->host, afi, safi); + prefix_bgp_orf_remove_all (orf_name); + } } /* Reset keepalive and holdtime */ @@ -846,7 +964,14 @@ bgp_stop (struct peer *peer) peer->pcount[AFI_IP6][SAFI_MULTICAST] = 0; #endif /* 0 */ - return 0; + if (!CHECK_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE) && + !(CHECK_FLAG(peer->flags, PEER_FLAG_DELETE))) + { + peer_delete(peer); + ret = -1; + } + + return ret; } /* BGP peer is stoped by the error. */ @@ -860,9 +985,7 @@ bgp_stop_with_error (struct peer *peer) if (peer->v_start >= (60 * 2)) peer->v_start = (60 * 2); - bgp_stop (peer); - - return 0; + return(bgp_stop (peer)); } @@ -873,21 +996,10 @@ bgp_stop_with_notify (struct peer *peer, u_char code, u_char sub_code) /* Send notify to remote peer */ bgp_notify_send (peer, code, sub_code); - /* Sweep if it is temporary peer. */ - if (CHECK_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER)) - { - zlog_info ("%s [Event] Accepting BGP peer is deleted", peer->host); - peer_delete (peer); - return -1; - } - /* Clear start timer value to default. */ peer->v_start = BGP_INIT_START_TIMER; - /* bgp_stop needs to be invoked while in Established state */ - bgp_stop(peer); - - return 0; + return(bgp_stop(peer)); } @@ -896,16 +1008,25 @@ bgp_stop_with_notify (struct peer *peer, u_char code, u_char sub_code) static int bgp_connect_success (struct peer *peer) { + int ret = 0; + if (peer->fd < 0) { zlog_err ("bgp_connect_success peer's fd is negative value %d", peer->fd); + bgp_stop(peer); return -1; } - BGP_READ_ON (peer->t_read, bgp_read, peer->fd); - if (! CHECK_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER)) - bgp_getsockname (peer); + if (bgp_getsockname (peer) < 0) + { + zlog_err ("%s: bgp_getsockname(): failed for peer %s", __FUNCTION__, + peer->host); + bgp_notify_send(peer, BGP_NOTIFY_FSM_ERR, 0); /* internal error */ + return -1; + } + + BGP_READ_ON (peer->t_read, bgp_read, peer->fd); if (BGP_DEBUG (normal, NORMAL)) { @@ -918,8 +1039,7 @@ bgp_connect_success (struct peer *peer) zlog_debug ("%s passive open", peer->host); } - if (! CHECK_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER)) - bgp_open_send (peer); + bgp_open_send (peer); return 0; } @@ -928,8 +1048,7 @@ bgp_connect_success (struct peer *peer) static int bgp_connect_fail (struct peer *peer) { - bgp_stop (peer); - return 0; + return (bgp_stop (peer)); } /* This function is the first starting point of all BGP connection. It @@ -1014,9 +1133,14 @@ bgp_start (struct peer *peer) static int bgp_reconnect (struct peer *peer) { - bgp_stop (peer); - bgp_start (peer); - return 0; + int ret = 0; + + if (bgp_stop (peer) > 0) + bgp_start (peer); + else + ret = -1; + + return ret; } static int @@ -1077,6 +1201,19 @@ bgp_establish (struct peer *peer) afi_t afi; safi_t safi; int nsf_af_count = 0; + int ret = 0; + struct peer *other; + + other = peer->doppelganger; + peer = peer_xfer_conn(peer); + if (!peer) + { + zlog_err ("%%Neighbor failed in xfer_conn"); + return -1; + } + + if (other == peer) + ret = 1; /* bgp_establish specific code when xfer_conn happens. */ /* Reset capability open status flag. */ if (! CHECK_FLAG (peer->sflags, PEER_STATUS_CAPABILITY_OPEN)) @@ -1180,7 +1317,19 @@ bgp_establish (struct peer *peer) if (!bgp_update_delay_active(peer->bgp)) BGP_TIMER_ON (peer->t_routeadv, bgp_routeadv_timer, 0); - return 0; + if (peer->doppelganger && (peer->doppelganger->status != Deleted)) + { + if (BGP_DEBUG (events, EVENTS)) + zlog_debug("[Event] Deleting stub connection for peer %s", peer->host); + + if (peer->doppelganger->status > Active) + bgp_notify_send (peer->doppelganger, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_COLLISION_RESOLUTION); + else + peer_delete(peer->doppelganger); + } + + return ret; } /* Keepalive packet is received. */ @@ -1382,19 +1531,34 @@ static const char *bgp_event_str[] = int bgp_event (struct thread *thread) { - int ret = 0; int event; - int next; struct peer *peer; + int ret; peer = THREAD_ARG (thread); event = THREAD_VAL (thread); + ret = bgp_event_update(peer, event); + + return (ret); +} + +int +bgp_event_update (struct peer *peer, int event) +{ + int next; + int ret = 0; + struct peer *other; + int passive_conn = 0; + + other = peer->doppelganger; + passive_conn = (CHECK_FLAG(peer->sflags, PEER_STATUS_ACCEPT_PEER)) ? 1 : 0; + /* Logging this event. */ next = FSM [peer->status -1][event - 1].next_state; if (BGP_DEBUG (fsm, FSM) && peer->status != next) - plog_debug (peer->log, "%s [FSM] %s (%s->%s)", peer->host, + 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)); @@ -1406,13 +1570,28 @@ bgp_event (struct thread *thread) /* When function do not want proceed next job return -1. */ if (ret >= 0) { + if (ret == 1 && next == Established) + { + /* The case when doppelganger swap accurred in bgp_establish. + Update the peer pointer accordingly */ + peer = other; + } + /* If status is changed. */ if (next != peer->status) bgp_fsm_change_status (peer, next); /* Make sure timer is set. */ bgp_timer_set (peer); + + } + else if (!passive_conn && peer->bgp) + { + /* If we got a return value of -1, that means there was an error, restart + * the FSM. If the peer structure was deleted + */ + bgp_fsm_change_status(peer, Idle); + bgp_timer_set(peer); } - return ret; } diff --git a/bgpd/bgp_fsm.h b/bgpd/bgp_fsm.h index eecd131d66..a4c5a611d0 100644 --- a/bgpd/bgp_fsm.h +++ b/bgpd/bgp_fsm.h @@ -75,6 +75,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA /* Prototypes. */ extern int bgp_event (struct thread *); +extern int bgp_event_update (struct peer *, int event); extern int bgp_stop (struct peer *peer); extern void bgp_timer_set (struct peer *); extern void bgp_fsm_change_status (struct peer *peer, int status); diff --git a/bgpd/bgp_main.c b/bgpd/bgp_main.c index b0ba06fd6e..584bfa761d 100644 --- a/bgpd/bgp_main.c +++ b/bgpd/bgp_main.c @@ -232,19 +232,13 @@ bgp_exit (int status) /* it only makes sense for this to be called on a clean exit */ assert (status == 0); + bgp_close(); + /* reverse bgp_master_init */ for (ALL_LIST_ELEMENTS (bm->bgp, node, nnode, bgp)) bgp_delete (bgp); list_free (bm->bgp); - /* reverse bgp_master_init */ - for (ALL_LIST_ELEMENTS_RO(bm->listen_sockets, node, socket)) - { - if (close ((int)(long)socket) == -1) - zlog_err ("close (%d): %s", (int)(long)socket, safe_strerror (errno)); - } - list_delete (bm->listen_sockets); - /* reverse bgp_zebra_init/if_init */ if (retain_mode) if_add_hook (IF_DELETE_HOOK, NULL); diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index c0527447a1..e9d557b746 100644 --- a/bgpd/bgp_network.c +++ b/bgpd/bgp_network.c @@ -33,6 +33,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA #include "network.h" #include "bgpd/bgpd.h" +#include "bgpd/bgp_open.h" #include "bgpd/bgp_fsm.h" #include "bgpd/bgp_attr.h" #include "bgpd/bgp_debug.h" @@ -149,7 +150,7 @@ static void bgp_set_socket_ttl (struct peer *peer, int bgp_sock) { char buf[INET_ADDRSTRLEN]; - int ret; + int ret = 0; /* In case of peer is EBGP, we should set TTL for this connection. */ if (!peer->gtsm_hops && (peer_sort (peer) == BGP_PEER_EBGP)) @@ -223,48 +224,89 @@ bgp_accept (struct thread *thread) if (BGP_DEBUG (events, EVENTS)) zlog_debug ("[Event] BGP connection from host %s", inet_sutop (&su, buf)); - + /* Check remote IP address */ peer1 = peer_lookup (NULL, &su); - if (! peer1 || peer1->status == Idle) + if (! peer1) { if (BGP_DEBUG (events, EVENTS)) { - if (! peer1) - zlog_debug ("[Event] BGP connection IP address %s is not configured", - inet_sutop (&su, buf)); - else - zlog_debug ("[Event] BGP connection IP address %s is Idle state", - inet_sutop (&su, buf)); + zlog_debug ("[Event] BGP connection IP address %s is not configured", + inet_sutop (&su, buf)); } close (bgp_sock); return -1; } + if (CHECK_FLAG(peer1->flags, PEER_FLAG_SHUTDOWN)) + { + zlog_debug ("[Event] connection from %s rejected due to admin shutdown", + inet_sutop (&su, buf)); + close (bgp_sock); + return -1; + } + + /* + * Do not accept incoming connections in Clearing state. This can result + * in incorect state transitions - e.g., the connection goes back to + * Established and then the Clearing_Completed event is generated. Also, + * block incoming connection in Deleted state. + */ + if (peer1->status == Clearing || peer1->status == Deleted) + { + struct bgp *bgp = peer1->bgp; + + if (BGP_DEBUG (events, EVENTS)) + zlog_debug("[Event] Closing incoming conn for %s (0x%x) state %d", + peer1->host, peer1, peer1->status); + close (bgp_sock); + return -1; + } + + if (peer1->doppelganger) + { + /* We have an existing connection. Kill the existing one and run + with this one. + */ + if (BGP_DEBUG (events, EVENTS)) + zlog_debug ("[Event] New active connection from peer %s, Killing" + " previous active connection", peer1->host); + peer_delete(peer1->doppelganger); + } + bgp_set_socket_ttl (peer1, bgp_sock); - /* Make dummy peer until read Open packet. */ - if (BGP_DEBUG (events, EVENTS)) - zlog_debug ("[Event] Make dummy peer structure until read Open packet"); + peer = peer_create (&su, peer1->bgp, peer1->local_as, + peer1->as, 0, 0); - { - char buf[SU_ADDRSTRLEN]; + peer_xfer_config(peer, peer1); + UNSET_FLAG (peer->flags, PEER_FLAG_CONFIG_NODE); - peer = peer_create_accept (peer1->bgp); - SET_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER); - peer->su = su; - peer->fd = bgp_sock; - peer->status = Active; - peer->local_id = peer1->local_id; - peer->v_holdtime = peer1->v_holdtime; - peer->v_keepalive = peer1->v_keepalive; + peer->doppelganger = peer1; + peer1->doppelganger = peer; + peer->fd = bgp_sock; + bgp_fsm_change_status(peer, Active); + BGP_TIMER_OFF(peer->t_start); /* created in peer_create() */ - /* Make peer's address string. */ - sockunion2str (&su, buf, SU_ADDRSTRLEN); - peer->host = XSTRDUP (MTYPE_BGP_PEER_HOST, buf); - } + SET_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER); - BGP_EVENT_ADD (peer, TCP_connection_open); + /* Make dummy peer until read Open packet. */ + if (peer1->status == Established && + CHECK_FLAG (peer1->sflags, PEER_STATUS_NSF_MODE)) + { + /* If we have an existing established connection with graceful restart + * capability announced with one or more address families, then drop + * existing established connection and move state to connect. + */ + peer1->last_reset = PEER_DOWN_NSF_CLOSE_SESSION; + SET_FLAG (peer1->sflags, PEER_STATUS_NSF_WAIT); + bgp_event_update(peer1, TCP_connection_closed); + } + + if (peer_active (peer)) + { + BGP_EVENT_ADD (peer, TCP_connection_open); + } return 0; } @@ -415,7 +457,7 @@ bgp_connect (struct peer *peer) } /* After TCP connection is established. Get local address and port. */ -void +int bgp_getsockname (struct peer *peer) { if (peer->su_local) @@ -431,9 +473,13 @@ bgp_getsockname (struct peer *peer) } peer->su_local = sockunion_getsockname (peer->fd); + if (!peer->su_local) return -1; peer->su_remote = sockunion_getpeername (peer->fd); + if (!peer->su_remote) return -1; bgp_nexthop_set (peer->su_local, peer->su_remote, &peer->nexthop, peer); + + return 0; } @@ -596,6 +642,9 @@ bgp_close (void) struct listnode *node, *next; struct bgp_listener *listener; + if (bm->listen_sockets == NULL) + return; + for (ALL_LIST_ELEMENTS (bm->listen_sockets, node, next, listener)) { thread_cancel (listener->thread); diff --git a/bgpd/bgp_network.h b/bgpd/bgp_network.h index 127684306b..403393e84f 100644 --- a/bgpd/bgp_network.h +++ b/bgpd/bgp_network.h @@ -26,7 +26,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA extern int bgp_socket (unsigned short, const char *); extern void bgp_close (void); extern int bgp_connect (struct peer *); -extern void bgp_getsockname (struct peer *); +extern int bgp_getsockname (struct peer *); extern int bgp_md5_set (struct peer *); diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c index fe741aa3ac..a48f3f7121 100644 --- a/bgpd/bgp_open.c +++ b/bgpd/bgp_open.c @@ -816,7 +816,6 @@ bgp_open_option_parse (struct peer *peer, u_char length, int *mp_capability) peer->host); if (error != error_data) - bgp_notify_send_with_data (peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_UNSUP_CAPBL, diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index 0c12df891c..ecb96dce06 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -842,6 +842,8 @@ bgp_write_notify (struct peer *peer) if (peer->v_start >= (60 * 2)) peer->v_start = (60 * 2); + /* Handle Graceful Restart case where the state changes to + Connect instead of Idle */ BGP_EVENT_ADD (peer, BGP_Stop); return 0; @@ -1020,8 +1022,8 @@ bgp_notify_send_with_data (struct peer *peer, u_char code, u_char sub_code, } } else - zlog_info ("Notification sent to neighbor %s: configuration change", - peer->host); + zlog_info ("Notification sent to neighbor %s:%d: configuration change", + peer->host, peer->fd); /* Call immediately. */ BGP_WRITE_OFF (peer->t_write); @@ -1189,13 +1191,7 @@ static int bgp_collision_detect (struct peer *new, struct in_addr remote_id) { struct peer *peer; - struct listnode *node, *nnode; - struct bgp *bgp; - bgp = bgp_get_default (); - if (! bgp) - return 0; - /* Upon receipt of an OPEN message, the local system must examine all of its connections that are in the OpenConfirm state. A BGP speaker may also examine connections in an OpenSent state if it @@ -1205,31 +1201,42 @@ bgp_collision_detect (struct peer *new, struct in_addr remote_id) OPEN message, then the local system performs the following collision resolution procedure: */ - for (ALL_LIST_ELEMENTS (bgp->peer, node, nnode, peer)) + if ((peer = new->doppelganger) != NULL) { - /* Under OpenConfirm status, local peer structure already hold - remote router ID. */ - - if (peer != new - && (peer->status == OpenConfirm || peer->status == OpenSent) - && sockunion_same (&peer->su, &new->su)) + /* Do not accept the new connection in Established or Clearing states. + * Note that a peer GR is handled by closing the existing connection + * upon receipt of new one. + */ + if (peer->status == Established || peer->status == Clearing) + { + bgp_notify_send (new, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_COLLISION_RESOLUTION); + return (-1); + } + else if ((peer->status == OpenConfirm) || (peer->status == OpenSent)) { /* 1. The BGP Identifier of the local system is compared to the BGP Identifier of the remote system (as specified in the OPEN message). */ if (ntohl (peer->local_id.s_addr) < ntohl (remote_id.s_addr)) - { - /* 2. If the value of the local BGP Identifier is less - than the remote one, the local system closes BGP - connection that already exists (the one that is - already in the OpenConfirm state), and accepts BGP - connection initiated by the remote system. */ - - if (peer->fd >= 0) - bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_COLLISION_RESOLUTION); - return 1; - } + if (!CHECK_FLAG(peer->sflags, PEER_STATUS_ACCEPT_PEER)) + { + /* 2. If the value of the local BGP Identifier is less + than the remote one, the local system closes BGP + connection that already exists (the one that is + already in the OpenConfirm state), and accepts BGP + connection initiated by the remote system. */ + bgp_notify_send (peer, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_COLLISION_RESOLUTION); + return 1; + } + else + { + bgp_notify_send (new, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_COLLISION_RESOLUTION); + return -1; + } else { /* 3. Otherwise, the local system closes newly created @@ -1237,11 +1244,18 @@ bgp_collision_detect (struct peer *new, struct in_addr remote_id) received OPEN message), and continues to use the existing one (the one that is already in the OpenConfirm state). */ - - if (new->fd >= 0) - bgp_notify_send (new, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_COLLISION_RESOLUTION); - return -1; + if (CHECK_FLAG(peer->sflags, PEER_STATUS_ACCEPT_PEER)) + { + bgp_notify_send (peer, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_COLLISION_RESOLUTION); + return 1; + } + else + { + bgp_notify_send (new, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_COLLISION_RESOLUTION); + return -1; + } } } } @@ -1258,14 +1272,12 @@ bgp_open_receive (struct peer *peer, bgp_size_t size) u_int16_t send_holdtime; as_t remote_as; as_t as4 = 0; - struct peer *realpeer; + struct peer *active_peer = NULL; struct in_addr remote_id; int mp_capability; u_int8_t notify_data_remote_as[2]; u_int8_t notify_data_remote_id[4]; - realpeer = NULL; - /* Parse open packet. */ version = stream_getc (peer->ibuf); memcpy (notify_data_remote_as, stream_pnt (peer->ibuf), 2); @@ -1314,8 +1326,8 @@ bgp_open_receive (struct peer *peer, bgp_size_t size) { zlog_err ("%s [AS4] NEW speaker using AS_TRANS for AS4, not allowed", peer->host); - bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, - BGP_NOTIFY_OPEN_BAD_PEER_AS); + bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, + BGP_NOTIFY_OPEN_BAD_PEER_AS); return -1; } @@ -1345,148 +1357,6 @@ bgp_open_receive (struct peer *peer, bgp_size_t size) } } - /* Lookup peer from Open packet. */ - if (CHECK_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER)) - { - int as = 0; - - realpeer = peer_lookup_with_open (&peer->su, remote_as, &remote_id, &as); - - if (! realpeer) - { - /* Peer's source IP address is check in bgp_accept(), so this - must be AS number mismatch or remote-id configuration - mismatch. */ - if (as) - { - if (BGP_DEBUG (normal, NORMAL)) - zlog_debug ("%s bad OPEN, wrong router identifier %s", - peer->host, inet_ntoa (remote_id)); - bgp_notify_send_with_data (peer, BGP_NOTIFY_OPEN_ERR, - BGP_NOTIFY_OPEN_BAD_BGP_IDENT, - notify_data_remote_id, 4); - } - else - { - if (BGP_DEBUG (normal, NORMAL)) - zlog_debug ("%s bad OPEN, remote AS is %u, expected %u", - peer->host, remote_as, peer->as); - bgp_notify_send_with_data (peer, BGP_NOTIFY_OPEN_ERR, - BGP_NOTIFY_OPEN_BAD_PEER_AS, - notify_data_remote_as, 2); - } - return -1; - } - } - - /* When collision is detected and this peer is closed. Retrun - immidiately. */ - ret = bgp_collision_detect (peer, remote_id); - if (ret < 0) - return ret; - - /* Hack part. */ - if (CHECK_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER)) - { - if (realpeer->status == Established - && CHECK_FLAG (realpeer->sflags, PEER_STATUS_NSF_MODE)) - { - realpeer->last_reset = PEER_DOWN_NSF_CLOSE_SESSION; - SET_FLAG (realpeer->sflags, PEER_STATUS_NSF_WAIT); - } - else if (ret == 0 && realpeer->status != Active - && realpeer->status != OpenSent - && realpeer->status != OpenConfirm - && realpeer->status != Connect) - { - /* XXX: This is an awful problem.. - * - * According to the RFC we should just let this connection (of the - * accepted 'peer') continue on to Established if the other - * connection (the 'realpeer' one) is in state Connect, and deal - * with the more larval FSM as/when it gets far enough to receive - * an Open. We don't do that though, we instead close the (more - * developed) accepted connection. - * - * This means there's a race, which if hit, can loop: - * - * FSM for A FSM for B - * realpeer accept-peer realpeer accept-peer - * - * Connect Connect - * Active - * OpenSent OpenSent - * - * Idle Active - * OpenSent OpenSent - * - * Idle - * - * Connect Connect - * - * - * If both sides are Quagga, they're almost certain to wait for - * the same amount of time of course (which doesn't preclude other - * implementations also waiting for same time). The race is - * exacerbated by high-latency (in bgpd and/or the network). - * - * The reason we do this is because our FSM is tied to our peer - * structure, which carries our configuration information, etc. - * I.e. we can't let the accepted-peer FSM continue on as it is, - * cause it's not associated with any actual peer configuration - - * it's just a dummy. - * - * It's possible we could hack-fix this by just bgp_stop'ing the - * realpeer and continueing on with the 'transfer FSM' below. - * Ideally, we need to seperate FSMs from struct peer. - * - * Setting one side to passive avoids the race, as a workaround. - */ - if (BGP_DEBUG (events, EVENTS)) - zlog_debug ("%s peer status is %s close connection", - realpeer->host, LOOKUP (bgp_status_msg, - realpeer->status)); - bgp_notify_send (peer, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONNECT_REJECT); - - return -1; - } - - if (BGP_DEBUG (events, EVENTS)) - zlog_debug ("%s [Event] Transfer accept BGP peer to real (state %s)", - peer->host, - LOOKUP (bgp_status_msg, realpeer->status)); - - bgp_stop (realpeer); - - /* Transfer file descriptor. */ - realpeer->fd = peer->fd; - peer->fd = -1; - - /* Transfer input buffer. */ - stream_free (realpeer->ibuf); - realpeer->ibuf = peer->ibuf; - realpeer->packet_size = peer->packet_size; - peer->ibuf = NULL; - - /* Transfer status. */ - realpeer->status = peer->status; - bgp_stop (peer); - - /* peer pointer change. Open packet send to neighbor. */ - peer = realpeer; - bgp_open_send (peer); - if (peer->fd < 0) - { - zlog_err ("bgp_open_receive peer's fd is negative value %d", - peer->fd); - return -1; - } - BGP_READ_ON (peer->t_read, bgp_read, peer->fd); - } - /* remote router-id check. */ if (remote_id.s_addr == 0 || IPV4_CLASS_DE (ntohl (remote_id.s_addr)) @@ -1598,10 +1468,27 @@ bgp_open_receive (struct peer *peer, bgp_size_t size) peer->afc_nego[AFI_IP6][SAFI_MULTICAST] = peer->afc[AFI_IP6][SAFI_MULTICAST]; } + /* When collision is detected and this peer is closed. Retrun + immidiately. */ + ret = bgp_collision_detect (peer, remote_id); + if (ret < 0) + return ret; + /* Get sockname. */ - bgp_getsockname (peer); + if ((ret = bgp_getsockname (peer)) < 0) + { + zlog_err("%s: bgp_getsockname() failed for peer: %s", __FUNCTION__, + peer->host); + return (ret); + } - BGP_EVENT_ADD (peer, Receive_OPEN_message); + if ((ret = bgp_event_update(peer, Receive_OPEN_message)) < 0) + { + zlog_err("%s: BGP event update failed for peer: %s", __FUNCTION__, + peer->host); + /* DD: bgp send notify and reset state */ + return (ret); + } peer->packet_size = 0; if (peer->ibuf) @@ -1633,7 +1520,8 @@ bgp_check_update_delay(struct bgp *bgp) (bgp->v_establish_wait == bgp->v_update_delay)) for (ALL_LIST_ELEMENTS (bgp->peer, node, nnode, peer)) { - if (!CHECK_FLAG (peer->flags, PEER_FLAG_SHUTDOWN) + if (CHECK_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE) + && !CHECK_FLAG (peer->flags, PEER_FLAG_SHUTDOWN) && !peer->update_delay_over) { if (BGP_DEBUG (normal, NORMAL)) @@ -2787,11 +2675,5 @@ bgp_read (struct thread *thread) stream_reset (peer->ibuf); done: - if (CHECK_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER)) - { - if (BGP_DEBUG (events, EVENTS)) - zlog_debug ("%s [Event] Accepting BGP peer delete", peer->host); - peer_delete (peer); - } return 0; } diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index b2ec4811af..7a911c1812 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -1786,7 +1786,7 @@ bgp_maximum_prefix_restart_timer (struct thread *thread) zlog_debug ("%s Maximum-prefix restart timer expired, restore peering", peer->host); - peer_clear (peer); + peer_clear (peer, NULL); return 0; } diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index db09d583b7..bc18e7bd7d 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -1665,6 +1665,7 @@ DEFUN (no_neighbor, union sockunion su; struct peer_group *group; struct peer *peer; + struct peer *other; ret = str2sockunion (argv[0], &su); if (ret < 0) @@ -1682,7 +1683,12 @@ DEFUN (no_neighbor, { peer = peer_lookup (vty->index, &su); if (peer) - peer_delete (peer); + { + other = peer->doppelganger; + peer_delete (peer); + if (other && other->status != Deleted) + peer_delete(other); + } } return CMD_SUCCESS; @@ -4493,12 +4499,16 @@ bgp_clear (struct vty *vty, struct bgp *bgp, afi_t afi, safi_t safi, struct listnode *node, *nnode; /* Clear all neighbors. */ + /* + * Pass along pointer to next node to peer_clear() when walking all nodes + * on the BGP instance as that may get freed if it is a doppelganger + */ if (sort == clear_all) { for (ALL_LIST_ELEMENTS (bgp->peer, node, nnode, peer)) { if (stype == BGP_CLEAR_SOFT_NONE) - ret = peer_clear (peer); + ret = peer_clear (peer, &nnode); else ret = peer_clear_soft (peer, afi, safi, stype); @@ -4534,7 +4544,7 @@ bgp_clear (struct vty *vty, struct bgp *bgp, afi_t afi, safi_t safi, } if (stype == BGP_CLEAR_SOFT_NONE) - ret = peer_clear (peer); + ret = peer_clear (peer, NULL); else ret = peer_clear_soft (peer, afi, safi, stype); @@ -4560,7 +4570,7 @@ bgp_clear (struct vty *vty, struct bgp *bgp, afi_t afi, safi_t safi, { if (stype == BGP_CLEAR_SOFT_NONE) { - ret = peer_clear (peer); + ret = peer_clear (peer, NULL); continue; } @@ -4583,7 +4593,7 @@ bgp_clear (struct vty *vty, struct bgp *bgp, afi_t afi, safi_t safi, continue; if (stype == BGP_CLEAR_SOFT_NONE) - ret = peer_clear (peer); + ret = peer_clear (peer, &nnode); else ret = peer_clear_soft (peer, afi, safi, stype); @@ -4607,7 +4617,7 @@ bgp_clear (struct vty *vty, struct bgp *bgp, afi_t afi, safi_t safi, find = 1; if (stype == BGP_CLEAR_SOFT_NONE) - ret = peer_clear (peer); + ret = peer_clear (peer, &nnode); else ret = peer_clear_soft (peer, afi, safi, stype); @@ -7129,6 +7139,9 @@ bgp_show_summary (struct vty *vty, struct bgp *bgp, int afi, int safi, char *del for (ALL_LIST_ELEMENTS (bgp->peer, node, nnode, peer)) { + if (!CHECK_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE)) + continue; + if (peer->afc[afi][safi]) { if (delimit) @@ -8211,6 +8224,9 @@ bgp_show_neighbor (struct vty *vty, struct bgp *bgp, for (ALL_LIST_ELEMENTS (bgp->peer, node, nnode, peer)) { + if (!CHECK_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE)) + continue; + switch (type) { case show_all: diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index f1903630f0..3fbf9e6ace 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -73,6 +73,44 @@ struct bgp_master *bm; /* BGP community-list. */ struct community_list_handler *bgp_clist; + +static inline void +bgp_session_reset(struct peer *peer) +{ + if (peer->doppelganger && (peer->doppelganger->status != Deleted) + && !(CHECK_FLAG(peer->doppelganger->flags, PEER_FLAG_CONFIG_NODE))) + peer_delete(peer->doppelganger); + + BGP_EVENT_ADD (peer, BGP_Stop); +} + +/* + * During session reset, we may delete the doppelganger peer, which would + * be the next node to the current node. If the session reset was invoked + * during walk of peer list, we would end up accessing the freed next + * node. This function moves the next node along. + */ +static inline void +bgp_session_reset_safe(struct peer *peer, struct listnode **nnode) +{ + struct listnode *n; + struct peer *npeer; + + n = (nnode) ? *nnode : NULL; + npeer = (n) ? listgetdata(n) : NULL; + + if (peer->doppelganger && (peer->doppelganger->status != Deleted) + && !(CHECK_FLAG(peer->doppelganger->flags, PEER_FLAG_CONFIG_NODE))) + { + if (peer->doppelganger == npeer) + /* nnode and *nnode are confirmed to be non-NULL here */ + *nnode = (*nnode)->next; + peer_delete(peer->doppelganger); + } + + BGP_EVENT_ADD (peer, BGP_Stop); +} + /* BGP global flag manipulation. */ int bgp_option_set (int flag) @@ -308,9 +346,8 @@ bgp_confederation_id_set (struct bgp *bgp, as_t as) bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); } - else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset_safe(peer, &nnode); } } else @@ -329,7 +366,7 @@ bgp_confederation_id_set (struct bgp *bgp, as_t as) BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset_safe(peer, &nnode); } } } @@ -359,7 +396,7 @@ bgp_confederation_id_unset (struct bgp *bgp) } else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset_safe(peer, &nnode); } } return 0; @@ -422,7 +459,7 @@ bgp_confederation_peers_add (struct bgp *bgp, as_t as) BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset_safe(peer, &nnode); } } } @@ -478,7 +515,7 @@ bgp_confederation_peers_remove (struct bgp *bgp, as_t as) BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset_safe(peer, &nnode); } } } @@ -784,7 +821,7 @@ peer_unlock (struct peer *peer) return peer; } - + /* Allocate new peer object, implicitely locked. */ static struct peer * peer_new (struct bgp *bgp) @@ -809,11 +846,11 @@ peer_new (struct bgp *bgp) peer->v_asorig = BGP_DEFAULT_ASORIGINATE; peer->status = Idle; peer->ostatus = Idle; - peer->weight = 0; - peer->password = NULL; peer->bgp = bgp; peer = peer_lock (peer); /* initial reference */ bgp_lock (bgp); + peer->weight = 0; + peer->password = NULL; /* Set default flags. */ for (afi = AFI_IP; afi < AFI_MAX; afi++) @@ -834,6 +871,7 @@ peer_new (struct bgp *bgp) peer->work = stream_new (BGP_MAX_PACKET_SIZE); peer->scratch = stream_new (BGP_MAX_PACKET_SIZE); + bgp_sync_init (peer); /* Get service port number. */ @@ -843,8 +881,96 @@ peer_new (struct bgp *bgp) return peer; } +/* + * This function is invoked when a duplicate peer structure associated with + * a neighbor is being deleted. If this about-to-be-deleted structure is + * the one with all the config, then we have to copy over the info. + */ +void +peer_xfer_config (struct peer *peer_dst, struct peer *peer_src) +{ + afi_t afi; + safi_t safi; + + assert(peer_src); + assert(peer_dst); + + /* The following function is used by both peer group config copy to + * individual peer and when we transfer config + */ + if (peer_src->change_local_as) + peer_dst->change_local_as = peer_src->change_local_as; + + /* peer flags apply */ + peer_dst->flags = peer_src->flags; + peer_dst->cap = peer_src->cap; + peer_dst->config = peer_src->config; + + peer_dst->local_as = peer_src->local_as; + peer_dst->ifindex = peer_src->ifindex; + peer_dst->port = peer_src->port; + peer_sort(peer_dst); + peer_dst->rmap_type = peer_src->rmap_type; + + /* Timers */ + peer_dst->holdtime = peer_src->holdtime; + peer_dst->keepalive = peer_src->keepalive; + peer_dst->connect = peer_src->connect; + peer_dst->v_holdtime = peer_src->v_holdtime; + peer_dst->v_keepalive = peer_src->v_keepalive; + peer_dst->v_asorig = peer_src->v_asorig; + peer_dst->routeadv = peer_src->routeadv; + peer_dst->v_routeadv = peer_src->v_routeadv; + + /* password apply */ + if (peer_src->password && !peer_dst->password) + peer_dst->password = XSTRDUP (MTYPE_PEER_PASSWORD, peer_src->password); + + bgp_md5_set (peer_dst); + + for (afi = AFI_IP; afi < AFI_MAX; afi++) + for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++) + { + peer_dst->afc[afi][safi] = peer_src->afc[afi][safi]; + peer_dst->af_flags[afi][safi] = peer_src->af_flags[afi][safi]; + peer_dst->allowas_in[afi][safi] = peer_src->allowas_in[afi][safi]; + } + + /* update-source apply */ + if (peer_src->update_source) + { + if (peer_dst->update_source) + sockunion_free (peer_dst->update_source); + if (peer_dst->update_if) + { + XFREE (MTYPE_PEER_UPDATE_SOURCE, peer_dst->update_if); + peer_dst->update_if = NULL; + } + peer_dst->update_source = sockunion_dup (peer_src->update_source); + } + else if (peer_src->update_if) + { + if (peer_dst->update_if) + XFREE (MTYPE_PEER_UPDATE_SOURCE, peer_dst->update_if); + if (peer_dst->update_source) + { + sockunion_free (peer_dst->update_source); + peer_dst->update_source = NULL; + } + peer_dst->update_if = XSTRDUP (MTYPE_PEER_UPDATE_SOURCE, peer_src->update_if); + } + + if (peer_src->ifname) + { + if (peer_dst->ifname) + free(peer_dst->ifname); + + peer_dst->ifname = strdup(peer_src->ifname); + } +} + /* Create new BGP peer. */ -static struct peer * +struct peer * peer_create (union sockunion *su, struct bgp *bgp, as_t local_as, as_t remote_as, afi_t afi, safi_t safi) { @@ -878,6 +1004,8 @@ peer_create (union sockunion *su, struct bgp *bgp, as_t local_as, /* Default TTL set. */ peer->ttl = (peer->sort == BGP_PEER_IBGP) ? 255 : 1; + SET_FLAG (peer->flags, PEER_FLAG_CONFIG_NODE); + /* Make peer's address string. */ sockunion2str (su, buf, SU_ADDRSTRLEN); peer->host = XSTRDUP (MTYPE_BGP_PEER_HOST, buf); @@ -896,7 +1024,7 @@ peer_create_accept (struct bgp *bgp) struct peer *peer; peer = peer_new (bgp); - + peer = peer_lock (peer); /* bgp peer list reference */ listnode_add_sort (bgp->peer, peer); @@ -920,7 +1048,7 @@ peer_as_change (struct peer *peer, as_t as) BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); } type = peer_sort (peer); peer->as = as; @@ -1038,7 +1166,7 @@ peer_remote_as (struct bgp *bgp, union sockunion *su, as_t *as, && afi == AFI_IP && safi == SAFI_UNICAST) peer = peer_create (su, bgp, local_as, *as, 0, 0); else - peer = peer_create (su, bgp, local_as, *as, afi, safi); + peer = peer_create (su, bgp, local_as, *as, afi, safi); } return 0; @@ -1207,12 +1335,14 @@ peer_delete (struct peer *peer) struct listnode *pn; assert (peer->status != Deleted); - + bgp = peer->bgp; if (CHECK_FLAG (peer->sflags, PEER_STATUS_NSF_WAIT)) peer_nsf_stop (peer); + SET_FLAG(peer->flags, PEER_FLAG_DELETE); + /* If this peer belongs to peer group, clear up the relationship. */ if (peer->group) @@ -1231,6 +1361,13 @@ peer_delete (struct peer *peer) */ peer->last_reset = PEER_DOWN_NEIGHBOR_DELETE; bgp_stop (peer); + UNSET_FLAG(peer->flags, PEER_FLAG_DELETE); + + if (peer->doppelganger) + peer->doppelganger->doppelganger = NULL; + peer->doppelganger = NULL; + + UNSET_FLAG(peer->sflags, PEER_STATUS_ACCEPT_PEER); bgp_fsm_change_status (peer, Deleted); /* Password configuration */ @@ -1244,7 +1381,7 @@ peer_delete (struct peer *peer) } bgp_timer_set (peer); /* stops all timers for Deleted */ - + /* Delete from all peer list. */ if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP) && (pn = listnode_lookup (bgp->peer, peer))) @@ -1259,12 +1396,15 @@ peer_delete (struct peer *peer) peer_unlock (peer); /* rsclient list reference */ list_delete_node (bgp->rsclient, pn); - /* Clear our own rsclient ribs. */ - for (afi = AFI_IP; afi < AFI_MAX; afi++) - for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++) - if (CHECK_FLAG(peer->af_flags[afi][safi], - PEER_FLAG_RSERVER_CLIENT)) - bgp_clear_route (peer, afi, safi, BGP_CLEAR_ROUTE_MY_RSCLIENT); + if (CHECK_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE)) + { + /* Clear our own rsclient ribs. */ + for (afi = AFI_IP; afi < AFI_MAX; afi++) + for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++) + if (CHECK_FLAG(peer->af_flags[afi][safi], + PEER_FLAG_RSERVER_CLIENT)) + bgp_clear_route (peer, afi, safi, BGP_CLEAR_ROUTE_MY_RSCLIENT); + } } /* Free RIB for any family in which peer is RSERVER_CLIENT, and is not @@ -1705,14 +1845,21 @@ peer_group_delete (struct peer_group *group) { struct bgp *bgp; struct peer *peer; + struct peer *other; struct listnode *node, *nnode; bgp = group->bgp; for (ALL_LIST_ELEMENTS (group->peer, node, nnode, peer)) { + other = peer->doppelganger; peer->group = NULL; peer_delete (peer); + if (other && other->status != Deleted) + { + other->group = NULL; + peer_delete(other); + } } list_delete (group->peer); @@ -1733,7 +1880,7 @@ peer_group_delete (struct peer_group *group) int peer_group_remote_as_delete (struct peer_group *group) { - struct peer *peer; + struct peer *peer, *other; struct listnode *node, *nnode; if (! group->conf->as) @@ -1741,8 +1888,16 @@ peer_group_remote_as_delete (struct peer_group *group) for (ALL_LIST_ELEMENTS (group->peer, node, nnode, peer)) { + other = peer->doppelganger; + peer->group = NULL; peer_delete (peer); + + if (other && other->status != Deleted) + { + other->group = NULL; + peer_delete(other); + } } list_delete_all_node (group->peer); @@ -1779,6 +1934,7 @@ peer_group_bind (struct bgp *bgp, union sockunion *su, peer = peer_lock (peer); /* group->peer list reference */ listnode_add (group->peer, peer); peer_group2peer_config_copy (group, peer, afi, safi); + SET_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE); return 0; } @@ -1883,6 +2039,7 @@ peer_group_bind (struct bgp *bgp, union sockunion *su, } peer_group2peer_config_copy (group, peer, afi, safi); + SET_FLAG(peer->flags, PEER_FLAG_CONFIG_NODE); if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) { @@ -1891,7 +2048,7 @@ peer_group_bind (struct bgp *bgp, union sockunion *su, BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); return 0; } @@ -1900,6 +2057,8 @@ int peer_group_unbind (struct bgp *bgp, struct peer *peer, struct peer_group *group, afi_t afi, safi_t safi) { + struct peer *other; + if (! peer->af_group[afi][safi]) return 0; @@ -1919,9 +2078,20 @@ peer_group_unbind (struct bgp *bgp, struct peer *peer, peer_unlock (peer); /* peer group list reference */ listnode_delete (group->peer, peer); peer->group = NULL; + other = peer->doppelganger; if (group->conf->as) { peer_delete (peer); + if (other && other->status != Deleted) + { + if (other->group) + { + peer_unlock(other); + listnode_delete(group->peer, other); + } + other->group = NULL; + peer_delete(other); + } return 0; } peer_global_config_reset (peer); @@ -1934,7 +2104,7 @@ peer_group_unbind (struct bgp *bgp, struct peer *peer, BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); return 0; } @@ -2127,17 +2297,6 @@ bgp_delete (struct bgp *bgp) if (i != ZEBRA_ROUTE_BGP) bgp_redistribute_unset (bgp, afi, i); - for (ALL_LIST_ELEMENTS (bgp->peer, node, next, peer)) - { - if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) - { - /* Send notify to remote peer. */ - bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_ADMIN_SHUTDOWN); - } - - peer_delete (peer); - } - for (ALL_LIST_ELEMENTS (bgp->group, node, next, group)) { for (ALL_LIST_ELEMENTS (group->peer, node, next, peer)) @@ -2151,6 +2310,17 @@ bgp_delete (struct bgp *bgp) peer_group_delete (group); } + for (ALL_LIST_ELEMENTS (bgp->peer, node, next, peer)) + { + if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) + { + /* Send notify to remote peer. */ + bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_ADMIN_SHUTDOWN); + } + + peer_delete (peer); + } + assert (listcount (bgp->rsclient) == 0); if (bgp->peer_self) { @@ -2222,7 +2392,7 @@ peer_lookup (struct bgp *bgp, union sockunion *su) { for (ALL_LIST_ELEMENTS (bgp->peer, node, nnode, peer)) if (sockunion_same (&peer->su, su) - && ! CHECK_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER)) + && (CHECK_FLAG (peer->flags, PEER_FLAG_CONFIG_NODE))) return peer; } else if (bm->bgp != NULL) @@ -2232,55 +2402,12 @@ peer_lookup (struct bgp *bgp, union sockunion *su) for (ALL_LIST_ELEMENTS (bm->bgp, bgpnode, nbgpnode, bgp)) for (ALL_LIST_ELEMENTS (bgp->peer, node, nnode, peer)) if (sockunion_same (&peer->su, su) - && ! CHECK_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER)) + && (CHECK_FLAG (peer->flags, PEER_FLAG_CONFIG_NODE))) return peer; } return NULL; } -struct peer * -peer_lookup_with_open (union sockunion *su, as_t remote_as, - struct in_addr *remote_id, int *as) -{ - struct peer *peer; - struct listnode *node; - struct listnode *bgpnode; - struct bgp *bgp; - - if (! bm->bgp) - return NULL; - - for (ALL_LIST_ELEMENTS_RO (bm->bgp, bgpnode, bgp)) - { - for (ALL_LIST_ELEMENTS_RO (bgp->peer, node, peer)) - { - if (sockunion_same (&peer->su, su) - && ! CHECK_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER)) - { - if (peer->as == remote_as - && peer->remote_id.s_addr == remote_id->s_addr) - return peer; - if (peer->as == remote_as) - *as = 1; - } - } - - for (ALL_LIST_ELEMENTS_RO (bgp->peer, node, peer)) - { - if (sockunion_same (&peer->su, su) - && ! CHECK_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER)) - { - if (peer->as == remote_as - && peer->remote_id.s_addr == 0) - return peer; - if (peer->as == remote_as) - *as = 1; - } - } - } - return NULL; -} - /* If peer is configured at least one address family return 1. */ int peer_active (struct peer *peer) @@ -2327,16 +2454,31 @@ peer_change_action (struct peer *peer, afi_t afi, safi_t safi, return; if (type == peer_change_reset) - bgp_notify_send (peer, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + { + /* If we're resetting session, we've to delete both peer struct */ + if ((peer->doppelganger) && (peer->doppelganger->status != Deleted) + && (!CHECK_FLAG(peer->doppelganger->flags, + PEER_FLAG_CONFIG_NODE))) + peer_delete(peer->doppelganger); + + bgp_notify_send (peer, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_CONFIG_CHANGE); + } else if (type == peer_change_reset_in) { if (CHECK_FLAG (peer->cap, PEER_CAP_REFRESH_OLD_RCV) || CHECK_FLAG (peer->cap, PEER_CAP_REFRESH_NEW_RCV)) bgp_route_refresh_send (peer, afi, safi, 0, 0, 0); else - bgp_notify_send (peer, BGP_NOTIFY_CEASE, - BGP_NOTIFY_CEASE_CONFIG_CHANGE); + { + if ((peer->doppelganger) && (peer->doppelganger->status != Deleted) + && (!CHECK_FLAG(peer->doppelganger->flags, + PEER_FLAG_CONFIG_NODE))) + peer_delete(peer->doppelganger); + + bgp_notify_send (peer, BGP_NOTIFY_CEASE, + BGP_NOTIFY_CEASE_CONFIG_CHANGE); + } } else if (type == peer_change_reset_out) bgp_announce_route (peer, afi, safi); @@ -2458,14 +2600,14 @@ peer_flag_modify_action (struct peer *peer, u_int32_t flag) peer->host); } - if (CHECK_FLAG (peer->sflags, PEER_STATUS_NSF_WAIT)) - peer_nsf_stop (peer); + if (CHECK_FLAG (peer->sflags, PEER_STATUS_NSF_WAIT)) + peer_nsf_stop (peer); if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_ADMIN_SHUTDOWN); else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); } else { @@ -2486,7 +2628,7 @@ peer_flag_modify_action (struct peer *peer, u_int32_t flag) BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); } /* Change specified peer flag. */ @@ -2883,7 +3025,7 @@ peer_update_source_if_set (struct peer *peer, const char *ifname) BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); return 0; } @@ -2915,7 +3057,7 @@ peer_update_source_if_set (struct peer *peer, const char *ifname) BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); } return 0; } @@ -2939,6 +3081,7 @@ peer_update_source_addr_set (struct peer *peer, union sockunion *su) { XFREE (MTYPE_PEER_UPDATE_SOURCE, peer->update_if); peer->update_if = NULL; + } peer->update_source = sockunion_dup (su); @@ -2952,7 +3095,7 @@ peer_update_source_addr_set (struct peer *peer, union sockunion *su) BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); return 0; } @@ -2983,7 +3126,7 @@ peer_update_source_addr_set (struct peer *peer, union sockunion *su) BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); } return 0; } @@ -3034,7 +3177,7 @@ peer_update_source_unset (struct peer *peer) BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); return 0; } @@ -3064,7 +3207,7 @@ peer_update_source_unset (struct peer *peer) BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); } return 0; } @@ -3565,8 +3708,7 @@ peer_local_as_set (struct peer *peer, as_t as, int no_prepend, int replace_as) BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD (peer, BGP_Stop); - + bgp_session_reset(peer); return 0; } @@ -3641,7 +3783,7 @@ peer_local_as_unset (struct peer *peer) BGP_NOTIFY_CEASE_CONFIG_CHANGE); } else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); } return 0; } @@ -3671,7 +3813,7 @@ peer_password_set (struct peer *peer, const char *password) if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); return (bgp_md5_set (peer) >= 0) ? BGP_SUCCESS : BGP_ERR_TCPSIG_FAILED; } @@ -3689,7 +3831,7 @@ peer_password_set (struct peer *peer, const char *password) if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); if (bgp_md5_set (peer) < 0) ret = BGP_ERR_TCPSIG_FAILED; @@ -3717,7 +3859,7 @@ peer_password_unset (struct peer *peer) if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); if (peer->password) XFREE (MTYPE_PEER_PASSWORD, peer->password); @@ -3740,7 +3882,7 @@ peer_password_unset (struct peer *peer) if (BGP_IS_VALID_STATE_FOR_NOTIF(peer->status)) bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_CONFIG_CHANGE); else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); XFREE (MTYPE_PEER_PASSWORD, peer->password); peer->password = NULL; @@ -4573,7 +4715,11 @@ peer_ttl_security_hops_set (struct peer *peer, int gtsm_hops) if (! CHECK_FLAG (peer->sflags, PEER_STATUS_GROUP)) { if (peer->fd >= 0) - sockopt_minttl (peer->su.sa.sa_family, peer->fd, MAXTTL + 1 - gtsm_hops); + sockopt_minttl (peer->su.sa.sa_family, peer->fd, + MAXTTL + 1 - gtsm_hops); + if (peer->status != Established && peer->doppelganger) + sockopt_minttl (peer->su.sa.sa_family, peer->doppelganger->fd, + MAXTTL + 1 - gtsm_hops); } else { @@ -4597,7 +4743,7 @@ peer_ttl_security_hops_set (struct peer *peer, int gtsm_hops) { if (BGP_DEBUG (events, EVENTS)) zlog_debug ("%s Min-ttl changed", peer->host); - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset(peer); } } } @@ -4625,6 +4771,9 @@ peer_ttl_security_hops_unset (struct peer *peer) { if (peer->fd >= 0) sockopt_minttl (peer->su.sa.sa_family, peer->fd, 0); + + if (peer->status != Established && peer->doppelganger) + sockopt_minttl (peer->su.sa.sa_family, peer->doppelganger->fd, 0); } else { @@ -4635,14 +4784,24 @@ peer_ttl_security_hops_unset (struct peer *peer) if (peer->fd >= 0) sockopt_minttl (peer->su.sa.sa_family, peer->fd, 0); + + if (peer->status != Established && peer->doppelganger) + sockopt_minttl (peer->su.sa.sa_family, peer->doppelganger->fd, 0); } } return peer_ebgp_multihop_unset (opeer); } +/* + * If peer clear is invoked in a loop for all peers on the BGP instance, + * it may end up freeing the doppelganger, and if this was the next node + * to the current node, we would end up accessing the freed next node. + * Pass along additional parameter which can be updated if next node + * is freed; only required when walking the peer list on BGP instance. + */ int -peer_clear (struct peer *peer) +peer_clear (struct peer *peer, struct listnode **nnode) { if (! CHECK_FLAG (peer->flags, PEER_FLAG_SHUTDOWN)) { @@ -4665,7 +4824,7 @@ peer_clear (struct peer *peer) bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_ADMIN_RESET); else - BGP_EVENT_ADD (peer, BGP_Stop); + bgp_session_reset_safe(peer, nnode); } return 0; } @@ -5265,7 +5424,7 @@ bgp_config_write_family (struct vty *vty, struct bgp *bgp, afi_t afi, { if (peer->afc[afi][safi]) { - if (! CHECK_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER)) + if (CHECK_FLAG (peer->flags, PEER_FLAG_CONFIG_NODE)) { bgp_config_write_family_header (vty, afi, safi, &write); bgp_config_write_peer (vty, bgp, peer, afi, safi); @@ -5454,7 +5613,7 @@ bgp_config_write (struct vty *vty) /* Normal neighbor configuration. */ for (ALL_LIST_ELEMENTS (bgp->peer, node, nnode, peer)) { - if (! CHECK_FLAG (peer->sflags, PEER_STATUS_ACCEPT_PEER)) + if (CHECK_FLAG (peer->flags, PEER_FLAG_CONFIG_NODE)) bgp_config_write_peer (vty, bgp, peer, AFI_IP, SAFI_UNICAST); } @@ -5550,12 +5709,25 @@ bgp_terminate (void) struct listnode *node, *nnode; struct listnode *mnode, *mnnode; + /* Close the listener sockets first as this prevents peers from attempting + * to reconnect on receiving the peer unconfig message. In the presence + * of a large number of peers this will ensure that no peer is left with + * a dangling connection + */ + /* reverse bgp_master_init */ + bgp_close(); + if (bm->listen_sockets) + list_free(bm->listen_sockets); + bm->listen_sockets = NULL; + for (ALL_LIST_ELEMENTS (bm->bgp, mnode, mnnode, bgp)) for (ALL_LIST_ELEMENTS (bgp->peer, node, nnode, peer)) - if (peer->status == Established) + if (peer->status == Established || + peer->status == OpenSent || + peer->status == OpenConfirm) bgp_notify_send (peer, BGP_NOTIFY_CEASE, BGP_NOTIFY_CEASE_PEER_UNCONFIG); - + bgp_cleanup_routes (); if (bm->process_main_queue) diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index d32bdcf043..ef22c45f13 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -347,6 +347,9 @@ struct peer */ struct stream *scratch; + /* the doppelganger peer structure, due to dual TCP conn setup */ + struct peer *doppelganger; + /* Status of the peer. */ int status; int ostatus; @@ -419,6 +422,8 @@ struct peer #define PEER_FLAG_DISABLE_CONNECTED_CHECK (1 << 6) /* disable-connected-check */ #define PEER_FLAG_LOCAL_AS_NO_PREPEND (1 << 7) /* local-as no-prepend */ #define PEER_FLAG_LOCAL_AS_REPLACE_AS (1 << 8) /* local-as no-prepend replace-as */ +#define PEER_FLAG_DELETE (1 << 9) /* mark the peer for deleting */ +#define PEER_FLAG_CONFIG_NODE (1 << 10) /* the node to update configs on */ /* NSF mode (graceful restart) */ u_char nsf[AFI_MAX][SAFI_MAX]; @@ -878,14 +883,15 @@ extern struct bgp *bgp_lookup_by_name (const char *); extern struct peer *peer_lookup (struct bgp *, union sockunion *); extern struct peer_group *peer_group_lookup (struct bgp *, const char *); extern struct peer_group *peer_group_get (struct bgp *, const char *); -extern struct peer *peer_lookup_with_open (union sockunion *, as_t, struct in_addr *, - int *); extern struct peer *peer_lock (struct peer *); extern struct peer *peer_unlock (struct peer *); extern bgp_peer_sort_t peer_sort (struct peer *peer); extern int peer_active (struct peer *); extern int peer_active_nego (struct peer *); +extern struct peer *peer_create(union sockunion *su, struct bgp *bgp, as_t local_as, + as_t remote_as, afi_t afi, safi_t safi); extern struct peer *peer_create_accept (struct bgp *); +extern void peer_xfer_config (struct peer *dst, struct peer *src); extern char *peer_uptime (time_t, char *, size_t); extern int bgp_config_write (struct vty *); extern void bgp_config_write_family_header (struct vty *, afi_t, safi_t, int *); @@ -1011,7 +1017,7 @@ extern int peer_unsuppress_map_unset (struct peer *, afi_t, safi_t); extern int peer_maximum_prefix_set (struct peer *, afi_t, safi_t, u_int32_t, u_char, int, u_int16_t); extern int peer_maximum_prefix_unset (struct peer *, afi_t, safi_t); -extern int peer_clear (struct peer *); +extern int peer_clear (struct peer *, struct listnode **); extern int peer_clear_soft (struct peer *, afi_t, safi_t, enum bgp_clear_type); extern int peer_ttl_security_hops_set (struct peer *, int); -- 2.39.5