L3 / L3 V6 Egress ACL table creation failure (#2561)

* In L3/L3V6 Egress ACL, Range type qualifier is not supported for Broadcom Platforms. Check is added to ignore the range type qualifier for Broadcom platforms alone.
sonic-net · Dec 16, 2022 · f2d2fb3 · f2d2fb3
1 parent 577f696
commit f2d2fb3
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 13 deletions.
diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp
@@ -356,8 +356,41 @@ static acl_table_match_field_lookup_t stageMandatoryMatchFields =
                 }
             }
         }
+    },
+    {
+        TABLE_TYPE_L3,
+        {
+            {
+                ACL_STAGE_INGRESS,
+                {
+                    SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE
+                }
+            },
+            {
+                ACL_STAGE_EGRESS,
+                {
+                    SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE
+                }
+            }
+        }
+    },
+    {
+        TABLE_TYPE_L3V6,
+        {
+            {
+                ACL_STAGE_INGRESS,
+                {
+                    SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE
+                }
+            },
+            {
+                ACL_STAGE_EGRESS,
+                {
+                    SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE
+                }
+            }
+        }
     }
-
 };
 
 static acl_ip_type_lookup_t aclIpTypeLookup =
@@ -2091,6 +2124,29 @@ bool AclTable::addMandatoryActions()
     return true;
 }
 
+bool AclTable::addStageMandatoryRangeFields()
+{
+    SWSS_LOG_ENTER();
+
+    string platform = getenv("platform") ? getenv("platform") : "";
+    auto match = SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE;
+
+    if ((platform == BRCM_PLATFORM_SUBSTRING) &&
+        (stage == ACL_STAGE_EGRESS))
+    {
+        return false;
+    }
+
+    type.addMatch(make_shared<AclTableRangeMatch>(set<sai_acl_range_type_t>{
+                    {SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}}));
+    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::addStageMandatoryMatchFields()
 {
     SWSS_LOG_ENTER();
@@ -2108,10 +2164,17 @@ bool AclTable::addStageMandatoryMatchFields()
             // 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);
+                if (match != SAI_ACL_TABLE_ATTR_FIELD_ACL_RANGE_TYPE)
+                {
+                    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);
+                }
+                else
+                {
+                    addStageMandatoryRangeFields();
+                }
             }
         }
     }
@@ -3035,8 +3098,6 @@ void AclOrch::initDefaultTableTypes()
             .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT))
             .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT))
             .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS))
-            .withMatch(make_shared<AclTableRangeMatch>(set<sai_acl_range_type_t>{
-                {SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}}))
             .build()
     );
 
@@ -3054,8 +3115,6 @@ void AclOrch::initDefaultTableTypes()
             .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT))
             .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT))
             .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS))
-            .withMatch(make_shared<AclTableRangeMatch>(set<sai_acl_range_type_t>{
-                {SAI_ACL_RANGE_TYPE_L4_SRC_PORT_RANGE, SAI_ACL_RANGE_TYPE_L4_DST_PORT_RANGE}}))
             .build()
     );
 

diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h
@@ -393,6 +393,9 @@ class AclTable
     // Add stage mandatory matching fields to ACL table
     bool addStageMandatoryMatchFields();
 
+    // Add stage mandatory range fields to ACL table
+    bool addStageMandatoryRangeFields();
+
     // 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
@@ -25,11 +25,11 @@
 PFCWD_TABLE_NAME = "PFCWD_TEST"
 PFCWD_BIND_PORTS = ["Ethernet0", "Ethernet4", "Ethernet8", "Ethernet12"]
 class TestAcl:
-    @pytest.fixture
-    def l3_acl_table(self, dvs_acl):
+    @pytest.fixture(params=['ingress', 'egress'])
+    def l3_acl_table(self, dvs_acl, request):
         try:
-            dvs_acl.create_acl_table(L3_TABLE_NAME, L3_TABLE_TYPE, L3_BIND_PORTS)
-            yield dvs_acl.get_acl_table_ids(1)[0]
+            dvs_acl.create_acl_table(L3_TABLE_NAME, L3_TABLE_TYPE, L3_BIND_PORTS, stage=request.param)
+            yield dvs_acl.get_acl_table_ids(1)[0], request.param
         finally:
             dvs_acl.remove_acl_table(L3_TABLE_NAME)
             dvs_acl.verify_acl_table_count(0)
@@ -577,6 +577,20 @@ def test_AclTableMandatoryMatchFields(self, dvs, pfcwd_acl_table):
             assert match_in_ports
         else:
             assert not match_in_ports
+
+    def test_AclTableMandatoryRangeFields(self, dvs, l3_acl_table):
+        """
+        The test case is to verify range qualifier is not applied for egress ACL
+        """
+        table_oid, stage = l3_acl_table
+        match_range_qualifier = 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_ACL_RANGE_TYPE" and v == "true":
+                match_range_qualifier = True
+
+        assert not match_range_qualifier
+
 class TestAclCrmUtilization:
     @pytest.fixture(scope="class", autouse=True)
     def configure_crm_polling_interval_for_test(self, dvs):