Skip to content

Commit

Permalink
[acl-loader] Only add default deny rule when table is L3 or L3V6 (#27…
Browse files Browse the repository at this point in the history 
…96) (#2826)

What I did
1. Update acl-loader to only add default deny rule when table is L3 or L3V6.
2. Update unittest to cover it.

How I did it
Update function deny_rule and return {} if table is not L3 or L3V6.

How to verify it
1. Update unittest and run all testcases to verify.
2. Built the package and installed on DUT to verify.

Signed-off-by: Zhijian Li <zhijianli@microsoft.com>
  • Loading branch information
@lizhijianrd
lizhijianrd authored May 4, 2023
1 parent b49d78c commit f6359bc
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 6 deletions.
9 changes: 6 additions & 3 deletions acl_loader/main.py
View file
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
View file
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
View file
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
View file
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
View file
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"
},
"VLAN|Vlan1000": {
"dhcp_servers@": "192.0.0.1,192.0.0.2,192.0.0.3,192.0.0.4",
"vlanid": "1000"
Expand Down

0 comments on commit f6359bc

Please sign in to comment.