Skip to content

Commit

Permalink
[muxorch] handling multiple mux nexthops for route
Browse files Browse the repository at this point in the history 
What I did: added logic to handle when a route points to a nexthop
group with mux neighbors. In this case, only one active neighbor, or the
tunnel nexthop will be programmed to the ASIC.

Why I did it: having a route with multiple mux neighbors caused a data
loop which lead to packet loss when different neighbors were in
different states.

How I did it: added logic to update routes when a route is changed, a
mux neighbor is changed, or there is a mux state change.

HLD: sonic-net/SONiC#1256

Signed-off-by: Nikola Dancejic <ndancejic@microsoft.com>
  • Loading branch information
@Ndancejic
Ndancejic committed Mar 7, 2023
1 parent 35385ad commit 4c8c0ef
Show file tree
Hide file tree
Showing 5 changed files with 569 additions and 83 deletions.
213 changes: 210 additions & 3 deletions orchagent/muxorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,34 @@ static sai_status_t remove_route(IpPrefix &pfx)
return status;
}

/**
* @brief sets the given route to point to the given nexthop
* @param pfx IpPrefix of the route
* @param nexthop NextHopKey of the nexthop
* @return SAI_STATUS_SUCCESS on success
*/
static sai_status_t set_route(const IpPrefix& pfx, sai_object_id_t next_hop_id)
{
/* set route entry to point to nh */
sai_route_entry_t route_entry;
sai_attribute_t route_attr;

route_entry.vr_id = gVirtualRouterId;
route_entry.switch_id = gSwitchId;
copy(route_entry.destination, pfx);

route_attr.id = SAI_ROUTE_ENTRY_ATTR_NEXT_HOP_ID;
route_attr.value.oid = next_hop_id;

sai_status_t status = sai_route_api->set_route_entry_attribute(&route_entry, &route_attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to set route entry %s nh %" PRIx64 " rv:%d",
pfx.to_string().c_str(), next_hop_id, status);
}
return status;
}

static sai_object_id_t create_tunnel(
const IpAddress* p_dst_ip,
const IpAddress* p_src_ip,
Expand Down Expand Up @@ -534,9 +562,11 @@ bool MuxCable::isIpInSubnet(IpAddress ip)

bool MuxCable::nbrHandler(bool enable, bool update_rt)
{
bool ret;
if (enable)
{
return nbr_handler_->enable(update_rt);
ret = nbr_handler_->enable(update_rt);
updateRoutes();
}
else
{
Expand All @@ -546,9 +576,10 @@ bool MuxCable::nbrHandler(bool enable, bool update_rt)
SWSS_LOG_INFO("Null NH object id, retry for %s", peer_ip4_.to_string().c_str());
return false;
}

return nbr_handler_->disable(tnh);
updateRoutes();
ret = nbr_handler_->disable(tnh);
}
return ret;
}

void MuxCable::updateNeighbor(NextHopKey nh, bool add)
Expand All @@ -562,6 +593,29 @@ void MuxCable::updateNeighbor(NextHopKey nh, bool add)
else if (mux_name_ == mux_orch_->getNexthopMuxName(nh))
{
mux_orch_->removeNexthop(nh);
nbr_handler_->removeNeighborRoutes(nh);
}
updateRoutes();
}

/**
* @brief updates all routes pointing to the cables neighbor list
*/
void MuxCable::updateRoutes()
{
MuxNeighbor neighbors = nbr_handler_->getNeighbors();
string alias = nbr_handler_->getAlias();
for (auto nh = neighbors.begin(); nh != neighbors.end(); nh ++)
{
std::set<RouteKey> routes;
NextHopKey nhkey(nh->first, alias);
if (gRouteOrch->getRoutesForNexthop(routes, nhkey))
{
for (auto rt = routes.begin(); rt != routes.end(); rt++)
{
mux_orch_->updateRoute(rt->prefix, false);
}
}
}
}

Expand Down Expand Up @@ -749,6 +803,45 @@ bool MuxNbrHandler::disable(sai_object_id_t tnh)
return true;
}

/**
* @brief removes routes programmed by updateRoute
* @param nh NextHopKey that points to neighbor for which to remove routes
*/
void MuxNbrHandler::removeNeighborRoutes(NextHopKey nh)
{
std::set<RouteKey> routes;
NextHopGroupKey nhg_key;
NextHopGroupEntry nhg_entry;

if (gRouteOrch->getRoutesForNexthop(routes, nh))
{
for (auto rt = routes.begin(); rt != routes.end(); rt++)
{
SWSS_LOG_INFO("Removing multi-nexthop route for %s", rt->prefix.to_string().c_str());
return false;
/* get nexthop group key from syncd */
nhg_key = gRouteOrch->getSyncdRouteNhgKey(gVirtualRouterId, rt->prefix);

/* check for cross-mux neighbors */
if (nhg_key.getSize() > 1)
{
sai_route_entry_t route_entry;
route_entry.switch_id = gSwitchId;
route_entry.vr_id = gVirtualRouterId;
copy(route_entry.destination, rt->prefix);
subnet(route_entry.destination, route_entry.destination);

sai_status_t status = sai_route_api->remove_route_entry(&route_entry);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to remove route %s, rv:%d",
rt->prefix.to_string().c_str(), status);
}
}
}
}
}

sai_object_id_t MuxNbrHandler::getNextHopId(const NextHopKey nhKey)
{
auto it = neighbors_.find(nhKey.ip_address);
Expand Down Expand Up @@ -973,6 +1066,91 @@ sai_object_id_t MuxOrch::getNextHopTunnelId(std::string tunnelKey, IpAddress& ip
return it->second.nh_id;
}

/**
* @brief updates the given route to point to a single active NH or tunnel
* @param pfx IpPrefix of route to update
* @param remove bool only true when route is getting removed
*/
void MuxOrch::updateRoute(const IpPrefix &pfx, bool remove)
{
NextHopGroupKey nhg_key;
NextHopGroupEntry nhg_entry;

if (remove)
{
mux_multi_nh_route_tb.erase(pfx);
return;
}

/* get nexthop group key from syncd */
nhg_key = gRouteOrch->getSyncdRouteNhgKey(gVirtualRouterId, pfx);

/* check for cross-mux neighbors */
if (nhg_key.getSize() > 1)
{
std::set<NextHopKey> nextHops;
sai_object_id_t next_hop_id;
sai_status_t status;
bool active_found = false;

/* get nexthops from nexthop group */
nextHops = nhg_key.getNextHops();

auto it = mux_multi_nh_route_tb.find(pfx);
if (it != mux_multi_nh_route_tb.end())
{
MuxCable* cable = findMuxCableInSubnet(it->second.ip_address);
if (cable == nullptr || cable->isActive())
{
SWSS_LOG_NOTICE("Route %s pointing to active neighbor %s",
pfx.to_string().c_str(), it->second.to_string().c_str());
return;
}
}

SWSS_LOG_NOTICE("Updating route %s pointing to Mux nexthops %s",
pfx.to_string().c_str(), nhg_key.to_string().c_str());

for (auto it = nextHops.begin(); it != nextHops.end(); it++)
{
NextHopKey nexthop = *it;
MuxCable* cable = findMuxCableInSubnet(nexthop.ip_address);
if (cable == nullptr || (cable->isActive() && !active_found))
{
next_hop_id = gNeighOrch->getLocalNextHopId(nexthop);
/* set route entry to point to nh */
status = set_route(pfx, next_hop_id);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to set route entry %s to nexthop %s",
pfx.to_string().c_str(), nexthop.to_string().c_str());
continue;
}
SWSS_LOG_INFO("setting route %s with nexthop %s %" PRIx64 "",
pfx.to_string().c_str(), nexthop.to_string().c_str(), next_hop_id);
mux_multi_nh_route_tb[pfx] = nexthop;
active_found = true;
break;
}
}

if (!active_found)
{
next_hop_id = getNextHopTunnelId(MUX_TUNNEL, mux_peer_switch_);
/* no active nexthop found, point to first */
SWSS_LOG_NOTICE("No Active neighbors found, setting route %s to point to tun",
pfx.getIp().to_string().c_str());
status = set_route(pfx, next_hop_id);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to set route entry %s to tunnel",
pfx.getIp().to_string().c_str());
}
mux_multi_nh_route_tb.erase(pfx);
}
}
}

MuxCable* MuxOrch::findMuxCableInSubnet(IpAddress ip)
{
for (auto it = mux_cable_tb_.begin(); it != mux_cable_tb_.end(); it++)
Expand Down Expand Up @@ -1201,6 +1379,35 @@ void MuxOrch::removeNexthop(NextHopKey nh)
mux_nexthop_tb_.erase(nh);
}

/**
* @brief checks if mux nexthop tb contains nexthop
* @param nexthop NextHopKey
* @return true if a mux contains the nexthop
*/
bool MuxOrch::containsNextHop(const NextHopKey& nexthop)
{
return mux_nexthop_tb_.find(nexthop) != mux_nexthop_tb_.end();
}

/**
* @brief checks if a given nexthop group belongs to a mux
* @param nextHops NextHopGroupKey
* @return true if a mux contains any of the nexthops in the group
* false if none of the nexthops belong to a mux
*/
bool MuxOrch::isMuxNexthops(const NextHopGroupKey& nextHops)
{
const std::set<NextHopKey> s_nexthops = nextHops.getNextHops();
for (auto it = s_nexthops.begin(); it != s_nexthops.end(); it ++)
{
if (this->containsNextHop(*it))
{
return true;
}
}
return false;
}

string MuxOrch::getNexthopMuxName(NextHopKey nh)
{
if (mux_nexthop_tb_.find(nh) == mux_nexthop_tb_.end())
Expand Down
13 changes: 13 additions & 0 deletions orchagent/muxorch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@ class MuxNbrHandler
bool disable(sai_object_id_t);
void update(NextHopKey nh, sai_object_id_t, bool = true, MuxState = MuxState::MUX_STATE_INIT);

void removeNeighborRoutes(NextHopKey nh);

sai_object_id_t getNextHopId(const NextHopKey);
MuxNeighbor getNeighbors() const { return neighbors_; };
string getAlias() const { return alias_; };

private:
inline void updateTunnelRoute(NextHopKey, bool = true);
Expand Down Expand Up @@ -102,6 +106,7 @@ class MuxCable

bool isIpInSubnet(IpAddress ip);
void updateNeighbor(NextHopKey nh, bool add);
void updateRoutes();
sai_object_id_t getNextHopId(const NextHopKey nh)
{
return nbr_handler_->getNextHopId(nh);
Expand Down Expand Up @@ -158,6 +163,7 @@ typedef std::unique_ptr<MuxCable> MuxCable_T;
typedef std::map<std::string, MuxCable_T> MuxCableTb;
typedef std::map<IpAddress, NHTunnel> MuxTunnelNHs;
typedef std::map<NextHopKey, std::string> NextHopTb;
typedef std::map<IpPrefix, NextHopKey> MuxRouteTb;

class MuxCfgRequest : public Request
{
Expand Down Expand Up @@ -195,13 +201,17 @@ class MuxOrch : public Orch2, public Observer, public Subject

void addNexthop(NextHopKey, string = "");
void removeNexthop(NextHopKey);
bool containsNextHop(const NextHopKey&);
bool isMuxNexthops(const NextHopGroupKey&);
string getNexthopMuxName(NextHopKey);
sai_object_id_t getNextHopId(const NextHopKey&);

sai_object_id_t createNextHopTunnel(std::string tunnelKey, IpAddress& ipAddr);
bool removeNextHopTunnel(std::string tunnelKey, IpAddress& ipAddr);
sai_object_id_t getNextHopTunnelId(std::string tunnelKey, IpAddress& ipAddr);

void updateRoute(const IpPrefix &pfx, bool remove);

private:
virtual bool addOperation(const Request& request);
virtual bool delOperation(const Request& request);
Expand Down Expand Up @@ -241,6 +251,9 @@ class MuxOrch : public Orch2, public Observer, public Subject
MuxTunnelNHs mux_tunnel_nh_;
NextHopTb mux_nexthop_tb_;

/* contains reference of programmed routes by updateRoute */
MuxRouteTb mux_multi_nh_route_tb;

handler_map handler_map_;

TunnelDecapOrch *decap_orch_;
Expand Down
Loading

0 comments on commit 4c8c0ef

Please sign in to comment.