-
Notifications
You must be signed in to change notification settings - Fork 667
[DPB]: Fix stale queue counter maps in COUNTERS_DB after port breakout #3982
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
Conversation
…kout Fix cleans up COUNTERS_*_NAME_MAP of the port during deinit of the port and regenerate the NAME_MAP's with new OID's during port init if queue flex counters are already enabled. Signed-off-by: Ravi Minnikanti <[email protected]>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| { | ||
| SWSS_LOG_ENTER(); | ||
|
|
||
| if (gHFTOrch) |
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.
@Pterosaur can you please review?
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Pterosaur
left a comment
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.
@kperumalbfn Could you please help to review this PR?
If we merge this PR, I will close my PR: #3967 that is duplicated.
| bool queueFcEnabled = flex_counters_orch->getQueueCountersState() || | ||
| flex_counters_orch->getQueueWatermarkCountersState() || | ||
| flex_counters_orch->getWredQueueCountersState(); | ||
| if (queueFcEnabled && !p.m_queue_ids.empty()) |
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.
@rminnikanti could you please check the DPB sonic-mgmt tests? If this is not covered, could you please add them?
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.
@kperumalbfn I don't see DPB test component in PTF. As far as I understand, DPB testing in sonic-mgmt can be performed on ports that are not part of the topology.
The mock_tests included in this PR verifies the regeneration of the NAME_MAP's. I’ve also validated this behavior on a device and shared the redis-dump output in the PR description for reference.
kperumalbfn
left a comment
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.
@Pterosaur could you close the PR - #3967
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-swss |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@rminnikanti can you create a cherry-pick PR to msft-202412 |
@sdszhang created PR - Azure/sonic-swss.msft#170 |
Signed-off-by: Ravi Minnikanti <[email protected]>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@r12f @ZhaohuiS Fixed. PR checks passed. @r12f I have propagated fix to Azure/sonic-swss.msft#170 |
|
Cherry-pick PR to msft-202412: |
|
Hi @rminnikanti , we are hitting test failure on 202412 due to the PR being incorrectly ported, do you mind to help us look into it? @kperumalbfn for viz. |
@r12f I identified the fix. Its not exactly because of the cherry-picked PR but because of a missing mock_table function in 202412. I will fix it soon. |
|
Thanks a lot Ravi! |
Created Azure/sonic-swss.msft#175 |
sonic-net#3982) What I did Added cleanup of COUNTERS_*_NAME_MAP entries for a port during its deinit phase, and regenerated the NAME_MAP tables with fresh OIDs during port init when queue flex counters are already enabled. Why I did it After dynamic port breakout of a port, queue name map tables in the COUNTERS_DB table are not regenerated leaving stale entries resulting in CLI crash Signed-off-by: Kalash Nainwal <[email protected]>
|
@prsunny Could this fix be taken to 202511 please? Please add the appropriate labels for the same. @Pavan-Nokia viz |
sonic-net#3982) What I did Added cleanup of COUNTERS_*_NAME_MAP entries for a port during its deinit phase, and regenerated the NAME_MAP tables with fresh OIDs during port init when queue flex counters are already enabled. Why I did it After dynamic port breakout of a port, queue name map tables in the COUNTERS_DB table are not regenerated leaving stale entries resulting in CLI crash
|
Cherry-pick PR to 202511: #4109 |
PR:sonic-net/sonic-swss#3982 Also, added missing mock test function from master identified in: Azure#175 Signed-off-by: Dakota Crozier <[email protected]>
|
Hi @prsunny, can the fix be backported to msft-202503 as well? PR: Azure/sonic-swss.msft#196 |
…ost port breakout (#196) PR: sonic-net/sonic-swss#3982 Also, included missing mock test function from master. As identified in msft-202412: #175 Signed-off-by: Dakota Crozier <[email protected]>
Fixes #3983
What I did
Added cleanup of COUNTERS_*_NAME_MAP entries for a port during its deinit phase, and regenerated the NAME_MAP tables with fresh OIDs during port init when queue flex counters are already enabled.
Why I did it
After dynamic port breakout of a port, queue name map tables in the COUNTERS_DB table are not regenerated leaving stale entries resulting in CLI crash
How I verified it
Details if related