Address review comments

Add validations to allow mixing on IPv4 and IPv fields in the
same ACL rule.
Fix cases in the logs.

Author: rck-innovium

orchagent/aclorch.cpp

@@ -3109,7 +3109,7 @@ void AclOrch::init(vector<TableConnector>& connectors, PortsOrch *portOrch, Mirr
             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]",
+    SWSS_LOG_NOTICE("    TABLE_TYPE_L3V4V6: Ingress [%s], Egress [%s]",
             m_L3V4V6Capability[ACL_STAGE_INGRESS] ? "yes" : "no",
             m_L3V4V6Capability[ACL_STAGE_EGRESS] ? "yes" : "no");
 
@@ -3248,6 +3248,7 @@ 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<AclTableMatch>(SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS))
             .build()
     );
 
@@ -4576,6 +4577,8 @@ void AclOrch::doAclRuleTask(Consumer &consumer)
             }
             bool bHasTCPFlag = false;
             bool bHasIPProtocol = false;
+            bool bHasIPV4 = false;
+            bool bHasIPV6 = false;
             for (const auto& itr : kfvFieldsValues(t))
             {
                 string attr_name = to_upper(fvField(itr));
@@ -4586,6 +4589,14 @@ void AclOrch::doAclRuleTask(Consumer &consumer)
                 {
                     bHasTCPFlag = true;
                 }
+                if (attr_name == MATCH_SRC_IP || attr_name == MATCH_DST_IP)
+                {
+                    bHasIPV4 = true;
+                }
+                if (attr_name == MATCH_SRC_IPV6 || attr_name == MATCH_DST_IPV6)
+                {
+                    bHasIPV6 = true;
+                }
                 if (attr_name == MATCH_IP_PROTOCOL || attr_name == MATCH_NEXT_HEADER)
                 {
                     bHasIPProtocol = true;
@@ -4634,6 +4645,15 @@ void AclOrch::doAclRuleTask(Consumer &consumer)
                 }
             }
 
+            if (bHasIPV4 && bHasIPV6)
+	    {
+		    if (type == TABLE_TYPE_L3V4V6)
+		    {
+			    SWSS_LOG_ERROR("Rule '%s' is invalid since it has both v4 and v6 matchfields.", rule_id.c_str());
+			    bAllAttributesOk = false;
+		    }
+	    }
+
             // validate and create ACL rule
             if (bAllAttributesOk && newRule->validate())
             {

tests/test_acl_l3v4v6.py

@@ -39,7 +39,6 @@ def setup_teardown_neighbor(self, dvs):
 
     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]
@@ -55,6 +54,44 @@ def test_L3V4V6AclTableCreationDeletion(self, dvs_acl):
             # Verify the STATE_DB entry is removed
             dvs_acl.verify_acl_table_status(L3V4V6_TABLE_NAME, None)
 
+    def test_ValidAclRuleCreation_sip_dip(self, dvs_acl, l3v4v6_acl_table):
+        config_qualifiers = {"DST_IP": "20.0.0.1/32",
+                             "SRC_IP": "10.0.0.0/32"};
+
+        dvs_acl.create_acl_rule(L3V4V6_TABLE_NAME, "VALID_RULE", config_qualifiers)
+        # Verify status is written into STATE_DB
+        dvs_acl.verify_acl_rule_status(L3V4V6_TABLE_NAME, "VALID_RULE", "Active")
+
+        dvs_acl.remove_acl_rule(L3V4V6_TABLE_NAME, "VALID_RULE")
+        # Verify the STATE_DB entry is removed
+        dvs_acl.verify_acl_rule_status(L3V4V6_TABLE_NAME, "VALID_RULE", None)
+        dvs_acl.verify_no_acl_rules()
+
+    def test_InvalidAclRuleCreation_sip_sipv6(self, dvs_acl, l3v4v6_acl_table):
+        config_qualifiers = {"SRC_IPV6": "2777::0/64",
+                             "SRC_IP": "10.0.0.0/32"};
+
+        dvs_acl.create_acl_rule(L3V4V6_TABLE_NAME, "INVALID_RULE", config_qualifiers)
+        # Verify status is written into STATE_DB
+        dvs_acl.verify_acl_rule_status(L3V4V6_TABLE_NAME, "INVALID_RULE", "Inactive")
+
+        dvs_acl.remove_acl_rule(L3V4V6_TABLE_NAME, "INVALID_RULE")
+        # Verify the STATE_DB entry is removed
+        dvs_acl.verify_acl_rule_status(L3V4V6_TABLE_NAME, "INVALID_RULE", None)
+        dvs_acl.verify_no_acl_rules()
+
+    def test_InvalidAclRuleCreation_dip_sipv6(self, dvs_acl, l3v4v6_acl_table):
+        config_qualifiers = {"SRC_IPV6": "2777::0/64",
+                             "DST_IP": "10.0.0.0/32"};
+
+        dvs_acl.create_acl_rule(L3V4V6_TABLE_NAME, "INVALID_RULE", config_qualifiers)
+        # Verify status is written into STATE_DB
+        dvs_acl.verify_acl_rule_status(L3V4V6_TABLE_NAME, "INVALID_RULE", "Inactive")
+
+        dvs_acl.remove_acl_rule(L3V4V6_TABLE_NAME, "INVALID_RULE")
+        # Verify the STATE_DB entry is removed
+        dvs_acl.verify_acl_rule_status(L3V4V6_TABLE_NAME, "INVALID_RULE", None)
+        dvs_acl.verify_no_acl_rules()
 
 # Add Dummy always-pass test at end as workaroud
 # for issue when Flaky fail on final test it invokes module tear-down before retrying