diff --git a/doc/swss-schema.md b/doc/swss-schema.md index 3ccc7b74afe7..4da3cb3b2a86 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 94d66e00245a..ce56e8438ec8 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 6db46e01a57a..1bf37e711c6a 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 e3f9a4175e97..23d7a38b3b0b 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 3b5867a949b7..7a9a1fa330e2 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 87c294415a26..1e76916dd428 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 000000000000..4b2084e82afd --- /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 000000000000..ce99ef85e3ac --- /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 7883b9058ea9..cacf8672f49e 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 460d9052a558..1d1d9f77cfd1 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 9e9bbe9891e4..80b2b1e571c4 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 4f74db62dcd7..d28ba4322e3b 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 5fed8001a4e0..96f936a6224d 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 4cd76888837f..8d7b3b662319 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 7a40b2a94212..f1b600a22b3a 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