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.

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

[refactor]Refactoring sai handle status #2621

Merged
merged 3 commits into from
Jan 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions orchagent/cbf/cbfnhgorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -343,10 +343,10 @@ bool CbfNhg::sync()
SWSS_LOG_ERROR("Failed to create CBF next hop group %s, rv %d",
m_key.c_str(),
status);
task_process_status handle_status = gCbfNhgOrch->handleSaiCreateStatus(SAI_API_NEXT_HOP_GROUP, status);
task_process_status handle_status = handleSaiCreateStatus(SAI_API_NEXT_HOP_GROUP, status);
if (handle_status != task_success)
{
return gCbfNhgOrch->parseHandleSaiStatusFailure(handle_status);
return parseHandleSaiStatusFailure(handle_status);
}
}

Expand Down
1 change: 1 addition & 0 deletions orchagent/crmorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "crmorch.h"
#include "converter.h"
#include "timer.h"
#include "saihelper.h"

#define CRM_POLLING_INTERVAL "polling_interval"
#define CRM_COUNTERS_TABLE_KEY "STATS"
Expand Down
1 change: 1 addition & 0 deletions orchagent/fabricportsorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "schema.h"
#include "sai_serialize.h"
#include "timer.h"
#include "saihelper.h"

#define FABRIC_POLLING_INTERVAL_DEFAULT (30)
#define FABRIC_PORT_PREFIX "PORT"
Expand Down
5 changes: 0 additions & 5 deletions orchagent/nhgbase.h
Original file line number Diff line number Diff line change
Expand Up @@ -451,11 +451,6 @@ class NhgOrchCommon : public Orch
}
inline void decSyncedNhgCount() { NhgBase::decSyncedCount(); }

/* Handling SAI status*/
using Orch::handleSaiCreateStatus;
using Orch::handleSaiRemoveStatus;
using Orch::parseHandleSaiStatusFailure;

protected:
/*
* Map of synced next hop groups.
Expand Down
4 changes: 2 additions & 2 deletions orchagent/nhgorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -576,10 +576,10 @@ bool NextHopGroup::sync()
SWSS_LOG_ERROR("Failed to create next hop group %s, rv:%d",
m_key.to_string().c_str(), status);

task_process_status handle_status = gNhgOrch->handleSaiCreateStatus(SAI_API_NEXT_HOP_GROUP, status);
task_process_status handle_status = handleSaiCreateStatus(SAI_API_NEXT_HOP_GROUP, status);
if (handle_status != task_success)
{
return gNhgOrch->parseHandleSaiStatusFailure(handle_status);
return parseHandleSaiStatusFailure(handle_status);
}
}

Expand Down
199 changes: 0 additions & 199 deletions orchagent/orch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -856,205 +856,6 @@ Executor *Orch::getExecutor(string executorName)
return NULL;
}

task_process_status Orch::handleSaiCreateStatus(sai_api_t api, sai_status_t status, void *context)
{
/*
* This function aims to provide coarse handling of failures in sairedis create
* operation (i.e., notify users by throwing excepions when failures happen).
* Return value: task_success - Handled the status successfully. No need to retry this SAI operation.
* task_need_retry - Cannot handle the status. Need to retry the SAI operation.
* task_failed - Failed to handle the status but another attempt is unlikely to resolve the failure.
* TODO: 1. Add general handling logic for specific statuses (e.g., SAI_STATUS_ITEM_ALREADY_EXISTS)
* 2. Develop fine-grain failure handling mechanisms and replace this coarse handling
* in each orch.
* 3. Take the type of sai api into consideration.
*/
switch (api)
{
case SAI_API_FDB:
switch (status)
{
case SAI_STATUS_SUCCESS:
SWSS_LOG_WARN("SAI_STATUS_SUCCESS is not expected in handleSaiCreateStatus");
return task_success;
case SAI_STATUS_ITEM_ALREADY_EXISTS:
/*
* In FDB creation, there are scenarios where the hardware learns an FDB entry before orchagent.
* In such cases, the FDB SAI creation would report the status of SAI_STATUS_ITEM_ALREADY_EXISTS,
* and orchagent should ignore the error and treat it as entry was explicitly created.
*/
return task_success;
default:
SWSS_LOG_ERROR("Encountered failure in create operation, exiting orchagent, SAI API: %s, status: %s",
sai_serialize_api(api).c_str(), sai_serialize_status(status).c_str());
abort();
}
break;
case SAI_API_HOSTIF:
switch (status)
{
case SAI_STATUS_SUCCESS:
return task_success;
case SAI_STATUS_FAILURE:
/*
* Host interface maybe failed due to lane not available.
* In some scenarios, like SONiC virtual machine, the invalid lane may be not enabled by VM configuration,
* So just ignore the failure and report an error log.
*/
return task_ignore;
default:
SWSS_LOG_ERROR("Encountered failure in create operation, exiting orchagent, SAI API: %s, status: %s",
sai_serialize_api(api).c_str(), sai_serialize_status(status).c_str());
abort();
}
default:
switch (status)
{
case SAI_STATUS_SUCCESS:
SWSS_LOG_WARN("SAI_STATUS_SUCCESS is not expected in handleSaiCreateStatus");
return task_success;
default:
SWSS_LOG_ERROR("Encountered failure in create operation, exiting orchagent, SAI API: %s, status: %s",
sai_serialize_api(api).c_str(), sai_serialize_status(status).c_str());
abort();
}
}
return task_need_retry;
}

task_process_status Orch::handleSaiSetStatus(sai_api_t api, sai_status_t status, void *context)
{
/*
* This function aims to provide coarse handling of failures in sairedis set
* operation (i.e., notify users by throwing excepions when failures happen).
* Return value: task_success - Handled the status successfully. No need to retry this SAI operation.
* task_need_retry - Cannot handle the status. Need to retry the SAI operation.
* task_failed - Failed to handle the status but another attempt is unlikely to resolve the failure.
* TODO: 1. Add general handling logic for specific statuses
* 2. Develop fine-grain failure handling mechanisms and replace this coarse handling
* in each orch.
* 3. Take the type of sai api into consideration.
*/
if (status == SAI_STATUS_SUCCESS)
{
SWSS_LOG_WARN("SAI_STATUS_SUCCESS is not expected in handleSaiSetStatus");
return task_success;
}

switch (api)
{
case SAI_API_PORT:
switch (status)
{
case SAI_STATUS_INVALID_ATTR_VALUE_0:
/*
* If user gives an invalid attribute value, no need to retry or exit orchagent, just fail the current task
* and let user correct the configuration.
*/
SWSS_LOG_ERROR("Encountered SAI_STATUS_INVALID_ATTR_VALUE_0 in set operation, task failed, SAI API: %s, status: %s",
sai_serialize_api(api).c_str(), sai_serialize_status(status).c_str());
return task_failed;
default:
SWSS_LOG_ERROR("Encountered failure in set operation, exiting orchagent, SAI API: %s, status: %s",
sai_serialize_api(api).c_str(), sai_serialize_status(status).c_str());
abort();
}
case SAI_API_TUNNEL:
switch (status)
{
case SAI_STATUS_ATTR_NOT_SUPPORTED_0:
SWSS_LOG_ERROR("Encountered SAI_STATUS_ATTR_NOT_SUPPORTED_0 in set operation, task failed, SAI API: %s, status: %s",
sai_serialize_api(api).c_str(), sai_serialize_status(status).c_str());
return task_failed;
default:
SWSS_LOG_ERROR("Encountered failure in set operation, exiting orchagent, SAI API: %s, status: %s",
sai_serialize_api(api).c_str(), sai_serialize_status(status).c_str());
abort();
}
default:
SWSS_LOG_ERROR("Encountered failure in set operation, exiting orchagent, SAI API: %s, status: %s",
sai_serialize_api(api).c_str(), sai_serialize_status(status).c_str());
abort();
}

return task_need_retry;
}

task_process_status Orch::handleSaiRemoveStatus(sai_api_t api, sai_status_t status, void *context)
{
/*
* This function aims to provide coarse handling of failures in sairedis remove
* operation (i.e., notify users by throwing excepions when failures happen).
* Return value: task_success - Handled the status successfully. No need to retry this SAI operation.
* task_need_retry - Cannot handle the status. Need to retry the SAI operation.
* task_failed - Failed to handle the status but another attempt is unlikely to resolve the failure.
* TODO: 1. Add general handling logic for specific statuses (e.g., SAI_STATUS_OBJECT_IN_USE,
* SAI_STATUS_ITEM_NOT_FOUND)
* 2. Develop fine-grain failure handling mechanisms and replace this coarse handling
* in each orch.
* 3. Take the type of sai api into consideration.
*/
switch (status)
{
case SAI_STATUS_SUCCESS:
SWSS_LOG_WARN("SAI_STATUS_SUCCESS is not expected in handleSaiRemoveStatus");
return task_success;
default:
SWSS_LOG_ERROR("Encountered failure in remove operation, exiting orchagent, SAI API: %s, status: %s",
sai_serialize_api(api).c_str(), sai_serialize_status(status).c_str());
abort();
}
return task_need_retry;
}

task_process_status Orch::handleSaiGetStatus(sai_api_t api, sai_status_t status, void *context)
{
/*
* This function aims to provide coarse handling of failures in sairedis get
* operation (i.e., notify users by throwing excepions when failures happen).
* Return value: task_success - Handled the status successfully. No need to retry this SAI operation.
* task_need_retry - Cannot handle the status. Need to retry the SAI operation.
* task_failed - Failed to handle the status but another attempt is unlikely to resolve the failure.
* TODO: 1. Add general handling logic for specific statuses
* 2. Develop fine-grain failure handling mechanisms and replace this coarse handling
* in each orch.
* 3. Take the type of sai api into consideration.
*/
switch (status)
{
case SAI_STATUS_SUCCESS:
SWSS_LOG_WARN("SAI_STATUS_SUCCESS is not expected in handleSaiGetStatus");
return task_success;
case SAI_STATUS_NOT_IMPLEMENTED:
SWSS_LOG_ERROR("Encountered failure in get operation due to the function is not implemented, exiting orchagent, SAI API: %s",
sai_serialize_api(api).c_str());
throw std::logic_error("SAI get function not implemented");
default:
SWSS_LOG_ERROR("Encountered failure in get operation, SAI API: %s, status: %s",
sai_serialize_api(api).c_str(), sai_serialize_status(status).c_str());
}
return task_failed;
}

bool Orch::parseHandleSaiStatusFailure(task_process_status status)
{
/*
* This function parses task process status from SAI failure handling function to whether a retry is needed.
* Return value: true - no retry is needed.
* false - retry is needed.
*/
switch (status)
{
case task_need_retry:
return false;
case task_failed:
return true;
default:
SWSS_LOG_WARN("task_process_status %d is not expected in parseHandleSaiStatusFailure", status);
}
return true;
}

void Orch2::doTask(Consumer &consumer)
{
SWSS_LOG_ENTER();
Expand Down
7 changes: 0 additions & 7 deletions orchagent/orch.h
Original file line number Diff line number Diff line change
Expand Up @@ -246,13 +246,6 @@ class Orch
void addExecutor(Executor* executor);
Executor *getExecutor(std::string executorName);

/* Handling SAI status*/
virtual task_process_status handleSaiCreateStatus(sai_api_t api, sai_status_t status, void *context = nullptr);
virtual task_process_status handleSaiSetStatus(sai_api_t api, sai_status_t status, void *context = nullptr);
virtual task_process_status handleSaiRemoveStatus(sai_api_t api, sai_status_t status, void *context = nullptr);
virtual task_process_status handleSaiGetStatus(sai_api_t api, sai_status_t status, void *context = nullptr);
bool parseHandleSaiStatusFailure(task_process_status status);

ResponsePublisher m_publisher;
private:
void addConsumer(swss::DBConnector *db, std::string tableName, int pri = default_orch_pri);
Expand Down
26 changes: 26 additions & 0 deletions orchagent/p4orch/tests/test_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,32 @@ sai_my_mac_api_t *sai_my_mac_api;
sai_counter_api_t *sai_counter_api;
sai_generic_programmable_api_t *sai_generic_programmable_api;

task_process_status handleSaiCreateStatus(sai_api_t api, sai_status_t status, void *context)
{
return task_success;
}

task_process_status handleSaiSetStatus(sai_api_t api, sai_status_t status, void *context)
{
return task_success;
}

task_process_status handleSaiRemoveStatus(sai_api_t api, sai_status_t status, void *context)
{
return task_success;
}

task_process_status handleSaiGetStatus(sai_api_t api, sai_status_t status, void *context)
{
return task_success;
}

bool parseHandleSaiStatusFailure(task_process_status status)
{
return true;
}


namespace
{

Expand Down
Loading