From 6e0fc85daeb572311258f52885f86f3528a00f66 Mon Sep 17 00:00:00 2001 From: bingwang-ms <66248323+bingwang-ms@users.noreply.github.com> Date: Thu, 23 Jun 2022 04:07:40 +0800 Subject: [PATCH] [ACL] Support stage particular match fields (#2341) What I did This PR is to fix ACL table creation failure for certain types. We saw PFCWD table failed to be created at EGRESS stage. The error logs are Jun 21 07:00:03.409283 str2-7050cx3-acs-08 ERR syncd#syncd: [none] SAI_API_ACL:_brcm_sai_create_acl_table:11205 field group config create failed with error Feature unavailable (0xfffffff0). Jun 21 07:00:03.409738 str2-7050cx3-acs-08 ERR syncd#syncd: [none] SAI_API_ACL:brcm_sai_create_acl_table:298 create table entry failed with error -2. Jun 21 07:00:03.409738 str2-7050cx3-acs-08 ERR syncd#syncd: :- sendApiResponse: api SAI_COMMON_API_CREATE failed in syncd mode: SAI_STATUS_NOT_SUPPORTED Jun 21 07:00:03.409780 str2-7050cx3-acs-08 ERR syncd#syncd: :- processQuadEvent: attr: SAI_ACL_TABLE_ATTR_ACL_BIND_POINT_TYPE_LIST: 1:SAI_ACL_BIND_POINT_TYPE_PORT Jun 21 07:00:03.409820 str2-7050cx3-acs-08 ERR syncd#syncd: :- processQuadEvent: attr: SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS: true Jun 21 07:00:03.409820 str2-7050cx3-acs-08 ERR syncd#syncd: :- processQuadEvent: attr: SAI_ACL_TABLE_ATTR_FIELD_TC: true Jun 21 07:00:03.410144 str2-7050cx3-acs-08 ERR syncd#syncd: :- processQuadEvent: attr: SAI_ACL_TABLE_ATTR_ACL_ACTION_TYPE_LIST: 2:SAI_ACL_ACTION_TYPE_PACKET_ACTION,SAI_ACL_ACTION_TYPE_COUNTER Jun 21 07:00:03.410144 str2-7050cx3-acs-08 ERR syncd#syncd: :- processQuadEvent: attr: SAI_ACL_TABLE_ATTR_ACL_STAGE: SAI_ACL_STAGE_EGRESS Jun 21 07:00:03.410144 str2-7050cx3-acs-08 ERR swss#orchagent: :- create: create status: SAI_STATUS_NOT_SUPPORTED Jun 21 07:00:03.410144 str2-7050cx3-acs-08 ERR swss#orchagent: :- addAclTable: Failed to create ACL table pfcwd_egress The root cause for the issue is SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS is not supported at EGRESS stage. This PR addressed the issue by adding match field according to the stage. For ACL type TABLE_TYPE_PFCWD and TABLE_TYPE_DROP at INGRESS stage, the match field SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS is added, while for EGRESS the field is not added. Why I did it To fix ACL table creation issue. How I verified it Verified by vstest test_acl.py::TestAcl::test_AclTableMandatoryMatchFields[ingress] PASSED [ 87%] test_acl.py::TestAcl::test_AclTableMandatoryMatchFields[egress] PASSED [ 90%] Verified by building a new image and run on a TD3 device. Signed-off-by: bingwang --- orchagent/aclorch.cpp | 70 +++++++++++++++++++++++++++++++++++++++++-- orchagent/aclorch.h | 7 +++++ tests/test_acl.py | 31 +++++++++++++++++-- 3 files changed, 104 insertions(+), 4 deletions(-) diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp index 24166a9c548a..aa577110ec03 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 ee17ba4a1f66..ce3e9e5d6393 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 c246eefe53bb..5c542193f79a 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):