From b1352ed0ac39aaa7be7542275d1d43fa64ab28ac Mon Sep 17 00:00:00 2001 From: Eyal Bari Date: Fri, 7 Apr 2017 23:14:17 +0300 Subject: [PATCH] BD:unify bridge domain creation code Change-Id: I29082e7a0c556069180a157e55b3698cf8cd38c7 Signed-off-by: Eyal Bari --- src/vnet/api_errno.h | 4 +- src/vnet/l2/l2_api.c | 63 +++++++---------------------- src/vnet/l2/l2_bd.c | 92 ++++++++++++++++++++----------------------- src/vnet/l2/l2_bd.h | 37 ++++++++++++----- src/vnet/lisp-gpe/interface.c | 7 ++-- src/vpp/api/api.c | 12 +++--- 6 files changed, 97 insertions(+), 118 deletions(-) diff --git a/src/vnet/api_errno.h b/src/vnet/api_errno.h index e939404b412..0d5b2227b1d 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 026f1706b84..5a3c8dc2682 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 9d7a43d3623..cfaf4c994f5 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 83733411a42..e502d497e0b 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 4760f44833a..0598c048cf5 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 22410fcc22c..f1b6877f85b 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); } -- 2.16.6