bgpd: Rollback some of the changes made for invalid AS_PATH segment fix

Some of the changes made in commit cddb8112b80fa9867156c637d63e6e79eeac67bb
don't work particularly well for other changes that need to be made to
address BGP attribute error handling problems. In particular, returning
a pointer from complex attribute data parsing functions will not suffice
to express the require range of return status conditions.

* bgp_aspath.c: (assegments_parse) Rollback to a more minimal set of
  changes to fix the original problem.
  (aspath_parse) Slightly needless pushing around of code, and taking
  2 parameters to say whether ot use 2 or 4 byte encoding seems unnecessary.
* bgp_attr.c: (bgp_attr_as{,4}path) Rollback, in preparation for BGP
  attribute error handling update.

1 files changed, 43 insertions, 57 deletions

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index e2b6054b..7a0ab560 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -746,78 +746,51 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length,
 
   return 0;
 }
-/* Parse AS path information.  This function is wrapper of aspath_parse.
- *
- * Parses AS_PATH or AS4_PATH.
- *
- * Returns: if valid: address of struct aspath in the hash of known aspaths,
- *                    with reference count incremented.
- *              else: NULL
- *
- * NB: empty AS path (length == 0) is valid.  The returned struct aspath will
- *     have segments == NULL and str == zero length string (unique).
- */
-static struct aspath *
+
+/* Parse AS path information.  This function is wrapper of
+   aspath_parse. */
+static int
 bgp_attr_aspath (struct peer *peer, bgp_size_t length, 
-		 struct attr *attr, u_char flag, u_char *startp, int as4_path)
+		 struct attr *attr, u_char flag, u_char *startp)
 {
-  u_char require ;
-  struct aspath *asp ;
+  bgp_size_t total;
 
-  /* Check the attribute flags                                          */
-  require = as4_path ? BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS
-                     :                          BGP_ATTR_FLAG_TRANS ;
+  total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
 
-  if ((flag & (BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS)) != require)
+  /* Flag check. */
+  if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)
+      || ! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
     {
-      const char* path_type ;
-      bgp_size_t total;
-
-      path_type = as4_path ? "AS4_PATH" : "AS_PATH" ;
-
-      if (!CHECK_FLAG(flag, BGP_ATTR_FLAG_TRANS))
       zlog (peer->log, LOG_ERR, 
-            "%s attribute flag isn't transitive %d", path_type, flag) ;
-
-      if ((flag & BGP_ATTR_FLAG_OPTIONAL) != (require & BGP_ATTR_FLAG_OPTIONAL))
-        zlog (peer->log, LOG_ERR,
-            "%s attribute flag must %sbe optional %d", path_type,
-            (flag & BGP_ATTR_FLAG_OPTIONAL) ? "not " : "", flag) ;
-
-      total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
-
+	    "As-Path attribute flag isn't transitive %d", flag);
       bgp_notify_send_with_data (peer, 
 				 BGP_NOTIFY_UPDATE_ERR, 
 				 BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
 				 startp, total);
+      return -1;
+    }
 
-      return NULL ;
-    } ;
-
-  /* Parse the AS_PATH/AS4_PATH body.
-   *
-   * For AS_PATH  peer with AS4 => 4Byte ASN otherwise 2Byte ASN
-   *     AS4_PATH 4Byte ASN
+  /*
+   * peer with AS4 => will get 4Byte ASnums
+   * otherwise, will get 16 Bit
    */
-  asp = aspath_parse (peer->ibuf, length,
-               as4_path || CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV), as4_path) ;
+  attr->aspath = aspath_parse (peer->ibuf, length, 
+                               CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV));
 
-  if (asp != NULL)
-    {
-      attr->flag |= ATTR_FLAG_BIT (as4_path ? BGP_ATTR_AS4_PATH
-                                            : BGP_ATTR_AS_PATH) ;
-    }
-  else
+  /* In case of IBGP, length will be zero. */
+  if (! attr->aspath)
     {
       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);
-    } ;
+      return -1;
+    }
+
+  /* Set aspath attribute flag. */
+  attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS_PATH);
 
-  return asp ;
+  return 0;
 }
 
 static int bgp_attr_aspath_check( struct peer *peer, 
@@ -875,6 +848,21 @@ static int bgp_attr_aspath_check( struct peer *peer,
 
 }
 
+/* Parse AS4 path information.  This function is another wrapper of
+   aspath_parse. */
+static int
+bgp_attr_as4_path (struct peer *peer, bgp_size_t length, 
+		 struct attr *attr, struct aspath **as4_path)
+{
+  *as4_path = aspath_parse (peer->ibuf, length, 1);
+
+  /* Set aspath attribute flag. */
+  if (as4_path)
+    attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH);
+
+  return 0;
+}
+
 /* Nexthop attribute. */
 static int
 bgp_attr_nexthop (struct peer *peer, bgp_size_t length, 
@@ -1613,12 +1601,10 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 	  ret = bgp_attr_origin (peer, length, attr, flag, startp);
 	  break;
 	case BGP_ATTR_AS_PATH:
-          attr->aspath = bgp_attr_aspath (peer, length, attr, flag, startp, 0);
-          ret = attr->aspath ? 0 : -1 ;
+	  ret = bgp_attr_aspath (peer, length, attr, flag, startp);
 	  break;
 	case BGP_ATTR_AS4_PATH:
-          as4_path = bgp_attr_aspath (peer, length, attr, flag, startp, 1);
-          ret = as4_path  ? 0 : -1 ;
+	  ret = bgp_attr_as4_path (peer, length, attr, &as4_path );
 	  break;
 	case BGP_ATTR_NEXT_HOP:	
 	  ret = bgp_attr_nexthop (peer, length, attr, flag, startp);