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.

Sign up for GitHub

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

Jump to bottom

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

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

wants to merge 1 commit into from
+1,496 −71

Conversation

rpmarvell
Copy link

@rpmarvell rpmarvell commented Apr 24, 2023 •
edited
Loading

  • 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/master/doc/qos/ECN_and_WRED_statistics_HLD.md

Expected order of pull-request commits :

  1. sonic-swss common pull request : swss-common: WRED and ECN statistics specific changes sonic-swss-common#777
  2. sonic-swss : pull request : sonic-swss: Code changes for WRED and ECN statistics sonic-swss#2750
  3. sonic-yang-model pull requests : sonic-yang-models: WRED statistics yang sonic-buildimage#14758
  4. sonic-utilities pull request : sonic-utilities: WRED stats feature changes on sonic-utilities sonic-utilities#2807
  5. sonic-sairedis pull request : sonic-sairedis : Wred stats feature changes on Sai-redis and Syncd #1234

This was referenced Apr 25, 2023
Merged
Open
@rpmarvell rpmarvell force-pushed the rpmarvell_wredstats_sairedis branch from d8bbe32 to 5da8294 Compare April 25, 2023 16:40
@rpmarvell rpmarvell mentioned this pull request Apr 26, 2023
Merged
10 tasks
@rpmarvell rpmarvell force-pushed the rpmarvell_wredstats_sairedis branch from 5da8294 to 838bc2e Compare May 1, 2023 08:06
@kcudnik
Copy link
Collaborator

kcudnik commented May 4, 2023

please fix errors

@rpmarvell
Copy link
Author

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

msosyak
msosyak previously approved these changes May 8, 2023
View reviewed changes
@rpmarvell rpmarvell mentioned this pull request May 9, 2023
Merged
stephenxs
stephenxs reviewed May 25, 2023
View reviewed changes
lib/ClientSai.cpp Outdated Show resolved Hide resolved
lib/ClientSai.cpp Outdated Show resolved Hide resolved
lib/RedisRemoteSaiInterface.cpp Outdated Show resolved Hide resolved
@rpmarvell rpmarvell force-pushed the rpmarvell_wredstats_sairedis branch from 838bc2e to 8d57f14 Compare May 29, 2023 10:15
@kcudnik
Copy link
Collaborator

kcudnik commented May 29, 2023

please fix build errors

@rpmarvell
Copy link
Author

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 : sonic-net/sonic-swss-common#784

@stephenxs
Copy link
Contributor

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

@akokhan
Copy link
Contributor

akokhan commented May 31, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@akokhan
Copy link
Contributor

akokhan commented Jun 2, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@akokhan
Copy link
Contributor

akokhan commented Jun 2, 2023

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

@rpmarvell rpmarvell force-pushed the rpmarvell_wredstats_sairedis branch from 8d57f14 to 6b6d297 Compare June 3, 2023 17:37
@rpmarvell
Copy link
Author

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!

@rpmarvell rpmarvell force-pushed the rpmarvell_wredstats_sairedis branch 8 times, most recently from 68b4d42 to 380553f Compare June 4, 2023 10:31
@rpmarvell rpmarvell force-pushed the rpmarvell_wredstats_sairedis branch from 38c4bbb to 1d30975 Compare December 12, 2024 13:14
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rpmarvell rpmarvell force-pushed the rpmarvell_wredstats_sairedis branch from 1d30975 to e993d48 Compare December 12, 2024 21:49
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rpmarvell rpmarvell force-pushed the rpmarvell_wredstats_sairedis branch from e993d48 to 1b1d193 Compare December 12, 2024 22:10
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rpmarvell rpmarvell force-pushed the rpmarvell_wredstats_sairedis branch from 1b1d193 to 933dcee Compare December 12, 2024 22:32
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rpmarvell rpmarvell force-pushed the rpmarvell_wredstats_sairedis branch from 933dcee to 7d2e295 Compare January 17, 2025 18:16
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rpmarvell rpmarvell force-pushed the rpmarvell_wredstats_sairedis branch from 7d2e295 to c1fa156 Compare January 17, 2025 22:00
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rpmarvell
Copy link
Author

rpmarvell commented Jan 18, 2025 •
edited
Loading

Tests failing with the following error, looks like a test server issue. [This failure is found on other PRs as well]

2025-01-17T22:46:16.8973660Z ==============================================================================
2025-01-17T22:46:17.3329451Z Generating script.
2025-01-17T22:46:17.3338692Z ========================== Starting Command Output ===========================
2025-01-17T22:46:17.3367521Z [command]/usr/bin/bash --noprofile --norc /__w/_temp/0e9a17a9-17e3-4a96-9747-e38da04b1e3a.sh
2025-01-17T22:46:17.5177955Z rsyslogd: pidfile '/run/rsyslogd.pid' and pid 15467 already exist.
2025-01-17T22:46:17.5178488Z If you want to run multiple instances of rsyslog, you need to specify
2025-01-17T22:46:17.5178847Z different pid files for them (-i option).
2025-01-17T22:46:17.5179418Z rsyslogd: run failed with error -3000 (see rsyslog.h or try https://www.rsyslog.com/e/3000 to learn what that number means)
2025-01-17T22:46:17.5206996Z
2025-01-17T22:46:17.5277580Z ##[error]Bash exited with code '1'.
2025-01-17T22:46:17.5399247Z ##[section]Finishing: Update rsyslog.conf

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rpmarvell rpmarvell force-pushed the rpmarvell_wredstats_sairedis branch from ab639c3 to 846f9b9 Compare January 21, 2025 11:12
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rpmarvell
Copy link
Author

@prsunny @kcudnik : Could you please help resolve this error "rsyslogd: run failed with error -3000"

@rpmarvell @pavannaregundi
sonic-sairedis : Wred stats feature changes on Sai-redis and Syncd
39a3dd2 
* Stats capability query API support is added
* Marvell VS support for query stats capability added

Signed-off-by: rpmarvell <rperumal@marvell.com>
@rpmarvell rpmarvell force-pushed the rpmarvell_wredstats_sairedis branch from 846f9b9 to 39a3dd2 Compare January 22, 2025 08:42
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kcudnik
Copy link
Collaborator

kcudnik commented Jan 22, 2025

this is being handled here: #1502

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kcudnik kcudnik kcudnik requested changes

@stephenxs stephenxs stephenxs approved these changes

@msosyak msosyak Awaiting requested review from msosyak

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@rpmarvell @kcudnik @stephenxs @akokhan @zhangyanzhao @kperumalbfn @mssonicbld @msosyak