Skip to content

fpmsyncd: Re-allocating link cache when interface name cannot be resolved #61

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

Merged
stcheng merged 3 commits into from
Sep 2, 2016
Merged

fpmsyncd: Re-allocating link cache when interface name cannot be resolved #61

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1cd233b
fpmsyncd: Re-allocate link cache when interface name cannot be resolved
Aug 31, 2016
67bd5f7
fpmsyncd: Renaming variables
Sep 2, 2016
706c460
fpmsyncd: Removing unnecessary m_link_cache check
Sep 2, 2016
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
35 changes: 20 additions & 15 deletions fpmsyncd/routesync.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj)
{
struct rtnl_route *route_obj = (struct rtnl_route *)obj;
struct nl_addr *dip;
char addrStr[MAX_ADDR_SIZE + 1] = {0};
char ifname[MAX_ADDR_SIZE + 1] = {0};
uint32_t ipv4;
int prefix;

dip = rtnl_route_get_dst(route_obj);
/* Supports IPv4 address only for now */
if (rtnl_route_get_family(route_obj) != AF_INET)
{
nl_addr2str(dip, addrStr, MAX_ADDR_SIZE);
nl_addr2str(dip, ifname, MAX_ADDR_SIZE);
SWSS_LOG_INFO("%s: Unknown route family support: %s (object: %s)\n",
__FUNCTION__, addrStr, nl_object_get_type(obj));
__FUNCTION__, ifname, nl_object_get_type(obj));
return;
}

Expand All @@ -50,9 +50,9 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj)
}
else if (nlmsg_type != RTM_NEWROUTE)
{
nl_addr2str(dip, addrStr, MAX_ADDR_SIZE);
nl_addr2str(dip, ifname, MAX_ADDR_SIZE);
SWSS_LOG_INFO("%s: Unknown message-type: %d for %s\n",
__FUNCTION__, nlmsg_type, addrStr);
__FUNCTION__, nlmsg_type, ifname);
return;
}

Expand All @@ -72,9 +72,9 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj)
case RTN_MULTICAST:
case RTN_BROADCAST:
case RTN_LOCAL:
nl_addr2str(dip, addrStr, MAX_ADDR_SIZE);
nl_addr2str(dip, ifname, MAX_ADDR_SIZE);
SWSS_LOG_INFO("%s: BUM routes aren't supported yet (%s)\n",
__FUNCTION__, addrStr);
__FUNCTION__, ifname);
return;

default:
Expand All @@ -83,14 +83,14 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj)

/* Geting nexthop lists */
string nexthops;
string ifindexes;
string ifnames;

struct nl_list_head *nhs = rtnl_route_get_nexthops(route_obj);
if (!nhs)
{
nl_addr2str(dip, addrStr, MAX_ADDR_SIZE);
nl_addr2str(dip, ifname, MAX_ADDR_SIZE);
SWSS_LOG_INFO("%s: Nexthop list is empty for %s\n",
__FUNCTION__, addrStr);
__FUNCTION__, ifname);
return;
}

Expand All @@ -109,24 +109,29 @@ void RouteSync::onMsg(int nlmsg_type, struct nl_object *obj)
/* 0 means no ifindex */
if (m_link_cache)
{
rtnl_link_i2name(m_link_cache, ifindex, addrStr, MAX_ADDR_SIZE);
ifindexes += addrStr;
rtnl_link_i2name(m_link_cache, ifindex, ifname, MAX_ADDR_SIZE);
if (!strlen(ifname))
{
rtnl_link_alloc_cache(m_nl_sock, AF_UNSPEC, &m_link_cache);
rtnl_link_i2name(m_link_cache, ifindex, ifname, MAX_ADDR_SIZE);
Copy link
Copy Markdown
Contributor

@lguohan lguohan Sep 2, 2016

Choose a reason for hiding this comment

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

could the ifname still not get resolved even after updating the cache?
should we throw exception or print here if it happens?

Copy link
Copy Markdown
Contributor Author

@stcheng stcheng Sep 2, 2016

Choose a reason for hiding this comment

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

the ifname is currently only used as information. the orchagent doesn't depend on this value. i think for here, if the ifname cannot be resolved after updating the cache, we could just add "unknown" here.

Copy link
Copy Markdown
Contributor Author

@stcheng stcheng Sep 2, 2016

Choose a reason for hiding this comment

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

@lguohan

}
Copy link
Copy Markdown
Contributor

@lguohan lguohan Sep 2, 2016

Choose a reason for hiding this comment

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

after you refresh the cache, do you need to resolve the name again?

ifnames += ifname;
} else
{
ifindexes += to_string(ifindex);
ifnames += to_string(ifindex);
}


if (i + 1 < rtnl_route_get_nnexthops(route_obj))
{
nexthops += string(",");
ifindexes += string(",");
ifnames += string(",");
}
}

std::vector<FieldValueTuple> fvVector;
FieldValueTuple nh("nexthop", nexthops);
FieldValueTuple idx("ifindex", ifindexes);
FieldValueTuple idx("ifindex", ifnames);
Copy link
Copy Markdown
Contributor

@lguohan lguohan Sep 2, 2016

Choose a reason for hiding this comment

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

should we change this to ifname as well?

fvVector.push_back(nh);
fvVector.push_back(idx);
m_routeTable.set(destip.to_string(), fvVector);
Expand Down