Apply DSCP_TO_TC_MAP from PORT_QOS_MAP|global to switch level (sonic-net#2314)

bingwang-ms · web-flow · commit 43743483da43 · 2022-06-15T15:41:36.000+08:00
* Apply DSCP_TO_TC_MAP|AZURE to switch level

Signed-off-by: bingwang &lt;wang.bing@microsoft.com&gt;
diff --git a/cfgmgr/buffermgr.cpp b/cfgmgr/buffermgr.cpp
@@ -16,6 +16,8 @@
 using namespace std;
 using namespace swss;
 
+#define PORT_NAME_GLOBAL "global"
+
 BufferMgr::BufferMgr(DBConnector *cfgDb, DBConnector *applDb, string pg_lookup_file, const vector<string> &tableNames) :
         Orch(cfgDb, tableNames),
         m_cfgPortTable(cfgDb, CFG_PORT_TABLE_NAME),
@@ -413,6 +415,12 @@ void BufferMgr::doPortQosTableTask(Consumer &consumer)
     {
         KeyOpFieldsValuesTuple tuple = it->second;
         string port_name = kfvKey(tuple);
+        if (port_name == PORT_NAME_GLOBAL)
+        {
+            // Ignore the entry for global level
+            it = consumer.m_toSync.erase(it);
+            continue;
+        }
         string op = kfvOp(tuple);
         if (op == SET_COMMAND)
         {
diff --git a/orchagent/qosorch.cpp b/orchagent/qosorch.cpp
@@ -104,6 +104,8 @@ map<string, string> qos_to_ref_table_map = {
 #define DSCP_MAX_VAL 63
 #define EXP_MAX_VAL 7
 
+#define PORT_NAME_GLOBAL "global"
+
 task_process_status QosMapHandler::processWorkItem(Consumer& consumer, KeyOpFieldsValuesTuple &tuple)
 {
     SWSS_LOG_ENTER();
@@ -236,37 +238,6 @@ bool DscpToTcMapHandler::convertFieldValuesToAttributes(KeyOpFieldsValuesTuple &
     return true;
 }
 
-void DscpToTcMapHandler::applyDscpToTcMapToSwitch(sai_attr_id_t attr_id, sai_object_id_t map_id)
-{
-    SWSS_LOG_ENTER();
-    bool rv = true;
-
-    /* Query DSCP_TO_TC QoS map at switch capability */
-    rv = gSwitchOrch->querySwitchDscpToTcCapability(SAI_OBJECT_TYPE_SWITCH, SAI_SWITCH_ATTR_QOS_DSCP_TO_TC_MAP);
-    if (rv == false)
-    {
-        SWSS_LOG_ERROR("Switch level DSCP to TC QoS map configuration is not supported");
-        return;
-    }
-
-    /* Apply DSCP_TO_TC QoS map at switch */
-    sai_attribute_t attr;
-    attr.id = attr_id;
-    attr.value.oid = map_id;
-
-    sai_status_t status = sai_switch_api->set_switch_attribute(gSwitchId, &attr);
-    if (status != SAI_STATUS_SUCCESS)
-    {
-        SWSS_LOG_ERROR("Failed to apply DSCP_TO_TC QoS map to switch rv:%d", status);
-        return;
-    }
-
-    if (map_id != gQosOrch->m_globalDscpToTcMap)
-        gQosOrch->m_globalDscpToTcMap = map_id;
-
-    SWSS_LOG_NOTICE("Applied DSCP_TO_TC QoS map to switch successfully");
-}
-
 sai_object_id_t DscpToTcMapHandler::addQosItem(const vector<sai_attribute_t> &attributes)
 {
     SWSS_LOG_ENTER();
@@ -292,36 +263,13 @@ sai_object_id_t DscpToTcMapHandler::addQosItem(const vector<sai_attribute_t> &at
     }
     SWSS_LOG_DEBUG("created QosMap object:%" PRIx64, sai_object);
 
-    applyDscpToTcMapToSwitch(SAI_SWITCH_ATTR_QOS_DSCP_TO_TC_MAP, sai_object);
-
     return sai_object;
 }
 
 bool DscpToTcMapHandler::removeQosItem(sai_object_id_t sai_object)
 {
     SWSS_LOG_ENTER();
 
-    if (sai_object == gQosOrch->m_globalDscpToTcMap)
-    {
-        // The current global dscp to tc map is about to be removed.
-        // Find another one to set to the switch or NULL in case this is the last one
-        const auto &dscpToTcObjects = (*QosOrch::getTypeMap()[CFG_DSCP_TO_TC_MAP_TABLE_NAME]);
-        bool found = false;
-        for (const auto &ref : dscpToTcObjects)
-        {
-            if (ref.second.m_saiObjectId == sai_object)
-                continue;
-            SWSS_LOG_NOTICE("Current global dscp_to_tc map is about to be removed, set it to %s %" PRIx64, ref.first.c_str(), ref.second.m_saiObjectId);
-            applyDscpToTcMapToSwitch(SAI_SWITCH_ATTR_QOS_DSCP_TO_TC_MAP, ref.second.m_saiObjectId);
-            found = true;
-            break;
-        }
-        if (!found)
-        {
-            applyDscpToTcMapToSwitch(SAI_SWITCH_ATTR_QOS_DSCP_TO_TC_MAP, SAI_NULL_OBJECT_ID);
-        }
-    }
-
     SWSS_LOG_DEBUG("Removing DscpToTcMap object:%" PRIx64, sai_object);
     sai_status_t sai_status = sai_qos_map_api->remove_qos_map(sai_object);
     if (SAI_STATUS_SUCCESS != sai_status)
@@ -1717,12 +1665,112 @@ task_process_status QosOrch::handleQueueTable(Consumer& consumer, KeyOpFieldsVal
     return task_process_status::task_success;
 }
 
+bool QosOrch::applyDscpToTcMapToSwitch(sai_attr_id_t attr_id, sai_object_id_t map_id)
+{
+    SWSS_LOG_ENTER();
+
+    /* Query DSCP_TO_TC QoS map at switch capability */
+    bool rv = gSwitchOrch->querySwitchDscpToTcCapability(SAI_OBJECT_TYPE_SWITCH, SAI_SWITCH_ATTR_QOS_DSCP_TO_TC_MAP);
+    if (rv == false)
+    {
+        SWSS_LOG_ERROR("Switch level DSCP to TC QoS map configuration is not supported");
+        return true;
+    }
+
+    /* Apply DSCP_TO_TC QoS map at switch */
+    sai_attribute_t attr;
+    attr.id = attr_id;
+    attr.value.oid = map_id;
+
+    sai_status_t status = sai_switch_api->set_switch_attribute(gSwitchId, &attr);
+    if (status != SAI_STATUS_SUCCESS)
+    {
+        SWSS_LOG_ERROR("Failed to apply DSCP_TO_TC QoS map to switch rv:%d", status);
+        return false;
+    }
+
+    SWSS_LOG_NOTICE("Applied DSCP_TO_TC QoS map to switch successfully");
+    return true;
+}
+
+task_process_status QosOrch::handleGlobalQosMap(const string &OP, KeyOpFieldsValuesTuple &tuple)
+{
+    SWSS_LOG_ENTER();
+
+    task_process_status task_status = task_process_status::task_success;
+    
+    if (OP == DEL_COMMAND)
+    {
+        string referenced_obj;
+        if (!doesObjectExist(m_qos_maps, CFG_PORT_QOS_MAP_TABLE_NAME, PORT_NAME_GLOBAL, dscp_to_tc_field_name, referenced_obj))
+        {
+            return task_status;
+        }
+        // Set SAI_NULL_OBJECT_ID to switch level if PORT_QOS_MAP|global is removed
+        if (applyDscpToTcMapToSwitch(SAI_SWITCH_ATTR_QOS_DSCP_TO_TC_MAP, SAI_NULL_OBJECT_ID))
+        {
+            removeObject(m_qos_maps, CFG_PORT_QOS_MAP_TABLE_NAME, PORT_NAME_GLOBAL);
+            task_status = task_process_status::task_success;
+            SWSS_LOG_INFO("Global QoS map type %s is removed", dscp_to_tc_field_name.c_str());
+        }
+        else
+        {
+            task_status = task_process_status::task_failed;
+            SWSS_LOG_WARN("Failed to remove switch level QoS map type %s", dscp_to_tc_field_name.c_str());
+        }
+        return task_status;
+    }
+
+    for (auto it = kfvFieldsValues(tuple).begin(); it != kfvFieldsValues(tuple).end(); it++)
+    {
+        string map_type_name = fvField(*it);
+        string map_name = fvValue(*it);
+        if (map_type_name != dscp_to_tc_field_name)
+        {
+            SWSS_LOG_WARN("Qos map type %s is not supported at global level", map_type_name.c_str());
+            continue;
+        }
+
+        if (qos_to_attr_map.find(map_type_name) != qos_to_attr_map.end())
+        {
+            sai_object_id_t id;
+            string object_name;
+            ref_resolve_status status = resolveFieldRefValue(m_qos_maps, map_type_name, qos_to_ref_table_map.at(map_type_name), tuple, id, object_name);
+
+            if (status != ref_resolve_status::success)
+            {
+                SWSS_LOG_INFO("Global QoS map %s is not yet created", map_name.c_str());
+                task_status = task_process_status::task_need_retry;
+            }
+
+            if (applyDscpToTcMapToSwitch(SAI_SWITCH_ATTR_QOS_DSCP_TO_TC_MAP, id))
+            {
+                setObjectReference(m_qos_maps, CFG_PORT_QOS_MAP_TABLE_NAME, PORT_NAME_GLOBAL, map_type_name, object_name);
+                task_status = task_process_status::task_success;
+                SWSS_LOG_INFO("Applied QoS map type %s name %s to switch level", map_type_name.c_str(), object_name.c_str());
+            }
+            else
+            {
+                task_status = task_process_status::task_failed;
+                SWSS_LOG_INFO("Failed to apply QoS map type %s name %s to switch level", map_type_name.c_str(), object_name.c_str());
+            }
+        }
+    }
+    return task_status;
+}
+
 task_process_status QosOrch::handlePortQosMapTable(Consumer& consumer, KeyOpFieldsValuesTuple &tuple)
 {
     SWSS_LOG_ENTER();
 
     string key = kfvKey(tuple);
     string op = kfvOp(tuple);
+
+    if (key == PORT_NAME_GLOBAL)
+    {
+        return handleGlobalQosMap(op, tuple);
+    }
+
     vector<string> port_names = tokenize(key, list_item_delimiter);
 
     if (op == DEL_COMMAND)
diff --git a/orchagent/qosorch.h b/orchagent/qosorch.h
@@ -78,8 +78,6 @@ class DscpToTcMapHandler : public QosMapHandler
     bool convertFieldValuesToAttributes(KeyOpFieldsValuesTuple &tuple, vector<sai_attribute_t> &attributes) override;
     sai_object_id_t addQosItem(const vector<sai_attribute_t> &attributes) override;
     bool removeQosItem(sai_object_id_t sai_object);
-protected:
-    void applyDscpToTcMapToSwitch(sai_attr_id_t attr_id, sai_object_id_t sai_dscp_to_tc_map);
 };
 
 class MplsTcToTcMapHandler : public QosMapHandler
@@ -195,11 +193,13 @@ class QosOrch : public Orch
     task_process_status handleExpToFcTable(Consumer& consumer, KeyOpFieldsValuesTuple &tuple);
     task_process_status handleTcToDscpTable(Consumer& consumer, KeyOpFieldsValuesTuple &tuple);
 
+    task_process_status handleGlobalQosMap(const string &op, KeyOpFieldsValuesTuple &tuple);
+
     sai_object_id_t getSchedulerGroup(const Port &port, const sai_object_id_t queue_id);
 
     bool applySchedulerToQueueSchedulerGroup(Port &port, size_t queue_ind, sai_object_id_t scheduler_profile_id);
     bool applyWredProfileToQueue(Port &port, size_t queue_ind, sai_object_id_t sai_wred_profile);
-
+    bool applyDscpToTcMapToSwitch(sai_attr_id_t attr_id, sai_object_id_t sai_dscp_to_tc_map);
 private:
     qos_table_handler_map m_qos_handler_map;
 
@@ -212,9 +212,6 @@ class QosOrch : public Orch
 
     std::unordered_map<sai_object_id_t, SchedulerGroupPortInfo_t> m_scheduler_group_port_info;
 
-    // SAI OID of the global dscp to tc map
-    sai_object_id_t m_globalDscpToTcMap;
-
     friend QosMapHandler;
     friend DscpToTcMapHandler;
 };
diff --git a/tests/mock_tests/qosorch_ut.cpp b/tests/mock_tests/qosorch_ut.cpp
@@ -723,8 +723,6 @@ namespace qosorch_test
         static_cast<Orch *>(gQosOrch)->doTask();
         ASSERT_EQ(++current_sai_remove_qos_map_count, sai_remove_qos_map_count);
         ASSERT_EQ((*QosOrch::getTypeMap()[CFG_DSCP_TO_TC_MAP_TABLE_NAME]).count("AZURE"), 0);
-        // Global dscp to tc map should not be cleared
-        ASSERT_EQ((*QosOrch::getTypeMap()[CFG_DSCP_TO_TC_MAP_TABLE_NAME])["AZURE_1"].m_saiObjectId, switch_dscp_to_tc_map_id);
 
         // Make sure other dependencies are not touched
         CheckDependency(CFG_PORT_QOS_MAP_TABLE_NAME, "Ethernet0", "pfc_to_pg_map", CFG_PFC_PRIORITY_TO_PRIORITY_GROUP_MAP_TABLE_NAME, "AZURE");
@@ -931,12 +929,9 @@ namespace qosorch_test
 
     TEST_F(QosOrchTest, QosOrchTestGlobalDscpToTcMap)
     {
-        // Make sure dscp to tc map is correct
-        ASSERT_EQ((*QosOrch::getTypeMap()[CFG_DSCP_TO_TC_MAP_TABLE_NAME])["AZURE"].m_saiObjectId, switch_dscp_to_tc_map_id);
-
         // Create a new dscp to tc map
         std::deque<KeyOpFieldsValuesTuple> entries;
-        entries.push_back({"AZURE_1", "SET",
+        entries.push_back({"AZURE", "SET",
                            {
                                {"1", "0"},
                                {"0", "1"}
@@ -945,16 +940,42 @@ namespace qosorch_test
         auto consumer = dynamic_cast<Consumer *>(gQosOrch->getExecutor(CFG_DSCP_TO_TC_MAP_TABLE_NAME));
         consumer->addToSync(entries);
         entries.clear();
-        // Drain DSCP_TO_TC_MAP table
+
+        entries.push_back({"global", "SET",
+                            {
+                                {"dscp_to_tc_map", "AZURE"}
+                            }});
+        consumer = dynamic_cast<Consumer *>(gQosOrch->getExecutor(CFG_PORT_QOS_MAP_TABLE_NAME));
+        consumer->addToSync(entries);
+        entries.clear();
+
+        // Drain DSCP_TO_TC_MAP and PORT_QOS_MAP table
+        static_cast<Orch *>(gQosOrch)->doTask();
+        // Check DSCP_TO_TC_MAP|AZURE is applied to switch
+        ASSERT_EQ((*QosOrch::getTypeMap()[CFG_DSCP_TO_TC_MAP_TABLE_NAME])["AZURE"].m_saiObjectId, switch_dscp_to_tc_map_id);
+
+        // Remove global DSCP_TO_TC_MAP
+        entries.push_back({"global", "DEL", {}});
+        consumer = dynamic_cast<Consumer *>(gQosOrch->getExecutor(CFG_PORT_QOS_MAP_TABLE_NAME));
+        consumer->addToSync(entries);
+        entries.clear();
+        // Drain PORT_QOS_TABLE table
         static_cast<Orch *>(gQosOrch)->doTask();
-        ASSERT_EQ((*QosOrch::getTypeMap()[CFG_DSCP_TO_TC_MAP_TABLE_NAME])["AZURE_1"].m_saiObjectId, switch_dscp_to_tc_map_id);
+        // Check switch_level dscp_to_tc_map is set to NULL
+        ASSERT_EQ(SAI_NULL_OBJECT_ID, switch_dscp_to_tc_map_id);
 
-        entries.push_back({"AZURE_1", "DEL", {}});
+        entries.push_back({"AZURE", "DEL", {}});
+        consumer = dynamic_cast<Consumer *>(gQosOrch->getExecutor(CFG_DSCP_TO_TC_MAP_TABLE_NAME));
         consumer->addToSync(entries);
         entries.clear();
+
+        auto current_sai_remove_qos_map_count = sai_remove_qos_map_count;
         // Drain DSCP_TO_TC_MAP table
         static_cast<Orch *>(gQosOrch)->doTask();
-        ASSERT_EQ((*QosOrch::getTypeMap()[CFG_DSCP_TO_TC_MAP_TABLE_NAME])["AZURE"].m_saiObjectId, switch_dscp_to_tc_map_id);
+        // Check DSCP_TO_TC_MAP|AZURE is removed, and the switch_level dscp_to_tc_map is set to NULL
+        ASSERT_EQ(current_sai_remove_qos_map_count + 1, sai_remove_qos_map_count);
+        ASSERT_EQ((*QosOrch::getTypeMap()[CFG_DSCP_TO_TC_MAP_TABLE_NAME]).count("AZURE"), 0);
+        
     }
 
     TEST_F(QosOrchTest, QosOrchTestRetryFirstItem)
diff --git a/tests/test_qos_map.py b/tests/test_qos_map.py
@@ -370,6 +370,76 @@ def test_port_mpls_tc(self, dvs):
         port_cnt = len(swsscommon.Table(self.config_db, CFG_PORT_TABLE_NAME).getKeys())
         assert port_cnt == cnt
 
+class TestDscpToTcMap(object):
+    ASIC_QOS_MAP_STR = "ASIC_STATE:SAI_OBJECT_TYPE_QOS_MAP"
+    ASIC_PORT_STR = "ASIC_STATE:SAI_OBJECT_TYPE_PORT"
+    ASIC_SWITCH_STR = "ASIC_STATE:SAI_OBJECT_TYPE_SWITCH"
+
+    def init_test(self, dvs):
+        dvs.setup_db()
+        self.asic_db = dvs.get_asic_db()
+        self.config_db = dvs.get_config_db()
+        self.asic_qos_map_ids = self.asic_db.get_keys(self.ASIC_QOS_MAP_STR)
+        self.asic_qos_map_count = len(self.asic_qos_map_ids)
+        self.dscp_to_tc_table = swsscommon.Table(self.config_db.db_connection, swsscommon.CFG_DSCP_TO_TC_MAP_TABLE_NAME)
+        self.port_qos_table = swsscommon.Table(self.config_db.db_connection, swsscommon.CFG_PORT_QOS_MAP_TABLE_NAME)
+
+    def get_qos_id(self):
+        diff = set(self.asic_db.get_keys(self.ASIC_QOS_MAP_STR)) - set(self.asic_qos_map_ids)
+        assert len(diff) <= 1
+        return None if len(diff) == 0 else diff.pop()
+    
+    def test_dscp_to_tc_map_applied_to_switch(self, dvs):
+        self.init_test(dvs)
+        dscp_to_tc_map_id = None
+        created_new_map = False
+        try:
+            existing_map = self.dscp_to_tc_table.getKeys()
+            if "AZURE" not in existing_map: 
+                # Create a DSCP_TO_TC map
+                dscp_to_tc_map = [(str(i), str(i)) for i in range(0, 63)]
+                self.dscp_to_tc_table.set("AZURE", swsscommon.FieldValuePairs(dscp_to_tc_map))
+
+                self.asic_db.wait_for_n_keys(self.ASIC_QOS_MAP_STR, self.asic_qos_map_count + 1)
+
+                # Get the DSCP_TO_TC map ID
+                dscp_to_tc_map_id = self.get_qos_id()
+                assert(dscp_to_tc_map_id is not None)
+
+                # Assert the expected values
+                fvs = self.asic_db.get_entry(self.ASIC_QOS_MAP_STR, dscp_to_tc_map_id)
+                assert(fvs.get("SAI_QOS_MAP_ATTR_TYPE") == "SAI_QOS_MAP_TYPE_DSCP_TO_TC")
+                created_new_map = True
+            else:
+                for id in self.asic_qos_map_ids:
+                    fvs = self.asic_db.get_entry(self.ASIC_QOS_MAP_STR, id)
+                    if fvs.get("SAI_QOS_MAP_ATTR_TYPE") == "SAI_QOS_MAP_TYPE_DSCP_TO_TC":
+                        dscp_to_tc_map_id = id
+                        break
+            switch_oid = dvs.getSwitchOid()
+            # Check switch level DSCP_TO_TC_MAP doesn't before PORT_QOS_MAP|global is created
+            fvs = self.asic_db.get_entry(self.ASIC_SWITCH_STR, switch_oid)
+            assert("SAI_SWITCH_ATTR_QOS_DSCP_TO_TC_MAP" not in fvs)
+
+            # Insert switch level map entry 
+            self.port_qos_table.set("global", [("dscp_to_tc_map", "AZURE")])
+            time.sleep(1)
+
+            # Check the switch level DSCP_TO_TC_MAP is applied
+            fvs = self.asic_db.get_entry(self.ASIC_SWITCH_STR, switch_oid)
+            assert(fvs.get("SAI_SWITCH_ATTR_QOS_DSCP_TO_TC_MAP") == dscp_to_tc_map_id)
+
+            # Remove the global level DSCP_TO_TC_MAP
+            self.port_qos_table._del("global")
+            time.sleep(1)
+
+            # Check the global level DSCP_TO_TC_MAP is set to SAI_
+            fvs = self.asic_db.get_entry(self.ASIC_SWITCH_STR, switch_oid)
+            assert(fvs.get("SAI_SWITCH_ATTR_QOS_DSCP_TO_TC_MAP") == "oid:0x0")
+        finally:
+            if created_new_map:
+                self.dscp_to_tc_table._del("AZURE")
+        
 
 # Add Dummy always-pass test at end as workaroud
 # for issue when Flaky fail on final test it invokes module tear-down before retrying
