From 451caf5a566bd95c3c0910a0048e4cec122e69d9 Mon Sep 17 00:00:00 2001 From: Kamil Cudnik Date: Mon, 14 Dec 2020 21:27:53 +0100 Subject: [PATCH] [syncd] Translate removed RIDs in fdb notification (#734) --- syncd/NotificationProcessor.cpp | 10 +++--- syncd/RedisClient.cpp | 40 ++++++++++++++++++------ syncd/Syncd.cpp | 41 ++++++++++++++++++++++++ syncd/VirtualOidTranslator.cpp | 55 ++++++++++++++++++++++++++------- syncd/VirtualOidTranslator.h | 13 +++++--- 5 files changed, 129 insertions(+), 30 deletions(-) diff --git a/syncd/NotificationProcessor.cpp b/syncd/NotificationProcessor.cpp index 70fee03d3772..89d7acdc2598 100644 --- a/syncd/NotificationProcessor.cpp +++ b/syncd/NotificationProcessor.cpp @@ -140,7 +140,7 @@ void NotificationProcessor::redisPutFdbEntryToAsicView( sai_object_id_t bv_id = fdb->fdb_entry.bv_id; sai_object_id_t port_oid = 0; - sai_fdb_flush_entry_type_t type = SAI_FDB_FLUSH_ENTRY_TYPE_ALL; + sai_fdb_flush_entry_type_t type = SAI_FDB_FLUSH_ENTRY_TYPE_DYNAMIC; for (uint32_t i = 0; i < fdb->attr_count; i++) { @@ -236,7 +236,7 @@ bool NotificationProcessor::check_fdb_event_notification_data( bool result = true; - if (!m_translator->checkRidExists(data.fdb_entry.bv_id)) + if (!m_translator->checkRidExists(data.fdb_entry.bv_id, true)) { SWSS_LOG_ERROR("bv_id RID 0x%" PRIx64 " is not present on local ASIC DB: %s", data.fdb_entry.bv_id, sai_serialize_fdb_entry(data.fdb_entry).c_str()); @@ -268,7 +268,7 @@ bool NotificationProcessor::check_fdb_event_notification_data( if (meta->attrvaluetype != SAI_ATTR_VALUE_TYPE_OBJECT_ID) continue; - if (!m_translator->checkRidExists(attr.value.oid)) + if (!m_translator->checkRidExists(attr.value.oid, true)) { SWSS_LOG_WARN("RID 0x%" PRIx64 " on %s is not present on local ASIC DB", attr.value.oid, meta->attridname); @@ -322,9 +322,9 @@ void NotificationProcessor::process_on_fdb_event( fdb->fdb_entry.switch_id = m_translator->translateRidToVid(fdb->fdb_entry.switch_id, SAI_NULL_OBJECT_ID); - fdb->fdb_entry.bv_id = m_translator->translateRidToVid(fdb->fdb_entry.bv_id, fdb->fdb_entry.switch_id); + fdb->fdb_entry.bv_id = m_translator->translateRidToVid(fdb->fdb_entry.bv_id, fdb->fdb_entry.switch_id, true); - m_translator->translateRidToVid(SAI_OBJECT_TYPE_FDB_ENTRY, fdb->fdb_entry.switch_id, fdb->attr_count, fdb->attr); + m_translator->translateRidToVid(SAI_OBJECT_TYPE_FDB_ENTRY, fdb->fdb_entry.switch_id, fdb->attr_count, fdb->attr, true); /* * Currently because of brcm bug, we need to install fdb entries in diff --git a/syncd/RedisClient.cpp b/syncd/RedisClient.cpp index c600e0812231..9a84879761e8 100644 --- a/syncd/RedisClient.cpp +++ b/syncd/RedisClient.cpp @@ -865,16 +865,38 @@ void RedisClient::processFlushEvent( SWSS_LOG_NOTICE("pattern %s, portStr %s", pattern.c_str(), portStr.c_str()); - swss::RedisCommand command; + std::vector vals; // 0 - flush dynamic, 1 - flush static - int flush_static = (type == SAI_FDB_FLUSH_ENTRY_TYPE_STATIC) ? 1 : 0; + switch (type) + { + case SAI_FDB_FLUSH_ENTRY_TYPE_DYNAMIC: + vals.push_back(0); + break; + + case SAI_FDB_FLUSH_ENTRY_TYPE_STATIC: + vals.push_back(1); + break; + + case SAI_FDB_FLUSH_ENTRY_TYPE_ALL: + vals.push_back(0); + vals.push_back(1); + break; + + default: + SWSS_LOG_THROW("unknow fdb flush entry type: %d", type); + } + + for (int flush_static: vals) + { + swss::RedisCommand command; - command.format( - "EVALSHA %s 3 %s %s %s", - m_fdbFlushSha.c_str(), - pattern.c_str(), - portStr.c_str(), - std::to_string(flush_static).c_str()); + command.format( + "EVALSHA %s 3 %s %s %s", + m_fdbFlushSha.c_str(), + pattern.c_str(), + portStr.c_str(), + std::to_string(flush_static).c_str()); - swss::RedisReply r(m_dbAsic.get(), command); + swss::RedisReply r(m_dbAsic.get(), command); + } } diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index c9088e57a09f..a993547e566a 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -553,6 +553,7 @@ sai_status_t Syncd::processFdbFlush( } SaiAttributeList list(SAI_OBJECT_TYPE_FDB_FLUSH, values, false); + SaiAttributeList vidlist(SAI_OBJECT_TYPE_FDB_FLUSH, values, false); /* * Attribute list can't be const since we will use it to translate VID to @@ -568,6 +569,46 @@ sai_status_t Syncd::processFdbFlush( m_selectableChannel->set(sai_serialize_status(status), {} , REDIS_ASIC_STATE_COMMAND_FLUSHRESPONSE); + if (status == SAI_STATUS_SUCCESS) + { + SWSS_LOG_NOTICE("fdb flush succeeded, updating redis database"); + + // update database right after fdb flush success (not in notification) + // build artificial notification here to reuse code + + sai_fdb_flush_entry_type_t type = SAI_FDB_FLUSH_ENTRY_TYPE_DYNAMIC; + + sai_object_id_t bvId = SAI_NULL_OBJECT_ID; + sai_object_id_t bridgePortId = SAI_NULL_OBJECT_ID; + + attr_list = vidlist.get_attr_list(); + attr_count = vidlist.get_attr_count(); + + for (uint32_t i = 0; i < attr_count; i++) + { + switch (attr_list[i].id) + { + case SAI_FDB_FLUSH_ATTR_BRIDGE_PORT_ID: + bridgePortId = attr_list[i].value.oid; + break; + + case SAI_FDB_FLUSH_ATTR_BV_ID: + bvId = attr_list[i].value.oid; + break; + + case SAI_FDB_FLUSH_ATTR_ENTRY_TYPE: + type = (sai_fdb_flush_entry_type_t)attr_list[i].value.s32; + break; + + default: + SWSS_LOG_ERROR("unsupported attribute: %d, skipping", attr_list[i].id); + break; + } + } + + m_client->processFlushEvent(switchVid, bridgePortId, bvId, type); + } + return status; } diff --git a/syncd/VirtualOidTranslator.cpp b/syncd/VirtualOidTranslator.cpp index 6b7309eeccf9..6026aa1ea5f9 100644 --- a/syncd/VirtualOidTranslator.cpp +++ b/syncd/VirtualOidTranslator.cpp @@ -59,7 +59,8 @@ bool VirtualOidTranslator::tryTranslateRidToVid( sai_object_id_t VirtualOidTranslator::translateRidToVid( _In_ sai_object_id_t rid, - _In_ sai_object_id_t switchVid) + _In_ sai_object_id_t switchVid, + _In_ bool translateRemoved) { SWSS_LOG_ENTER(); @@ -99,6 +100,20 @@ sai_object_id_t VirtualOidTranslator::translateRidToVid( return vid; } + if (translateRemoved) + { + auto itr = m_removedRid2vid.find(rid); + + if (itr != m_removedRid2vid.end()) + { + SWSS_LOG_WARN("translating removed RID %s, to VID %s", + sai_serialize_object_id(rid).c_str(), + sai_serialize_object_id(itr->second).c_str()); + + return itr->second; + } + } + SWSS_LOG_DEBUG("spotted new RID %s", sai_serialize_object_id(rid).c_str()); sai_object_type_t object_type = m_vendorSai->objectTypeQuery(rid); // TODO move to std::function or wrapper class @@ -133,7 +148,8 @@ sai_object_id_t VirtualOidTranslator::translateRidToVid( } bool VirtualOidTranslator::checkRidExists( - _In_ sai_object_id_t rid) + _In_ sai_object_id_t rid, + _In_ bool checkRemoved) { SWSS_LOG_ENTER(); @@ -147,18 +163,28 @@ bool VirtualOidTranslator::checkRidExists( auto vid = m_client->getVidForRid(rid); - return vid != SAI_NULL_OBJECT_ID; + if (vid != SAI_NULL_OBJECT_ID) + return true; + + if (checkRemoved && (m_removedRid2vid.find(rid) != m_removedRid2vid.end())) + { + SWSS_LOG_WARN("removed RID %s exists", sai_serialize_object_id(rid).c_str()); + return true; + } + + return false; } void VirtualOidTranslator::translateRidToVid( _Inout_ sai_object_list_t &element, - _In_ sai_object_id_t switchVid) + _In_ sai_object_id_t switchVid, + _In_ bool translateRemoved) { SWSS_LOG_ENTER(); for (uint32_t i = 0; i < element.count; i++) { - element.list[i] = translateRidToVid(element.list[i], switchVid); + element.list[i] = translateRidToVid(element.list[i], switchVid, translateRemoved); } } @@ -166,7 +192,8 @@ void VirtualOidTranslator::translateRidToVid( _In_ sai_object_type_t objectType, _In_ sai_object_id_t switchVid, _In_ uint32_t attr_count, - _Inout_ sai_attribute_t *attrList) + _Inout_ sai_attribute_t *attrList, + _In_ bool translateRemoved) { SWSS_LOG_ENTER(); @@ -197,31 +224,31 @@ void VirtualOidTranslator::translateRidToVid( switch (meta->attrvaluetype) { case SAI_ATTR_VALUE_TYPE_OBJECT_ID: - attr.value.oid = translateRidToVid(attr.value.oid, switchVid); + attr.value.oid = translateRidToVid(attr.value.oid, switchVid, translateRemoved); break; case SAI_ATTR_VALUE_TYPE_OBJECT_LIST: - translateRidToVid(attr.value.objlist, switchVid); + translateRidToVid(attr.value.objlist, switchVid, translateRemoved); break; case SAI_ATTR_VALUE_TYPE_ACL_FIELD_DATA_OBJECT_ID: if (attr.value.aclfield.enable) - attr.value.aclfield.data.oid = translateRidToVid(attr.value.aclfield.data.oid, switchVid); + attr.value.aclfield.data.oid = translateRidToVid(attr.value.aclfield.data.oid, switchVid, translateRemoved); break; case SAI_ATTR_VALUE_TYPE_ACL_FIELD_DATA_OBJECT_LIST: if (attr.value.aclfield.enable) - translateRidToVid(attr.value.aclfield.data.objlist, switchVid); + translateRidToVid(attr.value.aclfield.data.objlist, switchVid, translateRemoved); break; case SAI_ATTR_VALUE_TYPE_ACL_ACTION_DATA_OBJECT_ID: if (attr.value.aclaction.enable) - attr.value.aclaction.parameter.oid = translateRidToVid(attr.value.aclaction.parameter.oid, switchVid); + attr.value.aclaction.parameter.oid = translateRidToVid(attr.value.aclaction.parameter.oid, switchVid, translateRemoved); break; case SAI_ATTR_VALUE_TYPE_ACL_ACTION_DATA_OBJECT_LIST: if (attr.value.aclaction.enable) - translateRidToVid(attr.value.aclaction.parameter.objlist, switchVid); + translateRidToVid(attr.value.aclaction.parameter.objlist, switchVid, translateRemoved); break; default: @@ -481,6 +508,8 @@ void VirtualOidTranslator::eraseRidAndVid( m_rid2vid.erase(rid); m_vid2rid.erase(vid); + + m_removedRid2vid[rid] = vid; } void VirtualOidTranslator::clearLocalCache() @@ -491,4 +520,6 @@ void VirtualOidTranslator::clearLocalCache() m_rid2vid.clear(); m_vid2rid.clear(); + + m_removedRid2vid.clear(); } diff --git a/syncd/VirtualOidTranslator.h b/syncd/VirtualOidTranslator.h index 26fe3b94ab84..a46d9e4790c7 100644 --- a/syncd/VirtualOidTranslator.h +++ b/syncd/VirtualOidTranslator.h @@ -40,7 +40,8 @@ namespace syncd */ sai_object_id_t translateRidToVid( _In_ sai_object_id_t rid, - _In_ sai_object_id_t switchVid); + _In_ sai_object_id_t switchVid, + _In_ bool translateRemoved = false); /* * This method will try get VID for given RID. @@ -54,7 +55,8 @@ namespace syncd void translateRidToVid( _Inout_ sai_object_list_t& objectList, - _In_ sai_object_id_t switchVid); + _In_ sai_object_id_t switchVid, + _In_ bool translateRemoved = false); /* * This method is required to translate RID to VIDs when we are doing @@ -66,7 +68,8 @@ namespace syncd _In_ sai_object_type_t objectType, _In_ sai_object_id_t switchVid, _In_ uint32_t attrCount, - _Inout_ sai_attribute_t *attrList); + _Inout_ sai_attribute_t *attrList, + _In_ bool translateRemoved = false); /** * @brief Check if RID exists on the ASIC DB. @@ -76,7 +79,8 @@ namespace syncd * @return True if exists or SAI_NULL_OBJECT_ID, otherwise false. */ bool checkRidExists( - _In_ sai_object_id_t rid); + _In_ sai_object_id_t rid, + _In_ bool checkRemoved = false); sai_object_id_t translateVidToRid( _In_ sai_object_id_t vid); @@ -121,6 +125,7 @@ namespace syncd std::unordered_map m_rid2vid; std::unordered_map m_vid2rid; + std::unordered_map m_removedRid2vid; std::shared_ptr m_client; };