Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[neighorch] VOQ encap index change handling #1729

Merged
merged 3 commits into from
Jul 2, 2021
Merged

[neighorch] VOQ encap index change handling #1729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e9c4a2e
[neighorch] VOQ encap index change handling
Apr 27, 2021
481cf18
[voq] Code review comments fix - 1
Jun 29, 2021
8cceec2
[neighorch] Log message change for voq neigh encap index update
Jun 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
124 changes: 102 additions & 22 deletions orchagent/neighorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,16 +783,16 @@ bool NeighOrch::addNeighbor(const NeighborEntry &neighborEntry, const MacAddress
MuxOrch* mux_orch = gDirectory.get<MuxOrch*>();
bool hw_config = isHwConfigured(neighborEntry);

if (!hw_config && mux_orch->isNeighborActive(ip_address, macAddress, alias))
if (gMySwitchType == "voq")
{
if (gMySwitchType == "voq")
if (!addVoqEncapIndex(alias, ip_address, neighbor_attrs))
{
if (!addVoqEncapIndex(alias, ip_address, neighbor_attrs))
{
return false;
}
return false;
}
}

if (!hw_config && mux_orch->isNeighborActive(ip_address, macAddress, alias))
{
status = sai_neighbor_api->create_neighbor_entry(&neighbor_entry,
(uint32_t)neighbor_attrs.size(), neighbor_attrs.data());
if (status != SAI_STATUS_SUCCESS)
Expand Down Expand Up @@ -1230,8 +1230,43 @@ void NeighOrch::doVoqSystemNeighTask(Consumer &consumer)
}

if (m_syncdNeighbors.find(neighbor_entry) == m_syncdNeighbors.end() ||
m_syncdNeighbors[neighbor_entry].mac != mac_address)
m_syncdNeighbors[neighbor_entry].mac != mac_address ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we check mac for remote neighbor? Isn't checking encap sufficient?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. For mac change, the encap index is not changed. If we only check encap index, we'll not be processing the mac change.

m_syncdNeighbors[neighbor_entry].voq_encap_index != encap_index)
{

if (m_syncdNeighbors.find(neighbor_entry) != m_syncdNeighbors.end() &&
m_syncdNeighbors[neighbor_entry].voq_encap_index != encap_index)
{

// Encap index changed. Set encap index attribute with new encap index
if (!updateVoqNeighborEncapIndex(neighbor_entry, encap_index))
{
// Setting encap index failed. SAI does not support change of encap index for
// existing neighbors. Remove the neighbor but do not errase from consumer sync
// buffer. The next iteration will add the neighbor back with new encap index

SWSS_LOG_NOTICE("VOQ encap index set failed for neighbor %s. Removing and re-adding", kfvKey(t).c_str());

//Remove neigh from SAI
if (removeNeighbor(neighbor_entry))
{
//neigh successfully deleted from SAI. Set STATE DB to signal to remove entries from kernel
m_stateSystemNeighTable->del(state_key);
}
else
{
SWSS_LOG_ERROR("Failed to remove voq neighbor %s from SAI during encap index update", kfvKey(t).c_str());
}
it++;
}
else
{
SWSS_LOG_NOTICE("VOQ encap index updated for neighbor %s", kfvKey(t).c_str());
it = consumer.m_toSync.erase(it);
}
continue;
}

//Add neigh to SAI
if (addNeighbor(neighbor_entry, mac_address))
{
Expand Down Expand Up @@ -1467,6 +1502,27 @@ void NeighOrch::voqSyncAddNeigh(string &alias, IpAddress &ip_address, const MacA
sai_attribute_t attr;
sai_status_t status;

// Get the encap index and store it for handling change of
// encap index for remote neighbors synced via CHASSIS_APP_DB

attr.id = SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_INDEX;

status = sai_neighbor_api->get_neighbor_entry_attribute(&neighbor_entry, 1, &attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get neighbor attribute for %s on %s, rv:%d", ip_address.to_string().c_str(), alias.c_str(), status);
return;
}

if (!attr.value.u32)
{
SWSS_LOG_ERROR("Invalid neighbor encap_index for %s on %s", ip_address.to_string().c_str(), alias.c_str());
return;
}

NeighborEntry nbrEntry = {ip_address, alias};
m_syncdNeighbors[nbrEntry].voq_encap_index = attr.value.u32;

//Sync only local neigh. Confirm for the local neigh and
//get the system port alias for key for syncing to CHASSIS_APP_DB
Port port;
Expand Down Expand Up @@ -1495,21 +1551,6 @@ void NeighOrch::voqSyncAddNeigh(string &alias, IpAddress &ip_address, const MacA
return;
}

attr.id = SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_INDEX;

status = sai_neighbor_api->get_neighbor_entry_attribute(&neighbor_entry, 1, &attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get neighbor attribute for %s on %s, rv:%d", ip_address.to_string().c_str(), alias.c_str(), status);
return;
}

if (!attr.value.u32)
{
SWSS_LOG_ERROR("Invalid neighbor encap_index for %s on %s", ip_address.to_string().c_str(), alias.c_str());
return;
}

vector<FieldValueTuple> attrs;

FieldValueTuple eiFv ("encap_index", to_string(attr.value.u32));
Expand Down Expand Up @@ -1555,3 +1596,42 @@ void NeighOrch::voqSyncDelNeigh(string &alias, IpAddress &ip_address)
string key = alias + m_tableVoqSystemNeighTable->getTableNameSeparator().c_str() + ip_address.to_string();
m_tableVoqSystemNeighTable->del(key);
}

bool NeighOrch::updateVoqNeighborEncapIndex(const NeighborEntry &neighborEntry, uint32_t encap_index)
{
SWSS_LOG_ENTER();

sai_status_t status;
IpAddress ip_address = neighborEntry.ip_address;
string alias = neighborEntry.alias;

sai_object_id_t rif_id = m_intfsOrch->getRouterIntfsId(alias);
if (rif_id == SAI_NULL_OBJECT_ID)
{
SWSS_LOG_INFO("Failed to get rif_id for %s", alias.c_str());
return false;
}

sai_neighbor_entry_t neighbor_entry;
neighbor_entry.rif_id = rif_id;
neighbor_entry.switch_id = gSwitchId;
copy(neighbor_entry.ip_address, ip_address);

sai_attribute_t neighbor_attr;
neighbor_attr.id = SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_INDEX;
neighbor_attr.value.u32 = encap_index;

status = sai_neighbor_api->set_neighbor_entry_attribute(&neighbor_entry, &neighbor_attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to update voq encap index for neighbor %s on %s, rv:%d",
ip_address.to_string().c_str(), alias.c_str(), status);
return false;
}

SWSS_LOG_NOTICE("Updated voq encap index for neighbor %s on %s", ip_address.to_string().c_str(), alias.c_str());

m_syncdNeighbors[neighborEntry].voq_encap_index = encap_index;

return true;
}
2 changes: 2 additions & 0 deletions orchagent/neighorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ struct NeighborData
{
MacAddress mac;
bool hw_configured = false; // False means, entry is not written to HW
uint32_t voq_encap_index = 0;
};

/* NeighborTable: NeighborEntry, neighbor MAC address */
Expand Down Expand Up @@ -108,6 +109,7 @@ 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 updateVoqNeighborEncapIndex(const NeighborEntry &neighborEntry, uint32_t encap_index);

bool resolveNeighborEntry(const NeighborEntry &, const MacAddress &);
void clearResolvedNeighborEntry(const NeighborEntry &);
Expand Down