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

[DASH] Make DASH vnet orch bulk logic consistent #3554

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[DASH] Make DASH vnet orch bulk logic consistent #3554

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
9305725
Make DASH vnet orch bulk logic consistent
theasianpianist Mar 10, 2025
8319b9e
update create logic for vnet maps
theasianpianist Mar 14, 2025
87c2234
update comment
theasianpianist Mar 14, 2025
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
49 changes: 26 additions & 23 deletions orchagent/dash/dashvnetorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ bool DashVnetOrch::addOutboundCaToPa(const string& key, VnetMapBulkContext& ctxt
else
{
SWSS_LOG_ERROR("Invalid encap type %d for %s", action.encap_type(), key.c_str());
return false;
return true;
}
outbound_ca_to_pa_attrs.push_back(outbound_ca_to_pa_attr);

Expand Down Expand Up @@ -372,10 +372,10 @@ bool DashVnetOrch::addOutboundCaToPa(const string& key, VnetMapBulkContext& ctxt
object_statuses.emplace_back();
outbound_ca_to_pa_bulker_.create_entry(&object_statuses.back(), &outbound_ca_to_pa_entry,
(uint32_t)outbound_ca_to_pa_attrs.size(), outbound_ca_to_pa_attrs.data());
return true;
return false;
}

bool DashVnetOrch::addPaValidation(const string& key, VnetMapBulkContext& ctxt)
void DashVnetOrch::addPaValidation(const string& key, VnetMapBulkContext& ctxt)
{
SWSS_LOG_ENTER();

Expand All @@ -394,7 +394,7 @@ bool DashVnetOrch::addPaValidation(const string& key, VnetMapBulkContext& ctxt)
SWSS_LOG_INFO("Increment PA refcount to %u for PA IP %s",
pa_refcount_table_[pa_ref_key],
underlay_ip_str.c_str());
return true;
return;
}

uint32_t attr_count = 1;
Expand All @@ -413,13 +413,13 @@ bool DashVnetOrch::addPaValidation(const string& key, VnetMapBulkContext& ctxt)
pa_refcount_table_[pa_ref_key] = 1;
SWSS_LOG_INFO("Initialize PA refcount to 1 for PA IP %s",
underlay_ip_str.c_str());
return true;
}

bool DashVnetOrch::addVnetMap(const string& key, VnetMapBulkContext& ctxt)
{
SWSS_LOG_ENTER();

bool remove_from_consumer = true;
bool exists = (vnet_map_table_.find(key) != vnet_map_table_.end());
if (!exists)
{
Expand All @@ -430,13 +430,18 @@ bool DashVnetOrch::addVnetMap(const string& key, VnetMapBulkContext& ctxt)
SWSS_LOG_INFO("Not creating VNET map for %s since VNET %s doesn't exist", key.c_str(), ctxt.vnet_name.c_str());
return false;
}
return addOutboundCaToPa(key, ctxt) && addPaValidation(key, ctxt);

remove_from_consumer = addOutboundCaToPa(key, ctxt);
if (!remove_from_consumer)
{
addPaValidation(key, ctxt);
}
}
/*
* If the VNET map is already added, don't add it to the bulker and
* return true so it's removed from the consumer
*/
return true;
return remove_from_consumer;
}

bool DashVnetOrch::addOutboundCaToPaPost(const string& key, const VnetMapBulkContext& ctxt)
Expand All @@ -455,8 +460,7 @@ bool DashVnetOrch::addOutboundCaToPaPost(const string& key, const VnetMapBulkCon
{
if (status == SAI_STATUS_ITEM_ALREADY_EXISTS)
{
// Retry if item exists in the bulker
return false;
return true;
}

SWSS_LOG_ERROR("Failed to create CA to PA entry for %s", key.c_str());
Expand Down Expand Up @@ -490,14 +494,13 @@ bool DashVnetOrch::addPaValidationPost(const string& key, const VnetMapBulkConte
sai_status_t status = *it_status++;
if (status != SAI_STATUS_SUCCESS)
{
/* PA validation entry add failed. Remove PA refcount entry */
pa_refcount_table_.erase(pa_ref_key);
if (status == SAI_STATUS_ITEM_ALREADY_EXISTS)
{
// Retry if item exists in the bulker
return false;
return true;
}

/* PA validation entry add failed. Remove PA refcount entry */
SWSS_LOG_ERROR("Failed to create PA validation entry for %s", key.c_str());
task_process_status handle_status = handleSaiCreateStatus((sai_api_t) SAI_API_DASH_PA_VALIDATION, status);
if (handle_status != task_success)
Expand All @@ -517,19 +520,19 @@ bool DashVnetOrch::addVnetMapPost(const string& key, const VnetMapBulkContext& c
{
SWSS_LOG_ENTER();

bool status = addOutboundCaToPaPost(key, ctxt) && addPaValidationPost(key, ctxt);
if (!status)
bool remove_from_consumer = addOutboundCaToPaPost(key, ctxt) && addPaValidationPost(key, ctxt);
if (!remove_from_consumer)
{
SWSS_LOG_ERROR("addVnetMapPost failed for %s ", key.c_str());
return false;
return remove_from_consumer;
}

string vnet_name = ctxt.vnet_name;
VnetMapEntry entry = { gVnetNameToId[vnet_name], ctxt.dip, ctxt.metadata };
VnetMapEntry entry = { gVnetNameToId[vnet_name], ctxt.dip, ctxt.metadata };
vnet_map_table_[key] = entry;
SWSS_LOG_INFO("Vnet map added for %s", key.c_str());

return true;
return remove_from_consumer;
}

void DashVnetOrch::removeOutboundCaToPa(const string& key, VnetMapBulkContext& ctxt)
Expand All @@ -556,7 +559,7 @@ void DashVnetOrch::removePaValidation(const string& key, VnetMapBulkContext& ctx
auto it = pa_refcount_table_.find(pa_ref_key);
if (it == pa_refcount_table_.end())
{
return;
SWSS_LOG_INFO("PA refcount entry not found for %s", key.c_str());
}
else
{
Expand All @@ -570,7 +573,6 @@ void DashVnetOrch::removePaValidation(const string& key, VnetMapBulkContext& ctx
SWSS_LOG_INFO("Decrement PA refcount to %u for PA IP %s",
pa_refcount_table_[pa_ref_key],
underlay_ip.c_str());
return;
}
else
{
Expand Down Expand Up @@ -681,15 +683,16 @@ bool DashVnetOrch::removeVnetMapPost(const string& key, const VnetMapBulkContext
{
SWSS_LOG_ENTER();

bool status = removeOutboundCaToPaPost(key, ctxt) && removePaValidationPost(key, ctxt);
if (!status)
bool remove_from_consumer = removeOutboundCaToPaPost(key, ctxt) && removePaValidationPost(key, ctxt);
if (!remove_from_consumer)
{
return false;
SWSS_LOG_ERROR("removeVnetMapPost failed for %s ", key.c_str());
return remove_from_consumer;
}
vnet_map_table_.erase(key);
SWSS_LOG_INFO("Vnet map removed for %s", key.c_str());

return true;
return remove_from_consumer;
}

void DashVnetOrch::doTaskVnetMapTable(ConsumerBase& consumer)
Expand Down
6 changes: 5 additions & 1 deletion orchagent/dash/dashvnetorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ class DashVnetOrch : public ZmqOrch
void doTask(ConsumerBase &consumer);
void doTaskVnetTable(ConsumerBase &consumer);
void doTaskVnetMapTable(ConsumerBase &consumer);

// The following add/remove methods will return true if the provided key should be removed from the
// consumer (i.e. task is done and no retries are required) and false otherwise.
// Methods which only have one possible outcome will have return type void.
bool addVnet(const std::string& key, DashVnetBulkContext& ctxt);
bool addVnetPost(const std::string& key, const DashVnetBulkContext& ctxt);
bool removeVnet(const std::string& key, DashVnetBulkContext& ctxt);
Expand All @@ -95,7 +99,7 @@ class DashVnetOrch : public ZmqOrch
bool addOutboundCaToPaPost(const std::string& key, const VnetMapBulkContext& ctxt);
void removeOutboundCaToPa(const std::string& key, VnetMapBulkContext& ctxt);
bool removeOutboundCaToPaPost(const std::string& key, const VnetMapBulkContext& ctxt);
bool addPaValidation(const std::string& key, VnetMapBulkContext& ctxt);
void addPaValidation(const std::string& key, VnetMapBulkContext& ctxt);
bool addPaValidationPost(const std::string& key, const VnetMapBulkContext& ctxt);
void removePaValidation(const std::string& key, VnetMapBulkContext& ctxt);
bool removePaValidationPost(const std::string& key, const VnetMapBulkContext& ctxt);
Expand Down