From f1aca8a7f2a4aa931ab5ef447cf8a291dc327eb3 Mon Sep 17 00:00:00 2001 From: Prince Sunny Date: Mon, 27 Sep 2021 08:12:40 -0700 Subject: [PATCH] Cache routes for single nexthop for faster retrieval (#1922) * Created a map to store single nexthop routes. This is for faster retrieval when you've to fetch the routes for a particular nexthop. No changes to ECMP route handling. --- orchagent/routeorch.cpp | 122 +++++++++++++++++++++++++++++++--------- orchagent/routeorch.h | 17 ++++++ tests/test_mux.py | 23 ++++++++ 3 files changed, 134 insertions(+), 28 deletions(-) diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index d54e205ded..9e9bbe9891 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -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 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()) + { + 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; @@ -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) @@ -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 && !nextHops.is_overlay_nexthop()) + { + 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); @@ -2048,6 +2111,9 @@ bool RouteOrch::removeRoutePost(const RouteBulkContext& ctx) { m_neighOrch->removeMplsNextHop(nexthop); } + + RouteKey r_key = { vrf_id, ipPrefix }; + removeNextHopRoute(nexthop, r_key); } } diff --git a/orchagent/routeorch.h b/orchagent/routeorch.h index 20e79699d5..4f74db62dc 100644 --- a/orchagent/routeorch.h +++ b/orchagent/routeorch.h @@ -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 NextHopGroupTable; /* RouteTable: destination network, NextHopGroupKey */ @@ -56,6 +68,8 @@ typedef std::map LabelRouteTables; typedef std::pair Host; /* NextHopObserverTable: Host, next hop observer entry */ typedef std::map NextHopObserverTable; +/* Single Nexthop to Routemap */ +typedef std::map> NextHopRouteTable; struct NextHopObserverEntry { @@ -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&); @@ -170,6 +186,7 @@ class RouteOrch : public Orch, public Subject RouteTables m_syncdRoutes; LabelRouteTables m_syncdLabelRoutes; NextHopGroupTable m_syncdNextHopGroups; + NextHopRouteTable m_nextHops; std::set> m_bulkNhgReducedRefCnt; /* m_bulkNhgReducedRefCnt: nexthop, vrf_id */ diff --git a/tests/test_mux.py b/tests/test_mux.py index 84458a841d..c9aa578f79 100644 --- a/tests/test_mux.py +++ b/tests/test_mux.py @@ -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")