diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index f8bf775868..5ad908f082 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -28,7 +28,6 @@ extern sai_switch_api_t* sai_switch_api; extern sai_object_id_t gSwitchId; extern PortsOrch* gPortsOrch; extern CrmOrch *gCrmOrch; -extern SwitchOrch *gSwitchOrch; extern string gMySwitchType; #define MIN_VLAN_ID 1 // 0 is a reserved VLAN ID @@ -47,7 +46,6 @@ const int TCP_PROTOCOL_NUM = 6; // TCP protocol number acl_rule_attr_lookup_t aclMatchLookup = { { MATCH_IN_PORTS, SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS }, - { MATCH_OUT_PORT, SAI_ACL_ENTRY_ATTR_FIELD_OUT_PORT }, { MATCH_OUT_PORTS, SAI_ACL_ENTRY_ATTR_FIELD_OUT_PORTS }, { MATCH_SRC_IP, SAI_ACL_ENTRY_ATTR_FIELD_SRC_IP }, { MATCH_DST_IP, SAI_ACL_ENTRY_ATTR_FIELD_DST_IP }, @@ -867,23 +865,6 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value) matchData.data.objlist.count = static_cast(outPorts.size()); matchData.data.objlist.list = outPorts.data(); } - else if (attr_name == MATCH_OUT_PORT) - { - auto alias = attr_value; - Port port; - if (!gPortsOrch->getPort(alias, port)) - { - SWSS_LOG_ERROR("Failed to locate port %s", alias.c_str()); - return false; - } - if (port.m_type != Port::PHY) - { - SWSS_LOG_ERROR("Cannot bind rule to %s: OUT_PORT can only match physical interfaces", alias.c_str()); - return false; - } - - matchData.data.oid = port.m_port_id; - } else if (attr_name == MATCH_IP_TYPE) { if (!processIpType(attr_value, matchData.data.u32)) @@ -3220,14 +3201,14 @@ void AclOrch::init(vector& connectors, PortsOrch *portOrch, Mirr m_mirrorV6TableId[stage] = ""; } - initDefaultTableTypes(platform, sub_platform); + initDefaultTableTypes(); // Attach observers m_mirrorOrch->attach(this); gPortsOrch->attach(this); } -void AclOrch::initDefaultTableTypes(const string& platform, const string& sub_platform) +void AclOrch::initDefaultTableTypes() { SWSS_LOG_ENTER(); @@ -3314,26 +3295,12 @@ void AclOrch::initDefaultTableTypes(const string& platform, const string& sub_pl .build() ); - // Use SAI_ACL_BIND_POINT_TYPE_SWITCH in BRCM DNX platforms to use shared egress ACL table for PFCWD. - if (platform == BRCM_PLATFORM_SUBSTRING && sub_platform == BRCM_DNX_PLATFORM_SUBSTRING) - { - addAclTableType( - builder.withName(TABLE_TYPE_PFCWD) - .withBindPointType(SAI_ACL_BIND_POINT_TYPE_SWITCH) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TC)) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_OUT_PORT)) - .build() - ); - } - else - { - addAclTableType( - builder.withName(TABLE_TYPE_PFCWD) - .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TC)) - .build() - ); - } + addAclTableType( + builder.withName(TABLE_TYPE_PFCWD) + .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) + .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TC)) + .build() + ); addAclTableType( builder.withName(TABLE_TYPE_DROP) @@ -4007,20 +3974,6 @@ bool AclOrch::addAclTable(AclTable &newTable) m_mirrorV6TableId[table_stage] = table_id; } - // We use SAI_ACL_BIND_POINT_TYPE_SWITCH for PFCWD table in DNX platform. - // This bind type requires to bind the table to switch. - string platform = getenv("platform") ? getenv("platform") : ""; - string sub_platform = getenv("sub_platform") ? getenv("sub_platform") : ""; - if (platform == BRCM_PLATFORM_SUBSTRING && sub_platform == BRCM_DNX_PLATFORM_SUBSTRING && - newTable.type.getName() == TABLE_TYPE_PFCWD) - { - if(!gSwitchOrch->bindAclTableToSwitch(ACL_STAGE_EGRESS, newTable.getOid())) - { - return false; - } - newTable.bindToSwitch = true; - } - return true; } else @@ -4045,18 +3998,6 @@ bool AclOrch::removeAclTable(string table_id) bool suc = m_AclTables[table_oid].clear(); if (!suc) return false; - // Unbind table from switch if needed. - AclTable &table = m_AclTables.at(table_oid); - if (table.bindToSwitch) - { - // Only bind egress table to switch for now. - assert(table->stage == ACL_STAGE_EGRESS); - if(!gSwitchOrch->unbindAclTableFromSwitch(ACL_STAGE_EGRESS, table.getOid())) - { - return false; - } - } - if (deleteUnbindAclTable(table_oid) == SAI_STATUS_SUCCESS) { auto stage = m_AclTables[table_oid].stage; @@ -5170,3 +5111,4 @@ void AclOrch::removeAllAclRuleStatus() m_aclRuleStateTable.del(key); } } + diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index 5458e970be..abeaf519e2 100644 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -23,7 +23,6 @@ #define RULE_PRIORITY "PRIORITY" #define MATCH_IN_PORTS "IN_PORTS" -#define MATCH_OUT_PORT "OUT_PORT" #define MATCH_OUT_PORTS "OUT_PORTS" #define MATCH_SRC_IP "SRC_IP" #define MATCH_DST_IP "DST_IP" @@ -452,9 +451,6 @@ class AclTable // Set to store the not configured ACL table port alias set pendingPortSet; - // Is the ACL table bound to switch? - bool bindToSwitch = false; - private: sai_object_id_t m_oid = SAI_NULL_OBJECT_ID; AclOrch *m_pAclOrch = nullptr; @@ -534,7 +530,7 @@ class AclOrch : public Orch, public Observer void doAclRuleTask(Consumer &consumer); void doAclTableTypeTask(Consumer &consumer); void init(vector& connectors, PortsOrch *portOrch, MirrorOrch *mirrorOrch, NeighOrch *neighOrch, RouteOrch *routeOrch); - void initDefaultTableTypes(const string& platform, const string& sub_platform); + void initDefaultTableTypes(); void queryMirrorTableCapability(); void queryAclActionCapability(); diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 047263c93a..d5bda136fb 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -676,25 +676,7 @@ bool OrchDaemon::init() SAI_QUEUE_ATTR_PAUSE_STATUS, }; - bool pfcDlrInit = gSwitchOrch->checkPfcDlrInitEnable(); - - // Override pfcDlrInit if needed, and this change is only for PFC tests. - if(getenv("PFC_DLR_INIT_ENABLE")) - { - string envPfcDlrInit = getenv("PFC_DLR_INIT_ENABLE"); - if(envPfcDlrInit == "1") - { - pfcDlrInit = true; - SWSS_LOG_NOTICE("Override PfcDlrInitEnable to true"); - } - else if(envPfcDlrInit == "0") - { - pfcDlrInit = false; - SWSS_LOG_NOTICE("Override PfcDlrInitEnable to false"); - } - } - - if(pfcDlrInit) + if(gSwitchOrch->checkPfcDlrInitEnable()) { m_orchList.push_back(new PfcWdSwOrch( m_configDb, diff --git a/orchagent/pfcactionhandler.cpp b/orchagent/pfcactionhandler.cpp index 32f984cea1..305ed4421d 100644 --- a/orchagent/pfcactionhandler.cpp +++ b/orchagent/pfcactionhandler.cpp @@ -344,58 +344,19 @@ PfcWdAclHandler::PfcWdAclHandler(sai_object_id_t port, sai_object_id_t queue, // Egress table/rule creation table_type = TABLE_TYPE_PFCWD; - - // Use shared egress acl table for BRCM DNX platform. - string platform = getenv("platform") ? getenv("platform") : ""; - string sub_platform = getenv("sub_platform") ? getenv("sub_platform") : ""; - shared_egress_acl_table = (platform == BRCM_PLATFORM_SUBSTRING && - sub_platform == BRCM_DNX_PLATFORM_SUBSTRING); - - if (shared_egress_acl_table) + m_strEgressTable = "EgressTable_PfcWdAclHandler_" + queuestr; + found = m_aclTables.find(m_strEgressTable); + if (found == m_aclTables.end()) { - Port p; - if (!gPortsOrch->getPort(port, p)) - { - SWSS_LOG_ERROR("Failed to get port structure from port oid 0x%" PRIx64, port); - return; - } - m_strEgressRule = "Egress_Rule_PfcWdAclHandler_" + p.m_alias + "_" + queuestr; - m_strEgressTable = "EgressTable_PfcWdAclHandler"; - found = m_aclTables.find(m_strEgressTable); - if (found == m_aclTables.end()) - { - // First time of handling PFC, create ACL table and also ACL rule. - createPfcAclTable(port, m_strEgressTable, false); - shared_ptr newRule = make_shared(gAclOrch, m_strEgressRule, m_strEgressTable); - createPfcAclRule(newRule, queueId, m_strEgressTable, port); - } - else - { - // ACL table already exists. Add ACL rule if needed. - AclRule* rule = gAclOrch->getAclRule(m_strEgressTable, m_strEgressRule); - if (rule == nullptr) - { - shared_ptr newRule = make_shared(gAclOrch, m_strEgressRule, m_strEgressTable); - createPfcAclRule(newRule, queueId, m_strEgressTable, port); - } - } + // First time of handling PFC for this queue, create ACL table, and bind + createPfcAclTable(port, m_strEgressTable, false); + shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strEgressTable); + createPfcAclRule(newRule, queueId, m_strEgressTable, port); } else { - m_strEgressTable = "EgressTable_PfcWdAclHandler_" + queuestr; - found = m_aclTables.find(m_strEgressTable); - if (found == m_aclTables.end()) - { - // First time of handling PFC for this queue, create ACL table, and bind - createPfcAclTable(port, m_strEgressTable, false); - shared_ptr newRule = make_shared(gAclOrch, m_strRule, m_strEgressTable); - createPfcAclRule(newRule, queueId, m_strEgressTable, port); - } - else - { - // Otherwise just bind ACL table with the port - found->second.bind(port); - } + // Otherwise just bind ACL table with the port + found->second.bind(port); } } @@ -421,20 +382,8 @@ PfcWdAclHandler::~PfcWdAclHandler(void) gAclOrch->updateAclRule(m_strIngressTable, m_strRule, MATCH_IN_PORTS, &port, RULE_OPER_DELETE); } - if (shared_egress_acl_table) - { - rule = gAclOrch->getAclRule(m_strEgressTable, m_strEgressRule); - if (rule == nullptr) - { - SWSS_LOG_THROW("Egress ACL Rule does not exist for rule %s", m_strEgressRule.c_str()); - } - gAclOrch->removeAclRule(m_strEgressTable, m_strEgressRule); - } - else - { - auto found = m_aclTables.find(m_strEgressTable); - found->second.unbind(port); - } + auto found = m_aclTables.find(m_strEgressTable); + found->second.unbind(port); } void PfcWdAclHandler::clear() @@ -469,11 +418,7 @@ void PfcWdAclHandler::createPfcAclTable(sai_object_id_t port, string strTable, b return; } - // Link port only for ingress ACL table or unshared egress ACL table. - if (ingress || !shared_egress_acl_table) - { - aclTable.link(port); - } + aclTable.link(port); if (ingress) { @@ -507,24 +452,18 @@ void PfcWdAclHandler::createPfcAclRule(shared_ptr rule, uint8_t q attr_value = to_string(queueId); rule->validateAddMatch(attr_name, attr_value); - // Add MATCH_IN_PORTS as match criteria for ingress table and MATCH_OUT_PORT as match creiteria for shared egress table. - if (strTable == INGRESS_TABLE_DROP || shared_egress_acl_table) + // Add MATCH_IN_PORTS as match criteria for ingress table + if (strTable == INGRESS_TABLE_DROP) { Port p; - if (strTable == INGRESS_TABLE_DROP) - { - attr_name = MATCH_IN_PORTS; - } - else if (shared_egress_acl_table) { - attr_name = MATCH_OUT_PORT; - } - + attr_name = MATCH_IN_PORTS; + if (!gPortsOrch->getPort(portOid, p)) { SWSS_LOG_ERROR("Failed to get port structure from port oid 0x%" PRIx64, portOid); return; } - + attr_value = p.m_alias; rule->validateAddMatch(attr_name, attr_value); } diff --git a/orchagent/pfcactionhandler.h b/orchagent/pfcactionhandler.h index 9fa409af30..acfc923423 100644 --- a/orchagent/pfcactionhandler.h +++ b/orchagent/pfcactionhandler.h @@ -107,12 +107,9 @@ class PfcWdAclHandler: public PfcWdLossyHandler // class shared dict: ACL table name -> ACL table static std::map m_aclTables; - bool shared_egress_acl_table = false; - string m_strIngressTable; string m_strEgressTable; string m_strRule; - string m_strEgressRule; void createPfcAclTable(sai_object_id_t port, string strTable, bool ingress); void createPfcAclRule(shared_ptr rule, uint8_t queueId, string strTable, sai_object_id_t port); void updatePfcAclRule(shared_ptr rule, uint8_t queueId, string strTable, vector port); diff --git a/orchagent/switchorch.cpp b/orchagent/switchorch.cpp index b8e0090259..a9579743d0 100644 --- a/orchagent/switchorch.cpp +++ b/orchagent/switchorch.cpp @@ -1522,51 +1522,3 @@ bool SwitchOrch::querySwitchCapability(sai_object_type_t sai_object, sai_attr_id } } } - -// Bind ACL table (with bind type switch) to switch -bool SwitchOrch::bindAclTableToSwitch(acl_stage_type_t stage, sai_object_id_t table_id) -{ - sai_attribute_t attr; - if ( stage == ACL_STAGE_INGRESS ) { - attr.id = SAI_SWITCH_ATTR_INGRESS_ACL; - } else { - attr.id = SAI_SWITCH_ATTR_EGRESS_ACL; - } - attr.value.oid = table_id; - sai_status_t status = sai_switch_api->set_switch_attribute(gSwitchId, &attr); - string stage_str = (stage == ACL_STAGE_INGRESS) ? "ingress" : "egress"; - if (status == SAI_STATUS_SUCCESS) - { - SWSS_LOG_NOTICE("Bind %s acl table %" PRIx64" to switch", stage_str.c_str(), table_id); - return true; - } - else - { - SWSS_LOG_ERROR("Failed to bind %s acl table %" PRIx64" to switch", stage_str.c_str(), table_id); - return false; - } -} - -// Unbind ACL table from swtich -bool SwitchOrch::unbindAclTableFromSwitch(acl_stage_type_t stage,sai_object_id_t table_id) -{ - sai_attribute_t attr; - if ( stage == ACL_STAGE_INGRESS ) { - attr.id = SAI_SWITCH_ATTR_INGRESS_ACL; - } else { - attr.id = SAI_SWITCH_ATTR_EGRESS_ACL; - } - attr.value.oid = SAI_NULL_OBJECT_ID; - sai_status_t status = sai_switch_api->set_switch_attribute(gSwitchId, &attr); - string stage_str = (stage == ACL_STAGE_INGRESS) ? "ingress" : "egress"; - if (status == SAI_STATUS_SUCCESS) - { - SWSS_LOG_NOTICE("Unbind %s acl table %" PRIx64" to switch", stage_str.c_str(), table_id); - return true; - } - else - { - SWSS_LOG_ERROR("Failed to unbind %s acl table %" PRIx64" to switch", stage_str.c_str(), table_id); - return false; - } -} diff --git a/orchagent/switchorch.h b/orchagent/switchorch.h index 45ceb58e5f..95cd04dbdf 100644 --- a/orchagent/switchorch.h +++ b/orchagent/switchorch.h @@ -65,9 +65,6 @@ class SwitchOrch : public Orch return (m_fatalEventCount != 0); } - bool bindAclTableToSwitch(acl_stage_type_t stage, sai_object_id_t table_id); - bool unbindAclTableFromSwitch(acl_stage_type_t stage, sai_object_id_t table_id); - private: void doTask(Consumer &consumer); void doTask(swss::SelectableTimer &timer); diff --git a/tests/test_pfcwd_shared_egress_acl_table.py b/tests/test_pfcwd_shared_egress_acl_table.py deleted file mode 100644 index d1686a629f..0000000000 --- a/tests/test_pfcwd_shared_egress_acl_table.py +++ /dev/null @@ -1,173 +0,0 @@ -import time -import pytest -from dvslib.dvs_common import wait_for_result, PollingConfig - -# set ASIC_TYPE=broadcom-dnx to test broadcom-dnx specific implementation. -# set PFC_DLR_INIT_ENABLE=0 to test PfcWdAclHandler, instead of PfcWdDlrHandler. -DVS_ENV = ["ASIC_TYPE=broadcom-dnx", "PFC_DLR_INIT_ENABLE=0"] - -class TestPfcwdFunc(object): - @pytest.fixture - def select_lc(self, vct): - # find a LC to test PFCWD - self.dvs = None - for name in vct.dvss.keys(): - dvs = vct.dvss[name] - config_db = dvs.get_config_db() - metatbl = config_db.get_entry("DEVICE_METADATA", "localhost") - cfg_switch_type = metatbl.get("switch_type") - if cfg_switch_type == "voq": - self.dvs = dvs - assert self.dvs - - @pytest.fixture - def setup_teardown_test(self, select_lc): - self.asic_db = self.dvs.get_asic_db() - self.config_db = self.dvs.get_config_db() - self.counters_db = self.dvs.get_counters_db() - - self.test_ports = ["Ethernet0"] - self.setup_test(self.dvs) - - self.port_oids = self.counters_db.get_entry("COUNTERS_PORT_NAME_MAP", "") - self.queue_oids = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP", "") - - yield - - self.teardown_test(self.dvs) - - def setup_test(self, dvs): - # save original cable length and set new cabel length - fvs = self.config_db.get_entry("CABLE_LENGTH", "AZURE") - self.orig_cable_len = dict() - for port in self.test_ports: - self.orig_cable_len[port] = fvs[port] - self.set_cable_len(port, "5m") - # startup port - dvs.port_admin_set(port, "up") - - # enable pfcwd - self.set_flex_counter_status("PFCWD", "enable") - # enable queue so that queue oids are generated - self.set_flex_counter_status("QUEUE", "enable") - - def teardown_test(self, dvs): - # disable pfcwd - self.set_flex_counter_status("PFCWD", "disable") - # disable queue - self.set_flex_counter_status("QUEUE", "disable") - - for port in self.test_ports: - if self.orig_cable_len: - self.set_cable_len(port, self.orig_cable_len[port]) - # shutdown port - dvs.port_admin_set(port, "down") - - def set_flex_counter_status(self, key, state): - fvs = {'FLEX_COUNTER_STATUS': state} - self.config_db.update_entry("FLEX_COUNTER_TABLE", key, fvs) - time.sleep(1) - - def set_ports_pfc(self, status='enable', pfc_queues=[3,4]): - keyname = 'pfcwd_sw_enable' - for port in self.test_ports: - if 'enable' in status: - queues = ",".join([str(q) for q in pfc_queues]) - fvs = {keyname: queues, 'pfc_enable': queues} - self.config_db.create_entry("PORT_QOS_MAP", port, fvs) - else: - self.config_db.delete_entry("PORT_QOS_MAP", port) - - def set_cable_len(self, port_name, cable_len): - fvs = {port_name: cable_len} - self.config_db.update_entry("CABLE_LEN", "AZURE", fvs) - - def start_pfcwd_on_ports(self, poll_interval="200", detection_time="200", restoration_time="200", action="drop"): - pfcwd_info = {"POLL_INTERVAL": poll_interval} - self.config_db.update_entry("PFC_WD", "GLOBAL", pfcwd_info) - - pfcwd_info = {"action": action, - "detection_time" : detection_time, - "restoration_time": restoration_time - } - for port in self.test_ports: - self.config_db.update_entry("PFC_WD", port, pfcwd_info) - - def stop_pfcwd_on_ports(self): - for port in self.test_ports: - self.config_db.delete_entry("PFC_WD", port) - - def set_storm_state(self, queues, state="enabled"): - fvs = {"DEBUG_STORM": state} - for port in self.test_ports: - for queue in queues: - queue_name = port + ":" + str(queue) - self.counters_db.update_entry("COUNTERS", self.queue_oids[queue_name], fvs) - - def verify_egress_acls(self, expected_acls=None): - def do_verify_egress_acls(): - egress_acl_table_oids = [] - acl_table_name = "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE" - for oid_key in self.asic_db.get_keys(acl_table_name): - entry = self.asic_db.get_entry(acl_table_name, oid_key) - # find egress ACL table by checking ACL table attributes - if (entry.get("SAI_ACL_TABLE_ATTR_ACL_STAGE") == "SAI_ACL_STAGE_EGRESS" and - entry.get("SAI_ACL_TABLE_ATTR_FIELD_TC") == "true" and - entry.get("SAI_ACL_TABLE_ATTR_FIELD_OUT_PORT") == "true" and - entry.get("SAI_ACL_TABLE_ATTR_ACL_BIND_POINT_TYPE_LIST") == "1:SAI_ACL_BIND_POINT_TYPE_SWITCH"): - egress_acl_table_oids.append(oid_key) - if len(egress_acl_table_oids) != 1: - return (False, None) - - # find installed ACL entries in egress ACL tables. - installed_acls = [] - acl_entry_table_name = "ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY" - for oid_key in self.asic_db.get_keys(acl_entry_table_name): - entry = self.asic_db.get_entry(acl_entry_table_name, oid_key) - if entry.get("SAI_ACL_ENTRY_ATTR_TABLE_ID") in egress_acl_table_oids: - port_oid = entry.get("SAI_ACL_ENTRY_ATTR_FIELD_OUT_PORT") - tc = entry.get("SAI_ACL_ENTRY_ATTR_FIELD_TC") - tc = int(tc.replace("&mask:0xff", "")) - installed_acls.append((port_oid, tc)) - - # verify installed ACLs against expected ones - return (sorted(installed_acls) == sorted(expected_acls), None) - - max_poll = PollingConfig(polling_interval=5, timeout=600, strict=True) - wait_for_result(do_verify_egress_acls, polling_config=max_poll) - - def test_pfcwd_shared_egress_acl_table(self, setup_teardown_test): - try: - # enable PFC on queues - test_queues = [3, 4] - self.set_ports_pfc(pfc_queues=test_queues) - - # start PFCWD on ports and PFC storm - self.start_pfcwd_on_ports() - storm_queue = test_queues - self.set_storm_state(storm_queue) - - # verify egress ACLs in asic db - expected_acls = [] - for port in self.test_ports: - for queue in storm_queue: - expected_acls.append((self.port_oids[port], queue)) - self.verify_egress_acls(expected_acls) - - # stop storm and PFCWD on port. - self.set_storm_state(storm_queue, state="disabled") - self.stop_pfcwd_on_ports() - - # verify egress ACLs and table are deleted. - expected_acls = [] - self.verify_egress_acls(expected_acls) - - finally: - self.stop_pfcwd_on_ports() - - -# -# Add Dummy always-pass test at end as workaroud -# for issue when Flaky fail on final test it invokes module tear-down before retrying -def test_nonflaky_dummy(): - pass