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.

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

[fdborch]Add the support for a clear fdb cli #426

Merged
merged 7 commits into from
Jan 26, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
125 changes: 120 additions & 5 deletions orchagent/fdborch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,24 @@
#include "logger.h"
#include "tokenize.h"
#include "fdborch.h"
#include "notifier.h"

extern sai_fdb_api_t *sai_fdb_api;

extern sai_object_id_t gSwitchId;
extern PortsOrch* gPortsOrch;

FdbOrch::FdbOrch(DBConnector *db, string tableName, PortsOrch *port) :
Orch(db, tableName),
m_portsOrch(port),
m_table(Table(db, tableName))
{
m_portsOrch->attach(this);
auto consumer = new NotificationConsumer(db, "FLUSHFDBREQUEST");
auto fdbNotification = new Notifier(consumer, this);
Orch::addExecutor("", fdbNotification);
}

void FdbOrch::update(sai_fdb_event_t type, const sai_fdb_entry_t* entry, sai_object_id_t bridge_port_id)
{
SWSS_LOG_ENTER();
Expand All @@ -34,21 +46,71 @@ void FdbOrch::update(sai_fdb_event_t type, const sai_fdb_entry_t* entry, sai_obj

(void)m_entries.insert(update.entry);
SWSS_LOG_DEBUG("FdbOrch notification: mac %s was inserted into vlan %d", update.entry.mac.to_string().c_str(), entry->vlan_id);

for (auto observer: m_observers)
{
observer->update(SUBJECT_TYPE_FDB_CHANGE, &update);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is about the fdb flush, why touch both LEARNED, AGED, MOVE events?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

separate the handling FLUSH from AGED and MOVE, at the same time remove unnecessary type cast. No impaction to the handling of other events.


break;

case SAI_FDB_EVENT_AGED:
case SAI_FDB_EVENT_FLUSHED:
case SAI_FDB_EVENT_MOVE:
update.add = false;

(void)m_entries.erase(update.entry);
SWSS_LOG_DEBUG("FdbOrch notification: mac %s was removed from vlan %d", update.entry.mac.to_string().c_str(), entry->vlan_id);

for (auto observer: m_observers)
{
observer->update(SUBJECT_TYPE_FDB_CHANGE, &update);
}

break;
}

case SAI_FDB_EVENT_FLUSHED:
if (bridge_port_id == SAI_NULL_OBJECT_ID && !entry->vlan_id)
{
for (auto itr = m_entries.begin(); itr != m_entries.end();)
{
/*
TODO: here should only delete the dynamic fdb entries,
but unfortunately in structure FdbEntry currently have
no member to indicate the fdb entry type,
if there is static mac added, here will have issue.
*/
update.entry.mac = itr->mac;
update.entry.vlan = itr->vlan;
update.add = false;

for (auto observer: m_observers)
{
observer->update(SUBJECT_TYPE_FDB_CHANGE, static_cast<void *>(&update));
itr = m_entries.erase(itr);

SWSS_LOG_DEBUG("FdbOrch notification: mac %s was removed", update.entry.mac.to_string().c_str());

for (auto observer: m_observers)
{
observer->update(SUBJECT_TYPE_FDB_CHANGE, &update);
}
}
}
else if (bridge_port_id && !entry->vlan_id)
{
/*this is a placeholder for flush port fdb case, not supported yet.*/
SWSS_LOG_ERROR("FdbOrch notification: not supported flush port fdb action, port_id = %lu, vlan_id = %d.", bridge_port_id, entry->vlan_id);
}
else if (bridge_port_id == SAI_NULL_OBJECT_ID && entry->vlan_id)
{
/*this is a placeholder for flush vlan fdb case, not supported yet.*/
SWSS_LOG_ERROR("FdbOrch notification: not supported flush vlan fdb action, port_id = %lu, vlan_id = %d.", bridge_port_id, entry->vlan_id);
}
else
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prsunny , in case the fdb flush will send notification of deleted mac one-by-one, does it fall into this category?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Observed in testing that per-entry delete is coming with event type SAI_FDB_EVENT_AGED instead of event FLUSHED, which is handled in the above case. Also, I think, deleting mac one-by-one doesn't fall into this category as there could be multiple mac entries learnt on the same port-vlan combination

{
SWSS_LOG_ERROR("FdbOrch notification: not supported flush fdb action, port_id = %lu, vlan_id = %d.", bridge_port_id, entry->vlan_id);
}
break;
}

return;
}

void FdbOrch::update(SubjectType type, void *cntx)
Expand Down Expand Up @@ -179,6 +241,59 @@ void FdbOrch::doTask(Consumer& consumer)
}
}

void FdbOrch::doTask(NotificationConsumer& consumer)
{
SWSS_LOG_ENTER();

if (!gPortsOrch->isInitDone())
{
return;
}

std::string op;
std::string data;
std::vector<swss::FieldValueTuple> values;

consumer.pop(op, data, values);

if (op == "ALL")
{
/*
* so far only support flush all the FDB entris
* flush per port and flush per vlan will be added later.
*/
sai_status_t status;
sai_attribute_t attr;
attr.id = SAI_FDB_FLUSH_ATTR_ENTRY_TYPE;
attr.value.s32 = SAI_FDB_FLUSH_ENTRY_TYPE_DYNAMIC;

status = sai_fdb_api->flush_fdb_entries(gSwitchId, 1, &attr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when flushing all, there should be 0 attributes passed, all static and dynamic entries should be cleared

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revised.

if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Flush fdb failed, return code %x", status);
}

return;
}
else if (op == "PORT")
{
/*place holder for flush port fdb*/
SWSS_LOG_ERROR("Received unsupported flush port fdb request");
return;
}
else if (op == "VLAN")
{
/*place holder for flush vlan fdb*/
SWSS_LOG_ERROR("Received unsupported flush vlan fdb request");
return;
}
else
{
SWSS_LOG_ERROR("Received unknown flush fdb request");
return;
}
}

void FdbOrch::updateVlanMember(const VlanMemberUpdate& update)
{
SWSS_LOG_ENTER();
Expand Down
10 changes: 3 additions & 7 deletions orchagent/fdborch.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,8 @@ typedef unordered_map<string, vector<SavedFdbEntry>> fdb_entries_by_port_t;
class FdbOrch: public Orch, public Subject, public Observer
{
public:
FdbOrch(DBConnector *db, string tableName, PortsOrch *port) :
Orch(db, tableName),
m_portsOrch(port),
m_table(Table(db, tableName))
{
m_portsOrch->attach(this);
}

FdbOrch(DBConnector *db, string tableName, PortsOrch *port);

~FdbOrch()
{
Expand All @@ -58,6 +53,7 @@ class FdbOrch: public Orch, public Subject, public Observer
Table m_table;

void doTask(Consumer& consumer);
void doTask(NotificationConsumer& consumer);

void updateVlanMember(const VlanMemberUpdate&);
bool addFdbEntry(const FdbEntry&, const string&, const string&);
Expand Down