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
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions portsyncd/linksync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,43 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
nlmsg_type, key.c_str(), admin, oper, addrStr, ifindex, master);
}

/* Insert or update the ifindex to key map */
m_ifindexNameMap[ifindex] = key;

/* teamd instances are dealt in teamsyncd */
if (type && !strcmp(type, TEAM_DRV_NAME))
{
return;
}

/* In the event of swss restart, it is possible to get netlink messages during bridge
* delete, interface delete etc which are part of cleanup. These netlink messages for
* the front-panel interface must not be published or it will update the statedb with
* old interface info and result in subsequent failures. A new interface creation shall
* not have master or admin status iff_up. So if the first netlink message comes with these
* values set, it is considered to be happening during a cleanup process.
* 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

if (m_ifindexNameMap.find(ifindex) == m_ifindexNameMap.end())
{
if (master)
{
m_ifindexOldNameMap[ifindex] = key;
SWSS_LOG_INFO("nlmsg type:%d Ignoring for %d, master %d", nlmsg_type, ifindex, master);
return;
}
else if (m_ifindexOldNameMap.find(ifindex) != m_ifindexOldNameMap.end())
{
if (m_ifindexOldNameMap[ifindex] == key)
{
SWSS_LOG_INFO("nlmsg type:%d Ignoring message for old interface %d", nlmsg_type, ifindex);
return;
}
}
}

/* Insert or update the ifindex to key map */
m_ifindexNameMap[ifindex] = key;

vector<FieldValueTuple> fvVector;
FieldValueTuple a("admin_status", admin ? "up" : "down");
FieldValueTuple m("mtu", to_string(mtu));
Expand Down