From 995e71f3babc6c5ca799f8f763e2341918f78e08 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak <38952541+stepanblyschak@users.noreply.github.com> Date: Wed, 26 Feb 2025 10:33:55 +0200 Subject: [PATCH] [sairedis/syncd] Implement bulk get support (#1509) DEPENDS ON sonic-net/sonic-swss-common#966. Implement bulk get support for SAIRedis, Syncd, SaiPlayer and VSlib. --- lib/ClientServerSai.cpp | 13 +- lib/Recorder.cpp | 34 ++ lib/Recorder.h | 8 + lib/RedisRemoteSaiInterface.cpp | 135 +++++++- lib/RedisRemoteSaiInterface.h | 25 ++ lib/Sai.cpp | 14 +- lib/ServerSai.cpp | 11 +- meta/Meta.cpp | 52 ++- saiplayer/SaiPlayer.cpp | 126 ++++++- saiplayer/SaiPlayer.h | 3 + syncd/Syncd.cpp | 206 +++++++++++- syncd/Syncd.h | 14 + syncd/VendorSai.cpp | 28 +- syncd/tests/TestSyncdBrcm.cpp | 317 ++++++++++++++++++ tests/BCM56850.pl | 8 + tests/BCM56850/port_bulk_get.rec | 9 + unittest/lib/TestClientServerSai.cpp | 2 +- unittest/lib/TestRedisRemoteSaiInterface.cpp | 24 -- unittest/lib/TestSai.cpp | 2 +- unittest/lib/TestServerSai.cpp | 2 +- unittest/meta/TestMeta.cpp | 2 +- unittest/syncd/TestVendorSai.cpp | 3 +- unittest/vslib/TestSai.cpp | 83 ++++- .../vslib/TestVirtualSwitchSaiInterface.cpp | 39 ++- vslib/Sai.cpp | 13 +- vslib/SwitchStateBase.cpp | 54 +++ vslib/SwitchStateBase.h | 8 + vslib/VirtualSwitchSaiInterface.cpp | 28 +- vslib/VirtualSwitchSaiInterface.h | 9 + 29 files changed, 1184 insertions(+), 88 deletions(-) create mode 100644 tests/BCM56850/port_bulk_get.rec diff --git a/lib/ClientServerSai.cpp b/lib/ClientServerSai.cpp index d0bc7577..5e5422d6 100644 --- a/lib/ClientServerSai.cpp +++ b/lib/ClientServerSai.cpp @@ -426,11 +426,18 @@ sai_status_t ClientServerSai::bulkGet( _In_ sai_bulk_op_error_mode_t mode, _Out_ sai_status_t *object_statuses) { + MUTEX(); SWSS_LOG_ENTER(); + REDIS_CHECK_API_INITIALIZED(); - SWSS_LOG_ERROR("not implemented, FIXME"); - - return SAI_STATUS_NOT_IMPLEMENTED; + return m_sai->bulkGet( + object_type, + object_count, + object_id, + attr_count, + attr_list, + mode, + object_statuses); } // BULK QUAD ENTRY diff --git a/lib/Recorder.cpp b/lib/Recorder.cpp index ffd876a5..b4e1e016 100644 --- a/lib/Recorder.cpp +++ b/lib/Recorder.cpp @@ -641,6 +641,40 @@ void Recorder::recordGenericGetResponse( recordLine("G|" + sai_serialize_status(status) + "|" + Globals::joinFieldValues(arguments)); } +void Recorder::recordBulkGenericGet( + _In_ const std::string& key, + _In_ const std::vector& arguments) +{ + SWSS_LOG_ENTER(); + + std::string joined; + + for (const auto &e: arguments) + { + joined += "||" + fvField(e) + "|" + fvValue(e); + } + + // capital 'B' stands for Bulk GET operation. Note: 'G' already used for get response. + + recordLine("B|" + key + joined); +} + +void Recorder::recordBulkGenericGetResponse( + _In_ sai_status_t status, + _In_ const std::vector& arguments) +{ + SWSS_LOG_ENTER(); + + std::string joined; + + for (const auto &e: arguments) + { + joined += "||" + fvField(e) + "|" + fvValue(e); + } + + recordLine("G|" + sai_serialize_status(status) + "|" + joined); +} + void Recorder::recordGenericGetStats( _In_ sai_object_type_t object_type, _In_ sai_object_id_t object_id, diff --git a/lib/Recorder.h b/lib/Recorder.h index 5e90791c..5b6ef18d 100644 --- a/lib/Recorder.h +++ b/lib/Recorder.h @@ -247,6 +247,14 @@ namespace sairedis _In_ uint32_t objectCount, _In_ const sai_status_t *objectStatuses); + void recordBulkGenericGet( + _In_ const std::string& key, + _In_ const std::vector& arguments); + + void recordBulkGenericGetResponse( + _In_ sai_status_t status, + _In_ const std::vector& arguments); + public: // SAI query interface API void recordFlushFdbEntries( diff --git a/lib/RedisRemoteSaiInterface.cpp b/lib/RedisRemoteSaiInterface.cpp index 46108c73..c72d6c4d 100644 --- a/lib/RedisRemoteSaiInterface.cpp +++ b/lib/RedisRemoteSaiInterface.cpp @@ -14,6 +14,8 @@ #include "meta/PerformanceIntervalTimer.h" #include "meta/Globals.h" +#include "swss/tokenize.h" + #include "config.h" #include @@ -1674,6 +1676,77 @@ sai_status_t RedisRemoteSaiInterface::waitForBulkResponse( return SAI_STATUS_SUCCESS; } +sai_status_t RedisRemoteSaiInterface::waitForBulkGetResponse( + _In_ sai_object_type_t objectType, + _In_ uint32_t object_count, + _In_ const uint32_t *attr_count, + _Inout_ sai_attribute_t **attr_list, + _Out_ sai_status_t *object_statuses) +{ + SWSS_LOG_ENTER(); + + swss::KeyOpFieldsValuesTuple kco; + + const auto status = m_communicationChannel->wait(REDIS_ASIC_STATE_COMMAND_GETRESPONSE, kco); + + const auto &values = kfvFieldsValues(kco); + + if (values.size() != object_count) + { + SWSS_LOG_THROW("wrong number of statuses, got %zu, expected %u", values.size(), object_count); + } + + for (size_t idx = 0; idx < values.size(); idx++) + { + // field = status + // value = attrid=attrvalue|... + + const auto& statusStr = fvField(values[idx]); + const auto& joined = fvValue(values[idx]); + + const auto v = swss::tokenize(joined, '|'); + + std::vector entries; // attributes per object id + entries.reserve(v.size()); + + for (size_t i = 0; i < v.size(); i++) + { + const std::string item = v.at(i); + + auto start = item.find_first_of("="); + + auto field = item.substr(0, start); + auto value = item.substr(start + 1); + + entries.emplace_back(field, value); + } + + // deserialize statuses for all objects + sai_deserialize_status(statusStr, object_statuses[idx]); + + const auto objectStatus = object_statuses[idx]; + + if (objectStatus == SAI_STATUS_SUCCESS || objectStatus == SAI_STATUS_BUFFER_OVERFLOW) + { + const auto countOnly = (objectStatus == SAI_STATUS_BUFFER_OVERFLOW); + + if (values.size() == 0) + { + SWSS_LOG_THROW("logic error, get response returned 0 values!, send api response or sync/async issue?"); + } + + SaiAttributeList list(objectType, entries, countOnly); + + // no need for id fix since this is overflow + transfer_attributes(objectType, attr_count[idx], list.get_attr_list(), attr_list[idx], countOnly); + } + } + + m_recorder->recordBulkGenericGetResponse(status, values); + + return status; +} + sai_status_t RedisRemoteSaiInterface::bulkRemove( _In_ sai_object_type_t object_type, _In_ uint32_t object_count, @@ -1764,9 +1837,67 @@ sai_status_t RedisRemoteSaiInterface::bulkGet( { SWSS_LOG_ENTER(); - SWSS_LOG_ERROR("not implemented, FIXME"); + std::vector serializedObjectIds; + serializedObjectIds.reserve(object_count); - return SAI_STATUS_NOT_IMPLEMENTED; + for (uint32_t idx = 0; idx < object_count; idx++) + { + serializedObjectIds.emplace_back(sai_serialize_object_id(object_id[idx])); + } + + return bulkGet(object_type, serializedObjectIds, attr_count, attr_list, mode, object_statuses); +} + +sai_status_t RedisRemoteSaiInterface::bulkGet( + _In_ sai_object_type_t object_type, + _In_ const std::vector &serialized_object_ids, + _In_ const uint32_t *attr_count, + _Inout_ sai_attribute_t **attr_list, + _In_ sai_bulk_op_error_mode_t mode, + _Inout_ sai_status_t *object_statuses) +{ + SWSS_LOG_ENTER(); + + const auto serializedObjectType = sai_serialize_object_type(object_type); + + std::vector entries; + entries.reserve(serialized_object_ids.size()); + + for (size_t idx = 0; idx < serialized_object_ids.size(); idx++) + { + /* + * Since user may reuse buffers, then oid list buffers maybe not cleared + * and contain some garbage, let's clean them so we send all oids as null to + * syncd. + */ + + Utils::clearOidValues(object_type, attr_count[idx], attr_list[idx]); + + const auto entry = SaiAttributeList::serialize_attr_list(object_type, attr_count[idx], attr_list[idx], false); + + const auto strAttr = Globals::joinFieldValues(entry); + + swss::FieldValueTuple fvt(serialized_object_ids[idx] , strAttr); + + entries.push_back(fvt); + } + + /* + * We are adding number of entries to actually add ':' to be compatible + * with previous + */ + + const auto key = serializedObjectType + ":" + std::to_string(entries.size()); + + m_communicationChannel->set(key, entries, REDIS_ASIC_STATE_COMMAND_BULK_GET); + + m_recorder->recordBulkGenericGet(serializedObjectType, entries); + + const auto object_count = static_cast(serialized_object_ids.size()); + + const auto status = waitForBulkGetResponse(object_type, object_count, attr_count, attr_list, object_statuses); + + return status; } sai_status_t RedisRemoteSaiInterface::bulkCreate( diff --git a/lib/RedisRemoteSaiInterface.h b/lib/RedisRemoteSaiInterface.h index 921b81b5..553c712b 100644 --- a/lib/RedisRemoteSaiInterface.h +++ b/lib/RedisRemoteSaiInterface.h @@ -276,6 +276,14 @@ namespace sairedis _In_ sai_bulk_op_error_mode_t mode, _Out_ sai_status_t *object_statuses); + sai_status_t bulkGet( + _In_ sai_object_type_t object_type, + _In_ const std::vector &serialized_object_ids, + _In_ const uint32_t *attr_count, + _Inout_ sai_attribute_t **attr_list, + _In_ sai_bulk_op_error_mode_t mode, + _Out_ sai_status_t *object_statuses); + private: // QUAD API response /** @@ -317,6 +325,23 @@ namespace sairedis _In_ uint32_t object_count, _Out_ sai_status_t *object_statuses); + /** + * @brief Wait for bulk GET response. + * + * Will wait for response from syncd. Method only used for bulk + * GET object. If object status is SUCCESS all values will be deserialized + * and transferred to user buffers. If object status is BUFFER_OVERFLOW + * then all non list values will be transferred, but LIST objects + * will only transfer COUNT item of list, without touching user + * list at all. + */ + sai_status_t waitForBulkGetResponse( + _In_ sai_object_type_t objectType, + _In_ uint32_t object_count, + _In_ const uint32_t *attr_count, + _Inout_ sai_attribute_t **attr_list, + _Out_ sai_status_t *object_statuses); + private: // stats API response sai_status_t waitForGetStatsResponse( diff --git a/lib/Sai.cpp b/lib/Sai.cpp index d0202821..f815cc97 100644 --- a/lib/Sai.cpp +++ b/lib/Sai.cpp @@ -524,11 +524,19 @@ sai_status_t Sai::bulkGet( _In_ sai_bulk_op_error_mode_t mode, _Out_ sai_status_t *object_statuses) { + MUTEX(); SWSS_LOG_ENTER(); + REDIS_CHECK_API_INITIALIZED(); + REDIS_CHECK_CONTEXT(*object_id); - SWSS_LOG_ERROR("not implemented, FIXME"); - - return SAI_STATUS_NOT_IMPLEMENTED; + return context->m_meta->bulkGet( + object_type, + object_count, + object_id, + attr_count, + attr_list, + mode, + object_statuses); } // BULK QUAD ENTRY diff --git a/lib/ServerSai.cpp b/lib/ServerSai.cpp index faa8de2e..9e1c896b 100644 --- a/lib/ServerSai.cpp +++ b/lib/ServerSai.cpp @@ -452,9 +452,14 @@ sai_status_t ServerSai::bulkGet( SWSS_LOG_ENTER(); REDIS_CHECK_API_INITIALIZED(); - SWSS_LOG_ERROR("not implemented, FIXME"); - - return SAI_STATUS_NOT_IMPLEMENTED; + return m_sai->bulkGet( + object_type, + object_count, + object_id, + attr_count, + attr_list, + mode, + object_statuses); } // BULK QUAD ENTRY diff --git a/meta/Meta.cpp b/meta/Meta.cpp index a19483e0..ee770b06 100644 --- a/meta/Meta.cpp +++ b/meta/Meta.cpp @@ -1270,9 +1270,57 @@ sai_status_t Meta::bulkGet( { SWSS_LOG_ENTER(); - SWSS_LOG_ERROR("not implemented, FIXME"); + PARAMETER_CHECK_IF_NOT_NULL(object_statuses); - return SAI_STATUS_NOT_IMPLEMENTED; + for (uint32_t idx = 0; idx < object_count; idx++) + { + object_statuses[idx] = SAI_STATUS_NOT_EXECUTED; + } + + PARAMETER_CHECK_OBJECT_TYPE_VALID(object_type); + PARAMETER_CHECK_POSITIVE(object_count); + PARAMETER_CHECK_IF_NOT_NULL(attr_count); + PARAMETER_CHECK_IF_NOT_NULL(attr_list); + + if (sai_metadata_get_enum_value_name(&sai_metadata_enum_sai_bulk_op_error_mode_t, mode) == nullptr) + { + SWSS_LOG_ERROR("mode value %d is not in range on %s", mode, sai_metadata_enum_sai_bulk_op_error_mode_t.name); + + return SAI_STATUS_INVALID_PARAMETER; + } + + std::vector vmk; + + for (uint32_t idx = 0; idx < object_count; idx++) + { + sai_status_t status = meta_sai_validate_oid(object_type, &object_id[idx], SAI_NULL_OBJECT_ID, false); + + CHECK_STATUS_SUCCESS(status); + + sai_object_meta_key_t meta_key = { .objecttype = object_type, .objectkey = { .key = { .object_id = object_id[idx] } } }; + + vmk.push_back(meta_key); + + status = meta_generic_validation_get(meta_key, attr_count[idx], attr_list[idx]); + + // FIXME: This macro returns on failure. + // When mode is SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR we should continue instead of return. + // This issue exists for all bulk operations. + CHECK_STATUS_SUCCESS(status); + } + + auto status = m_implementation->bulkGet(object_type, object_count, object_id, attr_count, attr_list, mode, object_statuses); + + for (uint32_t idx = 0; idx < object_count; idx++) + { + if (object_statuses[idx] == SAI_STATUS_SUCCESS) + { + sai_object_id_t switch_id = switchIdQuery(object_id[idx]); + meta_generic_validation_post_get(vmk[idx], switch_id, attr_count[idx], attr_list[idx]); + } + } + + return status; } sai_status_t Meta::bulkCreate( diff --git a/saiplayer/SaiPlayer.cpp b/saiplayer/SaiPlayer.cpp index 700fb25b..4922ea55 100644 --- a/saiplayer/SaiPlayer.cpp +++ b/saiplayer/SaiPlayer.cpp @@ -2354,6 +2354,39 @@ sai_status_t SaiPlayer::handle_bulk_object( } break; + case SAI_COMMON_API_BULK_GET: + + { + std::vector ids(object_count); + + for (uint32_t it = 0; it < object_count; it++) + { + ids[it] = translate_local_to_redis(local_ids[it]); + } + + std::vector attr_counts(object_count); + + std::vector attr_lists(object_count); + + for (uint32_t idx = 0; idx < object_count; idx++) + { + attr_counts[idx] = attributes[idx]->get_attr_count(); + attr_lists[idx] = attributes[idx]->get_attr_list(); + } + + status = m_sai->bulkGet(object_type, + object_count, + ids.data(), + attr_counts.data(), + attr_lists.data(), + mode, + statuses.data()); + + return status; + } + break; + + default: SWSS_LOG_THROW("generic other apis not implemented"); } @@ -2374,6 +2407,7 @@ void SaiPlayer::processBulk( if (api != SAI_COMMON_API_BULK_SET && api != SAI_COMMON_API_BULK_CREATE && + api != SAI_COMMON_API_BULK_GET && api != SAI_COMMON_API_BULK_REMOVE) { SWSS_LOG_THROW("bulk common api %d is not supported yet, FIXME", api); @@ -2398,7 +2432,7 @@ void SaiPlayer::processBulk( std::vector statuses(fields.size()); - // TODO currently we expect all bulk API will always succeed in sync mode + // TODO currently we expect bulk API except BULK_GET will always succeed in sync mode // we will need to update that, needs to be obtained from recording file std::vector expectedStatuses(fields.size(), SAI_STATUS_SUCCESS); @@ -2482,6 +2516,79 @@ void SaiPlayer::processBulk( break; } + if (api == SAI_COMMON_API_BULK_GET) + { + std::string response; + + do + { + // this line may be notification, we need to skip + std::getline(m_infile, response); + } + while (response[response.find_first_of("|") + 1] == 'n'); + + const auto tokens = tokenize(response, "||"); + const auto opAndStatus = tokenize(tokens.at(0), "|"); + + sai_status_t expectedStatus; + sai_deserialize_status(opAndStatus.at(2), expectedStatus); + + if (status != expectedStatus) + { + SWSS_LOG_WARN("status is: %s but expected: %s", + sai_serialize_status(status).c_str(), + sai_serialize_status(expectedStatus).c_str()); + return; + } + + auto valuesIter = tokens.begin() + 1; // Skip operation and status + for (size_t idx = 0; idx < object_ids.size(); idx++, valuesIter++) + { + auto attrValues = tokenize(*valuesIter, "|"); + + sai_status_t expectedObjectStatus; + sai_deserialize_status(attrValues.at(0), expectedObjectStatus); + + if (statuses[idx] != expectedObjectStatus) + { + SWSS_LOG_WARN("object status is: %s but expected: %s", + sai_serialize_status(statuses[idx]).c_str(), + sai_serialize_status(expectedObjectStatus).c_str()); + continue; + } + + std::vector values; + + for (size_t attrIdx = 1; attrIdx < attrValues.size(); attrIdx++) // skip status + { + auto& attrStr = attrValues[attrIdx]; + auto start = attrStr.find_first_of("="); + + auto field = attrStr.substr(0, start); + auto value = attrStr.substr(start + 1); + + swss::FieldValueTuple entry(field, value); + + values.push_back(entry); + } + + SaiAttributeList list(object_type, values, false); + + sai_attribute_t *attr_list = list.get_attr_list(); + uint32_t attr_count = list.get_attr_count(); + + match_list_lengths(object_type, attributes[idx]->get_attr_count(), + attributes[idx]->get_attr_list(), attr_count, attr_list); + + SWSS_LOG_DEBUG("list match"); + + match_redis_with_rec(object_type, attributes[idx]->get_attr_count(), + attributes[idx]->get_attr_list(), attr_count, attr_list); + + // NOTE: Primitive values are not matched (recording vs switch/vs), we can add that check + } + } + if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("handle bulk executed with failure, status = %s", sai_serialize_status(status).c_str()); @@ -2522,9 +2629,9 @@ int SaiPlayer::replay() SWSS_LOG_NOTICE("using file: %s", filename.c_str()); - std::ifstream infile(filename); + m_infile = std::ifstream{filename}; - if (!infile.is_open()) + if (!m_infile.is_open()) { SWSS_LOG_ERROR("failed to open file %s", filename.c_str()); return -1; @@ -2532,7 +2639,7 @@ int SaiPlayer::replay() std::string line; - while (std::getline(infile, line)) + while (std::getline(m_infile, line)) { // std::cout << "processing " << line << std::endl; @@ -2551,7 +2658,7 @@ int SaiPlayer::replay() do { // this line may be notification, we need to skip - if (!std::getline(infile, response)) + if (!std::getline(m_infile, response)) { SWSS_LOG_THROW("failed to read next file from file, previous: %s", line.c_str()); } @@ -2569,7 +2676,7 @@ int SaiPlayer::replay() do { // this line may be notification, we need to skip - if (!std::getline(infile, response)) + if (!std::getline(m_infile, response)) { SWSS_LOG_THROW("failed to read next file from file, previous: %s", line.c_str()); } @@ -2592,6 +2699,9 @@ int SaiPlayer::replay() case 's': api = SAI_COMMON_API_SET; break; + case 'B': + processBulk(SAI_COMMON_API_BULK_GET, line); + continue; case 'S': processBulk(SAI_COMMON_API_BULK_SET, line); continue; @@ -2727,7 +2837,7 @@ int SaiPlayer::replay() do { // this line may be notification, we need to skip - std::getline(infile, response); + std::getline(m_infile, response); } while (response[response.find_first_of("|") + 1] == 'n'); @@ -2772,7 +2882,7 @@ int SaiPlayer::replay() } } - infile.close(); + m_infile.close(); SWSS_LOG_NOTICE("finished replaying %s with SUCCESS", filename.c_str()); diff --git a/saiplayer/SaiPlayer.h b/saiplayer/SaiPlayer.h index b8adc10f..06d5c8ff 100644 --- a/saiplayer/SaiPlayer.h +++ b/saiplayer/SaiPlayer.h @@ -7,6 +7,7 @@ #include "syncd/ServiceMethodTable.h" #include "syncd/SwitchNotifications.h" +#include #include #include @@ -259,6 +260,8 @@ namespace saiplayer std::shared_ptr m_commandLineOptions; + std::ifstream m_infile; + std::map m_local_to_redis; std::map m_redis_to_local; diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index b798c6ee..2dc2b7a5 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -25,6 +25,7 @@ #include "meta/ZeroMQSelectableChannel.h" #include "meta/RedisSelectableChannel.h" #include "meta/PerformanceIntervalTimer.h" +#include "meta/Globals.h" #include "vslib/saivs.h" @@ -381,6 +382,9 @@ sai_status_t Syncd::processSingleEvent( if (op == REDIS_ASIC_STATE_COMMAND_BULK_SET) return processBulkQuadEvent(SAI_COMMON_API_BULK_SET, kco); + if (op == REDIS_ASIC_STATE_COMMAND_BULK_GET) + return processBulkQuadEvent(SAI_COMMON_API_BULK_GET, kco); + if (op == REDIS_ASIC_STATE_COMMAND_NOTIFY) return processNotifySyncd(kco); @@ -995,12 +999,12 @@ sai_status_t Syncd::processBulkQuadEventInInitViewMode( { SWSS_LOG_ENTER(); + const auto objectCount = static_cast(objectIds.size()); + std::vector statuses(objectIds.size()); - for (auto &a: statuses) - { - a = SAI_STATUS_SUCCESS; - } + const sai_status_t initialObjectStatus = api != SAI_COMMON_API_BULK_GET ? SAI_STATUS_SUCCESS : SAI_STATUS_NOT_EXECUTED; + statuses.assign(statuses.size(), initialObjectStatus); auto info = sai_metadata_get_object_type_info(objectType); @@ -1069,7 +1073,47 @@ sai_status_t Syncd::processBulkQuadEventInInitViewMode( return SAI_STATUS_SUCCESS; case SAI_COMMON_API_BULK_GET: - SWSS_LOG_THROW("GET bulk api is not implemented in init view mode, FIXME"); + if (info->isnonobjectid) + { + /* + * Those objects are user created, so if user created ROUTE he + * passed some attributes, there is no sense to support GET + * since user explicitly know what attributes were set, similar + * for other non object id types. + */ + + SWSS_LOG_ERROR("get is not supported on %s in init view mode", sai_serialize_object_type(objectType).c_str()); + + const sai_status_t status = SAI_STATUS_NOT_SUPPORTED; + sendBulkGetResponse(objectType, objectIds, status, attributes, statuses); + + return status; + } + else + { + for (size_t idx = 0; idx < objectCount; idx++) + { + const auto& strObjectId = objectIds[idx]; + + sai_object_id_t objectVid; + sai_deserialize_object_id(strObjectId, objectVid); + + if (isInitViewMode() && m_createdInInitView.find(objectVid) != m_createdInInitView.end()) + { + SWSS_LOG_WARN("GET api can't be used on %s (%s) since it's created in INIT_VIEW mode", + strObjectId.c_str(), + sai_serialize_object_type(objectType).c_str()); + + const sai_status_t status = SAI_STATUS_INVALID_OBJECT_ID; + sendBulkGetResponse(objectType, objectIds, status, attributes, statuses); + + return status; + } + + } + + return processBulkOid(objectType, objectIds, SAI_COMMON_API_BULK_GET, attributes, strAttributes); + } default: @@ -2185,6 +2229,56 @@ sai_status_t Syncd::processBulkOidSet( return status; } +sai_status_t Syncd::processBulkOidGet( + _In_ sai_object_type_t objectType, + _In_ sai_bulk_op_error_mode_t mode, + _In_ const std::vector& objectIds, + _In_ const std::vector>& attributes, + _Out_ std::vector& statuses) +{ + SWSS_LOG_ENTER(); + + const auto object_count = static_cast(objectIds.size()); + + if (!object_count) + { + SWSS_LOG_ERROR("container with objectIds is empty in processBulkOidGet"); + return SAI_STATUS_FAILURE; + } + + std::vector objectVids(object_count); + std::vector objectRids(object_count); + + std::vector attr_counts(object_count); + std::vector attr_lists(object_count); + + for (size_t idx = 0; idx < object_count; idx++) + { + sai_deserialize_object_id(objectIds[idx], objectVids[idx]); + objectRids[idx] = m_translator->translateVidToRid(objectVids[idx]); + + attr_counts[idx] = attributes[idx]->get_attr_count(); + attr_lists[idx] = attributes[idx]->get_attr_list(); + } + + const auto status = m_vendorSai->bulkGet(objectType, + object_count, + objectRids.data(), + attr_counts.data(), + attr_lists.data(), + mode, + statuses.data()); + + if (status == SAI_STATUS_NOT_IMPLEMENTED || status == SAI_STATUS_NOT_SUPPORTED) + { + SWSS_LOG_WARN("bulkGet api is not implemented or not supported, object_type = %s", + sai_serialize_object_type(objectType).c_str()); + return status; + } + + return status; +} + sai_status_t Syncd::processBulkOidRemove( _In_ sai_object_type_t objectType, _In_ sai_bulk_op_error_mode_t mode, @@ -2293,6 +2387,10 @@ sai_status_t Syncd::processBulkOid( all = processBulkOidSet(objectType, mode, objectIds, attributes, statuses); break; + case SAI_COMMON_API_BULK_GET: + all = processBulkOidGet(objectType, mode, objectIds, attributes, statuses); + break; + case SAI_COMMON_API_BULK_REMOVE: all = processBulkOidRemove(objectType, mode, objectIds, statuses); break; @@ -2304,9 +2402,17 @@ sai_status_t Syncd::processBulkOid( if (all != SAI_STATUS_NOT_SUPPORTED && all != SAI_STATUS_NOT_IMPLEMENTED) { - sendApiResponse(api, all, (uint32_t)objectIds.size(), statuses.data()); - syncUpdateRedisBulkQuadEvent(api, statuses, objectType, objectIds, strAttributes); + switch (api) + { + case SAI_COMMON_API_BULK_GET: + sendBulkGetResponse(objectType, objectIds, all, attributes, statuses); + break; + default: + sendApiResponse(api, all, (uint32_t)objectIds.size(), statuses.data()); + break; + } + syncUpdateRedisBulkQuadEvent(api, statuses, objectType, objectIds, strAttributes); return all; } } @@ -2336,6 +2442,10 @@ sai_status_t Syncd::processBulkOid( { status = processOid(objectType, objectIds[idx], SAI_COMMON_API_SET, attr_count, attr_list); } + else if (api == SAI_COMMON_API_BULK_GET) + { + status = processOid(objectType, objectIds[idx], SAI_COMMON_API_GET, attr_count, attr_list); + } else { SWSS_LOG_THROW("api %s is not supported in bulk mode", @@ -2357,7 +2467,15 @@ sai_status_t Syncd::processBulkOid( statuses[idx] = status; } - sendApiResponse(api, all, (uint32_t)objectIds.size(), statuses.data()); + switch (api) + { + case SAI_COMMON_API_BULK_GET: + sendBulkGetResponse(objectType, objectIds, all, attributes, statuses); + break; + default: + sendApiResponse(api, all, (uint32_t)objectIds.size(), statuses.data()); + break; + } syncUpdateRedisBulkQuadEvent(api, statuses, objectType, objectIds, strAttributes); @@ -3067,7 +3185,7 @@ void Syncd::syncUpdateRedisBulkQuadEvent( break; } - case SAI_COMMON_API_GET: + case SAI_COMMON_API_BULK_GET: break; // ignore get since get is not modifying db default: @@ -3680,6 +3798,76 @@ void Syncd::sendGetResponse( SWSS_LOG_INFO("response for GET api was send"); } +void Syncd::sendBulkGetResponse( + _In_ sai_object_type_t objectType, + _In_ const std::vector& strObjectIds, + _In_ sai_status_t status, + _In_ const std::vector>& attributes, + _In_ const std::vector& statuses) +{ + SWSS_LOG_ENTER(); + + std::vector entries; + entries.reserve(strObjectIds.size()); + + for (uint32_t idx = 0; idx < strObjectIds.size(); idx++) + { + const auto objectStatus = statuses[idx]; + const auto objectStatusStr = sai_serialize_status(statuses[idx]); + + if (objectStatus == SAI_STATUS_SUCCESS) + { + sai_object_id_t objectId{}; + sai_deserialize_object_id(strObjectIds[idx], objectId); + const auto switchVid = VidManager::switchIdQuery(objectId); + m_translator->translateRidToVid(objectType, switchVid, attributes[idx]->get_attr_count(), attributes[idx]->get_attr_list()); + + const auto entry = SaiAttributeList::serialize_attr_list(objectType, attributes[idx]->get_attr_count(), attributes[idx]->get_attr_list(), false); + const auto joined = Globals::joinFieldValues(entry); + + // Object IDs are not serialized. The attributes are assumed to be in order the object IDs were passed. + // Essentially, only status and attribute list is needed to be serialized and sent. + swss::FieldValueTuple fvt(objectStatusStr, joined); + + entries.push_back(fvt); + + /* + * All oid values here are VIDs. + */ + + snoopGetResponse(objectType, strObjectIds[idx], attributes[idx]->get_attr_count(), attributes[idx]->get_attr_list()); + } + else if (objectStatus == SAI_STATUS_BUFFER_OVERFLOW) + { + const auto entry = SaiAttributeList::serialize_attr_list(objectType, attributes[idx]->get_attr_count(), attributes[idx]->get_attr_list(), true); + const auto joined = Globals::joinFieldValues(entry); + + swss::FieldValueTuple fvt(objectStatusStr, joined); + + entries.push_back(fvt); + } + else + { + swss::FieldValueTuple fvt(objectStatusStr, Globals::joinFieldValues({})); + + entries.push_back(fvt); + } + } + + for (const auto &e: entries) + { + SWSS_LOG_DEBUG("attr: %s: %s", fvField(e).c_str(), fvValue(e).c_str()); + } + + const auto strStatus = sai_serialize_status(status); + + SWSS_LOG_INFO("sending response for bulk GET api with status: %s", strStatus.c_str()); + + m_selectableChannel->set(strStatus, entries, REDIS_ASIC_STATE_COMMAND_GETRESPONSE); + + SWSS_LOG_INFO("response for bulk GET api was send"); +} + void Syncd::snoopGetResponse( _In_ sai_object_type_t object_type, _In_ const std::string& strObjectId, // can be non object id diff --git a/syncd/Syncd.h b/syncd/Syncd.h index a6a9c7c2..12b98d86 100644 --- a/syncd/Syncd.h +++ b/syncd/Syncd.h @@ -252,6 +252,13 @@ namespace syncd _In_ const std::vector>& attributes, _Out_ std::vector& statuses); + sai_status_t processBulkOidGet( + _In_ sai_object_type_t objectType, + _In_ sai_bulk_op_error_mode_t mode, + _In_ const std::vector& objectIds, + _In_ const std::vector>& attributes, + _Out_ std::vector& statuses); + sai_status_t processBulkOidRemove( _In_ sai_object_type_t objectType, _In_ sai_bulk_op_error_mode_t mode, @@ -368,6 +375,13 @@ namespace syncd _In_ uint32_t attr_count, _In_ sai_attribute_t *attr_list); + void sendBulkGetResponse( + _In_ sai_object_type_t objectType, + _In_ const std::vector& strObjectIds, + _In_ sai_status_t status, + _In_ const std::vector>& attributes, + _In_ const std::vector& statuses); + void sendNotifyResponse( _In_ sai_status_t status); diff --git a/syncd/VendorSai.cpp b/syncd/VendorSai.cpp index fd9f0662..8235ca8e 100644 --- a/syncd/VendorSai.cpp +++ b/syncd/VendorSai.cpp @@ -662,11 +662,35 @@ sai_status_t VendorSai::bulkGet( _In_ sai_bulk_op_error_mode_t mode, _Out_ sai_status_t *object_statuses) { + MUTEX(); SWSS_LOG_ENTER(); + VENDOR_CHECK_API_INITIALIZED(); + + sai_bulk_object_get_attribute_fn ptr; + + switch (object_type) + { + case SAI_OBJECT_TYPE_PORT: + ptr = m_apis.port_api->get_ports_attribute; + break; - SWSS_LOG_ERROR("not implemented, FIXME"); + default: + SWSS_LOG_ERROR("not implemented %s, FIXME", sai_serialize_object_type(object_type).c_str()); + return SAI_STATUS_NOT_IMPLEMENTED; + } - return SAI_STATUS_NOT_IMPLEMENTED; + if (!ptr) + { + SWSS_LOG_INFO("get bulk not supported in SAI, object_type = %s", sai_serialize_object_type(object_type).c_str()); + return SAI_STATUS_NOT_SUPPORTED; + } + + return ptr(object_count, + object_id, + attr_count, + attr_list, + mode, + object_statuses); } // BULK GET diff --git a/syncd/tests/TestSyncdBrcm.cpp b/syncd/tests/TestSyncdBrcm.cpp index 21dd0fc4..b3caefc9 100644 --- a/syncd/tests/TestSyncdBrcm.cpp +++ b/syncd/tests/TestSyncdBrcm.cpp @@ -987,4 +987,321 @@ TEST_F(SyncdBrcmTest, bulkSetInInitViewForUnsupportedObjects) ASSERT_THROW(m_sairedis->bulkSet(SAI_OBJECT_TYPE_SWITCH, 1, oids, attrs, SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR, statuses), std::runtime_error); +} + +struct GetBulk +{ + std::vector oids; + std::vector attr_counts; + std::vector attrs; + std::vector statuses; + + std::vector> attrsContainer; + + void resize(size_t size, uint32_t attr_count) + { + SWSS_LOG_ENTER(); + + oids.resize(size); + attr_counts.resize(size); + attrs.resize(size); + attrsContainer.resize(size); + for (uint32_t i = 0; i < size; i++) + { + attrsContainer[i].resize(attr_count); + attr_counts[i] = attr_count; + } + statuses.resize(size, SAI_STATUS_NOT_EXECUTED); + } +}; + +TEST_F(SyncdBrcmTest, portBulkGet) +{ + sai_object_id_t switchId; + sai_attribute_t attrs[1]; + + GetBulk portGetBulk; + + // init view + + attrs[0].id = SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD; + attrs[0].value.s32 = SAI_REDIS_NOTIFY_SYNCD_INIT_VIEW; + + auto status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + // create switch + + attrs[0].id = SAI_SWITCH_ATTR_INIT_SWITCH; + attrs[0].value.booldata = true; + + status = m_sairedis->create(SAI_OBJECT_TYPE_SWITCH, &switchId, SAI_NULL_OBJECT_ID, 1, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + // Get port number + + attrs[0].id = SAI_SWITCH_ATTR_PORT_NUMBER; + status = m_sairedis->get(SAI_OBJECT_TYPE_SWITCH, switchId, 1, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + const auto portNumber = attrs[0].value.u32; + ASSERT_TRUE(attrs[0].value.u32 > 1); + + portGetBulk.resize(portNumber, 2); + + attrs[0].id = SAI_SWITCH_ATTR_PORT_LIST; + attrs[0].value.objlist.count = static_cast(portGetBulk.oids.size()); + attrs[0].value.objlist.list = portGetBulk.oids.data(); + status = m_sairedis->get(SAI_OBJECT_TYPE_SWITCH, switchId, 1, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + // Get port attributes in bulk + + for (size_t i = 0; i < portGetBulk.oids.size(); i++) + { + portGetBulk.attrsContainer[i][0].id = SAI_PORT_ATTR_ADMIN_STATE; + portGetBulk.attrsContainer[i][1].id = SAI_PORT_ATTR_SPEED; + + portGetBulk.attrs[i] = portGetBulk.attrsContainer[i].data(); + } + + status = m_sairedis->bulkGet(SAI_OBJECT_TYPE_PORT, static_cast(portGetBulk.oids.size()), portGetBulk.oids.data(), + portGetBulk.attr_counts.data(), portGetBulk.attrs.data(), SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR, portGetBulk.statuses.data()); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + for (size_t i = 0; i < portGetBulk.oids.size(); i++) + { + ASSERT_EQ(portGetBulk.statuses[i], SAI_STATUS_SUCCESS); + ASSERT_EQ(portGetBulk.attrs[i][0].value.booldata, false); + ASSERT_EQ(portGetBulk.attrs[i][1].value.u32, 40000); + } +} + +TEST_F(SyncdBrcmTest, portBulkGetInInitView) +{ + sai_object_id_t switchId; + sai_attribute_t attrs[1]; + + GetBulk portGetBulk; + + // init view + + attrs[0].id = SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD; + attrs[0].value.s32 = SAI_REDIS_NOTIFY_SYNCD_INIT_VIEW; + + auto status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + // create switch + + attrs[0].id = SAI_SWITCH_ATTR_INIT_SWITCH; + attrs[0].value.booldata = true; + + status = m_sairedis->create(SAI_OBJECT_TYPE_SWITCH, &switchId, SAI_NULL_OBJECT_ID, 1, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + // apply view + + attrs[0].id = SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD; + attrs[0].value.s32 = SAI_REDIS_NOTIFY_SYNCD_APPLY_VIEW; + + status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + // init view + + attrs[0].id = SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD; + attrs[0].value.s32 = SAI_REDIS_NOTIFY_SYNCD_INIT_VIEW; + + status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + // create switch + + attrs[0].id = SAI_SWITCH_ATTR_INIT_SWITCH; + attrs[0].value.booldata = true; + + status = m_sairedis->create(SAI_OBJECT_TYPE_SWITCH, &switchId, SAI_NULL_OBJECT_ID, 1, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + // Get port number + + attrs[0].id = SAI_SWITCH_ATTR_PORT_NUMBER; + status = m_sairedis->get(SAI_OBJECT_TYPE_SWITCH, switchId, 1, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + const auto portNumber = attrs[0].value.u32; + ASSERT_TRUE(attrs[0].value.u32 > 1); + + portGetBulk.resize(portNumber, 2); + + attrs[0].id = SAI_SWITCH_ATTR_PORT_LIST; + attrs[0].value.objlist.count = static_cast(portGetBulk.oids.size()); + attrs[0].value.objlist.list = portGetBulk.oids.data(); + status = m_sairedis->get(SAI_OBJECT_TYPE_SWITCH, switchId, 1, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + // Get port attributes in bulk + + for (size_t i = 0; i < portGetBulk.oids.size(); i++) + { + portGetBulk.attrsContainer[i][0].id = SAI_PORT_ATTR_ADMIN_STATE; + portGetBulk.attrsContainer[i][1].id = SAI_PORT_ATTR_SPEED; + + portGetBulk.attrs[i] = portGetBulk.attrsContainer[i].data(); + } + + status = m_sairedis->bulkGet(SAI_OBJECT_TYPE_PORT, static_cast(portGetBulk.oids.size()), portGetBulk.oids.data(), + portGetBulk.attr_counts.data(), portGetBulk.attrs.data(), SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR, portGetBulk.statuses.data()); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + for (size_t i = 0; i < portGetBulk.oids.size(); i++) + { + ASSERT_EQ(portGetBulk.statuses[i], SAI_STATUS_SUCCESS); + ASSERT_EQ(portGetBulk.attrs[i][0].value.booldata, false); + ASSERT_EQ(portGetBulk.attrs[i][1].value.u32, 40000); + } +} + +TEST_F(SyncdBrcmTest, portBulkGetObjectList) +{ + sai_object_id_t switchId; + sai_attribute_t attrs[1]; + + GetBulk portGetBulk; + + // init view + + attrs[0].id = SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD; + attrs[0].value.s32 = SAI_REDIS_NOTIFY_SYNCD_INIT_VIEW; + + auto status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + // create switch + + attrs[0].id = SAI_SWITCH_ATTR_INIT_SWITCH; + attrs[0].value.booldata = true; + + status = m_sairedis->create(SAI_OBJECT_TYPE_SWITCH, &switchId, SAI_NULL_OBJECT_ID, 1, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + attrs[0].id = SAI_SWITCH_ATTR_PORT_NUMBER; + status = m_sairedis->get(SAI_OBJECT_TYPE_SWITCH, switchId, 1, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + const auto portNumber = attrs[0].value.u32; + ASSERT_TRUE(attrs[0].value.u32 > 1); + + portGetBulk.resize(portNumber, 2); + + attrs[0].id = SAI_SWITCH_ATTR_PORT_LIST; + attrs[0].value.objlist.count = static_cast(portGetBulk.oids.size()); + attrs[0].value.objlist.list = portGetBulk.oids.data(); + status = m_sairedis->get(SAI_OBJECT_TYPE_SWITCH, switchId, 1, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + // Query number of queues + + attrs[0].id = SAI_PORT_ATTR_QOS_NUMBER_OF_QUEUES; + status = m_sairedis->get(SAI_OBJECT_TYPE_PORT, portGetBulk.oids[0], 1, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + std::vector> queues; + queues.resize(portGetBulk.oids.size()); + + for (size_t i = 0; i < queues.size(); i++) + { + queues[i].resize(attrs[0].value.u32); + } + + // Get port attributes in bulk + + portGetBulk.resize(portNumber, 1); + for (size_t i = 0; i < portGetBulk.oids.size(); i++) + { + // Query QUEUE_LIST to test object list get + portGetBulk.attrsContainer[i][0].id = SAI_PORT_ATTR_QOS_QUEUE_LIST; + portGetBulk.attrsContainer[i][0].value.objlist.count = static_cast(queues[i].size()); + portGetBulk.attrsContainer[i][0].value.objlist.list = queues[i].data(); + + portGetBulk.attrs[i] = portGetBulk.attrsContainer[i].data(); + } + + status = m_sairedis->bulkGet(SAI_OBJECT_TYPE_PORT, static_cast(portGetBulk.oids.size()), portGetBulk.oids.data(), + portGetBulk.attr_counts.data(), portGetBulk.attrs.data(), SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR, portGetBulk.statuses.data()); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + for (size_t i = 0; i < portGetBulk.oids.size(); i++) + { + ASSERT_EQ(portGetBulk.statuses[i], SAI_STATUS_SUCCESS); + } +} + +TEST_F(SyncdBrcmTest, portBulkGetObjectListInsufficientBuffer) +{ + sai_object_id_t switchId; + sai_attribute_t attrs[1]; + + GetBulk portGetBulk; + + // init view + + attrs[0].id = SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD; + attrs[0].value.s32 = SAI_REDIS_NOTIFY_SYNCD_INIT_VIEW; + + auto status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + // create switch + + attrs[0].id = SAI_SWITCH_ATTR_INIT_SWITCH; + attrs[0].value.booldata = true; + + status = m_sairedis->create(SAI_OBJECT_TYPE_SWITCH, &switchId, SAI_NULL_OBJECT_ID, 1, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + attrs[0].id = SAI_SWITCH_ATTR_PORT_NUMBER; + status = m_sairedis->get(SAI_OBJECT_TYPE_SWITCH, switchId, 1, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + const auto portNumber = attrs[0].value.u32; + ASSERT_TRUE(attrs[0].value.u32 > 1); + + portGetBulk.resize(portNumber, 1); + + attrs[0].id = SAI_SWITCH_ATTR_PORT_LIST; + attrs[0].value.objlist.count = static_cast(portGetBulk.oids.size()); + attrs[0].value.objlist.list = portGetBulk.oids.data(); + status = m_sairedis->get(SAI_OBJECT_TYPE_SWITCH, switchId, 1, attrs); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + std::vector> queues; + queues.resize(portGetBulk.oids.size()); + + for (size_t i = 0; i < queues.size(); i++) + { + queues[i].resize(1); // Insufficient size + } + + // Get port attributes in bulk + + portGetBulk.resize(portNumber, 1); + for (size_t i = 0; i < portGetBulk.oids.size(); i++) + { + // Query QUEUE_LIST to test object list get + portGetBulk.attrsContainer[i][0].id = SAI_PORT_ATTR_QOS_QUEUE_LIST; + portGetBulk.attrsContainer[i][0].value.objlist.count = static_cast(queues[i].size()); + portGetBulk.attrsContainer[i][0].value.objlist.list = queues[i].data(); + + portGetBulk.attrs[i] = portGetBulk.attrsContainer[i].data(); + } + + m_sairedis->bulkGet(SAI_OBJECT_TYPE_PORT, static_cast(portGetBulk.oids.size()), portGetBulk.oids.data(), + portGetBulk.attr_counts.data(), portGetBulk.attrs.data(), SAI_BULK_OP_ERROR_MODE_IGNORE_ERROR, portGetBulk.statuses.data()); + + for (size_t i = 0; i < portGetBulk.oids.size(); i++) + { + ASSERT_EQ(portGetBulk.statuses[i], SAI_STATUS_BUFFER_OVERFLOW); + } } \ No newline at end of file diff --git a/tests/BCM56850.pl b/tests/BCM56850.pl index 6b9c66c8..56ee5301 100755 --- a/tests/BCM56850.pl +++ b/tests/BCM56850.pl @@ -869,8 +869,16 @@ sub test_neighbor_next_hop } } +sub test_port_bulk_get +{ + fresh_start; + + play "port_bulk_get.rec"; +} + # RUN TESTS +test_port_bulk_get test_neighbor_next_hop; test_acl_pre_match_999; test_relaxed; diff --git a/tests/BCM56850/port_bulk_get.rec b/tests/BCM56850/port_bulk_get.rec new file mode 100644 index 00000000..a3fd886b --- /dev/null +++ b/tests/BCM56850/port_bulk_get.rec @@ -0,0 +1,9 @@ +2021-08-17.04:19:21.957051|a|INIT_VIEW +2021-08-17.04:19:28.152399|A|SAI_STATUS_SUCCESS +2021-08-17.04:19:28.154608|c|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_INIT_SWITCH=true|SAI_SWITCH_ATTR_SRC_MAC_ADDRESS=18:17:25:55:17:67 +2021-08-17.04:19:28.171193|g|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_SWITCH_ATTR_PORT_LIST=32:oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0 +2021-08-17.04:19:28.175104|G|SAI_STATUS_SUCCESS|SAI_SWITCH_ATTR_PORT_LIST=32:oid:0x1000000000124,oid:0x1000000000125,oid:0x1000000000126,oid:0x1000000000127,oid:0x1000000000128,oid:0x1000000000129,oid:0x100000000012a,oid:0x100000000012b,oid:0x100000000012c,oid:0x100000000012d,oid:0x100000000012e,oid:0x100000000012f,oid:0x1000000000130,oid:0x1000000000131,oid:0x1000000000132,oid:0x1000000000133,oid:0x1000000000134,oid:0x1000000000135,oid:0x1000000000136,oid:0x1000000000137,oid:0x1000000000138,oid:0x1000000000139,oid:0x100000000013a,oid:0x100000000013b,oid:0x100000000013c,oid:0x100000000013d,oid:0x100000000013e,oid:0x100000000013f,oid:0x1000000000140,oid:0x1000000000141,oid:0x1000000000142,oid:0x1000000000143 +2021-08-17.04:19:28.175104|B|SAI_OBJECT_TYPE_PORT||oid:0x1000000000124|SAI_PORT_ATTR_ADMIN_STATE=false|SAI_PORT_ATTR_SPEED=0|SAI_PORT_ATTR_QOS_QUEUE_LIST=20:oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0||oid:0x1000000000125|SAI_PORT_ATTR_ADMIN_STATE=false|SAI_PORT_ATTR_SPEED=0|SAI_PORT_ATTR_QOS_QUEUE_LIST=20:oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0,oid:0x0 +2021-08-17.04:19:28.175104|G|SAI_STATUS_SUCCESS||SAI_STATUS_SUCCESS|SAI_PORT_ATTR_ADMIN_STATE=true|SAI_PORT_ATTR_SPEED=100000|SAI_PORT_ATTR_QOS_QUEUE_LIST=20:oid:0x15000000000025,oid:0x15000000000026,oid:0x15000000000027,oid:0x15000000000028,oid:0x15000000000029,oid:0x1500000000002a,oid:0x1500000000002b,oid:0x1500000000002c,oid:0x1500000000002d,oid:0x1500000000002e,oid:0x1500000000002f,oid:0x15000000000030,oid:0x15000000000031,oid:0x15000000000032,oid:0x15000000000033,oid:0x15000000000034,oid:0x15000000000035,oid:0x15000000000036,oid:0x15000000000037,oid:0x15000000000038||SAI_STATUS_SUCCESS|SAI_PORT_ATTR_ADMIN_STATE=true|SAI_PORT_ATTR_SPEED=100000|SAI_PORT_ATTR_QOS_QUEUE_LIST=20:oid:0x15000000000039,oid:0x1500000000003a,oid:0x1500000000003b,oid:0x1500000000003c,oid:0x1500000000003d,oid:0x1500000000003e,oid:0x1500000000003f,oid:0x15000000000040,oid:0x15000000000041,oid:0x15000000000042,oid:0x15000000000043,oid:0x15000000000044,oid:0x15000000000045,oid:0x15000000000046,oid:0x15000000000047,oid:0x15000000000048,oid:0x15000000000049,oid:0x1500000000004a,oid:0x1500000000004b,oid:0x1500000000004c +2021-08-17.04:19:31.415208|a|APPLY_VIEW +2021-08-17.04:19:32.331089|A|SAI_STATUS_SUCCESS diff --git a/unittest/lib/TestClientServerSai.cpp b/unittest/lib/TestClientServerSai.cpp index e442dc22..baebec2e 100644 --- a/unittest/lib/TestClientServerSai.cpp +++ b/unittest/lib/TestClientServerSai.cpp @@ -241,7 +241,7 @@ TEST(ClientServerSai, bulkGet) sai_attribute_t* attrs[1] = {0}; sai_status_t statuses[1] = {0}; - EXPECT_EQ(SAI_STATUS_NOT_IMPLEMENTED, + EXPECT_NE(SAI_STATUS_SUCCESS, sai.bulkGet( SAI_OBJECT_TYPE_PORT, 1, diff --git a/unittest/lib/TestRedisRemoteSaiInterface.cpp b/unittest/lib/TestRedisRemoteSaiInterface.cpp index 1cfd2e1e..b9f7df02 100644 --- a/unittest/lib/TestRedisRemoteSaiInterface.cpp +++ b/unittest/lib/TestRedisRemoteSaiInterface.cpp @@ -5,29 +5,6 @@ using namespace sairedis; -TEST(RedisRemoteSaiInterface, bulkGet) -{ - auto ctx = ContextConfigContainer::loadFromFile("foo"); - auto rec = std::make_shared(); - - RedisRemoteSaiInterface sai(ctx->get(0), nullptr, rec); - - sai_object_id_t oids[1] = {0}; - uint32_t attrcount[1] = {0}; - sai_attribute_t* attrs[1] = {0}; - sai_status_t statuses[1] = {0}; - - EXPECT_EQ(SAI_STATUS_NOT_IMPLEMENTED, - sai.bulkGet( - SAI_OBJECT_TYPE_PORT, - 1, - oids, - attrcount, - attrs, - SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR, - statuses)); -} - TEST(RedisRemoteSaiInterface, queryStatsCapabilityNegative) { auto ctx = ContextConfigContainer::loadFromFile("foo"); @@ -40,4 +17,3 @@ TEST(RedisRemoteSaiInterface, queryStatsCapabilityNegative) SAI_OBJECT_TYPE_NULL, 0)); } - diff --git a/unittest/lib/TestSai.cpp b/unittest/lib/TestSai.cpp index 9d3e1179..89948e9a 100644 --- a/unittest/lib/TestSai.cpp +++ b/unittest/lib/TestSai.cpp @@ -49,7 +49,7 @@ TEST(Sai, bulkGet) sai_attribute_t* attrs[1] = {0}; sai_status_t statuses[1] = {0}; - EXPECT_EQ(SAI_STATUS_NOT_IMPLEMENTED, + EXPECT_NE(SAI_STATUS_SUCCESS, sai.bulkGet( SAI_OBJECT_TYPE_PORT, 1, diff --git a/unittest/lib/TestServerSai.cpp b/unittest/lib/TestServerSai.cpp index 2c6fe89a..9607b7cc 100644 --- a/unittest/lib/TestServerSai.cpp +++ b/unittest/lib/TestServerSai.cpp @@ -55,7 +55,7 @@ TEST(ServerSai, bulkGet) sai_attribute_t* attrs[1] = {0}; sai_status_t statuses[1] = {0}; - EXPECT_EQ(SAI_STATUS_NOT_IMPLEMENTED, + EXPECT_NE(SAI_STATUS_SUCCESS, sai.bulkGet( SAI_OBJECT_TYPE_PORT, 1, diff --git a/unittest/meta/TestMeta.cpp b/unittest/meta/TestMeta.cpp index db9e650d..6034ea17 100644 --- a/unittest/meta/TestMeta.cpp +++ b/unittest/meta/TestMeta.cpp @@ -1843,7 +1843,7 @@ TEST(Meta, bulkGet) sai_attribute_t* attrs[1] = {0}; sai_status_t statuses[1] = {0}; - EXPECT_EQ(SAI_STATUS_NOT_IMPLEMENTED, + EXPECT_EQ(SAI_STATUS_INVALID_PARAMETER, sai.bulkGet( SAI_OBJECT_TYPE_PORT, 1, diff --git a/unittest/syncd/TestVendorSai.cpp b/unittest/syncd/TestVendorSai.cpp index 241ad723..b610286d 100644 --- a/unittest/syncd/TestVendorSai.cpp +++ b/unittest/syncd/TestVendorSai.cpp @@ -1453,13 +1453,14 @@ TEST(VendorSai, bulk_dash_outbound_ca_to_pa_entry) TEST(VendorSai, bulkGet) { VendorSai sai; + sai.apiInitialize(0, &test_services); sai_object_id_t oids[1] = {0}; uint32_t attrcount[1] = {0}; sai_attribute_t* attrs[1] = {0}; sai_status_t statuses[1] = {0}; - EXPECT_EQ(SAI_STATUS_NOT_IMPLEMENTED, + EXPECT_EQ(SAI_STATUS_INVALID_PARAMETER, sai.bulkGet( SAI_OBJECT_TYPE_PORT, 1, diff --git a/unittest/vslib/TestSai.cpp b/unittest/vslib/TestSai.cpp index c45663de..dff513c2 100644 --- a/unittest/vslib/TestSai.cpp +++ b/unittest/vslib/TestSai.cpp @@ -1,4 +1,5 @@ #include "Sai.h" +#include "saivs.h" #include @@ -6,23 +7,81 @@ using namespace saivs; +static const char* profile_get_value( + _In_ sai_switch_profile_id_t profile_id, + _In_ const char* variable) +{ + SWSS_LOG_ENTER(); + + if (variable == NULL) + return NULL; + + if (strcmp(variable, SAI_KEY_VS_SWITCH_TYPE) == 0) + return SAI_VALUE_VS_SWITCH_TYPE_BCM56850; + + return nullptr; +} + +static int profile_get_next_value( + _In_ sai_switch_profile_id_t profile_id, + _Out_ const char** variable, + _Out_ const char** value) +{ + SWSS_LOG_ENTER(); + + return 0; +} + +static sai_service_method_table_t test_services = { + profile_get_value, + profile_get_next_value +}; + + TEST(Sai, bulkGet) { Sai sai; - sai_object_id_t oids[1] = {0}; - uint32_t attrcount[1] = {0}; - sai_attribute_t* attrs[1] = {0}; - sai_status_t statuses[1] = {0}; + sai.apiInitialize(0, &test_services); - EXPECT_EQ(SAI_STATUS_NOT_IMPLEMENTED, + sai_attribute_t attr; + + sai_object_id_t switch_id; + + attr.id = SAI_SWITCH_ATTR_INIT_SWITCH; + attr.value.booldata = true; + + EXPECT_EQ(sai.create(SAI_OBJECT_TYPE_SWITCH, &switch_id, SAI_NULL_OBJECT_ID, 1, &attr), SAI_STATUS_SUCCESS); + + attr.id = SAI_SWITCH_ATTR_PORT_NUMBER; + EXPECT_EQ(sai.get(SAI_OBJECT_TYPE_SWITCH, switch_id, 1, &attr), SAI_STATUS_SUCCESS); + + auto portNum = attr.value.u32; + + std::vector oids(portNum); + + attr.id = SAI_SWITCH_ATTR_PORT_LIST; + attr.value.objlist.count = portNum; + attr.value.objlist.list = oids.data(); + EXPECT_EQ(sai.get(SAI_OBJECT_TYPE_SWITCH, switch_id, 1, &attr), SAI_STATUS_SUCCESS); + + std::vector attrs(portNum); + std::vector attrCounts(portNum, 1); + std::vector statuses(portNum); + std::vector pattrs(portNum); + for (size_t i = 0; i < portNum; i++) + { + attrs[i].id = SAI_PORT_ATTR_ADMIN_STATE; + pattrs[i] = &attrs[i]; + } + + EXPECT_EQ(SAI_STATUS_SUCCESS, sai.bulkGet( SAI_OBJECT_TYPE_PORT, - 1, - oids, - attrcount, - attrs, + portNum, + oids.data(), + attrCounts.data(), + pattrs.data(), SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR, - statuses)); -} - + statuses.data())); +} \ No newline at end of file diff --git a/unittest/vslib/TestVirtualSwitchSaiInterface.cpp b/unittest/vslib/TestVirtualSwitchSaiInterface.cpp index ecb08b23..f9c57453 100644 --- a/unittest/vslib/TestVirtualSwitchSaiInterface.cpp +++ b/unittest/vslib/TestVirtualSwitchSaiInterface.cpp @@ -124,20 +124,39 @@ TEST_F(VirtualSwitchSaiInterfaceTest, queryApiVersion) TEST_F(VirtualSwitchSaiInterfaceTest, bulkGet) { - sai_object_id_t oids[1] = {0}; - uint32_t attrcount[1] = {0}; - sai_attribute_t* attrs[1] = {0}; - sai_status_t statuses[1] = {0}; + sai_attribute_t attr; + + attr.id = SAI_SWITCH_ATTR_PORT_NUMBER; + EXPECT_EQ(m_vssai->get(SAI_OBJECT_TYPE_SWITCH, m_swid, 1, &attr), SAI_STATUS_SUCCESS); + + auto portNum = attr.value.u32; + + std::vector oids(portNum); + + attr.id = SAI_SWITCH_ATTR_PORT_LIST; + attr.value.objlist.count = portNum; + attr.value.objlist.list = oids.data(); + EXPECT_EQ(m_vssai->get(SAI_OBJECT_TYPE_SWITCH, m_swid, 1, &attr), SAI_STATUS_SUCCESS); - EXPECT_EQ(SAI_STATUS_NOT_IMPLEMENTED, + std::vector attrs(portNum); + std::vector attrCounts(portNum, 1); + std::vector statuses(portNum); + std::vector pattrs(portNum); + for (size_t i = 0; i < portNum; i++) + { + attrs[i].id = SAI_PORT_ATTR_ADMIN_STATE; + pattrs[i] = &attrs[i]; + } + + EXPECT_EQ(SAI_STATUS_SUCCESS, m_vssai->bulkGet( SAI_OBJECT_TYPE_PORT, - 1, - oids, - attrcount, - attrs, + portNum, + oids.data(), + attrCounts.data(), + pattrs.data(), SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR, - statuses)); + statuses.data())); } TEST_F(VirtualSwitchSaiInterfaceTest, queryStatsCapability) diff --git a/vslib/Sai.cpp b/vslib/Sai.cpp index 97238f9d..fd29689c 100644 --- a/vslib/Sai.cpp +++ b/vslib/Sai.cpp @@ -630,11 +630,18 @@ sai_status_t Sai::bulkGet( _In_ sai_bulk_op_error_mode_t mode, _Out_ sai_status_t *object_statuses) { + MUTEX(); SWSS_LOG_ENTER(); + VS_CHECK_API_INITIALIZED(); - SWSS_LOG_ERROR("not implemented, FIXME"); - - return SAI_STATUS_NOT_IMPLEMENTED; + return m_meta->bulkGet( + object_type, + object_count, + object_id, + attr_count, + attr_list, + mode, + object_statuses); } // BULK QUAD ENTRY diff --git a/vslib/SwitchStateBase.cpp b/vslib/SwitchStateBase.cpp index 7aeacf04..6e61ca7c 100644 --- a/vslib/SwitchStateBase.cpp +++ b/vslib/SwitchStateBase.cpp @@ -847,6 +847,60 @@ sai_status_t SwitchStateBase::bulkSet( return status; } +sai_status_t SwitchStateBase::bulkGet( + _In_ sai_object_type_t object_type, + _In_ const std::vector &serialized_object_ids, + _In_ const uint32_t *attr_count, + _Inout_ sai_attribute_t **attr_list, + _In_ sai_bulk_op_error_mode_t mode, + _Out_ sai_status_t *object_statuses) +{ + SWSS_LOG_ENTER(); + + uint32_t it; + uint32_t object_count = (uint32_t) serialized_object_ids.size(); + sai_status_t status = SAI_STATUS_SUCCESS; + + if (!object_count || !attr_list || !attr_count || !object_statuses) + { + SWSS_LOG_ERROR("Invalid arguments"); + return SAI_STATUS_FAILURE; + } + + for (it = 0; it < object_count; it++) + { + if (!attr_list[it] || !attr_count[it]) + { + SWSS_LOG_ERROR("Invalid arguments"); + return SAI_STATUS_FAILURE; + } + } + + for (it = 0; it < object_count; it++) + { + object_statuses[it] = get(object_type, serialized_object_ids[it], attr_count[it], attr_list[it]); + + if (object_statuses[it] != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to get attribute for object with type = %u", object_type); + + status = SAI_STATUS_FAILURE; + + if (mode == SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR) + { + break; + } + } + } + + while (++it < object_count) + { + object_statuses[it] = SAI_STATUS_NOT_EXECUTED; + } + + return status; +} + int SwitchStateBase::get_default_gw_mac_address( _Out_ sai_mac_t& mac) { diff --git a/vslib/SwitchStateBase.h b/vslib/SwitchStateBase.h index 77c46d1d..4b4e4e7b 100644 --- a/vslib/SwitchStateBase.h +++ b/vslib/SwitchStateBase.h @@ -294,6 +294,14 @@ namespace saivs _In_ sai_bulk_op_error_mode_t mode, _Out_ sai_status_t *object_statuses); + virtual sai_status_t bulkGet( + _In_ sai_object_type_t object_type, + _In_ const std::vector &serialized_object_ids, + _In_ const uint32_t *attr_count, + _Inout_ sai_attribute_t **attr_list, + _In_ sai_bulk_op_error_mode_t mode, + _Out_ sai_status_t *object_statuses); + virtual sai_status_t queryAttrEnumValuesCapability( _In_ sai_object_id_t switch_id, _In_ sai_object_type_t object_type, diff --git a/vslib/VirtualSwitchSaiInterface.cpp b/vslib/VirtualSwitchSaiInterface.cpp index c2e6b6a1..5385d9d4 100644 --- a/vslib/VirtualSwitchSaiInterface.cpp +++ b/vslib/VirtualSwitchSaiInterface.cpp @@ -1268,9 +1268,33 @@ sai_status_t VirtualSwitchSaiInterface::bulkGet( { SWSS_LOG_ENTER(); - SWSS_LOG_ERROR("not implemented, FIXME"); + std::vector serializedObjectIds; - return SAI_STATUS_NOT_IMPLEMENTED; + for (uint32_t idx = 0; idx < object_count; idx++) + { + serializedObjectIds.emplace_back(sai_serialize_object_id(object_id[idx])); + } + + // Get switch ID from the first object ID, assuming all objects are within the same switch. + auto switchId = switchIdQuery(*object_id); + + return bulkGet(switchId, object_type, serializedObjectIds, attr_count, attr_list, mode, object_statuses); +} + +sai_status_t VirtualSwitchSaiInterface::bulkGet( + _In_ sai_object_id_t switchId, + _In_ sai_object_type_t object_type, + _In_ const std::vector &serialized_object_ids, + _In_ const uint32_t *attr_count, + _Inout_ sai_attribute_t **attr_list, + _In_ sai_bulk_op_error_mode_t mode, + _Out_ sai_status_t *object_statuses) +{ + SWSS_LOG_ENTER(); + + auto ss = m_switchStateMap.at(switchId); + + return ss->bulkGet(object_type, serialized_object_ids, attr_count, attr_list, mode, object_statuses); } sai_status_t VirtualSwitchSaiInterface::bulkCreate( diff --git a/vslib/VirtualSwitchSaiInterface.h b/vslib/VirtualSwitchSaiInterface.h index 25855fde..ec6052b7 100644 --- a/vslib/VirtualSwitchSaiInterface.h +++ b/vslib/VirtualSwitchSaiInterface.h @@ -245,6 +245,15 @@ namespace saivs _In_ sai_bulk_op_error_mode_t mode, _Out_ sai_status_t *object_statuses); + sai_status_t bulkGet( + _In_ sai_object_id_t switchId, + _In_ sai_object_type_t object_type, + _In_ const std::vector &serialized_object_ids, + _In_ const uint32_t *attr_count, + _Inout_ sai_attribute_t **attr_list, + _In_ sai_bulk_op_error_mode_t mode, + _Out_ sai_status_t *object_statuses); + private: // QUAD pre sai_status_t preSet(