Add synchronous clear_stats operation path

lib/inc/sai_redis_internal.h

@@ -150,7 +150,7 @@
     {                                                               \
         MUTEX();                                                    \
         SWSS_LOG_ENTER();                                           \
-        return  meta_sai_get_stats_oid(                             \
+        return meta_sai_get_stats_oid(                              \
                 SAI_OBJECT_TYPE_ ## OBJECT_TYPE,                    \
                 object_type ## _id,                                 \
                 &sai_metadata_enum_sai_ ## object_type ## _stat_t,  \
@@ -188,12 +188,13 @@
     {                                                               \
         MUTEX();                                                    \
         SWSS_LOG_ENTER();                                           \
-        return redis_generic_clear_stats(                           \
+        return meta_sai_clear_stats_oid(                            \
                 SAI_OBJECT_TYPE_ ## OBJECT_TYPE,                    \
                 object_type ## _id,                                 \
                 &sai_metadata_enum_sai_ ## object_type ## _stat_t,  \
                 number_of_counters,                                 \
-                (const int32_t*)counter_ids);                       \
+                (const int32_t*)counter_ids,                        \
+                &redis_generic_clear_stats);                        \
     }
 
 #define REDIS_GENERIC_STATS(OT, ot)    \

lib/src/sai_redis_generic_stats.cpp

@@ -179,6 +179,50 @@ sai_status_t internal_redis_get_stats_process(
     return status;
 }
 
+sai_status_t internal_redis_clear_stats_process(
+        _In_ sai_object_type_t object_type,
+        _In_ const sai_enum_metadata_t *stats_enum_metadata,
+        _In_ uint32_t count,
+        _In_ const int32_t *counter_id_list,
+        _In_ swss::KeyOpFieldsValuesTuple &kco)
+{
+    SWSS_LOG_ENTER();
+
+    // key:         sai_status
+    // field:       stat_id
+
+    const auto &key = kfvKey(kco);
+    sai_status_t status;
+    sai_deserialize_status(key, status);
+    if (status != SAI_STATUS_SUCCESS)
+    {
+        return status;
+    }
+
+    const auto &fvTuples = kfvFieldsValues(kco);
+    if (fvTuples.size() != count)
+    {
+        SWSS_LOG_ERROR("Field count %zu not as expected %u", fvTuples.size(), count);
+        return SAI_STATUS_FAILURE;
+    }
+
+    int32_t counter_id;
+    for (uint32_t i = 0; i < count; i++)
+    {
+        sai_deserialize_enum(fvField(fvTuples[i]).c_str(), stats_enum_metadata, &counter_id);
+        if (counter_id != counter_id_list[i])
+        {
+            SWSS_LOG_ERROR("Counter id %s not as expected %s",
+                fvField(fvTuples[i]).c_str(),
+                sai_serialize_enum(counter_id_list[i], stats_enum_metadata).c_str());
+            status = SAI_STATUS_FAILURE;
+            break;
+        }
+    }
+
+    return status;
+}
+
 std::vector<swss::FieldValueTuple> serialize_counter_id_list(
         _In_ const sai_enum_metadata_t *stats_enum,
         _In_ uint32_t count,
@@ -339,29 +383,102 @@ sai_status_t redis_generic_get_stats_ext(
             counters);
 }
 
+sai_status_t internal_redis_generic_clear_stats(
+        _In_ sai_object_type_t object_type,
+        _In_ const std::string &serialized_object_id,
+        _In_ const sai_enum_metadata_t *stats_enum,
+        _In_ uint32_t count,
+        _In_ const int32_t *counter_id_list)
+{
+    SWSS_LOG_ENTER();
+
+    std::vector<swss::FieldValueTuple> fvTuples = serialize_counter_id_list(
+            stats_enum,
+            count,
+            counter_id_list);
+
+    std::string str_object_type = sai_serialize_object_type(object_type);
+    std::string key = str_object_type + ":" + serialized_object_id;
+
+    SWSS_LOG_DEBUG("generic clear stats key: %s, fields: %lu", key.c_str(), fvTuples.size());
+
+    if (g_record)
+    {
+        recordLine("m|" + key + "|" + joinFieldValues(fvTuples));
+    }
+
+    // clear is special, it will not put data
+    // into asic view, only to message queue
+    g_asicState->set(key, fvTuples, "clear_stats");
+
+    // wait for response
+    swss::Select s;
+    s.addSelectable(g_redisGetConsumer.get());
+    while (true)
+    {
+        SWSS_LOG_DEBUG("wait for clear_stats response");
+
+        swss::Selectable *sel;
+        int result = s.select(&sel, GET_RESPONSE_TIMEOUT);
+        if (result == swss::Select::OBJECT)
+        {
+            swss::KeyOpFieldsValuesTuple kco;
+            g_redisGetConsumer->pop(kco);
+            const std::string &respKey = kfvKey(kco);
+            const std::string &respOp = kfvOp(kco);
+            SWSS_LOG_DEBUG("response: key = %s, op = %s", respKey.c_str(), respOp.c_str());
+
+            if (respOp != "getresponse") // ignore non response messages
+            {
+                continue;
+            }
+
+            if (g_record)
+            {
+                const auto &respFvTuples = kfvFieldsValues(kco);
+
+                // first serialized is status return by sai clear_stats
+                recordLine("M|" + respKey + "|" + joinFieldValues(respFvTuples));
+            }
+
+            sai_status_t status = internal_redis_clear_stats_process(
+                    object_type,
+                    stats_enum,
+                    count,
+                    counter_id_list,
+                    kco);
+            SWSS_LOG_DEBUG("generic clear stats status: %s", sai_serialize_status(status).c_str());
+            return status;
+        }
+
+        SWSS_LOG_ERROR("generic clear stats failed due to SELECT operation result");
+        break;
+    }
+
+    if (g_record)
+    {
+        recordLine("M|SAI_STATUS_FAILURE");
+    }
+
+    SWSS_LOG_ERROR("generic clear stats failed to get response");
+    return SAI_STATUS_FAILURE;
+}
+
 sai_status_t redis_generic_clear_stats(
         _In_ sai_object_type_t object_type,
         _In_ sai_object_id_t object_id,
-        _In_ const sai_enum_metadata_t *enum_metadata,
+        _In_ const sai_enum_metadata_t *enum_metadata_stats,
         _In_ uint32_t number_of_counters,
         _In_ const int32_t *counter_ids)
 {
     SWSS_LOG_ENTER();
 
-    /*
-     * Clear stats is the same as get stats ext with mode ==
-     * SAI_STATS_MODE_READ_AND_CLEAR and we just read counters locally and
-     * discard them, in that way.
-     */
-
-    uint64_t counters[REDIS_MAX_COUNTERS];
+    std::string str_object_id = sai_serialize_object_id(object_id);
 
-    return redis_generic_stats_function(
+    return internal_redis_generic_clear_stats(
             object_type,
-            object_id,
-            enum_metadata,
+            str_object_id,
+            enum_metadata_stats,
             number_of_counters,
-            counter_ids,
-            SAI_STATS_MODE_READ_AND_CLEAR,
-            counters);
+            counter_ids);
 }

meta/sai_meta.cpp

@@ -14,15 +14,15 @@
 #include <map>
 #include <iterator>
 
-#define META_LOG_STATUS(s)\
+#define META_LOG_STATUS(op, s)\
     if (s == SAI_STATUS_SUCCESS)                                           \
-        SWSS_LOG_DEBUG("get status: %s", sai_serialize_status(s).c_str()); \
+        SWSS_LOG_DEBUG(#op " status: %s", sai_serialize_status(s).c_str()); \
     else if (s == SAI_STATUS_BUFFER_OVERFLOW                               \
                || SAI_STATUS_IS_ATTR_NOT_IMPLEMENTED(s)                    \
                || SAI_STATUS_IS_ATTR_NOT_SUPPORTED(s))                     \
-        SWSS_LOG_INFO("get status: %s", sai_serialize_status(s).c_str());  \
+        SWSS_LOG_INFO(#op " status: %s", sai_serialize_status(s).c_str());  \
     else                                                                   \
-        SWSS_LOG_ERROR("get status: %s", sai_serialize_status(s).c_str());
+        SWSS_LOG_ERROR(#op " status: %s", sai_serialize_status(s).c_str());
 
 static volatile bool unittests_enabled = false;
 
@@ -3808,7 +3808,7 @@ sai_status_t meta_sai_get_fdb_entry(
 
     status = get(fdb_entry, attr_count, attr_list);
 
-    META_LOG_STATUS(status);
+    META_LOG_STATUS(get, status);
 
     if (status == SAI_STATUS_SUCCESS)
     {
@@ -4086,7 +4086,7 @@ sai_status_t meta_sai_get_mcast_fdb_entry(
 
     status = get(mcast_fdb_entry, attr_count, attr_list);
 
-    META_LOG_STATUS(status);
+    META_LOG_STATUS(get, status);
 
     if (status == SAI_STATUS_SUCCESS)
     {
@@ -4374,7 +4374,7 @@ sai_status_t meta_sai_get_neighbor_entry(
 
     status = get(neighbor_entry, attr_count, attr_list);
 
-    META_LOG_STATUS(status);
+    META_LOG_STATUS(get, status);
 
     if (status == SAI_STATUS_SUCCESS)
     {
@@ -4674,7 +4674,7 @@ sai_status_t meta_sai_get_route_entry(
 
     status = get(route_entry, attr_count, attr_list);
 
-    META_LOG_STATUS(status);
+    META_LOG_STATUS(get, status);
 
     if (status == SAI_STATUS_SUCCESS)
     {
@@ -4990,7 +4990,7 @@ sai_status_t meta_sai_get_l2mc_entry(
 
     status = get(l2mc_entry, attr_count, attr_list);
 
-    META_LOG_STATUS(status);
+    META_LOG_STATUS(get, status);
 
     if (status == SAI_STATUS_SUCCESS)
     {
@@ -5306,7 +5306,7 @@ sai_status_t meta_sai_get_ipmc_entry(
 
     status = get(ipmc_entry, attr_count, attr_list);
 
-    META_LOG_STATUS(status);
+    META_LOG_STATUS(get, status);
 
     if (status == SAI_STATUS_SUCCESS)
     {
@@ -5516,7 +5516,7 @@ sai_status_t meta_sai_get_inseg_entry(
 
     status = get(inseg_entry, attr_count, attr_list);
 
-    META_LOG_STATUS(status);
+    META_LOG_STATUS(get, status);
 
     if (status == SAI_STATUS_SUCCESS)
     {
@@ -5807,7 +5807,7 @@ sai_status_t meta_sai_get_oid(
 
     status = get(object_type, object_id, attr_count, attr_list);
 
-    META_LOG_STATUS(status);
+    META_LOG_STATUS(get, status);
 
     if (status == SAI_STATUS_SUCCESS)
     {
@@ -5930,7 +5930,110 @@ sai_status_t meta_sai_get_stats_oid(
 
     status = get_stats(object_type, object_id, stats_enum, count, counter_id_list, counter_list);
 
-    META_LOG_STATUS(status);
+    META_LOG_STATUS(get, status);
+
+    return status;
+}
+
+sai_status_t meta_generic_validation_clear_stats(
+        _In_ const sai_object_meta_key_t& meta_key,
+        _In_ const sai_enum_metadata_t* stats_enum,
+        _In_ uint32_t count,
+        _In_ const int32_t *counter_id_list)
+{
+    SWSS_LOG_ENTER();
+
+    if (meta_unittests_enabled() && (count & 0x80000000))
+    {
+        /*
+         * If last bit of counters count is set to high, and unittests are enabled,
+         * then this api can be used to SET counter values by user for debugging purposes.
+         */
+        count = count & ~0x80000000;
+    }
+
+    if (count < 1)
+    {
+        SWSS_LOG_ERROR("expected at least 1 stat when calling clear_stats, zero given");
+
+        return SAI_STATUS_INVALID_PARAMETER;
+    }
+
+    if (count > MAX_LIST_COUNT)
+    {
+        SWSS_LOG_ERROR("clear_stats count %u > max list count %u", count, MAX_LIST_COUNT);
+
+        return SAI_STATUS_INVALID_PARAMETER;
+    }
+
+    if (counter_id_list == NULL)
+    {
+        SWSS_LOG_ERROR("counter id list pointer is NULL");
+
+        return SAI_STATUS_INVALID_PARAMETER;
+    }
+
+    if (stats_enum == NULL)
+    {
+        SWSS_LOG_ERROR("enum metadata pointer is NULL, bug?");
+
+        return SAI_STATUS_FAILURE;
+    }
+
+    for (uint32_t i = 0; i < count; i++)
+    {
+        if (sai_metadata_get_enum_value_name(stats_enum, counter_id_list[i]) == NULL)
+        {
+            SWSS_LOG_ERROR("counter id %u is not allowed on %s", counter_id_list[i], stats_enum->name);
+
+            return SAI_STATUS_INVALID_PARAMETER;
+        }
+    }
+
+    return SAI_STATUS_SUCCESS;
+}
+
+sai_status_t meta_sai_clear_stats_oid(
+        _In_ sai_object_type_t object_type,
+        _In_ sai_object_id_t object_id,
+        _In_ const sai_enum_metadata_t* stats_enum,
+        _In_ uint32_t count,
+        _In_ const int32_t *counter_id_list,
+        _In_ sai_clear_generic_stats_fn clear_stats)
+{
+    SWSS_LOG_ENTER();
+
+    sai_object_id_t switch_id = sai_switch_id_query(object_id);
+
+    sai_status_t status = meta_sai_validate_oid(object_type, &object_id, switch_id, false);
+
+    if (status != SAI_STATUS_SUCCESS)
+    {
+        SWSS_LOG_ERROR("oid validation failed");
+        return status;
+    }
+
+    sai_object_meta_key_t meta_key = { .objecttype = object_type, .objectkey = { .key = { .object_id  = object_id } } };
+
+    // This ensures that all counter ids in counter id list are valid
+    // with regards to stats_enum before calling clear_stats()
+    status = meta_generic_validation_clear_stats(meta_key, stats_enum, count, counter_id_list);
+
+    if (status != SAI_STATUS_SUCCESS)
+    {
+        SWSS_LOG_ERROR("clear_stats generic validation failed");
+        return status;
+    }
+
+    if (clear_stats == NULL)
+    {
+        SWSS_LOG_ERROR("clear_stats function pointer is NULL");
+        return SAI_STATUS_INVALID_PARAMETER;
+    }
+
+    status = clear_stats(object_type, object_id, stats_enum, count, counter_id_list);
+
+    META_LOG_STATUS(clear, status);
 
     return status;
 }