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

Add SAI Notification support for host_tx_ready #1307

Merged
merged 8 commits into from
Nov 27, 2023
Merged

Add SAI Notification support for host_tx_ready #1307

merged 8 commits into from
Nov 27, 2023

Conversation

noaOrMlnx
Copy link
Contributor

@noaOrMlnx noaOrMlnx commented Oct 12, 2023 •
edited
Loading

New PORT_HOST_TX_READY notification logic was added to sonic-sairedis and syncd in order to support host_tx_ready synchronization enhancements.

HLD: sonic-net/SONiC#1453

@noaOrMlnx
Copy link
Contributor Author

/azpw run Azure.sonic-sairedis

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-sairedis

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@noaOrMlnx noaOrMlnx mentioned this pull request Oct 23, 2023
Merged
kcudnik
kcudnik reviewed Oct 29, 2023
View reviewed changes
meta/Meta.cpp Outdated Show resolved Hide resolved
@noaOrMlnx noaOrMlnx force-pushed the host_tx_ready_enhancements-sonic-sairedis branch from 650f665 to 886793d Compare October 29, 2023 17:29
@noaOrMlnx noaOrMlnx mentioned this pull request Oct 30, 2023
Merged
@Junchao-Mellanox
Copy link
Contributor

/azpw run Azure.sonic-sairedis

@kcudnik
Copy link
Collaborator

kcudnik commented Oct 31, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@noaOrMlnx noaOrMlnx force-pushed the host_tx_ready_enhancements-sonic-sairedis branch 2 times, most recently from 886793d to 2ca67a8 Compare October 31, 2023 10:31
@kcudnik
Copy link
Collaborator

kcudnik commented Oct 31, 2023

hey, you must update to latest master to fix notifications since SAI was updated, you can take a look here: https://github.com/sonic-net/sonic-sairedis/pull/1311/files

@kcudnik
Copy link
Collaborator

kcudnik commented Nov 6, 2023

please fix whitespaces:

../vslib/Switch.cpp:                m_switchNotifications.on_port_host_tx_ready = 
ERROR: some files contain white spaces at the end of line, please fix
FAIL: checkwhitespace.sh

@noaOrMlnx noaOrMlnx force-pushed the host_tx_ready_enhancements-sonic-sairedis branch 3 times, most recently from f6b1641 to 57dfc09 Compare November 7, 2023 12:42
@noaOrMlnx
Copy link
Contributor Author

Hi @kcudnik

I tried to remove all whitespaces, and there is no whitespace in the line stated in the UT.
Can you please point me to the issue?

@kcudnik
Copy link
Collaborator

kcudnik commented Nov 8, 2023

Please spend time to read compiler ero
Rorschach in AZ pipeline:

Checkig for white spaces ...
../saiplayer/SaiPlayer.cpp:    
../saiplayer/SaiPlayer.h:        
../vslib/Switch.cpp:                m_switchNotifications.on_port_host_tx_ready = 
ERROR: some files contain white spaces at the end of line, please fix
FAIL: checkwhitespace.sh

@noaOrMlnx noaOrMlnx force-pushed the host_tx_ready_enhancements-sonic-sairedis branch 6 times, most recently from bf170a8 to fdbedb1 Compare November 15, 2023 13:15
@noaOrMlnx noaOrMlnx force-pushed the host_tx_ready_enhancements-sonic-sairedis branch from fdbedb1 to 8c104e9 Compare November 15, 2023 17:37
@noaOrMlnx noaOrMlnx force-pushed the host_tx_ready_enhancements-sonic-sairedis branch from 8c104e9 to fc717d2 Compare November 16, 2023 07:32
@noaOrMlnx noaOrMlnx marked this pull request as ready for review November 16, 2023 07:41
@kcudnik kcudnik merged commit 4cbbeed into sonic-net:master Nov 27, 2023
14 checks passed
kcudnik
kcudnik reviewed Nov 27, 2023
View reviewed changes
unittest/lib/TestSwitch.cpp
@@ -36,6 +36,7 @@ TEST(Switch, updateNotifications)
attrs[5].value.ptr = (void*)1;
attrs[6].value.ptr = (void*)1;
attrs[7].value.ptr = (void*)1;
attrs[8].value.ptr = (void*)1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing value for [9]

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

@kcudnik kcudnik kcudnik left review comments

@Junchao-Mellanox Junchao-Mellanox Junchao-Mellanox approved these changes

@prgeor prgeor prgeor approved these changes

Assignees
No one assigned
Labels
Included in 202311 Branch Request for 202311 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@noaOrMlnx @mssonicbld @Junchao-Mellanox @kcudnik @prgeor @liushilongbuaa