[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.

Author: stepanblyschak

lib/src/Recorder.cpp

@@ -1032,6 +1032,8 @@ void Recorder::recordQueryAattributeEnumValuesCapability(
 {
     SWSS_LOG_ENTER();
 
+    std::vector<swss::FieldValueTuple> 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<swss::FieldValueTuple> values;
+
     auto meta = sai_metadata_get_attr_metadata(objectType, attrId);
 
     if (meta == NULL)
@@ -1078,20 +1086,13 @@ void Recorder::recordQueryAattributeEnumValuesCapabilityResponse(
         return;
     }
 
-    std::vector<swss::FieldValueTuple> 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);

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 <chrono>
 #include <vector>
 
+#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<std::string> 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<std::string> 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<std::string>& 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<int32_t> enumList,
+    const std::vector<std::string>& 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<int32_t>(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;
 }

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);