Add cold discovered oids (sonic-net#379)

syncd/syncd.cpp

@@ -1313,7 +1313,7 @@ sai_status_t handle_generic(
 
                         sai_object_id_t switch_vid = redis_sai_switch_id_query(object_id);
 
-                        if (switches.at(switch_vid)->isDefaultCreatedRid(rid))
+                        if (switches.at(switch_vid)->isDiscoveredRid(rid))
                         {
                             switches.at(switch_vid)->removeExistingObjectReference(rid);
                         }
@@ -3329,8 +3329,6 @@ void onSyncdStart(bool warmStart)
      * need to use a lock here to prevent that.
      */
 
-
-
     SWSS_LOG_TIMER("on syncd start");
 
     if (warmStart)

syncd/syncd.h

@@ -50,6 +50,7 @@ extern "C" {
 #define VIDCOUNTER                  "VIDCOUNTER"
 #define LANES                       "LANES"
 #define HIDDEN                      "HIDDEN"
+#define COLDVIDS                    "COLDVIDS"
 
 #define SAI_COLD_BOOT               0
 #define SAI_WARM_BOOT               1

syncd/syncd_applyview.cpp

@@ -4292,6 +4292,10 @@ bool isNonRemovableObject(
         return sw->isNonRemovableRid(rid);
     }
 
+    /*
+     * Object is non object id, like ROUTE_ENTRY etc, can be removed.
+     */
+
     return false;
 }
 
@@ -5361,8 +5365,6 @@ bool performObjectSetTransition(
          * values actually exists.
          */
 
-        bool isDefaultCreatedRid = false;
-
         if (currentBestMatch->isOidObject())
         {
             sai_object_id_t vid = currentBestMatch->getVid();
@@ -5379,9 +5381,7 @@ bool performObjectSetTransition(
 
                 auto sw = switches.begin()->second;
 
-                isDefaultCreatedRid = sw->isDefaultCreatedRid(rid);
-
-                if (isDefaultCreatedRid)
+                if (sw->isDiscoveredRid(rid))
                 {
                     SWSS_LOG_INFO("performing default on existing object VID %s: %s: %s, we need default dependency TREE, FIXME",
                             sai_serialize_object_id(vid).c_str(),
@@ -5481,7 +5481,7 @@ bool performObjectSetTransition(
                 }
 
                // SAI_QUEUE_ATTR_PARENT_SCHEDULER_NODE
-               // SAI_SCHEDULER_GROUP_ATTR_SCHEDULER_PROFILE_ID
+               // SAI_SCHEDULER_GROUP_ATTR_SCHEDULER_PROFILE_ID*
                // SAI_SCHEDULER_GROUP_ATTR_PARENT_NODE
                // SAI_BRIDGE_PORT_ATTR_BRIDGE_ID
                //
@@ -5492,6 +5492,7 @@ bool performObjectSetTransition(
                 // TODO SAI_SCHEDULER_GROUP_ATTR_SCHEDULER_PROFILE_ID is mandatory on create but also SET
                 // if attribute is set we and object is in MATCHED state then that means we are able to
                 // bring this attribute to default state not for all attributes!
+                // *SAI_SCHEDULER_GROUP_ATTR_SCHEDULER_PROFILE_ID - is not any more mandatory on create, so default should be NULL
 
                 SWSS_LOG_ERROR("current attribute is mandatory on create, crate and set, and object MATCHED, FIXME %s %s:%s",
                         currentBestMatch->str_object_id.c_str(),
@@ -6173,6 +6174,12 @@ void populateExistingObjects(
         return;
     }
 
+    // XXX we have only 1 switch, so we can get away with this
+
+    auto sw = switches.begin()->second;
+
+    auto coldBootDiscoveredVids = sw->getColdBootDiscoveredVids();
+
     /*
      * If some objects that are existing objects on switch are not present in
      * temporary view, just populate them with empty values.  Since vid2rid
@@ -6218,9 +6225,44 @@ void populateExistingObjects(
             continue;
         }
 
-        // TODO this object in current view may have some attributes, we need to copy them
-        // to temp view, so they match on compare, or in case of those objects matched
-        // just ignore operations ? what about attributes with oids?
+        /*
+         * In case of warm boot, it may happen that user set some created
+         * objects on default existing objects, like for example buffer profile
+         * on ingress priority group.  In this case buffer profile should not
+         * be considered as matched object and copied to temporary view, since
+         * this object was not decault existing object (on 1st cold boot) so in
+         * this case it must be processed by comparison logic and matched with
+         * possible new buffer profile created in temporary view. This may
+         * happen if OA will not care what was set previously on ingress
+         * priority group and just create new buffer profile and assign it.  In
+         * this case we don't want any asic operations to happen.  Also if we
+         * would pass this buffer profile as existing to temporary view, it
+         * would not be matched by comparison logic, and in the result we will
+         * end up with 2 buffer profiles, which 1st of them will be not
+         * assigned anywhere and this will be memory leak.
+         *
+         * Also a bunch of new asic operations will be generated for setting
+         * new user created buffer profile.  Thats why we need default existing
+         * vid list to distinguish between user created and default switch
+         * created obejcts.
+         *
+         * For default existing objects, we don't need to copy attributes, since
+         * if user didn't set them, we want them to be back to default values.
+         *
+         * NOTE: If we are here, then this RID exists only in current view, and
+         * if this object contains any OID attributes, discovery logic queried
+         * them so they are also existing in current view.
+         */
+
+        if (coldBootDiscoveredVids.find(vid) == coldBootDiscoveredVids.end())
+        {
+            SWSS_LOG_INFO("object is not on default existing list: %s RID %s VID %s",
+                    sai_serialize_object_type(sai_object_type_query(rid)).c_str(),
+                    sai_serialize_object_id(rid).c_str(),
+                    sai_serialize_object_id(vid).c_str());
+
+            continue;
+        }
 
         temporaryView.createDummyExistingObject(rid, vid);
 
@@ -6456,7 +6498,7 @@ sai_status_t syncdApplyView()
          * existing objects needs to be updated in the switch!
          */
 
-        const auto &existingObjects = sw->getExistingObjects();
+        const auto &existingObjects = sw->getDiscoveredRids();
 
         populateExistingObjects(current, temp, existingObjects);
 
@@ -6814,7 +6856,7 @@ sai_status_t asic_handle_generic(
 
                     auto sw = switches.begin()->second;
 
-                    if (sw->isDefaultCreatedRid(rid))
+                    if (sw->isDiscoveredRid(rid))
                     {
                         sw->removeExistingObjectReference(rid);
                     }

syncd/syncd_hard_reinit.cpp

@@ -264,7 +264,7 @@ void checkAllIds()
 
     for (sai_object_type_t ot: removeOrder)
     {
-        for (sai_object_id_t rid: g_sw->getExistingObjects())
+        for (sai_object_id_t rid: g_sw->getDiscoveredRids())
         {
             if (g_translatedR2V.find(rid) != g_translatedR2V.end())
             {
@@ -275,6 +275,20 @@ void checkAllIds()
                     ot == SAI_OBJECT_TYPE_NULL)
             {
                 g_sw->removeExistingObject(rid);
+
+                /*
+                 * If removing existing object, also make sure we remove it
+                 * from DEFAULTVID map since this object will no longer exists.
+                 */
+
+                if (g_ridToVidMap.find(rid) == g_ridToVidMap.end())
+                    continue;
+
+                std::string strVid = sai_serialize_object_id(g_ridToVidMap.at(rid));
+
+                SWSS_LOG_INFO("removeing existing VID: %s", strVid.c_str());
+
+                g_redisClient->hdel(COLDVIDS, strVid);
             }
         }
     }
@@ -441,6 +455,11 @@ void processSwitches()
         g_translatedV2R[switch_vid] = switch_rid;
         g_translatedR2V[switch_rid] = switch_vid;
 
+        /*
+         * SaiSwitch class object must be created before before any other
+         * object, so when doing discover we will get full default ASIC view.
+         */
+
         auto sw = switches[switch_vid] = std::make_shared<SaiSwitch>(switch_vid, switch_rid);
 
         /*
@@ -634,7 +653,7 @@ sai_object_id_t processSingleVid(
 
     sai_object_id_t rid;
 
-    if (g_sw->isDefaultCreatedRid(v2rMapIt->second))
+    if (g_sw->isDiscoveredRid(v2rMapIt->second))
     {
         rid = v2rMapIt->second;
 
@@ -741,7 +760,7 @@ sai_object_id_t processSingleVid(
                  * NOTE: We could do get here to see if it actually matches.
                  */
 
-                if (g_sw->isDefaultCreatedRid(rid))
+                if (g_sw->isDiscoveredRid(rid))
                 {
                     continue;
                 }
@@ -1160,7 +1179,7 @@ void hardReinit()
     processSwitches();
 
     {
-        SWSS_LOG_TIMER("processing objects after switch create");
+        //SWSS_LOG_TIMER("processing objects after switch create");
 
         processFdbs();
         processNeighbors();