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.

3 files changed, 32 insertions, 20 deletions

diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index 92f41ef6..c332a836 100644
--- 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 fd515cbb..4d24e4fa 100644
--- 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 0e142a7a..d348ebc1 100644
--- 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);