Combine v4 and v6 L3 ACL rules on optimized platforms sonic-net#1267

Support a new ACL table type called L3V4V6.
This table supports both v4 and v6 Match types.
Add unit tests for this new ACL table type.

HLD: sonic-net/SONiC#1267

Signed-off-by: Ravi(Marvell) [email protected]

orchagent/aclorch.cpp

@@ -34,6 +34,7 @@ extern CrmOrch *gCrmOrch;
 
 #define STATE_DB_ACL_ACTION_FIELD_IS_ACTION_LIST_MANDATORY "is_action_list_mandatory"
 #define STATE_DB_ACL_ACTION_FIELD_ACTION_LIST              "action_list"
+#define STATE_DB_ACL_L3V4V6_SUPPORTED                      "supported_L3V4V6"
 #define COUNTERS_ACL_COUNTER_RULE_MAP "ACL_COUNTER_RULE_MAP"
 
 #define ACL_COUNTER_DEFAULT_POLLING_INTERVAL_MS 10000 // ms
@@ -202,6 +203,26 @@ static acl_table_action_list_lookup_t defaultAclActionList =
             }
         }
     },
+    {
+        // L3V4V6
+        TABLE_TYPE_L3V4V6,
+        {
+            {
+                ACL_STAGE_INGRESS,
+                {
+                    SAI_ACL_ACTION_TYPE_PACKET_ACTION,
+                    SAI_ACL_ACTION_TYPE_REDIRECT
+                }
+            },
+            {
+                ACL_STAGE_EGRESS,
+                {
+                    SAI_ACL_ACTION_TYPE_PACKET_ACTION,
+                    SAI_ACL_ACTION_TYPE_REDIRECT
+                }
+            }
+        }
+    },
     {
         // MIRROR
         TABLE_TYPE_MIRROR,
@@ -2292,6 +2313,16 @@ bool AclTable::validate()
         return false;
     }
 
+    if (type.getName() == TABLE_TYPE_L3V4V6)
+    {
+	    if (!m_pAclOrch->isAclL3V4V6TableSupported(stage))
+	    {
+		    return false;
+	    }
+    }
+
+
+
     if (m_pAclOrch->isAclActionListMandatoryOnTableCreation(stage))
     {
         if (type.getActions().empty())
@@ -3049,11 +3080,36 @@ void AclOrch::init(vector<TableConnector>& connectors, PortsOrch *portOrch, Mirr
         };
     }
 
+    if ( platform == MRVL_PLATFORM_SUBSTRING ||
+            platform == INVM_PLATFORM_SUBSTRING ||
+            platform == VS_PLATFORM_SUBSTRING)
+    {
+	m_L3V4V6Capability =
+        {
+		{ACL_STAGE_INGRESS, true},
+		{ACL_STAGE_EGRESS, true},
+	};
+    }
+    else
+    {
+	m_L3V4V6Capability =
+        {
+		{ACL_STAGE_INGRESS, false},
+		{ACL_STAGE_EGRESS, false},
+	};
+
+    }
+
+
     SWSS_LOG_NOTICE("%s switch capability:", platform.c_str());
     SWSS_LOG_NOTICE("    TABLE_TYPE_MIRROR: %s",
             m_mirrorTableCapabilities[TABLE_TYPE_MIRROR] ? "yes" : "no");
     SWSS_LOG_NOTICE("    TABLE_TYPE_MIRRORV6: %s",
             m_mirrorTableCapabilities[TABLE_TYPE_MIRRORV6] ? "yes" : "no");
+    SWSS_LOG_NOTICE("    TABLE_TYPE_L3v4V6: Ingress [%s], Egress [%s]",
+            m_L3V4V6Capability[ACL_STAGE_INGRESS] ? "yes" : "no",
+            m_L3V4V6Capability[ACL_STAGE_EGRESS] ? "yes" : "no");
+
 
     // In Mellanox platform, V4 and V6 rules are stored in different tables
     // In Broadcom DNX platform also, V4 and V6 rules are stored in different tables
@@ -3168,6 +3224,30 @@ void AclOrch::initDefaultTableTypes()
             .build()
     );
 
+
+    addAclTableType(
+        builder.withName(TABLE_TYPE_L3V4V6)
+            .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT)
+            .withBindPointType(SAI_ACL_BIND_POINT_TYPE_LAG)
+            .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE))
+            .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID))
+            .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE))
+            .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_SRC_IP))
+            .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_DST_IP))
+            .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ICMP_TYPE))
+            .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ICMP_CODE))
+            .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL))
+            .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6))
+            .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6))
+            .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_CODE))
+            .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_ICMPV6_TYPE))
+            .withMatch(make_shared<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_IPV6_NEXT_HEADER))
+            .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))
+            .build()
+    );
+
     addAclTableType(
         builder.withName(TABLE_TYPE_MCLAG)
             .withBindPointType(SAI_ACL_BIND_POINT_TYPE_PORT)
@@ -3408,10 +3488,21 @@ void AclOrch::putAclActionCapabilityInDB(acl_stage_type_t stage)
         }
     }
 
+
     is_action_list_mandatory_stream << boolalpha << capabilities.isActionListMandatoryOnTableCreation;
 
     fvVector.emplace_back(STATE_DB_ACL_ACTION_FIELD_IS_ACTION_LIST_MANDATORY, is_action_list_mandatory_stream.str());
     fvVector.emplace_back(STATE_DB_ACL_ACTION_FIELD_ACTION_LIST, acl_action_value_stream.str());
+
+    for (auto const& it : m_L3V4V6Capability)
+    {
+        string value = it.second ? "true" : "false";
+        if (it.first == stage)
+        {
+            fvVector.emplace_back(STATE_DB_ACL_L3V4V6_SUPPORTED, value);
+        }
+    }
+
     m_aclStageCapabilityTable.set(stage_str, fvVector);
 }
 
@@ -4219,6 +4310,16 @@ bool AclOrch::isAclActionListMandatoryOnTableCreation(acl_stage_type_t stage) co
     return it->second.isActionListMandatoryOnTableCreation;
 }
 
+bool AclOrch::isAclL3V4V6TableSupported(acl_stage_type_t stage) const
+{
+    const auto& it = m_L3V4V6Capability.find(stage);
+    if (it == m_L3V4V6Capability.cend())
+    {
+        return false;
+    }
+    return it->second;
+}
+
 bool AclOrch::isAclActionSupported(acl_stage_type_t stage, sai_acl_action_type_t action) const
 {
     const auto& it = m_aclCapabilities.find(stage);

orchagent/aclorch.h

@@ -502,12 +502,14 @@ class AclOrch : public Orch, public Observer
     bool isAclMirrorV6Supported() const;
     bool isAclMirrorV4Supported() const;
     bool isAclMirrorTableSupported(string type) const;
+    bool isAclL3V4V6TableSupported(acl_stage_type_t stage) const;
     bool isAclActionListMandatoryOnTableCreation(acl_stage_type_t stage) const;
     bool isAclActionSupported(acl_stage_type_t stage, sai_acl_action_type_t action) const;
     bool isAclActionEnumValueSupported(sai_acl_action_type_t action, sai_acl_action_parameter_t param) const;
 
     bool m_isCombinedMirrorV6Table = true;
     map<string, bool> m_mirrorTableCapabilities;
+    map<acl_stage_type_t, bool> m_L3V4V6Capability;
 
     void registerFlexCounter(const AclRule& rule);
     void deregisterFlexCounter(const AclRule& rule);

orchagent/acltable.h

@@ -25,6 +25,7 @@ extern "C" {
 
 #define TABLE_TYPE_L3                   "L3"
 #define TABLE_TYPE_L3V6                 "L3V6"
+#define TABLE_TYPE_L3V4V6               "L3V4V6"
 #define TABLE_TYPE_MIRROR               "MIRROR"
 #define TABLE_TYPE_MIRRORV6             "MIRRORV6"
 #define TABLE_TYPE_MIRROR_DSCP          "MIRROR_DSCP"

tests/test_acl_l3v4v6.py

@@ -0,0 +1,62 @@
+import pytest
+from requests import request
+
+L3V4V6_TABLE_TYPE = "L3V4V6"
+L3V4V6_TABLE_NAME = "L3_V4V6_TEST"
+L3V4V6_BIND_PORTS = ["Ethernet0", "Ethernet4", "Ethernet8"]
+L3V4V6_RULE_NAME = "L3V4V6_TEST_RULE"
+
+class TestAcl:
+    @pytest.fixture
+    def l3v4v6_acl_table(self, dvs_acl):
+        try:
+            dvs_acl.create_acl_table(L3V4V6_TABLE_NAME,
+                                     L3V4V6_TABLE_TYPE,
+                                     L3V4V6_BIND_PORTS)
+            yield dvs_acl.get_acl_table_ids(1)[0]
+        finally:
+            dvs_acl.remove_acl_table(L3V4V6_TABLE_NAME)
+            dvs_acl.verify_acl_table_count(0)
+
+    @pytest.fixture
+    def setup_teardown_neighbor(self, dvs):
+        try:
+            # NOTE: set_interface_status has a dependency on cdb within dvs,
+            # so we still need to setup the db. This should be refactored.
+            dvs.setup_db()
+
+            # Bring up an IP interface with a neighbor
+            dvs.set_interface_status("Ethernet4", "up")
+            dvs.add_ip_address("Ethernet4", "10.0.0.1/24")
+            dvs.add_neighbor("Ethernet4", "10.0.0.2", "00:01:02:03:04:05")
+
+            yield dvs.get_asic_db().wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP", 1)[0]
+        finally:
+            # Clean up the IP interface and neighbor
+            dvs.remove_neighbor("Ethernet4", "10.0.0.2")
+            dvs.remove_ip_address("Ethernet4", "10.0.0.1/24")
+            dvs.set_interface_status("Ethernet4", "down")
+
+    def test_L3V4V6AclTableCreationDeletion(self, dvs_acl):
+        try:
+            # Create an L3V4V6 ACL table with default ACL actions
+            dvs_acl.create_acl_table(L3V4V6_TABLE_NAME, L3V4V6_TABLE_TYPE, L3V4V6_BIND_PORTS)
+
+            acl_table_id = dvs_acl.get_acl_table_ids(1)[0]
+            acl_table_group_ids = dvs_acl.get_acl_table_group_ids(len(L3V4V6_BIND_PORTS))
+
+            dvs_acl.verify_acl_table_group_members(acl_table_id, acl_table_group_ids, 1)
+            dvs_acl.verify_acl_table_port_binding(acl_table_id, L3V4V6_BIND_PORTS, 1)
+            # Verify status is written into STATE_DB
+            dvs_acl.verify_acl_table_status(L3V4V6_TABLE_NAME, "Active")
+        finally:
+            dvs_acl.remove_acl_table(L3V4V6_TABLE_NAME)
+            dvs_acl.verify_acl_table_count(0)
+            # Verify the STATE_DB entry is removed
+            dvs_acl.verify_acl_table_status(L3V4V6_TABLE_NAME, None)
+
+
+# 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