From f9242b2e2e1f16e05e66a5b8fe12a92fc27650a8 Mon Sep 17 00:00:00 2001 From: Stephen Sun <5379172+stephenxs@users.noreply.github.com> Date: Sat, 23 Nov 2024 00:40:10 +0800 Subject: [PATCH] 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. --- orchagent/flexcounterorch.cpp | 7 +++++ orchagent/p4orch/tests/fake_portorch.cpp | 4 +-- orchagent/port.h | 2 ++ orchagent/portsorch.cpp | 40 ++++++++++++++++++++++-- orchagent/portsorch.h | 4 +-- tests/test_flex_counters.py | 36 +++++++++++---------- 6 files changed, 71 insertions(+), 22 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index 19302face99..ec513cb8f8c 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -400,6 +400,13 @@ map FlexCounterOrch::getQueueConfigurations() { queuesStateVector.at(configPortName).enableQueueCounter(startIndex); } + + Port port; + gPortsOrch->getPort(configPortName, port); + if (port.m_host_tx_queue_configured && port.m_host_tx_queue <= maxQueueIndex) + { + queuesStateVector.at(configPortName).enableQueueCounter(port.m_host_tx_queue); + } } catch (std::invalid_argument const& e) { SWSS_LOG_ERROR("Invalid queue index [%s] for port [%s]", configPortQueues.c_str(), configPortName.c_str()); continue; diff --git a/orchagent/p4orch/tests/fake_portorch.cpp b/orchagent/p4orch/tests/fake_portorch.cpp index a34a30eb4b5..233b55d6928 100644 --- a/orchagent/p4orch/tests/fake_portorch.cpp +++ b/orchagent/p4orch/tests/fake_portorch.cpp @@ -189,11 +189,11 @@ void PortsOrch::generateQueueMapPerPort(const Port &port, FlexCounterQueueStates { } -void PortsOrch::createPortBufferQueueCounters(const Port &port, string queues) +void PortsOrch::createPortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue) { } -void PortsOrch::removePortBufferQueueCounters(const Port &port, string queues) +void PortsOrch::removePortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue) { } diff --git a/orchagent/port.h b/orchagent/port.h index 0ae9b97b673..77c2640cf5c 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -162,6 +162,8 @@ class Port sai_port_priority_flow_control_mode_t m_pfc_asym = SAI_PORT_PRIORITY_FLOW_CONTROL_MODE_COMBINED; uint8_t m_pfc_bitmask = 0; // PFC enable bit mask uint8_t m_pfcwd_sw_bitmask = 0; // PFC software watchdog enable + uint8_t m_host_tx_queue = 0; + bool m_host_tx_queue_configured = false; uint16_t m_tpid = DEFAULT_TPID; uint32_t m_nat_zone_id = 0; uint32_t m_vnid = VNID_NONE; diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index ba5336b063d..7ebdf670ced 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -3510,6 +3510,12 @@ bool PortsOrch::initPort(const PortConfig &port) m_recircPortRole[alias] = role; } + // We have to test the size of m_queue_ids here since it isn't initialized on some platforms (like DPU) + if (p.m_host_tx_queue_configured && p.m_queue_ids.size() > p.m_host_tx_queue) + { + createPortBufferQueueCounters(p, to_string(p.m_host_tx_queue), false); + } + SWSS_LOG_NOTICE("Initialized port %s", alias.c_str()); } else @@ -3540,6 +3546,11 @@ void PortsOrch::deInitPort(string alias, sai_object_id_t port_id) return; } + if (p.m_host_tx_queue_configured && p.m_queue_ids.size() > p.m_host_tx_queue) + { + removePortBufferQueueCounters(p, to_string(p.m_host_tx_queue), false); + } + /* remove port from flex_counter_table for updating counters */ auto flex_counters_orch = gDirectory.get(); if ((flex_counters_orch->getPortCountersState())) @@ -5951,6 +5962,9 @@ bool PortsOrch::addHostIntfs(Port &port, string alias, sai_object_id_t &host_int attr.id = SAI_HOSTIF_ATTR_QUEUE; attr.value.u32 = DEFAULT_HOSTIF_TX_QUEUE; attrs.push_back(attr); + + port.m_host_tx_queue = DEFAULT_HOSTIF_TX_QUEUE; + port.m_host_tx_queue_configured = true; } 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 queuesState { flexCounterQueueState.enableQueueCounters(0, maxQueueNumber - 1); } + else if (it.second.m_host_tx_queue_configured && it.second.m_host_tx_queue <= maxQueueNumber) + { + flexCounterQueueState.enableQueueCounters(it.second.m_host_tx_queue, it.second.m_host_tx_queue); + } queuesStateVector.insert(make_pair(it.second.m_alias, flexCounterQueueState)); } generateQueueMapPerPort(it.second, queuesStateVector.at(it.second.m_alias), false); @@ -7396,6 +7414,10 @@ void PortsOrch::addQueueFlexCounters(map queuesS { flexCounterQueueState.enableQueueCounters(0, maxQueueNumber - 1); } + else if (it.second.m_host_tx_queue_configured && it.second.m_host_tx_queue <= maxQueueNumber) + { + flexCounterQueueState.enableQueueCounters(it.second.m_host_tx_queue, it.second.m_host_tx_queue); + } queuesStateVector.insert(make_pair(it.second.m_alias, flexCounterQueueState)); } addQueueFlexCountersPerPort(it.second, queuesStateVector.at(it.second.m_alias)); @@ -7477,6 +7499,10 @@ void PortsOrch::addQueueWatermarkFlexCounters(map queuesStateVector); uint32_t getNumberOfPortSupportedQueueCounters(string port); - void createPortBufferQueueCounters(const Port &port, string queues); - void removePortBufferQueueCounters(const Port &port, string queues); + void createPortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue=true); + void removePortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue=true); void addQueueFlexCounters(map queuesStateVector); void addQueueWatermarkFlexCounters(map queuesStateVector); diff --git a/tests/test_flex_counters.py b/tests/test_flex_counters.py index f590b7748c3..9fa641fc456 100644 --- a/tests/test_flex_counters.py +++ b/tests/test_flex_counters.py @@ -716,12 +716,12 @@ 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): + @pytest.mark.parametrize('counter_type_id', [('queue_counter', '8'), ('pg_drop_counter', '7')]) + def test_create_only_config_db_buffers_false(self, dvs, counter_type_id): """ 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. + 2. Get the counter OID for the interface 'Ethernet0', queue 8 or PG 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. @@ -730,10 +730,11 @@ def test_create_only_config_db_buffers_false(self, dvs, counter_type): counter_type (str): The type of counter being tested """ self.setup_dbs(dvs) + counter_type, index = counter_type_id 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') + counter_oid = self.counters_db.db_connection.hget(meta_data['name_map'], 'Ethernet0:' + index) 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): @@ -765,12 +766,12 @@ 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): + @pytest.mark.parametrize('counter_type_id', [('queue_counter', '8'), ('pg_drop_counter', '7')]) + def test_create_only_config_db_buffers_true(self, dvs, counter_type_id): """ 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. + 2. Get the counter OID for the interface 'Ethernet0', queue 8 or PG 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. @@ -778,11 +779,12 @@ def test_create_only_config_db_buffers_true(self, dvs, counter_type): dvs (object): virtual switch object counter_type (str): The type of counter being tested """ + counter_type, index = counter_type_id 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') + counter_oid = self.counters_db.db_connection.hget(meta_data['name_map'], 'Ethernet0:' + index) 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): @@ -802,12 +804,12 @@ def test_create_remove_buffer_queue_counter(self, dvs): self.set_flex_counter_group_status(meta_data['key'], meta_data['name_map']) - self.config_db.update_entry('BUFFER_QUEUE', 'Ethernet0|7', {'profile': 'egress_lossless_profile'}) - counter_oid = self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '7', True) + self.config_db.update_entry('BUFFER_QUEUE', 'Ethernet0|8', {'profile': 'egress_lossless_profile'}) + counter_oid = self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '8', True) self.wait_for_id_list(meta_data['group_name'], "Ethernet0", counter_oid) - self.config_db.delete_entry('BUFFER_QUEUE', 'Ethernet0|7') - self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '7', False) + self.config_db.delete_entry('BUFFER_QUEUE', 'Ethernet0|8') + self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '8', False) self.wait_for_id_list_remove(meta_data['group_name'], "Ethernet0", counter_oid) 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): self.set_flex_counter_group_status(meta_data['key'], meta_data['name_map']) self.config_db.update_entry('BUFFER_PG', 'Ethernet0|7', {'profile': 'ingress_lossy_profile'}) - self.config_db.update_entry('BUFFER_QUEUE', 'Ethernet0|7', {'profile': 'egress_lossless_profile'}) + self.config_db.update_entry('BUFFER_QUEUE', 'Ethernet0|8', {'profile': 'egress_lossless_profile'}) for counterpoll_type, meta_data in counter_group_meta.items(): if 'queue' in counterpoll_type or 'pg' in counterpoll_type: - counter_oid = self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '7', True) + index = '8' if 'queue' in counterpoll_type else '7' + counter_oid = self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', index, True) self.wait_for_id_list(meta_data['group_name'], "Ethernet0", counter_oid) - self.config_db.delete_entry('BUFFER_QUEUE', 'Ethernet0|7') + self.config_db.delete_entry('BUFFER_QUEUE', 'Ethernet0|8') self.config_db.delete_entry('BUFFER_PG', 'Ethernet0|7') for counterpoll_type, meta_data in counter_group_meta.items(): if 'queue' in counterpoll_type or 'pg' in counterpoll_type: - self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', '7', False) + index = '8' if 'queue' in counterpoll_type else '7' + self.wait_for_buffer_pg_queue_counter(meta_data['name_map'], 'Ethernet0', index, False) self.wait_for_id_list_remove(meta_data['group_name'], "Ethernet0", counter_oid)