Skip to content

Commit ebe8de7

Browse files
authored
[FDB]Fixing FDB consolidated flush for Remote MACs (#2673)
* [FDB]Fixing FDB consolidated flush for Remote MACs * Removing todo
1 parent c9ae6aa commit ebe8de7

File tree

5 files changed

+122
-17
lines changed

5 files changed

+122
-17
lines changed

orchagent/fdborch.cpp

+24-12
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ bool FdbOrch::storeFdbEntryState(const FdbUpdate& update)
109109

110110
fdbdata.bridge_port_id = update.port.m_bridge_port_id;
111111
fdbdata.type = update.type;
112+
fdbdata.sai_fdb_type = update.sai_fdb_type;
112113
fdbdata.origin = FDB_ORIGIN_LEARN;
113114
fdbdata.remote_ip = "";
114115
fdbdata.esi = "";
@@ -206,20 +207,19 @@ Handles the SAI_FDB_EVENT_FLUSHED notification recieved from syncd
206207
*/
207208
void FdbOrch::handleSyncdFlushNotif(const sai_object_id_t& bv_id,
208209
const sai_object_id_t& bridge_port_id,
209-
const MacAddress& mac)
210+
const MacAddress& mac,
211+
const sai_fdb_entry_type_t& sai_fdb_type)
210212
{
211213
// Consolidated flush will have a zero mac
212214
MacAddress flush_mac("00:00:00:00:00:00");
213215

214-
/* TODO: Read the SAI_FDB_FLUSH_ATTR_ENTRY_TYPE attr from the flush notif
215-
and clear the entries accordingly, currently only non-static entries are flushed
216-
*/
217216
if (bridge_port_id == SAI_NULL_OBJECT_ID && bv_id == SAI_NULL_OBJECT_ID)
218217
{
219218
for (auto itr = m_entries.begin(); itr != m_entries.end();)
220219
{
221220
auto curr = itr++;
222-
if (curr->second.type != "static" && (curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending)
221+
if (curr->second.sai_fdb_type == sai_fdb_type &&
222+
(curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending)
223223
{
224224
clearFdbEntry(curr->first);
225225
}
@@ -233,7 +233,8 @@ void FdbOrch::handleSyncdFlushNotif(const sai_object_id_t& bv_id,
233233
auto curr = itr++;
234234
if (curr->second.bridge_port_id == bridge_port_id)
235235
{
236-
if (curr->second.type != "static" && (curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending)
236+
if (curr->second.sai_fdb_type == sai_fdb_type &&
237+
(curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending)
237238
{
238239
clearFdbEntry(curr->first);
239240
}
@@ -248,7 +249,8 @@ void FdbOrch::handleSyncdFlushNotif(const sai_object_id_t& bv_id,
248249
auto curr = itr++;
249250
if (curr->first.bv_id == bv_id)
250251
{
251-
if (curr->second.type != "static" && (curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending)
252+
if (curr->second.sai_fdb_type == sai_fdb_type &&
253+
(curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending)
252254
{
253255
clearFdbEntry(curr->first);
254256
}
@@ -263,7 +265,8 @@ void FdbOrch::handleSyncdFlushNotif(const sai_object_id_t& bv_id,
263265
auto curr = itr++;
264266
if (curr->first.bv_id == bv_id && curr->second.bridge_port_id == bridge_port_id)
265267
{
266-
if (curr->second.type != "static" && (curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending)
268+
if (curr->second.sai_fdb_type == sai_fdb_type &&
269+
(curr->first.mac == mac || mac == flush_mac) && curr->second.is_flush_pending)
267270
{
268271
clearFdbEntry(curr->first);
269272
}
@@ -274,7 +277,8 @@ void FdbOrch::handleSyncdFlushNotif(const sai_object_id_t& bv_id,
274277

275278
void FdbOrch::update(sai_fdb_event_t type,
276279
const sai_fdb_entry_t* entry,
277-
sai_object_id_t bridge_port_id)
280+
sai_object_id_t bridge_port_id,
281+
const sai_fdb_entry_type_t &sai_fdb_type)
278282
{
279283
SWSS_LOG_ENTER();
280284

@@ -365,6 +369,7 @@ void FdbOrch::update(sai_fdb_event_t type,
365369

366370
attr.id = SAI_FDB_ENTRY_ATTR_TYPE;
367371
attr.value.s32 = SAI_FDB_ENTRY_TYPE_DYNAMIC;
372+
update.sai_fdb_type = SAI_FDB_ENTRY_TYPE_DYNAMIC;
368373
attrs.push_back(attr);
369374

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

400405
update.add = true;
401406
update.entry.port_name = update.port.m_alias;
407+
update.sai_fdb_type = SAI_FDB_ENTRY_TYPE_DYNAMIC;
402408
update.type = "dynamic";
403409
update.port.m_fdb_count++;
404410
m_portsOrch->setPort(update.port.m_alias, update.port);
@@ -569,6 +575,7 @@ void FdbOrch::update(sai_fdb_event_t type,
569575
}
570576
update.port.m_fdb_count++;
571577
m_portsOrch->setPort(update.port.m_alias, update.port);
578+
update.sai_fdb_type = SAI_FDB_ENTRY_TYPE_DYNAMIC;
572579
storeFdbEntryState(update);
573580

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

595-
handleSyncdFlushNotif(entry->bv_id, bridge_port_id, update.entry.mac);
602+
handleSyncdFlushNotif(entry->bv_id, bridge_port_id, update.entry.mac, sai_fdb_type);
596603

597604
break;
598605
}
@@ -996,6 +1003,7 @@ void FdbOrch::doTask(NotificationConsumer& consumer)
9961003
{
9971004
uint32_t count;
9981005
sai_fdb_event_notification_data_t *fdbevent = nullptr;
1006+
sai_fdb_entry_type_t sai_fdb_type = SAI_FDB_ENTRY_TYPE_DYNAMIC;
9991007

10001008
sai_deserialize_fdb_event_ntf(data, count, &fdbevent);
10011009

@@ -1008,11 +1016,14 @@ void FdbOrch::doTask(NotificationConsumer& consumer)
10081016
if (fdbevent[i].attr[j].id == SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID)
10091017
{
10101018
oid = fdbevent[i].attr[j].value.oid;
1011-
break;
1019+
}
1020+
else if (fdbevent[i].attr[j].id == SAI_FDB_ENTRY_ATTR_TYPE)
1021+
{
1022+
sai_fdb_type = (sai_fdb_entry_type_t)fdbevent[i].attr[j].value.s32;
10121023
}
10131024
}
10141025

1015-
this->update(fdbevent[i].event_type, &fdbevent[i].fdb_entry, oid);
1026+
this->update(fdbevent[i].event_type, &fdbevent[i].fdb_entry, oid, sai_fdb_type);
10161027
}
10171028

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

13441356
attrs.push_back(attr);
13451357

orchagent/fdborch.h

+5-2
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ struct FdbUpdate
3636
Port port;
3737
string type;
3838
bool add;
39+
sai_fdb_entry_type_t sai_fdb_type;
3940
};
4041

4142
struct FdbFlushUpdate
@@ -63,6 +64,7 @@ struct FdbData
6364
string remote_ip;
6465
string esi;
6566
unsigned int vni;
67+
sai_fdb_entry_type_t sai_fdb_type;
6668
};
6769

6870
struct SavedFdbEntry
@@ -91,7 +93,7 @@ class FdbOrch: public Orch, public Subject, public Observer
9193
}
9294

9395
bool bake() override;
94-
void update(sai_fdb_event_t, const sai_fdb_entry_t *, sai_object_id_t);
96+
void update(sai_fdb_event_t, const sai_fdb_entry_t *, sai_object_id_t, const sai_fdb_entry_type_t &);
9597
void update(SubjectType type, void *cntx);
9698
bool getPort(const MacAddress&, uint16_t, Port&);
9799

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

127129
void clearFdbEntry(const FdbEntry&);
128-
void handleSyncdFlushNotif(const sai_object_id_t&, const sai_object_id_t&, const MacAddress& );
130+
void handleSyncdFlushNotif(const sai_object_id_t&, const sai_object_id_t&, const MacAddress&,
131+
const sai_fdb_entry_type_t&);
129132
};
130133

131134
#endif /* SWSS_FDBORCH_H */

tests/mock_tests/fdborch/flush_syncd_notif_ut.cpp

+91-3
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
#define ETH0 "Ethernet0"
1212
#define VLAN40 "Vlan40"
13+
#define VXLAN_REMOTE "Vxlan_1.1.1.1"
1314

1415
extern redisReply *mockReply;
1516
extern CrmOrch* gCrmOrch;
@@ -19,6 +20,28 @@ Test Fixture
1920
*/
2021
namespace fdb_syncd_flush_test
2122
{
23+
24+
sai_fdb_api_t ut_sai_fdb_api;
25+
sai_fdb_api_t *pold_sai_fdb_api;
26+
27+
sai_status_t _ut_stub_sai_create_fdb_entry (
28+
_In_ const sai_fdb_entry_t *fdb_entry,
29+
_In_ uint32_t attr_count,
30+
_In_ const sai_attribute_t *attr_list)
31+
{
32+
return SAI_STATUS_SUCCESS;
33+
}
34+
void _hook_sai_fdb_api()
35+
{
36+
ut_sai_fdb_api = *sai_fdb_api;
37+
pold_sai_fdb_api = sai_fdb_api;
38+
ut_sai_fdb_api.create_fdb_entry = _ut_stub_sai_create_fdb_entry;
39+
sai_fdb_api = &ut_sai_fdb_api;
40+
}
41+
void _unhook_sai_fdb_api()
42+
{
43+
sai_fdb_api = pold_sai_fdb_api;
44+
}
2245
struct FdbOrchTest : public ::testing::Test
2346
{
2447
std::shared_ptr<swss::DBConnector> m_config_db;
@@ -40,7 +63,7 @@ namespace fdb_syncd_flush_test
4063
};
4164

4265
ut_helper::initSaiApi(profile);
43-
66+
4467
/* Create Switch */
4568
sai_attribute_t attr;
4669
attr.id = SAI_SWITCH_ATTR_INIT_SWITCH;
@@ -70,6 +93,8 @@ namespace fdb_syncd_flush_test
7093
// 2) Crmorch
7194
ASSERT_EQ(gCrmOrch, nullptr);
7295
gCrmOrch = new CrmOrch(m_config_db.get(), CFG_CRM_TABLE_NAME);
96+
VxlanTunnelOrch *vxlan_tunnel_orch_1 = new VxlanTunnelOrch(m_state_db.get(), m_app_db.get(), APP_VXLAN_TUNNEL_TABLE_NAME);
97+
gDirectory.set(vxlan_tunnel_orch_1);
7398

7499
// Construct fdborch
75100
vector<table_name_with_pri_t> app_fdb_tables = {
@@ -91,7 +116,7 @@ namespace fdb_syncd_flush_test
91116
virtual void TearDown() override {
92117
delete gCrmOrch;
93118
gCrmOrch = nullptr;
94-
119+
gDirectory.m_values.clear();
95120
ut_helper::uninitSaiApi();
96121
}
97122
};
@@ -126,6 +151,17 @@ namespace fdb_syncd_flush_test
126151
m_portsOrch->saiOidToAlias[oid] = alias;
127152
}
128153

154+
void setUpVxlanPort(PortsOrch* m_portsOrch){
155+
/* Updates portsOrch internal cache for Ethernet0 */
156+
std::string alias = VXLAN_REMOTE;
157+
sai_object_id_t oid = 0x10000000004a5;
158+
159+
Port port(alias, Port::PHY);
160+
m_portsOrch->m_portList[alias] = port;
161+
m_portsOrch->saiOidToAlias[oid] = alias;
162+
}
163+
164+
129165
void setUpVlanMember(PortsOrch* m_portsOrch){
130166
/* Updates portsOrch internal cache for adding Ethernet0 into Vlan40 */
131167
sai_object_id_t bridge_port_id = 0x3a000000002c33;
@@ -136,6 +172,16 @@ namespace fdb_syncd_flush_test
136172
m_portsOrch->m_portList[VLAN40].m_members.insert(ETH0);
137173
}
138174

175+
void setUpVxlanMember(PortsOrch* m_portsOrch){
176+
/* Updates portsOrch internal cache for adding Ethernet0 into Vlan40 */
177+
sai_object_id_t bridge_port_id = 0x3a000000002c34;
178+
179+
/* Add Bridge Port */
180+
m_portsOrch->m_portList[VXLAN_REMOTE].m_bridge_port_id = bridge_port_id;
181+
m_portsOrch->saiOidToAlias[bridge_port_id] = VXLAN_REMOTE;
182+
m_portsOrch->m_portList[VLAN40].m_members.insert(VXLAN_REMOTE);
183+
}
184+
139185
void triggerUpdate(FdbOrch* m_fdborch,
140186
sai_fdb_event_t type,
141187
vector<uint8_t> mac_addr,
@@ -146,7 +192,7 @@ namespace fdb_syncd_flush_test
146192
*(entry.mac_address+i) = mac_addr[i];
147193
}
148194
entry.bv_id = bv_id;
149-
m_fdborch->update(type, &entry, bridge_port_id);
195+
m_fdborch->update(type, &entry, bridge_port_id, SAI_FDB_ENTRY_TYPE_DYNAMIC);
150196
}
151197
}
152198

@@ -445,4 +491,46 @@ namespace fdb_syncd_flush_test
445491
ASSERT_EQ(m_fdborch->m_fdbStateTable.hget("Vlan40:7c:fe:90:12:22:ec", "port", port), false);
446492
ASSERT_EQ(m_fdborch->m_fdbStateTable.hget("Vlan40:7c:fe:90:12:22:ec", "type", entry_type), false);
447493
}
494+
495+
/* Test Consolidated Flush with origin VXLAN */
496+
TEST_F(FdbOrchTest, ConsolidatedFlushAllVxLAN)
497+
{
498+
_hook_sai_fdb_api();
499+
ASSERT_NE(m_portsOrch, nullptr);
500+
setUpVlan(m_portsOrch.get());
501+
setUpVxlanPort(m_portsOrch.get());
502+
ASSERT_NE(m_portsOrch->m_portList.find(VLAN40), m_portsOrch->m_portList.end());
503+
ASSERT_NE(m_portsOrch->m_portList.find(VXLAN_REMOTE), m_portsOrch->m_portList.end());
504+
setUpVxlanMember(m_portsOrch.get());
505+
506+
FdbData fdbData;
507+
fdbData.bridge_port_id = SAI_NULL_OBJECT_ID;
508+
fdbData.type = "dynamic";
509+
fdbData.origin = FDB_ORIGIN_VXLAN_ADVERTIZED;
510+
fdbData.remote_ip = "1.1.1.1";
511+
fdbData.esi = "";
512+
fdbData.vni = 100;
513+
FdbEntry entry;
514+
515+
MacAddress mac1 = MacAddress("52:54:00:ac:3a:99");
516+
entry.mac = mac1;
517+
entry.port_name = VXLAN_REMOTE;
518+
519+
entry.bv_id = m_portsOrch->m_portList[VLAN40].m_vlan_info.vlan_oid;
520+
m_fdborch->addFdbEntry(entry, VXLAN_REMOTE, fdbData);
521+
522+
/* Make sure fdb_count is incremented as expected */
523+
ASSERT_EQ(m_portsOrch->m_portList[VLAN40].m_fdb_count, 1);
524+
ASSERT_EQ(m_portsOrch->m_portList[VXLAN_REMOTE].m_fdb_count, 1);
525+
526+
/* Event2: Send a Consolidated Flush response from syncd */
527+
vector<uint8_t> flush_mac_addr = {0, 0, 0, 0, 0, 0};
528+
triggerUpdate(m_fdborch.get(), SAI_FDB_EVENT_FLUSHED, flush_mac_addr, SAI_NULL_OBJECT_ID,
529+
SAI_NULL_OBJECT_ID);
530+
531+
/* make sure fdb_counters are decremented */
532+
ASSERT_EQ(m_portsOrch->m_portList[VLAN40].m_fdb_count, 1);
533+
ASSERT_EQ(m_portsOrch->m_portList[VXLAN_REMOTE].m_fdb_count, 1);
534+
_unhook_sai_fdb_api();
535+
}
448536
}

tests/mock_tests/mock_orchagent_main.h

+1
Original file line numberDiff line numberDiff line change
@@ -82,3 +82,4 @@ extern sai_udf_api_t* sai_udf_api;
8282
extern sai_mpls_api_t* sai_mpls_api;
8383
extern sai_counter_api_t* sai_counter_api;
8484
extern sai_samplepacket_api_t *sai_samplepacket_api;
85+
extern sai_fdb_api_t* sai_fdb_api;

tests/mock_tests/ut_saihelper.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ namespace ut_helper
8686
sai_api_query(SAI_API_QUEUE, (void **)&sai_queue_api);
8787
sai_api_query(SAI_API_MPLS, (void**)&sai_mpls_api);
8888
sai_api_query(SAI_API_COUNTER, (void**)&sai_counter_api);
89+
sai_api_query(SAI_API_FDB, (void**)&sai_fdb_api);
8990

9091
return SAI_STATUS_SUCCESS;
9192
}

0 commit comments

Comments
 (0)