Create counter for the queue to which the host CPU traffic is sent wh…

…en 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

@@ -408,6 +408,13 @@ map<string, FlexCounterQueueStates> 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;

orchagent/p4orch/tests/fake_portorch.cpp

@@ -191,11 +191,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)
 {
 }
 

orchagent/port.h

@@ -201,6 +201,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;

orchagent/portsorch.cpp

@@ -3570,6 +3570,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
@@ -3600,6 +3606,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<FlexCounterOrch*>();
     if ((flex_counters_orch->getPortCountersState()))
@@ -6013,6 +6024,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());
@@ -7320,6 +7334,10 @@ void PortsOrch::generateQueueMap(map<string, FlexCounterQueueStates> 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);
@@ -7458,6 +7476,10 @@ void PortsOrch::addQueueFlexCounters(map<string, FlexCounterQueueStates> 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));
@@ -7539,6 +7561,10 @@ void PortsOrch::addQueueWatermarkFlexCounters(map<string, FlexCounterQueueStates
                 {
                     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));
             }
             addQueueWatermarkFlexCountersPerPort(it.second, queuesStateVector.at(it.second.m_alias));
@@ -7586,7 +7612,7 @@ void PortsOrch::addQueueWatermarkFlexCountersPerPortPerQueueIndex(const Port& po
     startFlexCounterPolling(gSwitchId, key, counters_str, QUEUE_COUNTER_ID_LIST);
 }
 
-void PortsOrch::createPortBufferQueueCounters(const Port &port, string queues)
+void PortsOrch::createPortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue)
 {
     SWSS_LOG_ENTER();
 
@@ -7606,6 +7632,11 @@ void PortsOrch::createPortBufferQueueCounters(const Port &port, string queues)
 
     for (auto queueIndex = startIndex; queueIndex <= endIndex; queueIndex++)
     {
+        if (queueIndex == (uint32_t)port.m_host_tx_queue && skip_host_tx_queue)
+        {
+            continue;
+        }
+
         std::ostringstream name;
         name << port.m_alias << ":" << queueIndex;
 
@@ -7643,7 +7674,7 @@ void PortsOrch::createPortBufferQueueCounters(const Port &port, string queues)
     CounterCheckOrch::getInstance().addPort(port);
 }
 
-void PortsOrch::removePortBufferQueueCounters(const Port &port, string queues)
+void PortsOrch::removePortBufferQueueCounters(const Port &port, string queues, bool skip_host_tx_queue)
 {
     SWSS_LOG_ENTER();
 
@@ -7659,6 +7690,11 @@ void PortsOrch::removePortBufferQueueCounters(const Port &port, string queues)
 
     for (auto queueIndex = startIndex; queueIndex <= endIndex; queueIndex++)
     {
+        if (queueIndex == (uint32_t)port.m_host_tx_queue && skip_host_tx_queue)
+        {
+            continue;
+        }
+
         std::ostringstream name;
         name << port.m_alias << ":" << queueIndex;
         const auto id = sai_serialize_object_id(port.m_queue_ids[queueIndex]);

orchagent/portsorch.h

@@ -186,8 +186,8 @@ class PortsOrch : public Orch, public Subject
 
     void generateQueueMap(map<string, FlexCounterQueueStates> 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<string, FlexCounterQueueStates> queuesStateVector);
     void addQueueWatermarkFlexCounters(map<string, FlexCounterQueueStates> queuesStateVector);
 

tests/test_flex_counters.py

@@ -609,12 +609,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.
 
@@ -623,10 +623,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):
@@ -658,24 +659,25 @@ 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.
 
         Args:
             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):
@@ -695,12 +697,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):
@@ -723,16 +725,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)