Skip to content
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.

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

[acl-loader] egress mirror action support and action ASIC support check #575

Merged
merged 14 commits into from
Oct 7, 2019
Merged
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,6 @@ build/
deb_dist/
dist/
*.egg-info/
*.pyc
.cache
*.tar.gz
stcheng marked this conversation as resolved.
Show resolved Hide resolved
167 changes: 136 additions & 31 deletions acl_loader/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,31 @@ def deep_update(dst, src):
return dst


class AclAction:
""" namespace for ACL action keys """

PACKET = "PACKET_ACTION"
REDIRECT = "REDIRECT_ACTION"
MIRROR = "MIRROR_ACTION"
MIRROR_INGRESS = "MIRROR_INGRESS_ACTION"
MIRROR_EGRESS = "MIRROR_EGRESS_ACTION"


class PacketAction:
""" namespace for ACL packet actions """

DROP = "DROP"
FORWARD = "FORWARD"
ACCEPT = "ACCEPT"


class Stage:
""" namespace for ACL stages """

INGRESS = "INGRESS"
EGRESS = "EGRESS"


class AclLoaderException(Exception):
pass

Expand All @@ -52,6 +77,9 @@ class AclLoader(object):
STATE_MIRROR_SESSION_TABLE = "MIRROR_SESSION_TABLE"
POLICER = "POLICER"
SESSION_PREFIX = "everflow"
SWITCH_CAPABILITY_TABLE = "SWITCH_CAPABILITY"
ACL_ACTIONS_CAPABILITY_FIELD = "ACL_ACTIONS"
ACL_ACTION_CAPABILITY_FIELD = "ACL_ACTION"

min_priority = 1
max_priority = 10000
Expand Down Expand Up @@ -81,6 +109,7 @@ class AclLoader(object):
def __init__(self):
self.yang_acl = None
self.requested_session = None
self.mirror_stage = None
self.current_table = None
self.tables_db_info = {}
self.rules_db_info = {}
Expand Down Expand Up @@ -176,6 +205,14 @@ def set_session_name(self, session_name):

self.requested_session = session_name

def set_mirror_stage(self, stage):
"""
Set mirror stage to be used in ACL mirror rule action
:param session_name: stage 'ingress'/'egress'
:return:
"""
self.mirror_stage = stage.upper()

def set_max_priority(self, priority):
"""
Set rules max priority
Expand Down Expand Up @@ -229,25 +266,72 @@ def convert_action(self, table_name, rule_idx, rule):

if rule.actions.config.forwarding_action == "ACCEPT":
if self.is_table_control_plane(table_name):
rule_props["PACKET_ACTION"] = "ACCEPT"
rule_props[AclAction.PACKET] = PacketAction.ACCEPT
elif self.is_table_mirror(table_name):
session_name = self.get_session_name()
if not session_name:
raise AclLoaderException("Mirroring session does not exist")

rule_props["MIRROR_ACTION"] = session_name
if self.mirror_stage == Stage.INGRESS:
mirror_action = AclAction.MIRROR_INGRESS
elif self.mirror_stage == Stage.EGRESS:
mirror_action = AclAction.MIRROR_EGRESS
else:
raise AclLoaderException("Invalid mirror stage passed {}".format(self.mirror_stage))

rule_props[mirror_action] = session_name
else:
rule_props["PACKET_ACTION"] = "FORWARD"
rule_props[AclAction.PACKET] = PacketAction.FORWARD
elif rule.actions.config.forwarding_action == "DROP":
rule_props["PACKET_ACTION"] = "DROP"
rule_props[AclAction.PACKET] = PacketAction.DROP
elif rule.actions.config.forwarding_action == "REJECT":
rule_props["PACKET_ACTION"] = "DROP"
rule_props[AclAction.PACKET] = PacketAction.DROP
else:
raise AclLoaderException("Unknown rule action %s in table %s, rule %d" % (
raise AclLoaderException("Unknown rule action {} in table {}, rule {}".format(
rule.actions.config.forwarding_action, table_name, rule_idx))

if not self.validate_actions(table_name, rule_props):
raise AclLoaderException("Rule action {} is not supported in table {}, rule {}".format(
rule.actions.config.forwarding_action, table_name, rule_idx))

return rule_props

def validate_actions(self, table_name, action_props):
if self.is_table_control_plane(table_name):
return True

action_count = len(action_props)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right now you're validating the actions based on the number of the actions before and after the check. however, it is based on the assumption that the input action_props only contains actions. but this assumption is based on the order of the conversion in the convert_rule_to_db_schema, which seems weak to me.

will it be better to raise the exceptions immediately when the rules violate the capabilities of the switch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

convert_rule_to_db_schema performs deep_update of rule_props dict (action, l2, ip, etc. key/values). It does not depend on the order. Thus convert_action should return a dict with acl actions only, so safely to assume action_props has only actions


if table_name not in self.tables_db_info:
raise AclLoaderException("Table {} does not exist".format(table_name))

stage = self.tables_db_info[table_name].get("stage", Stage.INGRESS)
capability = self.statedb.get_all(self.statedb.STATE_DB, "{}|switch".format(self.SWITCH_CAPABILITY_TABLE))
for action_key in dict(action_props):
key = "{}|{}".format(self.ACL_ACTIONS_CAPABILITY_FIELD, stage.upper())
if key not in capability:
del action_props[action_key]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not raise immediately?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method validate_actions can validate several actions in one call. This is done to allow later check if some combination of actions in one rule is allowed

continue

values = capability[key].split(",")
if action_key.upper() not in values:
del action_props[action_key]
continue

if action_key == AclAction.PACKET:
# Check if action_value is supported
action_value = action_props[action_key]
key = "{}|{}".format(self.ACL_ACTION_CAPABILITY_FIELD, action_key.upper())
if key not in capability:
del action_props[action_key]
continue

if action_value not in capability[key]:
del action_props[action_key]
continue

return action_count == len(action_props)

def convert_l2(self, table_name, rule_idx, rule):
rule_props = {}

Expand Down Expand Up @@ -507,30 +591,32 @@ def show_table(self, table_name):
:param table_name: Optional. ACL table name. Filter tables by specified name.
:return:
"""
header = ("Name", "Type", "Binding", "Description")
header = ("Name", "Type", "Binding", "Description", "Stage")

data = []
for key, val in self.get_tables_db_info().iteritems():
if table_name and key != table_name:
continue

stage = val.get("stage", Stage.INGRESS).lower()

if val["type"] == AclLoader.ACL_TABLE_TYPE_CTRLPLANE:
services = natsorted(val["services"])
data.append([key, val["type"], services[0], val["policy_desc"]])
data.append([key, val["type"], services[0], val["policy_desc"], stage])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to specify the ingress stage for control plane ACLs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

control plane ACLs are in INPUT chain. Does it make sense to mark them as ingress in this case?


if len(services) > 1:
for service in services[1:]:
data.append(["", "", service, ""])
data.append(["", "", service, "", ""])
else:
if not val["ports"]:
data.append([key, val["type"], "", val["policy_desc"]])
data.append([key, val["type"], "", val["policy_desc"], stage])
else:
ports = natsorted(val["ports"])
data.append([key, val["type"], ports[0], val["policy_desc"]])
data.append([key, val["type"], ports[0], val["policy_desc"], stage])

if len(ports) > 1:
for port in ports[1:]:
data.append(["", "", port, ""])
data.append(["", "", port, "", ""])

print(tabulate.tabulate(data, headers=header, tablefmt="simple", missingval=""))

Expand Down Expand Up @@ -582,7 +668,33 @@ def show_rule(self, table_name, rule_id):
"""
header = ("Table", "Rule", "Priority", "Action", "Match")

ignore_list = ["PRIORITY", "PACKET_ACTION", "MIRROR_ACTION"]
def pop_priority(val):
priority = val.pop("PRIORITY")
return priority

def pop_action(val):
action = ""

for key in dict(val):
key = key.upper()
if key == AclAction.PACKET:
action = val.pop(key)
elif key == AclAction.REDIRECT:
action = "REDIRECT: {}".format(val.pop(key))
elif key in (AclAction.MIRROR, AclAction.MIRROR_INGRESS):
action = "MIRROR INGRESS: {}".format(val.pop(key))
elif key == AclAction.MIRROR_EGRESS:
action = "MIRROR EGRESS: {}".format(val.pop(key))
else:
continue

return action

def pop_matches(val):
matches = list(sorted(["%s: %s" % (k, val[k]) for k in val]))
if len(matches) == 0:
matches.append("N/A")
return matches

raw_data = []
for (tname, rid), val in self.get_rules_db_info().iteritems():
Expand All @@ -593,22 +705,9 @@ def show_rule(self, table_name, rule_id):
if rule_id and rule_id != rid:
continue

priority = val["PRIORITY"]

action = ""
if "PACKET_ACTION" in val:
action = val["PACKET_ACTION"]
elif "MIRROR_ACTION" in val:
action = "MIRROR: %s" % val["MIRROR_ACTION"]
else:
continue

matches = ["%s: %s" % (k, v) for k, v in val.iteritems() if k not in ignore_list]

matches.sort()

if len(matches) == 0:
matches.append("N/A")
priority = pop_priority(val)
action = pop_action(val)
matches = pop_matches(val)

rule_data = [[tname, rid, priority, action, matches[0]]]
if len(matches) > 1:
Expand Down Expand Up @@ -714,9 +813,10 @@ def update(ctx):
@click.argument('filename', type=click.Path(exists=True))
@click.option('--table_name', type=click.STRING, required=False)
@click.option('--session_name', type=click.STRING, required=False)
@click.option('--mirror_stage', type=click.Choice(["ingress", "egress"]), default="ingress")
@click.option('--max_priority', type=click.INT, required=False)
@click.pass_context
def full(ctx, filename, table_name, session_name, max_priority):
def full(ctx, filename, table_name, session_name, mirror_stage, max_priority):
"""
Full update of ACL rules configuration.
If a table_name is provided, the operation will be restricted in the specified table.
Expand All @@ -729,6 +829,8 @@ def full(ctx, filename, table_name, session_name, max_priority):
if session_name:
acl_loader.set_session_name(session_name)

acl_loader.set_mirror_stage(mirror_stage)

if max_priority:
acl_loader.set_max_priority(max_priority)

Expand All @@ -739,9 +841,10 @@ def full(ctx, filename, table_name, session_name, max_priority):
@update.command()
@click.argument('filename', type=click.Path(exists=True))
@click.option('--session_name', type=click.STRING, required=False)
@click.option('--mirror_stage', type=click.Choice(["ingress", "egress"]), default="ingress")
@click.option('--max_priority', type=click.INT, required=False)
@click.pass_context
def incremental(ctx, filename, session_name, max_priority):
def incremental(ctx, filename, session_name, mirror_stage, max_priority):
"""
Incremental update of ACL rule configuration.
"""
Expand All @@ -750,6 +853,8 @@ def incremental(ctx, filename, session_name, max_priority):
if session_name:
acl_loader.set_session_name(session_name)

acl_loader.set_mirror_stage(mirror_stage)

if max_priority:
acl_loader.set_max_priority(max_priority)

Expand Down
7 changes: 5 additions & 2 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -997,7 +997,7 @@ def remove(ctx, interface_name, ip_addr):
ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]")
except ValueError:
ctx.fail("'ip_addr' is not valid.")

exists = False
if if_table:
interfaces = config_db.get_table(if_table)
Expand Down Expand Up @@ -1060,7 +1060,8 @@ def get_acl_bound_ports():
@click.argument("table_type", metavar="<table_type>")
@click.option("-d", "--description")
@click.option("-p", "--ports")
def table(table_name, table_type, description, ports):
@click.option("-s", "--stage", type=click.Choice(["ingress", "egress"]), default="ingress")
def table(table_name, table_type, description, ports, stage):
"""
Add ACL table
"""
Expand All @@ -1079,6 +1080,8 @@ def table(table_name, table_type, description, ports):
else:
table_info["ports@"] = ",".join(get_acl_bound_ports())

table_info["stage"] = stage

config_db.set_entry("ACL_TABLE", table_name, table_info)

#
Expand Down
Loading