-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Conversation
…guration 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
Outdated
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" |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rephrased the warning message.
Parser code changes looks fine to me. However, in some iterations of the test, it was observed ACL table creation failed if
|
@prsunny agree need to handle the case that portchannel interface go up later than ACL Table loaded. |
You may have to make changes in sonic-swss for handling this case. IMO, that will be a separate PR and once ready, the two PRs can be merged along with submodule update. |
The last comment was handled in sonic-net/sonic-swss#494 |
- What I did
Fix minigraph parser issue when handling LAG related case for ACL table:
- How I did it
- How to verify it
sonic-cfggen test during build.
run ACL and Everflow test on different topo.
- Description for the changelog
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
- A picture of a cute animal (not mandatory but encouraged)