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 #1712

Merged
merged 4 commits into from
May 26, 2018

Conversation

keboliu
Copy link
Collaborator

@keboliu keboliu commented May 15, 2018

This is a replication of PR #1609 , which was reverted and need to be merged after PR [AclOrch] aclOrch enhancement to handle LAG port not configured case . The modifcation in this PR already been reviewed and approved.

- What I did
Fix minigraph parser issue when handling LAG related case for ACL table:

  1. Previously when attach ACL to a LAG port, it will break the LAG into member ports and then add all these member ports to the ACL table, correct way shall add LAG interface direcly to ACL table.
  2. When handling 'erspan' table, it just add all the front panel interfaces to the ACL table, but in some topo like T1-LAG, some front panel interfaces already added to LAG port, attach ACL to these front panel ports will fail.
  3. Give a warning when detected trying to attach ACL to LAG member interface
  4. Update the "test_minigraph_acl" test expectation in test_cfggen.py

- How I did it

  1. Stop breaking LAG port into member ports when in the case to attach ACL to LAG port.
  2. When handling 'erspan' table, if front panel port already been added to LAG, add the LAG instead of the front panel port.
  3. Raise warning against configuration that attach ACL to LAG member interface.

- 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)

…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
@keboliu
Copy link
Collaborator Author

keboliu commented May 26, 2018

PR updated to pick up the change in #sonic-net/sonic-swss#494

@qiluo-msft qiluo-msft merged commit a917517 into sonic-net:master May 26, 2018
qiluo-msft pushed a commit to qiluo-msft/sonic-buildimage that referenced this pull request May 27, 2018
…ed ACL table configuration (sonic-net#1712)

* Fix minigraph parser issue when handling LAG related ACL table configuration
* rephrase the warning message.
* pick up swss change in sonic-net/sonic-swss#494
lguohan pushed a commit that referenced this pull request May 30, 2018
…ed ACL table configuration (#1712)

* Fix minigraph parser issue when handling LAG related ACL table configuration
* rephrase the warning message.
* pick up swss change in sonic-net/sonic-swss#494
qiluo-msft added a commit to qiluo-msft/sonic-buildimage that referenced this pull request Jun 4, 2018
…AG related ACL table configuration (sonic-net#1712)"

This reverts commit d7ed638.
lguohan pushed a commit that referenced this pull request Jun 5, 2018
…AG related ACL table configuration (#1712)" (#1764)

This reverts commit d7ed638.
stephenxs added a commit to stephenxs/sonic-buildimage that referenced this pull request Jun 29, 2021
bb383be2 [Dynamic Buffer Calc][Mellanox] Bug fixes and enhancements for the lua plugins for buffer pool calculation and headroom checking (sonic-net#1781)
f949dfe9 [Dynamic Buffer Calc] Avoid creating lossy PG for admin down ports during initialization (sonic-net#1776)
def0a914 Fix config prompt question issue (sonic-net#1799)
21f97506 [ci]: Merge azure pipelines from master to 202012 branch (sonic-net#1764)
a83a2a42 [vstest]: add dvs_route fixture
849bdf9c [Mux] Add support for mux metrics to State DB (sonic-net#1757)
386de717 [qosorch] Dot1p map list initialization fix (sonic-net#1746)
f99abdca [sub intf] Port object reference count update (sonic-net#1712)
4a00042d [vstest/nhg]: use dvs_route fixture to make test_nhg more robust

Signed-off-by: Stephen Sun <stephens@nvidia.com>
arlakshm added a commit that referenced this pull request Jul 21, 2021
This update includes the following commits

acb5d84 Neetha John     2021-07-20      [configlet] Python3 compatible syntax for extracting a key from the dict (#1721)
9b7c58b arlakshm        2021-07-20      Load the  database global_db only once for show cli  (#1712)

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
judyjoseph added a commit that referenced this pull request Aug 7, 2021
8b149a3 Load the  database global_db only once for show cli  (#1712)
cd0e560 [config][interface][speed] Fixed the config interface speed in multiasic issue (#1739)
b595ba6 [fast-reboot] revert the change of disabling counter polling before fast-reboot (#1744)
8518820 [minigraph] Donot enable PFC watchdog for MgmtTsToR (#1734)
2213774 [CLI][show][bgp] Fix the show ip bgp network command (#1733)
3526507 [configlet] Python3 compatible syntax for extracting a key from the dict (#1721)
5b56b97 [sonic_installer] don't print errors when installing an image not supporting app ext (#1719)
a581955 [LLDP] Fix lldpshow script to enable display multiple MAC addresses on the same remote physical interface (#1657)
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
This update includes the following commits

acb5d84 Neetha John     2021-07-20      [configlet] Python3 compatible syntax for extracting a key from the dict (sonic-net#1721)
9b7c58b arlakshm        2021-07-20      Load the  database global_db only once for show cli  (sonic-net#1712)

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
theasianpianist pushed a commit to theasianpianist/sonic-buildimage that referenced this pull request Feb 5, 2022
Signed-off-by: Wenda Ni <wonda.ni@gmail.com>

There are two parts of changes in this pr:
The motivation is that for each Port object instantiated, it should be initialized in PortsOrch::m_port_ref_count. For each Port object to be removed, its m_port_ref_count check should be zero to proceed further. These operations are generic and therefore should also apply to sub port Port object.

Currently, Port object reference count increments
- When a router interface is created on top.
- When an ACL table is bound to Port (i.e., becoming a member of the ACL table group associated with the Port object).

Port object reference count decrements
- When a router interface on top is removed.
- When an ACL table is unbound from Port (i.e, removing membership from the ACL table group associated with the Port object).
Item 1, router interface creation and removal, is of direct relevance to sub port Port object.

The other part of change is the reference count update to parent port Port object when a sub port Port object is added/removed on top. This part is motivated by change in Add reference counting to ports for ACL bindings.

What I did
At sub port Port object instantiation
  - Init sub port Port object reference count
  - Increase parent port Port object reference count
At sub port Port object removal
  - Check sub port Port object reference count drops to zero
  - Decrease parent port Port object reference count
At physical port removal
  - Check physical port Port object reference count drops to zero

How I verified it
vs test extension
Issue parent port removal at APPL_DB level before removing sub port interface. Verify that parent port persists in ASIC_DB until sub interface is removed.
Without the change, extended vs test fails:
taras-keryk pushed a commit to taras-keryk/sonic-buildimage that referenced this pull request Apr 28, 2022
Introduce a new function `load_db_config()`. This function will load the global database file or database config file based on the platform.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants