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

HLD for ECN and WRED statistics support in SONiC #1273

Conversation

rpmarvell
Copy link
Contributor

@rpmarvell rpmarvell commented Feb 23, 2023

This document describes the high-level design of

  • ECN-marked queue statistics
  • WRED-drop port statistics
  • WRED-drop queue statistics

Code PR details :

Repo PR title State
sonic-swss-common WRED and ECN stats flexcounter macros and lua script updates for stats capability query Merged
sonic-sairedis Statistics capability query is implemented in sairedis and syncd Approved, Yet to be merged
sonic-swss WRED and ECN statistics implementation Approved, yet to be merged
sonic-buildimage Added new flex counter groups in yang file Merged
sonic-utilities CLI support for WRED and ECN statistics feature Approved, yet to be merged

@rpmarvell rpmarvell force-pushed the rpmarvell_ecn_wred_statistics_on_sonic_hld branch from dc71cf1 to 73a7e86 Compare February 28, 2023 14:01
@zhangyanzhao
Copy link
Collaborator

Please update the Command Reference document for your new CLI and update/define YANG model for the updated Config_DB

@zhangyanzhao
Copy link
Collaborator

@liat-grozovik
Copy link
Collaborator

@stephenxs could you please help to review?
IMO sonic design should take into consideration that SAI will not support part of it and thus it should be used by stat capabilities.
also, need to ensure that the out put of the CLIs is aligned with the fact that some capabilities are not available and what should be the cli output for it

doc/qos/ECN_and_WRED_statistics_HLD.md Outdated Show resolved Hide resolved
doc/qos/ECN_and_WRED_statistics_HLD.md Show resolved Hide resolved
doc/qos/ECN_and_WRED_statistics_HLD.md Outdated Show resolved Hide resolved
doc/qos/ECN_and_WRED_statistics_HLD.md Outdated Show resolved Hide resolved
doc/qos/ECN_and_WRED_statistics_HLD.md Outdated Show resolved Hide resolved
doc/qos/ECN_and_WRED_statistics_HLD.md Outdated Show resolved Hide resolved
doc/qos/ECN_and_WRED_statistics_HLD.md Outdated Show resolved Hide resolved
doc/qos/ECN_and_WRED_statistics_HLD.md Outdated Show resolved Hide resolved
doc/qos/ECN_and_WRED_statistics_HLD.md Outdated Show resolved Hide resolved
doc/qos/ECN_and_WRED_statistics_HLD.md Outdated Show resolved Hide resolved
@msosyak
Copy link

msosyak commented May 8, 2023

Please update the description with implementation PR references according to pull-request-process

@rpmarvell rpmarvell requested a review from neethajohn May 18, 2023 19:42
@rpmarvell rpmarvell force-pushed the rpmarvell_ecn_wred_statistics_on_sonic_hld branch from 064a451 to c127b2b Compare May 24, 2023 20:39
- ECN queue statistics
- WRED queue statistics
- WRED port statistics

Signed-off-by: rpmarvell <rperumal@marvell.com>

Review comments addressed
@rpmarvell rpmarvell force-pushed the rpmarvell_ecn_wred_statistics_on_sonic_hld branch from a86d824 to 60fb686 Compare May 29, 2023 10:00
@rpmarvell
Copy link
Contributor Author

@neethajohn : Could you please help merge this HLD

@zhangyanzhao
Copy link
Collaborator

@rpmarvell there are open comments to be addressed in the code PRs, can you please help to check and address? Thanks.

@zhangyanzhao
Copy link
Collaborator

approved by Nvidia and merge

@zhangyanzhao zhangyanzhao merged commit 38c3ec6 into sonic-net:master Feb 5, 2024
1 check passed
@Yuval-Mellanox
Copy link
Contributor

@rpmarvell What is holding merging the code prs?

@rpmarvell
Copy link
Contributor Author

@rpmarvell What is holding merging the code prs?

Sonic-sairedis review comments are being addressed. It will be merged as soon as the testing is done for the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🏗 In Progress
Development

Successfully merging this pull request may close these issues.

None yet

8 participants