diff --git a/syncd/syncd.cpp b/syncd/syncd.cpp index 7e6be2831c9b..e359750d1a6e 100644 --- a/syncd/syncd.cpp +++ b/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) diff --git a/syncd/syncd.h b/syncd/syncd.h index 89ca04032c1e..214239a7bf7b 100644 --- a/syncd/syncd.h +++ b/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 diff --git a/syncd/syncd_applyview.cpp b/syncd/syncd_applyview.cpp index b225f0b4fa93..e0e3ebae0f92 100644 --- a/syncd/syncd_applyview.cpp +++ b/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); } diff --git a/syncd/syncd_hard_reinit.cpp b/syncd/syncd_hard_reinit.cpp index 5a4d13a0f74c..617601425819 100644 --- a/syncd/syncd_hard_reinit.cpp +++ b/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(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(); diff --git a/syncd/syncd_saiswitch.cpp b/syncd/syncd_saiswitch.cpp index e11af057f2f0..36607e795143 100644 --- a/syncd/syncd_saiswitch.cpp +++ b/syncd/syncd_saiswitch.cpp @@ -413,7 +413,7 @@ std::string SaiSwitch::getHardwareInfo() const return m_hardware_info; } -bool SaiSwitch::isDefaultCreatedRid( +bool SaiSwitch::isDiscoveredRid( _In_ sai_object_id_t rid) const { SWSS_LOG_ENTER(); @@ -421,7 +421,7 @@ bool SaiSwitch::isDefaultCreatedRid( return m_discovered_rids.find(rid) != m_discovered_rids.end(); } -std::set SaiSwitch::getExistingObjects() const +std::set SaiSwitch::getDiscoveredRids() const { SWSS_LOG_ENTER(); @@ -619,6 +619,39 @@ sai_object_id_t SaiSwitch::getSwitchDefaultAttrOid( return it->second; } +bool SaiSwitch::isColdBootDiscoveredRid( + _In_ sai_object_id_t rid) const +{ + SWSS_LOG_ENTER(); + + auto coldBootDiscoveredVids = getColdBootDiscoveredVids(); + + /* + * If obejct was discovered in cold boot, it must have valid RID assigned, + * except objects that were removed like VLAN_MEMBER. + */ + + sai_object_id_t vid = translate_rid_to_vid(rid, m_switch_vid); + + return coldBootDiscoveredVids.find(vid) != coldBootDiscoveredVids.end(); +} + +bool SaiSwitch::isSwitchObjectDefaultRid( + _In_ sai_object_id_t rid) const +{ + SWSS_LOG_ENTER(); + + for (const auto &p: m_default_rid_map) + { + if (p.second == rid) + { + return true; + } + } + + return false; +} + bool SaiSwitch::isNonRemovableRid( _In_ sai_object_id_t rid) const { @@ -629,21 +662,23 @@ bool SaiSwitch::isNonRemovableRid( SWSS_LOG_THROW("NULL rid passed"); } - if (!isDefaultCreatedRid(rid)) + if (!isColdBootDiscoveredRid(rid)) { /* - * Non discovered obejct, it can be removed. + * This object was not discovered on cold boot so it can be removed. */ return false; } - for (const auto &p: m_default_rid_map) + /* + * Check for SAI_SWITCH_ATTR_DEFAULT_* oids like cpu, default virtual + * router. Those objects can't be removed if user ask for it. + */ + + if (isSwitchObjectDefaultRid(rid)) { - if (p.second == rid) - { - return true; - } + return true; } sai_object_type_t ot = sai_object_type_query(rid); @@ -672,6 +707,12 @@ bool SaiSwitch::isNonRemovableRid( case SAI_OBJECT_TYPE_VLAN_MEMBER: case SAI_OBJECT_TYPE_STP_PORT: case SAI_OBJECT_TYPE_BRIDGE_PORT: + + /* + * Those objects were discovered during cold boot, but they can + * still be removed since switch allows that. + */ + return false; case SAI_OBJECT_TYPE_PORT: @@ -752,7 +793,6 @@ void SaiSwitch::saiDiscover( discovered.insert(rid); } - // TODO later use sai_metadata_get_object_type_info(ot); const sai_object_type_info_t *info = sai_metadata_get_object_type_info(ot); /* @@ -926,8 +966,11 @@ void SaiSwitch::helperDiscover() { SWSS_LOG_TIMER("discover"); + set_sai_api_log_min_prio("SAI_LOG_LEVEL_CRITICAL"); + saiDiscover(m_switch_rid, m_discovered_rids); + set_sai_api_log_min_prio("SAI_LOG_LEVEL_NOTICE"); } @@ -951,11 +994,100 @@ void SaiSwitch::helperDiscover() } } -void SaiSwitch::helperPutDiscoveredRidsToRedis() +void SaiSwitch::helperLoadColdVids() { SWSS_LOG_ENTER(); - SWSS_LOG_TIMER("put discovered objects to redis"); + auto hash = g_redisClient->hgetall(COLDVIDS); + + /* + * NOTE: some objects may not exists after 2nd restart, like VLAN_MEMBER or + * BRIDGE_PORT, since user could decide to remove them on previous boot. + */ + + for (auto kvp: hash) + { + auto strVid = kvp.first; + + sai_object_id_t vid; + sai_deserialize_object_id(strVid, vid); + + /* + * Just make sure that vid in COLDVIDS is present in current vid2rid map + */ + + auto rid = g_redisClient->hget(VIDTORID, strVid); + + if (rid == nullptr) + { + SWSS_LOG_INFO("no RID for VID %s, probably object was removed previously", strVid.c_str()); + } + + m_coldBootDiscoveredVids.insert(vid); + } + + SWSS_LOG_NOTICE("read %zu COLD VIDS", m_coldBootDiscoveredVids.size()); +} + +std::set SaiSwitch::getColdBootDiscoveredVids() const +{ + SWSS_LOG_ENTER(); + + if (m_coldBootDiscoveredVids.size() != 0) + { + return m_coldBootDiscoveredVids; + } + + /* + * Normally we should throw here, but we want to keep backward + * compatybility and don't break anything. + */ + + SWSS_LOG_WARN("cold boot discovered VIDs set is empty, using discovered set"); + + std::set discoveredVids; + + for (sai_object_id_t rid: m_discovered_rids) + { + sai_object_id_t vid = translate_rid_to_vid(rid, m_switch_vid); + + discoveredVids.insert(vid); + } + + return discoveredVids; +} + +void SaiSwitch::redisSaveColdBootDiscoveredVids() const +{ + SWSS_LOG_ENTER(); + + for (sai_object_id_t rid: m_discovered_rids) + { + sai_object_id_t vid = translate_rid_to_vid(rid, m_switch_vid); + + sai_object_type_t objectType = sai_object_type_query(rid); + + if (objectType == SAI_OBJECT_TYPE_NULL) + { + SWSS_LOG_THROW("sai_object_type_query returned NULL type for RID: %s", + sai_serialize_object_id(rid).c_str()); + } + + std::string strObjectType = sai_serialize_object_type(objectType); + + std::string strVid = sai_serialize_object_id(vid); + + g_redisClient->hset(COLDVIDS, strVid, strObjectType); + } + + SWSS_LOG_NOTICE("put default discovered vids to redis"); +} + +void SaiSwitch::helperSaveDiscoveredObjectsToRedis() +{ + SWSS_LOG_ENTER(); + + SWSS_LOG_TIMER("save discovered objects to redis"); /* * There is a problem: @@ -1030,6 +1162,24 @@ void SaiSwitch::helperPutDiscoveredRidsToRedis() redisSetDummyAsicStateForRealObjectId(rid); } + + /* + * If we are here, this is probably COLD boot, since any previous boot + * would put lots of objects into redis DB (ports, queues, scheduler_groups + * etc), and since this is cold boot, we can put those discovered objects + * to cold boot objects map to redis DB. This will become handy when doing + * warm boot and figureing out which object is default created and which is + * user created, since after warm boot user could previously assign buffer + * profile on ingress priority group and this buffer profile will be + * discovered by sai discovery logic. + * + * Question is here whether we should put VID here or RID. And after cold + * boot when hard reinit logic happens, we need to remap them, also note + * that some object could be removed like VLAN members and they will not + * have existing corresponding OID. + */ + + redisSaveColdBootDiscoveredVids(); } void SaiSwitch::helperInternalOids() @@ -1104,11 +1254,13 @@ SaiSwitch::SaiSwitch( helperDiscover(); - helperPutDiscoveredRidsToRedis(); + helperSaveDiscoveredObjectsToRedis(); helperInternalOids(); helperCheckLaneMap(); + helperLoadColdVids(); + saiGetMacAddress(m_default_mac_address); } diff --git a/syncd/syncd_saiswitch.h b/syncd/syncd_saiswitch.h index 9aa8ab20eba5..b618377bf505 100644 --- a/syncd/syncd_saiswitch.h +++ b/syncd/syncd_saiswitch.h @@ -45,11 +45,39 @@ class SaiSwitch * If user will remove such RID, then this function will no longer * return true for that RID. * + * If in WARM boot mode this function will also return true for objects + * that were user created and present on the switch during init. + * * @param rid Real ID to be examined. * * @return True if RID was discovered during init. */ - bool isDefaultCreatedRid( + bool isDiscoveredRid( + _In_ sai_object_id_t rid) const; + + /** + * @brief Indicates whether RID was discovered on switch init at cold boot. + * + * During switch operation some RIDs are removable, like vlan member. + * If user will remove such RID, then this function will no longer + * return true for that RID. + * + * @param rid Real ID to be examined. + * + * @return True if RID was discovered during cald boot init. + */ + bool isColdBootDiscoveredRid( + _In_ sai_object_id_t rid) const; + + /** + * @brfief Indicates whether RID is one of default switch objects + * like CPU port, default virtual router etc. + * + * @param rid Real object id to examine. + * + * @return True if object is default switch object. + */ + bool isSwitchObjectDefaultRid( _In_ sai_object_id_t rid) const; /** @@ -77,16 +105,18 @@ class SaiSwitch _In_ const std::string &key); /** - * @brief Gets existing objects on the switch. + * @brief Gets discovered objects on the switch. * * This set can be different from discovered objects after switch init * when for example default VLAN members will be removed. * * This set can't grow, but it can be reduced. * - * @returns Default existing objects on the switch. + * Also if in WARM boot mode it can contain user created objects. + * + * @returns Discovered objects during switch init. */ - std::set getExistingObjects() const; + std::set getDiscoveredRids() const; /** * @brief Gets default object based on switch attribute. @@ -146,6 +176,13 @@ class SaiSwitch _In_ sai_object_id_t rid, _In_ sai_attr_id_t attr_id); + /** + * @biref Get cold boot discovered VIDs. + * + * @return Set of cold boot discovered VIDs after cold boot. + */ + std::set getColdBootDiscoveredVids() const; + private: /* @@ -203,6 +240,10 @@ class SaiSwitch * Set of object IDs discovered after calling saiDiscovery method. * This set will contain all objects present on the switch right after * switch init. + * + * This set depending on the boot, can contain user created objects if + * switch was in WARM boot mode. This set can also change if user + * decides to remove some objects like VLAN_MEMBER. */ std::set m_discovered_rids; @@ -252,6 +293,16 @@ class SaiSwitch void redisSetDummyAsicStateForRealObjectId( _In_ sai_object_id_t rid) const; + /** + * @brief Put cold boot discovered VIDs to redis DB. + * + * This method will only be called after cold boot and it will save + * only VIDs that are present on the switch after swtich is initialized + * so it will contain only discovered objects. In case of warm boot + * this method will not be called. + */ + void redisSaveColdBootDiscoveredVids() const; + /* * Helper Methods. */ @@ -285,10 +336,12 @@ class SaiSwitch */ void helperDiscover(); - void helperPutDiscoveredRidsToRedis(); + void helperSaveDiscoveredObjectsToRedis(); void helperInternalOids(); + void helperLoadColdVids(); + /* * Other Methods. */ @@ -314,6 +367,8 @@ class SaiSwitch * m_defaultOidMap[0x17][SAI_SCHEDULER_GROUP_ATTR_SCHEDULER_PROFILE_ID] == 0x16 */ std::unordered_map> m_defaultOidMap; + + std::set m_coldBootDiscoveredVids; }; extern std::map> switches;