From 014b670e02cc1f38e8e4e786269fc1787412f9b7 Mon Sep 17 00:00:00 2001
From: Denis Ovsienko <pilot@etcnet.org>
Date: Tue, 23 Jun 2009 21:10:45 +0400
Subject: [bgpd] review 32-bit AS-path hotfix for 0.99.12

The patch by Chris Caputo, which was used to prepare 0.99.12
release, consists of three parts:

1. memory allocation fix itself
2. fix for warnings about constant variables
3. fix for printf format specs (%d was used instead of %u)

It was confirmed later, that:
a. a much simpler bugfix was available for memory allocation
b. committed version of the bugfix wasn't optimal CPU-wise

At this point I consider reasonable to revert the allocation
portion of that patch and to replace it with the shorter
version, which is:

-#define ASN_STR_LEN (5 + 1)
+#define ASN_STR_LEN (10 + 1)

Other two parts of Mr. Caputo's patch remain intact.
---
 bgpd/bgp_aspath.c | 81 +++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 52 insertions(+), 29 deletions(-)

(limited to 'bgpd/bgp_aspath.c')

diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
index 5881abe2..002fff9f 100644
--- a/bgpd/bgp_aspath.c
+++ b/bgpd/bgp_aspath.c
@@ -389,6 +389,25 @@ aspath_delimiter_char (u_char type, u_char which)
   return ' ';
 }
 
+/* countup asns from this segment and index onward */
+static int
+assegment_count_asns (struct assegment *seg, int from)
+{
+  int count = 0;
+  while (seg)
+    {
+      if (!from)
+        count += seg->length;
+      else
+        {
+          count += (seg->length - from);
+          from = 0;
+        }
+      seg = seg->next;
+    }
+  return count;
+}
+
 unsigned int
 aspath_count_confeds (struct aspath *aspath)
 {
@@ -498,21 +517,6 @@ aspath_count_numas (struct aspath *aspath)
   return num;
 }
 
-static void
-aspath_make_str_big_enough (int len,
-			    char **str_buf,
-			    int *str_size,
-			    int count_to_be_added)
-{
-#define TERMINATOR 1
-  while (len + count_to_be_added + TERMINATOR > *str_size)
-    {
-      *str_size *= 2;
-      *str_buf = XREALLOC (MTYPE_AS_STR, *str_buf, *str_size);
-    }
-#undef TERMINATOR
-}
-
 /* Convert aspath structure to string expression. */
 static char *
 aspath_make_str_count (struct aspath *as)
@@ -532,7 +536,18 @@ aspath_make_str_count (struct aspath *as)
   
   seg = as->segments;
   
-  str_size = ASPATH_STR_DEFAULT_LEN;
+  /* ASN takes 5 to 10 chars plus seperator, see below.
+   * If there is one differing segment type, we need an additional
+   * 2 chars for segment delimiters, and the final '\0'.
+   * Hopefully this is large enough to avoid hitting the realloc
+   * code below for most common sequences.
+   *
+   * This was changed to 10 after the well-known BGP assertion, which
+   * had hit some parts of the Internet in May of 2009.
+   */
+#define ASN_STR_LEN (10 + 1)
+  str_size = MAX (assegment_count_asns (seg, 0) * ASN_STR_LEN + 2 + 1,
+                  ASPATH_STR_DEFAULT_LEN);
   str_buf = XMALLOC (MTYPE_AS_STR, str_size);
 
   while (seg)
@@ -556,24 +571,32 @@ aspath_make_str_count (struct aspath *as)
             return NULL;
         }
       
+      /* We might need to increase str_buf, particularly if path has
+       * differing segments types, our initial guesstimate above will
+       * have been wrong. Need 10 chars for ASN, a seperator each and
+       * potentially two segment delimiters, plus a space between each
+       * segment and trailing zero.
+       *
+       * This definitely didn't work with the value of 5 bytes and
+       * 32-bit ASNs.
+       */
+#define SEGMENT_STR_LEN(X) (((X)->length * ASN_STR_LEN) + 2 + 1 + 1)
+      if ( (len + SEGMENT_STR_LEN(seg)) > str_size)
+        {
+          str_size = len + SEGMENT_STR_LEN(seg);
+          str_buf = XREALLOC (MTYPE_AS_STR, str_buf, str_size);
+        }
+#undef ASN_STR_LEN
+#undef SEGMENT_STR_LEN
+      
       if (seg->type != AS_SEQUENCE)
-	{
-	  aspath_make_str_big_enough (len, &str_buf, &str_size, 1); /* %c */
-	  len += snprintf (str_buf + len, str_size - len, 
-			   "%c", 
-			   aspath_delimiter_char (seg->type, AS_SEG_START));
-	}
+        len += snprintf (str_buf + len, str_size - len, 
+			 "%c", 
+                         aspath_delimiter_char (seg->type, AS_SEG_START));
       
       /* write out the ASNs, with their seperators, bar the last one*/
       for (i = 0; i < seg->length; i++)
         {
-#define APPROX_DIG_CNT(x) (x < 100000U ? 5 : 10)
-	  /* %u + %c + %c + " " (last two are below loop) */
-	  aspath_make_str_big_enough (len,
-				      &str_buf,
-				      &str_size,
-				      APPROX_DIG_CNT(seg->as[i]) + 1 + 1 + 1);
-
           len += snprintf (str_buf + len, str_size - len, "%u", seg->as[i]);
           
           if (i < (seg->length - 1))
-- 
cgit v1.2.1