Skip to content

Commit d7cd056

Browse files
keboliuLiat Grozovik
authored and
Liat Grozovik
committed
[minigraph parser] Fix minigraph parser issue when handling LAG related ACL table configuration (#1609)
* 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 [email protected]
1 parent 1102ace commit d7cd056

File tree

2 files changed

+18
-4
lines changed

2 files changed

+18
-4
lines changed

src/sonic-config-engine/minigraph.py

+17-3
Original file line numberDiff line numberDiff line change
@@ -149,12 +149,15 @@ def parse_dpg(dpg, hname):
149149
pcintfs = child.find(str(QName(ns, "PortChannelInterfaces")))
150150
pc_intfs = []
151151
pcs = {}
152+
intfs_inpc = [] # List to hold all the LAG member interfaces
152153
for pcintf in pcintfs.findall(str(QName(ns, "PortChannel"))):
153154
pcintfname = pcintf.find(str(QName(ns, "Name"))).text
154155
pcintfmbr = pcintf.find(str(QName(ns, "AttachTo"))).text
155156
pcmbr_list = pcintfmbr.split(';')
157+
pc_intfs.append(pcintfname)
156158
for i, member in enumerate(pcmbr_list):
157159
pcmbr_list[i] = port_alias_map.get(member, member)
160+
intfs_inpc.append(pcmbr_list[i])
158161
if pcintf.find(str(QName(ns, "Fallback"))) != None:
159162
pcs[pcintfname] = {'members': pcmbr_list, 'fallback': pcintf.find(str(QName(ns, "Fallback"))).text}
160163
else:
@@ -202,15 +205,26 @@ def parse_dpg(dpg, hname):
202205
for member in aclattach:
203206
member = member.strip()
204207
if pcs.has_key(member):
205-
acl_intfs.extend(pcs[member]['members']) # For ACL attaching to port channels, we break them into port channel members
208+
# If try to attach ACL to a LAG interface then we shall add the LAG to
209+
# to acl_intfs directly instead of break it into member ports, ACL attach
210+
# to LAG will be applied to all the LAG members internally by SAI/SDK
211+
acl_intfs.append(member)
206212
elif vlans.has_key(member):
207213
print >> sys.stderr, "Warning: ACL " + aclname + " is attached to a Vlan interface, which is currently not supported"
208214
elif port_alias_map.has_key(member):
209215
acl_intfs.append(port_alias_map[member])
216+
# Give a warning if trying to attach ACL to a LAG member interface, correct way is to attach ACL to the LAG interface
217+
if port_alias_map[member] in intfs_inpc:
218+
print >> sys.stderr, "Warning: ACL " + aclname + " is attached to a LAG member interface " + port_alias_map[member] + ", instead of LAG interface"
210219
elif member.lower() == 'erspan':
211220
is_mirror = True;
212-
# Erspan session will be attached to all front panel ports
213-
acl_intfs = port_alias_map.values()
221+
# Erspan session will be attached to all front panel ports,
222+
# if panel ports is a member port of LAG, should add the LAG
223+
# to acl table instead of the panel ports
224+
acl_intfs = pc_intfs
225+
for panel_port in port_alias_map.values():
226+
if panel_port not in intfs_inpc:
227+
acl_intfs.append(panel_port)
214228
break;
215229
if acl_intfs:
216230
acls[aclname] = {'policy_desc': aclname,

src/sonic-config-engine/tests/test_cfggen.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ def test_minigraph_acl(self):
8181
self.assertEqual(output.strip(), "Warning: Ignoring Control Plane ACL NTP_ACL without type\n"
8282
"{'SSH_ACL': {'services': ['SSH'], 'type': 'CTRLPLANE', 'policy_desc': 'SSH_ACL'},"
8383
" 'SNMP_ACL': {'services': ['SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'SNMP_ACL'},"
84-
" 'DATAACL': {'type': 'L3', 'policy_desc': 'DATAACL', 'ports': ['Ethernet112', 'Ethernet116', 'Ethernet120', 'Ethernet124']},"
84+
" 'DATAACL': {'type': 'L3', 'policy_desc': 'DATAACL', 'ports': ['PortChannel01', 'PortChannel02', 'PortChannel03', 'PortChannel04']},"
8585
" 'NTP_ACL': {'services': ['NTP'], 'type': 'CTRLPLANE', 'policy_desc': 'NTP_ACL'},"
8686
" 'ROUTER_PROTECT': {'services': ['SSH', 'SNMP'], 'type': 'CTRLPLANE', 'policy_desc': 'ROUTER_PROTECT'}}")
8787

0 commit comments

Comments
 (0)