BGP: Handle corner cases in event ordering

When BGP connection is opened, it may happen that rx hook (with remote
OPEN) is called before tx hook (for local OPEN). Therefore, we need to do
internal changes (like setting local_caps) synchronously with OPENSENT
transition and we need to ensure that OPEN is sent before KEEPALIVE.

diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index 92f41ef60d66caef70d3bb867680ca3af2c7f57c..c332a836ad1f66f81b3bdf2a25efb741e0d6135f 100644 (file)
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -892,6 +892,7 @@ bgp_send_open(struct bgp_conn *conn)
   conn->sk->rx_hook = bgp_rx;
   conn->sk->tx_hook = bgp_tx;
   tm_stop(conn->connect_timer);
+  bgp_prepare_capabilities(conn);
   bgp_schedule_packet(conn, NULL, PKT_OPEN);
   bgp_conn_set_state(conn, BS_OPENSENT);
   bgp_start_timer(conn->hold_timer, conn->bgp->cf->initial_hold_time);

diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index fd515cbbe6abc194299f6f08f201b4e32c12d24e..4d24e4fa3678872441ef3281f39f257e3ee9857b 100644 (file)
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -218,6 +218,8 @@ struct bgp_caps {
   u16 gr_time;                         /* Graceful restart time in seconds */
 
   u8 llgr_aware;                       /* Long-lived GR capability, RFC draft */
+  u8 any_ext_next_hop;                 /* Bitwise OR of per-AF ext_next_hop */
+  u8 any_add_path;                     /* Bitwise OR of per-AF add_path */
 
   u16 af_count;                                /* Number of af_data items */
   u16 length;                          /* Length of capabilities in OPEN msg */
@@ -556,6 +558,7 @@ void bgp_get_route_info(struct rte *, byte *buf);
 /* packets.c */
 
 void bgp_dump_state_change(struct bgp_conn *conn, uint old, uint new);
+void bgp_prepare_capabilities(struct bgp_conn *conn);
 const struct bgp_af_desc *bgp_get_af_desc(u32 afi);
 const struct bgp_af_caps *bgp_find_af_caps(struct bgp_caps *caps, u32 afi);
 void bgp_schedule_packet(struct bgp_conn *conn, struct bgp_channel *c, int type);

diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c
index 0e142a7a27416d005c907d963bb380eae4c1a477..d348ebc1ec456c70700fd927fe8f530cb6d3e7a2 100644 (file)
--- a/proto/bgp/packets.c
+++ b/proto/bgp/packets.c
@@ -208,20 +208,22 @@ bgp_af_caps_cmp(const void *X, const void *Y)
 }
 
 
-static byte *
-bgp_write_capabilities(struct bgp_conn *conn, byte *buf)
+void
+bgp_prepare_capabilities(struct bgp_conn *conn)
 {
   struct bgp_proto *p = conn->bgp;
   struct bgp_channel *c;
   struct bgp_caps *caps;
   struct bgp_af_caps *ac;
-  uint any_ext_next_hop = 0;
-  uint any_add_path = 0;
-  byte *buf_head = buf;
-  byte *data;
 
-  /* Prepare bgp_caps structure */
+  if (!p->cf->capabilities)
+  {
+    /* Just prepare empty local_caps */
+    conn->local_caps = mb_allocz(p->p.pool, sizeof(struct bgp_caps));
+    return;
+  }
 
+  /* Prepare bgp_caps structure */
   int n = list_length(&p->p.channels);
   caps = mb_allocz(p->p.pool, sizeof(struct bgp_caps) + n * sizeof(struct bgp_af_caps));
   conn->local_caps = caps;
@@ -252,10 +254,10 @@ bgp_write_capabilities(struct bgp_conn *conn, byte *buf)
     ac->ready = 1;
 
     ac->ext_next_hop = bgp_channel_is_ipv4(c) && c->cf->ext_next_hop;
-    any_ext_next_hop |= ac->ext_next_hop;
+    caps->any_ext_next_hop |= ac->ext_next_hop;
 
     ac->add_path = c->cf->add_path;
-    any_add_path |= ac->add_path;
+    caps->any_add_path |= ac->add_path;
 
     if (c->cf->gr_able)
     {
@@ -277,7 +279,16 @@ bgp_write_capabilities(struct bgp_conn *conn, byte *buf)
 
   /* Sort capability fields by AFI/SAFI */
   qsort(caps->af_data, caps->af_count, sizeof(struct bgp_af_caps), bgp_af_caps_cmp);
+}
 
+static byte *
+bgp_write_capabilities(struct bgp_conn *conn, byte *buf)
+{
+  struct bgp_proto *p = conn->bgp;
+  struct bgp_caps *caps = conn->local_caps;
+  struct bgp_af_caps *ac;
+  byte *buf_head = buf;
+  byte *data;
 
   /* Create capability list in buffer */
 
@@ -302,7 +313,7 @@ bgp_write_capabilities(struct bgp_conn *conn, byte *buf)
     *buf++ = 0;                        /* Capability data length */
   }
 
-  if (any_ext_next_hop)
+  if (caps->any_ext_next_hop)
   {
     *buf++ = 5;                        /* Capability 5: Support for extended next hop */
     *buf++ = 0;                        /* Capability data length, will be fixed later */
@@ -354,7 +365,7 @@ bgp_write_capabilities(struct bgp_conn *conn, byte *buf)
     buf += 4;
   }
 
-  if (any_add_path)
+  if (caps->any_add_path)
   {
     *buf++ = 69;               /* Capability 69: Support for ADD-PATH */
     *buf++ = 0;                        /* Capability data length, will be fixed later */
@@ -676,9 +687,6 @@ bgp_create_open(struct bgp_conn *conn, byte *buf)
   }
   else
   {
-    /* Prepare empty local_caps */
-    conn->local_caps = mb_allocz(p->p.pool, sizeof(struct bgp_caps));
-
     buf[9] = 0;                        /* No optional parameters */
     return buf + 10;
   }
@@ -2684,6 +2692,12 @@ bgp_fire_tx(struct bgp_conn *conn)
     end = bgp_create_notification(conn, pkt);
     return bgp_send(conn, PKT_NOTIFICATION, end - buf);
   }
+  else if (s & (1 << PKT_OPEN))
+  {
+    conn->packets_to_send &= ~(1 << PKT_OPEN);
+    end = bgp_create_open(conn, pkt);
+    return bgp_send(conn, PKT_OPEN, end - buf);
+  }
   else if (s & (1 << PKT_KEEPALIVE))
   {
     conn->packets_to_send &= ~(1 << PKT_KEEPALIVE);
@@ -2691,12 +2705,6 @@ bgp_fire_tx(struct bgp_conn *conn)
     bgp_start_timer(conn->keepalive_timer, conn->keepalive_time);
     return bgp_send(conn, PKT_KEEPALIVE, BGP_HEADER_LENGTH);
   }
-  else if (s & (1 << PKT_OPEN))
-  {
-    conn->packets_to_send &= ~(1 << PKT_OPEN);
-    end = bgp_create_open(conn, pkt);
-    return bgp_send(conn, PKT_OPEN, end - buf);
-  }
   else while (conn->channels_to_send)
   {
     c = bgp_get_channel_to_send(p, conn);