[ACL] Support stage particular match fields (sonic-net#2341)

bingwang-ms · web-flow · commit 6e0fc85daeb5 · 2022-06-22T13:07:40.000-07:00
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 &lt;wang.bing@microsoft.com&gt;
diff --git 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<AclTableMatchInterface> 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<AclTableMatch>(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,15 +3049,13 @@ void AclOrch::initDefaultTableTypes()
         builder.withName(TABLE_TYPE_PFCWD)
             .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT)
             .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TC))
-            .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS))
             .build()
     );
 
     addAclTableType(
         builder.withName(TABLE_TYPE_DROP)
             .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT)
             .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TC))
-            .withMatch(make_shared<AclTableMatch>(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
@@ -116,6 +116,9 @@ typedef map<sai_acl_action_type_t, set<int32_t>> acl_action_enum_values_capabili
 typedef map<acl_stage_type_t, set<sai_acl_action_type_t> > acl_stage_action_list_t;
 typedef map<string, acl_stage_action_list_t> acl_table_action_list_lookup_t;
 
+typedef map<acl_stage_type_t, set<sai_acl_table_attr_t> > acl_stage_match_field_t;
+typedef map<string, acl_stage_match_field_t> acl_table_match_field_lookup_t;
+
 class AclRule;
 
 class AclTableMatchInterface
@@ -160,6 +163,7 @@ class AclTableType
     const set<sai_acl_action_type_t>& getActions() const;
 
     bool addAction(sai_acl_action_type_t action);
+    bool addMatch(shared_ptr<AclTableMatchInterface> 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
@@ -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):
