From 352234ae773dc6010ea2fd3b3446ced48f33ee0a Mon Sep 17 00:00:00 2001 From: Vijaya Kumar Abbaraju Date: Sat, 12 Oct 2024 02:58:21 +0530 Subject: [PATCH 01/19] Schema.h Changes to support PAC functionality. (#871) #### Why I did it Added below tables to support PAC functionality. #define APP_PAC_PORT_TABLE_NAME "PAC_PORT_TABLE" #define CFG_PAC_PORT_CONFIG_TABLE "PAC_PORT_CONFIG_TABLE" #define CFG_PAC_GLOBAL_CONFIG_TABLE "PAC_GLOBAL_CONFIG_TABLE" #define CFG_PAC_HOSTAPD_GLOBAL_CONFIG_TABLE "HOSTAPD_GLOBAL_CONFIG_TABLE" #define STATE_PAC_GLOBAL_OPER_TABLE "PAC_GLOBAL_OPER_TABLE" #define STATE_PAC_PORT_OPER_TABLE "PAC_PORT_OPER_TABLE" #define STATE_PAC_AUTHENTICATED_CLIENT_OPER_TABLE "PAC_AUTHENTICATED_CLIENT_OPER_TABLE" #define STATE_OPER_VLAN_TABLE_NAME "OPER_VLAN" #define STATE_OPER_VLAN_MEMBER_TABLE_NAME "OPER_VLAN_MEMBER" #define STATE_OPER_FDB_TABLE_NAME "OPER_FDB" #define STATE_OPER_PORT_TABLE_NAME "OPER_PORT" --- common/schema.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/common/schema.h b/common/schema.h index 48d4d5d83..27aecdb0c 100644 --- a/common/schema.h +++ b/common/schema.h @@ -176,6 +176,8 @@ namespace swss { #define APP_DASH_PA_VALIDATION_TABLE_NAME "DASH_PA_VALIDATION_TABLE" #define APP_DASH_ROUTING_APPLIANCE_TABLE_NAME "DASH_ROUTING_APPLIANCE_TABLE" +#define APP_PAC_PORT_TABLE_NAME "PAC_PORT_TABLE" + /***** TO BE REMOVED *****/ #define APP_TC_TO_QUEUE_MAP_TABLE_NAME "TC_TO_QUEUE_MAP_TABLE" @@ -465,6 +467,11 @@ namespace swss { #define CFG_SUPPRESS_ASIC_SDK_HEALTH_EVENT_NAME "SUPPRESS_ASIC_SDK_HEALTH_EVENT" +#define CFG_PAC_PORT_CONFIG_TABLE "PAC_PORT_CONFIG_TABLE" +#define CFG_PAC_GLOBAL_CONFIG_TABLE "PAC_GLOBAL_CONFIG_TABLE" +#define CFG_PAC_HOSTAPD_GLOBAL_CONFIG_TABLE "HOSTAPD_GLOBAL_CONFIG_TABLE" + + /***** STATE DATABASE *****/ #define STATE_SWITCH_CAPABILITY_TABLE_NAME "SWITCH_CAPABILITY" @@ -549,6 +556,16 @@ namespace swss { #define STATE_ACL_TABLE_TABLE_NAME "ACL_TABLE_TABLE" #define STATE_ACL_RULE_TABLE_NAME "ACL_RULE_TABLE" + +/*PAC*/ +#define STATE_PAC_GLOBAL_OPER_TABLE "PAC_GLOBAL_OPER_TABLE" +#define STATE_PAC_PORT_OPER_TABLE "PAC_PORT_OPER_TABLE" +#define STATE_PAC_AUTHENTICATED_CLIENT_OPER_TABLE "PAC_AUTHENTICATED_CLIENT_OPER_TABLE" +#define STATE_OPER_VLAN_TABLE_NAME "OPER_VLAN" +#define STATE_OPER_VLAN_MEMBER_TABLE_NAME "OPER_VLAN_MEMBER" +#define STATE_OPER_FDB_TABLE_NAME "OPER_FDB" +#define STATE_OPER_PORT_TABLE_NAME "OPER_PORT" + /***** Counter capability tables for Queue and Port ****/ #define STATE_QUEUE_COUNTER_CAPABILITIES_NAME "QUEUE_COUNTER_CAPABILITIES" #define STATE_PORT_COUNTER_CAPABILITIES_NAME "PORT_COUNTER_CAPABILITIES" From 45d7cb010e709d340da8a2a21e1efcea239a8499 Mon Sep 17 00:00:00 2001 From: erer1243 <1377477+erer1243@users.noreply.github.com> Date: Tue, 29 Oct 2024 20:23:24 -0400 Subject: [PATCH 02/19] Initial implementation of C api (#915) Implement a C interface to some of libswsscommon in support of sonic-dash-ha. Related: https://github.com/sonic-net/sonic-dash-ha/pull/6 https://github.com/sonic-net/sonic-swss-common/pull/921 Incoming follow up PR: https://github.com/erer1243/sonic-swss-common/pull/1 --------- Co-authored-by: erer1243 --- common/Makefile.am | 11 +- common/binaryserializer.h | 26 +- common/c-api/consumerstatetable.cpp | 34 +++ common/c-api/consumerstatetable.h | 28 +++ common/c-api/dbconnector.cpp | 84 +++++++ common/c-api/dbconnector.h | 101 ++++++++ common/c-api/producerstatetable.cpp | 53 ++++ common/c-api/producerstatetable.h | 44 ++++ common/c-api/subscriberstatetable.cpp | 52 ++++ common/c-api/subscriberstatetable.h | 43 ++++ common/c-api/util.cpp | 3 + common/c-api/util.h | 181 ++++++++++++++ common/c-api/zmqclient.cpp | 35 +++ common/c-api/zmqclient.h | 30 +++ common/c-api/zmqconsumerstatetable.cpp | 58 +++++ common/c-api/zmqconsumerstatetable.h | 48 ++++ common/c-api/zmqproducerstatetable.cpp | 29 +++ common/c-api/zmqproducerstatetable.h | 32 +++ common/c-api/zmqserver.cpp | 14 ++ common/c-api/zmqserver.h | 20 ++ common/dbconnector.cpp | 33 ++- common/dbconnector.h | 10 +- common/zmqserver.cpp | 5 +- debian/libswsscommon-dev.install | 1 + tests/Makefile.am | 1 + tests/c_api_ut.cpp | 325 +++++++++++++++++++++++++ tests/main.cpp | 4 + 27 files changed, 1282 insertions(+), 23 deletions(-) create mode 100644 common/c-api/consumerstatetable.cpp create mode 100644 common/c-api/consumerstatetable.h create mode 100644 common/c-api/dbconnector.cpp create mode 100644 common/c-api/dbconnector.h create mode 100644 common/c-api/producerstatetable.cpp create mode 100644 common/c-api/producerstatetable.h create mode 100644 common/c-api/subscriberstatetable.cpp create mode 100644 common/c-api/subscriberstatetable.h create mode 100644 common/c-api/util.cpp create mode 100644 common/c-api/util.h create mode 100644 common/c-api/zmqclient.cpp create mode 100644 common/c-api/zmqclient.h create mode 100644 common/c-api/zmqconsumerstatetable.cpp create mode 100644 common/c-api/zmqconsumerstatetable.h create mode 100644 common/c-api/zmqproducerstatetable.cpp create mode 100644 common/c-api/zmqproducerstatetable.h create mode 100644 common/c-api/zmqserver.cpp create mode 100644 common/c-api/zmqserver.h create mode 100644 tests/c_api_ut.cpp diff --git a/common/Makefile.am b/common/Makefile.am index 18cfd8035..5d1de753e 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -68,7 +68,16 @@ common_libswsscommon_la_SOURCES = \ common/zmqclient.cpp \ common/zmqserver.cpp \ common/asyncdbupdater.cpp \ - common/redis_table_waiter.cpp + common/redis_table_waiter.cpp \ + common/c-api/util.cpp \ + common/c-api/dbconnector.cpp \ + common/c-api/consumerstatetable.cpp \ + common/c-api/producerstatetable.cpp \ + common/c-api/subscriberstatetable.cpp \ + common/c-api/zmqclient.cpp \ + common/c-api/zmqserver.cpp \ + common/c-api/zmqconsumerstatetable.cpp \ + common/c-api/zmqproducerstatetable.cpp common_libswsscommon_la_CXXFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CFLAGS) $(CODE_COVERAGE_CXXFLAGS) common_libswsscommon_la_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CPPFLAGS) $(CODE_COVERAGE_CPPFLAGS) diff --git a/common/binaryserializer.h b/common/binaryserializer.h index 413ca5010..6ae4dcd25 100644 --- a/common/binaryserializer.h +++ b/common/binaryserializer.h @@ -2,6 +2,8 @@ #define __BINARY_SERIALIZER__ #include "common/armhelper.h" +#include "common/rediscommand.h" +#include "common/table.h" #include @@ -11,6 +13,26 @@ namespace swss { class BinarySerializer { public: + static size_t serializedSize(const string &dbName, const string &tableName, + const vector &kcos) { + size_t n = 0; + n += dbName.size() + sizeof(size_t); + n += tableName.size() + sizeof(size_t); + + for (const KeyOpFieldsValuesTuple &kco : kcos) { + const vector &fvs = kfvFieldsValues(kco); + n += kfvKey(kco).size() + sizeof(size_t); + n += to_string(fvs.size()).size() + sizeof(size_t); + + for (const FieldValueTuple &fv : fvs) { + n += fvField(fv).size() + sizeof(size_t); + n += fvValue(fv).size() + sizeof(size_t); + } + } + + return n + sizeof(size_t); + } + static size_t serializeBuffer( const char* buffer, const size_t size, @@ -192,8 +214,8 @@ class BinarySerializer { { if ((size_t)(m_current_position - m_buffer + datalen + sizeof(size_t)) > m_buffer_size) { - SWSS_LOG_THROW("There are not enough buffer for binary serializer to serialize,\ - key count: %zu, data length %zu, buffer size: %zu", + SWSS_LOG_THROW("There are not enough buffer for binary serializer to serialize,\n" + " key count: %zu, data length %zu, buffer size: %zu", m_kvp_count, datalen, m_buffer_size); diff --git a/common/c-api/consumerstatetable.cpp b/common/c-api/consumerstatetable.cpp new file mode 100644 index 000000000..c01ed8229 --- /dev/null +++ b/common/c-api/consumerstatetable.cpp @@ -0,0 +1,34 @@ +#include +#include +#include + +#include "../consumerstatetable.h" +#include "../dbconnector.h" +#include "../table.h" +#include "consumerstatetable.h" +#include "util.h" + +using namespace swss; +using namespace std; + +SWSSConsumerStateTable SWSSConsumerStateTable_new(SWSSDBConnector db, const char *tableName, + const int32_t *p_popBatchSize, + const int32_t *p_pri) { + int popBatchSize = p_popBatchSize ? numeric_cast(*p_popBatchSize) + : TableConsumable::DEFAULT_POP_BATCH_SIZE; + int pri = p_pri ? numeric_cast(*p_pri) : 0; + SWSSTry(return (SWSSConsumerStateTable) new ConsumerStateTable( + (DBConnector *)db, string(tableName), popBatchSize, pri)); +} + +void SWSSConsumerStateTable_free(SWSSConsumerStateTable tbl) { + SWSSTry(delete (ConsumerStateTable *)tbl); +} + +SWSSKeyOpFieldValuesArray SWSSConsumerStateTable_pops(SWSSConsumerStateTable tbl) { + SWSSTry({ + deque vkco; + ((ConsumerStateTable *)tbl)->pops(vkco); + return makeKeyOpFieldValuesArray(vkco); + }); +} diff --git a/common/c-api/consumerstatetable.h b/common/c-api/consumerstatetable.h new file mode 100644 index 000000000..bd2fdaaf0 --- /dev/null +++ b/common/c-api/consumerstatetable.h @@ -0,0 +1,28 @@ +#ifndef SWSS_COMMON_C_API_CONSUMERSTATETABLE_H +#define SWSS_COMMON_C_API_CONSUMERSTATETABLE_H + +#include "dbconnector.h" +#include "util.h" + +#ifdef __cplusplus +extern "C" { +#endif + +#include + +typedef struct SWSSConsumerStateTableOpaque *SWSSConsumerStateTable; + +// Pass NULL for popBatchSize and/or pri to use the default values +SWSSConsumerStateTable SWSSConsumerStateTable_new(SWSSDBConnector db, const char *tableName, + const int32_t *popBatchSize, const int32_t *pri); + +void SWSSConsumerStateTable_free(SWSSConsumerStateTable tbl); + +// Result array and all of its members must be freed using free() +SWSSKeyOpFieldValuesArray SWSSConsumerStateTable_pops(SWSSConsumerStateTable tbl); + +#ifdef __cplusplus +} +#endif + +#endif diff --git a/common/c-api/dbconnector.cpp b/common/c-api/dbconnector.cpp new file mode 100644 index 000000000..bb32f42aa --- /dev/null +++ b/common/c-api/dbconnector.cpp @@ -0,0 +1,84 @@ +#include +#include + +#include "../dbconnector.h" +#include "dbconnector.h" +#include "util.h" + +using namespace swss; +using namespace std; + +void SWSSSonicDBConfig_initialize(const char *path) { + SWSSTry(SonicDBConfig::initialize(path)); +} + +void SWSSSonicDBConfig_initializeGlobalConfig(const char *path) { + SWSSTry(SonicDBConfig::initializeGlobalConfig(path)); +} + +SWSSDBConnector SWSSDBConnector_new_tcp(int32_t dbId, const char *hostname, uint16_t port, + uint32_t timeout) { + SWSSTry(return (SWSSDBConnector) new DBConnector(dbId, string(hostname), port, timeout)); +} + +SWSSDBConnector SWSSDBConnector_new_unix(int32_t dbId, const char *sock_path, uint32_t timeout) { + SWSSTry(return (SWSSDBConnector) new DBConnector(dbId, string(sock_path), timeout)); +} + +SWSSDBConnector SWSSDBConnector_new_named(const char *dbName, uint32_t timeout_ms, uint8_t isTcpConn) { + SWSSTry(return (SWSSDBConnector) new DBConnector(string(dbName), timeout_ms, isTcpConn)); +} + +void SWSSDBConnector_free(SWSSDBConnector db) { + delete (DBConnector *)db; +} + +int8_t SWSSDBConnector_del(SWSSDBConnector db, const char *key) { + SWSSTry(return ((DBConnector *)db)->del(string(key)) ? 1 : 0); +} + +void SWSSDBConnector_set(SWSSDBConnector db, const char *key, const char *value) { + SWSSTry(((DBConnector *)db)->set(string(key), string(value))); +} + +char *SWSSDBConnector_get(SWSSDBConnector db, const char *key) { + SWSSTry({ + shared_ptr s = ((DBConnector *)db)->get(string(key)); + return s ? strdup(s->c_str()) : nullptr; + }); +} + +int8_t SWSSDBConnector_exists(SWSSDBConnector db, const char *key) { + SWSSTry(return ((DBConnector *)db)->exists(string(key)) ? 1 : 0); +} + +int8_t SWSSDBConnector_hdel(SWSSDBConnector db, const char *key, const char *field) { + SWSSTry(return ((DBConnector *)db)->hdel(string(key), string(field)) ? 1 : 0); +} + +void SWSSDBConnector_hset(SWSSDBConnector db, const char *key, const char *field, + const char *value) { + SWSSTry(((DBConnector *)db)->hset(string(key), string(field), string(value))); +} + +char *SWSSDBConnector_hget(SWSSDBConnector db, const char *key, const char *field) { + SWSSTry({ + shared_ptr s = ((DBConnector *)db)->hget(string(key), string(field)); + return s ? strdup(s->c_str()) : nullptr; + }); +} + +SWSSFieldValueArray SWSSDBConnector_hgetall(SWSSDBConnector db, const char *key) { + SWSSTry({ + auto map = ((DBConnector *)db)->hgetall(key); + return makeFieldValueArray(map); + }); +} + +int8_t SWSSDBConnector_hexists(SWSSDBConnector db, const char *key, const char *field) { + SWSSTry(return ((DBConnector *)db)->hexists(string(key), string(field)) ? 1 : 0); +} + +int8_t SWSSDBConnector_flushdb(SWSSDBConnector db) { + SWSSTry(return ((DBConnector *)db)->flushdb() ? 1 : 0); +} diff --git a/common/c-api/dbconnector.h b/common/c-api/dbconnector.h new file mode 100644 index 000000000..8e6c51e0b --- /dev/null +++ b/common/c-api/dbconnector.h @@ -0,0 +1,101 @@ +#ifndef SWSS_COMMON_C_API_DBCONNECTOR_H +#define SWSS_COMMON_C_API_DBCONNECTOR_H + +#include "util.h" +#ifdef __cplusplus +extern "C" { +#endif + +#include + +void SWSSSonicDBConfig_initialize(const char *path); + +void SWSSSonicDBConfig_initializeGlobalConfig(const char *path); + +typedef struct SWSSDBConnectorOpaque *SWSSDBConnector; + +// Pass 0 to timeout for infinity +SWSSDBConnector SWSSDBConnector_new_tcp(int32_t dbId, const char *hostname, uint16_t port, + uint32_t timeout_ms); + +// Pass 0 to timeout for infinity +SWSSDBConnector SWSSDBConnector_new_unix(int32_t dbId, const char *sock_path, uint32_t timeout_ms); + +// Pass 0 to timeout for infinity +SWSSDBConnector SWSSDBConnector_new_named(const char *dbName, uint32_t timeout_ms, uint8_t isTcpConn); + +void SWSSDBConnector_free(SWSSDBConnector db); + +// Returns 0 when key doesn't exist, 1 when key was deleted +int8_t SWSSDBConnector_del(SWSSDBConnector db, const char *key); + +void SWSSDBConnector_set(SWSSDBConnector db, const char *key, const char *value); + +// Returns NULL if key doesn't exist. +// Result must be freed using free() +char *SWSSDBConnector_get(SWSSDBConnector db, const char *key); + +// Returns 0 for false, 1 for true +int8_t SWSSDBConnector_exists(SWSSDBConnector db, const char *key); + +// Returns 0 when key or field doesn't exist, 1 when field was deleted +int8_t SWSSDBConnector_hdel(SWSSDBConnector db, const char *key, const char *field); + +void SWSSDBConnector_hset(SWSSDBConnector db, const char *key, const char *field, + const char *value); + +// Returns NULL if key or field doesn't exist. +// Result must be freed using free() +char *SWSSDBConnector_hget(SWSSDBConnector db, const char *key, const char *field); + +// Returns an empty map when the key doesn't exist. +// Result array and all of its elements must be freed using free() +SWSSFieldValueArray SWSSDBConnector_hgetall(SWSSDBConnector db, const char *key); + +// Returns 0 when key or field doesn't exist, 1 when field exists +int8_t SWSSDBConnector_hexists(SWSSDBConnector db, const char *key, const char *field); + +// std::vector keys(const std::string &key); + +// std::pair> scan(int cursor = 0, const char +// *match = "", uint32_t count = 10); + +// template +// void hmset(const std::string &key, InputIterator start, InputIterator stop); + +// void hmset(const std::unordered_map>>& multiHash); + +// std::shared_ptr get(const std::string &key); + +// std::shared_ptr hget(const std::string &key, const std::string +// &field); + +// int64_t incr(const std::string &key); + +// int64_t decr(const std::string &key); + +// int64_t rpush(const std::string &list, const std::string &item); + +// std::shared_ptr blpop(const std::string &list, int timeout); + +// void subscribe(const std::string &pattern); + +// void psubscribe(const std::string &pattern); + +// void punsubscribe(const std::string &pattern); + +// int64_t publish(const std::string &channel, const std::string &message); + +// void config_set(const std::string &key, const std::string &value); + +// Returns 1 on success, 0 on failure +int8_t SWSSDBConnector_flushdb(SWSSDBConnector db); + +// std::map>> getall(); +#ifdef __cplusplus +} +#endif + +#endif diff --git a/common/c-api/producerstatetable.cpp b/common/c-api/producerstatetable.cpp new file mode 100644 index 000000000..083536d7e --- /dev/null +++ b/common/c-api/producerstatetable.cpp @@ -0,0 +1,53 @@ +#include +#include + +#include "../dbconnector.h" +#include "../producerstatetable.h" +#include "dbconnector.h" +#include "producerstatetable.h" +#include "util.h" + +using namespace swss; +using namespace std; + +SWSSProducerStateTable SWSSProducerStateTable_new(SWSSDBConnector db, const char *tableName) { + SWSSTry(return (SWSSProducerStateTable) new ProducerStateTable((DBConnector *)db, + string(tableName))); +} + +void SWSSProducerStateTable_free(SWSSProducerStateTable tbl) { + SWSSTry(delete ((ProducerStateTable *)tbl)); +} + +void SWSSProducerStateTable_setBuffered(SWSSProducerStateTable tbl, uint8_t buffered) { + SWSSTry(((ProducerStateTable *)tbl)->setBuffered((bool)buffered)) +} + +void SWSSProducerStateTable_set(SWSSProducerStateTable tbl, const char *key, + SWSSFieldValueArray values) { + SWSSTry(((ProducerStateTable *)tbl)->set(string(key), takeFieldValueArray(values))); +} + +void SWSSProducerStateTable_del(SWSSProducerStateTable tbl, const char *key) { + SWSSTry(((ProducerStateTable *)tbl)->del(string(key))); +} + +void SWSSProducerStateTable_flush(SWSSProducerStateTable tbl) { + SWSSTry(((ProducerStateTable *)tbl)->flush()); +} + +int64_t SWSSProducerStateTable_count(SWSSProducerStateTable tbl) { + SWSSTry(return ((ProducerStateTable *)tbl)->count()); +} + +void SWSSProducerStateTable_clear(SWSSProducerStateTable tbl) { + SWSSTry(((ProducerStateTable *)tbl)->clear()); +} + +void SWSSProducerStateTable_create_temp_view(SWSSProducerStateTable tbl) { + SWSSTry(((ProducerStateTable *)tbl)->create_temp_view()); +} + +void SWSSProducerStateTable_apply_temp_view(SWSSProducerStateTable tbl) { + SWSSTry(((ProducerStateTable *)tbl)->apply_temp_view()); +} diff --git a/common/c-api/producerstatetable.h b/common/c-api/producerstatetable.h new file mode 100644 index 000000000..e8db2c65d --- /dev/null +++ b/common/c-api/producerstatetable.h @@ -0,0 +1,44 @@ +#ifndef SWSS_COMMON_C_API_PRODUCERSTATETABLE_H +#define SWSS_COMMON_C_API_PRODUCERSTATETABLE_H + +#include "dbconnector.h" +#include "util.h" + +#ifdef __cplusplus +extern "C" { +#endif + +#include + +typedef struct SWSSProducerStateTableOpaque *SWSSProducerStateTable; + +SWSSProducerStateTable SWSSProducerStateTable_new(SWSSDBConnector db, const char *tableName); + +void SWSSProducerStateTable_free(SWSSProducerStateTable tbl); + +void SWSSProducerStateTable_setBuffered(SWSSProducerStateTable tbl, uint8_t buffered); + +void SWSSProducerStateTable_set(SWSSProducerStateTable tbl, const char *key, SWSSFieldValueArray values); + +void SWSSProducerStateTable_del(SWSSProducerStateTable tbl, const char *key); + +// Batched version of set() and del(). +// virtual void set(const std::vector& values); + +// virtual void del(const std::vector& keys); + +void SWSSProducerStateTable_flush(SWSSProducerStateTable tbl); + +int64_t SWSSProducerStateTable_count(SWSSProducerStateTable tbl); + +void SWSSProducerStateTable_clear(SWSSProducerStateTable tbl); + +void SWSSProducerStateTable_create_temp_view(SWSSProducerStateTable tbl); + +void SWSSProducerStateTable_apply_temp_view(SWSSProducerStateTable tbl); + +#ifdef __cplusplus +} +#endif + +#endif diff --git a/common/c-api/subscriberstatetable.cpp b/common/c-api/subscriberstatetable.cpp new file mode 100644 index 000000000..b64829117 --- /dev/null +++ b/common/c-api/subscriberstatetable.cpp @@ -0,0 +1,52 @@ +#include +#include +#include +#include + +#include "../dbconnector.h" +#include "../subscriberstatetable.h" +#include "../table.h" +#include "subscriberstatetable.h" +#include "util.h" + +using namespace swss; +using namespace std; + +SWSSSubscriberStateTable SWSSSubscriberStateTable_new(SWSSDBConnector db, const char *tableName, + const int32_t *p_popBatchSize, + const int32_t *p_pri) { + int popBatchSize = p_popBatchSize ? numeric_cast(*p_popBatchSize) + : TableConsumable::DEFAULT_POP_BATCH_SIZE; + int pri = p_pri ? numeric_cast(*p_pri) : 0; + SWSSTry(return (SWSSSubscriberStateTable) new SubscriberStateTable( + (DBConnector *)db, string(tableName), popBatchSize, pri)); +} + +void SWSSSubscriberStateTable_free(SWSSSubscriberStateTable tbl) { + delete (SubscriberStateTable *)tbl; +} + +SWSSKeyOpFieldValuesArray SWSSSubscriberStateTable_pops(SWSSSubscriberStateTable tbl) { + SWSSTry({ + deque vkco; + ((SubscriberStateTable *)tbl)->pops(vkco); + return makeKeyOpFieldValuesArray(vkco); + }); +} + +uint8_t SWSSSubscriberStateTable_hasData(SWSSSubscriberStateTable tbl) { + SWSSTry(return ((SubscriberStateTable *)tbl)->hasData() ? 1 : 0); +} + +uint8_t SWSSSubscriberStateTable_hasCachedData(SWSSSubscriberStateTable tbl) { + SWSSTry(return ((SubscriberStateTable *)tbl)->hasCachedData() ? 1 : 0); +} + +uint8_t SWSSSubscriberStateTable_initializedWithData(SWSSSubscriberStateTable tbl) { + SWSSTry(return ((SubscriberStateTable *)tbl)->initializedWithData() ? 1 : 0); +} + +SWSSSelectResult SWSSSubscriberStateTable_readData(SWSSSubscriberStateTable tbl, + uint32_t timeout_ms) { + SWSSTry(return selectOne((SubscriberStateTable *)tbl, timeout_ms)); +} diff --git a/common/c-api/subscriberstatetable.h b/common/c-api/subscriberstatetable.h new file mode 100644 index 000000000..4501a3af4 --- /dev/null +++ b/common/c-api/subscriberstatetable.h @@ -0,0 +1,43 @@ +#ifndef SWSS_COMMON_C_API_SUBSCRIBERSTATETABLE_H +#define SWSS_COMMON_C_API_SUBSCRIBERSTATETABLE_H + +#include "dbconnector.h" +#include "util.h" + +#ifdef __cplusplus +extern "C" { +#endif + +#include + +typedef struct SWSSSubscriberStateTableOpaque *SWSSSubscriberStateTable; + +// Pass NULL for popBatchSize and/or pri to use the default values +SWSSSubscriberStateTable SWSSSubscriberStateTable_new(SWSSDBConnector db, const char *tableName, + const int32_t *popBatchSize, + const int32_t *pri); + +void SWSSSubscriberStateTable_free(SWSSSubscriberStateTable tbl); + +// Result array and all of its members must be freed using free() +SWSSKeyOpFieldValuesArray SWSSSubscriberStateTable_pops(SWSSSubscriberStateTable tbl); + +// Returns 0 for false, 1 for true +uint8_t SWSSSubscriberStateTable_hasData(SWSSSubscriberStateTable tbl); + +// Returns 0 for false, 1 for true +uint8_t SWSSSubscriberStateTable_hasCachedData(SWSSSubscriberStateTable tbl); + +// Returns 0 for false, 1 for true +uint8_t SWSSSubscriberStateTable_initializedWithData(SWSSSubscriberStateTable tbl); + +// Block until data is available to read or until a timeout elapses. +// A timeout of 0 means the call will return immediately. +SWSSSelectResult SWSSSubscriberStateTable_readData(SWSSSubscriberStateTable tbl, + uint32_t timeout_ms); + +#ifdef __cplusplus +} +#endif + +#endif diff --git a/common/c-api/util.cpp b/common/c-api/util.cpp new file mode 100644 index 000000000..fb983d5cf --- /dev/null +++ b/common/c-api/util.cpp @@ -0,0 +1,3 @@ +#include "util.h" + +bool swss::cApiTestingDisableAbort = false; diff --git a/common/c-api/util.h b/common/c-api/util.h new file mode 100644 index 000000000..79eb93cfd --- /dev/null +++ b/common/c-api/util.h @@ -0,0 +1,181 @@ +#ifndef SWSS_COMMON_C_API_UTIL_H +#define SWSS_COMMON_C_API_UTIL_H + +// External utilities (c-facing) +#ifdef __cplusplus +extern "C" { +#endif + +#include + +typedef struct { + const char *field; + const char *value; +} SWSSFieldValuePair; + +typedef struct { + uint64_t len; + const SWSSFieldValuePair *data; +} SWSSFieldValueArray; + +typedef struct { + const char *key; + const char *operation; + SWSSFieldValueArray fieldValues; +} SWSSKeyOpFieldValues; + +typedef struct { + uint64_t len; + const SWSSKeyOpFieldValues *data; +} SWSSKeyOpFieldValuesArray; + +typedef enum { + SWSSSelectResult_DATA = 0, + SWSSSelectResult_TIMEOUT = 1, + SWSSSelectResult_SIGNAL = 2, +} SWSSSelectResult; + +#ifdef __cplusplus +} +#endif + +// Internal utilities (used to help define c-facing functions) +#ifdef __cplusplus +#include +#include +#include +#include +#include +#include + +#include "../logger.h" +#include "../rediscommand.h" +#include "../select.h" + +using boost::numeric_cast; + +namespace swss { + +extern bool cApiTestingDisableAbort; + +// In the catch block, we must abort because passing an exception across an ffi boundary is +// undefined behavior. It was also decided that no exceptions in swss-common are recoverable, so +// there is no reason to convert exceptions into a returnable type. +#define SWSSTry(...) \ + if (cApiTestingDisableAbort) { \ + { __VA_ARGS__; } \ + } else { \ + try { \ + { __VA_ARGS__; } \ + } catch (std::exception & e) { \ + std::cerr << "Aborting due to exception: " << e.what() << std::endl; \ + SWSS_LOG_ERROR("Aborting due to exception: %s", e.what()); \ + std::abort(); \ + } \ + } + +static inline SWSSSelectResult selectOne(swss::Selectable *s, uint32_t timeout_ms) { + Select select; + Selectable *sOut; + select.addSelectable(s); + int ret = select.select(&sOut, numeric_cast(timeout_ms)); + switch (ret) { + case Select::OBJECT: + return SWSSSelectResult_DATA; + case Select::ERROR: + throw std::system_error(errno, std::generic_category()); + case Select::TIMEOUT: + return SWSSSelectResult_TIMEOUT; + case Select::SIGNALINT: + return SWSSSelectResult_SIGNAL; + default: + SWSS_LOG_THROW("impossible: unhandled Select::select() return value: %d", ret); + } +} + +// malloc() with safe numeric casting of the size parameter +template static inline void *mallocN(N size) { + return malloc(numeric_cast(size)); +} + +// T is anything that has a .size() method and which can be iterated over for pair +// eg unordered_map or vector> +template static inline SWSSFieldValueArray makeFieldValueArray(const T &in) { + SWSSFieldValuePair *data = + (SWSSFieldValuePair *)mallocN(in.size() * sizeof(SWSSFieldValuePair)); + + size_t i = 0; + for (const auto &pair : in) { + SWSSFieldValuePair entry; + entry.field = strdup(pair.first.c_str()); + entry.value = strdup(pair.second.c_str()); + data[i++] = entry; + } + + SWSSFieldValueArray out; + out.len = (uint64_t)in.size(); + out.data = data; + return out; +} + +static inline std::vector +takeFieldValueArray(const SWSSFieldValueArray &in) { + std::vector out; + for (uint64_t i = 0; i < in.len; i++) { + auto field = std::string(in.data[i].field); + auto value = std::string(in.data[i].value); + out.push_back(std::make_pair(field, value)); + } + return out; +} + +static inline SWSSKeyOpFieldValues makeKeyOpFieldValues(const swss::KeyOpFieldsValuesTuple &in) { + SWSSKeyOpFieldValues out; + out.key = strdup(kfvKey(in).c_str()); + out.operation = strdup(kfvOp(in).c_str()); + out.fieldValues = makeFieldValueArray(kfvFieldsValues(in)); + return out; +} + +static inline swss::KeyOpFieldsValuesTuple takeKeyOpFieldValues(const SWSSKeyOpFieldValues &in) { + std::string key(in.key), op(in.operation); + auto fieldValues = takeFieldValueArray(in.fieldValues); + return std::make_tuple(key, op, fieldValues); +} + +template static inline const T &getReference(const T &t) { + return t; +} + +template static inline const T &getReference(const std::shared_ptr &t) { + return *t; +} + +// T is anything that has a .size() method and which can be iterated over for +// swss::KeyOpFieldValuesTuple +template static inline SWSSKeyOpFieldValuesArray makeKeyOpFieldValuesArray(const T &in) { + SWSSKeyOpFieldValues *data = + (SWSSKeyOpFieldValues *)mallocN(in.size() * sizeof(SWSSKeyOpFieldValues)); + + size_t i = 0; + for (const auto &kfv : in) + data[i++] = makeKeyOpFieldValues(getReference(kfv)); + + SWSSKeyOpFieldValuesArray out; + out.len = (uint64_t)in.size(); + out.data = data; + return out; +} + +static inline std::vector +takeKeyOpFieldValuesArray(const SWSSKeyOpFieldValuesArray &in) { + std::vector out; + for (uint64_t i = 0; i < in.len; i++) + out.push_back(takeKeyOpFieldValues(in.data[i])); + return out; +} + +} // namespace swss + +#endif +#endif diff --git a/common/c-api/zmqclient.cpp b/common/c-api/zmqclient.cpp new file mode 100644 index 000000000..7e4a58f87 --- /dev/null +++ b/common/c-api/zmqclient.cpp @@ -0,0 +1,35 @@ +#include "../zmqclient.h" +#include "../binaryserializer.h" +#include "util.h" +#include "zmqclient.h" + +using namespace swss; +using namespace std; + +SWSSZmqClient SWSSZmqClient_new(const char *endpoint) { + SWSSTry(return (SWSSZmqClient) new ZmqClient(endpoint)); +} + +void SWSSZmqClient_free(SWSSZmqClient zmqc) { + SWSSTry(delete (ZmqClient *)zmqc); +} + +// Returns 0 for false, 1 for true +int8_t SWSSZmqClient_isConnected(SWSSZmqClient zmqc) { + SWSSTry(return ((ZmqClient *)zmqc)->isConnected() ? 1 : 0); +} + +void SWSSZmqClient_connect(SWSSZmqClient zmqc) { + SWSSTry(((ZmqClient *)zmqc)->connect()); +} + +void SWSSZmqClient_sendMsg(SWSSZmqClient zmqc, const char *dbName, const char *tableName, + const SWSSKeyOpFieldValuesArray *arr) { + SWSSTry({ + vector kcos = takeKeyOpFieldValuesArray(*arr); + size_t bufSize = BinarySerializer::serializedSize(dbName, tableName, kcos); + vector v(bufSize); + ((ZmqClient *)zmqc) + ->sendMsg(string(dbName), string(tableName), kcos, v); + }); +} diff --git a/common/c-api/zmqclient.h b/common/c-api/zmqclient.h new file mode 100644 index 000000000..47cd1efba --- /dev/null +++ b/common/c-api/zmqclient.h @@ -0,0 +1,30 @@ +#ifndef SWSS_COMMON_C_API_ZMQCLIENT_H +#define SWSS_COMMON_C_API_ZMQCLIENT_H + +#include "util.h" + +#ifdef __cplusplus +extern "C" { +#endif + +#include + +typedef struct SWSSZmqClientOpaque *SWSSZmqClient; + +SWSSZmqClient SWSSZmqClient_new(const char *endpoint); + +void SWSSZmqClient_free(SWSSZmqClient zmqc); + +// Returns 0 for false, 1 for true +int8_t SWSSZmqClient_isConnected(SWSSZmqClient zmqc); + +void SWSSZmqClient_connect(SWSSZmqClient zmqc); + +void SWSSZmqClient_sendMsg(SWSSZmqClient zmqc, const char *dbName, const char *tableName, + const SWSSKeyOpFieldValuesArray *kcos); + +#ifdef __cplusplus +} +#endif + +#endif diff --git a/common/c-api/zmqconsumerstatetable.cpp b/common/c-api/zmqconsumerstatetable.cpp new file mode 100644 index 000000000..38cd87f93 --- /dev/null +++ b/common/c-api/zmqconsumerstatetable.cpp @@ -0,0 +1,58 @@ +#include "../zmqconsumerstatetable.h" +#include "../table.h" +#include "util.h" +#include "zmqconsumerstatetable.h" +#include "zmqserver.h" + +using namespace swss; +using namespace std; + +// Pass NULL for popBatchSize and/or pri to use the default values +SWSSZmqConsumerStateTable SWSSZmqConsumerStateTable_new(SWSSDBConnector db, const char *tableName, + SWSSZmqServer zmqs, + const int32_t *p_popBatchSize, + const int32_t *p_pri) { + + int popBatchSize = p_popBatchSize ? numeric_cast(*p_popBatchSize) + : TableConsumable::DEFAULT_POP_BATCH_SIZE; + int pri = p_pri ? numeric_cast(*p_pri) : 0; + SWSSTry(return (SWSSZmqConsumerStateTable) new ZmqConsumerStateTable( + (DBConnector *)db, string(tableName), *(ZmqServer *)zmqs, popBatchSize, pri)); +} + +void SWSSZmqConsumerStateTable_free(SWSSZmqConsumerStateTable tbl) { + SWSSTry(delete (ZmqConsumerStateTable *)tbl); +} + +SWSSKeyOpFieldValuesArray SWSSZmqConsumerStateTable_pops(SWSSZmqConsumerStateTable tbl) { + SWSSTry({ + deque vkco; + ((ZmqConsumerStateTable *)tbl)->pops(vkco); + return makeKeyOpFieldValuesArray(vkco); + }); +} + +SWSSSelectResult SWSSZmqConsumerStateTable_readData(SWSSZmqConsumerStateTable tbl, + uint32_t timeout_ms) { + SWSSTry(return selectOne((ZmqConsumerStateTable *)tbl, timeout_ms)); +} + +// Returns 0 for false, 1 for true +uint8_t SWSSZmqConsumerStateTable_hasData(SWSSZmqConsumerStateTable tbl) { + SWSSTry(return ((ZmqConsumerStateTable *)tbl)->hasData() ? 1 : 0); +} + +// Returns 0 for false, 1 for true +uint8_t SWSSZmqConsumerStateTable_hasCachedData(SWSSZmqConsumerStateTable tbl) { + SWSSTry(return ((ZmqConsumerStateTable *)tbl)->hasCachedData() ? 1 : 0); +} + +// Returns 0 for false, 1 for true +uint8_t SWSSZmqConsumerStateTable_initializedWithData(SWSSZmqConsumerStateTable tbl) { + SWSSTry(return ((ZmqConsumerStateTable *)tbl)->initializedWithData() ? 1 : 0); +} + +const struct SWSSDBConnectorOpaque * +SWSSZmqConsumerStateTable_getDbConnector(SWSSZmqConsumerStateTable tbl) { + SWSSTry(return (const SWSSDBConnectorOpaque *)((ZmqConsumerStateTable *)tbl)->getDbConnector()); +} diff --git a/common/c-api/zmqconsumerstatetable.h b/common/c-api/zmqconsumerstatetable.h new file mode 100644 index 000000000..4810c3ef5 --- /dev/null +++ b/common/c-api/zmqconsumerstatetable.h @@ -0,0 +1,48 @@ +#ifndef SWSS_COMMON_C_API_ZMQCONSUMERSTATETABLE_H +#define SWSS_COMMON_C_API_ZMQCONSUMERSTATETABLE_H + +#include "dbconnector.h" +#include "util.h" +#include "zmqserver.h" + +#ifdef __cplusplus +extern "C" { +#endif + +#include + +typedef struct SWSSZmqConsumerStateTableOpaque *SWSSZmqConsumerStateTable; + +// Pass NULL for popBatchSize and/or pri to use the default values +SWSSZmqConsumerStateTable SWSSZmqConsumerStateTable_new(SWSSDBConnector db, const char *tableName, + SWSSZmqServer zmqs, + const int32_t *popBatchSize, + const int32_t *pri); + +void SWSSZmqConsumerStateTable_free(SWSSZmqConsumerStateTable tbl); + +// Result array and all of its members must be freed using free() +SWSSKeyOpFieldValuesArray SWSSZmqConsumerStateTable_pops(SWSSZmqConsumerStateTable tbl); + +// Block until data is available to read or until a timeout elapses. +// A timeout of 0 means the call will return immediately. +SWSSSelectResult SWSSZmqConsumerStateTable_readData(SWSSZmqConsumerStateTable tbl, + uint32_t timeout_ms); + +// Returns 0 for false, 1 for true +uint8_t SWSSZmqConsumerStateTable_hasData(SWSSZmqConsumerStateTable tbl); + +// Returns 0 for false, 1 for true +uint8_t SWSSZmqConsumerStateTable_hasCachedData(SWSSZmqConsumerStateTable tbl); + +// Returns 0 for false, 1 for true +uint8_t SWSSZmqConsumerStateTable_initializedWithData(SWSSZmqConsumerStateTable tbl); + +const struct SWSSDBConnectorOpaque * +SWSSZmqConsumerStateTable_getDbConnector(SWSSZmqConsumerStateTable tbl); + +#ifdef __cplusplus +} +#endif + +#endif diff --git a/common/c-api/zmqproducerstatetable.cpp b/common/c-api/zmqproducerstatetable.cpp new file mode 100644 index 000000000..3e50916e1 --- /dev/null +++ b/common/c-api/zmqproducerstatetable.cpp @@ -0,0 +1,29 @@ +#include "zmqproducerstatetable.h" +#include "../zmqproducerstatetable.h" + +using namespace std; +using namespace swss; + +SWSSZmqProducerStateTable SWSSZmqProducerStateTable_new(SWSSDBConnector db, const char *tableName, + SWSSZmqClient zmqc, uint8_t dbPersistence) { + + SWSSTry(return (SWSSZmqProducerStateTable) new ZmqProducerStateTable( + (DBConnector *)db, string(tableName), *(ZmqClient *)zmqc, dbPersistence)); +} + +void SWSSZmqProducerStateTable_free(SWSSZmqProducerStateTable tbl) { + SWSSTry(delete (ZmqProducerStateTable *)tbl); +} + +void SWSSZmqProducerStateTable_set(SWSSZmqProducerStateTable tbl, const char *key, + SWSSFieldValueArray values) { + SWSSTry(((ZmqProducerStateTable *)tbl)->set(string(key), takeFieldValueArray(values))); +} + +void SWSSZmqProducerStateTable_del(SWSSZmqProducerStateTable tbl, const char *key) { + SWSSTry(((ZmqProducerStateTable *)tbl)->del(string(key))); +} + +uint64_t SWSSZmqProducerStateTable_dbUpdaterQueueSize(SWSSZmqProducerStateTable tbl) { + SWSSTry(return numeric_cast(((ZmqProducerStateTable *)tbl)->dbUpdaterQueueSize())); +} diff --git a/common/c-api/zmqproducerstatetable.h b/common/c-api/zmqproducerstatetable.h new file mode 100644 index 000000000..08d059186 --- /dev/null +++ b/common/c-api/zmqproducerstatetable.h @@ -0,0 +1,32 @@ +#ifndef SWSS_COMMON_C_API_ZMQPRODUCERSTATETABLE_H +#define SWSS_COMMON_C_API_ZMQPRODUCERSTATETABLE_H + +#include "dbconnector.h" +#include "util.h" +#include "zmqclient.h" + +#ifdef __cplusplus +extern "C" { +#endif + +#include "stdint.h" + +typedef struct SWSSZmqProducerStateTableOpaque *SWSSZmqProducerStateTable; + +SWSSZmqProducerStateTable SWSSZmqProducerStateTable_new(SWSSDBConnector db, const char *tableName, + SWSSZmqClient zmqc, uint8_t dbPersistence); + +void SWSSZmqProducerStateTable_free(SWSSZmqProducerStateTable tbl); + +void SWSSZmqProducerStateTable_set(SWSSZmqProducerStateTable tbl, const char *key, + SWSSFieldValueArray values); + +void SWSSZmqProducerStateTable_del(SWSSZmqProducerStateTable tbl, const char *key); + +uint64_t SWSSZmqProducerStateTable_dbUpdaterQueueSize(SWSSZmqProducerStateTable tbl); + +#ifdef __cplusplus +} +#endif + +#endif diff --git a/common/c-api/zmqserver.cpp b/common/c-api/zmqserver.cpp new file mode 100644 index 000000000..50452e22d --- /dev/null +++ b/common/c-api/zmqserver.cpp @@ -0,0 +1,14 @@ +#include "zmqserver.h" +#include "../zmqserver.h" +#include "util.h" + +using namespace swss; +using namespace std; + +SWSSZmqServer SWSSZmqServer_new(const char *endpoint) { + SWSSTry(return (SWSSZmqServer) new ZmqServer(string(endpoint))); +} + +void SWSSZmqServer_free(SWSSZmqServer zmqs) { + SWSSTry(delete (ZmqServer *)zmqs); +} diff --git a/common/c-api/zmqserver.h b/common/c-api/zmqserver.h new file mode 100644 index 000000000..decd0e0dc --- /dev/null +++ b/common/c-api/zmqserver.h @@ -0,0 +1,20 @@ +#ifndef SWSS_COMMON_C_API_ZMQSERVER_H +#define SWSS_COMMON_C_API_ZMQSERVER_H + +#include "util.h" + +#ifdef __cplusplus +extern "C" { +#endif + +typedef struct SWSSZmqServerOpaque *SWSSZmqServer; + +SWSSZmqServer SWSSZmqServer_new(const char *endpoint); + +void SWSSZmqServer_free(SWSSZmqServer zmqs); + +#ifdef __cplusplus +} +#endif + +#endif diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index 0e044f3ea..96334780f 100755 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -645,39 +645,46 @@ DBConnector::DBConnector(int dbId, const RedisContext& ctx) select(this); } +static struct timeval ms_to_timeval(unsigned int ms) { + return { + .tv_sec = (time_t)ms / 1000, + .tv_usec = ((suseconds_t)ms % 1000) * 1000 + }; +} + DBConnector::DBConnector(int dbId, const string& hostname, int port, - unsigned int timeout) + unsigned int timeout_ms) : m_dbId(dbId) { - struct timeval tv = {0, (suseconds_t)timeout * 1000}; - struct timeval *ptv = timeout ? &tv : NULL; + struct timeval tv = ms_to_timeval(timeout_ms); + struct timeval *ptv = timeout_ms ? &tv : NULL; initContext(hostname.c_str(), port, ptv); select(this); } -DBConnector::DBConnector(int dbId, const string& unixPath, unsigned int timeout) +DBConnector::DBConnector(int dbId, const string& unixPath, unsigned int timeout_ms) : m_dbId(dbId) { - struct timeval tv = {0, (suseconds_t)timeout * 1000}; - struct timeval *ptv = timeout ? &tv : NULL; + struct timeval tv = ms_to_timeval(timeout_ms); + struct timeval *ptv = timeout_ms ? &tv : NULL; initContext(unixPath.c_str(), ptv); select(this); } -DBConnector::DBConnector(const string& dbName, unsigned int timeout, bool isTcpConn, const string& netns) - : DBConnector(dbName, timeout, isTcpConn, SonicDBKey(netns)) +DBConnector::DBConnector(const string& dbName, unsigned int timeout_ms, bool isTcpConn, const string& netns) + : DBConnector(dbName, timeout_ms, isTcpConn, SonicDBKey(netns)) { } -DBConnector::DBConnector(const string& dbName, unsigned int timeout, bool isTcpConn, const SonicDBKey &key) +DBConnector::DBConnector(const string& dbName, unsigned int timeout_ms, bool isTcpConn, const SonicDBKey &key) : m_dbId(SonicDBConfig::getDbId(dbName, key)) , m_dbName(dbName) , m_key(key) { - struct timeval tv = {0, (suseconds_t)timeout * 1000}; - struct timeval *ptv = timeout ? &tv : NULL; + struct timeval tv = ms_to_timeval(timeout_ms); + struct timeval *ptv = timeout_ms ? &tv : NULL; if (isTcpConn) { initContext(SonicDBConfig::getDbHostname(dbName, m_key).c_str(), SonicDBConfig::getDbPort(dbName, m_key), ptv); @@ -690,8 +697,8 @@ DBConnector::DBConnector(const string& dbName, unsigned int timeout, bool isTcpC select(this); } -DBConnector::DBConnector(const string& dbName, unsigned int timeout, bool isTcpConn) - : DBConnector(dbName, timeout, isTcpConn, SonicDBKey()) +DBConnector::DBConnector(const string& dbName, unsigned int timeout_ms, bool isTcpConn) + : DBConnector(dbName, timeout_ms, isTcpConn, SonicDBKey()) { // Empty constructor } diff --git a/common/dbconnector.h b/common/dbconnector.h index c5bd48ad6..832983ed9 100644 --- a/common/dbconnector.h +++ b/common/dbconnector.h @@ -213,11 +213,11 @@ class DBConnector : public RedisContext */ explicit DBConnector(const DBConnector &other); DBConnector(int dbId, const RedisContext &ctx); - DBConnector(int dbId, const std::string &hostname, int port, unsigned int timeout); - DBConnector(int dbId, const std::string &unixPath, unsigned int timeout); - DBConnector(const std::string &dbName, unsigned int timeout, bool isTcpConn = false); - DBConnector(const std::string &dbName, unsigned int timeout, bool isTcpConn, const std::string &netns); - DBConnector(const std::string &dbName, unsigned int timeout, bool isTcpConn, const SonicDBKey &key); + DBConnector(int dbId, const std::string &hostname, int port, unsigned int timeout_ms); + DBConnector(int dbId, const std::string &unixPath, unsigned int timeout_ms); + DBConnector(const std::string &dbName, unsigned int timeout_ms, bool isTcpConn = false); + DBConnector(const std::string &dbName, unsigned int timeout_ms, bool isTcpConn, const std::string &netns); + DBConnector(const std::string &dbName, unsigned int timeout_ms, bool isTcpConn, const SonicDBKey &key); DBConnector& operator=(const DBConnector&) = delete; int getDbId() const; diff --git a/common/zmqserver.cpp b/common/zmqserver.cpp index 4800b9ba2..dca107405 100644 --- a/common/zmqserver.cpp +++ b/common/zmqserver.cpp @@ -106,9 +106,10 @@ void ZmqServer::mqPollThread() int rc = zmq_bind(socket, m_endpoint.c_str()); if (rc != 0) { - SWSS_LOG_THROW("zmq_bind failed on endpoint: %s, zmqerrno: %d", + SWSS_LOG_THROW("zmq_bind failed on endpoint: %s, zmqerrno: %d, message: %s", m_endpoint.c_str(), - zmq_errno()); + zmq_errno(), + strerror(zmq_errno())); } // zmq_poll will use less CPU diff --git a/debian/libswsscommon-dev.install b/debian/libswsscommon-dev.install index 1dd2670e9..85e3c4bca 100644 --- a/debian/libswsscommon-dev.install +++ b/debian/libswsscommon-dev.install @@ -1,2 +1,3 @@ common/*.h usr/include/swss +common/c-api/*.h usr/include/swss/c-api pyext/swsscommon.i usr/share/swss diff --git a/tests/Makefile.am b/tests/Makefile.am index a07fadc2e..39712b233 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -42,6 +42,7 @@ tests_tests_SOURCES = tests/redis_ut.cpp \ tests/binary_serializer_ut.cpp \ tests/zmq_state_ut.cpp \ tests/profileprovider_ut.cpp \ + tests/c_api_ut.cpp \ tests/main.cpp tests_tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS) diff --git a/tests/c_api_ut.cpp b/tests/c_api_ut.cpp new file mode 100644 index 000000000..d16dac7c1 --- /dev/null +++ b/tests/c_api_ut.cpp @@ -0,0 +1,325 @@ +#include +#include +#include + +#include "common/c-api/consumerstatetable.h" +#include "common/c-api/dbconnector.h" +#include "common/c-api/producerstatetable.h" +#include "common/c-api/subscriberstatetable.h" +#include "common/c-api/util.h" +#include "common/c-api/zmqclient.h" +#include "common/c-api/zmqconsumerstatetable.h" +#include "common/c-api/zmqproducerstatetable.h" +#include "common/c-api/zmqserver.h" +#include "common/select.h" +#include "common/subscriberstatetable.h" +#include "gtest/gtest.h" + +using namespace std; +using namespace swss; + +static void clearDB() { + DBConnector db("TEST_DB", 0, true); + RedisReply r(&db, "FLUSHALL", REDIS_REPLY_STATUS); + r.checkStatusOK(); +} + +static void sortKfvs(vector &kfvs) { + sort(kfvs.begin(), kfvs.end(), + [](const KeyOpFieldsValuesTuple &a, const KeyOpFieldsValuesTuple &b) { + return kfvKey(a) < kfvKey(b); + }); + + for (auto &kfv : kfvs) { + auto &fvs = kfvFieldsValues(kfv); + sort(fvs.begin(), fvs.end(), + [](const pair &a, const pair &b) { + return a.first < b.first; + }); + } +} + +template static void free(const T *ptr) { + std::free(const_cast(reinterpret_cast(ptr))); +} + +static void freeKeyOpFieldValuesArray(SWSSKeyOpFieldValuesArray arr) { + for (uint64_t i = 0; i < arr.len; i++) { + free(arr.data[i].key); + free(arr.data[i].operation); + for (uint64_t j = 0; j < arr.data[i].fieldValues.len; j++) { + free(arr.data[i].fieldValues.data[j].field); + free(arr.data[i].fieldValues.data[j].value); + } + free(arr.data[i].fieldValues.data); + } + free(arr.data); +} + +TEST(c_api, DBConnector) { + clearDB(); + + EXPECT_THROW(SWSSDBConnector_new_named("does not exist", 0, true), out_of_range); + SWSSDBConnector db = SWSSDBConnector_new_named("TEST_DB", 1000, true); + EXPECT_FALSE(SWSSDBConnector_get(db, "mykey")); + EXPECT_FALSE(SWSSDBConnector_exists(db, "mykey")); + SWSSDBConnector_set(db, "mykey", "myval"); + const char *val = SWSSDBConnector_get(db, "mykey"); + EXPECT_STREQ(val, "myval"); + free(val); + EXPECT_TRUE(SWSSDBConnector_exists(db, "mykey")); + EXPECT_TRUE(SWSSDBConnector_del(db, "mykey")); + EXPECT_FALSE(SWSSDBConnector_del(db, "mykey")); + + EXPECT_FALSE(SWSSDBConnector_hget(db, "mykey", "myfield")); + EXPECT_FALSE(SWSSDBConnector_hexists(db, "mykey", "myfield")); + SWSSDBConnector_hset(db, "mykey", "myfield", "myval"); + val = SWSSDBConnector_hget(db, "mykey", "myfield"); + EXPECT_STREQ(val, "myval"); + free(val); + EXPECT_TRUE(SWSSDBConnector_hexists(db, "mykey", "myfield")); + EXPECT_FALSE(SWSSDBConnector_hget(db, "mykey", "notmyfield")); + EXPECT_FALSE(SWSSDBConnector_hexists(db, "mykey", "notmyfield")); + EXPECT_TRUE(SWSSDBConnector_hdel(db, "mykey", "myfield")); + EXPECT_FALSE(SWSSDBConnector_hdel(db, "mykey", "myfield")); + + EXPECT_TRUE(SWSSDBConnector_flushdb(db)); + SWSSDBConnector_free(db); +} + +TEST(c_api, ConsumerProducerStateTables) { + clearDB(); + + SWSSDBConnector db = SWSSDBConnector_new_named("TEST_DB", 1000, true); + SWSSProducerStateTable pst = SWSSProducerStateTable_new(db, "mytable"); + SWSSConsumerStateTable cst = SWSSConsumerStateTable_new(db, "mytable", nullptr, nullptr); + + SWSSKeyOpFieldValuesArray arr = SWSSConsumerStateTable_pops(cst); + ASSERT_EQ(arr.len, 0); + freeKeyOpFieldValuesArray(arr); + + SWSSFieldValuePair data[2] = {{.field = "myfield1", .value = "myvalue1"}, + {.field = "myfield2", .value = "myvalue2"}}; + SWSSFieldValueArray values = { + .len = 2, + .data = data, + }; + SWSSProducerStateTable_set(pst, "mykey1", values); + + data[0] = {.field = "myfield3", .value = "myvalue3"}; + values.len = 1; + SWSSProducerStateTable_set(pst, "mykey2", values); + + arr = SWSSConsumerStateTable_pops(cst); + vector kfvs = takeKeyOpFieldValuesArray(arr); + sortKfvs(kfvs); + freeKeyOpFieldValuesArray(arr); + + ASSERT_EQ(kfvs.size(), 2); + EXPECT_EQ(kfvKey(kfvs[0]), "mykey1"); + EXPECT_EQ(kfvOp(kfvs[0]), "SET"); + vector> &fieldValues0 = kfvFieldsValues(kfvs[0]); + ASSERT_EQ(fieldValues0.size(), 2); + EXPECT_EQ(fieldValues0[0].first, "myfield1"); + EXPECT_EQ(fieldValues0[0].second, "myvalue1"); + EXPECT_EQ(fieldValues0[1].first, "myfield2"); + EXPECT_EQ(fieldValues0[1].second, "myvalue2"); + + EXPECT_EQ(kfvKey(kfvs[1]), "mykey2"); + EXPECT_EQ(kfvOp(kfvs[1]), "SET"); + vector> &fieldValues1 = kfvFieldsValues(kfvs[1]); + ASSERT_EQ(fieldValues1.size(), 1); + EXPECT_EQ(fieldValues1[0].first, "myfield3"); + EXPECT_EQ(fieldValues1[0].second, "myvalue3"); + + arr = SWSSConsumerStateTable_pops(cst); + EXPECT_EQ(arr.len, 0); + freeKeyOpFieldValuesArray(arr); + + SWSSProducerStateTable_del(pst, "mykey3"); + SWSSProducerStateTable_del(pst, "mykey4"); + SWSSProducerStateTable_del(pst, "mykey5"); + + arr = SWSSConsumerStateTable_pops(cst); + kfvs = takeKeyOpFieldValuesArray(arr); + sortKfvs(kfvs); + freeKeyOpFieldValuesArray(arr); + + ASSERT_EQ(kfvs.size(), 3); + EXPECT_EQ(kfvKey(kfvs[0]), "mykey3"); + EXPECT_EQ(kfvOp(kfvs[0]), "DEL"); + EXPECT_EQ(kfvFieldsValues(kfvs[0]).size(), 0); + EXPECT_EQ(kfvKey(kfvs[1]), "mykey4"); + EXPECT_EQ(kfvOp(kfvs[1]), "DEL"); + EXPECT_EQ(kfvFieldsValues(kfvs[1]).size(), 0); + EXPECT_EQ(kfvKey(kfvs[2]), "mykey5"); + EXPECT_EQ(kfvOp(kfvs[2]), "DEL"); + EXPECT_EQ(kfvFieldsValues(kfvs[2]).size(), 0); + + SWSSProducerStateTable_free(pst); + SWSSConsumerStateTable_free(cst); + SWSSDBConnector_flushdb(db); + SWSSDBConnector_free(db); +} + +TEST(c_api, SubscriberStateTable) { + clearDB(); + + SWSSDBConnector db = SWSSDBConnector_new_named("TEST_DB", 1000, true); + SWSSSubscriberStateTable sst = SWSSSubscriberStateTable_new(db, "mytable", nullptr, nullptr); + + EXPECT_EQ(SWSSSubscriberStateTable_readData(sst, 300), SWSSSelectResult_TIMEOUT); + EXPECT_FALSE(SWSSSubscriberStateTable_hasData(sst)); + SWSSKeyOpFieldValuesArray arr = SWSSSubscriberStateTable_pops(sst); + EXPECT_EQ(arr.len, 0); + freeKeyOpFieldValuesArray(arr); + + SWSSDBConnector_hset(db, "mytable:mykey", "myfield", "myvalue"); + EXPECT_EQ(SWSSSubscriberStateTable_readData(sst, 300), SWSSSelectResult_DATA); + EXPECT_EQ(SWSSSubscriberStateTable_readData(sst, 300), SWSSSelectResult_TIMEOUT); + EXPECT_TRUE(SWSSSubscriberStateTable_hasData(sst)); + + arr = SWSSSubscriberStateTable_pops(sst); + vector kfvs = takeKeyOpFieldValuesArray(arr); + sortKfvs(kfvs); + freeKeyOpFieldValuesArray(arr); + + EXPECT_FALSE(SWSSSubscriberStateTable_hasData(sst)); + ASSERT_EQ(kfvs.size(), 1); + EXPECT_EQ(kfvKey(kfvs[0]), "mykey"); + EXPECT_EQ(kfvOp(kfvs[0]), "SET"); + ASSERT_EQ(kfvFieldsValues(kfvs[0]).size(), 1); + EXPECT_EQ(kfvFieldsValues(kfvs[0])[0].first, "myfield"); + EXPECT_EQ(kfvFieldsValues(kfvs[0])[0].second, "myvalue"); + + SWSSSubscriberStateTable_free(sst); + SWSSDBConnector_flushdb(db); + SWSSDBConnector_free(db); +} + +TEST(c_api, ZmqConsumerProducerStateTable) { + clearDB(); + + SWSSDBConnector db = SWSSDBConnector_new_named("TEST_DB", 1000, true); + + SWSSZmqServer srv = SWSSZmqServer_new("tcp://127.0.0.1:42312"); + SWSSZmqClient cli = SWSSZmqClient_new("tcp://127.0.0.1:42312"); + EXPECT_TRUE(SWSSZmqClient_isConnected(cli)); + SWSSZmqClient_connect(cli); // This should be idempotent/not throw + + SWSSZmqProducerStateTable pst = SWSSZmqProducerStateTable_new(db, "mytable", cli, false); + SWSSZmqConsumerStateTable cst = + SWSSZmqConsumerStateTable_new(db, "mytable", srv, nullptr, nullptr); + + ASSERT_EQ(SWSSZmqConsumerStateTable_getDbConnector(cst), db); + + EXPECT_FALSE(SWSSZmqConsumerStateTable_hasData(cst)); + EXPECT_FALSE(SWSSZmqConsumerStateTable_hasCachedData(cst)); + EXPECT_FALSE(SWSSZmqConsumerStateTable_initializedWithData(cst)); + SWSSKeyOpFieldValuesArray arr = SWSSZmqConsumerStateTable_pops(cst); + ASSERT_EQ(arr.len, 0); + freeKeyOpFieldValuesArray(arr); + + // On flag = 0, we use the ZmqProducerStateTable + // On flag = 1, we use the ZmqClient directly + for (int flag = 0; flag < 2; flag++) { + SWSSFieldValuePair values_key1_data[2] = {{.field = "myfield1", .value = "myvalue1"}, + {.field = "myfield2", .value = "myvalue2"}}; + SWSSFieldValueArray values_key1 = { + .len = 2, + .data = values_key1_data, + }; + + SWSSFieldValuePair values_key2_data[1] = {{.field = "myfield3", .value = "myvalue3"}}; + SWSSFieldValueArray values_key2 = { + .len = 1, + .data = values_key2_data, + }; + + SWSSKeyOpFieldValues arr_data[2] = { + {.key = "mykey1", .operation = "SET", .fieldValues = values_key1}, + {.key = "mykey2", .operation = "SET", .fieldValues = values_key2}}; + arr = {.len = 2, .data = arr_data}; + + if (flag == 0) + for (uint64_t i = 0; i < arr.len; i++) + SWSSZmqProducerStateTable_set(pst, arr.data[i].key, arr.data[i].fieldValues); + else + SWSSZmqClient_sendMsg(cli, "TEST_DB", "mytable", &arr); + + ASSERT_EQ(SWSSZmqConsumerStateTable_readData(cst, 500), SWSSSelectResult_DATA); + EXPECT_TRUE(SWSSZmqConsumerStateTable_hasData(cst)); + EXPECT_TRUE(SWSSZmqConsumerStateTable_hasCachedData(cst)); + arr = SWSSZmqConsumerStateTable_pops(cst); + EXPECT_FALSE(SWSSZmqConsumerStateTable_hasData(cst)); + EXPECT_FALSE(SWSSZmqConsumerStateTable_hasCachedData(cst)); + EXPECT_EQ(SWSSZmqConsumerStateTable_readData(cst, 500), SWSSSelectResult_TIMEOUT); + + vector kfvs = takeKeyOpFieldValuesArray(arr); + sortKfvs(kfvs); + freeKeyOpFieldValuesArray(arr); + + ASSERT_EQ(kfvs.size(), 2); + EXPECT_EQ(kfvKey(kfvs[0]), "mykey1"); + EXPECT_EQ(kfvOp(kfvs[0]), "SET"); + vector> &fieldValues0 = kfvFieldsValues(kfvs[0]); + ASSERT_EQ(fieldValues0.size(), 2); + EXPECT_EQ(fieldValues0[0].first, "myfield1"); + EXPECT_EQ(fieldValues0[0].second, "myvalue1"); + EXPECT_EQ(fieldValues0[1].first, "myfield2"); + EXPECT_EQ(fieldValues0[1].second, "myvalue2"); + + EXPECT_EQ(kfvKey(kfvs[1]), "mykey2"); + EXPECT_EQ(kfvOp(kfvs[1]), "SET"); + vector> &fieldValues1 = kfvFieldsValues(kfvs[1]); + ASSERT_EQ(fieldValues1.size(), 1); + EXPECT_EQ(fieldValues1[0].first, "myfield3"); + EXPECT_EQ(fieldValues1[0].second, "myvalue3"); + + EXPECT_FALSE(SWSSZmqConsumerStateTable_hasData(cst)); + arr = SWSSZmqConsumerStateTable_pops(cst); + ASSERT_EQ(arr.len, 0); + freeKeyOpFieldValuesArray(arr); + + arr_data[0] = {.key = "mykey3", .operation = "DEL", .fieldValues = {}}; + arr_data[1] = {.key = "mykey4", .operation = "DEL", .fieldValues = {}}; + arr = { .len = 2, .data = arr_data }; + + if (flag == 0) + for (uint64_t i = 0; i < arr.len; i++) + SWSSZmqProducerStateTable_del(pst, arr.data[i].key); + else + SWSSZmqClient_sendMsg(cli, "TEST_DB", "mytable", &arr); + + ASSERT_EQ(SWSSZmqConsumerStateTable_readData(cst, 500), SWSSSelectResult_DATA); + EXPECT_TRUE(SWSSZmqConsumerStateTable_hasData(cst)); + EXPECT_TRUE(SWSSZmqConsumerStateTable_hasCachedData(cst)); + arr = SWSSZmqConsumerStateTable_pops(cst); + EXPECT_FALSE(SWSSZmqConsumerStateTable_hasData(cst)); + EXPECT_FALSE(SWSSZmqConsumerStateTable_hasCachedData(cst)); + EXPECT_EQ(SWSSZmqConsumerStateTable_readData(cst, 500), SWSSSelectResult_TIMEOUT); + + kfvs = takeKeyOpFieldValuesArray(arr); + sortKfvs(kfvs); + freeKeyOpFieldValuesArray(arr); + + ASSERT_EQ(kfvs.size(), 2); + EXPECT_EQ(kfvKey(kfvs[0]), "mykey3"); + EXPECT_EQ(kfvOp(kfvs[0]), "DEL"); + EXPECT_EQ(kfvFieldsValues(kfvs[0]).size(), 0); + EXPECT_EQ(kfvKey(kfvs[1]), "mykey4"); + EXPECT_EQ(kfvOp(kfvs[1]), "DEL"); + EXPECT_EQ(kfvFieldsValues(kfvs[1]).size(), 0); + } + + // Server must be freed first to safely release message handlers (ZmqConsumerStateTable) + SWSSZmqServer_free(srv); + + SWSSZmqProducerStateTable_free(pst); + SWSSZmqConsumerStateTable_free(cst); + + SWSSZmqClient_free(cli); + + SWSSDBConnector_flushdb(db); + SWSSDBConnector_free(db); +} diff --git a/tests/main.cpp b/tests/main.cpp index 6cbaf251d..440978a46 100644 --- a/tests/main.cpp +++ b/tests/main.cpp @@ -1,5 +1,6 @@ #include "gtest/gtest.h" #include "common/dbconnector.h" +#include "common/c-api/util.h" #include using namespace std; @@ -84,6 +85,9 @@ class SwsscommonEnvironment : public ::testing::Environment { SonicDBConfig::initializeGlobalConfig(global_existing_file); cout<<"INIT: load global db config file, isInit = "< Date: Fri, 1 Nov 2024 07:55:03 +0800 Subject: [PATCH 03/19] Fix unit test for goext (#938) Why I did it #937 PR checker is blocked by goext build How I did it Remove go build, and keep go test to run unit test. How to verify it Pass all UT and E2E test cases. --- goext/Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/goext/Makefile b/goext/Makefile index d46d9908a..50beb76dc 100644 --- a/goext/Makefile +++ b/goext/Makefile @@ -17,7 +17,6 @@ all: $(SWIG) $(SWIG_FLAG) -I/usr/include/swss/ swsscommon.i check: - sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) build sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test clean: From deee7d6b23e0619d868ba9cc2c3e27a120edd150 Mon Sep 17 00:00:00 2001 From: Shuai Shang <47099361+shuaishang@users.noreply.github.com> Date: Fri, 1 Nov 2024 17:40:38 +0800 Subject: [PATCH 04/19] add PIC_CONTEXT_TABLE to shcema.h (#919) Co-authored-by: wenwang <2437730491@qq.com> --- common/schema.h | 1 + 1 file changed, 1 insertion(+) diff --git a/common/schema.h b/common/schema.h index 27aecdb0c..17dc3ca8b 100644 --- a/common/schema.h +++ b/common/schema.h @@ -156,6 +156,7 @@ namespace swss { #define APP_SRV6_SID_LIST_TABLE_NAME "SRV6_SID_LIST_TABLE" #define APP_SRV6_MY_SID_TABLE_NAME "SRV6_MY_SID_TABLE" +#define APP_PIC_CONTEXT_TABLE_NAME "PIC_CONTEXT_TABLE" #define APP_DASH_VNET_TABLE_NAME "DASH_VNET_TABLE" #define APP_DASH_QOS_TABLE_NAME "DASH_QOS_TABLE" From c6364d1bace34f4431adcdba26983f1a45ce4e94 Mon Sep 17 00:00:00 2001 From: Yijiao Qin Date: Fri, 1 Nov 2024 08:40:42 -0700 Subject: [PATCH 05/19] [common] add PerformanceTimer (#893) What I did Extend the PerformanceIntervalTimer utility in sonic-sairedis repo to measure API performance. To optimize performance of a periodically invoked function, we not only pay attention to how long it takes to finish execution (busy time), but also care about how long it waits to get executed (idle time). The total time determines the actual throughput in the real world system. Hence we want to see data in 3 ways: 1. Idle time between two calls (a gap between this start and the last end, during which the thread just waits) 2. Busy time of each API call (interval between this start and this end, it's the true execution time) 3. Total time elapsed (idle + busy) | <- Initial Gap -> | <- - 1st **Execution** - -> | <- 2nd Gap -> | <- - 2nd **Execution** - -> | | <---------------- 1st **Total** --------------> | <------------ 2nd **Total**--------------> | Other features 1. calculate the number of tasks finished per call (Routes Per Second in the context of routes loading) 2. batch the logging output 3. adjust the SWSSLOGLEVEL using a file indicator, default level is "INFO", hence we could enable perf logs in a dynamic way. Why I did it original utility has limited functionality we need gap data to obtain more insight on the mutual influence of upstream and downstream modules original utility is in libsairedis, to use it in sonic-swss, there would be much Makefile.am change, not necessary. a utility tool for swss should be included in swss-common when enabled on-demand, it could help measure the API performance under scaled traffic one use case: help measuring perf in pr optimized bgp loading speed --- common/Makefile.am | 3 +- common/performancetimer.cpp | 133 ++++++++++++++++++++++++++++++++++ common/performancetimer.h | 63 ++++++++++++++++ tests/Makefile.am | 1 + tests/performancetimer_ut.cpp | 43 +++++++++++ 5 files changed, 242 insertions(+), 1 deletion(-) create mode 100644 common/performancetimer.cpp create mode 100644 common/performancetimer.h create mode 100644 tests/performancetimer_ut.cpp diff --git a/common/Makefile.am b/common/Makefile.am index 5d1de753e..df41c3be1 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -77,7 +77,8 @@ common_libswsscommon_la_SOURCES = \ common/c-api/zmqclient.cpp \ common/c-api/zmqserver.cpp \ common/c-api/zmqconsumerstatetable.cpp \ - common/c-api/zmqproducerstatetable.cpp + common/c-api/zmqproducerstatetable.cpp \ + common/performancetimer.cpp common_libswsscommon_la_CXXFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CFLAGS) $(CODE_COVERAGE_CXXFLAGS) common_libswsscommon_la_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CPPFLAGS) $(CODE_COVERAGE_CPPFLAGS) diff --git a/common/performancetimer.cpp b/common/performancetimer.cpp new file mode 100644 index 000000000..400a55c8b --- /dev/null +++ b/common/performancetimer.cpp @@ -0,0 +1,133 @@ +#include "performancetimer.h" + +#include "logger.h" +#include +#include + +using namespace swss; + +bool PerformanceTimer::m_enable = true; +#define LIMIT 5 +#define INDICATOR "/var/log/syslog_notice_flag" + +PerformanceTimer::PerformanceTimer( + _In_ std::string funcName, + _In_ uint64_t threshold, + _In_ bool verbose): + m_name(funcName), + m_threshold(threshold), + m_verbose(verbose) +{ + reset(); + m_stop = std::chrono::steady_clock::now(); +} + +void PerformanceTimer::reset() +{ + SWSS_LOG_ENTER(); + + m_tasks = 0; + m_calls = 0; + m_busy = 0; + m_idle = 0; + + m_intervals.clear(); + m_gaps.clear(); + m_incs.clear(); +} + +void PerformanceTimer::start() +{ + SWSS_LOG_ENTER(); + + m_start = std::chrono::steady_clock::now(); + // measures the gap between this start() and the last stop() + m_gaps.push_back(std::chrono::duration_cast(m_start-m_stop).count()); +} + +void PerformanceTimer::stop() +{ + SWSS_LOG_ENTER(); + m_stop = std::chrono::steady_clock::now(); +} + +std::string PerformanceTimer::inc(uint64_t count) +{ + SWSS_LOG_ENTER(); + + std::string output = ""; + + m_calls += 1; + + m_tasks += count; + + m_idle += m_gaps.back(); + + uint64_t interval = std::chrono::duration_cast(m_stop - m_start).count(); + + m_busy += interval; + + if (count == 0) { + m_gaps.pop_back(); + m_calls -= 1; + return output; + } + + if (m_incs.size() <= LIMIT) { + m_incs.push_back(count); + m_intervals.push_back(interval/1000000); + } else { + m_gaps.pop_back(); + } + + if (m_tasks >= m_threshold) + { + uint64_t mseconds = m_busy/1000000; + + if (m_enable && mseconds > 0) + { + output = getTimerState(); + std::ifstream indicator(INDICATOR); + if (indicator.good()) { + SWSS_LOG_NOTICE("%s", output.c_str()); + } else { + SWSS_LOG_INFO("%s", output.c_str()); + } + } + + reset(); + } + + return output; +} + +std::string PerformanceTimer::getTimerState() +{ + nlohmann::json data; + data["API"] = m_name; + data["Tasks"] = m_tasks; + data["busy[ms]"] = m_busy/1000000; + data["idle[ms]"] = m_idle; + data["Total[ms]"] = m_busy/1000000 + m_idle; + double ratio = static_cast(m_tasks) / static_cast(m_busy/1000000 + m_idle); + data["RPS[k]"] = std::round(ratio * 10.0) / 10.0f; + if (m_verbose) { + data["m_intervals"] = m_intervals; + data["m_gaps"] = m_gaps; + data["m_incs"] = m_incs; + } + + return data.dump(); +} + +void PerformanceTimer::setTimerName(const std::string& funcName) { + m_name = funcName; +} + +void PerformanceTimer::setTimerThreshold(uint64_t threshold) { + m_threshold = threshold; +} + +void PerformanceTimer::setTimerVerbose(bool verbose) { + m_verbose = verbose; +} diff --git a/common/performancetimer.h b/common/performancetimer.h new file mode 100644 index 000000000..545aeeae5 --- /dev/null +++ b/common/performancetimer.h @@ -0,0 +1,63 @@ +#pragma once + +#include "sal.h" +#include + +#include +#include +#include +#include +namespace swss +{ + class PerformanceTimer + { + public: + + PerformanceTimer( + _In_ std::string funcName = "", + _In_ uint64_t threshold = 10000, + _In_ bool verbose = false + ); + + ~PerformanceTimer() = default; + + public: + + void start(); + + void stop(); + + std::string inc(uint64_t count = 1); + + void reset(); + + std::string getTimerState(); + + static bool m_enable; + + void setTimerName(const std::string& funcName); + void setTimerThreshold(uint64_t threshold); + void setTimerVerbose(bool verbose); + + private: + + std::string m_name; // records what this timer measures about + uint64_t m_threshold; // reset the timer when the m_tasks reachs m_threshold + bool m_verbose; // decides whether to print in verbose when m_threshold is reached + + std::chrono::time_point m_start; + std::chrono::time_point m_stop; + + /* records how long the timer has idled between last stop and this start */ + std::deque m_gaps; + /* records how long each call takes */ + std::deque m_intervals; + /* records how many tasks each call finishes */ + std::deque m_incs; + + uint64_t m_tasks; // sum of m_incs + uint64_t m_calls; // how many times the timer is used + uint64_t m_busy; // sum of m_intervals + uint64_t m_idle; // sum of m_gaps + }; +} diff --git a/tests/Makefile.am b/tests/Makefile.am index 39712b233..9642b09ab 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -43,6 +43,7 @@ tests_tests_SOURCES = tests/redis_ut.cpp \ tests/zmq_state_ut.cpp \ tests/profileprovider_ut.cpp \ tests/c_api_ut.cpp \ + tests/performancetimer_ut.cpp \ tests/main.cpp tests_tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS) diff --git a/tests/performancetimer_ut.cpp b/tests/performancetimer_ut.cpp new file mode 100644 index 000000000..4bdaf74ef --- /dev/null +++ b/tests/performancetimer_ut.cpp @@ -0,0 +1,43 @@ +#include "common/performancetimer.h" +#include +#include "gtest/gtest.h" +#include + +using namespace std; + +#define PRINT_ALL 1 + +TEST(PerformancetimerTest, basic) +{ + std::string expected; + + static swss::PerformanceTimer timer("basic", PRINT_ALL); + timer.start(); + this_thread::sleep_for(chrono::milliseconds(100)); + timer.stop(); + std::string output = timer.inc(1000); + + expected = R"({"API":"basic","RPS[k]":10.0,"Tasks":1000,"Total[ms]":100,"busy[ms]":100,"idle[ms]":0})"; + EXPECT_EQ(output, expected); + + timer.setTimerName("basic_set_name"); + timer.setTimerVerbose(true); + timer.setTimerThreshold(3000); + + timer.start(); + this_thread::sleep_for(chrono::milliseconds(100)); + timer.stop(); + output = timer.inc(1000); + EXPECT_EQ(output, ""); + + this_thread::sleep_for(chrono::milliseconds(200)); + + timer.start(); + this_thread::sleep_for(chrono::milliseconds(300)); + timer.stop(); + output = timer.inc(2000); + + expected = R"({"API":"basic_set_name","RPS[k]":5.0,"Tasks":3000,"Total[ms]":600,"busy[ms]":400,"idle[ms]":200,"m_gaps":[0,200],"m_incs":[1000,2000],"m_intervals":[100,300]})"; + + EXPECT_EQ(output, expected); +} From ace2080462d6ef36ce52e69a1391e2d9ebcfdabc Mon Sep 17 00:00:00 2001 From: ganglv <88995770+ganglyu@users.noreply.github.com> Date: Sat, 2 Nov 2024 19:28:38 +0800 Subject: [PATCH 06/19] Fix goext ut for bookworm (#940) Why I did it #937 PR checker is blocked by goext build How I did it Remove go build, and keep go test to run unit test. How to verify it Pass all UT and E2E test cases. --- goext/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/goext/Makefile b/goext/Makefile index 50beb76dc..76fddb7f5 100644 --- a/goext/Makefile +++ b/goext/Makefile @@ -17,6 +17,7 @@ all: $(SWIG) $(SWIG_FLAG) -I/usr/include/swss/ swsscommon.i check: + $(GO) mod init goext sudo CGO_LDFLAGS="$(CGO_LDFLAGS)" CGO_CXXFLAGS="$(CGO_CXXFLAGS)" $(GO) test clean: From bd0f2eb0613aba8a448645b3bf53a44a50bd763f Mon Sep 17 00:00:00 2001 From: Prince George <45705344+prgeor@users.noreply.github.com> Date: Sun, 3 Nov 2024 19:08:00 -0800 Subject: [PATCH 07/19] Added new PORT_OPERR_TABLE table in STATE_DB (#941) Signed-off-by: Prince George --- common/schema.h | 1 + 1 file changed, 1 insertion(+) diff --git a/common/schema.h b/common/schema.h index 17dc3ca8b..009c3cf34 100644 --- a/common/schema.h +++ b/common/schema.h @@ -479,6 +479,7 @@ namespace swss { #define STATE_ACL_STAGE_CAPABILITY_TABLE_NAME "ACL_STAGE_CAPABILITY_TABLE" #define STATE_PBH_CAPABILITIES_TABLE_NAME "PBH_CAPABILITIES" #define STATE_PORT_TABLE_NAME "PORT_TABLE" +#define STATE_PORT_OPER_ERR_TABLE_NAME "PORT_OPERR_TABLE" #define STATE_LAG_TABLE_NAME "LAG_TABLE" #define STATE_VLAN_TABLE_NAME "VLAN_TABLE" #define STATE_VLAN_MEMBER_TABLE_NAME "VLAN_MEMBER_TABLE" From e81295491aed73735feba4c178db93024af3c339 Mon Sep 17 00:00:00 2001 From: vvbrcm <56567015+vvbrcm@users.noreply.github.com> Date: Tue, 5 Nov 2024 14:53:39 -0800 Subject: [PATCH 08/19] Adding VRRP_TABLE name in APPL_DB (#927) --- common/schema.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/schema.h b/common/schema.h index 009c3cf34..2bc8ec399 100644 --- a/common/schema.h +++ b/common/schema.h @@ -103,6 +103,8 @@ namespace swss { #define APP_NAPT_POOL_IP_TABLE_NAME "NAPT_POOL_IP_TABLE" #define APP_NAT_DNAT_POOL_TABLE_NAME "NAT_DNAT_POOL_TABLE" +#define APP_VRRP_TABLE_NAME "VRRP_TABLE" + #define APP_STP_VLAN_TABLE_NAME "STP_VLAN_TABLE" #define APP_STP_VLAN_PORT_TABLE_NAME "STP_VLAN_PORT_TABLE" #define APP_STP_VLAN_INSTANCE_TABLE_NAME "STP_VLAN_INSTANCE_TABLE" From acc4805a8c2766a4d200c17fdfaba04b94792274 Mon Sep 17 00:00:00 2001 From: Hua Liu <58683130+liuh-80@users.noreply.github.com> Date: Fri, 8 Nov 2024 09:36:21 +0800 Subject: [PATCH 09/19] Fix the ZMQ producer/consumer table does not update changes to the database issue. (#942) #### Why I did it Fix a race condition bug: when deleting ZmqProducerStateTable immediately after setting/deleting data, the data is sent to ZMQ but not updated in the database. #### How I did it Verify and wait for the data update before the db update thread exits. ##### Work item tracking #### How to verify it Pass all test cases. #### Which release branch to backport (provide reason below if selected) - [ ] 201811 - [ ] 201911 - [ ] 202006 - [ ] 202012 - [ ] 202106 - [ ] 202111 #### Description for the changelog Fix the ZMQ producer/consumer table does not update changes to the database issue. #### Link to config_db schema for YANG module changes #### A picture of a cute animal (not mandatory but encouraged) --- common/asyncdbupdater.cpp | 17 ++++++++++++++++- tests/zmq_state_ut.cpp | 27 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/common/asyncdbupdater.cpp b/common/asyncdbupdater.cpp index 4cf150d9f..cf3d74c5f 100644 --- a/common/asyncdbupdater.cpp +++ b/common/asyncdbupdater.cpp @@ -30,6 +30,7 @@ AsyncDBUpdater::~AsyncDBUpdater() // notify db update thread exit m_dbUpdateDataNotifyCv.notify_all(); m_dbUpdateThread->join(); + SWSS_LOG_DEBUG("AsyncDBUpdater dtor tableName: %s", m_tableName.c_str()); } void AsyncDBUpdater::update(std::shared_ptr pkco) @@ -61,16 +62,30 @@ void AsyncDBUpdater::dbUpdateThread() std::mutex cvMutex; std::unique_lock cvLock(cvMutex); - while (m_runThread) + while (true) { size_t count; count = queueSize(); if (count == 0) { + // Check if there still data in queue before exit + if (!m_runThread) + { + SWSS_LOG_NOTICE("dbUpdateThread for table: %s is exiting", m_tableName.c_str()); + break; + } + // when queue is empty, wait notification, when data come, continue to check queue size again m_dbUpdateDataNotifyCv.wait(cvLock); continue; } + else + { + if (!m_runThread) + { + SWSS_LOG_DEBUG("dbUpdateThread for table: %s still has %d records that need to be sent before exiting", m_tableName.c_str(), (int)count); + } + } for (size_t ie = 0; ie < count; ie++) { diff --git a/tests/zmq_state_ut.cpp b/tests/zmq_state_ut.cpp index 4818b7fd8..56a8299f9 100644 --- a/tests/zmq_state_ut.cpp +++ b/tests/zmq_state_ut.cpp @@ -438,3 +438,30 @@ TEST(ZmqConsumerStateTableBatchBufferOverflow, test) } EXPECT_ANY_THROW(p.send(kcos)); } + +TEST(ZmqProducerStateTableDeleteAfterSend, test) +{ + std::string testTableName = "ZMQ_PROD_DELETE_UT"; + std::string pushEndpoint = "tcp://localhost:1234"; + std::string pullEndpoint = "tcp://*:1234"; + std::string testKey = "testKey"; + + ZmqServer server(pullEndpoint); + + DBConnector db(TEST_DB, 0, true); + ZmqClient client(pushEndpoint); + + auto *p = new ZmqProducerStateTable(&db, testTableName, client, true); + std::vector values; + FieldValueTuple t("test", "test"); + values.push_back(t); + p->set(testKey,values); + delete p; + + sleep(1); + + Table table(&db, testTableName); + std::vector keys; + table.getKeys(keys); + EXPECT_EQ(keys.front(), testKey); +} From dc75f2335b6f18b4f82986b19f8a5b65e3456790 Mon Sep 17 00:00:00 2001 From: erer1243 <1377477+erer1243@users.noreply.github.com> Date: Sat, 9 Nov 2024 03:26:28 -0500 Subject: [PATCH 10/19] C-api: refine interface on Selectable classes (#945) Previously, methods like hasData/hasCachedData were exposed through the FFI, but they are only useful for swss::Select which is not exposed, so those were deleted. Methods *_getFd were added so that ffi code may select on those fds separately from swss::Select (we need this in Rust for tokio::AsyncFd). Comments were added to better explain the intended use - Either call readData with a timeout to block in place, or use getFd to select on the fd and then call readData with a zero timeout to reset the fd. Co-authored-by: erer1243 --- common/c-api/consumerstatetable.cpp | 9 +++++++ common/c-api/consumerstatetable.h | 11 ++++++++ common/c-api/subscriberstatetable.cpp | 17 ++++-------- common/c-api/subscriberstatetable.h | 16 +++++------- common/c-api/util.h | 10 +++++-- common/c-api/zmqconsumerstatetable.cpp | 23 +++++----------- common/c-api/zmqconsumerstatetable.h | 18 ++++++------- tests/c_api_ut.cpp | 36 +++++++++----------------- 8 files changed, 67 insertions(+), 73 deletions(-) diff --git a/common/c-api/consumerstatetable.cpp b/common/c-api/consumerstatetable.cpp index c01ed8229..df0aba112 100644 --- a/common/c-api/consumerstatetable.cpp +++ b/common/c-api/consumerstatetable.cpp @@ -32,3 +32,12 @@ SWSSKeyOpFieldValuesArray SWSSConsumerStateTable_pops(SWSSConsumerStateTable tbl return makeKeyOpFieldValuesArray(vkco); }); } + +uint32_t SWSSConsumerStateTable_getFd(SWSSConsumerStateTable tbl) { + SWSSTry(return numeric_cast(((ConsumerStateTable *)tbl)->getFd())); +} + +SWSSSelectResult SWSSConsumerStateTable_readData(SWSSConsumerStateTable tbl, uint32_t timeout_ms, + uint8_t interrupt_on_signal) { + SWSSTry(return selectOne((ConsumerStateTable *)tbl, timeout_ms, interrupt_on_signal)); +} diff --git a/common/c-api/consumerstatetable.h b/common/c-api/consumerstatetable.h index bd2fdaaf0..468fb644b 100644 --- a/common/c-api/consumerstatetable.h +++ b/common/c-api/consumerstatetable.h @@ -21,6 +21,17 @@ void SWSSConsumerStateTable_free(SWSSConsumerStateTable tbl); // Result array and all of its members must be freed using free() SWSSKeyOpFieldValuesArray SWSSConsumerStateTable_pops(SWSSConsumerStateTable tbl); +// Return the underlying fd for polling/selecting on. +// Callers must NOT read/write on fd, it may only be used for epoll or similar. +// After the fd becomes readable, SWSSConsumerStateTable_readData must be used to +// reset the fd and read data into internal data structures. +uint32_t SWSSConsumerStateTable_getFd(SWSSConsumerStateTable tbl); + +// Block until data is available to read or until a timeout elapses. +// A timeout of 0 means the call will return immediately. +SWSSSelectResult SWSSConsumerStateTable_readData(SWSSConsumerStateTable tbl, uint32_t timeout_ms, + uint8_t interrupt_on_signal); + #ifdef __cplusplus } #endif diff --git a/common/c-api/subscriberstatetable.cpp b/common/c-api/subscriberstatetable.cpp index b64829117..8bafa3903 100644 --- a/common/c-api/subscriberstatetable.cpp +++ b/common/c-api/subscriberstatetable.cpp @@ -34,19 +34,12 @@ SWSSKeyOpFieldValuesArray SWSSSubscriberStateTable_pops(SWSSSubscriberStateTable }); } -uint8_t SWSSSubscriberStateTable_hasData(SWSSSubscriberStateTable tbl) { - SWSSTry(return ((SubscriberStateTable *)tbl)->hasData() ? 1 : 0); -} - -uint8_t SWSSSubscriberStateTable_hasCachedData(SWSSSubscriberStateTable tbl) { - SWSSTry(return ((SubscriberStateTable *)tbl)->hasCachedData() ? 1 : 0); -} - -uint8_t SWSSSubscriberStateTable_initializedWithData(SWSSSubscriberStateTable tbl) { - SWSSTry(return ((SubscriberStateTable *)tbl)->initializedWithData() ? 1 : 0); +uint32_t SWSSSubscriberStateTable_getFd(SWSSSubscriberStateTable tbl) { + SWSSTry(return numeric_cast(((SubscriberStateTable *)tbl)->getFd())); } SWSSSelectResult SWSSSubscriberStateTable_readData(SWSSSubscriberStateTable tbl, - uint32_t timeout_ms) { - SWSSTry(return selectOne((SubscriberStateTable *)tbl, timeout_ms)); + uint32_t timeout_ms, + uint8_t interrupt_on_signal) { + SWSSTry(return selectOne((SubscriberStateTable *)tbl, timeout_ms, interrupt_on_signal)); } diff --git a/common/c-api/subscriberstatetable.h b/common/c-api/subscriberstatetable.h index 4501a3af4..ed0924c81 100644 --- a/common/c-api/subscriberstatetable.h +++ b/common/c-api/subscriberstatetable.h @@ -22,19 +22,17 @@ void SWSSSubscriberStateTable_free(SWSSSubscriberStateTable tbl); // Result array and all of its members must be freed using free() SWSSKeyOpFieldValuesArray SWSSSubscriberStateTable_pops(SWSSSubscriberStateTable tbl); -// Returns 0 for false, 1 for true -uint8_t SWSSSubscriberStateTable_hasData(SWSSSubscriberStateTable tbl); - -// Returns 0 for false, 1 for true -uint8_t SWSSSubscriberStateTable_hasCachedData(SWSSSubscriberStateTable tbl); - -// Returns 0 for false, 1 for true -uint8_t SWSSSubscriberStateTable_initializedWithData(SWSSSubscriberStateTable tbl); +// Return the underlying fd for polling/selecting on. +// Callers must NOT read/write on fd, it may only be used for epoll or similar. +// After the fd becomes readable, SWSSSubscriberStateTable_readData must be used to +// reset the fd and read data into internal data structures. +uint32_t SWSSSubscriberStateTable_getFd(SWSSSubscriberStateTable tbl); // Block until data is available to read or until a timeout elapses. // A timeout of 0 means the call will return immediately. SWSSSelectResult SWSSSubscriberStateTable_readData(SWSSSubscriberStateTable tbl, - uint32_t timeout_ms); + uint32_t timeout_ms, + uint8_t interrupt_on_sugnal); #ifdef __cplusplus } diff --git a/common/c-api/util.h b/common/c-api/util.h index 79eb93cfd..357818cbe 100644 --- a/common/c-api/util.h +++ b/common/c-api/util.h @@ -29,9 +29,14 @@ typedef struct { const SWSSKeyOpFieldValues *data; } SWSSKeyOpFieldValuesArray; +// FFI version of swss::Select::{OBJECT, TIMEOUT, SIGNALINT}. +// swss::Select::ERROR is left out because errors are handled separately typedef enum { + // Data is available in the object SWSSSelectResult_DATA = 0, + // Timed out waiting for data SWSSSelectResult_TIMEOUT = 1, + // Waiting was interrupted by a signal SWSSSelectResult_SIGNAL = 2, } SWSSSelectResult; @@ -74,11 +79,12 @@ extern bool cApiTestingDisableAbort; } \ } -static inline SWSSSelectResult selectOne(swss::Selectable *s, uint32_t timeout_ms) { +static inline SWSSSelectResult selectOne(swss::Selectable *s, uint32_t timeout_ms, + uint8_t interrupt_on_signal) { Select select; Selectable *sOut; select.addSelectable(s); - int ret = select.select(&sOut, numeric_cast(timeout_ms)); + int ret = select.select(&sOut, numeric_cast(timeout_ms), interrupt_on_signal); switch (ret) { case Select::OBJECT: return SWSSSelectResult_DATA; diff --git a/common/c-api/zmqconsumerstatetable.cpp b/common/c-api/zmqconsumerstatetable.cpp index 38cd87f93..62b0fe221 100644 --- a/common/c-api/zmqconsumerstatetable.cpp +++ b/common/c-api/zmqconsumerstatetable.cpp @@ -3,6 +3,7 @@ #include "util.h" #include "zmqconsumerstatetable.h" #include "zmqserver.h" +#include using namespace swss; using namespace std; @@ -32,24 +33,14 @@ SWSSKeyOpFieldValuesArray SWSSZmqConsumerStateTable_pops(SWSSZmqConsumerStateTab }); } -SWSSSelectResult SWSSZmqConsumerStateTable_readData(SWSSZmqConsumerStateTable tbl, - uint32_t timeout_ms) { - SWSSTry(return selectOne((ZmqConsumerStateTable *)tbl, timeout_ms)); -} - -// Returns 0 for false, 1 for true -uint8_t SWSSZmqConsumerStateTable_hasData(SWSSZmqConsumerStateTable tbl) { - SWSSTry(return ((ZmqConsumerStateTable *)tbl)->hasData() ? 1 : 0); +uint32_t SWSSZmqConsumerStateTable_getFd(SWSSZmqConsumerStateTable tbl) { + SWSSTry(return numeric_cast(((ZmqConsumerStateTable *)tbl)->getFd())); } -// Returns 0 for false, 1 for true -uint8_t SWSSZmqConsumerStateTable_hasCachedData(SWSSZmqConsumerStateTable tbl) { - SWSSTry(return ((ZmqConsumerStateTable *)tbl)->hasCachedData() ? 1 : 0); -} - -// Returns 0 for false, 1 for true -uint8_t SWSSZmqConsumerStateTable_initializedWithData(SWSSZmqConsumerStateTable tbl) { - SWSSTry(return ((ZmqConsumerStateTable *)tbl)->initializedWithData() ? 1 : 0); +SWSSSelectResult SWSSZmqConsumerStateTable_readData(SWSSZmqConsumerStateTable tbl, + uint32_t timeout_ms, + uint8_t interrupt_on_signal) { + SWSSTry(return selectOne((ZmqConsumerStateTable *)tbl, timeout_ms, interrupt_on_signal)); } const struct SWSSDBConnectorOpaque * diff --git a/common/c-api/zmqconsumerstatetable.h b/common/c-api/zmqconsumerstatetable.h index 4810c3ef5..f5b934258 100644 --- a/common/c-api/zmqconsumerstatetable.h +++ b/common/c-api/zmqconsumerstatetable.h @@ -24,19 +24,17 @@ void SWSSZmqConsumerStateTable_free(SWSSZmqConsumerStateTable tbl); // Result array and all of its members must be freed using free() SWSSKeyOpFieldValuesArray SWSSZmqConsumerStateTable_pops(SWSSZmqConsumerStateTable tbl); +// Return the underlying fd for polling/selecting on. +// Callers must NOT read/write on fd, it may only be used for epoll or similar. +// After the fd becomes readable, SWSSZmqConsumerStateTable_readData must be used to +// reset the fd and read data into internal data structures. +uint32_t SWSSZmqConsumerStateTable_getFd(SWSSZmqConsumerStateTable tbl); + // Block until data is available to read or until a timeout elapses. // A timeout of 0 means the call will return immediately. SWSSSelectResult SWSSZmqConsumerStateTable_readData(SWSSZmqConsumerStateTable tbl, - uint32_t timeout_ms); - -// Returns 0 for false, 1 for true -uint8_t SWSSZmqConsumerStateTable_hasData(SWSSZmqConsumerStateTable tbl); - -// Returns 0 for false, 1 for true -uint8_t SWSSZmqConsumerStateTable_hasCachedData(SWSSZmqConsumerStateTable tbl); - -// Returns 0 for false, 1 for true -uint8_t SWSSZmqConsumerStateTable_initializedWithData(SWSSZmqConsumerStateTable tbl); + uint32_t timeout_ms, + uint8_t interrupt_on_signal); const struct SWSSDBConnectorOpaque * SWSSZmqConsumerStateTable_getDbConnector(SWSSZmqConsumerStateTable tbl); diff --git a/tests/c_api_ut.cpp b/tests/c_api_ut.cpp index d16dac7c1..90af97162 100644 --- a/tests/c_api_ut.cpp +++ b/tests/c_api_ut.cpp @@ -94,6 +94,8 @@ TEST(c_api, ConsumerProducerStateTables) { SWSSProducerStateTable pst = SWSSProducerStateTable_new(db, "mytable"); SWSSConsumerStateTable cst = SWSSConsumerStateTable_new(db, "mytable", nullptr, nullptr); + SWSSConsumerStateTable_getFd(cst); + SWSSKeyOpFieldValuesArray arr = SWSSConsumerStateTable_pops(cst); ASSERT_EQ(arr.len, 0); freeKeyOpFieldValuesArray(arr); @@ -110,6 +112,7 @@ TEST(c_api, ConsumerProducerStateTables) { values.len = 1; SWSSProducerStateTable_set(pst, "mykey2", values); + ASSERT_EQ(SWSSConsumerStateTable_readData(cst, 300, true), SWSSSelectResult_DATA); arr = SWSSConsumerStateTable_pops(cst); vector kfvs = takeKeyOpFieldValuesArray(arr); sortKfvs(kfvs); @@ -131,7 +134,7 @@ TEST(c_api, ConsumerProducerStateTables) { ASSERT_EQ(fieldValues1.size(), 1); EXPECT_EQ(fieldValues1[0].first, "myfield3"); EXPECT_EQ(fieldValues1[0].second, "myvalue3"); - + arr = SWSSConsumerStateTable_pops(cst); EXPECT_EQ(arr.len, 0); freeKeyOpFieldValuesArray(arr); @@ -168,23 +171,20 @@ TEST(c_api, SubscriberStateTable) { SWSSDBConnector db = SWSSDBConnector_new_named("TEST_DB", 1000, true); SWSSSubscriberStateTable sst = SWSSSubscriberStateTable_new(db, "mytable", nullptr, nullptr); - EXPECT_EQ(SWSSSubscriberStateTable_readData(sst, 300), SWSSSelectResult_TIMEOUT); - EXPECT_FALSE(SWSSSubscriberStateTable_hasData(sst)); + SWSSSubscriberStateTable_getFd(sst); + + EXPECT_EQ(SWSSSubscriberStateTable_readData(sst, 300, true), SWSSSelectResult_TIMEOUT); SWSSKeyOpFieldValuesArray arr = SWSSSubscriberStateTable_pops(sst); EXPECT_EQ(arr.len, 0); freeKeyOpFieldValuesArray(arr); SWSSDBConnector_hset(db, "mytable:mykey", "myfield", "myvalue"); - EXPECT_EQ(SWSSSubscriberStateTable_readData(sst, 300), SWSSSelectResult_DATA); - EXPECT_EQ(SWSSSubscriberStateTable_readData(sst, 300), SWSSSelectResult_TIMEOUT); - EXPECT_TRUE(SWSSSubscriberStateTable_hasData(sst)); - + ASSERT_EQ(SWSSSubscriberStateTable_readData(sst, 300, true), SWSSSelectResult_DATA); arr = SWSSSubscriberStateTable_pops(sst); vector kfvs = takeKeyOpFieldValuesArray(arr); sortKfvs(kfvs); freeKeyOpFieldValuesArray(arr); - EXPECT_FALSE(SWSSSubscriberStateTable_hasData(sst)); ASSERT_EQ(kfvs.size(), 1); EXPECT_EQ(kfvKey(kfvs[0]), "mykey"); EXPECT_EQ(kfvOp(kfvs[0]), "SET"); @@ -211,11 +211,10 @@ TEST(c_api, ZmqConsumerProducerStateTable) { SWSSZmqConsumerStateTable cst = SWSSZmqConsumerStateTable_new(db, "mytable", srv, nullptr, nullptr); + SWSSZmqConsumerStateTable_getFd(cst); + ASSERT_EQ(SWSSZmqConsumerStateTable_getDbConnector(cst), db); - EXPECT_FALSE(SWSSZmqConsumerStateTable_hasData(cst)); - EXPECT_FALSE(SWSSZmqConsumerStateTable_hasCachedData(cst)); - EXPECT_FALSE(SWSSZmqConsumerStateTable_initializedWithData(cst)); SWSSKeyOpFieldValuesArray arr = SWSSZmqConsumerStateTable_pops(cst); ASSERT_EQ(arr.len, 0); freeKeyOpFieldValuesArray(arr); @@ -247,13 +246,8 @@ TEST(c_api, ZmqConsumerProducerStateTable) { else SWSSZmqClient_sendMsg(cli, "TEST_DB", "mytable", &arr); - ASSERT_EQ(SWSSZmqConsumerStateTable_readData(cst, 500), SWSSSelectResult_DATA); - EXPECT_TRUE(SWSSZmqConsumerStateTable_hasData(cst)); - EXPECT_TRUE(SWSSZmqConsumerStateTable_hasCachedData(cst)); + ASSERT_EQ(SWSSZmqConsumerStateTable_readData(cst, 1500, true), SWSSSelectResult_DATA); arr = SWSSZmqConsumerStateTable_pops(cst); - EXPECT_FALSE(SWSSZmqConsumerStateTable_hasData(cst)); - EXPECT_FALSE(SWSSZmqConsumerStateTable_hasCachedData(cst)); - EXPECT_EQ(SWSSZmqConsumerStateTable_readData(cst, 500), SWSSSelectResult_TIMEOUT); vector kfvs = takeKeyOpFieldValuesArray(arr); sortKfvs(kfvs); @@ -276,7 +270,6 @@ TEST(c_api, ZmqConsumerProducerStateTable) { EXPECT_EQ(fieldValues1[0].first, "myfield3"); EXPECT_EQ(fieldValues1[0].second, "myvalue3"); - EXPECT_FALSE(SWSSZmqConsumerStateTable_hasData(cst)); arr = SWSSZmqConsumerStateTable_pops(cst); ASSERT_EQ(arr.len, 0); freeKeyOpFieldValuesArray(arr); @@ -291,13 +284,8 @@ TEST(c_api, ZmqConsumerProducerStateTable) { else SWSSZmqClient_sendMsg(cli, "TEST_DB", "mytable", &arr); - ASSERT_EQ(SWSSZmqConsumerStateTable_readData(cst, 500), SWSSSelectResult_DATA); - EXPECT_TRUE(SWSSZmqConsumerStateTable_hasData(cst)); - EXPECT_TRUE(SWSSZmqConsumerStateTable_hasCachedData(cst)); + ASSERT_EQ(SWSSZmqConsumerStateTable_readData(cst, 500, true), SWSSSelectResult_DATA); arr = SWSSZmqConsumerStateTable_pops(cst); - EXPECT_FALSE(SWSSZmqConsumerStateTable_hasData(cst)); - EXPECT_FALSE(SWSSZmqConsumerStateTable_hasCachedData(cst)); - EXPECT_EQ(SWSSZmqConsumerStateTable_readData(cst, 500), SWSSSelectResult_TIMEOUT); kfvs = takeKeyOpFieldValuesArray(arr); sortKfvs(kfvs); From b686bb0942a20b3b2832baec91019dcd81a397a8 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak <38952541+stepanblyschak@users.noreply.github.com> Date: Sat, 9 Nov 2024 10:31:19 +0200 Subject: [PATCH 11/19] Add helper function to validate interface name length (#931) Signed-off-by: Stepan Blyschak Co-authored-by: afeigin --- common/Makefile.am | 1 + common/interface.h | 19 +++++++++++++++++++ pyext/swsscommon.i | 4 +++- tests/test_interface.py | 8 ++++++++ 4 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 common/interface.h create mode 100644 tests/test_interface.py diff --git a/common/Makefile.am b/common/Makefile.am index df41c3be1..724805e60 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -69,6 +69,7 @@ common_libswsscommon_la_SOURCES = \ common/zmqserver.cpp \ common/asyncdbupdater.cpp \ common/redis_table_waiter.cpp \ + common/interface.h \ common/c-api/util.cpp \ common/c-api/dbconnector.cpp \ common/c-api/consumerstatetable.cpp \ diff --git a/common/interface.h b/common/interface.h new file mode 100644 index 000000000..320ac883a --- /dev/null +++ b/common/interface.h @@ -0,0 +1,19 @@ +#ifndef __INTERFACE__ +#define __INTERFACE__ + +#include +#include + +namespace swss +{ + +const size_t IFACE_NAME_MAX_LEN = IFNAMSIZ - 1; + +bool isInterfaceNameValid(const std::string &ifaceName) +{ + return !ifaceName.empty() && (ifaceName.length() < IFNAMSIZ); +} + +} + +#endif diff --git a/pyext/swsscommon.i b/pyext/swsscommon.i index 2bf953b11..b3d015e03 100644 --- a/pyext/swsscommon.i +++ b/pyext/swsscommon.i @@ -58,6 +58,7 @@ #include "zmqproducerstatetable.h" #include #include +#include "interface.h" %} %include @@ -282,6 +283,7 @@ T castSelectableObj(swss::Selectable *temp) %include "zmqserver.h" %include "zmqclient.h" %include "zmqconsumerstatetable.h" +%include "interface.h" %extend swss::DBConnector { %template(hgetall) hgetall>; @@ -296,7 +298,7 @@ T castSelectableObj(swss::Selectable *temp) %include "table.h" #ifdef ENABLE_YANG_MODULES %include "decoratortable.h" -#endif +#endif %clear std::vector &keys; %clear std::vector &ops; %clear std::vector>> &fvss; diff --git a/tests/test_interface.py b/tests/test_interface.py new file mode 100644 index 000000000..25c809ce3 --- /dev/null +++ b/tests/test_interface.py @@ -0,0 +1,8 @@ +from swsscommon import swsscommon + +def test_is_interface_name_valid(): + invalid_interface_name = "TooLongInterfaceName" + assert not swsscommon.isInterfaceNameValid(invalid_interface_name) + + validInterfaceName = "OkInterfaceName" + assert swsscommon.isInterfaceNameValid(validInterfaceName) From 1408db8bb0c5cbf00c787167793cbeff4c47e6c6 Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox <57339448+Junchao-Mellanox@users.noreply.github.com> Date: Sat, 9 Nov 2024 17:30:22 +0800 Subject: [PATCH 12/19] Add more information when connect redis fail (#882) Co-authored-by: Liat Grozovik <44433539+liat-grozovik@users.noreply.github.com> Co-authored-by: Guohan Lu --- common/dbconnector.cpp | 4 ++-- tests/redis_ut.cpp | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/common/dbconnector.cpp b/common/dbconnector.cpp index 96334780f..47fe80d3b 100755 --- a/common/dbconnector.cpp +++ b/common/dbconnector.cpp @@ -562,7 +562,7 @@ void RedisContext::initContext(const char *host, int port, const timeval *tv) if (m_conn->err) throw system_error(make_error_code(errc::address_not_available), - "Unable to connect to redis"); + "Unable to connect to redis - " + std::string(m_conn->errstr) + "(" + std::to_string(m_conn->err) + ")"); } void RedisContext::initContext(const char *path, const timeval *tv) @@ -578,7 +578,7 @@ void RedisContext::initContext(const char *path, const timeval *tv) if (m_conn->err) throw system_error(make_error_code(errc::address_not_available), - "Unable to connect to redis (unix-socket)"); + "Unable to connect to redis (unix-socket) - " + std::string(m_conn->errstr) + "(" + std::to_string(m_conn->err) + ")"); } redisContext *RedisContext::getContext() const diff --git a/tests/redis_ut.cpp b/tests/redis_ut.cpp index 4f691e88a..f53f891d4 100644 --- a/tests/redis_ut.cpp +++ b/tests/redis_ut.cpp @@ -3,6 +3,8 @@ #include #include #include +#include +#include #include "gtest/gtest.h" #include "common/dbconnector.h" #include "common/producertable.h" @@ -20,6 +22,7 @@ using namespace std; using namespace swss; +using namespace testing; #define NUMBER_OF_THREADS (64) // Spawning more than 256 threads causes libc++ to except #define NUMBER_OF_OPS (1000) @@ -1139,3 +1142,32 @@ TEST(Connector, hmset) // test empty multi hash db.hmset({}); } + +TEST(Connector, connectFail) +{ + // connect to an ip which is not a redis server + EXPECT_THROW({ + try + { + DBConnector db(0, "1.1.1.1", 6379, 1); + } + catch(const std::system_error& e) + { + EXPECT_THAT(e.what(), HasSubstr("Unable to connect to redis - ")); + throw; + } + }, std::system_error); + + // connect to an invalid unix socket address + EXPECT_THROW({ + try + { + DBConnector db(0, "/tmp/invalid", 1); + } + catch(const std::system_error& e) + { + EXPECT_THAT(e.what(), HasSubstr("Unable to connect to redis (unix-socket) - ")); + throw; + } + }, std::system_error); +} From f6c1614227f25dfa81ab5ccd0cb8cca265aaad7d Mon Sep 17 00:00:00 2001 From: divyagayathri-hcl <159437886+divyagayathri-hcl@users.noreply.github.com> Date: Sat, 9 Nov 2024 15:03:35 +0530 Subject: [PATCH 13/19] Add function ActionSchemaByNameAndObjectType to allow different ActionSchema formats. (#909) Summary: Add function ActionSchemaByNameAndObjectType to allow different ActionSchema formats. The SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT action redirects to different object types, which have different formats. Multicast groups are hex strings, while next hops and ports are regular strings. --- common/saiaclschema.cpp | 27 +++++++++++++++++++++++++++ common/saiaclschema.h | 4 ++++ tests/saiaclschema_ut.cpp | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+) diff --git a/common/saiaclschema.cpp b/common/saiaclschema.cpp index 6fd32214d..88c6f5175 100644 --- a/common/saiaclschema.cpp +++ b/common/saiaclschema.cpp @@ -328,5 +328,32 @@ const ActionSchema &ActionSchemaByName(const std::string &action_name) return lookup->second; } +const ActionSchema& ActionSchemaByNameAndObjectType( + const std::string& action_name, const std::string& object_type) { + static const auto* const kRedirectObjectTypes = + new std::unordered_map({ + {"SAI_OBJECT_TYPE_IPMC_GROUP", + {.format = Format::kHexString, .bitwidth = 16}}, + {"SAI_OBJECT_TYPE_L2MC_GROUP", + {.format = Format::kHexString, .bitwidth = 16}}, + // SAI_OBJECT_TYPE_BRIDGE_PORT + // SAI_OBJECT_TYPE_LAG + // SAI_OBJECT_TYPE_NEXT_HOP + // SAI_OBJECT_TYPE_NEXT_HOP_GROUP + // SAI_OBJECT_TYPE_PORT + // SAI_OBJECT_TYPE_SYSTEM_PORT + }); + + if (action_name == "SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT") { + auto lookup = kRedirectObjectTypes->find(object_type); + if (lookup != kRedirectObjectTypes->end()) { + return lookup->second; + } + } + // If we haven't defined the object type, fall through to the default + // SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT format. + return ActionSchemaByName(action_name); +} + } // namespace acl } // namespace swss diff --git a/common/saiaclschema.h b/common/saiaclschema.h index 156148b14..88e664232 100644 --- a/common/saiaclschema.h +++ b/common/saiaclschema.h @@ -83,6 +83,10 @@ const MatchFieldSchema &MatchFieldSchemaByName(const std::string &match_field_na // Throws std::invalid_argument for unknown actions and actions without schemas. const ActionSchema &ActionSchemaByName(const std::string &action_name); +// Allow further format differentiation based on a SAI object type. +const ActionSchema& ActionSchemaByNameAndObjectType( + const std::string& action_name, const std::string& object_type); + } // namespace acl } // namespace swss diff --git a/tests/saiaclschema_ut.cpp b/tests/saiaclschema_ut.cpp index fff9158d5..1f828f77b 100644 --- a/tests/saiaclschema_ut.cpp +++ b/tests/saiaclschema_ut.cpp @@ -60,6 +60,37 @@ TEST(SaiAclSchemaTest, ActionSchemaByNameSucceeds) AllOf(Field(&ActionSchema::format, Format::kHexString), Field(&ActionSchema::bitwidth, 12))); } +TEST(SaiAclSchemaTest, ActionSchemaByNameAndObjectTypeSucceeds) { + EXPECT_THAT( + ActionSchemaByNameAndObjectType("SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT", + "SAI_OBJECT_TYPE_IPMC_GROUP"), + AllOf(Field(&ActionSchema::format, Format::kHexString), + Field(&ActionSchema::bitwidth, 16))); + EXPECT_THAT( + ActionSchemaByNameAndObjectType("SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT", + "SAI_OBJECT_TYPE_L2MC_GROUP"), + AllOf(Field(&ActionSchema::format, Format::kHexString), + Field(&ActionSchema::bitwidth, 16))); + EXPECT_THAT( + ActionSchemaByNameAndObjectType("SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT", + "SAI_OBJECT_TYPE_NEXT_HOP"), + AllOf(Field(&ActionSchema::format, Format::kString), + Field(&ActionSchema::bitwidth, 0))); + EXPECT_THAT(ActionSchemaByNameAndObjectType( + "SAI_ACL_ENTRY_ATTR_ACTION_REDIRECT", "SAI_OBJECT_TYPE_PORT"), + AllOf(Field(&ActionSchema::format, Format::kString), + Field(&ActionSchema::bitwidth, 0))); +} + +TEST(SaiAclSchemaTest, + ActionSchemaByNameAndObjectTypeWithNonRedirectActionSucceeds) { + EXPECT_THAT( + ActionSchemaByNameAndObjectType("SAI_ACL_ENTRY_ATTR_ACTION_DECREMENT_TTL", + "SAI_OBJECT_TYPE_UNKNOWN"), + AllOf(Field(&ActionSchema::format, Format::kHexString), + Field(&ActionSchema::bitwidth, 1))); +} + // Invalid Lookup Tests TEST(SaiAclSchemaTest, InvalidFormatNameThrowsException) @@ -82,6 +113,11 @@ TEST(SaiAclSchemaTest, InvalidActionNameThrowsException) EXPECT_THROW(ActionSchemaByName("Foo"), std::invalid_argument); } +TEST(SaiAclSchemaTest, InvalidActionNameAndObjectTypeThrowsException) { + EXPECT_THROW(ActionSchemaByNameAndObjectType("Foo", "unknown"), + std::invalid_argument); +} + } // namespace } // namespace acl } // namespace swss From 11e4055ae3fcb8872253b5e9df7d75e74cbe6cd1 Mon Sep 17 00:00:00 2001 From: Yevhen Fastiuk Date: Mon, 11 Nov 2024 18:35:29 +0200 Subject: [PATCH 14/19] Add LOGGING table (#783) Signed-off-by: Yevhen Fastiuk Co-authored-by: Saikrishna Arcot --- common/schema.h | 1 + 1 file changed, 1 insertion(+) diff --git a/common/schema.h b/common/schema.h index 2bc8ec399..e616f1282 100644 --- a/common/schema.h +++ b/common/schema.h @@ -460,6 +460,7 @@ namespace swss { #define CFG_TWAMP_SESSION_TABLE_NAME "TWAMP_SESSION" #define CFG_BANNER_MESSAGE_TABLE_NAME "BANNER_MESSAGE" +#define CFG_LOGGING_TABLE_NAME "LOGGING" #define CFG_DHCP_TABLE "DHCP_RELAY" From 378e82818165d75aa2fdc66961e72c4852eae69b Mon Sep 17 00:00:00 2001 From: erer1243 <1377477+erer1243@users.noreply.github.com> Date: Thu, 14 Nov 2024 00:45:32 -0500 Subject: [PATCH 15/19] C API: improve performance by not memcpy-ing string DB values across ffi boundary (#935) This introduces a couple of new FFI types, representing owned C++ strings and arrays. We use those owned strings for database values, so they do not need to be memcpy'd across ffi boundaries. This is in response to Riff's concerns that database values which hamgrd will be reading may be large and expensive to memcpy. Partner pr to sonic-net/sonic-dash-ha#11 Co-authored-by: erer1243 --- common/c-api/consumerstatetable.cpp | 2 + common/c-api/dbconnector.cpp | 29 +++-- common/c-api/dbconnector.h | 57 ++------ common/c-api/producerstatetable.cpp | 2 +- common/c-api/producerstatetable.h | 5 - common/c-api/subscriberstatetable.cpp | 2 + common/c-api/util.cpp | 30 +++++ common/c-api/util.h | 172 ++++++++++++++++++------- common/c-api/zmqclient.cpp | 4 +- common/c-api/zmqclient.h | 2 +- common/c-api/zmqconsumerstatetable.cpp | 2 + common/c-api/zmqproducerstatetable.cpp | 3 + tests/c_api_ut.cpp | 84 +++++++----- 13 files changed, 251 insertions(+), 143 deletions(-) diff --git a/common/c-api/consumerstatetable.cpp b/common/c-api/consumerstatetable.cpp index df0aba112..9765ceec3 100644 --- a/common/c-api/consumerstatetable.cpp +++ b/common/c-api/consumerstatetable.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -10,6 +11,7 @@ using namespace swss; using namespace std; +using boost::numeric_cast; SWSSConsumerStateTable SWSSConsumerStateTable_new(SWSSDBConnector db, const char *tableName, const int32_t *p_popBatchSize, diff --git a/common/c-api/dbconnector.cpp b/common/c-api/dbconnector.cpp index bb32f42aa..83f237cc0 100644 --- a/common/c-api/dbconnector.cpp +++ b/common/c-api/dbconnector.cpp @@ -1,5 +1,6 @@ #include #include +#include #include "../dbconnector.h" #include "dbconnector.h" @@ -37,14 +38,14 @@ int8_t SWSSDBConnector_del(SWSSDBConnector db, const char *key) { SWSSTry(return ((DBConnector *)db)->del(string(key)) ? 1 : 0); } -void SWSSDBConnector_set(SWSSDBConnector db, const char *key, const char *value) { - SWSSTry(((DBConnector *)db)->set(string(key), string(value))); +void SWSSDBConnector_set(SWSSDBConnector db, const char *key, SWSSStrRef value) { + SWSSTry(((DBConnector *)db)->set(string(key), takeStrRef(value))); } -char *SWSSDBConnector_get(SWSSDBConnector db, const char *key) { +SWSSString SWSSDBConnector_get(SWSSDBConnector db, const char *key) { SWSSTry({ shared_ptr s = ((DBConnector *)db)->get(string(key)); - return s ? strdup(s->c_str()) : nullptr; + return s ? makeString(move(*s)) : nullptr; }); } @@ -57,21 +58,29 @@ int8_t SWSSDBConnector_hdel(SWSSDBConnector db, const char *key, const char *fie } void SWSSDBConnector_hset(SWSSDBConnector db, const char *key, const char *field, - const char *value) { - SWSSTry(((DBConnector *)db)->hset(string(key), string(field), string(value))); + SWSSStrRef value) { + SWSSTry(((DBConnector *)db)->hset(string(key), string(field), takeStrRef(value))); } -char *SWSSDBConnector_hget(SWSSDBConnector db, const char *key, const char *field) { +SWSSString SWSSDBConnector_hget(SWSSDBConnector db, const char *key, const char *field) { SWSSTry({ shared_ptr s = ((DBConnector *)db)->hget(string(key), string(field)); - return s ? strdup(s->c_str()) : nullptr; + return s ? makeString(move(*s)) : nullptr; }); } SWSSFieldValueArray SWSSDBConnector_hgetall(SWSSDBConnector db, const char *key) { SWSSTry({ - auto map = ((DBConnector *)db)->hgetall(key); - return makeFieldValueArray(map); + auto map = ((DBConnector *)db)->hgetall(string(key)); + + // We can't move keys out of the map, we have to copy them, until C++17 map::extract so we + // copy them here into a vector to avoid needing an overload on makeFieldValueArray + vector> pairs; + pairs.reserve(map.size()); + for (auto &pair : map) + pairs.push_back(make_pair(pair.first, move(pair.second))); + + return makeFieldValueArray(std::move(pairs)); }); } diff --git a/common/c-api/dbconnector.h b/common/c-api/dbconnector.h index 8e6c51e0b..fe4acdf4d 100644 --- a/common/c-api/dbconnector.h +++ b/common/c-api/dbconnector.h @@ -29,11 +29,11 @@ void SWSSDBConnector_free(SWSSDBConnector db); // Returns 0 when key doesn't exist, 1 when key was deleted int8_t SWSSDBConnector_del(SWSSDBConnector db, const char *key); -void SWSSDBConnector_set(SWSSDBConnector db, const char *key, const char *value); +void SWSSDBConnector_set(SWSSDBConnector db, const char *key, SWSSStrRef value); -// Returns NULL if key doesn't exist. -// Result must be freed using free() -char *SWSSDBConnector_get(SWSSDBConnector db, const char *key); +// Returns NULL if key doesn't exist +// Result must be freed using SWSSString_free() +SWSSString SWSSDBConnector_get(SWSSDBConnector db, const char *key); // Returns 0 for false, 1 for true int8_t SWSSDBConnector_exists(SWSSDBConnector db, const char *key); @@ -41,59 +41,22 @@ int8_t SWSSDBConnector_exists(SWSSDBConnector db, const char *key); // Returns 0 when key or field doesn't exist, 1 when field was deleted int8_t SWSSDBConnector_hdel(SWSSDBConnector db, const char *key, const char *field); -void SWSSDBConnector_hset(SWSSDBConnector db, const char *key, const char *field, - const char *value); +void SWSSDBConnector_hset(SWSSDBConnector db, const char *key, const char *field, SWSSStrRef value); -// Returns NULL if key or field doesn't exist. -// Result must be freed using free() -char *SWSSDBConnector_hget(SWSSDBConnector db, const char *key, const char *field); +// Returns NULL if key or field doesn't exist +// Result must be freed using SWSSString_free() +SWSSString SWSSDBConnector_hget(SWSSDBConnector db, const char *key, const char *field); -// Returns an empty map when the key doesn't exist. -// Result array and all of its elements must be freed using free() +// Returns an empty map when the key doesn't exist +// Result array and all of its elements must be freed using appropriate free functions SWSSFieldValueArray SWSSDBConnector_hgetall(SWSSDBConnector db, const char *key); // Returns 0 when key or field doesn't exist, 1 when field exists int8_t SWSSDBConnector_hexists(SWSSDBConnector db, const char *key, const char *field); -// std::vector keys(const std::string &key); - -// std::pair> scan(int cursor = 0, const char -// *match = "", uint32_t count = 10); - -// template -// void hmset(const std::string &key, InputIterator start, InputIterator stop); - -// void hmset(const std::unordered_map>>& multiHash); - -// std::shared_ptr get(const std::string &key); - -// std::shared_ptr hget(const std::string &key, const std::string -// &field); - -// int64_t incr(const std::string &key); - -// int64_t decr(const std::string &key); - -// int64_t rpush(const std::string &list, const std::string &item); - -// std::shared_ptr blpop(const std::string &list, int timeout); - -// void subscribe(const std::string &pattern); - -// void psubscribe(const std::string &pattern); - -// void punsubscribe(const std::string &pattern); - -// int64_t publish(const std::string &channel, const std::string &message); - -// void config_set(const std::string &key, const std::string &value); - // Returns 1 on success, 0 on failure int8_t SWSSDBConnector_flushdb(SWSSDBConnector db); -// std::map>> getall(); #ifdef __cplusplus } #endif diff --git a/common/c-api/producerstatetable.cpp b/common/c-api/producerstatetable.cpp index 083536d7e..276d7c680 100644 --- a/common/c-api/producerstatetable.cpp +++ b/common/c-api/producerstatetable.cpp @@ -25,7 +25,7 @@ void SWSSProducerStateTable_setBuffered(SWSSProducerStateTable tbl, uint8_t buff void SWSSProducerStateTable_set(SWSSProducerStateTable tbl, const char *key, SWSSFieldValueArray values) { - SWSSTry(((ProducerStateTable *)tbl)->set(string(key), takeFieldValueArray(values))); + SWSSTry(((ProducerStateTable *)tbl)->set(string(key), takeFieldValueArray(std::move(values)))); } void SWSSProducerStateTable_del(SWSSProducerStateTable tbl, const char *key) { diff --git a/common/c-api/producerstatetable.h b/common/c-api/producerstatetable.h index e8db2c65d..1acb9af37 100644 --- a/common/c-api/producerstatetable.h +++ b/common/c-api/producerstatetable.h @@ -22,11 +22,6 @@ void SWSSProducerStateTable_set(SWSSProducerStateTable tbl, const char *key, SWS void SWSSProducerStateTable_del(SWSSProducerStateTable tbl, const char *key); -// Batched version of set() and del(). -// virtual void set(const std::vector& values); - -// virtual void del(const std::vector& keys); - void SWSSProducerStateTable_flush(SWSSProducerStateTable tbl); int64_t SWSSProducerStateTable_count(SWSSProducerStateTable tbl); diff --git a/common/c-api/subscriberstatetable.cpp b/common/c-api/subscriberstatetable.cpp index 8bafa3903..4d3a04953 100644 --- a/common/c-api/subscriberstatetable.cpp +++ b/common/c-api/subscriberstatetable.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -11,6 +12,7 @@ using namespace swss; using namespace std; +using boost::numeric_cast; SWSSSubscriberStateTable SWSSSubscriberStateTable_new(SWSSDBConnector db, const char *tableName, const int32_t *p_popBatchSize, diff --git a/common/c-api/util.cpp b/common/c-api/util.cpp index fb983d5cf..1dc6cd451 100644 --- a/common/c-api/util.cpp +++ b/common/c-api/util.cpp @@ -1,3 +1,33 @@ #include "util.h" +using namespace swss; + bool swss::cApiTestingDisableAbort = false; + +SWSSString SWSSString_new(const char *data, uint64_t length) { + SWSSTry(return makeString(std::string(data, numeric_cast(length)))); +} + +SWSSString SWSSString_new_c_str(const char *c_str) { + SWSSTry(return makeString(std::string(c_str))); +} + +const char *SWSSStrRef_c_str(SWSSStrRef s) { + SWSSTry(return ((std::string *)s)->c_str()); +} + +uint64_t SWSSStrRef_length(SWSSStrRef s) { + SWSSTry(return ((std::string *)s)->length()); +} + +void SWSSString_free(SWSSString s) { + SWSSTry(delete (std::string *)s); +} + +void SWSSFieldValueArray_free(SWSSFieldValueArray arr) { + SWSSTry(delete[] arr.data); +} + +void SWSSKeyOpFieldValuesArray_free(SWSSKeyOpFieldValuesArray kfvs) { + SWSSTry(delete[] kfvs.data); +} diff --git a/common/c-api/util.h b/common/c-api/util.h index 357818cbe..06aeac15e 100644 --- a/common/c-api/util.h +++ b/common/c-api/util.h @@ -8,25 +8,44 @@ extern "C" { #include +// FFI version of std::string&& +// This can be converted to an SWSSStrRef with a standard cast +typedef struct SWSSStringOpaque *SWSSString; + +// FFI version of std::string& +// This can be converted to an SWSSString with a standard cast +// Functions that take SWSSString will move data out of the underlying string, +// but functions that take SWSSStrRef will only view it. +typedef struct SWSSStrRefOpaque *SWSSStrRef; + +// FFI version of swss::FieldValueTuple typedef struct { const char *field; - const char *value; -} SWSSFieldValuePair; + SWSSString value; +} SWSSFieldValueTuple; +// FFI version of std::vector typedef struct { uint64_t len; - const SWSSFieldValuePair *data; + SWSSFieldValueTuple *data; } SWSSFieldValueArray; +typedef enum { + SWSSKeyOperation_SET, + SWSSKeyOperation_DEL, +} SWSSKeyOperation; + +// FFI version of swss::KeyOpFieldValuesTuple typedef struct { const char *key; - const char *operation; + SWSSKeyOperation operation; SWSSFieldValueArray fieldValues; } SWSSKeyOpFieldValues; +// FFI version of std::vector typedef struct { uint64_t len; - const SWSSKeyOpFieldValues *data; + SWSSKeyOpFieldValues *data; } SWSSKeyOpFieldValuesArray; // FFI version of swss::Select::{OBJECT, TIMEOUT, SIGNALINT}. @@ -40,21 +59,52 @@ typedef enum { SWSSSelectResult_SIGNAL = 2, } SWSSSelectResult; +// data should not include a null terminator +SWSSString SWSSString_new(const char *data, uint64_t length); + +// c_str should include a null terminator +SWSSString SWSSString_new_c_str(const char *c_str); + +// It is safe to pass null to this function (not to any other SWSSString functions). This is +// useful to take SWSSStrings from other SWSS structs - you can replace the strs in the +// structs with null and still safely free the structs. Then, you can call this function with the +// populated SWSSString later. +void SWSSString_free(SWSSString s); + +const char *SWSSStrRef_c_str(SWSSStrRef s); + +// Returns the length of the string, not including the null terminator that is implicitly added by +// SWSSStrRef_c_str. +uint64_t SWSSStrRef_length(SWSSStrRef s); + +// arr.data may be null. This is not recursive - elements must be freed separately (for finer +// grained control of ownership). +void SWSSFieldValueArray_free(SWSSFieldValueArray arr); + +// kfvs.data may be null. This is not recursive - elements must be freed separately (for finer +// grained control of ownership). +void SWSSKeyOpFieldValuesArray_free(SWSSKeyOpFieldValuesArray kfvs); + #ifdef __cplusplus } #endif // Internal utilities (used to help define c-facing functions) #ifdef __cplusplus -#include -#include + +#include +#include #include #include +#include +#include #include #include +#include #include "../logger.h" -#include "../rediscommand.h" +#include "../redisapi.h" +#include "../schema.h" #include "../select.h" using boost::numeric_cast; @@ -67,7 +117,7 @@ extern bool cApiTestingDisableAbort; // undefined behavior. It was also decided that no exceptions in swss-common are recoverable, so // there is no reason to convert exceptions into a returnable type. #define SWSSTry(...) \ - if (cApiTestingDisableAbort) { \ + if (swss::cApiTestingDisableAbort) { \ { __VA_ARGS__; } \ } else { \ try { \ @@ -99,22 +149,21 @@ static inline SWSSSelectResult selectOne(swss::Selectable *s, uint32_t timeout_m } } -// malloc() with safe numeric casting of the size parameter -template static inline void *mallocN(N size) { - return malloc(numeric_cast(size)); +static inline SWSSString makeString(std::string &&s) { + std::string *data_s = new std::string(std::move(s)); + return (struct SWSSStringOpaque *)data_s; } // T is anything that has a .size() method and which can be iterated over for pair -// eg unordered_map or vector> -template static inline SWSSFieldValueArray makeFieldValueArray(const T &in) { - SWSSFieldValuePair *data = - (SWSSFieldValuePair *)mallocN(in.size() * sizeof(SWSSFieldValuePair)); +// eg vector> +template static inline SWSSFieldValueArray makeFieldValueArray(T &&in) { + SWSSFieldValueTuple *data = new SWSSFieldValueTuple[in.size()]; size_t i = 0; - for (const auto &pair : in) { - SWSSFieldValuePair entry; + for (auto &pair : in) { + SWSSFieldValueTuple entry; entry.field = strdup(pair.first.c_str()); - entry.value = strdup(pair.second.c_str()); + entry.value = makeString(std::move(pair.second)); data[i++] = entry; } @@ -124,48 +173,40 @@ template static inline SWSSFieldValueArray makeFieldValueArray(const T return out; } -static inline std::vector -takeFieldValueArray(const SWSSFieldValueArray &in) { - std::vector out; - for (uint64_t i = 0; i < in.len; i++) { - auto field = std::string(in.data[i].field); - auto value = std::string(in.data[i].value); - out.push_back(std::make_pair(field, value)); +static inline SWSSKeyOperation makeKeyOperation(std::string &op) { + if (strcmp(op.c_str(), SET_COMMAND) == 0) { + return SWSSKeyOperation_SET; + } else if (strcmp(op.c_str(), DEL_COMMAND) == 0) { + return SWSSKeyOperation_DEL; + } else { + SWSS_LOG_THROW("Invalid key operation %s", op.c_str()); } - return out; } -static inline SWSSKeyOpFieldValues makeKeyOpFieldValues(const swss::KeyOpFieldsValuesTuple &in) { +static inline SWSSKeyOpFieldValues makeKeyOpFieldValues(swss::KeyOpFieldsValuesTuple &&in) { SWSSKeyOpFieldValues out; out.key = strdup(kfvKey(in).c_str()); - out.operation = strdup(kfvOp(in).c_str()); + out.operation = makeKeyOperation(kfvOp(in)); out.fieldValues = makeFieldValueArray(kfvFieldsValues(in)); return out; } -static inline swss::KeyOpFieldsValuesTuple takeKeyOpFieldValues(const SWSSKeyOpFieldValues &in) { - std::string key(in.key), op(in.operation); - auto fieldValues = takeFieldValueArray(in.fieldValues); - return std::make_tuple(key, op, fieldValues); -} - -template static inline const T &getReference(const T &t) { +template static inline T &getReference(T &t) { return t; } -template static inline const T &getReference(const std::shared_ptr &t) { +template static inline T &getReference(std::shared_ptr &t) { return *t; } // T is anything that has a .size() method and which can be iterated over for -// swss::KeyOpFieldValuesTuple -template static inline SWSSKeyOpFieldValuesArray makeKeyOpFieldValuesArray(const T &in) { - SWSSKeyOpFieldValues *data = - (SWSSKeyOpFieldValues *)mallocN(in.size() * sizeof(SWSSKeyOpFieldValues)); +// swss::KeyOpFieldValuesTuple, eg vector or deque +template static inline SWSSKeyOpFieldValuesArray makeKeyOpFieldValuesArray(T &&in) { + SWSSKeyOpFieldValues *data = new SWSSKeyOpFieldValues[in.size()]; size_t i = 0; - for (const auto &kfv : in) - data[i++] = makeKeyOpFieldValues(getReference(kfv)); + for (auto &kfv : in) + data[i++] = makeKeyOpFieldValues(std::move(getReference(kfv))); SWSSKeyOpFieldValuesArray out; out.len = (uint64_t)in.size(); @@ -173,11 +214,50 @@ template static inline SWSSKeyOpFieldValuesArray makeKeyOpFieldValuesA return out; } +static inline std::string takeString(SWSSString s) { + return std::string(std::move(*((std::string *)s))); +} + +static inline std::string &takeStrRef(SWSSStrRef s) { + return *((std::string *)s); +} + +static inline std::vector takeFieldValueArray(SWSSFieldValueArray in) { + std::vector out; + for (uint64_t i = 0; i < in.len; i++) { + const char *field = in.data[i].field; + SWSSString value = in.data[i].value; + auto pair = std::make_pair(std::string(field), takeString(std::move(value))); + out.push_back(pair); + } + return out; +} + +static inline std::string takeKeyOperation(SWSSKeyOperation op) { + switch (op) { + case SWSSKeyOperation_SET: + return SET_COMMAND; + case SWSSKeyOperation_DEL: + return DEL_COMMAND; + default: + SWSS_LOG_THROW("Impossible SWSSKeyOperation"); + } +} + +static inline swss::KeyOpFieldsValuesTuple takeKeyOpFieldValues(SWSSKeyOpFieldValues in) { + std::string key = in.key; + std::string op = takeKeyOperation(in.operation); + auto fieldValues = takeFieldValueArray(in.fieldValues); + return std::make_tuple(key, op, fieldValues); +} + static inline std::vector -takeKeyOpFieldValuesArray(const SWSSKeyOpFieldValuesArray &in) { +takeKeyOpFieldValuesArray(SWSSKeyOpFieldValuesArray in) { std::vector out; - for (uint64_t i = 0; i < in.len; i++) - out.push_back(takeKeyOpFieldValues(in.data[i])); + for (uint64_t i = 0; i < in.len; i++) { + SWSSKeyOpFieldValues kfv = in.data[i]; + out.push_back(takeKeyOpFieldValues(std::move(kfv))); + } return out; } diff --git a/common/c-api/zmqclient.cpp b/common/c-api/zmqclient.cpp index 7e4a58f87..49a9e05f7 100644 --- a/common/c-api/zmqclient.cpp +++ b/common/c-api/zmqclient.cpp @@ -24,9 +24,9 @@ void SWSSZmqClient_connect(SWSSZmqClient zmqc) { } void SWSSZmqClient_sendMsg(SWSSZmqClient zmqc, const char *dbName, const char *tableName, - const SWSSKeyOpFieldValuesArray *arr) { + SWSSKeyOpFieldValuesArray arr) { SWSSTry({ - vector kcos = takeKeyOpFieldValuesArray(*arr); + vector kcos = takeKeyOpFieldValuesArray(arr); size_t bufSize = BinarySerializer::serializedSize(dbName, tableName, kcos); vector v(bufSize); ((ZmqClient *)zmqc) diff --git a/common/c-api/zmqclient.h b/common/c-api/zmqclient.h index 47cd1efba..da832ab30 100644 --- a/common/c-api/zmqclient.h +++ b/common/c-api/zmqclient.h @@ -21,7 +21,7 @@ int8_t SWSSZmqClient_isConnected(SWSSZmqClient zmqc); void SWSSZmqClient_connect(SWSSZmqClient zmqc); void SWSSZmqClient_sendMsg(SWSSZmqClient zmqc, const char *dbName, const char *tableName, - const SWSSKeyOpFieldValuesArray *kcos); + SWSSKeyOpFieldValuesArray kcos); #ifdef __cplusplus } diff --git a/common/c-api/zmqconsumerstatetable.cpp b/common/c-api/zmqconsumerstatetable.cpp index 62b0fe221..ed416488e 100644 --- a/common/c-api/zmqconsumerstatetable.cpp +++ b/common/c-api/zmqconsumerstatetable.cpp @@ -1,3 +1,4 @@ +#include #include "../zmqconsumerstatetable.h" #include "../table.h" #include "util.h" @@ -7,6 +8,7 @@ using namespace swss; using namespace std; +using boost::numeric_cast; // Pass NULL for popBatchSize and/or pri to use the default values SWSSZmqConsumerStateTable SWSSZmqConsumerStateTable_new(SWSSDBConnector db, const char *tableName, diff --git a/common/c-api/zmqproducerstatetable.cpp b/common/c-api/zmqproducerstatetable.cpp index 3e50916e1..e1c186806 100644 --- a/common/c-api/zmqproducerstatetable.cpp +++ b/common/c-api/zmqproducerstatetable.cpp @@ -1,8 +1,11 @@ +#include + #include "zmqproducerstatetable.h" #include "../zmqproducerstatetable.h" using namespace std; using namespace swss; +using boost::numeric_cast; SWSSZmqProducerStateTable SWSSZmqProducerStateTable_new(SWSSDBConnector db, const char *tableName, SWSSZmqClient zmqc, uint8_t dbPersistence) { diff --git a/tests/c_api_ut.cpp b/tests/c_api_ut.cpp index 90af97162..ed814607e 100644 --- a/tests/c_api_ut.cpp +++ b/tests/c_api_ut.cpp @@ -1,4 +1,3 @@ -#include #include #include @@ -39,44 +38,63 @@ static void sortKfvs(vector &kfvs) { } } -template static void free(const T *ptr) { - std::free(const_cast(reinterpret_cast(ptr))); -} +#define free(x) std::free(const_cast(reinterpret_cast(x))); static void freeKeyOpFieldValuesArray(SWSSKeyOpFieldValuesArray arr) { for (uint64_t i = 0; i < arr.len; i++) { free(arr.data[i].key); - free(arr.data[i].operation); for (uint64_t j = 0; j < arr.data[i].fieldValues.len; j++) { free(arr.data[i].fieldValues.data[j].field); - free(arr.data[i].fieldValues.data[j].value); + SWSSString_free(arr.data[i].fieldValues.data[j].value); } - free(arr.data[i].fieldValues.data); + SWSSFieldValueArray_free(arr.data[i].fieldValues); } - free(arr.data); + SWSSKeyOpFieldValuesArray_free(arr); } +struct SWSSStringManager { + vector m_strings; + + SWSSString makeString(const char *c_str) { + SWSSString s = SWSSString_new_c_str(c_str); + m_strings.push_back(s); + return s; + } + + SWSSStrRef makeStrRef(const char *c_str) { + return (SWSSStrRef)makeString(c_str); + } + + ~SWSSStringManager() { + for (SWSSString s : m_strings) + SWSSString_free(s); + } +}; + TEST(c_api, DBConnector) { clearDB(); + SWSSStringManager sm; EXPECT_THROW(SWSSDBConnector_new_named("does not exist", 0, true), out_of_range); SWSSDBConnector db = SWSSDBConnector_new_named("TEST_DB", 1000, true); - EXPECT_FALSE(SWSSDBConnector_get(db, "mykey")); + EXPECT_EQ(SWSSDBConnector_get(db, "mykey"), nullptr); EXPECT_FALSE(SWSSDBConnector_exists(db, "mykey")); - SWSSDBConnector_set(db, "mykey", "myval"); - const char *val = SWSSDBConnector_get(db, "mykey"); - EXPECT_STREQ(val, "myval"); - free(val); + + SWSSDBConnector_set(db, "mykey", sm.makeStrRef("myval")); + SWSSString val = SWSSDBConnector_get(db, "mykey"); + EXPECT_STREQ(SWSSStrRef_c_str((SWSSStrRef)val), "myval"); + SWSSString_free(val); EXPECT_TRUE(SWSSDBConnector_exists(db, "mykey")); EXPECT_TRUE(SWSSDBConnector_del(db, "mykey")); EXPECT_FALSE(SWSSDBConnector_del(db, "mykey")); EXPECT_FALSE(SWSSDBConnector_hget(db, "mykey", "myfield")); EXPECT_FALSE(SWSSDBConnector_hexists(db, "mykey", "myfield")); - SWSSDBConnector_hset(db, "mykey", "myfield", "myval"); + SWSSDBConnector_hset(db, "mykey", "myfield", sm.makeStrRef("myval")); val = SWSSDBConnector_hget(db, "mykey", "myfield"); - EXPECT_STREQ(val, "myval"); - free(val); + EXPECT_STREQ(SWSSStrRef_c_str((SWSSStrRef)val), "myval"); + SWSSString_free(val); + EXPECT_TRUE(SWSSDBConnector_hexists(db, "mykey", "myfield")); EXPECT_FALSE(SWSSDBConnector_hget(db, "mykey", "notmyfield")); EXPECT_FALSE(SWSSDBConnector_hexists(db, "mykey", "notmyfield")); @@ -89,6 +107,7 @@ TEST(c_api, DBConnector) { TEST(c_api, ConsumerProducerStateTables) { clearDB(); + SWSSStringManager sm; SWSSDBConnector db = SWSSDBConnector_new_named("TEST_DB", 1000, true); SWSSProducerStateTable pst = SWSSProducerStateTable_new(db, "mytable"); @@ -100,15 +119,16 @@ TEST(c_api, ConsumerProducerStateTables) { ASSERT_EQ(arr.len, 0); freeKeyOpFieldValuesArray(arr); - SWSSFieldValuePair data[2] = {{.field = "myfield1", .value = "myvalue1"}, - {.field = "myfield2", .value = "myvalue2"}}; + SWSSFieldValueTuple data[2] = { + {.field = "myfield1", .value = sm.makeString("myvalue1")}, + {.field = "myfield2", .value = sm.makeString("myvalue2")}}; SWSSFieldValueArray values = { .len = 2, .data = data, }; SWSSProducerStateTable_set(pst, "mykey1", values); - data[0] = {.field = "myfield3", .value = "myvalue3"}; + data[0] = {.field = "myfield3", .value = sm.makeString("myvalue3")}; values.len = 1; SWSSProducerStateTable_set(pst, "mykey2", values); @@ -167,6 +187,7 @@ TEST(c_api, ConsumerProducerStateTables) { TEST(c_api, SubscriberStateTable) { clearDB(); + SWSSStringManager sm; SWSSDBConnector db = SWSSDBConnector_new_named("TEST_DB", 1000, true); SWSSSubscriberStateTable sst = SWSSSubscriberStateTable_new(db, "mytable", nullptr, nullptr); @@ -178,8 +199,8 @@ TEST(c_api, SubscriberStateTable) { EXPECT_EQ(arr.len, 0); freeKeyOpFieldValuesArray(arr); - SWSSDBConnector_hset(db, "mytable:mykey", "myfield", "myvalue"); - ASSERT_EQ(SWSSSubscriberStateTable_readData(sst, 300, true), SWSSSelectResult_DATA); + SWSSDBConnector_hset(db, "mytable:mykey", "myfield", sm.makeStrRef("myvalue")); + EXPECT_EQ(SWSSSubscriberStateTable_readData(sst, 300, true), SWSSSelectResult_DATA); arr = SWSSSubscriberStateTable_pops(sst); vector kfvs = takeKeyOpFieldValuesArray(arr); sortKfvs(kfvs); @@ -199,6 +220,7 @@ TEST(c_api, SubscriberStateTable) { TEST(c_api, ZmqConsumerProducerStateTable) { clearDB(); + SWSSStringManager sm; SWSSDBConnector db = SWSSDBConnector_new_named("TEST_DB", 1000, true); @@ -222,29 +244,29 @@ TEST(c_api, ZmqConsumerProducerStateTable) { // On flag = 0, we use the ZmqProducerStateTable // On flag = 1, we use the ZmqClient directly for (int flag = 0; flag < 2; flag++) { - SWSSFieldValuePair values_key1_data[2] = {{.field = "myfield1", .value = "myvalue1"}, - {.field = "myfield2", .value = "myvalue2"}}; + SWSSFieldValueTuple values_key1_data[2] = {{.field = "myfield1", .value = sm.makeString("myvalue1")}, + {.field = "myfield2", .value = sm.makeString("myvalue2")}}; SWSSFieldValueArray values_key1 = { .len = 2, .data = values_key1_data, }; - SWSSFieldValuePair values_key2_data[1] = {{.field = "myfield3", .value = "myvalue3"}}; + SWSSFieldValueTuple values_key2_data[1] = {{.field = "myfield3", .value = sm.makeString("myvalue3")}}; SWSSFieldValueArray values_key2 = { .len = 1, .data = values_key2_data, }; SWSSKeyOpFieldValues arr_data[2] = { - {.key = "mykey1", .operation = "SET", .fieldValues = values_key1}, - {.key = "mykey2", .operation = "SET", .fieldValues = values_key2}}; + {.key = "mykey1", .operation = SWSSKeyOperation_SET, .fieldValues = values_key1}, + {.key = "mykey2", .operation = SWSSKeyOperation_SET, .fieldValues = values_key2}}; arr = {.len = 2, .data = arr_data}; if (flag == 0) for (uint64_t i = 0; i < arr.len; i++) SWSSZmqProducerStateTable_set(pst, arr.data[i].key, arr.data[i].fieldValues); else - SWSSZmqClient_sendMsg(cli, "TEST_DB", "mytable", &arr); + SWSSZmqClient_sendMsg(cli, "TEST_DB", "mytable", arr); ASSERT_EQ(SWSSZmqConsumerStateTable_readData(cst, 1500, true), SWSSSelectResult_DATA); arr = SWSSZmqConsumerStateTable_pops(cst); @@ -274,15 +296,15 @@ TEST(c_api, ZmqConsumerProducerStateTable) { ASSERT_EQ(arr.len, 0); freeKeyOpFieldValuesArray(arr); - arr_data[0] = {.key = "mykey3", .operation = "DEL", .fieldValues = {}}; - arr_data[1] = {.key = "mykey4", .operation = "DEL", .fieldValues = {}}; - arr = { .len = 2, .data = arr_data }; + arr_data[0] = {.key = "mykey3", .operation = SWSSKeyOperation_DEL, .fieldValues = {}}; + arr_data[1] = {.key = "mykey4", .operation = SWSSKeyOperation_DEL, .fieldValues = {}}; + arr = {.len = 2, .data = arr_data}; if (flag == 0) for (uint64_t i = 0; i < arr.len; i++) SWSSZmqProducerStateTable_del(pst, arr.data[i].key); else - SWSSZmqClient_sendMsg(cli, "TEST_DB", "mytable", &arr); + SWSSZmqClient_sendMsg(cli, "TEST_DB", "mytable", arr); ASSERT_EQ(SWSSZmqConsumerStateTable_readData(cst, 500, true), SWSSSelectResult_DATA); arr = SWSSZmqConsumerStateTable_pops(cst); From 901f3b4482b7162848c3dd5536cf494f1039c9ac Mon Sep 17 00:00:00 2001 From: Yijiao Qin Date: Mon, 18 Nov 2024 09:45:53 -0800 Subject: [PATCH 16/19] [common] enable redispipeline to only publish after flush (#895) What I did optimize redispipeline flush performance by remove unnecessary publish commands add a new parameterbool flushPub in producerstatetable constructor function to enable/disable batch publish feature default value of m_flushPub is false, so no impact on existing codes optimization is effective only explicitly set this option remove individual publish command from the producerstatetable APIs' lua scripts add a publish command when the pipeline flushes [if m_flushPub is true] Why I did it save TCP traffic and increase fpmsyncd efficiency It's a feature included in BGP Loading Optimization HLD #1521 --- common/producerstatetable.cpp | 93 +++++++++++++++++++++------------- common/producerstatetable.h | 4 ++ common/redispipeline.h | 44 ++++++++++++++++ tests/redis_piped_state_ut.cpp | 56 ++++++++++++++++++++ 4 files changed, 162 insertions(+), 35 deletions(-) diff --git a/common/producerstatetable.cpp b/common/producerstatetable.cpp index d0db5e2a5..c7a35475e 100644 --- a/common/producerstatetable.cpp +++ b/common/producerstatetable.cpp @@ -14,39 +14,71 @@ using namespace std; namespace swss { ProducerStateTable::ProducerStateTable(DBConnector *db, const string &tableName) - : ProducerStateTable(new RedisPipeline(db, 1), tableName, false) + : ProducerStateTable(new RedisPipeline(db, 1), tableName, false, false) { m_pipeowned = true; } ProducerStateTable::ProducerStateTable(RedisPipeline *pipeline, const string &tableName, bool buffered) + : ProducerStateTable(pipeline, tableName, buffered, false) {} + +ProducerStateTable::ProducerStateTable(RedisPipeline *pipeline, const string &tableName, bool buffered, bool flushPub) : TableBase(tableName, SonicDBConfig::getSeparator(pipeline->getDBConnector())) , TableName_KeySet(tableName) + , m_flushPub(flushPub) , m_buffered(buffered) , m_pipeowned(false) , m_tempViewActive(false) , m_pipe(pipeline) { + reloadRedisScript(); + + string luaClear = + "redis.call('DEL', KEYS[1])\n" + "local keys = redis.call('KEYS', KEYS[2] .. '*')\n" + "for i,k in pairs(keys) do\n" + " redis.call('DEL', k)\n" + "end\n" + "redis.call('DEL', KEYS[3])\n"; + m_shaClear = m_pipe->loadRedisScript(luaClear); + + string luaApplyView = loadLuaScript("producer_state_table_apply_view.lua"); + m_shaApplyView = m_pipe->loadRedisScript(luaApplyView); +} + +ProducerStateTable::~ProducerStateTable() +{ + if (m_pipeowned) + { + delete m_pipe; + } +} + +void ProducerStateTable::reloadRedisScript() +{ + // Set m_flushPub to remove publish from a single lua string and let pipeline do publish once per flush + + // However, if m_buffered is false, follow the original one publish per lua design + // Hence we need to check both m_buffered and m_flushPub, and reload the redis script once setBuffered() changes m_buffered + + /* 1. Inform the pipeline of what channel to publish, when flushPub feature is enabled */ + if (m_buffered && m_flushPub) + m_pipe->addChannel(getChannelName(m_pipe->getDbId())); + + /* 2. Setup lua strings: determine whether to attach luaPub after each lua string */ + // num in luaSet and luaDel means number of elements that were added to the key set, // not including all the elements already present into the set. string luaSet = "local added = redis.call('SADD', KEYS[2], ARGV[2])\n" "for i = 0, #KEYS - 3 do\n" " redis.call('HSET', KEYS[3 + i], ARGV[3 + i * 2], ARGV[4 + i * 2])\n" - "end\n" - " if added > 0 then \n" - " redis.call('PUBLISH', KEYS[1], ARGV[1])\n" "end\n"; - m_shaSet = m_pipe->loadRedisScript(luaSet); string luaDel = "local added = redis.call('SADD', KEYS[2], ARGV[2])\n" "redis.call('SADD', KEYS[4], ARGV[2])\n" - "redis.call('DEL', KEYS[3])\n" - "if added > 0 then \n" - " redis.call('PUBLISH', KEYS[1], ARGV[1])\n" - "end\n"; - m_shaDel = m_pipe->loadRedisScript(luaDel); + "redis.call('DEL', KEYS[3])\n"; string luaBatchedSet = "local added = 0\n" @@ -59,11 +91,7 @@ ProducerStateTable::ProducerStateTable(RedisPipeline *pipeline, const string &ta " redis.call('HSET', KEYS[3] .. KEYS[4 + i], attr, val)\n" " end\n" " idx = idx + tonumber(ARGV[idx]) * 2 + 1\n" - "end\n" - "if added > 0 then \n" - " redis.call('PUBLISH', KEYS[1], ARGV[1])\n" "end\n"; - m_shaBatchedSet = m_pipe->loadRedisScript(luaBatchedSet); string luaBatchedDel = "local added = 0\n" @@ -71,36 +99,31 @@ ProducerStateTable::ProducerStateTable(RedisPipeline *pipeline, const string &ta " added = added + redis.call('SADD', KEYS[2], KEYS[5 + i])\n" " redis.call('SADD', KEYS[3], KEYS[5 + i])\n" " redis.call('DEL', KEYS[4] .. KEYS[5 + i])\n" - "end\n" - "if added > 0 then \n" - " redis.call('PUBLISH', KEYS[1], ARGV[1])\n" "end\n"; - m_shaBatchedDel = m_pipe->loadRedisScript(luaBatchedDel); - string luaClear = - "redis.call('DEL', KEYS[1])\n" - "local keys = redis.call('KEYS', KEYS[2] .. '*')\n" - "for i,k in pairs(keys) do\n" - " redis.call('DEL', k)\n" - "end\n" - "redis.call('DEL', KEYS[3])\n"; - m_shaClear = m_pipe->loadRedisScript(luaClear); - - string luaApplyView = loadLuaScript("producer_state_table_apply_view.lua"); - m_shaApplyView = m_pipe->loadRedisScript(luaApplyView); -} - -ProducerStateTable::~ProducerStateTable() -{ - if (m_pipeowned) + if (!m_flushPub || !m_buffered) { - delete m_pipe; + string luaPub = + "if added > 0 then \n" + " redis.call('PUBLISH', KEYS[1], ARGV[1])\n" + "end\n"; + luaSet += luaPub; + luaDel += luaPub; + luaBatchedSet += luaPub; + luaBatchedDel += luaPub; } + + /* 3. load redis script based on the lua string */ + m_shaSet = m_pipe->loadRedisScript(luaSet); + m_shaDel = m_pipe->loadRedisScript(luaDel); + m_shaBatchedSet = m_pipe->loadRedisScript(luaBatchedSet); + m_shaBatchedDel = m_pipe->loadRedisScript(luaBatchedDel); } void ProducerStateTable::setBuffered(bool buffered) { m_buffered = buffered; + reloadRedisScript(); } void ProducerStateTable::set(const string &key, const vector &values, diff --git a/common/producerstatetable.h b/common/producerstatetable.h index b6fa78684..b00453a5a 100644 --- a/common/producerstatetable.h +++ b/common/producerstatetable.h @@ -12,6 +12,7 @@ class ProducerStateTable : public TableBase, public TableName_KeySet public: ProducerStateTable(DBConnector *db, const std::string &tableName); ProducerStateTable(RedisPipeline *pipeline, const std::string &tableName, bool buffered = false); + ProducerStateTable(RedisPipeline *pipeline, const std::string &tableName, bool buffered, bool flushPub); virtual ~ProducerStateTable(); void setBuffered(bool buffered); @@ -51,6 +52,7 @@ class ProducerStateTable : public TableBase, public TableName_KeySet void apply_temp_view(); private: + bool m_flushPub; // publish per piepeline flush intead of per redis script bool m_buffered; bool m_pipeowned; bool m_tempViewActive; @@ -62,6 +64,8 @@ class ProducerStateTable : public TableBase, public TableName_KeySet std::string m_shaClear; std::string m_shaApplyView; TableDump m_tempViewState; + + void reloadRedisScript(); // redis script may change if m_buffered changes }; } diff --git a/common/redispipeline.h b/common/redispipeline.h index b8efa3840..be7561b6b 100644 --- a/common/redispipeline.h +++ b/common/redispipeline.h @@ -2,7 +2,10 @@ #include #include +#include #include +#include +#include #include "redisreply.h" #include "rediscommand.h" #include "dbconnector.h" @@ -22,9 +25,11 @@ class RedisPipeline { RedisPipeline(const DBConnector *db, size_t sz = 128) : COMMAND_MAX(sz) , m_remaining(0) + , m_shaPub("") { m_db = db->newConnector(NEWCONNECTOR_TIMEOUT); initializeOwnerTid(); + lastHeartBeat = std::chrono::steady_clock::now(); } ~RedisPipeline() { @@ -113,11 +118,19 @@ class RedisPipeline { void flush() { + lastHeartBeat = std::chrono::steady_clock::now(); + + if (m_remaining == 0) { + return; + } + while(m_remaining) { // Construct an object to use its dtor, so that resource is released RedisReply r(pop()); } + + publish(); } size_t size() @@ -145,12 +158,43 @@ class RedisPipeline { m_ownerTid = gettid(); } + void addChannel(std::string channel) + { + if (m_channels.find(channel) != m_channels.end()) + return; + + m_channels.insert(channel); + m_luaPub += "redis.call('PUBLISH', '" + channel + "', 'G');"; + m_shaPub = loadRedisScript(m_luaPub); + } + + int getIdleTime(std::chrono::time_point tcurrent=std::chrono::steady_clock::now()) + { + return static_cast(std::chrono::duration_cast(tcurrent - lastHeartBeat).count()); + } + + void publish() { + if (m_shaPub.empty()) { + return; + } + RedisCommand cmd; + cmd.format( + "EVALSHA %s 0", + m_shaPub.c_str()); + RedisReply r(m_db, cmd); + } + private: DBConnector *m_db; std::queue m_expectedTypes; size_t m_remaining; long int m_ownerTid; + std::string m_luaPub; + std::string m_shaPub; + std::chrono::time_point lastHeartBeat; // marks the timestamp of latest pipeline flush being invoked + std::unordered_set m_channels; + void mayflush() { if (m_remaining >= COMMAND_MAX) diff --git a/tests/redis_piped_state_ut.cpp b/tests/redis_piped_state_ut.cpp index ca3291907..f3173876e 100644 --- a/tests/redis_piped_state_ut.cpp +++ b/tests/redis_piped_state_ut.cpp @@ -730,3 +730,59 @@ TEST(ConsumerStateTable, async_multitable) cout << endl << "Done." << endl; } + +TEST(ConsumerStateTable, flushPub) +{ + clearDB(); + + /* Prepare producer */ + int index = 0; + string tableName = "UT_REDIS_THREAD_" + to_string(index); + DBConnector db(TEST_DB, 0, true); + RedisPipeline pipeline(&db); + ProducerStateTable p(&pipeline, tableName, false, true); + p.setBuffered(true); + + string key = "TheKey"; + int maxNumOfFields = 2; + + /* Set operation */ + { + vector fields; + for (int j = 0; j < maxNumOfFields; j++) + { + FieldValueTuple t(field(j), value(j)); + fields.push_back(t); + } + p.set(key, fields); + } + + /* Del operation */ + p.del(key); + p.flush(); + + /* Prepare consumer */ + ConsumerStateTable c(&db, tableName); + Select cs; + Selectable *selectcs; + cs.addSelectable(&c); + + /* First pop operation */ + { + int ret = cs.select(&selectcs); + EXPECT_EQ(ret, Select::OBJECT); + KeyOpFieldsValuesTuple kco; + c.pop(kco); + EXPECT_EQ(kfvKey(kco), key); + EXPECT_EQ(kfvOp(kco), "DEL"); + + auto fvs = kfvFieldsValues(kco); + EXPECT_EQ(fvs.size(), 0U); + } + + /* Second select operation */ + { + int ret = cs.select(&selectcs, 1000); + EXPECT_EQ(ret, Select::TIMEOUT); + } +} \ No newline at end of file From fe30ccdc567f7bce1a6291318573a70c351c3547 Mon Sep 17 00:00:00 2001 From: Sundara Gurunathan <105081231+sundar-pds@users.noreply.github.com> Date: Tue, 19 Nov 2024 09:43:43 -0800 Subject: [PATCH 17/19] [DASH] Add DASH Meter Policy , Rule , Counter table definitions (#949) * Adding DASH Meter Policy and Rule table definitions * Adding DASH Meter Counter ID list --- common/schema.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/schema.h b/common/schema.h index e616f1282..108fbc8dd 100644 --- a/common/schema.h +++ b/common/schema.h @@ -177,6 +177,8 @@ namespace swss { #define APP_DASH_ROUTE_GROUP_TABLE_NAME "DASH_ROUTE_GROUP_TABLE" #define APP_DASH_TUNNEL_TABLE_NAME "DASH_TUNNEL_TABLE" #define APP_DASH_PA_VALIDATION_TABLE_NAME "DASH_PA_VALIDATION_TABLE" +#define APP_DASH_METER_POLICY_TABLE_NAME "DASH_METER_POLICY_TABLE" +#define APP_DASH_METER_RULE_TABLE_NAME "DASH_METER_RULE_TABLE" #define APP_DASH_ROUTING_APPLIANCE_TABLE_NAME "DASH_ROUTING_APPLIANCE_TABLE" #define APP_PAC_PORT_TABLE_NAME "PAC_PORT_TABLE" @@ -261,6 +263,7 @@ namespace swss { #define QUEUE_ATTR_ID_LIST "QUEUE_ATTR_ID_LIST" #define BUFFER_POOL_COUNTER_ID_LIST "BUFFER_POOL_COUNTER_ID_LIST" #define ENI_COUNTER_ID_LIST "ENI_COUNTER_ID_LIST" +#define DASH_METER_COUNTER_ID_LIST "DASH_METER_COUNTER_ID_LIST" #define PFC_WD_STATE_TABLE "PFC_WD_STATE_TABLE" #define PFC_WD_PORT_COUNTER_ID_LIST "PORT_COUNTER_ID_LIST" #define PFC_WD_QUEUE_COUNTER_ID_LIST "QUEUE_COUNTER_ID_LIST" From ebd2afb0a2946420f6a42ba1f54f8b2c971781be Mon Sep 17 00:00:00 2001 From: Philo <135693886+philo-micas@users.noreply.github.com> Date: Thu, 21 Nov 2024 09:40:18 +0800 Subject: [PATCH 18/19] Supports FRR-VRRP configuration (#813) * Supports FRR-VRRP configuration Signed-off-by: philo * Update schema.h * Update schema.h * Update schema.h * triggle rebuild * triggle rebuild * triggle rebuild * triggle rebuild * triggle rebuild * Update schema.h * triggle rebuild * triggle rebuild --------- Signed-off-by: philo --- common/schema.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/schema.h b/common/schema.h index 108fbc8dd..e99f2ad81 100644 --- a/common/schema.h +++ b/common/schema.h @@ -429,7 +429,8 @@ namespace swss { #define CFG_MCLAG_UNIQUE_IP_TABLE_NAME "MCLAG_UNIQUE_IP" #define CFG_PORT_STORM_CONTROL_TABLE_NAME "PORT_STORM_CONTROL" - +#define CFG_VRRP_TABLE_NAME "VRRP" +#define CFG_VRRP6_TABLE_NAME "VRRP6" #define CFG_RATES_TABLE_NAME "RATES" #define CFG_FEATURE_TABLE_NAME "FEATURE" From 6bac82be1884f8e2c7e43aef2c8a9e6ee20c440f Mon Sep 17 00:00:00 2001 From: Hua Liu <58683130+liuh-80@users.noreply.github.com> Date: Mon, 25 Nov 2024 09:53:37 +0800 Subject: [PATCH 19/19] Improve memory usage by move ZMQ serialize buffer from ZmqProducerStateTable to ZmqClient (#955) #### Why I did it Every ZmqProducerStateTable will allocate 16MB buffer, this can be improve by share same buffer in ZmqClient. #### How I did it Improve memory usage by move ZMQ serialize buffer from ZmqProducerStateTable to ZmqClient. ##### Work item tracking #### How to verify it Pass all test cases. #### Which release branch to backport (provide reason below if selected) - [ ] 201811 - [ ] 201911 - [ ] 202006 - [ ] 202012 - [ ] 202106 - [ ] 202111 #### Description for the changelog Improve memory usage by move ZMQ serialize buffer from ZmqProducerStateTable to ZmqClient. #### Link to config_db schema for YANG module changes #### A picture of a cute animal (not mandatory but encouraged) --- common/c-api/zmqclient.cpp | 4 +--- common/zmqclient.cpp | 10 +++++----- common/zmqclient.h | 5 +++-- common/zmqproducerstatetable.cpp | 17 +++++------------ common/zmqproducerstatetable.h | 2 -- 5 files changed, 14 insertions(+), 24 deletions(-) diff --git a/common/c-api/zmqclient.cpp b/common/c-api/zmqclient.cpp index 49a9e05f7..fa1d59ca2 100644 --- a/common/c-api/zmqclient.cpp +++ b/common/c-api/zmqclient.cpp @@ -27,9 +27,7 @@ void SWSSZmqClient_sendMsg(SWSSZmqClient zmqc, const char *dbName, const char *t SWSSKeyOpFieldValuesArray arr) { SWSSTry({ vector kcos = takeKeyOpFieldValuesArray(arr); - size_t bufSize = BinarySerializer::serializedSize(dbName, tableName, kcos); - vector v(bufSize); ((ZmqClient *)zmqc) - ->sendMsg(string(dbName), string(tableName), kcos, v); + ->sendMsg(string(dbName), string(tableName), kcos); }); } diff --git a/common/zmqclient.cpp b/common/zmqclient.cpp index 0225d4374..5a84160e9 100644 --- a/common/zmqclient.cpp +++ b/common/zmqclient.cpp @@ -51,6 +51,7 @@ void ZmqClient::initialize(const std::string& endpoint, const std::string& vrf) m_context = nullptr; m_socket = nullptr; m_vrf = vrf; + m_sendbuffer.resize(MQ_RESPONSE_MAX_COUNT); connect(); } @@ -116,12 +117,11 @@ void ZmqClient::connect() void ZmqClient::sendMsg( const std::string& dbName, const std::string& tableName, - const std::vector& kcos, - std::vector& sendbuffer) + const std::vector& kcos) { int serializedlen = (int)BinarySerializer::serializeBuffer( - sendbuffer.data(), - sendbuffer.size(), + m_sendbuffer.data(), + m_sendbuffer.size(), dbName, tableName, kcos); @@ -144,7 +144,7 @@ void ZmqClient::sendMsg( std::lock_guard lock(m_socketMutex); // Use none block mode to use all bandwidth: http://api.zeromq.org/2-1%3Azmq-send - rc = zmq_send(m_socket, sendbuffer.data(), serializedlen, ZMQ_NOBLOCK); + rc = zmq_send(m_socket, m_sendbuffer.data(), serializedlen, ZMQ_NOBLOCK); } if (rc >= 0) diff --git a/common/zmqclient.h b/common/zmqclient.h index 313e65735..adc36b053 100644 --- a/common/zmqclient.h +++ b/common/zmqclient.h @@ -22,8 +22,7 @@ class ZmqClient void sendMsg(const std::string& dbName, const std::string& tableName, - const std::vector& kcos, - std::vector& sendbuffer); + const std::vector& kcos); private: void initialize(const std::string& endpoint, const std::string& vrf); @@ -38,6 +37,8 @@ class ZmqClient bool m_connected; std::mutex m_socketMutex; + + std::vector m_sendbuffer; }; } diff --git a/common/zmqproducerstatetable.cpp b/common/zmqproducerstatetable.cpp index ec9396b39..e2a31446b 100644 --- a/common/zmqproducerstatetable.cpp +++ b/common/zmqproducerstatetable.cpp @@ -38,8 +38,6 @@ ZmqProducerStateTable::ZmqProducerStateTable(RedisPipeline *pipeline, const stri void ZmqProducerStateTable::initialize(DBConnector *db, const std::string &tableName, bool dbPersistence) { - m_sendbuffer.resize(MQ_RESPONSE_MAX_COUNT); - if (dbPersistence) { SWSS_LOG_DEBUG("Database persistence enabled, tableName: %s", tableName.c_str()); @@ -64,8 +62,7 @@ void ZmqProducerStateTable::set( m_zmqClient.sendMsg( m_dbName, m_tableNameStr, - kcos, - m_sendbuffer); + kcos); if (m_asyncDBUpdater != nullptr) { @@ -93,8 +90,7 @@ void ZmqProducerStateTable::del( m_zmqClient.sendMsg( m_dbName, m_tableNameStr, - kcos, - m_sendbuffer); + kcos); if (m_asyncDBUpdater != nullptr) { @@ -112,8 +108,7 @@ void ZmqProducerStateTable::set(const std::vector &value m_zmqClient.sendMsg( m_dbName, m_tableNameStr, - values, - m_sendbuffer); + values); if (m_asyncDBUpdater != nullptr) { @@ -136,8 +131,7 @@ void ZmqProducerStateTable::del(const std::vector &keys) m_zmqClient.sendMsg( m_dbName, m_tableNameStr, - kcos, - m_sendbuffer); + kcos); if (m_asyncDBUpdater != nullptr) { @@ -157,8 +151,7 @@ void ZmqProducerStateTable::send(const std::vector &kcos m_zmqClient.sendMsg( m_dbName, m_tableNameStr, - kcos, - m_sendbuffer); + kcos); if (m_asyncDBUpdater != nullptr) { diff --git a/common/zmqproducerstatetable.h b/common/zmqproducerstatetable.h index 749107825..015419bd2 100644 --- a/common/zmqproducerstatetable.h +++ b/common/zmqproducerstatetable.h @@ -42,8 +42,6 @@ class ZmqProducerStateTable : public ProducerStateTable void initialize(DBConnector *db, const std::string &tableName, bool dbPersistence); ZmqClient& m_zmqClient; - - std::vector m_sendbuffer; const std::string m_dbName; const std::string m_tableNameStr;