-
Notifications
You must be signed in to change notification settings - Fork 545
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
[mlagsyncd]Fix mlag socket read fail #1832
base: master
Are you sure you want to change the base?
[mlagsyncd]Fix mlag socket read fail #1832
Conversation
Signed-off-by: pettershao-ragilenetworks <pettershao@ragilenetworks.com>
@lguohan @qiluo-msft @Praveen-Brcm help review this, is a sometime issue, thanks! |
@pettershao-ragilenetworks Thanks for the change. Am not added as reviwer, if someone can add me will provide approals from my end. |
@lguohan @qiluo-msft help forward this, thanks! |
@@ -204,7 +204,6 @@ namespace swss { | |||
std::string m_system_mac; | |||
std::set<vlan_mbr> m_vlan_mbrship; //set of vlan,mbr tuples | |||
|
|||
const int MSG_BATCH_SIZE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const int MSG_BATCH_SIZE; | |
static const int MSG_BATCH_SIZE = 256; | |
``` #Closed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this fix is easier than defining as a macro.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea, accepted.
@lguohan help merge this, thanks! |
@lguohan @qiluo-msft help merge this, thanks! |
@lguohan @qiluo-msft help forward this, thanks! |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@lguohan @qiluo-msft help forward this, thanks! |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@lguohan @qiluo-msft As it is approved long time ago, help merge this, thanks! |
@pettershao-ragilenetworks The issue you tried to fix here is already fixed in #2112 by fixing the initialization order. |
What I did
fix mclagysncd server socket read fail
Why I did it
if socket read fail, mclagsyncd can't communicate with iccpd, the whole mlag can't work
How I verified it
check the socket state
Details if related
issue description:
from below strace log, the read size for fd 34 is 0, which means the initiazation of this variable is not correct. MSG_BATCH_SIZE should be a macro, otherwise it may be inited after size and cause size 0 .
below show a normal read strace log: