From fd0fc13731858f65feed2ffb4f53ab0d5ebb5383 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Thu, 25 Feb 2021 21:35:50 +0000 Subject: [PATCH 1/4] For egress ACL attaching to vlan, we break them into vlan members --- src/sonic-config-engine/minigraph.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 0635f9171ebd..a387d5401695 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -579,7 +579,7 @@ def parse_dpg(dpg, hname): 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.") + sys.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 @@ -596,7 +596,11 @@ def parse_dpg(dpg, hname): # to LAG will be applied to all the LAG members internally by SAI/SDK acl_intfs.append(member) elif member in vlans: - acl_intfs.append(member) + # For egress ACL attaching to vlan, we break them into vlan members + if stage == "egress": + acl_intfs.extend(vlans[member]['members']) + else: + acl_intfs.append(member) elif member in port_alias_map: 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 From 319b3c697e3ab43f45444edcc239509c63e97003 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Thu, 25 Feb 2021 22:52:04 +0000 Subject: [PATCH 2/4] Add new output field VLAN[vlan].members for minigraph parser ref: https://github.com/Azure/sonic-swss/blob/master/doc/Configuration.md#vlan --- src/sonic-config-engine/minigraph.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index a387d5401695..923d2fe05619 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -522,7 +522,6 @@ def parse_dpg(dpg, hname): dpg_ecmp_content['ipv4'] = ipv4_content dpg_ecmp_content['ipv6'] = ipv6_content vlanintfs = child.find(str(QName(ns, "VlanInterfaces"))) - vlan_intfs = [] vlans = {} vlan_members = {} vlantype_name = "" @@ -551,7 +550,7 @@ def parse_dpg(dpg, hname): else: vlan_members[(sonic_vlan_member_name, vmbr_list[i])] = {'tagging_mode': 'untagged'} - vlan_attributes = {'vlanid': vlanid} + vlan_attributes = {'vlanid': vlanid, 'members': vmbr_list } # If this VLAN requires a DHCP relay agent, it will contain a element # containing a list of DHCP server IPs From 29f6812aa3460bfa66da64b77a64618686a09a43 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Thu, 25 Feb 2021 23:10:11 +0000 Subject: [PATCH 3/4] Fix test golden data --- src/sonic-config-engine/tests/test_cfggen.py | 8 ++++---- src/sonic-config-engine/tests/test_minigraph_case.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index 9fbf351c3a47..b8be21539392 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -207,10 +207,10 @@ def test_minigraph_vlans(self): self.assertEqual( utils.to_dict(output.strip()), utils.to_dict( - "{'Vlan1000': {'alias': 'ab1', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '1000'}, " - "'Vlan2001': {'alias': 'ab3', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '2001'}," - "'Vlan2000': {'alias': 'ab2', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '2000'}," - "'Vlan2020': {'alias': 'kk1', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '2020'}}" + "{'Vlan1000': {'alias': 'ab1', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '1000', 'members': ['Ethernet8']}, " + "'Vlan2001': {'alias': 'ab3', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '2001', 'members': ['Ethernet12']}," + "'Vlan2000': {'alias': 'ab2', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '2000', 'members': ['Ethernet12']}," + "'Vlan2020': {'alias': 'kk1', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '2020', 'members': ['Ethernet12']}}" ) ) diff --git a/src/sonic-config-engine/tests/test_minigraph_case.py b/src/sonic-config-engine/tests/test_minigraph_case.py index 64e0bb09d730..806916639b76 100644 --- a/src/sonic-config-engine/tests/test_minigraph_case.py +++ b/src/sonic-config-engine/tests/test_minigraph_case.py @@ -94,7 +94,7 @@ def test_minigraph_vlans(self): output = self.run_script(argument) self.assertEqual( utils.to_dict(output.strip()), - utils.to_dict("{'Vlan1000': {'alias': 'ab1', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '1000', 'mac': '00:aa:bb:cc:dd:ee' }}") + utils.to_dict("{'Vlan1000': {'alias': 'ab1', 'dhcp_servers': ['192.0.0.1', '192.0.0.2'], 'vlanid': '1000', 'mac': '00:aa:bb:cc:dd:ee', 'members': ['Ethernet8'] }}") ) def test_minigraph_vlan_members(self): From 5ffc9ca292a88376b45cfe27206d6650a5d88293 Mon Sep 17 00:00:00 2001 From: Qi Luo Date: Thu, 25 Feb 2021 23:42:59 +0000 Subject: [PATCH 4/4] Remove duplication in acl ports list, add test case --- src/sonic-config-engine/minigraph.py | 8 +++++++- .../tests/t0-sample-graph.xml | 16 +++++++++++++++- src/sonic-config-engine/tests/test_cfggen.py | 2 +- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 923d2fe05619..70f7f839f91b 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -623,9 +623,15 @@ def parse_dpg(dpg, hname): acl_intfs.append(panel_port) break if acl_intfs: + # Remove duplications + dedup_intfs = [] + for intf in acl_intfs: + if intf not in dedup_intfs: + dedup_intfs.append(intf) + acls[aclname] = {'policy_desc': aclname, 'stage': stage, - 'ports': acl_intfs} + 'ports': dedup_intfs} if is_mirror: acls[aclname]['type'] = 'MIRROR' elif is_mirror_v6: diff --git a/src/sonic-config-engine/tests/t0-sample-graph.xml b/src/sonic-config-engine/tests/t0-sample-graph.xml index 63f892fe6d04..d3d0a7f93d53 100644 --- a/src/sonic-config-engine/tests/t0-sample-graph.xml +++ b/src/sonic-config-engine/tests/t0-sample-graph.xml @@ -260,6 +260,20 @@ + + Vlan98 + fortyGigE0/100;PortChannel01;PortChannel03 + False + 0.0.0.0/0 + + UserDefinedL2Vlan + 192.0.0.1;192.0.0.2 + 98 + 98 + + + + @@ -331,7 +345,7 @@ DataPlane - PortChannel01;PortChannel02 + PortChannel01;PortChannel02;Vlan98 DataAclEgress DataPlane diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index b8be21539392..89984a6fedd1 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -177,7 +177,7 @@ def test_minigraph_acl(self): "'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'}, " + "'DATAACLEGRESS': {'stage': 'egress', 'type': 'L3', 'ports': ['PortChannel01', 'PortChannel02', 'Ethernet100', 'PortChannel03'], 'policy_desc': 'DATAACLEGRESS'}, " "'EVERFLOWV6': {'stage': 'ingress', 'type': 'MIRRORV6', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04', 'Ethernet4', 'Ethernet100'], 'policy_desc': 'EVERFLOWV6'}}" ) )