From 3c485e519db3ba96f4d4c0401ddfdfdf35767682 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak <38952541+stepanblyschak@users.noreply.github.com> Date: Wed, 30 Jun 2021 19:16:05 +0300 Subject: [PATCH] [recorder] Fix incorrect attribute enum value capability query (#843) Recorder was assuming that enum value capability query is executed for an attribute that has a value type of a s32 list. When querying SAI_DEBUG_COUNTER_ATTR_TYPE using sai_query_attribute_enum_values_capability a warning is printed in syslog: "enum value 4 not found in enum sai_debug_counter_type". This happens because SAI_DEBUG_COUNTER_ATTR_TYPE attrvaluetype is int32 (enum value) and sai_s32_list_t structure was casted to int32. Since we initialize sai_s32_list with .count = 4 (the number of values in sai_debug_counter_type_t enum) value 4 is printed in the syslog. --- lib/src/Recorder.cpp | 35 +++++----- lib/src/tests.cpp | 148 +++++++++++++++++++++++++++++++++++++++++++ meta/sai_serialize.h | 5 ++ 3 files changed, 171 insertions(+), 17 deletions(-) diff --git a/lib/src/Recorder.cpp b/lib/src/Recorder.cpp index 24f7b4ca4e30..5469492c9622 100644 --- a/lib/src/Recorder.cpp +++ b/lib/src/Recorder.cpp @@ -1032,6 +1032,8 @@ void Recorder::recordQueryAattributeEnumValuesCapability( { SWSS_LOG_ENTER(); + std::vector values; + auto meta = sai_metadata_get_attr_metadata(objectType, attrId); if (meta == NULL) @@ -1041,14 +1043,18 @@ void Recorder::recordQueryAattributeEnumValuesCapability( return; } - auto key = sai_serialize_object_type(SAI_OBJECT_TYPE_SWITCH) + ":" + sai_serialize_object_id(switchId); + if (!meta->enummetadata) + { + SWSS_LOG_ERROR("Attribute %s is not enum/enumlist!", meta->attridname); + return; + } - sai_attribute_t attr; + auto key = sai_serialize_object_type(SAI_OBJECT_TYPE_SWITCH) + ":" + sai_serialize_object_id(switchId); - attr.id = attrId; - attr.value.s32list = *enumValuesCapability; + auto str_attr_id = sai_serialize_attr_id(*meta); + auto str_enum_list = sai_serialize_enum_list(*enumValuesCapability, meta->enummetadata, true); // we only need to serialize count - auto values = SaiAttributeList::serialize_attr_list(objectType, 1, &attr, true); // we only need to serialize count + values.emplace_back(str_attr_id, str_enum_list); SWSS_LOG_DEBUG("Query arguments: switch %s, attribute: %s, count: %u", key.c_str(), meta->attridname, enumValuesCapability->count); @@ -1063,6 +1069,8 @@ void Recorder::recordQueryAattributeEnumValuesCapabilityResponse( { SWSS_LOG_ENTER(); + std::vector values; + auto meta = sai_metadata_get_attr_metadata(objectType, attrId); if (meta == NULL) @@ -1078,20 +1086,13 @@ void Recorder::recordQueryAattributeEnumValuesCapabilityResponse( return; } - std::vector values; - - sai_attribute_t attr; - - attr.id = attrId; - attr.value.s32list = *enumValuesCapability; + bool countOnly = (status == SAI_STATUS_BUFFER_OVERFLOW); - if (status == SAI_STATUS_SUCCESS) - { - values = SaiAttributeList::serialize_attr_list(objectType, 1, &attr, false); - } - else if (status == SAI_STATUS_BUFFER_OVERFLOW) + if (status == SAI_STATUS_SUCCESS || status == SAI_STATUS_BUFFER_OVERFLOW) { - values = SaiAttributeList::serialize_attr_list(objectType, 1, &attr, true); + auto str_attr_id = sai_serialize_attr_id(*meta); + auto str_enum_list = sai_serialize_enum_list(*enumValuesCapability, meta->enummetadata, countOnly); + values.emplace_back(str_attr_id, str_enum_list); } recordQueryAttributeEnumValuesCapabilityResponse(status, values); diff --git a/lib/src/tests.cpp b/lib/src/tests.cpp index 3af53aa7b02e..c3a8c494d14a 100644 --- a/lib/src/tests.cpp +++ b/lib/src/tests.cpp @@ -8,6 +8,8 @@ extern "C" { #include "swss/table.h" #include "swss/tokenize.h" +#include "lib/inc/Recorder.h" + #include "meta/sai_serialize.h" #include "meta/SaiAttributeList.h" @@ -17,9 +19,13 @@ extern "C" { #include #include +#define ASSERT_EQ(a,b) if ((a) != (b)) { SWSS_LOG_THROW("ASSERT EQ FAILED: " #a " != " #b); } + using namespace saimeta; using namespace sairedis; +const std::string SairedisRecFilename = "sairedis.rec"; + sai_object_type_t sai_object_type_query( _In_ sai_object_id_t objectId) { @@ -761,6 +767,144 @@ static sai_object_type_t deserialize_object_type( return object_type; } +static std::vector parseFirstRecordedAPI() +{ + SWSS_LOG_ENTER(); + + const auto delimiter = '|'; + std::ifstream infile(SairedisRecFilename); + std::string line; + // skip first line + std::getline(infile, line); + std::getline(infile, line); + + std::vector tokens; + std::stringstream sstream(line); + std::string token; + // skip first, it is a timestamp + std::getline(sstream, token, delimiter); + while(std::getline(sstream, token, delimiter)) { + tokens.push_back(token); + } + return tokens; +} + +static void test_recorder_enum_value_capability_query_request( + sai_object_id_t switch_id, + sai_object_type_t object_type, + sai_attr_id_t attr_id, + const std::vector& expectedOutput) +{ + SWSS_LOG_ENTER(); + + remove(SairedisRecFilename.c_str()); + + Recorder recorder; + recorder.enableRecording(true); + + sai_s32_list_t enum_values_capability {.count = 0, .list = nullptr}; + + recorder.recordQueryAattributeEnumValuesCapability( + switch_id, + object_type, + attr_id, + &enum_values_capability + ); + + auto tokens = parseFirstRecordedAPI(); + ASSERT_EQ(tokens, expectedOutput); +} + +static void test_recorder_enum_value_capability_query_response( + sai_status_t status, + sai_object_type_t object_type, + sai_attr_id_t attr_id, + std::vector enumList, + const std::vector& expectedOutput) +{ + SWSS_LOG_ENTER(); + + remove(SairedisRecFilename.c_str()); + + Recorder recorder; + recorder.enableRecording(true); + + sai_s32_list_t enum_values_capability; + enum_values_capability.count = static_cast(enumList.size()); + enum_values_capability.list = enumList.data(); + + recorder.recordQueryAattributeEnumValuesCapabilityResponse( + status, + object_type, + attr_id, + &enum_values_capability + ); + + auto tokens = parseFirstRecordedAPI(); + ASSERT_EQ(tokens, expectedOutput); +} + +static void test_recorder_enum_value_capability_query() +{ + SWSS_LOG_ENTER(); + + test_recorder_enum_value_capability_query_request( + 1, + SAI_OBJECT_TYPE_DEBUG_COUNTER, + SAI_DEBUG_COUNTER_ATTR_TYPE, + { + "q", + "attribute_enum_values_capability", + "SAI_OBJECT_TYPE_SWITCH:oid:0x1", + "SAI_DEBUG_COUNTER_ATTR_TYPE=0", + } + ); + test_recorder_enum_value_capability_query_response( + SAI_STATUS_SUCCESS, + SAI_OBJECT_TYPE_DEBUG_COUNTER, + SAI_DEBUG_COUNTER_ATTR_TYPE, + { + SAI_DEBUG_COUNTER_TYPE_PORT_IN_DROP_REASONS, + SAI_DEBUG_COUNTER_TYPE_PORT_OUT_DROP_REASONS, + SAI_DEBUG_COUNTER_TYPE_SWITCH_IN_DROP_REASONS, + SAI_DEBUG_COUNTER_TYPE_SWITCH_OUT_DROP_REASONS, + }, + { + "Q", + "attribute_enum_values_capability", + "SAI_STATUS_SUCCESS", + "SAI_DEBUG_COUNTER_ATTR_TYPE=4:SAI_DEBUG_COUNTER_TYPE_PORT_IN_DROP_REASONS,SAI_DEBUG_COUNTER_TYPE_PORT_OUT_DROP_REASONS," + "SAI_DEBUG_COUNTER_TYPE_SWITCH_IN_DROP_REASONS,SAI_DEBUG_COUNTER_TYPE_SWITCH_OUT_DROP_REASONS", + } + ); + test_recorder_enum_value_capability_query_request( + 1, + SAI_OBJECT_TYPE_DEBUG_COUNTER, + SAI_DEBUG_COUNTER_ATTR_IN_DROP_REASON_LIST, + { + "q", + "attribute_enum_values_capability", + "SAI_OBJECT_TYPE_SWITCH:oid:0x1", + "SAI_DEBUG_COUNTER_ATTR_IN_DROP_REASON_LIST=0", + } + ); + test_recorder_enum_value_capability_query_response( + SAI_STATUS_SUCCESS, + SAI_OBJECT_TYPE_DEBUG_COUNTER, + SAI_DEBUG_COUNTER_ATTR_IN_DROP_REASON_LIST, + { + SAI_IN_DROP_REASON_L2_ANY, + SAI_IN_DROP_REASON_L3_ANY + }, + { + "Q", + "attribute_enum_values_capability", + "SAI_STATUS_SUCCESS", + "SAI_DEBUG_COUNTER_ATTR_IN_DROP_REASON_LIST=2:SAI_IN_DROP_REASON_L2_ANY,SAI_IN_DROP_REASON_L3_ANY" + } + ); +} + void test_tokenize_bulk_route_entry() { SWSS_LOG_ENTER(); @@ -881,5 +1025,9 @@ int main() test_serialize_bulk_create_route_entry(10,10000); test_serialize_bulk_create_oid(10,10000); + std::cout << " * test recorder" << std::endl; + + test_recorder_enum_value_capability_query(); + return 0; } diff --git a/meta/sai_serialize.h b/meta/sai_serialize.h index c4caaf1c2e38..59242199eaa3 100644 --- a/meta/sai_serialize.h +++ b/meta/sai_serialize.h @@ -153,6 +153,11 @@ std::string sai_serialize_enum( _In_ const int32_t value, _In_ const sai_enum_metadata_t* meta); +std::string sai_serialize_enum_list( + _In_ const sai_s32_list_t& list, + _In_ const sai_enum_metadata_t* meta, + _In_ bool countOnly); + std::string sai_serialize_number( _In_ uint32_t number, _In_ bool hex = false);