Merge branch 'attr-errors'

Contains BGP fixes:

- set extcommunity crash: tihs patch tries to make the refcounting more robust
  but does not fully solve the problem, sadly.
- BGP attribute error handling: Little testing.

15 files changed, 880 insertions, 455 deletions

diff --git a/bgpd/bgp_advertise.c b/bgpd/bgp_advertise.c
index 87eb7ac7..666218fa 100644
--- a/bgpd/bgp_advertise.c
+++ b/bgpd/bgp_advertise.c
@@ -140,13 +140,13 @@ bgp_advertise_unintern (struct hash *hash, struct bgp_advertise_attr *baa)
     baa->refcnt--;
 
   if (baa->refcnt && baa->attr)
-    bgp_attr_unintern (baa->attr);
+    bgp_attr_unintern (&baa->attr);
   else
     {
       if (baa->attr)
 	{
 	  hash_release (hash, baa);
-	  bgp_attr_unintern (baa->attr);
+	  bgp_attr_unintern (&baa->attr);
 	}
       baa_free (baa);
     }
@@ -319,7 +319,7 @@ bgp_adj_out_remove (struct bgp_node *rn, struct bgp_adj_out *adj,
 		    struct peer *peer, afi_t afi, safi_t safi)
 {
   if (adj->attr)
-    bgp_attr_unintern (adj->attr);
+    bgp_attr_unintern (&adj->attr);
 
   if (adj->adv)
     bgp_advertise_clean (peer, adj, afi, safi);
@@ -339,7 +339,7 @@ bgp_adj_in_set (struct bgp_node *rn, struct peer *peer, struct attr *attr)
 	{
 	  if (adj->attr != attr)
 	    {
-	      bgp_attr_unintern (adj->attr);
+	      bgp_attr_unintern (&adj->attr);
 	      adj->attr = bgp_attr_intern (attr);
 	    }
 	  return;
@@ -355,7 +355,7 @@ bgp_adj_in_set (struct bgp_node *rn, struct peer *peer, struct attr *attr)
 void
 bgp_adj_in_remove (struct bgp_node *rn, struct bgp_adj_in *bai)
 {
-  bgp_attr_unintern (bai->attr);
+  bgp_attr_unintern (&bai->attr);
   BGP_ADJ_IN_DEL (rn, bai);
   peer_unlock (bai->peer); /* adj_in peer reference */
   XFREE (MTYPE_BGP_ADJ_IN, bai);
diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
index cf930427..ba4f5b48 100644
--- a/bgpd/bgp_aspath.c
+++ b/bgpd/bgp_aspath.c
@@ -340,19 +340,21 @@ aspath_free (struct aspath *aspath)
 
 /* Unintern aspath from AS path bucket. */
 void
-aspath_unintern (struct aspath *aspath)
+aspath_unintern (struct aspath **aspath)
 {
   struct aspath *ret;
+  struct aspath *asp = *aspath;
+  
+  if (asp->refcnt)
+    asp->refcnt--;
 
-  if (aspath->refcnt)
-    aspath->refcnt--;
-
-  if (aspath->refcnt == 0)
+  if (asp->refcnt == 0)
     {
       /* This aspath must exist in aspath hash table. */
-      ret = hash_release (ashash, aspath);
+      ret = hash_release (ashash, asp);
       assert (ret != NULL);
-      aspath_free (aspath);
+      aspath_free (asp);
+      *aspath = NULL;
     }
 }
 
@@ -671,80 +673,79 @@ aspath_hash_alloc (void *arg)
   return aspath;
 }
 
-/* parse as-segment byte stream in struct assegment
- *
- * Returns NULL if the AS_PATH or AS4_PATH is not valid.
- */
-static struct assegment *
-assegments_parse (struct stream *s, size_t length, int use32bit, int as4_path)
+/* parse as-segment byte stream in struct assegment */
+static int
+assegments_parse (struct stream *s, size_t length, 
+                  struct assegment **result, int use32bit)
 {
   struct assegment_header segh;
   struct assegment *seg, *prev = NULL, *head = NULL;
+  size_t bytes = 0;
   
-  assert (length > 0);  /* does not expect empty AS_PATH or AS4_PATH    */
+  /* empty aspath (ie iBGP or somesuch) */
+  if (length == 0)
+    return 0;
   
   if (BGP_DEBUG (as4, AS4_SEGMENT))
     zlog_debug ("[AS4SEG] Parse aspath segment: got total byte length %lu",
 		(unsigned long) length);
-
-  /* double check that length does not exceed stream    */
-  if (STREAM_READABLE(s) < length)
-    return NULL;
+  /* basic checks */
+  if ((STREAM_READABLE(s) < length)
+      || (STREAM_READABLE(s) < AS_HEADER_SIZE) 
+      || (length % AS16_VALUE_SIZE ))
+    return -1;
   
-  /* deal with each segment in turn                             */
-  while (length > 0)
+  while (bytes < length)
     {
       int i;
       size_t seg_size;
       
-      /* softly softly, get the header first on its own */
-      if (length < AS_HEADER_SIZE)
+      if ((length - bytes) <= AS_HEADER_SIZE)
         {
-          assegment_free_all (head);
-          return NULL;
+          if (head)
+            assegment_free_all (head);
+          return -1;
         }
       
+      /* softly softly, get the header first on its own */
       segh.type = stream_getc (s);
       segh.length = stream_getc (s);
       
       seg_size = ASSEGMENT_SIZE(segh.length, use32bit);
-                                      /* includes the header bytes */
 
       if (BGP_DEBUG (as4, AS4_SEGMENT))
 	zlog_debug ("[AS4SEG] Parse aspath segment: got type %d, length %d",
                     segh.type, segh.length);
       
+      /* check it.. */
+      if ( ((bytes + seg_size) > length)
+          /* 1771bis 4.3b: seg length contains one or more */
+          || (segh.length == 0) 
+          /* Paranoia in case someone changes type of segment length.
+           * Shift both values by 0x10 to make the comparison operate
+           * on more, than 8 bits (otherwise it's a warning, bug #564).
+           */
+          || ((sizeof segh.length > 1) 
+              && (0x10 + segh.length > 0x10 + AS_SEGMENT_MAX)))
+        {
+          if (head)
+            assegment_free_all (head);
+          return -1;
+        }
+      
       switch (segh.type)
         {
           case AS_SEQUENCE:
           case AS_SET:
-            break ;
-
           case AS_CONFED_SEQUENCE:
           case AS_CONFED_SET:
-            if (!as4_path)
-              break ;
-              /* RFC4893 3: "invalid for the AS4_PATH attribute"            */
-              /* fall through */
-
-          default:    /* reject unknown or invalid AS_PATH segment types  */
-            seg_size = 0 ;
-        }
-     
-     /* Stop now if segment is not valid (discarding anything collected to date)
-      *
-      * RFC4271 4.3, Path Attributes, b) AS_PATH:
-      *
-      *   "path segment value field contains one or more AS numbers"
-           */
-      if ((seg_size == 0) || (seg_size > length) || (segh.length == 0))
-        {
-          assegment_free_all (head);
-          return NULL;
+            break;
+          default:
+            if (head)
+              assegment_free_all (head);
+            return -1;
         }
       
-      length -= seg_size ;
-      
       /* now its safe to trust lengths */
       seg = assegment_new (segh.type, segh.length);
       
@@ -756,52 +757,47 @@ assegments_parse (struct stream *s, size_t length, int use32bit, int as4_path)
       for (i = 0; i < segh.length; i++)
 	seg->as[i] = (use32bit) ? stream_getl (s) : stream_getw (s);
 
+      bytes += seg_size;
+      
       if (BGP_DEBUG (as4, AS4_SEGMENT))
-	zlog_debug ("[AS4SEG] Parse aspath segment: length left: %lu",
-	            (unsigned long) length);
+	zlog_debug ("[AS4SEG] Parse aspath segment: Bytes now: %lu",
+	            (unsigned long) bytes);
       
       prev = seg;
     }
  
-  return assegment_normalise (head);
+  *result = assegment_normalise (head);
+  return 0;
 }
 
-/* AS path parse function -- parses AS_PATH and AS4_PATH attributes
- *
- * Requires: s        -- stream, currently positioned before first segment
- *                       of AS_PATH or AS4_PATH (ie after attribute header)
- *           length   -- length of the value of the AS_PATH or AS4_PATH
- *           use32bit -- true <=> 4Byte ASN, otherwise 2Byte ASN
- *           as4_path -- true <=> AS4_PATH, otherwise AS_PATH
- *
- * Returns: if valid: address of struct aspath in the hash of known aspaths,
- *                    with reference count incremented.
- *              else: NULL
- *
- * NB: empty AS path (length == 0) is valid.  The returned struct aspath will
- *     have segments == NULL and str == zero length string (unique).
+/* AS path parse function.  pnt is a pointer to byte stream and length
+   is length of byte stream.  If there is same AS path in the the AS
+   path hash then return it else make new AS path structure. 
+   
+   On error NULL is returned.
  */
 struct aspath *
-aspath_parse (struct stream *s, size_t length, int use32bit, int as4_path)
+aspath_parse (struct stream *s, size_t length, int use32bit)
 {
   struct aspath as;
   struct aspath *find;
 
-  /* Parse each segment and construct normalised list of struct assegment */
-  memset (&as, 0, sizeof (struct aspath));
-  if (length != 0)
-    {
-      as.segments = assegments_parse (s, length, use32bit, as4_path);
+  /* If length is odd it's malformed AS path. */
+  /* Nit-picking: if (use32bit == 0) it is malformed if odd,
+   * otherwise its malformed when length is larger than 2 and (length-2) 
+   * is not dividable by 4.
+   * But... this time we're lazy
+   */
+  if (length % AS16_VALUE_SIZE )
+    return NULL;
 
-      if (as.segments == NULL)
-        return NULL ;   /* Invalid AS_PATH or AS4_PATH  */
-    } ;
+  memset (&as, 0, sizeof (struct aspath));
+  if (assegments_parse (s, length, &as.segments, use32bit) < 0)
+    return NULL;
   
   /* If already same aspath exist then return it. */
   find = hash_get (ashash, &as, aspath_hash_alloc);
   
-  assert(find) ;        /* valid aspath, so must find or create */
-  
   /* aspath_hash_alloc dupes segments too. that probably could be
    * optimised out.
    */
@@ -809,6 +805,8 @@ aspath_parse (struct stream *s, size_t length, int use32bit, int as4_path)
   if (as.str)
     XFREE (MTYPE_AS_STR, as.str);
   
+  if (! find)
+    return NULL;
   find->refcnt++;
 
   return find;
@@ -1632,7 +1630,7 @@ aspath_segment_add (struct aspath *as, int type)
 struct aspath *
 aspath_empty (void)
 {
-  return aspath_parse (NULL, 0, 1, 0); /* 32Bit ;-) not AS4_PATH */
+  return aspath_parse (NULL, 0, 1); /* 32Bit ;-) */
 }
 
 struct aspath *
diff --git a/bgpd/bgp_aspath.h b/bgpd/bgp_aspath.h
index d63b914c..d55f9ce6 100644
--- a/bgpd/bgp_aspath.h
+++ b/bgpd/bgp_aspath.h
@@ -65,7 +65,7 @@ struct aspath
 /* Prototypes. */
 extern void aspath_init (void);
 extern void aspath_finish (void);
-extern struct aspath *aspath_parse (struct stream *, size_t, int, int);
+extern struct aspath *aspath_parse (struct stream *, size_t, int);
 extern struct aspath *aspath_dup (struct aspath *);
 extern struct aspath *aspath_aggregate (struct aspath *, struct aspath *);
 extern struct aspath *aspath_prepend (struct aspath *, struct aspath *);
@@ -80,7 +80,7 @@ extern struct aspath *aspath_empty_get (void);
 extern struct aspath *aspath_str2aspath (const char *);
 extern void aspath_free (struct aspath *);
 extern struct aspath *aspath_intern (struct aspath *);
-extern void aspath_unintern (struct aspath *);
+extern void aspath_unintern (struct aspath **);
 extern const char *aspath_print (struct aspath *);
 extern void aspath_print_vty (struct vty *, const char *, struct aspath *, const char *);
 extern void aspath_print_all_vty (struct vty *);
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 01598c87..d43c104f 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -500,6 +500,7 @@ bgp_attr_intern (struct attr *attr)
             attre->ecommunity = ecommunity_intern (attre->ecommunity);
           else
             attre->ecommunity->refcnt++;
+          
         }
       if (attre->cluster)
         {
@@ -516,10 +517,10 @@ bgp_attr_intern (struct attr *attr)
             attre->transit->refcnt++;
         }
     }
-
+  
   find = (struct attr *) hash_get (attrhash, attr, bgp_attr_hash_alloc);
   find->refcnt++;
-
+  
   return find;
 }
 
@@ -561,7 +562,7 @@ bgp_attr_default_intern (u_char origin)
   new = bgp_attr_intern (&attr);
   bgp_attr_extra_free (&attr);
   
-  aspath_unintern (new->aspath);
+  aspath_unintern (&new->aspath);
   return new;
 }
 
@@ -613,52 +614,67 @@ bgp_attr_aggregate_intern (struct bgp *bgp, u_char origin,
   new = bgp_attr_intern (&attr);
   bgp_attr_extra_free (&attr);
   
-  aspath_unintern (new->aspath);
+  aspath_unintern (&new->aspath);
   return new;
 }
 
+/* Unintern just the sub-components of the attr, but not the attr */
+void
+bgp_attr_unintern_sub (struct attr *attr)
+{
+  /* aspath refcount shoud be decrement. */
+  if (attr->aspath)
+    aspath_unintern (&attr->aspath);
+  UNSET_FLAG(attr->flag, BGP_ATTR_AS_PATH);
+  
+  if (attr->community)
+    community_unintern (&attr->community);
+  UNSET_FLAG(attr->flag, BGP_ATTR_COMMUNITIES);
+  
+  if (attr->extra)
+    {
+      if (attr->extra->ecommunity)
+        ecommunity_unintern (&attr->extra->ecommunity);
+      UNSET_FLAG(attr->flag, BGP_ATTR_EXT_COMMUNITIES);
+      
+      if (attr->extra->cluster)
+        cluster_unintern (attr->extra->cluster);
+      UNSET_FLAG(attr->flag, BGP_ATTR_CLUSTER_LIST);
+      
+      if (attr->extra->transit)
+        transit_unintern (attr->extra->transit);
+    }
+}
+
 /* Free bgp attribute and aspath. */
 void
-bgp_attr_unintern (struct attr *attr)
+bgp_attr_unintern (struct attr **attr)
 {
   struct attr *ret;
-  struct aspath *aspath;
-  struct community *community;
-  struct ecommunity *ecommunity = NULL;
-  struct cluster_list *cluster = NULL;
-  struct transit *transit = NULL;
-
+  struct attr tmp;
+  
   /* Decrement attribute reference. */
-  attr->refcnt--;
-  aspath = attr->aspath;
-  community = attr->community;
-  if (attr->extra)
+  (*attr)->refcnt--;
+  
+  tmp = *(*attr);
+  
+  if ((*attr)->extra)
     {
-      ecommunity = attr->extra->ecommunity;
-      cluster = attr->extra->cluster;
-      transit = attr->extra->transit;
+      tmp.extra = bgp_attr_extra_new ();
+      memcpy (tmp.extra, (*attr)->extra, sizeof (struct attr_extra));
     }
-
+  
   /* If reference becomes zero then free attribute object. */
-  if (attr->refcnt == 0)
+  if ((*attr)->refcnt == 0)
     {    
-      ret = hash_release (attrhash, attr);
+      ret = hash_release (attrhash, *attr);
       assert (ret != NULL);
-      bgp_attr_extra_free (attr);
-      XFREE (MTYPE_ATTR, attr);
+      bgp_attr_extra_free (*attr);
+      XFREE (MTYPE_ATTR, *attr);
+      *attr = NULL;
     }
 
-  /* aspath refcount shoud be decrement. */
-  if (aspath)
-    aspath_unintern (aspath);
-  if (community)
-    community_unintern (community);
-  if (ecommunity)
-    ecommunity_unintern (ecommunity);
-  if (cluster)
-    cluster_unintern (cluster);
-  if (transit)
-    transit_unintern (transit);
+  bgp_attr_unintern_sub (&tmp);
 }
 
 void
@@ -671,8 +687,9 @@ bgp_attr_flush (struct attr *attr)
   if (attr->extra)
     {
       struct attr_extra *attre = attr->extra;
+
       if (attre->ecommunity && ! attre->ecommunity->refcnt)
-        ecommunity_free (attre->ecommunity);
+        ecommunity_free (&attre->ecommunity);
       if (attre->cluster && ! attre->cluster->refcnt)
         cluster_free (attre->cluster);
       if (attre->transit && ! attre->transit->refcnt)
@@ -680,8 +697,69 @@ bgp_attr_flush (struct attr *attr)
     }
 }
 
+/* Implement draft-scudder-idr-optional-transitive behaviour and
+ * avoid resetting sessions for malformed attributes which are
+ * are partial/optional and hence where the error likely was not
+ * introduced by the sending neighbour.
+ */
+static bgp_attr_parse_ret_t
+bgp_attr_malformed (struct peer *peer, u_char type, u_char flag,
+                    u_char subcode, u_char *startp, bgp_size_t length)
+{
+  /* Only relax error handling for eBGP peers */
+  if (peer_sort (peer) != BGP_PEER_EBGP)
+    {
+      bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, subcode,
+                                 startp, length);
+      return BGP_ATTR_PARSE_ERROR;
+
+    }
+  
+  switch (type) {
+    /* where an optional attribute is inconsequential, e.g. it does not affect
+     * route selection, and can be safely ignored then any such attributes
+     * which are malformed should just be ignored and the route processed as
+     * normal.
+     */
+    case BGP_ATTR_AS4_AGGREGATOR:
+    case BGP_ATTR_AGGREGATOR:
+    case BGP_ATTR_ATOMIC_AGGREGATE:
+      return BGP_ATTR_PARSE_PROCEED;
+    
+    /* Core attributes, particularly ones which may influence route
+     * selection should always cause session resets
+     */
+    case BGP_ATTR_ORIGIN:
+    case BGP_ATTR_AS_PATH:
+    case BGP_ATTR_NEXT_HOP:
+    case BGP_ATTR_MULTI_EXIT_DISC:
+    case BGP_ATTR_LOCAL_PREF:
+    case BGP_ATTR_COMMUNITIES:
+    case BGP_ATTR_ORIGINATOR_ID:
+    case BGP_ATTR_CLUSTER_LIST:
+    case BGP_ATTR_MP_REACH_NLRI:
+    case BGP_ATTR_MP_UNREACH_NLRI:
+    case BGP_ATTR_EXT_COMMUNITIES:
+      bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, subcode,
+                                 startp, length);
+      return BGP_ATTR_PARSE_ERROR;
+  }
+  
+  /* Partial optional attributes that are malformed should not cause
+   * the whole session to be reset. Instead treat it as a withdrawal
+   * of the routes, if possible.
+   */
+  if (CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS)
+      && CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)
+      && CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
+    return BGP_ATTR_PARSE_WITHDRAW;
+  
+  /* default to reset */
+  return BGP_ATTR_PARSE_ERROR;
+}
+
 /* Get origin attribute of the update message. */
-static int
+static bgp_attr_parse_ret_t
 bgp_attr_origin (struct peer *peer, bgp_size_t length, 
 		 struct attr *attr, u_char flag, u_char *startp)
 {
@@ -699,11 +777,9 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length,
     {
       zlog (peer->log, LOG_ERR, 
 	    "Origin attribute flag isn't transitive %d", flag);
-      bgp_notify_send_with_data (peer, 
-				 BGP_NOTIFY_UPDATE_ERR, 
-				 BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
-				 startp, total);
-      return -1;
+      return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag,
+                                 BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
+                                 startp, total);
     }
 
   /* If any recognized attribute has Attribute Length that conflicts
@@ -715,10 +791,9 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length,
     {
       zlog (peer->log, LOG_ERR, "Origin attribute length is not one %d",
 	    length);
-      bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, 
-				 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
-				 startp, total);
-      return -1;
+      return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag,
+                                 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 startp, total);
     }
 
   /* Fetch origin attribute. */
@@ -733,12 +808,9 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length,
     {
       zlog (peer->log, LOG_ERR, "Origin attribute value is invalid %d",
 	      attr->origin);
-
-      bgp_notify_send_with_data (peer, 
-				 BGP_NOTIFY_UPDATE_ERR, 
-				 BGP_NOTIFY_UPDATE_INVAL_ORIGIN,
-				 startp, total);
-      return -1;
+      return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag,
+                                 BGP_NOTIFY_UPDATE_INVAL_ORIGIN,
+                                 startp, total);
     }
 
   /* Set oring attribute flag. */
@@ -746,82 +818,54 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length,
 
   return 0;
 }
-/* Parse AS path information.  This function is wrapper of aspath_parse.
- *
- * Parses AS_PATH or AS4_PATH.
- *
- * Returns: if valid: address of struct aspath in the hash of known aspaths,
- *                    with reference count incremented.
- *              else: NULL
- *
- * NB: empty AS path (length == 0) is valid.  The returned struct aspath will
- *     have segments == NULL and str == zero length string (unique).
- */
-static struct aspath *
+
+/* Parse AS path information.  This function is wrapper of
+   aspath_parse. */
+static int
 bgp_attr_aspath (struct peer *peer, bgp_size_t length, 
-		 struct attr *attr, u_char flag, u_char *startp, int as4_path)
+		 struct attr *attr, u_char flag, u_char *startp)
 {
-  u_char require ;
-  struct aspath *asp ;
+  bgp_size_t total;
 
-  /* Check the attribute flags                                          */
-  require = as4_path ? BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS
-                     :                          BGP_ATTR_FLAG_TRANS ;
+  total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
 
-  if ((flag & (BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS)) != require)
+  /* Flag check. */
+  if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)
+      || ! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
     {
-      const char* path_type ;
-      bgp_size_t total;
-
-      path_type = as4_path ? "AS4_PATH" : "AS_PATH" ;
-
-      if (!CHECK_FLAG(flag, BGP_ATTR_FLAG_TRANS))
       zlog (peer->log, LOG_ERR, 
-            "%s attribute flag isn't transitive %d", path_type, flag) ;
-
-      if ((flag & BGP_ATTR_FLAG_OPTIONAL) != (require & BGP_ATTR_FLAG_OPTIONAL))
-        zlog (peer->log, LOG_ERR,
-            "%s attribute flag must %sbe optional %d", path_type,
-            (flag & BGP_ATTR_FLAG_OPTIONAL) ? "not " : "", flag) ;
-
-      total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
-
-      bgp_notify_send_with_data (peer, 
-				 BGP_NOTIFY_UPDATE_ERR, 
-				 BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
-				 startp, total);
-
-      return NULL ;
-    } ;
+	    "As-Path attribute flag isn't transitive %d", flag);
+      return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
+                                 BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
+                                 startp, total);
+    }
 
-  /* Parse the AS_PATH/AS4_PATH body.
-   *
-   * For AS_PATH  peer with AS4 => 4Byte ASN otherwise 2Byte ASN
-   *     AS4_PATH 4Byte ASN
+  /*
+   * peer with AS4 => will get 4Byte ASnums
+   * otherwise, will get 16 Bit
    */
-  asp = aspath_parse (peer->ibuf, length,
-               as4_path || CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV), as4_path) ;
+  attr->aspath = aspath_parse (peer->ibuf, length, 
+                               CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV));
 
-  if (asp != NULL)
+  /* In case of IBGP, length will be zero. */
+  if (! attr->aspath)
     {
-      attr->flag |= ATTR_FLAG_BIT (as4_path ? BGP_ATTR_AS4_PATH
-                                            : BGP_ATTR_AS_PATH) ;
+      zlog (peer->log, LOG_ERR,
+            "Malformed AS path from %s, length is %d",
+            peer->host, length);
+      return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
+                                 BGP_NOTIFY_UPDATE_MAL_AS_PATH,
+                                 NULL, 0);
     }
-  else
-    {
-      zlog (peer->log, LOG_ERR, "Malformed AS path length is %d", length);
 
-      /* TODO: should BGP_NOTIFY_UPDATE_MAL_AS_PATH be sent for AS4_PATH ??  */
-      bgp_notify_send (peer, 
-		       BGP_NOTIFY_UPDATE_ERR, 
-		       BGP_NOTIFY_UPDATE_MAL_AS_PATH);
-    } ;
+  /* Set aspath attribute flag. */
+  attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS_PATH);
 
-  return asp ;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
-static int bgp_attr_aspath_check( struct peer *peer, 
-		struct attr *attr)
+static bgp_attr_parse_ret_t
+bgp_attr_aspath_check (struct peer *peer, struct attr *attr, u_char flag)
 {
   /* These checks were part of bgp_attr_aspath, but with
    * as4 we should to check aspath things when
@@ -840,10 +884,9 @@ static int bgp_attr_aspath_check( struct peer *peer,
      (peer_sort (peer) == BGP_PEER_EBGP && aspath_confed_check (attr->aspath)))
     {
       zlog (peer->log, LOG_ERR, "Malformed AS path from %s", peer->host);
-      bgp_notify_send (peer, 
-		       BGP_NOTIFY_UPDATE_ERR, 
-		       BGP_NOTIFY_UPDATE_MAL_AS_PATH);
-      return -1;
+      return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
+                                 BGP_NOTIFY_UPDATE_MAL_AS_PATH,
+                                 NULL, 0);
     }
 
   /* First AS check for EBGP. */
@@ -854,10 +897,9 @@ static int bgp_attr_aspath_check( struct peer *peer,
  	{
  	  zlog (peer->log, LOG_ERR,
  		"%s incorrect first AS (must be %u)", peer->host, peer->as);
- 	  bgp_notify_send (peer,
- 			   BGP_NOTIFY_UPDATE_ERR,
- 			   BGP_NOTIFY_UPDATE_MAL_AS_PATH);
-	  return -1;
+          return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
+                                     BGP_NOTIFY_UPDATE_MAL_AS_PATH,
+                                     NULL, 0);
  	}
     }
 
@@ -867,16 +909,57 @@ static int bgp_attr_aspath_check( struct peer *peer,
     {
       aspath = aspath_dup (attr->aspath);
       aspath = aspath_add_seq (aspath, peer->change_local_as);
-      aspath_unintern (attr->aspath);
+      aspath_unintern (&attr->aspath);
       attr->aspath = aspath_intern (aspath);
     }
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
+}
 
+/* Parse AS4 path information.  This function is another wrapper of
+   aspath_parse. */
+static int
+bgp_attr_as4_path (struct peer *peer, bgp_size_t length,
+		 struct attr *attr, u_char flag, u_char *startp,
+		 struct aspath **as4_path)
+{
+  bgp_size_t total;
+
+  total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
+
+  /* Flag check. */
+  if (!CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)
+      || !CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
+    {
+      zlog (peer->log, LOG_ERR, 
+	    "As4-Path attribute flag isn't optional/transitive %d", flag);
+      return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
+                                 BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
+                                 startp, total);
+    }
+
+  *as4_path = aspath_parse (peer->ibuf, length, 1);
+
+  /* In case of IBGP, length will be zero. */
+  if (!*as4_path)
+    {
+      zlog (peer->log, LOG_ERR,
+            "Malformed AS4 path from %s, length is %d",
+            peer->host, length);
+      return bgp_attr_malformed (peer, BGP_ATTR_AS4_PATH, flag,
+                                 BGP_NOTIFY_UPDATE_MAL_AS_PATH,
+                                 NULL, 0);
+    }
+
+  /* Set aspath attribute flag. */
+  if (as4_path)
+    attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH);
+
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Nexthop attribute. */
-static int
+static bgp_attr_parse_ret_t
 bgp_attr_nexthop (struct peer *peer, bgp_size_t length, 
 		  struct attr *attr, u_char flag, u_char *startp)
 {
@@ -890,11 +973,9 @@ bgp_attr_nexthop (struct peer *peer, bgp_size_t length,
     {
       zlog (peer->log, LOG_ERR, 
 	    "Origin attribute flag isn't transitive %d", flag);
-      bgp_notify_send_with_data (peer, 
-				 BGP_NOTIFY_UPDATE_ERR, 
-				 BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
-				 startp, total);
-      return -1;
+      return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag,
+                                 BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
+                                 startp, total);
     }
 
   /* Check nexthop attribute length. */
@@ -903,21 +984,19 @@ bgp_attr_nexthop (struct peer *peer, bgp_size_t length,
       zlog (peer->log, LOG_ERR, "Nexthop attribute length isn't four [%d]",
 	      length);
 
-      bgp_notify_send_with_data (peer, 
-				 BGP_NOTIFY_UPDATE_ERR, 
-				 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
-				 startp, total);
-      return -1;
+      return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag,
+                                 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 startp, total);
     }
 
   attr->nexthop.s_addr = stream_get_ipv4 (peer->ibuf);
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* MED atrribute. */
-static int
+static bgp_attr_parse_ret_t
 bgp_attr_med (struct peer *peer, bgp_size_t length, 
 	      struct attr *attr, u_char flag, u_char *startp)
 {
@@ -930,23 +1009,21 @@ bgp_attr_med (struct peer *peer, bgp_size_t length,
     {
       zlog (peer->log, LOG_ERR, 
 	    "MED attribute length isn't four [%d]", length);
-      
-      bgp_notify_send_with_data (peer, 
-				 BGP_NOTIFY_UPDATE_ERR, 
-				 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
-				 startp, total);
-      return -1;
+
+      return bgp_attr_malformed (peer, BGP_ATTR_MULTI_EXIT_DISC, flag,
+                                 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 startp, total);
     }
 
   attr->med = stream_getl (peer->ibuf);
 
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Local preference attribute. */
-static int
+static bgp_attr_parse_ret_t
 bgp_attr_local_pref (struct peer *peer, bgp_size_t length, 
 		     struct attr *attr, u_char flag)
 {
@@ -956,7 +1033,7 @@ bgp_attr_local_pref (struct peer *peer, bgp_size_t length,
   if (peer_sort (peer) == BGP_PEER_EBGP)
     {
       stream_forward_getp (peer->ibuf, length);
-      return 0;
+      return BGP_ATTR_PARSE_PROCEED;
     }
 
   if (length == 4) 
@@ -967,7 +1044,7 @@ bgp_attr_local_pref (struct peer *peer, bgp_size_t length,
   /* Set atomic aggregate flag. */
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_LOCAL_PREF);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Atomic aggregate. */
@@ -979,16 +1056,15 @@ bgp_attr_atomic (struct peer *peer, bgp_size_t length,
     {
       zlog (peer->log, LOG_ERR, "Bad atomic aggregate length %d", length);
 
-      bgp_notify_send (peer, 
-		       BGP_NOTIFY_UPDATE_ERR, 
-		       BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
-      return -1;
+      return bgp_attr_malformed (peer, BGP_ATTR_ATOMIC_AGGREGATE, flag,
+                                 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 NULL, 0);
     }
 
   /* Set atomic aggregate flag. */
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_ATOMIC_AGGREGATE);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Aggregator attribute */
@@ -1000,17 +1076,16 @@ bgp_attr_aggregator (struct peer *peer, bgp_size_t length,
   struct attr_extra *attre = bgp_attr_extra_get (attr);
   
   /* peer with AS4 will send 4 Byte AS, peer without will send 2 Byte */
-  if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV ) )
+  if (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV))
     wantedlen = 8;
   
   if (length != wantedlen)
     {
       zlog (peer->log, LOG_ERR, "Aggregator length is not %d [%d]", wantedlen, length);
 
-      bgp_notify_send (peer,
-		       BGP_NOTIFY_UPDATE_ERR,
-		       BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
-      return -1;
+      return bgp_attr_malformed (peer, BGP_ATTR_AGGREGATOR, flag,
+                                 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 NULL, 0);
     }
   
   if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV ) )
@@ -1022,36 +1097,35 @@ bgp_attr_aggregator (struct peer *peer, bgp_size_t length,
   /* Set atomic aggregate flag. */
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* New Aggregator attribute */
-static int
+static bgp_attr_parse_ret_t
 bgp_attr_as4_aggregator (struct peer *peer, bgp_size_t length,
-		     struct attr *attr, as_t *as4_aggregator_as,
+		     struct attr *attr, u_char flag, 
+		     as_t *as4_aggregator_as,
 		     struct in_addr *as4_aggregator_addr)
 {
   if (length != 8)
     {
       zlog (peer->log, LOG_ERR, "New Aggregator length is not 8 [%d]", length);
-
-      bgp_notify_send (peer,
-		       BGP_NOTIFY_UPDATE_ERR,
-		       BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
-      return -1;
+      return bgp_attr_malformed (peer, BGP_ATTR_AS4_AGGREGATOR, flag,
+                                 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 NULL, 0);
     }
   *as4_aggregator_as = stream_getl (peer->ibuf);
   as4_aggregator_addr->s_addr = stream_get_ipv4 (peer->ibuf);
 
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Munge Aggregator and New-Aggregator, AS_PATH and NEW_AS_PATH.
  */
-static int
-bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
+static bgp_attr_parse_ret_t
+bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr, u_char flag,
                           struct aspath *as4_path, as_t as4_aggregator,
                           struct in_addr *as4_aggregator_addr)
 {
@@ -1059,7 +1133,7 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
   struct aspath *newpath;
   struct attr_extra *attre = attr->extra;
     
-  if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV) )
+  if (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV))
     {
       /* peer can do AS4, so we ignore AS4_PATH and AS4_AGGREGATOR
        * if given.
@@ -1077,11 +1151,11 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
                         peer->host, "AS4 capable peer, yet it sent");
         }
       
-      return 0;
+      return BGP_ATTR_PARSE_PROCEED;
     }
   
-  if (attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS4_PATH))
-      && !(attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS_PATH))))
+  if (attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH))
+      && !(attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS_PATH))))
     {
       /* Hu? This is not supposed to happen at all!
        * got as4_path and no aspath,
@@ -1093,10 +1167,9 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
       zlog (peer->log, LOG_ERR, 
             "%s BGP not AS4 capable peer sent AS4_PATH but"
             " no AS_PATH, cant do anything here", peer->host);
-      bgp_notify_send (peer, 
-                       BGP_NOTIFY_UPDATE_ERR, 
-                       BGP_NOTIFY_UPDATE_MAL_ATTR);
-      return -1;
+      return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
+                                 BGP_NOTIFY_UPDATE_MAL_ATTR,
+                                 NULL, 0);
     }
 
   /* We have a asn16 peer.  First, look for AS4_AGGREGATOR
@@ -1104,7 +1177,7 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
    */
   if (attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR) ) )
     {
-      if ( attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR) ) )
+      if (attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR) ) )
         {
           assert (attre);
           
@@ -1120,7 +1193,7 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
            *        Aggregating node and the AS_PATH is to be
            *        constructed "as in all other cases"
            */
-          if ( attre->aggregator_as != BGP_AS_TRANS )
+          if (attre->aggregator_as != BGP_AS_TRANS)
             {
               /* ignore */
               if ( BGP_DEBUG(as4, AS4))
@@ -1155,24 +1228,27 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
     }
 
   /* need to reconcile NEW_AS_PATH and AS_PATH */
-  if ( !ignore_as4_path && (attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS4_PATH))) )
+  if (!ignore_as4_path && (attr->flag & (ATTR_FLAG_BIT( BGP_ATTR_AS4_PATH))))
     {
        newpath = aspath_reconcile_as4 (attr->aspath, as4_path);
-       aspath_unintern (attr->aspath);
+       aspath_unintern (&attr->aspath);
        attr->aspath = aspath_intern (newpath);
     }
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Community attribute. */
-static int
+static bgp_attr_parse_ret_t
 bgp_attr_community (struct peer *peer, bgp_size_t length, 
-		    struct attr *attr, u_char flag)
+		    struct attr *attr, u_char flag, u_char *startp)
 {
+  bgp_size_t total
+    = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
+  
   if (length == 0)
     {
       attr->community = NULL;
-      return 0;
+      return BGP_ATTR_PARSE_PROCEED;
     }
   
   attr->community =
@@ -1182,15 +1258,17 @@ bgp_attr_community (struct peer *peer, bgp_size_t length,
   stream_forward_getp (peer->ibuf, length);
 
   if (!attr->community)
-    return -1;
+    return bgp_attr_malformed (peer, BGP_ATTR_COMMUNITIES, flag,
+                               BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
+                               startp, total);
   
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_COMMUNITIES);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Originator ID attribute. */
-static int
+static bgp_attr_parse_ret_t
 bgp_attr_originator_id (struct peer *peer, bgp_size_t length, 
 			struct attr *attr, u_char flag)
 {
@@ -1198,10 +1276,9 @@ bgp_attr_originator_id (struct peer *peer, bgp_size_t length,
     {
       zlog (peer->log, LOG_ERR, "Bad originator ID length %d", length);
 
-      bgp_notify_send (peer, 
-		       BGP_NOTIFY_UPDATE_ERR, 
-		       BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
-      return -1;
+      return bgp_attr_malformed (peer, BGP_ATTR_ORIGINATOR_ID, flag,
+                                 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 NULL, 0);
     }
 
   (bgp_attr_extra_get (attr))->originator_id.s_addr 
@@ -1209,11 +1286,11 @@ bgp_attr_originator_id (struct peer *peer, bgp_size_t length,
 
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_ORIGINATOR_ID);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Cluster list attribute. */
-static int
+static bgp_attr_parse_ret_t
 bgp_attr_cluster_list (struct peer *peer, bgp_size_t length, 
 		       struct attr *attr, u_char flag)
 {
@@ -1222,20 +1299,20 @@ bgp_attr_cluster_list (struct peer *peer, bgp_size_t length,
     {
       zlog (peer->log, LOG_ERR, "Bad cluster list length %d", length);
 
-      bgp_notify_send (peer, 
-		       BGP_NOTIFY_UPDATE_ERR, 
-		       BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
-      return -1;
+      return bgp_attr_malformed (peer, BGP_ATTR_CLUSTER_LIST, flag,
+                                 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 NULL, 0);
     }
 
   (bgp_attr_extra_get (attr))->cluster 
     = cluster_parse ((struct in_addr *)stream_pnt (peer->ibuf), length);
-
-  stream_forward_getp (peer->ibuf, length);;
+  
+  /* XXX: Fix cluster_parse to use stream API and then remove this */
+  stream_forward_getp (peer->ibuf, length);
 
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_CLUSTER_LIST);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Multiprotocol reachability information parse. */
@@ -1262,7 +1339,7 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
     {
       zlog_info ("%s: %s sent invalid length, %lu", 
 		 __func__, peer->host, (unsigned long)length);
-      return -1;
+      return BGP_ATTR_PARSE_ERROR;
     }
   
   /* Load AFI, SAFI. */
@@ -1276,7 +1353,7 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
     {
       zlog_info ("%s: %s, MP nexthop length, %u, goes past end of attribute", 
 		 __func__, peer->host, attre->mp_nexthop_len);
-      return -1;
+      return BGP_ATTR_PARSE_ERROR;
     }
   
   /* Nexthop length check. */
@@ -1324,14 +1401,14 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
     default:
       zlog_info ("%s: (%s) Wrong multiprotocol next hop length: %d", 
 		 __func__, peer->host, attre->mp_nexthop_len);
-      return -1;
+      return BGP_ATTR_PARSE_ERROR;
     }
 
   if (!LEN_LEFT)
     {
       zlog_info ("%s: (%s) Failed to read SNPA and NLRI(s)",
                  __func__, peer->host);
-      return -1;
+      return BGP_ATTR_PARSE_ERROR;
     }
   
   {
@@ -1347,7 +1424,7 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
     {
       zlog_info ("%s: (%s) Failed to read NLRI",
                  __func__, peer->host);
-      return -1;
+      return BGP_ATTR_PARSE_ERROR;
     }
  
   if (safi != BGP_SAFI_VPNV4)
@@ -1357,7 +1434,7 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
         {
           zlog_info ("%s: (%s) NLRI doesn't pass sanity check",
                      __func__, peer->host);
-	  return -1;
+	  return BGP_ATTR_PARSE_ERROR;
 	}
     }
 
@@ -1368,7 +1445,7 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
 
   stream_forward_getp (s, nlri_len);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 #undef LEN_LEFT
 }
 
@@ -1387,7 +1464,7 @@ bgp_mp_unreach_parse (struct peer *peer, bgp_size_t length,
   
 #define BGP_MP_UNREACH_MIN_SIZE 3
   if ((length > STREAM_READABLE(s)) || (length <  BGP_MP_UNREACH_MIN_SIZE))
-    return -1;
+    return BGP_ATTR_PARSE_ERROR;
   
   afi = stream_getw (s);
   safi = stream_getc (s);
@@ -1398,7 +1475,7 @@ bgp_mp_unreach_parse (struct peer *peer, bgp_size_t length,
     {
       ret = bgp_nlri_sanity_check (peer, afi, stream_pnt (s), withdraw_len);
       if (ret < 0)
-	return -1;
+	return BGP_ATTR_PARSE_ERROR;
     }
 
   mp_withdraw->afi = afi;
@@ -1408,20 +1485,23 @@ bgp_mp_unreach_parse (struct peer *peer, bgp_size_t length,
 
   stream_forward_getp (s, withdraw_len);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Extended Community attribute. */
-static int
+static bgp_attr_parse_ret_t
 bgp_attr_ext_communities (struct peer *peer, bgp_size_t length, 
-			  struct attr *attr, u_char flag)
+			  struct attr *attr, u_char flag, u_char *startp)
 {
+  bgp_size_t total
+    = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
+  
   if (length == 0)
     {
       if (attr->extra)
         attr->extra->ecommunity = NULL;
       /* Empty extcomm doesn't seem to be invalid per se */
-      return 0;
+      return BGP_ATTR_PARSE_PROCEED;
     }
 
   (bgp_attr_extra_get (attr))->ecommunity =
@@ -1430,15 +1510,17 @@ bgp_attr_ext_communities (struct peer *peer, bgp_size_t length,
   stream_forward_getp (peer->ibuf, length);
   
   if (!attr->extra->ecommunity)
-    return -1;
+    return bgp_attr_malformed (peer, BGP_ATTR_EXT_COMMUNITIES,
+                               flag, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
+                               startp, total);
   
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* BGP unknown attribute treatment. */
-static int
+static bgp_attr_parse_ret_t
 bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag,
 		  u_char type, bgp_size_t length, u_char *startp)
 {
@@ -1464,20 +1546,17 @@ bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag,
      then the Error Subcode is set to Unrecognized Well-known
      Attribute.  The Data field contains the unrecognized attribute
      (type, length and value). */
-  if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
+  if (!CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
     {
-      /* Adjust startp to do not include flag value. */
-      bgp_notify_send_with_data (peer, 
-				 BGP_NOTIFY_UPDATE_ERR, 
-				 BGP_NOTIFY_UPDATE_UNREC_ATTR,
-				 startp, total);
-      return -1;
+      return bgp_attr_malformed (peer, type, flag,
+                                 BGP_NOTIFY_UPDATE_UNREC_ATTR,
+                                 startp, total);
     }
 
   /* Unrecognized non-transitive optional attributes must be quietly
      ignored and not passed along to other BGP peers. */
   if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
-    return 0;
+    return BGP_ATTR_PARSE_PROCEED;
 
   /* If a path with recognized transitive optional attribute is
      accepted and passed along to other BGP peers and the Partial bit
@@ -1500,17 +1579,17 @@ bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag,
   memcpy (transit->val + transit->length, startp, total);
   transit->length += total;
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Read attribute of update packet.  This function is called from
    bgp_update() in bgpd.c.  */
-int
+bgp_attr_parse_ret_t
 bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 		struct bgp_nlri *mp_update, struct bgp_nlri *mp_withdraw)
 {
   int ret;
-  u_char flag;
+  u_char flag = 0;
   u_char type = 0;
   bgp_size_t length;
   u_char *startp, *endp;
@@ -1527,7 +1606,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 
   /* End pointer of BGP attribute. */
   endp = BGP_INPUT_PNT (peer) + size;
-
+  
   /* Get attributes to the end of attribute length. */
   while (BGP_INPUT_PNT (peer) < endp)
     {
@@ -1543,7 +1622,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 	  bgp_notify_send (peer, 
 			   BGP_NOTIFY_UPDATE_ERR, 
 			   BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
-	  return -1;
+	  return BGP_ATTR_PARSE_ERROR;
 	}
 
       /* Fetch attribute flag and type. */
@@ -1563,7 +1642,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 	  bgp_notify_send (peer, 
 			   BGP_NOTIFY_UPDATE_ERR, 
 			   BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
-	  return -1;
+	  return BGP_ATTR_PARSE_ERROR;
 	}
 
       /* Check extended attribue length bit. */
@@ -1585,7 +1664,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 	  bgp_notify_send (peer, 
 			   BGP_NOTIFY_UPDATE_ERR, 
 			   BGP_NOTIFY_UPDATE_MAL_ATTR);
-	  return -1;
+	  return BGP_ATTR_PARSE_ERROR;
 	}
 
       /* Set type to bitmap to check duplicate attribute.  `type' is
@@ -1603,7 +1682,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 	  bgp_notify_send (peer, 
 			   BGP_NOTIFY_UPDATE_ERR, 
 			   BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
-	  return -1;
+	  return BGP_ATTR_PARSE_ERROR;
 	}
 
       /* OK check attribute and store it's value. */
@@ -1613,12 +1692,10 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 	  ret = bgp_attr_origin (peer, length, attr, flag, startp);
 	  break;
 	case BGP_ATTR_AS_PATH:
-          attr->aspath = bgp_attr_aspath (peer, length, attr, flag, startp, 0);
-          ret = attr->aspath ? 0 : -1 ;
+	  ret = bgp_attr_aspath (peer, length, attr, flag, startp);
 	  break;
 	case BGP_ATTR_AS4_PATH:
-          as4_path = bgp_attr_aspath (peer, length, attr, flag, startp, 1);
-          ret = as4_path  ? 0 : -1 ;
+	  ret = bgp_attr_as4_path (peer, length, attr, flag, startp, &as4_path);
 	  break;
 	case BGP_ATTR_NEXT_HOP:	
 	  ret = bgp_attr_nexthop (peer, length, attr, flag, startp);
@@ -1636,10 +1713,12 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 	  ret = bgp_attr_aggregator (peer, length, attr, flag);
 	  break;
 	case BGP_ATTR_AS4_AGGREGATOR:
-	  ret = bgp_attr_as4_aggregator (peer, length, attr, &as4_aggregator, &as4_aggregator_addr);
+	  ret = bgp_attr_as4_aggregator (peer, length, attr, flag,
+	                                 &as4_aggregator, 
+	                                 &as4_aggregator_addr);
 	  break;
 	case BGP_ATTR_COMMUNITIES:
-	  ret = bgp_attr_community (peer, length, attr, flag);
+	  ret = bgp_attr_community (peer, length, attr, flag, startp);
 	  break;
 	case BGP_ATTR_ORIGINATOR_ID:
 	  ret = bgp_attr_originator_id (peer, length, attr, flag);
@@ -1654,26 +1733,39 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 	  ret = bgp_mp_unreach_parse (peer, length, mp_withdraw);
 	  break;
 	case BGP_ATTR_EXT_COMMUNITIES:
-	  ret = bgp_attr_ext_communities (peer, length, attr, flag);
+	  ret = bgp_attr_ext_communities (peer, length, attr, flag, startp);
 	  break;
 	default:
 	  ret = bgp_attr_unknown (peer, attr, flag, type, length, startp);
 	  break;
 	}
-
-      /* If error occured immediately return to the caller. */
-      if (ret < 0)
+      
+      /* If hard error occured immediately return to the caller. */
+      if (ret == BGP_ATTR_PARSE_ERROR)
         {
           zlog (peer->log, LOG_WARNING,
                 "%s: Attribute %s, parse error", 
                 peer->host, 
                 LOOKUP (attr_str, type));
-           bgp_notify_send (peer, 
-                            BGP_NOTIFY_UPDATE_ERR,
-                            BGP_NOTIFY_UPDATE_MAL_ATTR);
-           return ret;
+          bgp_notify_send (peer, 
+                           BGP_NOTIFY_UPDATE_ERR,
+                           BGP_NOTIFY_UPDATE_MAL_ATTR);
+          if (as4_path)
+            aspath_unintern (&as4_path);
+          return ret;
         }
-
+      if (ret == BGP_ATTR_PARSE_WITHDRAW)
+        {
+          
+          zlog (peer->log, LOG_WARNING,
+                "%s: Attribute %s, parse error - treating as withdrawal",
+                peer->host,
+                LOOKUP (attr_str, type));
+          if (as4_path)
+            aspath_unintern (&as4_path);
+          return ret;
+        }
+      
       /* Check the fetched length. */
       if (BGP_INPUT_PNT (peer) != attr_endp)
 	{
@@ -1683,7 +1775,9 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 	  bgp_notify_send (peer, 
 			   BGP_NOTIFY_UPDATE_ERR, 
 			   BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
-	  return -1;
+          if (as4_path)
+            aspath_unintern (&as4_path);
+	  return BGP_ATTR_PARSE_ERROR;
 	}
     }
 
@@ -1696,7 +1790,9 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
       bgp_notify_send (peer, 
 		       BGP_NOTIFY_UPDATE_ERR, 
 		       BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
-      return -1;
+      if (as4_path)
+        aspath_unintern (&as4_path);
+      return BGP_ATTR_PARSE_ERROR;
     }
 
   /* 
@@ -1710,19 +1806,22 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
    * all attributes first, including these 32bit ones, and now,
    * afterwards, we look what and if something is to be done for as4.
    */
-  if (bgp_attr_munge_as4_attrs (peer, attr, as4_path,
+  if (bgp_attr_munge_as4_attrs (peer, attr, flag, as4_path,
                                 as4_aggregator, &as4_aggregator_addr))
-    return -1;
+    {
+      if (as4_path)
+        aspath_unintern (&as4_path);
+      return BGP_ATTR_PARSE_ERROR;
+    }
 
   /* At this stage, we have done all fiddling with as4, and the
    * resulting info is in attr->aggregator resp. attr->aspath
    * so we can chuck as4_aggregator and as4_path alltogether in
    * order to save memory
    */
-  if ( as4_path )
+  if (as4_path)
     {
-      aspath_unintern( as4_path ); /* unintern - it is in the hash */
-      as4_path = NULL;
+      aspath_unintern (&as4_path); /* unintern - it is in the hash */
       /* The flag that we got this is still there, but that does not
        * do any trouble
        */
@@ -1737,10 +1836,10 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
    * Finally do the checks on the aspath we did not do yet
    * because we waited for a potentially synthesized aspath.
    */
-  if ( attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS_PATH)))
+  if (attr->flag & (ATTR_FLAG_BIT(BGP_ATTR_AS_PATH)))
     {
-      ret = bgp_attr_aspath_check( peer, attr );
-      if ( ret < 0 )
+      ret = bgp_attr_aspath_check (peer, attr, flag);
+      if (ret != BGP_ATTR_PARSE_PROCEED)
 	return ret;
     }
 
@@ -1748,7 +1847,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
   if (attr->extra && attr->extra->transit)
     attr->extra->transit = transit_intern (attr->extra->transit);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Well-known attribute check. */
@@ -1779,9 +1878,9 @@ bgp_attr_check (struct peer *peer, struct attr *attr)
 				 BGP_NOTIFY_UPDATE_ERR, 
 				 BGP_NOTIFY_UPDATE_MISS_ATTR,
 				 &type, 1);
-      return -1;
+      return BGP_ATTR_PARSE_ERROR;
     }
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 int stream_put_prefix (struct stream *, struct prefix *);
diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
index af9dcf5e..c0112514 100644
--- a/bgpd/bgp_attr.h
+++ b/bgpd/bgp_attr.h
@@ -132,17 +132,25 @@ struct transit
 
 #define ATTR_FLAG_BIT(X)  (1 << ((X) - 1))
 
+typedef enum {
+ BGP_ATTR_PARSE_PROCEED = 0,
+ BGP_ATTR_PARSE_ERROR = -1,
+ BGP_ATTR_PARSE_WITHDRAW = -2,
+} bgp_attr_parse_ret_t;
+
 /* Prototypes. */
 extern void bgp_attr_init (void);
 extern void bgp_attr_finish (void);
-extern int bgp_attr_parse (struct peer *, struct attr *, bgp_size_t,
-		    struct bgp_nlri *, struct bgp_nlri *);
+extern bgp_attr_parse_ret_t bgp_attr_parse (struct peer *, struct attr *,
+                                           bgp_size_t, struct bgp_nlri *,
+                                           struct bgp_nlri *);
 extern int bgp_attr_check (struct peer *, struct attr *);
 extern struct attr_extra *bgp_attr_extra_get (struct attr *);
 extern void bgp_attr_extra_free (struct attr *);
 extern void bgp_attr_dup (struct attr *, struct attr *);
 extern struct attr *bgp_attr_intern (struct attr *attr);
-extern void bgp_attr_unintern (struct attr *);
+extern void bgp_attr_unintern_sub (struct attr *);
+extern void bgp_attr_unintern (struct attr **);
 extern void bgp_attr_flush (struct attr *);
 extern struct attr *bgp_attr_default_set (struct attr *attr, u_char);
 extern struct attr *bgp_attr_default_intern (u_char);
diff --git a/bgpd/bgp_clist.c b/bgpd/bgp_clist.c
index d6601674..6c9976e3 100644
--- a/bgpd/bgp_clist.c
+++ b/bgpd/bgp_clist.c
@@ -70,7 +70,7 @@ community_entry_free (struct community_entry *entry)
       if (entry->config)
         XFREE (MTYPE_ECOMMUNITY_STR, entry->config);
       if (entry->u.ecom)
-        ecommunity_free (entry->u.ecom);
+        ecommunity_free (&entry->u.ecom);
       break;
     case COMMUNITY_LIST_EXPANDED:
     case EXTCOMMUNITY_LIST_EXPANDED:
@@ -806,7 +806,7 @@ extcommunity_list_unset (struct community_list_handler *ch,
     entry = community_list_entry_lookup (list, str, direct);
 
   if (ecom)
-    ecommunity_free (ecom);
+    ecommunity_free (&ecom);
   if (regex)
     bgp_regex_free (regex);
 
diff --git a/bgpd/bgp_community.c b/bgpd/bgp_community.c
index ae1d7a15..2ba45f6e 100644
--- a/bgpd/bgp_community.c
+++ b/bgpd/bgp_community.c
@@ -321,21 +321,22 @@ community_intern (struct community *com)
 
 /* Free community attribute. */
 void
-community_unintern (struct community *com)
+community_unintern (struct community **com)
 {
   struct community *ret;
 
-  if (com->refcnt)
-    com->refcnt--;
+  if ((*com)->refcnt)
+    (*com)->refcnt--;
 
   /* Pull off from hash.  */
-  if (com->refcnt == 0)
+  if ((*com)->refcnt == 0)
     {
       /* Community value com must exist in hash. */
-      ret = (struct community *) hash_release (comhash, com);
+      ret = (struct community *) hash_release (comhash, *com);
       assert (ret != NULL);
 
-      community_free (com);
+      community_free (*com);
+      *com = NULL;
     }
 }
 
diff --git a/bgpd/bgp_community.h b/bgpd/bgp_community.h
index bc1e56ef..9e483770 100644
--- a/bgpd/bgp_community.h
+++ b/bgpd/bgp_community.h
@@ -57,7 +57,7 @@ extern void community_free (struct community *);
 extern struct community *community_uniq_sort (struct community *);
 extern struct community *community_parse (u_int32_t *, u_short);
 extern struct community *community_intern (struct community *);
-extern void community_unintern (struct community *);
+extern void community_unintern (struct community **);
 extern char *community_str (struct community *);
 extern unsigned int community_hash_make (struct community *);
 extern struct community *community_str2com (const char *);
diff --git a/bgpd/bgp_ecommunity.c b/bgpd/bgp_ecommunity.c
index 8d5fa741..8d91c746 100644
--- a/bgpd/bgp_ecommunity.c
+++ b/bgpd/bgp_ecommunity.c
@@ -42,13 +42,14 @@ ecommunity_new (void)
 
 /* Allocate ecommunities.  */
 void
-ecommunity_free (struct ecommunity *ecom)
+ecommunity_free (struct ecommunity **ecom)
 {
-  if (ecom->val)
-    XFREE (MTYPE_ECOMMUNITY_VAL, ecom->val);
-  if (ecom->str)
-    XFREE (MTYPE_ECOMMUNITY_STR, ecom->str);
-  XFREE (MTYPE_ECOMMUNITY, ecom);
+  if ((*ecom)->val)
+    XFREE (MTYPE_ECOMMUNITY_VAL, (*ecom)->val);
+  if ((*ecom)->str)
+    XFREE (MTYPE_ECOMMUNITY_STR, (*ecom)->str);
+  XFREE (MTYPE_ECOMMUNITY, *ecom);
+  ecom = NULL;
 }
 
 /* Add a new Extended Communities value to Extended Communities
@@ -197,7 +198,7 @@ ecommunity_intern (struct ecommunity *ecom)
   find = (struct ecommunity *) hash_get (ecomhash, ecom, hash_alloc_intern);
 
   if (find != ecom)
-    ecommunity_free (ecom);
+    ecommunity_free (&ecom);
 
   find->refcnt++;
 
@@ -209,18 +210,18 @@ ecommunity_intern (struct ecommunity *ecom)
 
 /* Unintern Extended Communities Attribute.  */
 void
-ecommunity_unintern (struct ecommunity *ecom)
+ecommunity_unintern (struct ecommunity **ecom)
 {
   struct ecommunity *ret;
 
-  if (ecom->refcnt)
-    ecom->refcnt--;
-
+  if ((*ecom)->refcnt)
+    (*ecom)->refcnt--;
+    
   /* Pull off from hash.  */
-  if (ecom->refcnt == 0)
+  if ((*ecom)->refcnt == 0)
     {
       /* Extended community must be in the hash.  */
-      ret = (struct ecommunity *) hash_release (ecomhash, ecom);
+      ret = (struct ecommunity *) hash_release (ecomhash, *ecom);
       assert (ret != NULL);
 
       ecommunity_free (ecom);
@@ -516,7 +517,7 @@ ecommunity_str2com (const char *str, int type, int keyword_included)
 	  if (! keyword_included || keyword)
 	    {
 	      if (ecom)
-		ecommunity_free (ecom);
+		ecommunity_free (&ecom);
 	      return NULL;
 	    }
 	  keyword = 1;
@@ -536,7 +537,7 @@ ecommunity_str2com (const char *str, int type, int keyword_included)
 	      if (! keyword)
 		{
 		  if (ecom)
-		    ecommunity_free (ecom);
+		    ecommunity_free (&ecom);
 		  return NULL;
 		}
 	      keyword = 0;
@@ -549,7 +550,7 @@ ecommunity_str2com (const char *str, int type, int keyword_included)
 	case ecommunity_token_unknown:
 	default:
 	  if (ecom)
-	    ecommunity_free (ecom);
+	    ecommunity_free (&ecom);
 	  return NULL;
 	}
     }
diff --git a/bgpd/bgp_ecommunity.h b/bgpd/bgp_ecommunity.h
index 942fdc73..2f59dc40 100644
--- a/bgpd/bgp_ecommunity.h
+++ b/bgpd/bgp_ecommunity.h
@@ -67,13 +67,13 @@ struct ecommunity_val
 
 extern void ecommunity_init (void);
 extern void ecommunity_finish (void);
-extern void ecommunity_free (struct ecommunity *);
+extern void ecommunity_free (struct ecommunity **);
 extern struct ecommunity *ecommunity_parse (u_int8_t *, u_short);
 extern struct ecommunity *ecommunity_dup (struct ecommunity *);
 extern struct ecommunity *ecommunity_merge (struct ecommunity *, struct ecommunity *);
 extern struct ecommunity *ecommunity_intern (struct ecommunity *);
 extern int ecommunity_cmp (const void *, const void *);
-extern void ecommunity_unintern (struct ecommunity *);
+extern void ecommunity_unintern (struct ecommunity **);
 extern unsigned int ecommunity_hash_make (void *);
 extern struct ecommunity *ecommunity_str2com (const char *, int, int);
 extern char *ecommunity_ecom2str (struct ecommunity *, int);
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index ed2cb73e..1d9fcc97 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -206,7 +206,7 @@ bgp_update_packet (struct peer *peer, afi_t afi, safi_t safi)
 
       /* Synchnorize attribute.  */
       if (adj->attr)
-	bgp_attr_unintern (adj->attr);
+	bgp_attr_unintern (&adj->attr);
       else
 	peer->scount[afi][safi]++;
 
@@ -1583,26 +1583,47 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
 		       BGP_NOTIFY_UPDATE_MAL_ATTR);
       return -1;
     }
+  
+  /* Certain attribute parsing errors should not be considered bad enough
+   * to reset the session for, most particularly any partial/optional
+   * attributes that have 'tunneled' over speakers that don't understand
+   * them. Instead we withdraw only the prefix concerned.
+   * 
+   * Complicates the flow a little though..
+   */
+  bgp_attr_parse_ret_t attr_parse_ret = BGP_ATTR_PARSE_PROCEED;
+  /* This define morphs the update case into a withdraw when lower levels
+   * have signalled an error condition where this is best.
+   */
+#define NLRI_ATTR_ARG (attr_parse_ret != BGP_ATTR_PARSE_WITHDRAW ? &attr : NULL)
 
   /* Parse attribute when it exists. */
   if (attribute_len)
     {
-      ret = bgp_attr_parse (peer, &attr, attribute_len, 
+      attr_parse_ret = bgp_attr_parse (peer, &attr, attribute_len, 
 			    &mp_update, &mp_withdraw);
-      if (ret < 0)
+      if (attr_parse_ret == BGP_ATTR_PARSE_ERROR)
 	return -1;
     }
-
+  
   /* Logging the attribute. */
-  if (BGP_DEBUG (update, UPDATE_IN))
+  if (attr_parse_ret == BGP_ATTR_PARSE_WITHDRAW
+      || BGP_DEBUG (update, UPDATE_IN))
     {
       ret= bgp_dump_attr (peer, &attr, attrstr, BUFSIZ);
+      int lvl = (attr_parse_ret == BGP_ATTR_PARSE_WITHDRAW)
+                 ? LOG_ERR : LOG_DEBUG;
+      
+      if (attr_parse_ret == BGP_ATTR_PARSE_WITHDRAW)
+        zlog (peer->log, LOG_ERR,
+              "%s rcvd UPDATE with errors in attr(s)!! Withdrawing route.",
+              peer->host);
 
       if (ret)
-	zlog (peer->log, LOG_DEBUG, "%s rcvd UPDATE w/ attr: %s",
+	zlog (peer->log, lvl, "%s rcvd UPDATE w/ attr: %s",
 	      peer->host, attrstr);
     }
-
+  
   /* Network Layer Reachability Information. */
   update_len = end - stream_pnt (s);
 
@@ -1611,7 +1632,12 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
       /* Check NLRI packet format and prefix length. */
       ret = bgp_nlri_sanity_check (peer, AFI_IP, stream_pnt (s), update_len);
       if (ret < 0)
-	return -1;
+        {
+          bgp_attr_unintern_sub (&attr);
+          if (attr.extra)
+            bgp_attr_extra_free (&attr);
+	  return -1;
+	}
 
       /* Set NLRI portion to structure. */
       update.afi = AFI_IP;
@@ -1634,15 +1660,20 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
 	     update. */
 	  ret = bgp_attr_check (peer, &attr);
 	  if (ret < 0)
-	    return -1;
+	    {
+	      bgp_attr_unintern_sub (&attr);
+              if (attr.extra)
+                bgp_attr_extra_free (&attr);
+	      return -1;
+            }
 
-	  bgp_nlri_parse (peer, &attr, &update);
+	  bgp_nlri_parse (peer, NLRI_ATTR_ARG, &update);
 	}
 
       if (mp_update.length
 	  && mp_update.afi == AFI_IP 
 	  && mp_update.safi == SAFI_UNICAST)
-	bgp_nlri_parse (peer, &attr, &mp_update);
+	bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update);
 
       if (mp_withdraw.length
 	  && mp_withdraw.afi == AFI_IP 
@@ -1669,7 +1700,7 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
       if (mp_update.length
 	  && mp_update.afi == AFI_IP 
 	  && mp_update.safi == SAFI_MULTICAST)
-	bgp_nlri_parse (peer, &attr, &mp_update);
+	bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update);
 
       if (mp_withdraw.length
 	  && mp_withdraw.afi == AFI_IP 
@@ -1699,7 +1730,7 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
       if (mp_update.length 
 	  && mp_update.afi == AFI_IP6 
 	  && mp_update.safi == SAFI_UNICAST)
-	bgp_nlri_parse (peer, &attr, &mp_update);
+	bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update);
 
       if (mp_withdraw.length 
 	  && mp_withdraw.afi == AFI_IP6 
@@ -1728,7 +1759,7 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
       if (mp_update.length 
 	  && mp_update.afi == AFI_IP6 
 	  && mp_update.safi == SAFI_MULTICAST)
-	bgp_nlri_parse (peer, &attr, &mp_update);
+	bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update);
 
       if (mp_withdraw.length 
 	  && mp_withdraw.afi == AFI_IP6 
@@ -1756,7 +1787,7 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
       if (mp_update.length 
 	  && mp_update.afi == AFI_IP 
 	  && mp_update.safi == BGP_SAFI_VPNV4)
-	bgp_nlri_parse_vpnv4 (peer, &attr, &mp_update);
+	bgp_nlri_parse_vpnv4 (peer, NLRI_ATTR_ARG, &mp_update);
 
       if (mp_withdraw.length 
 	  && mp_withdraw.afi == AFI_IP 
@@ -1778,21 +1809,10 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
 
   /* Everything is done.  We unintern temporary structures which
      interned in bgp_attr_parse(). */
-  if (attr.aspath)
-    aspath_unintern (attr.aspath);
-  if (attr.community)
-    community_unintern (attr.community);
+  bgp_attr_unintern_sub (&attr);
   if (attr.extra)
-    {
-      if (attr.extra->ecommunity)
-        ecommunity_unintern (attr.extra->ecommunity);
-      if (attr.extra->cluster)
-        cluster_unintern (attr.extra->cluster);
-      if (attr.extra->transit)
-        transit_unintern (attr.extra->transit);
-      bgp_attr_extra_free (&attr);
-    }
-
+    bgp_attr_extra_free (&attr);
+  
   /* If peering is stopped due to some reason, do not generate BGP
      event.  */
   if (peer->status != Established)
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 5c516f02..aabd264a 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -137,7 +137,7 @@ static void
 bgp_info_free (struct bgp_info *binfo)
 {
   if (binfo->attr)
-    bgp_attr_unintern (binfo->attr);
+    bgp_attr_unintern (&binfo->attr);
   
   bgp_info_extra_free (&binfo->extra);
 
@@ -1802,14 +1802,14 @@ bgp_update_rsclient (struct peer *rsclient, afi_t afi, safi_t safi,
   /* Apply import policy. */
   if (bgp_import_modifier (rsclient, peer, p, &new_attr, afi, safi) == RMAP_DENY)
     {
-      bgp_attr_unintern (attr_new2);
+      bgp_attr_unintern (&attr_new2);
 
       reason = "import-policy;";
       goto filtered;
     }
 
   attr_new = bgp_attr_intern (&new_attr);
-  bgp_attr_unintern (attr_new2);
+  bgp_attr_unintern (&attr_new2);
 
   /* IPv4 unicast next hop check.  */
   if (afi == AFI_IP && safi == SAFI_UNICAST)
@@ -1818,7 +1818,7 @@ bgp_update_rsclient (struct peer *rsclient, afi_t afi, safi_t safi,
       if (new_attr.nexthop.s_addr == 0
          || ntohl (new_attr.nexthop.s_addr) >= 0xe0000000)
        {
-         bgp_attr_unintern (attr_new);
+         bgp_attr_unintern (&attr_new);
 
          reason = "martian next-hop;";
          goto filtered;
@@ -1848,7 +1848,7 @@ bgp_update_rsclient (struct peer *rsclient, afi_t afi, safi_t safi,
                     p->prefixlen, rsclient->host);
 
           bgp_unlock_node (rn);
-          bgp_attr_unintern (attr_new);
+          bgp_attr_unintern (&attr_new);
 
           return;
         }
@@ -1868,7 +1868,7 @@ bgp_update_rsclient (struct peer *rsclient, afi_t afi, safi_t safi,
       bgp_info_set_flag (rn, ri, BGP_INFO_ATTR_CHANGED);
 
       /* Update to new attribute.  */
-      bgp_attr_unintern (ri->attr);
+      bgp_attr_unintern (&ri->attr);
       ri->attr = attr_new;
 
       /* Update MPLS tag.  */
@@ -2128,7 +2128,7 @@ bgp_update_main (struct peer *peer, struct prefix *p, struct attr *attr,
 	    }
 
 	  bgp_unlock_node (rn);
-	  bgp_attr_unintern (attr_new);
+	  bgp_attr_unintern (&attr_new);
 	  bgp_attr_extra_free (&new_attr);
 	  
 	  return 0;
@@ -2175,7 +2175,7 @@ bgp_update_main (struct peer *peer, struct prefix *p, struct attr *attr,
 	}
 	
       /* Update to new attribute.  */
-      bgp_attr_unintern (ri->attr);
+      bgp_attr_unintern (&ri->attr);
       ri->attr = attr_new;
 
       /* Update MPLS tag.  */
@@ -2467,7 +2467,7 @@ bgp_default_originate (struct peer *peer, afi_t afi, safi_t safi, int withdraw)
     }
   
   bgp_attr_extra_free (&attr);
-  aspath_unintern (aspath);
+  aspath_unintern (&aspath);
 }
 
 static void
@@ -3215,7 +3215,7 @@ bgp_static_update_rsclient (struct peer *rsclient, struct prefix *p,
           bgp_attr_flush (&attr_tmp);
 
           /* Unintern original. */
-          aspath_unintern (attr.aspath);
+          aspath_unintern (&attr.aspath);
           bgp_static_withdraw_rsclient (bgp, rsclient, p, afi, safi);
           bgp_attr_extra_free (&attr);
           
@@ -3242,8 +3242,8 @@ bgp_static_update_rsclient (struct peer *rsclient, struct prefix *p,
 
       bgp->peer_self->rmap_type = 0;
 
-      bgp_attr_unintern (attr_new);
-      aspath_unintern (attr.aspath);
+      bgp_attr_unintern (&attr_new);
+      aspath_unintern (&attr.aspath);
       bgp_attr_extra_free (&attr);
 
       bgp_static_withdraw_rsclient (bgp, rsclient, p, afi, safi);
@@ -3253,7 +3253,7 @@ bgp_static_update_rsclient (struct peer *rsclient, struct prefix *p,
 
   bgp->peer_self->rmap_type = 0;
 
-  bgp_attr_unintern (attr_new);
+  bgp_attr_unintern (&attr_new);
   attr_new = bgp_attr_intern (&new_attr);
   bgp_attr_extra_free (&new_attr);
 
@@ -3268,8 +3268,8 @@ bgp_static_update_rsclient (struct peer *rsclient, struct prefix *p,
 	  !CHECK_FLAG(ri->flags, BGP_INFO_REMOVED))
         {
           bgp_unlock_node (rn);
-          bgp_attr_unintern (attr_new);
-          aspath_unintern (attr.aspath);
+          bgp_attr_unintern (&attr_new);
+          aspath_unintern (&attr.aspath);
           bgp_attr_extra_free (&attr);
           return;
        }
@@ -3281,14 +3281,14 @@ bgp_static_update_rsclient (struct peer *rsclient, struct prefix *p,
           /* Rewrite BGP route information. */
 	  if (CHECK_FLAG(ri->flags, BGP_INFO_REMOVED))
 	    bgp_info_restore(rn, ri);
-          bgp_attr_unintern (ri->attr);
+          bgp_attr_unintern (&ri->attr);
           ri->attr = attr_new;
           ri->uptime = bgp_clock ();
 
           /* Process change. */
           bgp_process (bgp, rn, afi, safi);
           bgp_unlock_node (rn);
-          aspath_unintern (attr.aspath);
+          aspath_unintern (&attr.aspath);
           bgp_attr_extra_free (&attr);
           return;
         }
@@ -3313,7 +3313,7 @@ bgp_static_update_rsclient (struct peer *rsclient, struct prefix *p,
   bgp_process (bgp, rn, afi, safi);
 
   /* Unintern original. */
-  aspath_unintern (attr.aspath);
+  aspath_unintern (&attr.aspath);
   bgp_attr_extra_free (&attr);
 }
 
@@ -3363,7 +3363,7 @@ bgp_static_update_main (struct bgp *bgp, struct prefix *p,
 	  bgp_attr_flush (&attr_tmp);
 
 	  /* Unintern original. */
-	  aspath_unintern (attr.aspath);
+	  aspath_unintern (&attr.aspath);
 	  bgp_attr_extra_free (&attr);
 	  bgp_static_withdraw (bgp, p, afi, safi);
 	  return;
@@ -3384,8 +3384,8 @@ bgp_static_update_main (struct bgp *bgp, struct prefix *p,
 	  !CHECK_FLAG(ri->flags, BGP_INFO_REMOVED))
 	{
 	  bgp_unlock_node (rn);
-	  bgp_attr_unintern (attr_new);
-	  aspath_unintern (attr.aspath);
+	  bgp_attr_unintern (&attr_new);
+	  aspath_unintern (&attr.aspath);
 	  bgp_attr_extra_free (&attr);
 	  return;
 	}
@@ -3399,7 +3399,7 @@ bgp_static_update_main (struct bgp *bgp, struct prefix *p,
 	    bgp_info_restore(rn, ri);
 	  else
 	    bgp_aggregate_decrement (bgp, p, ri, afi, safi);
-	  bgp_attr_unintern (ri->attr);
+	  bgp_attr_unintern (&ri->attr);
 	  ri->attr = attr_new;
 	  ri->uptime = bgp_clock ();
 
@@ -3407,7 +3407,7 @@ bgp_static_update_main (struct bgp *bgp, struct prefix *p,
 	  bgp_aggregate_increment (bgp, p, ri, afi, safi);
 	  bgp_process (bgp, rn, afi, safi);
 	  bgp_unlock_node (rn);
-	  aspath_unintern (attr.aspath);
+	  aspath_unintern (&attr.aspath);
 	  bgp_attr_extra_free (&attr);
 	  return;
 	}
@@ -3435,7 +3435,7 @@ bgp_static_update_main (struct bgp *bgp, struct prefix *p,
   bgp_process (bgp, rn, afi, safi);
 
   /* Unintern original. */
-  aspath_unintern (attr.aspath);
+  aspath_unintern (&attr.aspath);
   bgp_attr_extra_free (&attr);
 }
 
@@ -5299,7 +5299,7 @@ bgp_redistribute_add (struct prefix *p, struct in_addr *nexthop,
 		  bgp_attr_extra_free (&attr_new);
 		  
 		  /* Unintern original. */
-		  aspath_unintern (attr.aspath);
+		  aspath_unintern (&attr.aspath);
 		  bgp_attr_extra_free (&attr);
 		  bgp_redistribute_delete (p, type);
 		  return;
@@ -5322,8 +5322,8 @@ bgp_redistribute_add (struct prefix *p, struct in_addr *nexthop,
  	      if (attrhash_cmp (bi->attr, new_attr) &&
 		  !CHECK_FLAG(bi->flags, BGP_INFO_REMOVED))
  		{
- 		  bgp_attr_unintern (new_attr);
- 		  aspath_unintern (attr.aspath);
+ 		  bgp_attr_unintern (&new_attr);
+ 		  aspath_unintern (&attr.aspath);
  		  bgp_attr_extra_free (&attr);
  		  bgp_unlock_node (bn);
  		  return;
@@ -5338,7 +5338,7 @@ bgp_redistribute_add (struct prefix *p, struct in_addr *nexthop,
 		    bgp_info_restore(bn, bi);
 		  else
 		    bgp_aggregate_decrement (bgp, p, bi, afi, SAFI_UNICAST);
- 		  bgp_attr_unintern (bi->attr);
+ 		  bgp_attr_unintern (&bi->attr);
  		  bi->attr = new_attr;
  		  bi->uptime = bgp_clock ();
  
@@ -5346,7 +5346,7 @@ bgp_redistribute_add (struct prefix *p, struct in_addr *nexthop,
  		  bgp_aggregate_increment (bgp, p, bi, afi, SAFI_UNICAST);
  		  bgp_process (bgp, bn, afi, SAFI_UNICAST);
  		  bgp_unlock_node (bn);
- 		  aspath_unintern (attr.aspath);
+ 		  aspath_unintern (&attr.aspath);
  		  bgp_attr_extra_free (&attr);
  		  return;
  		} 
@@ -5368,7 +5368,7 @@ bgp_redistribute_add (struct prefix *p, struct in_addr *nexthop,
     }
 
   /* Unintern original. */
-  aspath_unintern (attr.aspath);
+  aspath_unintern (&attr.aspath);
   bgp_attr_extra_free (&attr);
 }
 
diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index 255b25ae..178af603 100644
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -1482,10 +1482,10 @@ route_set_ecommunity_rt (void *rule, struct prefix *prefix,
       else
 	new_ecom = ecommunity_dup (ecom);
 
-      bgp_info->attr->extra->ecommunity = new_ecom;
+      bgp_info->attr->extra->ecommunity = ecommunity_intern (new_ecom);
 
       if (old_ecom)
-	ecommunity_free (old_ecom);
+	ecommunity_unintern (&old_ecom);
 
       bgp_info->attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES);
     }
@@ -1501,7 +1501,7 @@ route_set_ecommunity_rt_compile (const char *arg)
   ecom = ecommunity_str2com (arg, ECOMMUNITY_ROUTE_TARGET, 0);
   if (! ecom)
     return NULL;
-  return ecom;
+  return ecommunity_intern (ecom);
 }
 
 /* Free function for set community. */
@@ -1509,7 +1509,7 @@ static void
 route_set_ecommunity_rt_free (void *rule)
 {
   struct ecommunity *ecom = rule;
-  ecommunity_free (ecom);
+  ecommunity_unintern (&ecom);
 }
 
 /* Set community rule structure. */
@@ -1528,7 +1528,7 @@ static route_map_result_t
 route_set_ecommunity_soo (void *rule, struct prefix *prefix, 
 			 route_map_object_t type, void *object)
 {
-  struct ecommunity *ecom;
+  struct ecommunity *ecom, *old_ecom, *new_ecom;
   struct bgp_info *bgp_info;
 
   if (type == RMAP_BGP)
@@ -1539,8 +1539,19 @@ route_set_ecommunity_soo (void *rule, struct prefix *prefix,
       if (! ecom)
 	return RMAP_OKAY;
     
+      old_ecom = (bgp_attr_extra_get (bgp_info->attr))->ecommunity;
+      
+      if (old_ecom)
+	new_ecom = ecommunity_merge (ecommunity_dup (old_ecom), ecom);
+      else
+	new_ecom = ecommunity_dup (ecom);
+
+      bgp_info->attr->extra->ecommunity = ecommunity_intern (new_ecom);
+
+      if (old_ecom)
+	ecommunity_unintern (&old_ecom);
+
       bgp_info->attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES);
-      (bgp_attr_extra_get (bgp_info->attr))->ecommunity = ecommunity_dup (ecom);
     }
   return RMAP_OKAY;
 }
@@ -1555,7 +1566,7 @@ route_set_ecommunity_soo_compile (const char *arg)
   if (! ecom)
     return NULL;
   
-  return ecom;
+  return ecommunity_intern (ecom);
 }
 
 /* Free function for set community. */
@@ -1563,7 +1574,7 @@ static void
 route_set_ecommunity_soo_free (void *rule)
 {
   struct ecommunity *ecom = rule;
-  ecommunity_free (ecom);
+  ecommunity_unintern (&ecom);
 }
 
 /* Set community rule structure. */
diff --git a/tests/aspath_test.c b/tests/aspath_test.c
index 9e51e8dd..4a2ce9aa 100644
--- a/tests/aspath_test.c
+++ b/tests/aspath_test.c
@@ -6,6 +6,7 @@
 
 #include "bgpd/bgpd.h"
 #include "bgpd/bgp_aspath.h"
+#include "bgpd/bgp_attr.h"
 
 #define VT100_RESET "\x1b[0m"
 #define VT100_RED "\x1b[31m"
@@ -407,7 +408,7 @@ static struct test_segment {
     "#ASNs = 0, data = seq(8466 3 52737 4096 3456)",
     { 0x2,0x0, 0x21,0x12, 0x00,0x03, 0xce,0x01, 0x10,0x00, 0x0d,0x80 },
     12,
-    { "", "",
+    { NULL, NULL,
       0, 0, 0, 0, 0, 0 },
   },
   { /* 26  */ 
@@ -417,10 +418,190 @@ static struct test_segment {
       0x2,0x2, 0x10,0x00, 0x0d,0x80 },
     14
     ,
-    { "", "",
+    { NULL, NULL,
       0, 0, 0, 0, 0, 0 },
   },
-  { NULL, NULL, {0}, 0, { NULL, 0, 0 } }
+  { /* 27  */ 
+    "invalid segment type",
+    "type=8(4096 3456)",
+    { 0x8,0x2, 0x10,0x00, 0x0d,0x80 },
+    14
+    ,
+    { NULL, NULL,
+      0, 0, 0, 0, 0, 0 },
+  },  { NULL, NULL, {0}, 0, { NULL, 0, 0 } }
+};
+
+/* */
+static struct aspath_tests {
+  const char *desc;
+  const struct test_segment *segment;
+  const char *shouldbe;  /* String it should evaluate to */
+  const enum as4 { AS4_DATA, AS2_DATA }
+          as4; 	/* whether data should be as4 or not (ie as2) */
+  const int result;	/* expected result for bgp_attr_parse */
+  const int cap;	/* capabilities to set for peer */
+  const char attrheader [1024];
+  size_t len;
+} aspath_tests [] =
+{
+  /* 0 */
+  {
+    "basic test",
+    &test_segments[0],
+    "8466 3 52737 4096",
+    AS2_DATA, 0,
+    0,
+    { BGP_ATTR_FLAG_TRANS,
+      BGP_ATTR_AS_PATH, 
+      10,
+    },
+    3,
+  },
+  /* 1 */
+  {
+    "length too short",
+    &test_segments[0],
+    "8466 3 52737 4096",
+    AS2_DATA, -1,
+    0,
+    { BGP_ATTR_FLAG_TRANS,
+      BGP_ATTR_AS_PATH, 
+      8,
+    },
+    3,
+  },
+  /* 2 */
+  {
+    "length too long",
+    &test_segments[0],
+    "8466 3 52737 4096",
+    AS2_DATA, -1,
+    0,
+    { BGP_ATTR_FLAG_TRANS,
+      BGP_ATTR_AS_PATH, 
+      12,
+    },
+    3,
+  },
+  /* 3 */
+  {
+    "incorrect flag",
+    &test_segments[0],
+    "8466 3 52737 4096",
+    AS2_DATA, -1,
+    0,
+    { BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_OPTIONAL,
+      BGP_ATTR_AS_PATH, 
+      10,
+    },
+    3,
+  },
+  /* 4 */
+  {
+    "as4_path, with as2 format data",
+    &test_segments[0],
+    "8466 3 52737 4096",
+    AS2_DATA, -1,
+    0,
+    { BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_OPTIONAL,
+      BGP_ATTR_AS4_PATH, 
+      10,
+    },
+    3,
+  },
+  /* 5 */
+  {
+    "as4, with incorrect attr length",
+    &test_segments[0],
+    "8466 3 52737 4096",
+    AS4_DATA, -1,
+    PEER_CAP_AS4_RCV,
+    { BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_OPTIONAL,
+      BGP_ATTR_AS4_PATH, 
+      10,
+    },
+    3,
+  },
+  /* 6 */
+  {
+    "basic 4-byte as-path",
+    &test_segments[0],
+    "8466 3 52737 4096",
+    AS4_DATA, 0,
+    PEER_CAP_AS4_RCV|PEER_CAP_AS4_ADV,
+    { BGP_ATTR_FLAG_TRANS,
+      BGP_ATTR_AS_PATH, 
+      18,
+    },
+    3,
+  },
+  /* 7 */
+  {
+    "4b AS_PATH: too short",
+    &test_segments[0],
+    "8466 3 52737 4096",
+    AS4_DATA, -1,
+    PEER_CAP_AS4_RCV|PEER_CAP_AS4_ADV,
+    { BGP_ATTR_FLAG_TRANS,
+      BGP_ATTR_AS_PATH, 
+      16,
+    },
+    3,
+  },
+  /* 8 */
+  {
+    "4b AS_PATH: too long",
+    &test_segments[0],
+    "8466 3 52737 4096",
+    AS4_DATA, -1,
+    PEER_CAP_AS4_RCV|PEER_CAP_AS4_ADV,
+    { BGP_ATTR_FLAG_TRANS,
+      BGP_ATTR_AS_PATH, 
+      20,
+    },
+    3,
+  },
+  /* 9 */
+  {
+    "4b AS_PATH: too long2",
+    &test_segments[0],
+    "8466 3 52737 4096",
+    AS4_DATA, -1,
+    PEER_CAP_AS4_RCV|PEER_CAP_AS4_ADV,
+    { BGP_ATTR_FLAG_TRANS,
+      BGP_ATTR_AS_PATH, 
+      22,
+    },
+    3,
+  },
+  /* 10 */
+  {
+    "4b AS_PATH: bad flags",
+    &test_segments[0],
+    "8466 3 52737 4096",
+    AS4_DATA, -1,
+    PEER_CAP_AS4_RCV|PEER_CAP_AS4_ADV,
+    { BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_OPTIONAL,
+      BGP_ATTR_AS_PATH, 
+      18,
+    },
+    3,
+  },
+  /* 11 */
+  {
+    "4b AS_PATH: confed",
+    &test_segments[6],
+    "8466 3 52737 4096",
+    AS4_DATA, -1,
+    PEER_CAP_AS4_ADV,
+    { BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_OPTIONAL,
+      BGP_ATTR_AS4_PATH, 
+      14,
+    },
+    3,
+  },
+  { NULL, NULL, NULL, 0, 0, 0, { 0 }, 0 },
 };
 
 /* prepending tests */
@@ -430,21 +611,25 @@ static struct tests {
   struct test_spec sp;
 } prepend_tests[] = 
 {
+  /* 0 */
   { &test_segments[0], &test_segments[1],
     { "8466 3 52737 4096 8722 4",
       "8466 3 52737 4096 8722 4",
       6, 0, NOT_ALL_PRIVATE, 4096, 1, 8466 },
   },
+  /* 1 */
   { &test_segments[1], &test_segments[3],
     { "8722 4 8482 51457 {5204}",
       "8722 4 8482 51457 {5204}",
       5, 0, NOT_ALL_PRIVATE, 5204, 1, 8722 }
   },
+  /* 2 */
   { &test_segments[3], &test_segments[4],
     { "8482 51457 {5204} 8467 59649 {4196,48658} {17322,30745}",
       "8482 51457 {5204} 8467 59649 {4196,48658} {17322,30745}",
       7, 0, NOT_ALL_PRIVATE, 5204, 1, 8482 },
   },
+  /* 3 */
   { &test_segments[4], &test_segments[5],
     { "8467 59649 {4196,48658} {17322,30745} 6435 59408 21665"
       " {2457,4369,61697} 1842 41590 51793",
@@ -452,11 +637,13 @@ static struct tests {
       " {2457,4369,61697} 1842 41590 51793",
       11, 0, NOT_ALL_PRIVATE, 61697, 1, 8467 }
   },
+  /* 4 */
   { &test_segments[5], &test_segments[6],
-    { "6435 59408 21665 {2457,4369,61697} 1842 41590 51793 (123 456 789)",
-      "6435 59408 21665 {2457,4369,61697} 1842 41590 51793 (123 456 789)",
-      7, 3, NOT_ALL_PRIVATE, 123, 1, 6435 },
+    { "6435 59408 21665 {2457,4369,61697} 1842 41590 51793",
+      "6435 59408 21665 {2457,4369,61697} 1842 41590 51793",
+      7, 0, NOT_ALL_PRIVATE, 1842, 1, 6435 },
   },
+  /* 5 */
   { &test_segments[6], &test_segments[7],
     { "(123 456 789) (123 456 789) (111 222)",
       "",
@@ -649,7 +836,7 @@ make_aspath (const u_char *data, size_t len, int use32bit)
       s = stream_new (len);
       stream_put (s, data, len);
     }
-  as = aspath_parse (s, len, use32bit, 0);
+  as = aspath_parse (s, len, use32bit);
   
   if (s)
     stream_free (s);
@@ -682,6 +869,12 @@ validate (struct aspath *as, const struct test_spec *sp)
   static struct stream *s;
   struct aspath *asinout, *asconfeddel, *asstr, *as4;
   
+  if (as == NULL && sp->shouldbe == NULL)
+    {
+      printf ("Correctly failed to parse\n");
+      return fails;
+    }
+  
   out = aspath_snmp_pathseg (as, &bytes);
   asinout = make_aspath (out, bytes, 0);
   
@@ -826,7 +1019,7 @@ parse_test (struct test_segment *t)
   printf ("%s: %s\n", t->name, t->desc);
 
   asp = make_aspath (t->asdata, t->len, 0);
-
+  
   printf ("aspath: %s\nvalidating...:\n", aspath_print (asp));
 
   if (!validate (asp, &t->sp))
@@ -835,7 +1028,9 @@ parse_test (struct test_segment *t)
     printf (FAILED "\n");
   
   printf ("\n");
-  aspath_unintern (asp);
+  
+  if (asp)
+    aspath_unintern (asp);
 }
 
 /* prepend testing */
@@ -891,7 +1086,8 @@ empty_prepend_test (struct test_segment *t)
     printf (FAILED "!\n");
   
   printf ("\n");
-  aspath_unintern (asp1);
+  if (asp1)
+    aspath_unintern (asp1);
   aspath_free (asp2);
 }
 
@@ -993,30 +1189,106 @@ cmp_test ()
       aspath_unintern (asp2);
     }
 }
-     
+
+static int
+handle_attr_test (struct aspath_tests *t)
+{
+  struct bgp bgp = { 0 }; 
+  struct peer peer = { 0 };
+  struct attr attr = { 0 };  
+  int ret;
+  int initfail = failed;
+  struct aspath *asp;
+  size_t datalen;
+  
+  asp = make_aspath (t->segment->asdata, t->segment->len, 0);
+    
+  peer.ibuf = stream_new (BGP_MAX_PACKET_SIZE);
+  peer.obuf = stream_fifo_new ();
+  peer.bgp = &bgp;
+  peer.host = (char *)"none";
+  peer.fd = -1;
+  peer.cap = t->cap;
+  
+  stream_write (peer.ibuf, t->attrheader, t->len);
+  datalen = aspath_put (peer.ibuf, asp, t->as4 == AS4_DATA);
+  
+  ret = bgp_attr_parse (&peer, &attr, t->len + datalen, NULL, NULL);
+  
+  if (ret != t->result)
+    {
+      printf ("bgp_attr_parse returned %d, expected %d\n", ret, t->result);
+      printf ("datalen %d\n", datalen);
+      failed++;
+    }
+  if (ret != 0)
+    goto out;
+  
+  if (attr.aspath == NULL)
+    {
+      printf ("aspath is NULL!\n");
+      failed++;
+    }
+  if (attr.aspath && strcmp (attr.aspath->str, t->shouldbe))
+    {
+      printf ("attr str and 'shouldbe' mismatched!\n"
+              "attr str:  %s\n"
+              "shouldbe:  %s\n",
+              attr.aspath->str, t->shouldbe);
+      failed++;
+    }
+
+out:
+  if (attr.aspath)
+    aspath_unintern (attr.aspath);
+  if (asp)
+    aspath_unintern (asp);
+  return failed - initfail;
+}
+
+static void
+attr_test (struct aspath_tests *t)
+{
+    printf ("%s\n", t->desc);
+    printf ("%s\n\n", handle_attr_test (t) ? FAILED : OK);  
+}
+
 int
 main (void)
 {
   int i = 0;
-  aspath_init();
+  bgp_master_init ();
+  master = bm->master;
+  bgp_attr_init ();
+  
   while (test_segments[i].name)
     {
+      printf ("test %u\n", i);
       parse_test (&test_segments[i]);
       empty_prepend_test (&test_segments[i++]);
     }
   
   i = 0;
   while (prepend_tests[i].test1)
-    prepend_test (&prepend_tests[i++]);
+    {
+      printf ("prepend test %u\n", i);
+      prepend_test (&prepend_tests[i++]);
+    }
   
   i = 0;
   while (aggregate_tests[i].test1)
-    aggregate_test (&aggregate_tests[i++]);
+    {
+      printf ("aggregate test %u\n", i);
+      aggregate_test (&aggregate_tests[i++]);
+    }
   
   i = 0;
   
   while (reconcile_tests[i].test1)
-    as4_reconcile_test (&reconcile_tests[i++]);
+    {
+      printf ("reconcile test %u\n", i);
+      as4_reconcile_test (&reconcile_tests[i++]);
+    }
   
   i = 0;
   
@@ -1026,6 +1298,14 @@ main (void)
   
   empty_get_test();
   
+  i = 0;
+  
+  while (aspath_tests[i].desc)
+    {
+      printf ("aspath_attr test %d\n", i);
+      attr_test (&aspath_tests[i++]);
+    }
+  
   printf ("failures: %d\n", failed);
   printf ("aspath count: %ld\n", aspath_count());
   
diff --git a/tools/multiple-bgpd.sh b/tools/multiple-bgpd.sh
index 028ad696..d6a38ed4 100644
--- a/tools/multiple-bgpd.sh
+++ b/tools/multiple-bgpd.sh
@@ -25,13 +25,14 @@ for H in `seq 1 ${NUM}` ; do
 		NEXTAS=$((${ASBASE} + $NEXT))
 		PREVADDR="${PREFIX}${PREV}"
 		PREVAS=$((${ASBASE} + $PREV))
+		ASN=$((64560+${H}))
 		
 		# Edit config to suit.
 		cat > "$CONF" <<- EOF
 			password whatever
 			service advanced-vty
 			!
-			router bgp $((64560+${H}))
+			router bgp ${ASN}
 			 bgp router-id ${ADDR}
 			 network 10.${H}.1.0/24 pathlimit 1
 			 network 10.${H}.2.0/24 pathlimit 2
@@ -40,6 +41,7 @@ for H in `seq 1 ${NUM}` ; do
 			 neighbor default update-source ${ADDR}
 			 neighbor default capability orf prefix-list both
 			 neighbor default soft-reconfiguration inbound
+			 neighbor default route-map test out
 			 neighbor ${NEXTADDR} remote-as ${NEXTAS}
 			 neighbor ${NEXTADDR} peer-group default
 			 neighbor ${PREVADDR} remote-as ${PREVAS}
@@ -53,10 +55,15 @@ for H in `seq 1 ${NUM}` ; do
 			 neighbor default activate
 			 neighbor default capability orf prefix-list both
 			 neighbor default default-originate
+			 neighbor default route-map test out
 			 neighbor ${NEXTADDR} peer-group default
 			 neighbor ${PREVADDR} peer-group default
 			 exit-address-family
 			!
+			route-map test permit 10
+			 set extcommunity rt ${ASN}:1
+			 set extcommunity soo ${ASN}:2
+			 set community ${ASN}:1
 			line vty
 			!
 			end