Skip to content

Commit 38c8e00

Browse files
daalldaall
authored
[acl-loader] Add support for matching on ICMP and VLAN info (sonic-net#1469)
- Add ICMP and VLAN fields - Add new unit test cases Signed-off-by: Danny Allen <daall@microsoft.com>
1 parent e555ea9 commit 38c8e00

14 files changed

Lines changed: 590 additions & 10 deletions

‎acl_loader/main.py‎

Lines changed: 53 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ class AclLoader(object):
103103
"IP_RSVP": 46,
104104
"IP_GRE": 47,
105105
"IP_AUTH": 51,
106+
"IP_ICMPV6": 58,
106107
"IP_L2TP": 115
107108
}
108109

@@ -290,6 +291,14 @@ def is_table_mirror(self, tname):
290291
"""
291292
return self.tables_db_info[tname]['type'].upper().startswith(self.ACL_TABLE_TYPE_MIRROR)
292293

294+
def is_table_ipv6(self, tname):
295+
"""
296+
Check if ACL table type is IPv6 (L3V6 or MIRRORV6)
297+
:param tname: ACL table name
298+
:return: True if table type is IPv6 else False
299+
"""
300+
return self.tables_db_info[tname]["type"].upper() in ("L3V6", "MIRRORV6")
301+
293302
def is_table_control_plane(self, tname):
294303
"""
295304
Check if ACL table type is ACL_TABLE_TYPE_CTRLPLANE
@@ -409,9 +418,18 @@ def convert_l2(self, table_name, rule_idx, rule):
409418
else:
410419
try:
411420
rule_props["ETHER_TYPE"] = int(rule.l2.config.ethertype)
412-
except:
413-
raise AclLoaderException("Failed to convert ethertype %s table %s rule %s" % (
414-
rule.l2.config.ethertype, table_name, rule_idx))
421+
except Exception as e:
422+
raise AclLoaderException(
423+
"Failed to convert ethertype %s; table %s rule %s; exception=%s" %
424+
(rule.l2.config.ethertype, table_name, rule_idx, str(e)))
425+
426+
if rule.l2.config.vlan_id != "" and rule.l2.config.vlan_id != "null":
427+
vlan_id = rule.l2.config.vlan_id
428+
429+
if vlan_id <= 0 or vlan_id >= 4096:
430+
raise AclLoaderException("VLAN ID %d is out of bounds (0, 4096)" % (vlan_id))
431+
432+
rule_props["VLAN_ID"] = vlan_id
415433

416434
return rule_props
417435

@@ -422,7 +440,12 @@ def convert_ip(self, table_name, rule_idx, rule):
422440
# so there isn't currently a good way to check if the user defined proto=0 or not.
423441
if rule.ip.config.protocol:
424442
if rule.ip.config.protocol in self.ip_protocol_map:
425-
rule_props["IP_PROTOCOL"] = self.ip_protocol_map[rule.ip.config.protocol]
443+
# Special case: ICMP has different protocol numbers for IPv4 and IPv6, so if we receive
444+
# "IP_ICMP" we need to pick the correct protocol number for the IP version
445+
if rule.ip.config.protocol == "IP_ICMP" and self.is_table_ipv6(table_name):
446+
rule_props["IP_PROTOCOL"] = self.ip_protocol_map["IP_ICMPV6"]
447+
else:
448+
rule_props["IP_PROTOCOL"] = self.ip_protocol_map[rule.ip.config.protocol]
426449
else:
427450
try:
428451
int(rule.ip.config.protocol)
@@ -453,6 +476,31 @@ def convert_ip(self, table_name, rule_idx, rule):
453476

454477
return rule_props
455478

479+
def convert_icmp(self, table_name, rule_idx, rule):
480+
rule_props = {}
481+
482+
is_table_v6 = self.is_table_ipv6(table_name)
483+
type_key = "ICMPV6_TYPE" if is_table_v6 else "ICMP_TYPE"
484+
code_key = "ICMPV6_CODE" if is_table_v6 else "ICMP_CODE"
485+
486+
if rule.icmp.config.type != "" and rule.icmp.config.type != "null":
487+
icmp_type = rule.icmp.config.type
488+
489+
if icmp_type < 0 or icmp_type > 255:
490+
raise AclLoaderException("ICMP type %d is out of bounds [0, 255]" % (icmp_type))
491+
492+
rule_props[type_key] = icmp_type
493+
494+
if rule.icmp.config.code != "" and rule.icmp.config.code != "null":
495+
icmp_code = rule.icmp.config.code
496+
497+
if icmp_code < 0 or icmp_code > 255:
498+
raise AclLoaderException("ICMP code %d is out of bounds [0, 255]" % (icmp_code))
499+
500+
rule_props[code_key] = icmp_code
501+
502+
return rule_props
503+
456504
def convert_port(self, port):
457505
"""
458506
Convert port field format from openconfig ACL to Config DB schema
@@ -527,6 +575,7 @@ def convert_rule_to_db_schema(self, table_name, rule):
527575
deep_update(rule_props, self.convert_action(table_name, rule_idx, rule))
528576
deep_update(rule_props, self.convert_l2(table_name, rule_idx, rule))
529577
deep_update(rule_props, self.convert_ip(table_name, rule_idx, rule))
578+
deep_update(rule_props, self.convert_icmp(table_name, rule_idx, rule))
530579
deep_update(rule_props, self.convert_transport(table_name, rule_idx, rule))
531580
deep_update(rule_props, self.convert_input_interface(table_name, rule_idx, rule))
532581

‎tests/acl_input/acl1.json‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,108 @@
141141
"config": {
142142
"name": "everflowV6"
143143
}
144+
},
145+
"DATAACL": {
146+
"acl-entries": {
147+
"acl-entry": {
148+
"1": {
149+
"config": {
150+
"sequence-id": 1
151+
},
152+
"actions": {
153+
"config": {
154+
"forwarding-action": "ACCEPT"
155+
}
156+
},
157+
"ip": {
158+
"config": {
159+
"protocol": "IP_ICMP",
160+
"source-ip-address": "20.0.0.2/32",
161+
"destination-ip-address": "30.0.0.3/32"
162+
}
163+
},
164+
"icmp": {
165+
"config": {
166+
"type": "3",
167+
"code": "0"
168+
}
169+
}
170+
},
171+
"2": {
172+
"config": {
173+
"sequence-id": 2
174+
},
175+
"actions": {
176+
"config": {
177+
"forwarding-action": "ACCEPT"
178+
}
179+
},
180+
"l2": {
181+
"config": {
182+
"vlan-id": "369"
183+
}
184+
},
185+
"ip": {
186+
"config": {
187+
"protocol": "IP_TCP",
188+
"source-ip-address": "20.0.0.2/32",
189+
"destination-ip-address": "30.0.0.3/32"
190+
}
191+
}
192+
}
193+
}
194+
}
195+
},
196+
"DATAACLV6": {
197+
"acl-entries": {
198+
"acl-entry": {
199+
"1": {
200+
"config": {
201+
"sequence-id": 1
202+
},
203+
"actions": {
204+
"config": {
205+
"forwarding-action": "ACCEPT"
206+
}
207+
},
208+
"ip": {
209+
"config": {
210+
"protocol": "IP_ICMP",
211+
"source-ip-address": "::1/128",
212+
"destination-ip-address": "::1/128"
213+
}
214+
},
215+
"icmp": {
216+
"config": {
217+
"type": "1",
218+
"code": "0"
219+
}
220+
}
221+
},
222+
"2": {
223+
"config": {
224+
"sequence-id": 100
225+
},
226+
"actions": {
227+
"config": {
228+
"forwarding-action": "ACCEPT"
229+
}
230+
},
231+
"ip": {
232+
"config": {
233+
"protocol": "IP_ICMP",
234+
"source-ip-address": "::1/128",
235+
"destination-ip-address": "::1/128"
236+
}
237+
},
238+
"icmp": {
239+
"config": {
240+
"type": "128"
241+
}
242+
}
243+
}
244+
}
245+
}
144246
}
145247
}
146248
}

‎tests/acl_input/illegal_icmp_code_300.json‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
{
2+
"acl": {
3+
"acl-sets": {
4+
"acl-set": {
5+
"DATAACL": {
6+
"acl-entries": {
7+
"acl-entry": {
8+
"1": {
9+
"config": {
10+
"sequence-id": 1
11+
},
12+
"actions": {
13+
"config": {
14+
"forwarding-action": "ACCEPT"
15+
}
16+
},
17+
"ip": {
18+
"config": {
19+
"protocol": "IP_ICMP",
20+
"source-ip-address": "20.0.0.2/32",
21+
"destination-ip-address": "30.0.0.3/32"
22+
}
23+
},
24+
"icmp": {
25+
"config": {
26+
"type": "3",
27+
"code": "300"
28+
}
29+
}
30+
}
31+
}
32+
}
33+
}
34+
}
35+
}
36+
}
37+
}

‎tests/acl_input/illegal_icmp_code_nan.json‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
{
2+
"acl": {
3+
"acl-sets": {
4+
"acl-set": {
5+
"DATAACL": {
6+
"acl-entries": {
7+
"acl-entry": {
8+
"1": {
9+
"config": {
10+
"sequence-id": 1
11+
},
12+
"actions": {
13+
"config": {
14+
"forwarding-action": "ACCEPT"
15+
}
16+
},
17+
"ip": {
18+
"config": {
19+
"protocol": "IP_ICMP",
20+
"source-ip-address": "20.0.0.2/32",
21+
"destination-ip-address": "30.0.0.3/32"
22+
}
23+
},
24+
"icmp": {
25+
"config": {
26+
"type": "3",
27+
"code": "foo"
28+
}
29+
}
30+
}
31+
}
32+
}
33+
}
34+
}
35+
}
36+
}
37+
}

‎tests/acl_input/illegal_icmp_code_neg_1.json‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
{
2+
"acl": {
3+
"acl-sets": {
4+
"acl-set": {
5+
"DATAACL": {
6+
"acl-entries": {
7+
"acl-entry": {
8+
"1": {
9+
"config": {
10+
"sequence-id": 1
11+
},
12+
"actions": {
13+
"config": {
14+
"forwarding-action": "ACCEPT"
15+
}
16+
},
17+
"ip": {
18+
"config": {
19+
"protocol": "IP_ICMP",
20+
"source-ip-address": "20.0.0.2/32",
21+
"destination-ip-address": "30.0.0.3/32"
22+
}
23+
},
24+
"icmp": {
25+
"config": {
26+
"type": "3",
27+
"code": "-1"
28+
}
29+
}
30+
}
31+
}
32+
}
33+
}
34+
}
35+
}
36+
}
37+
}

‎tests/acl_input/illegal_icmp_type_300.json‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
{
2+
"acl": {
3+
"acl-sets": {
4+
"acl-set": {
5+
"DATAACL": {
6+
"acl-entries": {
7+
"acl-entry": {
8+
"1": {
9+
"config": {
10+
"sequence-id": 1
11+
},
12+
"actions": {
13+
"config": {
14+
"forwarding-action": "ACCEPT"
15+
}
16+
},
17+
"ip": {
18+
"config": {
19+
"protocol": "IP_ICMP",
20+
"source-ip-address": "20.0.0.2/32",
21+
"destination-ip-address": "30.0.0.3/32"
22+
}
23+
},
24+
"icmp": {
25+
"config": {
26+
"type": "300",
27+
"code": "0"
28+
}
29+
}
30+
}
31+
}
32+
}
33+
}
34+
}
35+
}
36+
}
37+
}

‎tests/acl_input/illegal_icmp_type_nan.json‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
{
2+
"acl": {
3+
"acl-sets": {
4+
"acl-set": {
5+
"DATAACL": {
6+
"acl-entries": {
7+
"acl-entry": {
8+
"1": {
9+
"config": {
10+
"sequence-id": 1
11+
},
12+
"actions": {
13+
"config": {
14+
"forwarding-action": "ACCEPT"
15+
}
16+
},
17+
"ip": {
18+
"config": {
19+
"protocol": "IP_ICMP",
20+
"source-ip-address": "20.0.0.2/32",
21+
"destination-ip-address": "30.0.0.3/32"
22+
}
23+
},
24+
"icmp": {
25+
"config": {
26+
"type": "foo",
27+
"code": "0"
28+
}
29+
}
30+
}
31+
}
32+
}
33+
}
34+
}
35+
}
36+
}
37+
}

0 commit comments

Comments
 (0)