Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[fdborch] Fix FDB flush issues #2136

Merged
merged 7 commits into from
Aug 9, 2022
Merged

[fdborch] Fix FDB flush issues #2136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
8909431
[fdborch] Fix FDB flush issues
oleksandrx-kolomeiets Feb 7, 2022
e22d21c
Fix coverage
oleksandrx-kolomeiets Apr 18, 2022
388c8fb
Fix test_FdbAddedAfterMemberCreated.
oleksandrx-kolomeiets Jul 4, 2022
0d046d4
Merge branch 'master' into fix_fdb_flush
oleksandrx-kolomeiets Jul 4, 2022
8b6acac
Fix flush_syncd_notif_ut.cpp.
oleksandrx-kolomeiets Jul 18, 2022
601590e
Merge branch 'master' into fix_fdb_flush
oleksandrx-kolomeiets Jul 18, 2022
3095361
Merge branch 'master' into fix_fdb_flush
oleksandrx-kolomeiets Aug 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 27 additions & 4 deletions orchagent/fdborch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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<FdbEntry, FdbData>::iterator it = m_entries.begin();
it != m_entries.end(); it++)
{
it->second.is_flush_pending = true;
}
}

return;
}
else if (op == "PORT")
Expand Down Expand Up @@ -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<FdbEntry, FdbData>::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)
Expand Down
1 change: 1 addition & 0 deletions orchagent/fdborch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
24 changes: 24 additions & 0 deletions tests/mock_tests/fdborch/flush_syncd_notif_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,10 @@ namespace fdb_syncd_flush_test

/* Event 2: Generate a FDB Flush per port and per vlan */
vector<uint8_t> flush_mac_addr = {0, 0, 0, 0, 0, 0};
for (map<FdbEntry, FdbData>::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);

Expand Down Expand Up @@ -228,6 +232,10 @@ namespace fdb_syncd_flush_test

/* Event2: Send a Consolidated Flush response from syncd */
vector<uint8_t> flush_mac_addr = {0, 0, 0, 0, 0, 0};
for (map<FdbEntry, FdbData>::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);

Expand Down Expand Up @@ -272,6 +280,10 @@ namespace fdb_syncd_flush_test

/* Event2: Send a Consolidated Flush response from syncd for vlan */
vector<uint8_t> flush_mac_addr = {0, 0, 0, 0, 0, 0};
for (map<FdbEntry, FdbData>::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);

Expand Down Expand Up @@ -316,6 +328,10 @@ namespace fdb_syncd_flush_test

/* Event2: Send a Consolidated Flush response from syncd for a port */
vector<uint8_t> flush_mac_addr = {0, 0, 0, 0, 0, 0};
for (map<FdbEntry, FdbData>::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);

Expand Down Expand Up @@ -366,6 +382,10 @@ namespace fdb_syncd_flush_test

/* Event 2: Generate a FDB Flush per port and per vlan */
vector<uint8_t> flush_mac_addr = {0, 0, 0, 0, 0, 0};
for (map<FdbEntry, FdbData>::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);

Expand Down Expand Up @@ -410,6 +430,10 @@ namespace fdb_syncd_flush_test

/* Event 2: Generate a non-consilidated FDB Flush per port and per vlan */
vector<uint8_t> flush_mac_addr = {124, 254, 144, 18, 34, 236};
for (map<FdbEntry, FdbData>::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);

Expand Down