-
Notifications
You must be signed in to change notification settings - Fork 547
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
Ignore netlink messages for old interface during swss restart #389
Conversation
* Fix to ignore this and any further messages for this ifindex | ||
*/ | ||
|
||
static std::map<unsigned int, std::string> m_ifindexOldNameMap; |
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.
no need for std::
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.
Why use static variable, can you add it as a member attribute of LinkSync?
When would m_ifindexOldNameMap be cleared? If it never gets cleared, will it stop working for the second restart?
I suggest you use unordered_map to get O(1) look up time.
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.
@stcheng , will do in future commits.
@yxieca, Thought about adding it as a static member attribute, but it is kind of specific handling for this particular functionality and didn't want to expose it in the class definition.
Everytime you restart swss, the map would be reinitialized. Also, the old interface index won't be reused, and so I think we are ok in not clearing it during the old interface delete scenario.
Yes, unordered_map would've been a better solution but I just followed the declaration of existing map - m_ifindexNameMap
This reverts commit a3b1ec0.
* Use redis binary dump * Move root check first
…ic-net#389) * [mlnx|ffb] Add fast-fast boot option in syncd Signed-off-by: Stepan Blyschak <stepanb@mellanox.com> * [mlnx|ffb]: Add support of "config end" event for mlnx fast-fast boot Signed-off-by: Volodymyr Samotiy <volodymyrs@mellanox.com> * [mlnx|ffb]: Fix misspelled words for aspell check Signed-off-by: Volodymyr Samotiy <volodymyrs@mellanox.com> * [Mellanox|FFB]: Fix review comments * Change naming convention from "fast-fast" to "fastfast" Signed-off-by: Volodymyr Samotiy <volodymyrs@mellanox.com> * [Mellanox|FFB]: Add misspelled word 'fastfast' to aspellcheck dictionary
What I did
In netlink process for link events, a new interface creation always comes without a master or admin status set to iff_up. These are set after the interface is created. In this way, if the first netlink message comes with master set, add it to
old-interface-map
and ignore this and all subsequent netlink message for thisifIndex
.Why I did it
Without this fix, portsynd publishes the old interface events to stateDB. This resulted in failure of bridge creation and member port addition. Also, the newly created interfaces were not made admin-up
Related bug - sonic-net/sonic-buildimage#1153
How I verified it
Restart swss in t0/t1-lag topology
Execute 'sudo brctl show', 'ip link show' and ensure bridge is added with member ports and links are coming up.
Details if related
N/A