Skip to content

Commit

Permalink
Resolve neighbor when nexthop does not exist (sonic-net#1704)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
shi-su authored Apr 20, 2021
1 parent 500e2e9 commit b34f783
Show file tree
Hide file tree
Showing 6 changed files with 368 additions and 6 deletions.
7 changes: 7 additions & 0 deletions cfgmgr/nbrmgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
25 changes: 25 additions & 0 deletions orchagent/neighorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 7 additions & 1 deletion orchagent/neighorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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&);

Expand All @@ -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);
Expand All @@ -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 */
10 changes: 9 additions & 1 deletion orchagent/routeorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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,
Expand Down
Loading

0 comments on commit b34f783

Please sign in to comment.