Fix deleting final ACL from a port

faucet/valve_acl.py

@@ -511,7 +511,7 @@ def del_port(self, port):
         ofmsgs = []
         if self._port_acls_allowed(port):
             in_port_match = self.port_acl_table.match(in_port=port.number)
-            ofmsgs.append(self.port_acl_table.flowdel(in_port_match, self.acl_priority))
+            ofmsgs.append(self.port_acl_table.flowdel(in_port_match))
         return ofmsgs
 
     def cold_start_port(self, port):

faucet/valve_of.py

@@ -1143,7 +1143,7 @@ def _partition_ofmsgs(input_ofmsgs):
 
 
 def _flowmodkey(ofmsg):
-    return (ofmsg.match, ofmsg.cookie, ofmsg.priority, ofmsg.table_id)
+    return (tuple(ofmsg.match.items()), ofmsg.cookie, ofmsg.priority, ofmsg.table_id)
 
 
 def _none_flowmodkey(ofmsg):

tests/unit/faucet/test_valve_config.py

@@ -721,6 +721,93 @@ def test_add_vlan(self):
         self.update_and_revert_config(self.CONFIG, self.MORE_CONFIG, "cold")
 
 
+class ValveAddACLTestCase(ValveTestBases.ValveTestNetwork):
+    """Test addition of an ACL to a port."""
+
+    ACLS = """
+    acl_a:
+      - rule:
+        eth_type: 0x0804
+        actions:
+          allow: 0
+      - rule:
+        actions:
+          allow: 1
+"""
+
+    CONFIG = """
+acls:
+%s
+dps:
+    s1:
+%s
+        interfaces:
+            p1:
+                number: 1
+                native_vlan: 0x100
+                acl_in: acl_a
+            p2:
+                number: 2
+                native_vlan: 0x200
+""" % (
+        ACLS,
+        DP1_CONFIG,
+    )
+
+    ADD_ACL_P2_CONFIG = """
+acls:
+%s
+dps:
+    s1:
+%s
+        interfaces:
+            p1:
+                number: 1
+                native_vlan: 0x100
+                acl_in: acl_a
+            p2:
+                number: 2
+                native_vlan: 0x200
+                acl_in: acl_a
+""" % (
+        ACLS,
+        DP1_CONFIG,
+    )
+
+    def setUp(self):
+        """Setup basic ACL config"""
+        self.setup_valves(self.CONFIG)
+
+    def test_add_port_acl(self):
+        """Test port ACL can be added."""
+        table = self.network.tables[self.DP_ID]
+
+        self.assertFalse(
+            table.is_output({"in_port": 1, "vlan_vid": 0, "eth_type": 0x0804}),
+            msg="Packet not blocked by ACL",
+        )
+        self.assertTrue(
+            table.is_output({"in_port": 2, "vlan_vid": 0, "eth_type": 0x0804}),
+            msg="Packet not allowed by ACL",
+        )
+
+        def verify_func():
+            for port in [1, 2]:
+                self.assertFalse(
+                    table.is_output(
+                        {"in_port": port, "vlan_vid": 0, "eth_type": 0x0804}
+                    ),
+                    msg="Packet not blocked by ACL",
+                )
+
+        self.update_and_revert_config(
+            self.CONFIG,
+            self.ADD_ACL_P2_CONFIG,
+            reload_type="warm",
+            verify_func=verify_func,
+        )
+
+
 class ValveChangeACLTestCase(ValveTestBases.ValveTestNetwork):
     """Test changes to ACL on a port."""
 
@@ -848,6 +935,91 @@ def verify_func():
         verify_func()
 
 
+class ValveDeleteACLTestCase(ValveTestBases.ValveTestNetwork):
+    """Test deletion of an ACL from a port."""
+
+    ACLS = """
+    acl_a:
+      - rule:
+        eth_type: 0x0804
+        actions:
+          allow: 0
+      - rule:
+        actions:
+          allow: 1
+"""
+
+    CONFIG = """
+acls:
+%s
+dps:
+    s1:
+%s
+        interfaces:
+            p1:
+                number: 1
+                native_vlan: 0x100
+                acl_in: acl_a
+            p2:
+                number: 2
+                native_vlan: 0x200
+                acl_in: acl_a
+""" % (
+        ACLS,
+        DP1_CONFIG,
+    )
+
+    DELETE_ACL_P2_CONFIG = """
+acls:
+%s
+dps:
+    s1:
+%s
+        interfaces:
+            p1:
+                number: 1
+                native_vlan: 0x100
+                acl_in: acl_a
+            p2:
+                number: 2
+                native_vlan: 0x200
+""" % (
+        ACLS,
+        DP1_CONFIG,
+    )
+
+    def setUp(self):
+        """Setup basic ACL config"""
+        self.setup_valves(self.CONFIG)
+
+    def test_delete_port_acl(self):
+        """Test port ACL can be deleted."""
+        table = self.network.tables[self.DP_ID]
+
+        for port in [1, 2]:
+            self.assertFalse(
+                table.is_output({"in_port": port, "vlan_vid": 0, "eth_type": 0x0804}),
+                msg="Packet not blocked by ACL",
+            )
+
+        def verify_func():
+            self.assertFalse(
+                table.is_output({"in_port": 1, "vlan_vid": 0, "eth_type": 0x0804}),
+                msg="Packet not blocked by ACL",
+            )
+            self.assertTrue(
+                table.is_output({"in_port": 2, "vlan_vid": 0, "eth_type": 0x0804}),
+                msg="Packet not allowed by ACL",
+            )
+
+        self.update_and_revert_config(
+            self.CONFIG,
+            self.DELETE_ACL_P2_CONFIG,
+            reload_type="warm",
+            verify_func=verify_func,
+        )
+
+
 class ValveChangeMirrorTestCase(ValveTestBases.ValveTestNetwork):
     """Test changes mirroring port."""