teamsyncd: fix VoQ system LAG ID regression after teamdctl gate (#27245) - #4768
Conversation
|
/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). |
|
/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:
|
|
/azpw retry |
|
Retrying failed(or canceled) jobs... |
|
Retrying failed(or canceled) stages in build 1173018: ✅Stage Build:
✅Stage BuildAsan:
|
|
This PR has backport request label(s) for branch(es): msft-202601, but is missing required test information. Please make sure you tick the tested branch(es) in the Tested branch section and provide test evidence (e.g., 202601: <test result>) in the Test result section as well in your PR description. ---Powered by SONiC BuildBot
|
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
@saiarcot895 , I have fixed the PR checked failures. The force-push with some unit test changes after rebase dismissed your approval. Can you approve and merge it |
86d8f43 to
3bc3971
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Signed-off-by: saksarav <sakthivadivu.saravanaraj@nokia.com>
Signed-off-by: saksarav <sakthivadivu.saravanaraj@nokia.com>
Signed-off-by: saksarav <sakthivadivu.saravanaraj@nokia.com>
3bc3971 to
e763b53
Compare
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azpw run |
|
Retrying failed(or canceled) jobs... |
|
Retrying failed(or canceled) stages in build 1197170: ✅Stage TestAsan:
✅Stage Test:
|
|
/azpw run |
|
Retrying failed(or canceled) jobs... |
|
Retrying failed(or canceled) stages in build 1197170: ✅Stage TestAsan:
|
|
@rlhui @saiarcot895 , can you merge this PR? |
|
@ysmanman , @kenneth-arista , would you please review this? thanks |
|
The change is not in msft-202601 yet. @saksarav-nokia, please manually create the cherry pick PR for branch msft-202601. ---Powered by SONiC BuildBot
|
PR for msft-202601 |
5 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