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

[routesync] Fix for stale dynamic neighbor #2553

Merged
merged 2 commits into from
Jan 12, 2023
Merged

[routesync] Fix for stale dynamic neighbor #2553

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a923a54
[routesync] Fix for stale dynamic neighbor
vganesan-nokia Nov 29, 2022
e5f590c
[routesync] Fix to handle warm restart
vganesan-nokia Jan 10, 2023
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
26 changes: 26 additions & 0 deletions fpmsyncd/routesync.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,32 @@ void RouteSync::onRouteMsg(int nlmsg_type, struct nl_object *obj, char *vrf)
{
SWSS_LOG_DEBUG("Skip routes to eth0 or docker0: %s %s %s",
destipprefix, gw_list.c_str(), intf_list.c_str());
// If intf_list has only this interface, that means all of the next hops of this route
// have been removed and the next hop on the eth0/docker0 has become the only next hop.
// In this case since we do not want the route with next hop on eth0/docker0, we return.
// But still we need to clear the route from the APPL_DB. Otherwise the APPL_DB and data
// path will be left with stale route entry
if(alsv.size() == 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

From the context, it seems for del msg, should we check it by
if((nlmsg_type == RTM_DELROUTE) && (alsv.size() == 1))
?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vganesan-nokia , do you want to address the comment?

Copy link
Contributor

Choose a reason for hiding this comment

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

Even it is a RTM_NEWROUTE on eth0/docker0, it may run into

if (!warmRestartInProgress)
{
...
m_routeTable.del(destipprefix)
}

then the route is deleted from APPL_DB and ASIC_DB ?
It happened on my real device with default route (0.0.0.0/0) is removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vganesan-nokia , do you want to address the comment?

Yes. I'll make a PR to fix this.

{
if (!warmRestartInProgress)
{
SWSS_LOG_NOTICE("RouteTable del msg for route with only one nh on eth0/docker0: %s %s %s %s",
destipprefix, gw_list.c_str(), intf_list.c_str(), mpls_list.c_str());

m_routeTable.del(destipprefix);
}
else
{
SWSS_LOG_NOTICE("Warm-Restart mode: Receiving delete msg for route with only nh on eth0/docker0: %s %s %s %s",
destipprefix, gw_list.c_str(), intf_list.c_str(), mpls_list.c_str());

vector<FieldValueTuple> fvVector;
const KeyOpFieldsValuesTuple kfv = std::make_tuple(destipprefix,
DEL_COMMAND,
fvVector);
m_warmStartHelper.insertRefreshMap(kfv);
}
}
return;
}
}
Expand Down