Skip to content

Commit

Permalink
[FDB]Fixing FDB consolidated flush for Remote MACs (sonic-net#2673)
Browse files Browse the repository at this point in the history 
* [FDB]Fixing FDB consolidated flush for Remote MACs

* Removing todo
  • Loading branch information
@dgsudharsan
dgsudharsan authored Mar 1, 2023
1 parent c9ae6aa commit ebe8de7
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 17 deletions.
36 changes: 24 additions & 12 deletions orchagent/fdborch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ bool FdbOrch::storeFdbEntryState(const FdbUpdate& update)

fdbdata.bridge_port_id = update.port.m_bridge_port_id;
fdbdata.type = update.type;
fdbdata.sai_fdb_type = update.sai_fdb_type;
fdbdata.origin = FDB_ORIGIN_LEARN;
fdbdata.remote_ip = "";
fdbdata.esi = "";
Expand Down Expand Up @@ -206,20 +207,19 @@ Handles the SAI_FDB_EVENT_FLUSHED notification recieved from syncd
*/
void FdbOrch::handleSyncdFlushNotif(const sai_object_id_t& bv_id,
const sai_object_id_t& bridge_port_id,
const MacAddress& mac)
const MacAddress& mac,
const sai_fdb_entry_type_t& sai_fdb_type)
{
// Consolidated flush will have a zero mac
MacAddress flush_mac("00:00:00:00:00:00");

/* TODO: Read the SAI_FDB_FLUSH_ATTR_ENTRY_TYPE attr from the flush notif
and clear the entries accordingly, currently only non-static entries are flushed
*/
if (bridge_port_id == SAI_NULL_OBJECT_ID && bv_id == SAI_NULL_OBJECT_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) && curr->second.is_flush_pending)
if (curr->second.sai_fdb_type == sai_fdb_type &&
(curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending)
{
clearFdbEntry(curr->first);
}
Expand All @@ -233,7 +233,8 @@ 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) && curr->second.is_flush_pending)
if (curr->second.sai_fdb_type == sai_fdb_type &&
(curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending)
{
clearFdbEntry(curr->first);
}
Expand All @@ -248,7 +249,8 @@ 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) && curr->second.is_flush_pending)
if (curr->second.sai_fdb_type == sai_fdb_type &&
(curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending)
{
clearFdbEntry(curr->first);
}
Expand All @@ -263,7 +265,8 @@ 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) && curr->second.is_flush_pending)
if (curr->second.sai_fdb_type == sai_fdb_type &&
(curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending)
{
clearFdbEntry(curr->first);
}
Expand All @@ -274,7 +277,8 @@ void FdbOrch::handleSyncdFlushNotif(const sai_object_id_t& bv_id,

void FdbOrch::update(sai_fdb_event_t type,
const sai_fdb_entry_t* entry,
sai_object_id_t bridge_port_id)
sai_object_id_t bridge_port_id,
const sai_fdb_entry_type_t &sai_fdb_type)
{
SWSS_LOG_ENTER();

Expand Down Expand Up @@ -365,6 +369,7 @@ void FdbOrch::update(sai_fdb_event_t type,

attr.id = SAI_FDB_ENTRY_ATTR_TYPE;
attr.value.s32 = SAI_FDB_ENTRY_TYPE_DYNAMIC;
update.sai_fdb_type = SAI_FDB_ENTRY_TYPE_DYNAMIC;
attrs.push_back(attr);

attr.id = SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID;
Expand Down Expand Up @@ -399,6 +404,7 @@ void FdbOrch::update(sai_fdb_event_t type,

update.add = true;
update.entry.port_name = update.port.m_alias;
update.sai_fdb_type = SAI_FDB_ENTRY_TYPE_DYNAMIC;
update.type = "dynamic";
update.port.m_fdb_count++;
m_portsOrch->setPort(update.port.m_alias, update.port);
Expand Down Expand Up @@ -569,6 +575,7 @@ void FdbOrch::update(sai_fdb_event_t type,
}
update.port.m_fdb_count++;
m_portsOrch->setPort(update.port.m_alias, update.port);
update.sai_fdb_type = SAI_FDB_ENTRY_TYPE_DYNAMIC;
storeFdbEntryState(update);

notify(SUBJECT_TYPE_FDB_CHANGE, &update);
Expand All @@ -592,7 +599,7 @@ void FdbOrch::update(sai_fdb_event_t type,
SWSS_LOG_INFO("FDB Flush: [ %s , %s ] = { port: %s }", update.entry.mac.to_string().c_str(),
vlanName.c_str(), update.port.m_alias.c_str());

handleSyncdFlushNotif(entry->bv_id, bridge_port_id, update.entry.mac);
handleSyncdFlushNotif(entry->bv_id, bridge_port_id, update.entry.mac, sai_fdb_type);

break;
}
Expand Down Expand Up @@ -996,6 +1003,7 @@ void FdbOrch::doTask(NotificationConsumer& consumer)
{
uint32_t count;
sai_fdb_event_notification_data_t *fdbevent = nullptr;
sai_fdb_entry_type_t sai_fdb_type = SAI_FDB_ENTRY_TYPE_DYNAMIC;

sai_deserialize_fdb_event_ntf(data, count, &fdbevent);

Expand All @@ -1008,11 +1016,14 @@ void FdbOrch::doTask(NotificationConsumer& consumer)
if (fdbevent[i].attr[j].id == SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID)
{
oid = fdbevent[i].attr[j].value.oid;
break;
}
else if (fdbevent[i].attr[j].id == SAI_FDB_ENTRY_ATTR_TYPE)
{
sai_fdb_type = (sai_fdb_entry_type_t)fdbevent[i].attr[j].value.s32;
}
}

this->update(fdbevent[i].event_type, &fdbevent[i].fdb_entry, oid);
this->update(fdbevent[i].event_type, &fdbevent[i].fdb_entry, oid, sai_fdb_type);
}

sai_deserialize_free_fdb_event_ntf(count, fdbevent);
Expand Down Expand Up @@ -1340,6 +1351,7 @@ bool FdbOrch::addFdbEntry(const FdbEntry& entry, const string& port_name,
{
attr.value.s32 = (fdbData.type == "dynamic") ? SAI_FDB_ENTRY_TYPE_DYNAMIC : SAI_FDB_ENTRY_TYPE_STATIC;
}
fdbData.sai_fdb_type = (sai_fdb_entry_type_t)attr.value.s32;

attrs.push_back(attr);

Expand Down
7 changes: 5 additions & 2 deletions orchagent/fdborch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ struct FdbUpdate
Port port;
string type;
bool add;
sai_fdb_entry_type_t sai_fdb_type;
};

struct FdbFlushUpdate
Expand Down Expand Up @@ -63,6 +64,7 @@ struct FdbData
string remote_ip;
string esi;
unsigned int vni;
sai_fdb_entry_type_t sai_fdb_type;
};

struct SavedFdbEntry
Expand Down Expand Up @@ -91,7 +93,7 @@ class FdbOrch: public Orch, public Subject, public Observer
}

bool bake() override;
void update(sai_fdb_event_t, const sai_fdb_entry_t *, sai_object_id_t);
void update(sai_fdb_event_t, const sai_fdb_entry_t *, sai_object_id_t, const sai_fdb_entry_type_t &);
void update(SubjectType type, void *cntx);
bool getPort(const MacAddress&, uint16_t, Port&);

Expand Down Expand Up @@ -125,7 +127,8 @@ class FdbOrch: public Orch, public Subject, public Observer
void notifyTunnelOrch(Port& port);

void clearFdbEntry(const FdbEntry&);
void handleSyncdFlushNotif(const sai_object_id_t&, const sai_object_id_t&, const MacAddress& );
void handleSyncdFlushNotif(const sai_object_id_t&, const sai_object_id_t&, const MacAddress&,
const sai_fdb_entry_type_t&);
};

#endif /* SWSS_FDBORCH_H */
94 changes: 91 additions & 3 deletions tests/mock_tests/fdborch/flush_syncd_notif_ut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#define ETH0 "Ethernet0"
#define VLAN40 "Vlan40"
#define VXLAN_REMOTE "Vxlan_1.1.1.1"

extern redisReply *mockReply;
extern CrmOrch* gCrmOrch;
Expand All @@ -19,6 +20,28 @@ Test Fixture
*/
namespace fdb_syncd_flush_test
{

sai_fdb_api_t ut_sai_fdb_api;
sai_fdb_api_t *pold_sai_fdb_api;

sai_status_t _ut_stub_sai_create_fdb_entry (
_In_ const sai_fdb_entry_t *fdb_entry,
_In_ uint32_t attr_count,
_In_ const sai_attribute_t *attr_list)
{
return SAI_STATUS_SUCCESS;
}
void _hook_sai_fdb_api()
{
ut_sai_fdb_api = *sai_fdb_api;
pold_sai_fdb_api = sai_fdb_api;
ut_sai_fdb_api.create_fdb_entry = _ut_stub_sai_create_fdb_entry;
sai_fdb_api = &ut_sai_fdb_api;
}
void _unhook_sai_fdb_api()
{
sai_fdb_api = pold_sai_fdb_api;
}
struct FdbOrchTest : public ::testing::Test
{
std::shared_ptr<swss::DBConnector> m_config_db;
Expand All @@ -40,7 +63,7 @@ namespace fdb_syncd_flush_test
};

ut_helper::initSaiApi(profile);

/* Create Switch */
sai_attribute_t attr;
attr.id = SAI_SWITCH_ATTR_INIT_SWITCH;
Expand Down Expand Up @@ -70,6 +93,8 @@ namespace fdb_syncd_flush_test
// 2) Crmorch
ASSERT_EQ(gCrmOrch, nullptr);
gCrmOrch = new CrmOrch(m_config_db.get(), CFG_CRM_TABLE_NAME);
VxlanTunnelOrch *vxlan_tunnel_orch_1 = new VxlanTunnelOrch(m_state_db.get(), m_app_db.get(), APP_VXLAN_TUNNEL_TABLE_NAME);
gDirectory.set(vxlan_tunnel_orch_1);

// Construct fdborch
vector<table_name_with_pri_t> app_fdb_tables = {
Expand All @@ -91,7 +116,7 @@ namespace fdb_syncd_flush_test
virtual void TearDown() override {
delete gCrmOrch;
gCrmOrch = nullptr;

gDirectory.m_values.clear();
ut_helper::uninitSaiApi();
}
};
Expand Down Expand Up @@ -126,6 +151,17 @@ namespace fdb_syncd_flush_test
m_portsOrch->saiOidToAlias[oid] = alias;
}

void setUpVxlanPort(PortsOrch* m_portsOrch){
/* Updates portsOrch internal cache for Ethernet0 */
std::string alias = VXLAN_REMOTE;
sai_object_id_t oid = 0x10000000004a5;

Port port(alias, Port::PHY);
m_portsOrch->m_portList[alias] = port;
m_portsOrch->saiOidToAlias[oid] = alias;
}


void setUpVlanMember(PortsOrch* m_portsOrch){
/* Updates portsOrch internal cache for adding Ethernet0 into Vlan40 */
sai_object_id_t bridge_port_id = 0x3a000000002c33;
Expand All @@ -136,6 +172,16 @@ namespace fdb_syncd_flush_test
m_portsOrch->m_portList[VLAN40].m_members.insert(ETH0);
}

void setUpVxlanMember(PortsOrch* m_portsOrch){
/* Updates portsOrch internal cache for adding Ethernet0 into Vlan40 */
sai_object_id_t bridge_port_id = 0x3a000000002c34;

/* Add Bridge Port */
m_portsOrch->m_portList[VXLAN_REMOTE].m_bridge_port_id = bridge_port_id;
m_portsOrch->saiOidToAlias[bridge_port_id] = VXLAN_REMOTE;
m_portsOrch->m_portList[VLAN40].m_members.insert(VXLAN_REMOTE);
}

void triggerUpdate(FdbOrch* m_fdborch,
sai_fdb_event_t type,
vector<uint8_t> mac_addr,
Expand All @@ -146,7 +192,7 @@ namespace fdb_syncd_flush_test
*(entry.mac_address+i) = mac_addr[i];
}
entry.bv_id = bv_id;
m_fdborch->update(type, &entry, bridge_port_id);
m_fdborch->update(type, &entry, bridge_port_id, SAI_FDB_ENTRY_TYPE_DYNAMIC);
}
}

Expand Down Expand Up @@ -445,4 +491,46 @@ namespace fdb_syncd_flush_test
ASSERT_EQ(m_fdborch->m_fdbStateTable.hget("Vlan40:7c:fe:90:12:22:ec", "port", port), false);
ASSERT_EQ(m_fdborch->m_fdbStateTable.hget("Vlan40:7c:fe:90:12:22:ec", "type", entry_type), false);
}

/* Test Consolidated Flush with origin VXLAN */
TEST_F(FdbOrchTest, ConsolidatedFlushAllVxLAN)
{
_hook_sai_fdb_api();
ASSERT_NE(m_portsOrch, nullptr);
setUpVlan(m_portsOrch.get());
setUpVxlanPort(m_portsOrch.get());
ASSERT_NE(m_portsOrch->m_portList.find(VLAN40), m_portsOrch->m_portList.end());
ASSERT_NE(m_portsOrch->m_portList.find(VXLAN_REMOTE), m_portsOrch->m_portList.end());
setUpVxlanMember(m_portsOrch.get());

FdbData fdbData;
fdbData.bridge_port_id = SAI_NULL_OBJECT_ID;
fdbData.type = "dynamic";
fdbData.origin = FDB_ORIGIN_VXLAN_ADVERTIZED;
fdbData.remote_ip = "1.1.1.1";
fdbData.esi = "";
fdbData.vni = 100;
FdbEntry entry;

MacAddress mac1 = MacAddress("52:54:00:ac:3a:99");
entry.mac = mac1;
entry.port_name = VXLAN_REMOTE;

entry.bv_id = m_portsOrch->m_portList[VLAN40].m_vlan_info.vlan_oid;
m_fdborch->addFdbEntry(entry, VXLAN_REMOTE, fdbData);

/* Make sure fdb_count is incremented as expected */
ASSERT_EQ(m_portsOrch->m_portList[VLAN40].m_fdb_count, 1);
ASSERT_EQ(m_portsOrch->m_portList[VXLAN_REMOTE].m_fdb_count, 1);

/* Event2: Send a Consolidated Flush response from syncd */
vector<uint8_t> flush_mac_addr = {0, 0, 0, 0, 0, 0};
triggerUpdate(m_fdborch.get(), SAI_FDB_EVENT_FLUSHED, flush_mac_addr, SAI_NULL_OBJECT_ID,
SAI_NULL_OBJECT_ID);

/* make sure fdb_counters are decremented */
ASSERT_EQ(m_portsOrch->m_portList[VLAN40].m_fdb_count, 1);
ASSERT_EQ(m_portsOrch->m_portList[VXLAN_REMOTE].m_fdb_count, 1);
_unhook_sai_fdb_api();
}
}
1 change: 1 addition & 0 deletions tests/mock_tests/mock_orchagent_main.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,4 @@ extern sai_udf_api_t* sai_udf_api;
extern sai_mpls_api_t* sai_mpls_api;
extern sai_counter_api_t* sai_counter_api;
extern sai_samplepacket_api_t *sai_samplepacket_api;
extern sai_fdb_api_t* sai_fdb_api;
1 change: 1 addition & 0 deletions tests/mock_tests/ut_saihelper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ namespace ut_helper
sai_api_query(SAI_API_QUEUE, (void **)&sai_queue_api);
sai_api_query(SAI_API_MPLS, (void**)&sai_mpls_api);
sai_api_query(SAI_API_COUNTER, (void**)&sai_counter_api);
sai_api_query(SAI_API_FDB, (void**)&sai_fdb_api);

return SAI_STATUS_SUCCESS;
}
Expand Down

0 comments on commit ebe8de7

Please sign in to comment.