Fix problems with handling of oversize BGP messagesex24b

The objective is to (as gracefully as possible) deal with outgoing UPDATE
messages which simply do not in the maximum size BGP message.  This can
happen if very large AS_PATH or Community attributes come in, and are
then extended.  (Sending an UPDATE to an AS2 speaking peer can create
a complete second copy of the AS_PATH !)  Previous versions of Quagga
would crash under these circumstances.

Recent version were changed to tolerate and detect oversize messages.
Any oversize message is logged as an error and discarded.

This version will also withdraw prefixes if the selected route simply
cannot be advertised.

This version also corrects the handling of messages which are limited to
the maximum BGP message size -- in particular UPDATE messages which
carry a large number of IPv4/Unicast prefixes.  (This was broken by the
earlier efforts to handle oversize messages.)

Changes in this commit:

  * update version to 0.99.20ex24b

  * where a set of attributes is too big to fit into a BGP message, or
    so big that not even one prefix will also fit into a BGP message,
    the following steps are now taken:

      - the BGP message is discarded and an error logged (as in other
        recent versions -- Quagga does not crash).

      - any prefixes which should have been advertised with the (broken)
        attributes will be withdrawn, if they have been advertised earlier
        with valid attributes.

        A further error is logged, listing the affected prefixes.

      - prefixes which are suppressed in this way are not counted as
        having been sent.

    At present there is no "show" command which will show which prefixes have
    been suppress -- TBA.

  * for all AFI/SAFI announce as many withdrawn prefixes as will fit in a
    BGP message.

    Previously, all AFI/SAFI other than IPv4/Unicast would send one withdrawn
    prefix per message.

    (This is still the case for announcements.  It seems unlikely that
     many IPv6 prefixes will have the same attributes... so this does not
     seem worth fixing immediately.)

  * ensure that NOTIFICATION message cannot exceed the maximum length of a
    BGP message, no matter how much data is sent (!).

  * where IPv4/Unicast prefixes share the same attributes, they are now
    announced in the order received.

    Previously, when prefixes were added to the list hung off the
    'struct bgp_advertise_attr', they were in LIFO order.

  * add SAFI name table.

  * log pthread attributes when creating a pthread.

  * fix scheduling of withdrawn prefixes to reduce number of BGP messages
    generated.

Bug fixes:

  * fix discard of invalid IPv6 link-local nexthop (recent bug).

  * where amount of information that is put into a BGP message is limited by
    the maximum message size, fix so that this works again.  (This was
    broken recently.)

  * set "Bottom of Stack" bit in MPLS VPN "tag" in MP_REACH and MP_UNREACH
    outgoing attributes.

17 files changed, 695 insertions, 175 deletions

diff --git a/bgpd/bgp_advertise.c b/bgpd/bgp_advertise.c
index 7246bab1..37fad456 100644
--- a/bgpd/bgp_advertise.c
+++ b/bgpd/bgp_advertise.c
@@ -101,24 +101,40 @@ bgp_advertise_free (struct bgp_advertise *adv)
 
 static void
 bgp_advertise_add (struct bgp_advertise_attr *baa,
-		   struct bgp_advertise *adv)
+		   bgp_advertise adv)
 {
-  adv->adv_next = baa->adv;
-  if (baa->adv)
-    baa->adv->adv_prev = adv;
-  baa->adv = adv;
+  bgp_advertise last ;
+
+  if (baa->base.head == NULL)
+    {
+      last = NULL ;
+      baa->base.head = adv ;
+    }
+  else
+    {
+      last = baa->base.tail ;
+      last->adv_next = adv ;
+    } ;
+
+  adv->adv_next  = NULL ;
+  adv->adv_prev  = last ;
+
+  baa->base.tail = adv ;
 }
 
 static void
 bgp_advertise_delete (struct bgp_advertise_attr *baa,
 		      struct bgp_advertise *adv)
 {
-  if (adv->adv_next)
+  if (adv->adv_next != NULL)
     adv->adv_next->adv_prev = adv->adv_prev;
-  if (adv->adv_prev)
+  else
+    baa->base.tail = adv->adv_prev ;
+
+  if (adv->adv_prev != NULL)
     adv->adv_prev->adv_next = adv->adv_next;
   else
-    baa->adv = adv->adv_next;
+    baa->base.head = adv->adv_next;
 }
 
 static struct bgp_advertise_attr *
@@ -191,7 +207,7 @@ bgp_advertise_clean (struct peer *peer, struct bgp_adj_out *adj,
       bgp_advertise_delete (baa, adv);
 
       /* Fetch next advertise candidate. */
-      next = baa->adv;
+      next = baa->base.head ;
 
       /* Unintern BGP advertise attribute.  */
       bgp_advertise_unintern (peer->hash[afi][safi], baa);
@@ -296,10 +312,10 @@ bgp_adj_out_unset (struct bgp_node *rn, struct peer *peer, struct prefix *p,
   assert(rn == adj->rn) ;
 
   /* Clear up previous advertisement.                           */
-  if (adj->adv)
+  if (adj->adv != NULL)
     bgp_advertise_clean (peer, adj, afi, safi);
 
-  if (adj->attr)
+  if (adj->attr != NULL)
     {
       /* We need advertisement structure.                       */
       adj->adv = bgp_advertise_new ();
@@ -310,8 +326,9 @@ bgp_adj_out_unset (struct bgp_node *rn, struct peer *peer, struct prefix *p,
       /* Add to synchronization entry for withdraw announcement */
       bgp_advertise_fifo_add(&peer->sync[afi][safi]->withdraw, adv);
 
-      /* Schedule packet write. */
-      bgp_write(peer, NULL) ;
+      /* Schedule flush of withdraws
+       */
+      bgp_withdraw_schedule(peer) ;
     }
   else
     bgp_adj_out_remove(rn, adj, peer, afi, safi) ;
diff --git a/bgpd/bgp_advertise.h b/bgpd/bgp_advertise.h
index afa812f6..6e974594 100644
--- a/bgpd/bgp_advertise.h
+++ b/bgpd/bgp_advertise.h
@@ -45,7 +45,7 @@ struct bgp_advertise_fifo_base
 struct bgp_advertise_attr
 {
   /* Head of advertisement pointer. */
-  struct bgp_advertise *adv;
+  struct bgp_advertise_fifo_base base ;
 
   /* Reference counter.  */
   unsigned long refcnt;
diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
index 6b7fedd5..5227a1a6 100644
--- a/bgpd/bgp_aspath.c
+++ b/bgpd/bgp_aspath.c
@@ -333,8 +333,7 @@ aspath_unintern (struct aspath **aspath)
   struct aspath *ret;
   struct aspath *asp = *aspath;
 
-  if (asp->refcnt)
-    asp->refcnt--;
+  asp->refcnt--;
 
   if (asp->refcnt == 0)
     {
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index c6c77f88..0cbf6b94 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -755,6 +755,8 @@ bgp_attr_unintern_sub (struct attr *attr, bool free_extra)
  *
  * Can do this to an attribute object which has not been interned, because its
  * reference count SHOULD be zero.
+ *
+ * Sets *attr = NULL iff the attributes are discarded.
  */
 void
 bgp_attr_unintern (struct attr **attr)
@@ -1664,7 +1666,7 @@ bgp_mp_reach_parse (bgp_attr_parser_args args)
                        siptoa(AF_INET6, &attre->mp_nexthop_global).str,
                        siptoa(AF_INET6, &attre->mp_nexthop_local).str) ;
 
-	  attre->mp_nexthop_len = 16;
+	  nexthop_len = 16;     /* discard mp_nexthop_local     */
 	}
       break;
 #endif /* HAVE_IPV6 */
@@ -2418,7 +2420,6 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer,
    * - send an AS_PATH out, but put 16Bit ASnums in it, not 32bit, and change
    *   all ASnums > 65535 to BGP_AS_TRANS
    */
-
   stream_putc (s, BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_EXTLEN);
   stream_putc (s, BGP_ATTR_AS_PATH);
   aspath_sizep = stream_get_endp (s);
@@ -2635,6 +2636,7 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer,
   if (p->family == AF_INET && safi == SAFI_MPLS_VPN)
     {
       unsigned long sizep;
+      uint tp ;
 
       stream_putc (s, BGP_ATTR_FLAG_OPTIONAL);
       stream_putc (s, BGP_ATTR_MP_REACH_NLRI);
@@ -2653,7 +2655,11 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer,
 
       /* Tag, RD, Prefix write. */
       stream_putc (s, p->prefixlen + 88);
-      stream_put (s, tag, 3);
+      stream_put (s, tag, 3);           /* zeros if tag == NULL */
+
+      tp = stream_get_endp (s) - 1 ;
+      stream_putc_at(s, tp, stream_getc_from(s, tp) | 1) ;
+
       stream_put (s, prd->val, 8);
       stream_put (s, &p->u.prefix, PSIZE (p->prefixlen));
 
@@ -2785,9 +2791,8 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer,
 }
 
 bgp_size_t
-bgp_packet_withdraw (struct peer *peer, struct stream *s, struct prefix *p,
-		     afi_t afi, safi_t safi, struct prefix_rd *prd,
-		     u_char *tag)
+bgp_packet_withdraw (struct stream *s, struct prefix *p,
+		     afi_t afi, safi_t safi, struct prefix_rd *prd)
 {
   unsigned long cp;
   unsigned long attrlen_pnt;
@@ -2801,25 +2806,16 @@ bgp_packet_withdraw (struct peer *peer, struct stream *s, struct prefix *p,
   attrlen_pnt = stream_get_endp (s);
   stream_putc (s, 0);		/* Length of this attribute. */
 
-  stream_putw (s, family2afi (p->family));
+  stream_putw (s, afi);
 
   if (safi == SAFI_MPLS_VPN)
     {
-      /* SAFI */
       stream_putc (s, SAFI_MPLS_LABELED_VPN);
-
-      /* prefix. */
-      stream_putc (s, p->prefixlen + 88);
-      stream_put (s, tag, 3);
-      stream_put (s, prd->val, 8);
-      stream_put (s, &p->u.prefix, PSIZE (p->prefixlen));
+      bgp_packet_withdraw_vpn_prefix (s, p, prd) ;
     }
   else
     {
-      /* SAFI */
       stream_putc (s, safi);
-
-      /* prefix */
       stream_put_prefix (s, p);
     }
 
@@ -2830,6 +2826,16 @@ bgp_packet_withdraw (struct peer *peer, struct stream *s, struct prefix *p,
   return stream_get_endp (s) - cp;
 }
 
+void
+bgp_packet_withdraw_vpn_prefix (struct stream *s, struct prefix *p,
+                                                          struct prefix_rd *prd)
+{
+  stream_putc (s, p->prefixlen + (( 3 + 8) * 8));
+  stream_put (s, "\x00\x00\x01", 3);
+  stream_put (s, prd->val, 8);
+  stream_put (s, &p->u.prefix, PSIZE (p->prefixlen));
+}
+
 /* Initialization of attribute. */
 void
 bgp_attr_init (void)
diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
index 553a91b5..748a5dc7 100644
--- a/bgpd/bgp_attr.h
+++ b/bgpd/bgp_attr.h
@@ -193,9 +193,11 @@ extern bgp_size_t bgp_packet_attribute (struct bgp *bgp, struct peer *,
                                  struct stream *, struct attr *,
                                  struct prefix *, afi_t, safi_t,
                                  struct peer *, struct prefix_rd *, u_char *);
-extern bgp_size_t bgp_packet_withdraw (struct peer *peer, struct stream *s,
-                                struct prefix *p, afi_t, safi_t,
-                                struct prefix_rd *, u_char *);
+extern bgp_size_t bgp_packet_withdraw (struct stream *s, struct prefix *p,
+                                          afi_t, safi_t, struct prefix_rd *);
+extern void bgp_packet_withdraw_vpn_prefix(struct stream *s, struct prefix *p,
+                                                        struct prefix_rd *prd) ;
+
 extern void bgp_dump_routes_attr (struct stream *, struct attr *,
 				  struct prefix *);
 extern int attrhash_cmp (const void *, const void *);
diff --git a/bgpd/bgp_msg_write.c b/bgpd/bgp_msg_write.c
index 3020ef7e..3e1ba205 100644
--- a/bgpd/bgp_msg_write.c
+++ b/bgpd/bgp_msg_write.c
@@ -81,6 +81,8 @@
  *     fail !
  *
  * NB: requires the session LOCKED -- connection-wise
+ *
+ * NB: clamps the length of the data to fit in the maximum size BGP message.
  */
 extern int
 bgp_msg_write_notification(bgp_connection connection, bgp_notify notification)
@@ -109,8 +111,14 @@ bgp_msg_write_notification(bgp_connection connection, bgp_notify notification)
   if (length != 0)
     stream_put(s, bgp_notify_get_data(notification), length) ;
 
-  /* Set BGP packet length.
+  /* Set BGP packet length -- clamping to maximum (if required)
    */
+  if (bgp_packet_check_size(s, connection->su_remote) == 0)
+    {
+      stream_set_endp(s, BGP_MSG_MAX_L) ;
+      stream_clear_overflow(s) ;
+    } ;
+
   bgp_packet_set_size(s);
 
   /* Set flag so that write_action raises required event when buffer becomes
@@ -151,7 +159,8 @@ bgp_msg_send_keepalive(bgp_connection connection, bool must_send)
 
   ++connection->session->stats.keepalive_out ;
 
-  /* Make KEEPALIVE message -- comprises header only    */
+  /* Make KEEPALIVE message -- comprises header only, so guaranteed to fit !
+   */
   bgp_packet_set_marker(s, BGP_MSG_KEEPALIVE);
   length = bgp_packet_set_size(s);
 
@@ -218,6 +227,9 @@ bgp_msg_send_open(bgp_connection connection, bgp_open_state open_state)
   bgp_open_options(s, open_state, connection->cap_suppress) ;
 
   /* Set BGP message length.
+   *
+   * Cannot overflow the BGP Message size, and if it did, there is damn all
+   * we could do about it !
    */
   length = bgp_packet_set_size(s) ;
 
@@ -517,11 +529,14 @@ bgp_msg_send_route_refresh(bgp_connection connection, bgp_route_refresh rr)
       stream_putc(s, 0);
       stream_putc(s, rr->safi);
 
-      /* Process as many (remaining) ORF entries as can into message    */
+      /* Process as many (remaining) ORF entries as can into message
+       */
       if (!done)
         done = bgp_msg_orf_part(s, connection, rr) ;
 
-      /* Set BGP message length & dispatch.                             */
+      /* Set BGP message length & dispatch -- noting that orf entry
+       * construction ensures that the length does not exceed the maximum.
+       */
       length = bgp_packet_set_size(s) ;
 
       if (BGP_DEBUG (normal, NORMAL))
@@ -831,6 +846,8 @@ bgp_msg_send_end_of_rib(bgp_connection connection, iAFI_t afi, iSAFI_t safi)
        stream_putw_at(s, attrp-2, stream_get_endp(s) - attrp) ;
     }
 
+  /* Cannot overflow BGP Message
+   */
   bgp_packet_set_size(s);
 
   if (BGP_DEBUG (normal, NORMAL))
diff --git a/bgpd/bgp_names.c b/bgpd/bgp_names.c
index 89623246..b22b7d11 100644
--- a/bgpd/bgp_names.c
+++ b/bgpd/bgp_names.c
@@ -260,3 +260,16 @@ const char* bgp_afi_name_map_body[] =
 const map_direct_t bgp_afi_name_map =
       map_direct_s(bgp_afi_name_map_body, "unknown AFI(%u)") ;
 
+/*------------------------------------------------------------------------------
+ * SAFI names
+ */
+const char* bgp_safi_name_map_body[] =
+{
+  [SAFI_UNICAST]              = "SAFI_UNICAST",
+  [SAFI_MULTICAST]            = "SAFI_MULTICAST",
+  [SAFI_MPLS_VPN]             = "SAFI_MPLS_VPN",
+};
+
+const map_direct_t bgp_safi_name_map =
+      map_direct_s(bgp_safi_name_map_body, "unknown SAFI(%u)") ;
+
diff --git a/bgpd/bgp_names.h b/bgpd/bgp_names.h
index ab688575..e72cfaa7 100644
--- a/bgpd/bgp_names.h
+++ b/bgpd/bgp_names.h
@@ -43,5 +43,6 @@ extern const map_direct_t bgp_origin_short_map ;
 extern const map_direct_t bgp_origin_long_map ;
 extern const map_direct_t bgp_attr_name_map ;
 extern const map_direct_t bgp_afi_name_map ;
+extern const map_direct_t bgp_safi_name_map ;
 
 #endif /* _QUAGGA_BGP_NAMES_H */
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 74eed451..88254e5a 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -27,7 +27,6 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
 #include "command.h"
 #include "log.h"
 #include "memory.h"
-#include "sockunion.h"		/* for inet_ntop () */
 #include "linklist.h"
 
 #include "bgpd/bgpd.h"
@@ -53,6 +52,312 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
 #include "bgpd/bgp_names.h"
 #include "bgpd/bgp_msg_write.h"
 
+/* Prototypes
+ */
+static bgp_advertise bgp_updated(bgp_peer peer, bgp_advertise adv,
+                                     afi_t afi, safi_t safi, qstring updates,
+                                                              bool suppressed) ;
+
+/*------------------------------------------------------------------------------
+ * Construct an update from the given bgp_advertise object.
+ *
+ * Generates complete BGP message in the peer->work stream structure.
+ *
+ * Returns: peer->work -- if have something to be written.
+ *          NULL       -- otherwise
+ *
+ * NB: if the attributes overflow the BGP message, suppresses the update and
+ *     issues a withdraw for the affected prefixes if required.
+ */
+static struct stream *
+bgp_update_packet (bgp_peer peer, bgp_advertise adv, afi_t afi, safi_t safi)
+{
+  struct stream *s;
+  struct bgp_node *rn ;
+  struct bgp_info *binfo ;
+  ulen   attr_lp ;
+  ulen   attr_len ;
+  struct prefix_rd *prd ;
+  u_char *tag ;
+  struct peer *from ;
+  qstring updates ;
+  uint    count ;
+  bool    ipv4_unicast ;
+
+  qassert(adv != NULL) ;
+
+  ipv4_unicast = (afi == AFI_IP) && (safi == SAFI_UNICAST) ;
+
+  s = peer->work;
+  stream_reset (s);
+
+  if (BGP_DEBUG (update, UPDATE_OUT))
+    updates = qs_new(100) ;
+  else
+    updates = NULL ;
+
+  count = 0 ;
+
+  /* Generate the attributes part of the message.
+   *
+   * If is not AFI_IP/SAFI_UNICAST, includes the first prefix on the list.
+   *
+   * NB: this sends only one prefix per message if is not AFI_IP/SAFI_UNICAST.
+   */
+  prd = NULL;
+  rn  = adv->rn ;
+  assert(rn != NULL) ;
+  if (rn->prn != NULL)
+    prd = (struct prefix_rd *) &rn->prn->p ;
+
+  tag   = NULL;
+  from  = NULL;
+  binfo = adv->binfo ;
+  if (binfo != NULL)
+    {
+      from = binfo->peer;
+      if (binfo->extra)
+        tag = binfo->extra->tag;
+    } ;
+
+  bgp_packet_set_marker (s, BGP_MSG_UPDATE);
+  stream_putw (s, 0);   /* No AFI_IP/SAFI_UNICAST withdrawn     */
+
+  attr_lp = stream_get_endp (s);
+  qassert(attr_lp == (BGP_MH_HEAD_L + 2)) ;
+
+  stream_putw (s, 0);   /* Attributes length                    */
+
+  attr_len = bgp_packet_attribute (NULL, peer, s,
+                                         adv->baa->attr,
+                                         &rn->p, afi, safi,
+                                         from, prd, tag);
+  stream_putw_at (s, attr_lp, attr_len) ;
+
+  /* For AFI_IP/SAFI_UNICAST, append the first prefix.
+   *
+   * Once we have done this, all AFI/SAFI are in the same state, we have
+   * the attributes and one prefix in the message.
+   */
+  if (ipv4_unicast)
+    stream_put_prefix (s, &rn->p);
+
+  /* If the attributes with at least one prefix have fitted, then all is well,
+   * and for AFI_IP/SAFI_UNICAST we can tack on other prefixes which share the
+   * current attributes.
+   *
+   * Otherwise, we have a problem, and we issue a route withdraw, instead.
+   *
+   * NB: we allocate BGP_STREAM_SIZE, which is larger than BGP_MSG_MAX_L,
+   *     so that if the overflow is marginal, we can tell what it was.
+   */
+  if (bgp_packet_check_size(s, peer->su_remote) > 0)
+    {
+      /* Eat the prefix we have already included in the message.
+       *
+       * Then for AFI_IP/SAFI_UNICAST, eat as many further prefixes as we can
+       * fit into the message.
+       */
+      if (ipv4_unicast)
+        {
+          qassert(!stream_has_overflowed(s)) ;
+
+          while (1)
+            {
+              ulen   len_was ;
+
+              adv = bgp_updated(peer, adv, afi, safi, updates,
+                                                   false /* not suppressed */) ;
+              ++count ;
+
+              if (adv == NULL)
+                break ;
+
+              rn = adv->rn ;
+              assert(rn != NULL);
+
+              len_was = stream_get_len(s) ;
+              stream_put_prefix (s, &rn->p) ;
+
+              if (stream_has_written_beyond(s, BGP_MSG_MAX_L))
+                {
+                  stream_set_endp(s, len_was) ;
+                  stream_clear_overflow(s) ;
+                  break ;
+                } ;
+            } ;
+        }
+      else
+        {
+          bgp_updated(peer, adv, afi, safi, updates,
+                                                   false /* not suppressed */) ;
+          ++count ;
+        } ;
+
+      /* Report the update if required.
+       */
+      if (updates != NULL)
+        {
+          zlog (peer->log, LOG_DEBUG, "%s send %u UPDATE(S) %s/%s:%s",
+                                  peer->host, count,
+                                  map_direct(bgp_afi_name_map, afi).str,
+                                  map_direct(bgp_safi_name_map, safi).str,
+                                                           qs_string(updates)) ;
+          qs_free(updates) ;
+        } ;
+    }
+  else
+    {
+      /* Turn advertisement into withdraw of prefixes for which we are
+       * completely unable to generate an update message.
+       *
+       * NB: the result looks as though the prefixes *have* been advertised.
+       *
+       *     This avoids trying to send the same set of attributes again...
+       *
+       *     ...but is not a complete solution, yet.   TODO
+       */
+      uint withdrawn = 0 ;
+
+      if (updates == NULL)
+        updates = qs_new(100) ;
+
+      stream_set_endp(s, attr_lp) ;     /* as you was   */
+      stream_clear_overflow(s) ;
+
+      qassert(attr_lp == (BGP_MH_HEAD_L + 2)) ;
+
+      if (ipv4_unicast)
+        {
+          /* Fill in withdrawn AFI_IP/SAFI_UNICAST
+           *
+           * We are guaranteed to be able to fit at least one withdraw !
+           * Cope with running out of room in the message, though.
+           */
+          ulen  start ;
+
+          qassert(!stream_has_overflowed(s)) ;
+
+          start = attr_lp ;     /* start of withdrawn nlri              */
+
+          while(1)
+            {
+              if (adv->adj->attr != NULL)
+                {
+                  stream_put_prefix (s, &rn->p);
+
+                  if (stream_has_written_beyond(s, BGP_MSG_MAX_L - 2))
+                    {
+                      stream_set_endp(s, attr_lp) ; /* back one     */
+                      stream_clear_overflow(s) ;
+                      break ;
+                    } ;
+
+                  ++withdrawn ;
+                } ;
+
+              attr_lp = stream_get_endp(s) ;
+
+              adv = bgp_updated(peer, adv, afi, safi, updates,
+                                                         true /*suppressed */) ;
+              ++count ;
+
+              if (adv == NULL)
+                break ;
+
+              rn = adv->rn ;
+              assert(rn != NULL);
+            } ;
+
+          stream_putw_at(s, start - 2, attr_lp - start) ;
+          stream_putw(s, 0) ;
+        }
+      else
+        {
+          if (adv->adj->attr != NULL)
+            {
+              stream_putw (s, 0);   /* Attributes length        */
+
+              attr_len = bgp_packet_withdraw (s, &rn->p, afi, safi, prd);
+              stream_putw_at (s, attr_lp, attr_len);
+              ++withdrawn ;
+            } ;
+
+          bgp_updated(peer, adv, afi, safi, updates, true /*suppressed */) ;
+          ++count ;
+        } ;
+
+      /* Now log the error
+       */
+      zlog_err("%s FORCED %u/%u WITHDRAW(S) %s/%s:%s",
+                                  peer->host, withdrawn, count,
+                                  map_direct(bgp_afi_name_map, afi).str,
+                                  map_direct(bgp_safi_name_map, safi).str,
+                                                           qs_string(updates)) ;
+      qs_free(updates) ;
+
+      /* If we have no actual withdraws, exit now
+       */
+      if (withdrawn == 0)
+        return NULL ;
+    } ;
+
+  /* The message is complete -- and kept to size, above.
+   */
+  bgp_packet_set_size (s) ;
+
+  return s ;
+} ;
+
+/*------------------------------------------------------------------------------
+ * Have added the prefix for the given advertisement to the UPDATE message
+ * in construction.
+ *
+ * Update corresponding adjacency to reflect the attributes last sent for
+ * the prefix.  This either replaces the existing attributes, or sets a new
+ * set.  If setting a new set, we increment the count of prefixes sent.
+ *
+ * Then remove the bgp_advertise object from the lists it lives on, and
+ * return the next (which will have the same attributes), if any.
+ */
+static bgp_advertise
+bgp_updated(bgp_peer peer, bgp_advertise adv, afi_t afi, safi_t safi,
+                                               qstring updates, bool suppressed)
+{
+  struct bgp_adj_out *adj;
+
+  if (updates != NULL)
+    {
+      qs_append_str(updates, " ") ;
+      qs_append_str(updates, spfxtoa(&adv->rn->p).str) ;
+    } ;
+
+  adj = adv->adj ;
+
+  if (suppressed)
+    {
+      if (adj->attr != NULL)
+        {
+          bgp_attr_unintern (&adj->attr);
+          adj->attr = NULL ;
+          peer->scount[afi][safi]--;
+        } ;
+    }
+  else
+    {
+      if (adj->attr != NULL)
+        bgp_attr_unintern (&adj->attr);
+      else
+        peer->scount[afi][safi]++;
+
+      adj->attr = bgp_attr_intern (adv->baa->attr);
+    } ;
+
+  return bgp_advertise_clean (peer, adj, afi, safi) ;
+} ;
+
+#if 0                           // Replaced by the above
+
 /*------------------------------------------------------------------------------
  * Construct an update from head of peer->sync[afi][safi]->update.
  *
@@ -146,6 +451,7 @@ bgp_update_packet (struct peer *peer, afi_t afi, safi_t safi)
   bgp_packet_set_size (s) ;
   return s ;
 }
+#endif
 
 /*------------------------------------------------------------------------------
  * Construct an End-of-RIB update message for given AFI/SAFI.
@@ -164,7 +470,8 @@ bgp_update_packet_eor (struct peer *peer, afi_t afi, safi_t safi)
     return NULL;
 
   if (BGP_DEBUG (normal, NORMAL))
-    zlog_debug ("send End-of-RIB for %s to %s", afi_safi_print (afi, safi), peer->host);
+    zlog_debug ("send End-of-RIB for %s to %s", afi_safi_print (afi, safi),
+                                                                    peer->host);
 
   s = peer->work;
   stream_reset (s);
@@ -175,7 +482,7 @@ bgp_update_packet_eor (struct peer *peer, afi_t afi, safi_t safi)
   /* Unfeasible Routes Length */
   stream_putw (s, 0);
 
-  if (afi == AFI_IP && safi == SAFI_UNICAST)
+  if ((afi == AFI_IP) && (safi == SAFI_UNICAST))
     {
       /* Total Path Attribute Length */
       stream_putw (s, 0);
@@ -191,6 +498,8 @@ bgp_update_packet_eor (struct peer *peer, afi_t afi, safi_t safi)
       stream_putc (s, safi);
     }
 
+  /* Cannot exceed maximum message size !
+   */
   bgp_packet_set_size (s);
   return s ;
 }
@@ -202,6 +511,8 @@ bgp_update_packet_eor (struct peer *peer, afi_t afi, safi_t safi)
  *
  * Returns: peer->work -- if have something to be written.
  *          NULL       -- otherwise
+ *
+ * NB: returns NULL iff the peer's withdraw queue is empty.
  */
 static struct stream *
 bgp_withdraw_packet (struct peer *peer, afi_t afi, safi_t safi)
@@ -210,73 +521,122 @@ bgp_withdraw_packet (struct peer *peer, afi_t afi, safi_t safi)
   struct bgp_adj_out *adj;
   struct bgp_advertise *adv;
   struct bgp_node *rn;
-  unsigned long pos;
-  bgp_size_t unfeasible_len;
-  bgp_size_t total_attr_len;
-  char buf[BUFSIZ];
+  uint withdrawn ;
+  qstring updates ;
+  bool  ipv4_unicast ;
+  ulen  len_p, len_ap, limit, end_p ;
+
+  if (BGP_DEBUG (update, UPDATE_OUT))
+    updates = qs_new(100) ;
+  else
+    updates = NULL ;
+
+  ipv4_unicast = (afi == AFI_IP) && (safi == SAFI_UNICAST) ;
 
   s = peer->work;
   stream_reset (s);
 
+  bgp_packet_set_marker (s, BGP_MSG_UPDATE);
+  stream_putw (s, 0) ;          /* Withdraw length      */
+
+  if (ipv4_unicast)
+    {
+      len_p  = stream_get_endp(s) ;
+      len_ap = 0 ;
+      limit  = BGP_MSG_MAX_L - 2 ;
+    }
+  else
+    {
+      stream_putw(s, 0) ;       /* Attributes length    */
+
+      len_p  = stream_get_endp(s) ;
+
+      stream_putc(s, BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_EXTLEN) ;
+      stream_putc(s, BGP_ATTR_MP_UNREACH_NLRI) ;
+      stream_putw(s, 0) ;
+
+      len_ap = stream_get_endp(s) ;
+      limit  = BGP_MSG_MAX_L ;
+
+      stream_putw (s, afi);
+
+      if (safi == SAFI_MPLS_VPN)
+        stream_putc (s, SAFI_MPLS_LABELED_VPN);
+      else
+        stream_putc (s, safi);
+    } ;
+
+  withdrawn = 0 ;
+  end_p = stream_get_endp(s) ;
+
   while ((adv = bgp_advertise_fifo_head(&peer->sync[afi][safi]->withdraw))
                                                                         != NULL)
     {
-      assert (adv->rn);
       adj = adv->adj;
-      rn = adv->rn;
+      rn  = adv->rn;
+      assert (rn != NULL);
 
-      if (STREAM_REMAIN (s)
-	  < (BGP_NLRI_LENGTH + BGP_TOTAL_ATTR_LEN + PSIZE (rn->p.prefixlen)))
-	break;
-
-      if (stream_is_empty (s))
-	{
-	  bgp_packet_set_marker (s, BGP_MSG_UPDATE);
-	  stream_putw (s, 0);
-	}
+      if (adj->attr != NULL)
+        {
+          if (safi != SAFI_MPLS_VPN)
+            stream_put_prefix(s, &rn->p);
+          else
+            bgp_packet_withdraw_vpn_prefix(s, &rn->p,
+                                             (struct prefix_rd *) &rn->prn->p) ;
 
-      if (afi == AFI_IP && safi == SAFI_UNICAST)
-	stream_put_prefix (s, &rn->p);
-      else
-	{
-	  struct prefix_rd *prd = NULL;
+          if (stream_has_written_beyond(s, limit))
+            {
+              stream_set_endp(s, end_p) ;       /* as you was   */
+              stream_clear_overflow(s) ;
 
-	  if (rn->prn)
-	    prd = (struct prefix_rd *) &rn->prn->p;
-	  pos = stream_get_endp (s);
-	  stream_putw (s, 0);
-	  total_attr_len
-	    = bgp_packet_withdraw (peer, s, &rn->p, afi, safi, prd, NULL);
+              break ;
+            } ;
 
-	  /* Set total path attribute length. */
-	  stream_putw_at (s, pos, total_attr_len);
-	}
+          end_p = stream_get_endp(s) ;
 
-      if (BGP_DEBUG (update, UPDATE_OUT))
-	zlog (peer->log, LOG_DEBUG, "%s send UPDATE %s/%d -- unreachable",
-	      peer->host,
-	      inet_ntop (rn->p.family, &(rn->p.u.prefix), buf, BUFSIZ),
-	      rn->p.prefixlen);
+          peer->scount[afi][safi]--;
+          ++withdrawn ;
 
-      peer->scount[afi][safi]--;
+          if (updates != NULL)
+            {
+              qs_append_str(updates, " ") ;
+              qs_append_str(updates, spfxtoa(&rn->p).str) ;
+            } ;
+        } ;
 
       bgp_adj_out_remove (rn, adj, peer, afi, safi);
-
-      if (! (afi == AFI_IP && safi == SAFI_UNICAST))
-	break;
     }
 
-  if (stream_is_empty (s))
+  if (withdrawn == 0)
     return NULL ;
 
-  if (afi == AFI_IP && safi == SAFI_UNICAST)
+  /* For ipv4_unicast: set Withdrawn Routes Length
+   *                   then set Total Path Attributes Length == 0
+   *
+   *        otherwise: set Total Path Attributes Length
+   *                   then set length of the MP_UNREACH attribute
+   */
+  stream_putw_at(s, len_p - 2, end_p - len_p) ;
+
+  if (ipv4_unicast)
+    stream_putw(s, 0) ;         /* no attributes        */
+  else
+    stream_putw_at(s, len_ap - 2, end_p - len_ap) ;
+
+  /* Debug logging as required
+   */
+  if (updates != NULL)
     {
-      unfeasible_len
-	    = stream_get_endp (s) - BGP_HEADER_SIZE - BGP_UNFEASIBLE_LEN;
-      stream_putw_at (s, BGP_HEADER_SIZE, unfeasible_len);
-      stream_putw (s, 0);
+      zlog (peer->log, LOG_DEBUG, "%s send %u WITHDRAW(S) %s/%s:%s",
+                              peer->host, withdrawn,
+                              map_direct(bgp_afi_name_map, afi).str,
+                              map_direct(bgp_safi_name_map, safi).str,
+                                                       qs_string(updates)) ;
+      qs_free(updates) ;
     } ;
 
+  /* Kept within maximum message length, above.
+   */
   bgp_packet_set_size (s);
   return s ;
 }
@@ -285,6 +645,17 @@ bgp_withdraw_packet (struct peer *peer, afi_t afi, safi_t safi)
  * Construct an update for the default route, place it in the obuf queue
  * and kick write.
  *
+ * Note that this jumps all queues -- because the default route generated is
+ * special.  Also, this is called (a) when a table is about to be announced, so
+ * this will be the first route sent and (b) when the configuration option
+ * is set, so the ordering wrt other routes and routeadv timer is moot.
+ *
+ * Note that this may also trigger the output of pending withdraws and updates.
+ * Tant pis.
+ *
+ * Note also that it is assumed that (a) the attributes are essentially
+ * trivial, and (b) that they are 99.9% likely to be unique.
+ *
  * Uses peer->work stream structure, but copies result to new stream, which is
  * pushed onto the obuf queue.
  */
@@ -321,25 +692,29 @@ bgp_default_update_send (struct peer *peer, struct attr *attr,
   s = peer->work ;
   stream_reset (s);
 
-  /* Make BGP update packet. */
+  /* Make BGP update packet and set empty withdrawn NLRI
+   */
   bgp_packet_set_marker (s, BGP_MSG_UPDATE);
-
-  /* Unfeasible Routes Length. */
   stream_putw (s, 0);
 
-  /* Make place for total attribute length.  */
+  /* Construct attribute -- including NLRI for not AFI_IP/SAFI_UNICAST
+   */
   pos = stream_get_endp (s);
   stream_putw (s, 0);
-  total_attr_len = bgp_packet_attribute (NULL, peer, s, attr, &p, afi, safi, from, NULL, NULL);
 
-  /* Set Total Path Attribute Length. */
+  total_attr_len = bgp_packet_attribute (NULL, peer, s, attr, &p, afi, safi,
+                                                              from, NULL, NULL);
   stream_putw_at (s, pos, total_attr_len);
 
-  /* NLRI set. */
+  /* NLRI for AFI_IP/SAFI_UNICAST.
+   */
   if (p.family == AF_INET && safi == SAFI_UNICAST)
     stream_put_prefix (s, &p);
 
-  /* Set size. */
+  /* Set size -- note that it is essentially impossible that the message has
+   *             overflowed, but if it has there is nothing we can do about it
+   *             other than suppress and treat as error (the default action).
+   */
   bgp_packet_set_size (s);
 
   /* Dump packet if debug option is set. */
@@ -355,6 +730,14 @@ bgp_default_update_send (struct peer *peer, struct attr *attr,
  * Construct a withdraw update for the default route, place it in the obuf
  * queue and kick write.
  *
+ * Note that this jumps even the withdraw queue.  This is called when the
+ * configuration option is unset, so the ordering wrt other routes and
+ * routeadv timer is moot.  If there were other withdraws pending, they could
+ * be merged in -- but that seems like a lot of work for little benefit.
+ *
+ * Note that this may also trigger the output of pending withdraws and updates.
+ * Tant pis.
+ *
  * Uses peer->work stream structure, but copies result to new stream, which is
  * pushed onto the obuf queue.
  */
@@ -414,12 +797,14 @@ bgp_default_withdraw_send (struct peer *peer, afi_t afi, safi_t safi)
     {
       pos = stream_get_endp (s);
       stream_putw (s, 0);
-      total_attr_len = bgp_packet_withdraw (peer, s, &p, afi, safi, NULL, NULL);
+      total_attr_len = bgp_packet_withdraw (s, &p, afi, safi, NULL);
 
       /* Set total path attribute length. */
       stream_putw_at (s, pos, total_attr_len);
     }
 
+  /* Impossible to overflow the BGP Message !
+   */
   bgp_packet_set_size (s);
 
   /* Add packet to the peer. */
@@ -439,54 +824,89 @@ bgp_write_packet (struct peer *peer)
 {
   afi_t afi;
   safi_t safi;
-  struct stream *s = NULL;
+  struct stream *s ;
   struct bgp_advertise *adv;
 
+  s = NULL ;            /* nothing to send, yet */
+
   for (afi = AFI_IP; afi < AFI_MAX; afi++)
     for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++)
       {
 	adv = bgp_advertise_fifo_head(&peer->sync[afi][safi]->withdraw);
 	if (adv)
 	  {
+	    /* Note that -- unlike bgp_update_packet() -- this guarantees to
+	     * generate a packet, unless there is absolutely nothing to be
+	     * withdrawn -- in which case ->withdraw *will* be empty.
+	     */
 	    s = bgp_withdraw_packet (peer, afi, safi);
 	    if (s)
 	      return s;
-	  }
-      }
+	  } ;
+
+	qassert(bgp_advertise_fifo_head(&peer->sync[afi][safi]->withdraw)
+	                                                              == NULL) ;
+      } ;
 
   for (afi = AFI_IP; afi < AFI_MAX; afi++)
     for (safi = SAFI_UNICAST; safi < SAFI_MAX; safi++)
       {
-	adv = bgp_advertise_fifo_head(&peer->sync[afi][safi]->update);
-	if (adv)
+	while (1)
 	  {
-            if (adv->binfo && adv->binfo->uptime < peer->synctime)
-	      {
-		if (CHECK_FLAG (adv->binfo->peer->cap, PEER_CAP_RESTART_RCV)
-		    && CHECK_FLAG (adv->binfo->peer->cap, PEER_CAP_RESTART_ADV)
-		    && ! CHECK_FLAG (adv->binfo->flags, BGP_INFO_STALE)
-		    && safi != SAFI_MPLS_VPN)
-		  {
-		    if (CHECK_FLAG (adv->binfo->peer->af_sflags[afi][safi],
-			PEER_STATUS_EOR_RECEIVED))
-		      s = bgp_update_packet (peer, afi, safi);
-		  }
-		else
-		  s = bgp_update_packet (peer, afi, safi);
-	      }
+	    adv = bgp_advertise_fifo_head(&peer->sync[afi][safi]->update) ;
+
+	    if (adv == NULL)
+	      break ;
+
+            if (adv->binfo->uptime >= peer->synctime)
+              break ;                   /* leave for later      */
+
+            /* This is waiting for EOR from the peer before sending updates, if
+             * we are both doing RESTART.
+             *
+             * TODO: Not sure why would want to send the earlier update if it
+             *       is BGP_INFO_STALE or MPLS ?
+             */
+            if (CHECK_FLAG (adv->binfo->peer->cap, PEER_CAP_RESTART_RCV)
+                    && CHECK_FLAG (adv->binfo->peer->cap, PEER_CAP_RESTART_ADV)
+                    && ! CHECK_FLAG (adv->binfo->flags, BGP_INFO_STALE)
+                    && safi != SAFI_MPLS_VPN)
+              {
+                if (!CHECK_FLAG (adv->binfo->peer->af_sflags[afi][safi],
+                                                      PEER_STATUS_EOR_RECEIVED))
+                  break;
+              }
 
-	    if (s)
+            /* We have an adv which want to send.
+             *
+             * bgp_update_packet() will always take the adv off the ->update
+             * list.
+             *
+             * Generally it will generate an update packet.  However, if the
+             * attributes are impossible and it is not necessary to withdraw
+             * any previous announcements, then will return NULL -- and we can
+             * go on to the next advertisement.
+             */
+            s = bgp_update_packet (peer, adv, afi, safi);
+
+            if (s != NULL)
 	      return s;
-	  }
+	  } ;
 
-	if (CHECK_FLAG (peer->cap, PEER_CAP_RESTART_RCV))
+	/* If there is nothing left to advertise, then this is a good moment
+	 * to send an EOR, if one is required.
+	 */
+	if ((adv == NULL) && CHECK_FLAG (peer->cap, PEER_CAP_RESTART_RCV))
 	  {
 	    if (peer->afc_nego[afi][safi] && peer->synctime
 		&& ! CHECK_FLAG (peer->af_sflags[afi][safi], PEER_STATUS_EOR_SEND)
 		&& safi != SAFI_MPLS_VPN)
 	      {
 		SET_FLAG (peer->af_sflags[afi][safi], PEER_STATUS_EOR_SEND);
-		return bgp_update_packet_eor (peer, afi, safi);
+		s = bgp_update_packet_eor (peer, afi, safi);
+
+		if (s != NULL)
+	          return s;
 	      }
 	  }
       }
@@ -655,7 +1075,10 @@ bgp_capability_send (struct peer *peer, afi_t afi, safi_t safi,
 		   "Advertising" : "Removing", afi, safi);
     }
 
-  /* Set packet size. */
+  /* Set packet size.
+   *
+   * Impossible to overflow the BGP Message buffer
+   */
   length = bgp_packet_set_size (s);
 
   if (BGP_DEBUG (normal, NORMAL))
diff --git a/bgpd/bgp_peer.c b/bgpd/bgp_peer.c
index 2cb58c34..a32f23bc 100644
--- a/bgpd/bgp_peer.c
+++ b/bgpd/bgp_peer.c
@@ -1756,6 +1756,34 @@ bgp_routeadv_timer (struct thread *thread)
   return 0;
 }
 
+static int
+bgp_withdraw_event (struct thread *thread)
+{
+  bgp_peer  peer ;
+
+  peer = THREAD_ARG (thread);
+  peer->t_withdraw = NULL;
+
+  if (BGP_DEBUG (fsm, FSM))
+    zlog (peer->log, LOG_DEBUG,
+          "%s [FSM] bgp_withdraw_event",
+          peer->host);
+
+  bgp_write(peer, NULL);
+  return 0;
+}
+
+extern void
+bgp_withdraw_schedule(bgp_peer peer)
+{
+  if (peer->t_withdraw == NULL)
+    {
+      /* TODO: replace legacy event use
+       */
+      peer->t_withdraw = thread_add_event(master, bgp_withdraw_event, peer, 0) ;
+    } ;
+} ;
+
 /*------------------------------------------------------------------------------
  * BGP Peer Down Causes mapped to strings
  */
diff --git a/bgpd/bgp_peer.h b/bgpd/bgp_peer.h
index 39cb54a5..c5155b3f 100644
--- a/bgpd/bgp_peer.h
+++ b/bgpd/bgp_peer.h
@@ -379,7 +379,7 @@ struct peer
   u_int32_t v_gr_restart;
 
   /* Threads.                                           */
-  struct thread *t_asorig;
+  struct thread *t_withdraw ;
   struct thread *t_routeadv;
   struct thread *t_pmax_restart;
   struct thread *t_gr_restart;
@@ -393,6 +393,8 @@ struct peer
   struct bgp_synchronize *sync[AFI_MAX][SAFI_MAX];
   time_t synctime;
 
+  bool   do_updates ;
+
   /* Send prefix count.                                 */
   unsigned long scount[AFI_MAX][SAFI_MAX];
 
@@ -448,6 +450,7 @@ struct peer
       THREAD_TIMER_OFF(T);			\
   } while (0)
 
+#if 0
 #define BGP_EVENT_ADD(P,E)			\
   do {						\
     if ((P)->state != bgp_peer_pDeleting)			\
@@ -467,8 +470,10 @@ extern int bgp_stop (struct peer *peer);
 extern void bgp_timer_set (struct peer *);
 #endif
 extern void bgp_fsm_change_status (struct peer *peer, int status);
-extern const char *peer_down_str[];
 
+#endif
+
+extern const char *peer_down_str[];
 
 /*==============================================================================
  *
@@ -491,6 +496,7 @@ extern bgp_peer bgp_peer_unlock (bgp_peer peer) ;
 extern int bgp_peer_delete (bgp_peer peer);
 extern sockunion bgp_peer_get_ifaddress(bgp_peer peer, const char* ifname,
                                                                sa_family_t af) ;
+extern void bgp_withdraw_schedule(bgp_peer peer) ;
 
 #endif /* _QUAGGA_BGP_PEER_H */
 
diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 0ef710c5..aa05c8ac 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -1630,7 +1630,8 @@ DEFUN (neighbor_set_peer_group,
 
   if (ret == BGP_ERR_PEER_GROUP_PEER_TYPE_DIFFERENT)
     {
-      vty_out (vty, "%% Peer with AS %u cannot be in this peer-group, members must be all internal or all external%s", as, VTY_NEWLINE);
+      vty_out (vty, "%% Peer with AS %u cannot be in this peer-group, "
+             "members must be all internal or all external%s", as, VTY_NEWLINE);
       return CMD_WARNING;
     }
 
diff --git a/configure.ac b/configure.ac
index 45653375..48d1b0ca 100755
--- a/configure.ac
+++ b/configure.ac
@@ -7,7 +7,7 @@
 ##
 AC_PREREQ(2.53)
 
-AC_INIT(Quagga, 0.99.20ex23b, [http://bugzilla.quagga.net])
+AC_INIT(Quagga, 0.99.20ex24b, [http://bugzilla.quagga.net])
 AC_CONFIG_SRCDIR(lib/zebra.h)
 AC_CONFIG_MACRO_DIR([m4])
 
diff --git a/lib/qpthreads.c b/lib/qpthreads.c
index ebaf6f19..4288b9ff 100644
--- a/lib/qpthreads.c
+++ b/lib/qpthreads.c
@@ -568,9 +568,8 @@ qpt_thread_create(void* (*start)(void*), void* arg, qpt_thread_attr_t* attr)
   {
     qstring qs ;
 
-    qs = qpt_thread_attr_form(&thread_attr, "  ") ;
-
-    fputs(qs_string(qs), stderr) ;
+    qs = qpt_thread_attr_form(&thread_attr) ;
+    zlog_info("Thread created: %s", qs_string(qs)) ;
 
     qs_free(qs) ;
   }
@@ -1392,7 +1391,7 @@ qpt_data_delete(qpt_data data)
  * NB: it is the caller's responsibility to dispose of the qstring
  */
 extern qstring
-qpt_thread_attr_form(qpt_thread_attr_t* attr, const char *prefix)
+qpt_thread_attr_form(qpt_thread_attr_t* attr)
 {
   int err, i ;
   qstring qs ;
@@ -1402,131 +1401,117 @@ qpt_thread_attr_form(qpt_thread_attr_t* attr, const char *prefix)
 
   qs = qs_new(500) ;
 
-                  /* 12345678901234567890 */
-  qs_printf_a(qs, "%sDetach state        ", prefix) ;
   err = pthread_attr_getdetachstate(attr, &i);
   if (err != 0)
-    qs_printf_a(qs, " *error* %s\n", errtoa(err, 0).str) ;
+    qs_printf_a(qs, "[getdetachstate *error* %s]", errtoa(err, 0).str) ;
   else
     {
       switch(i)
         {
           case PTHREAD_CREATE_DETACHED:
-            qs_printf_a(qs, "= %s\n", "PTHREAD_CREATE_DETACHED") ;
+            qs_printf_a(qs, "Detached") ;
             break ;
 
           case PTHREAD_CREATE_JOINABLE:
-            qs_printf_a(qs, "= %s\n", "PTHREAD_CREATE_JOINABLE") ;
+            qs_printf_a(qs, "Joinable") ;
             break ;
 
           default:
-            qs_printf_a(qs, "= *UNKNOWN* %d\n", i) ;
+            qs_printf_a(qs, "[getdetachstate *unknown* %d]", i) ;
             break ;
         } ;
     } ;
 
-                  /* 12345678901234567890 */
-  qs_printf_a(qs, "%sScope               ", prefix) ;
-  err = pthread_attr_getscope(attr, &i);
+  err = pthread_attr_getinheritsched(attr, &i);
   if (err != 0)
-    qs_printf_a(qs, " *error* %s\n", errtoa(err, 0).str) ;
+    qs_printf_a(qs, " [getinheritsched *error* %s]", errtoa(err, 0).str) ;
   else
     {
       switch(i)
         {
-          case PTHREAD_SCOPE_SYSTEM:
-            qs_printf_a(qs, "= %s\n", "PTHREAD_SCOPE_SYSTEM") ;
+          case PTHREAD_INHERIT_SCHED:
+            qs_printf_a(qs, " Inherit-Sched") ;
             break ;
 
-          case PTHREAD_SCOPE_PROCESS:
-            qs_printf_a(qs, "= %s\n", "PTHREAD_SCOPE_PROCESS") ;
+          case PTHREAD_EXPLICIT_SCHED:
+            qs_printf_a(qs, " Explicit-Sched") ;
             break ;
 
           default:
-            qs_printf_a(qs, "= *UNKNOWN* %d\n", i) ;
+            qs_printf_a(qs, " [getinheritsched *unknown* %d]", i) ;
             break ;
         } ;
     } ;
 
-                  /* 12345678901234567890 */
-  qs_printf_a(qs, "%sInherit scheduler   ", prefix) ;
-  err = pthread_attr_getinheritsched(attr, &i);
+  err = pthread_attr_getscope(attr, &i);
   if (err != 0)
-    qs_printf_a(qs, " *error* %s\n", errtoa(err, 0).str) ;
+    qs_printf_a(qs, " [getscope *error* %s]", errtoa(err, 0).str) ;
   else
     {
       switch(i)
         {
-          case PTHREAD_INHERIT_SCHED:
-            qs_printf_a(qs, "= %s\n", "PTHREAD_INHERIT_SCHED") ;
+          case PTHREAD_SCOPE_SYSTEM:
+            qs_printf_a(qs, " System-Scope") ;
             break ;
 
-          case PTHREAD_EXPLICIT_SCHED:
-            qs_printf_a(qs, "= %s\n", "PTHREAD_EXPLICIT_SCHED") ;
+          case PTHREAD_SCOPE_PROCESS:
+            qs_printf_a(qs, " Process-Scope") ;
             break ;
 
           default:
-            qs_printf_a(qs, "= *UNKNOWN* %d\n", i) ;
+            qs_printf_a(qs, " [getscope *unknown* %d]", i) ;
             break ;
         } ;
     } ;
 
-                  /* 12345678901234567890 */
-  qs_printf_a(qs, "%sScheduling policy   ", prefix) ;
   err = pthread_attr_getschedpolicy(attr, &i);
   if (err != 0)
-    qs_printf_a(qs, " *error* %s\n", errtoa(err, 0).str) ;
+    qs_printf_a(qs, " [getschedpolicy *error* %s]", errtoa(err, 0).str) ;
   else
     {
       switch(i)
         {
           case SCHED_OTHER:
-            qs_printf_a(qs, "= %s\n", "SCHED_OTHER") ;
+            qs_printf_a(qs, " SCHED_OTHER") ;
             break ;
 
           case SCHED_FIFO:
-            qs_printf_a(qs, "= %s\n", "SCHED_FIFO") ;
+            qs_printf_a(qs, " SCHED_FIFO") ;
             break ;
 
           case SCHED_RR:
-            qs_printf_a(qs, "= %s\n", "SCHED_RR") ;
+            qs_printf_a(qs, " SCHED_RR") ;
             break ;
 
 #ifdef SCHED_SPORADIC
           case SCHED_SPORADIC:
-            qs_printf_a(qs, "= %s\n", "SCHED_SPORADIC") ;
+            qs_printf_a(qs, " SCHED_SPORADIC") ;
             break ;
 #endif
 
           default:
-            qs_printf_a(qs, "= *UNKNOWN* %d\n", i) ;
+            qs_printf_a(qs, " SCHED_UNKNOWN=%d", i) ;
             break ;
         } ;
     } ;
 
-                  /* 12345678901234567890 */
-  qs_printf_a(qs, "%sScheduling priority ", prefix) ;
   err = pthread_attr_getschedparam(attr, sp);
   if (err != 0)
-    qs_printf_a(qs, " *error* %s\n", errtoa(err, 0).str) ;
+    qs_printf_a(qs, " [getschedparam *error* %s]", errtoa(err, 0).str) ;
   else
-    qs_printf_a(qs, "= %d\n", sp->sched_priority) ;
+    qs_printf_a(qs, " Priority=%d", sp->sched_priority) ;
 
-                  /* 12345678901234567890 */
-  qs_printf_a(qs, "%sGuard size          ", prefix) ;
   err = pthread_attr_getguardsize(attr, &v);
   if (err != 0)
-    qs_printf_a(qs, " *error* %s\n", errtoa(err, 0).str) ;
+    qs_printf_a(qs, " [getguardsize *error* %s]", errtoa(err, 0).str) ;
   else
-    qs_printf_a(qs, "= %u\n", (uint)v) ;
+    qs_printf_a(qs, " Guard-Size=%u", (uint)v) ;
 
-                  /* 12345678901234567890 */
-  qs_printf_a(qs, "%sStack address/size  ", prefix) ;
   err = pthread_attr_getstack(attr, &stkaddr, &v) ;
   if (err != 0)
-    qs_printf_a(qs, " *error* %s\n", errtoa(err, 0).str) ;
+    qs_printf_a(qs, " [getstack *error* %s]", errtoa(err, 0).str) ;
   else
-    qs_printf_a(qs, "= %p/%u\n", stkaddr, (uint)v) ;
+    qs_printf_a(qs, " Stack=%p/%u", stkaddr, (uint)v) ;
 
   return qs ;
 } ;
diff --git a/lib/qpthreads.h b/lib/qpthreads.h
index 363fbead..78c39a6e 100644
--- a/lib/qpthreads.h
+++ b/lib/qpthreads.h
@@ -194,8 +194,7 @@ qpt_thread_join(qpt_thread_t thread_id) ;
 extern clockid_t qpt_get_cpu_clock(qpt_thread_t thread_id) ;
 extern qtime_t qpt_cpu_time(clockid_t clock_id) ;
 
-extern qstring qpt_thread_attr_form(qpt_thread_attr_t* attr,
-                                                           const char *prefix) ;
+extern qstring qpt_thread_attr_form(qpt_thread_attr_t* attr) ;
 
 /*==============================================================================
  * Signal Handling.
diff --git a/lib/stream.h b/lib/stream.h
index b4d981b1..a7906573 100644
--- a/lib/stream.h
+++ b/lib/stream.h
@@ -177,6 +177,8 @@ Inline bool stream_has_overflowed(struct stream* s) ;
 Inline void stream_clear_overrun(struct stream* s) ;
 Inline void stream_clear_overflow(struct stream* s) ;
 
+Inline bool stream_has_written_beyond(struct stream* s, size_t limit) ;
+
 Inline void stream_set_getp(struct stream *, size_t);
 Inline void stream_set_endp(struct stream *, size_t);
 Inline void stream_set_startp(struct stream* s, size_t) ;
@@ -503,6 +505,27 @@ stream_clear_overflow(struct stream* s)
 }
 
 /*------------------------------------------------------------------------------
+ * Test if the endp is beyond the given limit
+ *
+ * A stream may be set up to be longer than some actual limit, so that the
+ * extent of overflowing beyond that limit can be measured.  This test can
+ * then be used to see if the endp is *currently* within the given limit.
+ *
+ * Returns:  true <=> endp is *beyond* the given limit.
+ *
+ * NB: the limit MUST be less than the size -- for if not, this is *always*
+ *     going to return false, because the s->endp is *always* <= s->size !
+ */
+Inline bool
+stream_has_written_beyond(struct stream* s, size_t limit)
+{
+  qassert_stream(s) ;
+  qassert(limit < s->size) ;
+
+  return (s->endp > limit) ;
+} ;
+
+/*------------------------------------------------------------------------------
  * Set s->getp to given value.
  *
  * If value > s->endp will force to s->endp and set s->overrun.
diff --git a/vtysh/vtysh_config.c b/vtysh/vtysh_config.c
index 75e2337e..2dde3065 100644
--- a/vtysh/vtysh_config.c
+++ b/vtysh/vtysh_config.c
@@ -1807,7 +1807,7 @@ vtysh_config_get_name(config_collection collection, config_item parent,
         break ;
 
       default:
-        qassert(false) ;
+        assert(false) ;
     } ;
 
   tokens = cmd_tokens_concat(collection->parsed, ti, tn - ti) ;