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

Get Bulk Stats API #1352

Merged
merged 9 commits into from
Jan 15, 2022
Merged

Get Bulk Stats API #1352

merged 9 commits into from
Jan 15, 2022

Conversation

JaiOCP
Copy link
Contributor

@JaiOCP JaiOCP commented Dec 6, 2021

This API introduces ability to query bulk stats for an object list.
Return counter buffer is populated in the same order as the counter id for each object in a list.
Since same counter ids are applicable to all objects in the list, objects in the object list need to be of the same type. This is consistent with other bulk APIs.

This API introduces ability to query bulk stats for an object list.
Return counter buffer is populated in the same order as the counter id for each object in a list.
Since same counter ids are applicable to all objects in the list, objects need to be of the same type.

Signed-off-by: Jai Kumar <jai.kumar@broadcom.com>
Signed-off-by: Jai Kumar <jai.kumar@broadcom.com>
inc/saitypes.h Outdated Show resolved Hide resolved
Signed-off-by: Jai Kumar <jai.kumar@broadcom.com>
Signed-off-by: Jai Kumar <jai.kumar@broadcom.com>
inc/saiobject.h Outdated Show resolved Hide resolved
inc/saiobject.h Show resolved Hide resolved
inc/saiobject.h Show resolved Hide resolved
@rck-innovium
Copy link
Contributor

We should extend sai_query_stats_capability() to return if bulk get stats is supported for a given object type.

+++ b/inc/saitypes.h
@@ -1428,6 +1428,10 @@ typedef enum _sai_stats_mode_t
      * @brief Read and clear after reading
      */
     SAI_STATS_MODE_READ_AND_CLEAR = 1 << 1,
+
+    SAI_STATS_MODE_BULK_READ = 1 << 2,
+
+    SAI_STATS_MODE_BULK_READ_AND_CLEAR = 1 << 3,
 } sai_stats_mode_t;

@gechiang
Copy link
Contributor

gechiang commented Dec 9, 2021

2 asks:

  1. Please add the Clear Bulk API
  2. Please Add Capability queries for both APIs so that there in case it is not supported by SAI vendor, the OS can choose to use the original non-bulk APIs to perform these operations.

Signed-off-by: Jai Kumar <jai.kumar@broadcom.com>
@JaiOCP
Copy link
Contributor Author

JaiOCP commented Dec 10, 2021

We should extend sai_query_stats_capability() to return if bulk get stats is supported for a given object type.

+++ b/inc/saitypes.h
@@ -1428,6 +1428,10 @@ typedef enum _sai_stats_mode_t
      * @brief Read and clear after reading
      */
     SAI_STATS_MODE_READ_AND_CLEAR = 1 << 1,
+
+    SAI_STATS_MODE_BULK_READ = 1 << 2,
+
+    SAI_STATS_MODE_BULK_READ_AND_CLEAR = 1 << 3,
 } sai_stats_mode_t;

Added the capability flags
9a07186

@JaiOCP
Copy link
Contributor Author

JaiOCP commented Dec 10, 2021

2 asks:

  1. Please add the Clear Bulk API
  2. Please Add Capability queries for both APIs so that there in case it is not supported by SAI vendor, the OS can choose to use the original non-bulk APIs to perform these operations.

Taken care of both review comments

@JaiOCP
Copy link
Contributor Author

JaiOCP commented Dec 10, 2021

@gechiang @rck-innovium @marian-pritsak @mikeberesford
Review comments are taken care of.
Please take a look again and approve.
9a07186

inc/saiobject.h Outdated Show resolved Hide resolved
Signed-off-by: Jai Kumar <jai.kumar@broadcom.com>
inc/saiobject.h Show resolved Hide resolved
Signed-off-by: Jai Kumar <jai.kumar@broadcom.com>
Signed-off-by: Jai Kumar <jai.kumar@broadcom.com>
@rck-innovium
Copy link
Contributor

+1

@JaiOCP JaiOCP requested a review from kcudnik December 16, 2021 17:07
@JaiOCP
Copy link
Contributor Author

JaiOCP commented Dec 16, 2021

@kcudnik Please re-review
@mikeberesford @gechiang Please approve

inc/saitypes.h Outdated Show resolved Hide resolved
inc/saitypes.h Show resolved Hide resolved
Signed-off-by: Jai Kumar <jai.kumar@broadcom.com>
inc/saitypes.h Show resolved Hide resolved
Copy link
Contributor

@gechiang gechiang left a comment

Choose a reason for hiding this comment

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

LGTM.

@JaiOCP
Copy link
Contributor Author

JaiOCP commented Jan 10, 2022 via email

@JaiOCP
Copy link
Contributor Author

JaiOCP commented Jan 13, 2022

@kcudnik Can you please approve

@rlhui rlhui merged commit 773a12c into opencomputeproject:master Jan 15, 2022
qiluo-msft pushed a commit to sonic-net/sonic-sairedis that referenced this pull request Aug 8, 2022
Note: the build may fail due to SAI header dependency. Vendor SAI implementation shall include this PR: opencomputeproject/SAI#1352

HLD: https://github.com/sonic-net/SONiC/blob/master/doc/bulk_counter/bulk_counter.md

**Why I did this?**

PR https://github.com/opencomputeproject/SAI/pull/1352/files introduced new SAI APIs that supports bulk stats:

sai_bulk_object_get_stats
sai_bulk_object_clear_stats
SONiC flex counter infrastructure shall utilize bulk stats API to gain better performance. This document discusses how to integrate these two new APIs to SONiC.

**What I did?**

1. Support using bulk stats APIs based on object type. E.g. for a counter group that queries queue and pg stats, queue stats support bulk while pg stats does not, in that case queue stats shall use bulk API, pg stats shall use non bulk API
2. Automatically fall back to old way if bulk stats APIs are not supported

**How I test this**

Almost full unit test coverage
Manual test
pettershao-ragilenetworks pushed a commit to pettershao-ragilenetworks/sonic-sairedis that referenced this pull request Nov 18, 2022
Note: the build may fail due to SAI header dependency. Vendor SAI implementation shall include this PR: opencomputeproject/SAI#1352

HLD: https://github.com/sonic-net/SONiC/blob/master/doc/bulk_counter/bulk_counter.md

**Why I did this?**

PR https://github.com/opencomputeproject/SAI/pull/1352/files introduced new SAI APIs that supports bulk stats:

sai_bulk_object_get_stats
sai_bulk_object_clear_stats
SONiC flex counter infrastructure shall utilize bulk stats API to gain better performance. This document discusses how to integrate these two new APIs to SONiC.

**What I did?**

1. Support using bulk stats APIs based on object type. E.g. for a counter group that queries queue and pg stats, queue stats support bulk while pg stats does not, in that case queue stats shall use bulk API, pg stats shall use non bulk API
2. Automatically fall back to old way if bulk stats APIs are not supported

**How I test this**

Almost full unit test coverage
Manual test
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.

7 participants