[orchagent] add & remove port counters dynamically each time port was added or removed

orchagent/debugcounterorch.cpp

@@ -5,6 +5,7 @@
 #include "schema.h"
 #include "drop_counter.h"
 #include <memory>
+#include "observer.h"
 
 using std::string;
 using std::unordered_map;
@@ -34,13 +35,61 @@ DebugCounterOrch::DebugCounterOrch(DBConnector *db, const vector<string>& table_
 {
     SWSS_LOG_ENTER();
     publishDropCounterCapabilities();
+
+    gPortsOrch->attach(this);
 }
 
 DebugCounterOrch::~DebugCounterOrch(void)
 {
     SWSS_LOG_ENTER();
 }
 
+void DebugCounterOrch::update(SubjectType type, void *cntx)
+{
+    SWSS_LOG_ENTER();
+
+    if (type == SUBJECT_TYPE_PORT_CHANGE) 
+    {
+        if (!cntx) 
+        {
+            SWSS_LOG_ERROR("cntx is NULL");
+            return;
+        }
+
+        PortUpdate *update = static_cast<PortUpdate *>(cntx);
+        Port &port = update->port;
+
+        if (update->add) 
+        {
+            for (const auto& debug_counter: debug_counters)
+            {
+                DebugCounter *counter = debug_counter.second.get();
+                auto counter_type = counter->getCounterType();
+                auto counter_stat = counter->getDebugCounterSAIStat();
+                auto flex_counter_type = getFlexCounterType(counter_type);
+                if (flex_counter_type == CounterType::PORT_DEBUG)
+                {
+                    installDebugFlexCounters(counter_type, counter_stat, port.m_port_id);
+                }
+            }
+        } 
+        else 
+        {
+            for (const auto& debug_counter: debug_counters)
+            {
+                DebugCounter *counter = debug_counter.second.get();
+                auto counter_type = counter->getCounterType();
+                auto counter_stat = counter->getDebugCounterSAIStat();
+                auto flex_counter_type = getFlexCounterType(counter_type);
+                if (flex_counter_type == CounterType::PORT_DEBUG)
+                {
+                    uninstallDebugFlexCounters(counter_type, counter_stat, port.m_port_id);
+                }
+            }
+        }
+    }
+}
+
 // doTask processes updates from the consumer and modifies the state of the
 // following components:
 //     1) The ASIC, by creating, modifying, and deleting debug counters
@@ -476,7 +525,8 @@ CounterType DebugCounterOrch::getFlexCounterType(const string& counter_type)
 }
 
 void DebugCounterOrch::installDebugFlexCounters(const string& counter_type,
-                                                const string& counter_stat)
+                                                const string& counter_stat,
+                                                sai_object_id_t port_id)
 {
     SWSS_LOG_ENTER();
     CounterType flex_counter_type = getFlexCounterType(counter_type);
@@ -489,6 +539,14 @@ void DebugCounterOrch::installDebugFlexCounters(const string& counter_type,
     {
         for (auto const &curr : gPortsOrch->getAllPorts())
         {
+            if (port_id != SAI_NULL_OBJECT_ID)
+            {
+                if (curr.second.m_port_id != port_id)
+                {
+                    continue;
+                }
+            }
+
             if (curr.second.m_type != Port::Type::PHY)
             {
                 continue;
@@ -503,7 +561,8 @@ void DebugCounterOrch::installDebugFlexCounters(const string& counter_type,
 }
 
 void DebugCounterOrch::uninstallDebugFlexCounters(const string& counter_type,
-                                                  const string& counter_stat)
+                                                  const string& counter_stat,
+                                                  sai_object_id_t port_id)
 {
     SWSS_LOG_ENTER();
     CounterType flex_counter_type = getFlexCounterType(counter_type);
@@ -516,6 +575,14 @@ void DebugCounterOrch::uninstallDebugFlexCounters(const string& counter_type,
     {
         for (auto const &curr : gPortsOrch->getAllPorts())
         {
+            if (port_id != SAI_NULL_OBJECT_ID)
+            {
+                if (curr.second.m_port_id != port_id)
+                {
+                    continue;
+                }
+            }
+
             if (curr.second.m_type != Port::Type::PHY)
             {
                 continue;
@@ -616,3 +683,6 @@ bool DebugCounterOrch::isDropReasonValid(const string& drop_reason) const
 
     return true;
 }
+
+
+

orchagent/debugcounterorch.h

@@ -10,23 +10,27 @@
 #include "flex_counter_stat_manager.h"
 #include "debug_counter.h"
 #include "drop_counter.h"
+#include "observer.h"
 
 extern "C" {
 #include "sai.h"
 }
 
 #define DEBUG_COUNTER_FLEX_COUNTER_GROUP "DEBUG_COUNTER"
 
+using DebugCounterMap = std::unordered_map<std::string, std::unique_ptr<DebugCounter>>;
+
 // DebugCounterOrch is an orchestrator for managing debug counters. It handles
 // the creation, deletion, and modification of debug counters.
-class DebugCounterOrch: public Orch
+class DebugCounterOrch: public Orch, public Observer
 {
 public:
     DebugCounterOrch(swss::DBConnector *db, const std::vector<std::string>& table_names, int poll_interval);
     virtual ~DebugCounterOrch(void);
 
     void doTask(Consumer& consumer);
 
+    void update(SubjectType, void *cntx);
 private:
     // Debug Capability Reporting Functions
     void publishDropCounterCapabilities();
@@ -48,10 +52,12 @@ class DebugCounterOrch: public Orch
     CounterType getFlexCounterType(const std::string& counter_type) noexcept(false);
     void installDebugFlexCounters(
             const std::string& counter_type,
-            const std::string& counter_stat);
+            const std::string& counter_stat,
+            sai_object_id_t port_id = SAI_NULL_OBJECT_ID);
     void uninstallDebugFlexCounters(
             const std::string& counter_type,
-            const std::string& counter_stat);
+            const std::string& counter_stat,
+            sai_object_id_t port_id = SAI_NULL_OBJECT_ID);
 
     // Debug Counter Initialization Helper Functions
     std::string getDebugCounterType(
@@ -83,7 +89,7 @@ class DebugCounterOrch: public Orch
 
     FlexCounterStatManager flex_counter_manager;
 
-    std::unordered_map<std::string, std::unique_ptr<DebugCounter>> debug_counters;
+    DebugCounterMap debug_counters;
 
     // free_drop_counters are drop counters that have been created by a user
     // that do not have any drop reasons associated with them yet. Because

orchagent/orchdaemon.cpp

@@ -52,6 +52,7 @@ CoppOrch *gCoppOrch;
 P4Orch *gP4Orch;
 BfdOrch *gBfdOrch;
 Srv6Orch *gSrv6Orch;
+DebugCounterOrch *gDebugCounterOrch;
 
 bool gIsNatSupported = false;
 
@@ -282,7 +283,7 @@ bool OrchDaemon::init()
         CFG_DEBUG_COUNTER_DROP_REASON_TABLE_NAME
     };
 
-    DebugCounterOrch *debug_counter_orch = new DebugCounterOrch(m_configDb, debug_counter_tables, 1000);
+    gDebugCounterOrch = new DebugCounterOrch(m_configDb, debug_counter_tables, 1000);
 
     const int natorch_base_pri = 50;
 
@@ -330,7 +331,7 @@ bool OrchDaemon::init()
      * when iterating ConsumerMap. This is ensured implicitly by the order of keys in ordered map.
      * For cases when Orch has to process tables in specific order, like PortsOrch during warm start, it has to override Orch::doTask()
      */
-    m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, mux_orch, mux_cb_orch, gIntfsOrch, gNeighOrch, gNhgMapOrch, gNhgOrch, gCbfNhgOrch, gRouteOrch, gCoppOrch, gQosOrch, wm_orch, policer_orch, tunnel_decap_orch, sflow_orch, debug_counter_orch, gMacsecOrch, gBfdOrch, gSrv6Orch};
+    m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, mux_orch, mux_cb_orch, gIntfsOrch, gNeighOrch, gNhgMapOrch, gNhgOrch, gCbfNhgOrch, gRouteOrch, gCoppOrch, gQosOrch, wm_orch, policer_orch, tunnel_decap_orch, sflow_orch, gDebugCounterOrch, gMacsecOrch, gBfdOrch, gSrv6Orch};
 
     bool initialize_dtel = false;
     if (platform == BFN_PLATFORM_SUBSTRING || platform == VS_PLATFORM_SUBSTRING)

orchagent/portsorch.cpp

@@ -2378,6 +2378,18 @@ bool PortsOrch::initPort(const string &alias, const string &role, const int inde
                     port_buffer_drop_stat_manager.setCounterIdList(p.m_port_id, CounterType::PORT, port_buffer_drop_stats);
                 }
 
+                /* when a port is added and priority group map counter is enabled --> we need to add pg counter for it */
+                if (m_isPriorityGroupMapGenerated)
+                {
+                    generatePriorityGroupMapPerPort(p);
+                }
+
+                /* when a port is added and queue map counter is enabled --> we need to add queue map counter for it */
+                if (m_isQueueMapGenerated)
+                {
+                    generateQueueMapPerPort(p);
+                }
+
                 PortUpdate update = { p, true };
                 notify(SUBJECT_TYPE_PORT_CHANGE, static_cast<void *>(&update));
 
@@ -2410,8 +2422,13 @@ void PortsOrch::deInitPort(string alias, sai_object_id_t port_id)
 {
     SWSS_LOG_ENTER();
 
-    Port p(alias, Port::PHY);
-    p.m_port_id = port_id;
+    Port p;
+
+    if (!getPort(port_id, p))
+    {
+        SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64, port_id);
+        return;
+    }
 
     /* remove port from flex_counter_table for updating counters  */
     auto flex_counters_orch = gDirectory.get<FlexCounterOrch*>();
@@ -2425,9 +2442,20 @@ void PortsOrch::deInitPort(string alias, sai_object_id_t port_id)
         port_buffer_drop_stat_manager.clearCounterIdList(p.m_port_id);
     }
 
+    /* remove pg port counters */
+    if (m_isPriorityGroupMapGenerated)
+    {
+        removePriorityGroupMapPerPort(p);
+    }
+
+    /* remove queue port counters */
+    if (m_isQueueMapGenerated)
+    {
+        removeQueueMapPerPort(p);
+    }
 
     /* remove port name map from counter table */
-    m_counter_db->hdel(COUNTERS_PORT_NAME_MAP, alias);
+    m_counterTable->hdel("", alias);
 
     /* Remove the associated port serdes attribute */
     removePortSerdesAttribute(p.m_port_id);
@@ -2436,7 +2464,6 @@ void PortsOrch::deInitPort(string alias, sai_object_id_t port_id)
     SWSS_LOG_NOTICE("De-Initialized port %s", alias.c_str());
 }
 
-
 bool PortsOrch::bake()
 {
     SWSS_LOG_ENTER();
@@ -5398,6 +5425,44 @@ void PortsOrch::generateQueueMap()
     m_isQueueMapGenerated = true;
 }
 
+void PortsOrch::removeQueueMapPerPort(const Port& port)
+{
+    /* Remove the Queue map in the Counter DB */
+
+    for (size_t queueIndex = 0; queueIndex < port.m_queue_ids.size(); ++queueIndex)
+    {
+        std::ostringstream name;
+        name << port.m_alias << ":" << queueIndex;
+        std::unordered_set<string> counter_stats;
+
+        const auto id = sai_serialize_object_id(port.m_queue_ids[queueIndex]);
+
+        m_queueTable->hdel("",name.str());
+        m_queuePortTable->hdel("",id);
+
+        string queueType;
+        uint8_t queueRealIndex = 0;
+        if (getQueueTypeAndIndex(port.m_queue_ids[queueIndex], queueType, queueRealIndex))
+        {
+            m_queueTypeTable->hdel("",id);
+            m_queueIndexTable->hdel("",id);
+        }
+
+        for (const auto& it: queue_stat_ids)
+        {
+            counter_stats.emplace(sai_serialize_queue_stat(it));
+        }
+        queue_stat_manager.clearCounterIdList(port.m_queue_ids[queueIndex]);
+
+        /* remove watermark queue counters */
+        string key = getQueueWatermarkFlexCounterTableKey(id);
+
+        m_flexCounterTable->del(key);
+    }
+
+    CounterCheckOrch::getInstance().removePort(port);
+}
+
 void PortsOrch::generateQueueMapPerPort(const Port& port)
 {
     /* Create the Queue map in the Counter DB */
@@ -5476,6 +5541,32 @@ void PortsOrch::generatePriorityGroupMap()
     m_isPriorityGroupMapGenerated = true;
 }
 
+void PortsOrch::removePriorityGroupMapPerPort(const Port& port)
+{
+    /* Remove the PG map in the Counter DB */
+
+    for (size_t pgIndex = 0; pgIndex < port.m_priority_group_ids.size(); ++pgIndex)
+    {
+        std::ostringstream name;
+        name << port.m_alias << ":" << pgIndex;
+
+        const auto id = sai_serialize_object_id(port.m_priority_group_ids[pgIndex]);
+        string key = getPriorityGroupWatermarkFlexCounterTableKey(id);
+
+        m_pgTable->hdel("",name.str());
+        m_pgPortTable->hdel("",id);
+        m_pgIndexTable->hdel("",id);
+
+        m_flexCounterTable->del(key);
+
+        key = getPriorityGroupDropPacketsFlexCounterTableKey(id);
+        /* remove dropped packets counters to flex_counter */
+        m_flexCounterTable->del(key);
+    }
+
+    CounterCheckOrch::getInstance().removePort(port);
+}
+
 void PortsOrch::generatePriorityGroupMapPerPort(const Port& port)
 {
     /* Create the PG map in the Counter DB */
@@ -5633,7 +5724,7 @@ void PortsOrch::doTask(NotificationConsumer &consumer)
                     updateDbPortOperSpeed(port, 0);
                 }
             }
-            
+
             /* update m_portList */
             m_portList[port.m_alias] = port;
         }

orchagent/portsorch.h

@@ -312,9 +312,11 @@ class PortsOrch : public Orch, public Subject
 
     bool m_isQueueMapGenerated = false;
     void generateQueueMapPerPort(const Port& port);
+    void removeQueueMapPerPort(const Port& port);
 
     bool m_isPriorityGroupMapGenerated = false;
     void generatePriorityGroupMapPerPort(const Port& port);
+    void removePriorityGroupMapPerPort(const Port& port);
 
     bool m_isPortCounterMapGenerated = false;
     bool m_isPortBufferDropCounterMapGenerated = false;

tests/conftest.py

@@ -22,6 +22,7 @@
 from dvslib.dvs_pbh import DVSPbh
 from dvslib.dvs_route import DVSRoute
 from dvslib import dvs_vlan
+from dvslib import dvs_port
 from dvslib import dvs_lag
 from dvslib import dvs_mirror
 from dvslib import dvs_policer
@@ -1765,7 +1766,11 @@ def dvs_vlan_manager(request, dvs):
                                             dvs.get_counters_db(),
                                             dvs.get_app_db())
 
-
+@pytest.yield_fixture(scope="class")
+def dvs_port_manager(request, dvs):
+    request.cls.dvs_port = dvs_port.DVSPort(dvs.get_asic_db(),
+                                            dvs.get_config_db())
+
 @pytest.yield_fixture(scope="class")
 def dvs_mirror_manager(request, dvs):
     request.cls.dvs_mirror = dvs_mirror.DVSMirror(dvs.get_asic_db(),