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

Ignore netlink messages for old interface during swss restart #389

Merged
merged 2 commits into from
Nov 16, 2017

Conversation

prsunny
Copy link
Collaborator

@prsunny prsunny commented Nov 16, 2017

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 this ifIndex.

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

* Fix to ignore this and any further messages for this ifindex
*/

static std::map<unsigned int, std::string> m_ifindexOldNameMap;
Copy link
Contributor

Choose a reason for hiding this comment

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

no need for std::

Copy link
Contributor

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.

Copy link
Collaborator Author

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

@lguohan lguohan merged commit 8f4d3c4 into sonic-net:master Nov 16, 2017
praveen-li pushed a commit to praveen-li/sonic-swss that referenced this pull request Aug 24, 2020
praveen-li pushed a commit to praveen-li/sonic-swss that referenced this pull request Aug 24, 2020
EdenGri pushed a commit to EdenGri/sonic-swss that referenced this pull request Feb 28, 2022
* Use redis binary dump
* Move root check first
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants