Fix issue config qos reload causing orchagent aborted via tracking de…

…pendencies among QoS tables (#2116)

- What I did
Fix issue config qos reload causing orchagent aborted via tracking dependencies among QoS tables

1. Track dependencies among QoS tables.
2. Won't call SAI remove API for an object if it is still referenced.
3. Support removing/replacing one field in PORT_QOS_MAP and QUEUE tables.
4. Optimize logic to handle QUEUE table.
5. Remove switch level DSCP_TO_TC map before the map is removed.
6. Add mock test

- Why I did it
Fix issue.

- How I verified it
Manually test and mock test.

Signed-off-by: Stephen Sun <stephens@nvidia.com>

orchagent/orch.cpp

@@ -410,7 +410,8 @@ void Orch::removeMeFromObjsReferencedByMe(
     const string &table,
     const string &obj_name,
     const string &field,
-    const string &old_referenced_obj_name)
+    const string &old_referenced_obj_name,
+    bool remove_field)
 {
     vector<string> objects = tokenize(old_referenced_obj_name, list_item_delimiter);
     for (auto &obj : objects)
@@ -426,6 +427,12 @@ void Orch::removeMeFromObjsReferencedByMe(
                       referenced_table.c_str(), ref_obj_name.c_str(),
                       to_string(old_referenced_obj.m_objsDependingOnMe.size()).c_str());
     }
+
+    if (remove_field)
+    {
+        auto &referencing_object = (*type_maps[table])[obj_name];
+        referencing_object.m_objsReferencingByMe.erase(field);
+    }
 }
 
 void Orch::setObjectReference(
@@ -439,7 +446,7 @@ void Orch::setObjectReference(
     auto field_ref = obj.m_objsReferencingByMe.find(field);
 
     if (field_ref != obj.m_objsReferencingByMe.end())
-        removeMeFromObjsReferencedByMe(type_maps, table, obj_name, field, field_ref->second);
+        removeMeFromObjsReferencedByMe(type_maps, table, obj_name, field, field_ref->second, false);
 
     obj.m_objsReferencingByMe[field] = referenced_obj;
 
@@ -459,16 +466,44 @@ void Orch::setObjectReference(
     }
 }
 
+bool Orch::doesObjectExist(
+    type_map &type_maps,
+    const string &table,
+    const string &obj_name,
+    const string &field,
+    string &referenced_obj)
+{
+    auto &&searchRef = (*type_maps[table]).find(obj_name);
+    if (searchRef != (*type_maps[table]).end())
+    {
+        auto &obj = searchRef->second;
+        auto &&searchReferencingObjectRef = obj.m_objsReferencingByMe.find(field);
+        if (searchReferencingObjectRef != obj.m_objsReferencingByMe.end())
+        {
+            referenced_obj = searchReferencingObjectRef->second;
+            return true;
+        }
+    }
+
+    return false;
+}
+
 void Orch::removeObject(
     type_map &type_maps,
     const string &table,
     const string &obj_name)
 {
-    auto &obj = (*type_maps[table])[obj_name];
+    auto &&searchRef = (*type_maps[table]).find(obj_name);
+    if (searchRef == (*type_maps[table]).end())
+    {
+        return;
+    }
+
+    auto &obj = searchRef->second;
 
     for (auto field_ref : obj.m_objsReferencingByMe)
     {
-        removeMeFromObjsReferencedByMe(type_maps, table, obj_name, field_ref.first, field_ref.second);
+        removeMeFromObjsReferencedByMe(type_maps, table, obj_name, field_ref.first, field_ref.second, false);
     }
 
     // Update the field store

orchagent/orch.h

@@ -233,9 +233,11 @@ class Orch
     bool parseReference(type_map &type_maps, std::string &ref, const std::string &table_name, std::string &object_name);
     ref_resolve_status resolveFieldRefArray(type_map&, const std::string&, const std::string&, swss::KeyOpFieldsValuesTuple&, std::vector<sai_object_id_t>&, std::string&);
     void setObjectReference(type_map&, const std::string&, const std::string&, const std::string&, const std::string&);
+    bool doesObjectExist(type_map&, const std::string&, const std::string&, const std::string&, std::string&);
     void removeObject(type_map&, const std::string&, const std::string&);
     bool isObjectBeingReferenced(type_map&, const std::string&, const std::string&);
     std::string objectReferenceInfo(type_map&, const std::string&, const std::string&);
+    void removeMeFromObjsReferencedByMe(type_map &type_maps, const std::string &table, const std::string &obj_name, const std::string &field, const std::string &old_referenced_obj_name, bool remove_field=true);
 
     /* Note: consumer will be owned by this class */
     void addExecutor(Executor* executor);
@@ -250,7 +252,6 @@ class Orch
 
     ResponsePublisher m_publisher;
 private:
-    void removeMeFromObjsReferencedByMe(type_map &type_maps, const std::string &table, const std::string &obj_name, const std::string &field, const std::string &old_referenced_obj_name);
     void addConsumer(swss::DBConnector *db, std::string tableName, int pri = default_orch_pri);
 };
 

orchagent/orchdaemon.cpp

@@ -41,6 +41,7 @@ PbhOrch *gPbhOrch;
 MirrorOrch *gMirrorOrch;
 CrmOrch *gCrmOrch;
 BufferOrch *gBufferOrch;
+QosOrch *gQosOrch;
 SwitchOrch *gSwitchOrch;
 Directory<Orch*> gDirectory;
 NatOrch *gNatOrch;
@@ -216,7 +217,7 @@ bool OrchDaemon::init()
         CFG_DSCP_TO_FC_MAP_TABLE_NAME,
         CFG_EXP_TO_FC_MAP_TABLE_NAME
     };
-    QosOrch *qos_orch = new QosOrch(m_configDb, qos_tables);
+    gQosOrch = new QosOrch(m_configDb, qos_tables);
 
     vector<string> buffer_tables = {
         APP_BUFFER_POOL_TABLE_NAME,
@@ -325,7 +326,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, qos_orch, 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, debug_counter_orch, gMacsecOrch, gBfdOrch, gSrv6Orch};
 
     bool initialize_dtel = false;
     if (platform == BFN_PLATFORM_SUBSTRING || platform == VS_PLATFORM_SUBSTRING)

orchagent/qosorch.h

@@ -72,6 +72,7 @@ class DscpToTcMapHandler : public QosMapHandler
 public:
     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);
 };
@@ -196,5 +197,11 @@ 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;
 };
 #endif /* SWSS_QOSORCH_H */

tests/mock_tests/Makefile.am

@@ -24,6 +24,7 @@ LDADD_GTEST = -L/usr/src/gtest
 tests_SOURCES = aclorch_ut.cpp \
                 portsorch_ut.cpp \
                 routeorch_ut.cpp \
+                qosorch_ut.cpp \
                 saispy_ut.cpp \
                 consumer_ut.cpp \
                 ut_saihelper.cpp \

tests/mock_tests/mock_orchagent_main.h

@@ -10,6 +10,7 @@
 #include "fdborch.h"
 #include "mirrororch.h"
 #include "bufferorch.h"
+#include "qosorch.h"
 #include "vrforch.h"
 #include "vnetorch.h"
 #include "vxlanorch.h"
@@ -46,6 +47,7 @@ extern NeighOrch *gNeighOrch;
 extern FdbOrch *gFdbOrch;
 extern MirrorOrch *gMirrorOrch;
 extern BufferOrch *gBufferOrch;
+extern QosOrch *gQosOrch;
 extern VRFOrch *gVrfOrch;
 extern NhgOrch *gNhgOrch;
 extern Srv6Orch  *gSrv6Orch;
@@ -65,6 +67,10 @@ extern sai_tunnel_api_t *sai_tunnel_api;
 extern sai_next_hop_api_t *sai_next_hop_api;
 extern sai_hostif_api_t *sai_hostif_api;
 extern sai_buffer_api_t *sai_buffer_api;
+extern sai_qos_map_api_t *sai_qos_map_api;
+extern sai_scheduler_api_t *sai_scheduler_api;
+extern sai_scheduler_group_api_t *sai_scheduler_group_api;
+extern sai_wred_api_t *sai_wred_api;
 extern sai_queue_api_t *sai_queue_api;
 extern sai_udf_api_t* sai_udf_api;
 extern sai_mpls_api_t* sai_mpls_api;

tests/mock_tests/ut_saihelper.cpp

@@ -77,6 +77,10 @@ namespace ut_helper
         sai_api_query(SAI_API_ACL, (void **)&sai_acl_api);
         sai_api_query(SAI_API_HOSTIF, (void **)&sai_hostif_api);
         sai_api_query(SAI_API_BUFFER, (void **)&sai_buffer_api);
+        sai_api_query(SAI_API_QOS_MAP, (void **)&sai_qos_map_api);
+        sai_api_query(SAI_API_SCHEDULER_GROUP, (void **)&sai_scheduler_group_api);
+        sai_api_query(SAI_API_SCHEDULER, (void **)&sai_scheduler_api);
+        sai_api_query(SAI_API_WRED, (void **)&sai_wred_api);
         sai_api_query(SAI_API_QUEUE, (void **)&sai_queue_api);
         sai_api_query(SAI_API_MPLS, (void**)&sai_mpls_api);