From a04ab9778c09b56a5ee313e8c81fbd1e219b8944 Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Fri, 6 Mar 2020 00:04:56 +0000 Subject: [PATCH 1/4] [minigraph.py] Add support for 'OutAcl' keyword and attaching ACLs to VLAN interfaces --- src/sonic-config-engine/minigraph.py | 13 ++++++++++--- src/sonic-config-engine/tests/test_cfggen.py | 14 +++++++------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 3ba6362c6ff0..5f47e11ec10d 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -257,7 +257,12 @@ def parse_dpg(dpg, hname): aclintfs = child.find(str(QName(ns, "AclInterfaces"))) acls = {} for aclintf in aclintfs.findall(str(QName(ns, "AclInterface"))): - aclname = aclintf.find(str(QName(ns, "InAcl"))).text.upper().replace(" ", "_").replace("-", "_") + try: + aclname = aclintf.find(str(QName(ns, "InAcl"))).text.upper().replace(" ", "_").replace("-", "_") + stage = "ingress" + except: + aclname = aclintf.find(str(QName(ns, "OutAcl"))).text.upper().replace(" ", "_").replace("-", "_") + stage = "egress" aclattach = aclintf.find(str(QName(ns, "AttachTo"))).text.split(';') acl_intfs = [] is_mirror = False @@ -274,7 +279,7 @@ def parse_dpg(dpg, hname): # to LAG will be applied to all the LAG members internally by SAI/SDK acl_intfs.append(member) elif vlans.has_key(member): - print >> sys.stderr, "Warning: ACL " + aclname + " is attached to a Vlan interface, which is currently not supported" + acl_intfs.append(member) elif port_alias_map.has_key(member): acl_intfs.append(port_alias_map[member]) # Give a warning if trying to attach ACL to a LAG member interface, correct way is to attach ACL to the LAG interface @@ -297,13 +302,14 @@ def parse_dpg(dpg, hname): break if acl_intfs: acls[aclname] = {'policy_desc': aclname, + 'stage': stage, 'ports': acl_intfs} if is_mirror: acls[aclname]['type'] = 'MIRROR' elif is_mirror_v6: acls[aclname]['type'] = 'MIRRORV6' else: - acls[aclname]['type'] = 'L3' + acls[aclname]['type'] = 'L3' if aclname.lower().find('v6') == -1 else 'L3V6' else: # This ACL has no interfaces to attach to -- consider this a control plane ACL try: @@ -321,6 +327,7 @@ def parse_dpg(dpg, hname): else: acls[aclname] = {'policy_desc': aclname, 'type': 'CTRLPLANE', + 'stage': stage, 'services': [aclservice]} except: print >> sys.stderr, "Warning: Ignoring Control Plane ACL %s without type" % aclname diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index 8288b729584c..c104afe4f685 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -104,13 +104,13 @@ def test_minigraph_acl(self): self.assertEqual(output.strip(), "Warning: Ignoring Control Plane ACL NTP_ACL without type\n" "Warning: ignore interface 'fortyGigE0/2' as it is not in the port_config.ini\n" "Warning: ignore interface 'fortyGigE0/2' in DEVICE_NEIGHBOR as it is not in the port_config.ini\n" - "{'DATAACL': {'type': 'L3', 'policy_desc': 'DATAACL', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04']}, " - "'NTP_ACL': {'services': ['NTP'], 'type': 'CTRLPLANE', 'policy_desc': 'NTP_ACL'}, " - "'EVERFLOW': {'type': 'MIRROR', 'policy_desc': 'EVERFLOW', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04', 'Ethernet4']}, " - "'ROUTER_PROTECT': {'services': ['SSH', 'SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'ROUTER_PROTECT'}, " - "'SNMP_ACL': {'services': ['SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'SNMP_ACL'}, " - "'SSH_ACL': {'services': ['SSH'], 'type': 'CTRLPLANE', 'policy_desc': 'SSH_ACL'}, " - "'EVERFLOWV6': {'type': 'MIRRORV6', 'policy_desc': 'EVERFLOWV6', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04', 'Ethernet4']}}") + "{'DATAACL': {'stage': 'ingress', 'type': 'L3', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04'], 'policy_desc': 'DATAACL'}, " + "'NTP_ACL': {'services': ['NTP'], 'type': 'CTRLPLANE', 'policy_desc': 'NTP_ACL', 'stage': 'ingress'}, " + "'EVERFLOW': {'stage': 'ingress', 'type': 'MIRROR', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04', 'Ethernet4'], 'policy_desc': 'EVERFLOW'}, " + "'ROUTER_PROTECT': {'services': ['SSH', 'SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'ROUTER_PROTECT', 'stage': 'ingress'}, " + "'SNMP_ACL': {'services': ['SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'SNMP_ACL', 'stage': 'ingress'}, " + "'SSH_ACL': {'services': ['SSH'], 'type': 'CTRLPLANE', 'policy_desc': 'SSH_ACL', 'stage': 'ingress'}, " + "'EVERFLOWV6': {'stage': 'ingress', 'type': 'MIRRORV6', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04', 'Ethernet4'], 'policy_desc': 'EVERFLOWV6'}}") # everflow portion is not used # def test_minigraph_everflow(self): From 74277fab5c0b01991e2a274607bb54042e2e4ea4 Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Fri, 13 Mar 2020 18:42:16 +0000 Subject: [PATCH 2/4] Exit in error if AclInterface contains neither an InAcl or OutAcl element --- src/sonic-config-engine/minigraph.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 5f47e11ec10d..3e14534e5245 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -257,12 +257,14 @@ def parse_dpg(dpg, hname): aclintfs = child.find(str(QName(ns, "AclInterfaces"))) acls = {} for aclintf in aclintfs.findall(str(QName(ns, "AclInterface"))): - try: + if aclintf.find(str(QName(ns, "InAcl"))) is not None: aclname = aclintf.find(str(QName(ns, "InAcl"))).text.upper().replace(" ", "_").replace("-", "_") stage = "ingress" - except: + elif aclintf.find(str(QName(ns, "OutAcl"))) is not None: aclname = aclintf.find(str(QName(ns, "OutAcl"))).text.upper().replace(" ", "_").replace("-", "_") stage = "egress" + else: + system.exit("Error: 'AclInterface' must contain either an 'InAcl' or 'OutAcl' subelement.") aclattach = aclintf.find(str(QName(ns, "AttachTo"))).text.split(';') acl_intfs = [] is_mirror = False From 7488d66ea7afcb9bbe8a9dafc603e537533be53d Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Fri, 13 Mar 2020 18:44:41 +0000 Subject: [PATCH 3/4] Address comment --- src/sonic-config-engine/minigraph.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 3e14534e5245..0f2cbfc60522 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -311,7 +311,7 @@ def parse_dpg(dpg, hname): elif is_mirror_v6: acls[aclname]['type'] = 'MIRRORV6' else: - acls[aclname]['type'] = 'L3' if aclname.lower().find('v6') == -1 else 'L3V6' + acls[aclname]['type'] = 'L3V6' if 'v6' in aclname.lower() else 'L3' else: # This ACL has no interfaces to attach to -- consider this a control plane ACL try: From ad470ba3861dd22f4b99d54d09aaad10b41ef543 Mon Sep 17 00:00:00 2001 From: Joe LeVeque Date: Fri, 13 Mar 2020 19:17:32 +0000 Subject: [PATCH 4/4] Add egress dataplane ACl to sample T0 graph and update unit test to check it --- src/sonic-config-engine/tests/t0-sample-graph.xml | 7 ++++++- src/sonic-config-engine/tests/test_cfggen.py | 5 +++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/sonic-config-engine/tests/t0-sample-graph.xml b/src/sonic-config-engine/tests/t0-sample-graph.xml index 0c641107da06..47985f870e50 100644 --- a/src/sonic-config-engine/tests/t0-sample-graph.xml +++ b/src/sonic-config-engine/tests/t0-sample-graph.xml @@ -305,7 +305,12 @@ PortChannel01;PortChannel02;PortChannel03;PortChannel04 - DataAcl + DataAclIngress + DataPlane + + + PortChannel01;PortChannel02 + DataAclEgress DataPlane diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index c104afe4f685..2ae28946a905 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -104,12 +104,13 @@ def test_minigraph_acl(self): self.assertEqual(output.strip(), "Warning: Ignoring Control Plane ACL NTP_ACL without type\n" "Warning: ignore interface 'fortyGigE0/2' as it is not in the port_config.ini\n" "Warning: ignore interface 'fortyGigE0/2' in DEVICE_NEIGHBOR as it is not in the port_config.ini\n" - "{'DATAACL': {'stage': 'ingress', 'type': 'L3', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04'], 'policy_desc': 'DATAACL'}, " - "'NTP_ACL': {'services': ['NTP'], 'type': 'CTRLPLANE', 'policy_desc': 'NTP_ACL', 'stage': 'ingress'}, " + "{'NTP_ACL': {'services': ['NTP'], 'type': 'CTRLPLANE', 'policy_desc': 'NTP_ACL', 'stage': 'ingress'}, " "'EVERFLOW': {'stage': 'ingress', 'type': 'MIRROR', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04', 'Ethernet4'], 'policy_desc': 'EVERFLOW'}, " "'ROUTER_PROTECT': {'services': ['SSH', 'SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'ROUTER_PROTECT', 'stage': 'ingress'}, " + "'DATAACLINGRESS': {'stage': 'ingress', 'type': 'L3', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04'], 'policy_desc': 'DATAACLINGRESS'}, " "'SNMP_ACL': {'services': ['SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'SNMP_ACL', 'stage': 'ingress'}, " "'SSH_ACL': {'services': ['SSH'], 'type': 'CTRLPLANE', 'policy_desc': 'SSH_ACL', 'stage': 'ingress'}, " + "'DATAACLEGRESS': {'stage': 'egress', 'type': 'L3', 'ports': ['PortChannel01', 'PortChannel02'], 'policy_desc': 'DATAACLEGRESS'}, " "'EVERFLOWV6': {'stage': 'ingress', 'type': 'MIRRORV6', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04', 'Ethernet4'], 'policy_desc': 'EVERFLOWV6'}}") # everflow portion is not used