BD:unify bridge domain creation code

Change-Id: I29082e7a0c556069180a157e55b3698cf8cd38c7
Signed-off-by: Eyal Bari <ebari@cisco.com>

6 files changed, 97 insertions, 118 deletions

diff --git a/src/vnet/api_errno.h b/src/vnet/api_errno.h
index e939404b..0d5b2227 100644
--- a/src/vnet/api_errno.h
+++ b/src/vnet/api_errno.h
@@ -107,7 +107,9 @@ _(ADDRESS_FOUND_FOR_INTERFACE, -114, "Address found for interface")	\
 _(SESSION_CONNECT_FAIL, -115, "Session failed to connect")              \
 _(ENTRY_ALREADY_EXISTS, -116, "Entry already exists")			\
 _(SVM_SEGMENT_CREATE_FAIL, -117, "svm segment create fail")		\
-_(APPLICATION_NOT_ATTACHED, -118, "application not attached")
+_(APPLICATION_NOT_ATTACHED, -118, "application not attached")           \
+_(BD_ALREADY_EXISTS, -119, "Bridge domain already exists")              \
+_(BD_IN_USE, -120, "Bridge domain has member interfaces")
 
 typedef enum
 {
diff --git a/src/vnet/l2/l2_api.c b/src/vnet/l2/l2_api.c
index 026f1706..5a3c8dc2 100644
--- a/src/vnet/l2/l2_api.c
+++ b/src/vnet/l2/l2_api.c
@@ -324,54 +324,20 @@ out:
 static void
 vl_api_bridge_domain_add_del_t_handler (vl_api_bridge_domain_add_del_t * mp)
 {
-  vlib_main_t *vm = vlib_get_main ();
-  bd_main_t *bdm = &bd_main;
-  vl_api_bridge_domain_add_del_reply_t *rmp;
-  int rv = 0;
-  u32 enable_flags = 0, disable_flags = 0;
-  u32 bd_id = ntohl (mp->bd_id);
-  u32 bd_index;
-
-  if (mp->is_add)
-    {
-      bd_index = bd_find_or_add_bd_index (bdm, bd_id);
-
-      if (mp->flood)
-	enable_flags |= L2_FLOOD;
-      else
-	disable_flags |= L2_FLOOD;
-
-      if (mp->uu_flood)
-	enable_flags |= L2_UU_FLOOD;
-      else
-	disable_flags |= L2_UU_FLOOD;
-
-      if (mp->forward)
-	enable_flags |= L2_FWD;
-      else
-	disable_flags |= L2_FWD;
-
-      if (mp->arp_term)
-	enable_flags |= L2_ARP_TERM;
-      else
-	disable_flags |= L2_ARP_TERM;
-
-      if (mp->learn)
-	enable_flags |= L2_LEARN;
-      else
-	disable_flags |= L2_LEARN;
-
-      if (enable_flags)
-	bd_set_flags (vm, bd_index, enable_flags, 1 /* enable */ );
-
-      if (disable_flags)
-	bd_set_flags (vm, bd_index, disable_flags, 0 /* disable */ );
-
-      bd_set_mac_age (vm, bd_index, mp->mac_age);
-    }
-  else
-    rv = bd_delete_bd_index (bdm, bd_id);
+  l2_bridge_domain_add_del_args_t a = {
+    .is_add = mp->is_add,
+    .flood = mp->flood,
+    .uu_flood = mp->uu_flood,
+    .forward = mp->forward,
+    .learn = mp->learn,
+    .arp_term = mp->arp_term,
+    .mac_age = mp->mac_age,
+    .bd_id = ntohl (mp->bd_id),
+  };
+
+  int rv = bd_add_del (&a);
 
+  vl_api_bridge_domain_add_del_reply_t *rmp;
   REPLY_MACRO (VL_API_BRIDGE_DOMAIN_ADD_DEL_REPLY);
 }
 
@@ -436,7 +402,8 @@ vl_api_bridge_domain_dump_t_handler (vl_api_bridge_domain_dump_t * mp)
 
   bd_id = ntohl (mp->bd_id);
 
-  bd_index = (bd_id == ~0) ? 0 : bd_find_or_add_bd_index (bdm, bd_id);
+  bd_index = (bd_id == ~0) ? 0 : bd_find_index (bdm, bd_id);
+  ASSERT (bd_index != ~0);
   end = (bd_id == ~0) ? vec_len (l2im->bd_configs) : bd_index + 1;
   for (; bd_index < end; bd_index++)
     {
diff --git a/src/vnet/l2/l2_bd.c b/src/vnet/l2/l2_bd.c
index 9d7a43d3..cfaf4c99 100644
--- a/src/vnet/l2/l2_bd.c
+++ b/src/vnet/l2/l2_bd.c
@@ -50,42 +50,35 @@ bd_main_t bd_main;
 void
 bd_validate (l2_bridge_domain_t * bd_config)
 {
-  if (!bd_is_valid (bd_config))
-    {
-      bd_config->feature_bitmap = ~L2INPUT_FEAT_ARP_TERM;
-      bd_config->bvi_sw_if_index = ~0;
-      bd_config->members = 0;
-      bd_config->flood_count = 0;
-      bd_config->tun_master_count = 0;
-      bd_config->tun_normal_count = 0;
-      bd_config->mac_by_ip4 = 0;
-      bd_config->mac_by_ip6 = hash_create_mem (0, sizeof (ip6_address_t),
-					       sizeof (uword));
-    }
+  if (bd_is_valid (bd_config))
+    return;
+  bd_config->feature_bitmap = ~L2INPUT_FEAT_ARP_TERM;
+  bd_config->bvi_sw_if_index = ~0;
+  bd_config->members = 0;
+  bd_config->flood_count = 0;
+  bd_config->tun_master_count = 0;
+  bd_config->tun_normal_count = 0;
+  bd_config->mac_by_ip4 = 0;
+  bd_config->mac_by_ip6 = hash_create_mem (0, sizeof (ip6_address_t),
+					   sizeof (uword));
 }
 
 u32
-bd_find_or_add_bd_index (bd_main_t * bdm, u32 bd_id)
+bd_find_index (bd_main_t * bdm, u32 bd_id)
 {
-  uword *p;
-  u32 rv;
-
-  if (bd_id == ~0)
-    {
-      bd_id = 0;
-      while (hash_get (bdm->bd_index_by_bd_id, bd_id))
-	bd_id++;
-    }
-  else
-    {
-      p = hash_get (bdm->bd_index_by_bd_id, bd_id);
-      if (p)
-	return (p[0]);
-    }
+  u32 *p = (u32 *) hash_get (bdm->bd_index_by_bd_id, bd_id);
+  if (!p)
+    return ~0;
+  return p[0];
+}
 
-  rv = clib_bitmap_first_clear (bdm->bd_index_bitmap);
+u32
+bd_add_bd_index (bd_main_t * bdm, u32 bd_id)
+{
+  ASSERT (!hash_get (bdm->bd_index_by_bd_id, bd_id));
+  u32 rv = clib_bitmap_first_clear (bdm->bd_index_bitmap);
 
-  /* mark this index busy */
+  /* mark this index taken */
   bdm->bd_index_bitmap = clib_bitmap_set (bdm->bd_index_bitmap, rv, 1);
 
   hash_set (bdm->bd_index_by_bd_id, bd_id, rv);
@@ -96,21 +89,14 @@ bd_find_or_add_bd_index (bd_main_t * bdm, u32 bd_id)
   return rv;
 }
 
-int
-bd_delete_bd_index (bd_main_t * bdm, u32 bd_id)
+static int
+bd_delete (bd_main_t * bdm, u32 bd_index)
 {
-  uword *p;
-  u32 bd_index;
-
-  p = hash_get (bdm->bd_index_by_bd_id, bd_id);
-  if (p == 0)
-    return VNET_API_ERROR_NO_SUCH_ENTRY;
-
-  bd_index = p[0];
+  u32 bd_id = l2input_main.bd_configs[bd_index].bd_id;
+  hash_unset (bdm->bd_index_by_bd_id, bd_id);
 
   /* mark this index clear */
   bdm->bd_index_bitmap = clib_bitmap_set (bdm->bd_index_bitmap, bd_index, 0);
-  hash_unset (bdm->bd_index_by_bd_id, bd_id);
 
   l2input_main.bd_configs[bd_index].bd_id = ~0;
   l2input_main.bd_configs[bd_index].feature_bitmap = 0;
@@ -202,14 +188,13 @@ clib_error_t *
 l2bd_init (vlib_main_t * vm)
 {
   bd_main_t *bdm = &bd_main;
-  u32 bd_index;
   bdm->bd_index_by_bd_id = hash_create (0, sizeof (uword));
   /*
    * create a dummy bd with bd_id of 0 and bd_index of 0 with feature set
    * to packet drop only. Thus, packets received from any L2 interface with
    * uninitialized bd_index of 0 can be dropped safely.
    */
-  bd_index = bd_find_or_add_bd_index (bdm, 0);
+  u32 bd_index = bd_add_bd_index (bdm, 0);
   ASSERT (bd_index == 0);
   l2input_main.bd_configs[0].feature_bitmap = L2INPUT_FEAT_DROP;
 
@@ -1087,16 +1072,16 @@ bd_add_del (l2_bridge_domain_add_del_args_t * a)
 {
   bd_main_t *bdm = &bd_main;
   vlib_main_t *vm = bdm->vlib_main;
-  u32 enable_flags = 0, disable_flags = 0;
-  u32 bd_index = ~0;
   int rv = 0;
 
+  u32 bd_index = bd_find_index (bdm, a->bd_id);
   if (a->is_add)
     {
-      bd_index = bd_find_or_add_bd_index (bdm, a->bd_id);
-      if (bd_index == ~0)
-	return bd_index;
+      if (bd_index != ~0)
+	return VNET_API_ERROR_BD_ALREADY_EXISTS;
+      bd_index = bd_add_bd_index (bdm, a->bd_id);
 
+      u32 enable_flags = 0, disable_flags = 0;
       if (a->flood)
 	enable_flags |= L2_FLOOD;
       else
@@ -1131,7 +1116,13 @@ bd_add_del (l2_bridge_domain_add_del_args_t * a)
       bd_set_mac_age (vm, bd_index, a->mac_age);
     }
   else
-    rv = bd_delete_bd_index (bdm, a->bd_id);
+    {
+      if (bd_index == ~0)
+	return VNET_API_ERROR_NO_SUCH_ENTRY;
+      if (vec_len (l2input_main.bd_configs[bd_index].members))
+	return VNET_API_ERROR_BD_IN_USE;
+      rv = bd_delete (bdm, bd_index);
+    }
 
   return rv;
 }
@@ -1215,6 +1206,9 @@ bd_add_del_command_fn (vlib_main_t * vm, unformat_input_t * input,
       if (is_add)
 	vlib_cli_output (vm, "bridge-domain %d", bd_id);
       break;
+    case VNET_API_ERROR_BD_IN_USE:
+      error = clib_error_return (0, "bridge domain in use - remove members");
+      goto done;
     case VNET_API_ERROR_NO_SUCH_ENTRY:
       error = clib_error_return (0, "bridge domain id does not exist");
       goto done;
diff --git a/src/vnet/l2/l2_bd.h b/src/vnet/l2/l2_bd.h
index 83733411..e502d497 100644
--- a/src/vnet/l2/l2_bd.h
+++ b/src/vnet/l2/l2_bd.h
@@ -128,28 +128,47 @@ u32 bd_remove_member (l2_bridge_domain_t * bd_config, u32 sw_if_index);
 
 u32 bd_set_flags (vlib_main_t * vm, u32 bd_index, u32 flags, u32 enable);
 void bd_set_mac_age (vlib_main_t * vm, u32 bd_index, u8 age);
+int bd_add_del (l2_bridge_domain_add_del_args_t * args);
 
 /**
- * \brief Get or create a bridge domain.
+ * \brief Get a bridge domain.
+ *
+ * Get a bridge domain with the given bridge domain ID.
+ *
+ * \param bdm bd_main pointer.
+ * \param bd_id The bridge domain ID
+ * \return The bridge domain index in \c l2input_main->l2_bridge_domain_t vector.
+ */
+u32 bd_find_index (bd_main_t * bdm, u32 bd_id);
+
+/**
+ * \brief Create a bridge domain.
  *
- * Get or create a bridge domain with the given bridge domain ID.
+ * Create a bridge domain with the given bridge domain ID
  *
  * \param bdm bd_main pointer.
- * \param bd_id The bridge domain ID or ~0 if an arbitrary unused bridge domain should be used.
  * \return The bridge domain index in \c l2input_main->l2_bridge_domain_t vector.
  */
-u32 bd_find_or_add_bd_index (bd_main_t * bdm, u32 bd_id);
+u32 bd_add_bd_index (bd_main_t * bdm, u32 bd_id);
 
 /**
- * \brief Delete a bridge domain.
+ * \brief Get or create a bridge domain.
  *
- * Delete an existing bridge domain with the given bridge domain ID.
+ * Get a bridge domain with the given bridge domain ID, if one exists, otherwise
+ * create one with the given ID, or the first unused ID if the given ID is ~0..
  *
  * \param bdm bd_main pointer.
- * \param bd_id The bridge domain ID.
- * \return 0 on success and -1 if the bridge domain does not exist.
+ * \param bd_id The bridge domain ID
+ * \return The bridge domain index in \c l2input_main->l2_bridge_domain_t vector.
  */
-int bd_delete_bd_index (bd_main_t * bdm, u32 bd_id);
+static inline u32
+bd_find_or_add_bd_index (bd_main_t * bdm, u32 bd_id)
+{
+  u32 bd_index = bd_find_index (bdm, bd_id);
+  if (bd_index == ~0)
+    return bd_add_bd_index (bdm, bd_id);
+  return bd_index;
+}
 
 u32 bd_add_del_ip_mac (u32 bd_index,
 		       u8 * ip_addr, u8 * mac_addr, u8 is_ip6, u8 is_add);
diff --git a/src/vnet/lisp-gpe/interface.c b/src/vnet/lisp-gpe/interface.c
index 4760f448..0598c048 100644
--- a/src/vnet/lisp-gpe/interface.c
+++ b/src/vnet/lisp-gpe/interface.c
@@ -705,11 +705,10 @@ void
 lisp_gpe_del_l2_iface (lisp_gpe_main_t * lgm, u32 vni, u32 bd_id)
 {
   tunnel_lookup_t *l2_ifaces = &lgm->l2_ifaces;
-  u16 bd_index;
-  uword *hip;
 
-  bd_index = bd_find_or_add_bd_index (&bd_main, bd_id);
-  hip = hash_get (l2_ifaces->hw_if_index_by_dp_table, bd_index);
+  u32 bd_index = bd_find_index (&bd_main, bd_id);
+  ASSERT (bd_index != ~0);
+  uword *hip = hash_get (l2_ifaces->hw_if_index_by_dp_table, bd_index);
 
   if (hip == 0)
     {
diff --git a/src/vpp/api/api.c b/src/vpp/api/api.c
index 22410fcc..f1b6877f 100644
--- a/src/vpp/api/api.c
+++ b/src/vpp/api/api.c
@@ -410,21 +410,19 @@ static void
   bd_main_t *bdm = &bd_main;
   vl_api_sw_interface_set_l2_bridge_reply_t *rmp;
   int rv = 0;
-  u32 rx_sw_if_index = ntohl (mp->rx_sw_if_index);
-  u32 bd_id = ntohl (mp->bd_id);
-  u32 bd_index;
-  u32 bvi = mp->bvi;
-  u8 shg = mp->shg;
   vlib_main_t *vm = vlib_get_main ();
   vnet_main_t *vnm = vnet_get_main ();
 
   VALIDATE_RX_SW_IF_INDEX (mp);
+  u32 rx_sw_if_index = ntohl (mp->rx_sw_if_index);
 
-  bd_index = bd_find_or_add_bd_index (bdm, bd_id);
 
   if (mp->enable)
     {
-      //VALIDATE_TX_SW_IF_INDEX(mp);
+      u32 bd_id = ntohl (mp->bd_id);
+      u32 bd_index = bd_find_or_add_bd_index (bdm, bd_id);
+      u32 bvi = mp->bvi;
+      u8 shg = mp->shg;
       rv = set_int_l2_mode (vm, vnm, MODE_L2_BRIDGE,
 			    rx_sw_if_index, bd_index, bvi, shg, 0);
     }