From f248e2666aec833f762a0109a01fe171afecd161 Mon Sep 17 00:00:00 2001 From: "thomas.cappleman@metaswitch.com" Date: Fri, 15 Oct 2021 17:12:27 +0100 Subject: [PATCH] [orchagent] Add separate next hop table and orch (#1702) Added a new next hop group table to APP_DB, and orchagent support to use this as an alternative to including next hop information in the route table. Added a new "nghOrch" that subscribes to next hop group table events to program next hop groups into SAI layer. Improves performance and occupancy usage when using the new table Extended SWSS unit tests to cover new code, and ran existing tests --- doc/swss-schema.md | 29 +- orchagent/Makefile.am | 1 + orchagent/mplsrouteorch.cpp | 306 ++++--- orchagent/neighorch.cpp | 20 +- orchagent/neighorch.h | 1 + orchagent/nexthopkey.h | 3 +- orchagent/nhgorch.cpp | 1078 +++++++++++++++++++++++ orchagent/nhgorch.h | 249 ++++++ orchagent/orchdaemon.cpp | 6 +- orchagent/orchdaemon.h | 1 + orchagent/routeorch.cpp | 381 +++++--- orchagent/routeorch.h | 46 +- tests/mock_tests/Makefile.am | 1 + tests/mock_tests/aclorch_ut.cpp | 2 +- tests/test_nhg.py | 1434 ++++++++++++++++++++++++++----- 15 files changed, 3084 insertions(+), 474 deletions(-) create mode 100644 orchagent/nhgorch.cpp create mode 100644 orchagent/nhgorch.h diff --git a/doc/swss-schema.md b/doc/swss-schema.md index 3ccc7b74af..4da3cb3b2a 100644 --- a/doc/swss-schema.md +++ b/doc/swss-schema.md @@ -159,8 +159,35 @@ and reflects the LAG ports into the redis under: `LAG_TABLE::port` ;Status: Mandatory key = ROUTE_TABLE:prefix nexthop = *prefix, ;IP addresses separated “,” (empty indicates no gateway) - intf = ifindex? PORT_TABLE.key ; zero or more separated by “,” (zero indicates no interface) + ifname = ifindex? PORT_TABLE.key ; zero or more separated by “,” (zero indicates no interface) + mpls_nh = STRING ; Comma-separated list of MPLS NH info. blackhole = BIT ; Set to 1 if this route is a blackhole (or null0) + weight = weight_list ; List of weights. + nexthop_group = string ; index within the NEXTHOP_GROUP_TABLE, used instead of nexthop and intf fields + +--------------------------------------------- + +###### LABEL_ROUTE_TABLE + ; Defines schema for MPLS label route table attributes + key = LABEL_ROUTE_TABLE:mpls_label ; MPLS label + ; field = value + nexthop = STRING ; Comma-separated list of nexthops. + ifname = STRING ; Comma-separated list of interfaces. + mpls_nh = STRING ; Comma-separated list of MPLS NH info. + mpls_pop = STRING ; Number of ingress MPLS labels to POP + weight = STRING ; Comma-separated list of weights. + blackhole = BIT ; Set to 1 if this route is a blackhole (or null0) + nexthop_group = string ; index within the NEXTHOP_GROUP_TABLE, used instead of nexthop and intf fields + +--------------------------------------------- +### NEXTHOP_GROUP_TABLE + ;Stores a list of groups of one or more next hops + ;Status: Mandatory + key = NEXTHOP_GROUP_TABLE:string ; arbitrary index for the next hop group + nexthop = *prefix, ;IP addresses separated “,” (empty indicates no gateway) + ifname = ifindex? PORT_TABLE.key ; zero or more separated by “,” (zero indicates no interface) + mpls_nh = STRING ; Comma-separated list of MPLS NH info. + weight = weight_list ; List of weights. --------------------------------------------- ### NEIGH_TABLE diff --git a/orchagent/Makefile.am b/orchagent/Makefile.am index 94d66e0024..ce56e8438e 100644 --- a/orchagent/Makefile.am +++ b/orchagent/Makefile.am @@ -39,6 +39,7 @@ orchagent_SOURCES = \ orchdaemon.cpp \ orch.cpp \ notifications.cpp \ + nhgorch.cpp \ routeorch.cpp \ mplsrouteorch.cpp \ neighorch.cpp \ diff --git a/orchagent/mplsrouteorch.cpp b/orchagent/mplsrouteorch.cpp index 6db46e01a5..1bf37e711c 100644 --- a/orchagent/mplsrouteorch.cpp +++ b/orchagent/mplsrouteorch.cpp @@ -6,12 +6,14 @@ #include "swssnet.h" #include "converter.h" #include "crmorch.h" +#include "nhgorch.h" #include "directory.h" extern sai_object_id_t gVirtualRouterId; extern sai_object_id_t gSwitchId; extern CrmOrch *gCrmOrch; +extern NhgOrch *gNhgOrch; void RouteOrch::doLabelTask(Consumer& consumer) { @@ -128,6 +130,7 @@ void RouteOrch::doLabelTask(Consumer& consumer) string weights; bool& excp_intfs_flag = ctx.excp_intfs_flag; bool blackhole = false; + string nhg_index; for (auto i : kfvFieldsValues(t)) { @@ -148,85 +151,122 @@ void RouteOrch::doLabelTask(Consumer& consumer) if (fvField(i) == "weight") weights = fvValue(i); + + if (fvField(i) == "nexthop_group") + nhg_index = fvValue(i); } - vector ipv = tokenize(ips, ','); - vector alsv = tokenize(aliases, ','); - vector mpls_nhv = tokenize(mpls_nhs, ','); - /* Resize the ip vector to match ifname vector - * as tokenize(",", ',') will miss the last empty segment. */ - if (alsv.size() == 0 && !blackhole) + /* + * A route should not fill both nexthop_group and ips / + * aliases. + */ + if (!nhg_index.empty() && (!ips.empty() || !aliases.empty())) { - SWSS_LOG_WARN("Skip the route %s, for it has an empty ifname field.", key.c_str()); + SWSS_LOG_ERROR("Route %s has both nexthop_group and ips/aliases", + key.c_str()); it = consumer.m_toSync.erase(it); continue; } - else if (alsv.size() != ipv.size()) - { - SWSS_LOG_NOTICE("Route %s: resize ipv to match alsv, %zd -> %zd.", key.c_str(), ipv.size(), alsv.size()); - ipv.resize(alsv.size()); - } - /* Set the empty ip(s) to zero - * as IpAddress("") will construct a incorrect ip. */ - for (auto &ip : ipv) + ctx.nhg_index = nhg_index; + + vector ipv; + vector alsv; + vector mpls_nhv; + + /* + * If the nexthop_group is empty, create the next hop group key + * based on the IPs and aliases. Otherwise, get the key from + * the NhgOrch. + */ + if (nhg_index.empty()) { - if (ip.empty()) + + ipv = tokenize(ips, ','); + alsv = tokenize(aliases, ','); + mpls_nhv = tokenize(mpls_nhs, ','); + + /* Resize the ip vector to match ifname vector + * as tokenize(",", ',') will miss the last empty segment. */ + if (alsv.size() == 0 && !blackhole) { - SWSS_LOG_NOTICE("Route %s: set the empty nexthop ip to zero.", key.c_str()); - ip = "0.0.0.0"; + SWSS_LOG_WARN("Skip the route %s, for it has an empty ifname field.", key.c_str()); + it = consumer.m_toSync.erase(it); + continue; + } + else if (alsv.size() != ipv.size()) + { + SWSS_LOG_NOTICE("Route %s: resize ipv to match alsv, %zd -> %zd.", key.c_str(), ipv.size(), alsv.size()); + ipv.resize(alsv.size()); } - } - for (auto alias : alsv) - { - /* skip route to management, docker, loopback - * TODO: for route to loopback interface, the proper - * way is to create loopback interface and then create - * route pointing to it, so that we can traps packets to - * CPU */ - if (alias == "eth0" || alias == "docker0" || - alias == "lo" || !alias.compare(0, strlen(LOOPBACK_PREFIX), LOOPBACK_PREFIX)) + for (auto alias : alsv) { - excp_intfs_flag = true; - break; + /* skip route to management, docker, loopback + * TODO: for route to loopback interface, the proper + * way is to create loopback interface and then create + * route pointing to it, so that we can traps packets to + * CPU */ + if (alias == "eth0" || alias == "docker0" || + alias == "lo" || !alias.compare(0, strlen(LOOPBACK_PREFIX), LOOPBACK_PREFIX)) + { + excp_intfs_flag = true; + break; + } } - } - // TODO: cannot trust m_portsOrch->getPortIdByAlias because sometimes alias is empty - if (excp_intfs_flag) - { - /* If any existing routes are updated to point to the - * above interfaces, remove them from the ASIC. */ - if (removeLabelRoute(ctx)) - it = consumer.m_toSync.erase(it); - else - it++; - continue; - } + // TODO: cannot trust m_portsOrch->getPortIdByAlias because sometimes alias is empty + if (excp_intfs_flag) + { + /* If any existing routes are updated to point to the + * above interfaces, remove them from the ASIC. */ + if (removeLabelRoute(ctx)) + it = consumer.m_toSync.erase(it); + else + it++; + continue; + } - string nhg_str = ""; - NextHopGroupKey& nhg = ctx.nhg; + string nhg_str = ""; + NextHopGroupKey& nhg = ctx.nhg; - if (blackhole) - { - nhg = NextHopGroupKey(); + if (blackhole) + { + nhg = NextHopGroupKey(); + } + else + { + for (uint32_t i = 0; i < ipv.size(); i++) + { + if (i) nhg_str += NHG_DELIMITER; + if (!mpls_nhv.empty() && mpls_nhv[i] != "na") + { + nhg_str += mpls_nhv[i] + LABELSTACK_DELIMITER; + } + nhg_str += ipv[i] + NH_DELIMITER + alsv[i]; + } + + nhg = NextHopGroupKey(nhg_str, weights); + } } else { - for (uint32_t i = 0; i < ipv.size(); i++) + try { - if (i) nhg_str += NHG_DELIMITER; - if (!mpls_nhv.empty() && mpls_nhv[i] != "na") - { - nhg_str += mpls_nhv[i] + LABELSTACK_DELIMITER; - } - nhg_str += ipv[i] + NH_DELIMITER + alsv[i]; + const NextHopGroup& nh_group = gNhgOrch->getNhg(nhg_index); + ctx.nhg = nh_group.getKey(); + ctx.using_temp_nhg = nh_group.isTemp(); + } + catch (const std::out_of_range& e) + { + SWSS_LOG_ERROR("Next hop group %s does not exist", nhg_index.c_str()); + ++it; + continue; } - - nhg = NextHopGroupKey(nhg_str, weights); } + NextHopGroupKey& nhg = ctx.nhg; + if (nhg.getSize() == 1 && nhg.hasIntfNextHop()) { /* blackhole to be done */ @@ -251,7 +291,8 @@ void RouteOrch::doLabelTask(Consumer& consumer) } else if (m_syncdLabelRoutes.find(vrf_id) == m_syncdLabelRoutes.end() || m_syncdLabelRoutes.at(vrf_id).find(label) == m_syncdLabelRoutes.at(vrf_id).end() || - m_syncdLabelRoutes.at(vrf_id).at(label) != nhg) + m_syncdLabelRoutes.at(vrf_id).at(label) != RouteNhg(nhg, nhg_index) || + ctx.using_temp_nhg) { if (addLabelRoute(ctx, nhg)) it = consumer.m_toSync.erase(it); @@ -259,12 +300,15 @@ void RouteOrch::doLabelTask(Consumer& consumer) it++; } else + { /* Duplicate entry */ + SWSS_LOG_INFO("Route %s is duplicate entry", key.c_str()); it = consumer.m_toSync.erase(it); + } // If already exhaust the nexthop groups, and there are pending removing routes in bulker, // flush the bulker and possibly collect some released nexthop groups - if (m_nextHopGroupCount >= m_maxNextHopGroupCount && + if (gNhgOrch->getNhgCount() >= gNhgOrch->getMaxNhgCount() && gLabelRouteBulker.removing_entries_count() > 0) { break; @@ -341,7 +385,8 @@ void RouteOrch::doLabelTask(Consumer& consumer) } else if (m_syncdLabelRoutes.find(vrf_id) == m_syncdLabelRoutes.end() || m_syncdLabelRoutes.at(vrf_id).find(label) == m_syncdLabelRoutes.at(vrf_id).end() || - m_syncdLabelRoutes.at(vrf_id).at(label) != nhg) + m_syncdLabelRoutes.at(vrf_id).at(label) != RouteNhg(nhg, ctx.nhg_index) || + ctx.using_temp_nhg) { if (addLabelRoutePost(ctx, nhg)) it_prev = consumer.m_toSync.erase(it_prev); @@ -381,10 +426,14 @@ void RouteOrch::addTempLabelRoute(LabelRouteBulkContext& ctx, const NextHopGroup /* Remove next hops that are not in m_syncdNextHops */ for (auto it = next_hop_set.begin(); it != next_hop_set.end();) { - if (!m_neighOrch->hasNextHop(*it)) + /* + * Check if the IP next hop exists in NeighOrch. The next hop may be + * a labeled one, which are created by RouteOrch or NhgOrch if the IP + * next hop exists. + */ + if (!m_neighOrch->isNeighborResolved(*it)) { - SWSS_LOG_INFO("Failed to get next hop %s for %u", - (*it).to_string().c_str(), label); + SWSS_LOG_INFO("Failed to get next hop %s for %u", (*it).to_string().c_str(), label); it = next_hop_set.erase(it); } else @@ -425,7 +474,20 @@ bool RouteOrch::addLabelRoute(LabelRouteBulkContext& ctx, const NextHopGroupKey auto it_route = m_syncdLabelRoutes.at(vrf_id).find(label); - if (nextHops.getSize() == 0) + if (!ctx.nhg_index.empty()) + { + try + { + const NextHopGroup& nhg = gNhgOrch->getNhg(ctx.nhg_index); + next_hop_id = nhg.getId(); + } + catch(const std::out_of_range& e) + { + SWSS_LOG_WARN("Next hop group key %s does not exist", ctx.nhg_index.c_str()); + return false; + } + } + else if (nextHops.getSize() == 0) { /* The route is pointing to a blackhole */ blackhole = true; @@ -453,7 +515,7 @@ bool RouteOrch::addLabelRoute(LabelRouteBulkContext& ctx, const NextHopGroupKey } /* For non-existent MPLS NH, check if IP neighbor NH exists */ else if (nexthop.isMplsNextHop() && - m_neighOrch->hasNextHop(NextHopKey(nexthop.ip_address, nexthop.alias))) + m_neighOrch->isNeighborResolved(nexthop)) { /* since IP neighbor NH exists, neighbor is resolved, add MPLS NH */ m_neighOrch->addNextHop(nexthop); @@ -492,9 +554,10 @@ bool RouteOrch::addLabelRoute(LabelRouteBulkContext& ctx, const NextHopGroupKey /* If the current next hop is part of the next hop group to sync, * then return false and no need to add another temporary route. */ - if (it_route != m_syncdLabelRoutes.at(vrf_id).end() && it_route->second.getSize() == 1) + if (it_route != m_syncdLabelRoutes.at(vrf_id).end() && + it_route->second.nhg_key.getSize() == 1) { - const NextHopKey& nexthop = *it_route->second.getNextHops().begin(); + const NextHopKey& nexthop = *it_route->second.nhg_key.getNextHops().begin(); if (nextHops.contains(nexthop)) { return false; @@ -515,7 +578,6 @@ bool RouteOrch::addLabelRoute(LabelRouteBulkContext& ctx, const NextHopGroupKey /* Sync the inseg entry */ sai_inseg_entry_t inseg_entry; - // route_entry.vr_id = vrf_id; No VRF support for MPLS? inseg_entry.switch_id = gSwitchId; inseg_entry.label = label; @@ -559,7 +621,7 @@ bool RouteOrch::addLabelRoute(LabelRouteBulkContext& ctx, const NextHopGroupKey else { /* Set the packet action to forward when there was no next hop (dropped) */ - if (it_route->second.getSize() == 0 && !blackhole) + if (it_route->second.nhg_key.getSize() == 0 && !blackhole) { inseg_attr.id = SAI_INSEG_ENTRY_ATTR_PACKET_ACTION; inseg_attr.value.s32 = SAI_PACKET_ACTION_FORWARD; @@ -607,17 +669,27 @@ bool RouteOrch::addLabelRoutePost(const LabelRouteBulkContext& ctx, const NextHo /* next_hop_id indicates the next hop id or next hop group id of this route */ sai_object_id_t next_hop_id; - if (nextHops.getSize() == 0) + /* Check that the next hop group is not owned by NhgOrch. */ + if (!ctx.nhg_index.empty()) + { + if (!gNhgOrch->hasNhg(ctx.nhg_index)) + { + SWSS_LOG_WARN("Failed to get next hop group with index %s", ctx.nhg_index.c_str()); + return false; + } + } + else if (nextHops.getSize() == 0) { /* The route is pointing to a blackhole */ blackhole = true; } - if (nextHops.getSize() == 1) + /* The route is pointing to a next hop */ + else if (nextHops.getSize() == 1) { - /* The route is pointing to a next hop */ const NextHopKey& nexthop = *nextHops.getNextHops().begin(); if (nexthop.isIntfNextHop()) { + next_hop_id = m_intfsOrch->getRouterIntfsId(nexthop.alias); /* rif is not created yet */ if (next_hop_id == SAI_NULL_OBJECT_ID) @@ -668,7 +740,15 @@ bool RouteOrch::addLabelRoutePost(const LabelRouteBulkContext& ctx, const NextHo gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_MPLS_INSEG); /* Increase the ref_count for the next hop (group) entry */ - increaseNextHopRefCount(nextHops); + if (ctx.nhg_index.empty()) + { + increaseNextHopRefCount(nextHops); + } + else + { + gNhgOrch->incNhgRefCount(ctx.nhg_index); + } + SWSS_LOG_INFO("Post create label %u with next hop(s) %s", label, nextHops.to_string().c_str()); } @@ -677,7 +757,7 @@ bool RouteOrch::addLabelRoutePost(const LabelRouteBulkContext& ctx, const NextHo sai_status_t status; /* Set the packet action to forward when there was no next hop (dropped) and not pointing to blackhole */ - if (it_route->second.getSize() == 0 && !blackhole) + if (it_route->second.nhg_key.getSize() == 0 && !blackhole) { status = *it_status++; if (status != SAI_STATUS_SUCCESS) @@ -704,14 +784,30 @@ bool RouteOrch::addLabelRoutePost(const LabelRouteBulkContext& ctx, const NextHo } } - /* Increase the ref_count for the next hop (group) entry */ - increaseNextHopRefCount(nextHops); + /* Decrease the ref count for the previous next hop group. */ + if (it_route->second.nhg_index.empty()) + { + decreaseNextHopRefCount(it_route->second.nhg_key); + if (it_route->second.nhg_key.getSize() > 1 + && m_syncdNextHopGroups[it_route->second.nhg_key].ref_count == 0) + { + m_bulkNhgReducedRefCnt.emplace(it_route->second.nhg_key, 0); + } + } + else + { + /* The next hop group is owned by NeighOrch. */ + gNhgOrch->decNhgRefCount(it_route->second.nhg_index); + } - decreaseNextHopRefCount(it_route->second); - if (it_route->second.getSize() > 1 - && m_syncdNextHopGroups[it_route->second].ref_count == 0) + /* Increase the ref_count for the next hop (group) entry */ + if (ctx.nhg_index.empty()) + { + increaseNextHopRefCount(nextHops); + } + else { - m_bulkNhgReducedRefCnt.emplace(it_route->second, 0); + gNhgOrch->incNhgRefCount(ctx.nhg_index); } if (blackhole) @@ -734,7 +830,7 @@ bool RouteOrch::addLabelRoutePost(const LabelRouteBulkContext& ctx, const NextHo label, nextHops.to_string().c_str()); } - m_syncdLabelRoutes[vrf_id][label] = nextHops; + m_syncdLabelRoutes[vrf_id][label] = RouteNhg(nextHops, ctx.nhg_index); return true; } @@ -754,7 +850,6 @@ bool RouteOrch::removeLabelRoute(LabelRouteBulkContext& ctx) } sai_inseg_entry_t inseg_entry; - //inseg_entry.vr_id = vrf_id; No VRF support for MPLS inseg_entry.switch_id = gSwitchId; inseg_entry.label = label; @@ -807,31 +902,38 @@ bool RouteOrch::removeLabelRoutePost(const LabelRouteBulkContext& ctx) gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_MPLS_INSEG); - /* - * Decrease the reference count only when the route is pointing to a next hop. - */ - decreaseNextHopRefCount(it_route->second); - if (it_route->second.getSize() > 1 - && m_syncdNextHopGroups[it_route->second].ref_count == 0) + if (it_route->second.nhg_index.empty()) { - m_bulkNhgReducedRefCnt.emplace(it_route->second, 0); - } - /* - * Additionally check if the NH has label and its ref count == 0, then - * remove the label next hop. - */ - else if (it_route->second.getSize() == 1) - { - const NextHopKey& nexthop = *it_route->second.getNextHops().begin(); - if (nexthop.isMplsNextHop() && - (m_neighOrch->getNextHopRefCount(nexthop) == 0)) + /* + * Decrease the reference count only when the route is pointing to a next hop. + */ + decreaseNextHopRefCount(it_route->second.nhg_key); + if (it_route->second.nhg_key.getSize() > 1 + && m_syncdNextHopGroups[it_route->second.nhg_key].ref_count == 0) { - m_neighOrch->removeMplsNextHop(nexthop); + m_bulkNhgReducedRefCnt.emplace(it_route->second.nhg_key, 0); + } + /* + * Additionally check if the NH has label and its ref count == 0, then + * remove the label next hop. + */ + else if (it_route->second.nhg_key.getSize() == 1) + { + const NextHopKey& nexthop = *it_route->second.nhg_key.getNextHops().begin(); + if (nexthop.isMplsNextHop() && + (m_neighOrch->getNextHopRefCount(nexthop) == 0)) + { + m_neighOrch->removeMplsNextHop(nexthop); + } } } + else + { + gNhgOrch->decNhgRefCount(it_route->second.nhg_index); + } - SWSS_LOG_INFO("Remove label %u with next hop(s) %s", - label, it_route->second.to_string().c_str()); + SWSS_LOG_INFO("Remove label route %u with next hop(s) %s", + label, it_route->second.nhg_key.to_string().c_str()); it_route_table->second.erase(label); @@ -842,4 +944,4 @@ bool RouteOrch::removeLabelRoutePost(const LabelRouteBulkContext& ctx) } return true; -} +} \ No newline at end of file diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index e3f9a4175e..23d7a38b3b 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -7,6 +7,7 @@ #include "directory.h" #include "muxorch.h" #include "subscriberstatetable.h" +#include "nhgorch.h" extern sai_neighbor_api_t* sai_neighbor_api; extern sai_next_hop_api_t* sai_next_hop_api; @@ -15,6 +16,7 @@ extern PortsOrch *gPortsOrch; extern sai_object_id_t gSwitchId; extern CrmOrch *gCrmOrch; extern RouteOrch *gRouteOrch; +extern NhgOrch *gNhgOrch; extern FgNhgOrch *gFgNhgOrch; extern Directory gDirectory; extern string gMySwitchType; @@ -32,7 +34,7 @@ NeighOrch::NeighOrch(DBConnector *appDb, string tableName, IntfsOrch *intfsOrch, SWSS_LOG_ENTER(); m_fdbOrch->attach(this); - + if(gMySwitchType == "voq") { //Add subscriber to process VOQ system neigh @@ -169,6 +171,16 @@ bool NeighOrch::hasNextHop(const NextHopKey &nexthop) return m_syncdNextHops.find(nexthop) != m_syncdNextHops.end(); } +// Check if the underlying neighbor is resolved for a given next hop key. +bool NeighOrch::isNeighborResolved(const NextHopKey &nexthop) +{ + // Extract the IP address and interface from the next hop key, and check if the next hop + // for just that pair exists. + NextHopKey base_nexthop = NextHopKey(nexthop.ip_address, nexthop.alias); + + return hasNextHop(base_nexthop); +} + bool NeighOrch::addNextHop(const NextHopKey &nh) { SWSS_LOG_ENTER(); @@ -322,6 +334,7 @@ bool NeighOrch::setNextHopFlag(const NextHopKey &nexthop, const uint32_t nh_flag { case NHFLAGS_IFDOWN: rc = gRouteOrch->invalidnexthopinNextHopGroup(nexthop, count); + rc &= gNhgOrch->invalidateNextHop(nexthop); break; default: assert(0); @@ -351,6 +364,7 @@ bool NeighOrch::clearNextHopFlag(const NextHopKey &nexthop, const uint32_t nh_fl { case NHFLAGS_IFDOWN: rc = gRouteOrch->validnexthopinNextHopGroup(nexthop, count); + rc &= gNhgOrch->validateNextHop(nexthop); break; default: assert(0); @@ -1024,7 +1038,7 @@ bool NeighOrch::removeNeighbor(const NeighborEntry &neighborEntry, bool disable) NeighborUpdate update = { neighborEntry, MacAddress(), false }; notify(SUBJECT_TYPE_NEIGH_CHANGE, static_cast(&update)); - + if(gMySwitchType == "voq") { //Sync the neighbor to delete from the CHASSIS_APP_DB @@ -1298,7 +1312,7 @@ void NeighOrch::doVoqSystemNeighTask(Consumer &consumer) // the owner asic's mac is not readily avaiable here, the owner asic mac is derived from // the switch id and lower 5 bytes of asic mac which is assumed to be same for all asics // in the VS system. - // Therefore to make VOQ chassis systems work in VS platform based setups like the setups + // Therefore to make VOQ chassis systems work in VS platform based setups like the setups // using KVMs, it is required that all asics have same base mac in the format given below // :<6th byte = switch_id> diff --git a/orchagent/neighorch.h b/orchagent/neighorch.h index 3b5867a949..7a9a1fa330 100644 --- a/orchagent/neighorch.h +++ b/orchagent/neighorch.h @@ -49,6 +49,7 @@ class NeighOrch : public Orch, public Subject, public Observer ~NeighOrch(); bool hasNextHop(const NextHopKey&); + bool isNeighborResolved(const NextHopKey&); bool addNextHop(const NextHopKey&); bool removeMplsNextHop(const NextHopKey&); diff --git a/orchagent/nexthopkey.h b/orchagent/nexthopkey.h index 87c294415a..1e76916dd4 100644 --- a/orchagent/nexthopkey.h +++ b/orchagent/nexthopkey.h @@ -4,6 +4,7 @@ #include "ipaddress.h" #include "tokenize.h" #include "label.h" +#include "intfsorch.h" #define LABELSTACK_DELIMITER '+' #define NH_DELIMITER '@' @@ -160,7 +161,7 @@ struct NextHopKey std::string str; if (isMplsNextHop()) { - label_stack.to_string() + LABELSTACK_DELIMITER; + str = label_stack.to_string() + LABELSTACK_DELIMITER; } return str; } diff --git a/orchagent/nhgorch.cpp b/orchagent/nhgorch.cpp new file mode 100644 index 0000000000..4b2084e82a --- /dev/null +++ b/orchagent/nhgorch.cpp @@ -0,0 +1,1078 @@ +#include "nhgorch.h" +#include "neighorch.h" +#include "crmorch.h" +#include "routeorch.h" +#include "bulker.h" +#include "logger.h" +#include "swssnet.h" + +extern sai_object_id_t gSwitchId; + +extern PortsOrch *gPortsOrch; +extern CrmOrch *gCrmOrch; +extern NeighOrch *gNeighOrch; +extern RouteOrch *gRouteOrch; +extern NhgOrch *gNhgOrch; + +extern size_t gMaxBulkSize; + +extern sai_next_hop_group_api_t* sai_next_hop_group_api; +extern sai_next_hop_api_t* sai_next_hop_api; +extern sai_switch_api_t* sai_switch_api; + +unsigned int NextHopGroup::m_count = 0; + +NhgOrch::NhgOrch(DBConnector *db, string tableName) : + Orch(db, tableName) +{ + SWSS_LOG_ENTER(); +} + +/* + * Purpose: Perform the operations requested by APPL_DB users. + * Description: Iterate over the untreated operations list and resolve them. + * The operations supported are SET and DEL. If an operation + * could not be resolved, it will either remain in the list, or be + * removed, depending on the case. + * Params: IN consumer - The cosumer object. + * Returns: Nothing. + */ +void NhgOrch::doTask(Consumer& consumer) +{ + SWSS_LOG_ENTER(); + + if (!gPortsOrch->allPortsReady()) + { + return; + } + + auto it = consumer.m_toSync.begin(); + + while (it != consumer.m_toSync.end()) + { + KeyOpFieldsValuesTuple t = it->second; + + string index = kfvKey(t); + string op = kfvOp(t); + + bool success = false; + const auto& nhg_it = m_syncdNextHopGroups.find(index); + + if (op == SET_COMMAND) + { + string ips; + string aliases; + string weights; + string mpls_nhs; + + /* Get group's next hop IPs and aliases */ + for (auto i : kfvFieldsValues(t)) + { + if (fvField(i) == "nexthop") + ips = fvValue(i); + + if (fvField(i) == "ifname") + aliases = fvValue(i); + + if (fvField(i) == "weight") + weights = fvValue(i); + + if (fvField(i) == "mpls_nh") + mpls_nhs = fvValue(i); + } + + /* Split ips and alaises strings into vectors of tokens. */ + vector ipv = tokenize(ips, ','); + vector alsv = tokenize(aliases, ','); + vector mpls_nhv = tokenize(mpls_nhs, ','); + + /* Create the next hop group key. */ + string nhg_str; + + for (uint32_t i = 0; i < ipv.size(); i++) + { + if (i) nhg_str += NHG_DELIMITER; + if (!mpls_nhv.empty() && mpls_nhv[i] != "na") + { + nhg_str += mpls_nhv[i] + LABELSTACK_DELIMITER; + } + nhg_str += ipv[i] + NH_DELIMITER + alsv[i]; + } + + NextHopGroupKey nhg_key = NextHopGroupKey(nhg_str, weights); + + /* If the group does not exist, create one. */ + if (nhg_it == m_syncdNextHopGroups.end()) + { + /* + * If we've reached the NHG limit, we're going to create a temporary + * group, represented by one of it's NH only until we have + * enough resources to sync the whole group. The item is going + * to be kept in the sync list so we keep trying to create the + * actual group when there are enough resources. + */ + if (gRouteOrch->getNhgCount() + NextHopGroup::getCount() >= gRouteOrch->getMaxNhgCount()) + { + SWSS_LOG_DEBUG("Next hop group count reached its limit."); + + try + { + auto nhg = std::make_unique(createTempNhg(nhg_key)); + if (nhg->sync()) + { + m_syncdNextHopGroups.emplace(index, NhgEntry(std::move(nhg))); + } + else + { + SWSS_LOG_INFO("Failed to sync temporary NHG %s with %s", + index.c_str(), + nhg_key.to_string().c_str()); + } + } + catch (const std::exception& e) + { + SWSS_LOG_INFO("Got exception: %s while adding temp group %s", + e.what(), + nhg_key.to_string().c_str()); + } + } + else + { + auto nhg = std::make_unique(nhg_key, false); + success = nhg->sync(); + + if (success) + { + m_syncdNextHopGroups.emplace(index, NhgEntry(std::move(nhg))); + } + } + } + /* If the group exists, update it. */ + else + { + const auto& nhg_ptr = nhg_it->second.nhg; + + /* + * If the update would mandate promoting a temporary next hop + * group to a multiple next hops group and we do not have the + * resources yet, we have to skip it until we have enough + * resources. + */ + if (nhg_ptr->isTemp() && + (gRouteOrch->getNhgCount() + NextHopGroup::getCount() >= gRouteOrch->getMaxNhgCount())) + { + /* + * If the group was updated in such way that the previously + * chosen next hop does not represent the new group key, + * update the temporary group to choose a new next hop from + * the new key. Otherwise, this will be a no-op as we have + * to wait for resources in order to promote the group. + */ + if (!nhg_key.contains(nhg_ptr->getKey())) + { + try + { + /* Create the new temporary next hop group. */ + auto new_nhg = std::make_unique(createTempNhg(nhg_key)); + + /* + * If we successfully sync the new group, update + * only the next hop group entry's pointer so we + * don't mess up the reference counter, as other + * objects may already reference it. + */ + if (new_nhg->sync()) + { + nhg_it->second.nhg = std::move(new_nhg); + } + else + { + SWSS_LOG_INFO("Failed to sync updated temp NHG %s with %s", + index.c_str(), + nhg_key.to_string().c_str()); + } + } + catch (const std::exception& e) + { + SWSS_LOG_INFO("Got exception: %s while adding temp group %s", + e.what(), + nhg_key.to_string().c_str()); + } + } + } + /* + * If the group is temporary but can now be promoted, create and sync a new group for + * the desired next hops. + */ + else if (nhg_ptr->isTemp()) + { + auto nhg = std::make_unique(nhg_key, false); + success = nhg->sync(); + + if (success) + { + /* + * Placing the new group in the map will replace the temporary group, causing + * it to be removed and freed. + */ + nhg_it->second.nhg = std::move(nhg); + } + } + /* Common update, when all the requirements are met. */ + else + { + success = nhg_ptr->update(nhg_key); + } + } + } + else if (op == DEL_COMMAND) + { + /* If the group does not exist, do nothing. */ + if (nhg_it == m_syncdNextHopGroups.end()) + { + SWSS_LOG_INFO("Unable to find group with key %s to remove", index.c_str()); + /* Mark the operation as successful to consume it. */ + success = true; + } + /* If the group does exist, but it's still referenced, skip. */ + else if (nhg_it->second.ref_count > 0) + { + SWSS_LOG_INFO("Unable to remove group %s which is referenced", index.c_str()); + } + /* Else, if the group is no more referenced, remove it. */ + else + { + const auto& nhg = nhg_it->second.nhg; + + success = nhg->remove(); + + if (success) + { + m_syncdNextHopGroups.erase(nhg_it); + } + } + } + else + { + SWSS_LOG_ERROR("Unknown operation type %s\n", op.c_str()); + /* Mark the operation as successful to consume it. */ + success = true; + } + + /* Depending on the operation success, consume it or skip it. */ + if (success) + { + it = consumer.m_toSync.erase(it); + } + else + { + ++it; + } + } +} + +/* + * Purpose: Validate a next hop for any groups that contains it. + * Description: Iterate over all next hop groups and validate the next hop in + * those who contain it. + * Params: IN nh_key - The next hop to validate. + * Returns: true, if the next hop was successfully validated in all + * containing groups; + * false, otherwise. + */ +bool NhgOrch::validateNextHop(const NextHopKey& nh_key) +{ + SWSS_LOG_ENTER(); + + /* + * Iterate through all groups and validate the next hop in those who + * contain it. + */ + for (auto& it : m_syncdNextHopGroups) + { + auto& nhg = it.second.nhg; + + if (nhg->hasNextHop(nh_key)) + { + /* + * If sync fails, exit right away, as we expect it to be due to a + * raeson for which any other future validations will fail too. + */ + if (!nhg->validateNextHop(nh_key)) + { + SWSS_LOG_ERROR("Failed to validate next hop %s in group %s", + nh_key.to_string().c_str(), + it.first.c_str()); + return false; + } + } + } + + return true; +} + +/* + * Purpose: Invalidate a next hop for any groups containing it. + * Description: Iterate through the next hop groups and remove the next hop + * from those that contain it. + * Params: IN nh_key - The next hop to invalidate. + * Returns: true, if the next hop was successfully invalidatedd from all + * containing groups; + * false, otherwise. + */ +bool NhgOrch::invalidateNextHop(const NextHopKey& nh_key) +{ + SWSS_LOG_ENTER(); + + /* + * Iterate through all groups and invalidate the next hop from those who + * contain it. + */ + for (auto& it : m_syncdNextHopGroups) + { + auto& nhg = it.second.nhg; + + if (nhg->hasNextHop(nh_key)) + { + /* If the remove fails, exit right away. */ + if (!nhg->invalidateNextHop(nh_key)) + { + SWSS_LOG_WARN("Failed to invalidate next hop %s from group %s", + nh_key.to_string().c_str(), + it.first.c_str()); + return false; + } + } + } + + return true; +} + +/* + * Purpose: Increase the ref count for a next hop group. + * Description: Increment the ref count for a next hop group by 1. + * Params: IN index - The index of the next hop group. + * Returns: Nothing. + */ +void NhgOrch::incNhgRefCount(const std::string& index) +{ + SWSS_LOG_ENTER(); + + NhgEntry& nhg_entry = m_syncdNextHopGroups.at(index); + ++nhg_entry.ref_count; +} + +/* + * Purpose: Decrease the ref count for a next hop group. + * Description: Decrement the ref count for a next hop group by 1. + * Params: IN index - The index of the next hop group. + * Returns: Nothing. + */ +void NhgOrch::decNhgRefCount(const std::string& index) +{ + SWSS_LOG_ENTER(); + + NhgEntry& nhg_entry = m_syncdNextHopGroups.at(index); + + /* Sanity check so we don't overflow. */ + assert(nhg_entry.ref_count > 0); + --nhg_entry.ref_count; +} + +/* + * Purpose: Get the next hop ID of the member. + * Description: Get the SAI ID of the next hop from NeighOrch. + * Params: None. + * Returns: The SAI ID of the next hop, or SAI_NULL_OBJECT_ID if the next + * hop is not valid. + */ +sai_object_id_t NextHopGroupMember::getNhId() const +{ + SWSS_LOG_ENTER(); + + sai_object_id_t nh_id = SAI_NULL_OBJECT_ID; + + if (gNeighOrch->hasNextHop(m_nh_key)) + { + nh_id = gNeighOrch->getNextHopId(m_nh_key); + } + /* + * If the next hop is labeled and the IP next hop exists, create the + * labeled one over NeighOrch as it doesn't know about these next hops. + * We don't do this in the constructor as the IP next hop may be added + * after the object is created and would never create the labeled next hop + * afterwards. + */ + else if (isLabeled() && gNeighOrch->isNeighborResolved(m_nh_key)) + { + gNeighOrch->addNextHop(m_nh_key); + nh_id = gNeighOrch->getNextHopId(m_nh_key); + } + else + { + gNeighOrch->resolveNeighbor(m_nh_key); + } + + return nh_id; +} + +/* + * Purpose: Move assignment operator. + * Description: Perform member-wise swap. + * Params: IN nhgm - The next hop group member to swap. + * Returns: Reference to this object. + */ +NextHopGroupMember& NextHopGroupMember::operator=(NextHopGroupMember&& nhgm) +{ + SWSS_LOG_ENTER(); + + std::swap(m_nh_key, nhgm.m_nh_key); + std::swap(m_gm_id, nhgm.m_gm_id); + + return *this; +} + +/* + * Purpose: Update the weight of a member. + * Description: Set the new member's weight and if the member is synced, update + * the SAI attribute as well. + * Params: IN weight - The weight of the next hop group member. + * Returns: true, if the operation was successful; + * false, otherwise. + */ +bool NextHopGroupMember::updateWeight(uint32_t weight) +{ + SWSS_LOG_ENTER(); + + bool success = true; + + m_nh_key.weight = weight; + + if (isSynced()) + { + sai_attribute_t nhgm_attr; + nhgm_attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT; + nhgm_attr.value.s32 = m_nh_key.weight; + + sai_status_t status = sai_next_hop_group_api->set_next_hop_group_member_attribute(m_gm_id, &nhgm_attr); + success = status == SAI_STATUS_SUCCESS; + } + + return success; +} + +/* + * Purpose: Sync the group member with the given group member ID. + * Description: Set the group member's SAI ID to the the one given and + * increment the appropriate ref counters. + * Params: IN gm_id - The group member SAI ID to set. + * Returns: Nothing. + */ +void NextHopGroupMember::sync(sai_object_id_t gm_id) +{ + SWSS_LOG_ENTER(); + + /* The SAI ID should be updated from invalid to something valid. */ + assert((m_gm_id == SAI_NULL_OBJECT_ID) && (gm_id != SAI_NULL_OBJECT_ID)); + + m_gm_id = gm_id; + gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP_MEMBER); + gNeighOrch->increaseNextHopRefCount(m_nh_key); +} + +/* + * Purpose: Remove the group member, resetting it's SAI ID. + * Description: Reset the group member's SAI ID and decrement the appropriate + * ref counters. + * Params: None. + * Returns: Nothing. + */ +void NextHopGroupMember::remove() +{ + SWSS_LOG_ENTER(); + + /* + * If the member is already removed, exit so we don't decrement the ref + * counters more than once. + */ + if (!isSynced()) + { + return; + } + + m_gm_id = SAI_NULL_OBJECT_ID; + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP_MEMBER); + gNeighOrch->decreaseNextHopRefCount(m_nh_key); +} + +/* + * Purpose: Destructor. + * Description: Assert the group member is removed and remove the labeled + * next hop from NeighOrch if it is unreferenced. + * Params: None. + * Returns: Nothing. + */ +NextHopGroupMember::~NextHopGroupMember() +{ + SWSS_LOG_ENTER(); + + /* + * The group member should be removed from its group before destroying it. + */ + assert(!isSynced()); + + /* + * If the labeled next hop is unreferenced, remove it from NeighOrch as + * NhgOrch and RouteOrch are the ones controlling it's lifetime. They both + * watch over these labeled next hops, so it doesn't matter who created + * them as they're both doing the same checks before removing a labeled + * next hop. + */ + if (isLabeled() && + gNeighOrch->hasNextHop(m_nh_key) && + (gNeighOrch->getNextHopRefCount(m_nh_key) == 0)) + { + gNeighOrch->removeMplsNextHop(m_nh_key); + } +} + +/* + * Purpose: Constructor. + * Description: Initialize the group's members based on the next hop group key. + * Params: IN key - The next hop group's key. + * Returns: Nothing. + */ +NextHopGroup::NextHopGroup(const NextHopGroupKey& key, bool is_temp) : + m_key(key), + m_id(SAI_NULL_OBJECT_ID), + m_is_temp(is_temp) +{ + SWSS_LOG_ENTER(); + + /* Parse the key and create the members. */ + for (const auto& it : m_key.getNextHops()) + { + m_members.emplace(it, NextHopGroupMember(it)); + } +} + +/* + * Purpose: Move constructor. + * Description: Initialize the members by doing member-wise move construct. + * Params: IN nhg - The rvalue object to initialize from. + * Returns: Nothing. + */ +NextHopGroup::NextHopGroup(NextHopGroup&& nhg) : + m_key(std::move(nhg.m_key)), + m_id(std::move(nhg.m_id)), + m_members(std::move(nhg.m_members)), + m_is_temp(nhg.m_is_temp) +{ + SWSS_LOG_ENTER(); + + /* Invalidate the rvalue SAI ID. */ + nhg.m_id = SAI_NULL_OBJECT_ID; +} + +/* + * Purpose: Move assignment operator. + * Description: Perform member-wise swap. + * Params: IN nhg - The rvalue object to swap with. + * Returns: Referene to this object. + */ +NextHopGroup& NextHopGroup::operator=(NextHopGroup&& nhg) +{ + SWSS_LOG_ENTER(); + + std::swap(m_key, nhg.m_key); + std::swap(m_id, nhg.m_id); + std::swap(m_members, nhg.m_members); + m_is_temp = nhg.m_is_temp; + + return *this; +} + +/* + * Purpose: Sync a next hop group. + * Description: Fill in the NHG ID. If the group contains only one NH, this ID + * will be the SAI ID of the next hop that NeighOrch owns. If it + * has more than one NH, create a group over the SAI API and then + * add it's members. + * Params: None. + * Returns: true, if the operation was successful; + * false, otherwise. + */ +bool NextHopGroup::sync() +{ + SWSS_LOG_ENTER(); + + /* If the group is already synced, exit. */ + if (isSynced()) + { + return true; + } + + /* + * If the group is temporary, the group ID will be the only member's NH + * ID. + */ + if (m_is_temp) + { + const NextHopGroupMember& nhgm = m_members.begin()->second; + sai_object_id_t nhid = nhgm.getNhId(); + + if (nhid == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_WARN("Next hop %s is not synced", nhgm.getNhKey().to_string().c_str()); + return false; + } + else + { + m_id = nhid; + } + } + else + { + /* Assert the group is not empty. */ + assert(!m_members.empty()); + + /* Create the group over SAI. */ + sai_attribute_t nhg_attr; + vector nhg_attrs; + + nhg_attr.id = SAI_NEXT_HOP_GROUP_ATTR_TYPE; + nhg_attr.value.s32 = SAI_NEXT_HOP_GROUP_TYPE_ECMP; + nhg_attrs.push_back(nhg_attr); + + sai_status_t status = sai_next_hop_group_api->create_next_hop_group( + &m_id, + gSwitchId, + (uint32_t)nhg_attrs.size(), + nhg_attrs.data()); + + /* If the operation fails, exit. */ + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to create next hop group %s, rv:%d", + m_key.to_string().c_str(), status); + + task_process_status handle_status = gNhgOrch->handleSaiCreateStatus(SAI_API_NEXT_HOP_GROUP, status); + if (handle_status != task_success) + { + return gNhgOrch->parseHandleSaiStatusFailure(handle_status); + } + } + + /* Increment the amount of programmed next hop groups. */ + gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP); + + /* Increment the number of synced NHGs. */ + ++m_count; + + /* + * Try creating the next hop group's members over SAI. + */ + if (!syncMembers(m_key.getNextHops())) + { + SWSS_LOG_WARN("Failed to create next hop members of group %s", + to_string().c_str()); + return false; + } + } + + return true; +} + +/* + * Purpose: Create a temporary next hop group when resources are exhausted. + * Description: Choose one member to represent the group and create a group + * with only that next hop as a member. Any object referencing + * the SAI ID of a temporary group should keep querying NhgOrch in + * case the group is updated, as it's SAI ID will change at that + * point. + * Params: IN index - The CP index of the next hop group. + * Returns: The created temporary next hop group. + */ +NextHopGroup NhgOrch::createTempNhg(const NextHopGroupKey& nhg_key) +{ + SWSS_LOG_ENTER(); + + /* Get a list of all valid next hops in the group. */ + std::list valid_nhs; + + for (const auto& nh_key : nhg_key.getNextHops()) + { + /* + * Check if the IP next hop exists. We check for the IP next hop as + * the group might contain labeled NHs which we should create if their + * IP next hop does exist. + */ + if (gNeighOrch->isNeighborResolved(nh_key)) + { + valid_nhs.push_back(nh_key); + } + } + + /* If there is no valid member, exit. */ + if (valid_nhs.empty()) + { + SWSS_LOG_INFO("There is no valid NH to sync temporary group %s", + nhg_key.to_string().c_str()); + throw std::logic_error("No valid NH in the key"); + } + + /* Randomly select the valid NH to represent the group. */ + auto it = valid_nhs.begin(); + advance(it, rand() % valid_nhs.size()); + + /* Create the temporary group. */ + NextHopGroup nhg(NextHopGroupKey(it->to_string()), true); + + return nhg; +} + +/* + * Purpose: Remove the next hop group. + * Description: Reset the group's SAI ID. If the group has more than one + * members, remove the members and the group. + * Params: None. + * Returns: true, if the operation was successful; + * false, otherwise + */ +bool NextHopGroup::remove() +{ + SWSS_LOG_ENTER(); + + /* If the group is already removed, or is temporary, there is nothing to be done - + * just reset the ID. + */ + if (!isSynced() || m_is_temp) + { + m_id = SAI_NULL_OBJECT_ID; + return true; + } + + /* Remove group's members. If we failed to remove the members, exit. */ + if (!removeMembers(m_key.getNextHops())) + { + SWSS_LOG_ERROR("Failed to remove group %s members", to_string().c_str()); + return false; + } + + /* Remove the group. */ + sai_status_t status = sai_next_hop_group_api-> + remove_next_hop_group(m_id); + + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to remove next hop group %s, rv: %d", + m_key.to_string().c_str(), status); + + task_process_status handle_status = gNhgOrch->handleSaiRemoveStatus(SAI_API_NEXT_HOP_GROUP, status); + if (handle_status != task_success) + { + return gNhgOrch->parseHandleSaiStatusFailure(handle_status); + } + } + + /* If the operation is successful, release the resources. */ + gCrmOrch->decCrmResUsedCounter( + CrmResourceType::CRM_NEXTHOP_GROUP); + --m_count; + + /* Reset the group ID. */ + m_id = SAI_NULL_OBJECT_ID; + + return true; +} + +/* + * Purpose: Sync the given next hop group's members over the SAI API. + * Description: Iterate over the given members and sync them. If the member + * is already synced, we skip it. If any of the next hops isn't + * already synced by the neighOrch, this will fail. Any next hop + * which has the neighbor interface down will be skipped. + * Params: IN nh_keys - The next hop keys of the members to sync. + * Returns: true, if the members were added succesfully; + * false, otherwise. + */ +bool NextHopGroup::syncMembers(const std::set& nh_keys) +{ + SWSS_LOG_ENTER(); + + ObjectBulker nextHopGroupMemberBulker(sai_next_hop_group_api, gSwitchId, gMaxBulkSize); + + /* + * Iterate over the given next hops. + * If the group member is already synced, skip it. + * If any next hop is not synced, thus neighOrch doesn't have it, stop + * immediately. + * If a next hop's interface is down, skip it from being synced. + */ + std::map syncingMembers; + + for (const auto& nh_key : nh_keys) + { + NextHopGroupMember& nhgm = m_members.at(nh_key); + + /* If the member is already synced, continue. */ + if (nhgm.getGmId() != SAI_NULL_OBJECT_ID) + { + continue; + } + + /* + * If the next hop doesn't exist, stop from syncing the members. + */ + if (nhgm.getNhId() == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_WARN("Failed to get next hop %s in group %s", + nhgm.to_string().c_str(), to_string().c_str()); + return false; + } + + /* If the neighbor's interface is down, skip from being syncd. */ + if (gNeighOrch->isNextHopFlagSet(nh_key, NHFLAGS_IFDOWN)) + { + SWSS_LOG_WARN("Skip next hop %s in group %s, interface is down", + nh_key.to_string().c_str(), to_string().c_str()); + continue; + } + + /* Create the next hop group member's attributes and fill them. */ + vector nhgm_attrs = createNhgmAttrs(nhgm); + + /* Add a bulker entry for this member. */ + nextHopGroupMemberBulker.create_entry(&syncingMembers[nh_key], + (uint32_t)nhgm_attrs.size(), + nhgm_attrs.data()); + } + + /* Flush the bulker to perform the sync. */ + nextHopGroupMemberBulker.flush(); + + /* + * Go through the synced members and increment the Crm ref count for the + * successful ones. + */ + bool success = true; + for (const auto& mbr : syncingMembers) + { + /* Check that the returned member ID is valid. */ + if (mbr.second == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_ERROR("Failed to create next hop group %s's member %s", + m_key.to_string().c_str(), mbr.first.to_string().c_str()); + success = false; + } + else + { + m_members.at(mbr.first).sync(mbr.second); + } + } + + return success; +} +/* + * Purpose: Remove the given group's members over the SAI API. + * Description: Go through the given members and remove them. + * Params: IN nh_keys - The next hop keys of the members to remove. + * Returns: true, if the operation was successful; + * false, otherwise + */ +bool NextHopGroup::removeMembers(const std::set& nh_keys) +{ + SWSS_LOG_ENTER(); + + ObjectBulker nextHopGroupMemberBulker( + sai_next_hop_group_api, gSwitchId, gMaxBulkSize); + + /* + * Iterate through the given group members add them to be removed. + * + * Keep track of the SAI remove statuses in case one of them returns an + * error. We assume that removal should always succeed. If for some + * reason it doesn't, there's nothing we can do, but we'll log an error + * later. + */ + std::map statuses; + + for (const auto& nh_key : nh_keys) + { + const NextHopGroupMember& nhgm = m_members.at(nh_key); + + if (nhgm.isSynced()) + { + nextHopGroupMemberBulker.remove_entry(&statuses[nh_key], nhgm.getGmId()); + } + } + + /* Flush the bulker to apply the changes. */ + nextHopGroupMemberBulker.flush(); + + /* + * Iterate over the statuses map and check if the removal was successful. + * If it was, decrement the Crm counter and reset the member's ID. If it + * wasn't, log an error message. + */ + bool success = true; + + for (const auto& status : statuses) + { + if (status.second == SAI_STATUS_SUCCESS) + { + m_members.at(status.first).remove(); + } + else + { + SWSS_LOG_ERROR("Could not remove next hop group member %s, rv: %d", + status.first.to_string().c_str(), status.second); + success = false; + } + } + + return success; +} + +/* + * Purpose: Update the next hop group based on a new next hop group key. + * Description: Update the group's members by removing the members that aren't + * in the new next hop group and adding the new members. We first + * remove the missing members to avoid cases where we reached the + * ASIC group members limit. This will not update the group's SAI + * ID in any way, unless we are promoting a temporary group. + * Params: IN nhg_key - The new next hop group key to update to. + * Returns: true, if the operation was successful; + * false, otherwise. + */ +bool NextHopGroup::update(const NextHopGroupKey& nhg_key) +{ + SWSS_LOG_ENTER(); + + /* Update the key. */ + m_key = nhg_key; + + std::set new_nh_keys = nhg_key.getNextHops(); + std::set removed_nh_keys; + + /* Mark the members that need to be removed. */ + for (auto& mbr_it : m_members) + { + const NextHopKey& nh_key = mbr_it.first; + + /* Look for the existing member inside the new ones. */ + const auto& new_nh_key_it = new_nh_keys.find(nh_key); + + /* If the member is not found, then it needs to be removed. */ + if (new_nh_key_it == new_nh_keys.end()) + { + removed_nh_keys.insert(nh_key); + } + /* If the member is updated, update it's weight. */ + else + { + if (!mbr_it.second.updateWeight(new_nh_key_it->weight)) + { + SWSS_LOG_WARN("Failed to update member %s weight", nh_key.to_string().c_str()); + return false; + } + + /* + * Erase the member from the new members list as it already + * exists. + */ + new_nh_keys.erase(new_nh_key_it); + } + } + + /* Remove the removed members. */ + if (!removeMembers(removed_nh_keys)) + { + SWSS_LOG_WARN("Failed to remove members from group %s", to_string().c_str()); + return false; + } + + /* Remove the removed members. */ + for (const auto& nh_key : removed_nh_keys) + { + m_members.erase(nh_key); + } + + /* Add any new members to the group. */ + for (const auto& it : new_nh_keys) + { + m_members.emplace(it, NextHopGroupMember(it)); + } + + /* + * Sync all the members of the group. We sync all of them because + * there may be previous members that were not successfully synced + * before the update, so we must make sure we sync those as well. + */ + if (!syncMembers(m_key.getNextHops())) + { + SWSS_LOG_WARN("Failed to sync new members for group %s", to_string().c_str()); + return false; + } + + return true; +} + +/* + * Purpose: Create the attributes vector for a next hop group member. + * Description: Create the group ID and next hop ID attributes. + * Params: IN nhgm - The next hop group member. + * Returns: The attributes vector for the given next hop. + */ +vector NextHopGroup::createNhgmAttrs(const NextHopGroupMember& nhgm) const +{ + SWSS_LOG_ENTER(); + + vector nhgm_attrs; + sai_attribute_t nhgm_attr; + + /* Fill in the group ID. */ + nhgm_attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID; + nhgm_attr.value.oid = m_id; + nhgm_attrs.push_back(nhgm_attr); + + /* Fill in the next hop ID. */ + nhgm_attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID; + nhgm_attr.value.oid = nhgm.getNhId(); + nhgm_attrs.push_back(nhgm_attr); + + /* Fill in the wright. */ + nhgm_attr.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT; + nhgm_attr.value.s32 = nhgm.getWeight(); + nhgm_attrs.push_back(nhgm_attr); + + return nhgm_attrs; +} + +/* + * Purpose: Validate a next hop in the group. + * Description: Sync the validated next hop group member. + * Params: IN nh_key - The next hop to validate. + * Returns: true, if the operation was successful; + * false, otherwise. + */ +bool NextHopGroup::validateNextHop(const NextHopKey& nh_key) +{ + SWSS_LOG_ENTER(); + + return syncMembers({nh_key}); +} + +/* + * Purpose: Invalidate a next hop in the group. + * Description: Sync the invalidated next hop group member. + * Params: IN nh_key - The next hop to invalidate. + * Returns: true, if the operation was successful; + * false, otherwise. + */ +bool NextHopGroup::invalidateNextHop(const NextHopKey& nh_key) +{ + SWSS_LOG_ENTER(); + + return removeMembers({nh_key}); +} diff --git a/orchagent/nhgorch.h b/orchagent/nhgorch.h new file mode 100644 index 0000000000..ce99ef85e3 --- /dev/null +++ b/orchagent/nhgorch.h @@ -0,0 +1,249 @@ +#pragma once + +#include "orch.h" +#include "nexthopgroupkey.h" + +class NextHopGroupMember +{ +public: + /* Constructors / Assignment operators. */ + NextHopGroupMember(const NextHopKey& nh_key) : + m_nh_key(nh_key), + m_gm_id(SAI_NULL_OBJECT_ID) {} + + NextHopGroupMember(NextHopGroupMember&& nhgm) : + m_nh_key(std::move(nhgm.m_nh_key)), + m_gm_id(nhgm.m_gm_id) + { nhgm.m_gm_id = SAI_NULL_OBJECT_ID; } + + NextHopGroupMember& operator=(NextHopGroupMember&& nhgm); + + /* + * Prevent object copying so we don't end up having multiple objects + * referencing the same SAI objects. + */ + NextHopGroupMember(const NextHopGroupMember&) = delete; + void operator=(const NextHopGroupMember&) = delete; + + /* Destructor. */ + virtual ~NextHopGroupMember(); + + /* Update member's weight and update the SAI attribute as well. */ + bool updateWeight(uint32_t weight); + + /* Sync / Remove. */ + void sync(sai_object_id_t gm_id); + void remove(); + + /* Getters / Setters. */ + inline const NextHopKey& getNhKey() const { return m_nh_key; } + inline uint32_t getWeight() const { return m_nh_key.weight; } + sai_object_id_t getNhId() const; + inline sai_object_id_t getGmId() const { return m_gm_id; } + inline bool isSynced() const { return m_gm_id != SAI_NULL_OBJECT_ID; } + + /* Check if the next hop is labeled. */ + inline bool isLabeled() const { return !m_nh_key.label_stack.empty(); } + + /* Convert member's details to string. */ + std::string to_string() const + { + return m_nh_key.to_string() + ", SAI ID: " + std::to_string(m_gm_id); + } + +private: + /* The key of the next hop of this member. */ + NextHopKey m_nh_key; + + /* The group member SAI ID for this member. */ + sai_object_id_t m_gm_id; +}; + +/* Map indexed by NextHopKey, containing the SAI ID of the group member. */ +typedef std::map NhgMembers; + +/* + * NextHopGroup class representing a next hop group object. + */ +class NextHopGroup +{ +public: + /* Constructors. */ + explicit NextHopGroup(const NextHopGroupKey& key, bool is_temp); + NextHopGroup(NextHopGroup&& nhg); + NextHopGroup& operator=(NextHopGroup&& nhg); + + /* Destructor. */ + virtual ~NextHopGroup() { remove(); } + + /* Sync the group, creating the group's and members SAI IDs. */ + bool sync(); + + /* Remove the group, reseting the group's and members SAI IDs. */ + bool remove(); + + /* + * Update the group based on a new next hop group key. This will also + * perform any sync / remove necessary. + */ + bool update(const NextHopGroupKey& nhg_key); + + /* Check if the group contains the given next hop. */ + inline bool hasNextHop(const NextHopKey& nh_key) const + { + return m_members.find(nh_key) != m_members.end(); + } + + /* Validate a next hop in the group, syncing it. */ + bool validateNextHop(const NextHopKey& nh_key); + + /* Invalidate a next hop in the group, removing it. */ + bool invalidateNextHop(const NextHopKey& nh_key); + + /* Increment the number of existing groups. */ + static inline void incCount() { ++m_count; } + + /* Decrement the number of existing groups. */ + static inline void decCount() { assert(m_count > 0); --m_count; } + + /* Getters / Setters. */ + inline const NextHopGroupKey& getKey() const { return m_key; } + inline sai_object_id_t getId() const { return m_id; } + static inline unsigned int getCount() { return m_count; } + inline bool isTemp() const { return m_is_temp; } + inline bool isSynced() const { return m_id != SAI_NULL_OBJECT_ID; } + inline size_t getSize() const { return m_members.size(); } + + /* Convert NHG's details to a string. */ + std::string to_string() const + { + return m_key.to_string() + ", SAI ID: " + std::to_string(m_id); + } + +private: + + /* The next hop group key of this group. */ + NextHopGroupKey m_key; + + /* The SAI ID of the group. */ + sai_object_id_t m_id; + + /* Members of this next hop group. */ + NhgMembers m_members; + + /* Whether the group is temporary or not. */ + bool m_is_temp; + + /* + * Number of existing groups. Incremented when an object is created and + * decremented when an object is destroyed. This will also account for the + * groups created by RouteOrch. + */ + static unsigned int m_count; + + /* Add group's members over the SAI API for the given keys. */ + bool syncMembers(const std::set& nh_keys); + + /* Remove group's members the SAI API from the given keys. */ + bool removeMembers(const std::set& nh_keys); + + /* Create the attributes vector for a next hop group member. */ + vector createNhgmAttrs(const NextHopGroupMember& nhgm) const; +}; + +/* + * Structure describing a next hop group which NhgOrch owns. Beside having a + * unique pointer to that next hop group, we also want to keep a ref count so + * NhgOrch knows how many other objects reference the next hop group in order + * not to remove them while still being referenced. + */ +struct NhgEntry +{ + /* Pointer to the next hop group. NhgOrch is the sole owner of it. */ + std::unique_ptr nhg; + + /* Number of external objects referencing this next hop group. */ + unsigned int ref_count; + + NhgEntry() = default; + explicit NhgEntry(std::unique_ptr&& _nhg, + unsigned int _ref_count = 0) : + nhg(std::move(_nhg)), ref_count(_ref_count) {} +}; + +/* + * Map indexed by next hop group's CP ID, containing the next hop group for + * that ID and the number of objects referencing it. + */ +typedef std::unordered_map NhgTable; + +/* + * Next Hop Group Orchestrator class that handles NEXTHOP_GROUP_TABLE + * updates. + */ +class NhgOrch : public Orch +{ +public: + /* + * Constructor. + */ + NhgOrch(DBConnector *db, string tableName); + + /* Check if the next hop group given by it's index exists. */ + inline bool hasNhg(const std::string& index) const + { + return m_syncdNextHopGroups.find(index) != m_syncdNextHopGroups.end(); + } + + /* + * Get the next hop group given by it's index. If the index does not exist + * in map, a std::out_of_range exception will be thrown. + */ + inline const NextHopGroup& getNhg(const std::string& index) const + { return *m_syncdNextHopGroups.at(index).nhg; } + + /* Add a temporary next hop group when resources are exhausted. */ + NextHopGroup createTempNhg(const NextHopGroupKey& nhg_key); + + /* Getters / Setters. */ + inline unsigned int getMaxNhgCount() const { return m_maxNhgCount; } + static inline unsigned int getNhgCount() { return NextHopGroup::getCount(); } + + /* Validate / Invalidate a next hop. */ + bool validateNextHop(const NextHopKey& nh_key); + bool invalidateNextHop(const NextHopKey& nh_key); + + /* Increase / Decrease the number of next hop groups. */ + inline void incNhgCount() + { + assert(NextHopGroup::getCount() < m_maxNhgCount); + NextHopGroup::incCount(); + } + inline void decNhgCount() { NextHopGroup::decCount(); } + + /* Increase / Decrease ref count for a NHG given by it's index. */ + void incNhgRefCount(const std::string& index); + void decNhgRefCount(const std::string& index); + + /* Handling SAI status*/ + task_process_status handleSaiCreateStatus(sai_api_t api, sai_status_t status, void *context = nullptr) + { return Orch::handleSaiCreateStatus(api, status, context); } + task_process_status handleSaiRemoveStatus(sai_api_t api, sai_status_t status, void *context = nullptr) + { return Orch::handleSaiRemoveStatus(api, status, context); } + bool parseHandleSaiStatusFailure(task_process_status status) + { return Orch::parseHandleSaiStatusFailure(status); } + +private: + + /* + * Switch's maximum number of next hop groups capacity. + */ + unsigned int m_maxNhgCount; + + /* + * The next hop group table. + */ + NhgTable m_syncdNextHopGroups; + + void doTask(Consumer& consumer); +}; diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 7883b9058e..cacf8672f4 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -31,6 +31,7 @@ FdbOrch *gFdbOrch; IntfsOrch *gIntfsOrch; NeighOrch *gNeighOrch; RouteOrch *gRouteOrch; +NhgOrch *gNhgOrch; FgNhgOrch *gFgNhgOrch; AclOrch *gAclOrch; PbhOrch *gPbhOrch; @@ -164,6 +165,7 @@ bool OrchDaemon::init() { APP_LABEL_ROUTE_TABLE_NAME, routeorch_pri } }; gRouteOrch = new RouteOrch(m_applDb, route_tables, gSwitchOrch, gNeighOrch, gIntfsOrch, vrf_orch, gFgNhgOrch); + gNhgOrch = new NhgOrch(m_applDb, APP_NEXTHOP_GROUP_TABLE_NAME); CoppOrch *copp_orch = new CoppOrch(m_applDb, APP_COPP_TABLE_NAME); TunnelDecapOrch *tunnel_decap_orch = new TunnelDecapOrch(m_applDb, APP_TUNNEL_DECAP_TABLE_NAME); @@ -288,7 +290,7 @@ bool OrchDaemon::init() }; gMacsecOrch = new MACsecOrch(m_applDb, m_stateDb, macsec_app_tables, gPortsOrch); - + /* * The order of the orch list is important for state restore of warm start and * the queued processing in m_toSync map after gPortsOrch->allPortsReady() is set. @@ -297,7 +299,7 @@ bool OrchDaemon::init() * when iterating ConsumerMap. This is ensured implicitly by the order of keys in ordered map. * For cases when Orch has to process tables in specific order, like PortsOrch during warm start, it has to override Orch::doTask() */ - m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, gIntfsOrch, gNeighOrch, gRouteOrch, copp_orch, tunnel_decap_orch, qos_orch, wm_orch, policer_orch, sflow_orch, debug_counter_orch, gMacsecOrch}; + m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, gIntfsOrch, gNeighOrch, gNhgOrch, gRouteOrch, copp_orch, tunnel_decap_orch, qos_orch, wm_orch, policer_orch, sflow_orch, debug_counter_orch, gMacsecOrch}; bool initialize_dtel = false; if (platform == BFN_PLATFORM_SUBSTRING || platform == VS_PLATFORM_SUBSTRING) diff --git a/orchagent/orchdaemon.h b/orchagent/orchdaemon.h index 460d9052a5..1d1d9f77cf 100644 --- a/orchagent/orchdaemon.h +++ b/orchagent/orchdaemon.h @@ -11,6 +11,7 @@ #include "intfsorch.h" #include "neighorch.h" #include "routeorch.h" +#include "nhgorch.h" #include "copporch.h" #include "tunneldecaporch.h" #include "qosorch.h" diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index 9e9bbe9891..80b2b1e571 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -2,6 +2,7 @@ #include #include #include "routeorch.h" +#include "nhgorch.h" #include "logger.h" #include "swssnet.h" #include "crmorch.h" @@ -18,6 +19,7 @@ extern sai_switch_api_t* sai_switch_api; extern PortsOrch *gPortsOrch; extern CrmOrch *gCrmOrch; extern Directory gDirectory; +extern NhgOrch *gNhgOrch; extern size_t gMaxBulkSize; @@ -96,7 +98,7 @@ RouteOrch::RouteOrch(DBConnector *db, vector &tableNames, gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_IPV4_ROUTE); /* Add default IPv4 route into the m_syncdRoutes */ - m_syncdRoutes[gVirtualRouterId][default_ip_prefix] = NextHopGroupKey(); + m_syncdRoutes[gVirtualRouterId][default_ip_prefix] = RouteNhg(); SWSS_LOG_NOTICE("Create IPv4 default route with packet action drop"); @@ -115,7 +117,7 @@ RouteOrch::RouteOrch(DBConnector *db, vector &tableNames, gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_IPV6_ROUTE); /* Add default IPv6 route into the m_syncdRoutes */ - m_syncdRoutes[gVirtualRouterId][v6_default_ip_prefix] = NextHopGroupKey(); + m_syncdRoutes[gVirtualRouterId][v6_default_ip_prefix] = RouteNhg(); SWSS_LOG_NOTICE("Create IPv6 default route with packet action drop"); @@ -277,7 +279,7 @@ void RouteOrch::attach(Observer *observer, const IpAddress& dstAddr, sai_object_ SWSS_LOG_NOTICE("Attached next hop observer of route %s for destination IP %s", observerEntry->second.routeTable.rbegin()->first.to_string().c_str(), dstAddr.to_string().c_str()); - NextHopUpdate update = { vrf_id, dstAddr, route->first, route->second }; + NextHopUpdate update = { vrf_id, dstAddr, route->first, route->second.nhg_key }; observer->update(SUBJECT_TYPE_NEXTHOP_CHANGE, static_cast(&update)); } } @@ -555,6 +557,7 @@ void RouteOrch::doTask(Consumer& consumer) string vni_labels; string remote_macs; string weights; + string nhg_index; bool& excp_intfs_flag = ctx.excp_intfs_flag; bool overlay_nh = false; bool blackhole = false; @@ -583,106 +586,151 @@ void RouteOrch::doTask(Consumer& consumer) if (fvField(i) == "weight") weights = fvValue(i); - } - vector ipv = tokenize(ips, ','); - vector alsv = tokenize(aliases, ','); - vector mpls_nhv = tokenize(mpls_nhs, ','); - vector vni_labelv = tokenize(vni_labels, ','); - vector rmacv = tokenize(remote_macs, ','); + if (fvField(i) == "nexthop_group") + nhg_index = fvValue(i); + } /* - * For backward compatibility, adjust ip string from old format to - * new format. Meanwhile it can deal with some abnormal cases. + * A route should not fill both nexthop_group and ips / + * aliases. */ - - /* Resize the ip vector to match ifname vector - * as tokenize(",", ',') will miss the last empty segment. */ - if (alsv.size() == 0 && !blackhole) + if (!nhg_index.empty() && (!ips.empty() || !aliases.empty())) { - SWSS_LOG_WARN("Skip the route %s, for it has an empty ifname field.", key.c_str()); + SWSS_LOG_ERROR("Route %s has both nexthop_group and ips/aliases", key.c_str()); it = consumer.m_toSync.erase(it); continue; } - else if (alsv.size() != ipv.size()) - { - SWSS_LOG_NOTICE("Route %s: resize ipv to match alsv, %zd -> %zd.", key.c_str(), ipv.size(), alsv.size()); - ipv.resize(alsv.size()); - } - /* Set the empty ip(s) to zero - * as IpAddress("") will construct a incorrect ip. */ - for (auto &ip : ipv) + ctx.nhg_index = nhg_index; + + /* + * If the nexthop_group is empty, create the next hop group key + * based on the IPs and aliases. Otherwise, get the key from + * the NhgOrch. + */ + vector ipv; + vector alsv; + vector mpls_nhv; + vector vni_labelv; + vector rmacv; + NextHopGroupKey& nhg = ctx.nhg; + + /* Check if the next hop group is owned by the NhgOrch. */ + if (nhg_index.empty()) { - if (ip.empty()) + ipv = tokenize(ips, ','); + alsv = tokenize(aliases, ','); + mpls_nhv = tokenize(mpls_nhs, ','); + vni_labelv = tokenize(vni_labels, ','); + rmacv = tokenize(remote_macs, ','); + + /* + * For backward compatibility, adjust ip string from old format to + * new format. Meanwhile it can deal with some abnormal cases. + */ + + /* Resize the ip vector to match ifname vector + * as tokenize(",", ',') will miss the last empty segment. */ + if (alsv.size() == 0 && !blackhole) { - SWSS_LOG_NOTICE("Route %s: set the empty nexthop ip to zero.", key.c_str()); - ip = ip_prefix.isV4() ? "0.0.0.0" : "::"; + SWSS_LOG_WARN("Skip the route %s, for it has an empty ifname field.", key.c_str()); + it = consumer.m_toSync.erase(it); + continue; + } + else if (alsv.size() != ipv.size()) + { + SWSS_LOG_NOTICE("Route %s: resize ipv to match alsv, %zd -> %zd.", key.c_str(), ipv.size(), alsv.size()); + ipv.resize(alsv.size()); } - } - for (auto alias : alsv) - { - /* skip route to management, docker, loopback - * TODO: for route to loopback interface, the proper - * way is to create loopback interface and then create - * route pointing to it, so that we can traps packets to - * CPU */ - if (alias == "eth0" || alias == "docker0" || - alias == "lo" || !alias.compare(0, strlen(LOOPBACK_PREFIX), LOOPBACK_PREFIX)) + /* Set the empty ip(s) to zero + * as IpAddress("") will construct a incorrect ip. */ + for (auto &ip : ipv) { - excp_intfs_flag = true; - break; + if (ip.empty()) + { + SWSS_LOG_NOTICE("Route %s: set the empty nexthop ip to zero.", key.c_str()); + ip = ip_prefix.isV4() ? "0.0.0.0" : "::"; + } } - } - // TODO: cannot trust m_portsOrch->getPortIdByAlias because sometimes alias is empty - if (excp_intfs_flag) - { - /* If any existing routes are updated to point to the - * above interfaces, remove them from the ASIC. */ - if (removeRoute(ctx)) - it = consumer.m_toSync.erase(it); - else - it++; - continue; - } + for (auto alias : alsv) + { + /* skip route to management, docker, loopback + * TODO: for route to loopback interface, the proper + * way is to create loopback interface and then create + * route pointing to it, so that we can traps packets to + * CPU */ + if (alias == "eth0" || alias == "docker0" || + alias == "lo" || !alias.compare(0, strlen(LOOPBACK_PREFIX), LOOPBACK_PREFIX)) + { + excp_intfs_flag = true; + break; + } + } - string nhg_str = ""; - NextHopGroupKey& nhg = ctx.nhg; + // TODO: cannot trust m_portsOrch->getPortIdByAlias because sometimes alias is empty + if (excp_intfs_flag) + { + /* If any existing routes are updated to point to the + * above interfaces, remove them from the ASIC. */ + if (removeRoute(ctx)) + it = consumer.m_toSync.erase(it); + else + it++; + continue; + } - if (blackhole) - { - nhg = NextHopGroupKey(); - } - else if (overlay_nh == false) - { - for (uint32_t i = 0; i < ipv.size(); i++) + string nhg_str = ""; + + if (blackhole) { - if (i) nhg_str += NHG_DELIMITER; - if (alsv[i] == "tun0" && !(IpAddress(ipv[i]).isZero())) + nhg = NextHopGroupKey(); + } + else if (overlay_nh == false) + { + for (uint32_t i = 0; i < ipv.size(); i++) { - alsv[i] = gIntfsOrch->getRouterIntfsAlias(ipv[i]); + if (i) nhg_str += NHG_DELIMITER; + if (alsv[i] == "tun0" && !(IpAddress(ipv[i]).isZero())) + { + alsv[i] = gIntfsOrch->getRouterIntfsAlias(ipv[i]); + } + if (!mpls_nhv.empty() && mpls_nhv[i] != "na") + { + nhg_str += mpls_nhv[i] + LABELSTACK_DELIMITER; + } + nhg_str += ipv[i] + NH_DELIMITER + alsv[i]; } - if (!mpls_nhv.empty() && mpls_nhv[i] != "na") + + nhg = NextHopGroupKey(nhg_str, weights); + } + else + { + for (uint32_t i = 0; i < ipv.size(); i++) { - nhg_str += mpls_nhv[i] + LABELSTACK_DELIMITER; + if (i) nhg_str += NHG_DELIMITER; + nhg_str += ipv[i] + NH_DELIMITER + "vni" + alsv[i] + NH_DELIMITER + vni_labelv[i] + NH_DELIMITER + rmacv[i]; } - nhg_str += ipv[i] + NH_DELIMITER + alsv[i]; - } - - nhg = NextHopGroupKey(nhg_str, weights); + nhg = NextHopGroupKey(nhg_str, overlay_nh); + } } else { - for (uint32_t i = 0; i < ipv.size(); i++) + try { - if (i) nhg_str += NHG_DELIMITER; - nhg_str += ipv[i] + NH_DELIMITER + "vni" + alsv[i] + NH_DELIMITER + vni_labelv[i] + NH_DELIMITER + rmacv[i]; + const NextHopGroup& nh_group = gNhgOrch->getNhg(nhg_index); + nhg = nh_group.getKey(); + ctx.using_temp_nhg = nh_group.isTemp(); + } + catch (const std::out_of_range& e) + { + SWSS_LOG_ERROR("Next hop group %s does not exist", nhg_index.c_str()); + ++it; + continue; } - - nhg = NextHopGroupKey(nhg_str, overlay_nh); } if (nhg.getSize() == 1 && nhg.hasIntfNextHop()) @@ -720,9 +768,15 @@ void RouteOrch::doTask(Consumer& consumer) it++; } } + /* + * Check if the route does not exist or needs to be updated or + * if the route is using a temporary next hop group owned by + * NhgOrch. + */ else if (m_syncdRoutes.find(vrf_id) == m_syncdRoutes.end() || m_syncdRoutes.at(vrf_id).find(ip_prefix) == m_syncdRoutes.at(vrf_id).end() || - m_syncdRoutes.at(vrf_id).at(ip_prefix) != nhg) + m_syncdRoutes.at(vrf_id).at(ip_prefix) != RouteNhg(nhg, ctx.nhg_index) || + ctx.using_temp_nhg) { if (addRoute(ctx, nhg)) it = consumer.m_toSync.erase(it); @@ -730,12 +784,15 @@ void RouteOrch::doTask(Consumer& consumer) it++; } else + { /* Duplicate entry */ it = consumer.m_toSync.erase(it); + } // If already exhaust the nexthop groups, and there are pending removing routes in bulker, // flush the bulker and possibly collect some released nexthop groups - if (m_nextHopGroupCount >= m_maxNextHopGroupCount && gRouteBulker.removing_entries_count() > 0) + if (m_nextHopGroupCount + gNhgOrch->getNhgCount() >= m_maxNextHopGroupCount && + gRouteBulker.removing_entries_count() > 0) { break; } @@ -809,8 +866,9 @@ void RouteOrch::doTask(Consumer& consumer) it_prev++; } else if (m_syncdRoutes.find(vrf_id) == m_syncdRoutes.end() || - m_syncdRoutes.at(vrf_id).find(ip_prefix) == m_syncdRoutes.at(vrf_id).end() || - m_syncdRoutes.at(vrf_id).at(ip_prefix) != nhg) + m_syncdRoutes.at(vrf_id).find(ip_prefix) == m_syncdRoutes.at(vrf_id).end() || + m_syncdRoutes.at(vrf_id).at(ip_prefix) != RouteNhg(nhg, ctx.nhg_index) || + ctx.using_temp_nhg) { if (addRoutePost(ctx, nhg)) it_prev = consumer.m_toSync.erase(it_prev); @@ -871,13 +929,13 @@ void RouteOrch::notifyNextHopChangeObservers(sai_object_id_t vrf_id, const IpPre update_required = true; } - entry.second.routeTable.emplace(prefix, nexthops); + entry.second.routeTable.emplace(prefix, RouteNhg(nexthops, "")); } else { - if (route->second != nexthops) + if (route->second.nhg_key != nexthops) { - route->second = nexthops; + route->second.nhg_key = nexthops; /* If changed route is best match update observers */ if (entry.second.routeTable.rbegin()->first == route->first) { @@ -908,7 +966,7 @@ void RouteOrch::notifyNextHopChangeObservers(sai_object_id_t vrf_id, const IpPre assert(!entry.second.routeTable.empty()); auto route = entry.second.routeTable.rbegin(); - NextHopUpdate update = { vrf_id, entry.first.second, route->first, route->second }; + NextHopUpdate update = { vrf_id, entry.first.second, route->first, route->second.nhg_key }; for (auto observer : entry.second.observers) { @@ -985,7 +1043,7 @@ const NextHopGroupKey RouteOrch::getSyncdRouteNhgKey(sai_object_id_t vrf_id, con auto route_entry = route_table->second.find(ipPrefix); if (route_entry != route_table->second.end()) { - nhg = route_entry->second; + nhg = route_entry->second.nhg_key; } } return nhg; @@ -995,7 +1053,7 @@ bool RouteOrch::createFineGrainedNextHopGroup(sai_object_id_t &next_hop_group_id { SWSS_LOG_ENTER(); - if (m_nextHopGroupCount >= m_maxNextHopGroupCount) + if (m_nextHopGroupCount + gNhgOrch->getNhgCount() >= m_maxNextHopGroupCount) { SWSS_LOG_DEBUG("Failed to create new next hop group. \ Reaching maximum number of next hop groups."); @@ -1050,7 +1108,7 @@ bool RouteOrch::addNextHopGroup(const NextHopGroupKey &nexthops) assert(!hasNextHopGroup(nexthops)); - if (m_nextHopGroupCount >= m_maxNextHopGroupCount) + if (m_nextHopGroupCount + gNhgOrch->getNhgCount() >= m_maxNextHopGroupCount) { SWSS_LOG_DEBUG("Failed to create new next hop group. \ Reaching maximum number of next hop groups."); @@ -1084,7 +1142,6 @@ bool RouteOrch::addNextHopGroup(const NextHopGroupKey &nexthops) it.to_string().c_str(), nexthops.to_string().c_str()); return false; } - // skip next hop group member create for neighbor from down port if (m_neighOrch->isNextHopFlagSet(it, NHFLAGS_IFDOWN)) { @@ -1126,7 +1183,7 @@ bool RouteOrch::addNextHopGroup(const NextHopGroupKey &nexthops) } } - m_nextHopGroupCount ++; + m_nextHopGroupCount++; SWSS_LOG_NOTICE("Create next hop group %s", nexthops.to_string().c_str()); gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP); @@ -1280,13 +1337,14 @@ bool RouteOrch::removeNextHopGroup(const NextHopGroupKey &nexthops) } } - m_nextHopGroupCount --; + m_nextHopGroupCount--; gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_NEXTHOP_GROUP); set next_hop_set = nexthops.getNextHops(); for (auto it : next_hop_set) { m_neighOrch->decreaseNextHopRefCount(it); + if (overlay_nh && !m_neighOrch->getNextHopRefCount(it)) { if(!m_neighOrch->removeTunnelNextHop(it)) @@ -1421,7 +1479,12 @@ void RouteOrch::addTempRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextH /* Remove next hops that are not in m_syncdNextHops */ for (auto it = next_hop_set.begin(); it != next_hop_set.end();) { - if (!m_neighOrch->hasNextHop(*it)) + /* + * Check if the IP next hop exists in NeighOrch. The next hop may be + * a labeled one, which are created by RouteOrch or NhgOrch if the IP + * next hop exists. + */ + if (!m_neighOrch->isNeighborResolved(*it)) { SWSS_LOG_INFO("Failed to get next hop %s for %s", (*it).to_string().c_str(), ipPrefix.to_string().c_str()); @@ -1488,6 +1551,21 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops) return false; } } + /* NhgOrch owns the NHG */ + else if (!ctx.nhg_index.empty()) + { + try + { + const NextHopGroup& nhg = gNhgOrch->getNhg(ctx.nhg_index); + next_hop_id = nhg.getId(); + } + catch(const std::out_of_range& e) + { + SWSS_LOG_INFO("Next hop group key %s does not exist", ctx.nhg_index.c_str()); + return false; + } + } + /* RouteOrch owns the NHG */ else if (nextHops.getSize() == 0) { /* The route is pointing to a blackhole */ @@ -1525,7 +1603,7 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops) } /* For non-existent MPLS NH, check if IP neighbor NH exists */ else if (nexthop.isMplsNextHop() && - m_neighOrch->hasNextHop(NextHopKey(nexthop.ip_address, nexthop.alias))) + m_neighOrch->isNeighborResolved(nexthop)) { /* since IP neighbor NH exists, neighbor is resolved, add MPLS NH */ m_neighOrch->addNextHop(nexthop); @@ -1603,9 +1681,9 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops) /* If the current next hop is part of the next hop group to sync, * then return false and no need to add another temporary route. */ - if (it_route != m_syncdRoutes.at(vrf_id).end() && it_route->second.getSize() == 1) + if (it_route != m_syncdRoutes.at(vrf_id).end() && it_route->second.nhg_key.getSize() == 1) { - const NextHopKey& nexthop = *it_route->second.getNextHops().begin(); + const NextHopKey& nexthop = *it_route->second.nhg_key.getNextHops().begin(); if (nextHops.contains(nexthop)) { return false; @@ -1665,7 +1743,7 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops) else { /* Set the packet action to forward when there was no next hop (dropped) and not pointing to blackhole*/ - if (it_route->second.getSize() == 0 && !blackhole) + if (it_route->second.nhg_key.getSize() == 0 && !blackhole) { route_attr.id = SAI_ROUTE_ENTRY_ATTR_PACKET_ACTION; route_attr.value.s32 = SAI_PACKET_ACTION_FORWARD; @@ -1676,11 +1754,11 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops) if (curNhgIsFineGrained && !isFineGrainedNextHopIdChanged) { - /* Don't change route entry if the route is previously fine grained and new nhg is also fine grained. + /* Don't change route entry if the route is previously fine grained and new nhg is also fine grained. * We already modifed sai nhg objs as part of setFgNhg to account for nhg change. */ object_statuses.emplace_back(SAI_STATUS_SUCCESS); } - else + else { route_attr.id = SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID; route_attr.value.oid = next_hop_id; @@ -1719,14 +1797,21 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey return false; } - /* next_hop_id indicates the next hop id or next hop group id of this route */ - sai_object_id_t next_hop_id; - if (m_fgNhgOrch->isRouteFineGrained(vrf_id, ipPrefix, nextHops)) { /* Route is pointing to Fine Grained ECMP nexthop group */ isFineGrained = true; } + /* NhgOrch owns the NHG. */ + else if (!ctx.nhg_index.empty()) + { + if (!gNhgOrch->hasNhg(ctx.nhg_index)) + { + SWSS_LOG_INFO("Failed to get next hop group with index %s", ctx.nhg_index.c_str()); + return false; + } + } + /* RouteOrch owns the NHG */ else if (nextHops.getSize() == 0) { /* The route is pointing to a blackhole */ @@ -1738,7 +1823,7 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey const NextHopKey& nexthop = *nextHops.getNextHops().begin(); if (nexthop.isIntfNextHop()) { - next_hop_id = m_intfsOrch->getRouterIntfsId(nexthop.alias); + auto next_hop_id = m_intfsOrch->getRouterIntfsId(nexthop.alias); /* rif is not created yet */ if (next_hop_id == SAI_NULL_OBJECT_ID) { @@ -1799,16 +1884,16 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey else { /* Route already exists */ - auto nh_entry = m_syncdNextHopGroups.find(it_route->second); + auto nh_entry = m_syncdNextHopGroups.find(it_route->second.nhg_key); if (nh_entry != m_syncdNextHopGroups.end()) { /* Case where route was pointing to non-fine grained nhs in the past, * and transitioned to Fine Grained ECMP */ - decreaseNextHopRefCount(it_route->second); - if (it_route->second.getSize() > 1 - && m_syncdNextHopGroups[it_route->second].ref_count == 0) + decreaseNextHopRefCount(it_route->second.nhg_key); + if (it_route->second.nhg_key.getSize() > 1 + && m_syncdNextHopGroups[it_route->second.nhg_key].ref_count == 0) { - m_bulkNhgReducedRefCnt.emplace(it_route->second, 0); + m_bulkNhgReducedRefCnt.emplace(it_route->second.nhg_key, 0); } } SWSS_LOG_INFO("FG Post set route %s with next hop(s) %s", @@ -1822,9 +1907,11 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey { SWSS_LOG_ERROR("Failed to create route %s with next hop(s) %s", ipPrefix.to_string().c_str(), nextHops.to_string().c_str()); - /* Clean up the newly created next hop group entry */ - if (nextHops.getSize() > 1) + + /* Check that the next hop group is not owned by NhgOrch. */ + if (ctx.nhg_index.empty() && nextHops.getSize() > 1) { + /* Clean up the newly created next hop group entry */ removeNextHopGroup(nextHops); } task_process_status handle_status = handleSaiCreateStatus(SAI_API_ROUTE, status); @@ -1843,8 +1930,15 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_IPV6_ROUTE); } - /* Increase the ref_count for the next hop (group) entry */ - increaseNextHopRefCount(nextHops); + /* Increase the ref_count for the next hop group. */ + if (ctx.nhg_index.empty()) + { + increaseNextHopRefCount(nextHops); + } + else + { + gNhgOrch->incNhgRefCount(ctx.nhg_index); + } SWSS_LOG_INFO("Post create route %s with next hop(s) %s", ipPrefix.to_string().c_str(), nextHops.to_string().c_str()); @@ -1854,7 +1948,7 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey sai_status_t status; /* Set the packet action to forward when there was no next hop (dropped) and not pointing to blackhole */ - if (it_route->second.getSize() == 0 && !blackhole) + if (it_route->second.nhg_key.getSize() == 0 && !blackhole) { status = *it_status++; if (status != SAI_STATUS_SUCCESS) @@ -1881,22 +1975,20 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey } } - /* Increase the ref_count for the next hop (group) entry */ - increaseNextHopRefCount(nextHops); - if (m_fgNhgOrch->syncdContainsFgNhg(vrf_id, ipPrefix)) { /* Remove FG nhg since prefix now points to standard nhg/nhs */ m_fgNhgOrch->removeFgNhg(vrf_id, ipPrefix); } - else + /* Decrease the ref count for the previous next hop group. */ + else if (it_route->second.nhg_index.empty()) { - decreaseNextHopRefCount(it_route->second); - auto ol_nextHops = it_route->second; - if (it_route->second.getSize() > 1 - && m_syncdNextHopGroups[it_route->second].ref_count == 0) + decreaseNextHopRefCount(it_route->second.nhg_key); + auto ol_nextHops = it_route->second.nhg_key; + if (ol_nextHops.getSize() > 1 + && m_syncdNextHopGroups[ol_nextHops].ref_count == 0) { - m_bulkNhgReducedRefCnt.emplace(it_route->second, 0); + m_bulkNhgReducedRefCnt.emplace(ol_nextHops, 0); } else if (ol_nextHops.is_overlay_nexthop()) { @@ -1910,6 +2002,11 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey removeNextHopRoute(nexthop, r_key); } } + else + { + /* The next hop group is owned by NhgOrch. */ + gNhgOrch->decNhgRefCount(it_route->second.nhg_index); + } if (blackhole) { @@ -1927,11 +2024,21 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey } } + if (ctx.nhg_index.empty()) + { + /* Increase the ref_count for the next hop (group) entry */ + increaseNextHopRefCount(nextHops); + } + else + { + gNhgOrch->incNhgRefCount(ctx.nhg_index); + } + SWSS_LOG_INFO("Post set route %s with next hop(s) %s", ipPrefix.to_string().c_str(), nextHops.to_string().c_str()); } - if (nextHops.getSize() == 1 && !nextHops.is_overlay_nexthop()) + if (ctx.nhg_index.empty() && nextHops.getSize() == 1 && !nextHops.is_overlay_nexthop()) { RouteKey r_key = { vrf_id, ipPrefix }; auto nexthop = NextHopKey(nextHops.to_string()); @@ -1941,10 +2048,16 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey } } - m_syncdRoutes[vrf_id][ipPrefix] = nextHops; + m_syncdRoutes[vrf_id][ipPrefix] = RouteNhg(nextHops, ctx.nhg_index); notifyNextHopChangeObservers(vrf_id, ipPrefix, nextHops, true); - return true; + + /* + * If the route uses a temporary synced NHG owned by NhgOrch, return false + * in order to keep trying to update the route in case the NHG is updated, + * which will update the SAI ID of the group as well. + */ + return !ctx.using_temp_nhg; } bool RouteOrch::removeRoute(RouteBulkContext& ctx) @@ -2080,19 +2193,25 @@ bool RouteOrch::removeRoutePost(const RouteBulkContext& ctx) /* Delete Fine Grained nhg if the revmoved route pointed to it */ m_fgNhgOrch->removeFgNhg(vrf_id, ipPrefix); } + /* Check if the next hop group is not owned by NhgOrch. */ + else if (!it_route->second.nhg_index.empty()) + { + gNhgOrch->decNhgRefCount(it_route->second.nhg_index); + } + /* The NHG is owned by RouteOrch */ else { /* * Decrease the reference count only when the route is pointing to a next hop. */ - decreaseNextHopRefCount(it_route->second); + decreaseNextHopRefCount(it_route->second.nhg_key); - auto ol_nextHops = it_route->second; + auto ol_nextHops = it_route->second.nhg_key; - if (it_route->second.getSize() > 1 - && m_syncdNextHopGroups[it_route->second].ref_count == 0) + if (it_route->second.nhg_key.getSize() > 1 + && m_syncdNextHopGroups[it_route->second.nhg_key].ref_count == 0) { - m_bulkNhgReducedRefCnt.emplace(it_route->second, 0); + m_bulkNhgReducedRefCnt.emplace(it_route->second.nhg_key, 0); } else if (ol_nextHops.is_overlay_nexthop()) { @@ -2103,9 +2222,9 @@ bool RouteOrch::removeRoutePost(const RouteBulkContext& ctx) * Additionally check if the NH has label and its ref count == 0, then * remove the label next hop. */ - else if (it_route->second.getSize() == 1) + else if (it_route->second.nhg_key.getSize() == 1) { - const NextHopKey& nexthop = *it_route->second.getNextHops().begin(); + const NextHopKey& nexthop = *it_route->second.nhg_key.getNextHops().begin(); if (nexthop.isMplsNextHop() && (m_neighOrch->getNextHopRefCount(nexthop) == 0)) { @@ -2118,14 +2237,14 @@ bool RouteOrch::removeRoutePost(const RouteBulkContext& ctx) } SWSS_LOG_INFO("Remove route %s with next hop(s) %s", - ipPrefix.to_string().c_str(), it_route->second.to_string().c_str()); + ipPrefix.to_string().c_str(), it_route->second.nhg_key.to_string().c_str()); if (ipPrefix.isDefaultRoute() && vrf_id == gVirtualRouterId) { - it_route_table->second[ipPrefix] = NextHopGroupKey(); + it_route_table->second[ipPrefix] = RouteNhg(); /* Notify about default route next hop change */ - notifyNextHopChangeObservers(vrf_id, ipPrefix, it_route_table->second[ipPrefix], true); + notifyNextHopChangeObservers(vrf_id, ipPrefix, it_route_table->second[ipPrefix].nhg_key, true); } else { diff --git a/orchagent/routeorch.h b/orchagent/routeorch.h index 4f74db62dc..d28ba4322e 100644 --- a/orchagent/routeorch.h +++ b/orchagent/routeorch.h @@ -40,6 +40,30 @@ struct NextHopUpdate NextHopGroupKey nexthopGroup; }; +/* + * Structure describing the next hop group used by a route. As the next hop + * groups can either be owned by RouteOrch or by NhgOrch, we have to keep track + * of the next hop group index, as it is the one telling us which one owns it. + */ +struct RouteNhg +{ + NextHopGroupKey nhg_key; + + /* + * Index of the next hop group used. Filled only if referencing a + * NhgOrch's owned next hop group. + */ + std::string nhg_index; + + RouteNhg() = default; + RouteNhg(const NextHopGroupKey& key, const std::string& index) : + nhg_key(key), nhg_index(index) {} + + bool operator==(const RouteNhg& rnhg) + { return ((nhg_key == rnhg.nhg_key) && (nhg_index == rnhg.nhg_index)); } + bool operator!=(const RouteNhg& rnhg) { return !(*this == rnhg); } +}; + struct NextHopObserverEntry; /* Route destination key for a nexthop */ @@ -57,11 +81,11 @@ struct RouteKey /* NextHopGroupTable: NextHopGroupKey, NextHopGroupEntry */ typedef std::map NextHopGroupTable; /* RouteTable: destination network, NextHopGroupKey */ -typedef std::map RouteTable; +typedef std::map RouteTable; /* RouteTables: vrf_id, RouteTable */ typedef std::map RouteTables; /* LabelRouteTable: destination label, next hop address(es) */ -typedef std::map LabelRouteTable; +typedef std::map LabelRouteTable; /* LabelRouteTables: vrf_id, LabelRouteTable */ typedef std::map LabelRouteTables; /* Host: vrf_id, IpAddress */ @@ -82,12 +106,15 @@ struct RouteBulkContext std::deque object_statuses; // Bulk statuses NextHopGroupKey tmp_next_hop; // Temporary next hop NextHopGroupKey nhg; + std::string nhg_index; sai_object_id_t vrf_id; IpPrefix ip_prefix; bool excp_intfs_flag; + // using_temp_nhg will track if the NhgOrch's owned NHG is temporary or not + bool using_temp_nhg; RouteBulkContext() - : excp_intfs_flag(false) + : excp_intfs_flag(false), using_temp_nhg(false) { } @@ -102,6 +129,7 @@ struct RouteBulkContext nhg.clear(); excp_intfs_flag = false; vrf_id = SAI_NULL_OBJECT_ID; + using_temp_nhg = false; } }; @@ -110,13 +138,16 @@ struct LabelRouteBulkContext std::deque object_statuses; // Bulk statuses NextHopGroupKey tmp_next_hop; // Temporary next hop NextHopGroupKey nhg; + std::string nhg_index; sai_object_id_t vrf_id; Label label; bool excp_intfs_flag; uint8_t pop_count; + // using_temp_nhg will track if the NhgOrch's owned NHG is temporary or not + bool using_temp_nhg; LabelRouteBulkContext() - : excp_intfs_flag(false) + : excp_intfs_flag(false), using_temp_nhg(false) { } @@ -172,6 +203,9 @@ class RouteOrch : public Orch, public Subject void delLinkLocalRouteToMe(sai_object_id_t vrf_id, IpPrefix linklocal_prefix); std::string getLinkLocalEui64Addr(void); + unsigned int getNhgCount() { return m_nextHopGroupCount; } + unsigned int getMaxNhgCount() { return m_maxNextHopGroupCount; } + private: SwitchOrch *m_switchOrch; NeighOrch *m_neighOrch; @@ -179,8 +213,8 @@ class RouteOrch : public Orch, public Subject VRFOrch *m_vrfOrch; FgNhgOrch *m_fgNhgOrch; - int m_nextHopGroupCount; - int m_maxNextHopGroupCount; + unsigned int m_nextHopGroupCount; + unsigned int m_maxNextHopGroupCount; bool m_resync; RouteTables m_syncdRoutes; diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am index 5fed8001a4..96f936a622 100644 --- a/tests/mock_tests/Makefile.am +++ b/tests/mock_tests/Makefile.am @@ -39,6 +39,7 @@ tests_SOURCES = aclorch_ut.cpp \ $(top_srcdir)/orchagent/routeorch.cpp \ $(top_srcdir)/orchagent/mplsrouteorch.cpp \ $(top_srcdir)/orchagent/fgnhgorch.cpp \ + $(top_srcdir)/orchagent/nhgorch.cpp \ $(top_srcdir)/orchagent/neighorch.cpp \ $(top_srcdir)/orchagent/intfsorch.cpp \ $(top_srcdir)/orchagent/portsorch.cpp \ diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index 4cd7688883..8d7b3b6623 100644 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -337,7 +337,7 @@ namespace aclorch_test { APP_VXLAN_FDB_TABLE_NAME, FdbOrch::fdborch_pri}, { APP_MCLAG_FDB_TABLE_NAME, fdborch_pri} }; - + TableConnector stateDbFdb(m_state_db.get(), STATE_FDB_TABLE_NAME); TableConnector stateMclagDbFdb(m_state_db.get(), STATE_MCLAG_REMOTE_FDB_TABLE_NAME); ASSERT_EQ(gFdbOrch, nullptr); diff --git a/tests/test_nhg.py b/tests/test_nhg.py index 7a40b2a942..f1b600a22b 100644 --- a/tests/test_nhg.py +++ b/tests/test_nhg.py @@ -8,65 +8,669 @@ from swsscommon import swsscommon +class TestNextHopGroupBase(object): + ASIC_NHS_STR = "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP" + ASIC_NHG_STR = "ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP" + ASIC_NHGM_STR = ASIC_NHG_STR + "_MEMBER" + ASIC_RT_STR = "ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY" + ASIC_INSEG_STR = "ASIC_STATE:SAI_OBJECT_TYPE_INSEG_ENTRY" + + def get_route_id(self, prefix): + for k in self.asic_db.get_keys(self.ASIC_RT_STR): + if json.loads(k)['dest'] == prefix: + return k + + return None + + def get_inseg_id(self, label): + for k in self.asic_db.get_keys(self.ASIC_INSEG_STR): + if json.loads(k)['label'] == label: + return k + + return None + + def get_nhg_id(self, nhg_index): + # Add a route with the given index, then retrieve the next hop group ID + # from that route + asic_rts_count = len(self.asic_db.get_keys(self.ASIC_RT_STR)) + + fvs = swsscommon.FieldValuePairs([('nexthop_group', nhg_index)]) + prefix = '255.255.255.255/24' + ps = swsscommon.ProducerStateTable(self.dvs.get_app_db().db_connection, swsscommon.APP_ROUTE_TABLE_NAME) + ps.set(prefix, fvs) + + # Assert the route is created + try: + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, asic_rts_count + 1) + except Exception as e: + ps._del(prefix) + return None + else: + # Get the route ID for the created route + rt_id = self.get_route_id(prefix) + assert rt_id != None -class TestNextHopGroup(object): - def test_route_nhg(self, dvs, dvs_route, testlog): - config_db = dvs.get_config_db() - fvs = {"NULL": "NULL"} - config_db.create_entry("INTERFACE", "Ethernet0", fvs) - config_db.create_entry("INTERFACE", "Ethernet4", fvs) - config_db.create_entry("INTERFACE", "Ethernet8", fvs) - config_db.create_entry("INTERFACE", "Ethernet0|10.0.0.0/31", fvs) - config_db.create_entry("INTERFACE", "Ethernet4|10.0.0.2/31", fvs) - config_db.create_entry("INTERFACE", "Ethernet8|10.0.0.4/31", fvs) - dvs.runcmd("config interface startup Ethernet0") - dvs.runcmd("config interface startup Ethernet4") - dvs.runcmd("config interface startup Ethernet8") - - dvs.runcmd("arp -s 10.0.0.1 00:00:00:00:00:01") - dvs.runcmd("arp -s 10.0.0.3 00:00:00:00:00:02") - dvs.runcmd("arp -s 10.0.0.5 00:00:00:00:00:03") - - assert dvs.servers[0].runcmd("ip link set down dev eth0") == 0 - assert dvs.servers[1].runcmd("ip link set down dev eth0") == 0 - assert dvs.servers[2].runcmd("ip link set down dev eth0") == 0 - - assert dvs.servers[0].runcmd("ip link set up dev eth0") == 0 - assert dvs.servers[1].runcmd("ip link set up dev eth0") == 0 - assert dvs.servers[2].runcmd("ip link set up dev eth0") == 0 + # Get the NHGID + nhgid = self.asic_db.get_entry(self.ASIC_RT_STR, rt_id)["SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID"] + ps._del(prefix) + self.asic_db.wait_for_deleted_entry(self.ASIC_RT_STR, rt_id) - rtprefix = "2.2.2.0/24" + return nhgid + + def get_nhgm_ids(self, nhg_index): + nhgid = self.get_nhg_id(nhg_index) + nhgms = [] + + for k in self.asic_db.get_keys(self.ASIC_NHGM_STR): + fvs = self.asic_db.get_entry(self.ASIC_NHGM_STR, k) + + # Sometimes some of the NHGMs have no fvs for some reason, so + # we skip those + try: + if fvs['SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID'] == nhgid: + nhgms.append(k) + except KeyError as e: + pass + + return nhgms + + def port_name(self, i): + return "Ethernet" + str(i * 4) + + def port_ip(self, i): + return "10.0.0." + str(i * 2) + + def port_ipprefix(self, i): + return self.port_ip(i) + "/31" + + def peer_ip(self, i): + return "10.0.0." + str(i * 2 + 1) + + def port_mac(self, i): + return "00:00:00:00:00:0" + str(i) + + def config_intf(self, i): + fvs = {'NULL': 'NULL'} + + self.config_db.create_entry("INTERFACE", self.port_name(i), fvs) + self.config_db.create_entry("INTERFACE", "{}|{}".format(self.port_name(i), self.port_ipprefix(i)), fvs) + self.dvs.runcmd("config interface startup " + self.port_name(i)) + self.dvs.runcmd("arp -s {} {}".format(self.peer_ip(i), self.port_mac(i))) + assert self.dvs.servers[i].runcmd("ip link set down dev eth0") == 0 + assert self.dvs.servers[i].runcmd("ip link set up dev eth0") == 0 + + def flap_intf(self, i, status): + assert status in ['up', 'down'] + + self.dvs.servers[i].runcmd("ip link set {} dev eth0".format(status)) == 0 + time.sleep(2) + fvs = self.dvs.get_app_db().get_entry("PORT_TABLE", "Ethernet%d" % (i * 4)) + assert bool(fvs) + assert fvs["oper_status"] == status + + def init_test(self, dvs, num_intfs): + self.dvs = dvs + self.app_db = self.dvs.get_app_db() + self.asic_db = self.dvs.get_asic_db() + self.config_db = self.dvs.get_config_db() + self.nhg_ps = swsscommon.ProducerStateTable(self.app_db.db_connection, swsscommon.APP_NEXTHOP_GROUP_TABLE_NAME) + self.rt_ps = swsscommon.ProducerStateTable(self.app_db.db_connection, swsscommon.APP_ROUTE_TABLE_NAME) + self.lr_ps = swsscommon.ProducerStateTable(self.app_db.db_connection, swsscommon.APP_LABEL_ROUTE_TABLE_NAME) + + for i in range(num_intfs): + self.config_intf(i) + + self.asic_nhgs_count = len(self.asic_db.get_keys(self.ASIC_NHG_STR)) + self.asic_nhgms_count = len(self.asic_db.get_keys(self.ASIC_NHGM_STR)) + self.asic_insgs_count = len(self.asic_db.get_keys(self.ASIC_INSEG_STR)) + self.asic_nhs_count = len(self.asic_db.get_keys(self.ASIC_NHS_STR)) + self.asic_rts_count = len(self.asic_db.get_keys(self.ASIC_RT_STR)) + + def nhg_exists(self, nhg_index): + return self.get_nhg_id(nhg_index) is not None + +class TestNextHopGroupExhaust(TestNextHopGroupBase): + MAX_ECMP_COUNT = 512 + MAX_PORT_COUNT = 10 + + def init_test(self, dvs): + super().init_test(dvs, self.MAX_PORT_COUNT) + self.r = 0 + + def gen_nhg_fvs(self, binary): + nexthop = [] + ifname = [] + + for i in range(self.MAX_PORT_COUNT): + if binary[i] == '1': + nexthop.append(self.peer_ip(i)) + ifname.append(self.port_name(i)) + + nexthop = ','.join(nexthop) + ifname = ','.join(ifname) + fvs = swsscommon.FieldValuePairs([("nexthop", nexthop), ("ifname", ifname)]) + + return fvs + + def gen_valid_binary(self): + while True: + self.r += 1 + binary = self.gen_valid_binary.fmt.format(self.r) + # We need at least 2 ports for a nexthop group + if binary.count('1') <= 1: + continue + return binary + gen_valid_binary.fmt = '{{0:0{}b}}'.format(MAX_PORT_COUNT) + + def test_nhgorch_nhg_exhaust(self, dvs, testlog): + def gen_nhg_index(nhg_number): + return "group{}".format(nhg_number) + + def create_temp_nhg(): + binary = self.gen_valid_binary() + nhg_fvs = self.gen_nhg_fvs(binary) + nhg_index = gen_nhg_index(self.nhg_count) + self.nhg_ps.set(nhg_index, nhg_fvs) + self.nhg_count += 1 + + return nhg_index, binary + + def delete_nhg(): + del_nhg_index = gen_nhg_index(self.first_valid_nhg) + del_nhg_id = self.asic_nhgs[del_nhg_index] + + self.nhg_ps._del(del_nhg_index) + self.asic_nhgs.pop(del_nhg_index) + self.first_valid_nhg += 1 + + return del_nhg_id + + # Test scenario: + # - create a NHG and assert a NHG object doesn't get added to ASIC DB + # - delete a NHG and assert the newly created one is created in ASIC DB and its SAI ID changed + def temporary_group_promotion_test(): + # Add a new next hop group - it should create a temporary one instead + prev_nhgs = self.asic_db.get_keys(self.ASIC_NHG_STR) + nhg_index, _ = create_temp_nhg() + + # Save the temporary NHG's SAI ID + time.sleep(1) + nhg_id = self.get_nhg_id(nhg_index) + + # Assert no new group has been added + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + + # Assert the same NHGs are in ASIC DB + assert prev_nhgs == self.asic_db.get_keys(self.ASIC_NHG_STR) + + # Delete an existing next hop group + del_nhg_id = delete_nhg() + + # Wait for the key to be deleted + self.asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, del_nhg_id) + + # Wait for the temporary group to be promoted and replace the deleted + # NHG + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + + # Assert the SAI ID of the previously temporary NHG has been updated + assert nhg_id != self.get_nhg_id(nhg_index) + + # Save the promoted NHG index/ID + self.asic_nhgs[nhg_index] = self.get_nhg_id(nhg_index) + + # Test scenario: + # - update an existing NHG and assert the update is performed + def group_update_test(): + # Update a group + binary = self.gen_valid_binary() + nhg_fvs = self.gen_nhg_fvs(binary) + nhg_index = gen_nhg_index(self.first_valid_nhg) + + # Save the previous members + prev_nhg_members = self.get_nhgm_ids(nhg_index) + self.nhg_ps.set(nhg_index, nhg_fvs) + + # Wait a second so the NHG members get updated + time.sleep(1) + + # Assert the group was updated by checking it's members + assert self.get_nhgm_ids(nhg_index) != prev_nhg_members + + # Test scenario: + # - create and delete a NHG while the ASIC DB is full and assert nothing changes + def create_delete_temporary_test(): + # Create a new temporary group + nhg_index, _ = create_temp_nhg() + time.sleep(1) + + # Delete the temporary group + self.nhg_ps._del(nhg_index) + + # Assert the NHG does not exist anymore + assert not self.nhg_exists(nhg_index) + + # Assert the number of groups is the same + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + + # Test scenario: + # - create a temporary NHG + # - update the NHG with a different number of members + # - delete a NHG and assert the new one is added and it has the updated number of members + def update_temporary_group_test(): + # Create a new temporary group + nhg_index, binary = create_temp_nhg() + + # Save the number of group members + binary_count = binary.count('1') + + # Update the temporary group with a different number of members + while True: + binary = self.gen_valid_binary() + if binary.count('1') == binary_count: + continue + binary_count = binary.count('1') + break + nhg_fvs = self.gen_nhg_fvs(binary) + self.nhg_ps.set(nhg_index, nhg_fvs) + + # Delete a group + del_nhg_id = delete_nhg() + + # Wait for the group to be deleted + self.asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, del_nhg_id) + + # The temporary group should be promoted + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + + # Save the promoted NHG index/ID + self.asic_nhgs[nhg_index] = self.get_nhg_id(nhg_index) + + # Assert it has the updated details by checking the number of members + assert len(self.get_nhgm_ids(nhg_index)) == binary_count + + # Test scenario: + # - create a route pointing to a NHG and assert it is added + # - create a temporary NHG and update the route to point to it, asserting the route's SAI NHG ID changes + # - update the temporary NHG to contain completely different members and assert the SAI ID changes + # - delete a NHG and assert the temporary NHG is promoted and its SAI ID also changes + def route_nhg_update_test(): + # Add a route + nhg_index = gen_nhg_index(self.first_valid_nhg) + rt_fvs = swsscommon.FieldValuePairs([('nexthop_group', nhg_index)]) + self.rt_ps.set('2.2.2.0/24', rt_fvs) + + # Assert the route is created + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + 1) + + # Save the previous NHG ID + prev_nhg_id = self.asic_nhgs[nhg_index] + + # Create a new temporary group + nhg_index, binary = create_temp_nhg() + + # Get the route ID + rt_id = self.get_route_id('2.2.2.0/24') + assert rt_id != None + + # Update the route to point to the temporary NHG + rt_fvs = swsscommon.FieldValuePairs([('nexthop_group', nhg_index)]) + self.rt_ps.set('2.2.2.0/24', rt_fvs) + + # Wait for the route to change its NHG ID + self.asic_db.wait_for_field_negative_match(self.ASIC_RT_STR, + rt_id, + {'SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID': prev_nhg_id}) + + # Save the new route NHG ID + prev_nhg_id = self.asic_db.get_entry(self.ASIC_RT_STR, rt_id)['SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID'] + + # Update the temporary NHG with one that has different NHs + + # Create a new binary that uses the other interfaces than the previous + # binary was using + new_binary = [] + + for i in range(len(binary)): + if binary[i] == '1': + new_binary.append('0') + else: + new_binary.append('1') + + binary = ''.join(new_binary) + assert binary.count('1') > 1 + + nhg_fvs = self.gen_nhg_fvs(binary) + self.nhg_ps.set(nhg_index, nhg_fvs) + + # The NHG ID of the route should change + self.asic_db.wait_for_field_negative_match(self.ASIC_RT_STR, + rt_id, + {'SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID': prev_nhg_id}) + + # Delete a NHG. + del_nhg_id = delete_nhg() + + # Wait for the NHG to be deleted + self.asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, del_nhg_id) + + # The temporary group should get promoted. + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + + # Save the promoted NHG index/ID + self.asic_nhgs[nhg_index] = self.get_nhg_id(nhg_index) + + # Assert the NHGID of the route changed due to temporary group being + # promoted. + self.asic_db.wait_for_field_match(self.ASIC_RT_STR, + rt_id, + {'SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID': self.asic_nhgs[nhg_index]}) + + # Test scenario: + # - create a temporary NHG containing labeled NHs and assert a new NH is added to represent the group + # - delete a NHG and assert the temporary NHG is promoted and all its NHs are added + def labeled_nhg_temporary_promotion_test(): + # Create a next hop group that contains labeled NHs that do not exist + # in NeighOrch + self.asic_nhs_count = len(self.asic_db.get_keys(self.ASIC_NHS_STR)) + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + ('mpls_nh', 'push1,push3'), + ('ifname', 'Ethernet0,Ethernet4')]) + nhg_index = gen_nhg_index(self.nhg_count) + self.nhg_ps.set(nhg_index, fvs) + self.nhg_count += 1 + + # A temporary next hop should be elected to represent the group and + # thus a new labeled next hop should be created + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count + 1) + + # Delete a next hop group + delete_nhg() + + # The group should be promoted and the other labeled NH should also get + # created + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count + 2) + + # Save the promoted NHG index/ID + self.asic_nhgs[nhg_index] = self.get_nhg_id(nhg_index) + + # Test scenario: + # - update route to own its NHG and assert no new NHG is added + # - remove a NHG and assert the temporary NHG is promoted and added to ASIC DB + def back_compatibility_test(): + # Update the route with a RouteOrch's owned NHG + binary = self.gen_valid_binary() + nhg_fvs = self.gen_nhg_fvs(binary) + self.rt_ps.set('2.2.2.0/24', nhg_fvs) + + # Assert no new group has been added + time.sleep(1) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + + # Delete a next hop group + del_nhg_id = delete_nhg() + self.asic_db.wait_for_deleted_entry(self.ASIC_NHG_STR, del_nhg_id) + + # The temporary group should be promoted + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + + # Test scenario: + # - create a NHG with all NHs not existing and assert the NHG is not created + # - update the NHG to have valid NHs and assert a temporary NHG is created + # - update the NHG to all invalid NHs again and assert the update is not performed and thus it has the same SAI + # ID + # - delete the temporary NHG + def invalid_temporary_test(): + # Create a temporary NHG that contains only NHs that do not exist + nhg_fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.21,10.0.0.23'), + ('ifname', 'Ethernet40,Ethernet44')]) + nhg_index = gen_nhg_index(self.nhg_count) + self.nhg_count += 1 + self.nhg_ps.set(nhg_index, nhg_fvs) + + # Assert the group is not created + time.sleep(1) + assert not self.nhg_exists(nhg_index) + + # Update the temporary NHG to a valid one + binary = self.gen_valid_binary() + nhg_fvs = self.gen_nhg_fvs(binary) + self.nhg_ps.set(nhg_index, nhg_fvs) + + # Assert the temporary group was updated and the group got created + nhg_id = self.get_nhg_id(nhg_index) + assert nhg_id is not None + + # Update the temporary NHG to an invalid one again + nhg_fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.21,10.0.0.23'), + ('ifname', 'Ethernet40,Ethernet44')]) + self.nhg_ps.set(nhg_index, nhg_fvs) + + # The update should fail and the temporary NHG should still be pointing + # to the old valid NH + assert self.get_nhg_id(nhg_index) == nhg_id + + # Delete the temporary group + self.nhg_ps._del(nhg_index) + + self.init_test(dvs) + + self.nhg_count = self.asic_nhgs_count + self.first_valid_nhg = self.nhg_count + self.asic_nhgs = {} - app_db = dvs.get_app_db() - ps = swsscommon.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") + # Add first batch of next hop groups to reach the NHG limit + while self.nhg_count < self.MAX_ECMP_COUNT: + binary = self.gen_valid_binary() + nhg_fvs = self.gen_nhg_fvs(binary) + nhg_index = gen_nhg_index(self.nhg_count) + self.nhg_ps.set(nhg_index, nhg_fvs) - asic_db = dvs.get_asic_db() + # Save the NHG index/ID pair + self.asic_nhgs[nhg_index] = self.get_nhg_id(nhg_index) + + # Increase the number of NHGs in ASIC DB + self.nhg_count += 1 + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + + temporary_group_promotion_test() + group_update_test() + create_delete_temporary_test() + update_temporary_group_test() + route_nhg_update_test() + labeled_nhg_temporary_promotion_test() + back_compatibility_test() + invalid_temporary_test() + + # Cleanup + + # Delete the route + self.rt_ps._del('2.2.2.0/24') + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count) + + # Delete the next hop groups + for k in self.asic_nhgs: + self.nhg_ps._del(k) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count) + + + def test_route_nhg_exhaust(self, dvs, testlog): + """ + Test the situation of exhausting ECMP group, assume SAI_SWITCH_ATTR_NUMBER_OF_ECMP_GROUPS is 512 + + In order to achieve that, we will config + 1. 9 ports + 2. 512 routes with different nexthop group + + See Also + -------- + SwitchStateBase::set_number_of_ecmp_groups() + https://github.com/Azure/sonic-sairedis/blob/master/vslib/src/SwitchStateBase.cpp + + """ + + # TODO: check ECMP 512 + + def gen_ipprefix(r): + """ Construct route like 2.X.X.0/24 """ + ip = ipaddress.IPv4Address(IP_INTEGER_BASE + r * 256) + ip = str(ip) + ipprefix = ip + "/24" + return ipprefix + + def asic_route_nhg_fvs(k): + fvs = self.asic_db.get_entry(self.ASIC_RT_STR, k) + if not fvs: + return None + + nhgid = fvs.get("SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID") + if nhgid is None: + return None + + fvs = self.asic_db.get_entry(self.ASIC_NHG_STR, nhgid) + return fvs + + if sys.version_info < (3, 0): + IP_INTEGER_BASE = int(ipaddress.IPv4Address(unicode("2.2.2.0"))) + else: + IP_INTEGER_BASE = int(ipaddress.IPv4Address(str("2.2.2.0"))) + + self.init_test(dvs) + + # Add first batch of routes with unique nexthop groups in AppDB + route_count = 0 + while route_count < self.MAX_ECMP_COUNT: + binary = self.gen_valid_binary() + fvs = self.gen_nhg_fvs(binary) + route_ipprefix = gen_ipprefix(route_count) + self.rt_ps.set(route_ipprefix, fvs) + route_count += 1 + + # Wait and check ASIC DB the count of nexthop groups used + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + + # Wait and check ASIC DB the count of routes + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + self.MAX_ECMP_COUNT) + self.asic_rts_count += self.MAX_ECMP_COUNT + + # Add a route with labeled NHs + self.asic_nhs_count = len(self.asic_db.get_keys(self.ASIC_NHS_STR)) + route_ipprefix = gen_ipprefix(route_count) + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + ('mpls_nh', 'push1,push3'), + ('ifname', 'Ethernet0,Ethernet4')]) + self.rt_ps.set(route_ipprefix, fvs) + route_count += 1 + + # A temporary route should be created + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + 1) + + # A NH should be elected as the temporary NHG and it should be created + # as it doesn't exist. + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count + 1) + + # Delete the route. The route and the added labeled NH should be + # removed. + self.rt_ps._del(route_ipprefix) + route_count -= 1 + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count) + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count) + + # Add second batch of routes with unique nexthop groups in AppDB + # Add more routes with new nexthop group in AppDBdd + route_ipprefix = gen_ipprefix(route_count) + base_ipprefix = route_ipprefix + base = route_count + route_count = 0 + while route_count < 10: + binary = self.gen_valid_binary() + fvs = self.gen_nhg_fvs(binary) + route_ipprefix = gen_ipprefix(base + route_count) + self.rt_ps.set(route_ipprefix, fvs) + route_count += 1 + last_ipprefix = route_ipprefix + + # Wait until we get expected routes and check ASIC DB on the count of nexthop groups used, and it should not increase + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + 10) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.MAX_ECMP_COUNT) + + # Check the route points to next hop group + # Note: no need to wait here + k = self.get_route_id("2.2.2.0/24") + assert k is not None + fvs = asic_route_nhg_fvs(k) + assert fvs is not None + + # Check the second batch does not point to next hop group + k = self.get_route_id(base_ipprefix) + assert k is not None + fvs = asic_route_nhg_fvs(k) + assert not(fvs) + + # Remove first batch of routes with unique nexthop groups in AppDB + route_count = 0 + self.r = 0 + while route_count < self.MAX_ECMP_COUNT: + route_ipprefix = gen_ipprefix(route_count) + self.rt_ps._del(route_ipprefix) + route_count += 1 + self.asic_rts_count -= self.MAX_ECMP_COUNT + + # Wait and check the second batch points to next hop group + # Check ASIC DB on the count of nexthop groups used, and it should not increase or decrease + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, 10) + k = self.get_route_id(base_ipprefix) + assert k is not None + fvs = asic_route_nhg_fvs(k) + assert fvs is not None + k = self.get_route_id(last_ipprefix) + assert k is not None + fvs = asic_route_nhg_fvs(k) + assert fvs is not None + + # Cleanup + + # Remove second batch of routes + for i in range(10): + route_ipprefix = gen_ipprefix(self.MAX_ECMP_COUNT + i) + self.rt_ps._del(route_ipprefix) + + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, 0) + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count) + +class TestNextHopGroup(TestNextHopGroupBase): + + def test_route_nhg(self, dvs, dvs_route, testlog): + self.init_test(dvs, 3) + + rtprefix = "2.2.2.0/24" dvs_route.check_asicdb_deleted_route_entries([rtprefix]) # nexthop group without weight fvs = swsscommon.FieldValuePairs([("nexthop","10.0.0.1,10.0.0.3,10.0.0.5"), ("ifname", "Ethernet0,Ethernet4,Ethernet8")]) - ps.set(rtprefix, fvs) + self.rt_ps.set(rtprefix, fvs) # check if route was propagated to ASIC DB rtkeys = dvs_route.check_asicdb_route_entries([rtprefix]) # assert the route points to next hop group - fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY", rtkeys[0]) + fvs = self.asic_db.get_entry(self.ASIC_RT_STR, rtkeys[0]) nhgid = fvs["SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID"] - fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP", nhgid) + fvs = self.asic_db.get_entry(self.ASIC_NHG_STR, nhgid) assert bool(fvs) - keys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER") + keys = self.asic_db.get_keys(self.ASIC_NHGM_STR) assert len(keys) == 3 for k in keys: - fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER", k) + fvs = self.asic_db.get_entry(self.ASIC_NHGM_STR, k) assert fvs["SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID"] == nhgid @@ -74,7 +678,7 @@ def test_route_nhg(self, dvs, dvs_route, testlog): assert fvs.get("SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT") is None # Remove route 2.2.2.0/24 - ps._del(rtprefix) + self.rt_ps._del(rtprefix) # Wait for route 2.2.2.0/24 to be removed dvs_route.check_asicdb_deleted_route_entries([rtprefix]) @@ -83,26 +687,26 @@ def test_route_nhg(self, dvs, dvs_route, testlog): fvs = swsscommon.FieldValuePairs([("nexthop","10.0.0.1,10.0.0.3,10.0.0.5"), ("ifname", "Ethernet0,Ethernet4,Ethernet8"), ("weight", "10,30")]) - ps.set(rtprefix, fvs) + self.rt_ps.set(rtprefix, fvs) # check if route was propagated to ASIC DB rtkeys = dvs_route.check_asicdb_route_entries([rtprefix]) # assert the route points to next hop group - fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY", rtkeys[0]) + fvs = self.asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY", rtkeys[0]) nhgid = fvs["SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID"] - fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP", nhgid) + fvs = self.asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP", nhgid) assert bool(fvs) - keys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER") + keys = self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER") assert len(keys) == 3 for k in keys: - fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER", k) + fvs = self.asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER", k) assert fvs["SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID"] == nhgid @@ -110,7 +714,7 @@ def test_route_nhg(self, dvs, dvs_route, testlog): assert fvs.get("SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT") is None # Remove route 2.2.2.0/24 - ps._del(rtprefix) + self.rt_ps._del(rtprefix) # Wait for route 2.2.2.0/24 to be removed dvs_route.check_asicdb_deleted_route_entries([rtprefix]) @@ -118,26 +722,26 @@ def test_route_nhg(self, dvs, dvs_route, testlog): fvs = swsscommon.FieldValuePairs([("nexthop","10.0.0.1,10.0.0.3,10.0.0.5"), ("ifname", "Ethernet0,Ethernet4,Ethernet8"), ("weight", "10,30,50")]) - ps.set(rtprefix, fvs) + self.rt_ps.set(rtprefix, fvs) # check if route was propagated to ASIC DB rtkeys = dvs_route.check_asicdb_route_entries([rtprefix]) # assert the route points to next hop group - fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY", rtkeys[0]) + fvs = self.asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY", rtkeys[0]) nhgid = fvs["SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID"] - fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP", nhgid) + fvs = self.asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP", nhgid) assert bool(fvs) - keys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER") + keys = self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER") assert len(keys) == 3 for k in keys: - fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER", k) + fvs = self.asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER", k) assert fvs["SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID"] == nhgid @@ -145,7 +749,7 @@ def test_route_nhg(self, dvs, dvs_route, testlog): nhid = fvs["SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID"] weight = fvs["SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT"] - fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP", nhid) + fvs = self.asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP", nhid) nhip = fvs["SAI_NEXT_HOP_ATTR_IP"].split('.') expected_weight = int(nhip[3]) * 10 @@ -156,21 +760,21 @@ def test_route_nhg(self, dvs, dvs_route, testlog): fvs = swsscommon.FieldValuePairs([("nexthop","10.0.0.1,10.0.0.3,10.0.0.5"), ("ifname", "Ethernet0,Ethernet4,Ethernet8"), ("weight", "20,30,40")]) - ps.set(rtprefix2, fvs) + self.rt_ps.set(rtprefix2, fvs) # wait for route to be programmed time.sleep(1) - keys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP") + keys = self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP") assert len(keys) == 2 - keys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER") + keys = self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER") assert len(keys) == 6 # Remove route 3.3.3.0/24 - ps._del(rtprefix2) + self.rt_ps._del(rtprefix2) # Wait for route 3.3.3.0/24 to be removed dvs_route.check_asicdb_deleted_route_entries([rtprefix2]) @@ -178,226 +782,602 @@ def test_route_nhg(self, dvs, dvs_route, testlog): # bring links down one-by-one for i in [0, 1, 2]: - dvs.servers[i].runcmd("ip link set down dev eth0") == 0 - - time.sleep(1) - - fvs = app_db.get_entry("PORT_TABLE", "Ethernet%d" % (i * 4)) - - assert bool(fvs) - assert fvs["oper_status"] == "down" + self.flap_intf(i, 'down') - keys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER") + keys = self.asic_db.get_keys(self.ASIC_NHGM_STR) assert len(keys) == 2 - i # bring links up one-by-one for i in [0, 1, 2]: - dvs.servers[i].runcmd("ip link set up dev eth0") == 0 - - time.sleep(1) - - fvs = app_db.get_entry("PORT_TABLE", "Ethernet%d" % (i * 4)) - - assert bool(fvs) - assert fvs["oper_status"] == "up" + self.flap_intf(i, 'up') - keys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER") + keys = self.asic_db.get_keys(self.ASIC_NHGM_STR) assert len(keys) == i + 1 for k in keys: - fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER", k) + fvs = self.asic_db.get_entry(self.ASIC_NHGM_STR, k) assert fvs["SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID"] == nhgid # Remove route 2.2.2.0/24 - ps._del(rtprefix) + self.rt_ps._del(rtprefix) # Wait for route 2.2.2.0/24 to be removed dvs_route.check_asicdb_deleted_route_entries([rtprefix]) - def test_route_nhg_exhaust(self, dvs, testlog): - """ - Test the situation of exhausting ECMP group, assume SAI_SWITCH_ATTR_NUMBER_OF_ECMP_GROUPS is 512 - - In order to achieve that, we will config - 1. 9 ports - 2. 512 routes with different nexthop group - - See Also - -------- - SwitchStateBase::set_number_of_ecmp_groups() - https://github.com/Azure/sonic-sairedis/blob/master/vslib/src/SwitchStateBase.cpp - - """ - - # TODO: check ECMP 512 - - def port_name(i): - return "Ethernet" + str(i * 4) - - def port_ip(i): - return "10.0.0." + str(i * 2) - - def peer_ip(i): - return "10.0.0." + str(i * 2 + 1) + def test_label_route_nhg(self, dvs, testlog): + self.init_test(dvs, 3) - def port_ipprefix(i): - return port_ip(i) + "/31" + # add label route + fvs = swsscommon.FieldValuePairs([("nexthop","10.0.0.1,10.0.0.3,10.0.0.5"), + ("ifname", "Ethernet0,Ethernet4,Ethernet8")]) + self.lr_ps.set("10", fvs) + self.asic_db.wait_for_n_keys(self.ASIC_INSEG_STR, self.asic_insgs_count + 1) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 1) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 3) - def port_mac(i): - return "00:00:00:00:00:0" + str(i) + k = self.get_inseg_id('10') + assert k is not None - def gen_ipprefix(r): - """ Construct route like 2.X.X.0/24 """ - ip = ipaddress.IPv4Address(IP_INTEGER_BASE + r * 256) - ip = str(ip) - ipprefix = ip + "/24" - return ipprefix + # assert the route points to next hop group + fvs = self.asic_db.get_entry(self.ASIC_INSEG_STR, k) + nhgid = fvs["SAI_INSEG_ENTRY_ATTR_NEXT_HOP_ID"] + fvs = self.asic_db.get_entry(self.ASIC_NHG_STR, nhgid) + assert bool(fvs) - def gen_nhg_fvs(binary): - nexthop = [] - ifname = [] - for i in range(MAX_PORT_COUNT): - if binary[i] == '1': - nexthop.append(peer_ip(i)) - ifname.append(port_name(i)) + keys = self.asic_db.get_keys(self.ASIC_NHGM_STR) + assert len(keys) == 3 + for k in keys: + fvs = self.asic_db.get_entry(self.ASIC_NHGM_STR, k) + assert fvs["SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID"] == nhgid - nexthop = ','.join(nexthop) - ifname = ','.join(ifname) - fvs = swsscommon.FieldValuePairs([("nexthop", nexthop), ("ifname", ifname)]) - return fvs + # bring links down one-by-one + for i in [0, 1, 2]: + self.flap_intf(i, 'down') + keys = self.asic_db.get_keys(self.ASIC_NHGM_STR) + assert len(keys) == 2 - i - def asic_route_exists(keys, ipprefix): + # bring links up one-by-one + for i in [0, 1, 2]: + self.flap_intf(i, 'up') + keys = self.asic_db.get_keys(self.ASIC_NHGM_STR) + assert len(keys) == i + 1 for k in keys: - rt_key = json.loads(k) + fvs = self.asic_db.get_entry(self.ASIC_NHGM_STR, k) + assert fvs["SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID"] == nhgid - if rt_key['dest'] == ipprefix: - return k - else: - return None + # Remove label route 10 + self.lr_ps._del("10") + + # Wait for label route 10 to be removed + self.asic_db.wait_for_n_keys(self.ASIC_INSEG_STR, self.asic_insgs_count) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count) + + def test_nhgorch_labeled_nhs(self, dvs, testlog): + # Test scenario: + # - create a NHG with all labeled and weighted NHs and assert 2 new NHs are created + # - create a NHG with an existing label and assert no new NHs are created + # - create a NHG with a new label and assert a new NH is created + # - remove the third NHG and assert the NH is deleted + # - delete the second group and assert no NH is deleted because it is still referenced by the first group + # - remove the weights from the first NHG and change the labels, leaving one NH unlabeled; assert one NH is + # deleted + # - delete the first NHG and perform cleanup + def mainline_labeled_nhs_test(): + # Add a group containing labeled weighted NHs + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + ('mpls_nh', 'push1,push3'), + ('ifname', 'Ethernet0,Ethernet4'), + ('weight', '2,4')]) + self.nhg_ps.set('group1', fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 1) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 2) + + # NhgOrch should create two next hops for the labeled ones + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count + 2) + + # Assert the weights are properly set + nhgm_ids = self.get_nhgm_ids('group1') + weights = [] + for k in nhgm_ids: + fvs = self.asic_db.get_entry(self.ASIC_NHGM_STR, k) + weights.append(fvs['SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT']) + assert set(weights) == set(['2', '4']) + + # Create a new single next hop with the same label + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1'), + ('mpls_nh', 'push1'), + ('ifname', 'Ethernet0')]) + self.nhg_ps.set('group2', fvs) + + # No new next hop should be added + time.sleep(1) + assert len(self.asic_db.get_keys(self.ASIC_NHS_STR)) == self.asic_nhs_count + 2 - def asic_route_nhg_fvs(k): - fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY", k) - if not fvs: - return None + # Create a new single next hop with a different label + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1'), + ('mpls_nh', 'push2'), + ('ifname', 'Ethernet0')]) + self.nhg_ps.set('group3', fvs) - print(fvs) - nhgid = fvs.get("SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID") - if nhgid is None: - return None + # A new next hop should be added + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count + 3) - fvs = asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP", nhgid) - return fvs + # Delete group3 + self.nhg_ps._del('group3') - MAX_ECMP_COUNT = 512 - MAX_PORT_COUNT = 10 - if sys.version_info < (3, 0): - IP_INTEGER_BASE = int(ipaddress.IPv4Address(unicode("2.2.2.0"))) - else: - IP_INTEGER_BASE = int(ipaddress.IPv4Address(str("2.2.2.0"))) + # Group3's NH should be deleted + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count + 2) - config_db = dvs.get_config_db() - fvs = {"NULL": "NULL"} + # Delete group2 + self.nhg_ps._del('group2') - for i in range(MAX_PORT_COUNT): - config_db.create_entry("INTERFACE", port_name(i), fvs) - config_db.create_entry("INTERFACE", "{}|{}".format(port_name(i), port_ipprefix(i)), fvs) - dvs.runcmd("config interface startup " + port_name(i)) - dvs.runcmd("arp -s {} {}".format(peer_ip(i), port_mac(i))) - assert dvs.servers[i].runcmd("ip link set down dev eth0") == 0 - assert dvs.servers[i].runcmd("ip link set up dev eth0") == 0 + # The number of NHs should be the same as they are still referenced by + # group1 + time.sleep(1) + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count + 2) + + # Update group1 with no weights and both labeled and unlabeled NHs + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + ('mpls_nh', 'push2,na'), + ('ifname', 'Ethernet0,Ethernet4')]) + self.nhg_ps.set('group1', fvs) + + # Group members should be replaced and one NH should get deleted + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 2) + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count + 1) + + # Assert the weights of the NHGMs are the expected ones + nhgm_ids = self.get_nhgm_ids('group1') + weights = [] + for nhgm_id in nhgm_ids: + fvs = self.asic_db.get_entry(self.ASIC_NHGM_STR, nhgm_id) + weights.append(fvs['SAI_NEXT_HOP_GROUP_MEMBER_ATTR_WEIGHT']) + assert weights == ['0', '0'] + + # Delete group1 + self.nhg_ps._del('group1') + + # Wait for the group and it's members to be deleted + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count) + + # The two next hops should also get deleted + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count) + + # Test scenario: + # - create a route with labeled and weighted NHs and assert a NHG and 2 NHs are created + # - create a NHG with the same details as the one being used by the route and assert a NHG is created and no + # new NHs are added + # - update the NHG by changing the first NH's label and assert a new NH is created + # - remove the route and assert that only one (now unreferenced) NH is removed + # - remove the NHG and perform cleanup + def routeorch_nhgorch_interop_test(): + # Create a route with labeled NHs + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + ('mpls_nh', 'push1,push3'), + ('ifname', 'Ethernet0,Ethernet4'), + ('weight', '2,4')]) + self.rt_ps.set('2.2.2.0/24', fvs) + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + 1) + + # A NHG should be created + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 1) + + # Two new next hops should be created + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count + 2) + + # Create a NHG with the same details + self.nhg_ps.set('group1', fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 2) + + # No new next hops should be created + assert len(self.asic_db.get_keys(self.ASIC_NHS_STR)) == self.asic_nhs_count + 2 + + # Update the group with a different NH + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + ('mpls_nh', 'push2,push3'), + ('ifname', 'Ethernet0,Ethernet4'), + ('weight', '2,4')]) + self.nhg_ps.set('group1', fvs) + + # A new next hop should be created + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count + 3) + + # group1 should be updated and a new NHG shouldn't be created + time.sleep(1) + assert len(self.asic_db.get_keys(self.ASIC_NHG_STR)) == self.asic_nhgs_count + 2 - app_db = dvs.get_app_db() - asic_db = dvs.get_asic_db() - ps = swsscommon.ProducerStateTable(app_db.db_connection, "ROUTE_TABLE") + # Remove the route + self.rt_ps._del('2.2.2.0/24') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 1) - # Add first batch of routes with unique nexthop groups in AppDB - route_count = 0 - r = 0 - asic_routes_count = len(asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY")) - while route_count < MAX_ECMP_COUNT: - r += 1 - fmt = '{{0:0{}b}}'.format(MAX_PORT_COUNT) - binary = fmt.format(r) - # We need at least 2 ports for a nexthop group - if binary.count('1') <= 1: - continue - fvs = gen_nhg_fvs(binary) - route_ipprefix = gen_ipprefix(route_count) - ps.set(route_ipprefix, fvs) - route_count += 1 + # One NH should become unreferenced and should be deleted. The other + # one is still referenced by NhgOrch's owned NHG. + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count + 2) - # Wait and check ASIC DB the count of nexthop groups used - asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP", MAX_ECMP_COUNT) + # Remove the group + self.nhg_ps._del('group1') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count) - # Wait and check ASIC DB the count of routes - asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY", asic_routes_count + MAX_ECMP_COUNT) - asic_routes_count += MAX_ECMP_COUNT + # Both new next hops should be deleted + self.asic_db.wait_for_n_keys(self.ASIC_NHS_STR, self.asic_nhs_count) - # Add second batch of routes with unique nexthop groups in AppDB - # Add more routes with new nexthop group in AppDBdd - route_ipprefix = gen_ipprefix(route_count) - base_ipprefix = route_ipprefix - base = route_count - route_count = 0 - while route_count < 10: - r += 1 - fmt = '{{0:0{}b}}'.format(MAX_PORT_COUNT) - binary = fmt.format(r) - # We need at least 2 ports for a nexthop group - if binary.count('1') <= 1: - continue - fvs = gen_nhg_fvs(binary) - route_ipprefix = gen_ipprefix(base + route_count) - ps.set(route_ipprefix, fvs) - route_count += 1 - last_ipprefix = route_ipprefix + self.init_test(dvs, 2) - # Wait until we get expected routes and check ASIC DB on the count of nexthop groups used, and it should not increase - keys = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY", asic_routes_count + 10) - asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP", MAX_ECMP_COUNT) + mainline_labeled_nhs_test() + routeorch_nhgorch_interop_test() - # Check the route points to next hop group - # Note: no need to wait here - k = asic_route_exists(keys, "2.2.2.0/24") - assert k is not None - fvs = asic_route_nhg_fvs(k) - assert fvs is not None + def test_nhgorch_excp_group_cases(self, dvs, testlog): + # Test scenario: + # - remove a NHG that does not exist and assert the number of NHGs in ASIC DB remains the same + def remove_inexistent_nhg_test(): + # Remove a group that does not exist + self.nhg_ps._del("group1") + time.sleep(1) + assert len(self.asic_db.get_keys(self.ASIC_NHG_STR)) == self.asic_nhgs_count + + # Test scenario: + # - create a NHG with a member which does not exist and assert no NHG is created + # - update the NHG to contain all valid members and assert the NHG is created and it has 2 members + def nhg_members_validation_test(): + # Create a next hop group with a member that does not exist - should fail + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3,10.0.0.63'), + ("ifname", "Ethernet0,Ethernet4,Ethernet124")]) + self.nhg_ps.set("group1", fvs) + time.sleep(1) + assert len(self.asic_db.get_keys(self.ASIC_NHG_STR)) == self.asic_nhgs_count + + # Issue an update for this next hop group that doesn't yet exist, + # which contains only valid NHs. This will overwrite the previous + # operation and create the group. + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.5'), + ("ifname", "Ethernet0,Ethernet8")]) + self.nhg_ps.set("group1", fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 1) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 2) + + # Check the group has its two members + assert len(self.get_nhgm_ids('group1')) == 2 + + # Test scenario: + # - create a route pointing to the NHG created in `test_nhg_members_validation` and assert it is being created + # - remove the NHG and assert it fails as it is being referenced + # - create a new NHG and assert it and its members are being created + # - update the route to point to the new NHG and assert the first NHG is now deleted as it's not referenced + # anymore + def remove_referenced_nhg_test(): + # Add a route referencing the new group + fvs = swsscommon.FieldValuePairs([('nexthop_group', 'group1')]) + self.rt_ps.set('2.2.2.0/24', fvs) + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + 1) + + # Try removing the group while it still has references - should fail + self.nhg_ps._del('group1') + time.sleep(1) + assert len(self.asic_db.get_keys(self.ASIC_NHG_STR)) == self.asic_nhgs_count + 1 + + # Create a new group + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + ('ifname', 'Ethernet0,Ethernet4')]) + self.nhg_ps.set("group2", fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 2) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 4) + + # Update the route to point to the new group + fvs = swsscommon.FieldValuePairs([('nexthop_group', 'group2')]) + self.rt_ps.set('2.2.2.0/24', fvs) + + # The first group should have got deleted + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 1) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 2) + + # The route's group should have changed to the new one + assert self.asic_db.get_entry(self.ASIC_RT_STR, self.get_route_id('2.2.2.0/24'))['SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID'] == self.get_nhg_id('group2') + + # Test scenario: + # - update the route created in `test_remove_referenced_nhg` to own the NHG with the same details as the + # previous one and assert a new NHG and 2 new NHGMs are added + # - update the route to point back to the original NHG and assert the routeOrch's owned NHG is deleted + def routeorch_nhgorch_interop_test(): + rt_id = self.get_route_id('2.2.2.0/24') + assert rt_id is not None + + # Update the route with routeOrch's owned next hop group + nhgid = self.asic_db.get_entry(self.ASIC_RT_STR, rt_id)['SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID'] + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + ('ifname', 'Ethernet0,Ethernet4')]) + self.rt_ps.set('2.2.2.0/24', fvs) + + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 2) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 4) + + # Assert the next hop group ID changed + time.sleep(1) + assert self.asic_db.get_entry(self.ASIC_RT_STR, rt_id)['SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID'] != nhgid + nhgid = self.asic_db.get_entry(self.ASIC_RT_STR, rt_id)['SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID'] + + # Update the route to point back to group2 + fvs = swsscommon.FieldValuePairs([('nexthop_group', 'group2')]) + self.rt_ps.set('2.2.2.0/24', fvs) + + # The routeOrch's owned next hop group should get deleted + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 1) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 2) + + # Assert the route points back to group2 + assert self.asic_db.get_entry(self.ASIC_RT_STR, rt_id)['SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID'] != nhgid + + # Test scenario: + # - create a new NHG with the same details as the previous NHG and assert a new NHG and 2 new NHGMs are created + # - update the route to point to the new NHG and assert its SAI NHG ID changes + def identical_nhgs_test(): + rt_id = self.get_route_id('2.2.2.0/24') + assert rt_id is not None + + # Create a new group with the same members as group2 + nhgid = self.asic_db.get_entry(self.ASIC_RT_STR, rt_id)['SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID'] + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + ('ifname', 'Ethernet0,Ethernet4')]) + self.nhg_ps.set("group1", fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 2) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 4) + + # Update the route to point to the new group + fvs = swsscommon.FieldValuePairs([('nexthop_group', 'group1')]) + self.rt_ps.set('2.2.2.0/24', fvs) + time.sleep(1) - # Check the second batch does not point to next hop group - k = asic_route_exists(keys, base_ipprefix) + # Assert the next hop group ID changed + assert self.asic_db.get_entry(self.ASIC_RT_STR, rt_id)['SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID'] != nhgid + + # Test scenario: + # - create a route referencing a NHG that does not exist and assert it is not created + def create_route_inexistent_nhg_test(): + # Add a route with a NHG that does not exist + fvs = swsscommon.FieldValuePairs([('nexthop_group', 'group3')]) + self.rt_ps.set('2.2.3.0/24', fvs) + time.sleep(1) + assert self.get_route_id('2.2.3.0/24') is None + + # Remove the pending route + self.rt_ps._del('2.2.3.0/24') + + self.init_test(dvs, 3) + + remove_inexistent_nhg_test() + nhg_members_validation_test() + remove_referenced_nhg_test() + routeorch_nhgorch_interop_test() + identical_nhgs_test() + create_route_inexistent_nhg_test() + + # Cleanup + + # Remove the route + self.rt_ps._del('2.2.2.0/24') + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count) + + # Remove the groups + self.nhg_ps._del('group1') + self.nhg_ps._del('group2') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count) + + def test_nhgorch_nh_group(self, dvs, testlog): + # Test scenario: + # - create NHG 'group1' and assert it is being added to ASIC DB along with its members + def create_nhg_test(): + # create next hop group in APPL DB + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3,10.0.0.5'), + ("ifname", "Ethernet0,Ethernet4,Ethernet8")]) + self.nhg_ps.set("group1", fvs) + + # check if group was propagated to ASIC DB + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 1) + assert self.nhg_exists('group1') + + # check if members were propagated to ASIC DB + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 3) + assert len(self.get_nhgm_ids('group1')) == 3 + + # Test scenario: + # - create a route pointing to `group1` and assert it is being added to ASIC DB and pointing to its SAI ID + # - delete the route and assert it is being removed + def create_route_nhg_test(): + # create route in APPL DB + fvs = swsscommon.FieldValuePairs([("nexthop_group", "group1")]) + self.rt_ps.set("2.2.2.0/24", fvs) + + # check if route was propagated to ASIC DB + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count + 1) + + k = self.get_route_id('2.2.2.0/24') + assert k is not None + + # assert the route points to next hop group + fvs = self.asic_db.get_entry(self.ASIC_RT_STR, k) + assert fvs["SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID"] == self.get_nhg_id('group1') + + # Remove route 2.2.2.0/24 + self.rt_ps._del("2.2.2.0/24") + self.asic_db.wait_for_n_keys(self.ASIC_RT_STR, self.asic_rts_count) + + # Test scenario: + # - bring the links down one by one and assert the group1's members are subsequently removed and the group + # still exists + # - bring the liks up one by one and assert the group1's members are subsequently added back + def link_flap_test(): + # bring links down one-by-one + for i in [0, 1, 2]: + self.flap_intf(i, 'down') + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 2 - i) + assert len(self.get_nhgm_ids('group1')) == 2 - i + assert self.nhg_exists('group1') + + # bring links up one-by-one + for i in [0, 1, 2]: + self.flap_intf(i, 'up') + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + i + 1) + assert len(self.get_nhgm_ids('group1')) == i + 1 + + # Test scenario: + # - bring a link down and assert a NHGM of `group1` is removed + # - create NHG `group2` which has a member pointing to the link being down and assert the group gets created + # but the member referencing the link is not added + # - update `group1` by removing a member while having another member referencing the link which is down and + # assert it'll only have a member added in ASIC DB + # - bring the link back up and assert the missing 2 members of `group1` and `group2` are added + # - remove `group2` and assert it and its members are removed + def validate_invalidate_group_member_test(): + # Bring an interface down + self.flap_intf(1, 'down') + + # One group member will get deleted + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 2) + + # Create a group that contains a NH that uses the down link + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), + ("ifname", "Ethernet0,Ethernet4")]) + self.nhg_ps.set('group2', fvs) + + # The group should get created, but it will not contained the NH that + # has the link down + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 2) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 3) + assert len(self.get_nhgm_ids('group2')) == 1 + + # Update the NHG with one interface down + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.3,10.0.0.1'), + ("ifname", "Ethernet4,Ethernet0")]) + self.nhg_ps.set("group1", fvs) + + # Wait for group members to update - the group will contain only the + # members that have their links up + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 2) + assert len(self.get_nhgm_ids('group1')) == 1 + + # Bring the interface up + self.flap_intf(1, 'up') + + # Check that the missing member of group1 and group2 is being added + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 4) + + # Remove group2 + self.nhg_ps._del('group2') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 1) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 2) + + # Test scenario: + # - create NHG `group2` with a NH that does not exist and assert it isn't created + # - update `group1` to contain the invalid NH and assert it remains only with the unremoved members + # - configure the invalid NH's interface and assert `group2` gets created and `group1`'s NH is added + # - delete `group` and assert it is being removed + def inexistent_group_member_test(): + # Create group2 with a NH that does not exist + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.3,10.0.0.63'), + ("ifname", "Ethernet4,Ethernet124")]) + self.nhg_ps.set("group2", fvs) + + # The groups should not be created + time.sleep(1) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 1) + + # Update group1 with a NH that does not exist + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.3,10.0.0.63'), + ("ifname", "Ethernet4,Ethernet124")]) + self.nhg_ps.set("group1", fvs) + + # The update should fail, leaving group1 with only the unremoved + # members + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 1) + assert len(self.get_nhgm_ids('group1')) == 1 + + # Configure the missing NH's interface + self.config_intf(31) + + # A couple more routes will be added to ASIC DB + self.asic_rts_count += 2 + + # Group2 should get created and group1 should be updated + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 2) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 4) + assert len(self.get_nhgm_ids('group1')) == 2 + assert len(self.get_nhgm_ids('group2')) == 2 + + # Delete group2 + self.nhg_ps._del('group2') + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 1) + + # Test scenario: + # - update `group1` to have 4 members and assert they are all added + # - update `group1` to have only 1 member and assert the other 3 are removed + # - update `group1` to have 2 members and assert a new one is added + def update_nhgm_count_test(): + # Update the NHG, adding two new members + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3,10.0.0.5,10.0.0.7'), + ("ifname", "Ethernet0,Ethernet4,Ethernet8,Ethernet12")]) + self.nhg_ps.set("group1", fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 4) + assert len(self.get_nhgm_ids('group1')) == 4 + + # Update the group to one NH only + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1'), ("ifname", "Ethernet0")]) + self.nhg_ps.set("group1", fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 1) + assert len(self.get_nhgm_ids('group1')) == 1 + + # Update the group to 2 NHs + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3'), ("ifname", "Ethernet0,Ethernet4")]) + self.nhg_ps.set("group1", fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 2) + assert len(self.get_nhgm_ids('group1')) == 2 + + self.init_test(dvs, 4) + + create_nhg_test() + create_route_nhg_test() + link_flap_test() + validate_invalidate_group_member_test() + inexistent_group_member_test() + update_nhgm_count_test() + + # Cleanup + + # Remove group1 + self.nhg_ps._del("group1") + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count) + + def test_nhgorch_label_route(self, dvs, testlog): + self.init_test(dvs, 4) + + # create next hop group in APPL DB + fvs = swsscommon.FieldValuePairs([('nexthop', '10.0.0.1,10.0.0.3,10.0.0.5'), + ("ifname", "Ethernet0,Ethernet4,Ethernet8")]) + self.nhg_ps.set("group1", fvs) + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count + 1) + self.asic_db.wait_for_n_keys(self.ASIC_NHGM_STR, self.asic_nhgms_count + 3) + + # create label route in APPL DB pointing to the NHG + fvs = swsscommon.FieldValuePairs([("nexthop_group", "group1")]) + self.lr_ps.set("20", fvs) + self.asic_db.wait_for_n_keys(self.ASIC_INSEG_STR, self.asic_insgs_count + 1) + + k = self.get_inseg_id('20') assert k is not None - fvs = asic_route_nhg_fvs(k) - assert not(fvs) - # Remove first batch of routes with unique nexthop groups in AppDB - route_count = 0 - r = 0 - while route_count < MAX_ECMP_COUNT: - r += 1 - fmt = '{{0:0{}b}}'.format(MAX_PORT_COUNT) - binary = fmt.format(r) - # We need at least 2 ports for a nexthop group - if binary.count('1') <= 1: - continue - route_ipprefix = gen_ipprefix(route_count) - ps._del(route_ipprefix) - route_count += 1 + # assert the route points to next hop group + fvs = self.asic_db.get_entry(self.ASIC_INSEG_STR, k) + assert fvs["SAI_INSEG_ENTRY_ATTR_NEXT_HOP_ID"] == self.get_nhg_id('group1') - # Wait and check the second batch points to next hop group - # Check ASIC DB on the count of nexthop groups used, and it should not increase or decrease - asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_NEXT_HOP_GROUP", 10) - keys = asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY") - k = asic_route_exists(keys, base_ipprefix) - assert k is not None - fvs = asic_route_nhg_fvs(k) - assert fvs is not None - k = asic_route_exists(keys, last_ipprefix) - assert k is not None - fvs = asic_route_nhg_fvs(k) - assert fvs is not None + # Remove label route 20 + self.lr_ps._del("20") + self.asic_db.wait_for_n_keys(self.ASIC_INSEG_STR, self.asic_insgs_count) + # Remove group1 + self.nhg_ps._del("group1") + self.asic_db.wait_for_n_keys(self.ASIC_NHG_STR, self.asic_nhgs_count) # Add Dummy always-pass test at end as workaroud # for issue when Flaky fail on final test it invokes module tear-down before retrying