From 7b9656aba290cc02265f2091e60468662108f652 Mon Sep 17 00:00:00 2001 From: hieunt79 Date: Mon, 10 Nov 2025 19:20:48 +0700 Subject: [PATCH 1/7] feat: update VLAN ACL change detection to warm reload --- faucet/dp.py | 151 +++++++++++++++++-------- faucet/faucet.py | 9 +- faucet/valve.py | 63 ++++++++++- faucet/valves_manager.py | 8 +- tests/unit/faucet/test_valve_config.py | 2 +- 5 files changed, 176 insertions(+), 57 deletions(-) diff --git a/faucet/dp.py b/faucet/dp.py index 697a579583..acb60aea69 100644 --- a/faucet/dp.py +++ b/faucet/dp.py @@ -1484,7 +1484,9 @@ def _get_vlan_config_changes(self, logger, new_dp, changed_acls): Returns: changes (tuple) of: deleted_vlans (set): deleted VLAN IDs. - changed_vlans (set): changed/added VLAN IDs. + changed_vlans (set): changed VLAN IDs. + added_vlans (set): added VLAN IDs. + changed_acl_vlans (set): changed/added VLAN ACL VLAN IDs. """ ( _, @@ -1501,16 +1503,33 @@ def _get_vlan_config_changes(self, logger, new_dp, changed_acls): diff=True, ignore_keys=frozenset(["acls_in"]), ) - changed_vlans = added_vlans.union(changed_vlans) - # TODO: optimize for warm start. + logger.debug(""" + _: {}, + deleted_vlans: {}, + added_vlans: {}, + changed_vlans: {}, + same_vlans: {}, + _: {}, + """.format( + _, + deleted_vlans, + added_vlans, + changed_vlans, + same_vlans, + _, + )) + # changed_vlans = added_vlans.union(changed_vlans) + # DOING: optimize for warm start. + changed_acl_vlans = set() for vlan_id in same_vlans: old_vlan = self.vlans[vlan_id] new_vlan = new_dp.vlans[vlan_id] if self._acl_ref_changes( "VLAN %u" % vlan_id, old_vlan, new_vlan, changed_acls, logger ): - changed_vlans.add(vlan_id) - return (deleted_vlans, changed_vlans) + changed_acl_vlans.add(vlan_id) + logger.debug("deleted_vlans: {}, changed_vlans: {}".format(deleted_vlans, changed_vlans)) + return (deleted_vlans, changed_vlans, added_vlans, changed_acl_vlans) def _acl_ref_changes(self, conf_desc, old_conf, new_conf, changed_acls, logger): changed = False @@ -1522,6 +1541,7 @@ def _acl_ref_changes(self, conf_desc, old_conf, new_conf, changed_acls, logger): old_acl_ids = old_conf.acls_in if old_acl_ids: old_acl_ids = [acl._id for acl in old_acl_ids] + logger.debug("conf_desc: {}, old_acl_ids: {}, new_acl_ids: {}, conf_acls_changed: {}".format(conf_desc, old_acl_ids, new_acl_ids, conf_acls_changed)) if conf_acls_changed: changed = True logger.info( @@ -1536,7 +1556,7 @@ def _acl_ref_changes(self, conf_desc, old_conf, new_conf, changed_acls, logger): return changed def _get_port_config_changes( - self, logger, new_dp, changed_vlans, deleted_vlans, changed_acls + self, logger, new_dp, changed_vlans, added_vlans, deleted_vlans, changed_acls ): """Detect any config changes to ports. @@ -1570,6 +1590,27 @@ def _get_port_config_changes( diff=True, ignore_keys=frozenset(["acls_in"]), ) + logger.debug(""" + _: {}, + deleted_ports: {}, + added_ports: {}, + changed_ports: {}, + same_ports: {}, + changed_acls: {}, + """.format( + _, + deleted_ports, + added_ports, + changed_ports, + same_ports, + changed_acls, + )) + # What is port change? + # - Detect port added/deleted/changed by using _get_conf_changes + # - If port added or deleted, their related opfmgs are processed later by Valve._apply_config_changes() + # - Port other config changed like vlan id or port ACL? + # - Vlan membership added / changed / deleted? -> go through all vlan changes to find affected ports + # - Port ACL changed changed_acl_ports = set() all_ports_changed = False @@ -1590,53 +1631,47 @@ def _get_port_config_changes( if not same_ports: all_ports_changed = True - # TODO: optimize case where only VLAN ACL changed. - elif changed_vlans: + # DOING: optimize case where only VLAN ACL changed. + # Separately handle VLAN changes to detect port changes. + if changed_vlans: all_ports = frozenset(new_dp.ports.keys()) + logger.debug("VLANs changed: %s" % changed_vlans) + logger.debug("all_ports: %s" % all_ports) new_changed_vlans = { vlan for vlan in new_dp.vlans.values() if vlan.vid in changed_vlans } + old_vlans = { + vlan for vlan in self.vlans.values() if vlan.vid in changed_vlans + } + logger.debug("new_changed_vlans: %s" % new_changed_vlans) + logger.debug("old_vlans: %s" % old_vlans) for vlan in new_changed_vlans: - changed_port_nums = {port.number for port in vlan.get_ports()} - changed_ports.update(changed_port_nums) + for old_vlan in old_vlans: + if old_vlan.vid == vlan.vid: + changed_port_nums = {port.number for port in vlan.get_ports()} + old_port_nums = {port.number for port in old_vlan.get_ports()} + # changed_port_nums = old_port_nums.symmetric_difference(changed_port_nums) + changed_port_nums -= old_port_nums + logger.debug("VLAN %s changed ports: %s" % (vlan.vid, changed_port_nums)) + changed_ports.update(changed_port_nums) + logger.debug("Total changed ports from VLAN changes: %s" % changed_ports) all_ports_changed = changed_ports == all_ports - # Detect changes to VLANs and ACLs based on port changes. - if not all_ports_changed: + if added_vlans: + logger.debug("VLANs added: %s" % added_vlans) + all_ports = frozenset(new_dp.ports.keys()) + new_added_vlans = { + vlan for vlan in new_dp.vlans.values() if vlan.vid in added_vlans + } + for vlan in new_added_vlans: + added_port_nums = {port.number for port in vlan.get_ports()} + logger.debug("VLAN %s added ports: %s" % (vlan.vid, added_port_nums)) + changed_ports.update(added_port_nums) + logger.debug("Total changed ports from VLAN additions: %s" % changed_ports) + all_ports_changed = changed_ports == all_ports - def get_vids(vlans): - if not vlans: - return set() - if isinstance(vlans, Iterable): - return {vlan.vid for vlan in vlans} - return {vlans.vid} - - def _add_changed_vlan_port(port, port_dp): - changed_vlans.update(get_vids(port.vlans())) - if port.stack: - changed_vlans.update(get_vids(port_dp.vlans.values())) - - def _add_changed_vlans(old_port, new_port): - if old_port.vlans() != new_port.vlans(): - old_vids = get_vids(old_port.vlans()) - new_vids = get_vids(new_port.vlans()) - changed_vlans.update(old_vids.symmetric_difference(new_vids)) - # stacking dis/enabled on a port. - if bool(old_port.stack) != bool(new_port.stack): - changed_vlans.update(get_vids(new_dp.vlans.values())) - - for port_no in changed_ports: - if port_no not in self.ports: - continue - old_port = self.ports[port_no] - new_port = new_dp.ports[port_no] - _add_changed_vlans(old_port, new_port) - for port_no in deleted_ports: - port = self.ports[port_no] - _add_changed_vlan_port(port, self) - for port_no in added_ports: - port = new_dp.ports[port_no] - _add_changed_vlan_port(port, new_dp) + # Detect port ACL change + if not all_ports_changed: for port_no in same_ports: old_port = self.ports[port_no] new_port = new_dp.ports[port_no] @@ -1669,6 +1704,21 @@ def _add_changed_vlans(old_port, new_port): ) all_ports_changed = True + logger.debug(""" + all_ports_changed {} + deleted_ports {} + changed_ports {} + added_ports {} + changed_acl_ports {} + changed_vlans {} + """.format( + all_ports_changed, + deleted_ports, + changed_ports, + added_ports, + changed_acl_ports, + changed_vlans) + ) return ( all_ports_changed, deleted_ports, @@ -1714,6 +1764,7 @@ def get_config_changes(self, logger, new_dp): changed_acl_ports (set): changed ACL only port numbers. deleted_vlans (set): deleted VLAN IDs. changed_vlans (set): changed/added VLAN IDs. + changed_acl_vlans (set): changed/added VLAN ACLs VLAN IDs. all_ports_changed (bool): True if all ports changed. all_meters_changed (bool): True if all meters changed deleted_meters (set): deleted meter numbers @@ -1736,8 +1787,8 @@ def get_config_changes(self, logger, new_dp): ) else: changed_acls = self._get_acl_config_changes(logger, new_dp) - deleted_vlans, changed_vlans = self._get_vlan_config_changes( - logger, new_dp, changed_acls + deleted_vlans, changed_vlans, added_vlans, changed_acl_vlans = ( + self._get_vlan_config_changes(logger, new_dp, changed_acls) ) ( all_meters_changed, @@ -1753,7 +1804,7 @@ def get_config_changes(self, logger, new_dp): changed_acl_ports, changed_vlans, ) = self._get_port_config_changes( - logger, new_dp, changed_vlans, deleted_vlans, changed_acls + logger, new_dp, changed_vlans, added_vlans, deleted_vlans, changed_acls ) return ( deleted_ports, @@ -1762,6 +1813,8 @@ def get_config_changes(self, logger, new_dp): changed_acl_ports, deleted_vlans, changed_vlans, + added_vlans, + changed_acl_vlans, all_ports_changed, all_meters_changed, deleted_meters, @@ -1776,6 +1829,8 @@ def get_config_changes(self, logger, new_dp): set(), set(), set(), + set(), + set(), True, True, set(), diff --git a/faucet/faucet.py b/faucet/faucet.py index bd25525360..4f8e4845d4 100644 --- a/faucet/faucet.py +++ b/faucet/faucet.py @@ -218,9 +218,16 @@ def _send_flow_msgs(self, valve, flow_msgs, ryu_dp=None): ryu_dp: Override datapath from DPSet. """ if ryu_dp is None: - ryu_dp = self.dpset.get(valve.dp.dp_id) + try: + ryu_dp = self.dpset.get(valve.dp.dp_id) + valve.logger.warning("value of self.dpset.get_all is {}".format(self.dpset.get_all())) + except Exception as e: + valve.logger.warning( + "error: {} when getting DP".format(e), + ) if not ryu_dp: valve.logger.error("send_flow_msgs: DP not up") + valve.logger.error("send_flow_msgs: DP type is {}".format(ryu_dp)) return valve.send_flows(ryu_dp, flow_msgs, time.time()) diff --git a/faucet/valve.py b/faucet/valve.py index bf0d8833cd..6ec9b953d7 100644 --- a/faucet/valve.py +++ b/faucet/valve.py @@ -1580,7 +1580,9 @@ def _apply_config_changes(self, new_dp, changes, valves=None): added_ports (set): added port numbers. changed_acl_ports (set): changed ACL only port numbers. deleted_vids (set): deleted VLAN IDs. - changed_vids (set): changed/added VLAN IDs. + changed_vids (set): changed VLAN IDs. + added_vids (set): added VLAN IDs. + changed_acl_vlans (set): changed/added VLAN ACL VLAN IDs. all_ports_changed (bool): True if all ports changed. all_meters_changed (bool): True if all meters changed. deleted_meters: (set): deleted meter numbers. @@ -1599,6 +1601,8 @@ def _apply_config_changes(self, new_dp, changes, valves=None): changed_acl_ports, deleted_vids, changed_vids, + added_vids, + changed_acl_vlans, all_ports_changed, _, deleted_meters, @@ -1607,7 +1611,35 @@ def _apply_config_changes(self, new_dp, changes, valves=None): ) = changes restart_type = "cold" ofmsgs = [] - + self.logger.debug("""Valve changes are: + deleted_ports: {}, + changed_ports: {}, + added_ports: {}, + changed_acl_ports: {}, + deleted_vids: {}, + changed_vids: {}, + added_vids: {}, + changed_acl_vlans: {}, + all_ports_changed: {}, + _: {}, + deleted_meters: {}, + added_meters: {}, + changed_meters: {}, + """.format( + deleted_ports, + changed_ports, + added_ports, + changed_acl_ports, + deleted_vids, + changed_vids, + added_vids, + changed_acl_vlans, + all_ports_changed, + _, + deleted_meters, + added_meters, + changed_meters, + )) # If pipeline or all ports changed, default to cold start. if self._pipeline_change(new_dp): self.dp_init(new_dp, valves) @@ -1631,11 +1663,21 @@ def _apply_config_changes(self, new_dp, changes, valves=None): if deleted_ports: ofmsgs.extend(self.ports_delete(deleted_ports)) + if changed_ports: ofmsgs.extend(self.ports_delete(changed_ports)) + if deleted_vids: deleted_vlans = [self.dp.vlans[vid] for vid in deleted_vids] ofmsgs.extend(self.del_vlans(deleted_vlans)) + + # if vlan acl changed and there are changed/deletd vids then delete vlan acl. + if self.acl_manager and changed_acl_vlans.union(changed_vids, deleted_vids): + changed_vlans = [self.dp.vlans[vid] for vid in changed_acl_vlans.union(changed_vids, deleted_vids)] + for vlan in changed_vlans: + ofmsgs.extend(self.acl_manager.del_vlan(vlan)) + self.logger.debug("Number of deleted Openflow messages generated: {}".format(len(ofmsgs))) + # TODO: optimize for all meters being erased if changed_meters: # If a meter changed meter IDs, delete the old ID first and consider @@ -1669,14 +1711,20 @@ def _apply_config_changes(self, new_dp, changes, valves=None): for port_num in changed_acl_ports: port = self.dp.ports[port_num] ofmsgs.extend(self.acl_manager.cold_start_port(port)) + if added_vids: + added_vlans = [self.dp.vlans[vid] for vid in added_vids] + ofmsgs.extend(self.add_vlans(added_vlans, cold_start=True)) if changed_vids: changed_vlans = [self.dp.vlans[vid] for vid in changed_vids] - # TODO: handle change versus add separately so can avoid delete first. - ofmsgs.extend(self.del_vlans(changed_vlans)) - # The proceeding delete operation means we don't have to generate more deletes. ofmsgs.extend(self.add_vlans(changed_vlans, cold_start=True)) + if self.acl_manager and changed_acl_vlans.union(changed_vids, added_vids): + changed_vlans = [self.dp.vlans[vid] for vid in changed_acl_vlans.union(changed_vids, added_vids)] + for vlan in changed_vlans: + ofmsgs.extend(self.acl_manager.add_vlan(vlan, cold_start=False)) + self.logger.debug("Number of added Openflow messages generated: {}".format(len(ofmsgs))) if self.stack_manager: ofmsgs.extend(self.stack_manager.add_tunnel_acls()) + self.logger.info("Openflow messages generated: {}".format(len(ofmsgs))) return restart_type, ofmsgs def reload_config(self, _now, new_dp, valves=None): @@ -1711,7 +1759,10 @@ def reload_config(self, _now, new_dp, valves=None): elif restart_type == "warm": # DP not currently up, so no messages to send. if not self.dp.dyn_running: - ofmsgs = [] + # TEST: Ensure we do a cold start if DP is not running. + ofmsgs = None + restart_type = "cold" + self.logger.info("DP is not running, change restart_type from warm to cold") self.notify({"CONFIG_CHANGE": {"restart_type": restart_type}}) return ofmsgs diff --git a/faucet/valves_manager.py b/faucet/valves_manager.py index 6d6424d021..229de43a7b 100644 --- a/faucet/valves_manager.py +++ b/faucet/valves_manager.py @@ -18,6 +18,7 @@ # limitations under the License. from collections import defaultdict +import time from faucet.conf import InvalidConfigError from faucet.config_parser_util import config_changed, CONFIG_HASH_FUNC @@ -345,9 +346,14 @@ def request_reload_configs(self, now, new_config_file, delete_dp=None): """Process a request to load config changes.""" if self.config_watcher.content_changed(new_config_file): self.logger.info( - "configuration %s changed, analyzing differences", new_config_file + "configuration %s changed, start analyzing differences", new_config_file ) + start_time = time.time() result = self.load_configs(now, new_config_file, delete_dp=delete_dp) + self.logger.info( + "configuration %s changed, analyzing duration is %.2f second", + new_config_file, time.time() - start_time + ) self._notify( { "CONFIG_CHANGE": { diff --git a/tests/unit/faucet/test_valve_config.py b/tests/unit/faucet/test_valve_config.py index 81ff0ad86f..565e2f62da 100755 --- a/tests/unit/faucet/test_valve_config.py +++ b/tests/unit/faucet/test_valve_config.py @@ -186,7 +186,7 @@ def setUp(self): def test_change_vlan_acl(self): """Test vlan ACL change is detected.""" - self.update_and_revert_config(self.CONFIG, self.MORE_CONFIG, "cold") + self.update_and_revert_config(self.CONFIG, self.MORE_CONFIG, "warm") class ValveChangePortTestCase(ValveTestBases.ValveTestNetwork): From 1456ee8480344768ae2a15d11ce15a71abbf25de Mon Sep 17 00:00:00 2001 From: hieunt79 Date: Tue, 11 Nov 2025 14:55:40 +0700 Subject: [PATCH 2/7] update: pylint and remove unnecessary log --- faucet/dp.py | 22 +++++++++++++--------- faucet/faucet.py | 9 +-------- faucet/valve.py | 7 +------ 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/faucet/dp.py b/faucet/dp.py index acb60aea69..462a69f911 100644 --- a/faucet/dp.py +++ b/faucet/dp.py @@ -1504,19 +1504,15 @@ def _get_vlan_config_changes(self, logger, new_dp, changed_acls): ignore_keys=frozenset(["acls_in"]), ) logger.debug(""" - _: {}, deleted_vlans: {}, added_vlans: {}, changed_vlans: {}, same_vlans: {}, - _: {}, """.format( - _, deleted_vlans, added_vlans, changed_vlans, same_vlans, - _, )) # changed_vlans = added_vlans.union(changed_vlans) # DOING: optimize for warm start. @@ -1528,7 +1524,10 @@ def _get_vlan_config_changes(self, logger, new_dp, changed_acls): "VLAN %u" % vlan_id, old_vlan, new_vlan, changed_acls, logger ): changed_acl_vlans.add(vlan_id) - logger.debug("deleted_vlans: {}, changed_vlans: {}".format(deleted_vlans, changed_vlans)) + logger.debug( + "deleted_vlans: {}, changed_vlans: {}".format( + deleted_vlans, changed_vlans) + ) return (deleted_vlans, changed_vlans, added_vlans, changed_acl_vlans) def _acl_ref_changes(self, conf_desc, old_conf, new_conf, changed_acls, logger): @@ -1541,7 +1540,10 @@ def _acl_ref_changes(self, conf_desc, old_conf, new_conf, changed_acls, logger): old_acl_ids = old_conf.acls_in if old_acl_ids: old_acl_ids = [acl._id for acl in old_acl_ids] - logger.debug("conf_desc: {}, old_acl_ids: {}, new_acl_ids: {}, conf_acls_changed: {}".format(conf_desc, old_acl_ids, new_acl_ids, conf_acls_changed)) + logger.debug( + "conf_desc: {}, old_acl_ids: {}, new_acl_ids: {}, conf_acls_changed: {}".format( + conf_desc, old_acl_ids, new_acl_ids, conf_acls_changed) + ) if conf_acls_changed: changed = True logger.info( @@ -1564,6 +1566,7 @@ def _get_port_config_changes( logger (ValveLogger): logger instance. new_dp (DP): new dataplane configuration. changed_vlans (set): changed/added VLAN IDs. + added_vlans (set): added VLAN IDs. deleted_vlans (set): deleted VLAN IDs. changed_acls (set): changed/added ACL IDs. Returns: @@ -1607,9 +1610,11 @@ def _get_port_config_changes( )) # What is port change? # - Detect port added/deleted/changed by using _get_conf_changes - # - If port added or deleted, their related opfmgs are processed later by Valve._apply_config_changes() + # - If port added or deleted, their related opfmgs are processed later + # by Valve._apply_config_changes() # - Port other config changed like vlan id or port ACL? - # - Vlan membership added / changed / deleted? -> go through all vlan changes to find affected ports + # - Vlan membership added / changed / deleted? + # -> go through all vlan changes to find affected ports # - Port ACL changed changed_acl_ports = set() @@ -1650,7 +1655,6 @@ def _get_port_config_changes( if old_vlan.vid == vlan.vid: changed_port_nums = {port.number for port in vlan.get_ports()} old_port_nums = {port.number for port in old_vlan.get_ports()} - # changed_port_nums = old_port_nums.symmetric_difference(changed_port_nums) changed_port_nums -= old_port_nums logger.debug("VLAN %s changed ports: %s" % (vlan.vid, changed_port_nums)) changed_ports.update(changed_port_nums) diff --git a/faucet/faucet.py b/faucet/faucet.py index 4f8e4845d4..bd25525360 100644 --- a/faucet/faucet.py +++ b/faucet/faucet.py @@ -218,16 +218,9 @@ def _send_flow_msgs(self, valve, flow_msgs, ryu_dp=None): ryu_dp: Override datapath from DPSet. """ if ryu_dp is None: - try: - ryu_dp = self.dpset.get(valve.dp.dp_id) - valve.logger.warning("value of self.dpset.get_all is {}".format(self.dpset.get_all())) - except Exception as e: - valve.logger.warning( - "error: {} when getting DP".format(e), - ) + ryu_dp = self.dpset.get(valve.dp.dp_id) if not ryu_dp: valve.logger.error("send_flow_msgs: DP not up") - valve.logger.error("send_flow_msgs: DP type is {}".format(ryu_dp)) return valve.send_flows(ryu_dp, flow_msgs, time.time()) diff --git a/faucet/valve.py b/faucet/valve.py index 6ec9b953d7..4fa8e411e2 100644 --- a/faucet/valve.py +++ b/faucet/valve.py @@ -1621,7 +1621,6 @@ def _apply_config_changes(self, new_dp, changes, valves=None): added_vids: {}, changed_acl_vlans: {}, all_ports_changed: {}, - _: {}, deleted_meters: {}, added_meters: {}, changed_meters: {}, @@ -1635,7 +1634,6 @@ def _apply_config_changes(self, new_dp, changes, valves=None): added_vids, changed_acl_vlans, all_ports_changed, - _, deleted_meters, added_meters, changed_meters, @@ -1759,10 +1757,7 @@ def reload_config(self, _now, new_dp, valves=None): elif restart_type == "warm": # DP not currently up, so no messages to send. if not self.dp.dyn_running: - # TEST: Ensure we do a cold start if DP is not running. - ofmsgs = None - restart_type = "cold" - self.logger.info("DP is not running, change restart_type from warm to cold") + ofmsgs = [] self.notify({"CONFIG_CHANGE": {"restart_type": restart_type}}) return ofmsgs From 98c831339936ec749ccdf56caf8bee20cd4ef836 Mon Sep 17 00:00:00 2001 From: hieunt79 Date: Tue, 7 Apr 2026 18:02:18 +0700 Subject: [PATCH 3/7] update acl test case and update reload rule log --- faucet/valves_manager.py | 2 ++ tests/integration/mininet_tests.py | 9 ++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/faucet/valves_manager.py b/faucet/valves_manager.py index 229de43a7b..c44ca2827c 100644 --- a/faucet/valves_manager.py +++ b/faucet/valves_manager.py @@ -307,7 +307,9 @@ def _apply_configs(self, new_dps, now, delete_dp): self.logger.info("Reconfiguring existing datapath %s", dpid_log(dp_id)) valve = self.valves[dp_id] ofmsgs = valve.reload_config(now, new_dp, list(self.valves.values())) + start_time = time.time() self.send_flows_to_dp_by_id(valve, ofmsgs) + self.logger.info("Rule application time: %.2f seconds", time.time() - start_time) sent[dp_id] = valve.dp.dyn_running else: self.logger.info("Add new datapath %s", dpid_log(new_dp.dp_id)) diff --git a/tests/integration/mininet_tests.py b/tests/integration/mininet_tests.py index 63f917f504..9cf3208ba7 100644 --- a/tests/integration/mininet_tests.py +++ b/tests/integration/mininet_tests.py @@ -3297,7 +3297,8 @@ def test_vlan_acl_update(self): new_yaml_acl_conf, self.acl_config_file, # pytype: disable=attribute-error restart=True, - cold_start=True, + cold_start=False, + # cold_start=True, ) self.wait_until_matching_flow({"dl_type": 0x800}, table_id=self._VLAN_ACL_TABLE) self.wait_until_matching_flow({"dl_type": 0x806}, table_id=self._VLAN_ACL_TABLE) @@ -7727,7 +7728,8 @@ def test_untagged(self): self._ip_neigh(second_host, second_faucet_vip.ip, 4), self.FAUCET_MAC2 ) self.change_vlan_config( - "vlanb", "vid", vlanb_vid, restart=True, cold_start=True + # "vlanb", "vid", vlanb_vid, restart=True, cold_start=True + "vlanb", "vid", vlanb_vid, restart=True, cold_start=False ) @@ -7793,7 +7795,8 @@ def test_connectivity(host_a, host_b): self.port_map["port_3"], {"native_vlan": "vlana"}, restart=True, - cold_start=True, + cold_start=False, + # cold_start=True, ) test_connectivity(third_host, second_host) From 8e7369bc25c8f6d5919d5a8538518f7189bd58b6 Mon Sep 17 00:00:00 2001 From: hieunt79 Date: Tue, 7 Apr 2026 18:34:00 +0700 Subject: [PATCH 4/7] update testcases for VLAN ACL --- tests/integration/mininet_tests.py | 2 +- tests/unit/faucet/test_valve_stack.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/mininet_tests.py b/tests/integration/mininet_tests.py index 9cf3208ba7..faffab2b73 100644 --- a/tests/integration/mininet_tests.py +++ b/tests/integration/mininet_tests.py @@ -3308,7 +3308,7 @@ def test_vlan_acl_update(self): orig_yaml_acl_conf, self.acl_config_file, # pytype: disable=attribute-error restart=True, - cold_start=True, + cold_start=False, ) self.wait_until_matching_flow({"dl_type": 0x800}, table_id=self._VLAN_ACL_TABLE) self.wait_until_no_matching_flow( diff --git a/tests/unit/faucet/test_valve_stack.py b/tests/unit/faucet/test_valve_stack.py index 31709fdbbd..331e3da1d4 100644 --- a/tests/unit/faucet/test_valve_stack.py +++ b/tests/unit/faucet/test_valve_stack.py @@ -4046,7 +4046,7 @@ def test_reload_topology_change(self): for new_dp in new_dps: valve = self.valves_manager.valves[new_dp.dp_id] changes = valve.dp.get_config_changes(valve.logger, new_dp) - changed_ports, all_ports_changed = changes[1], changes[6] + changed_ports, all_ports_changed = changes[1], changes[8] for port in valve.dp.stack_ports(): if not all_ports_changed: self.assertIn( From 9432dcd8a7a76d5b7ecee44cc9d666ae5a53bdf4 Mon Sep 17 00:00:00 2001 From: hieunt79 Date: Thu, 9 Apr 2026 15:20:38 +0700 Subject: [PATCH 5/7] fix: integration test FaucetConfigReloadTest --- faucet/dp.py | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/faucet/dp.py b/faucet/dp.py index 462a69f911..18979d96d7 100644 --- a/faucet/dp.py +++ b/faucet/dp.py @@ -1676,6 +1676,41 @@ def _get_port_config_changes( # Detect port ACL change if not all_ports_changed: + def get_vids(vlans): + if not vlans: + return set() + try: + return {vlan.vid for vlan in vlans} + except TypeError: + return {vlans.vid} + + def _add_changed_vlan_port(port, port_dp): + changed_vlans.update(get_vids(port.vlans())) + if port.stack: + changed_vlans.update(get_vids(port_dp.vlans.values())) + + def _add_changed_vlans(old_port, new_port): + if old_port.vlans() != new_port.vlans(): + old_vids = get_vids(old_port.vlans()) + new_vids = get_vids(new_port.vlans()) + changed_vlans.update(old_vids.symmetric_difference(new_vids)) + # stacking dis/enabled on a port. + if bool(old_port.stack) != bool(new_port.stack): + changed_vlans.update(get_vids(new_dp.vlans.values())) + + for port_no in changed_ports: + if port_no not in self.ports: + continue + old_port = self.ports[port_no] + new_port = new_dp.ports[port_no] + _add_changed_vlans(old_port, new_port) + for port_no in deleted_ports: + port = self.ports[port_no] + _add_changed_vlan_port(port, self) + for port_no in added_ports: + port = new_dp.ports[port_no] + _add_changed_vlan_port(port, new_dp) + for port_no in same_ports: old_port = self.ports[port_no] new_port = new_dp.ports[port_no] From 3ab60015196c2b6b4f647a05cacb236681848414 Mon Sep 17 00:00:00 2001 From: hieunt79 Date: Thu, 9 Apr 2026 17:57:30 +0700 Subject: [PATCH 6/7] revert wrong change in test_untagged --- tests/integration/mininet_tests.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/integration/mininet_tests.py b/tests/integration/mininet_tests.py index faffab2b73..82c7f7e02a 100644 --- a/tests/integration/mininet_tests.py +++ b/tests/integration/mininet_tests.py @@ -7728,8 +7728,7 @@ def test_untagged(self): self._ip_neigh(second_host, second_faucet_vip.ip, 4), self.FAUCET_MAC2 ) self.change_vlan_config( - # "vlanb", "vid", vlanb_vid, restart=True, cold_start=True - "vlanb", "vid", vlanb_vid, restart=True, cold_start=False + "vlanb", "vid", vlanb_vid, restart=True, cold_start=True ) @@ -7795,8 +7794,7 @@ def test_connectivity(host_a, host_b): self.port_map["port_3"], {"native_vlan": "vlana"}, restart=True, - cold_start=False, - # cold_start=True, + cold_start=True, ) test_connectivity(third_host, second_host) From f3b63b371216e3d101a9f53f132fb556a0bfcd8b Mon Sep 17 00:00:00 2001 From: hieunt79 Date: Fri, 24 Apr 2026 17:28:33 +0700 Subject: [PATCH 7/7] fix: force-delete port acl flows and adding integration test --- faucet/valve.py | 9 +++- faucet/valve_acl.py | 15 +++++++ tests/integration/mininet_tests.py | 68 ++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 2 deletions(-) diff --git a/faucet/valve.py b/faucet/valve.py index 4fa8e411e2..0941f80fe8 100644 --- a/faucet/valve.py +++ b/faucet/valve.py @@ -1689,6 +1689,10 @@ def _apply_config_changes(self, new_dp, changes, valves=None): if self.acl_manager: if deleted_meters: ofmsgs.extend(self.acl_manager.del_meters(deleted_meters)) + if changed_acl_ports: + for port_num in changed_acl_ports: + port = self.dp.ports[port_num] + ofmsgs.extend(self.acl_manager.del_port_force(port)) self.dp_init(new_dp, valves) @@ -1708,7 +1712,7 @@ def _apply_config_changes(self, new_dp, changes, valves=None): if self.acl_manager and changed_acl_ports: for port_num in changed_acl_ports: port = self.dp.ports[port_num] - ofmsgs.extend(self.acl_manager.cold_start_port(port)) + ofmsgs.extend(self.acl_manager.add_port(port)) if added_vids: added_vlans = [self.dp.vlans[vid] for vid in added_vids] ofmsgs.extend(self.add_vlans(added_vlans, cold_start=True)) @@ -1722,7 +1726,8 @@ def _apply_config_changes(self, new_dp, changes, valves=None): self.logger.debug("Number of added Openflow messages generated: {}".format(len(ofmsgs))) if self.stack_manager: ofmsgs.extend(self.stack_manager.add_tunnel_acls()) - self.logger.info("Openflow messages generated: {}".format(len(ofmsgs))) + self.logger.info("Number of Openflow messages: {}".format(len(ofmsgs))) + self.logger.debug("Detail of generated Openflow messages: {}".format(ofmsgs)) return restart_type, ofmsgs def reload_config(self, _now, new_dp, valves=None): diff --git a/faucet/valve_acl.py b/faucet/valve_acl.py index df25e19bf1..32d2e69079 100644 --- a/faucet/valve_acl.py +++ b/faucet/valve_acl.py @@ -507,6 +507,21 @@ def add_port(self, port): ) return ofmsgs + def del_port_force(self, port): + """Force-delete all ACL flows for a port. + + Uses a priority-less delete so the flowmodkey differs from + any subsequently-added default rule (which carries acl_priority), preventing + valve_flowreorder's remove_overlap_ofmsgs from suppressing this delete. + In OpenFlow, OFPFC_DELETE (non-strict) ignores priority and deletes all + matching flows regardless of their priority. + """ + 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)) + return ofmsgs + def del_port(self, port): ofmsgs = [] if self._port_acls_allowed(port): diff --git a/tests/integration/mininet_tests.py b/tests/integration/mininet_tests.py index 82c7f7e02a..decdf7bf2a 100644 --- a/tests/integration/mininet_tests.py +++ b/tests/integration/mininet_tests.py @@ -3865,6 +3865,74 @@ def test_port_change_vlan(self): self.assertLess(len(self.scrape_prometheus(var="learned_l2_port")), 4) +class FaucetConfigReloadPortAclRemoveTest(FaucetConfigReloadTestBase): + CONFIG_GLOBAL = """ +vlans: + 100: + description: "office network" +""" + ACL = """ +acls: + block-ssl: + - rule: + dl_type: 0x800 + ip_proto: 6 + tcp_dst: 443 + actions: + allow: 0 + block-http: + - rule: + dl_type: 0x800 + ip_proto: 6 + tcp_dst: 80 + actions: + allow: 0 + block-ping: + - rule: + dl_type: 0x800 + ip_proto: 1 + actions: + allow: 0 +""" + CONFIG = """ + interfaces: + %(port_1)d: + native_vlan: 100 + acls_in: [block-ping] + %(port_2)d: + native_vlan: 100 + acls_in: [block-ping, block-http, block-ssl] + %(port_3)d: + native_vlan: 100 +""" + + def test_remove_port_acl(self): + hup = not self.STAT_RELOAD + first_host, second_host, third_host = self.hosts_name_ordered()[:3] + + # We don't really care about ping success, we care about flow existence. + # Check that port 2 has the block-ping rule + in_port_2_match = { + "in_port": int(self.port_map["port_2"]), + "eth_type": 0x800, + "ip_proto": 1, + } + self.wait_until_matching_flow(in_port_2_match, table_id=self._PORT_ACL_TABLE) + + # Remove the ACL from port 2 ONLY. + self.change_port_config( + self.port_map["port_2"], + "acls_in", + ["block-http", "block-ssl"], + restart=True, + cold_start=False, + hup=hup, + ) + + # Verify port_2 no longer has the block-ping flow + self.wait_until_no_matching_flow(in_port_2_match, table_id=self._PORT_ACL_TABLE, timeout=30) + + class FaucetConfigReloadEmptyAclTest(FaucetConfigReloadTestBase): CONFIG = """ interfaces: