Major changes to BGP

Fixes two race conditions causing crash of Bird, several unhandled
cases during BGP initialization, and some other bugs. Also changes
handling of startup delay to be more useful and implement
reporting of last error in 'show protocols' command.

1 files changed, 352 insertions, 144 deletions

diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index 29d2e09f..46b28906 100644
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -53,7 +53,7 @@
  * Unknown transitive attributes are attached to the route as %EAF_TYPE_OPAQUE byte streams.
  */
 
-#undef LOCAL_DEBUG
+#define LOCAL_DEBUG
 
 #include "nest/bird.h"
 #include "nest/iface.h"
@@ -70,20 +70,69 @@
 struct linpool *bgp_linpool;		/* Global temporary pool */
 static sock *bgp_listen_sk;		/* Global listening socket */
 static int bgp_counter;			/* Number of protocol instances using the listening socket */
-static char *bgp_state_names[] = { "Idle", "Connect", "Active", "OpenSent", "OpenConfirm", "Established" };
 
+static void bgp_close(struct bgp_proto *p, int apply_md5);
 static void bgp_connect(struct bgp_proto *p);
+static void bgp_active(struct bgp_proto *p, int delay);
 static void bgp_initiate(struct bgp_proto *p);
-static void bgp_setup_listen_sk(void);
+static void bgp_stop(struct bgp_proto *p);
+static sock *bgp_setup_listen_sk(void);
 
 
+/**
+ * bgp_open - open a BGP instance
+ * @p: BGP instance
+ *
+ * This function allocates and configures shared BGP resources.
+ * Should be called as the last step during initialization
+ * (when lock is acquired and neighbor is ready).
+ * When error, state changed to PS_DOWN, -1 is returned and caller
+ * should return immediately.
+ */
+static int
+bgp_open(struct bgp_proto *p)
+{
+  bgp_counter++;
+
+  if (!bgp_listen_sk)
+    bgp_listen_sk = bgp_setup_listen_sk();
+
+  if (!bgp_linpool)
+    bgp_linpool = lp_new(&root_pool, 4080);
+
+  if (p->cf->password)
+    {
+      int rv = sk_set_md5_auth(bgp_listen_sk, p->cf->remote_ip, p->cf->password);
+      if (rv < 0)
+	{
+	  bgp_close(p, 0);
+	  p->p.disabled = 1;
+	  bgp_store_error(p, NULL, BE_MISC, BEM_INVALID_MD5);
+	  proto_notify_state(&p->p, PS_DOWN);
+	  return -1;
+	}
+    }
+
+  p->start_state = BSS_CONNECT;
+  return 0;
+}
+
+/**
+ * bgp_close - close a BGP instance
+ * @p: BGP instance
+ * @apply_md5: 0 to disable unsetting MD5 auth
+ *
+ * This function frees and deconfigures shared BGP resources.
+ * @apply_md5 is set to 0 when bgp_close is called as a cleanup
+ * from failed bgp_open().
+ */
 static void
-bgp_close(struct bgp_proto *p)
+bgp_close(struct bgp_proto *p, int apply_md5)
 {
   ASSERT(bgp_counter);
   bgp_counter--;
 
-  if (p->cf->password)
+  if (p->cf->password && apply_md5)
     sk_set_md5_auth(bgp_listen_sk, p->cf->remote_ip, NULL);
 
   if (!bgp_counter)
@@ -123,18 +172,11 @@ bgp_start_timer(timer *t, int value)
  *
  * This function takes a connection described by the &bgp_conn structure,
  * closes its socket and frees all resources associated with it.
- *
- * If the connection is being closed due to a protocol error, adjust
- * the connection restart timer as well according to the error recovery
- * policy set in the configuration.
- *
- * If the connection was marked as primary, it shuts down the protocol as well.
  */
 void
 bgp_close_conn(struct bgp_conn *conn)
 {
   struct bgp_proto *p = conn->bgp;
-  struct bgp_config *cf = p->cf;
 
   DBG("BGP: Closing connection\n");
   conn->packets_to_send = 0;
@@ -146,54 +188,184 @@ bgp_close_conn(struct bgp_conn *conn)
   conn->hold_timer = NULL;
   rfree(conn->sk);
   conn->sk = NULL;
-  conn->state = BS_IDLE;
-  if (conn->error_flag > 1)
+  rfree(conn->tx_ev);
+  conn->tx_ev = NULL;
+}
+
+
+/**
+ * bgp_update_startup_delay - update a startup delay
+ * @p: BGP instance
+ * @conn: related BGP connection
+ * @code: BGP error code
+ * @subcode: BGP error subcode
+ *
+ * This function updates a startup delay that is used to postpone next BGP connect.
+ * It also handles disable_after_error and might stop BGP instance when error
+ * happened and disable_after_error is on.
+ *
+ * It should be called when BGP protocol error happened.
+ */
+void
+bgp_update_startup_delay(struct bgp_proto *p, struct bgp_conn *conn, unsigned code, unsigned subcode)
+{
+  struct bgp_config *cf = p->cf;
+
+  /* Don't handle cease messages as errors */
+  if (code == 6 && !subcode)
     {
-      if (cf->disable_after_error)
-	p->p.disabled = 1;
-      if (p->last_connect && (bird_clock_t)(p->last_connect + cf->error_amnesia_time) < now)
-	p->startup_delay = 0;
-      if (!p->startup_delay)
-	p->startup_delay = cf->error_delay_time_min;
-      else
-	{
-	  p->startup_delay *= 2;
-	  if (p->startup_delay > cf->error_delay_time_max)
-	    p->startup_delay = cf->error_delay_time_max;
-	}
+      p->startup_delay = 0;
+      return;
+    }
+
+  /* During start, we only consider errors on outgoing connection, because
+     otherwise delay timer for outgoing connection is already running and
+     we could increase delay time two times (or more) per one attempt to
+     connect.
+  */
+  if ((p->p.proto_state == PS_START) && (conn != &p->outgoing_conn))
+    return;
+
+  DBG("BGP: Updating startup delay %d %d\n", code, subcode);
+
+  p->last_proto_error = now;
+
+  if (cf->disable_after_error)
+    {
+      p->startup_delay = 0;
+      p->p.disabled = 1;
+      if (p->p.proto_state == PS_START)
+	bgp_stop(p);
+
+      return;
     }
-  if (conn->primary)
+
+  if (p->last_proto_error && ((now - p->last_proto_error) >= cf->error_amnesia_time))
+    p->startup_delay = 0;
+
+  if (!p->startup_delay)
+    p->startup_delay = cf->error_delay_time_min;
+  else
     {
-      bgp_close(p);
-      p->conn = NULL;
-      proto_notify_state(&p->p, PS_DOWN);
+      p->startup_delay *= 2;
+      if (p->startup_delay > cf->error_delay_time_max)
+	p->startup_delay = cf->error_delay_time_max;
     }
-  else if (conn->error_flag > 1)
-    bgp_initiate(p);
 }
 
-static int
-bgp_graceful_close_conn(struct bgp_conn *c)
+static void
+bgp_graceful_close_conn(struct bgp_conn *conn)
 {
-  switch (c->state)
+  switch (conn->state)
     {
     case BS_IDLE:
-      return 0;
+    case BS_CLOSE:
+      return;
     case BS_CONNECT:
     case BS_ACTIVE:
-      bgp_close_conn(c);
-      return 1;
+      bgp_conn_enter_idle_state(conn);
+      return;
     case BS_OPENSENT:
     case BS_OPENCONFIRM:
     case BS_ESTABLISHED:
-      bgp_error(c, 6, 0, NULL, 0);
-      return 1;
+      bgp_error(conn, 6, 0, NULL, 0);
+      return;
     default:
-      bug("bgp_graceful_close_conn: Unknown state %d", c->state);
+      bug("bgp_graceful_close_conn: Unknown state %d", conn->state);
     }
 }
 
 static void
+bgp_down(struct bgp_proto *p)
+{
+  if (p->start_state > BSS_PREPARE)
+    bgp_close(p, 1);
+
+  DBG("BGP: DOWN\n");
+  proto_notify_state(&p->p, PS_DOWN);
+}
+
+static void
+bgp_decision(void *vp)
+{
+  struct bgp_proto *p = vp;
+
+  DBG("BGP: Decision start\n");
+  if ((p->p.proto_state == PS_START)
+      && (p->outgoing_conn.state == BS_IDLE))
+    bgp_initiate(p);
+
+  if ((p->p.proto_state == PS_STOP)
+      && (p->outgoing_conn.state == BS_IDLE)
+      && (p->incoming_conn.state == BS_IDLE))
+    bgp_down(p);
+}
+
+static void
+bgp_stop(struct bgp_proto *p)
+{
+  proto_notify_state(&p->p, PS_STOP);
+  bgp_graceful_close_conn(&p->outgoing_conn);
+  bgp_graceful_close_conn(&p->incoming_conn);
+  ev_schedule(p->event);
+}
+
+void
+bgp_conn_enter_established_state(struct bgp_conn *conn)
+{
+  struct bgp_proto *p = conn->bgp;
+ 
+  BGP_TRACE(D_EVENTS, "BGP session established");
+  DBG("BGP: UP!!!\n");
+
+  p->conn = conn;
+  p->last_error_class = 0;
+  p->last_error_code = 0;
+  bgp_attr_init(conn->bgp);
+  conn->state = BS_ESTABLISHED;
+  proto_notify_state(&p->p, PS_UP);
+}
+
+static void
+bgp_conn_leave_established_state(struct bgp_proto *p)
+{
+  BGP_TRACE(D_EVENTS, "BGP session closed");
+  p->conn = NULL;
+
+  if (p->p.proto_state == PS_UP)
+    bgp_stop(p);
+}
+
+void
+bgp_conn_enter_close_state(struct bgp_conn *conn)
+{
+  struct bgp_proto *p = conn->bgp;
+  int os = conn->state;
+
+  conn->state = BS_CLOSE;
+  tm_stop(conn->hold_timer);
+  tm_stop(conn->keepalive_timer);
+  conn->sk->rx_hook = NULL;
+
+  if (os == BS_ESTABLISHED)
+    bgp_conn_leave_established_state(p);
+}
+
+void
+bgp_conn_enter_idle_state(struct bgp_conn *conn)
+{
+  struct bgp_proto *p = conn->bgp;
+  int os = conn->state;
+
+  bgp_close_conn(conn);
+  conn->state = BS_IDLE;
+  ev_schedule(p->event);
+
+  if (os == BS_ESTABLISHED)
+    bgp_conn_leave_established_state(p);
+}
+
+static void
 bgp_send_open(struct bgp_conn *conn)
 {
   DBG("BGP: Sending open\n");
@@ -222,8 +394,13 @@ bgp_connect_timeout(timer *t)
   struct bgp_proto *p = conn->bgp;
 
   DBG("BGP: connect_timeout\n");
-  bgp_close_conn(conn);
-  bgp_connect(p);
+  if (p->p.proto_state == PS_START)
+    {
+      bgp_close_conn(conn);
+      bgp_connect(p);
+    }
+  else
+    bgp_conn_enter_idle_state(conn);
 }
 
 static void
@@ -232,26 +409,14 @@ bgp_sock_err(sock *sk, int err)
   struct bgp_conn *conn = sk->data;
   struct bgp_proto *p = conn->bgp;
 
+  bgp_store_error(p, conn, BE_SOCKET, err);
+
   if (err)
     BGP_TRACE(D_EVENTS, "Connection lost (%M)", err);
   else
     BGP_TRACE(D_EVENTS, "Connection closed");
-  switch (conn->state)
-    {
-    case BS_CONNECT:
-    case BS_OPENSENT:
-      rfree(conn->sk);
-      conn->sk = NULL;
-      conn->state = BS_ACTIVE;
-      bgp_start_timer(conn->connect_retry_timer, p->cf->connect_retry_time);
-      break;
-    case BS_OPENCONFIRM:
-    case BS_ESTABLISHED:
-      bgp_close_conn(conn);
-      break;
-    default:
-      bug("bgp_sock_err called in invalid state %d", conn->state);
-    }
+
+  bgp_conn_enter_idle_state(conn);
 }
 
 static void
@@ -280,8 +445,6 @@ bgp_setup_conn(struct bgp_proto *p, struct bgp_conn *conn)
   conn->sk = NULL;
   conn->bgp = p;
   conn->packets_to_send = 0;
-  conn->error_flag = 0;
-  conn->primary = 0;
 
   t = conn->connect_retry_timer = tm_new(p->p.pool);
   t->hook = bgp_connect_timeout;
@@ -292,6 +455,9 @@ bgp_setup_conn(struct bgp_proto *p, struct bgp_conn *conn)
   t = conn->keepalive_timer = tm_new(p->p.pool);
   t->hook = bgp_keepalive_timeout;
   t->data = conn;
+  conn->tx_ev = ev_new(p->p.pool);
+  conn->tx_ev->hook = bgp_kick_tx;
+  conn->tx_ev->data = conn;
 }
 
 static void
@@ -302,6 +468,17 @@ bgp_setup_sk(struct bgp_proto *p, struct bgp_conn *conn, sock *s)
   conn->sk = s;
 }
 
+static void
+bgp_active(struct bgp_proto *p, int delay)
+{
+  struct bgp_conn *conn = &p->outgoing_conn;
+
+  BGP_TRACE(D_EVENTS, "Connect delayed by %d seconds", delay);
+  bgp_setup_conn(p, conn);
+  conn->state = BS_ACTIVE;
+  bgp_start_timer(conn->connect_retry_timer, delay);
+}
+
 /**
  * bgp_connect - initiate an outgoing connection
  * @p: BGP instance
@@ -317,7 +494,6 @@ bgp_connect(struct bgp_proto *p)	/* Enter Connect state and start establishing c
   struct bgp_conn *conn = &p->outgoing_conn;
 
   DBG("BGP: Connecting\n");
-  p->last_connect = now;
   s = sk_new(p->p.pool);
   s->type = SK_TCP_ACTIVE;
   if (ipa_nonzero(p->cf->source_addr))
@@ -348,17 +524,10 @@ bgp_connect(struct bgp_proto *p)	/* Enter Connect state and start establishing c
 static void
 bgp_initiate(struct bgp_proto *p)
 {
-  unsigned delay;
+  unsigned delay = MAX(p->startup_delay, p->cf->start_delay_time);
 
-  delay = p->cf->start_delay_time;
-  if (p->startup_delay > delay)
-    delay = p->startup_delay;
   if (delay)
-    {
-      BGP_TRACE(D_EVENTS, "Connect delayed by %d seconds", delay);
-      bgp_setup_conn(p, &p->outgoing_conn);
-      bgp_start_timer(p->outgoing_conn.connect_retry_timer, delay);
-    }
+    bgp_active(p, delay);
   else
     bgp_connect(p);
 }
@@ -389,7 +558,7 @@ bgp_incoming_connection(sock *sk, int dummy UNUSED)
 	if (ipa_equal(p->cf->remote_ip, sk->daddr))
 	  {
 	    match = 1;
-	    if ((p->p.proto_state == PS_START || p->p.proto_state == PS_UP) && p->neigh && p->neigh->iface)
+	    if ((p->p.proto_state == PS_START || p->p.proto_state == PS_UP) && (p->start_state > BSS_PREPARE))
 	      {
 		BGP_TRACE(D_EVENTS, "Incoming connection from %I port %d", sk->daddr, sk->dport);
 		if (p->incoming_conn.sk)
@@ -411,27 +580,25 @@ bgp_incoming_connection(sock *sk, int dummy UNUSED)
   return 0;
 }
 
-static void
+static sock *
 bgp_setup_listen_sk(void)
 {
-  if (!bgp_listen_sk)
+  sock *s = sk_new(&root_pool);
+  DBG("BGP: Creating incoming socket\n");
+  s->type = SK_TCP_PASSIVE;
+  s->sport = BGP_PORT;
+  s->tos = IP_PREC_INTERNET_CONTROL;
+  s->rbsize = BGP_RX_BUFFER_SIZE;
+  s->tbsize = BGP_TX_BUFFER_SIZE;
+  s->rx_hook = bgp_incoming_connection;
+  if (sk_open(s))
     {
-      sock *s = sk_new(&root_pool);
-      DBG("BGP: Creating incoming socket\n");
-      s->type = SK_TCP_PASSIVE;
-      s->sport = BGP_PORT;
-      s->tos = IP_PREC_INTERNET_CONTROL;
-      s->rbsize = BGP_RX_BUFFER_SIZE;
-      s->tbsize = BGP_TX_BUFFER_SIZE;
-      s->rx_hook = bgp_incoming_connection;
-      if (sk_open(s))
-	{
-	  log(L_ERR "Unable to open incoming BGP socket");
-	  rfree(s);
-	}
-      else
-	bgp_listen_sk = s;
+      log(L_ERR "Unable to open incoming BGP socket");
+      rfree(s);
+      return NULL;
     }
+  else
+    return s;
 }
 
 static void
@@ -452,6 +619,11 @@ bgp_start_neighbor(struct bgp_proto *p)
     DBG("BGP: Selected link-level address %I\n", p->local_link);
   }
 #endif
+
+  int rv = bgp_open(p);
+  if (rv < 0)
+    return;
+
   bgp_initiate(p);
 }
 
@@ -462,16 +634,20 @@ bgp_neigh_notify(neighbor *n)
 
   if (n->iface)
     {
-      BGP_TRACE(D_EVENTS, "Neighbor found");
-      bgp_start_neighbor(p);
+      if ((p->p.proto_state == PS_START) && (p->start_state == BSS_PREPARE))
+	{
+	  BGP_TRACE(D_EVENTS, "Neighbor found");
+	  bgp_start_neighbor(p);
+	}
     }
   else
     {
-      BGP_TRACE(D_EVENTS, "Neighbor lost");
-      /* Send cease packets, but don't wait for them to be delivered */
-      bgp_graceful_close_conn(&p->outgoing_conn);
-      bgp_graceful_close_conn(&p->incoming_conn);
-      proto_notify_state(&p->p, PS_DOWN);
+      if ((p->p.proto_state == PS_START) || (p->p.proto_state == PS_UP))
+	{
+	  BGP_TRACE(D_EVENTS, "Neighbor lost");
+	  bgp_store_error(p, NULL, BE_MISC, BEM_NEIGHBOR_LOST);
+	  bgp_stop(p);
+	}
     }
 }
 
@@ -481,6 +657,12 @@ bgp_start_locked(struct object_lock *lock)
   struct bgp_proto *p = lock->data;
   struct bgp_config *cf = p->cf;
 
+  if (p->p.proto_state != PS_START)
+    {
+      DBG("BGP: Got lock in different state %d\n", p->p.proto_state);
+    return;
+    }
+
   DBG("BGP: Got lock\n");
   p->local_id = cf->c.global->router_id;
   p->next_hop = cf->multihop ? cf->multihop_via : cf->remote_ip;
@@ -497,10 +679,14 @@ bgp_start_locked(struct object_lock *lock)
   if (!p->neigh)
     {
       log(L_ERR "%s: Invalid next hop %I", p->p.name, p->next_hop);
+      /* As we do not start yet, we can just disable protocol */
       p->p.disabled = 1;
+      bgp_store_error(p, NULL, BE_MISC, BEM_INVALID_NEXT_HOP);
       proto_notify_state(&p->p, PS_DOWN);
+      return;
     }
-  else if (p->neigh->iface)
+  
+  if (p->neigh->iface)
     bgp_start_neighbor(p);
   else
     BGP_TRACE(D_EVENTS, "Waiting for %I to become my neighbor", p->next_hop);
@@ -513,16 +699,14 @@ bgp_start(struct proto *P)
   struct object_lock *lock;
 
   DBG("BGP: Startup.\n");
+  p->start_state = BSS_PREPARE;
   p->outgoing_conn.state = BS_IDLE;
   p->incoming_conn.state = BS_IDLE;
-  p->startup_delay = 0;
   p->neigh = NULL;
 
-  bgp_counter++;
-  bgp_setup_listen_sk();
-
-  if (!bgp_linpool)
-    bgp_linpool = lp_new(&root_pool, 4080);
+  p->event = ev_new(p->p.pool);
+  p->event->hook = bgp_decision;
+  p->event->data = p;
 
   /*
    *  Before attempting to create the connection, we need to lock the
@@ -539,16 +723,6 @@ bgp_start(struct proto *P)
   lock->data = p;
   olock_acquire(lock);
 
-  /* We should create security association after we get a lock not to 
-   * break existing connections.
-   */
-  if (p->cf->password)
-    {
-      int rv = sk_set_md5_auth(bgp_listen_sk, p->cf->remote_ip, p->cf->password);
-      if (rv < 0)
-	return PS_STOP;
-    }
-
   return PS_START;
 }
 
@@ -558,31 +732,11 @@ bgp_shutdown(struct proto *P)
   struct bgp_proto *p = (struct bgp_proto *) P;
 
   BGP_TRACE(D_EVENTS, "Shutdown requested");
+  bgp_store_error(p, NULL, BE_MAN_DOWN, 0);
+  p->startup_delay = 0;
+  bgp_stop(p);
 
-  /*
-   *  We want to send the Cease notification message to all connections
-   *  we have open, but we don't want to wait for all of them to complete.
-   *  We are willing to handle the primary connection carefully, but for
-   *  the others we just try to send the packet and if there is no buffer
-   *  space free, we'll gracefully finish.
-   */
-
-  proto_notify_state(&p->p, PS_STOP);
-  if (!p->conn)
-    {
-      if (p->outgoing_conn.state != BS_IDLE)
-	p->outgoing_conn.primary = 1;	/* Shuts protocol down after connection close */
-      else if (p->incoming_conn.state != BS_IDLE)
-	p->incoming_conn.primary = 1;
-    }
-  if (bgp_graceful_close_conn(&p->outgoing_conn) || bgp_graceful_close_conn(&p->incoming_conn))
-    return p->p.proto_state;
-  else
-    {
-      /* No connections open, shutdown automatically */
-      bgp_close(p);
-      return PS_DOWN;
-    }
+  return p->p.proto_state;
 }
 
 static struct proto *
@@ -618,19 +772,48 @@ bgp_init(struct proto_config *C)
 void
 bgp_error(struct bgp_conn *c, unsigned code, unsigned subcode, byte *data, int len)
 {
-  if (c->error_flag)
+  if (c->state == BS_CLOSE)
     return;
+
   bgp_log_error(c->bgp, "Error", code, subcode, data, (len > 0) ? len : -len);
-  c->error_flag = 1 + (code != 6);
+  bgp_store_error(c->bgp, c, BE_BGP_TX, (code << 16) | subcode);
+  bgp_update_startup_delay(c->bgp, c, code, subcode);
+  bgp_conn_enter_close_state(c);
+
   c->notify_code = code;
   c->notify_subcode = subcode;
   c->notify_data = data;
   c->notify_size = (len > 0) ? len : 0;
-  if (c->primary)
-    proto_notify_state(&c->bgp->p, PS_STOP);
   bgp_schedule_packet(c, PKT_NOTIFICATION);
 }
 
+/**
+ * bgp_store_error - store last error for status report
+ * @p: BGP instance
+ * @c: connection
+ * @class: error class (BE_xxx constants)
+ * @code: error code (class specific)
+ *
+ * bgp_store_error() decides whether given error is interesting enough
+ * and store that error to last_error variables of @p
+ */
+void
+bgp_store_error(struct bgp_proto *p, struct bgp_conn *c, u8 class, u32 code)
+{
+  /* During PS_UP, we ignore errors on secondary connection */
+  if ((p->p.proto_state == PS_UP) && c && (c != p->conn))
+    return;
+
+  /* During PS_STOP, we ignore any errors, as we want to report
+   * the error that caused transition to PS_STOP
+   */
+  if (p->p.proto_state == PS_STOP)
+    return;
+
+  p->last_error_class = class;
+  p->last_error_code = code;
+}
+
 void
 bgp_check(struct bgp_config *c)
 {
@@ -639,7 +822,7 @@ bgp_check(struct bgp_config *c)
   if (!c->remote_as)
     cf_error("Neighbor must be configured");
   if (!bgp_as4_support && c->enable_as4)
-    cf_error("AS4 support disabled globbaly");
+    cf_error("AS4 support disabled globally");
   if (!c->enable_as4 && (c->local_as > 0xFFFF))
     cf_error("Local AS number out of range");
   if (!c->enable_as4 && (c->remote_as > 0xFFFF))
@@ -650,15 +833,40 @@ bgp_check(struct bgp_config *c)
     cf_error("Only external neighbor can be RS client");
 }
 
+static char *bgp_state_names[] = { "Idle", "Connect", "Active", "OpenSent", "OpenConfirm", "Established", "Close" };
+static char *bgp_err_classes[] = { "", "Error: ", "Socket: ", "Received: ", "BGP Error: ", "Automatic shutdown", ""};
+static char *bgp_misc_errors[] = { "", "Neighbor lost", "Invalid next hop", "Kernel MD5 auth failed" };
+
+
 static void
 bgp_get_status(struct proto *P, byte *buf)
 {
   struct bgp_proto *p = (struct bgp_proto *) P;
 
+  const byte *err1 = bgp_err_classes[p->last_error_class];
+  const byte *err2 = "";
+  byte errbuf[32];
+
+  switch (p->last_error_class)
+    {
+    case BE_MISC:
+      err2 = bgp_misc_errors[p->last_error_code];
+      break;
+    case BE_SOCKET:
+      err2 = (p->last_error_code == 0) ? "Connection closed" : strerror(p->last_error_code);
+      break;
+    case BE_BGP_RX:
+    case BE_BGP_TX:
+      err2 = bgp_error_dsc(errbuf, p->last_error_code >> 16, p->last_error_code & 0xFF);
+      break;
+    }
+
   if (P->proto_state == PS_DOWN)
-    buf[0] = 0;
+    bsprintf(buf, "%s%s", err1, err2);
   else
-    strcpy(buf, bgp_state_names[MAX(p->incoming_conn.state, p->outgoing_conn.state)]);
+    bsprintf(buf, "%-14s%s%s",
+	     bgp_state_names[MAX(p->incoming_conn.state, p->outgoing_conn.state)],
+	     err1, err2);
 }
 
 static int