From d4db44e2df25a1c26bdcf314ef732cf2722d6690 Mon Sep 17 00:00:00 2001 From: Nikola Dancejic Date: Mon, 13 May 2024 22:57:22 +0000 Subject: [PATCH] [muxorch] Using bulker to program routes/neighbors during switchover Uses entity bulker to program routes and neighbors during mux switchover. Mux switchover performance suffers when switching over with a large number of neighbors on the mux port. This uses the optimization of programming the neighbors and routes in bulk to avoid sequentially programming each. Signed-off-by: Nikola Dancejic --- orchagent/muxorch.cpp | 206 ++++++++++++++-- orchagent/muxorch.h | 28 ++- orchagent/neighorch.cpp | 519 ++++++++++++++++++++++++++++++++++++++++ orchagent/neighorch.h | 23 ++ tests/test_mux.py | 61 +++++ 5 files changed, 817 insertions(+), 20 deletions(-) diff --git a/orchagent/muxorch.cpp b/orchagent/muxorch.cpp index ea3ade347c..5c529a309d 100644 --- a/orchagent/muxorch.cpp +++ b/orchagent/muxorch.cpp @@ -744,6 +744,8 @@ void MuxNbrHandler::update(NextHopKey nh, sai_object_id_t tunnelId, bool add, Mu bool MuxNbrHandler::enable(bool update_rt) { NeighborEntry neigh; + std::list bulk_neigh_ctx_list; + std::list bulk_route_ctx_list; auto it = neighbors_.begin(); while (it != neighbors_.end()) @@ -751,12 +753,18 @@ bool MuxNbrHandler::enable(bool update_rt) SWSS_LOG_INFO("Enabling neigh %s on %s", it->first.to_string().c_str(), alias_.c_str()); neigh = NeighborEntry(it->first, alias_); - if (!gNeighOrch->enableNeighbor(neigh)) - { - SWSS_LOG_INFO("Enabling neigh failed for %s", neigh.ip_address.to_string().c_str()); - return false; - } + bulk_neigh_ctx_list.push_back(NeighborBulkContext(neigh, true)); + it++; + } + + if (!gNeighOrch->createBulkNeighborEntries(bulk_neigh_ctx_list) || !gNeighOrch->flushBulkNeighborEntries(bulk_neigh_ctx_list)) + { + return false; + } + it = neighbors_.begin(); + while (it != neighbors_.end()) + { /* Update NH to point to learned neighbor */ it->second = gNeighOrch->getLocalNextHopId(neigh); @@ -795,22 +803,48 @@ bool MuxNbrHandler::enable(bool update_rt) IpPrefix pfx = it->first.to_string(); if (update_rt) { - if (remove_route(pfx) != SAI_STATUS_SUCCESS) - { - return false; - } - updateTunnelRoute(nh_key, false); + bulk_route_ctx_list.push_back(MuxRouteBulkContext(pfx, false)); } it++; } + if (update_rt) + { + if (!createBulkRouteEntries(bulk_route_ctx_list)) + { + gRouteBulker.clear(); + return false; + } + gRouteBulker.flush(); + if (!processBulkRouteEntries(bulk_route_ctx_list)) + { + gRouteBulker.clear(); + return false; + } + + it = neighbors_.begin(); + while (it != neighbors_.end()) + { + NextHopKey nh_key = NextHopKey(it->first, alias_); + if (update_rt) + { + updateTunnelRoute(nh_key, false); + } + + it++; + } + } + + gRouteBulker.clear(); return true; } bool MuxNbrHandler::disable(sai_object_id_t tnh) { NeighborEntry neigh; + std::list bulk_neigh_ctx_list; + std::list bulk_route_ctx_list; auto it = neighbors_.begin(); while (it != neighbors_.end()) @@ -852,21 +886,32 @@ bool MuxNbrHandler::disable(sai_object_id_t tnh) updateTunnelRoute(nh_key, true); IpPrefix pfx = it->first.to_string(); - if (create_route(pfx, it->second) != SAI_STATUS_SUCCESS) - { - return false; - } + bulk_route_ctx_list.push_back(MuxRouteBulkContext(pfx, it->second, true)); neigh = NeighborEntry(it->first, alias_); - if (!gNeighOrch->disableNeighbor(neigh)) - { - SWSS_LOG_INFO("Disabling neigh failed for %s", neigh.ip_address.to_string().c_str()); - return false; - } + bulk_neigh_ctx_list.push_back(NeighborBulkContext(neigh, false)); it++; } + if (!createBulkRouteEntries(bulk_route_ctx_list)) + { + gRouteBulker.clear(); + return false; + } + gRouteBulker.flush(); + if (!processBulkRouteEntries(bulk_route_ctx_list)) + { + gRouteBulker.clear(); + return false; + } + + if (!gNeighOrch->createBulkNeighborEntries(bulk_neigh_ctx_list) || !gNeighOrch->flushBulkNeighborEntries(bulk_neigh_ctx_list)) + { + return false; + } + + gRouteBulker.clear(); return true; } @@ -881,6 +926,129 @@ sai_object_id_t MuxNbrHandler::getNextHopId(const NextHopKey nhKey) return SAI_NULL_OBJECT_ID; } +bool MuxNbrHandler::createBulkRouteEntries(std::list& bulk_ctx_list) +{ + int count = 0; + + SWSS_LOG_INFO("Creating %d bulk route entries", (int)bulk_ctx_list.size()); + + for (auto ctx = bulk_ctx_list.begin(); ctx != bulk_ctx_list.end(); ctx++) + { + auto& object_statuses = ctx->object_statuses; + sai_route_entry_t route_entry; + route_entry.switch_id = gSwitchId; + route_entry.vr_id = gVirtualRouterId; + copy(route_entry.destination, ctx->pfx); + subnet(route_entry.destination, route_entry.destination); + + SWSS_LOG_INFO("Creating route entry %s, nh %" PRIx64 ", add:%d", ctx->pfx.getIp().to_string().c_str(), ctx->nh, ctx->add); + + object_statuses.emplace_back(); + if (ctx->add) + { + sai_attribute_t attr; + vector attrs; + + attr.id = SAI_ROUTE_ENTRY_ATTR_PACKET_ACTION; + attr.value.s32 = SAI_PACKET_ACTION_FORWARD; + attrs.push_back(attr); + + attr.id = SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID; + attr.value.oid = ctx->nh; + attrs.push_back(attr); + + sai_status_t status = gRouteBulker.create_entry(&object_statuses.back(), &route_entry, (uint32_t)attrs.size(), attrs.data()); + if (status == SAI_STATUS_ITEM_ALREADY_EXISTS) + { + SWSS_LOG_ERROR("Failed to create add entry for tunnel route in bulker object, entry already exists %s,nh %" PRIx64, + ctx->pfx.getIp().to_string().c_str(), ctx->nh); + continue; + } + } + else + { + sai_status_t status = gRouteBulker.remove_entry(&object_statuses.back(), &route_entry); + if (status == SAI_STATUS_ITEM_ALREADY_EXISTS) + { + SWSS_LOG_ERROR("Failed to create remove entry for tunnel route in bulker object, entry already exists %s,nh %" PRIx64, + ctx->pfx.getIp().to_string().c_str(), ctx->nh); + continue; + } + } + count++; + } + + SWSS_LOG_INFO("Successfully created %d bulk neighbor entries", count); + return true; +} + +bool MuxNbrHandler::processBulkRouteEntries(std::list& bulk_ctx_list) +{ + for (auto ctx = bulk_ctx_list.begin(); ctx != bulk_ctx_list.end(); ctx++) + { + auto& object_statuses = ctx->object_statuses; + auto it_status = object_statuses.begin(); + sai_status_t status = *it_status++; + + sai_route_entry_t route_entry; + route_entry.switch_id = gSwitchId; + route_entry.vr_id = gVirtualRouterId; + copy(route_entry.destination, ctx->pfx); + subnet(route_entry.destination, route_entry.destination); + + if (ctx->add) + { + if (status != SAI_STATUS_SUCCESS) + { + if (status == SAI_STATUS_ITEM_ALREADY_EXISTS) { + SWSS_LOG_NOTICE("Tunnel route to %s already exists", ctx->pfx.to_string().c_str()); + continue; + } + SWSS_LOG_ERROR("Failed to create tunnel route %s,nh %" PRIx64 " rv:%d", + ctx->pfx.getIp().to_string().c_str(), ctx->nh, status); + return false; + } + + if (route_entry.destination.addr_family == SAI_IP_ADDR_FAMILY_IPV4) + { + gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_IPV4_ROUTE); + } + else + { + gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_IPV6_ROUTE); + } + + SWSS_LOG_NOTICE("Created tunnel route to %s ", ctx->pfx.to_string().c_str()); + } + else + { + if (status != SAI_STATUS_SUCCESS) + { + if (status == SAI_STATUS_ITEM_NOT_FOUND) { + SWSS_LOG_NOTICE("Tunnel route to %s already removed", ctx->pfx.to_string().c_str()); + continue; + } + SWSS_LOG_ERROR("Failed to remove tunnel route %s, rv:%d", + ctx->pfx.getIp().to_string().c_str(), status); + return false; + } + + if (route_entry.destination.addr_family == SAI_IP_ADDR_FAMILY_IPV4) + { + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV4_ROUTE); + } + else + { + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV6_ROUTE); + } + + SWSS_LOG_NOTICE("Removed tunnel route to %s ", ctx->pfx.to_string().c_str()); + return status; + } + } + return true; +} + void MuxNbrHandler::updateTunnelRoute(NextHopKey nh, bool add) { MuxOrch* mux_orch = gDirectory.get(); diff --git a/orchagent/muxorch.h b/orchagent/muxorch.h index 22f01ce27d..4f4ab8117c 100644 --- a/orchagent/muxorch.h +++ b/orchagent/muxorch.h @@ -10,6 +10,7 @@ #include "tunneldecaporch.h" #include "aclorch.h" #include "neighorch.h" +#include "bulker.h" enum MuxState { @@ -35,6 +36,27 @@ enum MuxCableType ACTIVE_ACTIVE }; +struct MuxRouteBulkContext +{ + std::deque object_statuses; // Bulk statuses + IpPrefix pfx; // Route prefix + sai_object_id_t nh; // nexthop id + bool add; // add route bool + + MuxRouteBulkContext(IpPrefix pfx, bool add) + : pfx(pfx), add(add) + { + } + + MuxRouteBulkContext(IpPrefix pfx, sai_object_id_t nh, bool add) + : pfx(pfx), nh(nh), add(add) + { + } +}; + +extern size_t gMaxBulkSize; +extern sai_route_api_t* sai_route_api; + // Forward Declarations class MuxOrch; class MuxCableOrch; @@ -64,7 +86,7 @@ typedef std::map MuxNeighbor; class MuxNbrHandler { public: - MuxNbrHandler() = default; + MuxNbrHandler() : gRouteBulker(sai_route_api, gMaxBulkSize) {}; bool enable(bool update_rt); bool disable(sai_object_id_t); @@ -75,11 +97,15 @@ class MuxNbrHandler string getAlias() const { return alias_; }; private: + bool createBulkRouteEntries(std::list& bulk_ctx_list); + bool processBulkRouteEntries(std::list& bulk_ctx_list); + inline void updateTunnelRoute(NextHopKey, bool = true); private: MuxNeighbor neighbors_; string alias_; + EntityBulker gRouteBulker; }; // Mux Cable object diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index a2bdebbc62..07e21b0400 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -22,10 +22,12 @@ extern Directory gDirectory; extern string gMySwitchType; extern int32_t gVoqMySwitchId; extern BfdOrch *gBfdOrch; +extern size_t gMaxBulkSize; const int neighorch_pri = 30; NeighOrch::NeighOrch(DBConnector *appDb, string tableName, IntfsOrch *intfsOrch, FdbOrch *fdbOrch, PortsOrch *portsOrch, DBConnector *chassisAppDb) : + gNeighBulker(sai_neighbor_api, gMaxBulkSize), Orch(appDb, tableName, neighorch_pri), m_intfsOrch(intfsOrch), m_fdbOrch(fdbOrch), @@ -1199,6 +1201,434 @@ bool NeighOrch::removeNeighbor(const NeighborEntry &neighborEntry, bool disable) return true; } +/** + * @brief Creates a neighbor add entry and adds it to bulker. + * @param ctx NeighborBulkContext contains neighbor information and list of object statuses. + */ +bool NeighOrch::addBulkNeighbor(NeighborBulkContext& ctx) +{ + SWSS_LOG_ENTER(); + + sai_status_t status; + auto& object_statuses = ctx.object_statuses; + + const MacAddress &macAddress = ctx.mac; + const NeighborEntry neighborEntry = ctx.neighborEntry; + string alias = neighborEntry.alias; + IpAddress ip_address = neighborEntry.ip_address; + + SWSS_LOG_INFO("Adding neighbor entry %s on %s to bulker.", ip_address.to_string().c_str(), alias.c_str()); + + sai_object_id_t rif_id = m_intfsOrch->getRouterIntfsId(alias); + if (rif_id == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_INFO("Failed to get rif_id for %s", alias.c_str()); + return false; + } + + sai_neighbor_entry_t neighbor_entry; + neighbor_entry.rif_id = rif_id; + neighbor_entry.switch_id = gSwitchId; + copy(neighbor_entry.ip_address, ip_address); + + vector neighbor_attrs; + sai_attribute_t neighbor_attr; + + neighbor_attr.id = SAI_NEIGHBOR_ENTRY_ATTR_DST_MAC_ADDRESS; + memcpy(neighbor_attr.value.mac, macAddress.getMac(), 6); + neighbor_attrs.push_back(neighbor_attr); + + if ((ip_address.getAddrScope() == IpAddress::LINK_SCOPE) && (ip_address.isV4())) + { + /* Check if this prefix is a configured ip, if not allow */ + IpPrefix ipll_prefix(ip_address.getV4Addr(), 16); + if (!m_intfsOrch->isPrefixSubnet (ipll_prefix, alias)) + { + neighbor_attr.id = SAI_NEIGHBOR_ENTRY_ATTR_NO_HOST_ROUTE; + neighbor_attr.value.booldata = 1; + neighbor_attrs.push_back(neighbor_attr); + } + } + + PortsOrch* ports_orch = gDirectory.get(); + auto vlan_ports = ports_orch->getAllVlans(); + + for (auto vlan_port: vlan_ports) + { + if (vlan_port == alias) + { + continue; + } + NeighborEntry temp_entry = { ip_address, vlan_port }; + if (m_syncdNeighbors.find(temp_entry) != m_syncdNeighbors.end()) + { + SWSS_LOG_NOTICE("Neighbor %s on %s already exists, removing before adding new neighbor", ip_address.to_string().c_str(), vlan_port.c_str()); + if (!removeNeighbor(temp_entry)) + { + SWSS_LOG_ERROR("Failed to create remove neighbor entry %s on %s", ip_address.to_string().c_str(), vlan_port.c_str()); + return false; + } + } + } + + if (gMySwitchType == "voq") + { + if (!addVoqEncapIndex(alias, ip_address, neighbor_attrs)) + { + return false; + } + } + + bool hw_config = isHwConfigured(neighborEntry); + MuxOrch* mux_orch = gDirectory.get(); + if (!hw_config && mux_orch->isNeighborActive(ip_address, macAddress, alias)) + { + object_statuses.emplace_back(); + status = gNeighBulker.create_entry(&object_statuses.back(), &neighbor_entry, (uint32_t)neighbor_attrs.size(), neighbor_attrs.data()); + if (status == SAI_STATUS_ITEM_ALREADY_EXISTS) + { + SWSS_LOG_NOTICE("Neighbor add entry %s already exists in bulker.", ip_address.to_string().c_str()); + return true; + } + } + else if (hw_config) + { + ctx.set_neigh_attr_count = (int)neighbor_attrs.size(); + for (int i = 0; i < ctx.set_neigh_attr_count; i++) + { + object_statuses.emplace_back(); + gNeighBulker.set_entry_attribute(&object_statuses.back(), &neighbor_entry, neighbor_attrs.data()); + } + } + + return true; +} + +/** + * @brief Checks statuses of bulker add operations. + * @param ctx NeighborBulkContext contains NeighborEntry and status list + */ +bool NeighOrch::addBulkNeighborPost(NeighborBulkContext& ctx) +{ + SWSS_LOG_ENTER(); + + const auto& object_statuses = ctx.object_statuses; + auto it_status = object_statuses.begin(); + sai_status_t status; + + const MacAddress &macAddress = ctx.mac; + const NeighborEntry neighborEntry = ctx.neighborEntry; + string alias = neighborEntry.alias; + IpAddress ip_address = neighborEntry.ip_address; + + SWSS_LOG_INFO("Checking neighbor entry %s on %s status.", ip_address.to_string().c_str(), alias.c_str()); + + sai_object_id_t rif_id = m_intfsOrch->getRouterIntfsId(alias); + if (rif_id == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_INFO("Failed to get rif_id for %s", alias.c_str()); + return false; + } + + sai_neighbor_entry_t neighbor_entry; + neighbor_entry.rif_id = rif_id; + neighbor_entry.switch_id = gSwitchId; + copy(neighbor_entry.ip_address, ip_address); + + bool hw_config = isHwConfigured(neighborEntry); + MuxOrch* mux_orch = gDirectory.get(); + if (!hw_config && mux_orch->isNeighborActive(ip_address, macAddress, alias)) + { + status = *it_status++; + if (status != SAI_STATUS_SUCCESS) + { + if (status == SAI_STATUS_ITEM_ALREADY_EXISTS) + { + SWSS_LOG_ERROR("Neighbor exists: neighbor %s on %s, skipping: status:%s", + macAddress.to_string().c_str(), alias.c_str(), sai_serialize_status(status).c_str()); + /* Returning True so as to skip retry */ + return true; + } + else + { + SWSS_LOG_ERROR("Failed to create neighbor %s on %s, status:%s", + macAddress.to_string().c_str(), alias.c_str(), sai_serialize_status(status).c_str()); + task_process_status handle_status = handleSaiCreateStatus(SAI_API_NEIGHBOR, status); + if (handle_status != task_success) + { + return parseHandleSaiStatusFailure(handle_status); + } + } + } + + SWSS_LOG_NOTICE("Created neighbor ip %s, %s on %s", ip_address.to_string().c_str(), + macAddress.to_string().c_str(), alias.c_str()); + + m_intfsOrch->increaseRouterIntfsRefCount(alias); + + if (neighbor_entry.ip_address.addr_family == SAI_IP_ADDR_FAMILY_IPV4) + { + gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_IPV4_NEIGHBOR); + } + else + { + gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_IPV6_NEIGHBOR); + } + + if (!addNextHop(NextHopKey(ip_address, alias))) + { + status = sai_neighbor_api->remove_neighbor_entry(&neighbor_entry); + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to remove neighbor %s on %s, rv:%d", + macAddress.to_string().c_str(), alias.c_str(), status); + task_process_status handle_status = handleSaiRemoveStatus(SAI_API_NEIGHBOR, status); + if (handle_status != task_success) + { + return parseHandleSaiStatusFailure(handle_status); + } + } + m_intfsOrch->decreaseRouterIntfsRefCount(alias); + + if (neighbor_entry.ip_address.addr_family == SAI_IP_ADDR_FAMILY_IPV4) + { + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV4_NEIGHBOR); + } + else + { + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV6_NEIGHBOR); + } + + return false; + } + hw_config = true; + } + else if (hw_config) + { + for (int i = 0; i < ctx.set_neigh_attr_count; i++) + { + status = *it_status++; + if (status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Bulker failed to update neighbor %s on %s, rv:%d", + macAddress.to_string().c_str(), alias.c_str(), status); + task_process_status handle_status = handleSaiSetStatus(SAI_API_NEIGHBOR, status); + if (handle_status != task_success) + { + return parseHandleSaiStatusFailure(handle_status); + } + } + } + SWSS_LOG_NOTICE("Bulker updated neighbor %s on %s", macAddress.to_string().c_str(), alias.c_str()); + } + + m_syncdNeighbors[neighborEntry] = { macAddress, hw_config }; + + NeighborUpdate update = { neighborEntry, macAddress, true }; + notify(SUBJECT_TYPE_NEIGH_CHANGE, static_cast(&update)); + + if(gMySwitchType == "voq") + { + //Sync the neighbor to add to the CHASSIS_APP_DB + voqSyncAddNeigh(alias, ip_address, macAddress, neighbor_entry); + } + + return true; +} + +/** + * @brief Creates a neighbor remove entry and adds it to bulker. + * @param ctx NeighborBulkContext contains neighbor information and list of object statuses. + */ +bool NeighOrch::removeBulkNeighbor(NeighborBulkContext& ctx) +{ + SWSS_LOG_ENTER(); + + sai_status_t status; + auto& object_statuses = ctx.object_statuses; + + const NeighborEntry neighborEntry = ctx.neighborEntry; + string alias = neighborEntry.alias; + IpAddress ip_address = neighborEntry.ip_address; + + NextHopKey nexthop = { ip_address, alias }; + if(m_intfsOrch->isRemoteSystemPortIntf(alias)) + { + //For remote system ports kernel nexthops are always on inband. Change the key + Port inbp; + gPortsOrch->getInbandPort(inbp); + assert(inbp.m_alias.length()); + + nexthop.alias = inbp.m_alias; + } + + if (m_syncdNeighbors.find(neighborEntry) == m_syncdNeighbors.end()) + { + return true; + } + + if (m_syncdNextHops.find(nexthop) != m_syncdNextHops.end() && m_syncdNextHops[nexthop].ref_count > 0) + { + SWSS_LOG_INFO("Failed to remove still referenced neighbor %s on %s", + m_syncdNeighbors[neighborEntry].mac.to_string().c_str(), alias.c_str()); + return false; + } + + if (isHwConfigured(neighborEntry)) + { + sai_object_id_t rif_id = m_intfsOrch->getRouterIntfsId(alias); + + sai_neighbor_entry_t neighbor_entry; + neighbor_entry.rif_id = rif_id; + neighbor_entry.switch_id = gSwitchId; + copy(neighbor_entry.ip_address, ip_address); + + sai_object_id_t next_hop_id = m_syncdNextHops[nexthop].next_hop_id; + status = sai_next_hop_api->remove_next_hop(next_hop_id); + if (status != SAI_STATUS_SUCCESS) + { + /* When next hop is not found, we continue to remove neighbor entry. */ + if (status == SAI_STATUS_ITEM_NOT_FOUND) + { + SWSS_LOG_NOTICE("Next hop %s on %s doesn't exist, rv:%d", + ip_address.to_string().c_str(), alias.c_str(), status); + } + else + { + SWSS_LOG_ERROR("Failed to remove next hop %s on %s, rv:%d", + ip_address.to_string().c_str(), alias.c_str(), status); + task_process_status handle_status = handleSaiRemoveStatus(SAI_API_NEXT_HOP, status); + if (handle_status != task_success) + { + return parseHandleSaiStatusFailure(handle_status); + } + } + } + + if (status != SAI_STATUS_ITEM_NOT_FOUND) + { + if (neighbor_entry.ip_address.addr_family == SAI_IP_ADDR_FAMILY_IPV4) + { + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV4_NEXTHOP); + } + else + { + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV6_NEXTHOP); + } + } + + SWSS_LOG_NOTICE("Removed next hop %s on %s", + ip_address.to_string().c_str(), alias.c_str()); + + object_statuses.emplace_back(); + status = gNeighBulker.remove_entry(&object_statuses.back(), &neighbor_entry); + if (status == SAI_STATUS_ITEM_ALREADY_EXISTS) + { + SWSS_LOG_ERROR("Failed to remove neighbor %s: already exists in bulker", ip_address.to_string().c_str()); + return false; + } + } + + return true; +} + +/** + * @brief Checks statuses of bulker remove operations. + * @param ctx NeighborBulkContext contains NeighborEntry and status list + */ +bool NeighOrch::removeBulkNeighborPost(NeighborBulkContext& ctx, bool disable) +{ + SWSS_LOG_ENTER(); + + const auto& object_statuses = ctx.object_statuses; + auto it_status = object_statuses.begin(); + sai_status_t status; + + const NeighborEntry neighborEntry = ctx.neighborEntry; + string alias = neighborEntry.alias; + IpAddress ip_address = neighborEntry.ip_address; + + if (m_syncdNeighbors.find(neighborEntry) == m_syncdNeighbors.end()) + { + return true; + } + + SWSS_LOG_NOTICE("Removing neighbor %s on %s", ip_address.to_string().c_str(), + m_syncdNeighbors[neighborEntry].mac.to_string().c_str()); + + if (object_statuses.empty()) + { + return true; + } + + if (isHwConfigured(neighborEntry)) + { + sai_object_id_t rif_id = m_intfsOrch->getRouterIntfsId(alias); + + sai_neighbor_entry_t neighbor_entry; + neighbor_entry.rif_id = rif_id; + neighbor_entry.switch_id = gSwitchId; + copy(neighbor_entry.ip_address, ip_address); + + status = *it_status++; + if (status != SAI_STATUS_SUCCESS) + { + if (status == SAI_STATUS_ITEM_NOT_FOUND) + { + SWSS_LOG_NOTICE("Bulker skipped, neighbor %s on %s already removed, rv:%d", + m_syncdNeighbors[neighborEntry].mac.to_string().c_str(), alias.c_str(), status); + } + else + { + SWSS_LOG_ERROR("Bulker failed to remove neighbor %s on %s, rv:%d", + m_syncdNeighbors[neighborEntry].mac.to_string().c_str(), alias.c_str(), status); + task_process_status handle_status = handleSaiRemoveStatus(SAI_API_NEIGHBOR, status); + if (handle_status != task_success) + { + return parseHandleSaiStatusFailure(handle_status); + } + } + } + else + { + if (neighbor_entry.ip_address.addr_family == SAI_IP_ADDR_FAMILY_IPV4) + { + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV4_NEIGHBOR); + } + else + { + gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV6_NEIGHBOR); + } + + removeNextHop(ip_address, alias); + m_intfsOrch->decreaseRouterIntfsRefCount(alias); + SWSS_LOG_NOTICE("Removed neighbor %s on %s", + m_syncdNeighbors[neighborEntry].mac.to_string().c_str(), alias.c_str()); + } + } + + + /* Do not delete entry from cache if its disable request */ + if (disable) + { + m_syncdNeighbors[neighborEntry].hw_configured = false; + return true; + } + + m_syncdNeighbors.erase(neighborEntry); + + 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 + voqSyncDelNeigh(alias, ip_address); + } + + return true; +} + bool NeighOrch::isHwConfigured(const NeighborEntry& neighborEntry) { if (m_syncdNeighbors.find(neighborEntry) == m_syncdNeighbors.end()) @@ -1247,6 +1677,95 @@ bool NeighOrch::disableNeighbor(const NeighborEntry& neighborEntry) return removeNeighbor(neighborEntry, true); } +/** + * @brief Enters neighbor entries into neighbor bulker. + * @param bulk_ctx_list List of NeighborBulkContext entries to add to bulker. + */ +bool NeighOrch::createBulkNeighborEntries(std::list& bulk_ctx_list) +{ + int count = 0; + + SWSS_LOG_INFO("Creating %d bulk neighbor entries", (int)bulk_ctx_list.size()); + + for (auto ctx = bulk_ctx_list.begin(); ctx != bulk_ctx_list.end(); ctx++) + { + const NeighborEntry& neighborEntry = ctx->neighborEntry; + ctx->mac = m_syncdNeighbors[neighborEntry].mac; + + if (m_syncdNeighbors.find(neighborEntry) == m_syncdNeighbors.end()) + { + SWSS_LOG_INFO("Neighbor %s not found", neighborEntry.ip_address.to_string().c_str()); + continue; + } + + if (ctx->enable && isHwConfigured(neighborEntry)) + { + SWSS_LOG_INFO("Neighbor %s is already programmed to HW", neighborEntry.ip_address.to_string().c_str()); + continue; + } + + if (ctx->enable) + { + SWSS_LOG_NOTICE("Neighbor enable request for %s ", neighborEntry.ip_address.to_string().c_str()); + + if(!addBulkNeighbor(*ctx)) + { + SWSS_LOG_INFO("Adding bulk neighbor entry failed for %s", neighborEntry.ip_address.to_string().c_str()); + return false; + } + } + else + { + SWSS_LOG_NOTICE("Neighbor disable request for %s ", neighborEntry.ip_address.to_string().c_str()); + + if(!removeBulkNeighbor(*ctx)) + { + SWSS_LOG_INFO("Removing bulk neighbor entry failed for %s", neighborEntry.ip_address.to_string().c_str()); + return false; + } + } + count++; + } + SWSS_LOG_INFO("Successfully created %d bulk neighbor entries", count); + return true; +} + +/** + * @brief Processes neighbor entries in bulker. + * @param bulk_ctx_list List of neighbor context entries to be processed. + */ +bool NeighOrch::flushBulkNeighborEntries(std::list& bulk_ctx_list) +{ + SWSS_LOG_INFO("Processing %d bulk add neighbor entries", (int)bulk_ctx_list.size()); + gNeighBulker.flush(); + + for (auto ctx = bulk_ctx_list.begin(); ctx != bulk_ctx_list.end(); ctx++) + { + const NeighborEntry& neighborEntry = ctx->neighborEntry; + if (ctx->enable) + { + if (!addBulkNeighborPost(*ctx)) + { + SWSS_LOG_INFO("Enable neighbor failed for %s", neighborEntry.ip_address.to_string().c_str()); + gNeighBulker.clear(); + return false; + } + } + else + { + if (!removeBulkNeighborPost(*ctx, true)) + { + gNeighBulker.clear(); + return false; + } + } + } + + SWSS_LOG_INFO("Succeeded in processing %d bulk add neighbor entries", (int)bulk_ctx_list.size()); + gNeighBulker.clear(); + return true; +} + sai_object_id_t NeighOrch::addTunnelNextHop(const NextHopKey& nh) { SWSS_LOG_ENTER(); diff --git a/orchagent/neighorch.h b/orchagent/neighorch.h index e72979ad07..1152b74b85 100644 --- a/orchagent/neighorch.h +++ b/orchagent/neighorch.h @@ -12,6 +12,7 @@ #include "producerstatetable.h" #include "schema.h" #include "bfdorch.h" +#include "bulker.h" #define NHFLAGS_IFDOWN 0x1 // nexthop's outbound i/f is down @@ -43,6 +44,20 @@ struct NeighborUpdate bool add; }; +struct NeighborBulkContext +{ + std::deque object_statuses; // Bulk statuses + NeighborEntry neighborEntry; // Neighbor entry to process + MacAddress mac; // neighbor mac + bool enable; // enable/disable + int set_neigh_attr_count = 0; // Keeps track of number of attr set + + NeighborBulkContext(NeighborEntry neighborEntry, bool enable) + : neighborEntry(neighborEntry), enable(enable) + { + } +}; + class NeighOrch : public Orch, public Subject, public Observer { public: @@ -66,6 +81,8 @@ class NeighOrch : public Orch, public Subject, public Observer bool enableNeighbor(const NeighborEntry&); bool disableNeighbor(const NeighborEntry&); + bool createBulkNeighborEntries(std::list&); + bool flushBulkNeighborEntries(std::list&); bool isHwConfigured(const NeighborEntry&); sai_object_id_t addTunnelNextHop(const NextHopKey&); @@ -93,10 +110,16 @@ class NeighOrch : public Orch, public Subject, public Observer std::set m_neighborToResolve; + EntityBulker gNeighBulker; + bool removeNextHop(const IpAddress&, const string&); bool addNeighbor(const NeighborEntry&, const MacAddress&); bool removeNeighbor(const NeighborEntry&, bool disable = false); + bool addBulkNeighbor(NeighborBulkContext& ctx); + bool addBulkNeighborPost(NeighborBulkContext& ctx); + bool removeBulkNeighbor(NeighborBulkContext& ctx); + bool removeBulkNeighborPost(NeighborBulkContext& ctx, bool disable = false); bool setNextHopFlag(const NextHopKey &, const uint32_t); bool clearNextHopFlag(const NextHopKey &, const uint32_t); diff --git a/tests/test_mux.py b/tests/test_mux.py index 207ec6741b..c81565e022 100644 --- a/tests/test_mux.py +++ b/tests/test_mux.py @@ -100,6 +100,8 @@ class TestMuxTunnelBase(): DSCP_TO_TC_MAP = {str(i):str(1) for i in range(0, 64)} TC_TO_PRIORITY_GROUP_MAP = {str(i):str(i) for i in range(0, 8)} + BULK_NEIGHBOR_COUNT = 254 + def check_syslog(self, dvs, marker, err_log, expected_cnt): (exitcode, num) = dvs.runcmd(['sh', '-c', "awk \'/%s/,ENDFILE {print;}\' /var/log/syslog | grep \"%s\" | wc -l" % (marker, err_log)]) assert num.strip() >= str(expected_cnt) @@ -336,8 +338,67 @@ def del_route(self, dvs, route): ps = swsscommon.ProducerStateTable(apdb.db_connection, self.APP_ROUTE_TABLE) ps._del(route) + def wait_for_mux_state(self, dvs, interface, expected_state): + """ + Waits until state change completes - expected state is in state_db + """ + + apdb = dvs.get_app_db() + expected_field = {"state": expected_state} + apdb.wait_for_field_match(self.APP_MUX_CABLE, interface, expected_field) + + def bulk_neighbor_test(self, confdb, appdb, asicdb, dvs, dvs_route): + dvs.runcmd("swssloglevel -l INFO -c orchagent") + dvs.runcmd("ip neigh flush all") + self.add_fdb(dvs, "Ethernet0", "00-00-00-00-11-11") + self.set_mux_state(appdb, "Ethernet0", "active") + + class neighbor_info: + ipv4_key = "" + ipv6_key = "" + ipv4 = "" + ipv6 = "" + + def __init__(self, i): + self.ipv4 = "192.168.1." + str(i) + self.ipv6 = "fc02:1001::" + str(i) + + neighbor_list = [neighbor_info(i) for i in range(100, self.BULK_NEIGHBOR_COUNT)] + for neigh_info in neighbor_list: + self.add_neighbor(dvs, neigh_info.ipv4, "00:00:00:00:11:11") + self.add_neighbor(dvs, neigh_info.ipv6, "00:00:00:00:11:11") + neigh_info.ipv4_key = self.check_neigh_in_asic_db(asicdb, neigh_info.ipv4) + neigh_info.ipv6_key = self.check_neigh_in_asic_db(asicdb, neigh_info.ipv6) + + try: + self.set_mux_state(appdb, "Ethernet0", "standby") + self.wait_for_mux_state(dvs, "Ethernet0", "standby") + + for neigh_info in neighbor_list: + asicdb.wait_for_deleted_entry(self.ASIC_NEIGH_TABLE, neigh_info.ipv4_key) + asicdb.wait_for_deleted_entry(self.ASIC_NEIGH_TABLE, neigh_info.ipv6_key) + dvs_route.check_asicdb_route_entries( + [neigh_info.ipv4+self.IPV4_MASK, neigh_info.ipv6+self.IPV6_MASK] + ) + + self.set_mux_state(appdb, "Ethernet0", "active") + self.wait_for_mux_state(dvs, "Ethernet0", "active") + + for neigh_info in neighbor_list: + dvs_route.check_asicdb_deleted_route_entries( + [neigh_info.ipv4+self.IPV4_MASK, neigh_info.ipv6+self.IPV6_MASK] + ) + neigh_info.ipv4_key = self.check_neigh_in_asic_db(asicdb, neigh_info.ipv4) + neigh_info.ipv6_key = self.check_neigh_in_asic_db(asicdb, neigh_info.ipv6) + + finally: + for neigh_info in neighbor_list: + self.del_neighbor(dvs, neigh_info.ipv4) + self.del_neighbor(dvs, neigh_info.ipv6) + def create_and_test_neighbor(self, confdb, appdb, asicdb, dvs, dvs_route): + self.bulk_neighbor_test(confdb, appdb, asicdb, dvs, dvs_route) self.set_mux_state(appdb, "Ethernet0", "active") self.set_mux_state(appdb, "Ethernet4", "standby")