From 283a229c30fd54f6ddacf6d20304f765850a55bb Mon Sep 17 00:00:00 2001 From: Vasant Patil <36455926+vasant17@users.noreply.github.com> Date: Fri, 22 Nov 2019 09:46:31 -0800 Subject: [PATCH] DPB ACL dependency implementation and test cases (#11) --- orchagent/aclorch.cpp | 272 ++++++++++++++++++------ orchagent/aclorch.h | 14 +- orchagent/pfcactionhandler.cpp | 2 +- orchagent/pfcwdorch.cpp | 7 +- orchagent/port.h | 25 +++ orchagent/portsorch.cpp | 369 +++++++++++++++++++++++---------- orchagent/portsorch.h | 24 ++- portsyncd/linksync.cpp | 4 +- tests/conftest.py | 108 ++++++++++ tests/port_dpb.py | 7 +- tests/test_acl_portchannel.py | 6 +- tests/test_port_dpb_acl.py | 169 +++++++++++++++ 12 files changed, 813 insertions(+), 194 deletions(-) create mode 100644 tests/test_port_dpb_acl.py diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 83cfcdd510..4598e4df8b 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -1072,7 +1072,7 @@ bool AclRuleMirror::validateAddAction(string attr_name, string attr_value) bool AclRuleMirror::validateAddMatch(string attr_name, string attr_value) { if ((m_tableType == ACL_TABLE_L3 || m_tableType == ACL_TABLE_L3V6) - && attr_name == MATCH_DSCP) + && attr_name == MATCH_DSCP) { SWSS_LOG_ERROR("DSCP match is not supported for the table of type L3"); return false; @@ -1274,9 +1274,6 @@ bool AclTable::validate() if (type == ACL_TABLE_UNKNOWN || stage == ACL_STAGE_UNKNOWN) return false; - if (portSet.empty() && pendingPortSet.empty()) - return false; - return true; } @@ -1310,7 +1307,7 @@ bool AclTable::create() table_attrs.push_back(attr); attr.id = SAI_ACL_TABLE_ATTR_ACL_STAGE; - attr.value.s32 = stage == ACL_STAGE_INGRESS ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS; + attr.value.s32 = (stage == ACL_STAGE_INGRESS) ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS; table_attrs.push_back(attr); sai_status_t status = sai_acl_api->create_acl_table(&m_oid, gSwitchId, (uint32_t)table_attrs.size(), table_attrs.data()); @@ -1330,8 +1327,8 @@ bool AclTable::create() table_attrs.push_back(attr); attr.id = SAI_ACL_TABLE_ATTR_ACL_STAGE; - attr.value.s32 = stage == ACL_STAGE_INGRESS - ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS; + attr.value.s32 = (stage == ACL_STAGE_INGRESS) ? + SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS; table_attrs.push_back(attr); sai_status_t status = sai_acl_api->create_acl_table( @@ -1527,37 +1524,46 @@ void AclTable::update(SubjectType type, void *cntx) PortUpdate *update = static_cast(cntx); Port &port = update->port; + sai_object_id_t bind_port_id; + if (!gPortsOrch->getAclBindPortId(port.m_alias, bind_port_id)) + { + SWSS_LOG_ERROR("Failed to get port %s bind port ID", + port.m_alias.c_str()); + return; + } + if (update->add) { if (pendingPortSet.find(port.m_alias) != pendingPortSet.end()) { - sai_object_id_t bind_port_id; - if (gPortsOrch->getAclBindPortId(port.m_alias, bind_port_id)) - { - link(bind_port_id); - bind(bind_port_id); + link(bind_port_id); + bind(bind_port_id); - pendingPortSet.erase(port.m_alias); - portSet.emplace(port.m_alias); + pendingPortSet.erase(port.m_alias); + portSet.emplace(port.m_alias); - SWSS_LOG_NOTICE("Bound port %s to ACL table %s", - port.m_alias.c_str(), id.c_str()); - } - else - { - SWSS_LOG_ERROR("Failed to get port %s bind port ID", - port.m_alias.c_str()); - return; - } + SWSS_LOG_NOTICE("Bound port %s to ACL table %s", + port.m_alias.c_str(), id.c_str()); } } else { // TODO: deal with port removal scenario + if (portSet.find(port.m_alias) != portSet.end()) + { + unbind(bind_port_id); + unlink(bind_port_id); + + portSet.erase(port.m_alias); + pendingPortSet.emplace(port.m_alias); + + SWSS_LOG_NOTICE("Unbound port %s from ACL table %s", + port.m_alias.c_str(), id.c_str()); + } } + } -// TODO: make bind/unbind symmetric bool AclTable::bind(sai_object_id_t portOid) { SWSS_LOG_ENTER(); @@ -1567,11 +1573,12 @@ bool AclTable::bind(sai_object_id_t portOid) sai_object_id_t group_member_oid; if (!gPortsOrch->bindAclTable(portOid, m_oid, group_member_oid, stage)) { + SWSS_LOG_NOTICE("Failed to bind port oid: %" PRIx64 "", portOid); return false; } - + SWSS_LOG_NOTICE("Successfully bound port oid: %" PRIx64", group member oid:%" PRIx64 "", + portOid, group_member_oid); ports[portOid] = group_member_oid; - return true; } @@ -1579,13 +1586,16 @@ bool AclTable::unbind(sai_object_id_t portOid) { SWSS_LOG_ENTER(); - sai_object_id_t member = ports[portOid]; - sai_status_t status = sai_acl_api->remove_acl_table_group_member(member); - if (status != SAI_STATUS_SUCCESS) { - SWSS_LOG_ERROR("Failed to unbind table %" PRIu64 " as member %" PRIu64 " from ACL table: %d", - m_oid, member, status); + assert(ports.find(portOid) != ports.end()); + + sai_object_id_t group_member_oid = ports[portOid]; + if (!gPortsOrch->unbindAclTable(portOid, m_oid, group_member_oid, stage)) + { return false; } + SWSS_LOG_NOTICE("%" PRIx64" port is unbound from %s ACL table", + portOid, id.c_str()); + ports[portOid] = SAI_NULL_OBJECT_ID; return true; } @@ -1622,6 +1632,13 @@ void AclTable::link(sai_object_id_t portOid) ports.emplace(portOid, SAI_NULL_OBJECT_ID); } +void AclTable::unlink(sai_object_id_t portOid) +{ + SWSS_LOG_ENTER(); + + ports.erase(portOid); +} + bool AclTable::add(shared_ptr newRule) { SWSS_LOG_ENTER(); @@ -2521,10 +2538,123 @@ void AclOrch::doTask(Consumer &consumer) } } -bool AclOrch::addAclTable(AclTable &newTable, string table_id) +void AclOrch::getAddDeletePorts(AclTable &newT, + AclTable &curT, + set &addSet, + set &delSet) { + set newPortSet, curPortSet; + + // Collect new ports + for (auto p : newT.pendingPortSet) + { + newPortSet.insert(p); + } + for (auto p : newT.portSet) + { + newPortSet.insert(p); + } + + // Collect current ports + for (auto p : curT.pendingPortSet) + { + curPortSet.insert(p); + } + for (auto p : curT.portSet) + { + curPortSet.insert(p); + } + + // Get the difference newPortSet-curPortSet and curPortSet-newPortsSet + std::set_difference(newPortSet.begin(), newPortSet.end(), + curPortSet.begin(), curPortSet.end(), + std::inserter(addSet, addSet.end())); + std::set_difference(curPortSet.begin(), curPortSet.end(), + newPortSet.begin(), newPortSet.end(), + std::inserter(delSet, delSet.end())); + +} + +bool AclOrch::updateAclTablePorts(AclTable &newTable, AclTable &curTable) +{ + sai_object_id_t port_oid = SAI_NULL_OBJECT_ID; + set addPortSet, deletePortSet; + SWSS_LOG_ENTER(); + getAddDeletePorts(newTable, curTable, addPortSet, deletePortSet); + // Lets first unbind and unlink ports to be removed + for (auto p : deletePortSet) + { + SWSS_LOG_NOTICE("Deleting port %s from ACL list %s", + p.c_str(), curTable.id.c_str()); + if (curTable.pendingPortSet.find(p) != curTable.pendingPortSet.end()) + { + SWSS_LOG_NOTICE("Removed:%s from pendingPortSet", p.c_str()); + curTable.pendingPortSet.erase(p); + } else if (curTable.portSet.find(p) != curTable.portSet.end()) + { + gPortsOrch->getAclBindPortId(p, port_oid); + assert(port_oid != SAI_NULL_OBJECT_ID); + assert(curTable.ports.find(port_oid) != curTable.ports.end()); + if (curTable.ports[port_oid] != SAI_NULL_OBJECT_ID) + { + // Unbind and unlink + SWSS_LOG_NOTICE("Unbind and Unlink:%s", p.c_str()); + curTable.unbind(port_oid); + curTable.unlink(port_oid); + } + SWSS_LOG_NOTICE("Removed:%s from portSet", p.c_str()); + curTable.portSet.erase(p); + } + } + + // Now link and bind ports to be added + for (auto p : addPortSet) + { + SWSS_LOG_NOTICE("Adding port %s to ACL list %s", + p.c_str(), curTable.id.c_str()); + Port port; + if (!gPortsOrch->getPort(p, port)) + { + curTable.pendingPortSet.emplace(p); + continue; + } + + if (!gPortsOrch->getAclBindPortId(p, port_oid)) + { + throw runtime_error("updateAclTablePorts: Couldn't find portOID"); + } + + curTable.portSet.emplace(p); + + // Link and bind + SWSS_LOG_NOTICE("Link and Bind:%s", p.c_str()); + curTable.link(port_oid); + curTable.bind(port_oid); + } + return true; +} + +bool AclOrch::updateAclTable(AclTable ¤tTable, AclTable &newTable) +{ + SWSS_LOG_ENTER(); + + currentTable.description = newTable.description; + if (!updateAclTablePorts(newTable, currentTable)) + { + SWSS_LOG_ERROR("Failed to update ACL table port list"); + return false; + } + + return true; +} + +bool AclOrch::addAclTable(AclTable &newTable) +{ + SWSS_LOG_ENTER(); + + string table_id = newTable.id; if (newTable.type == ACL_TABLE_CTRLPLANE) { m_ctrlAclTables.emplace(table_id, newTable); @@ -2745,11 +2875,10 @@ void AclOrch::doAclTableTask(Consumer &consumer) AclTable newTable(this); bool bAllAttributesOk = true; + newTable.id = table_id; // Scan all attributes for (auto itp : kfvFieldsValues(t)) { - newTable.id = table_id; - string attr_name = to_upper(fvField(itp)); string attr_value = fvValue(itp); @@ -2802,13 +2931,39 @@ void AclOrch::doAclTableTask(Consumer &consumer) } } - // validate and create ACL Table + // validate and create/update ACL Table if (bAllAttributesOk && newTable.validate()) { - if (addAclTable(newTable, table_id)) - it = consumer.m_toSync.erase(it); - else - it++; + // If the the table already exists and meets the below condition(s) + // update the table. Otherwise delete and re-create + // Condition 1: Table's TYPE and STAGE hasn't changed + + sai_object_id_t table_oid = getTableById(table_id); + if (table_oid != SAI_NULL_OBJECT_ID && + !isAclTableTypeUpdated(newTable.type, + m_AclTables[table_oid]) && + !isAclTableStageUpdated(newTable.stage, + m_AclTables[table_oid])) + { + // Update the table existing table using the info in newTable + if (updateAclTable(m_AclTables[table_oid], newTable)) + { + SWSS_LOG_NOTICE("Successfully updated existing ACL table %s", + table_id.c_str()); + it = consumer.m_toSync.erase(it); + } else + { + SWSS_LOG_ERROR("Failed to update existing ACL table %s", + table_id.c_str()); + it++; + } + } else + { + if (addAclTable(newTable)) + it = consumer.m_toSync.erase(it); + else + it++; + } } else { @@ -2953,13 +3108,6 @@ bool AclOrch::processAclTablePorts(string portList, AclTable &aclTable) auto port_list = tokenize(portList, ','); set ports(port_list.begin(), port_list.end()); - // TODO: Support adding ports afterwards - if (ports.empty()) - { - SWSS_LOG_ERROR("Failed to process empty port list"); - return false; - } - for (auto alias : ports) { Port port; @@ -2976,7 +3124,7 @@ bool AclOrch::processAclTablePorts(string portList, AclTable &aclTable) { SWSS_LOG_ERROR("Failed to get port %s bind port ID for ACL table %s", alias.c_str(), aclTable.id.c_str()); - continue; + return false; } aclTable.link(bind_port_id); @@ -2986,6 +3134,10 @@ bool AclOrch::processAclTablePorts(string portList, AclTable &aclTable) return true; } +bool AclOrch::isAclTableTypeUpdated(acl_table_type_t table_type, AclTable &t) +{ + return (table_type != t.type); +} bool AclOrch::processAclTableType(string type, acl_table_type_t &table_type) { SWSS_LOG_ENTER(); @@ -3013,6 +3165,10 @@ bool AclOrch::processAclTableType(string type, acl_table_type_t &table_type) return true; } +bool AclOrch::isAclTableStageUpdated(acl_stage_type_t acl_stage, AclTable &t) +{ + return (acl_stage != t.stage); +} bool AclOrch::processAclTableStage(string stage, acl_stage_type_t &acl_stage) { SWSS_LOG_ENTER(); @@ -3085,7 +3241,7 @@ bool AclOrch::createBindAclTable(AclTable &aclTable, sai_object_id_t &table_oid) if (!suc) return false; table_oid = aclTable.getOid(); - sai_status_t status = bindAclTable(table_oid, aclTable); + sai_status_t status = bindAclTable(aclTable); if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to bind table %s to ports", @@ -3100,7 +3256,7 @@ sai_status_t AclOrch::deleteUnbindAclTable(sai_object_id_t table_oid) SWSS_LOG_ENTER(); sai_status_t status; - if ((status = bindAclTable(table_oid, m_AclTables[table_oid], false)) != SAI_STATUS_SUCCESS) + if ((status = bindAclTable(m_AclTables[table_oid], false)) != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to unbind table %s", m_AclTables[table_oid].id.c_str()); @@ -3134,31 +3290,21 @@ void AclOrch::doTask(SelectableTimer &timer) } } -sai_status_t AclOrch::bindAclTable(sai_object_id_t table_oid, AclTable &aclTable, bool bind) +sai_status_t AclOrch::bindAclTable(AclTable &aclTable, bool bind) { SWSS_LOG_ENTER(); sai_status_t status = SAI_STATUS_SUCCESS; - SWSS_LOG_INFO("%s table %s to ports", bind ? "Bind" : "Unbind", aclTable.id.c_str()); + SWSS_LOG_NOTICE("%s table %s to ports", bind ? "Bind" : "Unbind", aclTable.id.c_str()); if (aclTable.ports.empty()) { - if (bind) - { - SWSS_LOG_WARN("Binding port list is empty for %s table", aclTable.id.c_str()); - } + SWSS_LOG_WARN("Port list is empty for %s table", aclTable.id.c_str()); return SAI_STATUS_SUCCESS; } - if (bind) - { - aclTable.bind(); - } - else - { - aclTable.unbind(); - } + bind ? aclTable.bind() : aclTable.unbind(); return status; } diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index 202d369c87..ecc860a024 100644 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -374,6 +374,8 @@ class AclTable { bool unbind(); // Link the ACL table with a port, for future bind or unbind void link(sai_object_id_t portOid); + // Unlink the ACL table from a port after unbind + void unlink(sai_object_id_t portOid); // Add or overwrite a rule into the ACL table bool add(shared_ptr newRule); // Remove a rule from the ACL table @@ -408,8 +410,9 @@ class AclOrch : public Orch, public Observer RouteOrch *m_routeOrch; DTelOrch *m_dTelOrch; - bool addAclTable(AclTable &aclTable, string table_id); + bool addAclTable(AclTable &aclTable); bool removeAclTable(string table_id); + bool updateAclTable(AclTable ¤tTable, AclTable &newTable); bool addAclRule(shared_ptr aclRule, string table_id); bool removeAclRule(string table_id, string rule_id); @@ -442,13 +445,20 @@ class AclOrch : public Orch, public Observer static void collectCountersThread(AclOrch *pAclOrch); bool createBindAclTable(AclTable &aclTable, sai_object_id_t &table_oid); - sai_status_t bindAclTable(sai_object_id_t table_oid, AclTable &aclTable, bool bind = true); + sai_status_t bindAclTable(AclTable &aclTable, bool bind = true); sai_status_t deleteUnbindAclTable(sai_object_id_t table_oid); + bool isAclTableTypeUpdated(acl_table_type_t table_type, AclTable &aclTable); bool processAclTableType(string type, acl_table_type_t &table_type); + bool isAclTableStageUpdated(acl_stage_type_t acl_stage, AclTable &aclTable); bool processAclTableStage(string stage, acl_stage_type_t &acl_stage); bool processAclTablePorts(string portList, AclTable &aclTable); bool validateAclTable(AclTable &aclTable); + bool updateAclTablePorts(AclTable &newTable, AclTable &curTable); + void getAddDeletePorts(AclTable &newT, + AclTable &curT, + set &addSet, + set &delSet); sai_status_t createDTelWatchListTables(); sai_status_t deleteDTelWatchListTables(); diff --git a/orchagent/pfcactionhandler.cpp b/orchagent/pfcactionhandler.cpp index 153016c729..934007058f 100644 --- a/orchagent/pfcactionhandler.cpp +++ b/orchagent/pfcactionhandler.cpp @@ -299,7 +299,7 @@ void PfcWdAclHandler::createPfcAclTable(sai_object_id_t port, string strTable, b aclTable.link(port); aclTable.id = strTable; aclTable.stage = ingress ? ACL_STAGE_INGRESS : ACL_STAGE_EGRESS; - gAclOrch->addAclTable(aclTable, strTable); + gAclOrch->addAclTable(aclTable); } void PfcWdAclHandler::createPfcAclRule(shared_ptr rule, uint8_t queueId, string strTable) diff --git a/orchagent/pfcwdorch.cpp b/orchagent/pfcwdorch.cpp index d9de320527..53625966c0 100644 --- a/orchagent/pfcwdorch.cpp +++ b/orchagent/pfcwdorch.cpp @@ -560,11 +560,8 @@ bool PfcWdSwOrch::registerInWdDb(const Port& port, sai_serialize_object_id(queueId)); } - // Create egress ACL table group for each port of pfcwd's interest - sai_object_id_t groupId; - gPortsOrch->createBindAclTableGroup(port.m_port_id, groupId, ACL_STAGE_INGRESS); - gPortsOrch->createBindAclTableGroup(port.m_port_id, groupId, ACL_STAGE_EGRESS); - + // We do NOT need to create ACL table group here. It will be + // done when ACL tables are bound to ports return true; } diff --git a/orchagent/port.h b/orchagent/port.h index d5b8827c51..9249c27654 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -9,6 +9,7 @@ extern "C" { #include #include #include +#include #define DEFAULT_PORT_VLAN_ID 1 /* @@ -48,6 +49,14 @@ class Port UNKNOWN } ; + enum Dependency { + ACL_DEP, + FDB_DEP, + INTF_DEP, + LAG_DEP, + VLAN_DEP + }; + Port() {}; Port(std::string alias, Type type) : m_alias(alias), m_type(type) {}; @@ -89,6 +98,7 @@ class Port sai_object_id_t m_egress_acl_table_group_id = 0; vlan_members_t m_vlan_members; sai_object_id_t m_parent_port_id = 0; + uint32_t m_dependency_bitmap = 0; sai_port_oper_status_t m_oper_status = SAI_PORT_OPER_STATUS_UNKNOWN; std::set m_members; std::set m_child_ports; @@ -108,6 +118,21 @@ class Port std::vector m_queue_lock; std::vector m_priority_group_lock; + std::unordered_set m_ingress_acl_tables_uset; + std::unordered_set m_egress_acl_tables_uset; + + inline void set_dependency(Dependency dep) + { + m_dependency_bitmap |= (1 << dep); + } + inline void clear_dependency(Dependency dep) + { + m_dependency_bitmap &= ~(1 << dep); + } + inline bool has_dependency() + { + return (m_dependency_bitmap != 0); + } }; } diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 73f3203d51..110a854fa9 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -869,62 +869,200 @@ bool PortsOrch::setPortPfcAsym(Port &port, string pfc_asym) return true; } -bool PortsOrch::createBindAclTableGroup(sai_object_id_t id, sai_object_id_t &group_oid, acl_stage_type_t acl_stage) +/* + * Name: bindUnbindAclTableGroup + * + * Description: + * To bind a port to ACL table we need to do two things. + * 1. Create ACL table member, which maps + * ACL table group OID --> ACL table OID + * 2. Set ACL table group OID as value port attribute. + * + * This function performs the second step of binding. + * + * Also, while unbinding we use this function to + * set port attribute value to SAI_NULL_OBJECT_ID + * + * Port attribute name is derived from port type + * + * Return: true on success, false on failure + */ +bool PortsOrch::bindUnbindAclTableGroup(Port &port, + bool ingress, + bool bind) +{ + + sai_attribute_t attr; + sai_status_t status = SAI_STATUS_SUCCESS; + string bind_str = bind ? "bind" : "unbind"; + + attr.value.oid = bind ? (ingress ? port.m_ingress_acl_table_group_id : + port.m_egress_acl_table_group_id): + SAI_NULL_OBJECT_ID; + switch (port.m_type) + { + case Port::PHY: + { + attr.id = ingress ? + SAI_PORT_ATTR_INGRESS_ACL : SAI_PORT_ATTR_EGRESS_ACL; + status = sai_port_api->set_port_attribute(port.m_port_id, &attr); + break; + } + case Port::LAG: + { + attr.id = ingress ? + SAI_LAG_ATTR_INGRESS_ACL : SAI_LAG_ATTR_EGRESS_ACL; + status = sai_lag_api->set_lag_attribute(port.m_lag_id, &attr); + break; + } + case Port::VLAN: + { + attr.id = ingress ? + SAI_VLAN_ATTR_INGRESS_ACL : SAI_VLAN_ATTR_EGRESS_ACL; + status = + sai_vlan_api->set_vlan_attribute(port.m_vlan_info.vlan_oid, + &attr); + break; + } + default: + { + SWSS_LOG_ERROR("Failed to %s %s port with type %d", + bind_str.c_str(), port.m_alias.c_str(), port.m_type); + return false; + } + } + + if (SAI_STATUS_SUCCESS != status) + { + SWSS_LOG_ERROR("Failed to %s %s to ACL table group %" PRIx64 ", rv:%d", + bind_str.c_str(), port.m_alias.c_str(), attr.value.oid, status); + return false; + } + + return true; +} + +bool PortsOrch::unbindRemoveAclTableGroup(sai_object_id_t port_oid, + sai_object_id_t acl_table_oid, + acl_stage_type_t acl_stage) { SWSS_LOG_ENTER(); - if (acl_stage == ACL_STAGE_UNKNOWN) + sai_status_t status; + bool ingress = (acl_stage == ACL_STAGE_INGRESS); + Port port; + + if (!getPort(port_oid, port)) { - SWSS_LOG_ERROR("unknown ACL stage for table group creation"); + SWSS_LOG_ERROR("Failed to get port by port OID %" PRIx64, port_oid); return false; } - Port port; - if (!getPort(id, port)) + + sai_object_id_t &group_oid_ref = + ingress? port.m_ingress_acl_table_group_id : + port.m_egress_acl_table_group_id; + unordered_set &acl_list_ref = + ingress ? port.m_ingress_acl_tables_uset : + port.m_egress_acl_tables_uset; + + if (SAI_NULL_OBJECT_ID == group_oid_ref) + { + assert(acl_list_ref.find(acl_table_oid) == acl_list_ref.end()); + return true; + } + assert(acl_list_ref.find(acl_table_oid) != acl_list_ref.end()); + acl_list_ref.erase(acl_table_oid); + if (!acl_list_ref.empty()) + { + // This port is in more than one acl table's port list + // So, we need to preserve group OID + SWSS_LOG_NOTICE("Preserving port OID %" PRIx64" ACL table grop ID", port_oid); + setPort(port.m_alias, port); + return true; + } + + port.clear_dependency(Port::ACL_DEP); + SWSS_LOG_NOTICE("Removing port OID %" PRIx64" ACL table grop ID", port_oid); + + // Unbind ACL group + if (!bindUnbindAclTableGroup(port, ingress, false)) { - SWSS_LOG_ERROR("Failed to get port by port ID %" PRIx64, id); return false; } - sai_status_t status; - if ((acl_stage == ACL_STAGE_INGRESS) && (port.m_ingress_acl_table_group_id != 0)) + // Remove ACL group + status = sai_acl_api->remove_acl_table_group(group_oid_ref); + if (SAI_STATUS_SUCCESS != status) { - group_oid = port.m_ingress_acl_table_group_id; + SWSS_LOG_ERROR("Failed to remove ACL table group, rv:%d", status); + return false; } - else if ((acl_stage == ACL_STAGE_EGRESS) && (port.m_egress_acl_table_group_id != 0)) + sai_acl_bind_point_type_t bind_type; + if (!getSaiAclBindPointType(port.m_type, bind_type)) { - group_oid = port.m_egress_acl_table_group_id; + SWSS_LOG_ERROR("Unknown SAI ACL bind point type"); + return false; } - // Port ACL table group does not exist, create one - else if (acl_stage == ACL_STAGE_INGRESS or acl_stage == ACL_STAGE_EGRESS) + gCrmOrch->decCrmAclUsedCounter(CrmResourceType::CRM_ACL_GROUP, + ingress ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS, + bind_type, group_oid_ref); + + group_oid_ref = SAI_NULL_OBJECT_ID; + setPort(port.m_alias, port); + return true; +} + +bool PortsOrch::createBindAclTableGroup(sai_object_id_t port_oid, + sai_object_id_t acl_table_oid, + sai_object_id_t &group_oid, + acl_stage_type_t acl_stage) +{ + SWSS_LOG_ENTER(); + + if (ACL_STAGE_UNKNOWN == acl_stage) { - bool ingress = acl_stage == ACL_STAGE_INGRESS ? true : false; + SWSS_LOG_ERROR("unknown ACL stage for table group creation"); + return false; + } + assert(ACL_STAGE_INGRESS == acl_stage || ACL_STAGE_EGRESS == acl_stage); + + sai_status_t status; + Port port; + bool ingress = (ACL_STAGE_INGRESS == acl_stage) ? + true : false; + if (!getPort(port_oid, port)) + { + SWSS_LOG_ERROR("Failed to get port by port ID %" PRIx64, port_oid); + return false; + } + unordered_set &acl_list_ref = + ingress ? port.m_ingress_acl_tables_uset : + port.m_egress_acl_tables_uset; + sai_object_id_t &group_oid_ref = + ingress ? port.m_ingress_acl_table_group_id : + port.m_egress_acl_table_group_id; + + if (acl_list_ref.empty()) + { + // Port ACL table group does not exist, create one + assert(group_oid_ref == SAI_NULL_OBJECT_ID); sai_acl_bind_point_type_t bind_type; - switch (port.m_type) + if (!getSaiAclBindPointType(port.m_type, bind_type)) { - case Port::PHY: - bind_type = SAI_ACL_BIND_POINT_TYPE_PORT; - break; - case Port::LAG: - bind_type = SAI_ACL_BIND_POINT_TYPE_LAG; - break; - case Port::VLAN: - bind_type = SAI_ACL_BIND_POINT_TYPE_VLAN; - break; - default: - SWSS_LOG_ERROR("Failed to bind ACL table to port %s with unknown type %d", + SWSS_LOG_ERROR("Failed to bind ACL table to port %s with unknown type %d", port.m_alias.c_str(), port.m_type); - return false; + return false; } - sai_object_id_t bp_list[] = { bind_type }; vector group_attrs; sai_attribute_t group_attr; group_attr.id = SAI_ACL_TABLE_GROUP_ATTR_ACL_STAGE; - group_attr.value.s32 = ingress ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS; + group_attr.value.s32 = ingress ? SAI_ACL_STAGE_INGRESS : + SAI_ACL_STAGE_EGRESS; group_attrs.push_back(group_attr); group_attr.id = SAI_ACL_TABLE_GROUP_ATTR_ACL_BIND_POINT_TYPE_LIST; @@ -936,92 +1074,74 @@ bool PortsOrch::createBindAclTableGroup(sai_object_id_t id, sai_object_id_t &gro group_attr.value.s32 = SAI_ACL_TABLE_GROUP_TYPE_PARALLEL; group_attrs.push_back(group_attr); - status = sai_acl_api->create_acl_table_group(&group_oid, gSwitchId, (uint32_t)group_attrs.size(), group_attrs.data()); + status = sai_acl_api->create_acl_table_group(&group_oid_ref, gSwitchId, + (uint32_t)group_attrs.size(), group_attrs.data()); if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to create ACL table group, rv:%d", status); return false; } + assert(group_oid_ref != SAI_NULL_OBJECT_ID); - if (ingress) - { - port.m_ingress_acl_table_group_id = group_oid; - } - else + gCrmOrch->incCrmAclUsedCounter(CrmResourceType::CRM_ACL_GROUP, + ingress ? SAI_ACL_STAGE_INGRESS : + SAI_ACL_STAGE_EGRESS, bind_type); + + // Bind ACL table group + if (!bindUnbindAclTableGroup(port, ingress, true)) { - port.m_egress_acl_table_group_id = group_oid; + return false; } - setPort(port.m_alias, port); + port.set_dependency(Port::ACL_DEP); - gCrmOrch->incCrmAclUsedCounter(CrmResourceType::CRM_ACL_GROUP, ingress ? SAI_ACL_STAGE_INGRESS : SAI_ACL_STAGE_EGRESS, bind_type); + SWSS_LOG_NOTICE("Create %s ACL table group and bind port %s to it", + ingress ? "ingress" : "egress", port.m_alias.c_str()); + } - switch (port.m_type) - { - case Port::PHY: - { - // Bind this ACL group to physical port - sai_attribute_t port_attr; - port_attr.id = ingress ? SAI_PORT_ATTR_INGRESS_ACL : SAI_PORT_ATTR_EGRESS_ACL; - port_attr.value.oid = group_oid; + assert(group_oid_ref != SAI_NULL_OBJECT_ID); + group_oid = group_oid_ref; + acl_list_ref.insert(acl_table_oid); + setPort(port.m_alias, port); - status = sai_port_api->set_port_attribute(port.m_port_id, &port_attr); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to bind port %s to ACL table group %" PRIx64 ", rv:%d", - port.m_alias.c_str(), group_oid, status); - return false; - } - break; - } - case Port::LAG: - { - // Bind this ACL group to LAG - sai_attribute_t lag_attr; - lag_attr.id = ingress ? SAI_LAG_ATTR_INGRESS_ACL : SAI_LAG_ATTR_EGRESS_ACL; - lag_attr.value.oid = group_oid; - - status = sai_lag_api->set_lag_attribute(port.m_lag_id, &lag_attr); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to bind LAG %s to ACL table group %" PRIx64 ", rv:%d", - port.m_alias.c_str(), group_oid, status); - return false; - } - break; - } - case Port::VLAN: - { - // Bind this ACL group to VLAN - sai_attribute_t vlan_attr; - vlan_attr.id = ingress ? SAI_VLAN_ATTR_INGRESS_ACL : SAI_VLAN_ATTR_EGRESS_ACL; - vlan_attr.value.oid = group_oid; + return true; +} - status = sai_vlan_api->set_vlan_attribute(port.m_vlan_info.vlan_oid, &vlan_attr); - if (status != SAI_STATUS_SUCCESS) - { - SWSS_LOG_ERROR("Failed to bind VLAN %s to ACL table group %" PRIx64 ", rv:%d", - port.m_alias.c_str(), group_oid, status); - return false; - } - break; - } - default: - { - SWSS_LOG_ERROR("Failed to bind %s port with type %d", port.m_alias.c_str(), port.m_type); - return false; - } - } +bool PortsOrch::unbindAclTable(sai_object_id_t port_oid, + sai_object_id_t acl_table_oid, + sai_object_id_t acl_group_member_oid, + acl_stage_type_t acl_stage) +{ - SWSS_LOG_NOTICE("Create %s ACL table group and bind port %s to it", ingress ? "ingress" : "egress", port.m_alias.c_str()); + /* + * Do the following in-order + * 1. Delete ACL table group member + * 2. Unbind ACL table group + * 3. Delete ACL table group + */ + sai_status_t status = + sai_acl_api->remove_acl_table_group_member(acl_group_member_oid); + if (status != SAI_STATUS_SUCCESS) { + SWSS_LOG_ERROR("Failed to remove ACL group member: %" PRIu64 " ", + acl_group_member_oid); + return false; } - + unbindRemoveAclTableGroup(port_oid, acl_table_oid, acl_stage); return true; } -bool PortsOrch::bindAclTable(sai_object_id_t id, sai_object_id_t table_oid, sai_object_id_t &group_member_oid, acl_stage_type_t acl_stage) +bool PortsOrch::bindAclTable(sai_object_id_t port_oid, + sai_object_id_t table_oid, + sai_object_id_t &group_member_oid, + acl_stage_type_t acl_stage) { SWSS_LOG_ENTER(); + /* + * Do the following in-order + * 1. Create ACL table group + * 2. Bind ACL table group (set ACL table group ID on port) + * 3. Create ACL table group member + */ if (table_oid == SAI_NULL_OBJECT_ID) { @@ -1029,22 +1149,22 @@ bool PortsOrch::bindAclTable(sai_object_id_t id, sai_object_id_t table_oid, sai_ return false; } - sai_status_t status; - sai_object_id_t groupOid; + sai_object_id_t group_oid; + sai_status_t status; // Create an ACL table group and bind to port - if (!createBindAclTableGroup(id, groupOid, acl_stage)) + if (!createBindAclTableGroup(port_oid, table_oid, group_oid, acl_stage)) { - SWSS_LOG_ERROR("Fail to create or bind to port %" PRIx64 " ACL table group", id); + SWSS_LOG_ERROR("Fail to create or bind to port %" PRIx64 " ACL table group", port_oid); return false; } - // Create an ACL group member with table_oid and groupOid + // Create an ACL group member with table_oid and group_oid vector member_attrs; sai_attribute_t member_attr; member_attr.id = SAI_ACL_TABLE_GROUP_MEMBER_ATTR_ACL_TABLE_GROUP_ID; - member_attr.value.oid = groupOid; + member_attr.value.oid = group_oid; member_attrs.push_back(member_attr); member_attr.id = SAI_ACL_TABLE_GROUP_MEMBER_ATTR_ACL_TABLE_ID; @@ -1059,7 +1179,7 @@ bool PortsOrch::bindAclTable(sai_object_id_t id, sai_object_id_t table_oid, sai_ if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to create member in ACL table group %" PRIx64 " for ACL table %" PRIx64 ", rv:%d", - groupOid, table_oid, status); + group_oid, table_oid, status); return false; } @@ -2140,6 +2260,13 @@ void PortsOrch::doPortTask(Consumer &consumer) auto port_id = m_portList[alias].m_port_id; auto hif_id = m_portList[alias].m_hif_id; + if (m_portList[alias].has_dependency()) + { + // Port has one or more dependencies, cannot remove + SWSS_LOG_WARN("Cannot to remove port because of dependency"); + continue; + } + deInitPort(alias, port_id); SWSS_LOG_NOTICE("Removing hostif %lx for Port %s", hif_id, alias.c_str()); @@ -2484,6 +2611,13 @@ void PortsOrch::doLagTask(Consumer &consumer) continue; } + if (m_portList[alias].has_dependency()) + { + // LAG has one or more dependencies, cannot remove + SWSS_LOG_WARN("Cannot to remove LAG because of dependency"); + continue; + } + if (removeLag(lag)) it = consumer.m_toSync.erase(it); else @@ -3285,6 +3419,9 @@ bool PortsOrch::removeLag(Port lag) return false; } + PortUpdate update = { lag, false }; + notify(SUBJECT_TYPE_PORT_CHANGE, static_cast(&update)); + sai_status_t status = sai_lag_api->remove_lag(lag.m_lag_id); if (status != SAI_STATUS_SUCCESS) { @@ -3292,8 +3429,6 @@ bool PortsOrch::removeLag(Port lag) return false; } - removeAclTableGroup(lag); - SWSS_LOG_NOTICE("Remove LAG %s lid:%" PRIx64, lag.m_alias.c_str(), lag.m_lag_id); m_portList.erase(lag.m_alias); @@ -3735,24 +3870,36 @@ bool PortsOrch::getPortOperStatus(const Port& port, sai_port_oper_status_t& stat return true; } -bool PortsOrch::removeAclTableGroup(const Port &p) +bool PortsOrch::getSaiAclBindPointType(Port::Type type, + sai_acl_bind_point_type_t &sai_acl_bind_type) { - sai_acl_bind_point_type_t bind_type; - switch (p.m_type) + switch(type) { case Port::PHY: - bind_type = SAI_ACL_BIND_POINT_TYPE_PORT; + sai_acl_bind_type = SAI_ACL_BIND_POINT_TYPE_PORT; break; case Port::LAG: - bind_type = SAI_ACL_BIND_POINT_TYPE_LAG; + sai_acl_bind_type = SAI_ACL_BIND_POINT_TYPE_LAG; break; case Port::VLAN: - bind_type = SAI_ACL_BIND_POINT_TYPE_VLAN; + sai_acl_bind_type = SAI_ACL_BIND_POINT_TYPE_VLAN; break; default: // Dealing with port, lag and vlan for now. - return true; + return false; } + return true; +} + +bool PortsOrch::removeAclTableGroup(const Port &p) +{ + sai_acl_bind_point_type_t bind_type; + if (!getSaiAclBindPointType(p.m_type, bind_type)) + { + SWSS_LOG_ERROR("Unknown SAI ACL bind point type"); + return false; + } + sai_status_t ret; if (p.m_ingress_acl_table_group_id != 0) { diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 8d0840dda8..1cce011652 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -21,7 +21,6 @@ typedef std::vector PortSupportedSpeeds; - static const map oper_status_strings = { { SAI_PORT_OPER_STATUS_UNKNOWN, "unknown" }, @@ -78,9 +77,24 @@ class PortsOrch : public Orch, public Subject bool setHostIntfsOperStatus(const Port& port, bool up) const; void updateDbPortOperStatus(const Port& port, sai_port_oper_status_t status) const; - bool createBindAclTableGroup(sai_object_id_t id, sai_object_id_t &group_oid, acl_stage_type_t acl_stage = ACL_STAGE_EGRESS); - bool bindAclTable(sai_object_id_t id, sai_object_id_t table_oid, sai_object_id_t &group_member_oid, acl_stage_type_t acl_stage = ACL_STAGE_INGRESS); - + bool createBindAclTableGroup(sai_object_id_t port_oid, + sai_object_id_t acl_table_oid, + sai_object_id_t &group_oid, + acl_stage_type_t acl_stage = ACL_STAGE_EGRESS); + bool unbindRemoveAclTableGroup(sai_object_id_t port_oid, + sai_object_id_t acl_table_oid, + acl_stage_type_t acl_stage); + bool bindAclTable(sai_object_id_t id, + sai_object_id_t table_oid, + sai_object_id_t &group_member_oid, + acl_stage_type_t acl_stage = ACL_STAGE_INGRESS); + bool unbindAclTable(sai_object_id_t port_oid, + sai_object_id_t acl_table_oid, + sai_object_id_t acl_group_member_oid, + acl_stage_type_t acl_stage); + bool bindUnbindAclTableGroup(Port &port, + bool ingress, + bool bind); bool getPortPfc(sai_object_id_t portId, uint8_t *pfc_bitmask); bool setPortPfc(sai_object_id_t portId, uint8_t pfc_bitmask); @@ -218,6 +232,8 @@ class PortsOrch : public Orch, public Subject bool setPortSerdesAttribute(sai_object_id_t port_id, sai_attr_id_t attr_id, vector &serdes_val); + bool getSaiAclBindPointType(Port::Type type, + sai_acl_bind_point_type_t &sai_acl_bind_type); }; #endif /* SWSS_PORTSORCH_H */ diff --git a/portsyncd/linksync.cpp b/portsyncd/linksync.cpp index d12410c91c..d8f7930773 100644 --- a/portsyncd/linksync.cpp +++ b/portsyncd/linksync.cpp @@ -185,12 +185,12 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj) if (type) { - SWSS_LOG_INFO("nlmsg type:%d key:%s admin:%d oper:%d addr:%s ifindex:%d master:%d type:%s", + SWSS_LOG_NOTICE("nlmsg type:%d key:%s admin:%d oper:%d addr:%s ifindex:%d master:%d type:%s", nlmsg_type, key.c_str(), admin, oper, addrStr, ifindex, master, type); } else { - SWSS_LOG_INFO("nlmsg type:%d key:%s admin:%d oper:%d addr:%s ifindex:%d master:%d", + SWSS_LOG_NOTICE("nlmsg type:%d key:%s admin:%d oper:%d addr:%s ifindex:%d master:%d", nlmsg_type, key.c_str(), admin, oper, addrStr, ifindex, master); } diff --git a/tests/conftest.py b/tests/conftest.py index 79d6e00471..7232c8d075 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -829,6 +829,114 @@ def setReadOnlyAttr(self, obj, attr, val): ntf.send("set_ro", key, fvp) + def create_acl_table(self, table, type, ports): + tbl = swsscommon.Table(self.cdb, "ACL_TABLE") + fvs = swsscommon.FieldValuePairs([("policy_desc", table), + ("type", type), + ("ports", ",".join(ports))]) + tbl.set(table, fvs) + time.sleep(1) + + def remove_acl_table(self, table): + tbl = swsscommon.Table(self.cdb, "ACL_TABLE") + tbl._del(table) + time.sleep(1) + + def update_acl_table(self, table, fvs): + tbl = swsscommon.Table(self.cdb, "ACL_TABLE") + tbl.set(table, fvs) + time.sleep(1) + + def get_acl_table_ids(self): + tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE") + keys = tbl.getKeys() + + for k in self.asicdb.default_acl_tables: + assert k in keys + + acl_tables = [k for k in keys if k not in self.asicdb.default_acl_tables] + + return acl_tables + + def verify_if_any_acl_table_created(self): + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE") + keys = atbl.getKeys() + for k in dvs.asicdb.default_acl_tables: + assert k in keys + acl_tables = [k for k in keys if k not in dvs.asicdb.default_acl_tables] + + if len(acl_tables) != 0: + return True + + return False + + def clean_up_left_over(self): + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE_GROUP") + keys = atbl.getKeys() + for key in keys: + atbl._del(key) + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE_GROUP") + keys = atbl.getKeys() + assert len(keys) == 0 + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE_GROUP_MEMBER") + keys = atbl.getKeys() + for key in keys: + atbl._del(key) + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE_GROUP_MEMBER") + keys = atbl.getKeys() + assert len(keys) == 0 + + def verify_acl_group_num(self, expt): + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE_GROUP") + acl_table_groups = atbl.getKeys() + assert len(acl_table_groups) == expt + + for k in acl_table_groups: + (status, fvs) = atbl.get(k) + assert status == True + for fv in fvs: + if fv[0] == "SAI_ACL_TABLE_GROUP_ATTR_ACL_STAGE": + assert fv[1] == "SAI_ACL_STAGE_INGRESS" + elif fv[0] == "SAI_ACL_TABLE_GROUP_ATTR_ACL_BIND_POINT_TYPE_LIST": + assert fv[1] == "1:SAI_ACL_BIND_POINT_TYPE_PORT" + elif fv[0] == "SAI_ACL_TABLE_GROUP_ATTR_TYPE": + assert fv[1] == "SAI_ACL_TABLE_GROUP_TYPE_PARALLEL" + else: + assert False + + def get_acl_group_ids(self): + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE_GROUP") + acl_table_groups = atbl.getKeys() + return acl_table_groups + + def get_fvs_dict(self, fvs): + fvs_dict = {} + for fv in fvs: + fvs_dict.update({fv[0]:fv[1]}) + return fvs_dict + + def verify_acl_group_member(self, acl_group_id, acl_table_id): + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE_GROUP_MEMBER") + keys = atbl.getKeys() + + for k in keys: + (status, fvs) = atbl.get(k) + assert status == True + assert len(fvs) == 3 + fvs_dict = self.get_fvs_dict(fvs) + if (fvs_dict["SAI_ACL_TABLE_GROUP_MEMBER_ATTR_ACL_TABLE_GROUP_ID"] == acl_group_id and + fvs_dict["SAI_ACL_TABLE_GROUP_MEMBER_ATTR_ACL_TABLE_ID"] == acl_table_id) : + return True + assert False + + def verify_acl_port_binding(self, bind_ports): + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE_GROUP") + acl_table_groups = atbl.getKeys() + assert len(acl_table_groups) == len(bind_ports) + @pytest.yield_fixture(scope="module") def dvs(request): name = request.config.getoption("--dvsname") diff --git a/tests/port_dpb.py b/tests/port_dpb.py index 97abdf975a..c0c85fb11b 100644 --- a/tests/port_dpb.py +++ b/tests/port_dpb.py @@ -53,7 +53,7 @@ def set_index(self, index): self._index = index def set_oid(self, oid = None): - self._oid = oid + self._oid = oid def get_speed(self): return self._speed @@ -217,7 +217,7 @@ def breakin(self, dvs, port_names): for cp in child_ports: cp.delete_from_config_db() - # TBD, need vs lib to support removing hostif + # TBD, need vs lib to support removing hostif #dvs.runcmd("ip link delete " + cp.get_name()) #print "Deleted child ports:%s from config DB"%port_names @@ -285,9 +285,8 @@ def breakout(self, dvs, port_name, num_child_ports): assert(p.exists_in_config_db() == False) assert(p.exists_in_app_db() == False) assert(p.exists_in_asic_db() == False) - - self.create_child_ports(dvs, p, num_child_ports) + self.create_child_ports(dvs, p, num_child_ports) def change_speed_and_verify(self, dvs, port_names, speed = 100000): for port_name in port_names: diff --git a/tests/test_acl_portchannel.py b/tests/test_acl_portchannel.py index 63827656c4..7e5925e0d3 100644 --- a/tests/test_acl_portchannel.py +++ b/tests/test_acl_portchannel.py @@ -133,6 +133,8 @@ def test_PortChannelAfterAcl(self, dvs): time.sleep(2) used_counter = dvs.getCrmCounterValue('ACL_STATS:INGRESS:LAG', 'crm_stats_acl_group_used') + if used_counter is None: + used_counter = 0 # create port channel self.create_port_channel(dvs, "PortChannel01") @@ -142,8 +144,8 @@ def test_PortChannelAfterAcl(self, dvs): time.sleep(2) new_used_counter = dvs.getCrmCounterValue('ACL_STATS:INGRESS:LAG', 'crm_stats_acl_group_used') - if used_counter is None: - used_counter = 0 + if new_used_counter is None: + new_used_counter = 0 assert new_used_counter - used_counter == 1 # check ASIC table self.check_asic_table_existed(dvs) diff --git a/tests/test_port_dpb_acl.py b/tests/test_port_dpb_acl.py new file mode 100644 index 0000000000..f482a19e33 --- /dev/null +++ b/tests/test_port_dpb_acl.py @@ -0,0 +1,169 @@ +from swsscommon import swsscommon +import redis +import time +import os +import pytest +from pytest import * +import json +import re +from port_dpb import DPB + +@pytest.mark.usefixtures('dpb_setup_fixture') +class TestPortDPBAcl(object): + + ''' + @pytest.mark.skip() + ''' + def test_acl_table_empty_port_list(self, dvs): + dvs.setup_db() + + # Create ACL table "test" and bind it to Ethernet0 + bind_ports = [] + dvs.create_acl_table("test", "L3", bind_ports) + time.sleep(2) + acl_table_ids = dvs.get_acl_table_ids() + assert len(acl_table_ids) == 1 + dvs.verify_acl_group_num(0) + acl_group_ids = dvs.get_acl_group_ids() + assert len(acl_group_ids) == 0 + + bind_ports = ["Ethernet0"] + fvs = swsscommon.FieldValuePairs([("ports", ",".join(bind_ports))]) + dvs.update_acl_table("test", fvs) + time.sleep(2) + acl_table_ids = dvs.get_acl_table_ids() + assert len(acl_table_ids) == 1 + dvs.verify_acl_group_num(1) + acl_group_ids = dvs.get_acl_group_ids() + assert len(acl_group_ids) == 1 + dvs.verify_acl_group_member(acl_group_ids[0], acl_table_ids[0]) + dvs.verify_acl_port_binding(bind_ports) + + bind_ports = [] + fvs = swsscommon.FieldValuePairs([("ports", ",".join(bind_ports))]) + dvs.update_acl_table("test", fvs) + time.sleep(2) + acl_table_ids = dvs.get_acl_table_ids() + assert len(acl_table_ids) == 1 + dvs.verify_acl_group_num(0) + acl_group_ids = dvs.get_acl_group_ids() + assert len(acl_group_ids) == 0 + + ''' + @pytest.mark.skip() + ''' + def test_one_port_two_acl_tables(self, dvs): + dvs.setup_db() + + # Create ACL table "test" and bind it to Ethernet0 + bind_ports = ["Ethernet0"] + dvs.create_acl_table("test", "L3", bind_ports) + time.sleep(2) + acl_table_ids = dvs.get_acl_table_ids() + assert len(acl_table_ids) == 1 + dvs.verify_acl_group_num(1) + acl_group_ids = dvs.get_acl_group_ids() + assert len(acl_group_ids) == 1 + dvs.verify_acl_group_member(acl_group_ids[0], acl_table_ids[0]) + dvs.verify_acl_port_binding(bind_ports) + + # Create ACL table "test1" and bind it to Ethernet0 + bind_ports = ["Ethernet0"] + dvs.create_acl_table("test1", "L3", bind_ports) + time.sleep(2) + acl_table_ids = dvs.get_acl_table_ids() + assert len(acl_table_ids) == 2 + dvs.verify_acl_group_num(1) + dvs.verify_acl_group_member(acl_group_ids[0], acl_table_ids[0]) + dvs.verify_acl_group_member(acl_group_ids[0], acl_table_ids[1]) + dvs.verify_acl_port_binding(bind_ports) + + #Delete ACL tables + dvs.remove_acl_table("test") + time.sleep(2) + dvs.verify_acl_group_num(1) + dvs.remove_acl_table("test1") + time.sleep(2) + dvs.verify_acl_group_num(0) + + ''' + @pytest.mark.skip() + ''' + def test_one_acl_table_many_ports(self, dvs): + dvs.setup_db() + + # Create ACL table and bind it to Ethernet0 and Ethernet4 + bind_ports = ["Ethernet0", "Ethernet4"] + dvs.create_acl_table("test", "L3", bind_ports) + time.sleep(2) + acl_table_ids = dvs.get_acl_table_ids() + assert len(acl_table_ids) == 1 + dvs.verify_acl_group_num(2) + acl_group_ids = dvs.get_acl_group_ids() + dvs.verify_acl_group_member(acl_group_ids[0], acl_table_ids[0]) + dvs.verify_acl_group_member(acl_group_ids[1], acl_table_ids[0]) + dvs.verify_acl_port_binding(bind_ports) + + # Update bind list and verify + bind_ports = ["Ethernet4"] + fvs = swsscommon.FieldValuePairs([("ports", ",".join(bind_ports))]) + dvs.update_acl_table("test", fvs) + time.sleep(2) + dvs.verify_acl_group_num(1) + acl_group_ids = dvs.get_acl_group_ids() + dvs.verify_acl_group_member(acl_group_ids[0], acl_table_ids[0]) + dvs.verify_acl_port_binding(bind_ports) + + # Breakout Ethernet0 + dpb = DPB() + dpb.breakout(dvs, "Ethernet0", 4) + time.sleep(2) + + #Update bind list and verify + bind_ports = ["Ethernet0", "Ethernet1", "Ethernet2", "Ethernet3","Ethernet4"] + fvs = swsscommon.FieldValuePairs([("ports", ",".join(bind_ports))]) + dvs.update_acl_table("test", fvs) + time.sleep(2) + dvs.verify_acl_group_num(5) + acl_group_ids = dvs.get_acl_group_ids() + dvs.verify_acl_group_member(acl_group_ids[0], acl_table_ids[0]) + dvs.verify_acl_group_member(acl_group_ids[1], acl_table_ids[0]) + dvs.verify_acl_group_member(acl_group_ids[2], acl_table_ids[0]) + dvs.verify_acl_group_member(acl_group_ids[3], acl_table_ids[0]) + dvs.verify_acl_group_member(acl_group_ids[4], acl_table_ids[0]) + dvs.verify_acl_port_binding(bind_ports) + time.sleep(2) + + # Update bind list and verify + bind_ports = ["Ethernet4"] + fvs = swsscommon.FieldValuePairs([("ports", ",".join(bind_ports))]) + dvs.update_acl_table("test", fvs) + dvs.verify_acl_group_num(1) + acl_group_ids = dvs.get_acl_group_ids() + dvs.verify_acl_group_member(acl_group_ids[0], acl_table_ids[0]) + dvs.verify_acl_port_binding(bind_ports) + + #Breakin Ethernet0, 1, 2, 3 + dpb.breakin(dvs, ["Ethernet0", "Ethernet1", "Ethernet2", "Ethernet3"]) + time.sleep(2) + + # Update bind list and verify + bind_ports = ["Ethernet0", "Ethernet4"] + fvs = swsscommon.FieldValuePairs([("ports", ",".join(bind_ports))]) + dvs.update_acl_table("test", fvs) + time.sleep(2) + dvs.verify_acl_group_num(2) + acl_group_ids = dvs.get_acl_group_ids() + dvs.verify_acl_group_member(acl_group_ids[0], acl_table_ids[0]) + dvs.verify_acl_group_member(acl_group_ids[1], acl_table_ids[0]) + dvs.verify_acl_port_binding(bind_ports) + + #Delete ACL table + dvs.remove_acl_table("test") + time.sleep(2) + dvs.verify_acl_group_num(0) + + @pytest.mark.skip() + def test_one_port_many_acl_tables(self, dvs): + #TBD + return