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

Cache routes for single nexthop for faster retrieval #1922

Merged
merged 8 commits into from
Sep 27, 2021
Merged
Show file tree
Hide file tree
Changes from 7 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
122 changes: 94 additions & 28 deletions orchagent/routeorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1317,47 +1317,94 @@ bool RouteOrch::removeNextHopGroup(const NextHopGroupKey &nexthops)
return true;
}

void RouteOrch::addNextHopRoute(const NextHopKey& nextHop, const RouteKey& routeKey)
{
auto it = m_nextHops.find((nextHop));

if (it != m_nextHops.end())
{
if (it->second.find(routeKey) != it->second.end())
{
SWSS_LOG_INFO("Route already present in nh table %s",
routeKey.prefix.to_string().c_str());
return;
}

it->second.insert(routeKey);
}
else
{
set<RouteKey> routes;
routes.insert(routeKey);
m_nextHops.insert(make_pair(nextHop, routes));
}
}

void RouteOrch::removeNextHopRoute(const NextHopKey& nextHop, const RouteKey& routeKey)
{
auto it = m_nextHops.find((nextHop));

if (it != m_nextHops.end())
prsunny marked this conversation as resolved.
Show resolved Hide resolved
{
if (it->second.find(routeKey) == it->second.end())
{
SWSS_LOG_INFO("Route not present in nh table %s", routeKey.prefix.to_string().c_str());
return;
}

it->second.erase(routeKey);
if (it->second.empty())
{
m_nextHops.erase(nextHop);
}
}
else
{
SWSS_LOG_INFO("Nexthop %s not found in nexthop table", nextHop.to_string().c_str());
}
}

bool RouteOrch::updateNextHopRoutes(const NextHopKey& nextHop, uint32_t& numRoutes)
{
numRoutes = 0;
auto it = m_nextHops.find((nextHop));

if (it == m_nextHops.end())
{
SWSS_LOG_INFO("No routes found for NH %s", nextHop.ip_address.to_string().c_str());
return true;
}

sai_route_entry_t route_entry;
sai_attribute_t route_attr;
sai_object_id_t next_hop_id;

for (auto rt_table : m_syncdRoutes)
auto rt = it->second.begin();
while(rt != it->second.end())
{
for (auto rt_entry : rt_table.second)
{
// Skip routes with ecmp nexthops
if (rt_entry.second.getSize() > 1)
{
continue;
}
SWSS_LOG_INFO("Updating route %s", (*rt).prefix.to_string().c_str());
next_hop_id = m_neighOrch->getNextHopId(nextHop);

if (rt_entry.second.contains(nextHop))
{
SWSS_LOG_INFO("Updating route %s during nexthop status change",
rt_entry.first.to_string().c_str());
next_hop_id = m_neighOrch->getNextHopId(nextHop);

route_entry.vr_id = rt_table.first;
route_entry.switch_id = gSwitchId;
copy(route_entry.destination, rt_entry.first);
route_entry.vr_id = (*rt).vrf_id;
route_entry.switch_id = gSwitchId;
copy(route_entry.destination, (*rt).prefix);

route_attr.id = SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID;
route_attr.value.oid = next_hop_id;
route_attr.id = SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID;
route_attr.value.oid = next_hop_id;

sai_status_t status = sai_route_api->set_route_entry_attribute(&route_entry, &route_attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to update route %s, rv:%d",
rt_entry.first.to_string().c_str(), status);
return false;
}

++numRoutes;
sai_status_t status = sai_route_api->set_route_entry_attribute(&route_entry, &route_attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to update route %s, rv:%d", (*rt).prefix.to_string().c_str(), status);
task_process_status handle_status = handleSaiSetStatus(SAI_API_ROUTE, status);
if (handle_status != task_success)
{
return parseHandleSaiStatusFailure(handle_status);
}
}

++numRoutes;
++rt;
}

return true;
Expand Down Expand Up @@ -1856,6 +1903,12 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey
SWSS_LOG_NOTICE("Update overlay Nexthop %s", ol_nextHops.to_string().c_str());
m_bulkNhgReducedRefCnt.emplace(ol_nextHops, vrf_id);
}
else if (ol_nextHops.getSize() == 1)
{
RouteKey r_key = { vrf_id, ipPrefix };
auto nexthop = NextHopKey(ol_nextHops.to_string());
removeNextHopRoute(nexthop, r_key);
}
}

if (blackhole)
Expand All @@ -1878,6 +1931,16 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey
ipPrefix.to_string().c_str(), nextHops.to_string().c_str());
}

if (nextHops.getSize() == 1)
{
RouteKey r_key = { vrf_id, ipPrefix };
auto nexthop = NextHopKey(nextHops.to_string());
if (!nexthop.ip_address.isZero())
{
addNextHopRoute(nexthop, r_key);
}
}

m_syncdRoutes[vrf_id][ipPrefix] = nextHops;

notifyNextHopChangeObservers(vrf_id, ipPrefix, nextHops, true);
Expand Down Expand Up @@ -2048,6 +2111,9 @@ bool RouteOrch::removeRoutePost(const RouteBulkContext& ctx)
{
m_neighOrch->removeMplsNextHop(nexthop);
}

RouteKey r_key = { vrf_id, ipPrefix };
removeNextHopRoute(nexthop, r_key);
}
}

Expand Down
17 changes: 17 additions & 0 deletions orchagent/routeorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,18 @@ struct NextHopUpdate

struct NextHopObserverEntry;

/* Route destination key for a nexthop */
struct RouteKey
{
sai_object_id_t vrf_id;
IpPrefix prefix;

bool operator < (const RouteKey& rhs) const
{
return (vrf_id <= rhs.vrf_id && prefix < rhs.prefix);
}
};

/* NextHopGroupTable: NextHopGroupKey, NextHopGroupEntry */
typedef std::map<NextHopGroupKey, NextHopGroupEntry> NextHopGroupTable;
/* RouteTable: destination network, NextHopGroupKey */
Expand All @@ -56,6 +68,8 @@ typedef std::map<sai_object_id_t, LabelRouteTable> LabelRouteTables;
typedef std::pair<sai_object_id_t, IpAddress> Host;
/* NextHopObserverTable: Host, next hop observer entry */
typedef std::map<Host, NextHopObserverEntry> NextHopObserverTable;
/* Single Nexthop to Routemap */
typedef std::map<NextHopKey, std::set<RouteKey>> NextHopRouteTable;

struct NextHopObserverEntry
{
Expand Down Expand Up @@ -138,6 +152,8 @@ class RouteOrch : public Orch, public Subject
bool addNextHopGroup(const NextHopGroupKey&);
bool removeNextHopGroup(const NextHopGroupKey&);

void addNextHopRoute(const NextHopKey&, const RouteKey&);
void removeNextHopRoute(const NextHopKey&, const RouteKey&);
bool updateNextHopRoutes(const NextHopKey&, uint32_t&);

bool validnexthopinNextHopGroup(const NextHopKey&, uint32_t&);
Expand Down Expand Up @@ -170,6 +186,7 @@ class RouteOrch : public Orch, public Subject
RouteTables m_syncdRoutes;
LabelRouteTables m_syncdLabelRoutes;
NextHopGroupTable m_syncdNextHopGroups;
NextHopRouteTable m_nextHops;

std::set<std::pair<NextHopGroupKey, sai_object_id_t>> m_bulkNhgReducedRefCnt;
/* m_bulkNhgReducedRefCnt: nexthop, vrf_id */
Expand Down
23 changes: 23 additions & 0 deletions tests/test_mux.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,29 @@ def create_and_test_route(self, appdb, asicdb, dvs, dvs_route):

self.check_nexthop_in_asic_db(asicdb, rtkeys[0])

# Check route set flow and changing nexthop
self.set_mux_state(appdb, "Ethernet4", "active")

ps = swsscommon.ProducerStateTable(pdb.db_connection, "ROUTE_TABLE")
fvs = swsscommon.FieldValuePairs([("nexthop", self.SERV2_IPV4), ("ifname", "Vlan1000")])
ps.set(rtprefix, fvs)

# Check if route was propagated to ASIC DB
rtkeys = dvs_route.check_asicdb_route_entries([rtprefix])

# Change Mux status for Ethernet0 and expect no change to replaced route
self.set_mux_state(appdb, "Ethernet0", "standby")
self.check_nexthop_in_asic_db(asicdb, rtkeys[0])

self.set_mux_state(appdb, "Ethernet4", "standby")
self.check_nexthop_in_asic_db(asicdb, rtkeys[0], True)

# Delete the route
ps._del(rtprefix)

self.set_mux_state(appdb, "Ethernet4", "active")
dvs_route.check_asicdb_deleted_route_entries([rtprefix])

# Test ECMP routes

self.set_mux_state(appdb, "Ethernet0", "active")
Expand Down