1 files changed, 31 insertions, 21 deletions

diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index eed5fdc65..0ce2466f5 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -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);
 }
 
 /*