Add synchronous mode to sairedis library (sonic-net#617)

* [saiplayer] Add sync mode to saiplayer

* [syncd] Add RedisClient more redis methods

* [syncd] Update redis db in syncd only when success

* [test] Add sync mode unittest

Author: kcudnik

saiplayer/CommandLineOptions.cpp

@@ -17,6 +17,7 @@ CommandLineOptions::CommandLineOptions()
     m_skipNotifySyncd = false;
     m_enableDebug = false;
     m_sleep = false;
+    m_syncMode = false;
 }
 
 std::string CommandLineOptions::getCommandLineString() const
@@ -30,6 +31,7 @@ std::string CommandLineOptions::getCommandLineString() const
     ss << " SkipNotifySyncd=" << (m_skipNotifySyncd ? "YES" : "NO");
     ss << " EnableDebug=" << (m_enableDebug ? "YES" : "NO");
     ss << " Sleep=" << (m_sleep ? "YES" : "NO");
+    ss << " SyncMode=" << (m_syncMode ? "YES" : "NO");
 
     return ss.str();
 }

saiplayer/CommandLineOptions.h

@@ -31,6 +31,8 @@ namespace saiplayer
 
             bool m_sleep;
 
+            bool m_syncMode;
+
             std::vector<std::string> m_files;
     };
 }

saiplayer/CommandLineOptionsParser.cpp

@@ -16,7 +16,7 @@ std::shared_ptr<CommandLineOptions> CommandLineOptionsParser::parseCommandLine(
 
     auto options = std::make_shared<CommandLineOptions>();
 
-    const char* const optstring = "uiCdsh";
+    const char* const optstring = "uiCdsmh";
 
     while(true)
     {
@@ -27,6 +27,7 @@ std::shared_ptr<CommandLineOptions> CommandLineOptionsParser::parseCommandLine(
             { "skipNotifySyncd",  no_argument,       0, 'C' },
             { "enableDebug",      no_argument,       0, 'd' },
             { "sleep",            no_argument,       0, 's' },
+            { "syncMode",         no_argument,       0, 'm' },
             { "help",             no_argument,       0, 'h' },
         };
 
@@ -61,6 +62,10 @@ std::shared_ptr<CommandLineOptions> CommandLineOptionsParser::parseCommandLine(
                 options->m_sleep = true;
                 break;
 
+            case 'm':
+                options->m_syncMode = true;
+                break;
+
             case 'h':
                 printUsage();
                 exit(EXIT_SUCCESS);
@@ -95,7 +100,7 @@ void CommandLineOptionsParser::printUsage()
 {
     SWSS_LOG_ENTER();
 
-    std::cout << "Usage: saiplayer [-u] [-i] [-C] [-d] [-s] [-h] recordfile" << std::endl << std::endl;
+    std::cout << "Usage: saiplayer [-u] [-i] [-C] [-d] [-s] [-m] [-h] recordfile" << std::endl << std::endl;
 
     std::cout << "    -u --useTempView:" << std::endl;
     std::cout << "        Enable temporary view between init and apply" << std::endl << std::endl;
@@ -107,6 +112,8 @@ void CommandLineOptionsParser::printUsage()
     std::cout << "        Enable syslog debug messages" << std::endl << std::endl;
     std::cout << "    -s --sleep:" << std::endl;
     std::cout << "        Sleep after success reply, to notice any switch notifications" << std::endl << std::endl;
+    std::cout << "    -m --syncMode:" << std::endl;
+    std::cout << "        Enable synchronous mode" << std::endl << std::endl;
 
     std::cout << "    -h --help:" << std::endl;
     std::cout << "        Print out this message" << std::endl << std::endl;

saiplayer/SaiPlayer.cpp

@@ -1524,6 +1524,14 @@ int SaiPlayer::run()
         EXIT_ON_ERROR(m_sai->set(SAI_OBJECT_TYPE_SWITCH, switch_id, &attr));
     }
 
+    if (m_commandLineOptions->m_syncMode)
+    {
+        attr.id = SAI_REDIS_SWITCH_ATTR_SYNC_MODE;
+        attr.value.booldata = true;
+
+        EXIT_ON_ERROR(m_sai->set(SAI_OBJECT_TYPE_SWITCH, switch_id, &attr));
+    }
+
     int exitcode = 0;
 
     if (m_commandLineOptions->m_files.size() > 0)

syncd/RedisClient.cpp

@@ -479,6 +479,16 @@ void RedisClient::removeAsicObject(
     m_redisClient->del(key);
 }
 
+void RedisClient::removeTempAsicObject(
+        _In_ const sai_object_meta_key_t& metaKey)
+{
+    SWSS_LOG_ENTER();
+
+    std::string key = (TEMP_PREFIX ASIC_STATE_TABLE ":") + sai_serialize_object_meta_key(metaKey);
+
+    m_redisClient->del(key);
+}
+
 void RedisClient::setAsicObject(
         _In_ const sai_object_meta_key_t& metaKey,
         _In_ const std::string& attr,
@@ -523,6 +533,26 @@ void RedisClient::createAsicObject(
     }
 }
 
+void RedisClient::createTempAsicObject(
+        _In_ const sai_object_meta_key_t& metaKey,
+        _In_ const std::vector<swss::FieldValueTuple>& attrs)
+{
+    SWSS_LOG_ENTER();
+
+    std::string key = (TEMP_PREFIX ASIC_STATE_TABLE ":") + sai_serialize_object_meta_key(metaKey);
+
+    if (attrs.size() == 0)
+    {
+        m_redisClient->hset(key, "NULL", "NULL");
+        return;
+    }
+
+    for (const auto& e: attrs)
+    {
+        m_redisClient->hset(key, fvField(e), fvValue(e));
+    }
+}
+
 void RedisClient::setVidAndRidMap(
         _In_ const std::unordered_map<sai_object_id_t, sai_object_id_t>& map)
 {

syncd/RedisClient.h

@@ -83,10 +83,17 @@ namespace syncd
             void removeAsicObject(
                     _In_ const sai_object_meta_key_t& metaKey);
 
+            void removeTempAsicObject(
+                    _In_ const sai_object_meta_key_t& metaKey);
+
             void createAsicObject(
                     _In_ const sai_object_meta_key_t& metaKey,
                     _In_ const std::vector<swss::FieldValueTuple>& attrs);
 
+            void createTempAsicObject(
+                    _In_ const sai_object_meta_key_t& metaKey,
+                    _In_ const std::vector<swss::FieldValueTuple>& attrs);
+
             void setVidAndRidMap(
                     _In_ const std::unordered_map<sai_object_id_t, sai_object_id_t>& map);
 

syncd/Syncd.cpp

@@ -85,6 +85,8 @@ Syncd::Syncd(
     m_asicState = std::make_shared<swss::ConsumerTable>(m_dbAsic.get(), ASIC_STATE_TABLE);
     m_restartQuery = std::make_shared<swss::NotificationConsumer>(m_dbAsic.get(), SYNCD_NOTIFICATION_CHANNEL_RESTARTQUERY);
 
+    m_asicState->setModifyRedis(m_commandLineOptions->m_enableSyncMode ? false : true);
+
     // TODO to be moved to ASIC_DB
     m_dbFlexCounter = std::make_shared<swss::DBConnector>(m_contextConfig->m_dbFlex, 0);
     m_flexCounter = std::make_shared<swss::ConsumerTable>(m_dbFlexCounter.get(), FLEX_COUNTER_TABLE);
@@ -663,6 +665,13 @@ sai_status_t Syncd::processBulkQuadEventInInitViewMode(
 {
     SWSS_LOG_ENTER();
 
+    std::vector<sai_status_t> statuses(object_ids.size());
+
+    for (auto &a: statuses)
+    {
+        a = SAI_STATUS_SUCCESS;
+    }
+
     auto info = sai_metadata_get_object_type_info(objectType);
 
     switch (api)
@@ -673,7 +682,7 @@ sai_status_t Syncd::processBulkQuadEventInInitViewMode(
 
             if (info->isnonobjectid)
             {
-                sendApiResponse(api, SAI_STATUS_SUCCESS);
+                sendApiResponse(api, SAI_STATUS_SUCCESS, (uint32_t)statuses.size(), statuses.data());
                 return SAI_STATUS_SUCCESS;
             }
 
@@ -689,7 +698,7 @@ sai_status_t Syncd::processBulkQuadEventInInitViewMode(
 
                 default:
 
-                    sendApiResponse(api, SAI_STATUS_SUCCESS);
+                    sendApiResponse(api, SAI_STATUS_SUCCESS, (uint32_t)statuses.size(), statuses.data());
                     return SAI_STATUS_SUCCESS;
             }
 
@@ -1280,6 +1289,88 @@ void Syncd::processFlexCounterEvent( // TODO must be moved to go via ASIC channe
     }
 }
 
+void Syncd::syncUpdateRedisQuadEvent(
+        _In_ sai_status_t status,
+        _In_ sai_common_api_t api,
+        _In_ const swss::KeyOpFieldsValuesTuple &kco)
+{
+    SWSS_LOG_ENTER();
+
+    if (!m_commandLineOptions->m_enableSyncMode)
+    {
+        return;
+    }
+
+    if (status != SAI_STATUS_SUCCESS)
+    {
+        return;
+    }
+
+    // When in synchronous mode, we need to modify redis database when status
+    // is success, since consumer table on synchronous mode is not making redis
+    // changes and we only want to apply changes when api succeeded. This
+    // applies to init view mode and apply view mode.
+
+    const std::string& key = kfvKey(kco);
+
+    auto& values = kfvFieldsValues(kco);
+
+    const std::string& strObjectId = key.substr(key.find(":") + 1);
+
+    sai_object_meta_key_t metaKey;
+    sai_deserialize_object_meta_key(key, metaKey);
+
+    const bool initView = isInitViewMode();
+
+    switch (api)
+    {
+        case SAI_COMMON_API_CREATE:
+
+            {
+                if (initView)
+                    m_client->createTempAsicObject(metaKey, values);
+                else
+                    m_client->createAsicObject(metaKey, values);
+
+                return;
+            }
+
+        case SAI_COMMON_API_REMOVE:
+
+            {
+                if (initView)
+                    m_client->removeTempAsicObject(metaKey);
+                else
+                    m_client->removeAsicObject(metaKey);
+
+                return;
+            }
+
+        case SAI_COMMON_API_SET:
+
+            {
+                auto& first = values.at(0);
+
+                auto& attr = fvField(first);
+                auto& value = fvValue(first);
+
+                if (initView)
+                    m_client->setTempAsicObject(metaKey, attr, value);
+                else
+                    m_client->setAsicObject(metaKey, attr, value);
+
+                return;
+            }
+
+        case SAI_COMMON_API_GET:
+            return; // ignore get since get is not modifying db
+
+        default:
+
+            SWSS_LOG_THROW("api %d is not supported", api);
+    }
+}
+
 sai_status_t Syncd::processQuadEvent(
         _In_ sai_common_api_t api,
         _In_ const swss::KeyOpFieldsValuesTuple &kco)
@@ -1338,7 +1429,11 @@ sai_status_t Syncd::processQuadEvent(
 
     if (isInitViewMode())
     {
-        return processQuadEventInInitViewMode(metaKey.objecttype, strObjectId, api, attr_count, attr_list);
+        sai_status_t status = processQuadEventInInitViewMode(metaKey.objecttype, strObjectId, api, attr_count, attr_list);
+
+        syncUpdateRedisQuadEvent(status, api, kco);
+
+        return status;
     }
 
     if (api != SAI_COMMON_API_GET)
@@ -1402,16 +1497,23 @@ sai_status_t Syncd::processQuadEvent(
             SWSS_LOG_ERROR("attr: %s: %s", fvField(v).c_str(), fvValue(v).c_str());
         }
 
-        SWSS_LOG_THROW("failed to execute api: %s, key: %s, status: %s",
-                op.c_str(),
-                key.c_str(),
-                sai_serialize_status(status).c_str());
+        if (!m_commandLineOptions->m_enableSyncMode)
+        {
+            // throw only when sync mode is not enabled
+
+            SWSS_LOG_THROW("failed to execute api: %s, key: %s, status: %s",
+                    op.c_str(),
+                    key.c_str(),
+                    sai_serialize_status(status).c_str());
+        }
     }
     else // non GET api, status is SUCCESS
     {
         sendApiResponse(api, status);
     }
 
+    syncUpdateRedisQuadEvent(status, api, kco);
+
     return status;
 }
 

syncd/Syncd.h

@@ -214,6 +214,13 @@ namespace syncd
                     _In_ uint32_t attr_count,
                     _In_ sai_attribute_t *attr_list);
 
+        private:
+
+            void syncUpdateRedisQuadEvent(
+                    _In_ sai_status_t status,
+                    _In_ sai_common_api_t api,
+                    _In_ const swss::KeyOpFieldsValuesTuple &kco);
+
         public: // TODO to private
 
             sai_status_t processEntry(

tests/brcm.pl

@@ -504,6 +504,20 @@ sub test_brcm_warm_boot_port_remove
     request_warm_shutdown;
 }
 
+sub test_sync_brcm_warm_boot_port_remove
+{
+    sync_fresh_start;
+
+    sync_play "wb_port_remove_a.rec";
+
+    request_warm_shutdown;
+    sync_start_syncd_warm;
+
+    sync_play "wb_port_remove_b.rec";
+
+    request_warm_shutdown;
+}
+
 sub test_brcm_warm_boot_port_create
 {
     fresh_start;
@@ -534,6 +548,7 @@ sub test_brcm_query_object_type_get_availability
 
 # RUN TESTS
 
+test_sync_brcm_warm_boot_port_remove;
 test_brcm_warm_boot_port_remove;
 test_brcm_warm_boot_port_create;
 test_remove_port;