Skip to content

Commit

Permalink
Do not start counter polling for queues with zero buffer profiles if …
Browse files Browse the repository at this point in the history
…create_only_config_db_buffers is enabled (sonic-net#3360)

What I did

Do not start counter-polling for queues with zero buffer profiles if create_only_config_db_buffers is enabled.

Why I did it

If create_only_config_db_buffers is enabled, counters will be polled only on queues with a buffer configured based on the hypothesis that a customer must configure a buffer profile before using the queue.
The system performance can be optimized by reducing the number of counter polled.

However, zero buffer profiles are configured on queues of inactive ports to reclaim unused buffers. Those queues are not used by customers. Hence, we do not need to start counter polling for queues with such buffer profiles configured.
  • Loading branch information
stephenxs authored Dec 10, 2024
1 parent 90612d2 commit fa2b530
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 14 deletions.
60 changes: 50 additions & 10 deletions orchagent/bufferorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> &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();
Expand Down Expand Up @@ -797,6 +816,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());
Expand Down Expand Up @@ -856,7 +878,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))
{
Expand All @@ -874,11 +895,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;
Expand All @@ -887,13 +911,17 @@ 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
{
SWSS_LOG_ERROR("Unknown operation type %s", op.c_str());
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;
Expand Down Expand Up @@ -963,14 +991,16 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple)
{
auto flexCounterOrch = gDirectory.get<FlexCounterOrch*>();
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 %zd", 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 %zd", port_name.c_str(), ind);
gPortsOrch->removePortBufferQueueCounters(port, queues);
}
}
Expand Down Expand Up @@ -1057,6 +1087,9 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup
vector<string> 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);
Expand Down Expand Up @@ -1089,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))
{
Expand All @@ -1100,11 +1132,14 @@ 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)
{
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;
Expand All @@ -1119,6 +1154,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;
Expand Down Expand Up @@ -1161,14 +1199,16 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup
{
auto flexCounterOrch = gDirectory.get<FlexCounterOrch*>();
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 %zd", 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 %zd", port_name.c_str(), ind);
gPortsOrch->removePortBufferPgCounters(port, pgs);
}
}
Expand Down
1 change: 1 addition & 0 deletions orchagent/bufferorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 getBufferObjectsWithNonZeroProfile(vector<string> &nonZeroQueues, const string &table);

private:
typedef task_process_status (BufferOrch::*buffer_table_handler)(KeyOpFieldsValuesTuple &tuple);
Expand Down
8 changes: 4 additions & 4 deletions orchagent/flexcounterorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,11 +370,11 @@ map<string, FlexCounterQueueStates> FlexCounterOrch::getQueueConfigurations()
}

std::vector<std::string> portQueueKeys;
m_bufferQueueConfigTable.getKeys(portQueueKeys);
gBufferOrch->getBufferObjectsWithNonZeroProfile(portQueueKeys, APP_BUFFER_QUEUE_TABLE_NAME);

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());
Expand Down Expand Up @@ -439,11 +439,11 @@ map<string, FlexCounterPgStates> FlexCounterOrch::getPgConfigurations()
}

std::vector<std::string> 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());
Expand Down

0 comments on commit fa2b530

Please sign in to comment.