Skip to content

Commit

Permalink
[buffers] Add handler for the 'create_only_config_db_buffers' configu…
Browse files Browse the repository at this point in the history 
…ration knob (sonic-net#2883)

* [buffers] Add handler for "create_only_config_db_buffers" configuration knob

If the "create_only_config_db_buffers" is equal to "true" - buffers will be created according to the config_db configuration (for example BUFFER_QUEUE|* table).
If the "create_only_config_db_buffers" is equal to "false" or does not exist - the maximum available buffers (which are read from SAI) will be created, regardless of the config_db buffer config.
  • Loading branch information
@vadymhlushko-mlnx
vadymhlushko-mlnx authored Oct 4, 2023
1 parent 7f7bc33 commit 91e7a27
Show file tree
Hide file tree
Showing 5 changed files with 169 additions and 6 deletions.
39 changes: 39 additions & 0 deletions orchagent/flexcounterorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,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 @@ -328,11 +329,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 @@ -387,6 +418,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
View file
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
3 changes: 2 additions & 1 deletion orchagent/p4orch/tests/fake_flexcounterorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ FlexCounterOrch::FlexCounterOrch(swss::DBConnector *db, std::vector<std::string>
Orch(db, 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_bufferPgConfigTable(db, CFG_BUFFER_PG_TABLE_NAME),
m_deviceMetadataConfigTable(db, CFG_DEVICE_METADATA_TABLE_NAME)
{
}

Expand Down
72 changes: 72 additions & 0 deletions orchagent/portsorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6333,6 +6333,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 @@ -6341,6 +6349,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 @@ -6459,6 +6471,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 @@ -6467,6 +6487,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 @@ -6524,6 +6548,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 @@ -6532,6 +6564,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 @@ -6695,6 +6731,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 @@ -6703,6 +6747,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 @@ -6799,6 +6847,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 @@ -6807,6 +6863,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 @@ -6856,6 +6916,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 @@ -6864,6 +6932,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
View file
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

0 comments on commit 91e7a27

Please sign in to comment.