Resolve neighbor when nexthop does not exist (sonic-net#1704)

What I did Resolve neighbor entries if nexthop does not exist when adding routes. Why I did it In some scenarios, such as adding static routes, the nexthop may not be available, this PR would trigger an attempt to resolve the neighbor. Note that routeorch would only trigger resolution of a neighbor once even if it fails. The arp_update script would find the unresolved neighbor and retry neighbor resolution.
vivekrnv · Apr 20, 2021 · b34f783 · b34f783
1 parent 500e2e9
commit b34f783
Show file tree

Hide file tree

Showing 6 changed files with 368 additions and 6 deletions.
diff --git a/cfgmgr/nbrmgr.cpp b/cfgmgr/nbrmgr.cpp
@@ -234,6 +234,13 @@ void NbrMgr::doResolveNeighTask(Consumer &consumer)
     while (it != consumer.m_toSync.end())
     {
         KeyOpFieldsValuesTuple    t = it->second;
+        if (kfvOp(t) == DEL_COMMAND)
+        {
+            SWSS_LOG_INFO("Received DEL operation for %s, skipping", kfvKey(t).c_str());
+            it = consumer.m_toSync.erase(it);
+            continue;
+        }
+
         vector<string>            keys = parseAliasIp(kfvKey(t), consumer.getConsumerTable()->getTableNameSeparator().c_str());
 
         MacAddress                mac;

diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp
@@ -73,6 +73,25 @@ bool NeighOrch::resolveNeighborEntry(const NeighborEntry &entry, const MacAddres
     return true;
 }
 
+void NeighOrch::resolveNeighbor(const NeighborEntry &entry)
+{
+    if (m_neighborToResolve.find(entry) == m_neighborToResolve.end()) // TODO: Allow retry for unresolved neighbors
+    {
+        resolveNeighborEntry(entry, MacAddress());
+        m_neighborToResolve.insert(entry);
+    }
+
+    return;
+}
+
+void NeighOrch::clearResolvedNeighborEntry(const NeighborEntry &entry)
+{
+    string key, alias = entry.alias;
+    key = alias + ":" + entry.ip_address.to_string();
+    m_appNeighResolveProducer.del(key);
+    return;
+}
+
 /*
  * Function Name: processFDBFlushUpdate
  * Description:
@@ -214,6 +233,12 @@ bool NeighOrch::addNextHop(const IpAddress &ipAddress, const string &alias)
 
     SWSS_LOG_NOTICE("Created next hop %s on %s",
                     ipAddress.to_string().c_str(), alias.c_str());
+    if (m_neighborToResolve.find(nexthop) != m_neighborToResolve.end())
+    {
+        clearResolvedNeighborEntry(nexthop);
+        m_neighborToResolve.erase(nexthop);
+        SWSS_LOG_INFO("Resolved neighbor for %s", nexthop.to_string().c_str());
+    }
 
     NextHopEntry next_hop_entry;
     next_hop_entry.next_hop_id = next_hop_id;

diff --git a/orchagent/neighorch.h b/orchagent/neighorch.h
@@ -74,6 +74,8 @@ class NeighOrch : public Orch, public Subject, public Observer
     bool addInbandNeighbor(string alias, IpAddress ip_address);
     bool delInbandNeighbor(string alias, IpAddress ip_address);
 
+    void resolveNeighbor(const NeighborEntry &);
+
 private:
     PortsOrch *m_portsOrch;
     IntfsOrch *m_intfsOrch;
@@ -83,6 +85,8 @@ class NeighOrch : public Orch, public Subject, public Observer
     NeighborTable m_syncdNeighbors;
     NextHopTable m_syncdNextHops;
 
+    std::set<NextHopKey> m_neighborToResolve;
+
     bool addNextHop(const IpAddress&, const string&);
     bool removeNextHop(const IpAddress&, const string&);
 
@@ -93,7 +97,6 @@ class NeighOrch : public Orch, public Subject, public Observer
     bool clearNextHopFlag(const NextHopKey &, const uint32_t);
 
     void processFDBFlushUpdate(const FdbFlushUpdate &);
-    bool resolveNeighborEntry(const NeighborEntry &, const MacAddress &);
 
     void doTask(Consumer &consumer);
     void doVoqSystemNeighTask(Consumer &consumer);
@@ -104,6 +107,9 @@ class NeighOrch : public Orch, public Subject, public Observer
     bool addVoqEncapIndex(string &alias, IpAddress &ip, vector<sai_attribute_t> &neighbor_attrs);
     void voqSyncAddNeigh(string &alias, IpAddress &ip_address, const MacAddress &mac, sai_neighbor_entry_t &neighbor_entry);
     void voqSyncDelNeigh(string &alias, IpAddress &ip_address);
+
+    bool resolveNeighborEntry(const NeighborEntry &, const MacAddress &);
+    void clearResolvedNeighborEntry(const NeighborEntry &);
 };
 
 #endif /* SWSS_NEIGHORCH_H */
diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp
@@ -1436,8 +1436,9 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops)
                 }
                 else
                 {
-                    SWSS_LOG_INFO("Failed to get next hop %s for %s",
+                    SWSS_LOG_INFO("Failed to get next hop %s for %s, resolving neighbor",
                             nextHops.to_string().c_str(), ipPrefix.to_string().c_str());
+                    m_neighOrch->resolveNeighbor(nexthop);
                     return false;
                 }
             }
@@ -1484,8 +1485,15 @@ bool RouteOrch::addRoute(RouteBulkContext& ctx, const NextHopGroupKey &nextHops)
                                 return false;
                             }
                         }
+                        else
+                        {
+                            SWSS_LOG_INFO("Failed to get next hop %s in %s, resolving neighbor",
+                                    nextHop.to_string().c_str(), nextHops.to_string().c_str());
+                            m_neighOrch->resolveNeighbor(nextHop);
+                        }
                     }
                 }
+
                 /* Failed to create the next hop group and check if a temporary route is needed */
 
                 /* If the current next hop is part of the next hop group to sync,