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

[sai-gen] Add counter id generation support for counters. #503

Merged
merged 2 commits into from
Jan 11, 2024

Conversation

r12f
Copy link
Collaborator

@r12f r12f commented Jan 10, 2024

Problem

Currently, DASH only supports generating counters as SAI attributes upon DASH APIs, such as metering buckets. This might be ok for new things that has dedicated DASH concepts, i.e., metering buckets. However, for other more generic counters used for collecting telemetry, this approach could leads to feature missing and not fully utilizing what we already have in SAI and SONiC. For example, no get-and-reset operation support and need to build dedicated pipeline for pulling the counters and feeds them into the telemetry.

What are we doing in this change

This change adds support to generate the counters as SAI generic counters. Instead of generate the counters as readonly attributes, we generate a counter id for it. Then, we can leverage all the infrastructure already built for counters.

As the screenshot shows below, after removing as_attr="true" for metering_bucket_outbound, a counter id will be generated instead of an readonly counter attribute. It also supports handling the packets-and-bytes counter too, because generic counter already collects both information.

image

@chrispsommers
Copy link
Collaborator

Perhaps you explain this is in the WG mtg, you've made a number of enhancements recently and I think it'd be good to review for the community. Thx.

@r12f
Copy link
Collaborator Author

r12f commented Jan 10, 2024

yea! that sounds great to me! i can do a summary on how the latest SAI generator works and all the enhancements I did recently (so far, I still won't call it new, since the basic logic is still the same :D)

not sure if tomorrow is a good time, if not, next next week should be good, if we have time.

Copy link
Collaborator

@chrispsommers chrispsommers left a comment

Choose a reason for hiding this comment

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

lgtm

@r12f
Copy link
Collaborator Author

r12f commented Jan 11, 2024

Woot! Thank you so much, Chris!

@r12f r12f merged commit da0d5a8 into sonic-net:main Jan 11, 2024
4 checks passed
@r12f r12f deleted the user/r12f/counter-id branch January 11, 2024 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants