[acl] Enable VLAN ID qualifier for ACL rules (sonic-net#1648)

Signed-off-by: Danny Allen <daall@microsoft.com>
theasianpianist · Feb 23, 2021 · 99cfd58 · 99cfd58
1 parent 97b3913
commit 99cfd58
Show file tree

Hide file tree

Showing 7 changed files with 61 additions and 1 deletion.
diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp
@@ -32,6 +32,9 @@ extern sai_object_id_t   gSwitchId;
 extern PortsOrch*        gPortsOrch;
 extern CrmOrch *gCrmOrch;
 
+#define MIN_VLAN_ID 1    // 0 is a reserved VLAN ID
+#define MAX_VLAN_ID 4095 // 4096 is a reserved VLAN ID
+
 acl_rule_attr_lookup_t aclMatchLookup =
 {
     { MATCH_IN_PORTS,          SAI_ACL_ENTRY_ATTR_FIELD_IN_PORTS },
@@ -43,6 +46,7 @@ acl_rule_attr_lookup_t aclMatchLookup =
     { MATCH_L4_SRC_PORT,       SAI_ACL_ENTRY_ATTR_FIELD_L4_SRC_PORT },
     { MATCH_L4_DST_PORT,       SAI_ACL_ENTRY_ATTR_FIELD_L4_DST_PORT },
     { MATCH_ETHER_TYPE,        SAI_ACL_ENTRY_ATTR_FIELD_ETHER_TYPE },
+    { MATCH_VLAN_ID,           SAI_ACL_ENTRY_ATTR_FIELD_OUTER_VLAN_ID },
     { MATCH_IP_PROTOCOL,       SAI_ACL_ENTRY_ATTR_FIELD_IP_PROTOCOL },
     { MATCH_NEXT_HEADER,       SAI_ACL_ENTRY_ATTR_FIELD_IPV6_NEXT_HEADER },
     { MATCH_TCP_FLAGS,         SAI_ACL_ENTRY_ATTR_FIELD_TCP_FLAGS },
@@ -289,6 +293,17 @@ bool AclRule::validateAddMatch(string attr_name, string attr_value)
             value.aclfield.data.u16 = to_uint<uint16_t>(attr_value);
             value.aclfield.mask.u16 = 0xFFFF;
         }
+        else if (attr_name == MATCH_VLAN_ID)
+        {
+            value.aclfield.data.u16 = to_uint<uint16_t>(attr_value);
+            value.aclfield.mask.u16 = 0xFFF;
+
+            if (value.aclfield.data.u16 < MIN_VLAN_ID || value.aclfield.data.u16 > MAX_VLAN_ID)
+            {
+                SWSS_LOG_ERROR("Invalid VLAN ID: %s", attr_value.c_str());
+                return false;
+            }
+        }
         else if (attr_name == MATCH_DSCP)
         {
             /* Support both exact value match and value/mask match */
@@ -1415,6 +1430,10 @@ bool AclTable::create()
         table_attrs.push_back(attr);
     }
 
+    attr.id = SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID;
+    attr.value.booldata = true;
+    table_attrs.push_back(attr);
+
     attr.id = SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE;
     attr.value.booldata = true;
     table_attrs.push_back(attr);

diff --git a/orchagent/aclorch.h b/orchagent/aclorch.h
@@ -50,6 +50,7 @@
 #define MATCH_ETHER_TYPE        "ETHER_TYPE"
 #define MATCH_IP_PROTOCOL       "IP_PROTOCOL"
 #define MATCH_NEXT_HEADER       "NEXT_HEADER"
+#define MATCH_VLAN_ID           "VLAN_ID"
 #define MATCH_TCP_FLAGS         "TCP_FLAGS"
 #define MATCH_IP_TYPE           "IP_TYPE"
 #define MATCH_DSCP              "DSCP"

diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp
@@ -106,6 +106,7 @@ namespace aclorch_test
         auto v = vector<swss::FieldValueTuple>(
             { { "SAI_ACL_TABLE_ATTR_ACL_BIND_POINT_TYPE_LIST", "2:SAI_ACL_BIND_POINT_TYPE_PORT,SAI_ACL_BIND_POINT_TYPE_LAG" },
               { "SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE", "true" },
+              { "SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID", "true" },
               { "SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE", "true" },
               { "SAI_ACL_TABLE_ATTR_FIELD_IP_PROTOCOL", "true" },
               { "SAI_ACL_TABLE_ATTR_FIELD_SRC_IP", "true" },
@@ -420,8 +421,8 @@ namespace aclorch_test
             vector<swss::FieldValueTuple> fields;
 
             fields.push_back({ "SAI_ACL_TABLE_ATTR_ACL_BIND_POINT_TYPE_LIST", "2:SAI_ACL_BIND_POINT_TYPE_PORT,SAI_ACL_BIND_POINT_TYPE_LAG" });
+            fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID", "true" });
             fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_ACL_IP_TYPE", "true" });
-
             fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_L4_SRC_PORT", "true" });
             fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT", "true" });
             fields.push_back({ "SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS", "true" });

diff --git a/tests/test_acl.py b/tests/test_acl.py
@@ -135,6 +135,18 @@ def test_AclRuleOutPortsNonExistingInterface(self, dvs_acl, l3_acl_table):
         dvs_acl.verify_no_acl_rules()
         dvs_acl.remove_acl_rule(L3_TABLE_NAME, L3_RULE_NAME)
 
+    def test_AclRuleVlanId(self, dvs_acl, l3_acl_table):
+        config_qualifiers = {"VLAN_ID": "100"}
+        expected_sai_qualifiers = {
+            "SAI_ACL_ENTRY_ATTR_FIELD_OUTER_VLAN_ID": dvs_acl.get_simple_qualifier_comparator("100&mask:0xfff")
+        }
+
+        dvs_acl.create_acl_rule(L3_TABLE_NAME, L3_RULE_NAME, config_qualifiers)
+        dvs_acl.verify_acl_rule(expected_sai_qualifiers)
+
+        dvs_acl.remove_acl_rule(L3_TABLE_NAME, L3_RULE_NAME)
+        dvs_acl.verify_no_acl_rules()
+
     def test_V6AclTableCreationDeletion(self, dvs_acl):
         try:
             dvs_acl.create_acl_table(L3V6_TABLE_NAME,
@@ -288,6 +300,18 @@ def test_V6AclRuleL4DstPortRange(self, dvs_acl, l3v6_acl_table):
         dvs_acl.remove_acl_rule(L3V6_TABLE_NAME, L3V6_RULE_NAME)
         dvs_acl.verify_no_acl_rules()
 
+    def test_V6AclRuleVlanId(self, dvs_acl, l3v6_acl_table):
+        config_qualifiers = {"VLAN_ID": "100"}
+        expected_sai_qualifiers = {
+            "SAI_ACL_ENTRY_ATTR_FIELD_OUTER_VLAN_ID": dvs_acl.get_simple_qualifier_comparator("100&mask:0xfff")
+        }
+
+        dvs_acl.create_acl_rule(L3V6_TABLE_NAME, L3V6_RULE_NAME, config_qualifiers)
+        dvs_acl.verify_acl_rule(expected_sai_qualifiers)
+
+        dvs_acl.remove_acl_rule(L3V6_TABLE_NAME, L3V6_RULE_NAME)
+        dvs_acl.verify_no_acl_rules()
+
     def test_InsertAclRuleBetweenPriorities(self, dvs_acl, l3_acl_table):
         rule_priorities = ["10", "20", "30", "40"]
 

diff --git a/tests/test_acl_egress_table.py b/tests/test_acl_egress_table.py
@@ -137,6 +137,18 @@ def test_EgressAclInnerL4DstPort(self, dvs_acl, egress_acl_table):
         dvs_acl.remove_acl_rule(TABLE_NAME, RULE_NAME)
         dvs_acl.verify_no_acl_rules()
 
+    def test_AclRuleVlanId(self, dvs_acl, egress_acl_table):
+        config_qualifiers = {"VLAN_ID": "100"}
+        expected_sai_qualifiers = {
+            "SAI_ACL_ENTRY_ATTR_FIELD_OUTER_VLAN_ID": dvs_acl.get_simple_qualifier_comparator("100&mask:0xfff")
+        }
+
+        dvs_acl.create_acl_rule(TABLE_NAME, RULE_NAME, config_qualifiers, action="DROP", priority="1000")
+        dvs_acl.verify_acl_rule(expected_sai_qualifiers, action="DROP", priority="1000")
+
+        dvs_acl.remove_acl_rule(TABLE_NAME, RULE_NAME)
+        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

diff --git a/tests/test_mirror_ipv6_combined.py b/tests/test_mirror_ipv6_combined.py
@@ -172,6 +172,7 @@ def test_CombinedMirrorTableCreation(self, dvs, testlog):
             "SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS",
             "SAI_ACL_TABLE_ATTR_FIELD_DSCP",
             "SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE",
+            "SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID",
             "SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS",
             "SAI_ACL_TABLE_ATTR_FIELD_SRC_IPV6",
             "SAI_ACL_TABLE_ATTR_FIELD_DST_IPV6",

diff --git a/tests/test_mirror_ipv6_separate.py b/tests/test_mirror_ipv6_separate.py
@@ -169,6 +169,7 @@ def test_MirrorTableCreation(self, dvs, testlog):
             "SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS",
             "SAI_ACL_TABLE_ATTR_FIELD_DSCP",
             "SAI_ACL_TABLE_ATTR_FIELD_ETHER_TYPE",
+            "SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID",
             "SAI_ACL_TABLE_ATTR_FIELD_IN_PORTS",
         ]
 
@@ -236,6 +237,7 @@ def test_MirrorV6TableCreation(self, dvs, testlog):
             "SAI_ACL_TABLE_ATTR_FIELD_L4_DST_PORT",
             "SAI_ACL_TABLE_ATTR_FIELD_TCP_FLAGS",
             "SAI_ACL_TABLE_ATTR_FIELD_DSCP",
+            "SAI_ACL_TABLE_ATTR_FIELD_OUTER_VLAN_ID"
         ]
 
         expected_sai_list_attributes = [