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] Only add default deny rule when table is L3 or L3V6 #2796

Merged
merged 8 commits into from
Apr 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 6 additions & 3 deletions acl_loader/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -670,17 +670,20 @@ def convert_rule_to_db_schema(self, table_name, rule):
def deny_rule(self, table_name):
"""
Create default deny rule in Config DB format
Only create default deny rule when table is [L3, L3V6]
:param table_name: ACL table name to which rule belong
:return: dict with Config DB schema
"""
rule_props = {}
rule_data = {(table_name, "DEFAULT_RULE"): rule_props}
rule_props["PRIORITY"] = str(self.min_priority)
rule_props["PACKET_ACTION"] = "DROP"
if self.is_table_ipv6(table_name):
if self.is_table_l3v6(table_name):
rule_props["IP_TYPE"] = "IPV6ANY" # ETHERTYPE is not supported for DATAACLV6
else:
elif self.is_table_l3(table_name):
rule_props["ETHER_TYPE"] = str(self.ethertype_map["ETHERTYPE_IPV4"])
else:
return {} # Don't add default deny rule if table is not [L3, L3V6]
return rule_data

def convert_rules(self):
Expand All @@ -707,7 +710,7 @@ def convert_rules(self):
except AclLoaderException as ex:
error("Error processing rule %s: %s. Skipped." % (acl_entry_name, ex))

if not self.is_table_mirror(table_name) and not self.is_table_egress(table_name):
if not self.is_table_egress(table_name):
deep_update(self.rules_info, self.deny_rule(table_name))

def full_update(self):
Expand Down
57 changes: 57 additions & 0 deletions tests/acl_input/acl1.json
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,63 @@
}
}
}
},
"bmc_acl_northbound": {
"acl-entries": {
"acl-entry": {
"1": {
"config": {
"sequence-id": 1
},
"actions": {
"config": {
"forwarding-action": "ACCEPT"
}
},
"ip": {
"config": {
"protocol": "0",
"source-ip-address": "172.17.0.200/30"
}
},
"input_interface": {
"interface_ref": {
"config": {
"interface": "Ethernet0,Ethernet1,Ethernet2,Ethernet3,Ethernet4,Ethernet5,Ethernet6,Ethernet7,Ethernet8,Ethernet9,Ethernet10,Ethernet11,Ethernet12,Ethernet13,Ethernet14,Ethernet15,Ethernet16,Ethernet17,Ethernet18,Ethernet19,Ethernet20,Ethernet21,Ethernet22,Ethernet23,Ethernet25,Ethernet26,Ethernet27,Ethernet28,Ethernet29,Ethernet30,Ethernet31,Ethernet32,Ethernet33,Ethernet34,Ethernet35,Ethernet36,Ethernet37,Ethernet38,Ethernet39,Ethernet40,Ethernet41,Ethernet42,Ethernet43,Ethernet44,Ethernet45"
}
}
}
}
}
},
"config": {
"name": "bmc_acl_northbound"
}
},
"bmc_acl_northbound_v6": {
"acl-entries": {
"acl-entry": {
"1": {
"config": {
"sequence-id": 1
},
"actions": {
"config": {
"forwarding-action": "ACCEPT"
}
},
"ip": {
"config": {
"protocol": "0",
"destination-ip-address": "fc02::/64"
}
}
}
}
},
"config": {
"name": "bmc_acl_northbound_v6"
}
}
}
}
Expand Down
14 changes: 12 additions & 2 deletions tests/acl_loader_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def test_acl_empty(self):

def test_valid(self):
yang_acl = AclLoader.parse_acl_json(os.path.join(test_path, 'acl_input/acl1.json'))
assert len(yang_acl.acl.acl_sets.acl_set) == 6
assert len(yang_acl.acl.acl_sets.acl_set) == 8

def test_invalid(self):
with pytest.raises(AclLoaderException):
Expand Down Expand Up @@ -135,19 +135,29 @@ def test_icmpv6_translation(self, acl_loader):
}

def test_ingress_default_deny_rule(self, acl_loader):
acl_loader.set_mirror_stage("ingress")
acl_loader.get_session_name = mock.MagicMock(return_value="everflow_session_mock")
acl_loader.rules_info = {}
acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/acl1.json'))
print(acl_loader.rules_info)
# Verify L3 can add default deny rule correctly
assert acl_loader.rules_info[('DATAACL', 'DEFAULT_RULE')] == {
'PRIORITY': '1',
'PACKET_ACTION': 'DROP',
'ETHER_TYPE': '2048'
}
# Verify L3V6 can add default deny rule correctly
assert acl_loader.rules_info[('DATAACL_2', 'DEFAULT_RULE')] == {
'PRIORITY': '1',
'PACKET_ACTION': 'DROP',
'IP_TYPE': 'IPV6ANY'
}
# Verify acl-loader doesn't add default deny rule to MIRROR
assert ('EVERFLOW', 'DEFAULT_RULE') not in acl_loader.rules_info
# Verify acl-loader doesn't add default deny rule to MIRRORV6
assert ('EVERFLOWV6', 'DEFAULT_RULE') not in acl_loader.rules_info
# Verify acl-loader doesn't add default deny rule to custom ACL table types
assert ('BMC_ACL_NORTHBOUND', 'DEFAULT_RULE') not in acl_loader.rules_info
assert ('BMC_ACL_NORTHBOUND_V6', 'DEFAULT_RULE') not in acl_loader.rules_info

def test_egress_no_default_deny_rule(self, acl_loader):
acl_loader.rules_info = {}
Expand Down
2 changes: 1 addition & 1 deletion tests/aclshow_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
# Expected output for aclshow -r RULE_4,RULE_6 -vv
rule4_rule6_verbose_output = '' + \
"""Reading ACL info...
Total number of ACL Tables: 12
Total number of ACL Tables: 15
Total number of ACL Rules: 21

RULE NAME TABLE NAME PRIO PACKETS COUNT BYTES COUNT
Expand Down
28 changes: 28 additions & 0 deletions tests/mock_tables/config_db.json
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,18 @@
"ports@": "PortChannel0002,PortChannel0005,PortChannel0008,PortChannel0011,PortChannel0014,PortChannel0017,PortChannel0020,PortChannel0023",
"type": "L3V6"
},
"ACL_TABLE|BMC_ACL_NORTHBOUND": {
"policy_desc": "BMC_ACL_NORTHBOUND",
"ports@": "Ethernet8,Ethernet0,Ethernet17,Ethernet16,Ethernet37,Ethernet4,Ethernet13,Ethernet45,Ethernet19,Ethernet24,Ethernet31,Ethernet30,Ethernet39,Ethernet40,Ethernet27,Ethernet43,Ethernet7,Ethernet3,Ethernet20,Ethernet6,Ethernet12,Ethernet34,Ethernet26,Ethernet11,Ethernet42,Ethernet5,Ethernet32,Ethernet36,Ethernet15,Ethernet33,Ethernet35,Ethernet9,Ethernet29,Ethernet21,Ethernet18,Ethernet38,Ethernet23,Ethernet41,Ethernet14,Ethernet2,Ethernet28,Ethernet22,Ethernet10,Ethernet25,Ethernet44,Ethernet1",
"stage": "ingress",
"type": "BMCDATA"
},
"ACL_TABLE|BMC_ACL_NORTHBOUND_V6": {
"policy_desc": "BMC_ACL_NORTHBOUND_V6",
"ports@": "Ethernet13,Ethernet3,Ethernet32,Ethernet33,Ethernet34,Ethernet8,Ethernet6,Ethernet44,Ethernet39,Ethernet18,Ethernet15,Ethernet1,Ethernet19,Ethernet7,Ethernet14,Ethernet43,Ethernet40,Ethernet27,Ethernet4,Ethernet36,Ethernet41,Ethernet10,Ethernet31,Ethernet5,Ethernet9,Ethernet12,Ethernet16,Ethernet25,Ethernet24,Ethernet17,Ethernet35,Ethernet11,Ethernet38,Ethernet42,Ethernet29,Ethernet20,Ethernet45,Ethernet26,Ethernet21,Ethernet37,Ethernet0,Ethernet28,Ethernet23,Ethernet22,Ethernet2,Ethernet30",
"stage": "ingress",
"type": "BMCDATAV6"
},
"ACL_TABLE|DATAACL": {
"policy_desc": "DATAACL",
"ports@": "PortChannel0002,PortChannel0005,PortChannel0008,PortChannel0011,PortChannel0014,PortChannel0017,PortChannel0020,PortChannel0023,Ethernet64,Ethernet68,Ethernet72,Ethernet76,Ethernet80,Ethernet84,Ethernet88,Ethernet92,Ethernet96,Ethernet100,Ethernet104,Ethernet108,Ethernet112,Ethernet116,Ethernet120,Ethernet124",
Expand Down Expand Up @@ -549,6 +561,12 @@
"ports@": "PortChannel0002,PortChannel0005,PortChannel0008,PortChannel0011,PortChannel0014,PortChannel0017,PortChannel0020,PortChannel0023,Ethernet100,Ethernet104,Ethernet92,Ethernet96,Ethernet84,Ethernet88,Ethernet76,Ethernet80,Ethernet108,Ethernet112,Ethernet64,Ethernet120,Ethernet116,Ethernet124,Ethernet72,Ethernet68",
"type": "MIRROR"
},
"ACL_TABLE|EVERFLOWV6": {
"policy_desc": "EVERFLOWV6",
"ports@": "Ethernet0,Ethernet1,Ethernet2,Ethernet3,Ethernet4,Ethernet5,Ethernet6,Ethernet7,Ethernet8,Ethernet9,Ethernet10,Ethernet11,Ethernet12,Ethernet13,Ethernet14,Ethernet15,Ethernet16,Ethernet17,Ethernet18,Ethernet19,Ethernet20,Ethernet21,Ethernet22,Ethernet23,Ethernet24,Ethernet25,Ethernet26,Ethernet27,Ethernet28,Ethernet29,Ethernet30,Ethernet31,Ethernet32,Ethernet33,Ethernet34,Ethernet35,Ethernet36,Ethernet37,Ethernet38,Ethernet39,Ethernet40,Ethernet41,Ethernet42,Ethernet43,Ethernet44,Ethernet45,Ethernet46,Ethernet47",
"type": "MIRRORV6",
"stage": "ingress"
},
"ACL_TABLE|EVERFLOW_EGRESS": {
"policy_desc": "EGRESS EVERFLOW",
"ports@": "PortChannel0002,PortChannel0005,PortChannel0008,PortChannel0011,PortChannel0014,PortChannel0017,PortChannel0020,PortChannel0023,Ethernet100,Ethernet104,Ethernet92,Ethernet96,Ethernet84,Ethernet88,Ethernet76,Ethernet80,Ethernet108,Ethernet112,Ethernet64,Ethernet120,Ethernet116,Ethernet124,Ethernet72,Ethernet68",
Expand All @@ -565,6 +583,16 @@
"services@": "SSH",
"type": "CTRLPLANE"
},
"ACL_TABLE_TYPE|BMCDATA": {
"ACTIONS": "PACKET_ACTION,COUNTER",
"BIND_POINTS": "PORT",
"MATCHES": "SRC_IP,DST_IP,ETHER_TYPE,IP_TYPE,IP_PROTOCOL,IN_PORTS,TCP_FLAGS"
},
"ACL_TABLE_TYPE|BMCDATAV6": {
"ACTIONS": "PACKET_ACTION,COUNTER",
"BIND_POINTS": "PORT",
"MATCHES": "SRC_IPV6,DST_IPV6,ETHER_TYPE,IP_TYPE,IP_PROTOCOL,IN_PORTS,TCP_FLAGS"
},
"PBH_TABLE|pbh_table1": {
"description": "NVGRE",
"interface_list@": "Ethernet8,Ethernet60"
Expand Down