From 5b404cf7d308b2fdbfedb62509ff86945f12008b Mon Sep 17 00:00:00 2001 From: Brad Cowie Date: Wed, 3 Jun 2026 14:11:00 +1200 Subject: [PATCH 1/2] Fix deleting final ACL from a port --- faucet/valve_acl.py | 2 +- tests/unit/faucet/test_valve_config.py | 172 +++++++++++++++++++++++++ 2 files changed, 173 insertions(+), 1 deletion(-) diff --git a/faucet/valve_acl.py b/faucet/valve_acl.py index df25e19bf1..b85f6ea2f6 100644 --- a/faucet/valve_acl.py +++ b/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): diff --git a/tests/unit/faucet/test_valve_config.py b/tests/unit/faucet/test_valve_config.py index 81ff0ad86f..fd4414d0b0 100755 --- a/tests/unit/faucet/test_valve_config.py +++ b/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.""" From f5c5b725cd224d6e648ced99a9d7cda7d87f5bf7 Mon Sep 17 00:00:00 2001 From: Brad Cowie Date: Thu, 4 Jun 2026 16:10:09 +1200 Subject: [PATCH 2/2] Make tuple returned by _flowmodkey() comparable --- faucet/valve_of.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/faucet/valve_of.py b/faucet/valve_of.py index 83450d61e6..4892dba2d5 100644 --- a/faucet/valve_of.py +++ b/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):