Fix flow counter out-of-order issue by notifying counter operations using SelectableChannel

lib/RedisRemoteSaiInterface.cpp

@@ -476,6 +476,14 @@ sai_status_t RedisRemoteSaiInterface::setRedisExtensionAttribute(
 
             return SAI_STATUS_SUCCESS;
 
+        case SAI_REDIS_SWITCH_ATTR_FLEX_COUNTER_GROUP:
+            return notifyCounterGroupOperations(objectId,
+                                                reinterpret_cast<sai_redis_flex_counter_group_parameter_t*>(attr->value.ptr));
+
+        case SAI_REDIS_SWITCH_ATTR_FLEX_COUNTER:
+            return notifyCounterOperations(objectId,
+                                           reinterpret_cast<sai_redis_flex_counter_parameter_t*>(attr->value.ptr));
+
         default:
             break;
     }
@@ -485,6 +493,114 @@ sai_status_t RedisRemoteSaiInterface::setRedisExtensionAttribute(
     return SAI_STATUS_FAILURE;
 }
 
+bool RedisRemoteSaiInterface::isSaiS8ListValidString(const sai_s8_list_t &s8list, const char* hint)
+{
+    SWSS_LOG_ENTER();
+
+    if (s8list.list != nullptr && s8list.count > 0)
+    {
+        size_t len = strnlen((const char *)s8list.list, s8list.count);
+
+        if (len == (size_t)s8list.count)
+        {
+            return true;
+        }
+        else
+        {
+            SWSS_LOG_ERROR("%s: count (%u) is different than strnlen (%zu)", hint, s8list.count, len);
+        }
+    }
+
+    return false;
+}
+
+sai_status_t RedisRemoteSaiInterface::notifyCounterGroupOperations(
+        _In_ sai_object_id_t objectId,
+        _In_ const sai_redis_flex_counter_group_parameter_t *flexCounterGroupParam)
+{
+    SWSS_LOG_ENTER();
+
+    std::vector<swss::FieldValueTuple> entries;
+
+    if (flexCounterGroupParam == nullptr || !isSaiS8ListValidString(flexCounterGroupParam->counter_group_name, "COUNTER_GROUP_NAME"))
+    {
+        SWSS_LOG_ERROR("Invalid parameters when handling counter group operation");
+        return SAI_STATUS_FAILURE;
+    }
+
+    std::string key((const char*)flexCounterGroupParam->counter_group_name.list);
+
+    if (isSaiS8ListValidString(flexCounterGroupParam->poll_interval, POLL_INTERVAL_FIELD))
+    {
+        entries.emplace_back(POLL_INTERVAL_FIELD,
+                             std::string((const char*)flexCounterGroupParam->poll_interval.list, flexCounterGroupParam->poll_interval.count));
+    }
+
+    if (isSaiS8ListValidString(flexCounterGroupParam->stats_mode, STATS_MODE_FIELD))
+    {
+        entries.emplace_back(STATS_MODE_FIELD,
+                             std::string((const char*)flexCounterGroupParam->stats_mode.list, flexCounterGroupParam->stats_mode.count));
+    }
+
+    if (isSaiS8ListValidString(flexCounterGroupParam->plugins, "PLUGINS") && isSaiS8ListValidString(flexCounterGroupParam->plugin_name, "PLUGIN_NAME"))
+    {
+        entries.emplace_back(std::string((const char*)flexCounterGroupParam->plugin_name.list, flexCounterGroupParam->plugin_name.count),
+                             std::string((const char*)flexCounterGroupParam->plugins.list, flexCounterGroupParam->plugins.count));
+    }
+
+    if (isSaiS8ListValidString(flexCounterGroupParam->operation, FLEX_COUNTER_STATUS_FIELD))
+    {
+        entries.emplace_back(FLEX_COUNTER_STATUS_FIELD,
+                             std::string((const char*)flexCounterGroupParam->operation.list, flexCounterGroupParam->operation.count));
+    }
+
+    m_recorder->recordGenericSet(key, entries);
+
+    m_communicationChannel->set(key,
+                                entries,
+                                (entries.size() != 0) ? REDIS_FLEX_COUNTER_COMMAND_SET_GROUP : REDIS_FLEX_COUNTER_COMMAND_DEL_GROUP);
+
+    return waitForResponse(SAI_COMMON_API_SET);
+}
+
+sai_status_t RedisRemoteSaiInterface::notifyCounterOperations(
+        _In_ sai_object_id_t objectId,
+        _In_ const sai_redis_flex_counter_parameter_t *flexCounterParam)
+{
+    SWSS_LOG_ENTER();
+
+    if (flexCounterParam == nullptr || !isSaiS8ListValidString(flexCounterParam->counter_key, "COUNTER_KEY"))
+    {
+        SWSS_LOG_ERROR("Invalid parameters when handling counter operation");
+        return SAI_STATUS_FAILURE;
+    }
+
+    std::vector<swss::FieldValueTuple> entries;
+    std::string key((const char*)flexCounterParam->counter_key.list);
+    std::string command;
+
+    if (isSaiS8ListValidString(flexCounterParam->counter_ids, "COUNTER_IDS") && isSaiS8ListValidString(flexCounterParam->counter_field_name, "COUNTER_FIELD_NAME"))
+    {
+        entries.emplace_back(std::string((const char*)flexCounterParam->counter_field_name.list, flexCounterParam->counter_field_name.count),
+                             std::string((const char*)flexCounterParam->counter_ids.list, flexCounterParam->counter_ids.count));
+        command = REDIS_FLEX_COUNTER_COMMAND_START_POLL;
+        if (isSaiS8ListValidString(flexCounterParam->stats_mode, STATS_MODE_FIELD))
+        {
+            entries.emplace_back(std::string(STATS_MODE_FIELD),
+                                 std::string((const char*)flexCounterParam->stats_mode.list, flexCounterParam->stats_mode.count));
+        }
+    }
+    else
+    {
+        command = REDIS_FLEX_COUNTER_COMMAND_STOP_POLL;
+    }
+
+    m_recorder->recordGenericSet(key, entries);
+    m_communicationChannel->set(key, entries, command);
+
+    return waitForResponse(SAI_COMMON_API_SET);
+}
+
 sai_status_t RedisRemoteSaiInterface::set(
         _In_ sai_object_type_t objectType,
         _In_ sai_object_id_t objectId,

lib/RedisRemoteSaiInterface.h

@@ -343,6 +343,17 @@ namespace sairedis
                     _In_ sai_object_id_t objectId,
                     _In_ const sai_attribute_t *attr);
 
+            bool isSaiS8ListValidString(const sai_s8_list_t &s8list,
+                                        const char *hint);
+
+            sai_status_t notifyCounterGroupOperations(
+                    _In_ sai_object_id_t objectId,
+                    _In_ const sai_redis_flex_counter_group_parameter_t *flexCounterGroupParam);
+
+            sai_status_t notifyCounterOperations(
+                    _In_ sai_object_id_t objectId,
+                    _In_ const sai_redis_flex_counter_parameter_t *flexCounterParam);
+
         private:
 
             sai_status_t sai_redis_notify_syncd(

lib/sairedis.h

@@ -86,6 +86,88 @@ typedef enum _sai_redis_communication_mode_t
 
 } sai_redis_communication_mode_t;
 
+/**
+ * @brief Use Redis communication channel to handle counters.
+ *
+ * Originally, there are out-of-order issue between the objects and their counters.
+ * This is because the counters are handled on receiving flex counter database update.
+ * However, the objects are handled on receiving update from the redis communication channel.
+ *
+ * To resolve the issue, we use the redis communication channel to handle the counter updates.
+ *
+ * The struct sai_redis_flex_counter_group_parameter_t represents the counter group operations.
+ * The caller (usually orchagent) can change some or all the options of a counter group.
+ * The counter_group_name represents the counter group name which must be a valid list.
+ * For the rest fields, it means not changing it to pass an empty list.
+ */
+typedef struct _sai_redis_flex_counter_group_parameter_t
+{
+    /**
+     * @brief The flex counter group name.
+     *
+     * It is the key of FLEX_COUNTER_TABLE and FLEX_COUNTER_GROUP_TABLE table.
+     */
+    sai_s8_list_t counter_group_name;
+
+    /**
+     * @brief The polling interval of the counter group
+     *
+     * It should be a number representing the polling interval in seconds.
+     */
+    sai_s8_list_t poll_interval;
+
+    /**
+     * @brief The operation of the counter group
+     *
+     * It should be either "enable" or "disable"
+     */
+    sai_s8_list_t operation;
+
+    /**
+     * @brief The counter fetching mode.
+     *
+     * It should be either "STATS_MODE_READ" or "STATS_MODE_READ_AND_CLEAR"
+     */
+    sai_s8_list_t stats_mode;
+
+    /**
+     * @brief The name of the filed that represents the Lua plugin
+     */
+    sai_s8_list_t plugin_name;
+
+    /**
+     * @brief The SHA code of the Lua plugin
+     */
+    sai_s8_list_t plugins;
+
+} sai_redis_flex_counter_group_parameter_t;
+
+typedef struct _sai_redis_flex_counter_parameter_t
+{
+    /**
+     * @brief The key in the flex counter table
+     *
+     * It should be the serialized OID eg. "oid:0x15000000000001"
+     */
+    sai_s8_list_t counter_key;
+
+    /**
+     * @brief The list of counters' IDs that should be fetched.
+     */
+    sai_s8_list_t counter_ids;
+
+    /**
+     * @brief The name of the filed that represents the counters' IDs.
+     */
+    sai_s8_list_t counter_field_name;
+
+    /**
+     * @brief The counter fetch mode of the object.
+     */
+    sai_s8_list_t stats_mode;
+
+} sai_redis_flex_counter_parameter_t;
+
 typedef enum _sai_redis_switch_attr_t
 {
     /**
@@ -249,6 +331,25 @@ typedef enum _sai_redis_switch_attr_t
      * @default 60000
      */
     SAI_REDIS_SWITCH_ATTR_SYNC_OPERATION_RESPONSE_TIMEOUT,
+
+    /**
+     * @brief Flex counter group operations
+     *
+     * @type sai_redis_flex_counter_group_parameter_t
+     * @flags CREATE_AND_SET
+     * @default 0
+     */
+    SAI_REDIS_SWITCH_ATTR_FLEX_COUNTER_GROUP,
+
+    /**
+     * @brief Flex counter operations
+     *
+     * @type sai_redis_counter_parameter_t
+     * @flags CREATE_AND_SET
+     * @default 0
+     */
+    SAI_REDIS_SWITCH_ATTR_FLEX_COUNTER,
+
 } sai_redis_switch_attr_t;
 
 /**

lib/sairediscommon.h

@@ -52,6 +52,11 @@
 #define REDIS_ASIC_STATE_COMMAND_OBJECT_TYPE_GET_AVAILABILITY_QUERY     "object_type_get_availability_query"
 #define REDIS_ASIC_STATE_COMMAND_OBJECT_TYPE_GET_AVAILABILITY_RESPONSE  "object_type_get_availability_response"
 
+#define REDIS_FLEX_COUNTER_COMMAND_START_POLL       "start_poll"
+#define REDIS_FLEX_COUNTER_COMMAND_STOP_POLL        "stop_poll"
+#define REDIS_FLEX_COUNTER_COMMAND_SET_GROUP        "set_counter_group"
+#define REDIS_FLEX_COUNTER_COMMAND_DEL_GROUP        "del_counter_group"
+#define REDIS_FLEX_COUNTER_COMMAND_RESPONSE         "counter_response"
 /**
  * @brief Redis virtual object id counter key name.
  *