From ccd35dafcf5b09088ccbae775210f7c8a3a997f7 Mon Sep 17 00:00:00 2001 From: keboliu Date: Tue, 17 Apr 2018 08:15:51 +0300 Subject: [PATCH 1/3] Fix minigraph parser issue when handling LAG related ACL table configuration Changes to be committed: modified: src/sonic-config-engine/minigraph.py modified: src/sonic-config-engine/tests/test_cfggen.py signed-off-by kebol@mellanox.com --- src/sonic-config-engine/minigraph.py | 20 +++++++++++++++++--- src/sonic-config-engine/tests/test_cfggen.py | 2 +- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 4ac7e9d68c92..1aa0d718eb81 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -149,12 +149,15 @@ def parse_dpg(dpg, hname): pcintfs = child.find(str(QName(ns, "PortChannelInterfaces"))) pc_intfs = [] pcs = {} + intfs_inpc = [] # List to hold all the LAG member interfaces for pcintf in pcintfs.findall(str(QName(ns, "PortChannel"))): pcintfname = pcintf.find(str(QName(ns, "Name"))).text pcintfmbr = pcintf.find(str(QName(ns, "AttachTo"))).text pcmbr_list = pcintfmbr.split(';') + pc_intfs.append(pcintfname) for i, member in enumerate(pcmbr_list): pcmbr_list[i] = port_alias_map.get(member, member) + intfs_inpc.append(pcmbr_list[i]) if pcintf.find(str(QName(ns, "Fallback"))) != None: pcs[pcintfname] = {'members': pcmbr_list, 'fallback': pcintf.find(str(QName(ns, "Fallback"))).text} else: @@ -202,15 +205,26 @@ def parse_dpg(dpg, hname): for member in aclattach: member = member.strip() if pcs.has_key(member): - acl_intfs.extend(pcs[member]['members']) # For ACL attaching to port channels, we break them into port channel members + # If try to attach ACL to a LAG interface then we shall add the LAG to + # to acl_intfs directly instead of break it into member ports, ACL attach + # 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" 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 + if port_alias_map[member] in intfs_inpc: + print >> sys.stderr, "Warning: ACL " + aclname + " is attached to a LAG member interface " + port_alias_map[member] + ", shall attach to the LAG interface" elif member.lower() == 'erspan': is_mirror = True; - # Erspan session will be attached to all front panel ports - acl_intfs = port_alias_map.values() + # Erspan session will be attached to all front panel ports, + # if panel ports is a member port of LAG, should add the LAG + # to acl table instead of the panel ports + acl_intfs = pc_intfs + for panel_port in port_alias_map.values(): + if panel_port not in intfs_inpc: + acl_intfs.append(panel_port) break; if acl_intfs: acls[aclname] = {'policy_desc': aclname, diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index 901b882f7048..dacd3cecaf85 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -81,7 +81,7 @@ def test_minigraph_acl(self): self.assertEqual(output.strip(), "Warning: Ignoring Control Plane ACL NTP_ACL without type\n" "{'SSH_ACL': {'services': ['SSH'], 'type': 'CTRLPLANE', 'policy_desc': 'SSH_ACL'}," " 'SNMP_ACL': {'services': ['SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'SNMP_ACL'}," - " 'DATAACL': {'type': 'L3', 'policy_desc': 'DATAACL', 'ports': ['Ethernet112', 'Ethernet116', 'Ethernet120', 'Ethernet124']}," + " 'DATAACL': {'type': 'L3', 'policy_desc': 'DATAACL', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04']}," " 'NTP_ACL': {'services': ['NTP'], 'type': 'CTRLPLANE', 'policy_desc': 'NTP_ACL'}," " 'ROUTER_PROTECT': {'services': ['SSH', 'SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'ROUTER_PROTECT'}}") From fe9f8d2fb666807594eaeb74204001f03959866a Mon Sep 17 00:00:00 2001 From: keboliu Date: Tue, 24 Apr 2018 05:58:36 +0300 Subject: [PATCH 2/3] rephrase the warning message. --- 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 1aa0d718eb81..7c2662633db9 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -215,7 +215,7 @@ def parse_dpg(dpg, hname): 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 if port_alias_map[member] in intfs_inpc: - print >> sys.stderr, "Warning: ACL " + aclname + " is attached to a LAG member interface " + port_alias_map[member] + ", shall attach to the LAG interface" + print >> sys.stderr, "Warning: ACL " + aclname + " is attached to a LAG member interface " + port_alias_map[member] + ", instead of LAG interface" elif member.lower() == 'erspan': is_mirror = True; # Erspan session will be attached to all front panel ports, From f1c31e3ad02faf7c9234ed68733d9c8a640fe40c Mon Sep 17 00:00:00 2001 From: keboliu Date: Sat, 26 May 2018 03:33:00 +0300 Subject: [PATCH 3/3] pick up swss change in https://github.com/Azure/sonic-swss/pull/494 --- src/sonic-swss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sonic-swss b/src/sonic-swss index 7331f922de7a..4df9c289a6c2 160000 --- a/src/sonic-swss +++ b/src/sonic-swss @@ -1 +1 @@ -Subproject commit 7331f922de7af71b0f40d8b2bdd2e1d30fcba6b0 +Subproject commit 4df9c289a6c2d41e4dee09f3055f0ac6d8e98ded