Skip to content

Commit

Permalink
[AclOrch] aclOrch enhancement to handle LAG port not configured case (s…
Browse files Browse the repository at this point in the history 
…onic-net#494)

* enhance acl orch to handle the LAG port not configured case
* rename variable, function; redunce redundant code; fix memory issue
* revise according to comments
* one more blank line
* add new VS test cases for acl on LAG
* update with PR comments fix
* rename getValidPortId and move it to portOrch
* fix indent
* fix test case comments
  • Loading branch information
@keboliu @qiluo-msft
keboliu authored and qiluo-msft committed May 25, 2018
1 parent 44a8845 commit 4df9c28
Show file tree
Hide file tree
Showing 9 changed files with 453 additions and 89 deletions.
154 changes: 118 additions & 36 deletions orchagent/aclorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -936,7 +936,7 @@ bool AclTable::validate()
{
// Control plane ACLs are handled by a separate process
if (type == ACL_TABLE_UNKNOWN || type == ACL_TABLE_CTRLPLANE) return false;
if (ports.empty()) return false;
if (portSet.empty()) return false;
return true;
}

Expand Down Expand Up @@ -1361,8 +1361,8 @@ bool AclRange::remove()
return true;
}

AclOrch::AclOrch(DBConnector *db, vector<string> tableNames, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch) :
Orch(db, tableNames),
AclOrch::AclOrch(vector<TableConnector>& connectors, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch) :
Orch(connectors),
m_mirrorOrch(mirrorOrch),
m_neighOrch(neighOrch),
m_routeOrch(routeOrch)
Expand Down Expand Up @@ -1445,6 +1445,11 @@ void AclOrch::doTask(Consumer &consumer)
unique_lock<mutex> lock(m_countersMutex);
doAclRuleTask(consumer);
}
else if (table_name == STATE_LAG_TABLE_NAME)
{
unique_lock<mutex> lock(m_countersMutex);
doAclTablePortUpdateTask(consumer);
}
else
{
SWSS_LOG_ERROR("Invalid table %s", table_name.c_str());
Expand Down Expand Up @@ -1545,7 +1550,7 @@ void AclOrch::doAclTableTask(Consumer &consumer)
{
KeyOpFieldsValuesTuple t = it->second;
string key = kfvKey(t);
size_t found = key.find('|');
size_t found = key.find(consumer.getConsumerTable()->getTableNameSeparator().c_str());
string table_id = key.substr(0, found);
string op = kfvOp(t);

Expand Down Expand Up @@ -1580,7 +1585,7 @@ void AclOrch::doAclTableTask(Consumer &consumer)
}
else if (attr_name == TABLE_PORTS)
{
bool suc = processPorts(attr_value, [&](sai_object_id_t portOid) {
bool suc = processPorts(newTable, attr_value, [&](sai_object_id_t portOid) {
newTable.link(portOid);
});

Expand Down Expand Up @@ -1645,7 +1650,7 @@ void AclOrch::doAclRuleTask(Consumer &consumer)
{
KeyOpFieldsValuesTuple t = it->second;
string key = kfvKey(t);
size_t found = key.find('|');
size_t found = key.find(consumer.getConsumerTable()->getTableNameSeparator().c_str());
string table_id = key.substr(0, found);
string rule_id = key.substr(found + 1);
string op = kfvOp(t);
Expand Down Expand Up @@ -1725,17 +1730,79 @@ void AclOrch::doAclRuleTask(Consumer &consumer)
}
}

bool AclOrch::processPorts(string portsList, std::function<void (sai_object_id_t)> inserter)
void AclOrch::doAclTablePortUpdateTask(Consumer &consumer)
{
SWSS_LOG_ENTER();

auto it = consumer.m_toSync.begin();
while (it != consumer.m_toSync.end())
{
KeyOpFieldsValuesTuple t = it->second;
string key = kfvKey(t);
size_t found = key.find(consumer.getConsumerTable()->getTableNameSeparator().c_str());
string port_alias = key.substr(0, found);
string op = kfvOp(t);

SWSS_LOG_INFO("doAclTablePortUpdateTask: OP: %s, port_alias: %s", op.c_str(), port_alias.c_str());

if (op == SET_COMMAND)
{
for (auto itmap : m_AclTables)
{
auto table = itmap.second;
if (table.pendingPortSet.find(port_alias) != table.pendingPortSet.end())
{
SWSS_LOG_INFO("found the port: %s in ACL table: %s pending port list, bind it to ACL table.", port_alias.c_str(), table.description.c_str());

bool suc = processPendingPort(table, port_alias, [&](sai_object_id_t portOid) {
table.link(portOid);
});

if (!suc)
{
SWSS_LOG_ERROR("Failed to bind the ACL table: %s to port: %s", table.description.c_str(), port_alias.c_str());
}
else
{
table.pendingPortSet.erase(port_alias);
SWSS_LOG_DEBUG("port: %s bound to ACL table table: %s, remove it from pending list", port_alias.c_str(), table.description.c_str());
}
}
}
}
else if (op == DEL_COMMAND)
{
for (auto itmap : m_AclTables)
{
auto table = itmap.second;
if (table.portSet.find(port_alias) != table.portSet.end())
{
/*TODO: update the ACL table after port/lag deleted*/
table.pendingPortSet.emplace(port_alias);
SWSS_LOG_INFO("Add deleted port: %s to the pending list of ACL table: %s", port_alias.c_str(), table.description.c_str());
}
}
}
else
{
SWSS_LOG_ERROR("Unknown operation type %s", op.c_str());
}
it = consumer.m_toSync.erase(it);
}
}

bool AclOrch::processPorts(AclTable &aclTable, string portsList, std::function<void (sai_object_id_t)> inserter)
{
SWSS_LOG_ENTER();

vector<string> strList;

SWSS_LOG_INFO("Processing ACL table port list %s", portsList.c_str());
SWSS_LOG_DEBUG("Processing ACL table port list %s", portsList.c_str());

split(portsList, strList, ',');

set<string> strSet(strList.begin(), strList.end());
aclTable.portSet = strSet;

if (strList.size() != strSet.size())
{
Expand All @@ -1751,33 +1818,52 @@ bool AclOrch::processPorts(string portsList, std::function<void (sai_object_id_t

for (const auto& alias : strList)
{
sai_object_id_t port_id;
Port port;
if (!gPortsOrch->getPort(alias, port))
{
SWSS_LOG_ERROR("Failed to process port. Port %s doesn't exist", alias.c_str());
return false;
SWSS_LOG_INFO("Port %s not configured yet, add it to ACL table %s pending list", alias.c_str(), aclTable.description.c_str());
aclTable.pendingPortSet.emplace(alias);
continue;
}

switch (port.m_type)
if (gPortsOrch->getAclBindPortId(alias, port_id))
{
case Port::PHY:
if (port.m_lag_member_id != SAI_NULL_OBJECT_ID)
{
SWSS_LOG_ERROR("Failed to process port. Bind table to LAG member %s is not allowed", alias.c_str());
return false;
}
inserter(port.m_port_id);
break;
case Port::LAG:
inserter(port.m_lag_id);
break;
case Port::VLAN:
inserter(port.m_vlan_info.vlan_oid);
break;
default:
SWSS_LOG_ERROR("Failed to process port. Incorrect port %s type %d", alias.c_str(), port.m_type);
return false;
}
inserter(port_id);
}
else
{
return false;
}
}

return true;
}

bool AclOrch::processPendingPort(AclTable &aclTable, string portAlias, std::function<void (sai_object_id_t)> inserter)
{
SWSS_LOG_ENTER();

SWSS_LOG_DEBUG("Processing ACL table port %s", portAlias.c_str());

sai_object_id_t port_id;

Port port;
if (!gPortsOrch->getPort(portAlias, port))
{
SWSS_LOG_INFO("Port %s not configured yet, add it to ACL table %s pending list", portAlias.c_str(), aclTable.description.c_str());
aclTable.pendingPortSet.insert(portAlias);
return true;
}

if (gPortsOrch->getAclBindPortId(portAlias, port_id))
{
inserter(port_id);
aclTable.bind(port_id);
}
else
{
return false;
}

return true;
Expand Down Expand Up @@ -1894,18 +1980,14 @@ sai_status_t AclOrch::bindAclTable(sai_object_id_t table_oid, AclTable &aclTable
sai_status_t status = SAI_STATUS_SUCCESS;

SWSS_LOG_INFO("%s table %s to ports", bind ? "Bind" : "Unbind", aclTable.id.c_str());

if (aclTable.ports.empty())
{
if (bind)
{
SWSS_LOG_ERROR("Port list is not configured for %s table", aclTable.id.c_str());
return SAI_STATUS_FAILURE;
}
else
{
return SAI_STATUS_SUCCESS;
SWSS_LOG_WARN("Binding port list is empty for %s table", aclTable.id.c_str());
}
return SAI_STATUS_SUCCESS;
}

if (bind)
Expand Down
10 changes: 8 additions & 2 deletions orchagent/aclorch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,10 @@ class AclTable {
std::map<sai_object_id_t, sai_object_id_t> ports;
// Map rule name to rule data
map<string, shared_ptr<AclRule>> rules;
// Set to store the ACL table port alias
set<string> portSet;
// Set to store the not cofigured ACL table port alias
set<string> pendingPortSet;

AclTable()
: type(ACL_TABLE_UNKNOWN)
Expand Down Expand Up @@ -294,7 +298,7 @@ inline void split(string str, Iterable& out, char delim = ' ')
class AclOrch : public Orch, public Observer
{
public:
AclOrch(DBConnector *db, vector<string> tableNames, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch);
AclOrch(vector<TableConnector>& connectors, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch);
~AclOrch();
void update(SubjectType, void *);

Expand All @@ -319,6 +323,7 @@ class AclOrch : public Orch, public Observer
void doTask(Consumer &consumer);
void doAclTableTask(Consumer &consumer);
void doAclRuleTask(Consumer &consumer);
void doAclTablePortUpdateTask(Consumer &consumer);
void doTask(SelectableTimer &timer);

static void collectCountersThread(AclOrch *pAclOrch);
Expand All @@ -329,7 +334,8 @@ class AclOrch : public Orch, public Observer

bool processAclTableType(string type, acl_table_type_t &table_type);
bool processAclTableStage(string stage, acl_stage_type_t &acl_stage);
bool processPorts(string portsList, std::function<void (sai_object_id_t)> inserter);
bool processPorts(AclTable &aclTable, string portsList, std::function<void (sai_object_id_t)> inserter);
bool processPendingPort(AclTable &aclTable, string portAlias, std::function<void (sai_object_id_t)> inserter);
bool validateAclTable(AclTable &aclTable);

//vector <AclTable> m_AclTables;
Expand Down
10 changes: 7 additions & 3 deletions orchagent/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,13 +247,15 @@ int main(int argc, char **argv)
SWSS_LOG_NOTICE("Created underlay router interface ID %lx", gUnderlayIfId);

/* Initialize orchestration components */
DBConnector *appl_db = new DBConnector(APPL_DB, DBConnector::DEFAULT_UNIXSOCKET, 0);
DBConnector *config_db = new DBConnector(CONFIG_DB, DBConnector::DEFAULT_UNIXSOCKET, 0);
DBConnector appl_db(APPL_DB, DBConnector::DEFAULT_UNIXSOCKET, 0);
DBConnector config_db(CONFIG_DB, DBConnector::DEFAULT_UNIXSOCKET, 0);
DBConnector state_db(STATE_DB, DBConnector::DEFAULT_UNIXSOCKET, 0);

OrchDaemon *orchDaemon = new OrchDaemon(appl_db, config_db);
OrchDaemon *orchDaemon = new OrchDaemon(&appl_db, &config_db, &state_db);
if (!orchDaemon->init())
{
SWSS_LOG_ERROR("Failed to initialize orchstration daemon");
delete orchDaemon;
exit(EXIT_FAILURE);
}

Expand All @@ -268,6 +270,7 @@ int main(int argc, char **argv)
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to notify syncd APPLY_VIEW %d", status);
delete orchDaemon;
exit(EXIT_FAILURE);
}

Expand All @@ -282,5 +285,6 @@ int main(int argc, char **argv)
SWSS_LOG_ERROR("Failed due to exception: %s", e.what());
}

delete orchDaemon;
return 0;
}
2 changes: 1 addition & 1 deletion orchagent/orch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ bool Orch::parseIndexRange(const string &input, sai_uint32_t &range_low, sai_uin

void Orch::addConsumer(DBConnector *db, string tableName, int pri)
{
if (db->getDbId() == CONFIG_DB)
if (db->getDbId() == CONFIG_DB || db->getDbId() == STATE_DB)
{
addExecutor(tableName, new Consumer(new SubscriberStateTable(db, tableName, TableConsumable::DEFAULT_POP_BATCH_SIZE, pri), this));
}
Expand Down
22 changes: 13 additions & 9 deletions orchagent/orchdaemon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ RouteOrch *gRouteOrch;
AclOrch *gAclOrch;
CrmOrch *gCrmOrch;

OrchDaemon::OrchDaemon(DBConnector *applDb, DBConnector *configDb) :
OrchDaemon::OrchDaemon(DBConnector *applDb, DBConnector *configDb, DBConnector *stateDb) :
m_applDb(applDb),
m_configDb(configDb)
m_configDb(configDb),
m_stateDb(stateDb)
{
SWSS_LOG_ENTER();
}
Expand All @@ -39,9 +40,6 @@ OrchDaemon::~OrchDaemon()
SWSS_LOG_ENTER();
for (Orch *o : m_orchList)
delete(o);

delete(m_configDb);
delete(m_applDb);
}

bool OrchDaemon::init()
Expand Down Expand Up @@ -99,11 +97,17 @@ bool OrchDaemon::init()
MirrorOrch *mirror_orch = new MirrorOrch(appDbMirrorSession, confDbMirrorSession, gPortsOrch, gRouteOrch, gNeighOrch, gFdbOrch);
VRFOrch *vrf_orch = new VRFOrch(m_configDb, CFG_VRF_TABLE_NAME);

vector<string> acl_tables = {
CFG_ACL_TABLE_NAME,
CFG_ACL_RULE_TABLE_NAME
TableConnector confDbAclTable(m_configDb, CFG_ACL_TABLE_NAME);
TableConnector confDbAclRuleTable(m_configDb, CFG_ACL_RULE_TABLE_NAME);
TableConnector stateDbLagTable(m_stateDb, STATE_LAG_TABLE_NAME);

vector<TableConnector> acl_table_connectors = {
confDbAclTable,
confDbAclRuleTable,
stateDbLagTable
};
gAclOrch = new AclOrch(m_configDb, acl_tables, gPortsOrch, mirror_orch, gNeighOrch, gRouteOrch);

gAclOrch = new AclOrch(acl_table_connectors, gPortsOrch, mirror_orch, gNeighOrch, gRouteOrch);

m_orchList = { switch_orch, gCrmOrch, gPortsOrch, intfs_orch, gNeighOrch, gRouteOrch, copp_orch, tunnel_decap_orch, qos_orch, buffer_orch, mirror_orch, gAclOrch, gFdbOrch, vrf_orch };
m_select = new Select();
Expand Down
3 changes: 2 additions & 1 deletion orchagent/orchdaemon.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,15 @@ using namespace swss;
class OrchDaemon
{
public:
OrchDaemon(DBConnector *, DBConnector *);
OrchDaemon(DBConnector *, DBConnector *, DBConnector *);
~OrchDaemon();

bool init();
void start();
private:
DBConnector *m_applDb;
DBConnector *m_configDb;
DBConnector *m_stateDb;

std::vector<Orch *> m_orchList;
Select *m_select;
Expand Down
Loading

0 comments on commit 4df9c28

Please sign in to comment.