Skip to content

Commit f421397

Browse files
stephenxsmssonicbld
authored andcommitted
Create counter for the queue to which the host CPU traffic is sent when create_only_config_db_buffers is enabled (#3334)
Create counter for the queue to which the host CPU traffic is sent when create_only_config_db_buffers is enabled *There is a configuration to optimize the buffer performance DEVICE_METADATA|localhost.create_only_config_db_buffers. Using this configuration the buffer counters are created only for the queues and PGs with buffer configured for performance optimization. However, to have counters for CPU tx queue, we have to configure it in BUFFER_QUEUE, mainly for data traffic, which is a bad coupling. Using this PR the counter will be created for CPU tx queue without data buffer configuration.
1 parent fbe52b1 commit f421397

File tree

6 files changed

+71
-22
lines changed

6 files changed

+71
-22
lines changed

orchagent/flexcounterorch.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,13 @@ map<string, FlexCounterQueueStates> FlexCounterOrch::getQueueConfigurations()
400400
{
401401
queuesStateVector.at(configPortName).enableQueueCounter(startIndex);
402402
}
403+
404+
Port port;
405+
gPortsOrch->getPort(configPortName, port);
406+
if (port.m_host_tx_queue_configured && port.m_host_tx_queue <= maxQueueIndex)
407+
{
408+
queuesStateVector.at(configPortName).enableQueueCounter(port.m_host_tx_queue);
409+
}
403410
} catch (std::invalid_argument const& e) {
404411
SWSS_LOG_ERROR("Invalid queue index [%s] for port [%s]", configPortQueues.c_str(), configPortName.c_str());
405412
continue;

orchagent/p4orch/tests/fake_portorch.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,11 +189,11 @@ void PortsOrch::generateQueueMapPerPort(const Port &port, FlexCounterQueueStates
189189
{
190190
}
191191

192-
void PortsOrch::createPortBufferQueueCounters(const Port &port, string queues)
192+
void PortsOrch::createPortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue)
193193
{
194194
}
195195

196-
void PortsOrch::removePortBufferQueueCounters(const Port &port, string queues)
196+
void PortsOrch::removePortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue)
197197
{
198198
}
199199

orchagent/port.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,8 @@ class Port
162162
sai_port_priority_flow_control_mode_t m_pfc_asym = SAI_PORT_PRIORITY_FLOW_CONTROL_MODE_COMBINED;
163163
uint8_t m_pfc_bitmask = 0; // PFC enable bit mask
164164
uint8_t m_pfcwd_sw_bitmask = 0; // PFC software watchdog enable
165+
uint8_t m_host_tx_queue = 0;
166+
bool m_host_tx_queue_configured = false;
165167
uint16_t m_tpid = DEFAULT_TPID;
166168
uint32_t m_nat_zone_id = 0;
167169
uint32_t m_vnid = VNID_NONE;

orchagent/portsorch.cpp

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3510,6 +3510,12 @@ bool PortsOrch::initPort(const PortConfig &port)
35103510
m_recircPortRole[alias] = role;
35113511
}
35123512

3513+
// We have to test the size of m_queue_ids here since it isn't initialized on some platforms (like DPU)
3514+
if (p.m_host_tx_queue_configured && p.m_queue_ids.size() > p.m_host_tx_queue)
3515+
{
3516+
createPortBufferQueueCounters(p, to_string(p.m_host_tx_queue), false);
3517+
}
3518+
35133519
SWSS_LOG_NOTICE("Initialized port %s", alias.c_str());
35143520
}
35153521
else
@@ -3540,6 +3546,11 @@ void PortsOrch::deInitPort(string alias, sai_object_id_t port_id)
35403546
return;
35413547
}
35423548

3549+
if (p.m_host_tx_queue_configured && p.m_queue_ids.size() > p.m_host_tx_queue)
3550+
{
3551+
removePortBufferQueueCounters(p, to_string(p.m_host_tx_queue), false);
3552+
}
3553+
35433554
/* remove port from flex_counter_table for updating counters */
35443555
auto flex_counters_orch = gDirectory.get<FlexCounterOrch*>();
35453556
if ((flex_counters_orch->getPortCountersState()))
@@ -5951,6 +5962,9 @@ bool PortsOrch::addHostIntfs(Port &port, string alias, sai_object_id_t &host_int
59515962
attr.id = SAI_HOSTIF_ATTR_QUEUE;
59525963
attr.value.u32 = DEFAULT_HOSTIF_TX_QUEUE;
59535964
attrs.push_back(attr);
5965+
5966+
port.m_host_tx_queue = DEFAULT_HOSTIF_TX_QUEUE;
5967+
port.m_host_tx_queue_configured = true;
59545968
}
59555969

59565970
sai_status_t status = sai_hostif_api->create_hostif(&host_intfs_id, gSwitchId, (uint32_t)attrs.size(), attrs.data());
@@ -7258,6 +7272,10 @@ void PortsOrch::generateQueueMap(map<string, FlexCounterQueueStates> queuesState
72587272
{
72597273
flexCounterQueueState.enableQueueCounters(0, maxQueueNumber - 1);
72607274
}
7275+
else if (it.second.m_host_tx_queue_configured && it.second.m_host_tx_queue <= maxQueueNumber)
7276+
{
7277+
flexCounterQueueState.enableQueueCounters(it.second.m_host_tx_queue, it.second.m_host_tx_queue);
7278+
}
72617279
queuesStateVector.insert(make_pair(it.second.m_alias, flexCounterQueueState));
72627280
}
72637281
generateQueueMapPerPort(it.second, queuesStateVector.at(it.second.m_alias), false);
@@ -7396,6 +7414,10 @@ void PortsOrch::addQueueFlexCounters(map<string, FlexCounterQueueStates> queuesS
73967414
{
73977415
flexCounterQueueState.enableQueueCounters(0, maxQueueNumber - 1);
73987416
}
7417+
else if (it.second.m_host_tx_queue_configured && it.second.m_host_tx_queue <= maxQueueNumber)
7418+
{
7419+
flexCounterQueueState.enableQueueCounters(it.second.m_host_tx_queue, it.second.m_host_tx_queue);
7420+
}
73997421
queuesStateVector.insert(make_pair(it.second.m_alias, flexCounterQueueState));
74007422
}
74017423
addQueueFlexCountersPerPort(it.second, queuesStateVector.at(it.second.m_alias));
@@ -7477,6 +7499,10 @@ void PortsOrch::addQueueWatermarkFlexCounters(map<string, FlexCounterQueueStates
74777499
{
74787500
flexCounterQueueState.enableQueueCounters(0, maxQueueNumber - 1);
74797501
}
7502+
else if (it.second.m_host_tx_queue_configured && it.second.m_host_tx_queue <= maxQueueNumber)
7503+
{
7504+
flexCounterQueueState.enableQueueCounters(it.second.m_host_tx_queue, it.second.m_host_tx_queue);
7505+
}
74807506
queuesStateVector.insert(make_pair(it.second.m_alias, flexCounterQueueState));
74817507
}
74827508
addQueueWatermarkFlexCountersPerPort(it.second, queuesStateVector.at(it.second.m_alias));
@@ -7524,7 +7550,7 @@ void PortsOrch::addQueueWatermarkFlexCountersPerPortPerQueueIndex(const Port& po
75247550
startFlexCounterPolling(gSwitchId, key, counters_str, QUEUE_COUNTER_ID_LIST);
75257551
}
75267552

7527-
void PortsOrch::createPortBufferQueueCounters(const Port &port, string queues)
7553+
void PortsOrch::createPortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue)
75287554
{
75297555
SWSS_LOG_ENTER();
75307556

@@ -7544,6 +7570,11 @@ void PortsOrch::createPortBufferQueueCounters(const Port &port, string queues)
75447570

75457571
for (auto queueIndex = startIndex; queueIndex <= endIndex; queueIndex++)
75467572
{
7573+
if (queueIndex == (uint32_t)port.m_host_tx_queue && skip_host_tx_queue)
7574+
{
7575+
continue;
7576+
}
7577+
75477578
std::ostringstream name;
75487579
name << port.m_alias << ":" << queueIndex;
75497580

@@ -7581,7 +7612,7 @@ void PortsOrch::createPortBufferQueueCounters(const Port &port, string queues)
75817612
CounterCheckOrch::getInstance().addPort(port);
75827613
}
75837614

7584-
void PortsOrch::removePortBufferQueueCounters(const Port &port, string queues)
7615+
void PortsOrch::removePortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue)
75857616
{
75867617
SWSS_LOG_ENTER();
75877618

@@ -7597,6 +7628,11 @@ void PortsOrch::removePortBufferQueueCounters(const Port &port, string queues)
75977628

75987629
for (auto queueIndex = startIndex; queueIndex <= endIndex; queueIndex++)
75997630
{
7631+
if (queueIndex == (uint32_t)port.m_host_tx_queue && skip_host_tx_queue)
7632+
{
7633+
continue;
7634+
}
7635+
76007636
std::ostringstream name;
76017637
name << port.m_alias << ":" << queueIndex;
76027638
const auto id = sai_serialize_object_id(port.m_queue_ids[queueIndex]);

orchagent/portsorch.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,8 @@ class PortsOrch : public Orch, public Subject
184184

185185
void generateQueueMap(map<string, FlexCounterQueueStates> queuesStateVector);
186186
uint32_t getNumberOfPortSupportedQueueCounters(string port);
187-
void createPortBufferQueueCounters(const Port &port, string queues);
188-
void removePortBufferQueueCounters(const Port &port, string queues);
187+
void createPortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue=true);
188+
void removePortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue=true);
189189
void addQueueFlexCounters(map<string, FlexCounterQueueStates> queuesStateVector);
190190
void addQueueWatermarkFlexCounters(map<string, FlexCounterQueueStates> queuesStateVector);
191191

tests/test_flex_counters.py

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -716,12 +716,12 @@ def remove_ip_address(self, interface, ip):
716716
def set_admin_status(self, interface, status):
717717
self.config_db.update_entry("PORT", interface, {"admin_status": status})
718718

719-
@pytest.mark.parametrize('counter_type', [('queue_counter'), ('pg_drop_counter')])
720-
def test_create_only_config_db_buffers_false(self, dvs, counter_type):
719+
@pytest.mark.parametrize('counter_type_id', [('queue_counter', '8'), ('pg_drop_counter', '7')])
720+
def test_create_only_config_db_buffers_false(self, dvs, counter_type_id):
721721
"""
722722
Test steps:
723723
1. By default the configuration knob 'create_only_config_db_value' is missing.
724-
2. Get the counter OID for the interface 'Ethernet0:7' from the counters database.
724+
2. Get the counter OID for the interface 'Ethernet0', queue 8 or PG 7, from the counters database.
725725
3. Perform assertions based on the 'create_only_config_db_value':
726726
- If 'create_only_config_db_value' is 'false' or does not exist, assert that the counter OID has a valid OID value.
727727
@@ -730,10 +730,11 @@ def test_create_only_config_db_buffers_false(self, dvs, counter_type):
730730
counter_type (str): The type of counter being tested
731731
"""
732732
self.setup_dbs(dvs)
733+
counter_type, index = counter_type_id
733734
meta_data = counter_group_meta[counter_type]
734735
self.set_flex_counter_group_status(meta_data['key'], meta_data['name_map'])
735736

736-
counter_oid = self.counters_db.db_connection.hget(meta_data['name_map'], 'Ethernet0:7')
737+
counter_oid = self.counters_db.db_connection.hget(meta_data['name_map'], 'Ethernet0:' + index)
737738
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"
738739

739740
def test_create_remove_buffer_pg_watermark_counter(self, dvs):
@@ -765,24 +766,25 @@ def test_create_remove_buffer_pg_watermark_counter(self, dvs):
765766
self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '1', False)
766767
self.wait_for_id_list_remove(meta_data['group_name'], "Ethernet0", counter_oid)
767768

768-
@pytest.mark.parametrize('counter_type', [('queue_counter'), ('pg_drop_counter')])
769-
def test_create_only_config_db_buffers_true(self, dvs, counter_type):
769+
@pytest.mark.parametrize('counter_type_id', [('queue_counter', '8'), ('pg_drop_counter', '7')])
770+
def test_create_only_config_db_buffers_true(self, dvs, counter_type_id):
770771
"""
771772
Test steps:
772773
1. The 'create_only_config_db_buffers' was set to 'true' by previous test.
773-
2. Get the counter OID for the interface 'Ethernet0:7' from the counters database.
774+
2. Get the counter OID for the interface 'Ethernet0', queue 8 or PG 7, from the counters database.
774775
3. Perform assertions based on the 'create_only_config_db_value':
775776
- If 'create_only_config_db_value' is 'true', assert that the counter OID is None.
776777
777778
Args:
778779
dvs (object): virtual switch object
779780
counter_type (str): The type of counter being tested
780781
"""
782+
counter_type, index = counter_type_id
781783
self.setup_dbs(dvs)
782784
meta_data = counter_group_meta[counter_type]
783785
self.set_flex_counter_group_status(meta_data['key'], meta_data['name_map'])
784786

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

788790
def test_create_remove_buffer_queue_counter(self, dvs):
@@ -802,12 +804,12 @@ def test_create_remove_buffer_queue_counter(self, dvs):
802804

803805
self.set_flex_counter_group_status(meta_data['key'], meta_data['name_map'])
804806

805-
self.config_db.update_entry('BUFFER_QUEUE', 'Ethernet0|7', {'profile': 'egress_lossless_profile'})
806-
counter_oid = self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '7', True)
807+
self.config_db.update_entry('BUFFER_QUEUE', 'Ethernet0|8', {'profile': 'egress_lossless_profile'})
808+
counter_oid = self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '8', True)
807809
self.wait_for_id_list(meta_data['group_name'], "Ethernet0", counter_oid)
808810

809-
self.config_db.delete_entry('BUFFER_QUEUE', 'Ethernet0|7')
810-
self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '7', False)
811+
self.config_db.delete_entry('BUFFER_QUEUE', 'Ethernet0|8')
812+
self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '8', False)
811813
self.wait_for_id_list_remove(meta_data['group_name'], "Ethernet0", counter_oid)
812814

813815
def test_create_remove_buffer_watermark_queue_pg_counter(self, dvs):
@@ -830,16 +832,18 @@ def test_create_remove_buffer_watermark_queue_pg_counter(self, dvs):
830832
self.set_flex_counter_group_status(meta_data['key'], meta_data['name_map'])
831833

832834
self.config_db.update_entry('BUFFER_PG', 'Ethernet0|7', {'profile': 'ingress_lossy_profile'})
833-
self.config_db.update_entry('BUFFER_QUEUE', 'Ethernet0|7', {'profile': 'egress_lossless_profile'})
835+
self.config_db.update_entry('BUFFER_QUEUE', 'Ethernet0|8', {'profile': 'egress_lossless_profile'})
834836

835837
for counterpoll_type, meta_data in counter_group_meta.items():
836838
if 'queue' in counterpoll_type or 'pg' in counterpoll_type:
837-
counter_oid = self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '7', True)
839+
index = '8' if 'queue' in counterpoll_type else '7'
840+
counter_oid = self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', index, True)
838841
self.wait_for_id_list(meta_data['group_name'], "Ethernet0", counter_oid)
839842

840-
self.config_db.delete_entry('BUFFER_QUEUE', 'Ethernet0|7')
843+
self.config_db.delete_entry('BUFFER_QUEUE', 'Ethernet0|8')
841844
self.config_db.delete_entry('BUFFER_PG', 'Ethernet0|7')
842845
for counterpoll_type, meta_data in counter_group_meta.items():
843846
if 'queue' in counterpoll_type or 'pg' in counterpoll_type:
844-
self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '7', False)
847+
index = '8' if 'queue' in counterpoll_type else '7'
848+
self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', index, False)
845849
self.wait_for_id_list_remove(meta_data['group_name'], "Ethernet0", counter_oid)

0 commit comments

Comments
 (0)