Skip to content

Commit

Permalink
Support both port name and alias in ACL table AttachTo attribute (s…
Browse files Browse the repository at this point in the history
…onic-net#13444)

Why I did it
This PR is an enhancement of PR sonic-net#13105
Because the input string of AttachTo for ACL table can appear in both port name group and port alias group, I added a logic to determine whether the string should be port name or port alias

If all the input strings belong to port name group, then we treat all of them as port name
If all the input strings belong to port alias, then we treat all of them as port alias
If all the input string belongs to both port alias group and port name group, we prefer port alias. The behavior is as before.
How I did it
Walk through all port names/alias in the input to make a decision.

How to verify it
Verified by adding UT.
  • Loading branch information
bingwang-ms authored and mlok-nokia committed Jan 26, 2023
1 parent 03572f6 commit 8cd46bf
Show file tree
Hide file tree
Showing 5 changed files with 1,070 additions and 11 deletions.
48 changes: 41 additions & 7 deletions src/sonic-config-engine/minigraph.py
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,40 @@ def parse_dpg(dpg, hname):
is_mirror = False
is_mirror_v6 = False
is_mirror_dscp = False
use_port_alias = True

# Walk through all interface names/alias to determine whether the input string is
# port name or alias.We need this logic because there can be duplicaitons in port alias
# and port names
# The input port name/alias can be either port_name or port_alias. A mix of name and alias is not accepted
port_name_count = 0
port_alias_count = 0
total_count = 0
for member in aclattach:
member = member.strip()
if member in pcs or \
member in vlans or \
member.lower().startswith('erspan') or \
member.lower().startswith('egress_erspan') or \
member.lower().startswith('erspan_dscp'):
continue
total_count += 1
if member in port_alias_map:
port_alias_count += 1
if member in port_names_map:
port_name_count += 1
# All inputs are port alias
if port_alias_count == total_count:
use_port_alias = True
# All inputs are port name
elif port_name_count == total_count:
use_port_alias = False
# There are both port alias and port name, then port alias is preferred to keep the behavior not changed
else:
use_port_alias = True
# For CTRLPLANE ACL, both counters are 0
if (port_alias_count != 0) and (port_name_count != 0):
print("Warning: The given port name for ACL " + aclname + " is inconsistent. It must be either port name or alias ", file=sys.stderr)

# TODO: Ensure that acl_intfs will only ever contain front-panel interfaces (e.g.,
# maybe we should explicity ignore management and loopback interfaces?) because we
Expand All @@ -674,15 +708,15 @@ def parse_dpg(dpg, hname):
acl_intfs.extend(vlan_member_list[member])
else:
acl_intfs.append(member)
elif (member in port_alias_map) or (member in port_names_map):
if member in port_alias_map:
acl_intf = port_alias_map[member]
else:
acl_intf = member
acl_intfs.append(acl_intf)
elif use_port_alias and (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
if acl_intf in intfs_inpc:
if port_alias_map[member] in intfs_inpc:
print("Warning: ACL " + aclname + " is attached to a LAG member interface " + port_alias_map[member] + ", instead of LAG interface", file=sys.stderr)
elif (not use_port_alias) and (member in port_names_map):
acl_intfs.append(member)
if member in intfs_inpc:
print("Warning: ACL " + aclname + " is attached to a LAG member interface " + member + ", instead of LAG interface", file=sys.stderr)
elif member.lower().startswith('erspan') or member.lower().startswith('egress_erspan') or member.lower().startswith('erspan_dscp'):
if 'dscp' in member.lower():
is_mirror_dscp = True
Expand Down
Loading

0 comments on commit 8cd46bf

Please sign in to comment.