Add libsai RedisInterface for link event damping. by Ashish1805 · Pull Request #1331 · sonic-net/sonic-sairedis

Ashish1805 · 2023-12-06T10:04:03Z

This supports the link event damping config and algorithm set API.
Sends the link event damping config from libsai to syncd main thread.

HLD: sonic-net/SONiC#1071

Ashish1805 · 2023-12-13T03:50:21Z

@Junchao-Mellanox for review.

Ashish1805 · 2023-12-13T18:15:00Z

@kcudnik looks like swss pytest is failing - it seems unrelated to my changes. Any idea how to fix the swss pytest?

kcudnik · 2023-12-13T19:01:43Z

those vstests are sometime flaky so dont worry about that

Ashish1805 · 2023-12-14T18:07:05Z

@kcudnik I have updated the PR as per review comment. can you please review it when you get a chance? Thank you.

kcudnik · 2024-01-16T10:42:54Z

please fix build erros and code coverage

Ashish1805 · 2024-05-13T18:22:44Z

@kcudnik Added the code coverage but I see PR is failing for 2 tests during Update rsyslog.conf test. The test failure log doesn't give much info, can you suggest where can I find the helpful debug log to figure out what exactly failed and why?

kcudnik · 2024-05-13T18:42:47Z

@kcudnik Added the code coverage but I see PR is failing for 2 tests during Update rsyslog.conf test. The test failure log doesn't give much info, can you suggest where can I find the helpful debug log to figure out what exactly failed and why?

rsyslogd: pidfile '/run/rsyslogd.pid' and pid 18837 already exist.
If you want to run multiple instances of rsyslog, you need to specify

in ~/sonic-sairedis/.azure-pipelines/build-template.yml

68   - script: |
169       sudo cp azsyslog.conf /etc/rsyslog.conf
170       sudo service rsyslog restart
171     displayName: "Update rsyslog.conf"

seems like syslog service restart failed but why, no clue, we would need to look into syslog on pipeline but i dont know how :D maybe update buitld template to do "tail -n 100 /var/log/syslog ? after restaret command ?

Ashish1805 · 2024-05-21T19:24:32Z

/AzurePipelines run

azure-pipelines · 2024-05-21T19:24:38Z

Commenter does not have sufficient privileges for PR 1331 in repo sonic-net/sonic-sairedis

kcudnik · 2024-05-22T08:30:32Z

/azp run

azure-pipelines · 2024-05-22T08:30:43Z

Azure Pipelines successfully started running 1 pipeline(s).

- This supports the link event damping config and algorithm set API. - Sends the link event damping config from libsai to syncd main thread. HLD: sonic-net/SONiC#1071 Signed-off-by: Ashish Singh <ashishksingh@google.com>

kcudnik · 2024-05-28T20:17:33Z

+    if ((objectType != SAI_OBJECT_TYPE_PORT) || (attr == nullptr) || (attr->id < SAI_PORT_ATTR_CUSTOM_RANGE_START))
+    {
+        return false;
+    }
+
+    return true;


you can turn this to 1 single line by reversing condition

kcudnik · 2024-05-28T20:19:35Z

+    if (m_syncMode)
+    {
+        swss::KeyOpFieldsValuesTuple kco;
+        auto status = m_communicationChannel->wait(REDIS_ASIC_STATE_COMMAND_DAMPING_CONFIG_SET, kco);
+
+        m_recorder->recordGenericSetResponse(status);
+
+        return status;
+    }


make this api always synchronous, async mode is just for create/remove/set operations all other apis should be synchronous

DendroLabs · 2026-03-17T14:22:42Z

Hi @Ashish1805 and @kcudnik,

This PR has been inactive for nearly 2 years. I'm picking up the link event damping feature and plan to open a new PR that supersedes this one, addressing all outstanding review comments:

Make the damping config API always synchronous (not respect m_syncMode flag), per @kcudnik's feedback
Simplify condition (reverse and single-line)
Rebase onto current master

Will credit @Ashish1805 as original author. See also my comment on #1334 regarding the overall design clarification.

kcudnik · 2026-03-17T16:24:28Z

you have conflicts and didnt address comments last time

DendroLabs · 2026-03-29T13:23:07Z

@kcudnik Thanks for the note. We're picking up this feature from scratch — our superseding PRs address all outstanding review comments and have no conflicts.

We've already opened 3 PRs with all CI green:

sonic-sairedis #1798 — SelectablesTracker (supersedes [Link Event Damping] Add tracker to track the selectable timers used by link event damper #1323)
sonic-utilities #4367 — Config/show/clear CLI
sonic-buildimage #26231 — YANG model

The remaining two sairedis PRs (superseding this one and #1334) are pending your feedback on the design question we posted on #1334. Would appreciate your review when you get a chance.

@Ashish1805

Summary Supersedes #1323 by @Ashish1805. Adds the SelectablesTracker class for tracking selectable timers used by the link event damper in syncd. This is part of the Link Event Damping feature (HLD: sonic-net/SONiC#1071). All review feedback from #1323 has been addressed: Moved class members to private (@kcudnik) Replaced raw pointers with shared_ptr for memory safety (@kcudnik) Fixed code ordering -- null checks before getFd() call (@Junchao-Mellanox) Added empty line before if() blocks (@kcudnik) CodeQL constexpr fix already present on master Changes: syncd/SelectablesTracker.h -- Header with private members, shared_ptr interface syncd/SelectablesTracker.cpp -- Implementation with proper null checking order unittest/syncd/TestSelectablesTracker.cpp -- 11 unit test cases unittest/syncd/MockSelectablesTracker.h -- Mock class in separate file Build integration in both syncd/Makefile.am and unittest/syncd/Makefile.am Dependency chain: This PR is a prerequisite for the per-port link event damper (#1334) and the libsai RedisInterface (#1331). Test plan 11 new unit tests covering add/remove/get operations, null handling, duplicate detection Existing syncd unit tests pass (no regressions) Azure Pipelines CI green 🤖 Generated with Claude Code Co-Authored-By: Ashish Singh ashish.singh@google.com

DendroLabs · 2026-06-08T16:06:11Z

The superseding PR for this one is now open: #1935

It addresses all review feedback from this PR:

setLinkEventDampingConfig is always synchronous (never checks m_syncMode), per @kcudnik's review
isRedisPortAttribute simplified to single boolean expression, per @kcudnik's review
Syncd response is always sent (no m_enableSyncMode guard)
Uses a dedicated response command (link_event_damping_config_response) to avoid collision with GETRESPONSE
Clean rebase on current master, no conflicts

@Ashish1805 credited as original author in the PR description.

kcudnik reviewed Dec 6, 2023

View reviewed changes

Comment thread syncd/Syncd.cpp

Ashish1805 mentioned this pull request Dec 7, 2023

Link event damping HLD sonic-net/SONiC#1071

Merged

Ashish1805 force-pushed the ledamping_sairedis branch from bbd506e to 84655fc Compare December 12, 2023 22:13

Ashish1805 force-pushed the ledamping_sairedis branch from 629934b to f47c05d Compare January 12, 2024 22:22

Ashish1805 force-pushed the ledamping_sairedis branch 2 times, most recently from 2399261 to ec5cf30 Compare May 10, 2024 23:02

Ashish1805 force-pushed the ledamping_sairedis branch 2 times, most recently from 6aa7b5a to 5dfe7bf Compare May 20, 2024 23:59

Add libsai RedisInterface for link event damping.

6f5b983

- This supports the link event damping config and algorithm set API. - Sends the link event damping config from libsai to syncd main thread. HLD: sonic-net/SONiC#1071 Signed-off-by: Ashish Singh <ashishksingh@google.com>

Ashish1805 force-pushed the ledamping_sairedis branch from 5dfe7bf to 6f5b983 Compare May 28, 2024 20:01

kcudnik reviewed May 28, 2024

View reviewed changes

DendroLabs mentioned this pull request Mar 17, 2026

[Link Event Damping] Add per port link event damper class. #1334

Open

DendroLabs mentioned this pull request Mar 17, 2026

[Link Event Damping] Add SelectablesTracker for timer management #1798

Merged

3 tasks

sivat6 mentioned this pull request May 22, 2026

Link event damping feature #1906

Merged

DendroLabs mentioned this pull request Jun 8, 2026

[Link Event Damping] Add RedisInterface plumbing for damping config #1935

Closed

3 tasks

Conversation

Ashish1805 commented Dec 6, 2023

Uh oh!

Uh oh!

Ashish1805 commented Dec 13, 2023

Uh oh!

Ashish1805 commented Dec 13, 2023

Uh oh!

kcudnik commented Dec 13, 2023

Uh oh!

Ashish1805 commented Dec 14, 2023

Uh oh!

kcudnik commented Jan 16, 2024

Uh oh!

Ashish1805 commented May 13, 2024

Uh oh!

kcudnik commented May 13, 2024

Uh oh!

Ashish1805 commented May 21, 2024

Uh oh!

azure-pipelines Bot commented May 21, 2024

Uh oh!

kcudnik commented May 22, 2024

Uh oh!

azure-pipelines Bot commented May 22, 2024

Uh oh!

kcudnik May 28, 2024

Choose a reason for hiding this comment

Uh oh!

kcudnik May 28, 2024

Choose a reason for hiding this comment

Uh oh!

DendroLabs commented Mar 17, 2026

Uh oh!

kcudnik commented Mar 17, 2026

Uh oh!

DendroLabs commented Mar 29, 2026

Uh oh!

DendroLabs commented Jun 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants