From e23b6046d84e33d456b391b5bb8482b4a693025d Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Thu, 7 Nov 2024 07:15:44 +0000 Subject: [PATCH 1/3] Do not start counter polling for queues with zero buffer profiles Signed-off-by: Stephen Sun --- orchagent/bufferorch.cpp | 40 ++++++++++++++++++++++++++++++----- orchagent/bufferorch.h | 1 + orchagent/flexcounterorch.cpp | 4 ++-- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/orchagent/bufferorch.cpp b/orchagent/bufferorch.cpp index ccbc8e9b25..3570644095 100644 --- a/orchagent/bufferorch.cpp +++ b/orchagent/bufferorch.cpp @@ -797,6 +797,9 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple) sai_uint32_t range_low, range_high; bool need_update_sai = true; bool local_port = false; + bool counter_was_added = false; + bool counter_needs_to_add = false; + string old_buffer_profile_name; string local_port_name; SWSS_LOG_DEBUG("Processing:%s", key.c_str()); @@ -856,7 +859,6 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple) return task_process_status::task_failed; } - string old_buffer_profile_name; if (doesObjectExist(m_buffer_type_maps, APP_BUFFER_QUEUE_TABLE_NAME, key, buffer_profile_field_name, old_buffer_profile_name) && (old_buffer_profile_name == buffer_profile_name)) { @@ -874,11 +876,14 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple) SWSS_LOG_NOTICE("Set buffer queue %s to %s", key.c_str(), buffer_profile_name.c_str()); setObjectReference(m_buffer_type_maps, APP_BUFFER_QUEUE_TABLE_NAME, key, buffer_profile_field_name, buffer_profile_name); + + // Counter operation + counter_needs_to_add = buffer_profile_name.find("_zero_") == std::string::npos; + SWSS_LOG_INFO("%s to create counter for %s with new profile %s", counter_needs_to_add ? "Need" : "No need", key.c_str(), buffer_profile_name.c_str()); } else if (op == DEL_COMMAND) { - auto &typemap = (*m_buffer_type_maps[APP_BUFFER_QUEUE_TABLE_NAME]); - if (typemap.find(key) == typemap.end()) + if (!doesObjectExist(m_buffer_type_maps, APP_BUFFER_QUEUE_TABLE_NAME, key, buffer_profile_field_name, old_buffer_profile_name)) { SWSS_LOG_INFO("%s doesn't not exist, don't need to notfiy SAI", key.c_str()); need_update_sai = false; @@ -887,6 +892,7 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple) SWSS_LOG_NOTICE("Remove buffer queue %s", key.c_str()); removeObject(m_buffer_type_maps, APP_BUFFER_QUEUE_TABLE_NAME, key); m_partiallyAppliedQueues.erase(key); + counter_needs_to_add = false; } else { @@ -894,6 +900,9 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple) return task_process_status::task_invalid_entry; } + counter_was_added = !old_buffer_profile_name.empty() && old_buffer_profile_name.find("_zero_") == std::string::npos; + SWSS_LOG_INFO("%s to remove counter for %s with old profile %s", counter_was_added ? "Need" : "No need", key.c_str(), old_buffer_profile_name.c_str()); + sai_attribute_t attr; attr.id = SAI_QUEUE_ATTR_BUFFER_PROFILE_ID; attr.value.oid = sai_buffer_profile; @@ -963,14 +972,16 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple) { auto flexCounterOrch = gDirectory.get(); auto queues = tokens[1]; - if (op == SET_COMMAND && + if (!counter_was_added && counter_needs_to_add && (flexCounterOrch->getQueueCountersState() || flexCounterOrch->getQueueWatermarkCountersState())) { + SWSS_LOG_INFO("Creating counters for %s %lu", port_name.c_str(), ind); gPortsOrch->createPortBufferQueueCounters(port, queues); } - else if (op == DEL_COMMAND && + else if (counter_was_added && !counter_needs_to_add && (flexCounterOrch->getQueueCountersState() || flexCounterOrch->getQueueWatermarkCountersState())) { + SWSS_LOG_INFO("Removing counters for %s %lu", port_name.c_str(), ind); gPortsOrch->removePortBufferQueueCounters(port, queues); } } @@ -1044,6 +1055,25 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple) return task_process_status::task_success; } +void BufferOrch::getNonZeroQueues(vector &nonZeroQueues) +{ + for (auto &&queueRef: (*m_buffer_type_maps[APP_BUFFER_QUEUE_TABLE_NAME])) + { + for (auto &&profileRef: queueRef.second.m_objsReferencingByMe) + { + if (profileRef.second.find("_zero_") == std::string::npos) + { + SWSS_LOG_INFO("Selected key %s with profile %s", queueRef.first.c_str(), profileRef.second.c_str()); + nonZeroQueues.push_back(queueRef.first); + } + else + { + SWSS_LOG_INFO("Skipped key %s with profile %s", queueRef.first.c_str(), profileRef.second.c_str()); + } + } + } +} + /* Input sample "BUFFER_PG|Ethernet4,Ethernet45|10-15" */ diff --git a/orchagent/bufferorch.h b/orchagent/bufferorch.h index f78fe05318..7f62e8ffb3 100644 --- a/orchagent/bufferorch.h +++ b/orchagent/bufferorch.h @@ -38,6 +38,7 @@ class BufferOrch : public Orch static type_map m_buffer_type_maps; void generateBufferPoolWatermarkCounterIdList(void); const object_reference_map &getBufferPoolNameOidMap(void); + void getNonZeroQueues(std::vector &nonZeroQueues); private: typedef task_process_status (BufferOrch::*buffer_table_handler)(KeyOpFieldsValuesTuple &tuple); diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 705622bfa0..a70d6fe8e5 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -370,11 +370,11 @@ map FlexCounterOrch::getQueueConfigurations() } std::vector portQueueKeys; - m_bufferQueueConfigTable.getKeys(portQueueKeys); + gBufferOrch->getNonZeroQueues(portQueueKeys); for (const auto& portQueueKey : portQueueKeys) { - auto toks = tokenize(portQueueKey, '|'); + auto toks = tokenize(portQueueKey, ':'); if (toks.size() != 2) { SWSS_LOG_ERROR("Invalid BUFFER_QUEUE key: [%s]", portQueueKey.c_str()); From b8f3a7971e971dbe08f7ead5ada481257c18bd62 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Thu, 7 Nov 2024 08:25:25 +0000 Subject: [PATCH 2/3] Do not poll counters on priority groups with zero buffer profile as well Signed-off-by: Stephen Sun --- orchagent/bufferorch.cpp | 55 +++++++++++++++++++++-------------- orchagent/bufferorch.h | 2 +- orchagent/flexcounterorch.cpp | 6 ++-- 3 files changed, 37 insertions(+), 26 deletions(-) diff --git a/orchagent/bufferorch.cpp b/orchagent/bufferorch.cpp index 3570644095..795b3ef49a 100644 --- a/orchagent/bufferorch.cpp +++ b/orchagent/bufferorch.cpp @@ -360,6 +360,25 @@ const object_reference_map &BufferOrch::getBufferPoolNameOidMap(void) return *m_buffer_type_maps[APP_BUFFER_POOL_TABLE_NAME]; } +void BufferOrch::getBufferObjectsWithNonZeroProfile(vector &nonZeroQueues, const string &table) +{ + for (auto &&queueRef: (*m_buffer_type_maps[table])) + { + for (auto &&profileRef: queueRef.second.m_objsReferencingByMe) + { + if (profileRef.second.find("_zero_") == std::string::npos) + { + SWSS_LOG_INFO("Selected key %s with profile %s", queueRef.first.c_str(), profileRef.second.c_str()); + nonZeroQueues.push_back(queueRef.first); + } + else + { + SWSS_LOG_INFO("Skipped key %s with profile %s", queueRef.first.c_str(), profileRef.second.c_str()); + } + } + } +} + task_process_status BufferOrch::processBufferPool(KeyOpFieldsValuesTuple &tuple) { SWSS_LOG_ENTER(); @@ -1055,25 +1074,6 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple) return task_process_status::task_success; } -void BufferOrch::getNonZeroQueues(vector &nonZeroQueues) -{ - for (auto &&queueRef: (*m_buffer_type_maps[APP_BUFFER_QUEUE_TABLE_NAME])) - { - for (auto &&profileRef: queueRef.second.m_objsReferencingByMe) - { - if (profileRef.second.find("_zero_") == std::string::npos) - { - SWSS_LOG_INFO("Selected key %s with profile %s", queueRef.first.c_str(), profileRef.second.c_str()); - nonZeroQueues.push_back(queueRef.first); - } - else - { - SWSS_LOG_INFO("Skipped key %s with profile %s", queueRef.first.c_str(), profileRef.second.c_str()); - } - } - } -} - /* Input sample "BUFFER_PG|Ethernet4,Ethernet45|10-15" */ @@ -1087,6 +1087,9 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup vector tokens; sai_uint32_t range_low, range_high; bool need_update_sai = true; + bool counter_was_added = false; + bool counter_needs_to_add = false; + string old_buffer_profile_name; SWSS_LOG_DEBUG("processing:%s", key.c_str()); tokens = tokenize(key, delimiter); @@ -1119,7 +1122,6 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup return task_process_status::task_failed; } - string old_buffer_profile_name; if (doesObjectExist(m_buffer_type_maps, APP_BUFFER_PG_TABLE_NAME, key, buffer_profile_field_name, old_buffer_profile_name) && (old_buffer_profile_name == buffer_profile_name)) { @@ -1130,6 +1132,10 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup SWSS_LOG_NOTICE("Set buffer PG %s to %s", key.c_str(), buffer_profile_name.c_str()); setObjectReference(m_buffer_type_maps, APP_BUFFER_PG_TABLE_NAME, key, buffer_profile_field_name, buffer_profile_name); + + // Counter operation + counter_needs_to_add = buffer_profile_name.find("_zero_") == std::string::npos; + SWSS_LOG_INFO("%s to create counter for priority group %s with new profile %s", counter_needs_to_add ? "Need" : "No need", key.c_str(), buffer_profile_name.c_str()); } else if (op == DEL_COMMAND) { @@ -1149,6 +1155,9 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup return task_process_status::task_invalid_entry; } + counter_was_added = !old_buffer_profile_name.empty() && old_buffer_profile_name.find("_zero_") == std::string::npos; + SWSS_LOG_INFO("%s to remove counter for priority group %s with old profile %s", counter_was_added ? "Need" : "No need", key.c_str(), old_buffer_profile_name.c_str()); + sai_attribute_t attr; attr.id = SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE; attr.value.oid = sai_buffer_profile; @@ -1191,14 +1200,16 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup { auto flexCounterOrch = gDirectory.get(); auto pgs = tokens[1]; - if (op == SET_COMMAND && + if (!counter_was_added && counter_needs_to_add && (flexCounterOrch->getPgCountersState() || flexCounterOrch->getPgWatermarkCountersState())) { + SWSS_LOG_INFO("Creating counters for priority group %s %lu", port_name.c_str(), ind); gPortsOrch->createPortBufferPgCounters(port, pgs); } - else if (op == DEL_COMMAND && + else if (counter_was_added && !counter_needs_to_add && (flexCounterOrch->getPgCountersState() || flexCounterOrch->getPgWatermarkCountersState())) { + SWSS_LOG_INFO("Removing counters for priority group %s %lu", port_name.c_str(), ind); gPortsOrch->removePortBufferPgCounters(port, pgs); } } diff --git a/orchagent/bufferorch.h b/orchagent/bufferorch.h index 7f62e8ffb3..3d255b87dd 100644 --- a/orchagent/bufferorch.h +++ b/orchagent/bufferorch.h @@ -38,7 +38,7 @@ class BufferOrch : public Orch static type_map m_buffer_type_maps; void generateBufferPoolWatermarkCounterIdList(void); const object_reference_map &getBufferPoolNameOidMap(void); - void getNonZeroQueues(std::vector &nonZeroQueues); + void getBufferObjectsWithNonZeroProfile(vector &nonZeroQueues, const string &table); private: typedef task_process_status (BufferOrch::*buffer_table_handler)(KeyOpFieldsValuesTuple &tuple); diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index a70d6fe8e5..a19fcb5853 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -370,7 +370,7 @@ map FlexCounterOrch::getQueueConfigurations() } std::vector portQueueKeys; - gBufferOrch->getNonZeroQueues(portQueueKeys); + gBufferOrch->getBufferObjectsWithNonZeroProfile(portQueueKeys, APP_BUFFER_QUEUE_TABLE_NAME); for (const auto& portQueueKey : portQueueKeys) { @@ -432,11 +432,11 @@ map FlexCounterOrch::getPgConfigurations() } std::vector portPgKeys; - m_bufferPgConfigTable.getKeys(portPgKeys); + gBufferOrch->getBufferObjectsWithNonZeroProfile(portPgKeys, APP_BUFFER_PG_TABLE_NAME); for (const auto& portPgKey : portPgKeys) { - auto toks = tokenize(portPgKey, '|'); + auto toks = tokenize(portPgKey, ':'); if (toks.size() != 2) { SWSS_LOG_ERROR("Invalid BUFFER_PG key: [%s]", portPgKey.c_str()); From 960f9eb8b145581e3722dc8efac22d28e01fe9d4 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Mon, 11 Nov 2024 00:31:20 +0000 Subject: [PATCH 3/3] Fix issue: old profile is not fetched in delete command Signed-off-by: Stephen Sun --- orchagent/bufferorch.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/orchagent/bufferorch.cpp b/orchagent/bufferorch.cpp index 795b3ef49a..836ad0155c 100644 --- a/orchagent/bufferorch.cpp +++ b/orchagent/bufferorch.cpp @@ -1139,8 +1139,7 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup } else if (op == DEL_COMMAND) { - auto &typemap = (*m_buffer_type_maps[APP_BUFFER_PG_TABLE_NAME]); - if (typemap.find(key) == typemap.end()) + if (!doesObjectExist(m_buffer_type_maps, APP_BUFFER_PG_TABLE_NAME, key, buffer_profile_field_name, old_buffer_profile_name)) { SWSS_LOG_INFO("%s doesn't not exist, don't need to notfiy SAI", key.c_str()); need_update_sai = false;