Skip to content

[sonic-sairedis] Support bulk counter#1094

Merged
qiluo-msft merged 5 commits intosonic-net:masterfrom
Junchao-Mellanox:bulk-counter-review-1
Aug 8, 2022
Merged

[sonic-sairedis] Support bulk counter#1094
qiluo-msft merged 5 commits intosonic-net:masterfrom
Junchao-Mellanox:bulk-counter-review-1

Conversation

@Junchao-Mellanox
Copy link
Copy Markdown
Contributor

@Junchao-Mellanox Junchao-Mellanox commented Jul 28, 2022 •
edited
Loading

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

@Junchao-Mellanox
Copy link
Copy Markdown
Contributor Author

Junchao-Mellanox commented Jul 28, 2022

Hi @kcudnik , could you please review this?

stephenxs
stephenxs previously approved these changes Jul 28, 2022
View reviewed changes
@Junchao-Mellanox
Copy link
Copy Markdown
Contributor Author

Junchao-Mellanox commented Jul 28, 2022

Hi @liat-grozovik , could you please help add kcudnik as reviewer?

@keboliu keboliu requested a review from kcudnik July 28, 2022 08:59
@Junchao-Mellanox
Copy link
Copy Markdown
Contributor Author

Junchao-Mellanox commented Jul 29, 2022

The coverage report does not really reflect the coverage:

Source File Diff Coverage (%) Missing Lines
lib/ClientSai.cpp 0.0% 986,997-999,1001,1003,1006,1016-1018,1020,1022
lib/ClientServerSai.cpp 0.0% 300,311-313,315,318,328-330,332
lib/RedisRemoteSaiInterface.cpp 0.0% 1214,1225,1227,1229,1232,1242,1244,1246
lib/Sai.cpp 0.0% 404,415,417,420,430,432
lib/sai_redis_interfacequery.cpp 0.0% 252,263,265,268,278,280
lib/ServerSai.cpp 0.0% 320,331,333,336,346,348
meta/Meta.cpp 0.0% 1853,1864,1866,1869,1879,1881
syncd/FlexCounter.cpp 95.6% 507,694,702-703
syncd/VendorSai.cpp 0.0% 649,660-662,664,673,676,686-688,690,698
vslib/Sai.cpp 0.0% 522,533-535,537,540,550-552,554
vslib/sai_vs_interfacequery.cpp 0.0% 205,216,218,221,231,233
vslib/VirtualSwitchSaiInterface.cpp 0.0% 964,975,977,980,990,992


My major change is on FlexCounter.cpp whose coverage is 95%. Changes to other files are only to have a default implementation for new SAI APIs to make compiler happy. I am not sure that it is necessary to cover them as it does not contains real logic.

@kcudnik
Copy link
Copy Markdown
Collaborator

kcudnik commented Jul 29, 2022

please address code coverage

kcudnik
kcudnik previously approved these changes Jul 29, 2022
View reviewed changes
@Junchao-Mellanox Junchao-Mellanox dismissed stale reviews from kcudnik and stephenxs via 36267a0 August 1, 2022 02:49
@liushilongbuaa
Copy link
Copy Markdown
Contributor

liushilongbuaa commented Aug 1, 2022

/azpw run

@liushilongbuaa
Copy link
Copy Markdown
Contributor

liushilongbuaa commented Aug 1, 2022

/azp run

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Aug 1, 2022

Azure Pipelines successfully started running 1 pipeline(s).

@kcudnik
Copy link
Copy Markdown
Collaborator

kcudnik commented Aug 1, 2022

please address code coverage

@Junchao-Mellanox
Copy link
Copy Markdown
Contributor Author

Junchao-Mellanox commented Aug 3, 2022

Hi @kcudnik , could you please review and sign-off?

@Junchao-Mellanox
Copy link
Copy Markdown
Contributor Author

Junchao-Mellanox commented Aug 4, 2022

/azpw run Azure.sonic-sairedis

@mssonicbld
Copy link
Copy Markdown
Collaborator

mssonicbld commented Aug 4, 2022

/AzurePipelines run Azure.sonic-sairedis

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Aug 4, 2022

Azure Pipelines successfully started running 1 pipeline(s).

@Junchao-Mellanox
Copy link
Copy Markdown
Contributor Author

Junchao-Mellanox commented Aug 8, 2022

/azpw run Azure.sonic-sairedis

@mssonicbld
Copy link
Copy Markdown
Collaborator

mssonicbld commented Aug 8, 2022

/AzurePipelines run Azure.sonic-sairedis

@azure-pipelines
Copy link
Copy Markdown

azure-pipelines bot commented Aug 8, 2022

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Copy Markdown
Collaborator

liat-grozovik commented Aug 8, 2022

@kcudnik could you please help to merge?

@qiluo-msft qiluo-msft merged commit bfd37e3 into sonic-net:master Aug 8, 2022
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Aug 9, 2022
@dprital
[submodule] Update sonic-sairedis submodule
e94b16d 
Update sonic-sairedis submodule pointer to include the following:
* [sonic-sairedis] Support bulk counter ([sonic-net#1094](sonic-net/sonic-sairedis#1094))
* Transfer organization from Azure to sonic-net ([sonic-net#1095](sonic-net/sonic-sairedis#1095))
* [BFN] Provide unified approach to select P4 profile based on chip family ([sonic-net#1089](sonic-net/sonic-sairedis#1089))
* Add support of mdio IPC server class using sai switch api and unix socket ([sonic-net#1080](sonic-net/sonic-sairedis#1080))
* [FlexCounter] Refactor FlexCounter class  ([sonic-net#1073](sonic-net/sonic-sairedis#1073))

Signed-off-by: dprital <drorp@nvidia.com>
@dprital dprital mentioned this pull request Aug 9, 2022
Closed
7 tasks
@jimmyzhai jimmyzhai mentioned this pull request Aug 11, 2022
Merged
7 tasks
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Aug 29, 2022
@dprital
[submodule] Advance sonic-sairedis pointer
9987d5c 
Update sonic-sairedis submodule pointer to include the following:
* Run 20 vs tests at a time. ([sonic-net#1111](sonic-net/sonic-sairedis#1111))
* [asan] suppress the static variable leaks ([sonic-net#1085](sonic-net/sonic-sairedis#1085))
* [sonic-sairedis] Support bulk counter ([sonic-net#1094](sonic-net/sonic-sairedis#1094))
* Transfer organization from Azure to sonic-net ([sonic-net#1095](sonic-net/sonic-sairedis#1095))
* [BFN] Provide unified approach to select P4 profile based on chip family ([sonic-net#1089](sonic-net/sonic-sairedis#1089))

Signed-off-by: dprital <drorp@nvidia.com>
@dprital dprital mentioned this pull request Aug 29, 2022
Closed
7 tasks
@mlorrillere mlorrillere mentioned this pull request Sep 7, 2022
Closed
mlorrillere pushed a commit to mlorrillere/sonic-buildimage that referenced this pull request Sep 8, 2022
@mlorrillere-arista
[submodule] update sonic-sairedis
d0bf948 
* 660a920 [Chassis] Create fabric ports for switch_type fabric. (sonic-net/sonic-sairedis#1114)
* 8140c22 Fix issue: bulk counter feature cannot compile on platforms having no sai_bulk_object_get_stats/sai_bulk_object_clear_stats (sonic-net/sonic-sairedis#1105)
* 0aa60f5 [lgtm] Add uuid library (sonic-net/sonic-sairedis#1119)
* e8a01a8 Add retry on zmq functions if fail with EINTR. (sonic-net/sonic-sairedis#1109)
* 594b242 Add SAI_PORT_ATTR_OPER_SPEED support (sonic-net/sonic-sairedis#1107)
* 4c9e048 Add Xsight specific syncd start options (sonic-net/sonic-sairedis#1112)
* da26ace Run 20 vs tests at a time. (sonic-net/sonic-sairedis#1111)
* ffc4109 [asan] suppress the static variable leaks (sonic-net/sonic-sairedis#1085)
* bfd37e3 [sonic-sairedis] Support bulk counter (sonic-net/sonic-sairedis#1094)
* 90ba09a Transfer organization from Azure to sonic-net (sonic-net/sonic-sairedis#1095)
* 4853881 [BFN] Provide unified approach to select P4 profile based on chip family (sonic-net/sonic-sairedis#1089)
@Junchao-Mellanox Junchao-Mellanox mentioned this pull request Oct 12, 2022
Merged
pettershao-ragilenetworks pushed a commit to pettershao-ragilenetworks/sonic-sairedis that referenced this pull request Nov 18, 2022
@Junchao-Mellanox
[sonic-sairedis] Support bulk counter (sonic-net#1094)
76ba516 
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

Reviewers

@kcudnik kcudnik kcudnik approved these changes

@stephenxs stephenxs stephenxs left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@Junchao-Mellanox @kcudnik @liushilongbuaa @mssonicbld @liat-grozovik @stephenxs @qiluo-msft