From 8dae35645962b0ce8b9b4ddc80d64162b51b5f65 Mon Sep 17 00:00:00 2001 From: oleksandrx-kolomeiets Date: Tue, 9 Aug 2022 20:10:13 +0300 Subject: [PATCH] [fdborch] Fix FDB flush issues (#2136) * [fdborch] Fix FDB flush issues Signed-off-by: Oleksandr Kolomeiets * Fix coverage * Fix test_FdbAddedAfterMemberCreated. * Fix flush_syncd_notif_ut.cpp. --- orchagent/fdborch.cpp | 31 ++++++++++++++++--- orchagent/fdborch.h | 1 + .../fdborch/flush_syncd_notif_ut.cpp | 24 ++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/orchagent/fdborch.cpp b/orchagent/fdborch.cpp index a40114883604..720d6c5c6634 100644 --- a/orchagent/fdborch.cpp +++ b/orchagent/fdborch.cpp @@ -219,7 +219,7 @@ void FdbOrch::handleSyncdFlushNotif(const sai_object_id_t& bv_id, for (auto itr = m_entries.begin(); itr != m_entries.end();) { auto curr = itr++; - if (curr->second.type != "static" && (curr->first.mac == mac || mac == flush_mac)) + if (curr->second.type != "static" && (curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending) { clearFdbEntry(curr->first); } @@ -233,7 +233,7 @@ void FdbOrch::handleSyncdFlushNotif(const sai_object_id_t& bv_id, auto curr = itr++; if (curr->second.bridge_port_id == bridge_port_id) { - if (curr->second.type != "static" && (curr->first.mac == mac || mac == flush_mac)) + if (curr->second.type != "static" && (curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending) { clearFdbEntry(curr->first); } @@ -248,7 +248,7 @@ void FdbOrch::handleSyncdFlushNotif(const sai_object_id_t& bv_id, auto curr = itr++; if (curr->first.bv_id == bv_id) { - if (curr->second.type != "static" && (curr->first.mac == mac || mac == flush_mac)) + if (curr->second.type != "static" && (curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending) { clearFdbEntry(curr->first); } @@ -263,7 +263,7 @@ void FdbOrch::handleSyncdFlushNotif(const sai_object_id_t& bv_id, auto curr = itr++; if (curr->first.bv_id == bv_id && curr->second.bridge_port_id == bridge_port_id) { - if (curr->second.type != "static" && (curr->first.mac == mac || mac == flush_mac)) + if (curr->second.type != "static" && (curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending) { clearFdbEntry(curr->first); } @@ -819,6 +819,7 @@ void FdbOrch::doTask(Consumer& consumer) fdbData.remote_ip = remote_ip; fdbData.esi = esi; fdbData.vni = vni; + fdbData.is_flush_pending = false; if (addFdbEntry(entry, port, fdbData)) { if (origin == FDB_ORIGIN_MCLAG_ADVERTIZED) @@ -907,6 +908,14 @@ void FdbOrch::doTask(NotificationConsumer& consumer) SWSS_LOG_ERROR("Flush fdb failed, return code %x", status); } + if (status == SAI_STATUS_SUCCESS) { + for (map::iterator it = m_entries.begin(); + it != m_entries.end(); it++) + { + it->second.is_flush_pending = true; + } + } + return; } else if (op == "PORT") @@ -1071,6 +1080,20 @@ void FdbOrch::flushFDBEntries(sai_object_id_t bridge_port_oid, { SWSS_LOG_ERROR("Flushing FDB failed. rv:%d", rv); } + + if (SAI_STATUS_SUCCESS == rv) { + for (map::iterator it = m_entries.begin(); + it != m_entries.end(); it++) + { + if ((bridge_port_oid != SAI_NULL_OBJECT_ID && + it->second.bridge_port_id == bridge_port_oid) || + (vlan_oid != SAI_NULL_OBJECT_ID && + it->first.bv_id == vlan_oid)) + { + it->second.is_flush_pending = true; + } + } + } } void FdbOrch::notifyObserversFDBFlush(Port &port, sai_object_id_t& bvid) diff --git a/orchagent/fdborch.h b/orchagent/fdborch.h index 949ffbf289e6..d9f739823797 100644 --- a/orchagent/fdborch.h +++ b/orchagent/fdborch.h @@ -57,6 +57,7 @@ struct FdbData {"static", FDB_ORIGIN_PROVISIONED} => statically provisioned {"static", FDB_ORIGIN_ADVERTIZED} => sticky synced from remote device */ + bool is_flush_pending; /* Remote FDB related info */ string remote_ip; diff --git a/tests/mock_tests/fdborch/flush_syncd_notif_ut.cpp b/tests/mock_tests/fdborch/flush_syncd_notif_ut.cpp index 7bf22373c162..d0f8954fd86f 100644 --- a/tests/mock_tests/fdborch/flush_syncd_notif_ut.cpp +++ b/tests/mock_tests/fdborch/flush_syncd_notif_ut.cpp @@ -184,6 +184,10 @@ namespace fdb_syncd_flush_test /* Event 2: Generate a FDB Flush per port and per vlan */ vector flush_mac_addr = {0, 0, 0, 0, 0, 0}; + for (map::iterator it = m_fdborch->m_entries.begin(); it != m_fdborch->m_entries.end(); it++) + { + it->second.is_flush_pending = true; + } triggerUpdate(m_fdborch.get(), SAI_FDB_EVENT_FLUSHED, flush_mac_addr, m_portsOrch->m_portList[ETH0].m_bridge_port_id, m_portsOrch->m_portList[VLAN40].m_vlan_info.vlan_oid); @@ -228,6 +232,10 @@ namespace fdb_syncd_flush_test /* Event2: Send a Consolidated Flush response from syncd */ vector flush_mac_addr = {0, 0, 0, 0, 0, 0}; + for (map::iterator it = m_fdborch->m_entries.begin(); it != m_fdborch->m_entries.end(); it++) + { + it->second.is_flush_pending = true; + } triggerUpdate(m_fdborch.get(), SAI_FDB_EVENT_FLUSHED, flush_mac_addr, SAI_NULL_OBJECT_ID, SAI_NULL_OBJECT_ID); @@ -272,6 +280,10 @@ namespace fdb_syncd_flush_test /* Event2: Send a Consolidated Flush response from syncd for vlan */ vector flush_mac_addr = {0, 0, 0, 0, 0, 0}; + for (map::iterator it = m_fdborch->m_entries.begin(); it != m_fdborch->m_entries.end(); it++) + { + it->second.is_flush_pending = true; + } triggerUpdate(m_fdborch.get(), SAI_FDB_EVENT_FLUSHED, flush_mac_addr, SAI_NULL_OBJECT_ID, m_portsOrch->m_portList[VLAN40].m_vlan_info.vlan_oid); @@ -316,6 +328,10 @@ namespace fdb_syncd_flush_test /* Event2: Send a Consolidated Flush response from syncd for a port */ vector flush_mac_addr = {0, 0, 0, 0, 0, 0}; + for (map::iterator it = m_fdborch->m_entries.begin(); it != m_fdborch->m_entries.end(); it++) + { + it->second.is_flush_pending = true; + } triggerUpdate(m_fdborch.get(), SAI_FDB_EVENT_FLUSHED, flush_mac_addr, m_portsOrch->m_portList[ETH0].m_bridge_port_id, SAI_NULL_OBJECT_ID); @@ -366,6 +382,10 @@ namespace fdb_syncd_flush_test /* Event 2: Generate a FDB Flush per port and per vlan */ vector flush_mac_addr = {0, 0, 0, 0, 0, 0}; + for (map::iterator it = m_fdborch->m_entries.begin(); it != m_fdborch->m_entries.end(); it++) + { + it->second.is_flush_pending = true; + } triggerUpdate(m_fdborch.get(), SAI_FDB_EVENT_FLUSHED, flush_mac_addr, bridge_port_oid, m_portsOrch->m_portList[VLAN40].m_vlan_info.vlan_oid); @@ -410,6 +430,10 @@ namespace fdb_syncd_flush_test /* Event 2: Generate a non-consilidated FDB Flush per port and per vlan */ vector flush_mac_addr = {124, 254, 144, 18, 34, 236}; + for (map::iterator it = m_fdborch->m_entries.begin(); it != m_fdborch->m_entries.end(); it++) + { + it->second.is_flush_pending = true; + } triggerUpdate(m_fdborch.get(), SAI_FDB_EVENT_FLUSHED, flush_mac_addr, m_portsOrch->m_portList[ETH0].m_bridge_port_id, m_portsOrch->m_portList[VLAN40].m_vlan_info.vlan_oid);