Better handling of too long attributes

This patch extends the length for attributes from 1024 to 2048
(because both AS_PATH and AS4_PATH attributes take 2+4 B per AS).

If there is not enough space for attributes, Bird skips that
route group. Old behavior (skipping remaining attributes)
leads to skipping required attributes and session drop.

diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index d8a6aad445d05e1769ab3c0f2a5e39c9207cb788..91d4d0643dacc7ee0ae18a214b5a42055f97c8fd 100644 (file)
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -347,7 +347,7 @@ bgp_get_attr_len(eattr *a)
  * The bgp_encode_attrs() function takes a list of extended attributes
  * and converts it to its BGP representation (a part of an Update message).
  *
- * Result: Length of the attribute block generated.
+ * Result: Length of the attribute block generated or -1 if not enough space.
  */
 unsigned int
 bgp_encode_attrs(struct bgp_proto *p, byte *w, ea_list *attrs, int remains)
@@ -488,8 +488,7 @@ bgp_encode_attrs(struct bgp_proto *p, byte *w, ea_list *attrs, int remains)
   return w - start;
 
  err_no_buffer:
-  log(L_ERR "BGP: attribute list too long, ignoring the remaining attributes");
-  return w - start;
+  return -1;
 }
 
 static void

diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c
index fe7472bdb1f5922c5d30c19870fa927a8b59f404..4256489690dcbd1201f18a43cde221caea37b4d2 100644 (file)
--- a/proto/bgp/packets.c
+++ b/proto/bgp/packets.c
@@ -127,6 +127,18 @@ bgp_encode_prefixes(struct bgp_proto *p, byte *w, struct bgp_bucket *buck, unsig
   return w - start;
 }
 
+static void
+bgp_flush_prefixes(struct bgp_proto *p, struct bgp_bucket *buck)
+{
+  while (!EMPTY_LIST(buck->prefixes))
+    {
+      struct bgp_prefix *px = SKIP_BACK(struct bgp_prefix, bucket_node, HEAD(buck->prefixes));
+      log(L_ERR "%s: - route %I/%d skipped", p->p.name, px->n.prefix, px->n.pxlen);
+      rem_node(&px->bucket_node);
+      fib_delete(&p->prefix_fib, px);
+    }
+}
+
 #ifndef IPV6           /* IPv4 version */
 
 static byte *
@@ -150,7 +162,7 @@ bgp_create_update(struct bgp_conn *conn, byte *buf)
     }
   put_u16(buf, wd_size);
 
-  if (remains >= 2048)
+  if (remains >= 3072)
     {
       while ((buck = (struct bgp_bucket *) HEAD(p->bucket_queue))->send_node.next)
        {
@@ -161,8 +173,19 @@ bgp_create_update(struct bgp_conn *conn, byte *buf)
              bgp_free_bucket(p, buck);
              continue;
            }
+
          DBG("Processing bucket %p\n", buck);
-         a_size = bgp_encode_attrs(p, w+2, buck->eattrs, 1024);
+         a_size = bgp_encode_attrs(p, w+2, buck->eattrs, 2048);
+
+         if (a_size < 0)
+           {
+             log(L_ERR "%s: Attribute list too long, skipping corresponding route group", p->p.name);
+             bgp_flush_prefixes(p, buck);
+             rem_node(&buck->send_node);
+             bgp_free_bucket(p, buck);
+             continue;
+           }
+
          put_u16(w, a_size);
          w += a_size + 2;
          r_size = bgp_encode_prefixes(p, w, buck, remains - a_size);
@@ -211,11 +234,12 @@ bgp_create_update(struct bgp_conn *conn, byte *buf)
       *tmp++ = 1;
       ea->attrs[0].u.ptr->length = bgp_encode_prefixes(p, tmp, buck, remains-11);
       size = bgp_encode_attrs(p, w, ea, remains);
+      ASSERT(size >= 0);
       w += size;
       remains -= size;
     }
 
-  if (remains >= 2048)
+  if (remains >= 3072)
     {
       while ((buck = (struct bgp_bucket *) HEAD(p->bucket_queue))->send_node.next)
        {
@@ -226,8 +250,19 @@ bgp_create_update(struct bgp_conn *conn, byte *buf)
              bgp_free_bucket(p, buck);
              continue;
            }
+
          DBG("Processing bucket %p\n", buck);
-         size = bgp_encode_attrs(p, w, buck->eattrs, 1024);
+         size = bgp_encode_attrs(p, w, buck->eattrs, 2048);
+
+         if (size < 0)
+           {
+             log(L_ERR "%s: Attribute list too long, ignoring corresponding route group", p->p.name);
+             bgp_flush_prefixes(p, buck);
+             rem_node(&buck->send_node);
+             bgp_free_bucket(p, buck);
+             continue;
+           }
+
          w += size;
          remains -= size;
          tstart = tmp = bgp_attach_attr_wa(&ea, bgp_linpool, BA_MP_REACH_NLRI, remains-8);
@@ -274,7 +309,9 @@ bgp_create_update(struct bgp_conn *conn, byte *buf)
          *tmp++ = 0;                   /* No SNPA information */
          tmp += bgp_encode_prefixes(p, tmp, buck, remains - (8+3+32+1));
          ea->attrs[0].u.ptr->length = tmp - tstart;
-         w += bgp_encode_attrs(p, w, ea, remains);
+         size = bgp_encode_attrs(p, w, ea, remains);
+         ASSERT(size >= 0);
+         w += size;
          break;
        }
     }