From 345f179482dbf5258cad088414bc9eb230decd97 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Fri, 1 Dec 2017 11:23:11 -0800 Subject: [PATCH] Implement bulk create/remove for nhgm/route/fdb (#269) * Implement sai_bulk_remove_next_hop_group_members * Implement sai_bulk_remove_route_entry * Implement sai_bulk_create/remove_fdb_entry Signed-off-by: Qi Luo --- lib/inc/sai_redis.h | 11 ++ lib/inc/sairedis.h | 17 ++- lib/src/sai_redis_fdb.cpp | 159 +++++++++++++++++++++++++++ lib/src/sai_redis_generic_remove.cpp | 86 +++++++++++++++ lib/src/sai_redis_nexthopgroup.cpp | 10 +- lib/src/sai_redis_route.cpp | 22 +++- syncd/syncd.cpp | 31 +++--- syncd/tests.cpp | 120 +++++++++++++++++++- 8 files changed, 432 insertions(+), 24 deletions(-) diff --git a/lib/inc/sai_redis.h b/lib/inc/sai_redis.h index 590e2d5bd56e..bee7472008a4 100644 --- a/lib/inc/sai_redis.h +++ b/lib/inc/sai_redis.h @@ -171,6 +171,17 @@ sai_status_t redis_generic_remove_neighbor_entry( sai_status_t redis_generic_remove_route_entry( _In_ const sai_route_entry_t* route_entry); +sai_status_t redis_bulk_generic_remove( + _In_ sai_object_type_t object_type, + _In_ uint32_t object_count, + _In_ const sai_object_id_t *object_id, /* array */ + _Inout_ sai_status_t *object_statuses) /* array */; + +sai_status_t internal_redis_bulk_generic_remove( + _In_ sai_object_type_t object_type, + _In_ const std::vector &serialized_object_ids, + _Out_ sai_status_t *object_statuses) /* array */; + // SET sai_status_t redis_generic_set( diff --git a/lib/inc/sairedis.h b/lib/inc/sairedis.h index 25725f9bb44e..a655d13062df 100644 --- a/lib/inc/sairedis.h +++ b/lib/inc/sairedis.h @@ -217,7 +217,7 @@ sai_status_t sai_bulk_get_route_entry_attribute( _In_ sai_bulk_op_type_t type, _Out_ sai_status_t *object_statuses); -sai_status_t redis_bulk_object_create_next_hop_group_members( +sai_status_t sai_bulk_create_next_hop_group_members( _In_ sai_object_id_t switch_id, _In_ uint32_t object_count, _In_ const uint32_t *attr_count, @@ -226,10 +226,23 @@ sai_status_t redis_bulk_object_create_next_hop_group_members( _Out_ sai_object_id_t *object_id, _Out_ sai_status_t *object_statuses); -sai_status_t redis_bulk_object_remove_next_hop_group_members( +sai_status_t sai_bulk_remove_next_hop_group_members( _In_ uint32_t object_count, _In_ const sai_object_id_t *object_id, _In_ sai_bulk_op_type_t type, _Out_ sai_status_t *object_statuses); +sai_status_t sai_bulk_create_fdb_entry( + _In_ uint32_t object_count, + _In_ const sai_fdb_entry_t *fdb_entry, + _In_ const uint32_t *attr_count, + _In_ const sai_attribute_t *const *attr_list, + _In_ sai_bulk_op_type_t type, + _Out_ sai_status_t *object_statuses); + +sai_status_t sai_bulk_remove_fdb_entry( + _In_ uint32_t object_count, + _In_ const sai_fdb_entry_t *fdb_entry, + _In_ sai_bulk_op_type_t type, + _Out_ sai_status_t *object_statuses); #endif // __SAIREDIS__ diff --git a/lib/src/sai_redis_fdb.cpp b/lib/src/sai_redis_fdb.cpp index 5c754b79ebaa..5c262bef31c1 100644 --- a/lib/src/sai_redis_fdb.cpp +++ b/lib/src/sai_redis_fdb.cpp @@ -1,4 +1,5 @@ #include "sai_redis.h" +#include "meta/saiserialize.h" sai_status_t redis_flush_fdb_entries( _In_ sai_object_id_t switch_id, @@ -20,3 +21,161 @@ const sai_fdb_api_t redis_fdb_api = { redis_flush_fdb_entries, }; + +sai_status_t redis_dummy_create_fdb_entry( + _In_ const sai_fdb_entry_t *fdb_entry, + _In_ uint32_t attr_count, + _In_ const sai_attribute_t *attr_list) +{ + SWSS_LOG_ENTER(); + + /* + * Since we are using validation for each fdb in bulk operations, we + * can't execute actual CREATE, we need to do dummy create and then introduce + * internal bulk_create operation that will only touch redis db only once. + * So we are returning success here. + */ + + return SAI_STATUS_SUCCESS; +} + +sai_status_t sai_bulk_create_fdb_entry( + _In_ uint32_t object_count, + _In_ const sai_fdb_entry_t *fdb_entry, + _In_ const uint32_t *attr_count, + _In_ const sai_attribute_t *const *attr_list, + _In_ sai_bulk_op_type_t type, + _Out_ sai_status_t *object_statuses) +{ + std::lock_guard lock(g_apimutex); + + SWSS_LOG_ENTER(); + + if (object_count < 1) + { + SWSS_LOG_ERROR("expected at least 1 object to create"); + + return SAI_STATUS_INVALID_PARAMETER; + } + + if (fdb_entry == NULL) + { + SWSS_LOG_ERROR("fdb_entry is NULL"); + + return SAI_STATUS_INVALID_PARAMETER; + } + + if (attr_count == NULL) + { + SWSS_LOG_ERROR("attr_count is NULL"); + + return SAI_STATUS_INVALID_PARAMETER; + } + + if (attr_list == NULL) + { + SWSS_LOG_ERROR("attr_list is NULL"); + + return SAI_STATUS_INVALID_PARAMETER; + } + + switch (type) + { + case SAI_BULK_OP_TYPE_STOP_ON_ERROR: + case SAI_BULK_OP_TYPE_INGORE_ERROR: + // ok + break; + + default: + + SWSS_LOG_ERROR("invalid bulk operation type %d", type); + + return SAI_STATUS_INVALID_PARAMETER; + } + + if (object_statuses == NULL) + { + SWSS_LOG_ERROR("object_statuses is NULL"); + + return SAI_STATUS_INVALID_PARAMETER; + } + + std::vector serialized_object_ids; + + for (uint32_t idx = 0; idx < object_count; ++idx) + { + /* + * At the beginning set all statuses to not executed. + */ + + object_statuses[idx] = SAI_STATUS_NOT_EXECUTED; + + serialized_object_ids.push_back( + sai_serialize_fdb_entry(fdb_entry[idx])); + } + + for (uint32_t idx = 0; idx < object_count; ++idx) + { + sai_status_t status = + meta_sai_create_fdb_entry( + &fdb_entry[idx], + attr_count[idx], + attr_list[idx], + &redis_dummy_create_fdb_entry); + + object_statuses[idx] = status; + + if (status != SAI_STATUS_SUCCESS) + { + // TODO add attr id and value + + SWSS_LOG_ERROR("failed on index %u: %s", + idx, + serialized_object_ids[idx].c_str()); + + if (type == SAI_BULK_OP_TYPE_STOP_ON_ERROR) + { + SWSS_LOG_NOTICE("stop on error since previous operation failed"); + break; + } + } + } + + /* + * TODO: we need to record operation type + */ + + return internal_redis_bulk_generic_create( + SAI_OBJECT_TYPE_ROUTE_ENTRY, + serialized_object_ids, + attr_count, + attr_list, + object_statuses); +} + +sai_status_t sai_bulk_remove_fdb_entry( + _In_ uint32_t object_count, + _In_ const sai_fdb_entry_t *fdb_entry, + _In_ sai_bulk_op_type_t type, + _Out_ sai_status_t *object_statuses) +{ + std::lock_guard lock(g_apimutex); + + SWSS_LOG_ENTER(); + + std::vector serialized_object_ids; + + for (uint32_t idx = 0; idx < object_count; ++idx) + { + /* + * At the beginning set all statuses to not executed. + */ + + object_statuses[idx] = SAI_STATUS_NOT_EXECUTED; + + serialized_object_ids.push_back( + sai_serialize_fdb_entry(fdb_entry[idx])); + } + + return internal_redis_bulk_generic_remove(SAI_OBJECT_TYPE_ROUTE_ENTRY, serialized_object_ids, object_statuses); +} diff --git a/lib/src/sai_redis_generic_remove.cpp b/lib/src/sai_redis_generic_remove.cpp index ca52ffe56036..e3dea205d02b 100644 --- a/lib/src/sai_redis_generic_remove.cpp +++ b/lib/src/sai_redis_generic_remove.cpp @@ -51,6 +51,92 @@ sai_status_t redis_generic_remove( return status; } +sai_status_t redis_bulk_generic_remove( + _In_ sai_object_type_t object_type, + _In_ uint32_t object_count, + _In_ const sai_object_id_t *object_id, /* array */ + _Out_ sai_status_t *object_statuses) /* array */ +{ + SWSS_LOG_ENTER(); + + std::vector serialized_object_ids; + + // on create vid is put in db by syncd + for (uint32_t idx = 0; idx < object_count; idx++) + { + std::string str_object_id = sai_serialize_object_id(object_id[idx]); + serialized_object_ids.push_back(str_object_id); + } + + return internal_redis_bulk_generic_remove( + object_type, + serialized_object_ids, + object_statuses); +} + +sai_status_t internal_redis_bulk_generic_remove( + _In_ sai_object_type_t object_type, + _In_ const std::vector &serialized_object_ids, + _Out_ sai_status_t *object_statuses) /* array */ +{ + SWSS_LOG_ENTER(); + + std::string str_object_type = sai_serialize_object_type(object_type); + + std::vector entries; + + /* + * We are recording all entries and their statuses, but we send to sairedis + * only those that succeeded metadata check, since only those will be + * executed on syncd, so there is no need with bothering decoding statuses + * on syncd side. + */ + + for (size_t idx = 0; idx < serialized_object_ids.size(); ++idx) + { + std::string str_attr = ""; + + swss::FieldValueTuple fvtNoStatus(serialized_object_ids[idx], str_attr); + + entries.push_back(fvtNoStatus); + } + + /* + * We are adding number of entries to actualy add ':' to be compatible + * with previous + */ + + if (g_record) + { + std::string joined; + + for (const auto &e: entries) + { + // ||obj_id|attr=val|attr=val||obj_id|attr=val|attr=val + + joined += "||" + fvField(e) + "|" + fvValue(e); + } + + /* + * Capital 'C' stads for bulk CREATE operation. + */ + + recordLine("C|" + str_object_type + joined); + } + + // key: object_type:count + // field: object_id + // value: object_attrs + std::string key = str_object_type + ":" + std::to_string(entries.size()); + + if (entries.size()) + { + g_asicState->set(key, entries, "bulkremove"); + } + + return SAI_STATUS_SUCCESS; +} + sai_status_t redis_generic_remove_fdb_entry( _In_ const sai_fdb_entry_t* fdb_entry) { diff --git a/lib/src/sai_redis_nexthopgroup.cpp b/lib/src/sai_redis_nexthopgroup.cpp index 10b528f5738a..e75fbe4bf108 100644 --- a/lib/src/sai_redis_nexthopgroup.cpp +++ b/lib/src/sai_redis_nexthopgroup.cpp @@ -1,6 +1,6 @@ #include "sai_redis.h" -sai_status_t redis_bulk_object_create_next_hop_group_members( +sai_status_t sai_bulk_create_next_hop_group_members( _In_ sai_object_id_t switch_id, _In_ uint32_t object_count, _In_ const uint32_t *attr_count, @@ -17,7 +17,7 @@ sai_status_t redis_bulk_object_create_next_hop_group_members( , switch_id, attr_count, attrs, object_statuses); } -sai_status_t redis_bulk_object_remove_next_hop_group_members( +sai_status_t sai_bulk_remove_next_hop_group_members( _In_ uint32_t object_count, _In_ const sai_object_id_t *object_id, _In_ sai_bulk_op_type_t type, @@ -27,7 +27,7 @@ sai_status_t redis_bulk_object_remove_next_hop_group_members( SWSS_LOG_ENTER(); - return SAI_STATUS_NOT_IMPLEMENTED; + return redis_bulk_generic_remove(SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER, object_count, object_id, object_statuses); } REDIS_GENERIC_QUAD(NEXT_HOP_GROUP,next_hop_group); @@ -39,6 +39,6 @@ const sai_next_hop_group_api_t redis_next_hop_group_api = { REDIS_GENERIC_QUAD_API(next_hop_group_member) // TODO: upstream signiture fix to SAI repo - (sai_bulk_object_create_fn)redis_bulk_object_create_next_hop_group_members, - redis_bulk_object_remove_next_hop_group_members, + (sai_bulk_object_create_fn)sai_bulk_create_next_hop_group_members, + sai_bulk_remove_next_hop_group_members, }; diff --git a/lib/src/sai_redis_route.cpp b/lib/src/sai_redis_route.cpp index fef327460f0e..ab33720c2959 100644 --- a/lib/src/sai_redis_route.cpp +++ b/lib/src/sai_redis_route.cpp @@ -8,6 +8,12 @@ REDIS_GENERIC_QUAD_ENTRY(ROUTE_ENTRY,route_entry); const sai_route_api_t redis_route_api = { REDIS_GENERIC_QUAD_API(route_entry) + + // TODO: uncomment block after SAI 1.2 + // sai_bulk_create_route_entry, + // sai_bulk_remove_route_entry, + // sai_bulk_set_route_entry_attribute, + // sai_bulk_get_route_entry_attribute }; sai_status_t redis_dummy_create_route_entry( @@ -151,7 +157,21 @@ sai_status_t sai_bulk_remove_route_entry( SWSS_LOG_ENTER(); - return SAI_STATUS_NOT_IMPLEMENTED; + std::vector serialized_object_ids; + + for (uint32_t idx = 0; idx < object_count; ++idx) + { + /* + * At the beginning set all statuses to not executed. + */ + + object_statuses[idx] = SAI_STATUS_NOT_EXECUTED; + + serialized_object_ids.push_back( + sai_serialize_route_entry(route_entry[idx])); + } + + return internal_redis_bulk_generic_remove(SAI_OBJECT_TYPE_ROUTE_ENTRY, serialized_object_ids, object_statuses); } sai_status_t redis_dummy_set_route_entry( diff --git a/syncd/syncd.cpp b/syncd/syncd.cpp index d6f999e11f54..393bbf2ddfad 100644 --- a/syncd/syncd.cpp +++ b/syncd/syncd.cpp @@ -2094,20 +2094,20 @@ sai_status_t handle_bulk_generic( uint32_t attr_count = list->get_attr_count(); sai_object_meta_key_t meta_key; - - if (object_type == SAI_OBJECT_TYPE_ROUTE_ENTRY) - { - meta_key.objecttype = SAI_OBJECT_TYPE_ROUTE_ENTRY; - sai_deserialize_route_entry(object_ids[idx], meta_key.objectkey.key.route_entry); - } - else if (object_type == SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER) - { - meta_key.objecttype = SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER; - sai_deserialize_object_id(object_ids[idx], meta_key.objectkey.key.object_id); - } - else + meta_key.objecttype = object_type; + switch (object_type) { - throw std::invalid_argument("object_type"); + case SAI_OBJECT_TYPE_ROUTE_ENTRY: + sai_deserialize_route_entry(object_ids[idx], meta_key.objectkey.key.route_entry); + break; + case SAI_OBJECT_TYPE_FDB_ENTRY: + sai_deserialize_fdb_entry(object_ids[idx], meta_key.objectkey.key.fdb_entry); + break; + case SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER: + sai_deserialize_object_id(object_ids[idx], meta_key.objectkey.key.object_id); + break; + default: + throw std::invalid_argument("object_type"); } if (api == (sai_common_api_t)SAI_COMMON_API_BULK_SET) @@ -2118,6 +2118,10 @@ sai_status_t handle_bulk_generic( { status = handle_non_object_id(meta_key, SAI_COMMON_API_CREATE, attr_count, attr_list); } + else if (api == (sai_common_api_t)SAI_COMMON_API_BULK_REMOVE) + { + status = handle_non_object_id(meta_key, SAI_COMMON_API_REMOVE, attr_count, attr_list); + } else { SWSS_LOG_ERROR("api %d is not supported in bulk route", api); @@ -2220,6 +2224,7 @@ sai_status_t processBulkEvent( { case SAI_OBJECT_TYPE_ROUTE_ENTRY: case SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER: + case SAI_OBJECT_TYPE_FDB_ENTRY: status = handle_bulk_generic(object_type, object_ids, api, attributes); break; diff --git a/syncd/tests.cpp b/syncd/tests.cpp index 53edab8e567c..f13775da1791 100644 --- a/syncd/tests.cpp +++ b/syncd/tests.cpp @@ -250,9 +250,9 @@ void test_bulk_next_hop_group_member_create() std::vector statuses(count); std::vector object_id(count); - redis_bulk_object_create_next_hop_group_members(switch_id, count, nhgm_attrs_count.data(), nhgm_attrs_array.data() + sai_bulk_create_next_hop_group_members(switch_id, count, nhgm_attrs_count.data(), nhgm_attrs_array.data() , SAI_BULK_OP_TYPE_INGORE_ERROR, object_id.data(), statuses.data()); - ASSERT_SUCCESS("Failed to create nhgm"); + ASSERT_SUCCESS("Failed to bulk create nhgm"); for (size_t j = 0; j < statuses.size(); j++) { status = statuses[j]; @@ -262,7 +262,7 @@ void test_bulk_next_hop_group_member_create() consumerThreads->join(); delete consumerThreads; - // check the SAI api calling + // check the created nhgm for (size_t i = 0; i < created_next_hop_group_member.size(); i++) { auto& created = created_next_hop_group_member[i]; @@ -270,6 +270,114 @@ void test_bulk_next_hop_group_member_create() assert(created_attrs.size() == 2); assert(created_attrs[1].value.oid == nhgm_attrs[i][1].value.oid); } + + status = sai_bulk_remove_next_hop_group_members(count, object_id.data(), SAI_BULK_OP_TYPE_INGORE_ERROR, statuses.data()); + ASSERT_SUCCESS("Failed to bulk remove nhgm"); +} + +void test_bulk_fdb_create() +{ + SWSS_LOG_ENTER(); + + swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_NOTICE); + + clearDB(); + meta_init_db(); + redis_clear_switch_ids(); + + swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_DEBUG); + + sai_status_t status; + + sai_route_api_t *sai_fdb_api = NULL; + sai_switch_api_t *sai_switch_api = NULL; + + sai_api_query(SAI_API_FDB, (void**)&sai_fdb_api); + sai_api_query(SAI_API_SWITCH, (void**)&sai_switch_api); + + uint32_t count = 3; + + std::vector fdbs; + + uint32_t index = 15; + + sai_attribute_t swattr; + + swattr.id = SAI_SWITCH_ATTR_INIT_SWITCH; + swattr.value.booldata = true; + + sai_object_id_t switch_id; + status = sai_switch_api->create_switch(&switch_id, 1, &swattr); + + ASSERT_SUCCESS("Failed to create switch"); + + std::vector> fdb_attrs; + std::vector fdb_attrs_array; + std::vector fdb_attrs_count; + + for (uint32_t i = index; i < index + count; ++i) + { + // virtual router + sai_object_id_t vr = create_dummy_object_id(SAI_OBJECT_TYPE_VIRTUAL_ROUTER); + object_reference_insert(vr); + sai_object_meta_key_t meta_key_vr = { .objecttype = SAI_OBJECT_TYPE_VIRTUAL_ROUTER, .objectkey = { .key = { .object_id = vr } } }; + std::string vr_key = sai_serialize_object_meta_key(meta_key_vr); + ObjectAttrHash[vr_key] = { }; + + // bridge port + sai_object_id_t bridge_port = create_dummy_object_id(SAI_OBJECT_TYPE_BRIDGE_PORT); + object_reference_insert(bridge_port); + sai_object_meta_key_t meta_key_bridge_port = { .objecttype = SAI_OBJECT_TYPE_BRIDGE_PORT, .objectkey = { .key = { .object_id = bridge_port } } }; + std::string bridge_port_key = sai_serialize_object_meta_key(meta_key_bridge_port); + ObjectAttrHash[bridge_port_key] = { }; + + sai_fdb_entry_t fdb_entry; + fdb_entry.switch_id = switch_id; + memset(fdb_entry.mac_address, 0, sizeof(sai_mac_t)); + fdb_entry.mac_address[0] = 0xD; + fdb_entry.bridge_type = SAI_FDB_ENTRY_BRIDGE_TYPE_1Q; + fdb_entry.vlan_id = (unsigned short)(1011 + i); + fdb_entry.bridge_id = SAI_NULL_OBJECT_ID; + + fdbs.push_back(fdb_entry); + + std::vector attrs; + sai_attribute_t attr; + + attr.id = SAI_FDB_ENTRY_ATTR_TYPE; + attr.value.s32 = (i % 2) ? SAI_FDB_ENTRY_TYPE_DYNAMIC : SAI_FDB_ENTRY_TYPE_STATIC; + attrs.push_back(attr); + + attr.id = SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID; + attr.value.oid = bridge_port; + attrs.push_back(attr); + + attr.id = SAI_FDB_ENTRY_ATTR_PACKET_ACTION; + attr.value.s32 = SAI_PACKET_ACTION_FORWARD; + attrs.push_back(attr); + + fdb_attrs.push_back(attrs); + fdb_attrs_count.push_back((unsigned int)attrs.size()); + } + + for (size_t j = 0; j < fdb_attrs.size(); j++) + { + fdb_attrs_array.push_back(fdb_attrs[j].data()); + } + + std::vector statuses(count); + status = sai_bulk_create_fdb_entry(count, fdbs.data(), fdb_attrs_count.data(), fdb_attrs_array.data() + , SAI_BULK_OP_TYPE_INGORE_ERROR, statuses.data()); + ASSERT_SUCCESS("Failed to create fdb"); + for (size_t j = 0; j < statuses.size(); j++) + { + status = statuses[j]; + ASSERT_SUCCESS("Failed to create route # %zu", j); + } + + // Remove route entry + status = sai_bulk_remove_fdb_entry(count, fdbs.data(), SAI_BULK_OP_TYPE_INGORE_ERROR, statuses.data()); + ASSERT_SUCCESS("Failed to bulk remove route entry"); } void test_bulk_route_set() @@ -405,6 +513,10 @@ void test_bulk_route_set() // TODO we need to add consumer producer test here to see // if after consume we get pop we get expectd parameters + + // Remove route entry + status = sai_bulk_remove_route_entry(count, routes.data(), SAI_BULK_OP_TYPE_INGORE_ERROR, statuses.data()); + ASSERT_SUCCESS("Failed to bulk remove route entry"); } int main() @@ -423,6 +535,8 @@ int main() test_bulk_next_hop_group_member_create(); + test_bulk_fdb_create(); + test_bulk_route_set(); sai_api_uninitialize();