Skip to content

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

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
gizmoguy wants to merge 1 commit into
base: main
Choose a base branch
from
+285 −26
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
67 changes: 50 additions & 17 deletions faucet/dp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1485,6 +1485,7 @@ def _get_vlan_config_changes(self, logger, new_dp, changed_acls):
changes (tuple) of:
deleted_vlans (set): deleted VLAN IDs.
changed_vlans (set): changed/added VLAN IDs.
changed_acl_vlans (set): changed ACL only VLAN IDs.
"""
(
_,
Expand All @@ -1502,15 +1503,20 @@ def _get_vlan_config_changes(self, logger, new_dp, changed_acls):
ignore_keys=frozenset(["acls_in"]),
)
changed_vlans = added_vlans.union(changed_vlans)
# TODO: 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)

if changed_acl_vlans:
same_vlans -= changed_acl_vlans
logger.info("vlans where ACL only changed: %s" % changed_acl_vlans)

return (deleted_vlans, changed_vlans, changed_acl_vlans)

def _acl_ref_changes(self, conf_desc, old_conf, new_conf, changed_acls, logger):
changed = False
Expand All @@ -1536,7 +1542,13 @@ 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,
deleted_vlans,
changed_acl_vlans,
changed_acls,
):
"""Detect any config changes to ports.

Expand All @@ -1545,6 +1557,7 @@ def _get_port_config_changes(
new_dp (DP): new dataplane configuration.
changed_vlans (set): changed/added VLAN IDs.
deleted_vlans (set): deleted VLAN IDs.
changed_acl_vlans (set): changed ACL only VLAN IDs.
changed_acls (set): changed/added ACL IDs.
Returns:
changes (tuple) of:
Expand Down Expand Up @@ -1590,16 +1603,28 @@ def _get_port_config_changes(

if not same_ports:
all_ports_changed = True
# TODO: optimize case where only VLAN ACL changed.
elif changed_vlans:
all_ports = frozenset(new_dp.ports.keys())
new_changed_vlans = {
vlan for vlan in new_dp.vlans.values() if vlan.vid in changed_vlans
}
for vlan in new_changed_vlans:
changed_port_nums = {port.number for port in vlan.get_ports()}
changed_ports.update(changed_port_nums)
all_ports_changed = changed_ports == all_ports
else:
if changed_vlans:
all_ports = frozenset(new_dp.ports.keys())
new_changed_vlans = {
vlan for vlan in new_dp.vlans.values() if vlan.vid in changed_vlans
}
for vlan in new_changed_vlans:
changed_port_nums = {port.number for port in vlan.get_ports()}
changed_ports.update(changed_port_nums)
all_ports_changed = changed_ports == all_ports
if changed_acl_vlans:
# Adding first VLAN ACL or deleting final VLAN ACL affects
# whether packets for port need to use the VLAN ACL table or not,
# so add ports whose VLAN had an ACL change to the changed ports list
acl_changed_vlans = {
vlan
for vlan in new_dp.vlans.values()
if vlan.vid in changed_acl_vlans
}
for vlan in acl_changed_vlans:
changed_acl_port_nums = {port.number for port in vlan.get_ports()}
changed_ports.update(changed_acl_port_nums)

# Detect changes to VLANs and ACLs based on port changes.
if not all_ports_changed:
Expand Down Expand Up @@ -1714,6 +1739,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 ACL only 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 +1762,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, changed_acl_vlans = (
self._get_vlan_config_changes(logger, new_dp, changed_acls)
)
(
all_meters_changed,
Expand All @@ -1753,7 +1779,12 @@ 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,
deleted_vlans,
changed_acl_vlans,
changed_acls,
)
return (
deleted_ports,
Expand All @@ -1762,6 +1793,7 @@ def get_config_changes(self, logger, new_dp):
changed_acl_ports,
deleted_vlans,
changed_vlans,
changed_acl_vlans,
all_ports_changed,
all_meters_changed,
deleted_meters,
Expand All @@ -1776,6 +1808,7 @@ def get_config_changes(self, logger, new_dp):
set(),
set(),
set(),
set(),
True,
True,
set(),
Expand Down
15 changes: 11 additions & 4 deletions faucet/valve.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1599,6 +1599,7 @@ def _apply_config_changes(self, new_dp, changes, valves=None):
changed_acl_ports,
deleted_vids,
changed_vids,
changed_acl_vlans,
all_ports_changed,
_,
deleted_meters,
Expand Down Expand Up @@ -1665,10 +1666,16 @@ def _apply_config_changes(self, new_dp, changes, valves=None):
port for port in changed_ports if port in self.dp.dyn_up_port_nos
]
ofmsgs.extend(self.ports_add(all_up_port_nos))
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))
if self.acl_manager:
if 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))
if changed_acl_vlans:
for vid in changed_acl_vlans:
vlan = self.dp.vlans[vid]
ofmsgs.extend(self.acl_manager.del_vlan(vlan))
ofmsgs.extend(self.acl_manager.add_vlan(vlan, False))
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.
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/mininet_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3297,7 +3297,7 @@ 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,
)
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)
Expand All @@ -3307,7 +3307,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(
Expand Down
Loading