]> git.proxmox.com Git - mirror_frr.git/blobdiff - bgpd/bgp_attr.c
bgpd: general MP/SAFI improvements
[mirror_frr.git] / bgpd / bgp_attr.c
index 1300ab84ba3895326264a97da7a3033cb3e6d60a..2ffe464f0a8d7314a3d0bff0b1a600b49b2caea0 100644 (file)
@@ -28,6 +28,8 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
 #include "stream.h"
 #include "log.h"
 #include "hash.h"
+#include "jhash.h"
+#include "queue.h"
 
 #include "bgpd/bgpd.h"
 #include "bgpd/bgp_attr.h"
@@ -37,7 +39,8 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
 #include "bgpd/bgp_debug.h"
 #include "bgpd/bgp_packet.h"
 #include "bgpd/bgp_ecommunity.h"
-\f
+#include "bgpd/bgp_updgrp.h"
+
 /* Attribute strings for logging. */
 static const struct message attr_str [] = 
 {
@@ -50,7 +53,7 @@ static const struct message attr_str [] =
   { BGP_ATTR_AGGREGATOR,       "AGGREGATOR" }, 
   { BGP_ATTR_COMMUNITIES,      "COMMUNITY" }, 
   { BGP_ATTR_ORIGINATOR_ID,    "ORIGINATOR_ID" },
-  { BGP_ATTR_CLUSTER_LIST,     "CLUSTERLIST" }, 
+  { BGP_ATTR_CLUSTER_LIST,     "CLUSTER_LIST" }, 
   { BGP_ATTR_DPA,              "DPA" },
   { BGP_ATTR_ADVERTISER,       "ADVERTISER"} ,
   { BGP_ATTR_RCID_PATH,        "RCID_PATH" },
@@ -61,14 +64,23 @@ static const struct message attr_str [] =
   { BGP_ATTR_AS4_AGGREGATOR,   "AS4_AGGREGATOR" }, 
   { BGP_ATTR_AS_PATHLIMIT,     "AS_PATHLIMIT" },
 };
-static const int attr_str_max = sizeof(attr_str)/sizeof(attr_str[0]);
-\f
+static const int attr_str_max = array_size(attr_str);
+
+static const struct message attr_flag_str[] =
+{
+  { BGP_ATTR_FLAG_OPTIONAL, "Optional" },
+  { BGP_ATTR_FLAG_TRANS,    "Transitive" },
+  { BGP_ATTR_FLAG_PARTIAL,  "Partial" },
+  /* bgp_attr_flags_diagnose() relies on this bit being last in this list */
+  { BGP_ATTR_FLAG_EXTLEN,   "Extended Length" },
+};
+
 static struct hash *cluster_hash;
 
 static void *
 cluster_hash_alloc (void *p)
 {
-  struct cluster_list * val = (struct cluster_list *) p;
+  const struct cluster_list *val = (const struct cluster_list *) p;
   struct cluster_list *cluster;
 
   cluster = XMALLOC (MTYPE_CLUSTER, sizeof (struct cluster_list));
@@ -116,18 +128,9 @@ cluster_loop_check (struct cluster_list *cluster, struct in_addr originator)
 static unsigned int
 cluster_hash_key_make (void *p)
 {
-  struct cluster_list * cluster = (struct cluster_list *) p;
-  unsigned int key = 0;
-  int length;
-  caddr_t pnt;
+  const struct cluster_list *cluster = p;
 
-  length = cluster->length;
-  pnt = (caddr_t) cluster->list;
-  
-  while (length)
-    key += pnt[--length];
-
-  return key;
+  return jhash(cluster->list, cluster->length, 0);
 }
 
 static int
@@ -148,7 +151,6 @@ cluster_free (struct cluster_list *cluster)
   XFREE (MTYPE_CLUSTER, cluster);
 }
 
-#if 0
 static struct cluster_list *
 cluster_dup (struct cluster_list *cluster)
 {
@@ -167,7 +169,6 @@ cluster_dup (struct cluster_list *cluster)
   
   return new;
 }
-#endif
 
 static struct cluster_list *
 cluster_intern (struct cluster_list *cluster)
@@ -183,14 +184,12 @@ cluster_intern (struct cluster_list *cluster)
 void
 cluster_unintern (struct cluster_list *cluster)
 {
-  struct cluster_list *ret;
-
   if (cluster->refcnt)
     cluster->refcnt--;
 
   if (cluster->refcnt == 0)
     {
-      ret = hash_release (cluster_hash, cluster);
+      hash_release (cluster_hash, cluster);
       cluster_free (cluster);
     }
 }
@@ -207,7 +206,7 @@ cluster_finish (void)
   hash_free (cluster_hash);
   cluster_hash = NULL;
 }
-\f
+
 /* Unknown transit attribute. */
 static struct hash *transit_hash;
 
@@ -219,6 +218,23 @@ transit_free (struct transit *transit)
   XFREE (MTYPE_TRANSIT, transit);
 }
 
+static struct transit *
+transit_dup (struct transit *transit)
+{
+  struct transit *new;
+
+  new = XCALLOC (MTYPE_TRANSIT, sizeof (struct transit));
+  new->length = transit->length;
+  if (new->length)
+    {
+      new->val = XMALLOC (MTYPE_TRANSIT_VAL, transit->length);
+      memcpy (new->val, transit->val, transit->length);
+    }
+  else
+    new->val = NULL;
+
+  return new;
+}
 
 static void *
 transit_hash_alloc (void *p)
@@ -243,14 +259,12 @@ transit_intern (struct transit *transit)
 void
 transit_unintern (struct transit *transit)
 {
-  struct transit *ret;
-
   if (transit->refcnt)
     transit->refcnt--;
 
   if (transit->refcnt == 0)
     {
-      ret = hash_release (transit_hash, transit);
+      hash_release (transit_hash, transit);
       transit_free (transit);
     }
 }
@@ -258,18 +272,9 @@ transit_unintern (struct transit *transit)
 static unsigned int
 transit_hash_key_make (void *p)
 {
-  struct transit * transit = (struct transit *) p;
-  unsigned int key = 0;
-  int length;
-  caddr_t pnt;
-
-  length = transit->length;
-  pnt = (caddr_t) transit->val;
-  
-  while (length)
-    key += pnt[--length];
+  const struct transit * transit = p;
 
-  return key;
+  return jhash(transit->val, transit->length, 0);
 }
 
 static int
@@ -294,7 +299,7 @@ transit_finish (void)
   hash_free (transit_hash);
   transit_hash = NULL;
 }
-\f
+
 /* Attribute hash routines. */
 static struct hash *attrhash;
 
@@ -329,14 +334,71 @@ bgp_attr_extra_get (struct attr *attr)
 void
 bgp_attr_dup (struct attr *new, struct attr *orig)
 {
+  struct attr_extra *extra = new->extra;
+
   *new = *orig;
-  if (orig->extra)
+  /* if caller provided attr_extra space, use it in any case.
+   *
+   * This is neccesary even if orig->extra equals NULL, because otherwise
+   * memory may be later allocated on the heap by bgp_attr_extra_get.
+   *
+   * That memory would eventually be leaked, because the caller must not
+   * call bgp_attr_extra_free if he provided attr_extra on the stack.
+   */
+  if (extra)
+    {
+      new->extra = extra;
+      memset(new->extra, 0, sizeof(struct attr_extra));
+      if (orig->extra)
+        *new->extra = *orig->extra;
+    }
+  else if (orig->extra)
     {
       new->extra = bgp_attr_extra_new();
       *new->extra = *orig->extra;
     }
 }
 
+void
+bgp_attr_deep_dup (struct attr *new, struct attr *orig)
+{
+  if (orig->aspath)
+    new->aspath = aspath_dup(orig->aspath);
+
+  if (orig->community)
+    new->community = community_dup(orig->community);
+
+  if (orig->extra)
+    {
+      if (orig->extra->ecommunity)
+        new->extra->ecommunity = ecommunity_dup(orig->extra->ecommunity);
+      if (orig->extra->cluster)
+        new->extra->cluster = cluster_dup(orig->extra->cluster);
+      if (orig->extra->transit)
+        new->extra->transit = transit_dup(orig->extra->transit);
+    }
+}
+
+void
+bgp_attr_deep_free (struct attr *attr)
+{
+  if (attr->aspath)
+    aspath_free(attr->aspath);
+
+  if (attr->community)
+    community_free(attr->community);
+
+  if (attr->extra)
+    {
+      if (attr->extra->ecommunity)
+        ecommunity_free(&attr->extra->ecommunity);
+      if (attr->extra->cluster)
+        cluster_free(attr->extra->cluster);
+      if (attr->extra->transit)
+        transit_free(attr->extra->transit);
+    }
+}
+
 unsigned long int
 attr_count (void)
 {
@@ -352,46 +414,49 @@ attr_unknown_count (void)
 unsigned int
 attrhash_key_make (void *p)
 {
-  struct attr * attr = (struct attr *) p;
-  unsigned int key = 0;
+  const struct attr *attr = (struct attr *) p;
+  const struct attr_extra *extra = attr->extra;
+  uint32_t key = 0;
+#define MIX(val)       key = jhash_1word(val, key)
+
+  MIX(attr->origin);
+  MIX(attr->nexthop.s_addr);
+  MIX(attr->med);
+  MIX(attr->local_pref);
 
   key += attr->origin;
   key += attr->nexthop.s_addr;
   key += attr->med;
   key += attr->local_pref;
   
-  if (attr->extra)
+  if (extra)
     {
-      key += attr->extra->aggregator_as;
-      key += attr->extra->aggregator_addr.s_addr;
-      key += attr->extra->weight;
-      key += attr->extra->mp_nexthop_global_in.s_addr;
+      MIX(extra->aggregator_as);
+      MIX(extra->aggregator_addr.s_addr);
+      MIX(extra->weight);
+      MIX(extra->mp_nexthop_global_in.s_addr);
+      MIX(extra->originator_id.s_addr);
+      MIX(extra->tag);
     }
   
   if (attr->aspath)
-    key += aspath_key_make (attr->aspath);
+    MIX(aspath_key_make (attr->aspath));
   if (attr->community)
-    key += community_hash_make (attr->community);
+    MIX(community_hash_make (attr->community));
   
-  if (attr->extra)
+  if (extra)
     {
-      if (attr->extra->ecommunity)
-        key += ecommunity_hash_make (attr->extra->ecommunity);
-      if (attr->extra->cluster)
-        key += cluster_hash_key_make (attr->extra->cluster);
-      if (attr->extra->transit)
-        key += transit_hash_key_make (attr->extra->transit);
+      if (extra->ecommunity)
+        MIX(ecommunity_hash_make (extra->ecommunity));
+      if (extra->cluster)
+        MIX(cluster_hash_key_make (extra->cluster));
+      if (extra->transit)
+        MIX(transit_hash_key_make (extra->transit));
 
 #ifdef HAVE_IPV6
-      {
-        int i;
-        
-        key += attr->extra->mp_nexthop_len;
-        for (i = 0; i < 16; i++)
-          key += attr->extra->mp_nexthop_global.s6_addr[i];
-        for (i = 0; i < 16; i++)
-          key += attr->extra->mp_nexthop_local.s6_addr[i];
-      }
+      MIX(extra->mp_nexthop_len);
+      key = jhash(extra->mp_nexthop_global.s6_addr, IPV6_MAX_BYTELEN, key);
+      key = jhash(extra->mp_nexthop_local.s6_addr, IPV6_MAX_BYTELEN, key);
 #endif /* HAVE_IPV6 */
     }
 
@@ -410,7 +475,8 @@ attrhash_cmp (const void *p1, const void *p2)
       && attr1->aspath == attr2->aspath
       && attr1->community == attr2->community
       && attr1->med == attr2->med
-      && attr1->local_pref == attr2->local_pref)
+      && attr1->local_pref == attr2->local_pref
+      && attr1->rmap_change_flags == attr2->rmap_change_flags)
     {
       const struct attr_extra *ae1 = attr1->extra;
       const struct attr_extra *ae2 = attr2->extra;
@@ -419,6 +485,7 @@ attrhash_cmp (const void *p1, const void *p2)
           && ae1->aggregator_as == ae2->aggregator_as
           && ae1->aggregator_addr.s_addr == ae2->aggregator_addr.s_addr
           && ae1->weight == ae2->weight
+          && ae1->tag == ae2->tag
 #ifdef HAVE_IPV6
           && ae1->mp_nexthop_len == ae2->mp_nexthop_len
           && IPV6_ADDR_SAME (&ae1->mp_nexthop_global, &ae2->mp_nexthop_global)
@@ -427,7 +494,8 @@ attrhash_cmp (const void *p1, const void *p2)
           && IPV4_ADDR_SAME (&ae1->mp_nexthop_global_in, &ae2->mp_nexthop_global_in)
           && ae1->ecommunity == ae2->ecommunity
           && ae1->cluster == ae2->cluster
-          && ae1->transit == ae2->transit)
+          && ae1->transit == ae2->transit
+          && IPV4_ADDR_SAME (&ae1->originator_id, &ae2->originator_id))
         return 1;
       else if (ae1 || ae2)
         return 0;
@@ -472,7 +540,7 @@ attr_show_all (struct vty *vty)
 static void *
 bgp_attr_hash_alloc (void *p)
 {
-  struct attr * val = (struct attr *) p;
+  const struct attr * val = (const struct attr *) p;
   struct attr *attr;
 
   attr = XMALLOC (MTYPE_ATTR, sizeof (struct attr));
@@ -517,6 +585,7 @@ bgp_attr_intern (struct attr *attr)
             attre->ecommunity = ecommunity_intern (attre->ecommunity);
           else
             attre->ecommunity->refcnt++;
+          
         }
       if (attre->cluster)
         {
@@ -533,13 +602,47 @@ bgp_attr_intern (struct attr *attr)
             attre->transit->refcnt++;
         }
     }
-
+  
   find = (struct attr *) hash_get (attrhash, attr, bgp_attr_hash_alloc);
   find->refcnt++;
-
+  
   return find;
 }
 
+/**
+ * Increment the refcount on various structures that attr holds.
+ * Note on usage: call _only_ when the 'attr' object has already
+ * been 'intern'ed and exists in 'attrhash' table. The function
+ * serves to hold a reference to that (real) object.
+ * Note also that the caller can safely call bgp_attr_unintern()
+ * after calling bgp_attr_refcount(). That would release the
+ * reference and could result in a free() of the attr object.
+ */
+struct attr *
+bgp_attr_refcount (struct attr *attr)
+{
+  /* Intern referenced strucutre. */
+  if (attr->aspath)
+    attr->aspath->refcnt++;
+
+  if (attr->community)
+    attr->community->refcnt++;
+
+  if (attr->extra)
+    {
+      struct attr_extra *attre = attr->extra;
+      if (attre->ecommunity)
+       attre->ecommunity->refcnt++;
+
+      if (attre->cluster)
+       attre->cluster->refcnt++;
+
+      if (attre->transit)
+       attre->transit->refcnt++;
+    }
+  attr->refcnt++;
+  return attr;
+}
 
 /* Make network statement's attribute. */
 struct attr *
@@ -553,6 +656,7 @@ bgp_attr_default_set (struct attr *attr, u_char origin)
   attr->aspath = aspath_empty ();
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS_PATH);
   attr->extra->weight = BGP_ATTR_DEFAULT_WEIGHT;
+  attr->extra->tag = 0;
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP);
 #ifdef HAVE_IPV6
   attr->extra->mp_nexthop_len = IPV6_MAX_BYTELEN;
@@ -568,32 +672,31 @@ bgp_attr_default_intern (u_char origin)
 {
   struct attr attr;
   struct attr *new;
-  struct attr_extra *attre;
-  
-  memset (&attr, 0, sizeof (struct attr));
-  attre = bgp_attr_extra_get (&attr);
-  
+
   bgp_attr_default_set(&attr, origin);
 
   new = bgp_attr_intern (&attr);
   bgp_attr_extra_free (&attr);
   
-  aspath_unintern (new->aspath);
+  aspath_unintern (&new->aspath);
   return new;
 }
 
+/* Create the attributes for an aggregate */
 struct attr *
 bgp_attr_aggregate_intern (struct bgp *bgp, u_char origin,
                           struct aspath *aspath,
-                          struct community *community, int as_set)
+                          struct community *community, int as_set,
+                          u_char atomic_aggregate)
 {
   struct attr attr;
   struct attr *new;
-  struct attr_extra *attre;
+  struct attr_extra attre;
 
   memset (&attr, 0, sizeof (struct attr));
-  attre = bgp_attr_extra_get (&attr);
-  
+  memset (&attre, 0, sizeof (struct attr_extra));
+  attr.extra = &attre;
+
   /* Origin attribute. */
   attr.origin = origin;
   attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_ORIGIN);
@@ -614,68 +717,84 @@ bgp_attr_aggregate_intern (struct bgp *bgp, u_char origin,
       attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_COMMUNITIES);
     }
 
-  attre->weight = BGP_ATTR_DEFAULT_WEIGHT;
+  attre.weight = BGP_ATTR_DEFAULT_WEIGHT;
 #ifdef HAVE_IPV6
-  attre->mp_nexthop_len = IPV6_MAX_BYTELEN;
+  attre.mp_nexthop_len = IPV6_MAX_BYTELEN;
 #endif
-  if (! as_set)
+  if (! as_set || atomic_aggregate)
     attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_ATOMIC_AGGREGATE);
   attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR);
   if (CHECK_FLAG (bgp->config, BGP_CONFIG_CONFEDERATION))
-    attre->aggregator_as = bgp->confed_id;
+    attre.aggregator_as = bgp->confed_id;
   else
-    attre->aggregator_as = bgp->as;
-  attre->aggregator_addr = bgp->router_id;
+    attre.aggregator_as = bgp->as;
+  attre.aggregator_addr = bgp->router_id;
 
   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, ATTR_FLAG_BIT (BGP_ATTR_AS_PATH));
+  
+  if (attr->community)
+    community_unintern (&attr->community);
+  UNSET_FLAG(attr->flag, ATTR_FLAG_BIT (BGP_ATTR_COMMUNITIES));
+  
+  if (attr->extra)
+    {
+      if (attr->extra->ecommunity)
+        ecommunity_unintern (&attr->extra->ecommunity);
+      UNSET_FLAG(attr->flag, ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES));
+      
+      if (attr->extra->cluster)
+        cluster_unintern (attr->extra->cluster);
+      UNSET_FLAG(attr->flag, ATTR_FLAG_BIT (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 **pattr)
 {
+  struct attr *attr = *pattr;
   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;
+  struct attr_extra tmp_extra;
+  
   /* Decrement attribute reference. */
   attr->refcnt--;
-  aspath = attr->aspath;
-  community = attr->community;
+  
+  tmp = *attr;
+  
   if (attr->extra)
     {
-      ecommunity = attr->extra->ecommunity;
-      cluster = attr->extra->cluster;
-      transit = attr->extra->transit;
+      tmp.extra = &tmp_extra;
+      memcpy (tmp.extra, attr->extra, sizeof (struct attr_extra));
     }
-
+  
   /* If reference becomes zero then free attribute object. */
   if (attr->refcnt == 0)
-    {    
+    {
       ret = hash_release (attrhash, attr);
       assert (ret != NULL);
       bgp_attr_extra_free (attr);
       XFREE (MTYPE_ATTR, attr);
+      *pattr = 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
@@ -688,8 +807,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)
@@ -697,32 +817,212 @@ bgp_attr_flush (struct attr *attr)
     }
 }
 
-/* Get origin attribute of the update message. */
-static int
-bgp_attr_origin (struct peer *peer, bgp_size_t length, 
-                struct attr *attr, u_char flag, u_char *startp)
+/* 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 bgp_attr_parser_args *args, u_char subcode,
+                    bgp_size_t length)
 {
-  bgp_size_t total;
+  struct peer *const peer = args->peer; 
+  const u_int8_t flags = args->flags;
+  /* startp and length must be special-cased, as whether or not to
+   * send the attribute data with the NOTIFY depends on the error,
+   * the caller therefore signals this with the seperate length argument
+   */
+  u_char *notify_datap = (length > 0 ? args->startp : NULL);
+  
+  /* Only relax error handling for eBGP peers */
+  if (peer->sort != BGP_PEER_EBGP)
+    {
+      bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, subcode,
+                                 notify_datap, length);
+      return BGP_ATTR_PARSE_ERROR;
 
-  /* total is entire attribute length include Attribute Flags (1),
-     Attribute Type code (1) and Attribute length (1 or 2).  */
-  total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
+    }
+  
+  /* Adjust the stream getp to the end of the attribute, in case we can
+   * still proceed but the caller hasn't read all the attribute.
+   */
+  stream_set_getp (BGP_INPUT (peer),
+                   (args->startp - STREAM_DATA (BGP_INPUT (peer)))
+                    + args->total);
+  
+  switch (args->type) {
+    /* where an attribute is relatively 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,
+                                 notify_datap, 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 (flags, BGP_ATTR_FLAG_TRANS)
+      && CHECK_FLAG (flags, BGP_ATTR_FLAG_OPTIONAL)
+      && CHECK_FLAG (flags, BGP_ATTR_FLAG_PARTIAL))
+    return BGP_ATTR_PARSE_WITHDRAW;
+  
+  /* default to reset */
+  return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
+}
 
-  /* If any recognized attribute has Attribute Flags that conflict
-     with the Attribute Type Code, then the Error Subcode is set to
-     Attribute Flags Error.  The Data field contains the erroneous
-     attribute (type, length and value). */
-  if (flag != BGP_ATTR_FLAG_TRANS)
+/* Find out what is wrong with the path attribute flag bits and log the error.
+   "Flag bits" here stand for Optional, Transitive and Partial, but not for
+   Extended Length. Checking O/T/P bits at once implies, that the attribute
+   being diagnosed is defined by RFC as either a "well-known" or an "optional,
+   non-transitive" attribute. */
+static void
+bgp_attr_flags_diagnose (struct bgp_attr_parser_args *args,
+                         u_int8_t desired_flags /* how RFC says it must be */
+)
+{
+  u_char seen = 0, i;
+  u_char real_flags = args->flags;
+  const u_int8_t attr_code = args->type;
+  
+  desired_flags &= ~BGP_ATTR_FLAG_EXTLEN;
+  real_flags &= ~BGP_ATTR_FLAG_EXTLEN;
+  for (i = 0; i <= 2; i++) /* O,T,P, but not E */
+    if
+    (
+      CHECK_FLAG (desired_flags, attr_flag_str[i].key) !=
+      CHECK_FLAG (real_flags,    attr_flag_str[i].key)
+    )
+    {
+      zlog_err ("%s attribute must%s be flagged as \"%s\"",
+                LOOKUP (attr_str, attr_code),
+                CHECK_FLAG (desired_flags, attr_flag_str[i].key) ? "" : " not",
+                attr_flag_str[i].str);
+      seen = 1;
+    }
+  if (!seen)
+    {
+      zlog_debug ("Strange, %s called for attr %s, but no problem found with flags"
+                  " (real flags 0x%x, desired 0x%x)",
+                  __func__, LOOKUP (attr_str, attr_code),
+                  real_flags, desired_flags);
+    }
+}
+
+/* Required flags for attributes. EXTLEN will be masked off when testing,
+ * as will PARTIAL for optional+transitive attributes.
+ */
+const u_int8_t attr_flags_values [] = {
+  [BGP_ATTR_ORIGIN] =           BGP_ATTR_FLAG_TRANS,
+  [BGP_ATTR_AS_PATH] =          BGP_ATTR_FLAG_TRANS,
+  [BGP_ATTR_NEXT_HOP] =         BGP_ATTR_FLAG_TRANS,
+  [BGP_ATTR_MULTI_EXIT_DISC] =  BGP_ATTR_FLAG_OPTIONAL,
+  [BGP_ATTR_LOCAL_PREF] =       BGP_ATTR_FLAG_TRANS,
+  [BGP_ATTR_ATOMIC_AGGREGATE] = BGP_ATTR_FLAG_TRANS,
+  [BGP_ATTR_AGGREGATOR] =       BGP_ATTR_FLAG_TRANS | BGP_ATTR_FLAG_OPTIONAL,
+  [BGP_ATTR_COMMUNITIES] =      BGP_ATTR_FLAG_TRANS | BGP_ATTR_FLAG_OPTIONAL,
+  [BGP_ATTR_ORIGINATOR_ID] =    BGP_ATTR_FLAG_OPTIONAL,
+  [BGP_ATTR_CLUSTER_LIST] =     BGP_ATTR_FLAG_OPTIONAL,
+  [BGP_ATTR_MP_REACH_NLRI] =    BGP_ATTR_FLAG_OPTIONAL,
+  [BGP_ATTR_MP_UNREACH_NLRI] =  BGP_ATTR_FLAG_OPTIONAL,
+  [BGP_ATTR_EXT_COMMUNITIES] =  BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS,
+  [BGP_ATTR_AS4_PATH] =         BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS,
+  [BGP_ATTR_AS4_AGGREGATOR] =   BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS,
+};
+static const size_t attr_flags_values_max =
+  sizeof (attr_flags_values) / sizeof (attr_flags_values[0]);
+
+static int
+bgp_attr_flag_invalid (struct bgp_attr_parser_args *args)
+{
+  u_int8_t mask = BGP_ATTR_FLAG_EXTLEN;
+  const u_int8_t flags = args->flags;
+  const u_int8_t attr_code = args->type;
+  
+  /* there may be attributes we don't know about */
+  if (attr_code > attr_flags_values_max)
+    return 0;
+  if (attr_flags_values[attr_code] == 0)
+    return 0;
+  
+  /* RFC4271, "For well-known attributes, the Transitive bit MUST be set to
+   * 1."
+   */
+  if (!CHECK_FLAG (BGP_ATTR_FLAG_OPTIONAL, flags)
+      && !CHECK_FLAG (BGP_ATTR_FLAG_TRANS, flags))
+    {
+      zlog_err ("%s well-known attributes must have transitive flag set (%x)",
+                LOOKUP (attr_str, attr_code), flags);
+      return 1;
+    }
+  
+  /* "For well-known attributes and for optional non-transitive attributes,
+   *  the Partial bit MUST be set to 0." 
+   */
+  if (CHECK_FLAG (flags, BGP_ATTR_FLAG_PARTIAL))
     {
-      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;
+      if (!CHECK_FLAG (flags, BGP_ATTR_FLAG_OPTIONAL))
+        {
+          zlog_err ("%s well-known attribute "
+                    "must NOT have the partial flag set (%x)",
+                    LOOKUP (attr_str, attr_code), flags);
+          return 1;
+        }
+      if (CHECK_FLAG (flags, BGP_ATTR_FLAG_OPTIONAL)
+          && !CHECK_FLAG (flags, BGP_ATTR_FLAG_TRANS))
+        {
+          zlog_err ("%s optional + transitive attribute "
+                    "must NOT have the partial flag set (%x)",
+                    LOOKUP (attr_str, attr_code), flags);
+          return 1;
+        }
     }
+  
+  /* Optional transitive attributes may go through speakers that don't
+   * reocgnise them and set the Partial bit.
+   */
+  if (CHECK_FLAG (flags, BGP_ATTR_FLAG_OPTIONAL)
+      && CHECK_FLAG (flags, BGP_ATTR_FLAG_TRANS))
+    SET_FLAG (mask, BGP_ATTR_FLAG_PARTIAL);
+  
+  if ((flags & ~mask)
+      == attr_flags_values[attr_code])
+    return 0;
+  
+  bgp_attr_flags_diagnose (args, attr_flags_values[attr_code]);
+  return 1;
+}
 
+/* Get origin attribute of the update message. */
+static bgp_attr_parse_ret_t
+bgp_attr_origin (struct bgp_attr_parser_args *args)
+{
+  struct peer *const peer = args->peer;
+  struct attr *const attr = args->attr;
+  const bgp_size_t length = args->length;
+  
   /* If any recognized attribute has Attribute Length that conflicts
      with the expected length (based on the attribute type code), then
      the Error Subcode is set to Attribute Length Error.  The Data
@@ -730,12 +1030,10 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length,
      value). */
   if (length != 1)
     {
-      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;
+      zlog_err ("Origin attribute length is not one %d", length);
+      return bgp_attr_malformed (args,
+                                 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 args->total);
     }
 
   /* Fetch origin attribute. */
@@ -748,14 +1046,10 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length,
       && (attr->origin != BGP_ORIGIN_EGP)
       && (attr->origin != BGP_ORIGIN_INCOMPLETE))
     {
-      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;
+      zlog_err ("Origin attribute value is invalid %d", attr->origin);
+      return bgp_attr_malformed (args,
+                                 BGP_NOTIFY_UPDATE_INVAL_ORIGIN,
+                                 args->total);
     }
 
   /* Set oring attribute flag. */
@@ -763,82 +1057,38 @@ 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 *
-bgp_attr_aspath (struct peer *peer, bgp_size_t length, 
-                struct attr *attr, u_char flag, u_char *startp, int as4_path)
-{
-  u_char require ;
-  struct aspath *asp ;
-
-  /* Check the attribute flags                                          */
-  require = as4_path ? BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS
-                     :                          BGP_ATTR_FLAG_TRANS ;
-
-  if ((flag & (BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS)) != require)
-    {
-      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 ;
-    } ;
-
-  /* Parse the AS_PATH/AS4_PATH body.
-   *
-   * For AS_PATH  peer with AS4 => 4Byte ASN otherwise 2Byte ASN
-   *     AS4_PATH 4Byte ASN
+/* Parse AS path information.  This function is wrapper of
+   aspath_parse. */
+static int
+bgp_attr_aspath (struct bgp_attr_parser_args *args)
+{
+  struct attr *const attr = args->attr;
+  struct peer *const peer = args->peer; 
+  const bgp_size_t length = args->length;
+  
+  /*
+   * 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_err ("Malformed AS path from %s, length is %d", peer->host, length);
+      return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_MAL_AS_PATH, 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 *const peer, struct attr *const attr)
 {
   /* These checks were part of bgp_attr_aspath, but with
    * as4 we should to check aspath things when
@@ -850,31 +1100,26 @@ static int bgp_attr_aspath_check( struct peer *peer,
   struct bgp *bgp = peer->bgp;
   struct aspath *aspath;
 
-  bgp = peer->bgp;
-    
   /* Confederation sanity check. */
-  if ((peer_sort (peer) == BGP_PEER_CONFED && ! aspath_left_confed_check (attr->aspath)) ||
-     (peer_sort (peer) == BGP_PEER_EBGP && aspath_confed_check (attr->aspath)))
+  if ((peer->sort == BGP_PEER_CONFED && ! aspath_left_confed_check (attr->aspath)) ||
+     (peer->sort == 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;
+      zlog_err ("Malformed AS path from %s", peer->host);
+      bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR,
+                       BGP_NOTIFY_UPDATE_MAL_AS_PATH);
+      return BGP_ATTR_PARSE_ERROR;
     }
 
   /* First AS check for EBGP. */
   if (bgp != NULL && bgp_flag_check (bgp, BGP_FLAG_ENFORCE_FIRST_AS))
     {
-      if (peer_sort (peer) == BGP_PEER_EBGP 
+      if (peer->sort == BGP_PEER_EBGP
          && ! aspath_firstas_check (attr->aspath, peer->as))
        {
-         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;
+          zlog_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 BGP_ATTR_PARSE_ERROR;
        }
     }
 
@@ -884,67 +1129,58 @@ 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;
 }
 
-/* Nexthop attribute. */
+/* Parse AS4 path information.  This function is another wrapper of
+   aspath_parse. */
 static int
-bgp_attr_nexthop (struct peer *peer, bgp_size_t length, 
-                 struct attr *attr, u_char flag, u_char *startp)
+bgp_attr_as4_path (struct bgp_attr_parser_args *args, struct aspath **as4_path)
 {
-  bgp_size_t total;
-  in_addr_t nexthop_h, nexthop_n;
-
-  total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
+  struct peer *const peer = args->peer; 
+  struct attr *const attr = args->attr;
+  const bgp_size_t length = args->length;
+  
+  *as4_path = aspath_parse (peer->ibuf, length, 1);
 
-  /* Flags check. */
-  if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
-    {
-      zlog (peer->log, LOG_ERR,
-           "NEXT_HOP attribute must not be flagged as \"optional\" (%u)", flag);
-      bgp_notify_send_with_data (peer,
-                                BGP_NOTIFY_UPDATE_ERR,
-                                BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
-                                startp, total);
-      return -1;
-    }
-  if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
+  /* In case of IBGP, length will be zero. */
+  if (!*as4_path)
     {
-      zlog (peer->log, LOG_ERR,
-           "NEXT_HOP attribute must be flagged as \"transitive\" (%u)", flag);
-      bgp_notify_send_with_data (peer,
-                                BGP_NOTIFY_UPDATE_ERR,
-                                BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
-                                startp, total);
-      return -1;
-    }
-  if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
-    {
-      zlog (peer->log, LOG_ERR,
-           "NEXT_HOP attribute must not be flagged as \"partial\" (%u)", flag);
-      bgp_notify_send_with_data (peer,
-                                BGP_NOTIFY_UPDATE_ERR,
-                                BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
-                                startp, total);
-      return -1;
+      zlog_err ("Malformed AS4 path from %s, length is %d", peer->host, length);
+      return bgp_attr_malformed (args,
+                                 BGP_NOTIFY_UPDATE_MAL_AS_PATH,
+                                 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 bgp_attr_parse_ret_t
+bgp_attr_nexthop (struct bgp_attr_parser_args *args)
+{
+  struct peer *const peer = args->peer; 
+  struct attr *const attr = args->attr;
+  const bgp_size_t length = args->length;
+  
+  in_addr_t nexthop_h, nexthop_n;
+
   /* Check nexthop attribute length. */
   if (length != 4)
     {
-      zlog (peer->log, LOG_ERR, "Nexthop attribute length isn't four [%d]",
-             length);
+      zlog_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 (args,
+                                 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 args->total);
     }
 
   /* According to section 6.3 of RFC4271, syntactically incorrect NEXT_HOP
@@ -957,209 +1193,121 @@ bgp_attr_nexthop (struct peer *peer, bgp_size_t length,
   if (IPV4_NET0 (nexthop_h) || IPV4_NET127 (nexthop_h) || IPV4_CLASS_DE (nexthop_h))
     {
       char buf[INET_ADDRSTRLEN];
-      inet_ntop (AF_INET, &nexthop_h, buf, INET_ADDRSTRLEN);
-      zlog (peer->log, LOG_ERR, "Martian nexthop %s", buf);
-      bgp_notify_send_with_data (peer,
-                                BGP_NOTIFY_UPDATE_ERR,
-                                BGP_NOTIFY_UPDATE_INVAL_NEXT_HOP,
-                                startp, total);
-      return -1;
+      inet_ntop (AF_INET, &nexthop_n, buf, INET_ADDRSTRLEN);
+      zlog_err ("Martian nexthop %s", buf);
+      return bgp_attr_malformed (args,
+                                 BGP_NOTIFY_UPDATE_INVAL_NEXT_HOP,
+                                 args->total);
     }
 
   attr->nexthop.s_addr = nexthop_n;
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* MED atrribute. */
-static int
-bgp_attr_med (struct peer *peer, bgp_size_t length, 
-             struct attr *attr, u_char flag, u_char *startp)
+static bgp_attr_parse_ret_t
+bgp_attr_med (struct bgp_attr_parser_args *args)
 {
-  bgp_size_t total;
-
-  total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
-
-  /* Flag checks. */
-  if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
-    {
-      zlog (peer->log, LOG_ERR,
-           "MULTI_EXIT_DISC attribute must be flagged as \"optional\" (%u)", flag);
-      bgp_notify_send_with_data (peer,
-                                BGP_NOTIFY_UPDATE_ERR,
-                                BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
-                                startp, total);
-      return -1;
-    }
-  if (CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
-    {
-      zlog (peer->log, LOG_ERR,
-           "MULTI_EXIT_DISC attribute must not be flagged as \"transitive\" (%u)", flag);
-      bgp_notify_send_with_data (peer,
-                                BGP_NOTIFY_UPDATE_ERR,
-                                BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
-                                startp, total);
-      return -1;
-    }
-  if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
-    {
-      zlog (peer->log, LOG_ERR,
-           "MULTI_EXIT_DISC attribute must not be flagged as \"partial\" (%u)", flag);
-      bgp_notify_send_with_data (peer,
-                                BGP_NOTIFY_UPDATE_ERR,
-                                BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
-                                startp, total);
-      return -1;
-    }
-
+  struct peer *const peer = args->peer; 
+  struct attr *const attr = args->attr;
+  const bgp_size_t length = args->length;
+  
   /* Length check. */
   if (length != 4)
     {
-      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;
+      zlog_err ("MED attribute length isn't four [%d]", length);
+
+      return bgp_attr_malformed (args,
+                                 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 args->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
-bgp_attr_local_pref (struct peer *peer, bgp_size_t length, 
-                    struct attr *attr, u_char flag, u_char *startp)
+static bgp_attr_parse_ret_t
+bgp_attr_local_pref (struct bgp_attr_parser_args *args)
 {
-  bgp_size_t total;
-
-  total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
-  /* Flag checks. */
-  if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
-    {
-      zlog (peer->log, LOG_ERR,
-           "LOCAL_PREF attribute must be flagged as \"well-known\" (%u)", flag);
-      bgp_notify_send_with_data (peer,
-                                BGP_NOTIFY_UPDATE_ERR,
-                                BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
-                                startp, total);
-      return -1;
-    }
-  if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
-    {
-      zlog (peer->log, LOG_ERR,
-           "LOCAL_PREF attribute must be flagged as \"transitive\" (%u)", flag);
-      bgp_notify_send_with_data (peer,
-                                BGP_NOTIFY_UPDATE_ERR,
-                                BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
-                                startp, total);
-      return -1;
-    }
+  struct peer *const peer = args->peer; 
+  struct attr *const attr = args->attr;
+  const bgp_size_t length = args->length;
+  
+  /* Length check. */
+  if (length != 4)
+  {
+    zlog_err ("LOCAL_PREF attribute length isn't 4 [%u]", length);
+    return bgp_attr_malformed (args,
+                               BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                               args->total);
+  }
 
   /* If it is contained in an UPDATE message that is received from an
      external peer, then this attribute MUST be ignored by the
      receiving speaker. */
-  if (peer_sort (peer) == BGP_PEER_EBGP)
+  if (peer->sort == BGP_PEER_EBGP)
     {
       stream_forward_getp (peer->ibuf, length);
-      return 0;
+      return BGP_ATTR_PARSE_PROCEED;
     }
 
-  if (length == 4) 
-    attr->local_pref = stream_getl (peer->ibuf);
-  else 
-    attr->local_pref = 0;
+  attr->local_pref = stream_getl (peer->ibuf);
 
   /* Set atomic aggregate flag. */
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_LOCAL_PREF);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Atomic aggregate. */
 static int
-bgp_attr_atomic (struct peer *peer, bgp_size_t length, 
-                struct attr *attr, u_char flag, u_char *startp)
+bgp_attr_atomic (struct bgp_attr_parser_args *args)
 {
-  bgp_size_t total;
-
-  total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
-  /* Flag checks. */
-  if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
-    {
-      zlog (peer->log, LOG_ERR,
-           "ATOMIC_AGGREGATE attribute must not be flagged as \"optional\" (%u)", flag);
-      bgp_notify_send_with_data (peer,
-                                BGP_NOTIFY_UPDATE_ERR,
-                                BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
-                                startp, total);
-      return -1;
-    }
-  if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
-    {
-      zlog (peer->log, LOG_ERR,
-           "ATOMIC_AGGREGATE attribute must be flagged as \"transitive\" (%u)", flag);
-      bgp_notify_send_with_data (peer,
-                                BGP_NOTIFY_UPDATE_ERR,
-                                BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
-                                startp, total);
-      return -1;
-    }
-  if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
-    {
-      zlog (peer->log, LOG_ERR,
-           "ATOMIC_AGGREGATE attribute must not be flagged as \"partial\" (%u)", flag);
-      bgp_notify_send_with_data (peer,
-                                BGP_NOTIFY_UPDATE_ERR,
-                                BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
-                                startp, total);
-      return -1;
-    }
-
+  struct attr *const attr = args->attr;
+  const bgp_size_t length = args->length;
+  
   /* Length check. */
   if (length != 0)
     {
-      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;
+      zlog_err ("ATOMIC_AGGREGATE attribute length isn't 0 [%u]", length);
+      return bgp_attr_malformed (args,
+                                 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 args->total);
     }
 
   /* Set atomic aggregate flag. */
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_ATOMIC_AGGREGATE);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Aggregator attribute */
 static int
-bgp_attr_aggregator (struct peer *peer, bgp_size_t length,
-                    struct attr *attr, u_char flag)
+bgp_attr_aggregator (struct bgp_attr_parser_args *args)
 {
+  struct peer *const peer = args->peer; 
+  struct attr *const attr = args->attr;
+  const bgp_size_t length = args->length;
+  
   int wantedlen = 6;
   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;
+      zlog_err ("AGGREGATOR attribute length isn't %u [%u]", wantedlen, length);
+      return bgp_attr_malformed (args,
+                                 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 args->total);
     }
   
   if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV ) )
@@ -1171,44 +1319,58 @@ 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
-bgp_attr_as4_aggregator (struct peer *peer, bgp_size_t length,
-                    struct attr *attr, as_t *as4_aggregator_as,
-                    struct in_addr *as4_aggregator_addr)
+static bgp_attr_parse_ret_t
+bgp_attr_as4_aggregator (struct bgp_attr_parser_args *args,
+                        as_t *as4_aggregator_as,
+                        struct in_addr *as4_aggregator_addr)
 {
+  struct peer *const peer = args->peer; 
+  struct attr *const attr = args->attr;
+  const bgp_size_t length = args->length;
+      
   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;
+      zlog_err ("New Aggregator length is not 8 [%d]", length);
+      return bgp_attr_malformed (args,
+                                 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 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 *const peer,
+                          struct attr *const attr,
                           struct aspath *as4_path, as_t as4_aggregator,
                           struct in_addr *as4_aggregator_addr)
 {
   int ignore_as4_path = 0;
   struct aspath *newpath;
   struct attr_extra *attre = attr->extra;
-    
-  if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV) )
+
+  if (!attr->aspath)
+    {
+      /* NULL aspath shouldn't be possible as bgp_attr_parse should have
+       * checked that all well-known, mandatory attributes were present.
+       *
+       * Can only be a problem with peer itself - hard error
+       */
+      return BGP_ATTR_PARSE_ERROR;
+    }
+
+  if (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV))
     {
       /* peer can do AS4, so we ignore AS4_PATH and AS4_AGGREGATOR
        * if given.
@@ -1226,34 +1388,15 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
                         peer->host, "AS4 capable peer, yet it sent");
         }
       
-      return 0;
-    }
-  
-  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,
-       *   This should already
-       *   have been handled by 'well known attributes missing'
-       *   But... yeah, paranoia
-       * Take this as a "malformed attribute"
-       */
-      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_PARSE_PROCEED;
     }
-
+  
   /* We have a asn16 peer.  First, look for AS4_AGGREGATOR
    * because that may override AS4_PATH
    */
   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);
           
@@ -1269,7 +1412,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))
@@ -1304,24 +1447,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);
-       attr->aspath = aspath_intern (newpath);
+      newpath = aspath_reconcile_as4 (attr->aspath, as4_path);
+      aspath_unintern (&attr->aspath);
+      attr->aspath = aspath_intern (newpath);
     }
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Community attribute. */
-static int
-bgp_attr_community (struct peer *peer, bgp_size_t length, 
-                   struct attr *attr, u_char flag)
+static bgp_attr_parse_ret_t
+bgp_attr_community (struct bgp_attr_parser_args *args)
 {
+  struct peer *const peer = args->peer; 
+  struct attr *const attr = args->attr;  
+  const bgp_size_t length = args->length;
+  
   if (length == 0)
     {
       attr->community = NULL;
-      return 0;
+      return BGP_ATTR_PARSE_PROCEED;
     }
   
   attr->community =
@@ -1331,26 +1477,31 @@ 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 (args,
+                               BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
+                               args->total);
   
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_COMMUNITIES);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Originator ID attribute. */
-static int
-bgp_attr_originator_id (struct peer *peer, bgp_size_t length, 
-                       struct attr *attr, u_char flag)
+static bgp_attr_parse_ret_t
+bgp_attr_originator_id (struct bgp_attr_parser_args *args)
 {
+  struct peer *const peer = args->peer; 
+  struct attr *const attr = args->attr;
+  const bgp_size_t length = args->length;
+  
+  /* Length check. */
   if (length != 4)
     {
-      zlog (peer->log, LOG_ERR, "Bad originator ID length %d", length);
+      zlog_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 (args,
+                                 BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 args->total);
     }
 
   (bgp_attr_extra_get (attr))->originator_id.s_addr 
@@ -1358,46 +1509,52 @@ 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
-bgp_attr_cluster_list (struct peer *peer, bgp_size_t length, 
-                      struct attr *attr, u_char flag)
+static bgp_attr_parse_ret_t
+bgp_attr_cluster_list (struct bgp_attr_parser_args *args)
 {
+  struct peer *const peer = args->peer; 
+  struct attr *const attr = args->attr;
+  const bgp_size_t length = args->length;
+  
   /* Check length. */
   if (length % 4)
     {
-      zlog (peer->log, LOG_ERR, "Bad cluster list length %d", length);
+      zlog_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 (args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                 args->total);
     }
 
   (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. */
 int
-bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
-                   struct bgp_nlri *mp_update)
+bgp_mp_reach_parse (struct bgp_attr_parser_args *args,
+                    struct bgp_nlri *mp_update)
 {
   afi_t afi;
   safi_t safi;
   bgp_size_t nlri_len;
   size_t start;
   int ret;
+  int num_mp_pfx = 0;
   struct stream *s;
+  struct peer *const peer = args->peer;  
+  struct attr *const attr = args->attr;
+  const bgp_size_t length = args->length;
   struct attr_extra *attre = bgp_attr_extra_get(attr);
   
   /* Set end of packet. */
@@ -1411,7 +1568,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_NOTIFYPLS;
     }
   
   /* Load AFI, SAFI. */
@@ -1425,62 +1582,75 @@ 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_NOTIFYPLS;
     }
   
   /* Nexthop length check. */
   switch (attre->mp_nexthop_len)
     {
-    case 4:
-      stream_get (&attre->mp_nexthop_global_in, s, 4);
+    case BGP_ATTR_NHLEN_IPV4:
+      stream_get (&attre->mp_nexthop_global_in, s, IPV4_MAX_BYTELEN);
       /* Probably needed for RFC 2283 */
       if (attr->nexthop.s_addr == 0)
-        memcpy(&attr->nexthop.s_addr, &attre->mp_nexthop_global_in, 4);
+        memcpy(&attr->nexthop.s_addr, &attre->mp_nexthop_global_in, IPV4_MAX_BYTELEN);
       break;
-    case 12:
-      {
-       u_int32_t rd_high;
-       u_int32_t rd_low;
-
-       rd_high = stream_getl (s);
-       rd_low = stream_getl (s);
-       stream_get (&attre->mp_nexthop_global_in, s, 4);
-      }
+    case BGP_ATTR_NHLEN_VPNV4:
+      stream_getl (s); /* RD high */
+      stream_getl (s); /* RD low */
+      stream_get (&attre->mp_nexthop_global_in, s, IPV4_MAX_BYTELEN);
       break;
 #ifdef HAVE_IPV6
-    case 16:
-      stream_get (&attre->mp_nexthop_global, s, 16);
+    case BGP_ATTR_NHLEN_IPV6_GLOBAL:
+    case BGP_ATTR_NHLEN_VPNV6_GLOBAL:
+      if (attre->mp_nexthop_len == BGP_ATTR_NHLEN_VPNV6_GLOBAL)
+        {
+          stream_getl (s); /* RD high */
+          stream_getl (s); /* RD low */
+        }
+      stream_get (&attre->mp_nexthop_global, s, IPV6_MAX_BYTELEN);
       break;
-    case 32:
-      stream_get (&attre->mp_nexthop_global, s, 16);
-      stream_get (&attre->mp_nexthop_local, s, 16);
+    case BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL:
+    case BGP_ATTR_NHLEN_VPNV6_GLOBAL_AND_LL:
+      if (attre->mp_nexthop_len == BGP_ATTR_NHLEN_VPNV6_GLOBAL_AND_LL)
+        {
+          stream_getl (s); /* RD high */
+          stream_getl (s); /* RD low */
+        }
+      stream_get (&attre->mp_nexthop_global, s, IPV6_MAX_BYTELEN);
+      if (attre->mp_nexthop_len == BGP_ATTR_NHLEN_VPNV6_GLOBAL_AND_LL)
+        {
+          stream_getl (s); /* RD high */
+          stream_getl (s); /* RD low */
+        }
+      stream_get (&attre->mp_nexthop_local, s, IPV6_MAX_BYTELEN);
       if (! IN6_IS_ADDR_LINKLOCAL (&attre->mp_nexthop_local))
        {
          char buf1[INET6_ADDRSTRLEN];
          char buf2[INET6_ADDRSTRLEN];
 
-         if (BGP_DEBUG (update, UPDATE_IN))
-           zlog_debug ("%s got two nexthop %s %s but second one is not a link-local nexthop", peer->host,
+         if (bgp_debug_update(peer, NULL, NULL, 1))
+            zlog_debug ("%s rcvd nexthops %s, %s -- ignoring non-LL value",
+                        peer->host,
                       inet_ntop (AF_INET6, &attre->mp_nexthop_global,
                                  buf1, INET6_ADDRSTRLEN),
                       inet_ntop (AF_INET6, &attre->mp_nexthop_local,
                                  buf2, INET6_ADDRSTRLEN));
 
-         attre->mp_nexthop_len = 16;
+         attre->mp_nexthop_len = IPV6_MAX_BYTELEN;
        }
       break;
 #endif /* HAVE_IPV6 */
     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_NOTIFYPLS;
     }
 
   if (!LEN_LEFT)
     {
       zlog_info ("%s: (%s) Failed to read SNPA and NLRI(s)",
                  __func__, peer->host);
-      return -1;
+      return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
     }
   
   {
@@ -1496,17 +1666,18 @@ 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_NOTIFYPLS;
     }
  
   if (safi != SAFI_MPLS_LABELED_VPN)
     {
-      ret = bgp_nlri_sanity_check (peer, afi, stream_pnt (s), nlri_len);
+      ret = bgp_nlri_sanity_check (peer, afi, safi, stream_pnt (s),
+                                   nlri_len, &num_mp_pfx);
       if (ret < 0) 
         {
           zlog_info ("%s: (%s) NLRI doesn't pass sanity check",
                      __func__, peer->host);
-         return -1;
+         return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
        }
     }
 
@@ -1517,13 +1688,15 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
 
   stream_forward_getp (s, nlri_len);
 
-  return 0;
+  attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_MP_REACH_NLRI);
+
+  return BGP_ATTR_PARSE_PROCEED;
 #undef LEN_LEFT
 }
 
 /* Multiprotocol unreachable parse */
 int
-bgp_mp_unreach_parse (struct peer *peer, bgp_size_t length, 
+bgp_mp_unreach_parse (struct bgp_attr_parser_args *args,
                      struct bgp_nlri *mp_withdraw)
 {
   struct stream *s;
@@ -1531,12 +1704,16 @@ bgp_mp_unreach_parse (struct peer *peer, bgp_size_t length,
   safi_t safi;
   u_int16_t withdraw_len;
   int ret;
+  int num_mp_pfx = 0;
+  struct peer *const peer = args->peer;  
+  struct attr *const attr = args->attr;
+  const bgp_size_t length = args->length;
 
   s = peer->ibuf;
   
 #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_NOTIFYPLS;
   
   afi = stream_getw (s);
   safi = stream_getc (s);
@@ -1545,9 +1722,10 @@ bgp_mp_unreach_parse (struct peer *peer, bgp_size_t length,
 
   if (safi != SAFI_MPLS_LABELED_VPN)
     {
-      ret = bgp_nlri_sanity_check (peer, afi, stream_pnt (s), withdraw_len);
+      ret = bgp_nlri_sanity_check (peer, afi, safi, stream_pnt (s),
+                                  withdraw_len, &num_mp_pfx);
       if (ret < 0)
-       return -1;
+       return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
     }
 
   mp_withdraw->afi = afi;
@@ -1557,20 +1735,25 @@ bgp_mp_unreach_parse (struct peer *peer, bgp_size_t length,
 
   stream_forward_getp (s, withdraw_len);
 
-  return 0;
+  attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_MP_UNREACH_NLRI);
+
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Extended Community attribute. */
-static int
-bgp_attr_ext_communities (struct peer *peer, bgp_size_t length, 
-                         struct attr *attr, u_char flag)
+static bgp_attr_parse_ret_t
+bgp_attr_ext_communities (struct bgp_attr_parser_args *args)
 {
+  struct peer *const peer = args->peer;  
+  struct attr *const attr = args->attr;  
+  const bgp_size_t length = args->length;
+  
   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 =
@@ -1579,54 +1762,51 @@ 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 (args,
+                               BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
+                               args->total);
   
   attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES);
 
-  return 0;
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* BGP unknown attribute treatment. */
-static int
-bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag,
-                 u_char type, bgp_size_t length, u_char *startp)
+static bgp_attr_parse_ret_t
+bgp_attr_unknown (struct bgp_attr_parser_args *args)
 {
-  bgp_size_t total;
+  bgp_size_t total = args->total;
   struct transit *transit;
   struct attr_extra *attre;
-
-  if (BGP_DEBUG (normal, NORMAL))
-  zlog_debug ("%s Unknown attribute is received (type %d, length %d)",
-             peer->host, type, length);
+  struct peer *const peer = args->peer; 
+  struct attr *const attr = args->attr;
+  u_char *const startp = args->startp;
+  const u_char type = args->type;
+  const u_char flag = args->flags;  
+  const bgp_size_t length = args->length;
+  
+  if (bgp_debug_update(peer, NULL, NULL, 1))
+    zlog_debug ("%s Unknown attribute is received (type %d, length %d)",
+                peer->host, type, length);
   
-  if (BGP_DEBUG (events, EVENTS))
-    zlog (peer->log, LOG_DEBUG, 
-         "Unknown attribute type %d length %d is received", type, length);
-
   /* Forward read pointer of input stream. */
   stream_forward_getp (peer->ibuf, length);
 
-  /* Adjest total length to include type and length. */
-  total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
-
   /* If any of the mandatory well-known attributes are not recognized,
      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 (args,
+                                 BGP_NOTIFY_UPDATE_UNREC_ATTR,
+                                 args->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
@@ -1649,17 +1829,66 @@ 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;
+}
+
+/* Well-known attribute check. */
+static int
+bgp_attr_check (struct peer *peer, struct attr *attr)
+{
+  u_char type = 0;
+
+  /* BGP Graceful-Restart End-of-RIB for IPv4 unicast is signaled as an
+   * empty UPDATE.  */
+  if (CHECK_FLAG (peer->cap, PEER_CAP_RESTART_RCV) && !attr->flag)
+    return BGP_ATTR_PARSE_PROCEED;
+
+  /* "An UPDATE message that contains the MP_UNREACH_NLRI is not required
+     to carry any other path attributes.", though if MP_REACH_NLRI or NLRI
+     are present, it should.  Check for any other attribute being present
+     instead.
+   */
+  if (attr->flag == ATTR_FLAG_BIT (BGP_ATTR_MP_UNREACH_NLRI))
+    return BGP_ATTR_PARSE_PROCEED;
+
+  if (! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_ORIGIN)))
+    type = BGP_ATTR_ORIGIN;
+
+  if (! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_AS_PATH)))
+    type = BGP_ATTR_AS_PATH;
+
+  /* RFC 2858 makes Next-Hop optional/ignored, if MP_REACH_NLRI is present and
+   * NLRI is empty. We can't easily check NLRI empty here though.
+   */
+  if (!CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP))
+      && !CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_MP_REACH_NLRI)))
+    type = BGP_ATTR_NEXT_HOP;
+
+  if (peer->sort == BGP_PEER_IBGP
+      && ! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_LOCAL_PREF)))
+    type = BGP_ATTR_LOCAL_PREF;
+
+  if (type)
+    {
+      zlog_warn ("%s Missing well-known attribute %s.", peer->host,
+                 LOOKUP (attr_str, type));
+      bgp_notify_send_with_data (peer,
+                                BGP_NOTIFY_UPDATE_ERR,
+                                BGP_NOTIFY_UPDATE_MISS_ATTR,
+                                &type, 1);
+      return BGP_ATTR_PARSE_ERROR;
+    }
+  return BGP_ATTR_PARSE_PROCEED;
 }
 
 /* Read attribute of update packet.  This function is called from
-   bgp_update() in bgpd.c.  */
-int
+   bgp_update_receive() in bgp_packet.c.  */
+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;
@@ -1669,14 +1898,14 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
   /* same goes for as4_aggregator */
   struct aspath *as4_path = NULL;
   as_t as4_aggregator = 0;
-  struct in_addr as4_aggregator_addr = { 0 };
+  struct in_addr as4_aggregator_addr = { .s_addr = 0 };
 
   /* Initialize bitmap. */
   memset (seen, 0, BGP_ATTR_BITMAP_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)
     {
@@ -1684,37 +1913,38 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
       if (endp - BGP_INPUT_PNT (peer) < BGP_ATTR_MIN_LEN)
        {
          /* XXX warning: long int format, int arg (arg 5) */
-         zlog (peer->log, LOG_WARNING, 
-               "%s: error BGP attribute length %lu is smaller than min len",
-               peer->host,
-               (unsigned long) (endp - STREAM_PNT (BGP_INPUT (peer))));
+         zlog_warn ("%s: error BGP attribute length %lu is smaller than min len",
+                     peer->host,
+                    (unsigned long) (endp - STREAM_PNT (BGP_INPUT (peer))));
 
          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. */
       startp = BGP_INPUT_PNT (peer);
-      flag = stream_getc (BGP_INPUT (peer));
+      /* "The lower-order four bits of the Attribute Flags octet are
+         unused.  They MUST be zero when sent and MUST be ignored when
+         received." */
+      flag = 0xF0 & stream_getc (BGP_INPUT (peer));
       type = stream_getc (BGP_INPUT (peer));
 
       /* Check whether Extended-Length applies and is in bounds */
       if (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN)
           && ((endp - startp) < (BGP_ATTR_MIN_LEN + 1)))
        {
-         zlog (peer->log, LOG_WARNING, 
-               "%s: Extended length set, but just %lu bytes of attr header",
-               peer->host,
-               (unsigned long) (endp - STREAM_PNT (BGP_INPUT (peer))));
+         zlog_warn ("%s: Extended length set, but just %lu bytes of attr header",
+                    peer->host,
+                    (unsigned long) (endp - STREAM_PNT (BGP_INPUT (peer))));
 
          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. */
       if (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN))
        length = stream_getw (BGP_INPUT (peer));
@@ -1727,14 +1957,13 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 
       if (CHECK_BITMAP (seen, type))
        {
-         zlog (peer->log, LOG_WARNING,
-               "%s: error BGP attribute type %d appears twice in a message",
-               peer->host, type);
+         zlog_warn ("%s: error BGP attribute type %d appears twice in a message",
+                    peer->host, type);
 
          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
@@ -1747,107 +1976,162 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 
       if (attr_endp > endp)
        {
-         zlog (peer->log, LOG_WARNING, 
-               "%s: BGP type %d length %d is too large, attribute total length is %d.  attr_endp is %p.  endp is %p", peer->host, type, length, size, attr_endp, endp);
-         bgp_notify_send (peer, 
-                          BGP_NOTIFY_UPDATE_ERR, 
-                          BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
-         return -1;
+         zlog_warn ("%s: BGP type %d length %d is too large, attribute total length is %d.  attr_endp is %p.  endp is %p", peer->host, type, length, size, attr_endp, endp);
+          bgp_notify_send_with_data (peer,
+                                     BGP_NOTIFY_UPDATE_ERR,
+                                     BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+                                     startp, attr_endp - startp);
+         return BGP_ATTR_PARSE_ERROR;
        }
+       
+        struct bgp_attr_parser_args attr_args = {
+          .peer = peer,
+          .length = length,
+          .attr = attr,
+          .type = type,
+          .flags = flag,
+          .startp = startp,
+          .total = attr_endp - startp,
+        };
+      
+       
+      /* If any recognized attribute has Attribute Flags that conflict
+         with the Attribute Type Code, then the Error Subcode is set to
+         Attribute Flags Error.  The Data field contains the erroneous
+         attribute (type, length and value). */
+      if (bgp_attr_flag_invalid (&attr_args))
+        {
+          bgp_attr_parse_ret_t ret;
+          ret = bgp_attr_malformed (&attr_args,
+                                    BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
+                                    attr_args.total);
+          if (ret == BGP_ATTR_PARSE_PROCEED)
+            continue;
+          return ret;
+        }
 
       /* OK check attribute and store it's value. */
       switch (type)
        {
        case BGP_ATTR_ORIGIN:
-         ret = bgp_attr_origin (peer, length, attr, flag, startp);
+         ret = bgp_attr_origin (&attr_args);
          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 (&attr_args);
          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 (&attr_args, &as4_path);
          break;
        case BGP_ATTR_NEXT_HOP: 
-         ret = bgp_attr_nexthop (peer, length, attr, flag, startp);
+         ret = bgp_attr_nexthop (&attr_args);
          break;
        case BGP_ATTR_MULTI_EXIT_DISC:
-         ret = bgp_attr_med (peer, length, attr, flag, startp);
+         ret = bgp_attr_med (&attr_args);
          break;
        case BGP_ATTR_LOCAL_PREF:
-         ret = bgp_attr_local_pref (peer, length, attr, flag, startp);
+         ret = bgp_attr_local_pref (&attr_args);
          break;
        case BGP_ATTR_ATOMIC_AGGREGATE:
-         ret = bgp_attr_atomic (peer, length, attr, flag, startp);
+         ret = bgp_attr_atomic (&attr_args);
          break;
        case BGP_ATTR_AGGREGATOR:
-         ret = bgp_attr_aggregator (peer, length, attr, flag);
+         ret = bgp_attr_aggregator (&attr_args);
          break;
        case BGP_ATTR_AS4_AGGREGATOR:
-         ret = bgp_attr_as4_aggregator (peer, length, attr, &as4_aggregator, &as4_aggregator_addr);
+         ret = bgp_attr_as4_aggregator (&attr_args,
+                                        &as4_aggregator,
+                                        &as4_aggregator_addr);
          break;
        case BGP_ATTR_COMMUNITIES:
-         ret = bgp_attr_community (peer, length, attr, flag);
+         ret = bgp_attr_community (&attr_args);
          break;
        case BGP_ATTR_ORIGINATOR_ID:
-         ret = bgp_attr_originator_id (peer, length, attr, flag);
+         ret = bgp_attr_originator_id (&attr_args);
          break;
        case BGP_ATTR_CLUSTER_LIST:
-         ret = bgp_attr_cluster_list (peer, length, attr, flag);
+         ret = bgp_attr_cluster_list (&attr_args);
          break;
        case BGP_ATTR_MP_REACH_NLRI:
-         ret = bgp_mp_reach_parse (peer, length, attr, mp_update);
+         ret = bgp_mp_reach_parse (&attr_args, mp_update);
          break;
        case BGP_ATTR_MP_UNREACH_NLRI:
-         ret = bgp_mp_unreach_parse (peer, length, mp_withdraw);
+         ret = bgp_mp_unreach_parse (&attr_args, mp_withdraw);
          break;
        case BGP_ATTR_EXT_COMMUNITIES:
-         ret = bgp_attr_ext_communities (peer, length, attr, flag);
+         ret = bgp_attr_ext_communities (&attr_args);
          break;
        default:
-         ret = bgp_attr_unknown (peer, attr, flag, type, length, startp);
+         ret = bgp_attr_unknown (&attr_args);
          break;
        }
+      
+      if (ret == BGP_ATTR_PARSE_ERROR_NOTIFYPLS)
+       {
+         bgp_notify_send (peer, 
+                          BGP_NOTIFY_UPDATE_ERR,
+                          BGP_NOTIFY_UPDATE_MAL_ATTR);
+         ret = BGP_ATTR_PARSE_ERROR;
+       }
 
-      /* 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;
+          zlog_warn ("%s: Attribute %s, parse error", 
+                     peer->host, 
+                     LOOKUP (attr_str, type));
+          if (as4_path)
+            aspath_unintern (&as4_path);
+          return ret;
         }
-
+      if (ret == BGP_ATTR_PARSE_WITHDRAW)
+        {
+          
+          zlog_warn ("%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)
        {
-         zlog (peer->log, LOG_WARNING, 
-               "%s: BGP attribute %s, fetch error", 
-                peer->host, LOOKUP (attr_str, type));
+         zlog_warn ("%s: BGP attribute %s, fetch error",
+                     peer->host, LOOKUP (attr_str, type));
          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;
        }
     }
 
   /* Check final read pointer is same as end pointer. */
   if (BGP_INPUT_PNT (peer) != endp)
     {
-      zlog (peer->log, LOG_WARNING, 
-           "%s: BGP attribute %s, length mismatch",
-           peer->host, LOOKUP (attr_str, type));
+      zlog_warn ("%s: BGP attribute %s, length mismatch",
+                peer->host, LOOKUP (attr_str, type));
       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;
     }
 
+  /* Check all mandatory well-known attributes are present */
+  {
+    bgp_attr_parse_ret_t ret;
+    if ((ret = bgp_attr_check (peer, attr)) < 0)
+      {
+        if (as4_path)
+          aspath_unintern (&as4_path);
+        return ret;
+      }
+  }
+
   /* 
    * At this place we can see whether we got AS4_PATH and/or
    * AS4_AGGREGATOR from a 16Bit peer and act accordingly.
@@ -1858,20 +2142,32 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
    * So, to be defensive, we are not relying on any order and read
    * all attributes first, including these 32bit ones, and now,
    * afterwards, we look what and if something is to be done for as4.
+   *
+   * It is possible to not have AS_PATH, e.g. GR EoR and sole
+   * MP_UNREACH_NLRI.
    */
-  if (bgp_attr_munge_as4_attrs (peer, attr, as4_path,
+  /* actually... this doesn't ever return failure currently, but
+   * better safe than sorry */
+  if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT (BGP_ATTR_AS_PATH))
+      && bgp_attr_munge_as4_attrs (peer, attr, as4_path,
                                 as4_aggregator, &as4_aggregator_addr))
-    return -1;
+    {
+      bgp_notify_send (peer, 
+                      BGP_NOTIFY_UPDATE_ERR,
+                      BGP_NOTIFY_UPDATE_MAL_ATTR);
+      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
        */
@@ -1886,10 +2182,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);
+      if (ret != BGP_ATTR_PARSE_PROCEED)
        return ret;
     }
 
@@ -1897,50 +2193,146 @@ 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. */
-int
-bgp_attr_check (struct peer *peer, struct attr *attr)
+size_t
+bgp_packet_mpattr_start (struct stream *s, afi_t afi, safi_t safi, afi_t nh_afi,
+                        struct bpacket_attr_vec_arr *vecarr,
+                        struct attr *attr)
 {
-  u_char type = 0;
-  
-  if (! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_ORIGIN)))
-    type = BGP_ATTR_ORIGIN;
+  size_t sizep;
 
-  if (! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_AS_PATH)))
-    type = BGP_ATTR_AS_PATH;
+  /* Set extended bit always to encode the attribute length as 2 bytes */
+  stream_putc (s, BGP_ATTR_FLAG_OPTIONAL|BGP_ATTR_FLAG_EXTLEN);
+  stream_putc (s, BGP_ATTR_MP_REACH_NLRI);
+  sizep = stream_get_endp (s);
+  stream_putw (s, 0);  /* Marker: Attribute length. */
 
-  if (! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP)))
-    type = BGP_ATTR_NEXT_HOP;
+  stream_putw (s, afi);
+  stream_putc (s, (safi == SAFI_MPLS_VPN) ? SAFI_MPLS_LABELED_VPN : safi);
 
-  if (peer_sort (peer) == BGP_PEER_IBGP
-      && ! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_LOCAL_PREF)))
-    type = BGP_ATTR_LOCAL_PREF;
+  /* Nexthop */
+  switch (nh_afi)
+    {
+    case AFI_IP:
+      switch (safi)
+       {
+       case SAFI_MULTICAST:
+         bpacket_attr_vec_arr_set_vec (vecarr, BGP_ATTR_VEC_NH, s, attr);
+         stream_putc (s, 4);
+         stream_put_ipv4 (s, attr->nexthop.s_addr);
+         break;
+       case SAFI_MPLS_VPN:
+         bpacket_attr_vec_arr_set_vec (vecarr, BGP_ATTR_VEC_NH, s, attr);
+         stream_putc (s, 12);
+         stream_putl (s, 0);   /* RD = 0, per RFC */
+         stream_putl (s, 0);
+         stream_put (s, &attr->extra->mp_nexthop_global_in, 4);
+         break;
+       case SAFI_UNICAST:      /* invalid for IPv4 */
+       default:
+         break;
+       }
+      break;
+#ifdef HAVE_IPV6
+    case AFI_IP6:
+      switch (safi)
+      {
+      case SAFI_UNICAST:
+      case SAFI_MULTICAST:
+       {
+         struct attr_extra *attre = attr->extra;
+
+         assert (attr->extra);
+         bpacket_attr_vec_arr_set_vec (vecarr, BGP_ATTR_VEC_NH, s, attr);
+         stream_putc (s, attre->mp_nexthop_len);
+         stream_put (s, &attre->mp_nexthop_global, IPV6_MAX_BYTELEN);
+         if (attre->mp_nexthop_len == BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL)
+           stream_put (s, &attre->mp_nexthop_local, IPV6_MAX_BYTELEN);
+       }
+       break;
+      case SAFI_MPLS_VPN:
+       {
+         struct attr_extra *attre = attr->extra;
+
+         assert (attr->extra);
+          if (attre->mp_nexthop_len == 16) {
+            stream_putc (s, 24);
+            stream_putl (s, 0);   /* RD = 0, per RFC */
+            stream_putl (s, 0);
+            stream_put (s, &attre->mp_nexthop_global, 16);
+          } else if (attre->mp_nexthop_len == 32) {
+            stream_putc (s, 48);
+            stream_putl (s, 0);   /* RD = 0, per RFC */
+            stream_putl (s, 0);
+            stream_put (s, &attre->mp_nexthop_global, 16);
+            stream_putl (s, 0);   /* RD = 0, per RFC */
+            stream_putl (s, 0);
+            stream_put (s, &attre->mp_nexthop_local, 16);
+          }
+        }
+       break;
+      default:
+       break;
+      }
+      break;
+#endif /*HAVE_IPV6*/
+    default:
+      break;
+    }
 
-  if (type)
+  /* SNPA */
+  stream_putc (s, 0);
+  return sizep;
+}
+
+void
+bgp_packet_mpattr_prefix (struct stream *s, afi_t afi, safi_t safi,
+                         struct prefix *p, struct prefix_rd *prd,
+                          u_char *tag, int addpath_encode,
+                          u_int32_t addpath_tx_id)
+{
+  if (safi == SAFI_MPLS_VPN)
     {
-      zlog (peer->log, LOG_WARNING, 
-           "%s Missing well-known attribute %d.",
-           peer->host, type);
-      bgp_notify_send_with_data (peer, 
-                                BGP_NOTIFY_UPDATE_ERR, 
-                                BGP_NOTIFY_UPDATE_MISS_ATTR,
-                                &type, 1);
-      return -1;
+      if (addpath_encode)
+        stream_putl(s, addpath_tx_id);
+      /* Tag, RD, Prefix write. */
+      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));
     }
-  return 0;
+  else
+    stream_put_prefix_addpath (s, p, addpath_encode, addpath_tx_id);
+}
+
+size_t
+bgp_packet_mpattr_prefix_size (afi_t afi, safi_t safi, struct prefix *p)
+{
+  int size = PSIZE (p->prefixlen);
+  if (safi == SAFI_MPLS_VPN)
+      size += 88;
+  return size;
+}
+
+void
+bgp_packet_mpattr_end (struct stream *s, size_t sizep)
+{
+  /* Set MP attribute length. Don't count the (2) bytes used to encode
+     the attr length */
+  stream_putw_at (s, sizep, (stream_get_endp (s) - sizep) - 2);
 }
-\f
-int stream_put_prefix (struct stream *, struct prefix *);
 
 /* Make attribute packet. */
 bgp_size_t
 bgp_packet_attribute (struct bgp *bgp, struct peer *peer,
-                     struct stream *s, struct attr *attr, struct prefix *p,
-                     afi_t afi, safi_t safi, struct peer *from,
-                     struct prefix_rd *prd, u_char *tag)
+                     struct stream *s, struct attr *attr,
+                     struct bpacket_attr_vec_arr *vecarr,
+                     struct prefix *p, afi_t afi, safi_t safi,
+                     struct peer *from, struct prefix_rd *prd, u_char *tag,
+                      int addpath_encode,
+                      u_int32_t addpath_tx_id)
 {
   size_t cp;
   size_t aspath_sizep;
@@ -1950,11 +2342,24 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer,
   int use32bit = (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV)) ? 1 : 0;
 
   if (! bgp)
-    bgp = bgp_get_default ();
+    bgp = peer->bgp;
 
   /* Remember current pointer. */
   cp = stream_get_endp (s);
 
+  if (p && !((afi == AFI_IP && safi == SAFI_UNICAST) &&
+              !peer_cap_enhe(peer)))
+    {
+      size_t mpattrlen_pos = 0;
+
+      mpattrlen_pos = bgp_packet_mpattr_start(s, afi, safi,
+                                    (peer_cap_enhe(peer) ? AFI_IP6 : afi),
+                                    vecarr, attr);
+      bgp_packet_mpattr_prefix(s, afi, safi, p, prd, tag,
+                               addpath_encode, addpath_tx_id);
+      bgp_packet_mpattr_end(s, mpattrlen_pos);
+    }
+
   /* Origin attribute. */
   stream_putc (s, BGP_ATTR_FLAG_TRANS);
   stream_putc (s, BGP_ATTR_ORIGIN);
@@ -1964,28 +2369,37 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer,
   /* AS path attribute. */
 
   /* If remote-peer is EBGP */
-  if (peer_sort (peer) == BGP_PEER_EBGP
+  if (peer->sort == BGP_PEER_EBGP
       && (! CHECK_FLAG (peer->af_flags[afi][safi], PEER_FLAG_AS_PATH_UNCHANGED)
          || attr->aspath->segments == NULL)
       && (! CHECK_FLAG (peer->af_flags[afi][safi], PEER_FLAG_RSERVER_CLIENT)))
     {    
       aspath = aspath_dup (attr->aspath);
 
+      /* Even though we may not be configured for confederations we may have
+       * RXed an AS_PATH with AS_CONFED_SEQUENCE or AS_CONFED_SET */
+      aspath = aspath_delete_confed_seq (aspath);
+
       if (CHECK_FLAG(bgp->config, BGP_CONFIG_CONFEDERATION))
        {
-         /* Strip the confed info, and then stuff our path CONFED_ID
-            on the front */
-         aspath = aspath_delete_confed_seq (aspath);
+         /* Stuff our path CONFED_ID on the front */
          aspath = aspath_add_seq (aspath, bgp->confed_id);
        }
       else
        {
-         aspath = aspath_add_seq (aspath, peer->local_as);
-         if (peer->change_local_as)
+         if (peer->change_local_as) {
+            /* If replace-as is specified, we only use the change_local_as when
+               advertising routes. */
+            if( ! CHECK_FLAG (peer->flags, PEER_FLAG_LOCAL_AS_REPLACE_AS) ) {
+              aspath = aspath_add_seq (aspath, peer->local_as);
+            }
            aspath = aspath_add_seq (aspath, peer->change_local_as);
+          } else {
+            aspath = aspath_add_seq (aspath, peer->local_as);
+          }
        }
     }
-  else if (peer_sort (peer) == BGP_PEER_CONFED)
+  else if (peer->sort == BGP_PEER_CONFED)
     {
       /* A confed member, so we need to do the AS_CONFED_SEQUENCE thing */
       aspath = aspath_dup (attr->aspath);
@@ -2016,34 +2430,46 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer,
       send_as4_path = 1; /* we'll do this later, at the correct place */
   
   /* Nexthop attribute. */
-  if (attr->flag & ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP) && afi == AFI_IP)
+  if (afi == AFI_IP && safi == SAFI_UNICAST && !peer_cap_enhe(peer))
     {
-      stream_putc (s, BGP_ATTR_FLAG_TRANS);
-      stream_putc (s, BGP_ATTR_NEXT_HOP);
-      stream_putc (s, 4);
-      if (safi == SAFI_MPLS_VPN)
-       {
-         if (attr->nexthop.s_addr == 0)
-           stream_put_ipv4 (s, peer->nexthop.v4.s_addr);
-         else
-           stream_put_ipv4 (s, attr->nexthop.s_addr);
-       }
-      else
-       stream_put_ipv4 (s, attr->nexthop.s_addr);
+      if (attr->flag & ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP))
+        {
+          stream_putc (s, BGP_ATTR_FLAG_TRANS);
+          stream_putc (s, BGP_ATTR_NEXT_HOP);
+          bpacket_attr_vec_arr_set_vec (vecarr, BGP_ATTR_VEC_NH, s, attr);
+          stream_putc (s, 4);
+          stream_put_ipv4 (s, attr->nexthop.s_addr);
+        }
+      else if (safi == SAFI_UNICAST && peer_cap_enhe(from))
+        {
+          /*
+           * Likely this is the case when an IPv4 prefix was received with
+           * Extended Next-hop capability and now being advertised to
+           * non-ENHE peers.
+           * Setting the mandatory (ipv4) next-hop attribute here to enable
+           * implicit next-hop self with correct (ipv4 address family).
+           */
+          stream_putc (s, BGP_ATTR_FLAG_TRANS);
+          stream_putc (s, BGP_ATTR_NEXT_HOP);
+          bpacket_attr_vec_arr_set_vec (vecarr, BGP_ATTR_VEC_NH, s, NULL);
+          stream_putc (s, 4);
+          stream_put_ipv4 (s, 0);
+        }
     }
 
   /* MED attribute. */
-  if (attr->flag & ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC))
+  if (attr->flag & ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC) ||
+      bgp->maxmed_active)
     {
       stream_putc (s, BGP_ATTR_FLAG_OPTIONAL);
       stream_putc (s, BGP_ATTR_MULTI_EXIT_DISC);
       stream_putc (s, 4);
-      stream_putl (s, attr->med);
+      stream_putl (s, (bgp->maxmed_active ? bgp->maxmed_value : attr->med));
     }
 
   /* Local preference. */
-  if (peer_sort (peer) == BGP_PEER_IBGP ||
-      peer_sort (peer) == BGP_PEER_CONFED)
+  if (peer->sort == BGP_PEER_IBGP ||
+      peer->sort == BGP_PEER_CONFED)
     {
       stream_putc (s, BGP_ATTR_FLAG_TRANS);
       stream_putc (s, BGP_ATTR_LOCAL_PREF);
@@ -2116,9 +2542,9 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer,
     }
 
   /* Route Reflector. */
-  if (peer_sort (peer) == BGP_PEER_IBGP
+  if (peer->sort == BGP_PEER_IBGP
       && from
-      && peer_sort (from) == BGP_PEER_IBGP)
+      && from->sort == BGP_PEER_IBGP)
     {
       /* Originator ID. */
       stream_putc (s, BGP_ATTR_FLAG_OPTIONAL);
@@ -2156,96 +2582,6 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer,
        }
     }
 
-#ifdef HAVE_IPV6
-  /* If p is IPv6 address put it into attribute. */
-  if (p->family == AF_INET6)
-    {
-      unsigned long sizep;
-      struct attr_extra *attre = attr->extra;
-      
-      assert (attr->extra);
-      
-      stream_putc (s, BGP_ATTR_FLAG_OPTIONAL);
-      stream_putc (s, BGP_ATTR_MP_REACH_NLRI);
-      sizep = stream_get_endp (s);
-      stream_putc (s, 0);      /* Marker: Attribute length. */
-      stream_putw (s, AFI_IP6);        /* AFI */
-      stream_putc (s, safi);   /* SAFI */
-
-      stream_putc (s, attre->mp_nexthop_len);
-
-      if (attre->mp_nexthop_len == 16)
-       stream_put (s, &attre->mp_nexthop_global, 16);
-      else if (attre->mp_nexthop_len == 32)
-       {
-         stream_put (s, &attre->mp_nexthop_global, 16);
-         stream_put (s, &attre->mp_nexthop_local, 16);
-       }
-      
-      /* SNPA */
-      stream_putc (s, 0);
-
-      /* Prefix write. */
-      stream_put_prefix (s, p);
-
-      /* Set MP attribute length. */
-      stream_putc_at (s, sizep, (stream_get_endp (s) - sizep) - 1);
-    }
-#endif /* HAVE_IPV6 */
-
-  if (p->family == AF_INET && safi == SAFI_MULTICAST)
-    {
-      unsigned long sizep;
-
-      stream_putc (s, BGP_ATTR_FLAG_OPTIONAL);
-      stream_putc (s, BGP_ATTR_MP_REACH_NLRI);
-      sizep = stream_get_endp (s);
-      stream_putc (s, 0);      /* Marker: Attribute Length. */
-      stream_putw (s, AFI_IP); /* AFI */
-      stream_putc (s, SAFI_MULTICAST); /* SAFI */
-
-      stream_putc (s, 4);
-      stream_put_ipv4 (s, attr->nexthop.s_addr);
-
-      /* SNPA */
-      stream_putc (s, 0);
-
-      /* Prefix write. */
-      stream_put_prefix (s, p);
-
-      /* Set MP attribute length. */
-      stream_putc_at (s, sizep, (stream_get_endp (s) - sizep) - 1);
-    }
-
-  if (p->family == AF_INET && safi == SAFI_MPLS_VPN)
-    {
-      unsigned long sizep;
-
-      stream_putc (s, BGP_ATTR_FLAG_OPTIONAL);
-      stream_putc (s, BGP_ATTR_MP_REACH_NLRI);
-      sizep = stream_get_endp (s);
-      stream_putc (s, 0);      /* Length of this attribute. */
-      stream_putw (s, AFI_IP); /* AFI */
-      stream_putc (s, SAFI_MPLS_LABELED_VPN);  /* SAFI */
-
-      stream_putc (s, 12);
-      stream_putl (s, 0);
-      stream_putl (s, 0);
-      stream_put (s, &attr->extra->mp_nexthop_global_in, 4);
-
-      /* SNPA */
-      stream_putc (s, 0);
-
-      /* Tag, RD, Prefix write. */
-      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));
-
-      /* Set MP attribute length. */
-      stream_putc_at (s, sizep, (stream_get_endp (s) - sizep) - 1);
-    }
-
   /* Extended Communities attribute. */
   if (CHECK_FLAG (peer->af_flags[afi][safi], PEER_FLAG_SEND_EXT_COMMUNITY) 
       && (attr->flag & ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES)))
@@ -2254,8 +2590,8 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer,
       
       assert (attre);
       
-      if (peer_sort (peer) == BGP_PEER_IBGP 
-          || peer_sort (peer) == BGP_PEER_CONFED)
+      if (peer->sort == BGP_PEER_IBGP
+          || peer->sort == BGP_PEER_CONFED)
        {
          if (attre->ecommunity->size * 8 > 255)
            {
@@ -2367,50 +2703,48 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer,
   return stream_get_endp (s) - cp;
 }
 
-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)
+size_t
+bgp_packet_mpunreach_start (struct stream *s, afi_t afi, safi_t safi)
 {
-  unsigned long cp;
   unsigned long attrlen_pnt;
-  bgp_size_t size;
-
-  cp = stream_get_endp (s);
 
-  stream_putc (s, BGP_ATTR_FLAG_OPTIONAL);
+  /* Set extended bit always to encode the attribute length as 2 bytes */
+  stream_putc (s, BGP_ATTR_FLAG_OPTIONAL|BGP_ATTR_FLAG_EXTLEN);
   stream_putc (s, BGP_ATTR_MP_UNREACH_NLRI);
 
   attrlen_pnt = stream_get_endp (s);
-  stream_putc (s, 0);          /* Length of this attribute. */
+  stream_putw (s, 0);          /* Length of this attribute. */
 
-  stream_putw (s, family2afi (p->family));
+  stream_putw (s, afi);
+  stream_putc (s, (safi == SAFI_MPLS_VPN) ? SAFI_MPLS_LABELED_VPN : safi);
+  return attrlen_pnt;
+}
 
+void
+bgp_packet_mpunreach_prefix (struct stream *s, struct prefix *p,
+                            afi_t afi, safi_t safi, struct prefix_rd *prd,
+                            u_char *tag, int addpath_encode,
+                             u_int32_t addpath_tx_id)
+{
   if (safi == SAFI_MPLS_VPN)
     {
-      /* SAFI */
-      stream_putc (s, SAFI_MPLS_LABELED_VPN);
+      /* addpath TX ID */
+      if (addpath_encode)
+        stream_putl(s, addpath_tx_id);
 
-      /* 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));
     }
   else
-    {
-      /* SAFI */
-      stream_putc (s, safi);
-
-      /* prefix */
-      stream_put_prefix (s, p);
-    }
-
-  /* Set MP attribute length. */
-  size = stream_get_endp (s) - attrlen_pnt - 1;
-  stream_putc_at (s, attrlen_pnt, size);
+    stream_put_prefix_addpath (s, p, addpath_encode, addpath_tx_id);
+}
 
-  return stream_get_endp (s) - cp;
+void
+bgp_packet_mpunreach_end (struct stream *s, size_t attrlen_pnt)
+{
+  bgp_packet_mpattr_end (s, attrlen_pnt);
 }
 
 /* Initialization of attribute. */
@@ -2445,6 +2779,8 @@ bgp_dump_routes_attr (struct stream *s, struct attr *attr,
   unsigned long len;
   size_t aspath_lenp;
   struct aspath *aspath;
+  int addpath_encode = 0;
+  u_int32_t addpath_tx_id = 0;
 
   /* Remember current pointer. */
   cp = stream_get_endp (s);
@@ -2539,7 +2875,8 @@ bgp_dump_routes_attr (struct stream *s, struct attr *attr,
 #ifdef HAVE_IPV6
   /* Add a MP_NLRI attribute to dump the IPv6 next hop */
   if (prefix != NULL && prefix->family == AF_INET6 && attr->extra &&
-     (attr->extra->mp_nexthop_len == 16 || attr->extra->mp_nexthop_len == 32) )
+     (attr->extra->mp_nexthop_len == BGP_ATTR_NHLEN_IPV6_GLOBAL ||
+      attr->extra->mp_nexthop_len == BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL) )
     {
       int sizep;
       struct attr_extra *attre = attr->extra;
@@ -2555,15 +2892,15 @@ bgp_dump_routes_attr (struct stream *s, struct attr *attr,
 
       /* Next hop */
       stream_putc(s, attre->mp_nexthop_len);
-      stream_put(s, &attre->mp_nexthop_global, 16);
-      if (attre->mp_nexthop_len == 32)
-        stream_put(s, &attre->mp_nexthop_local, 16);
+      stream_put(s, &attre->mp_nexthop_global, IPV6_MAX_BYTELEN);
+      if (attre->mp_nexthop_len == BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL)
+        stream_put(s, &attre->mp_nexthop_local, IPV6_MAX_BYTELEN);
 
       /* SNPA */
       stream_putc(s, 0);
 
       /* Prefix */
-      stream_put_prefix(s, prefix);
+      stream_put_prefix_addpath (s, prefix, addpath_encode, addpath_tx_id);
 
       /* Set MP attribute length. */
       stream_putc_at (s, sizep, (stream_get_endp (s) - sizep) - 1);