Apply PR of Dynamic port configuration - add port buffer cfg to the port ref counter sonic-net#2022

Author: dprital

orchagent/bufferorch.cpp

@@ -42,6 +42,9 @@ map<string, string> buffer_to_ref_table_map = {
     {buffer_profile_list_field_name, APP_BUFFER_PROFILE_TABLE_NAME}
 };
 
+std::map<string, std::map<size_t, string>> pg_port_flags;
+std::map<string, std::map<size_t, string>> queue_port_flags;
+
 BufferOrch::BufferOrch(DBConnector *applDb, DBConnector *confDb, DBConnector *stateDb, vector<string> &tableNames) :
     Orch(applDb, tableNames),
     m_flexCounterDb(new DBConnector("FLEX_COUNTER_DB", 0)),
@@ -949,6 +952,38 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple)
                     }
                 }
             }
+
+            /* when we apply buffer configuration we need to increase the ref counter of this port
+             * or decrease the ref counter for this port when we remove buffer cfg
+             * so for each priority cfg in each port we will increase/decrease the ref counter
+             * also we need to know when the set command is for creating a buffer cfg or modifying buffer cfg -
+             * we need to increase ref counter only on create flow.
+             * so we added a map that will help us to know what was the last command for this port and priority -
+             * if the last command was set command then it is a modify command and we dont need to increase the buffer counter
+             * all other cases (no last command exist or del command was the last command) it means that we need to increase the ref counter */
+            if (op == SET_COMMAND) 
+            {
+                if (queue_port_flags[port_name][ind] != SET_COMMAND) 
+                {
+                    /* if the last operation was not "set" then it's create and not modify - need to increase ref counter */
+                    gPortsOrch->increasePortRefCount(port_name);
+                }
+            } 
+            else if (op == DEL_COMMAND)
+            {
+                if (queue_port_flags[port_name][ind] == SET_COMMAND) {
+                    /* we need to decrease ref counter only if the last operation was "SET_COMMAND" */
+                    gPortsOrch->decreasePortRefCount(port_name);
+                }
+            } 
+            else 
+            {
+                SWSS_LOG_ERROR("operation value is not SET or DEL (op = %s)", op.c_str());
+                return task_process_status::task_invalid_entry;
+            }
+            /* save the last command (set or delete) */
+            queue_port_flags[port_name][ind] = op;
+
         }
     }
 
@@ -1007,7 +1042,7 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup
     if (op == SET_COMMAND)
     {
         ref_resolve_status  resolve_result = resolveFieldRefValue(m_buffer_type_maps, buffer_profile_field_name,
-                                             buffer_to_ref_table_map.at(buffer_profile_field_name), tuple, 
+                                             buffer_to_ref_table_map.at(buffer_profile_field_name), tuple,
                                              sai_buffer_profile, buffer_profile_name);
         if (ref_resolve_status::success != resolve_result)
         {
@@ -1089,6 +1124,39 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup
                     }
                 }
             }
+
+            /* when we apply buffer configuration we need to increase the ref counter of this port
+             * or decrease the ref counter for this port when we remove buffer cfg
+             * so for each priority cfg in each port we will increase/decrease the ref counter
+             * also we need to know when the set command is for creating a buffer cfg or modifying buffer cfg -
+             * we need to increase ref counter only on create flow.
+             * so we added a map that will help us to know what was the last command for this port and priority -
+             * if the last command was set command then it is a modify command and we dont need to increase the buffer counter
+             * all other cases (no last command exist or del command was the last command) it means that we need to increase the ref counter */
+            if (op == SET_COMMAND) 
+            {
+                if (pg_port_flags[port_name][ind] != SET_COMMAND) 
+                {
+                    /* if the last operation was not "set" then it's create and not modify - need to increase ref counter */
+                    gPortsOrch->increasePortRefCount(port_name);
+                }
+            } 
+            else if (op == DEL_COMMAND)
+            {
+                if (pg_port_flags[port_name][ind] == SET_COMMAND) 
+                {
+                    /* we need to decrease ref counter only if the last operation was "SET_COMMAND" */
+                    gPortsOrch->decreasePortRefCount(port_name);
+                }
+            } 
+            else 
+            {
+                SWSS_LOG_ERROR("operation value is not SET or DEL (op = %s)", op.c_str());
+                return task_process_status::task_invalid_entry;
+            }
+            /* save the last command (set or delete) */
+            pg_port_flags[port_name][ind] = op;
+
         }
         if (portUpdated)
         {

tests/test_port_add_remove.py

@@ -0,0 +1,56 @@
+import pytest
+import time
+from dvslib.dvs_common import PollingConfig
+
+# the port to be removed and add
+PORT = "Ethernet0"
+
+class TestPortAddRemove(object):
+
+    def test_remove_port_with_buffer_cfg(self, dvs, testlog):
+        config_db = dvs.get_config_db()
+        asic_db = dvs.get_asic_db()
+
+        # get port info
+        port_info = config_db.get_entry("PORT", PORT)
+
+        # get the number of ports before removal
+        num_of_ports = len(asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT"))
+
+        # try to remove this port
+        config_db.delete_entry('PORT', PORT)
+        num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT",
+                                      num_of_ports-1,
+                                      polling_config = PollingConfig(polling_interval = 1, timeout = 5.00, strict = False))
+
+        # verify that the port wasn't removed since we still have buffer cfg
+        assert len(num) == num_of_ports
+
+        # remove buffer pg cfg for the port
+        pgs = config_db.get_keys('BUFFER_PG')
+        for key in pgs:
+            if PORT in key:
+                config_db.delete_entry('BUFFER_PG', key)
+
+        num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT",
+                              num_of_ports-1,
+                              polling_config = PollingConfig(polling_interval = 1, timeout = 5.00, strict = False))
+
+        # verify that the port wasn't removed since we still have buffer cfg
+        assert len(num) == num_of_ports
+
+        # modify buffer queue entry to egress_lossless_profile instead of egress_lossy_profile
+        config_db.update_entry("BUFFER_QUEUE", "%s|0-2"%PORT, {"profile": "egress_lossless_profile"})
+
+        # remove buffer queue cfg for the port
+        pgs = config_db.get_keys('BUFFER_QUEUE')
+        for key in pgs:
+            if PORT in key:
+                config_db.delete_entry('BUFFER_QUEUE', key)
+
+        num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT",
+                              num_of_ports-1,
+                              polling_config = PollingConfig(polling_interval = 1, timeout = 5.00, strict = True))
+
+        # verify that the port was removed properly since all buffer configuration was removed also
+        assert len(num) == num_of_ports - 1