diff --git a/lib/inc/sai_redis.h b/lib/inc/sai_redis.h index 09b68388a7..e44712f681 100644 --- a/lib/inc/sai_redis.h +++ b/lib/inc/sai_redis.h @@ -45,6 +45,11 @@ void check_notifications_pointers( // there is something wrong and we should fail #define GET_RESPONSE_TIMEOUT (6*60*1000) +// if we don't receive response for create/set/remove from syncd in 30 seconds +// there is something wrong and we should fail +#define CSR_RESPONSE_TIMEOUT (30*1000) + +extern sai_status_t redis_get_response(const std::string &key, const std::string &op, sai_object_type_t object_type); extern std::string getSelectResultAsString(int result); extern void clear_local_state(); extern void setRecording(bool record); @@ -65,6 +70,7 @@ extern sai_service_method_table_t g_services; extern std::shared_ptr g_asicState; extern std::shared_ptr g_redisGetConsumer; extern std::shared_ptr g_redisNotifications; +extern std::shared_ptr g_redisApiResponseNtf; extern std::shared_ptr g_redisClient; extern std::mutex g_apimutex; diff --git a/lib/inc/sairedis.h b/lib/inc/sairedis.h index 88409e8aad..6d3bf71681 100644 --- a/lib/inc/sairedis.h +++ b/lib/inc/sairedis.h @@ -245,4 +245,17 @@ sai_status_t sai_bulk_remove_fdb_entry( _In_ const sai_fdb_entry_t *fdb_entry, _In_ sai_bulk_op_error_mode_t mode, _Out_ sai_status_t *object_statuses); + +#define sai_return_obj_op_status(object_type) ({ \ + bool ret; \ + switch (object_type) \ + { \ + case SAI_OBJECT_TYPE_ROUTE_ENTRY: \ + ret = false; break; \ + default: \ + ret = true; break; \ + } \ + ret; \ +}) + #endif // __SAIREDIS__ diff --git a/lib/src/sai_redis_generic_create.cpp b/lib/src/sai_redis_generic_create.cpp index 30e34cc75c..11b27d9193 100644 --- a/lib/src/sai_redis_generic_create.cpp +++ b/lib/src/sai_redis_generic_create.cpp @@ -1,6 +1,7 @@ #include "sai_redis.h" #include "meta/sai_serialize.h" #include "meta/saiattributelist.h" +#include "sairedis.h" bool switch_ids[MAX_SWITCHES] = {}; @@ -197,6 +198,93 @@ void redis_free_virtual_object_id( } } +// wait for response +sai_status_t redis_get_response( + _In_ const std::string &key, + _In_ const std::string &op, + _In_ sai_object_type_t object_type) +{ + SWSS_LOG_ENTER(); + + swss::Select s; + + sai_status_t status; + std::string csr; + + // For certain object types, no response is expected + if (!sai_return_obj_op_status(object_type)) + { + return SAI_STATUS_SUCCESS; + } + + if (op == "create") + { + csr = "C"; + } + else if (op == "set") + { + csr = "S"; + } + else if (op == "remove") + { + csr = "R"; + } + else + { + //getting repsonse for other operations not supported. + return SAI_STATUS_SUCCESS; + } + + s.addSelectable(g_redisApiResponseNtf.get()); + + while (true) + { + SWSS_LOG_DEBUG("wait for response"); + + swss::Selectable *sel; + + int result = s.select(&sel, CSR_RESPONSE_TIMEOUT); + + if (result == swss::Select::OBJECT) + { + std::string retKey; + std::string str_sai_status; + std::vector values; + + g_redisApiResponseNtf->pop(retKey, str_sai_status, values); + + // op in response should match the original request + if (key != retKey) + { + SWSS_LOG_ERROR("response key = %s, not matching request key %s", retKey.c_str(), key.c_str()); + return SAI_STATUS_FAILURE; + } + SWSS_LOG_DEBUG("response: key = %s, op = %s, status = %s", key.c_str(), op.c_str(), str_sai_status.c_str()); + + sai_deserialize_status(str_sai_status, status); + + if (g_record) + { + // first serialized is status + recordLine(csr + "|" + str_sai_status + "|" + key); + } + + return status; + } + + SWSS_LOG_ERROR("generic %s on %s failed due to SELECT operation result: %s", + op.c_str(), key.c_str(), getSelectResultAsString(result).c_str()); + break; + } + + if (g_record) + { + recordLine(csr + "|SAI_STATUS_FAILURE|" + key); + } + + return SAI_STATUS_FAILURE; +} + sai_status_t internal_redis_generic_create( _In_ sai_object_type_t object_type, _In_ const std::string &serialized_object_id, @@ -233,9 +321,7 @@ sai_status_t internal_redis_generic_create( g_asicState->set(key, entry, "create"); - // we assume create will always succeed which may not be true - // we should make this synchronous call - return SAI_STATUS_SUCCESS; + return redis_get_response(key, "create", object_type); } sai_status_t redis_generic_create( diff --git a/lib/src/sai_redis_generic_remove.cpp b/lib/src/sai_redis_generic_remove.cpp index ec04749093..e2fad58b1f 100644 --- a/lib/src/sai_redis_generic_remove.cpp +++ b/lib/src/sai_redis_generic_remove.cpp @@ -21,7 +21,7 @@ sai_status_t internal_redis_generic_remove( g_asicState->del(key, "remove"); - return SAI_STATUS_SUCCESS; + return redis_get_response(key, "remove", object_type); } sai_status_t redis_generic_remove( diff --git a/lib/src/sai_redis_generic_set.cpp b/lib/src/sai_redis_generic_set.cpp index 33fb35b8f8..a33a520958 100644 --- a/lib/src/sai_redis_generic_set.cpp +++ b/lib/src/sai_redis_generic_set.cpp @@ -28,7 +28,7 @@ sai_status_t internal_redis_generic_set( g_asicState->set(key, entry, "set"); - return SAI_STATUS_SUCCESS; + return redis_get_response(key, "set", object_type); } sai_status_t internal_redis_bulk_generic_set( diff --git a/lib/src/sai_redis_interfacequery.cpp b/lib/src/sai_redis_interfacequery.cpp index 365c371448..e3d2ad4f4d 100644 --- a/lib/src/sai_redis_interfacequery.cpp +++ b/lib/src/sai_redis_interfacequery.cpp @@ -19,6 +19,7 @@ std::shared_ptr g_dbNtf; std::shared_ptr g_asicState; std::shared_ptr g_redisGetConsumer; std::shared_ptr g_redisNotifications; +std::shared_ptr g_redisApiResponseNtf; std::shared_ptr g_redisClient; void clear_local_state() @@ -120,6 +121,7 @@ sai_status_t sai_api_initialize( g_asicState = std::make_shared(g_db.get(), ASIC_STATE_TABLE); g_redisGetConsumer = std::make_shared(g_db.get(), "GETRESPONSE"); g_redisNotifications = std::make_shared(g_dbNtf.get(), "NOTIFICATIONS"); + g_redisApiResponseNtf = std::make_shared(g_dbNtf.get(), "APIRESPONSENTF"); g_redisClient = std::make_shared(g_db.get()); clear_local_state(); diff --git a/syncd/syncd.cpp b/syncd/syncd.cpp index ac2593779a..34104c368b 100644 --- a/syncd/syncd.cpp +++ b/syncd/syncd.cpp @@ -29,6 +29,7 @@ std::mutex g_mutex; std::shared_ptr g_redisClient; std::shared_ptr getResponse; std::shared_ptr notifications; +std::shared_ptr apiResponseNotifications; /* * TODO: Those are hard coded values for mlnx integration for v1.0.1 they need @@ -74,6 +75,32 @@ volatile bool g_asicInitViewMode = false; */ sai_object_id_t gSwitchId; +void internal_syncd_status_send( + _In_ const std::string &key, + _In_ sai_status_t status, + _In_ sai_object_type_t object_type, + _In_ const std::string &op) +{ + SWSS_LOG_ENTER(); + if (!sai_return_obj_op_status(object_type)) + { + return; + } + + // Supporting SAI api call status return for these operations. + if (op != "create" && op != "set" && op != "remove" && op != "none") + { + return; + } + + std::string str_status = sai_serialize_status(status); + + SWSS_LOG_DEBUG("sending response for %s on %s with status: %s", op.c_str(), key.c_str(), str_status.c_str()); + + std::vector entry; + apiResponseNotifications->send(key, str_status, entry); +} + struct cmdOptions { bool diagShell; @@ -1160,7 +1187,8 @@ sai_status_t handle_generic( if (info->isnonobjectid) { - SWSS_LOG_THROW("passing non object id %s as generic object", info->objecttypename); + SWSS_LOG_ERROR("passing non object id %s as generic object", info->objecttypename); + return SAI_STATUS_INVALID_OBJECT_TYPE; } switch (api) @@ -1176,7 +1204,8 @@ sai_status_t handle_generic( if (switch_id == SAI_NULL_OBJECT_ID) { - SWSS_LOG_THROW("invalid switch_id translated from VID 0x%lx", object_id); + SWSS_LOG_ERROR("invalid switch_id translated from VID 0x%lx", object_id); + return SAI_STATUS_INVALID_OBJECT_ID; } if (object_type != SAI_OBJECT_TYPE_SWITCH) @@ -1198,8 +1227,8 @@ sai_status_t handle_generic( * NOTE: to support multiple switches we need support * here for create. */ - - SWSS_LOG_THROW("creating multiple switches is not supported yet, FIXME"); + SWSS_LOG_ERROR("creating multiple switches is not supported yet, FIXME"); + return SAI_STATUS_NOT_SUPPORTED; } } @@ -1338,8 +1367,8 @@ sai_status_t handle_generic( } default: - - SWSS_LOG_THROW("common api (%s) is not implemented", sai_serialize_common_api(api).c_str()); + SWSS_LOG_ERROR("common api (%s) is not implemented", sai_serialize_common_api(api).c_str()); + return SAI_STATUS_NOT_SUPPORTED; } } @@ -1393,7 +1422,8 @@ sai_status_t handle_non_object_id( return info->get(&meta_key, attr_count, attr_list); default: - SWSS_LOG_THROW("other apis not implemented"); + SWSS_LOG_ERROR("other apis not implemented"); + return SAI_STATUS_NOT_SUPPORTED; } } @@ -2335,6 +2365,7 @@ sai_status_t processEvent( const std::string &str_object_type = key.substr(0, key.find(":")); const std::string &str_object_id = key.substr(key.find(":") + 1); + sai_status_t status; SWSS_LOG_INFO("key: %s op: %s", key.c_str(), op.c_str()); @@ -2378,7 +2409,10 @@ sai_status_t processEvent( } else { - SWSS_LOG_THROW("api '%s' is not implemented", op.c_str()); + status = SAI_STATUS_NOT_IMPLEMENTED; + SWSS_LOG_ERROR("api '%s' is not implemented", op.c_str()); + internal_syncd_status_send(key, status, SAI_OBJECT_TYPE_MAX, "none"); + return status; } sai_object_type_t object_type; @@ -2390,7 +2424,10 @@ sai_status_t processEvent( if (object_type == SAI_OBJECT_TYPE_NULL || object_type >= SAI_OBJECT_TYPE_MAX) { - SWSS_LOG_THROW("undefined object type %s", sai_serialize_object_type(object_type).c_str()); + status = SAI_STATUS_INVALID_OBJECT_TYPE; + SWSS_LOG_ERROR("undefined object type %s", sai_serialize_object_type(object_type).c_str()); + internal_syncd_status_send(key, status, object_type, op); + return status; } const std::vector &values = kfvFieldsValues(kco); @@ -2430,7 +2467,9 @@ sai_status_t processEvent( if (isInitViewMode()) { - return processEventInInitViewMode(object_type, str_object_id, api, attr_count, attr_list); + status = processEventInInitViewMode(object_type, str_object_id, api, attr_count, attr_list); + internal_syncd_status_send(key, status, object_type, op); + return status; } if (api != SAI_COMMON_API_GET) @@ -2449,8 +2488,6 @@ sai_status_t processEvent( // TODO use metadata utils auto info = sai_metadata_get_object_type_info(object_type); - sai_status_t status; - /* * TODO use sai meta key deserialize */ @@ -2461,6 +2498,8 @@ sai_status_t processEvent( meta_key.objecttype = object_type; + status = SAI_STATUS_SUCCESS; + switch (object_type) { case SAI_OBJECT_TYPE_FDB_ENTRY: @@ -2476,11 +2515,13 @@ sai_status_t processEvent( break; default: - - SWSS_LOG_THROW("non object id %s is not supported yet, FIXME", info->objecttypename); + status = SAI_STATUS_NOT_SUPPORTED; + SWSS_LOG_ERROR("non object id %s is not supported yet, FIXME", info->objecttypename); + } + if (status == SAI_STATUS_SUCCESS) + { + status = handle_non_object_id(meta_key, api, attr_count, attr_list); } - - status = handle_non_object_id(meta_key, api, attr_count, attr_list); } else { @@ -2505,6 +2546,7 @@ sai_status_t processEvent( sai_object_id_t switch_vid = extractSwitchVid(object_type, str_object_id); internal_syncd_get_send(object_type, str_object_id, switch_vid, status, attr_count, attr_list); + return status; } else if (status != SAI_STATUS_SUCCESS) { @@ -2525,12 +2567,13 @@ sai_status_t processEvent( SWSS_LOG_ERROR("attr: %s: %s", fvField(v).c_str(), fvValue(v).c_str()); } - SWSS_LOG_THROW("failed to execute api: %s, key: %s, status: %s", + SWSS_LOG_ERROR("failed to execute api: %s, key: %s, status: %s", op.c_str(), key.c_str(), sai_serialize_status(status).c_str()); } + internal_syncd_status_send(key, status, object_type, op); return status; } @@ -3269,6 +3312,7 @@ int syncd_main(int argc, char **argv) getResponse = std::make_shared(dbAsic.get(), "GETRESPONSE"); notifications = std::make_shared(dbNtf.get(), "NOTIFICATIONS"); + apiResponseNotifications = std::make_shared(dbNtf.get(), "APIRESPONSENTF"); g_veryFirstRun = isVeryFirstRun();