Add check ASIC vs DB state after comparison logic (sonic-net#393)

* Add check ASIC vs DB state after comparison logic

* Add aspell exceptions

* Add cmd flag to enable consistency check explicitly

Author: kcudnik

meta/sai_meta.cpp

@@ -2431,6 +2431,10 @@ sai_status_t meta_generic_validation_get(
                 // primitives
                 break;
 
+            case SAI_ATTR_VALUE_TYPE_ACL_CAPABILITY:
+                VALIDATION_LIST(md, value.aclcapability.action_list);
+                break;
+
             default:
 
                 // acl capability will is more complex since is in/out we need to check stage

syncd/syncd.cpp

@@ -70,6 +70,12 @@ std::map<sai_object_id_t, std::shared_ptr<SaiSwitch>> switches;
  */
 std::set<sai_object_id_t> initViewRemovedVidSet;
 
+/*
+ * When set to true will enable DB vs ASIC consistency check after comparison
+ * logic.
+ */
+bool g_enableConsistencyCheck = false;
+
 /*
  * By default we are in APPLY mode.
  */
@@ -1358,12 +1364,20 @@ sai_status_t handle_generic(
 }
 
 void translate_vid_to_rid_non_object_id(
-        _In_ sai_object_meta_key_t &meta_key)
+        _Inout_ sai_object_meta_key_t &meta_key)
 {
     SWSS_LOG_ENTER();
 
     auto info = sai_metadata_get_object_type_info(meta_key.objecttype);
 
+    if (info->isobjectid)
+    {
+        meta_key.objectkey.key.object_id =
+            translate_vid_to_rid(meta_key.objectkey.key.object_id);
+
+        return;
+    }
+
     for (size_t j = 0; j < info->structmemberscount; ++j)
     {
         const sai_struct_member_info_t *m = info->structmembers[j];
@@ -2942,6 +2956,8 @@ void printUsage()
     std::cout << "        Disable sleep when syncd crashes" << std::endl;
     std::cout << "    -U --eableUnittests" << std::endl;
     std::cout << "        Metadata enable unittests" << std::endl;
+    std::cout << "    -C --eableConsistencyCheck" << std::endl;
+    std::cout << "        Enable consisteny check DB vs ASIC after comparison logic" << std::endl;
 #ifdef SAITHRIFT
     std::cout << "    -r --rpcserver"           << std::endl;
     std::cout << "        Enable rpcserver"      << std::endl;
@@ -2961,27 +2977,28 @@ void handleCmdLine(int argc, char **argv)
 
 #ifdef SAITHRIFT
     options.run_rpc_server = false;
-    const char* const optstring = "dNUt:p:i:rm:huS";
+    const char* const optstring = "dNUCt:p:i:rm:huS";
 #else
-    const char* const optstring = "dNUt:p:i:huS";
+    const char* const optstring = "dNUCt:p:i:huS";
 #endif // SAITHRIFT
 
     while(true)
     {
         static struct option long_options[] =
         {
-            { "useTempView",      no_argument,       0, 'u' },
-            { "diag",             no_argument,       0, 'd' },
-            { "startType",        required_argument, 0, 't' },
-            { "profile",          required_argument, 0, 'p' },
-            { "help",             no_argument,       0, 'h' },
-            { "disableExitSleep", no_argument,       0, 'S' },
-            { "enableUnittests",  no_argument,       0, 'U' },
+            { "useTempView",             no_argument,       0, 'u' },
+            { "diag",                    no_argument,       0, 'd' },
+            { "startType",               required_argument, 0, 't' },
+            { "profile",                 required_argument, 0, 'p' },
+            { "help",                    no_argument,       0, 'h' },
+            { "disableExitSleep",        no_argument,       0, 'S' },
+            { "enableUnittests",         no_argument,       0, 'U' },
+            { "enableConsistencyCheck",  no_argument,       0, 'C' },
 #ifdef SAITHRIFT
-            { "rpcserver",        no_argument,       0, 'r' },
-            { "portmap",          required_argument, 0, 'm' },
+            { "rpcserver",               no_argument,       0, 'r' },
+            { "portmap",                 required_argument, 0, 'm' },
 #endif // SAITHRIFT
-            { 0,                  0,                 0,  0  }
+            { 0,                         0,                 0,  0  }
         };
 
         int option_index = 0;
@@ -3000,6 +3017,11 @@ void handleCmdLine(int argc, char **argv)
                 options.enableUnittests = true;
                 break;
 
+            case 'C':
+                SWSS_LOG_NOTICE("enable consistency check");
+                g_enableConsistencyCheck = true;
+                break;
+
             case 'u':
                 SWSS_LOG_NOTICE("enable use temp view");
                 options.useTempView = true;

syncd/syncd.h

@@ -77,6 +77,14 @@ void performWarmRestart();
 
 sai_object_id_t translate_vid_to_rid(_In_ sai_object_id_t vid);
 
+void translate_vid_to_rid_list(
+        _In_ sai_object_type_t object_type,
+        _In_ uint32_t attr_count,
+        _In_ sai_attribute_t *attr_list);
+
+void translate_vid_to_rid_non_object_id(
+        _Inout_ sai_object_meta_key_t &meta_key);
+
 void redisClearVidToRidMap();
 void redisClearRidToVidMap();
 
@@ -86,6 +94,8 @@ sai_object_type_t getObjectTypeFromVid(
 extern std::shared_ptr<swss::NotificationProducer>  notifications;
 extern std::shared_ptr<swss::RedisClient>   g_redisClient;
 
+extern bool g_enableConsistencyCheck;
+
 sai_object_id_t redis_create_virtual_object_id(
         _In_ sai_object_id_t switch_id,
         _In_ sai_object_type_t object_type);

syncd/syncd_applyview.cpp

@@ -2062,14 +2062,11 @@ bool hasEqualObjectList(
  * @return True if attributes are equal, false otherwise.
  */
 bool hasEqualQosMapList(
-        _In_ const std::shared_ptr<const SaiAttr> &current,
-        _In_ const std::shared_ptr<const SaiAttr> &temporary)
+        _In_ const sai_qos_map_list_t& c,
+        _In_ const sai_qos_map_list_t& t)
 {
     SWSS_LOG_ENTER();
 
-    auto c = current->getSaiAttr()->value.qosmap;
-    auto t = temporary->getSaiAttr()->value.qosmap;
-
     if (c.count != t.count)
         return false;
 
@@ -2102,6 +2099,18 @@ bool hasEqualQosMapList(
     return true;
 }
 
+bool hasEqualQosMapList(
+        _In_ const std::shared_ptr<const SaiAttr> &current,
+        _In_ const std::shared_ptr<const SaiAttr> &temporary)
+{
+    SWSS_LOG_ENTER();
+
+    auto c = current->getSaiAttr()->value.qosmap;
+    auto t = temporary->getSaiAttr()->value.qosmap;
+
+    return hasEqualQosMapList(c, t);
+}
+
 /**
  * @brief Check if current and temporary object has
  * the same attribute and attribute has the same value on both.
@@ -5992,6 +6001,9 @@ void processObjectForViewTransition(
              */
         }
 
+        // No need to store VID since at this point we don't have RID yet, it will be
+        // created on execute asic and RID will be saved to maps in both views.
+
         createNewObjectFromTemporaryObject(currentView, temporaryView, temporaryObj);
 
         return;
@@ -6026,6 +6038,10 @@ void processObjectForViewTransition(
         sai_object_id_t vid = temporaryObj->getVid();
 
         currentView.insertNewVidReference(vid);
+
+        // save temporary vid to rid after match
+        // can't be here since vim to rid map will not match
+        // currentView.vidToRid[vid] = currentView.vidToRid.at(currentBestMatch->getVid());
     }
 
     performObjectSetTransition(currentView, temporaryView, currentBestMatch, temporaryObj, true);
@@ -6573,6 +6589,162 @@ void logViewObjectCount(
     }
 }
 
+void checkAsicVsDatabaseConsistency(
+        _In_ const AsicView cur,
+        _In_ const AsicView &tmp)
+{
+    SWSS_LOG_ENTER();
+
+    bool hasErrors = false;
+
+    {
+        SWSS_LOG_TIMER("consistency check");
+
+        SWSS_LOG_WARN("performing consistency check");
+
+        for (const auto &pair: tmp.soAll)
+        {
+            const auto &obj = pair.second;
+
+            const auto &attrs = obj->getAllAttributes();
+
+            // get object meta key for get (object id or *entry)
+
+            sai_object_meta_key_t meta_key = obj->meta_key;
+
+            // translate all VID's to RIDs in non object is's
+
+            translate_vid_to_rid_non_object_id(meta_key);
+
+            auto info = sai_metadata_get_object_type_info(obj->getObjectType());
+
+            sai_attribute_t attr;
+
+            memset(&attr, 0, sizeof(attr));
+
+            if (attrs.size() == 0)
+            {
+                // get first attribute and do a get query to see if object exist's
+
+                auto meta = info->attrmetadata[0];
+
+                sai_status_t status = info->get(&meta_key, 1, &attr);
+
+                switch (status)
+                {
+                    case SAI_STATUS_SUCCESS:
+                    case SAI_STATUS_BUFFER_OVERFLOW:
+                        continue;
+
+                    case SAI_STATUS_NOT_IMPLEMENTED:
+                    case SAI_STATUS_NOT_SUPPORTED:
+
+                        SWSS_LOG_WARN("GET api for %s is not implemented on %s",
+                                meta->attridname,
+                                obj->str_object_id.c_str());
+                    continue;
+                }
+
+                SWSS_LOG_ERROR("failed to get %s on %s: %s",
+                        meta->attridname,
+                        obj->str_object_id.c_str(),
+                        sai_serialize_status(status).c_str());
+
+                hasErrors = true;
+
+                continue;
+            }
+
+            for (const auto &ap: attrs)
+            {
+                const auto &saiAttr = ap.second;
+
+                auto meta = saiAttr->getAttrMetadata();
+
+                // deserialize existing attribute so deserialize will allocate
+                // memory for all list's
+
+                attr.id = meta->attrid;
+
+                sai_deserialize_attr_value(saiAttr->getStrAttrValue(), *meta, attr, false);
+
+                // translate all VIDs from DB to RIDs for compare
+
+                translate_vid_to_rid_list(obj->getObjectType(), 1, &attr);
+
+                // get attr value with RIDs
+
+                const std::string& dbValue = sai_serialize_attr_value(*meta, attr);
+
+                sai_status_t status = info->get(&meta_key, 1, &attr);
+
+                if (meta->attrid == SAI_QOS_MAP_ATTR_MAP_TO_VALUE_LIST && meta->objecttype == SAI_OBJECT_TYPE_QOS_MAP && status == SAI_STATUS_SUCCESS)
+                {
+                    // order does not matter on this list
+
+                    if (hasEqualQosMapList(attr.value.qosmap, saiAttr->getSaiAttr()->value.qosmap))
+                    {
+                        sai_deserialize_free_attribute_value(meta->attrvaluetype, attr);
+                        continue;
+                    }
+                }
+
+                // free possible allocated lists
+
+                if (status != SAI_STATUS_SUCCESS)
+                {
+                    SWSS_LOG_ERROR("failed to get %s on %s %s",
+                            meta->attridname,
+                            obj->str_object_id.c_str(),
+                            sai_serialize_status(status).c_str());
+
+                    hasErrors = true;
+
+                    sai_deserialize_free_attribute_value(meta->attrvaluetype, attr);
+                    continue;
+                }
+
+                const std::string &asicValue = sai_serialize_attr_value(*meta, attr);
+
+                sai_deserialize_free_attribute_value(meta->attrvaluetype, attr);
+
+                // pointers will not be equal since those will be from
+                // different process memory maps so just check if both pointers
+                // are NULL or both are SET
+
+                if (meta->attrvaluetype == SAI_ATTR_VALUE_TYPE_POINTER)
+                {
+                    if (attr.value.ptr == NULL && saiAttr->getSaiAttr()->value.ptr == NULL)
+                        continue;
+
+                    if (attr.value.ptr != NULL && saiAttr->getSaiAttr()->value.ptr != NULL)
+                        continue;
+                }
+
+                if (asicValue == dbValue)
+                    continue;
+
+                SWSS_LOG_ERROR("value missmatch: %s on %s: ASIC: %s DB: %s, inconsistent state!",
+                        meta->attridname,
+                        obj->str_object_id.c_str(),
+                        asicValue.c_str(),
+                        dbValue.c_str());
+
+                hasErrors = true;
+            }
+        }
+
+        swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_INFO);
+    }
+
+    swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_NOTICE);
+
+    if (hasErrors && enableUnittests())
+    {
+        SWSS_LOG_THROW("ASIC content is differnt than DB content!");
+    }
+}
+
 sai_status_t syncdApplyView()
 {
     SWSS_LOG_ENTER();
@@ -6792,6 +6964,11 @@ sai_status_t syncdApplyView()
 
     updateRedisDatabase(current, temp);
 
+    if (g_enableConsistencyCheck)
+    {
+        checkAsicVsDatabaseConsistency(current, temp);
+    }
+
     return SAI_STATUS_SUCCESS;
 }
 
@@ -6845,6 +7022,8 @@ sai_object_id_t asic_translate_vid_to_rid(
 
     sai_object_id_t rid = currentIt->second;
 
+    SWSS_LOG_INFO("translated VID 0x%lx to RID 0x%lx", vid, rid);
+
     return rid;
 }
 
@@ -7099,6 +7278,14 @@ void asic_translate_vid_to_rid_non_object_id(
 
     auto info = sai_metadata_get_object_type_info(meta_key.objecttype);
 
+    if (info->isobjectid)
+    {
+        meta_key.objectkey.key.object_id =
+            asic_translate_vid_to_rid(current, temporary, meta_key.objectkey.key.object_id);
+
+        return;
+    }
+
     for (size_t idx = 0; idx < info->structmemberscount; ++idx)
     {
         const sai_struct_member_info_t *m = info->structmembers[idx];