[Routeorch] Fix next hop group reference count in bulk operation (son…

…ic-net#1501)

What I did
Remove next-hop groups after updating the reference counter for a bulk of routes instead of removing next-hop groups in the loop of updating the reference counter.
Fix sonic-net#5813

Why I did it
The bulk route API has two loops of updating the reference counter:

1. update the sairedis reference counter
2. update the orchagent reference counter

Before this commit, the removal of next-hop groups is triggered in the second loop of updating the orchagent reference counter when the reference counter decreases to zero. This may result in a reference counter mismatch between orchagent and sairedis since the sairedis reference counter has already included the operation of the whole bulk but the orchagent reference counter has not. Therefore, the removal of next-hop group may fail due to the mismatch in reference counter (e.g., there are some other routes point to the next-hop group but has not been counted in orchagent yet).

To fix this problem, the next-hop group removal operation should be done after updating the reference counter of the whole bulk to make sure the reference counters sairedis and orchagent matches.

orchagent/routeorch.cpp

@@ -640,6 +640,7 @@ void RouteOrch::doTask(Consumer& consumer)
 
         // Go through the bulker results
         auto it_prev = consumer.m_toSync.begin();
+        m_bulkNhgReducedRefCnt.clear();
         while (it_prev != it)
         {
             KeyOpFieldsValuesTuple t = it_prev->second;
@@ -708,6 +709,15 @@ void RouteOrch::doTask(Consumer& consumer)
                     it_prev++;
             }
         }
+
+        /* Remove next hop group if the reference count decreases to zero */
+        for (auto it_nhg = m_bulkNhgReducedRefCnt.begin(); it_nhg != m_bulkNhgReducedRefCnt.end(); it_nhg++)
+        {
+            if (m_syncdNextHopGroups[*it_nhg].ref_count == 0)
+            {
+                removeNextHopGroup(*it_nhg);
+            }
+        }
     }
 }
 
@@ -1395,7 +1405,7 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey
         if (it_route->second.getSize() > 1
             && m_syncdNextHopGroups[it_route->second].ref_count == 0)
         {
-            removeNextHopGroup(it_route->second);
+            m_bulkNhgReducedRefCnt.emplace(it_route->second);
         }
         SWSS_LOG_INFO("Post set route %s with next hop(s) %s",
                 ipPrefix.to_string().c_str(), nextHops.to_string().c_str());
@@ -1533,15 +1543,12 @@ bool RouteOrch::removeRoutePost(const RouteBulkContext& ctx)
 
     /*
      * Decrease the reference count only when the route is pointing to a next hop.
-     * Decrease the reference count when the route is pointing to a next hop group,
-     * and check whether the reference count decreases to zero. If yes, then we need
-     * to remove the next hop group.
      */
     decreaseNextHopRefCount(it_route->second);
     if (it_route->second.getSize() > 1
         && m_syncdNextHopGroups[it_route->second].ref_count == 0)
     {
-        removeNextHopGroup(it_route->second);
+        m_bulkNhgReducedRefCnt.emplace(it_route->second);
     }
 
     SWSS_LOG_INFO("Remove route %s with next hop(s) %s",

orchagent/routeorch.h

@@ -126,6 +126,8 @@ class RouteOrch : public Orch, public Subject
     RouteTables m_syncdRoutes;
     NextHopGroupTable m_syncdNextHopGroups;
 
+    std::set<NextHopGroupKey> m_bulkNhgReducedRefCnt;
+
     NextHopObserverTable m_nextHopObservers;
 
     EntityBulker<sai_route_api_t>           gRouteBulker;