VLAN ACL change detection to be able to warm reload by hieunt79 · Pull Request #4733 · faucetsdn/faucet

hieunt79 · 2025-11-10T12:48:58Z

I want to perform a warm reload when the VLAN ACL changes, so I made the following updates:

For port changes:
- Handle ports in changed_vlans and added_vlans separately.
- When a port ACL changes (changed_acl_ports), handle it later through Valve._apply_config_changes().
For VLAN changes: add: added_vlans, changed_acl_vlans
Lastly: optimize return OpenFlow messages in Valve _apply_config_change()

codecov · 2025-11-12T20:57:13Z

Codecov Report

❌ Patch coverage is 97.10145% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.47%. Comparing base (3e3bc9d) to head (f3b63b3).
⚠️ Report is 92 commits behind head on main.

Files with missing lines	Patch %	Lines
faucet/dp.py	94.59%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4733      +/-   ##
==========================================
+ Coverage   91.42%   91.47%   +0.05%     
==========================================
  Files          46       46              
  Lines        8923     8980      +57     
==========================================
+ Hits         8157     8214      +57     
  Misses        766      766

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gizmoguy · 2025-11-13T00:32:58Z

Thanks for the contribution, it appears that this patch has some test failures at the moment that need to be addressed.

If I look at this test failure:

FAIL: test_mclag_warmstart (mininet_multidp_tests.FaucetSingleLAGOnUniqueVLANTest.test_mclag_warmstart)
Test LACP MCLAG after a warm start
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/__w/faucet/faucet/tests/integration/mininet_multidp_tests.py", line 2156, in test_mclag_warmstart
    self.reload_conf(
  File "/faucet-src/clib/mininet_test_base.py", line 2020, in reload_conf
    verify_faucet_reconf_func()
  File "/faucet-src/clib/mininet_test_base.py", line 2784, in verify_faucet_reconf
    self.assertEqual(
AssertionError: 0 != 1 : FAUCET faucet-0-1176 faucet_config_reload_warm_total incremented: 1

It looks like this test was expecting a cold restart, but now with your changes a warm reload happens so the test fails. I think you'll need to update this test (and perhaps others) to account for this behavioural change.

hieunt79 · 2026-04-09T11:19:48Z

Hi @gizmoguy, I've revisited this PR and fixed the failing test cases. Could you please review it?

gizmoguy · 2026-04-22T21:58:57Z

Hi @hieunt79, I've been testing your patch and it seems to cover most scenarios and work well.

I've found one case that doesn't seem to work currently, and that is the case where all port ACLs are removed from a port, e.g start faucet with this configuration where we have 2 ports, both with ACLs:

vlans:
    office:
        vid: 100
        description: "office network"
acls:
    block-ping:
        - rule:
            dl_type: 0x800      # IPv4
            ip_proto: 1         # ICMP
            actions:
                allow: False
dps:
    sw1:
        dp_id: 0x1
        hardware: "Open vSwitch"
        interfaces:
            1:
                name: "host1"
                native_vlan: office
                acls_in:
                  - block-ping
            2:
                name: "host2"
                native_vlan: office
                acls_in:
                  - block-ping

and then remove the ACL from one of the ports and reload faucet, e.g:

vlans:
    office:
        vid: 100
        description: "office network"
acls:
    block-ping:
        - rule:
            dl_type: 0x800      # IPv4
            ip_proto: 1         # ICMP
            actions:
                allow: False
dps:
    sw1:
        dp_id: 0x1
        hardware: "Open vSwitch"
        interfaces:
            1:
                name: "host1"
                native_vlan: office
                acls_in:
                  - block-ping
            2:
                name: "host2"
                native_vlan: office

We will see faucet do the warm reload and correctly detect the change in configuration:

Apr 22 21:50:49 faucet INFO     Reloading configuration
Apr 22 21:50:49 faucet INFO     configuration /etc/faucet/faucet.yaml changed, start analyzing differences
Apr 22 21:50:49 faucet INFO     Reconfiguring existing datapath DPID 1 (0x1)
Apr 22 21:50:49 faucet.valve INFO     DPID 1 (0x1) sw1 no ACL changes
Apr 22 21:50:49 faucet.valve INFO     DPID 1 (0x1) sw1 no VLAN changes
Apr 22 21:50:49 faucet.valve INFO     DPID 1 (0x1) sw1 no METERS changes
Apr 22 21:50:49 faucet.valve INFO     DPID 1 (0x1) sw1 no port changes
Apr 22 21:50:49 faucet.valve INFO     DPID 1 (0x1) sw1 port 2 ACL changed (ACL ['port-block-ping'] to None)
Apr 22 21:50:49 faucet.valve INFO     DPID 1 (0x1) sw1 ports where ACL only changed: {2}
Apr 22 21:50:49 faucet.valve INFO     DPID 1 (0x1) sw1 Openflow messages generated: 2
Apr 22 21:50:49 faucet.valve INFO     DPID 1 (0x1) sw1 warm starting

However, the ACL flow rules do not get deleted from the datapath (unless you restart faucet to trigger a cold reload):

$ sudo ovs-ofctl -OOpenFlow13 dump-flows br0 | cut -d ',' -f 3- | sed -E 's/n_packets=[^,]+, //' | sed -E 's/n_bytes=[^,]+, //' | grep in_port=2 | grep table=0
 table=0, priority=20480,icmp,in_port=2 actions=drop

I also tested the same scenario for vlan ACLs and in that case everything worked fine, so it's just the port ACL case that seems to be broken currently.

hieunt79 · 2026-04-29T04:37:02Z

Hi @gizmoguy, thanks for pointing out the issue.

The generated flows are correct:

[OFPFlowMod(buffer_id=4294967295,command=3,cookie=11720004596349619985,cookie_mask=0,flags=0,hard_timeout=0,idle_timeout=0,instructions=[],match=OFPMatch(oxm_fields={'in_port': 3}),out_group=4294967295,out_port=4294967295,priority=20480,table_id=0), OFPFlowMod(buffer_id=4294967295,command=0,cookie=11720004596349619985,cookie_mask=0,flags=0,hard_timeout=0,idle_timeout=0,instructions=(OFPInstructionGotoTable(len=8,table_id=1,type=1),),match=OFPMatch(oxm_fields={'in_port': 3}),out_group=0,out_port=0,priority=20480,table_id=0)]

However, remove_overlap_ofmsgs in valve_of.py (lines 1269-1270) suppresses the delete flow:

if deletes and addmod:
    by_kind["delete"] = remove_overlap_ofmsgs(deletes, addmod)

To fix this, I added a new del_port_force method to force the deletion of the port and any ACL rules belonging to it. Do you think this is a good approach?

I have also added a test case (FaucetConfigReloadPortAclRemoveTest) for this scenario. The latest commit passes both unit and integration tests.

gizmoguy · 2026-05-01T04:46:35Z

Hi @hieunt79, thanks for digging into that edge case and adding a new test case, I understand better what is happening now.

I now see that to change an ACL on a VLAN or port, this patch will delete all the flow rules for the vlan/port and re-add the new flow rules from the new configuration. This approach is essentially what a cold-start does, except it does it for a whole datapath.

Here's what I am thinking is a better approach for implementing the warm start for ACL changes (which is similar to how we handle warm reloads for meter changes), that will truly be a warm start (i.e we only delete flow rules we are no longer using and add new flow that are missing):

In _get_port_config_changes() instead of returning the ports with changed acls in changed_acl_ports, for changed ports you could instead return either a list of all old port ACL objects (self.dp.ports.get(port_num).acls_in) and the new port ACL objects (new_dp.ports.get(port_num).acls_in), alternatively you could just return the added/removed/changed ACL objects
In _apply_config_changes() we can now loop over the port ACL objects and firstly call acl_manager.del_port_acl() on all old ACL objects, and then call acl_manager.add_port_acl() on new ACL objects. remove_overlap_ofmsgs() should hopefully cancel out the flowmods for ACLs that didn't change, but if not, it might be necessary to just delete the removed/changed ACL objects, and only add the new/changed ACL objects.

It should be possible to implement warm reload for VLAN ACLs in the same way, but I do note we are missing acl_manager methods for adding/deleting individual VLAN ACLs, so these would need to be added. I think that shouldn't be too difficult, but if it proves to be too hard then I guess the current implementation of deleting all the VLAN ACL flow rules and re-adding is currently working so we could probably merge the current implementation as is.

Let me know if you have any questions or if you find a problem in my design that I haven't considered that makes it difficult to implement or inefficient.

Changes to a VLAN's VIPs, router membership, or ACLs currently force a cold restart -- full datapath reconnection and a flow-table wipe. This is unnecessarily disruptive for deployments that update these dynamically (provisioning new tenant networks, IPAM-driven VLAN changes, dynamic router membership, ACL rule edits). The warm-restart path (del_vlan + dp_init + add_vlan) already handles VLAN-level changes incrementally, but two guards in dp.py were short-circuiting to cold start. Those guards are removed and the missing flow cleanup is added so warm restart is correct for VIP- and router-related state. VID replacement (a deleted + added VLAN pair on a DP whose ports cover the new VLAN) keeps its cold-start escalation -- per-port classification flows genuinely need rebuilding. For ACL changes the existing cold_start_port path emits a default goto-vlan add at the same flowmodkey as its preceding bulk delete, which valve_flowreorder.remove_overlap_ofmsgs strips, leaving stale per-port ACL flows on the datapath. cold_start_port is replaced with a diff: capture the addmods that acl_manager.add_port / add_vlan would produce for the old config, mirror them as flowdels, emit them alongside the new addmods. Unchanged-rule pairs share a flowmodkey and are cancelled by remove_overlap_ofmsgs before reaching the wire; only changed rules cross the channel. The mirroring approach reuses the priority/cookie scheme that put the existing flows there, so flowmodkeys align by construction -- without needing per-ACL priority offsets or new acl_manager methods. ValveTable.flowdel grows a cookie parameter so the addmods-to-flowdels conversion can propagate the rule cookie; without that the overlap check silently fails (different cookies = different flowmodkeys) and the granular path emits more ofmsgs than cold_start_port did. The default goto-vlan transition (removing the last ACL from a port, or adding the first to a previously-bare port) falls out of the diff for free and fixes the stale block-ping flow gizmoguy reproduced reviewing faucetsdn#4733. VLAN ACL changes get the same treatment via a new changed_acl_vlans set in dp.py that catches VLANs whose only change is an ACL ref or an edit to a referenced ACL's rules. VLANs that change in any other way still take the heavy del_vlans + add_vlans reinstall. The integration test FaucetConfigReloadPortAclRemoveTest is brought over from faucetsdn#4733 verbatim, with hieunt79 attributed via Co-Authored-By below. Co-Authored-By: hieunt79 <44926706+hieunt79@users.noreply.github.com>

Changes to a VLAN's VIPs, router membership, or ACLs currently force a cold restart -- full datapath reconnection and a flow-table wipe. This is unnecessarily disruptive for deployments that update these dynamically (provisioning new tenant networks, IPAM-driven VLAN changes, dynamic router membership, ACL rule edits). The warm-restart path (del_vlan + dp_init + add_vlan) already handles VLAN-level changes incrementally, but two guards in dp.py were short-circuiting to cold start. Those guards are removed and the missing flow cleanup is added so warm restart is correct for VIP- and router-related state. VID replacement (a deleted + added VLAN pair on a DP whose ports cover the new VLAN) keeps its cold-start escalation -- per-port classification flows genuinely need rebuilding. For ACL changes the existing cold_start_port path emits a default goto-vlan add at the same flowmodkey as its preceding bulk delete, which valve_flowreorder.remove_overlap_ofmsgs strips, leaving stale per-port ACL flows on the datapath. cold_start_port is replaced with a diff: capture the addmods that acl_manager.add_port / add_vlan would produce for the old config, mirror them as flowdels, emit them alongside the new addmods. Unchanged-rule pairs share a flowmodkey and are cancelled by remove_overlap_ofmsgs before reaching the wire; only changed rules cross the channel. The mirroring approach reuses the priority/cookie scheme that put the existing flows there, so flowmodkeys align by construction -- without needing per-ACL priority offsets or new acl_manager methods. ValveTable.flowdel grows a cookie parameter so the addmods-to-flowdels conversion can propagate the rule cookie; without that the overlap check silently fails (different cookies = different flowmodkeys) and the granular path emits more ofmsgs than cold_start_port did. The default goto-vlan transition (removing the last ACL from a port, or adding the first to a previously-bare port) falls out of the diff for free and fixes the stale block-ping flow gizmoguy reproduced reviewing faucetsdn#4733. VLAN ACL changes get the same treatment via a new changed_acl_vlans set in dp.py that catches VLANs whose only change is an ACL ref or an edit to a referenced ACL's rules. VLANs that change in any other way still take the heavy del_vlans + add_vlans reinstall. The integration test FaucetConfigReloadPortAclRemoveTest is brought over from faucetsdn#4733 verbatim, with hieunt79 attributed via Co-Authored-By below. Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

Changes to a VLAN's VIPs, router membership, or ACLs currently force a cold restart -- full datapath reconnection and a flow-table wipe. This is unnecessarily disruptive for deployments that update these dynamically (provisioning new tenant networks, IPAM-driven VLAN changes, dynamic router membership, ACL rule edits). The warm-restart path (del_vlan + dp_init + add_vlan) already handles VLAN-level changes incrementally, but two guards in dp.py were short-circuiting to cold start. Those guards are removed and the missing flow cleanup is added so warm restart is correct for VIP- and router-related state. VID replacement (a deleted + added VLAN pair on a DP whose ports cover the new VLAN) keeps its cold-start escalation -- per-port classification flows genuinely need rebuilding. For ACL changes the existing cold_start_port path emits a default goto-vlan add at the same flowmodkey as its preceding bulk delete, which valve_flowreorder.remove_overlap_ofmsgs strips, leaving stale per-port ACL flows on the datapath. cold_start_port is replaced with a diff: capture the addmods that acl_manager.add_port / add_vlan would produce for the old config, mirror them as flowdels, emit them alongside the new addmods. Each old flowdel carries the rule's specific match, so the existing flows on the OF table are removed exactly. The default goto-vlan transition (removing the last ACL from a port, or adding the first to a previously-bare port) falls out of the diff for free, fixing the stale block-ping flow gizmoguy reproduced reviewing faucetsdn#4733. ValveTable.flowdel grows a cookie parameter so the addmods-to-flowdels conversion can propagate the rule cookie -- without it the remove_overlap_ofmsgs python-level cancellation cannot align del+add keys even when the underlying matches are content-equal. _flowmodkey in valve_of also gets a small fix: it now compares match contents via tuple(sorted(match.items())) instead of relying on OFPMatch identity, since os_ken's OFPMatch returns NotImplemented from __eq__ and two semantically-identical matches don't compare equal. Together these let the granular path's unchanged-rule del+add pairs cancel when there's no real change, instead of needlessly churning flows on the wire. VLAN ACL changes get the same treatment via a new changed_acl_vlans set in dp.py that catches VLANs whose only change is an ACL ref or an edit to a referenced ACL's rules. VLANs that change in any other way still take the heavy del_vlans + add_vlans reinstall. The integration test FaucetConfigReloadPortAclRemoveTest is brought over from faucetsdn#4733 verbatim, with hieunt79 attributed via Co-Authored-By below. Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

Changes to a VLAN's VIPs, router membership, or ACLs currently force a cold restart -- full datapath reconnection and a flow-table wipe. This is unnecessarily disruptive for deployments that update these dynamically (provisioning new tenant networks, IPAM-driven VLAN changes, dynamic router membership, ACL rule edits). VLAN/VIP and router warm restart -------------------------------- The warm-restart path (del_vlan + dp_init + add_vlan) already handles VLAN-level changes incrementally, but two guards in dp.py were short- circuiting to cold start. Those guards are removed and the missing flow cleanup is added so warm restart is correct for VIP- and router- related state. VID replacement (a deleted + added VLAN pair on a DP whose ports cover the new VLAN) keeps its cold-start escalation -- per-port classification flows genuinely need rebuilding. Granular ACL warm reload ------------------------ For ACL changes the existing cold_start_port path emits a default goto-vlan add at the same flowmodkey as its preceding bulk delete, which valve_flowreorder.remove_overlap_ofmsgs strips, leaving stale per-port ACL flows on the datapath (the stale block-ping flow gizmoguy reproduced reviewing faucetsdn#4733). cold_start_port is replaced with a per-port/per-VLAN diff. Before dp_init swaps acl_manager, _apply_config_changes snapshots the addmods that the OLD acl_manager would emit for each affected port and VLAN. After dp_init, it asks the NEW acl_manager for the same and runs the two through valve_acl.diff_addmods, which compares them by (match, cookie, priority, table_id) and emits only the rule-level delta: a flowdel for each rule present only in old, a flowmod for each rule present only in new, nothing for rules unchanged across the reload. A 1-rule edit in a VLAN with hundreds of ACL rules costs O(k) wire ops, not O(N). addmods_to_flowdels uses OFPFC_DELETE_STRICT so a flowdel for a less- specific rule (e.g. a default-allow with no match fields) does not wildcard-delete more specific rules that share its match prefix. valve_table.flowdel grows a cookie kwarg so the addmods-to-flowdels conversion can carry the rule cookie through to the python-level diff key (cookie_mask=0 means OF still ignores it on the wire). _flowmodkey in valve_of canonicalizes match contents via frozenset(match.items()) instead of relying on OFPMatch identity, since os_ken's OFPMatch returns NotImplemented from __eq__ and two semantically-identical matches don't compare equal. VLAN ACL changes get the same treatment via a new changed_acl_vlans set in dp.py that catches VLANs whose only change is an ACL ref or an edit to a referenced ACL's rules (covers acls_in and acls_out). VLANs that change in any other way still take the heavy del_vlans + add_vlans reinstall; an assert in get_config_changes guarantees the two paths stay disjoint so ACL flows are never double-written. Tests ----- Unit coverage in test_valve_config.py exercises: removing the only ACL from a port, adding an ACL to a previously-bare port, the cookie/ priority/table_id alignment that diff_addmods relies on, granular port-ACL reload (asserts exactly 1 del + 1 add for a 1-of-4 rule edit and zero ofmsgs for the unchanged rules -- the cost-scales-with-k property), combined ACL+config VLAN changes, VLAN egress ACL changes, removed-rule cleanup in vlan_acl_table (symmetric stale-flow check), and a VIP change on a DP whose single VLAN owns every port (warm, not cold). Integration coverage in mininet_tests.py adds FaucetVIPChangeWarmStartTest (VIP edit on an all-ports VLAN), FaucetConfigReloadVlanAclChangeTest (granular VLAN ACL warm reload with all other flows untouched), and FaucetConfigReloadPortAclRemoveTest (brought over from faucetsdn#4733 verbatim, with hieunt79 attributed via Co-Authored-By below). The existing test_vlan_acl_update flips from cold_start=True to cold_start=False to match the new warm path. Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

VLAN/VIP, router, and ACL config changes now warm-start instead of cold. The warm path (del_vlan + dp_init + add_vlan) already handled the VLAN-level changes; two guards in dp.py were short-circuiting to cold start, and ACL-only changes still went through cold_start_port which left stale flows on the wire. VLAN/VIP and router: - Replace router cold-start guard with finer checks (BGP changes and router add/remove with no routing tables stay cold) - Replace VIP cold-start guard with sibling-VLAN expansion to keep proactive-learn FIB entries consistent - Separate added_vlans from changed_vlans so new VLANs skip the unnecessary del_vlans() - Move changed-VLAN deletion before dp_init() so old managers clean up old select_packets flows - Add pipeline.remove_select(); route manager del_vlan() now cleans select_packets flows alongside FIB flows - Keep cold-start escalation for VID replacement when affected ports cover the DP ACL: - Replace cold_start_port with diff-at-source: snapshot the addmods the OLD acl_manager would emit before dp_init, ask the NEW one for the same after, emit only the rule-level delta. Cost scales with k (rules changed), not N (rules total) - Use OFPFC_DELETE_STRICT for ACL flowdels so a less-specific rule doesn't wildcard-delete more specific rules sharing its prefix - New changed_acl_vlans set in dp.py for VLANs whose only change is an ACL ref or referenced ACL's contents (covers acls_in + acls_out) - valve_table.flowdel grows a cookie kwarg; _flowmodkey in valve_of canonicalizes match contents via frozenset(items()) instead of OFPMatch identity - Fixes the stale block-ping flow gizmoguy reproduced reviewing faucetsdn#4733 FaucetConfigReloadPortAclRemoveTest brought over from faucetsdn#4733 verbatim, with hieunt79 attributed below. Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

@gizmoguy

Adopts the per-ACL design @gizmoguy proposed in his 2026-05-01 review of faucetsdn#4733: for each port/VLAN whose only change is acls_in, iterate the old and new ACL lists and call del/add per-ACL with a cold-start priority decrement. remove_overlap_ofmsgs in valve_flowreorder cancels unchanged del+add pairs at flow-emit, so unchanged ACL rules don't churn on the wire. Inserting an ACL mid-list shifts subsequent priorities -- those re-emit (acceptable per gizmoguy's fallback in the linked review). dp.py - _acl_ref_changes returns (old_acls, new_acls) or None - _get_port_config_changes' changed_acl_ports becomes a dict port_num -> (old_acls, new_acls) - _get_vlan_config_changes returns a new changed_acl_vlans dict vid -> (old_acls, new_acls); ACL-only VLAN changes are no longer folded into changed_vlans so they warm-reload instead of cold valve.py _apply_config_changes - Per-ACL del+add loop replaces the prior cold_start_port handling for changed_acl_ports - New per-VLAN ACL loop using add_vlan_acl / del_vlan_acl - Empty<->non-empty port transitions need the wildcard rule flipped, so fall back to cold_start_port valve_acl.py - add_port_acl / del_port_acl get a priority kwarg defaulting to self.auth_priority so dot1x callers are unchanged - build_acl_port_of_msgs derives allow/force_port_vlan instructions from the pipeline (matching cold-start build_acl_ofmsgs), so rules with force_port_vlan: 1 land in the right table; dot1x ACLs never carry force_port_vlan: 1 so its behavior is preserved - New add_vlan_acl / del_vlan_acl symmetric to the port helpers (the methods gizmoguy noted were missing for per-ACL VLAN reload) Tests - FaucetConfigReloadVlanAclChangeTest and FaucetConfigReloadPortAclRemoveTest come from @hieunt79's faucetsdn#4733 (the latter exercises the bug gizmoguy reproduced reviewing faucetsdn#4733: removing one ACL from a port that has multiple ACLs must leave the others installed) - test_vlan_acl_update now expects warm reload (was cold) - ValveChangeVLANACLTestCase.test_change_vlan_acl now expects warm Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

@gizmoguy

Adopts the per-ACL design @gizmoguy proposed in his 2026-05-01 review of faucetsdn#4733: for each port/VLAN whose only change is acls_in, iterate the old and new ACL lists and call del/add per-ACL with a cold-start priority decrement. remove_overlap_ofmsgs in valve_flowreorder cancels unchanged del+add pairs at flow-emit, so unchanged ACL rules don't churn on the wire. Inserting an ACL mid-list shifts subsequent priorities -- those re-emit (acceptable per gizmoguy's fallback in the linked review). dp.py - _acl_ref_changes returns (old_acls, new_acls) or None - _get_port_config_changes' changed_acl_ports becomes a dict port_num -> (old_acls, new_acls) - _get_vlan_config_changes returns a new changed_acl_vlans dict vid -> (old_acls, new_acls); ACL-only VLAN changes are no longer folded into changed_vlans so they warm-reload instead of cold valve.py _apply_config_changes - Per-ACL del+add loop replaces the prior cold_start_port handling for changed_acl_ports - New per-VLAN ACL loop using add_vlan_acl / del_vlan_acl - Empty<->non-empty port transitions need the wildcard rule flipped, so fall back to cold_start_port valve_acl.py - add_port_acl / del_port_acl get a priority kwarg defaulting to self.auth_priority so dot1x callers are unchanged - build_acl_port_of_msgs derives allow/force_port_vlan instructions from the pipeline (matching cold-start build_acl_ofmsgs), so rules with force_port_vlan: 1 land in the right table; dot1x ACLs never carry force_port_vlan: 1 so its behavior is preserved - New add_vlan_acl / del_vlan_acl symmetric to the port helpers (the methods gizmoguy noted were missing for per-ACL VLAN reload) Tests - FaucetConfigReloadVlanAclChangeTest and FaucetConfigReloadPortAclRemoveTest come from @hieunt79's faucetsdn#4733 (the latter exercises the bug gizmoguy reproduced reviewing faucetsdn#4733: removing one ACL from a port that has multiple ACLs must leave the others installed) - test_vlan_acl_update now expects warm reload (was cold) - ValveChangeVLANACLTestCase.test_change_vlan_acl now expects warm Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

@gizmoguy

Adopts the per-ACL design @gizmoguy proposed in his 2026-05-01 review of faucetsdn#4733: for each port/VLAN whose only change is acls_in, iterate the old and new ACL lists and call del/add per-ACL with a cold-start priority decrement. remove_overlap_ofmsgs in valve_flowreorder cancels unchanged del+add pairs at flow-emit, so unchanged ACL rules don't churn on the wire. Inserting an ACL mid-list shifts subsequent priorities -- those re-emit (acceptable per gizmoguy's fallback in the linked review). dp.py - _acl_ref_changes returns (old_acls, new_acls) or None - _get_port_config_changes' changed_acl_ports becomes a dict port_num -> (old_acls, new_acls) - _get_vlan_config_changes returns a new changed_acl_vlans dict vid -> (old_acls, new_acls); ACL-only VLAN changes are no longer folded into changed_vlans so they warm-reload instead of cold valve.py _apply_config_changes - Per-ACL del+add loop replaces the prior cold_start_port handling for changed_acl_ports - New per-VLAN ACL loop using add_vlan_acl / del_vlan_acl - Empty<->non-empty port transitions need the wildcard rule flipped, so fall back to cold_start_port valve_acl.py - add_port_acl / del_port_acl get a priority kwarg defaulting to self.auth_priority so dot1x callers are unchanged - build_acl_port_of_msgs derives allow/force_port_vlan instructions from the pipeline (matching cold-start build_acl_ofmsgs), so rules with force_port_vlan: 1 land in the right table; dot1x ACLs never carry force_port_vlan: 1 so its behavior is preserved - New add_vlan_acl / del_vlan_acl symmetric to the port helpers (the methods gizmoguy noted were missing for per-ACL VLAN reload) Tests - FaucetConfigReloadVlanAclChangeTest and FaucetConfigReloadPortAclRemoveTest come from @hieunt79's faucetsdn#4733 (the latter exercises the bug gizmoguy reproduced reviewing faucetsdn#4733: removing one ACL from a port that has multiple ACLs must leave the others installed) - test_vlan_acl_update now expects warm reload (was cold) - ValveChangeVLANACLTestCase.test_change_vlan_acl now expects warm Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

@gizmoguy

Adopts the per-ACL design @gizmoguy proposed in his 2026-05-01 review of faucetsdn#4733: for each port/VLAN whose only change is acls_in, iterate the old and new ACL lists and call del/add per-ACL with a cold-start priority decrement. remove_overlap_ofmsgs in valve_flowreorder cancels unchanged del+add pairs at flow-emit, so unchanged ACL rules don't churn on the wire. Inserting an ACL mid-list shifts subsequent priorities -- those re-emit (acceptable per gizmoguy's fallback in the linked review). dp.py - _acl_ref_changes returns (old_acls, new_acls) or None - _get_port_config_changes' changed_acl_ports becomes a dict port_num -> (old_acls, new_acls) - _get_vlan_config_changes returns a new changed_acl_vlans dict vid -> (old_acls, new_acls); ACL-only VLAN changes are no longer folded into changed_vlans so they warm-reload instead of cold valve.py _apply_config_changes - Per-ACL del+add loop replaces the prior cold_start_port handling for changed_acl_ports - New per-VLAN ACL loop using add_vlan_acl / del_vlan_acl - Empty<->non-empty port transitions need the wildcard rule flipped, so fall back to cold_start_port valve_acl.py - add_port_acl / del_port_acl get a priority kwarg defaulting to self.auth_priority so dot1x callers are unchanged - build_acl_port_of_msgs derives allow/force_port_vlan instructions from the pipeline (matching cold-start build_acl_ofmsgs), so rules with force_port_vlan: 1 land in the right table; dot1x ACLs never carry force_port_vlan: 1 so its behavior is preserved - New add_vlan_acl / del_vlan_acl symmetric to the port helpers (the methods gizmoguy noted were missing for per-ACL VLAN reload) Tests - FaucetConfigReloadVlanAclChangeTest and FaucetConfigReloadPortAclRemoveTest come from @hieunt79's faucetsdn#4733 (the latter exercises the bug gizmoguy reproduced reviewing faucetsdn#4733: removing one ACL from a port that has multiple ACLs must leave the others installed) - test_vlan_acl_update now expects warm reload (was cold) - ValveChangeVLANACLTestCase.test_change_vlan_acl now expects warm Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

@gizmoguy

Adopts the per-ACL design @gizmoguy proposed in his 2026-05-01 review of faucetsdn#4733: for each port/VLAN whose only change is acls_in, iterate the old and new ACL lists and call del/add per-ACL with a cold-start priority decrement. remove_overlap_ofmsgs in valve_flowreorder cancels unchanged del+add pairs at flow-emit, so unchanged ACL rules don't churn on the wire. Inserting an ACL mid-list shifts subsequent priorities -- those re-emit (acceptable per gizmoguy's fallback in the linked review). dp.py - _acl_ref_changes returns (old_acls, new_acls) or None - _get_port_config_changes' changed_acl_ports becomes a dict port_num -> (old_acls, new_acls) - _get_vlan_config_changes returns a new changed_acl_vlans dict vid -> (old_acls, new_acls); ACL-only VLAN changes are no longer folded into changed_vlans so they warm-reload instead of cold valve.py _apply_config_changes - Per-ACL del+add loop replaces the prior cold_start_port handling for changed_acl_ports - New per-VLAN ACL loop using add_vlan_acl / del_vlan_acl - Empty<->non-empty port transitions need the wildcard rule flipped, so fall back to cold_start_port valve_acl.py - add_port_acl / del_port_acl get a priority kwarg defaulting to self.auth_priority so dot1x callers are unchanged - build_acl_port_of_msgs derives allow/force_port_vlan instructions from the pipeline (matching cold-start build_acl_ofmsgs), so rules with force_port_vlan: 1 land in the right table; dot1x ACLs never carry force_port_vlan: 1 so its behavior is preserved - New add_vlan_acl / del_vlan_acl symmetric to the port helpers (the methods gizmoguy noted were missing for per-ACL VLAN reload) - del_port's flowdel is now priority-less so its flowmodkey differs from the acl_priority wildcard that add_port emits for a port with no acls_in; without this, remove_overlap_ofmsgs cancels the delete and old ACL flows stay on the switch (the @hieunt79 fix from faucetsdn#4733) Tests - FaucetConfigReloadVlanAclChangeTest and FaucetConfigReloadPortAclRemoveTest come from @hieunt79's faucetsdn#4733 (the latter exercises the bug gizmoguy reproduced reviewing faucetsdn#4733: removing one ACL from a port that has multiple ACLs must leave the others installed) - FaucetConfigReloadPortAclRemoveAllTest covers removing the last ACL from a port; without the del_port fix above this hits the remove_overlap_ofmsgs cancellation and the old flow stays stale - test_vlan_acl_update now expects warm reload (was cold) - ValveChangeVLANACLTestCase.test_change_vlan_acl now expects warm Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

@gizmoguy

Adopts the per-ACL design @gizmoguy proposed in his 2026-05-01 review of faucetsdn#4733: for each port/VLAN whose only change is acls_in, iterate the old and new ACL lists and call del/add per-ACL with a cold-start priority decrement. remove_overlap_ofmsgs in valve_flowreorder cancels unchanged del+add pairs at flow-emit, so unchanged ACL rules don't churn on the wire. Inserting an ACL mid-list shifts subsequent priorities -- those re-emit (acceptable per gizmoguy's fallback in the linked review). dp.py - _acl_ref_changes returns (old_acls, new_acls) or None - _get_port_config_changes' changed_acl_ports becomes a dict port_num -> (old_acls, new_acls) - _get_vlan_config_changes returns a new changed_acl_vlans dict vid -> (old_acls, new_acls); ACL-only VLAN changes are no longer folded into changed_vlans so they warm-reload instead of cold valve.py _apply_config_changes - Per-ACL del+add loop replaces the prior cold_start_port handling for changed_acl_ports - New per-VLAN ACL loop using add_vlan_acl / del_vlan_acl - Empty<->non-empty port transitions need the wildcard rule flipped, so fall back to cold_start_port valve_acl.py - add_port_acl / del_port_acl get a priority kwarg defaulting to self.auth_priority so dot1x callers are unchanged - build_acl_port_of_msgs derives allow/force_port_vlan instructions from the pipeline (matching cold-start build_acl_ofmsgs), so rules with force_port_vlan: 1 land in the right table; dot1x ACLs never carry force_port_vlan: 1 so its behavior is preserved - New add_vlan_acl / del_vlan_acl symmetric to the port helpers (the methods gizmoguy noted were missing for per-ACL VLAN reload) - del_port's flowdel is now priority-less so its flowmodkey differs from the acl_priority wildcard that add_port emits for a port with no acls_in; without this, remove_overlap_ofmsgs cancels the delete and old ACL flows stay on the switch (the @hieunt79 fix from faucetsdn#4733) Tests - FaucetConfigReloadVlanAclChangeTest and FaucetConfigReloadPortAclRemoveTest come from @hieunt79's faucetsdn#4733 (the latter exercises the bug gizmoguy reproduced reviewing faucetsdn#4733: removing one ACL from a port that has multiple ACLs must leave the others installed) - FaucetConfigReloadPortAclRemoveAllTest covers removing the last ACL from a port; without the del_port fix above this hits the remove_overlap_ofmsgs cancellation and the old flow stays stale - test_vlan_acl_update now expects warm reload (was cold) - ValveChangeVLANACLTestCase.test_change_vlan_acl now expects warm Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

@gizmoguy

Adopts the per-ACL design @gizmoguy proposed in his 2026-05-01 review of faucetsdn#4733: for each port/VLAN whose only change is acls_in, iterate the old and new ACL lists and call del/add per-ACL with a cold-start priority decrement. remove_overlap_ofmsgs in valve_flowreorder cancels unchanged del+add pairs at flow-emit, so unchanged ACL rules don't churn on the wire. Inserting an ACL mid-list shifts subsequent priorities -- those re-emit (acceptable per gizmoguy's fallback in the linked review). dp.py - _acl_ref_changes returns (old_acls, new_acls) or None - _get_port_config_changes' changed_acl_ports becomes a dict port_num -> (old_acls, new_acls) - _get_vlan_config_changes returns a new changed_acl_vlans dict vid -> (old_acls, new_acls); ACL-only VLAN changes are no longer folded into changed_vlans so they warm-reload instead of cold valve.py _apply_config_changes - Per-ACL del+add loop replaces the prior cold_start_port handling for changed_acl_ports - New per-VLAN ACL loop using add_vlan_acl / del_vlan_acl - Empty<->non-empty port transitions need the wildcard rule flipped, so fall back to cold_start_port valve_acl.py - add_port_acl / del_port_acl get a priority kwarg defaulting to self.auth_priority so dot1x callers are unchanged - build_acl_port_of_msgs derives allow/force_port_vlan instructions from the pipeline (matching cold-start build_acl_ofmsgs), so rules with force_port_vlan: 1 land in the right table; dot1x ACLs never carry force_port_vlan: 1 so its behavior is preserved - New add_vlan_acl / del_vlan_acl symmetric to the port helpers (the methods gizmoguy noted were missing for per-ACL VLAN reload) - del_port's flowdel is now priority-less so its flowmodkey differs from the acl_priority wildcard that add_port emits for a port with no acls_in; without this, remove_overlap_ofmsgs cancels the delete and old ACL flows stay on the switch (the @hieunt79 fix from faucetsdn#4733) Tests - FaucetConfigReloadVlanAclChangeTest and FaucetConfigReloadPortAclRemoveTest come from @hieunt79's faucetsdn#4733 (the latter exercises the bug gizmoguy reproduced reviewing faucetsdn#4733: removing one ACL from a port that has multiple ACLs must leave the others installed) - FaucetConfigReloadPortAclRemoveAllTest covers removing the last ACL from a port; without the del_port fix above this hits the remove_overlap_ofmsgs cancellation and the old flow stays stale - test_vlan_acl_update now expects warm reload (was cold) - ValveChangeVLANACLTestCase.test_change_vlan_acl now expects warm Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

@gizmoguy

Adopts the per-ACL design @gizmoguy proposed in his 2026-05-01 review of faucetsdn#4733: for each port/VLAN whose only change is acls_in, iterate the old and new ACL lists and call del/add per-ACL with a cold-start priority decrement. remove_overlap_ofmsgs in valve_flowreorder cancels unchanged del+add pairs at flow-emit, so unchanged ACL rules don't churn on the wire. Inserting an ACL mid-list shifts subsequent priorities -- those re-emit (acceptable per gizmoguy's fallback in the linked review). dp.py - _acl_ref_changes returns (old_acls, new_acls) or None - _get_port_config_changes' changed_acl_ports becomes a dict port_num -> (old_acls, new_acls) - _get_vlan_config_changes returns a new changed_acl_vlans dict vid -> (old_acls, new_acls); ACL-only VLAN changes are no longer folded into changed_vlans so they warm-reload instead of cold valve.py _apply_config_changes - Per-ACL del+add loop replaces the prior cold_start_port handling for changed_acl_ports - New per-VLAN ACL loop using add_vlan_acl / del_vlan_acl - Empty<->non-empty port transitions need the wildcard rule flipped, so fall back to cold_start_port valve_acl.py - add_port_acl / del_port_acl get a priority kwarg defaulting to self.auth_priority so dot1x callers are unchanged - build_acl_port_of_msgs derives allow/force_port_vlan instructions from the pipeline (matching cold-start build_acl_ofmsgs), so rules with force_port_vlan: 1 land in the right table; dot1x ACLs never carry force_port_vlan: 1 so its behavior is preserved - New add_vlan_acl / del_vlan_acl symmetric to the port helpers (the methods gizmoguy noted were missing for per-ACL VLAN reload) - del_port's flowdel is now priority-less so its flowmodkey differs from the acl_priority wildcard that add_port emits for a port with no acls_in; without this, remove_overlap_ofmsgs cancels the delete and old ACL flows stay on the switch (the @hieunt79 fix from faucetsdn#4733) Tests - FaucetConfigReloadVlanAclChangeTest and FaucetConfigReloadPortAclRemoveTest come from @hieunt79's faucetsdn#4733 (the latter exercises the bug gizmoguy reproduced reviewing faucetsdn#4733: removing one ACL from a port that has multiple ACLs must leave the others installed) - FaucetConfigReloadPortAclRemoveAllTest covers removing the last ACL from a port; without the del_port fix above this hits the remove_overlap_ofmsgs cancellation and the old flow stays stale. Both ports share the same ACL so removing it from one port doesn't change the overall pipeline match set (which would otherwise force cold-start for table reconfiguration) - ValveRemoveAllPortACLsTestCase covers the same scenario at unit level - test_vlan_acl_update now expects warm reload (was cold) - ValveChangeVLANACLTestCase.test_change_vlan_acl now expects warm Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

@gizmoguy

Adopts the per-ACL design @gizmoguy proposed in his 2026-05-01 review of faucetsdn#4733: for each port/VLAN whose only change is acls_in, iterate the old and new ACL lists and call del/add per-ACL with a cold-start priority decrement. remove_overlap_ofmsgs in valve_flowreorder cancels unchanged del+add pairs at flow-emit, so unchanged ACL rules don't churn on the wire. Inserting an ACL mid-list shifts subsequent priorities -- those re-emit (acceptable per gizmoguy's fallback in the linked review). dp.py - _get_vlan_config_changes returns a new changed_acl_vlans set; ACL-only VLAN changes are no longer folded into changed_vlans so they warm-reload instead of cold valve.py _apply_config_changes - Snapshot the old acls_in lists before dp_init swaps self.dp - Per-ACL del+add loop replaces the prior cold_start_port handling for changed_acl_ports, and adds the equivalent for changed_acl_vlans - Empty<->non-empty port transitions need the wildcard rule flipped, so fall back to cold_start_port valve_acl.py - add_port_acl / del_port_acl get a priority kwarg defaulting to self.auth_priority so dot1x callers are unchanged - build_acl_port_of_msgs derives allow/force_port_vlan instructions from the pipeline (matching cold-start build_acl_ofmsgs), so rules with force_port_vlan: 1 land in the right table; dot1x ACLs never carry force_port_vlan: 1 so its behavior is preserved - New add_vlan_acl / del_vlan_acl symmetric to the port helpers (the methods gizmoguy noted were missing for per-ACL VLAN reload) - del_port's flowdel is now priority-less so its flowmodkey differs from the acl_priority wildcard that add_port emits for a port with no acls_in; without this, remove_overlap_ofmsgs cancels the delete and old ACL flows stay on the switch (the @hieunt79 fix from faucetsdn#4733) Tests - FaucetConfigReloadVlanAclChangeTest and FaucetConfigReloadPortAclRemoveTest come from @hieunt79's faucetsdn#4733 (the latter exercises the bug gizmoguy reproduced reviewing faucetsdn#4733: removing one ACL from a port that has multiple ACLs must leave the others installed) - FaucetConfigReloadPortAclRemoveAllTest covers removing the last ACL from a port; without the del_port fix above this hits the remove_overlap_ofmsgs cancellation and the old flow stays stale. Both ports share the same ACL so removing it from one port doesn't change the overall pipeline match set (which would otherwise force cold-start for table reconfiguration) - ValveRemoveAllPortACLsTestCase covers the same scenario at unit level - test_vlan_acl_update now expects warm reload (was cold) - ValveChangeVLANACLTestCase.test_change_vlan_acl now expects warm Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

@gizmoguy

Adopts the per-ACL design @gizmoguy proposed in his 2026-05-01 review of faucetsdn#4733: for each port/VLAN whose only change is acls_in, iterate the old and new ACL lists and call del/add per-ACL with a cold-start priority decrement. remove_overlap_ofmsgs in valve_flowreorder cancels unchanged del+add pairs at flow-emit, so unchanged ACL rules don't churn on the wire. Inserting an ACL mid-list shifts subsequent priorities -- those re-emit (acceptable per gizmoguy's fallback in the linked review). dp.py - _get_vlan_config_changes returns a new changed_acl_vlans set; ACL-only VLAN changes are no longer folded into changed_vlans so they warm-reload instead of cold valve.py _apply_config_changes - Snapshot the old acls_in lists before dp_init swaps self.dp - Per-ACL del+add loop replaces the prior cold_start_port handling for changed_acl_ports, and adds the equivalent for changed_acl_vlans - Empty<->non-empty port transitions need the wildcard rule flipped, so fall back to cold_start_port valve_acl.py - add_port_acl / del_port_acl get a priority kwarg defaulting to self.auth_priority so dot1x callers are unchanged - build_acl_port_of_msgs derives allow/force_port_vlan instructions from the pipeline (matching cold-start build_acl_ofmsgs), so rules with force_port_vlan: 1 land in the right table; dot1x ACLs never carry force_port_vlan: 1 so its behavior is preserved - New add_vlan_acl / del_vlan_acl symmetric to the port helpers (the methods gizmoguy noted were missing for per-ACL VLAN reload) - del_port's flowdel is now priority-less so its flowmodkey differs from the acl_priority wildcard that add_port emits for a port with no acls_in; without this, remove_overlap_ofmsgs cancels the delete and old ACL flows stay on the switch (the @hieunt79 fix from faucetsdn#4733) Tests - FaucetConfigReloadVlanAclChangeTest and FaucetConfigReloadPortAclRemoveTest come from @hieunt79's faucetsdn#4733 (the latter exercises the bug gizmoguy reproduced reviewing faucetsdn#4733: removing one ACL from a port that has multiple ACLs must leave the others installed) - FaucetConfigReloadPortAclRemoveAllTest covers removing the last ACL from a port; without the del_port fix above this hits the remove_overlap_ofmsgs cancellation and the old flow stays stale. Both ports share the same ACL so removing it from one port doesn't change the overall pipeline match set (which would otherwise force cold-start for table reconfiguration) - ValveRemoveAllPortACLsTestCase covers the same scenario at unit level - test_vlan_acl_update now expects warm reload (was cold) - ValveChangeVLANACLTestCase.test_change_vlan_acl now expects warm Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

@gizmoguy

Adopts the per-ACL design @gizmoguy proposed in his 2026-05-01 review of faucetsdn#4733: for each port/VLAN whose only change is acls_in, iterate the old and new ACL lists and call del/add per-ACL with a cold-start priority decrement. remove_overlap_ofmsgs in valve_flowreorder cancels unchanged del+add pairs at flow-emit, so unchanged ACL rules don't churn on the wire. Inserting an ACL mid-list shifts subsequent priorities -- those re-emit (acceptable per gizmoguy's fallback in the linked review). dp.py - _get_vlan_config_changes returns a new changed_acl_vlans set; ACL-only VLAN changes are no longer folded into changed_vlans so they warm-reload instead of cold valve.py _apply_config_changes - Snapshot the old acls_in lists before dp_init swaps self.dp - Per-ACL del+add loop replaces the prior cold_start_port handling for changed_acl_ports, and adds the equivalent for changed_acl_vlans - Empty<->non-empty port transitions need the wildcard rule flipped, so fall back to cold_start_port valve_acl.py - add_port_acl / del_port_acl get a priority kwarg defaulting to self.auth_priority so dot1x callers are unchanged - build_acl_port_of_msgs derives allow/force_port_vlan instructions from the pipeline (matching cold-start build_acl_ofmsgs), so rules with force_port_vlan: 1 land in the right table; dot1x ACLs never carry force_port_vlan: 1 so its behavior is preserved - New add_vlan_acl / del_vlan_acl symmetric to the port helpers (the methods gizmoguy noted were missing for per-ACL VLAN reload) - del_port's flowdel is now priority-less so its flowmodkey differs from the acl_priority wildcard that add_port emits for a port with no acls_in; without this, remove_overlap_ofmsgs cancels the delete and old ACL flows stay on the switch (the @hieunt79 fix from faucetsdn#4733) Tests - FaucetConfigReloadVlanAclChangeTest and FaucetConfigReloadPortAclRemoveTest come from @hieunt79's faucetsdn#4733 (the latter exercises the bug gizmoguy reproduced reviewing faucetsdn#4733: removing one ACL from a port that has multiple ACLs must leave the others installed) - FaucetConfigReloadPortAclRemoveAllTest covers removing the last ACL from a port; without the del_port fix above this hits the remove_overlap_ofmsgs cancellation and the old flow stays stale. Both ports share the same ACL so removing it from one port doesn't change the overall pipeline match set (which would otherwise force cold-start for table reconfiguration) - ValveRemoveAllPortACLsTestCase covers the same scenario at unit level - test_vlan_acl_update now expects warm reload (was cold) - ValveChangeVLANACLTestCase.test_change_vlan_acl now expects warm Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

@gizmoguy

Adopts the per-ACL design @gizmoguy proposed in his 2026-05-01 review of faucetsdn#4733: for each port/VLAN whose only change is acls_in, iterate the old and new ACL lists and call del/add per-ACL with a cold-start priority decrement. remove_overlap_ofmsgs in valve_flowreorder cancels unchanged del+add pairs at flow-emit, so unchanged ACL rules don't churn on the wire. Inserting an ACL mid-list shifts subsequent priorities -- those re-emit (acceptable per gizmoguy's fallback in the linked review). dp.py - _get_vlan_config_changes returns a new changed_acl_vlans set; ACL-only VLAN changes are no longer folded into changed_vlans so they warm-reload instead of cold valve.py _apply_config_changes - Snapshot the old acls_in lists before dp_init swaps self.dp - Per-ACL del+add loop replaces the prior cold_start_port handling for changed_acl_ports, and adds the equivalent for changed_acl_vlans - Empty<->non-empty port transitions need the wildcard rule flipped, so fall back to cold_start_port valve_acl.py - add_port_acl / del_port_acl get a priority kwarg defaulting to self.auth_priority so dot1x callers are unchanged - build_acl_port_of_msgs derives allow/force_port_vlan instructions from the pipeline (matching cold-start build_acl_ofmsgs), so rules with force_port_vlan: 1 land in the right table; dot1x ACLs never carry force_port_vlan: 1 so its behavior is preserved - New add_vlan_acl / del_vlan_acl symmetric to the port helpers (the methods gizmoguy noted were missing for per-ACL VLAN reload) - del_port's flowdel is now priority-less so its flowmodkey differs from the acl_priority wildcard that add_port emits for a port with no acls_in; without this, remove_overlap_ofmsgs cancels the delete and old ACL flows stay on the switch (the @hieunt79 fix from faucetsdn#4733) Tests - FaucetConfigReloadVlanAclChangeTest and FaucetConfigReloadPortAclRemoveTest come from @hieunt79's faucetsdn#4733 (the latter exercises the bug gizmoguy reproduced reviewing faucetsdn#4733: removing one ACL from a port that has multiple ACLs must leave the others installed) - FaucetConfigReloadPortAclRemoveAllTest covers removing the last ACL from a port; without the del_port fix above this hits the remove_overlap_ofmsgs cancellation and the old flow stays stale. Both ports share the same ACL so removing it from one port doesn't change the overall pipeline match set (which would otherwise force cold-start for table reconfiguration) - ValveRemoveAllPortACLsTestCase covers the same scenario at unit level - test_vlan_acl_update now expects warm reload (was cold) - ValveChangeVLANACLTestCase.test_change_vlan_acl now expects warm Co-Authored-By: hieunt79 <nguyenhieu264996@gmail.com>

gizmoguy · 2026-06-19T01:49:59Z

Hi @hieunt79, thanks for the hard work on this. I had a go at fixing some edge cases I found in this patch and submitted my own separate patch (#4819) which addresses these issues. I've credited your contribution on this issue by adding you as a co-author of the patch.

Please leave any comments and/or feedback on my PR if you find any issues with it and include any relevant faucet configuration files that are showing incorrect behaviour.

hieunt79 · 2026-06-22T08:52:12Z

Hi @gizmoguy, thanks for resolving this issue. Could you please correct my email in these commits? I made a typo, correct is nguyenhieu26496@gmail.com

gizmoguy · 2026-06-22T09:44:56Z

@hieunt79 email should be updated now

hieunt79 marked this pull request as draft April 8, 2026 07:09

hieunt79 marked this pull request as ready for review April 9, 2026 11:15

gizmoguy self-assigned this Apr 9, 2026

hieunt79 and others added 7 commits April 24, 2026 17:35

feat: update VLAN ACL change detection to warm reload

7b9656a

update: pylint and remove unnecessary log

1456ee8

update acl test case and update reload rule log

98c8313

update testcases for VLAN ACL

8e7369b

fix: integration test FaucetConfigReloadTest

9432dcd

revert wrong change in test_untagged

3ab6001

fix: force-delete port acl flows and adding integration test

f3b63b3

hieunt79 force-pushed the add_vlan_acl_warm_reload branch from 99f1e18 to f3b63b3 Compare April 24, 2026 10:41

courtland mentioned this pull request May 1, 2026

Allow warm restart for VLAN/VIP, router, and ACL configuration changes #4799

Closed

courtland mentioned this pull request May 9, 2026

[FIX BRANCH — DO NOT MERGE] ACL warm-restart staging over PR #4799 courtland/faucet#27

Closed

courtland mentioned this pull request May 19, 2026

Warm-reload for port and VLAN ACL configuration changes #4807

Closed

gizmoguy mentioned this pull request Jun 4, 2026

Fix deleting final ACL from a port #4815

Open

gizmoguy mentioned this pull request Jun 19, 2026

Always warm start for ACL-only VLAN configuration change #4819

Open

gizmoguy closed this Jun 19, 2026

Navigation Menu

Uh oh!

VLAN ACL change detection to be able to warm reload - #4733

VLAN ACL change detection to be able to warm reload#4733
hieunt79 wants to merge 7 commits into
faucetsdn:mainfrom
hieunt79:add_vlan_acl_warm_reload

hieunt79 commented Nov 10, 2025

Uh oh!

codecov Bot commented Nov 12, 2025 •

edited

Loading

Uh oh!

gizmoguy commented Nov 13, 2025

Uh oh!

hieunt79 commented Apr 9, 2026

Uh oh!

gizmoguy commented Apr 22, 2026 •

edited

Loading

Uh oh!

hieunt79 commented Apr 29, 2026

Uh oh!

gizmoguy commented May 1, 2026 •

edited

Loading

Uh oh!

gizmoguy commented Jun 19, 2026 •

edited

Loading

Uh oh!

hieunt79 commented Jun 22, 2026

Uh oh!

gizmoguy commented Jun 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

hieunt79 commented Nov 10, 2025

Uh oh!

codecov Bot commented Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gizmoguy commented Nov 13, 2025

Uh oh!

hieunt79 commented Apr 9, 2026

Uh oh!

gizmoguy commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hieunt79 commented Apr 29, 2026

Uh oh!

gizmoguy commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gizmoguy commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hieunt79 commented Jun 22, 2026

Uh oh!

gizmoguy commented Jun 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Nov 12, 2025 •

edited

Loading

gizmoguy commented Apr 22, 2026 •

edited

Loading

gizmoguy commented May 1, 2026 •

edited

Loading

gizmoguy commented Jun 19, 2026 •

edited

Loading