Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internal [202205][buffers] Add handler for the 'create_only_config_db_buffers' configuration knob #12

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions orchagent/flexcounterorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ FlexCounterOrch::FlexCounterOrch(DBConnector *db, vector<string> &tableNames):
m_flexCounterConfigTable(db, CFG_FLEX_COUNTER_TABLE_NAME),
m_bufferQueueConfigTable(db, CFG_BUFFER_QUEUE_TABLE_NAME),
m_bufferPgConfigTable(db, CFG_BUFFER_PG_TABLE_NAME),
m_deviceMetadataConfigTable(db, CFG_DEVICE_METADATA_TABLE_NAME),
m_flexCounterDb(new DBConnector("FLEX_COUNTER_DB", 0)),
m_flexCounterGroupTable(new ProducerTable(m_flexCounterDb.get(), FLEX_COUNTER_GROUP_TABLE)),
m_gbflexCounterDb(new DBConnector("GB_FLEX_COUNTER_DB", 0)),
Expand Down Expand Up @@ -329,11 +330,41 @@ bool FlexCounterOrch::bake()
return consumer->addToSync(entries);
}

static bool isCreateOnlyConfigDbBuffers(Table& deviceMetadataConfigTable)
{
std::string createOnlyConfigDbBuffersValue;

try
{
if (deviceMetadataConfigTable.hget("localhost", "create_only_config_db_buffers", createOnlyConfigDbBuffersValue))
{
if (createOnlyConfigDbBuffersValue == "true")
{
return true;
}
}
}
catch(const std::system_error& e)
{
SWSS_LOG_ERROR("System error: %s", e.what());
}

return false;
}

map<string, FlexCounterQueueStates> FlexCounterOrch::getQueueConfigurations()
{
SWSS_LOG_ENTER();

map<string, FlexCounterQueueStates> queuesStateVector;

if (!isCreateOnlyConfigDbBuffers(m_deviceMetadataConfigTable))
{
FlexCounterQueueStates flexCounterQueueState(0);
queuesStateVector.insert(make_pair(createAllAvailableBuffersStr, flexCounterQueueState));
return queuesStateVector;
}

std::vector<std::string> portQueueKeys;
m_bufferQueueConfigTable.getKeys(portQueueKeys);

Expand Down Expand Up @@ -393,6 +424,14 @@ map<string, FlexCounterPgStates> FlexCounterOrch::getPgConfigurations()
SWSS_LOG_ENTER();

map<string, FlexCounterPgStates> pgsStateVector;

if (!isCreateOnlyConfigDbBuffers(m_deviceMetadataConfigTable))
{
FlexCounterPgStates flexCounterPgState(0);
pgsStateVector.insert(make_pair(createAllAvailableBuffersStr, flexCounterPgState));
return pgsStateVector;
}

std::vector<std::string> portPgKeys;
m_bufferPgConfigTable.getKeys(portPgKeys);

Expand Down
3 changes: 3 additions & 0 deletions orchagent/flexcounterorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ extern "C" {
#include "sai.h"
}

const std::string createAllAvailableBuffersStr = "create_all_available_buffers";

class FlexCounterQueueStates
{
public:
Expand Down Expand Up @@ -68,6 +70,7 @@ class FlexCounterOrch: public Orch
Table m_flexCounterConfigTable;
Table m_bufferQueueConfigTable;
Table m_bufferPgConfigTable;
Table m_deviceMetadataConfigTable;
};

#endif
72 changes: 72 additions & 0 deletions orchagent/portsorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6072,6 +6072,14 @@ void PortsOrch::generateQueueMap(map<string, FlexCounterQueueStates> queuesState
return;
}

bool isCreateAllQueues = false;

if (queuesStateVector.count(createAllAvailableBuffersStr))
{
isCreateAllQueues = true;
queuesStateVector.clear();
}

for (const auto& it: m_portList)
{
if (it.second.m_type == Port::PHY)
Expand All @@ -6080,6 +6088,10 @@ void PortsOrch::generateQueueMap(map<string, FlexCounterQueueStates> queuesState
{
auto maxQueueNumber = getNumberOfPortSupportedQueueCounters(it.second.m_alias);
FlexCounterQueueStates flexCounterQueueState(maxQueueNumber);
if (isCreateAllQueues)
{
flexCounterQueueState.enableQueueCounters(0, maxQueueNumber - 1);
}
queuesStateVector.insert(make_pair(it.second.m_alias, flexCounterQueueState));
}
generateQueueMapPerPort(it.second, queuesStateVector.at(it.second.m_alias), false);
Expand Down Expand Up @@ -6198,6 +6210,14 @@ void PortsOrch::addQueueFlexCounters(map<string, FlexCounterQueueStates> queuesS
return;
}

bool isCreateAllQueues = false;

if (queuesStateVector.count(createAllAvailableBuffersStr))
{
isCreateAllQueues = true;
queuesStateVector.clear();
}

for (const auto& it: m_portList)
{
if (it.second.m_type == Port::PHY)
Expand All @@ -6206,6 +6226,10 @@ void PortsOrch::addQueueFlexCounters(map<string, FlexCounterQueueStates> queuesS
{
auto maxQueueNumber = getNumberOfPortSupportedQueueCounters(it.second.m_alias);
FlexCounterQueueStates flexCounterQueueState(maxQueueNumber);
if (isCreateAllQueues)
{
flexCounterQueueState.enableQueueCounters(0, maxQueueNumber - 1);
}
queuesStateVector.insert(make_pair(it.second.m_alias, flexCounterQueueState));
}
addQueueFlexCountersPerPort(it.second, queuesStateVector.at(it.second.m_alias));
Expand Down Expand Up @@ -6263,6 +6287,14 @@ void PortsOrch::addQueueWatermarkFlexCounters(map<string, FlexCounterQueueStates
return;
}

bool isCreateAllQueues = false;

if (queuesStateVector.count(createAllAvailableBuffersStr))
{
isCreateAllQueues = true;
queuesStateVector.clear();
}

for (const auto& it: m_portList)
{
if (it.second.m_type == Port::PHY)
Expand All @@ -6271,6 +6303,10 @@ void PortsOrch::addQueueWatermarkFlexCounters(map<string, FlexCounterQueueStates
{
auto maxQueueNumber = getNumberOfPortSupportedQueueCounters(it.second.m_alias);
FlexCounterQueueStates flexCounterQueueState(maxQueueNumber);
if (isCreateAllQueues)
{
flexCounterQueueState.enableQueueCounters(0, maxQueueNumber - 1);
}
queuesStateVector.insert(make_pair(it.second.m_alias, flexCounterQueueState));
}
addQueueWatermarkFlexCountersPerPort(it.second, queuesStateVector.at(it.second.m_alias));
Expand Down Expand Up @@ -6434,6 +6470,14 @@ void PortsOrch::generatePriorityGroupMap(map<string, FlexCounterPgStates> pgsSta
return;
}

bool isCreateAllPgs = false;

if (pgsStateVector.count(createAllAvailableBuffersStr))
{
isCreateAllPgs = true;
pgsStateVector.clear();
}

for (const auto& it: m_portList)
{
if (it.second.m_type == Port::PHY)
Expand All @@ -6442,6 +6486,10 @@ void PortsOrch::generatePriorityGroupMap(map<string, FlexCounterPgStates> pgsSta
{
auto maxPgNumber = getNumberOfPortSupportedPgCounters(it.second.m_alias);
FlexCounterPgStates flexCounterPgState(maxPgNumber);
if (isCreateAllPgs)
{
flexCounterPgState.enablePgCounters(0, maxPgNumber - 1);
}
pgsStateVector.insert(make_pair(it.second.m_alias, flexCounterPgState));
}
generatePriorityGroupMapPerPort(it.second, pgsStateVector.at(it.second.m_alias));
Expand Down Expand Up @@ -6538,6 +6586,14 @@ void PortsOrch::addPriorityGroupFlexCounters(map<string, FlexCounterPgStates> pg
return;
}

bool isCreateAllPgs = false;

if (pgsStateVector.count(createAllAvailableBuffersStr))
{
isCreateAllPgs = true;
pgsStateVector.clear();
}

for (const auto& it: m_portList)
{
if (it.second.m_type == Port::PHY)
Expand All @@ -6546,6 +6602,10 @@ void PortsOrch::addPriorityGroupFlexCounters(map<string, FlexCounterPgStates> pg
{
auto maxPgNumber = getNumberOfPortSupportedPgCounters(it.second.m_alias);
FlexCounterPgStates flexCounterPgState(maxPgNumber);
if (isCreateAllPgs)
{
flexCounterPgState.enablePgCounters(0, maxPgNumber - 1);
}
pgsStateVector.insert(make_pair(it.second.m_alias, flexCounterPgState));
}
addPriorityGroupFlexCountersPerPort(it.second, pgsStateVector.at(it.second.m_alias));
Expand Down Expand Up @@ -6595,6 +6655,14 @@ void PortsOrch::addPriorityGroupWatermarkFlexCounters(map<string, FlexCounterPgS
return;
}

bool isCreateAllPgs = false;

if (pgsStateVector.count(createAllAvailableBuffersStr))
{
isCreateAllPgs = true;
pgsStateVector.clear();
}

for (const auto& it: m_portList)
{
if (it.second.m_type == Port::PHY)
Expand All @@ -6603,6 +6671,10 @@ void PortsOrch::addPriorityGroupWatermarkFlexCounters(map<string, FlexCounterPgS
{
auto maxPgNumber = getNumberOfPortSupportedPgCounters(it.second.m_alias);
FlexCounterPgStates flexCounterPgState(maxPgNumber);
if (isCreateAllPgs)
{
flexCounterPgState.enablePgCounters(0, maxPgNumber - 1);
}
pgsStateVector.insert(make_pair(it.second.m_alias, flexCounterPgState));
}
addPriorityGroupWatermarkFlexCountersPerPort(it.second, pgsStateVector.at(it.second.m_alias));
Expand Down
58 changes: 53 additions & 5 deletions tests/test_flex_counters.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@ def set_flex_counter_group_interval(self, key, group, interval):
self.config_db.create_entry("FLEX_COUNTER_TABLE", key, group_stats_entry)
self.wait_for_interval_set(group, interval)

def set_only_config_db_buffers_field(self, value):
fvs = {'create_only_config_db_buffers' : value}
self.config_db.update_entry("DEVICE_METADATA", "localhost", fvs)

@pytest.mark.parametrize("counter_type", counter_group_meta.keys())
def test_flex_counters(self, dvs, counter_type):
"""
Expand Down Expand Up @@ -712,19 +716,43 @@ def remove_ip_address(self, interface, ip):
def set_admin_status(self, interface, status):
self.config_db.update_entry("PORT", interface, {"admin_status": status})

@pytest.mark.parametrize('counter_type', [('queue_counter'), ('pg_drop_counter')])
def test_create_only_config_db_buffers_false(self, dvs, counter_type):
"""
Test steps:
1. By default the configuration knob 'create_only_config_db_value' is missing.
2. Get the counter OID for the interface 'Ethernet0:7' from the counters database.
3. Perform assertions based on the 'create_only_config_db_value':
- If 'create_only_config_db_value' is 'false' or does not exist, assert that the counter OID has a valid OID value.

Args:
dvs (object): virtual switch object
counter_type (str): The type of counter being tested
"""
self.setup_dbs(dvs)
meta_data = counter_group_meta[counter_type]
self.set_flex_counter_group_status(meta_data['key'], meta_data['name_map'])

counter_oid = self.counters_db.db_connection.hget(meta_data['name_map'], 'Ethernet0:7')
assert counter_oid is not None, "Counter OID should have a valid OID value when create_only_config_db_value is 'false' or does not exist"

def test_create_remove_buffer_pg_watermark_counter(self, dvs):
"""
Test steps:
1. Enable PG flex counters.
2. Configure new buffer prioriy group for a port
3. Verify counter is automatically created
4. Remove the new buffer prioriy group for the port
5. Verify counter is automatically removed
1. Reset config_db
2. Set 'create_only_config_db_buffers' to 'true'
3. Enable PG flex counters.
4. Configure new buffer prioriy group for a port
5. Verify counter is automatically created
6. Remove the new buffer prioriy group for the port
7. Verify counter is automatically removed

Args:
dvs (object): virtual switch object
"""
dvs.restart()
self.setup_dbs(dvs)
self.set_only_config_db_buffers_field('true')
meta_data = counter_group_meta['pg_watermark_counter']

self.set_flex_counter_group_status(meta_data['key'], meta_data['name_map'])
Expand All @@ -737,6 +765,26 @@ def test_create_remove_buffer_pg_watermark_counter(self, dvs):
self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '1', False)
self.wait_for_id_list_remove(meta_data['group_name'], "Ethernet0", counter_oid)

@pytest.mark.parametrize('counter_type', [('queue_counter'), ('pg_drop_counter')])
def test_create_only_config_db_buffers_true(self, dvs, counter_type):
"""
Test steps:
1. The 'create_only_config_db_buffers' was set to 'true' by previous test.
2. Get the counter OID for the interface 'Ethernet0:7' from the counters database.
3. Perform assertions based on the 'create_only_config_db_value':
- If 'create_only_config_db_value' is 'true', assert that the counter OID is None.

Args:
dvs (object): virtual switch object
counter_type (str): The type of counter being tested
"""
self.setup_dbs(dvs)
meta_data = counter_group_meta[counter_type]
self.set_flex_counter_group_status(meta_data['key'], meta_data['name_map'])

counter_oid = self.counters_db.db_connection.hget(meta_data['name_map'], 'Ethernet0:7')
assert counter_oid is None, "Counter OID should be None when create_only_config_db_value is 'true'"

def test_create_remove_buffer_queue_counter(self, dvs):
"""
Test steps:
Expand Down