teamsyncd: fix VoQ system LAG ID regression after teamdctl gate (#27245)#4768
teamsyncd: fix VoQ system LAG ID regression after teamdctl gate (#27245)#4768saksarav-nokia wants to merge 2 commits into
Conversation
Signed-off-by: saksarav <sakthivadivu.saravanaraj@nokia.com>
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
saiarcot895
left a comment
saiarcot895
left a comment
There was a problem hiding this comment.
Thanks for fixing this.
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Signed-off-by: saksarav <sakthivadivu.saravanaraj@nokia.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azpw run |
|
Retrying failed(or canceled) jobs... |
|
Retrying failed(or canceled) stages in build 1173018: ✅Stage Build:
✅Stage BuildAsan:
|
|
/azpw run |
|
Retrying failed(or canceled) jobs... |
|
Retrying failed(or canceled) stages in build 1173018: ✅Stage Build:
✅Stage BuildAsan:
|
3 participants
What I did
Fixes the issue sonic-net/sonic-buildimage#27245 introduced by #3984
Always call removeLag() on RTM_DELLINK
Publish APP_LAG DEL even when TeamPortSync never ran (teamdctl failed on add). Orchagent can then run lagIdDel() and return system LAG IDs to SYSTEM_LAG_IDS_FREE_LIST.
Leave PR #3984 behavior for STATE_LAG
STATE_LAG is still written only after successful TeamPortSync / teamdctl. If teamd is not ready, there is still no STATE_LAG (intfmgrd and other STATE_DB users stay protected).
Publish APP_LAG for orch before teamd setup
After building attributes (including state=ok), m_lagTable.set() runs before attempting TeamPortSync, so orch sees SET on add and can pair it with DEL on delete during rapid port-channel churn. Success logging stays after teamd comes up.
Harden removeLag()
Use find() before walking members; always m_lagTable.del(); clear STATE_LAG and selectables only when the LAG was tracked.
Preserve #3984 for STATE_DB; fix APP_LAG teardown and publish timing for orch.
Why I did it
On VoQ T2 chassis, repeated add/delete of a port channel can leave system LAG IDs inconsistent in CHASSIS_APP_DB, failing pc/test_po_update.py::test_po_update_with_higher_lagids.
sonic-swss #3984 correctly defers STATE_LAG until teamd responds via teamdctl, but RTM_DELLINK still skipped removeLag() when TeamPortSync was never created. Orchagent then missed APP_LAG DEL while still seeing APP_LAG SET, so lagIdDel() did not run and IDs leaked or the free list did not advance.
This change keeps #3984’s STATE_DB behavior, always publishes APP_LAG DEL on interface delete, hardens removeLag() for untracked LAGs, and publishes APP_LAG SET (with state=ok) before teamd setup so orch can pair add/del under port-channel churn.
How I verified it
teamsync_ut: existing tests + new cases above.
VoQ T2: pytest tests/pc/test_po_update.py::test_po_update_with_higher_lagids - Ran the po_update tests multiple times and ensured the test passed.
Reason for small delay (~0.5sec) required in test_po_update_with_higher_lagids:
The teamsync fix makes SET/DEL correct when the kernel and teamsyncd finish each cycle. It does not make orchagent process APP_LAG as fast as the test pushes config.
increment_lag_id() calls config_portchannel(PortChannel999, "add") and config_portchannel(..., "del") back-to-back, with no wait for:
teammgrd / kernel → netlink → teamsyncd → APP_LAG in Redis
orchagent → lagIdAdd on create and lagIdDel on remove (SAI LAG create/remove, CHASSIS_APP_DB updates)
Orch handles APP_LAG on a single consumer queue. Under hundreds of rapid iterations, config can outrun orch. If delete is applied before create is done, doLagTask can ignore DEL when the LAG is not in m_portList yet, so that loop iteration does not advance the free-list head even when IDs are not leaked.
The test only checks SYSTEM_LAG_IDS_FREE_LIST once at the end and expects ~one successful add→del per iteration. Without a short wait after add (LAG/orch/DB visible) and/or after del (LAG gone / ID returned), the test can fail on timing, while lag_set_sanity() still passes.
Details if related