sonic-sairedis : Wred stats feature changes on Sai-redis and Syncd

[rpmarvell]

• Stats capability query API support is added

Details :

• Changes are done to support the stats capability query from Swss to SAI.
• This was implemented as part of WRED and ECN statistics
• Statistics capability query is implemented in sairedis/syncd

HLD : https://github.com/sonic-net/SONiC/blob/ebcd2a4a987f1d6027cd57677dc6806b8a9adcdb/doc/qos/ECN_and_WRED_statistics_HLD.md#show-interface-counters-cli-output-on-a-wred-drop-statistics-supported-platform

The Build is failing in this review because of the swss-common-dependency at PR : https://github.com/sonic-net/sonic-swss-common/pull/777

Expected order of pull-request commits :

1. sonic-swss common pull request : https://github.com/sonic-net/sonic-swss-common/pull/777
2. sonic-sairedis pull request : https://github.com/sonic-net/sonic-sairedis/pull/1234
3. sonic-swss : pull request : https://github.com/sonic-net/sonic-swss/pull/2750
4. sonic-yang-model pull requests : https://github.com/sonic-net/sonic-buildimage/pull/14758
5. sonic-utilities pull request : https://github.com/sonic-net/sonic-utilities/pull/2807

[kcudnik]

please fix errors

[rpmarvell]

This PR is dependent on swss-common PR 777 as i have mentioned in the description. Hence the errors. Could you please review and approve the swss-common PR if possible..! Thank you

[kcudnik]

please fix build errors

[rpmarvell]

please fix build errors

sonic-sairedis is dependent on sonic-swss-common PR https://github.com/sonic-net/sonic-swss-common/pull/777 . This sonic-swss-common PR has already been approved by maintainer but we are facing issues in merging this PR as the CI is unstable. There is an issue raised for this in the SONiC community by Myron Sosyak, Link for this issue : https://github.com/sonic-net/sonic-swss-common/issues/784

[stephenxs]

Hi @rpmarvell Please refer to my latest comment. I'm not able to unresolve a comment Thanks

[akokhan]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[akokhan]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[akokhan]

@rpmarvell , even swss-common changes are merged, sairedis CI is still failing. Please check.

[rpmarvell]

Hi @rpmarvell Please refer to my latest comment. I'm not able to unresolve a comment Thanks

I have addressed your review comment. Thank you!

[kcudnik]

please add some unittests to meet code coverage threshold

[rpmarvell]

please add some unittests to meet code coverage threshold ?

LGTM. Please add more unit test cases to cover the code change

I am not finding any infra to add unit tests for changes syncd/Syncd.cpp and lib/RedisRemoteSaiInterface.cpp As these functionalities lies in private functions of class, not even able to call them from UT. Any suggestions are appreciated

[zhangyanzhao]

@stephenxs @msosyak can you please help to review and approve this PR? Thanks.

[zhangyanzhao]

@rpmarvell can you please help to address the build failure? Thanks.

[stephenxs]

Hi @rpmarvell I'm OK with the code. Could you please add more test cases to satisfy the coverage?

Total: 533 lines
Missing: 386 lines
Coverage: 27%
Threshold: 80%
[Diff coverage](https://dev.azure.com/mssonic/be1b070f-be15-4154-aade-b1d3bfb17054/_build/results?buildId=297771&view=codecoverage-tab)

[stephenxs]

Hi @rpmarvell Could you please update the DD when the UT will be ready? Thanks

[kcudnik]

please resolve conflicts

[rpmarvell]

I have addressed the UT. Could you please help merge this change with required approvals.!

[kcudnik]

please fix build errors

In file included from orchdaemon.h:51,
                 from main.cpp:25:
dash/dashrouteorch.h:27:11: error: 'route' in namespace 'dash' does not name a type
   27 |     dash::route::Route metadata;
      |           ^~~~~
dash/dashrouteorch.h:46:11: error: 'route' in namespace 'dash' does not name a type
   46 |     dash::route::Route metadata;
      |           ^~~~~
make[4]: *** [Makefile:918: orchagent-main.o] Error 1
make[4]: *** Waiting for unfinished jobs....
In file included from orchdaemon.h:51,
                 from orchdaemon.cpp:5:
dash/dashrouteorch.h:27:11: error: 'route' in namespace 'dash' does not name a type
   27 |     dash::route::Route metadata;
      |           ^~~~~
dash/dashrouteorch.h:46:11: error: 'route' in namespace 'dash' does not name a type
   46 |     dash::route::Route metadata;
      |           ^~~~~

error is in swss, mybe not related to this change

[rpmarvell]

please fix build errors

In file included from orchdaemon.h:51,
                 from main.cpp:25:
dash/dashrouteorch.h:27:11: error: 'route' in namespace 'dash' does not name a type
   27 |     dash::route::Route metadata;
      |           ^~~~~
dash/dashrouteorch.h:46:11: error: 'route' in namespace 'dash' does not name a type
   46 |     dash::route::Route metadata;
      |           ^~~~~
make[4]: *** [Makefile:918: orchagent-main.o] Error 1
make[4]: *** Waiting for unfinished jobs....
In file included from orchdaemon.h:51,
                 from orchdaemon.cpp:5:
dash/dashrouteorch.h:27:11: error: 'route' in namespace 'dash' does not name a type
   27 |     dash::route::Route metadata;
      |           ^~~~~
dash/dashrouteorch.h:46:11: error: 'route' in namespace 'dash' does not name a type
   46 |     dash::route::Route metadata;
      |           ^~~~~

error is in swss, mybe not related to this change

This is an existing break in sonic-swss, not introduced by the changes in this PR. Even the recent PR https://github.com/sonic-net/sonic-sairedis/pull/1329 is having this build break..!

[rpmarvell]

@kcudnik , @stephenxs : Could you please approve and merge the changes as all tests are passing. Thank you..!

[stephenxs]

@rpmarvell could you please address Kamil's comments? thanks.

[zhangyanzhao]

@rpmarvell can you please address Kamil's comments? Those need be addressed before merging.

[stephenxs]

@rpmarvell kindly reminder to address Kamil's comments. Thanks

[rpmarvell]

@rpmarvell kindly reminder to address Kamil's comments. Thanks

I am working on addressing the comments. Will be updating this thread once the changes are ready. Thank you.

[rpmarvell]

@kcudnik : the vs tests are failing due to the following permission issue, could you help resolve this issue.

sh: 1: ./scripts/pahole-version.sh: Permission denied init/Kconfig:97: syntax error init/Kconfig:96: invalid statement make[1]: *** [scripts/kconfig/Makefile:77: allmodconfig] Error 1 make: *** [Makefile:630: allmodconfig] Error 2

[kcudnik]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[kperumalbfn]

@rpmarvell Could you rebase to master and fix the build issues.