Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[minigraph parser] Fix minigraph parser issue when handling LAG related ACL table configuration #1609

Merged
merged 2 commits into from
May 10, 2018
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/sonic-config-engine/minigraph.py
Original file line number Diff line number Diff line change
@@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rephrase this to something like below:

Warning: ACL DATAACL is attached to a LAG "member interface" Ethernet112, instead of LAG interface

Current message appears less understandable.

Warning: ACL DATAACL is attached to a LAG member interface Ethernet112, shall attach to the LAG interface

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rephrased the warning message.

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,
2 changes: 1 addition & 1 deletion src/sonic-config-engine/tests/test_cfggen.py
Original file line number Diff line number Diff line change
@@ -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'}}")