[muxorch] handling multiple mux nexthops for route

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 <[email protected]>

Author: Ndancejic

orchagent/muxorch.cpp

@@ -973,6 +973,127 @@ 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
+ */
+void MuxOrch::updateRoute(const IpPrefix &pfx)
+{
+    // Create Route entry
+    sai_route_entry_t route_entry;
+    route_entry.switch_id = gSwitchId;
+    route_entry.vr_id = gVirtualRouterId;
+    copy(route_entry.destination, pfx);
+    subnet(route_entry.destination, route_entry.destination);
+
+    // initialize attr
+    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;
+
+    // initialize route_key to get nexthops
+    RouteKey route_key;
+    route_key.vrf_id = gVirtualRouterId;
+    route_key.prefix = pfx;
+
+    // get list of nexthops
+    std::set<NextHopKey> nextHops = gRouteOrch->getNextHopsForRoute(route_key);
+    if (nextHops.size() > 1)
+    {
+        /* Multi-nexthop case should only program 1 active or 1 standby nexthop */
+
+        // If the route already points to an Active neighbor, do nothing.
+        RouteTables m_syncdRoutes = gRouteOrch->getSyncdRoutes();
+        RouteNhg rtnhg;
+        if (m_syncdRoutes.find(gVirtualRouterId) != m_syncdRoutes.end())
+        {
+            auto it = m_syncdRoutes[gVirtualRouterId].find(pfx);
+            if (it != m_syncdRoutes[gVirtualRouterId].end())
+            {
+                rtnhg = it->second;
+                NextHopGroupKey nhg_key = rtnhg.nhg_key;
+                std::set<NextHopKey> current_nexthops = nhg_key.getNextHops();
+                if (nhg_key.getSize() == 1 &&
+                    findMuxCableInSubnet(current_nexthops.begin()->ip_address)->isActive())
+                {
+                    SWSS_LOG_INFO("Route %s points to Active neighbor %s",
+                        pfx.getIp().to_string().c_str(), current_nexthops.begin()->to_string().c_str());
+                    return;
+                }
+                else if (gRouteOrch->hasNextHopGroup(nhg_key))
+                {
+                    SWSS_LOG_DEBUG("Removing route %s nexthop group: %s",
+                            pfx.getIp().to_string().c_str(), nhg_key.to_string().c_str());
+                    gRouteOrch->removeNextHopGroup(nhg_key);
+                }
+            }
+        }
+
+        SWSS_LOG_INFO("Updating route %s pointing to Mux neighbors",
+                    pfx.getIp().to_string().c_str());
+
+        // Loop through nexthops to find active neighbor
+        for (auto nh = nextHops.begin(); nh != nextHops.end(); nh++)
+        {
+            if (findMuxCableInSubnet(nh->ip_address)->isActive())
+            {
+                // If we find an active nexthop neighbor, program it to hardware.
+                attr.value.oid = gNeighOrch->getNextHopId(*nh);
+                attrs.push_back(attr);
+                SWSS_LOG_NOTICE("Updating route %s with nexthop: %" PRIx64 "",
+                    pfx.getIp().to_string().c_str(), attr.value.oid);
+                sai_status_t status = sai_route_api->create_route_entry(&route_entry, (uint32_t)attrs.size(), attrs.data());
+                if (status != SAI_STATUS_SUCCESS)
+                {
+                    SWSS_LOG_ERROR("Failed to create nexthop route %s,nh %" PRIx64 " rv:%d",
+                            pfx.getIp().to_string().c_str(), attr.value.oid, status);
+                }
+                return;
+            }
+        }
+
+        // No active neighbor was found. program tunnel route instead
+        auto tun = getNextHopTunnelId(MUX_TUNNEL, mux_peer_switch_);
+        attr.value.oid = tun;
+        attrs.push_back(attr);
+
+        SWSS_LOG_DEBUG("No Active neighbors found, setting route %s to point to tunnel: %" PRIx64 "",
+                    pfx.getIp().to_string().c_str(), tun);
+
+        sai_status_t status = sai_route_api->create_route_entry(&route_entry, (uint32_t)attrs.size(), attrs.data());
+        if (status != SAI_STATUS_SUCCESS)
+        {
+            SWSS_LOG_ERROR("Failed to create tunnel route %s,nh %" PRIx64 " rv:%d",
+                    pfx.getIp().to_string().c_str(), tun, status);
+            return;
+        }
+    }
+}
+
+/**
+ * @brief updates all routes pointing to the given cables neighbor list
+ * @param cable MuxCable to update
+ */
+void MuxOrch::updateRoutesForCable(MuxCable* cable)
+{
+    const MuxNeighbor neighbors = cable->getNeighbors();
+    for (auto nh = neighbors.begin(); nh != neighbors.end(); nh ++)
+    {
+        std::set<RouteKey> routes;
+        NextHopKey nhkey(nh->first, cable->getAlias()); 
+        if (gRouteOrch->getRoutesForNexthop(routes, nhkey))
+        {
+            for (auto rt = routes.begin(); rt != routes.end(); rt++)
+            {
+                updateRoute(rt->prefix);
+            }
+        }
+    }
+}
+
 MuxCable* MuxOrch::findMuxCableInSubnet(IpAddress ip)
 {
     for (auto it = mux_cable_tb_.begin(); it != mux_cable_tb_.end(); it++)
@@ -1086,12 +1207,14 @@ void MuxOrch::updateFdb(const FdbUpdate& update)
                 }
                 nh->second = update.entry.port_name;
                 ptr->updateNeighbor(nh->first, false);
+                updateRoutesForCable(ptr);
             }
 
             if (isMuxExists(update.entry.port_name))
             {
                 ptr = getMuxCable(update.entry.port_name);
                 ptr->updateNeighbor(nh->first, true);
+                updateRoutesForCable(ptr);
             }
         }
     }
@@ -1141,6 +1264,7 @@ void MuxOrch::updateNeighbor(const NeighborUpdate& update)
         if (ptr->isIpInSubnet(update.entry.ip_address))
         {
             ptr->updateNeighbor(update.entry, update.add);
+            updateRoutesForCable(ptr);
             return;
         }
     }
@@ -1181,13 +1305,15 @@ void MuxOrch::updateNeighbor(const NeighborUpdate& update)
     {
         ptr = getMuxCable(old_port);
         ptr->updateNeighbor(update.entry, false);
+        updateRoutesForCable(ptr);
         addNexthop(update.entry);
     }
 
     if (!port.empty() && isMuxExists(port))
     {
         ptr = getMuxCable(port);
         ptr->updateNeighbor(update.entry, update.add);
+        updateRoutesForCable(ptr);
     }
 }
 
@@ -1201,6 +1327,30 @@ void MuxOrch::removeNexthop(NextHopKey nh)
     mux_nexthop_tb_.erase(nh);
 }
 
+bool MuxOrch::containsNextHop(const NextHopKey& nh)
+{
+    return mux_nexthop_tb_.find(nh) != 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())
@@ -1574,6 +1724,8 @@ bool MuxCableOrch::addOperation(const Request& request)
         return true;
     }
 
+    mux_orch->updateRoutesForCable(mux_obj);
+
     SWSS_LOG_NOTICE("Mux State set to %s for port %s", state.c_str(), port_name.c_str());
 
     return true;

orchagent/muxorch.h

@@ -72,6 +72,8 @@ class MuxNbrHandler
     void update(NextHopKey nh, sai_object_id_t, bool = true, MuxState = MuxState::MUX_STATE_INIT);
 
     sai_object_id_t getNextHopId(const NextHopKey);
+    const MuxNeighbor getNeighbors() const {return neighbors_;}
+    const string getAlias() const {return alias_;}
 
 private:
     inline void updateTunnelRoute(NextHopKey, bool = true);
@@ -107,6 +109,9 @@ class MuxCable
         return nbr_handler_->getNextHopId(nh);
     }
 
+    const MuxNeighbor getNeighbors() const {return nbr_handler_->getNeighbors();}
+    const string getAlias() const {return nbr_handler_->getAlias();}
+
 private:
     bool stateActive();
     bool stateInitActive();
@@ -195,13 +200,18 @@ 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);
+    void updateRoutesForCable(MuxCable* cable);
+
 private:
     virtual bool addOperation(const Request& request);
     virtual bool delOperation(const Request& request);

orchagent/nexthopgroupkey.h

@@ -23,27 +23,13 @@ class NextHopGroupKey
     /* ip_string|if_alias|vni|router_mac separated by ',' */
     NextHopGroupKey(const std::string &nexthops, bool overlay_nh, bool srv6_nh = false)
     {
-        if (overlay_nh)
-        {
-            m_overlay_nexthops = true;
-            m_srv6_nexthops = false;
-            auto nhv = tokenize(nexthops, NHG_DELIMITER);
-            for (const auto &nh_str : nhv)
-            {
-                auto nh = NextHopKey(nh_str, overlay_nh, srv6_nh);
-                m_nexthops.insert(nh);
-            }
-        }
-        else if (srv6_nh)
+        m_overlay_nexthops = overlay_nh;
+        m_srv6_nexthops = srv6_nh && !overlay_nh;
+        auto nhv = tokenize(nexthops, NHG_DELIMITER);
+        for (const auto &nh_str : nhv)
         {
-            m_overlay_nexthops = false;
-            m_srv6_nexthops = true;
-            auto nhv = tokenize(nexthops, NHG_DELIMITER);
-            for (const auto &nh_str : nhv)
-            {
-                auto nh = NextHopKey(nh_str, overlay_nh, srv6_nh);
-                m_nexthops.insert(nh);
-            }
+            auto nh = NextHopKey(nh_str, overlay_nh, srv6_nh);
+            m_nexthops.insert(nh);
         }
     }
 

orchagent/routeorch.cpp

@@ -6,6 +6,7 @@
 #include "cbf/cbfnhgorch.h"
 #include "logger.h"
 #include "flowcounterrouteorch.h"
+#include "muxorch.h"
 #include "swssnet.h"
 #include "crmorch.h"
 #include "directory.h"
@@ -1580,6 +1581,43 @@ bool RouteOrch::updateNextHopRoutes(const NextHopKey& nextHop, uint32_t& numRout
     return true;
 }
 
+/**
+ * @brief returns a list of nexthops associated with route
+ * @param routeKey routekey to lookup
+ * @return std::set<NextHopKey> of nexthop keys
+ */
+std::set<NextHopKey> RouteOrch::getNextHopsForRoute(const RouteKey& routeKey)
+{
+    std::set<NextHopKey> nh_list;
+    for (auto nh = m_nextHops.begin(); nh != m_nextHops.end(); nh++)
+    {
+        auto route_key = nh->second.find(routeKey);
+        if (route_key != nh->second.end())
+        {
+            nh_list.emplace(nh->first);
+        }
+    }
+    return nh_list;
+}
+
+/**
+ * @brief returns a route prefix associated with nexthopkey
+ * @param routekey_set empty set of routekeys to populate
+ * @param nexthop_key nexthop key to lookup
+ * @return true if found, false if not found.
+ */
+bool RouteOrch::getRoutesForNexthop(std::set<RouteKey>& routekey_set, const NextHopKey& nexthop_key)
+{
+    auto it = m_nextHops.find(nexthop_key);
+
+    if (it != m_nextHops.end())
+    {
+        routekey_set = it->second;
+    }
+
+    return it != m_nextHops.end();
+}
+
 void RouteOrch::addTempRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops)
 {
     SWSS_LOG_ENTER();
@@ -1643,15 +1681,8 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops)
         m_vrfOrch->increaseVrfRefCount(vrf_id);
     }
 
-    if (nextHops.is_overlay_nexthop())
-    {
-        overlay_nh = true;
-    }
-
-    if (nextHops.is_srv6_nexthop())
-    {
-        srv6_nh = true;
-    }
+    overlay_nh = nextHops.is_overlay_nexthop();
+    srv6_nh = nextHops.is_srv6_nexthop();
 
     auto it_route = m_syncdRoutes.at(vrf_id).find(ipPrefix);
 
@@ -2202,14 +2233,23 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey
                 ipPrefix.to_string().c_str(), nextHops.to_string().c_str());
     }
 
-    if (ctx.nhg_index.empty() && nextHops.getSize() == 1 && !nextHops.is_overlay_nexthop() && !nextHops.is_srv6_nexthop())
+    MuxOrch* mux_orch = gDirectory.get<MuxOrch*>();
+    if ((ctx.nhg_index.empty() && nextHops.getSize() == 1 &&
+        !nextHops.is_overlay_nexthop() && !nextHops.is_srv6_nexthop()) ||
+        mux_orch->isMuxNexthops(nextHops))
     {
         RouteKey r_key = { vrf_id, ipPrefix };
-        auto nexthop = NextHopKey(nextHops.to_string());
-        if (!nexthop.ip_address.isZero())
+        auto nexthop_list = nextHops.getNextHops();
+
+        for (auto nh = nexthop_list.begin(); nh != nexthop_list.end(); nh++)
         {
-            addNextHopRoute(nexthop, r_key);
+            if (!nh->ip_address.isZero())
+            {
+                addNextHopRoute(*nh, r_key);
+            }
         }
+        // update routes to reflect mux state
+        mux_orch->updateRoute(ipPrefix);
     }
 
     if (ipPrefix.isDefaultRoute())

orchagent/routeorch.h

@@ -195,6 +195,8 @@ class RouteOrch : public Orch, public Subject
     void addNextHopRoute(const NextHopKey&, const RouteKey&);
     void removeNextHopRoute(const NextHopKey&, const RouteKey&);
     bool updateNextHopRoutes(const NextHopKey&, uint32_t&);
+    std::set<NextHopKey> getNextHopsForRoute(const RouteKey&);
+    bool getRoutesForNexthop(std::set<RouteKey>&, const NextHopKey&);
 
     bool validnexthopinNextHopGroup(const NextHopKey&, uint32_t&);
     bool invalidnexthopinNextHopGroup(const NextHopKey&, uint32_t&);