[orchagent] add & remove port counters dynamically each time port was added or removed (#2019)

- What I did
Add support for adding & removing port counters dynamically each time port was added or removed the counters that were supported are:

1. pg watermark counters
2. pg drop counters
3. queue stat counters
4. queue watermark counters
5. debug counters
6. buffer drop counters and port stat counters are already supported to be added or removed each time port is added/removed

Implemented according to the - 'HLD document on add/remove ports dynamically feature' sonic-net/SONiC#900

- Why I did it
In order to support dynamically add or removed ports on sonic

- How I verified it
tested manually that the flex counters were added or removed correctly whenever we add or remove ports
added new test cases to the following vs tests:

test_flex_counters.py
test_drop_counters.py
test_pg_drop_counter.py

Co-authored-by: dprital <drorp@nvidia.com>

Author: tomer-israel

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();
@@ -5409,6 +5436,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 */
@@ -5487,6 +5552,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 */
@@ -5644,7 +5735,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(),