Skip to content

Commit f6359bc

Browse files
lizhijianrdlizhijianrd
authored
[acl-loader] Only add default deny rule when table is L3 or L3V6 (#2796) (#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 <[email protected]>
1 parent b49d78c commit f6359bc

File tree

5 files changed

+104
-6
lines changed

5 files changed

+104
-6
lines changed

‎acl_loader/main.py

+6-3
Original file line numberDiff line numberDiff line change
@@ -670,17 +670,20 @@ def convert_rule_to_db_schema(self, table_name, rule):
670670
def deny_rule(self, table_name):
671671
"""
672672
Create default deny rule in Config DB format
673+
Only create default deny rule when table is [L3, L3V6]
673674
:param table_name: ACL table name to which rule belong
674675
:return: dict with Config DB schema
675676
"""
676677
rule_props = {}
677678
rule_data = {(table_name, "DEFAULT_RULE"): rule_props}
678679
rule_props["PRIORITY"] = str(self.min_priority)
679680
rule_props["PACKET_ACTION"] = "DROP"
680-
if self.is_table_ipv6(table_name):
681+
if self.is_table_l3v6(table_name):
681682
rule_props["IP_TYPE"] = "IPV6ANY" # ETHERTYPE is not supported for DATAACLV6
682-
else:
683+
elif self.is_table_l3(table_name):
683684
rule_props["ETHER_TYPE"] = str(self.ethertype_map["ETHERTYPE_IPV4"])
685+
else:
686+
return {} # Don't add default deny rule if table is not [L3, L3V6]
684687
return rule_data
685688

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

710-
if not self.is_table_mirror(table_name) and not self.is_table_egress(table_name):
713+
if not self.is_table_egress(table_name):
711714
deep_update(self.rules_info, self.deny_rule(table_name))
712715

713716
def full_update(self):

‎tests/acl_input/acl1.json

+57
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,63 @@
259259
}
260260
}
261261
}
262+
},
263+
"bmc_acl_northbound": {
264+
"acl-entries": {
265+
"acl-entry": {
266+
"1": {
267+
"config": {
268+
"sequence-id": 1
269+
},
270+
"actions": {
271+
"config": {
272+
"forwarding-action": "ACCEPT"
273+
}
274+
},
275+
"ip": {
276+
"config": {
277+
"protocol": "0",
278+
"source-ip-address": "172.17.0.200/30"
279+
}
280+
},
281+
"input_interface": {
282+
"interface_ref": {
283+
"config": {
284+
"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"
285+
}
286+
}
287+
}
288+
}
289+
}
290+
},
291+
"config": {
292+
"name": "bmc_acl_northbound"
293+
}
294+
},
295+
"bmc_acl_northbound_v6": {
296+
"acl-entries": {
297+
"acl-entry": {
298+
"1": {
299+
"config": {
300+
"sequence-id": 1
301+
},
302+
"actions": {
303+
"config": {
304+
"forwarding-action": "ACCEPT"
305+
}
306+
},
307+
"ip": {
308+
"config": {
309+
"protocol": "0",
310+
"destination-ip-address": "fc02::/64"
311+
}
312+
}
313+
}
314+
}
315+
},
316+
"config": {
317+
"name": "bmc_acl_northbound_v6"
318+
}
262319
}
263320
}
264321
}

‎tests/acl_loader_test.py

+12-2
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ def test_acl_empty(self):
2121

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

2626
def test_invalid(self):
2727
with pytest.raises(AclLoaderException):
@@ -135,19 +135,29 @@ def test_icmpv6_translation(self, acl_loader):
135135
}
136136

137137
def test_ingress_default_deny_rule(self, acl_loader):
138+
acl_loader.set_mirror_stage("ingress")
139+
acl_loader.get_session_name = mock.MagicMock(return_value="everflow_session_mock")
138140
acl_loader.rules_info = {}
139141
acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/acl1.json'))
140-
print(acl_loader.rules_info)
142+
# Verify L3 can add default deny rule correctly
141143
assert acl_loader.rules_info[('DATAACL', 'DEFAULT_RULE')] == {
142144
'PRIORITY': '1',
143145
'PACKET_ACTION': 'DROP',
144146
'ETHER_TYPE': '2048'
145147
}
148+
# Verify L3V6 can add default deny rule correctly
146149
assert acl_loader.rules_info[('DATAACL_2', 'DEFAULT_RULE')] == {
147150
'PRIORITY': '1',
148151
'PACKET_ACTION': 'DROP',
149152
'IP_TYPE': 'IPV6ANY'
150153
}
154+
# Verify acl-loader doesn't add default deny rule to MIRROR
155+
assert ('EVERFLOW', 'DEFAULT_RULE') not in acl_loader.rules_info
156+
# Verify acl-loader doesn't add default deny rule to MIRRORV6
157+
assert ('EVERFLOWV6', 'DEFAULT_RULE') not in acl_loader.rules_info
158+
# Verify acl-loader doesn't add default deny rule to custom ACL table types
159+
assert ('BMC_ACL_NORTHBOUND', 'DEFAULT_RULE') not in acl_loader.rules_info
160+
assert ('BMC_ACL_NORTHBOUND_V6', 'DEFAULT_RULE') not in acl_loader.rules_info
151161

152162
def test_egress_no_default_deny_rule(self, acl_loader):
153163
acl_loader.rules_info = {}

‎tests/aclshow_test.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@
9090
# Expected output for aclshow -r RULE_4,RULE_6 -vv
9191
rule4_rule6_verbose_output = '' + \
9292
"""Reading ACL info...
93-
Total number of ACL Tables: 12
93+
Total number of ACL Tables: 15
9494
Total number of ACL Rules: 21
9595
9696
RULE NAME TABLE NAME PRIO PACKETS COUNT BYTES COUNT

‎tests/mock_tables/config_db.json

+28
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,18 @@
511511
"ports@": "PortChannel0002,PortChannel0005,PortChannel0008,PortChannel0011,PortChannel0014,PortChannel0017,PortChannel0020,PortChannel0023",
512512
"type": "L3V6"
513513
},
514+
"ACL_TABLE|BMC_ACL_NORTHBOUND": {
515+
"policy_desc": "BMC_ACL_NORTHBOUND",
516+
"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",
517+
"stage": "ingress",
518+
"type": "BMCDATA"
519+
},
520+
"ACL_TABLE|BMC_ACL_NORTHBOUND_V6": {
521+
"policy_desc": "BMC_ACL_NORTHBOUND_V6",
522+
"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",
523+
"stage": "ingress",
524+
"type": "BMCDATAV6"
525+
},
514526
"ACL_TABLE|DATAACL": {
515527
"policy_desc": "DATAACL",
516528
"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",
@@ -549,6 +561,12 @@
549561
"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",
550562
"type": "MIRROR"
551563
},
564+
"ACL_TABLE|EVERFLOWV6": {
565+
"policy_desc": "EVERFLOWV6",
566+
"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",
567+
"type": "MIRRORV6",
568+
"stage": "ingress"
569+
},
552570
"ACL_TABLE|EVERFLOW_EGRESS": {
553571
"policy_desc": "EGRESS EVERFLOW",
554572
"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",
@@ -565,6 +583,16 @@
565583
"services@": "SSH",
566584
"type": "CTRLPLANE"
567585
},
586+
"ACL_TABLE_TYPE|BMCDATA": {
587+
"ACTIONS": "PACKET_ACTION,COUNTER",
588+
"BIND_POINTS": "PORT",
589+
"MATCHES": "SRC_IP,DST_IP,ETHER_TYPE,IP_TYPE,IP_PROTOCOL,IN_PORTS,TCP_FLAGS"
590+
},
591+
"ACL_TABLE_TYPE|BMCDATAV6": {
592+
"ACTIONS": "PACKET_ACTION,COUNTER",
593+
"BIND_POINTS": "PORT",
594+
"MATCHES": "SRC_IPV6,DST_IPV6,ETHER_TYPE,IP_TYPE,IP_PROTOCOL,IN_PORTS,TCP_FLAGS"
595+
},
568596
"VLAN|Vlan1000": {
569597
"dhcp_servers@": "192.0.0.1,192.0.0.2,192.0.0.3,192.0.0.4",
570598
"vlanid": "1000"

0 commit comments

Comments
 (0)