Merge pull request #1668 from qlyoung/fix-bgp-corrupt-header-assert

bgpd: fix race condition causing occasional assert
This commit is contained in:
Russ White 2018-01-25 09:15:20 -05:00 committed by GitHub
commit c6301d7eab
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -545,19 +545,26 @@ void bgp_open_send(struct peer *peer)
bgp_writes_on(peer);
}
/* This is only for sending NOTIFICATION message to neighbor. */
/*
* Writes NOTIFICATION message directly to a peer socket without waiting for
* the I/O thread.
*
* There must be exactly one stream on the peer->obuf FIFO, and the data within
* this stream must match the format of a BGP NOTIFICATION message.
* Transmission is best-effort.
*
* @requires peer->io_mtx
* @param peer
* @return 0
*/
static int bgp_write_notify(struct peer *peer)
{
int ret, val;
u_char type;
struct stream *s;
pthread_mutex_lock(&peer->io_mtx);
{
/* There should be at least one packet. */
s = stream_fifo_pop(peer->obuf);
}
pthread_mutex_unlock(&peer->io_mtx);
/* There should be at least one packet. */
s = stream_fifo_pop(peer->obuf);
if (!s)
return 0;
@ -622,6 +629,14 @@ static int bgp_write_notify(struct peer *peer)
* This function attempts to write the packet from the thread it is called
* from, to ensure the packet gets out ASAP.
*
* This function may be called from multiple threads. Since the function
* modifies I/O buffer(s) in the peer, these are locked for the duration of the
* call to prevent tampering from other threads.
*
* Delivery of the NOTIFICATION is attempted once and is best-effort. After
* return, the peer structure *must* be reset; no assumptions about session
* state are valid.
*
* @param peer
* @param code BGP error code
* @param sub_code BGP error subcode
@ -634,6 +649,10 @@ void bgp_notify_send_with_data(struct peer *peer, u_char code, u_char sub_code,
struct stream *s;
int length;
/* Lock I/O mutex to prevent other threads from pushing packets */
pthread_mutex_lock(&peer->io_mtx);
/* ============================================== */
/* Allocate new stream. */
s = stream_new(BGP_MAX_PACKET_SIZE);
@ -651,20 +670,8 @@ void bgp_notify_send_with_data(struct peer *peer, u_char code, u_char sub_code,
/* Set BGP packet length. */
length = bgp_packet_set_size(s);
/*
* Turn off keepalive generation for peer. This is necessary because
* otherwise between the time we wipe the output buffer and the time we
* push the NOTIFY onto it, the KA generation thread could have pushed
* a KEEPALIVE in the middle.
*/
bgp_keepalives_off(peer);
/* wipe output buffer */
pthread_mutex_lock(&peer->io_mtx);
{
stream_fifo_clean(peer->obuf);
}
pthread_mutex_unlock(&peer->io_mtx);
stream_fifo_clean(peer->obuf);
/*
* If possible, store last packet for debugging purposes. This check is
@ -728,9 +735,12 @@ void bgp_notify_send_with_data(struct peer *peer, u_char code, u_char sub_code,
peer->last_reset = PEER_DOWN_NOTIFY_SEND;
/* Add packet to peer's output queue */
bgp_packet_add(peer, s);
stream_fifo_push(peer->obuf, s);
bgp_write_notify(peer);
/* ============================================== */
pthread_mutex_unlock(&peer->io_mtx);
}
/*