Allow warm restart for VLAN/VIP, router, and ACL configuration changes - #4799
Allow warm restart for VLAN/VIP, router, and ACL configuration changes#4799courtland wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4799 +/- ##
==========================================
+ Coverage 91.42% 91.56% +0.15%
==========================================
Files 46 46
Lines 8923 9054 +131
==========================================
+ Hits 8157 8290 +133
+ Misses 766 764 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I tested this patch with the case where a single VLAN has its VIP change, e.g starting faucet with this configuration: vlans:
office:
vid: 100
description: "office network"
faucet_vips: ["10.0.0.1/24"]
faucet_mac: "00:00:00:00:00:11"
dps:
sw1:
dp_id: 0x1
hardware: "Open vSwitch"
interfaces:
1:
name: "host1"
native_vlan: office
2:
name: "host2"
native_vlan: officeand then changing only the VIP and reloading faucet with SIGUP, i.e: vlans:
office:
vid: 100
description: "office network"
faucet_vips: ["10.0.0.2/24"]
faucet_mac: "00:00:00:00:00:11"
dps:
sw1:
dp_id: 0x1
hardware: "Open vSwitch"
interfaces:
1:
name: "host1"
native_vlan: office
2:
name: "host2"
native_vlan: officeThis triggers a cold restart: Is this the expected behaviour? As I read the PR description, this should trigger the |
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>
13af1a1 to
2c70afc
Compare
courtland
changed the title
|
@gizmoguy — yes, single-VLAN-owns-all-ports VIP changes were still cold-restarting in the prior version. The cross-VLAN port-change escalation in This push also absorbs the warm-ACL-reload work from #4733 — supersession-with-attribution per our offline thread. @hieunt79 is Mechanism deviates slightly from your 2026-05-01 per-ACL Two supporting fixes — both touching |
|
Closing in favor of opening individual PRs for the various warm reload cases. |
2 participants
Summary
Changes to a VLAN's VIPs, router configuration, or ACL configuration currently force a cold restart — full datapath reconnection and a flow-table wipe. This is unnecessarily disruptive for deployments that update these dynamically (tenant provisioning, IPAM-driven VIP changes, dynamic router membership, ACL edits).
The warm-restart path (
del_vlan+dp_init+add_vlan) already handles VLAN-level changes; two guards indp.pywere short-circuiting to cold, and ACL-only changes still went throughcold_start_portwhich left stale flows on the wire. This PR removes the guards, fixes the cleanup, and replaces the ACL cold-start path with a diff-at-source granular reload that emits only changed rules — cost is O(k) where k = rules changed, regardless of total ACL size.What warm-starts now (previously cold)
acls_inacls_in/acls_outWhat still cold-starts
ipv4_fib/viptables)Scope rationale
VLAN/VIP and router warm-restart are inseparable: VIP changes must propagate to router-sibling VLANs for FIB consistency, and router membership changes drive the same VLAN reinstall plumbing. ACL warm-restart is technically a separable follow-up but rides the same
del_vlan / dp_init / add_vlaninfrastructure — absorbing it here (per offline thread with @gizmoguy) obsoletes #4733 in one merge instead of two._flowmodkeycanonicalises match contents and instructions because OFPMatch and OFPInstruction fall back to identity__eq__. Without canonicalisation the OLD/NEW addmod snapshots wouldn't diff structurally, and an ACL rule whose action flipped (e.g.allow:0 → allow:1, same match) would look unchanged on the wire. The canonicalisation is global rather than a diff-only key so it applies everywhere flowmod equality matters (dedup, overlap-cancellation).Relation to #4733
This PR now incorporates #4733's goals. Instead of looping per-ACL with
del_port_acl/add_port_acl, it snapshots the addmods thatadd_port/add_vlanwould emit and diffs them — the priority/cookie scheme matches what cold-start install put on the table by construction, which the per-ACL helpers don't (they reset priority per-ACL).FaucetConfigReloadPortAclRemoveTestis included verbatim withCo-Authored-By: hieunt79. The stale-block-pingflow @gizmoguy reproduced is fixed.If merged, #4733 can be closed.