diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 24166a9c54..aa577110ec 100644 --- a/orchagent/aclorch.cpp +++ b/orchagent/aclorch.cpp @@ -323,6 +323,38 @@ static acl_table_action_list_lookup_t defaultAclActionList = } }; +// The match fields for certain ACL table type are not exactly the same between INGRESS and EGRESS. +// For example, we can only match IN_PORT for PFCWD table type at INGRESS. +// Hence we need to specify stage particular matching fields in stageMandatoryMatchFields +static acl_table_match_field_lookup_t stageMandatoryMatchFields = +{ + { + // TABLE_TYPE_PFCWD + TABLE_TYPE_PFCWD, + { + { + ACL_STAGE_INGRESS, + { + SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS + } + } + } + }, + { + // TABLE_TYPE_DROP + TABLE_TYPE_DROP, + { + { + ACL_STAGE_INGRESS, + { + SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS + } + } + } + } + +}; + static acl_ip_type_lookup_t aclIpTypeLookup = { { IP_TYPE_ANY, SAI_ACL_IP_TYPE_ANY }, @@ -477,6 +509,12 @@ bool AclTableType::addAction(sai_acl_action_type_t action) return true; } +bool AclTableType::addMatch(shared_ptr match) +{ + m_matches.emplace(match->getId(), match); + return true; +} + AclTableTypeBuilder& AclTableTypeBuilder::withName(string name) { m_tableType.m_name = name; @@ -2020,6 +2058,34 @@ bool AclTable::addMandatoryActions() return true; } +bool AclTable::addStageMandatoryMatchFields() +{ + SWSS_LOG_ENTER(); + + if (stage == ACL_STAGE_UNKNOWN) + { + return false; + } + + if (stageMandatoryMatchFields.count(type.getName()) != 0) + { + auto &fields_for_stage = stageMandatoryMatchFields[type.getName()]; + if (fields_for_stage.count(stage) != 0) + { + // Add the stage particular matching fields + for (auto match : fields_for_stage[stage]) + { + type.addMatch(make_shared(match)); + SWSS_LOG_INFO("Added mandatory match field %s for table type %s stage %d", + sai_serialize_enum(match, &sai_metadata_enum_sai_acl_table_attr_t).c_str(), + type.getName().c_str(), stage); + } + } + } + + return true; +} + bool AclTable::validateAddType(const AclTableType &tableType) { SWSS_LOG_ENTER(); @@ -2983,7 +3049,6 @@ void AclOrch::initDefaultTableTypes() builder.withName(TABLE_TYPE_PFCWD) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TC)) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS)) .build() ); @@ -2991,7 +3056,6 @@ void AclOrch::initDefaultTableTypes() builder.withName(TABLE_TYPE_DROP) .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT) .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_TC)) - .withMatch(make_shared(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS)) .build() ); @@ -4108,6 +4172,8 @@ void AclOrch::doAclTableTask(Consumer &consumer) newTable.validateAddType(*tableType); + newTable.addStageMandatoryMatchFields(); + newTable.addMandatoryActions(); // validate and create/update ACL Table diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h index ee17ba4a1f..ce3e9e5d63 100644 --- a/orchagent/aclorch.h +++ b/orchagent/aclorch.h @@ -116,6 +116,9 @@ typedef map> acl_action_enum_values_capabili typedef map > acl_stage_action_list_t; typedef map acl_table_action_list_lookup_t; +typedef map > acl_stage_match_field_t; +typedef map acl_table_match_field_lookup_t; + class AclRule; class AclTableMatchInterface @@ -160,6 +163,7 @@ class AclTableType const set& getActions() const; bool addAction(sai_acl_action_type_t action); + bool addMatch(shared_ptr match); private: friend class AclTableTypeBuilder; @@ -384,6 +388,9 @@ class AclTable // Add actions to ACL table if mandatory action list is required on table creation. bool addMandatoryActions(); + // Add stage mandatory matching fields to ACL table + bool addStageMandatoryMatchFields(); + // validate AclRule match attribute against rule and table configuration bool validateAclRuleMatch(sai_acl_entry_attr_t matchId, const AclRule& rule) const; // validate AclRule action attribute against rule and table configuration diff --git a/tests/test_acl.py b/tests/test_acl.py index c246eefe53..5c542193f7 100644 --- a/tests/test_acl.py +++ b/tests/test_acl.py @@ -1,4 +1,5 @@ import pytest +from requests import request L3_TABLE_TYPE = "L3" L3_TABLE_NAME = "L3_TEST" @@ -20,6 +21,9 @@ MIRROR_BIND_PORTS = ["Ethernet0", "Ethernet4", "Ethernet8", "Ethernet12"] MIRROR_RULE_NAME = "MIRROR_TEST_RULE" +PFCWD_TABLE_TYPE = "PFCWD" +PFCWD_TABLE_NAME = "PFCWD_TEST" +PFCWD_BIND_PORTS = ["Ethernet0", "Ethernet4", "Ethernet8", "Ethernet12"] class TestAcl: @pytest.yield_fixture def l3_acl_table(self, dvs_acl): @@ -59,6 +63,15 @@ def mirror_acl_table(self, dvs_acl): dvs_acl.remove_acl_table(MIRROR_TABLE_NAME) dvs_acl.verify_acl_table_count(0) + @pytest.fixture(params=['ingress', 'egress']) + def pfcwd_acl_table(self, dvs_acl, request): + try: + dvs_acl.create_acl_table(PFCWD_TABLE_NAME, PFCWD_TABLE_TYPE, PFCWD_BIND_PORTS, request.param) + yield dvs_acl.get_acl_table_ids(1)[0], request.param + finally: + dvs_acl.remove_acl_table(PFCWD_TABLE_NAME) + dvs_acl.verify_acl_table_count(0) + @pytest.yield_fixture def setup_teardown_neighbor(self, dvs): try: @@ -548,8 +561,22 @@ def test_AclRuleRedirect(self, dvs, dvs_acl, l3_acl_table, setup_teardown_neighb dvs_acl.remove_acl_rule(L3_TABLE_NAME, L3_RULE_NAME) dvs_acl.verify_no_acl_rules() - - + + def test_AclTableMandatoryMatchFields(self, dvs, pfcwd_acl_table): + """ + The test case is to verify stage particular matching fields is applied + """ + table_oid, stage = pfcwd_acl_table + match_in_ports = False + entry = dvs.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE", table_oid) + for k, v in entry.items(): + if k == "SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS" and v == "true": + match_in_ports = True + + if stage == "ingress": + assert match_in_ports + else: + assert not match_in_ports class TestAclCrmUtilization: @pytest.fixture(scope="class", autouse=True) def configure_crm_polling_interval_for_test(self, dvs):