[dvs] Refactor acl_ctrl and acl_egress to use dvslib (sonic-net#1416)

Signed-off-by: Danny Allen <daall@microsoft.com>

tests/conftest.py

@@ -1494,6 +1494,8 @@ def dvs_acl(request, dvs) -> DVSAcl:
                   dvs.get_state_db(),
                   dvs.get_counters_db())
 
+# FIXME: The rest of these also need to be reverted back to normal fixtures to
+# appease the linter.
 @pytest.yield_fixture(scope="class")
 def dvs_lag_manager(request, dvs):
     request.cls.dvs_lag = dvs_lag.DVSLag(dvs.get_asic_db(),

tests/dvslib/dvs_acl.py

@@ -31,6 +31,11 @@ class DVSAcl:
         "egress": "SAI_ACL_ENTRY_ATTR_ACTION_MIRROR_EGRESS"
     }
 
+    ADB_PORT_ATTR_LOOKUP = {
+        "ingress": "SAI_PORT_ATTR_INGRESS_ACL",
+        "egress": "SAI_PORT_ATTR_EGRESS_ACL"
+    }
+
     def __init__(self, asic_db, config_db, state_db, counters_db):
         """Create a new DVS ACL Manager."""
         self.asic_db = asic_db
@@ -64,6 +69,25 @@ def create_acl_table(
 
         self.config_db.create_entry(self.CDB_ACL_TABLE_NAME, table_name, table_attrs)
 
+    def create_control_plane_acl_table(
+            self,
+            table_name: str,
+            services: List[str]
+    ) -> None:
+        """Create a new Control Plane ACL table in Config DB.
+
+        Args:
+            table_name: The name for the new ACL table.
+            services: A list of services to bind to the ACL table.
+        """
+        table_attrs = {
+            "policy_desc": table_name,
+            "type": "CTRLPLANE",
+            "services": ",".join(services)
+        }
+
+        self.config_db.create_entry(self.CDB_ACL_TABLE_NAME, table_name, table_attrs)
+
     def update_acl_table_port_list(self, table_name: str, ports: List[str]) -> None:
         """Update the port binding list for a given ACL table.
 
@@ -172,13 +196,20 @@ def verify_acl_table_group_members(self, acl_table_id: str, acl_table_group_ids:
 
         assert set(member_groups) == set(acl_table_group_ids)
 
-    def verify_acl_table_port_binding(self, acl_table_id: str, bind_ports: List[str], num_tables: int) -> None:
+    def verify_acl_table_port_binding(
+            self,
+            acl_table_id: str,
+            bind_ports: List[str],
+            num_tables: int,
+            stage: str = "ingress"
+    ) -> None:
         """Verify that the ACL table has been bound to the given list of ports.
 
         Args:
             acl_table_id: The ACL table that is being checked.
             bind_ports: The ports that should be bound to the given ACL table.
             num_tables: The total number of ACL tables in ASIC DB.
+            stage: The stage of the ACL table that was created.
         """
         acl_table_group_ids = self.asic_db.wait_for_n_keys(self.ADB_ACL_GROUP_TABLE_NAME, len(bind_ports))
 
@@ -187,7 +218,7 @@ def verify_acl_table_port_binding(self, acl_table_id: str, bind_ports: List[str]
             port_oid = self.counters_db.get_entry("COUNTERS_PORT_NAME_MAP", "").get(port)
             fvs = self.asic_db.wait_for_entry("ASIC_STATE:SAI_OBJECT_TYPE_PORT", port_oid)
 
-            acl_table_group_id = fvs.pop("SAI_PORT_ATTR_INGRESS_ACL", None)
+            acl_table_group_id = fvs.pop(self.ADB_PORT_ATTR_LOOKUP[stage], None)
             assert acl_table_group_id in acl_table_group_ids
             port_groups.append(acl_table_group_id)
 

tests/dvslib/dvs_database.py

@@ -1,4 +1,10 @@
-"""Utilities for interacting with redis when writing VS tests."""
+"""Utilities for interacting with redis when writing VS tests.
+
+FIXME:
+    - Reference DBs by name rather than ID/socket
+    - Move DEFAULT_POLLING_CONFIG to Common
+    - Add support for ProducerStateTable
+"""
 from typing import Dict, List
 from swsscommon import swsscommon
 from dvslib.dvs_common import wait_for_result, PollingConfig

tests/test_acl.py

@@ -11,7 +11,7 @@
 L3V6_RULE_NAME = "L3V6_TEST_RULE"
 
 
-class TestAcl():
+class TestAcl:
     @pytest.yield_fixture
     def l3_acl_table(self, dvs_acl):
         try:
@@ -57,6 +57,7 @@ def test_AclTableCreationDeletion(self, dvs_acl):
 
             acl_table_id = dvs_acl.get_acl_table_ids(1)[0]
             acl_table_group_ids = dvs_acl.get_acl_table_group_ids(len(L3_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, L3_BIND_PORTS, 1)
         finally:
@@ -413,7 +414,7 @@ def test_AclRuleRedirect(self, dvs, dvs_acl, l3_acl_table, setup_teardown_neighb
         dvs_acl.verify_no_acl_rules()
 
 
-class TestAclRuleValidation():
+class TestAclRuleValidation:
     """Test class for cases that check if orchagent corectly validates ACL rules input."""
 
     SWITCH_CAPABILITY_TABLE = "SWITCH_CAPABILITY"

tests/test_acl_ctrl.py

@@ -1,74 +1,24 @@
-import time
-import pytest
+TABLE_NAME = "CTRL_ACL_TEST"
+RULE_NAME = "CTRL_ACL_TEST_RULE"
 
-from swsscommon import swsscommon
 
+class TestPortChannelAcl:
+    def test_AclCtrl(self, dvs_acl):
+        # Create ACL table and ACL rule
+        dvs_acl.create_control_plane_acl_table(TABLE_NAME, ["SNMP"])
+        dvs_acl.create_acl_rule(TABLE_NAME, RULE_NAME, {"L4_SRC_PORT": "8888"}, priority="88")
 
-class TestPortChannelAcl(object):
-    def setup_db(self, dvs):
-        self.pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0)
-        self.adb = swsscommon.DBConnector(1, dvs.redis_sock, 0)
-        self.cdb = swsscommon.DBConnector(4, dvs.redis_sock, 0)
+        # Verify that no ASIC rules are created
+        dvs_acl.verify_acl_table_count(0)
+        dvs_acl.verify_no_acl_rules()
 
-    def create_acl_table(self, dvs):
-        tbl = swsscommon.Table(self.cdb, "ACL_TABLE")
-        fvs = swsscommon.FieldValuePairs([("POLICY_DESC", "CTRL_ACL_TEST"),
-                                          ("TYPE", "CTRLPLANE"),
-                                          ("SERVICES@", "SNMP")])
-        tbl.set("CTRL_ACL_TABLE", fvs)
-        time.sleep(1)
-
-    def remove_acl_table(self, dvs):
-        tbl = swsscommon.Table(self.cdb, "ACL_TABLE")
-        tbl._del("CTRL_ACL_TABLE")
-        time.sleep(1)
-
-    def create_acl_rule(self, dvs):
-        tbl = swsscommon.Table(self.cdb, "ACL_RULE")
-        fvs = swsscommon.FieldValuePairs([("PRIORITY", "88"),
-                                          ("PACKET_ACTION", "FORWARD"),
-                                          ("L4_SRC_PORT", "8888")])
-        tbl.set("CTRL_ACL_TABLE|CTRL_ACL_RULE", fvs)
-        time.sleep(1)
-
-    def remove_acl_rule(self, dvs):
-        tbl = swsscommon.Table(self.cdb, "ACL_RULE")
-        tbl._del("CTRL_ACL_TABLE|CTRL_ACL_RULE")
-        time.sleep(1)
-
-    def check_asic_table_absent(self, dvs):
-        tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_TABLE")
-        acl_tables = tbl.getKeys()
-        for key in dvs.asicdb.default_acl_tables:
-            assert key in acl_tables
-        acl_tables = [k for k in acl_tables if k not in dvs.asicdb.default_acl_tables]
-
-        assert len(acl_tables) == 0
-
-    def check_asic_rule_absent(self, dvs):
-        tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ACL_ENTRY")
-        acl_entries = tbl.getKeys()
-        for key in dvs.asicdb.default_acl_entries:
-            assert key in acl_entries
-        acl_entries = [k for k in acl_entries if k not in dvs.asicdb.default_acl_entries]
-
-        assert len(acl_entries) == 0
-
-    def test_AclCtrl(self, dvs):
-        self.setup_db(dvs)
-
-        # create ACL table and ACL rule
-        self.create_acl_table(dvs)
-        self.create_acl_rule(dvs)
-
-        # check ASIC table
-        self.check_asic_table_absent(dvs)
-        self.check_asic_rule_absent(dvs)
-
-        # remove ACL table
-        self.remove_acl_table(dvs)
-        self.remove_acl_rule(dvs)
+        # Cleanup ACL data from Config DB
+        dvs_acl.remove_acl_rule(TABLE_NAME, RULE_NAME)
+        dvs_acl.remove_acl_table(TABLE_NAME)
 
+        # Verify that the ASIC DB is clean
+        dvs_acl.verify_acl_table_count(0)
+        dvs_acl.verify_no_acl_rules()
 
 
 # Add Dummy always-pass test at end as workaroud