From 072f55ead0f34e35daa6f6abfda9aab563c50a1d Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Fri, 6 Sep 2024 12:12:50 +0300 Subject: [PATCH 01/10] Support customizing bulk size Signed-off-by: Stephen Sun --- orchagent/flexcounterorch.cpp | 4 ++++ orchagent/saihelper.cpp | 19 +++++++++++++++++++ orchagent/saihelper.h | 4 ++++ 3 files changed, 27 insertions(+) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 2832f0bd12..996bd07796 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -251,6 +251,10 @@ void FlexCounterOrch::doTask(Consumer &consumer) // If it is exist and the value is 'true' we need to skip the iteration in order to delay the counter creation. // The field will clear out and counter will be created when enable_counters script is called. } + else if (field == BULK_SIZE_FIELD) + { + setFlexCounterGroupBulkSize(flexCounterGroupMap[key], value, false); + } else { SWSS_LOG_NOTICE("Unsupported field %s", field.c_str()); diff --git a/orchagent/saihelper.cpp b/orchagent/saihelper.cpp index e7cf7fb018..72264251ab 100644 --- a/orchagent/saihelper.cpp +++ b/orchagent/saihelper.cpp @@ -854,6 +854,7 @@ static inline void initSaiRedisCounterEmptyParameter(sai_redis_flex_counter_grou initSaiRedisCounterEmptyParameter(flex_counter_group_param.stats_mode); initSaiRedisCounterEmptyParameter(flex_counter_group_param.plugin_name); initSaiRedisCounterEmptyParameter(flex_counter_group_param.plugins); + initSaiRedisCounterEmptyParameter(flex_counter_group_param.bulk_size); } static inline void initSaiRedisCounterParameterFromString(sai_s8_list_t &sai_s8_list, const std::string &str) @@ -938,6 +939,7 @@ void setFlexCounterGroupParameter(const string &group, attr.id = SAI_REDIS_SWITCH_ATTR_FLEX_COUNTER_GROUP; attr.value.ptr = &flex_counter_group_param; + initSaiRedisCounterEmptyParameter(flex_counter_group_param.bulk_size); initSaiRedisCounterParameterFromString(flex_counter_group_param.counter_group_name, group); initSaiRedisCounterParameterFromString(flex_counter_group_param.poll_interval, poll_interval); initSaiRedisCounterParameterFromString(flex_counter_group_param.operation, operation); @@ -1017,6 +1019,23 @@ void setFlexCounterGroupStatsMode(const std::string &group, notifySyncdCounterOperation(is_gearbox, attr); } +void setFlexCounterGroupBulkSize(const std::string &group, + const std::string &bulk_size, + bool is_gearbox) +{ + sai_attribute_t attr; + sai_redis_flex_counter_group_parameter_t flex_counter_group_param; + + attr.id = SAI_REDIS_SWITCH_ATTR_FLEX_COUNTER_GROUP; + attr.value.ptr = &flex_counter_group_param; + + initSaiRedisCounterEmptyParameter(flex_counter_group_param); + initSaiRedisCounterParameterFromString(flex_counter_group_param.counter_group_name, group); + initSaiRedisCounterParameterFromString(flex_counter_group_param.bulk_size, bulk_size); + + notifySyncdCounterOperation(is_gearbox, attr); +} + void delFlexCounterGroup(const std::string &group, bool is_gearbox) { diff --git a/orchagent/saihelper.h b/orchagent/saihelper.h index 7334adff35..d35abdf0dc 100644 --- a/orchagent/saihelper.h +++ b/orchagent/saihelper.h @@ -39,6 +39,10 @@ void setFlexCounterGroupStatsMode(const std::string &group, const std::string &stats_mode, bool is_gearbox=false); +void setFlexCounterGroupBulkSize(const std::string &group, + const std::string &bulk_size, + bool is_gearbox); + void delFlexCounterGroup(const std::string &group, bool is_gearbox=false); From 380be05368ec0f6bd7b1d95aa323b577347c07a4 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Mon, 30 Sep 2024 03:40:49 +0300 Subject: [PATCH 02/10] More debug functions Signed-off-by: Stephen Sun --- orchagent/pfc_detect_mellanox.lua | 11 +++++++++++ 1 file changed, 11 insertions(+) mode change 100644 => 100755 orchagent/pfc_detect_mellanox.lua diff --git a/orchagent/pfc_detect_mellanox.lua b/orchagent/pfc_detect_mellanox.lua old mode 100644 new mode 100755 index 5e6d8c00c5..e9806237fc --- a/orchagent/pfc_detect_mellanox.lua +++ b/orchagent/pfc_detect_mellanox.lua @@ -25,6 +25,9 @@ if timestamp_last ~= false then redis.call('HSET', 'TIMESTAMP', 'effective_pfcwd_poll_time_last', effective_poll_time) end +local debug_storm_global = redis.call('HGET', 'DEBUG_STORM', 'enabled') == 'true' +local debug_storm_threshold = tonumber(redis.call('HGET', 'DEBUG_STORM', 'threshold')) + -- Iterate through each queue local n = table.getn(KEYS) for i = n, 1, -1 do @@ -62,6 +65,10 @@ for i = n, 1, -1 do local pfc_rx_packets = redis.call('HGET', counters_table_name .. ':' .. port_id, pfc_rx_pkt_key) local pfc_duration = redis.call('HGET', counters_table_name .. ':' .. port_id, pfc_duration_key) + if debug_storm_global then + redis.call('PUBLISH', 'PFC_WD_DEBUG', 'Port ID ' .. port_id .. ' Queue index ' .. queue_index .. ' occupancy ' .. occupancy_bytes .. ' packets ' .. packets .. ' pfc rx ' .. pfc_rx_packets .. ' pfc duration ' .. pfc_duration .. ' effective poll time ' .. tostring(effective_poll_time)) + end + if occupancy_bytes and packets and pfc_rx_packets and pfc_duration then occupancy_bytes = tonumber(occupancy_bytes) packets = tonumber(packets) @@ -82,6 +89,10 @@ for i = n, 1, -1 do pfc_duration_last = tonumber(pfc_duration_last) local storm_condition = (pfc_duration - pfc_duration_last) > (effective_poll_time * 0.99) + if debug_storm_threshold ~= nil and (pfc_duration - pfc_duration_last) > (effective_poll_time * debug_storm_threshold / 100) then + redis.call('PUBLISH', 'PFC_WD_DEBUG', 'Port ID ' .. port_id .. ' Queue index ' .. queue_index .. ' occupancy ' .. occupancy_bytes .. ' packets ' .. packets .. ' pfc rx ' .. pfc_rx_packets .. ' pfc duration ' .. pfc_duration .. ' effective poll time ' .. tostring(effective_poll_time) .. ', triggered by threshold ' .. debug_storm_threshold .. '%') + end + -- Check actual condition of queue being in PFC storm if (occupancy_bytes > 0 and packets - packets_last == 0 and storm_condition) or -- DEBUG CODE START. Uncomment to enable From e97062ec2a196c7c6efcfe307c06f5cb7063fb05 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Mon, 30 Sep 2024 10:14:45 +0000 Subject: [PATCH 03/10] Precise time stamp Signed-off-by: Stephen Sun --- orchagent/pfc_detect_mellanox.lua | 37 ++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/orchagent/pfc_detect_mellanox.lua b/orchagent/pfc_detect_mellanox.lua index e9806237fc..c559b3e3c6 100755 --- a/orchagent/pfc_detect_mellanox.lua +++ b/orchagent/pfc_detect_mellanox.lua @@ -18,13 +18,17 @@ local timestamp_struct = redis.call('TIME') local timestamp_current = timestamp_struct[1] + timestamp_struct[2] / 1000000 local timestamp_string = tostring(timestamp_current) redis.call('HSET', 'TIMESTAMP', 'pfcwd_poll_timestamp_last', timestamp_string) -local effective_poll_time = poll_time -local effective_poll_time_lasttime = redis.call('HGET', 'TIMESTAMP', 'effective_pfcwd_poll_time_last') +local global_effective_poll_time = poll_time +local global_effective_poll_time_lasttime = redis.call('HGET', 'TIMESTAMP', 'effective_pfcwd_poll_time_last') if timestamp_last ~= false then - effective_poll_time = (timestamp_current - tonumber(timestamp_last)) * 1000000 - redis.call('HSET', 'TIMESTAMP', 'effective_pfcwd_poll_time_last', effective_poll_time) + global_effective_poll_time = (timestamp_current - tonumber(timestamp_last)) * 1000000 + redis.call('HSET', 'TIMESTAMP', 'effective_pfcwd_poll_time_last', global_effective_poll_time) end +local effective_poll_time +local effective_poll_time_lasttime +local port_timestamp_last_cache = {} + local debug_storm_global = redis.call('HGET', 'DEBUG_STORM', 'enabled') == 'true' local debug_storm_threshold = tonumber(redis.call('HGET', 'DEBUG_STORM', 'threshold')) @@ -59,6 +63,27 @@ for i = n, 1, -1 do local pfc_rx_pkt_key = 'SAI_PORT_STAT_PFC_' .. queue_index .. '_RX_PKTS' local pfc_duration_key = 'SAI_PORT_STAT_PFC_' .. queue_index .. '_RX_PAUSE_DURATION_US' + -- Get port specific timestamp + local port_timestamp_current = tonumber(redis.call('HGET', counters_table_name .. ':' .. port_id, 'PFC_WD_time_stamp')) + if port_timestamp_current ~= nil then + local port_timestamp_lasttime = port_timestamp_last_cache[port_id] + if port_timestamp_lasttime == nil then + port_timestamp_lasttime = tonumber(redis.call('HGET', counters_table_name .. ':' .. port_id, 'PFC_WD_time_stamp_last')) + port_timestamp_last_cache[port_id] = port_timestamp_lasttime + redis.call('HSET', counters_table_name .. ':' .. port_id, 'PFC_WD_time_stamp_last', port_timestamp_current) + end + + if port_timestamp_lasttime ~= nil then + effective_poll_time = (port_timestamp_current - port_timestamp_lasttime) / 1000 + else + effective_poll_time = global_effective_poll_time + effective_poll_time_lasttime = false + end + else + effective_poll_time = global_effective_poll_time + effective_poll_time_lasttime = global_effective_poll_time_lasttime + end + -- Get all counters local occupancy_bytes = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_QUEUE_STAT_CURR_OCCUPANCY_BYTES') local packets = redis.call('HGET', counters_table_name .. ':' .. KEYS[i], 'SAI_QUEUE_STAT_PACKETS') @@ -66,7 +91,7 @@ for i = n, 1, -1 do local pfc_duration = redis.call('HGET', counters_table_name .. ':' .. port_id, pfc_duration_key) if debug_storm_global then - redis.call('PUBLISH', 'PFC_WD_DEBUG', 'Port ID ' .. port_id .. ' Queue index ' .. queue_index .. ' occupancy ' .. occupancy_bytes .. ' packets ' .. packets .. ' pfc rx ' .. pfc_rx_packets .. ' pfc duration ' .. pfc_duration .. ' effective poll time ' .. tostring(effective_poll_time)) + redis.call('PUBLISH', 'PFC_WD_DEBUG', 'Port ID ' .. port_id .. ' Queue index ' .. queue_index .. ' occupancy ' .. occupancy_bytes .. ' packets ' .. packets .. ' pfc rx ' .. pfc_rx_packets .. ' pfc duration ' .. pfc_duration .. ' effective poll time ' .. tostring(effective_poll_time) .. '(global ' .. tostring(global_effective_poll_time) .. ')') end if occupancy_bytes and packets and pfc_rx_packets and pfc_duration then @@ -91,7 +116,7 @@ for i = n, 1, -1 do if debug_storm_threshold ~= nil and (pfc_duration - pfc_duration_last) > (effective_poll_time * debug_storm_threshold / 100) then redis.call('PUBLISH', 'PFC_WD_DEBUG', 'Port ID ' .. port_id .. ' Queue index ' .. queue_index .. ' occupancy ' .. occupancy_bytes .. ' packets ' .. packets .. ' pfc rx ' .. pfc_rx_packets .. ' pfc duration ' .. pfc_duration .. ' effective poll time ' .. tostring(effective_poll_time) .. ', triggered by threshold ' .. debug_storm_threshold .. '%') - end + end -- Check actual condition of queue being in PFC storm if (occupancy_bytes > 0 and packets - packets_last == 0 and storm_condition) or From 4bf16103eec30ae6489c5c673eea47dd0c12f33e Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Mon, 14 Oct 2024 16:43:28 +0300 Subject: [PATCH 04/10] Fix issue Signed-off-by: Stephen Sun --- orchagent/pfc_detect_mellanox.lua | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/orchagent/pfc_detect_mellanox.lua b/orchagent/pfc_detect_mellanox.lua index c559b3e3c6..e00243fa65 100755 --- a/orchagent/pfc_detect_mellanox.lua +++ b/orchagent/pfc_detect_mellanox.lua @@ -77,8 +77,8 @@ for i = n, 1, -1 do effective_poll_time = (port_timestamp_current - port_timestamp_lasttime) / 1000 else effective_poll_time = global_effective_poll_time - effective_poll_time_lasttime = false end + effective_poll_time_lasttime = false else effective_poll_time = global_effective_poll_time effective_poll_time_lasttime = global_effective_poll_time_lasttime From 0884f25b2e4c7e44d6297bc4aeb278ae47bb1879 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Sun, 10 Nov 2024 23:38:23 +0000 Subject: [PATCH 05/10] Rename Signed-off-by: Stephen Sun --- orchagent/flexcounterorch.cpp | 4 ++-- orchagent/saihelper.cpp | 12 ++++++------ orchagent/saihelper.h | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 996bd07796..931488b857 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -251,9 +251,9 @@ void FlexCounterOrch::doTask(Consumer &consumer) // If it is exist and the value is 'true' we need to skip the iteration in order to delay the counter creation. // The field will clear out and counter will be created when enable_counters script is called. } - else if (field == BULK_SIZE_FIELD) + else if (field == BULK_CHUNK_SIZE_FIELD) { - setFlexCounterGroupBulkSize(flexCounterGroupMap[key], value, false); + setFlexCounterGroupBulkChunkSize(flexCounterGroupMap[key], value, false); } else { diff --git a/orchagent/saihelper.cpp b/orchagent/saihelper.cpp index 72264251ab..91776a0ab3 100644 --- a/orchagent/saihelper.cpp +++ b/orchagent/saihelper.cpp @@ -854,7 +854,7 @@ static inline void initSaiRedisCounterEmptyParameter(sai_redis_flex_counter_grou initSaiRedisCounterEmptyParameter(flex_counter_group_param.stats_mode); initSaiRedisCounterEmptyParameter(flex_counter_group_param.plugin_name); initSaiRedisCounterEmptyParameter(flex_counter_group_param.plugins); - initSaiRedisCounterEmptyParameter(flex_counter_group_param.bulk_size); + initSaiRedisCounterEmptyParameter(flex_counter_group_param.bulk_chunk_size); } static inline void initSaiRedisCounterParameterFromString(sai_s8_list_t &sai_s8_list, const std::string &str) @@ -939,7 +939,7 @@ void setFlexCounterGroupParameter(const string &group, attr.id = SAI_REDIS_SWITCH_ATTR_FLEX_COUNTER_GROUP; attr.value.ptr = &flex_counter_group_param; - initSaiRedisCounterEmptyParameter(flex_counter_group_param.bulk_size); + initSaiRedisCounterEmptyParameter(flex_counter_group_param.bulk_chunk_size); initSaiRedisCounterParameterFromString(flex_counter_group_param.counter_group_name, group); initSaiRedisCounterParameterFromString(flex_counter_group_param.poll_interval, poll_interval); initSaiRedisCounterParameterFromString(flex_counter_group_param.operation, operation); @@ -1019,9 +1019,9 @@ void setFlexCounterGroupStatsMode(const std::string &group, notifySyncdCounterOperation(is_gearbox, attr); } -void setFlexCounterGroupBulkSize(const std::string &group, - const std::string &bulk_size, - bool is_gearbox) +void setFlexCounterGroupBulkChunkSize(const std::string &group, + const std::string &bulk_chunk_size, + bool is_gearbox) { sai_attribute_t attr; sai_redis_flex_counter_group_parameter_t flex_counter_group_param; @@ -1031,7 +1031,7 @@ void setFlexCounterGroupBulkSize(const std::string &group, initSaiRedisCounterEmptyParameter(flex_counter_group_param); initSaiRedisCounterParameterFromString(flex_counter_group_param.counter_group_name, group); - initSaiRedisCounterParameterFromString(flex_counter_group_param.bulk_size, bulk_size); + initSaiRedisCounterParameterFromString(flex_counter_group_param.bulk_chunk_size, bulk_chunk_size); notifySyncdCounterOperation(is_gearbox, attr); } diff --git a/orchagent/saihelper.h b/orchagent/saihelper.h index d35abdf0dc..8e1223f1ac 100644 --- a/orchagent/saihelper.h +++ b/orchagent/saihelper.h @@ -39,9 +39,9 @@ void setFlexCounterGroupStatsMode(const std::string &group, const std::string &stats_mode, bool is_gearbox=false); -void setFlexCounterGroupBulkSize(const std::string &group, - const std::string &bulk_size, - bool is_gearbox); +void setFlexCounterGroupBulkChunkSize(const std::string &group, + const std::string &bulk_size, + bool is_gearbox); void delFlexCounterGroup(const std::string &group, bool is_gearbox=false); From 715b1902cd2f76e0715d81a423f8b6944eb1290e Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Wed, 27 Nov 2024 03:43:17 +0200 Subject: [PATCH 06/10] Support split counter IDs based on prefix Signed-off-by: Stephen Sun --- orchagent/flexcounterorch.cpp | 4 ++++ orchagent/saihelper.cpp | 19 +++++++++++++++++++ orchagent/saihelper.h | 4 ++++ 3 files changed, 27 insertions(+) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 931488b857..4c7d601794 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -255,6 +255,10 @@ void FlexCounterOrch::doTask(Consumer &consumer) { setFlexCounterGroupBulkChunkSize(flexCounterGroupMap[key], value, false); } + else if (field == BULK_CHUNK_SIZE_PER_PREFIX_FIELD) + { + setFlexCounterGroupBulkChunkSizePerPrefix(flexCounterGroupMap[key], value, false); + } else { SWSS_LOG_NOTICE("Unsupported field %s", field.c_str()); diff --git a/orchagent/saihelper.cpp b/orchagent/saihelper.cpp index 91776a0ab3..0ef3e27285 100644 --- a/orchagent/saihelper.cpp +++ b/orchagent/saihelper.cpp @@ -855,6 +855,7 @@ static inline void initSaiRedisCounterEmptyParameter(sai_redis_flex_counter_grou initSaiRedisCounterEmptyParameter(flex_counter_group_param.plugin_name); initSaiRedisCounterEmptyParameter(flex_counter_group_param.plugins); initSaiRedisCounterEmptyParameter(flex_counter_group_param.bulk_chunk_size); + initSaiRedisCounterEmptyParameter(flex_counter_group_param.bulk_chunk_size_per_prefix); } static inline void initSaiRedisCounterParameterFromString(sai_s8_list_t &sai_s8_list, const std::string &str) @@ -940,6 +941,7 @@ void setFlexCounterGroupParameter(const string &group, attr.value.ptr = &flex_counter_group_param; initSaiRedisCounterEmptyParameter(flex_counter_group_param.bulk_chunk_size); + initSaiRedisCounterEmptyParameter(flex_counter_group_param.bulk_chunk_size_per_prefix); initSaiRedisCounterParameterFromString(flex_counter_group_param.counter_group_name, group); initSaiRedisCounterParameterFromString(flex_counter_group_param.poll_interval, poll_interval); initSaiRedisCounterParameterFromString(flex_counter_group_param.operation, operation); @@ -1036,6 +1038,23 @@ void setFlexCounterGroupBulkChunkSize(const std::string &group, notifySyncdCounterOperation(is_gearbox, attr); } +void setFlexCounterGroupBulkChunkSizePerPrefix(const std::string &group, + const std::string &bulk_chunk_size_per_prefix, + bool is_gearbox) +{ + sai_attribute_t attr; + sai_redis_flex_counter_group_parameter_t flex_counter_group_param; + + attr.id = SAI_REDIS_SWITCH_ATTR_FLEX_COUNTER_GROUP; + attr.value.ptr = &flex_counter_group_param; + + initSaiRedisCounterEmptyParameter(flex_counter_group_param); + initSaiRedisCounterParameterFromString(flex_counter_group_param.counter_group_name, group); + initSaiRedisCounterParameterFromString(flex_counter_group_param.bulk_chunk_size_per_prefix, bulk_chunk_size_per_prefix); + + notifySyncdCounterOperation(is_gearbox, attr); +} + void delFlexCounterGroup(const std::string &group, bool is_gearbox) { diff --git a/orchagent/saihelper.h b/orchagent/saihelper.h index 8e1223f1ac..3ba335fd6e 100644 --- a/orchagent/saihelper.h +++ b/orchagent/saihelper.h @@ -43,6 +43,10 @@ void setFlexCounterGroupBulkChunkSize(const std::string &group, const std::string &bulk_size, bool is_gearbox); +void setFlexCounterGroupBulkChunkSizePerPrefix(const std::string &group, + const std::string &bulk_chunk_size_per_prefix, + bool is_gearbox); + void delFlexCounterGroup(const std::string &group, bool is_gearbox=false); From 3854bbf9118b124a2ac6e7fed4b5aa457162b9f6 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Mon, 23 Dec 2024 08:56:20 +0000 Subject: [PATCH 07/10] Support remove bulk chunk size Signed-off-by: Stephen Sun --- orchagent/flexcounterorch.cpp | 14 ++++++++++++-- orchagent/saihelper.h | 4 ++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 4c7d601794..697ec6b302 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -118,6 +118,8 @@ void FlexCounterOrch::doTask(Consumer &consumer) { auto itDelay = std::find(std::begin(data), std::end(data), FieldValueTuple(FLEX_COUNTER_DELAY_STATUS_FIELD, "true")); string poll_interval; + string bulk_chunk_size = "NULL"; + string bulk_chunk_size_per_counter = "NULL"; if (itDelay != data.end()) { @@ -253,17 +255,25 @@ void FlexCounterOrch::doTask(Consumer &consumer) } else if (field == BULK_CHUNK_SIZE_FIELD) { - setFlexCounterGroupBulkChunkSize(flexCounterGroupMap[key], value, false); + bulk_chunk_size = value; } else if (field == BULK_CHUNK_SIZE_PER_PREFIX_FIELD) { - setFlexCounterGroupBulkChunkSizePerPrefix(flexCounterGroupMap[key], value, false); + bulk_chunk_size_per_counter = value; } else { SWSS_LOG_NOTICE("Unsupported field %s", field.c_str()); } } + + /* + * The bulk chunk sizes are stored in sairedis but not in orchagent. + * If user sets it and then removes it for a counter group without removing the counter group, + * the Flex Counter orch just receives a SET command without bulk chunk sizes field. + * To make it easy, we always send the fields to sairedis so that sairedis can handle removing flow*/ + setFlexCounterGroupBulkChunkSize(flexCounterGroupMap[key], bulk_chunk_size); + setFlexCounterGroupBulkChunkSizePerPrefix(flexCounterGroupMap[key], bulk_chunk_size_per_counter); } consumer.m_toSync.erase(it++); diff --git a/orchagent/saihelper.h b/orchagent/saihelper.h index 3ba335fd6e..f2dfbe37aa 100644 --- a/orchagent/saihelper.h +++ b/orchagent/saihelper.h @@ -41,11 +41,11 @@ void setFlexCounterGroupStatsMode(const std::string &group, void setFlexCounterGroupBulkChunkSize(const std::string &group, const std::string &bulk_size, - bool is_gearbox); + bool is_gearbox=false); void setFlexCounterGroupBulkChunkSizePerPrefix(const std::string &group, const std::string &bulk_chunk_size_per_prefix, - bool is_gearbox); + bool is_gearbox=false); void delFlexCounterGroup(const std::string &group, bool is_gearbox=false); From ef4030557f2d393fa845987eedb00e51fa8687d9 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Wed, 25 Dec 2024 13:57:49 +0000 Subject: [PATCH 08/10] Fix ut Signed-off-by: Stephen Sun --- tests/mock_tests/flexcounter_ut.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/mock_tests/flexcounter_ut.cpp b/tests/mock_tests/flexcounter_ut.cpp index fa3b62e795..64b2e5b09c 100644 --- a/tests/mock_tests/flexcounter_ut.cpp +++ b/tests/mock_tests/flexcounter_ut.cpp @@ -111,6 +111,10 @@ namespace flexcounter_test } else { + if (flexCounterGroupParam->bulk_chunk_size.list != nullptr || flexCounterGroupParam->bulk_chunk_size_per_prefix.list != nullptr) + { + return SAI_STATUS_SUCCESS; + } mockFlexCounterGroupTable->del(key); } From 3c79f137ff8314dfabffdf2bf2546f83f114c6aa Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Wed, 25 Dec 2024 13:58:50 +0000 Subject: [PATCH 09/10] Re-organize the code to avoid conflict in cherry-pick Signed-off-by: Stephen Sun --- orchagent/flexcounterorch.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 697ec6b302..6c2b2c8341 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -143,6 +143,14 @@ void FlexCounterOrch::doTask(Consumer &consumer) } } } + else if (field == BULK_CHUNK_SIZE_FIELD) + { + bulk_chunk_size = value; + } + else if (field == BULK_CHUNK_SIZE_PER_PREFIX_FIELD) + { + bulk_chunk_size_per_counter = value; + } else if(field == FLEX_COUNTER_STATUS_FIELD) { // Currently, the counters are disabled for polling by default @@ -253,14 +261,6 @@ void FlexCounterOrch::doTask(Consumer &consumer) // If it is exist and the value is 'true' we need to skip the iteration in order to delay the counter creation. // The field will clear out and counter will be created when enable_counters script is called. } - else if (field == BULK_CHUNK_SIZE_FIELD) - { - bulk_chunk_size = value; - } - else if (field == BULK_CHUNK_SIZE_PER_PREFIX_FIELD) - { - bulk_chunk_size_per_counter = value; - } else { SWSS_LOG_NOTICE("Unsupported field %s", field.c_str()); From 894e67176211a5fc04d01533214b3e35d1d16d3b Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Wed, 1 Jan 2025 14:41:49 +0000 Subject: [PATCH 10/10] Simplify the logic to remove bulk chunk size Signed-off-by: Stephen Sun --- orchagent/flexcounterorch.cpp | 23 ++++++++++++++--------- orchagent/flexcounterorch.h | 1 + orchagent/saihelper.cpp | 17 +---------------- orchagent/saihelper.h | 5 +---- 4 files changed, 17 insertions(+), 29 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 6c2b2c8341..6fa37f4950 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -118,8 +118,8 @@ void FlexCounterOrch::doTask(Consumer &consumer) { auto itDelay = std::find(std::begin(data), std::end(data), FieldValueTuple(FLEX_COUNTER_DELAY_STATUS_FIELD, "true")); string poll_interval; - string bulk_chunk_size = "NULL"; - string bulk_chunk_size_per_counter = "NULL"; + string bulk_chunk_size; + string bulk_chunk_size_per_counter; if (itDelay != data.end()) { @@ -267,13 +267,18 @@ void FlexCounterOrch::doTask(Consumer &consumer) } } - /* - * The bulk chunk sizes are stored in sairedis but not in orchagent. - * If user sets it and then removes it for a counter group without removing the counter group, - * the Flex Counter orch just receives a SET command without bulk chunk sizes field. - * To make it easy, we always send the fields to sairedis so that sairedis can handle removing flow*/ - setFlexCounterGroupBulkChunkSize(flexCounterGroupMap[key], bulk_chunk_size); - setFlexCounterGroupBulkChunkSizePerPrefix(flexCounterGroupMap[key], bulk_chunk_size_per_counter); + if (!bulk_chunk_size.empty() || !bulk_chunk_size_per_counter.empty()) + { + m_groupsWithBulkChunkSize.insert(key); + setFlexCounterGroupBulkChunkSize(flexCounterGroupMap[key], + bulk_chunk_size.empty() ? "NULL" : bulk_chunk_size, + bulk_chunk_size_per_counter.empty() ? "NULL" : bulk_chunk_size_per_counter); + } + else if (m_groupsWithBulkChunkSize.find(key) != m_groupsWithBulkChunkSize.end()) + { + setFlexCounterGroupBulkChunkSize(flexCounterGroupMap[key], "NULL", "NULL"); + m_groupsWithBulkChunkSize.erase(key); + } } consumer.m_toSync.erase(it++); diff --git a/orchagent/flexcounterorch.h b/orchagent/flexcounterorch.h index 4bc74dc3b8..f3aa03e6c0 100644 --- a/orchagent/flexcounterorch.h +++ b/orchagent/flexcounterorch.h @@ -67,6 +67,7 @@ class FlexCounterOrch: public Orch Table m_bufferQueueConfigTable; Table m_bufferPgConfigTable; Table m_deviceMetadataConfigTable; + std::unordered_set m_groupsWithBulkChunkSize; }; #endif diff --git a/orchagent/saihelper.cpp b/orchagent/saihelper.cpp index 0ef3e27285..773db489d6 100644 --- a/orchagent/saihelper.cpp +++ b/orchagent/saihelper.cpp @@ -1023,6 +1023,7 @@ void setFlexCounterGroupStatsMode(const std::string &group, void setFlexCounterGroupBulkChunkSize(const std::string &group, const std::string &bulk_chunk_size, + const std::string &bulk_chunk_size_per_prefix, bool is_gearbox) { sai_attribute_t attr; @@ -1034,22 +1035,6 @@ void setFlexCounterGroupBulkChunkSize(const std::string &group, initSaiRedisCounterEmptyParameter(flex_counter_group_param); initSaiRedisCounterParameterFromString(flex_counter_group_param.counter_group_name, group); initSaiRedisCounterParameterFromString(flex_counter_group_param.bulk_chunk_size, bulk_chunk_size); - - notifySyncdCounterOperation(is_gearbox, attr); -} - -void setFlexCounterGroupBulkChunkSizePerPrefix(const std::string &group, - const std::string &bulk_chunk_size_per_prefix, - bool is_gearbox) -{ - sai_attribute_t attr; - sai_redis_flex_counter_group_parameter_t flex_counter_group_param; - - attr.id = SAI_REDIS_SWITCH_ATTR_FLEX_COUNTER_GROUP; - attr.value.ptr = &flex_counter_group_param; - - initSaiRedisCounterEmptyParameter(flex_counter_group_param); - initSaiRedisCounterParameterFromString(flex_counter_group_param.counter_group_name, group); initSaiRedisCounterParameterFromString(flex_counter_group_param.bulk_chunk_size_per_prefix, bulk_chunk_size_per_prefix); notifySyncdCounterOperation(is_gearbox, attr); diff --git a/orchagent/saihelper.h b/orchagent/saihelper.h index f2dfbe37aa..0406427059 100644 --- a/orchagent/saihelper.h +++ b/orchagent/saihelper.h @@ -41,12 +41,9 @@ void setFlexCounterGroupStatsMode(const std::string &group, void setFlexCounterGroupBulkChunkSize(const std::string &group, const std::string &bulk_size, + const std::string &bulk_chunk_size_per_prefix, bool is_gearbox=false); -void setFlexCounterGroupBulkChunkSizePerPrefix(const std::string &group, - const std::string &bulk_chunk_size_per_prefix, - bool is_gearbox=false); - void delFlexCounterGroup(const std::string &group, bool is_gearbox=false);