Skip to content

Commit

Permalink
Skip the validation of action in acl-loader if capability table in ST…
Browse files Browse the repository at this point in the history
…ATE_DB is empty (sonic-net#3199)

* Add skip_action_validation option to acl-loader
  • Loading branch information
bingwang-ms authored and mssonicbld committed Mar 11, 2024
1 parent 9d5daca commit c68f4b1
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 12 deletions.
28 changes: 17 additions & 11 deletions acl_loader/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -413,17 +413,17 @@ def parse_acl_json(filename):
raise AclLoaderException("Invalid input file %s" % filename)
return yang_acl

def load_rules_from_file(self, filename):
def load_rules_from_file(self, filename, skip_action_validation=False):
"""
Load file with ACL rules configuration in openconfig ACL format. Convert rules
to Config DB schema.
:param filename: File in openconfig ACL format
:return:
"""
self.yang_acl = AclLoader.parse_acl_json(filename)
self.convert_rules()
self.convert_rules(skip_action_validation)

def convert_action(self, table_name, rule_idx, rule):
def convert_action(self, table_name, rule_idx, rule, skip_validation=False):
rule_props = {}

if rule.actions.config.forwarding_action == "ACCEPT":
Expand Down Expand Up @@ -452,13 +452,13 @@ def convert_action(self, table_name, rule_idx, rule):
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):
if not self.validate_actions(table_name, rule_props, skip_validation):
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):
def validate_actions(self, table_name, action_props, skip_validation=False):
if self.is_table_control_plane(table_name):
return True

Expand All @@ -481,6 +481,11 @@ def validate_actions(self, table_name, action_props):
else:
aclcapability = self.statedb.get_all(self.statedb.STATE_DB, "{}|{}".format(self.ACL_STAGE_CAPABILITY_TABLE, stage.upper()))
switchcapability = self.statedb.get_all(self.statedb.STATE_DB, "{}|switch".format(self.SWITCH_CAPABILITY_TABLE))
# In the load_minigraph path, it's possible that the STATE_DB entry haven't pop up because orchagent is stopped
# before loading acl.json. So we skip the validation if any table is empty
if skip_validation and (not aclcapability or not switchcapability):
warning("Skipped action validation as capability table is not present in STATE_DB")
return True
for action_key in dict(action_props):
action_list_key = self.ACL_ACTIONS_CAPABILITY_FIELD
if action_list_key not in aclcapability:
Expand Down Expand Up @@ -699,7 +704,7 @@ def validate_rule_fields(self, rule_props):
if ("ICMPV6_TYPE" in rule_props or "ICMPV6_CODE" in rule_props) and protocol != 58:
raise AclLoaderException("IP_PROTOCOL={} is not ICMPV6, but ICMPV6 fields were provided".format(protocol))

def convert_rule_to_db_schema(self, table_name, rule):
def convert_rule_to_db_schema(self, table_name, rule, skip_action_validation=False):
"""
Convert rules format from openconfig ACL to Config DB schema
:param table_name: ACL table name to which rule belong
Expand Down Expand Up @@ -729,7 +734,7 @@ def convert_rule_to_db_schema(self, table_name, rule):
elif self.is_table_l3(table_name):
rule_props["ETHER_TYPE"] = str(self.ethertype_map["ETHERTYPE_IPV4"])

deep_update(rule_props, self.convert_action(table_name, rule_idx, rule))
deep_update(rule_props, self.convert_action(table_name, rule_idx, rule, skip_action_validation))
deep_update(rule_props, self.convert_l2(table_name, rule_idx, rule))
deep_update(rule_props, self.convert_ip(table_name, rule_idx, rule))
deep_update(rule_props, self.convert_icmp(table_name, rule_idx, rule))
Expand Down Expand Up @@ -761,7 +766,7 @@ def deny_rule(self, table_name):
return {} # Don't add default deny rule if table is not [L3, L3V6]
return rule_data

def convert_rules(self):
def convert_rules(self, skip_aciton_validation=False):
"""
Convert rules in openconfig ACL format to Config DB schema
:return:
Expand All @@ -780,7 +785,7 @@ def convert_rules(self):
for acl_entry_name in acl_set.acl_entries.acl_entry:
acl_entry = acl_set.acl_entries.acl_entry[acl_entry_name]
try:
rule = self.convert_rule_to_db_schema(table_name, acl_entry)
rule = self.convert_rule_to_db_schema(table_name, acl_entry, skip_aciton_validation)
deep_update(self.rules_info, rule)
except AclLoaderException as ex:
error("Error processing rule %s: %s. Skipped." % (acl_entry_name, ex))
Expand Down Expand Up @@ -1149,8 +1154,9 @@ def update(ctx):
@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.option('--skip_action_validation', is_flag=True, default=False, help="Skip action validation")
@click.pass_context
def full(ctx, filename, table_name, session_name, mirror_stage, max_priority):
def full(ctx, filename, table_name, session_name, mirror_stage, max_priority, skip_action_validation):
"""
Full update of ACL rules configuration.
If a table_name is provided, the operation will be restricted in the specified table.
Expand All @@ -1168,7 +1174,7 @@ def full(ctx, filename, table_name, session_name, mirror_stage, max_priority):
if max_priority:
acl_loader.set_max_priority(max_priority)

acl_loader.load_rules_from_file(filename)
acl_loader.load_rules_from_file(filename, skip_action_validation)
acl_loader.full_update()


Expand Down
2 changes: 1 addition & 1 deletion config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -1745,7 +1745,7 @@ def load_minigraph(db, no_service_restart, traffic_shift_away, override_config,
update_sonic_environment()

if os.path.isfile('/etc/sonic/acl.json'):
clicommon.run_command(['acl-loader', 'update', 'full', '/etc/sonic/acl.json'], display_cmd=True)
clicommon.run_command(['acl-loader', 'update', 'full', '/etc/sonic/acl.json', '--skip_action_validation'], display_cmd=True)

# Load port_config.json
try:
Expand Down
30 changes: 30 additions & 0 deletions tests/acl_loader_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,36 @@ def test_validate_mirror_action(self, acl_loader):
assert acl_loader.validate_actions("DATAACL", forward_packet_action)
assert not acl_loader.validate_actions("DATAACL", drop_packet_action)

def test_load_rules_when_capability_table_is_empty(self, acl_loader):
"""
Test case to verify that acl_loader can still load dataplane acl rules when skip_action_validation
is true, and capability table in state_db is absent
"""
# Backup and empty the capability table from state_db
SWITCH_CAPABILITY = "SWITCH_CAPABILITY|switch"
if acl_loader.per_npu_statedb:
statedb = list(acl_loader.per_npu_statedb.values())[0]
else:
statedb = acl_loader.statedb
switchcapability = statedb.get_all("STATE_DB", SWITCH_CAPABILITY)
statedb.delete("STATE_DB", SWITCH_CAPABILITY)
try:
acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/acl1.json'), skip_action_validation=True)
assert acl_loader.rules_info[("DATAACL", "RULE_2")]
assert acl_loader.rules_info[("DATAACL", "RULE_2")] == {
"VLAN_ID": 369,
"ETHER_TYPE": "2048",
"IP_PROTOCOL": 6,
"SRC_IP": "20.0.0.2/32",
"DST_IP": "30.0.0.3/32",
"PACKET_ACTION": "FORWARD",
"PRIORITY": "9998"
}
finally:
# Restore the capability table in state_db
for key, value in switchcapability.items():
statedb.set("STATE_DB", SWITCH_CAPABILITY, key, value)

def test_vlan_id_translation(self, acl_loader):
acl_loader.rules_info = {}
acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/acl1.json'))
Expand Down

0 comments on commit c68f4b1

Please sign in to comment.