Fix issue: bulk counter feature cannot compile on platforms having no sai_bulk_object_get_stats/sai_bulk_object_clear_stats by Junchao-Mellanox · Pull Request #1105 · sonic-net/sonic-sairedis

Junchao-Mellanox · 2022-08-17T05:35:21Z

Why I did this?

bulk counter feature uses two new SAI API sai_bulk_object_get_stats/sai_bulk_object_clear_stats. Some vendors have no such API implemented in their SAI code. Need a way to make compilation pass.

How I fix this?

Add AC_CHECK_FUNCS for sai_bulk_object_get_stats/sai_bulk_object_clear_stats which generates macro HAVE_SAI_BULK_OBJECT_CLEAR_STATS/HAVE_SAI_BULK_OBJECT_GET_STATS in config.h.
Use ifdef to avoid compilation failure.

… sai_bulk_object_get_stats/sai_bulk_object_clear_stats

Junchao-Mellanox · 2022-08-18T04:33:30Z

/azpw run Azure.sonic-sairedis

mssonicbld · 2022-08-18T04:33:31Z

/AzurePipelines run Azure.sonic-sairedis

azure-pipelines · 2022-08-18T04:33:41Z

Azure Pipelines successfully started running 1 pipeline(s).

Junchao-Mellanox · 2022-08-18T04:35:10Z

Hi @kcudnik , can we ignore the coverage checker? I don't think we can cover that.

kcudnik · 2022-08-22T14:29:37Z

no we can't merge without coverage fail, since it will show up on monthly report and i will get into trouble based on that, this test must pass

kcudnik · 2022-08-22T14:31:53Z

why you need to add ifdef for those stats ?

Junchao-Mellanox · 2022-08-23T01:15:28Z

why you need to add ifdef for those stats ?

Some vendors do not have sai_bulk_object_get_stats and sai_bulk_object_clear_stats, it would cause compile issue. So, I have to use ifdef to make those vendor pass the compilation. Could you suggest a way to cover the #ifdef?

kcudnik · 2022-08-23T06:48:02Z

Some vendors do not have sai_bulk_object_get_stats and sai_bulk_object_clear_stats, it would cause compile issue. So, I have to use ifdef to make those vendor pass the compilation. Could you suggest a way to cover the #ifdef?

but why this is vendor specific? you are adding this in virutal switch not for specific vendor are you?
if you want to test specific fidef, then extra tests would need to be added for example in unitests/vslib/ and new executable would need to be added where you specify extra defines to c compiler

Junchao-Mellanox · 2022-08-25T11:19:45Z

/azpw run Azure.sonic-sairedis

mssonicbld · 2022-08-25T11:19:46Z

/AzurePipelines run Azure.sonic-sairedis

azure-pipelines · 2022-08-25T11:19:57Z

Azure Pipelines successfully started running 1 pipeline(s).

Junchao-Mellanox · 2022-08-26T00:52:55Z

/azpw run Azure.sonic-sairedis

mssonicbld · 2022-08-26T00:52:56Z

/AzurePipelines run Azure.sonic-sairedis

azure-pipelines · 2022-08-26T00:53:06Z

Azure Pipelines successfully started running 1 pipeline(s).

Junchao-Mellanox · 2022-08-26T08:04:30Z

/azpw run Azure.sonic-sairedis

mssonicbld · 2022-08-26T08:04:32Z

/AzurePipelines run Azure.sonic-sairedis

azure-pipelines · 2022-08-26T08:04:42Z

Azure Pipelines successfully started running 1 pipeline(s).

Junchao-Mellanox · 2022-08-27T09:47:03Z

/azpw run Azure.sonic-sairedis

mssonicbld · 2022-08-27T09:47:04Z

/AzurePipelines run Azure.sonic-sairedis

azure-pipelines · 2022-08-27T09:47:17Z

Azure Pipelines successfully started running 1 pipeline(s).

Junchao-Mellanox · 2022-09-01T01:03:42Z

/azpw run Azure.sonic-sairedis

mssonicbld · 2022-09-01T01:03:44Z

/AzurePipelines run Azure.sonic-sairedis

azure-pipelines · 2022-09-01T01:03:53Z

Azure Pipelines successfully started running 1 pipeline(s).

liat-grozovik · 2022-09-04T08:09:29Z

@Junchao-Mellanox could you please check the LGTM failure?

Junchao-Mellanox · 2022-09-05T01:54:56Z

@Junchao-Mellanox could you please check the LGTM failure?

Not an issue introduced by this PR, I have sent email to sonic admin add cc'ed you.

kcudnik · 2022-09-05T18:54:03Z

its probably missing library for swss-common on lgtm container build docker needs to be updated

In file included from events.cpp:1:
[2022-08-25 05:47:25] [build-stderr] events_pi.h:10:10: fatal error: uuid/uuid.h: No such file or directory
[2022-08-25 05:47:25] [build-stderr]    10 | #include <uuid/uuid.h>

the missing lib is uuid-dev lib, which was added in swss-common in this PR:

https://github.com/sonic-net/sonic-swss-common/pull/667/files

this lib would need to be added to this repo lgtm.yml

probably "uuid" and "uuid-dev" packages will be needed together

I added PR for that : #1119

kcudnik · 2022-09-06T08:04:59Z

i merged my fix, so if you want to retrigger you would need to submit some dummy code change since re-run from is not working somehow

Junchao-Mellanox · 2022-09-06T08:20:09Z

Thanks, will re-trigger

* 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)

… sai_bulk_object_get_stats/sai_bulk_object_clear_stats (sonic-net#1105) Why I did this? bulk counter feature uses two new SAI API sai_bulk_object_get_stats/sai_bulk_object_clear_stats. Some vendors have no such API implemented in their SAI code. Need a way to make compilation pass. How I fix this? Add AC_CHECK_FUNCS for sai_bulk_object_get_stats/sai_bulk_object_clear_stats which generates macro HAVE_SAI_BULK_OBJECT_CLEAR_STATS/HAVE_SAI_BULK_OBJECT_GET_STATS in config.h. Use ifdef to avoid compilation failure.

Fix issue: bulk counter feature cannot compile on platforms having no…

1a77240

… sai_bulk_object_get_stats/sai_bulk_object_clear_stats

jimmyzhai previously approved these changes Aug 17, 2022

View reviewed changes

jimmyzhai requested a review from kcudnik August 17, 2022 09:53

Update UT coverage

262420d

Junchao-Mellanox dismissed jimmyzhai’s stale review via 262420d August 25, 2022 05:44

kcudnik previously approved these changes Aug 31, 2022

View reviewed changes

Update VendorSai.cpp

fe7de12

Junchao-Mellanox dismissed kcudnik’s stale review via fe7de12 September 6, 2022 08:22

kcudnik approved these changes Sep 6, 2022

View reviewed changes

kcudnik merged commit 8140c22 into sonic-net:master Sep 6, 2022

Junchao-Mellanox deleted the add-bulk-macro branch September 7, 2022 03:30

mlorrillere mentioned this pull request Sep 7, 2022

[submodule] update sonic-sairedis sonic-net/sonic-buildimage#11997

Closed

Conversation

Junchao-Mellanox commented Aug 17, 2022

Uh oh!

Junchao-Mellanox commented Aug 18, 2022

Uh oh!

mssonicbld commented Aug 18, 2022

Uh oh!

azure-pipelines bot commented Aug 18, 2022

Uh oh!

Junchao-Mellanox commented Aug 18, 2022

Uh oh!

kcudnik commented Aug 22, 2022

Uh oh!

kcudnik commented Aug 22, 2022

Uh oh!

Junchao-Mellanox commented Aug 23, 2022

Uh oh!

kcudnik commented Aug 23, 2022

Uh oh!

Junchao-Mellanox commented Aug 25, 2022

Uh oh!

mssonicbld commented Aug 25, 2022

Uh oh!

azure-pipelines bot commented Aug 25, 2022

Uh oh!

Junchao-Mellanox commented Aug 26, 2022

Uh oh!

mssonicbld commented Aug 26, 2022

Uh oh!

azure-pipelines bot commented Aug 26, 2022

Uh oh!

Junchao-Mellanox commented Aug 26, 2022

Uh oh!

mssonicbld commented Aug 26, 2022

Uh oh!

azure-pipelines bot commented Aug 26, 2022

Uh oh!

Junchao-Mellanox commented Aug 27, 2022

Uh oh!

mssonicbld commented Aug 27, 2022

Uh oh!

azure-pipelines bot commented Aug 27, 2022

Uh oh!

Junchao-Mellanox commented Sep 1, 2022

Uh oh!

mssonicbld commented Sep 1, 2022

Uh oh!

azure-pipelines bot commented Sep 1, 2022

Uh oh!

liat-grozovik commented Sep 4, 2022

Uh oh!

Junchao-Mellanox commented Sep 5, 2022

Uh oh!

kcudnik commented Sep 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kcudnik commented Sep 6, 2022

Uh oh!

Junchao-Mellanox commented Sep 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kcudnik commented Sep 5, 2022 •

edited

Loading