From cf6cbd403eeebf2f5d7e39ba2ab57016f06e8a4d Mon Sep 17 00:00:00 2001 From: kcudnik Date: Mon, 25 Jan 2021 16:49:54 +0100 Subject: [PATCH 1/3] [sairedis] Add get response timeout knob Signed-off-by: kcudnik --- lib/inc/sai_redis.h | 5 +---- lib/inc/sairedis.h | 10 ++++++++++ lib/src/sai_redis_fdb.cpp | 2 +- lib/src/sai_redis_generic_get.cpp | 2 +- lib/src/sai_redis_generic_set.cpp | 2 +- lib/src/sai_redis_generic_stats.cpp | 4 ++-- lib/src/sai_redis_interfacequery.cpp | 4 ++-- lib/src/sai_redis_switch.cpp | 12 +++++++++++- 8 files changed, 29 insertions(+), 12 deletions(-) diff --git a/lib/inc/sai_redis.h b/lib/inc/sai_redis.h index a298e3da55..f8d268b1b8 100644 --- a/lib/inc/sai_redis.h +++ b/lib/inc/sai_redis.h @@ -41,10 +41,6 @@ void check_notifications_pointers( _In_ uint32_t attr_count, _In_ const sai_attribute_t *attr_list); -// if we don't receive response from syncd in 60 seconds -// there is something wrong and we should fail -#define GET_RESPONSE_TIMEOUT (60*1000) - extern std::string getSelectResultAsString(int result); extern void clear_local_state(); extern void setRecording(bool record); @@ -66,6 +62,7 @@ extern volatile bool g_useTempView; extern volatile bool g_asicInitViewMode; extern volatile bool g_logrotate; extern volatile bool g_syncMode; +extern volatile uint64_t g_responseTimeoutMs; extern sai_service_method_table_t g_services; extern std::shared_ptr g_asicState; diff --git a/lib/inc/sairedis.h b/lib/inc/sairedis.h index 3cafa4baf5..7036a158f1 100644 --- a/lib/inc/sairedis.h +++ b/lib/inc/sairedis.h @@ -134,6 +134,16 @@ typedef enum _sai_redis_switch_attr_t */ SAI_REDIS_SWITCH_ATTR_RECORDING_FILENAME, + /** + * @brief Get operation response timeout in milliseconds. + * + * Also used for every synchronous API call. + * + * @type sai_uint64_t + * @flgs CREATE_AND_SET + * @default 60000 + */ + SAI_REDIS_SWITCH_ATTR_GET_RESPONSE_TIMEOUT_MS, } sai_redis_switch_attr_t; diff --git a/lib/src/sai_redis_fdb.cpp b/lib/src/sai_redis_fdb.cpp index 684a72d624..72b380a879 100644 --- a/lib/src/sai_redis_fdb.cpp +++ b/lib/src/sai_redis_fdb.cpp @@ -44,7 +44,7 @@ sai_status_t internal_redis_flush_fdb_entries( swss::Selectable *sel; - int result = s.select(&sel, GET_RESPONSE_TIMEOUT); + int result = s.select(&sel, (int)g_responseTimeoutMs); if (result == swss::Select::OBJECT) { diff --git a/lib/src/sai_redis_generic_get.cpp b/lib/src/sai_redis_generic_get.cpp index f750ac8477..15dcd829c0 100644 --- a/lib/src/sai_redis_generic_get.cpp +++ b/lib/src/sai_redis_generic_get.cpp @@ -202,7 +202,7 @@ sai_status_t internal_redis_generic_get( swss::Selectable *sel; - int result = s.select(&sel, GET_RESPONSE_TIMEOUT); + int result = s.select(&sel, (int)g_responseTimeoutMs); if (result == swss::Select::OBJECT) { diff --git a/lib/src/sai_redis_generic_set.cpp b/lib/src/sai_redis_generic_set.cpp index 4b7637a11a..84087cd70a 100644 --- a/lib/src/sai_redis_generic_set.cpp +++ b/lib/src/sai_redis_generic_set.cpp @@ -30,7 +30,7 @@ sai_status_t internal_api_wait_for_response( swss::Selectable *sel; // get timeout and selector is used for all quad api's - int result = s.select(&sel, GET_RESPONSE_TIMEOUT); + int result = s.select(&sel, (int)g_responseTimeoutMs); if (result == swss::Select::OBJECT) { diff --git a/lib/src/sai_redis_generic_stats.cpp b/lib/src/sai_redis_generic_stats.cpp index 14e98633b4..82ea935a9a 100644 --- a/lib/src/sai_redis_generic_stats.cpp +++ b/lib/src/sai_redis_generic_stats.cpp @@ -289,7 +289,7 @@ sai_status_t internal_redis_generic_get_stats( swss::Selectable *sel; - int result = s.select(&sel, GET_RESPONSE_TIMEOUT); + int result = s.select(&sel, (int)g_responseTimeoutMs); if (result == swss::Select::OBJECT) { @@ -419,7 +419,7 @@ sai_status_t internal_redis_generic_clear_stats( SWSS_LOG_DEBUG("wait for clear_stats response"); swss::Selectable *sel; - int result = s.select(&sel, GET_RESPONSE_TIMEOUT); + int result = s.select(&sel, (int)g_responseTimeoutMs); if (result == swss::Select::OBJECT) { swss::KeyOpFieldsValuesTuple kco; diff --git a/lib/src/sai_redis_interfacequery.cpp b/lib/src/sai_redis_interfacequery.cpp index f0ddd90d13..8e0f6643bc 100644 --- a/lib/src/sai_redis_interfacequery.cpp +++ b/lib/src/sai_redis_interfacequery.cpp @@ -319,7 +319,7 @@ sai_status_t sai_query_attribute_enum_values_capability( swss::Selectable *sel; - auto result = callback.select(&sel, GET_RESPONSE_TIMEOUT); + auto result = callback.select(&sel, (int)g_responseTimeoutMs); if (result == swss::Select::OBJECT) { @@ -460,7 +460,7 @@ sai_status_t sai_object_type_get_availability( swss::Selectable *sel; - auto result = callback.select(&sel, GET_RESPONSE_TIMEOUT); + auto result = callback.select(&sel, (int)g_responseTimeoutMs); if (result == swss::Select::OBJECT) { diff --git a/lib/src/sai_redis_switch.cpp b/lib/src/sai_redis_switch.cpp index bf99cfafef..209d8ea322 100644 --- a/lib/src/sai_redis_switch.cpp +++ b/lib/src/sai_redis_switch.cpp @@ -5,9 +5,12 @@ #include +#define REDIS_ASIC_STATE_COMMAND_GETRESPONSE_TIMEOUT_MS (60*1000) + volatile bool g_asicInitViewMode = false; // default mode is apply mode volatile bool g_useTempView = false; volatile bool g_syncMode = false; +volatile uint64_t g_responseTimeoutMs = REDIS_ASIC_STATE_COMMAND_GETRESPONSE_TIMEOUT_MS; sai_status_t sai_redis_internal_notify_syncd( _In_ const std::string& key) @@ -36,7 +39,7 @@ sai_status_t sai_redis_internal_notify_syncd( swss::Selectable *sel; - int result = s.select(&sel, GET_RESPONSE_TIMEOUT); + int result = s.select(&sel, (int)g_responseTimeoutMs); if (result == swss::Select::OBJECT) { @@ -299,6 +302,13 @@ sai_status_t redis_set_switch_attribute( case SAI_REDIS_SWITCH_ATTR_RECORDING_FILENAME: return setRecordingOutputFile(*attr); + + case SAI_REDIS_SWITCH_ATTR_GET_RESPONSE_TIMEOUT_MS: + + g_responseTimeoutMs = attr->value.u64; + + return SAI_STATUS_SUCCESS; + default: break; } From 4cf275117280e10107fd1ccddbe61bba6d3dbf9c Mon Sep 17 00:00:00 2001 From: kcudnik Date: Tue, 2 Feb 2021 16:54:47 +0100 Subject: [PATCH 2/3] Move timeout define to sairedis.h --- lib/inc/sairedis.h | 12 +++++++++--- lib/src/sai_redis_switch.cpp | 6 ++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/inc/sairedis.h b/lib/inc/sairedis.h index 7036a158f1..6f44514735 100644 --- a/lib/inc/sairedis.h +++ b/lib/inc/sairedis.h @@ -17,6 +17,11 @@ const std::string attrEnumValuesCapabilityResponse("attr_enum_values_capability_ const std::string objectTypeGetAvailabilityQuery("object_type_get_availability_query"); const std::string objectTypeGetAvailabilityResponse("object_type_get_availability_response"); +/** + * @brief Default synchronous operation response timeout in milliseconds. + */ +#define SAI_REDIS_DEFAULT_SYNC_OPERATION_RESPONSE_TIMEOUT (60*1000) + typedef enum _sai_redis_notify_syncd_t { SAI_REDIS_NOTIFY_SYNCD_INIT_VIEW, @@ -135,15 +140,16 @@ typedef enum _sai_redis_switch_attr_t SAI_REDIS_SWITCH_ATTR_RECORDING_FILENAME, /** - * @brief Get operation response timeout in milliseconds. + * @brief Synchronous operation response timeout in milliseconds. * - * Also used for every synchronous API call. + * Used for every synchronous API call. In asynchronous mode used for GET + * operation. * * @type sai_uint64_t * @flgs CREATE_AND_SET * @default 60000 */ - SAI_REDIS_SWITCH_ATTR_GET_RESPONSE_TIMEOUT_MS, + SAI_REDIS_SWITCH_ATTR_SYNC_OPERATION_RESPONSE_TIMEOUT, } sai_redis_switch_attr_t; diff --git a/lib/src/sai_redis_switch.cpp b/lib/src/sai_redis_switch.cpp index 209d8ea322..5fbc5b78a4 100644 --- a/lib/src/sai_redis_switch.cpp +++ b/lib/src/sai_redis_switch.cpp @@ -5,12 +5,10 @@ #include -#define REDIS_ASIC_STATE_COMMAND_GETRESPONSE_TIMEOUT_MS (60*1000) - volatile bool g_asicInitViewMode = false; // default mode is apply mode volatile bool g_useTempView = false; volatile bool g_syncMode = false; -volatile uint64_t g_responseTimeoutMs = REDIS_ASIC_STATE_COMMAND_GETRESPONSE_TIMEOUT_MS; +volatile uint64_t g_responseTimeoutMs = SAI_REDIS_DEFAULT_SYNC_OPERATION_RESPONSE_TIMEOUT; sai_status_t sai_redis_internal_notify_syncd( _In_ const std::string& key) @@ -303,7 +301,7 @@ sai_status_t redis_set_switch_attribute( case SAI_REDIS_SWITCH_ATTR_RECORDING_FILENAME: return setRecordingOutputFile(*attr); - case SAI_REDIS_SWITCH_ATTR_GET_RESPONSE_TIMEOUT_MS: + case SAI_REDIS_SWITCH_ATTR_SYNC_OPERATION_RESPONSE_TIMEOUT: g_responseTimeoutMs = attr->value.u64; From b1927a35e6a471d84b96e6e5feb308f656426073 Mon Sep 17 00:00:00 2001 From: kcudnik Date: Tue, 2 Feb 2021 18:21:52 +0100 Subject: [PATCH 3/3] Fix spell --- lib/inc/sairedis.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/inc/sairedis.h b/lib/inc/sairedis.h index 6f44514735..703c3f05d2 100644 --- a/lib/inc/sairedis.h +++ b/lib/inc/sairedis.h @@ -146,7 +146,7 @@ typedef enum _sai_redis_switch_attr_t * operation. * * @type sai_uint64_t - * @flgs CREATE_AND_SET + * @flags CREATE_AND_SET * @default 60000 */ SAI_REDIS_SWITCH_ATTR_SYNC_OPERATION_RESPONSE_TIMEOUT,