Extend bgp_check_capabilities() to mark missing capabilities

diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index c8f90fadb5d773b3bd8b41d32767b89f37c620e1..87adc7af0492d0317e53b86cdecd35238d34247a 100644 (file)
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -2781,11 +2781,11 @@ bgp_reconfigure(struct proto *P, struct proto_config *CF)
 
   /* Check whether existing connections are compatible with required capabilities */
   struct bgp_conn *ci = &p->incoming_conn;
-  if (((ci->state == BS_OPENCONFIRM) || (ci->state == BS_ESTABLISHED)) && !bgp_check_capabilities(ci))
+  if (((ci->state == BS_OPENCONFIRM) || (ci->state == BS_ESTABLISHED)) && !bgp_check_capabilities(ci, NULL))
     return 0;
 
   struct bgp_conn *co = &p->outgoing_conn;
-  if (((co->state == BS_OPENCONFIRM) || (co->state == BS_ESTABLISHED)) && !bgp_check_capabilities(co))
+  if (((co->state == BS_OPENCONFIRM) || (co->state == BS_ESTABLISHED)) && !bgp_check_capabilities(co, NULL))
     return 0;
 
   proto_setup_mpls_map(P, RTS_BGP, 1);

diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index ddef0390b0f50bba4790fdade5d3d7978a3174f2..483eadd8b43a0f64dc406674871f31e53288f48a 100644 (file)
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -706,7 +706,7 @@ bgp_total_aigp_metric(rte *r)
 
 void bgp_dump_state_change(struct bgp_conn *conn, uint old, uint new);
 void bgp_prepare_capabilities(struct bgp_conn *conn);
-int bgp_check_capabilities(struct bgp_conn *conn);
+int bgp_check_capabilities(struct bgp_conn *conn, struct bgp_caps *failed_caps);
 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 2eabe6ec13c36f58098b8834d25caa46867f249f..95c34f025c388f6fd1cd9044a0d665a4bc059c7f 100644 (file)
--- a/proto/bgp/packets.c
+++ b/proto/bgp/packets.c
@@ -693,9 +693,29 @@ err:
   return -1;
 }
 
+/*
+ * @failed_caps: instance of bgp_caps for marking missing capabilities
+ */
 int
-bgp_check_capabilities(struct bgp_conn *conn)
-{
+bgp_check_capabilities(struct bgp_conn *conn, struct bgp_caps *failed_caps)
+{
+#define CAPS_CHECK_FAIL(capability,value)       \
+  do {                                          \
+    if (failed_caps)                            \
+      failed_caps->capability = value;          \
+    return 0;                                   \
+  } while (0)
+
+#define AF_CAPS_CHECK_FAIL(_afi,capability,value)       \
+  do {                                                  \
+    if (failed_caps) {                                  \
+      failed_caps->af_data[0].afi = _afi;               \
+      failed_caps->af_data[0].capability = value;       \
+      failed_caps->af_count = 1;                        \
+    }                                                   \
+    return 0;                                           \
+  } while (0)
+
   struct bgp_proto *p = conn->bgp;
   struct bgp_caps *local = conn->local_caps;
   struct bgp_caps *remote = conn->remote_caps;
@@ -706,28 +726,35 @@ bgp_check_capabilities(struct bgp_conn *conn)
      but we need to run this just after we receive OPEN message */
 
   if (p->cf->require_refresh && !remote->route_refresh)
-    return 0;
+    CAPS_CHECK_FAIL(route_refresh, 1);
 
   if (p->cf->require_enhanced_refresh && !remote->enhanced_refresh)
-    return 0;
+    CAPS_CHECK_FAIL(enhanced_refresh, 1);
 
   if (p->cf->require_as4 && !remote->as4_support)
-    return 0;
+    CAPS_CHECK_FAIL(as4_support, 1);
 
   if (p->cf->require_extended_messages && !remote->ext_messages)
-    return 0;
+    CAPS_CHECK_FAIL(ext_messages, 1);
 
   if (p->cf->require_hostname && !remote->hostname)
-    return 0;
+    CAPS_CHECK_FAIL(hostname, NULL);
 
   if (p->cf->require_gr && !remote->gr_aware)
-    return 0;
+    CAPS_CHECK_FAIL(gr_aware, 1);
 
   if (p->cf->require_llgr && !remote->llgr_aware)
-    return 0;
+    CAPS_CHECK_FAIL(llgr_aware, 1);
 
   /* No check for require_roles, as it uses error code 2.11 instead of 2.7 */
 
+  u32 first_active_channel_afi = 0;
+
+  /*
+   * @failed_caps has one bgp_af_caps entry, but af_count is 0.
+   * AF_CAPS_CHECK_FAIL marks missing capability and increases af_count to 1.
+   */
+
   BGP_WALK_CHANNELS(p, c)
   {
     const struct bgp_af_caps *loc = bgp_find_af_caps(local,  c->afi);
@@ -736,30 +763,50 @@ bgp_check_capabilities(struct bgp_conn *conn)
     /* Find out whether this channel will be active */
     int active = loc->ready && rem->ready;
 
+    if (loc->ready && !first_active_channel_afi)
+      first_active_channel_afi = c->afi;
+
     /* Mandatory must be active */
     if (c->cf->mandatory && !active)
-      return 0;
+      AF_CAPS_CHECK_FAIL(c->afi, ready, 1);
 
     if (active)
     {
       if (c->cf->require_ext_next_hop && !rem->ext_next_hop)
-       return 0;
+        AF_CAPS_CHECK_FAIL(c->afi, ext_next_hop, 1);
 
-      if (c->cf->require_add_path && (loc->add_path & BGP_ADD_PATH_RX) && !(rem->add_path & BGP_ADD_PATH_TX))
-       return 0;
+      u32 missing = 0;
 
-      if (c->cf->require_add_path && (loc->add_path & BGP_ADD_PATH_TX) && !(rem->add_path & BGP_ADD_PATH_RX))
-       return 0;
+      if (c->cf->require_add_path)
+      {
+        if ((loc->add_path & BGP_ADD_PATH_RX) && !(rem->add_path & BGP_ADD_PATH_TX))
+          missing |= BGP_ADD_PATH_TX;
+
+        if ((loc->add_path & BGP_ADD_PATH_TX) && !(rem->add_path & BGP_ADD_PATH_RX))
+          missing |= BGP_ADD_PATH_RX;
+      }
+
+      if (missing != 0)
+        AF_CAPS_CHECK_FAIL(c->afi, add_path, missing);
 
       count++;
     }
   }
 
   /* We need at least one channel active */
-  if (!count)
-    return 0;
+  if (count == 0)
+  {
+    if (!first_active_channel_afi)
+      return 0;
+
+    if (failed_caps)
+      AF_CAPS_CHECK_FAIL(first_active_channel_afi, ready, 1);
+  }
 
   return 1;
+
+#undef CAPS_CHECK_FAIL
+#undef AF_CAPS_CHECK_FAIL
 }
 
 static int
@@ -942,8 +989,12 @@ bgp_rx_open(struct bgp_conn *conn, byte *pkt, uint len)
   if (!id || (p->is_internal && id == p->local_id))
   { bgp_error(conn, 2, 3, pkt+24, -4); return; }
 
+  /* Instance of bgp_caps with single bgp_af_caps entry used to store missing capabilities */
+  struct bgp_caps *failed_caps = allocz(sizeof(*failed_caps) + sizeof(failed_caps->af_data[0]));
+  failed_caps->role = BGP_ROLE_UNDEFINED;
+
   /* RFC 5492 5 - check for required capabilities */
-  if (p->cf->capabilities && !bgp_check_capabilities(conn))
+  if (p->cf->capabilities && !bgp_check_capabilities(conn, failed_caps))
   { bgp_error(conn, 2, 7, NULL, 0); return; }
 
   struct bgp_caps *caps = conn->remote_caps;