Skip to content

[hft]: Fix queue and ipg stats missing if hft isn't supported #3967

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

[hft]: Fix queue and ipg stats missing if hft isn't supported #3967

wants to merge 3 commits into from

Conversation

@Pterosaur
Copy link
Contributor

@Pterosaur Pterosaur commented Oct 31, 2025 •
edited
Loading

What I did
Remove redundant HFT capability conditions in the CounterNameMapUpdater

Why I did it
This redundant HFT will skip the QUEUE and IPG stats if the platform doesn't support HFT
Fix the issue: https://github.com/aristanetworks/sonic-qual.msft/issues/861

How I verified it

Check the PG and QUEUE map are in the COUNTER DB.

admin@str4-7060x6-512-2:~$ redis-cli -n 2
127.0.0.1:6379[2]> keys *NAME*
1) "COUNTERS_BUFFER_POOL_NAME_MAP"
2) "COUNTERS_QUEUE_NAME_MAP"
3) "COUNTERS_LAG_NAME_MAP"
4) "COUNTERS_PORT_NAME_MAP"
5) "COUNTERS_PG_NAME_MAP"
6) "COUNTERS_RIF_NAME_MAP"
127.0.0.1:6379[2]>

Details if related

@Pterosaur Pterosaur requested a review from prsunny as a code owner October 31, 2025 01:04
Copilot AI review requested due to automatic review settings October 31, 2025 01:04
@mssonicbld
Copy link
Collaborator

mssonicbld commented Oct 31, 2025

/azp run

@Pterosaur Pterosaur requested a review from r12f October 31, 2025 01:04
@azure-pipelines
Copy link

azure-pipelines bot commented Oct 31, 2025

Azure Pipelines successfully started running 1 pipeline(s).

Copilot AI reviewed Oct 31, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the setCounterNameMap method in CounterNameMapUpdater by removing a redundant outer if (gHFTOrch) check. The change simplifies the code without affecting functionality, since the check is already performed in the single-parameter overload that gets called for each counter map entry.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@saksarav-nokia saksarav-nokia mentioned this pull request Oct 31, 2025
Open
@kperumalbfn
Copy link
Contributor

kperumalbfn commented Oct 31, 2025

@Pterosaur could you have a sonic-mgmt test to check the counters with HFT enabled and disabled?

@Pterosaur Pterosaur changed the title [hft]: Fix queue and ipg stats missing if hft isn't enabled [hft]: Fix queue and ipg stats missing if hft isn't supported Oct 31, 2025
@Pterosaur
Copy link
Contributor Author

Pterosaur commented Oct 31, 2025

@Pterosaur could you have a sonic-mgmt test to check the counters with HFT enabled and disabled?

@kperumalbfn

I think the current tests of sonic-mgmt should have covered the HFT "enabled/disabled" case.
Actually, the title isn't precise. It should be supported/not supported. I just fixed it.
To the platform w/o HFT supported, the stats of QUEUE and IPG will always show zero.

@Pterosaur
Copy link
Contributor Author

Pterosaur commented Nov 1, 2025

sonic-mgmt test: sonic-net/sonic-mgmt#21163

prasoon-nexthop added a commit to prasoon-nexthop/sonic-swss that referenced this pull request Nov 5, 2025
@prasoon-nexthop
[tests] Add unit tests for CounterNameMapUpdater
8e98f90 
Add comprehensive unit tests for CounterNameMapUpdater to verify that
counter name maps are correctly written to COUNTERS_DB regardless of
HFT (High Frequency Telemetry) support.

Tests cover:
- QUEUE counter maps without HFT support
- Priority Group counter maps without HFT support
- Handling of empty OID values
- Single counter name map operations

These tests verify the fix from sonic-net#3967
that removed the redundant outer 'if (gHFTOrch)' check which was preventing
counter registration on platforms without HFT support.

Signed-off-by: Prasoon Saurav <[email protected]>
@prasoon-nexthop prasoon-nexthop mentioned this pull request Nov 5, 2025
Open
@mssonicbld
Copy link
Collaborator

mssonicbld commented Nov 6, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Nov 6, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@Pterosaur
Copy link
Contributor Author

Pterosaur commented Nov 7, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Nov 7, 2025

Commenter does not have sufficient privileges for PR 3967 in repo sonic-net/sonic-swss

@Pterosaur
Copy link
Contributor Author

Pterosaur commented Nov 7, 2025

/azpw run

@mssonicbld
Copy link
Collaborator

mssonicbld commented Nov 7, 2025

/AzurePipelines run

@azure-pipelines
Copy link

azure-pipelines bot commented Nov 7, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@Pterosaur
Copy link
Contributor Author

Pterosaur commented Nov 7, 2025

/azpw run

@mssonicbld
Copy link
Collaborator

mssonicbld commented Nov 7, 2025

/AzurePipelines run

@azure-pipelines
Copy link

azure-pipelines bot commented Nov 7, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

mssonicbld commented Nov 9, 2025

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Nov 9, 2025

Azure Pipelines successfully started running 1 pipeline(s).

@Pterosaur Pterosaur mentioned this pull request Nov 10, 2025
Open
@Pterosaur
Copy link
Contributor Author

Pterosaur commented Nov 10, 2025

Duplicated #3982

@Pterosaur Pterosaur closed this Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@kperumalbfn kperumalbfn kperumalbfn approved these changes

@prsunny prsunny Awaiting requested review from prsunny prsunny is a code owner

@r12f r12f Awaiting requested review from r12f

Assignees

No one assigned

Labels

Approved for msft-202412 Branch Request for msft-202412 Branch

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Pterosaur @mssonicbld @kperumalbfn @StormLiangMS