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.

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

[Link Event Damping] Port state change handler class #1310

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

Ashish1805
Copy link
Contributor

@Ashish1805 Ashish1805 commented Oct 21, 2023

Class to handle the port state change event callback from SAI and sending the link events to syncd main thread for processing by link event damper.

HLD: sonic-net/SONiC#1071

@Ashish1805 Ashish1805 marked this pull request as draft October 21, 2023 03:35
@kcudnik
Copy link
Collaborator

kcudnik commented Oct 22, 2023

could you give more meaningful title and description ?

@Ashish1805 Ashish1805 force-pushed the ledamping_1 branch 2 times, most recently from 26ffcca to 3d91513 Compare October 23, 2023 22:18
@Ashish1805 Ashish1805 changed the title Ledamping 1 Port state change handler class Oct 23, 2023
@Ashish1805 Ashish1805 marked this pull request as ready for review October 24, 2023 16:20
@Ashish1805 Ashish1805 changed the title Port state change handler class [Link Event Damping] Port state change handler class Oct 24, 2023
@Ashish1805
Copy link
Contributor Author

could you give more meaningful title and description ?

Changed the title and description. Thank you.

@Ashish1805
Copy link
Contributor Author

Adding @Junchao-Mellanox for review.

syncd/PortStateChangeHandler.h Outdated Show resolved Hide resolved
syncd/PortStateChangeHandler.cpp Outdated Show resolved Hide resolved
syncd/PortStateChangeHandler.cpp Outdated Show resolved Hide resolved
@Ashish1805
Copy link
Contributor Author

Hi @kcudnik , I dont have permission to merge PRs. Can you please merge this PR? Thank you.

- Class to handle the port state change event callback from SAI and
  sending the events to syncd main thread for processing by link event
  damper.

Depends on: sonic-net#1297

HLD: sonic-net/SONiC#1071
@kcudnik
Copy link
Collaborator

kcudnik commented Nov 16, 2023

is this ready to merge ?

@Ashish1805
Copy link
Contributor Author

is this ready to merge ?

Yes, it is ready for merge. Thanks.

@kcudnik kcudnik merged commit 92da5b3 into sonic-net:master Nov 16, 2023
14 checks passed
@kcudnik
Copy link
Collaborator

kcudnik commented Nov 18, 2023

@Ashish1805 your change caused compilation issues on swss, please fix this right away or this change will be reverted: https://github.com/sonic-net/sonic-swss/actions/runs/6910957833/job/18804862990?pr=2902

@kcudnik
Copy link
Collaborator

kcudnik commented Nov 18, 2023

change

static constexpr size_t PortStateChangeHandler::PORT_STATE_CHANGE_QUEUE_SIZE;

to

constexpr size_t PortStateChangeHandler::PORT_STATE_CHANGE_QUEUE_SIZE;

first keyword static requires declaration in cpp file since it's considered a field

Ashish1805 pushed a commit to Ashish1805/sonic-sairedis that referenced this pull request Nov 20, 2023
reference.

- In PR sonic-net#1310, a
  constexpt static data member was added. Since sairedis repo uses
  c++14, definition of this data member needs to be provided in .cpp
  file too.
kcudnik pushed a commit that referenced this pull request Nov 21, 2023
…1324)

In PR #1310, a
  constexpt static data member was added. Since sairedis repo uses
  c++14, definition of this data member needs to be provided in .cpp file too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants