[routeorch]: Remove the logic of deciding if to add temp route or not (…

…#195)

- The current logic will have a deadlock when the following scenario
  happens:

  The maximum number of next hop groups is set to 1.
  prefix -> [ next_hop_1, next_hop_2]

  When a new entry prefix -> [ next_hop_1, next_hop_2, next_hop_3 ] comes,
  a new next hop group cannot be created because the maximum number of
  next hop groups is reached. However, it will also not create a temporary
  route as the current next hop group is a subset of the new next hop group.
  In this case, the new next hop group will never be syncd.

  With this commit of removing the logic of deciding whether to add or not,
  the new entry could be syncd with a temporary route inserted first so as
  to remove the previous entry and release the resource. On the second try,
  the new entry could be inserted as the old next hop group is removed.

Signed-off-by: Shuotian Cheng <[email protected]>

orchagent/routeorch.cpp

@@ -494,54 +494,32 @@ void RouteOrch::addTempRoute(IpPrefix ipPrefix, IpAddresses nextHops)
 {
     SWSS_LOG_ENTER();
 
-    bool to_add = false;
-    auto it_route = m_syncdRoutes.find(ipPrefix);
     auto next_hop_set = nextHops.getIpAddresses();
 
-    /*
-     * A temporary entry is added when route is not in m_syncdRoutes,
-     * or it is in m_syncdRoutes but the original next hop(s) is not a
-     * subset of the next hop group to be added.
-     */
-    if (it_route != m_syncdRoutes.end())
+    /* Remove next hops that are not in m_syncdNextHops */
+    for (auto it = next_hop_set.begin(); it != next_hop_set.end();)
     {
-        auto tmp_set = m_syncdRoutes[ipPrefix].getIpAddresses();
-        for (auto it : tmp_set)
+        if (!m_neighOrch->hasNextHop(*it))
         {
-            if (next_hop_set.find(it) == next_hop_set.end())
-                to_add = true;
+            SWSS_LOG_INFO("Failed to get next hop entry ip:%s",
+                   (*it).to_string().c_str());
+            it = next_hop_set.erase(it);
         }
+        else
+            it++;
     }
-    else
-        to_add = true;
 
-    if (to_add)
-    {
-        /* Remove next hops that are not in m_syncdNextHops */
-        for (auto it = next_hop_set.begin(); it != next_hop_set.end();)
-        {
-            if (!m_neighOrch->hasNextHop(*it))
-            {
-                SWSS_LOG_INFO("Failed to get next hop entry ip:%s",
-                       (*it).to_string().c_str());
-                it = next_hop_set.erase(it);
-            }
-            else
-                it++;
-        }
-
-        /* Return if next_hop_set is empty */
-        if (next_hop_set.empty())
-            return;
+    /* Return if next_hop_set is empty */
+    if (next_hop_set.empty())
+        return;
 
-        /* Randomly pick an address from the set */
-        auto it = next_hop_set.begin();
-        advance(it, rand() % next_hop_set.size());
+    /* Randomly pick an address from the set */
+    auto it = next_hop_set.begin();
+    advance(it, rand() % next_hop_set.size());
 
-        /* Set the route's temporary next hop to be the randomly picked one */
-        IpAddresses tmp_next_hop((*it).to_string());
-        addRoute(ipPrefix, tmp_next_hop);
-    }
+    /* Set the route's temporary next hop to be the randomly picked one */
+    IpAddresses tmp_next_hop((*it).to_string());
+    addRoute(ipPrefix, tmp_next_hop);
 }
 
 bool RouteOrch::addRoute(IpPrefix ipPrefix, IpAddresses nextHops)