From fc63dfc4b3082e3895d1f513bbbc0a2abc6a8788 Mon Sep 17 00:00:00 2001 From: kcudnik Date: Fri, 22 Jan 2021 14:53:11 +0100 Subject: [PATCH 1/4] [sairedis] Add get response timeout knob --- lib/inc/Channel.h | 9 +++++++++ lib/inc/RedisRemoteSaiInterface.h | 2 ++ lib/inc/sairedis.h | 11 +++++++++++ lib/src/Channel.cpp | 20 +++++++++++++++++++- lib/src/RedisChannel.cpp | 7 +------ lib/src/RedisRemoteSaiInterface.cpp | 18 ++++++++++++++++++ lib/src/ZeroMQChannel.cpp | 7 +------ 7 files changed, 61 insertions(+), 13 deletions(-) diff --git a/lib/inc/Channel.h b/lib/inc/Channel.h index 26fc2f5ec..266039ccf 100644 --- a/lib/inc/Channel.h +++ b/lib/inc/Channel.h @@ -28,6 +28,13 @@ namespace sairedis virtual ~Channel(); + public: + + void setResponseTimeout( + _In_ uint64_t responseTimeout); + + uint64_t getResponseTimeout() const; + public: virtual void setBuffered( @@ -56,6 +63,8 @@ namespace sairedis Callback m_callback; + uint64_t m_responseTimeoutMs; + protected: // notification /** diff --git a/lib/inc/RedisRemoteSaiInterface.h b/lib/inc/RedisRemoteSaiInterface.h index eb0ea9e78..4927e2321 100644 --- a/lib/inc/RedisRemoteSaiInterface.h +++ b/lib/inc/RedisRemoteSaiInterface.h @@ -465,6 +465,8 @@ namespace sairedis std::shared_ptr m_communicationChannel; + uint64_t m_responseTimeoutMs; + std::function)> m_notificationCallback; std::map m_tableDump; diff --git a/lib/inc/sairedis.h b/lib/inc/sairedis.h index fb24e4f14..74d907526 100644 --- a/lib/inc/sairedis.h +++ b/lib/inc/sairedis.h @@ -205,4 +205,15 @@ 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/Channel.cpp b/lib/src/Channel.cpp index 50b1e870f..8506d2fe3 100644 --- a/lib/src/Channel.cpp +++ b/lib/src/Channel.cpp @@ -4,9 +4,12 @@ using namespace sairedis; +#define REDIS_ASIC_STATE_COMMAND_GETRESPONSE_TIMEOUT_MS (60*1000) + Channel::Channel( _In_ Callback callback): - m_callback(callback) + m_callback(callback), + m_responseTimeoutMs(REDIS_ASIC_STATE_COMMAND_GETRESPONSE_TIMEOUT_MS) { SWSS_LOG_ENTER(); @@ -19,3 +22,18 @@ Channel::~Channel() // empty } + +void Channel::setResponseTimeout( + _In_ uint64_t responseTimeout) +{ + SWSS_LOG_ENTER(); + + m_responseTimeoutMs = responseTimeout; +} + +uint64_t Channel::getResponseTimeout() const +{ + SWSS_LOG_ENTER(); + + return m_responseTimeoutMs; +} diff --git a/lib/src/RedisChannel.cpp b/lib/src/RedisChannel.cpp index fa93465af..82f772a7e 100644 --- a/lib/src/RedisChannel.cpp +++ b/lib/src/RedisChannel.cpp @@ -9,11 +9,6 @@ using namespace sairedis; -/** - * @brief Get response timeout in milliseconds. - */ -#define REDIS_ASIC_STATE_COMMAND_GETRESPONSE_TIMEOUT_MS (60*1000) - RedisChannel::RedisChannel( _In_ const std::string& dbAsic, _In_ Channel::Callback callback): @@ -179,7 +174,7 @@ sai_status_t RedisChannel::wait( swss::Selectable *sel; - int result = s.select(&sel, REDIS_ASIC_STATE_COMMAND_GETRESPONSE_TIMEOUT_MS); + int result = s.select(&sel, (int)m_responseTimeoutMs); if (result == swss::Select::OBJECT) { diff --git a/lib/src/RedisRemoteSaiInterface.cpp b/lib/src/RedisRemoteSaiInterface.cpp index 574d357fe..3ec4a6002 100644 --- a/lib/src/RedisRemoteSaiInterface.cpp +++ b/lib/src/RedisRemoteSaiInterface.cpp @@ -92,6 +92,8 @@ sai_status_t RedisRemoteSaiInterface::initialize( std::bind(&RedisRemoteSaiInterface::handleNotification, this, _1, _2, _3)); } + m_responseTimeoutMs = m_communicationChannel->getResponseTimeout(); + m_db = std::make_shared(m_contextConfig->m_dbAsic, 0); m_redisVidIndexGenerator = std::make_shared(m_db, REDIS_KEY_VIDCOUNTER); @@ -355,6 +357,16 @@ sai_status_t RedisRemoteSaiInterface::setRedisExtensionAttribute( return SAI_STATUS_SUCCESS; + case SAI_REDIS_SWITCH_ATTR_GET_RESPONSE_TIMEOUT_MS: + + m_responseTimeoutMs = attr->value.u64; + + m_communicationChannel->setResponseTimeout(m_responseTimeoutMs); + + SWSS_LOG_NOTICE("set response timeout to %lu ms", m_responseTimeoutMs); + + return SAI_STATUS_SUCCESS; + case SAI_REDIS_SWITCH_ATTR_SYNC_MODE: SWSS_LOG_WARN("sync mode is depreacated, use communication mode"); @@ -402,6 +414,8 @@ sai_status_t RedisRemoteSaiInterface::setRedisExtensionAttribute( m_contextConfig->m_dbAsic, std::bind(&RedisRemoteSaiInterface::handleNotification, this, _1, _2, _3)); + m_communicationChannel->setResponseTimeout(m_responseTimeoutMs); + m_communicationChannel->setBuffered(true); return SAI_STATUS_SUCCESS; @@ -416,6 +430,8 @@ sai_status_t RedisRemoteSaiInterface::setRedisExtensionAttribute( m_contextConfig->m_dbAsic, std::bind(&RedisRemoteSaiInterface::handleNotification, this, _1, _2, _3)); + m_communicationChannel->setResponseTimeout(m_responseTimeoutMs); + m_communicationChannel->setBuffered(false); return SAI_STATUS_SUCCESS; @@ -432,6 +448,8 @@ sai_status_t RedisRemoteSaiInterface::setRedisExtensionAttribute( m_contextConfig->m_zmqNtfEndpoint, std::bind(&RedisRemoteSaiInterface::handleNotification, this, _1, _2, _3)); + m_communicationChannel->setResponseTimeout(m_responseTimeoutMs); + SWSS_LOG_NOTICE("zmq enabled, forcing sync mode"); m_syncMode = true; diff --git a/lib/src/ZeroMQChannel.cpp b/lib/src/ZeroMQChannel.cpp index 4808d354d..aa6dfcae0 100644 --- a/lib/src/ZeroMQChannel.cpp +++ b/lib/src/ZeroMQChannel.cpp @@ -12,11 +12,6 @@ using namespace sairedis; -/** - * @brief Get response timeout in milliseconds. - */ -#define ZMQ_GETRESPONSE_TIMEOUT_MS (60*1000) - #define ZMQ_RESPONSE_BUFFER_SIZE (4*1024*1024) ZeroMQChannel::ZeroMQChannel( @@ -273,7 +268,7 @@ sai_status_t ZeroMQChannel::wait( items[0].socket = m_socket; items[0].events = ZMQ_POLLIN; - int rc = zmq_poll(items, 1, ZMQ_GETRESPONSE_TIMEOUT_MS); + int rc = zmq_poll(items, 1, m_responseTimeoutMs); if (rc == 0) { From da9ac06b40b4edccd3b1afdcd939b9054c41467a Mon Sep 17 00:00:00 2001 From: kcudnik Date: Fri, 22 Jan 2021 16:52:52 +0100 Subject: [PATCH 2/4] [sairedis] Force cast for arm architecture --- lib/src/ZeroMQChannel.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/ZeroMQChannel.cpp b/lib/src/ZeroMQChannel.cpp index aa6dfcae0..c1cf9c65a 100644 --- a/lib/src/ZeroMQChannel.cpp +++ b/lib/src/ZeroMQChannel.cpp @@ -268,7 +268,7 @@ sai_status_t ZeroMQChannel::wait( items[0].socket = m_socket; items[0].events = ZMQ_POLLIN; - int rc = zmq_poll(items, 1, m_responseTimeoutMs); + int rc = zmq_poll(items, 1, (int)m_responseTimeoutMs); if (rc == 0) { From f9592d5cd1f622570c319988218f0c5438ef414c Mon Sep 17 00:00:00 2001 From: kcudnik Date: Tue, 2 Feb 2021 16:56:35 +0100 Subject: [PATCH 3/4] Move timeout define to sairedis.h --- lib/inc/sairedis.h | 12 +++++++++--- lib/src/Channel.cpp | 6 +++--- lib/src/RedisRemoteSaiInterface.cpp | 2 +- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/inc/sairedis.h b/lib/inc/sairedis.h index 74d907526..45ccee7fb 100644 --- a/lib/inc/sairedis.h +++ b/lib/inc/sairedis.h @@ -13,6 +13,11 @@ extern "C" { */ #define SAI_REDIS_KEY_CONTEXT_CONFIG "SAI_REDIS_CONTEXT_CONFIG" +/** + * @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, @@ -206,14 +211,15 @@ 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/Channel.cpp b/lib/src/Channel.cpp index 8506d2fe3..9df5919e0 100644 --- a/lib/src/Channel.cpp +++ b/lib/src/Channel.cpp @@ -1,15 +1,15 @@ #include "Channel.h" +#include "sairedis.h" + #include "swss/logger.h" using namespace sairedis; -#define REDIS_ASIC_STATE_COMMAND_GETRESPONSE_TIMEOUT_MS (60*1000) - Channel::Channel( _In_ Callback callback): m_callback(callback), - m_responseTimeoutMs(REDIS_ASIC_STATE_COMMAND_GETRESPONSE_TIMEOUT_MS) + m_responseTimeoutMs(SAI_REDIS_DEFAULT_SYNC_OPERATION_RESPONSE_TIMEOUT) { SWSS_LOG_ENTER(); diff --git a/lib/src/RedisRemoteSaiInterface.cpp b/lib/src/RedisRemoteSaiInterface.cpp index 3ec4a6002..d9255276e 100644 --- a/lib/src/RedisRemoteSaiInterface.cpp +++ b/lib/src/RedisRemoteSaiInterface.cpp @@ -357,7 +357,7 @@ sai_status_t RedisRemoteSaiInterface::setRedisExtensionAttribute( return SAI_STATUS_SUCCESS; - case SAI_REDIS_SWITCH_ATTR_GET_RESPONSE_TIMEOUT_MS: + case SAI_REDIS_SWITCH_ATTR_SYNC_OPERATION_RESPONSE_TIMEOUT: m_responseTimeoutMs = attr->value.u64; From 317cf462f4d6a86cb5657666ac8a4bdeb4ba7de8 Mon Sep 17 00:00:00 2001 From: kcudnik Date: Tue, 2 Feb 2021 18:23:35 +0100 Subject: [PATCH 4/4] 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 45ccee7fb..7445af569 100644 --- a/lib/inc/sairedis.h +++ b/lib/inc/sairedis.h @@ -217,7 +217,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,