Skip to content

Commit

Permalink
[refactor]Refactoring sai handle status (#2621)
Browse files Browse the repository at this point in the history
* [refactor]Refactoring sai handle status
  • Loading branch information
dgsudharsan authored Jan 19, 2023
1 parent cd95972 commit 383ee68
Show file tree
Hide file tree
Showing 11 changed files with 240 additions and 215 deletions.
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

0 comments on commit 383ee68

Please sign in to comment.