Skip to content

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

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

Closed
courtland wants to merge 1 commit into from
Closed

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

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
20 changes: 14 additions & 6 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 (dict): {vid: (old_acls, new_acls)}.
"""
(
_,
Expand All @@ -1502,15 +1503,17 @@ 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:
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 Down Expand Up @@ -1719,6 +1722,7 @@ def get_config_changes(self, logger, new_dp):
deleted_meters (set): deleted meter numbers
added_meters (set): Added meter numbers
changed_meters (set): changed/added meter numbers
changed_acl_vlans (set): changed ACL only VLAN IDs.
"""
if (
new_dp.stack
Expand All @@ -1736,9 +1740,11 @@ 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,
deleted_meters,
Expand Down Expand Up @@ -1767,6 +1773,7 @@ def get_config_changes(self, logger, new_dp):
deleted_meters,
added_meters,
changed_meters,
changed_acl_vlans,
)
# default cold start
return (
Expand All @@ -1781,6 +1788,7 @@ def get_config_changes(self, logger, new_dp):
set(),
set(),
set(),
set(),
)

def get_tables(self):
Expand Down
54 changes: 51 additions & 3 deletions faucet/valve.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1586,6 +1586,7 @@ def _apply_config_changes(self, new_dp, changes, valves=None):
deleted_meters: (set): deleted meter numbers.
changed_meters: (set): changed meter numbers.
added_meters: (set): added meter numbers.
changed_acl_vlans (set): changed ACL only VLAN IDs.
valves (list): List of other running valves
Returns:
tuple:
Expand All @@ -1604,6 +1605,7 @@ def _apply_config_changes(self, new_dp, changes, valves=None):
deleted_meters,
added_meters,
changed_meters,
changed_acl_vlans,
) = changes
restart_type = "cold"
ofmsgs = []
Expand All @@ -1629,6 +1631,14 @@ def _apply_config_changes(self, new_dp, changes, valves=None):
self.dp_init(new_dp)
return restart_type, ofmsgs

# Snapshot OLD acls_in lists before dp_init swaps self.dp.
old_port_acls = {
p: list(self.dp.ports[p].acls_in or []) for p in changed_acl_ports
}
old_vlan_acls = {
v: list(self.dp.vlans[v].acls_in or []) for v in changed_acl_vlans
}

if deleted_ports:
ofmsgs.extend(self.ports_delete(deleted_ports))
if changed_ports:
Expand Down Expand Up @@ -1665,10 +1675,48 @@ 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:
# Each port/VLAN: del each old ACL + add each new ACL at the same
# priorities cold-start install would use, so remove_overlap_ofmsgs
# cancels the del+add pairs for unchanged ACL rules. Ports gaining
# or losing all ACLs flip the wildcard "in_port -> goto vlan" rule
# add_port installs, so they take cold_start_port instead.
if self.acl_manager:
for port_num in changed_acl_ports:
port = self.dp.ports[port_num]
ofmsgs.extend(self.acl_manager.cold_start_port(port))
old = old_port_acls[port_num]
new = list(self.dp.ports[port_num].acls_in or [])
if not old or not new:
ofmsgs.extend(
self.acl_manager.cold_start_port(self.dp.ports[port_num])
)
continue
priority = self.acl_manager.acl_priority
for acl in old:
ofmsgs.extend(
self.acl_manager.del_port_acl(acl, port_num, priority=priority)
)
priority -= len(acl.rules)
priority = self.acl_manager.acl_priority
for acl in new:
ofmsgs.extend(
self.acl_manager.add_port_acl(acl, port_num, priority=priority)
)
priority -= len(acl.rules)
for vid in changed_acl_vlans:
vlan = self.dp.vlans[vid]
old = old_vlan_acls[vid]
new = list(vlan.acls_in or [])
priority = self.acl_manager.acl_priority
for acl in old:
ofmsgs.extend(
self.acl_manager.del_vlan_acl(acl, vlan, priority=priority)
)
priority -= len(acl.rules)
priority = self.acl_manager.acl_priority
for acl in new:
ofmsgs.extend(
self.acl_manager.add_vlan_acl(acl, vlan, priority=priority)
)
priority -= len(acl.rules)
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
54 changes: 42 additions & 12 deletions faucet/valve_acl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,15 +402,15 @@ def build_acl_ofmsgs(
return ofmsgs


def build_acl_port_of_msgs(acl, vid, port_num, acl_table, goto_table, priority):
def build_acl_port_of_msgs(acl, vid, port_num, acl_table, pipeline, priority):
"""A Helper function for building Openflow Mod Messages for Port ACLs"""
ofmsgs = None
if acl.rules:
ofmsgs = build_acl_ofmsgs(
[acl],
acl_table,
[valve_of.goto_table(goto_table)],
[valve_of.goto_table(goto_table)],
pipeline.accept_to_vlan(),
pipeline.accept_to_l2_forwarding(),
priority,
acl.meter,
acl.exact_match,
Expand Down Expand Up @@ -511,7 +511,10 @@ def del_port(self, port):
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, self.acl_priority))
# Priority-less so the flowmodkey differs from the acl_priority
# wildcard add_port emits when acls_in is empty -- otherwise
# remove_overlap_ofmsgs would cancel this delete.
ofmsgs.append(self.port_acl_table.flowdel(in_port_match))
return ofmsgs

def cold_start_port(self, port):
Expand Down Expand Up @@ -599,7 +602,7 @@ def del_authed_mac(self, port_num, mac=None, strict=True):
)
]

def del_port_acl(self, acl, port_num, mac=None):
def del_port_acl(self, acl, port_num, mac=None, priority=None):
"""Delete ACL rules for Port"""

def convert_to_flow_del(ofp_flowmods):
Expand All @@ -613,37 +616,64 @@ def convert_to_flow_del(ofp_flowmods):

return flowdels

pipeline_vlan_table = self.pipeline.vlan_table
if priority is None:
priority = self.auth_priority
flowmods = build_acl_port_of_msgs(
acl,
None,
port_num,
self.port_acl_table,
pipeline_vlan_table,
self.auth_priority,
self.pipeline,
priority,
)
for flow in flowmods:
flow.match = add_mac_address_to_match(flow.match, mac)

return convert_to_flow_del(flowmods)

def add_port_acl(self, acl, port_num, mac=None):
def add_port_acl(self, acl, port_num, mac=None, priority=None):
"""Create ACL openflow rules for Port"""
pipeline_vlan_table = self.pipeline.vlan_table
if priority is None:
priority = self.auth_priority
flowmods = build_acl_port_of_msgs(
acl,
None,
port_num,
self.port_acl_table,
pipeline_vlan_table,
self.auth_priority,
self.pipeline,
priority,
)

for flow in flowmods:
flow.match = add_mac_address_to_match(flow.match, mac)

return flowmods

def add_vlan_acl(self, acl, vlan, priority=None):
"""Create ACL openflow rules for a single VLAN ACL."""
if not acl.rules:
return []
if priority is None:
priority = self.acl_priority
return build_acl_ofmsgs(
[acl],
self.vlan_acl_table,
self.pipeline.accept_to_classification(),
self.pipeline.accept_to_l2_forwarding(),
priority,
acl.meter,
acl.exact_match,
vlan_vid=vlan.vid,
)

def del_vlan_acl(self, acl, vlan, priority=None):
"""Delete ACL rules for a single VLAN ACL."""
flowmods = self.add_vlan_acl(acl, vlan, priority=priority)
return [
self.vlan_acl_table.flowdel(match=fm.match, priority=fm.priority)
for fm in flowmods
]

def create_dot1x_flow_pair(self, port_num, nfv_sw_port_num, mac):
"""Create dot1x flow pair"""
ofmsgs = [
Expand Down
Loading