Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hieunt79 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af59d04
feat: update VLAN ACL change detection to warm reload
Nov 10, 2025
07280a6
update: pylint and remove unnecessary log
Nov 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 107 additions & 48 deletions faucet/dp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
(
_,
Expand All @@ -1501,16 +1503,32 @@ 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
Expand All @@ -1522,6 +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)
)
if conf_acls_changed:
changed = True
logger.info(
Expand All @@ -1536,14 +1558,15 @@ 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.

Args:
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:
Expand All @@ -1570,6 +1593,29 @@ 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
Expand All @@ -1590,53 +1636,46 @@ 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
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]
Expand Down Expand Up @@ -1669,6 +1708,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,
Expand Down Expand Up @@ -1714,6 +1768,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
Expand All @@ -1736,8 +1791,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,
Expand All @@ -1753,7 +1808,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,
Expand All @@ -1762,6 +1817,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,
Expand All @@ -1776,6 +1833,8 @@ def get_config_changes(self, logger, new_dp):
set(),
set(),
set(),
set(),
set(),
True,
True,
set(),
Expand Down
56 changes: 51 additions & 5 deletions faucet/valve.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand All @@ -1607,7 +1611,33 @@ 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)
Expand All @@ -1631,11 +1661,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
Expand Down Expand Up @@ -1669,14 +1709,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):
Expand Down
8 changes: 7 additions & 1 deletion faucet/valves_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/faucet/test_valve_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Loading