Skip to content

Commit

Permalink
[muxorch] Using bulker to program routes/neighbors during switchover (#…
Browse files Browse the repository at this point in the history
…3148)

* [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.

What I did
Changed mux switchover logic to use neighbor and bulk switchover instead of programming neighbors sequentially.

Why I did it
Testing shows this improves switchover time by an average of 30% at 128 neighbors.

How I verified it
added a test to vstests to test functionality of code, and tested performance on a dualtor lab testbed.
  • Loading branch information
Ndancejic authored Jun 4, 2024
1 parent 98012ed commit b3ebfc4
Show file tree
Hide file tree
Showing 11 changed files with 760 additions and 58 deletions.
189 changes: 171 additions & 18 deletions orchagent/muxorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -744,20 +744,30 @@ void MuxNbrHandler::update(NextHopKey nh, sai_object_id_t tunnelId, bool add, Mu
bool MuxNbrHandler::enable(bool update_rt)
{
NeighborEntry neigh;
std::list<NeighborContext> neigh_ctx_list;
std::list<MuxRouteBulkContext> route_ctx_list;

auto it = neighbors_.begin();
while (it != neighbors_.end())
{
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;
}
// Create neighbor context with bulk_op enabled
neigh_ctx_list.push_back(NeighborContext(neigh, true));
it++;
}

if (!gNeighOrch->enableNeighbors(neigh_ctx_list))
{
return false;
}

it = neighbors_.begin();
while (it != neighbors_.end())
{
/* Update NH to point to learned neighbor */
neigh = NeighborEntry(it->first, alias_);
it->second = gNeighOrch->getLocalNextHopId(neigh);

/* Reprogram route */
Expand Down Expand Up @@ -795,22 +805,26 @@ bool MuxNbrHandler::enable(bool update_rt)
IpPrefix pfx = it->first.to_string();
if (update_rt)
{
if (remove_route(pfx) != SAI_STATUS_SUCCESS)
{
return false;
}
route_ctx_list.push_back(MuxRouteBulkContext(pfx));
updateTunnelRoute(nh_key, false);
}

it++;
}

if (update_rt && !removeRoutes(route_ctx_list))
{
return false;
}

return true;
}

bool MuxNbrHandler::disable(sai_object_id_t tnh)
{
NeighborEntry neigh;
std::list<NeighborContext> neigh_ctx_list;
std::list<MuxRouteBulkContext> route_ctx_list;

auto it = neighbors_.begin();
while (it != neighbors_.end())
Expand Down Expand Up @@ -852,21 +866,25 @@ 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;
}
route_ctx_list.push_back(MuxRouteBulkContext(pfx, it->second));

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;
}
// Create neighbor context with bulk_op enabled
neigh_ctx_list.push_back(NeighborContext(neigh, true));

it++;
}

if (!addRoutes(route_ctx_list))
{
return false;
}

if (!gNeighOrch->disableNeighbors(neigh_ctx_list))
{
return false;
}

return true;
}

Expand All @@ -881,6 +899,141 @@ sai_object_id_t MuxNbrHandler::getNextHopId(const NextHopKey nhKey)
return SAI_NULL_OBJECT_ID;
}

bool MuxNbrHandler::addRoutes(std::list<MuxRouteBulkContext>& bulk_ctx_list)
{
sai_status_t status;
bool ret = true;

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("Adding route entry %s, nh %" PRIx64 " to bulker", ctx->pfx.getIp().to_string().c_str(), ctx->nh);

object_statuses.emplace_back();
sai_attribute_t attr;
vector<sai_attribute_t> 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);

status = gRouteBulker.create_entry(&object_statuses.back(), &route_entry, (uint32_t)attrs.size(), attrs.data());
}

gRouteBulker.flush();

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();
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 (status != SAI_STATUS_SUCCESS)
{
if (status == SAI_STATUS_ITEM_ALREADY_EXISTS) {
SWSS_LOG_INFO("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);
ret = false;
continue;
}

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());
}

gRouteBulker.clear();
return ret;
}

bool MuxNbrHandler::removeRoutes(std::list<MuxRouteBulkContext>& bulk_ctx_list)
{
sai_status_t status;
bool ret = true;

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("Removing route entry %s, nh %" PRIx64 "", ctx->pfx.getIp().to_string().c_str(), ctx->nh);

object_statuses.emplace_back();
status = gRouteBulker.remove_entry(&object_statuses.back(), &route_entry);
}

gRouteBulker.flush();

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();
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 (status != SAI_STATUS_SUCCESS)
{
if (status == SAI_STATUS_ITEM_NOT_FOUND) {
SWSS_LOG_INFO("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);
ret = false;
continue;
}

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());
}

gRouteBulker.clear();
return ret;
}

void MuxNbrHandler::updateTunnelRoute(NextHopKey nh, bool add)
{
MuxOrch* mux_orch = gDirectory.get<MuxOrch*>();
Expand Down
27 changes: 26 additions & 1 deletion orchagent/muxorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "tunneldecaporch.h"
#include "aclorch.h"
#include "neighorch.h"
#include "bulker.h"

enum MuxState
{
Expand All @@ -35,6 +36,26 @@ enum MuxCableType
ACTIVE_ACTIVE
};

struct MuxRouteBulkContext
{
std::deque<sai_status_t> object_statuses; // Bulk statuses
IpPrefix pfx; // Route prefix
sai_object_id_t nh; // nexthop id

MuxRouteBulkContext(IpPrefix pfx)
: pfx(pfx)
{
}

MuxRouteBulkContext(IpPrefix pfx, sai_object_id_t nh)
: pfx(pfx), nh(nh)
{
}
};

extern size_t gMaxBulkSize;
extern sai_route_api_t* sai_route_api;

// Forward Declarations
class MuxOrch;
class MuxCableOrch;
Expand Down Expand Up @@ -64,7 +85,7 @@ typedef std::map<IpAddress, sai_object_id_t> MuxNeighbor;
class MuxNbrHandler
{
public:
MuxNbrHandler() = default;
MuxNbrHandler() : gRouteBulker(sai_route_api, gMaxBulkSize) {};

bool enable(bool update_rt);
bool disable(sai_object_id_t);
Expand All @@ -75,11 +96,15 @@ class MuxNbrHandler
string getAlias() const { return alias_; };

private:
bool removeRoutes(std::list<MuxRouteBulkContext>& bulk_ctx_list);
bool addRoutes(std::list<MuxRouteBulkContext>& bulk_ctx_list);

inline void updateTunnelRoute(NextHopKey, bool = true);

private:
MuxNeighbor neighbors_;
string alias_;
EntityBulker<sai_route_api_t> gRouteBulker;
};

// Mux Cable object
Expand Down
Loading

0 comments on commit b3ebfc4

Please sign in to comment.