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 01/29] 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 02/29] 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 03/29] 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 04/29] [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 05/29] 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 06/29] 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 07/29] 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 08/29] 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 09/29] 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 10/29] 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 11/29] 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 12/29] 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 13/29] 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 14/29] 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 15/29] [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 16/29] [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 17/29] 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 18/29] 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; From aa1021fb14bd79c01e34ea99733b2213da70029a Mon Sep 17 00:00:00 2001 From: Yijiao Qin Date: Sun, 1 Dec 2024 18:35:15 -0800 Subject: [PATCH 19/29] Update redispipeline.h (#954) use "\n" instead of semicolon to separate redis commands in a c++ string --- common/redispipeline.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/redispipeline.h b/common/redispipeline.h index be7561b6b..96f97ab8b 100644 --- a/common/redispipeline.h +++ b/common/redispipeline.h @@ -164,7 +164,7 @@ class RedisPipeline { return; m_channels.insert(channel); - m_luaPub += "redis.call('PUBLISH', '" + channel + "', 'G');"; + m_luaPub += "redis.call('PUBLISH', '" + channel + "', 'G')\n"; m_shaPub = loadRedisScript(m_luaPub); } From fb6ce44e41f77dc8631f0e8bf43cdf58c3635d3a Mon Sep 17 00:00:00 2001 From: Arham-Nasir <100487254+Arham-Nasir@users.noreply.github.com> Date: Wed, 4 Dec 2024 00:15:35 +0500 Subject: [PATCH 20/29] Add definition for MEMORY_STATISTICS table in schema (#898) define table in common/schema.h file of sonic-swss-common --- common/schema.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/schema.h b/common/schema.h index e99f2ad81..461ea4d82 100644 --- a/common/schema.h +++ b/common/schema.h @@ -475,11 +475,12 @@ namespace swss { #define CFG_SUPPRESS_ASIC_SDK_HEALTH_EVENT_NAME "SUPPRESS_ASIC_SDK_HEALTH_EVENT" +#define CFG_MEMORY_STATISTICS_TABLE_NAME "MEMORY_STATISTICS" + #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" From 7425c4237b06c9c7b553a9a3bcb540c2f7720fcf Mon Sep 17 00:00:00 2001 From: Saikrishna Arcot Date: Fri, 6 Dec 2024 17:03:31 -0800 Subject: [PATCH 21/29] Update the Azure Pipeline build to use Bookworm and Ubuntu 22.04 (#937) Update the Azure Pipeline build to use Bookworm and Ubuntu 22.04 Signed-off-by: Saikrishna Arcot --- .azure-pipelines/build-sairedis-template.yml | 6 +- .azure-pipelines/build-template.yml | 14 +- .azure-pipelines/docker-sonic-vs/Dockerfile | 24 ++- .azure-pipelines/docker-sonic-vs/start.sh | 187 ++++++++++++++++++ .../test-docker-sonic-vs-template.yml | 38 +--- azure-pipelines.yml | 91 +++++---- 6 files changed, 273 insertions(+), 87 deletions(-) create mode 100755 .azure-pipelines/docker-sonic-vs/start.sh diff --git a/.azure-pipelines/build-sairedis-template.yml b/.azure-pipelines/build-sairedis-template.yml index b37d2fa44..65e7c536d 100644 --- a/.azure-pipelines/build-sairedis-template.yml +++ b/.azure-pipelines/build-sairedis-template.yml @@ -66,7 +66,6 @@ jobs: set -ex sudo apt-get update sudo apt-get install -qq -y \ - qtbase5-dev \ libdbus-glib-1-dev \ libpcsclite-dev \ docbook-to-man \ @@ -90,7 +89,7 @@ jobs: sudo mkdir -m 755 /var/run/sswsyncd sudo apt-get install -y rsyslog - sudo service rsyslog start + sudo rsyslogd displayName: "Install dependencies" - task: DownloadPipelineArtifact@2 @@ -137,7 +136,8 @@ jobs: displayName: "Compile sonic sairedis" - script: | sudo cp azsyslog.conf /etc/rsyslog.conf - sudo service rsyslog restart + sudo killall rsyslogd + sudo rsyslogd displayName: "Update rsyslog.conf" - ${{ if eq(parameters.run_unit_test, true) }}: - script: | diff --git a/.azure-pipelines/build-template.yml b/.azure-pipelines/build-template.yml index d1813c23a..a4b607efe 100644 --- a/.azure-pipelines/build-template.yml +++ b/.azure-pipelines/build-template.yml @@ -98,16 +98,6 @@ jobs: mv ../*.deb . displayName: "Compile sonic swss common with coverage enabled" - ${{ if eq(parameters.run_unit_test, true) }}: - - script: | - set -ex - git clone https://github.com/gcovr/gcovr.git - cd gcovr/ - git checkout 5.2 - sudo pip3 install setuptools - sudo python3 setup.py install - cd .. - sudo rm -rf gcovr - displayName: "Install gcovr 5.2 (for --exclude-throw-branches support)" - script: | set -ex sudo pip install Pympler==0.8 pytest @@ -142,9 +132,9 @@ jobs: set -ex # Install .NET CORE curl -sSL https://packages.microsoft.com/keys/microsoft.asc | sudo apt-key add - - sudo apt-add-repository https://packages.microsoft.com/debian/11/prod + sudo apt-add-repository https://packages.microsoft.com/debian/12/prod sudo apt-get update - sudo apt-get install -y dotnet-sdk-6.0 + sudo apt-get install -y dotnet-sdk-8.0 displayName: "Install .NET CORE" - task: PublishCodeCoverageResults@1 inputs: diff --git a/.azure-pipelines/docker-sonic-vs/Dockerfile b/.azure-pipelines/docker-sonic-vs/Dockerfile index 3d2198050..f598ea6c8 100644 --- a/.azure-pipelines/docker-sonic-vs/Dockerfile +++ b/.azure-pipelines/docker-sonic-vs/Dockerfile @@ -1,11 +1,31 @@ FROM docker-sonic-vs ARG docker_container_name +ARG need_dbg COPY ["debs", "/debs"] # Remove existing packages first before installing the new/current packages. This is to overcome limitations with # Docker's diff detection mechanism, where only the file size and the modification timestamp (which will remain the # same, even though contents have changed) are checked between the previous and current layer. -RUN dpkg --purge libswsscommon python3-swsscommon sonic-db-cli libsaimetadata libsairedis libsaivs syncd-vs swss sonic-eventd libdashapi -RUN dpkg -i /debs/libdashapi_1.0.0_amd64.deb /debs/libswsscommon_1.0.0_amd64.deb /debs/python3-swsscommon_1.0.0_amd64.deb /debs/sonic-db-cli_1.0.0_amd64.deb /debs/libsaimetadata_1.0.0_amd64.deb /debs/libsairedis_1.0.0_amd64.deb /debs/libsaivs_1.0.0_amd64.deb /debs/syncd-vs_1.0.0_amd64.deb /debs/swss_1.0.0_amd64.deb +RUN dpkg --purge libswsscommon python3-swsscommon sonic-db-cli libsaimetadata libsairedis libsaivs syncd-vs swss sonic-eventd libdashapi + +RUN apt-get update + +RUN apt install -y /debs/libdashapi_1.0.0_amd64.deb \ + /debs/libswsscommon_1.0.0_amd64.deb \ + /debs/python3-swsscommon_1.0.0_amd64.deb \ + /debs/sonic-db-cli_1.0.0_amd64.deb \ + /debs/libsaimetadata_1.0.0_amd64.deb \ + /debs/libsairedis_1.0.0_amd64.deb \ + /debs/libsaivs_1.0.0_amd64.deb \ + /debs/syncd-vs_1.0.0_amd64.deb \ + /debs/swss_1.0.0_amd64.deb + +RUN if [ "$need_dbg" = "y" ] ; then dpkg -i /debs/libswsscommon-dbgsym_1.0.0_amd64.deb ; fi + +COPY ["start.sh", "/usr/bin/"] + +RUN pip3 install scapy==2.5.0 + +RUN apt-get -y install software-properties-common libdatetime-perl libcapture-tiny-perl build-essential libcpanel-json-xs-perl git python3-protobuf diff --git a/.azure-pipelines/docker-sonic-vs/start.sh b/.azure-pipelines/docker-sonic-vs/start.sh new file mode 100755 index 000000000..f7dbde8dc --- /dev/null +++ b/.azure-pipelines/docker-sonic-vs/start.sh @@ -0,0 +1,187 @@ +#!/bin/bash -e + +# Generate configuration + +# NOTE: 'PLATFORM' and 'HWSKU' environment variables are set +# in the Dockerfile so that they persist for the life of the container + +ln -sf /usr/share/sonic/device/$PLATFORM /usr/share/sonic/platform +ln -sf /usr/share/sonic/device/$PLATFORM/$HWSKU /usr/share/sonic/hwsku + +SWITCH_TYPE=switch +PLATFORM_CONF=platform.json +if [[ $HWSKU == "DPU-2P" ]]; then + SWITCH_TYPE=dpu + PLATFORM_CONF=platform-dpu-2p.json +fi + +pushd /usr/share/sonic/hwsku + +# filter available front panel ports in lanemap.ini +[ -f lanemap.ini.orig ] || cp lanemap.ini lanemap.ini.orig +for p in $(ip link show | grep -oE "eth[0-9]+" | grep -v eth0); do + grep ^$p: lanemap.ini.orig +done > lanemap.ini + +# filter available sonic front panel ports in port_config.ini +[ -f port_config.ini.orig ] || cp port_config.ini port_config.ini.orig +grep ^# port_config.ini.orig > port_config.ini +for lanes in $(awk -F ':' '{print $2}' lanemap.ini); do + grep -E "\s$lanes\s" port_config.ini.orig +done >> port_config.ini + +popd + +[ -d /etc/sonic ] || mkdir -p /etc/sonic + +# Note: libswsscommon requires a dabase_config file in /var/run/redis/sonic-db/ +# Prepare this file before any dependent application, such as sonic-cfggen +mkdir -p /var/run/redis/sonic-db +cp /etc/default/sonic-db/database_config.json /var/run/redis/sonic-db/ + +SYSTEM_MAC_ADDRESS=$(ip link show eth0 | grep ether | awk '{print $2}') +sonic-cfggen -t /usr/share/sonic/templates/init_cfg.json.j2 -a "{\"system_mac\": \"$SYSTEM_MAC_ADDRESS\", \"switch_type\": \"$SWITCH_TYPE\"}" > /etc/sonic/init_cfg.json + +if [[ -f /usr/share/sonic/virtual_chassis/default_config.json ]]; then + sonic-cfggen -j /etc/sonic/init_cfg.json -j /usr/share/sonic/virtual_chassis/default_config.json --print-data > /tmp/init_cfg.json + mv /tmp/init_cfg.json /etc/sonic/init_cfg.json +fi + +if [ -f /etc/sonic/config_db.json ]; then + sonic-cfggen -j /etc/sonic/init_cfg.json -j /etc/sonic/config_db.json --print-data > /tmp/config_db.json + mv /tmp/config_db.json /etc/sonic/config_db.json +else + # generate and merge buffers configuration into config file + if [ -f /usr/share/sonic/hwsku/buffers.json.j2 ]; then + sonic-cfggen -k $HWSKU -p /usr/share/sonic/device/$PLATFORM/$PLATFORM_CONF -t /usr/share/sonic/hwsku/buffers.json.j2 > /tmp/buffers.json + buffers_cmd="-j /tmp/buffers.json" + fi + if [ -f /usr/share/sonic/hwsku/qos.json.j2 ]; then + sonic-cfggen -j /etc/sonic/init_cfg.json -t /usr/share/sonic/hwsku/qos.json.j2 > /tmp/qos.json + qos_cmd="-j /tmp/qos.json" + fi + + sonic-cfggen -p /usr/share/sonic/device/$PLATFORM/$PLATFORM_CONF -k $HWSKU --print-data > /tmp/ports.json + # change admin_status from up to down; Test cases dependent + sed -i "s/up/down/g" /tmp/ports.json + sonic-cfggen -j /etc/sonic/init_cfg.json $buffers_cmd $qos_cmd -j /tmp/ports.json --print-data > /etc/sonic/config_db.json +fi + +sonic-cfggen -t /usr/share/sonic/templates/copp_cfg.j2 > /etc/sonic/copp_cfg.json + +if [ "$HWSKU" == "Mellanox-SN2700" ]; then + cp /usr/share/sonic/hwsku/sai_mlnx.profile /usr/share/sonic/hwsku/sai.profile +elif [ "$HWSKU" == "DPU-2P" ]; then + cp /usr/share/sonic/hwsku/sai_dpu_2p.profile /usr/share/sonic/hwsku/sai.profile +fi + +mkdir -p /etc/swss/config.d/ + +rm -f /var/run/rsyslogd.pid + +supervisorctl start rsyslogd + +supervisord_cfg="/etc/supervisor/conf.d/supervisord.conf" +chassisdb_cfg_file="/usr/share/sonic/virtual_chassis/default_config.json" +chassisdb_cfg_file_default="/etc/default/sonic-db/default_chassis_cfg.json" +host_template="/usr/share/sonic/templates/hostname.j2" +db_cfg_file="/var/run/redis/sonic-db/database_config.json" +db_cfg_file_tmp="/var/run/redis/sonic-db/database_config.json.tmp" + +if [ -r "$chassisdb_cfg_file" ]; then + echo $(sonic-cfggen -j $chassisdb_cfg_file -t $host_template) >> /etc/hosts +else + chassisdb_cfg_file="$chassisdb_cfg_file_default" + echo "10.8.1.200 redis_chassis.server" >> /etc/hosts +fi + +supervisorctl start redis-server + +start_chassis_db=`sonic-cfggen -v DEVICE_METADATA.localhost.start_chassis_db -y $chassisdb_cfg_file` +if [[ "$HOSTNAME" == *"supervisor"* ]] || [ "$start_chassis_db" == "1" ]; then + supervisorctl start redis-chassis +fi + +conn_chassis_db=`sonic-cfggen -v DEVICE_METADATA.localhost.connect_to_chassis_db -y $chassisdb_cfg_file` +if [ "$start_chassis_db" != "1" ] && [ "$conn_chassis_db" != "1" ]; then + cp $db_cfg_file $db_cfg_file_tmp + update_chassisdb_config -j $db_cfg_file_tmp -d + cp $db_cfg_file_tmp $db_cfg_file +fi + +if [ "$conn_chassis_db" == "1" ]; then + if [ -f /usr/share/sonic/virtual_chassis/coreportindexmap.ini ]; then + cp /usr/share/sonic/virtual_chassis/coreportindexmap.ini /usr/share/sonic/hwsku/ + + pushd /usr/share/sonic/hwsku + + # filter available front panel ports in coreportindexmap.ini + [ -f coreportindexmap.ini.orig ] || cp coreportindexmap.ini coreportindexmap.ini.orig + for p in $(ip link show | grep -oE "eth[0-9]+" | grep -v eth0); do + grep ^$p: coreportindexmap.ini.orig + done > coreportindexmap.ini + + popd + fi +fi + +/usr/bin/configdb-load.sh + +if [ "$HWSKU" = "brcm_gearbox_vs" ]; then + supervisorctl start gbsyncd + supervisorctl start gearsyncd +fi + +supervisorctl start syncd + +supervisorctl start portsyncd + +supervisorctl start orchagent + +supervisorctl start coppmgrd + +supervisorctl start neighsyncd + +supervisorctl start fdbsyncd + +supervisorctl start teamsyncd + +supervisorctl start fpmsyncd + +supervisorctl start teammgrd + +supervisorctl start vrfmgrd + +supervisorctl start portmgrd + +supervisorctl start intfmgrd + +supervisorctl start vlanmgrd + +supervisorctl start zebra + +supervisorctl start mgmtd + +supervisorctl start staticd + +supervisorctl start buffermgrd + +supervisorctl start nbrmgrd + +supervisorctl start vxlanmgrd + +supervisorctl start sflowmgrd + +supervisorctl start natmgrd + +supervisorctl start natsyncd + +supervisorctl start tunnelmgrd + +supervisorctl start fabricmgrd + +# Start arp_update when VLAN exists +VLAN=`sonic-cfggen -d -v 'VLAN.keys() | join(" ") if VLAN'` +if [ "$VLAN" != "" ]; then + supervisorctl start arp_update +fi diff --git a/.azure-pipelines/test-docker-sonic-vs-template.yml b/.azure-pipelines/test-docker-sonic-vs-template.yml index 81af9bd82..0a2a7018c 100644 --- a/.azure-pipelines/test-docker-sonic-vs-template.yml +++ b/.azure-pipelines/test-docker-sonic-vs-template.yml @@ -15,7 +15,7 @@ jobs: displayName: vstest timeoutInMinutes: ${{ parameters.timeout }} - pool: sonic-common + pool: sonictest steps: - checkout: self @@ -52,11 +52,10 @@ jobs: - script: | set -ex - ls -l sudo sonic-swss-common/.azure-pipelines/build_and_install_module.sh sudo apt-get install -y libhiredis0.14 libyang0.16 - sudo dpkg -i $(Build.ArtifactStagingDirectory)/download/libprotobuf*_amd64.deb $(Build.ArtifactStagingDirectory)/download/libprotobuf-lite*_amd64.deb $(Build.ArtifactStagingDirectory)/download/python3-protobuf*_amd64.deb + sudo dpkg -i $(Build.ArtifactStagingDirectory)/download/libprotobuf*_amd64.deb $(Build.ArtifactStagingDirectory)/download/libprotobuf-lite*_amd64.deb $(Build.ArtifactStagingDirectory)/download/python3-protobuf*_amd64.deb sudo dpkg -i $(Build.ArtifactStagingDirectory)/download/libdashapi*.deb sudo dpkg -i --force-confask,confnew $(Build.ArtifactStagingDirectory)/download/libswsscommon_1.0.0_amd64.deb || apt-get install -f sudo dpkg -i $(Build.ArtifactStagingDirectory)/download/python3-swsscommon_1.0.0_amd64.deb @@ -72,34 +71,17 @@ jobs: sudo docker load -i $(Build.ArtifactStagingDirectory)/download/docker-sonic-vs.gz docker ps ip netns list + sudo /sbin/ip link add Vrf1 type vrf table 1001 || { echo 'vrf command failed' ; exit 1; } + sudo /sbin/ip link del Vrf1 type vrf table 1001 pushd sonic-swss/tests - # run pytests in sets of 20 - all_tests=$(ls test_*.py) + all_tests=$(ls test_*.py | xargs) all_tests="${all_tests} p4rt dash" - test_set=() - for test in ${all_tests}; do - test_set+=("${test}") - if [ ${#test_set[@]} -ge 20 ]; then - test_name=$(echo "${test_set[0]}" | cut -d "." -f 1) - echo "${test_set[*]}" | xargs sudo py.test -v --force-flaky --junitxml="${test_name}_tr.xml" --keeptb --imgname=docker-sonic-vs:$(Build.DefinitionName).$(Build.BuildNumber) - container_count=$(docker ps -q -a | wc -l) - if [ ${container_count} -gt 0 ]; then - docker stop $(docker ps -q -a) - docker rm $(docker ps -q -a) - fi - test_set=() - fi - done - if [ ${#test_set[@]} -gt 0 ]; then - test_name=$(echo "${test_set[0]}" | cut -d "." -f 1) - echo "${test_set[*]}" | xargs sudo py.test -v --force-flaky --junitxml="${test_name}_tr.xml" --keeptb --imgname=docker-sonic-vs:$(Build.DefinitionName).$(Build.BuildNumber) - container_count=$(docker ps -q -a | wc -l) - if [ ${container_count} -gt 0 ]; then - docker stop $(docker ps -q -a) - docker rm $(docker ps -q -a) - fi - fi + + # Run the tests in parallel and retry + retry=3 + IMAGE_NAME=docker-sonic-vs:$(Build.DefinitionName).$(Build.BuildNumber) + echo $all_tests | xargs -n 1 | xargs -P 8 -I TEST_MODULE sudo ./run-tests.sh "$IMAGE_NAME" "--force-recreate-dvs" "TEST_MODULE" 3 rm -rf $(Build.ArtifactStagingDirectory)/download displayName: "Run vs tests" diff --git a/azure-pipelines.yml b/azure-pipelines.yml index f5f233262..eb9743886 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -40,7 +40,7 @@ resources: parameters: - name: debian_version type: string - default: bullseye + default: bookworm variables: - name: BUILD_BRANCH ${{ if eq(variables['Build.Reason'], 'PullRequest') }}: @@ -62,7 +62,7 @@ stages: sudo apt-get update sudo apt-get install -y make libtool m4 autoconf dh-exec debhelper cmake pkg-config nlohmann-json3-dev \ libhiredis-dev libnl-3-dev libnl-genl-3-dev libnl-route-3-dev libnl-nf-3-dev swig3.0 \ - libpython2.7-dev libboost-dev libboost-serialization-dev uuid-dev libzmq5 libzmq3-dev + libpython2.7-dev libboost-dev libboost-serialization-dev uuid-dev libzmq3-dev sudo apt-get install -y sudo sudo apt-get install -y redis-server redis-tools sudo apt-get install -y python3-pip @@ -88,11 +88,47 @@ stages: artifact: sonic-swss-common.amd64.ubuntu20_04 displayName: "Archive swss common debian packages" + - job: + displayName: "amd64/ubuntu-22.04" + pool: + vmImage: 'ubuntu-22.04' + + steps: + - script: | + sudo apt-get update + sudo apt-get install -y make libtool m4 autoconf dh-exec debhelper cmake pkg-config nlohmann-json3-dev \ + libhiredis-dev libnl-3-dev libnl-genl-3-dev libnl-route-3-dev libnl-nf-3-dev swig4.0 \ + libpython3-dev libboost-dev libboost-serialization-dev uuid-dev libzmq3-dev + sudo apt-get install -y sudo + sudo apt-get install -y redis-server redis-tools + sudo apt-get install -y python3-pip + sudo pip3 install pytest + sudo apt-get install -y python + sudo apt-get install cmake libgtest-dev libgmock-dev libyang-dev + cd /usr/src/gtest && sudo cmake . && sudo make + ARCH=$(dpkg --print-architecture) + set -x + sudo curl -fsSL -o /usr/local/bin/bazel \ + https://github.com/bazelbuild/bazelisk/releases/latest/download/bazelisk-linux-${ARCH} + sudo chmod 755 /usr/local/bin/bazel + displayName: "Install dependencies" + - script: | + ./autogen.sh + dpkg-buildpackage -rfakeroot -us -uc -Pnopython2 -b -j$(nproc) && cp ../*.deb . + displayName: "Compile sonic swss common" + - script: | + bazel build //... + bazel test //... + displayName: "Compile and test all Bazel targets" + - publish: $(System.DefaultWorkingDirectory)/ + artifact: sonic-swss-common.amd64.ubuntu22_04 + displayName: "Archive swss common debian packages" + - template: .azure-pipelines/build-template.yml parameters: arch: amd64 sonic_slave: sonic-slave-${{ parameters.debian_version }}:$(BUILD_BRANCH) - artifact_name: sonic-swss-common + artifact_name: sonic-swss-common-${{ parameters.debian_version }} run_unit_test: true archive_gcov: true debian_version: ${{ parameters.debian_version }} @@ -107,7 +143,7 @@ stages: timeout: 180 pool: sonicbld-armhf sonic_slave: sonic-slave-${{ parameters.debian_version }}-armhf:$(BUILD_BRANCH) - artifact_name: sonic-swss-common.armhf + artifact_name: sonic-swss-common-${{ parameters.debian_version }}.armhf debian_version: ${{ parameters.debian_version }} - template: .azure-pipelines/build-template.yml @@ -116,36 +152,7 @@ stages: timeout: 180 pool: sonicbld-arm64 sonic_slave: sonic-slave-${{ parameters.debian_version }}-arm64:$(BUILD_BRANCH) - artifact_name: sonic-swss-common.arm64 - debian_version: ${{ parameters.debian_version }} - -- stage: BuildBookworm - dependsOn: BuildArm - condition: succeeded('BuildArm') - jobs: - - template: .azure-pipelines/build-template.yml - parameters: - arch: amd64 - sonic_slave: sonic-slave-bookworm:$(BUILD_BRANCH) - artifact_name: sonic-swss-common-bookworm - debian_version: ${{ parameters.debian_version }} - - - template: .azure-pipelines/build-template.yml - parameters: - arch: armhf - timeout: 180 - pool: sonicbld-armhf - sonic_slave: sonic-slave-bookworm-armhf:$(BUILD_BRANCH) - artifact_name: sonic-swss-common-bookworm.armhf - debian_version: ${{ parameters.debian_version }} - - - template: .azure-pipelines/build-template.yml - parameters: - arch: arm64 - timeout: 180 - pool: sonicbld-arm64 - sonic_slave: sonic-slave-bookworm-arm64:$(BUILD_BRANCH) - artifact_name: sonic-swss-common-bookworm.arm64 + artifact_name: sonic-swss-common-${{ parameters.debian_version }}.arm64 debian_version: ${{ parameters.debian_version }} - stage: BuildSairedis @@ -156,8 +163,8 @@ stages: parameters: arch: amd64 sonic_slave: sonic-slave-${{ parameters.debian_version }}:$(BUILD_BRANCH) - swss_common_artifact_name: sonic-swss-common - artifact_name: sonic-sairedis + swss_common_artifact_name: sonic-swss-common-${{ parameters.debian_version }} + artifact_name: sonic-sairedis-${{ parameters.debian_version }} syslog_artifact_name: sonic-sairedis.syslog debian_version: ${{ parameters.debian_version }} @@ -169,9 +176,9 @@ stages: parameters: arch: amd64 sonic_slave: sonic-slave-${{ parameters.debian_version }}:$(BUILD_BRANCH) - swss_common_artifact_name: sonic-swss-common - sairedis_artifact_name: sonic-sairedis - artifact_name: sonic-swss + swss_common_artifact_name: sonic-swss-common-${{ parameters.debian_version }} + sairedis_artifact_name: sonic-sairedis-${{ parameters.debian_version }} + artifact_name: sonic-swss-${{ parameters.debian_version }} debian_version: ${{ parameters.debian_version }} - stage: BuildDocker @@ -180,9 +187,9 @@ stages: jobs: - template: .azure-pipelines/build-docker-sonic-vs-template.yml parameters: - swss_common_artifact_name: sonic-swss-common - sairedis_artifact_name: sonic-sairedis - swss_artifact_name: sonic-swss + swss_common_artifact_name: sonic-swss-common-${{ parameters.debian_version }} + sairedis_artifact_name: sonic-sairedis-${{ parameters.debian_version }} + swss_artifact_name: sonic-swss-${{ parameters.debian_version }} artifact_name: docker-sonic-vs - stage: Test From ceccddc808f4dd2fa17da560a4c83623a99a7cf3 Mon Sep 17 00:00:00 2001 From: Shira <34212259+shiraez@users.noreply.github.com> Date: Sun, 22 Dec 2024 12:11:42 +0200 Subject: [PATCH 22/29] policer ID_LIST & MAP (#959) Added field for policer counter COUNTERS_POLICER_NAME_MAP POLICER_COUNTER_ID_LIST Signed-off-by: shiraez --- common/schema.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/schema.h b/common/schema.h index 461ea4d82..b1d0ca33f 100644 --- a/common/schema.h +++ b/common/schema.h @@ -212,6 +212,7 @@ namespace swss { #define COUNTERS_RIF_TYPE_MAP "COUNTERS_RIF_TYPE_MAP" #define COUNTERS_RIF_NAME_MAP "COUNTERS_RIF_NAME_MAP" #define COUNTERS_TRAP_NAME_MAP "COUNTERS_TRAP_NAME_MAP" +#define COUNTERS_POLICER_NAME_MAP "COUNTERS_POLICER_NAME_MAP" #define COUNTERS_CRM_TABLE "CRM" #define COUNTERS_BUFFER_POOL_NAME_MAP "COUNTERS_BUFFER_POOL_NAME_MAP" #define COUNTERS_SWITCH_NAME_MAP "COUNTERS_SWITCH_NAME_MAP" @@ -279,6 +280,7 @@ namespace swss { #define TUNNEL_ATTR_ID_LIST "TUNNEL_ATTR_ID_LIST" #define ACL_COUNTER_ATTR_ID_LIST "ACL_COUNTER_ATTR_ID_LIST" #define FLOW_COUNTER_ID_LIST "FLOW_COUNTER_ID_LIST" +#define POLICER_COUNTER_ID_LIST "POLICER_COUNTER_ID_LIST" #define PLUGIN_TABLE "PLUGIN_TABLE" #define LUA_PLUGIN_TYPE "LUA_PLUGIN_TYPE" #define SAI_OBJECT_TYPE "SAI_OBJECT_TYPE" From c872f42b5237444095808ce70d92fad913c682a4 Mon Sep 17 00:00:00 2001 From: Stephen Sun <5379172+stephenxs@users.noreply.github.com> Date: Tue, 24 Dec 2024 21:18:04 +0800 Subject: [PATCH 23/29] Add field for bulk chunk size in flex counter (#950) Added a new field in FLEX_COUNTER_TABLE to represent the bulk chunk size and bulk chunk size per counter ID for bulk counter polling. Signed-off-by: Stephen Sun --- common/schema.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/common/schema.h b/common/schema.h index b1d0ca33f..2713a6bc1 100644 --- a/common/schema.h +++ b/common/schema.h @@ -285,6 +285,8 @@ namespace swss { #define LUA_PLUGIN_TYPE "LUA_PLUGIN_TYPE" #define SAI_OBJECT_TYPE "SAI_OBJECT_TYPE" +#define BULK_CHUNK_SIZE_FIELD "BULK_CHUNK_SIZE" +#define BULK_CHUNK_SIZE_PER_PREFIX_FIELD "BULK_CHUNK_SIZE_PER_PREFIX" #define POLL_INTERVAL_FIELD "POLL_INTERVAL" #define STATS_MODE_FIELD "STATS_MODE" #define STATS_MODE_READ "STATS_MODE_READ" From 12c428ee25066fd62bc0f3185692c4d6bf846333 Mon Sep 17 00:00:00 2001 From: Yakiv Huryk <62013282+Yakiv-Huryk@users.noreply.github.com> Date: Wed, 8 Jan 2025 21:52:43 +0200 Subject: [PATCH 24/29] [schema] add SRv6 config db tables (#962) Add the following table names to the schema.h: CFG_SRV6_MY_SID_TABLE_NAME CFG_SRV6_MY_LOCATOR_TABLE_NAME Signed-off-by: Yakiv Huryk --- common/schema.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common/schema.h b/common/schema.h index 2713a6bc1..ade98f9f5 100644 --- a/common/schema.h +++ b/common/schema.h @@ -485,6 +485,9 @@ namespace swss { #define CFG_PAC_GLOBAL_CONFIG_TABLE "PAC_GLOBAL_CONFIG_TABLE" #define CFG_PAC_HOSTAPD_GLOBAL_CONFIG_TABLE "HOSTAPD_GLOBAL_CONFIG_TABLE" +#define CFG_SRV6_MY_SID_TABLE_NAME "SRV6_MY_SIDS" +#define CFG_SRV6_MY_LOCATOR_TABLE_NAME "SRV6_MY_LOCATORS" + /***** STATE DATABASE *****/ #define STATE_SWITCH_CAPABILITY_TABLE_NAME "SWITCH_CAPABILITY" From b58a5012e8b41aa527d94bdad5958b2e3e7a0885 Mon Sep 17 00:00:00 2001 From: erer1243 <1377477+erer1243@users.noreply.github.com> Date: Wed, 15 Jan 2025 10:26:08 -0500 Subject: [PATCH 25/29] Add swss::Table to c api (#964) --- common/Makefile.am | 1 + common/c-api/table.cpp | 71 ++++++++++++++++++++++++++++++++++++ common/c-api/table.h | 41 +++++++++++++++++++++ common/c-api/util.cpp | 4 +++ common/c-api/util.h | 25 +++++++++++++ tests/c_api_ut.cpp | 81 +++++++++++++++++++++++++++++++++++------- 6 files changed, 211 insertions(+), 12 deletions(-) create mode 100644 common/c-api/table.cpp create mode 100644 common/c-api/table.h diff --git a/common/Makefile.am b/common/Makefile.am index 724805e60..196fd6a5c 100644 --- a/common/Makefile.am +++ b/common/Makefile.am @@ -79,6 +79,7 @@ common_libswsscommon_la_SOURCES = \ common/c-api/zmqserver.cpp \ common/c-api/zmqconsumerstatetable.cpp \ common/c-api/zmqproducerstatetable.cpp \ + common/c-api/table.cpp \ common/performancetimer.cpp common_libswsscommon_la_CXXFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(LIBNL_CFLAGS) $(CODE_COVERAGE_CXXFLAGS) diff --git a/common/c-api/table.cpp b/common/c-api/table.cpp new file mode 100644 index 000000000..af08cbaa8 --- /dev/null +++ b/common/c-api/table.cpp @@ -0,0 +1,71 @@ +#include +#include + +#include "../dbconnector.h" +#include "../table.h" +#include "table.h" +#include "util.h" + +using namespace swss; +using namespace std; + +SWSSTable SWSSTable_new(SWSSDBConnector db, const char *tableName) { + SWSSTry(return (SWSSTable) new Table((DBConnector *)db, string(tableName))); +} + +void SWSSTable_free(SWSSTable tbl) { + SWSSTry(delete (Table *)tbl); +} + +int8_t SWSSTable_get(SWSSTable tbl, const char *key, SWSSFieldValueArray *outValues) { + SWSSTry({ + vector fvs; + bool exists = ((Table *)tbl)->get(string(key), fvs); + if (exists) { + *outValues = makeFieldValueArray(fvs); + return 1; + } else { + return 0; + } + }); +} + +int8_t SWSSTable_hget(SWSSTable tbl, const char *key, const char *field, SWSSString *outValue) { + SWSSTry({ + string s; + bool exists = ((Table *)tbl)->hget(string(key), string(field), s); + if (exists) { + *outValue = makeString(move(s)); + return 1; + } else { + return 0; + } + }); +} + +void SWSSTable_set(SWSSTable tbl, const char *key, SWSSFieldValueArray values) { + SWSSTry({ + vector fvs = takeFieldValueArray(values); + ((Table *)tbl)->set(string(key), fvs); + }); +} + +void SWSSTable_hset(SWSSTable tbl, const char *key, const char *field, SWSSStrRef value) { + SWSSTry({ ((Table *)tbl)->hset(string(key), string(field), takeStrRef(value)); }); +} + +void SWSSTable_del(SWSSTable tbl, const char *key) { + SWSSTry({ ((Table *)tbl)->del(string(key)); }); +} + +void SWSSTable_hdel(SWSSTable tbl, const char *key, const char *field) { + SWSSTry({ ((Table *)tbl)->hdel(string(key), string(field)); }); +} + +SWSSStringArray SWSSTable_getKeys(SWSSTable tbl) { + SWSSTry({ + vector keys; + ((Table *)tbl)->getKeys(keys); + return makeStringArray(move(keys)); + }) +} diff --git a/common/c-api/table.h b/common/c-api/table.h new file mode 100644 index 000000000..0d06e1e7d --- /dev/null +++ b/common/c-api/table.h @@ -0,0 +1,41 @@ +#ifndef SWSS_COMMON_C_API_TABLE_H +#define SWSS_COMMON_C_API_TABLE_H + +#include "dbconnector.h" +#include "util.h" + +#ifdef __cplusplus +extern "C" { +#endif + +#include + +typedef struct SWSSTableOpaque *SWSSTable; + +SWSSTable SWSSTable_new(SWSSDBConnector db, const char *tableName); + +void SWSSTable_free(SWSSTable tbl); + +// If the key exists, populates outValues with the table's values and returns 1. +// If the key doesn't exist, returns 0. +int8_t SWSSTable_get(SWSSTable tbl, const char *key, SWSSFieldValueArray *outValues); + +// If the key and field exist, populates outValue with the field's value and returns 1. +// If the key doesn't exist, returns 0. +int8_t SWSSTable_hget(SWSSTable tbl, const char *key, const char *field, SWSSString *outValue); + +void SWSSTable_set(SWSSTable tbl, const char *key, SWSSFieldValueArray values); + +void SWSSTable_hset(SWSSTable tbl, const char *key, const char *field, SWSSStrRef value); + +void SWSSTable_del(SWSSTable tbl, const char *key); + +void SWSSTable_hdel(SWSSTable tbl, const char *key, const char *field); + +SWSSStringArray SWSSTable_getKeys(SWSSTable tbl); + +#ifdef __cplusplus +} +#endif + +#endif diff --git a/common/c-api/util.cpp b/common/c-api/util.cpp index 1dc6cd451..26cc1b0f4 100644 --- a/common/c-api/util.cpp +++ b/common/c-api/util.cpp @@ -31,3 +31,7 @@ void SWSSFieldValueArray_free(SWSSFieldValueArray arr) { void SWSSKeyOpFieldValuesArray_free(SWSSKeyOpFieldValuesArray kfvs) { SWSSTry(delete[] kfvs.data); } + +void SWSSStringArray_free(SWSSStringArray arr) { + SWSSTry(delete[] arr.data); +} diff --git a/common/c-api/util.h b/common/c-api/util.h index 06aeac15e..aa0d66fbe 100644 --- a/common/c-api/util.h +++ b/common/c-api/util.h @@ -19,6 +19,7 @@ typedef struct SWSSStringOpaque *SWSSString; typedef struct SWSSStrRefOpaque *SWSSStrRef; // FFI version of swss::FieldValueTuple +// field should be freed with libc's free() typedef struct { const char *field; SWSSString value; @@ -59,6 +60,13 @@ typedef enum { SWSSSelectResult_SIGNAL = 2, } SWSSSelectResult; +// FFI version of std::vector +// data strings should be freed with libc's free() +typedef struct { + uint64_t len; + const char **data; +} SWSSStringArray; + // data should not include a null terminator SWSSString SWSSString_new(const char *data, uint64_t length); @@ -85,6 +93,10 @@ void SWSSFieldValueArray_free(SWSSFieldValueArray arr); // grained control of ownership). void SWSSKeyOpFieldValuesArray_free(SWSSKeyOpFieldValuesArray kfvs); +// arr.data may be null. This is not recursive - elements must be freed separately (for finer +// grained control of ownership). +void SWSSStringArray_free(SWSSStringArray arr); + #ifdef __cplusplus } #endif @@ -214,6 +226,19 @@ template static inline SWSSKeyOpFieldValuesArray makeKeyOpFieldValuesA return out; } +static inline SWSSStringArray makeStringArray(std::vector &&in) { + const char **data = new const char*[in.size()]; + + size_t i = 0; + for (std::string &s : in) + data[i++] = strdup(s.c_str()); + + SWSSStringArray out; + out.len = (uint64_t)in.size(); + out.data = data; + return out; +} + static inline std::string takeString(SWSSString s) { return std::string(std::move(*((std::string *)s))); } diff --git a/tests/c_api_ut.cpp b/tests/c_api_ut.cpp index ed814607e..90fffdaa2 100644 --- a/tests/c_api_ut.cpp +++ b/tests/c_api_ut.cpp @@ -5,6 +5,7 @@ #include "common/c-api/dbconnector.h" #include "common/c-api/producerstatetable.h" #include "common/c-api/subscriberstatetable.h" +#include "common/c-api/table.h" #include "common/c-api/util.h" #include "common/c-api/zmqclient.h" #include "common/c-api/zmqconsumerstatetable.h" @@ -40,14 +41,18 @@ static void sortKfvs(vector &kfvs) { #define free(x) std::free(const_cast(reinterpret_cast(x))); +static void freeFieldValuesArray(SWSSFieldValueArray arr) { + for (uint64_t i = 0; i < arr.len; i++) { + free(arr.data[i].field); + SWSSString_free(arr.data[i].value); + } + SWSSFieldValueArray_free(arr); +} + static void freeKeyOpFieldValuesArray(SWSSKeyOpFieldValuesArray arr) { for (uint64_t i = 0; i < arr.len; i++) { free(arr.data[i].key); - for (uint64_t j = 0; j < arr.data[i].fieldValues.len; j++) { - free(arr.data[i].fieldValues.data[j].field); - SWSSString_free(arr.data[i].fieldValues.data[j].value); - } - SWSSFieldValueArray_free(arr.data[i].fieldValues); + freeFieldValuesArray(arr.data[i].fieldValues); } SWSSKeyOpFieldValuesArray_free(arr); } @@ -105,6 +110,57 @@ TEST(c_api, DBConnector) { SWSSDBConnector_free(db); } +TEST(c_api, Table) { + clearDB(); + SWSSStringManager sm; + + SWSSDBConnector db = SWSSDBConnector_new_named("TEST_DB", 1000, true); + SWSSTable tbl = SWSSTable_new(db, "mytable"); + + SWSSFieldValueArray fvs; + SWSSString ss; + EXPECT_FALSE(SWSSTable_get(tbl, "mykey", &fvs)); + EXPECT_FALSE(SWSSTable_hget(tbl, "mykey", "myfield", &ss)); + + SWSSStringArray keys = SWSSTable_getKeys(tbl); + EXPECT_EQ(keys.len, 0); + SWSSStringArray_free(keys); + + SWSSTable_hset(tbl, "mykey", "myfield", sm.makeStrRef("myvalue")); + keys = SWSSTable_getKeys(tbl); + ASSERT_EQ(keys.len, 1); + EXPECT_STREQ(keys.data[0], "mykey"); + free(keys.data[0]); + SWSSStringArray_free(keys); + + ASSERT_TRUE(SWSSTable_hget(tbl, "mykey", "myfield", &ss)); + EXPECT_STREQ(SWSSStrRef_c_str((SWSSStrRef)ss), "myvalue"); + SWSSString_free(ss); + + SWSSTable_hdel(tbl, "mykey", "myfield"); + EXPECT_FALSE(SWSSTable_hget(tbl, "mykey", "myfield", &ss)); + + SWSSFieldValueTuple data[2] = {{.field = "myfield1", .value = sm.makeString("myvalue1")}, + {.field = "myfield2", .value = sm.makeString("myvalue2")}}; + fvs.len = 2; + fvs.data = data; + SWSSTable_set(tbl, "mykey", fvs); + + ASSERT_TRUE(SWSSTable_get(tbl, "mykey", &fvs)); + EXPECT_EQ(fvs.len, 2); + EXPECT_STREQ(data[0].field, fvs.data[0].field); + EXPECT_STREQ(data[1].field, fvs.data[1].field); + freeFieldValuesArray(fvs); + + SWSSTable_del(tbl, "mykey"); + keys = SWSSTable_getKeys(tbl); + EXPECT_EQ(keys.len, 0); + SWSSStringArray_free(keys); + + SWSSTable_free(tbl); + SWSSDBConnector_free(db); +} + TEST(c_api, ConsumerProducerStateTables) { clearDB(); SWSSStringManager sm; @@ -119,9 +175,8 @@ TEST(c_api, ConsumerProducerStateTables) { ASSERT_EQ(arr.len, 0); freeKeyOpFieldValuesArray(arr); - SWSSFieldValueTuple data[2] = { - {.field = "myfield1", .value = sm.makeString("myvalue1")}, - {.field = "myfield2", .value = sm.makeString("myvalue2")}}; + SWSSFieldValueTuple data[2] = {{.field = "myfield1", .value = sm.makeString("myvalue1")}, + {.field = "myfield2", .value = sm.makeString("myvalue2")}}; SWSSFieldValueArray values = { .len = 2, .data = data, @@ -154,7 +209,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); @@ -244,14 +299,16 @@ 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++) { - SWSSFieldValueTuple values_key1_data[2] = {{.field = "myfield1", .value = sm.makeString("myvalue1")}, - {.field = "myfield2", .value = sm.makeString("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, }; - SWSSFieldValueTuple values_key2_data[1] = {{.field = "myfield3", .value = sm.makeString("myvalue3")}}; + SWSSFieldValueTuple values_key2_data[1] = { + {.field = "myfield3", .value = sm.makeString("myvalue3")}}; SWSSFieldValueArray values_key2 = { .len = 1, .data = values_key2_data, From 5a4b4a573f32fbdad026acf01e0a8b7a900c7152 Mon Sep 17 00:00:00 2001 From: erer1243 <1377477+erer1243@users.noreply.github.com> Date: Wed, 15 Jan 2025 19:55:24 -0500 Subject: [PATCH 26/29] C API Exceptions (#967) * c-api exceptions on dbconnector * Remaining c api exceptions * Improve coverage of exception surface * Add comments explaining how structs should be freed --- common/c-api/consumerstatetable.cpp | 36 ++-- common/c-api/consumerstatetable.h | 21 +-- common/c-api/dbconnector.cpp | 59 ++++--- common/c-api/dbconnector.h | 56 +++--- common/c-api/producerstatetable.cpp | 31 ++-- common/c-api/producerstatetable.h | 24 +-- common/c-api/result.h | 45 +++++ common/c-api/subscriberstatetable.cpp | 39 +++-- common/c-api/subscriberstatetable.h | 26 +-- common/c-api/table.cpp | 40 ++--- common/c-api/table.h | 29 ++-- common/c-api/util.cpp | 18 +- common/c-api/util.h | 42 ++--- common/c-api/zmqclient.cpp | 17 +- common/c-api/zmqclient.h | 15 +- common/c-api/zmqconsumerstatetable.cpp | 47 ++--- common/c-api/zmqconsumerstatetable.h | 30 ++-- common/c-api/zmqproducerstatetable.cpp | 26 +-- common/c-api/zmqproducerstatetable.h | 16 +- common/c-api/zmqserver.cpp | 6 +- common/c-api/zmqserver.h | 5 +- tests/c_api_ut.cpp | 227 +++++++++++++++++-------- tests/main.cpp | 3 - 23 files changed, 500 insertions(+), 358 deletions(-) create mode 100644 common/c-api/result.h diff --git a/common/c-api/consumerstatetable.cpp b/common/c-api/consumerstatetable.cpp index 9765ceec3..426055da1 100644 --- a/common/c-api/consumerstatetable.cpp +++ b/common/c-api/consumerstatetable.cpp @@ -13,33 +13,37 @@ using namespace swss; using namespace std; using boost::numeric_cast; -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)); +SWSSResult SWSSConsumerStateTable_new(SWSSDBConnector db, const char *tableName, + const int32_t *p_popBatchSize, const int32_t *p_pri, + SWSSConsumerStateTable *outTbl) { + SWSSTry({ + int popBatchSize = p_popBatchSize ? numeric_cast(*p_popBatchSize) + : TableConsumable::DEFAULT_POP_BATCH_SIZE; + int pri = p_pri ? numeric_cast(*p_pri) : 0; + *outTbl = (SWSSConsumerStateTable) new ConsumerStateTable( + (DBConnector *)db, string(tableName), popBatchSize, pri); + }); } -void SWSSConsumerStateTable_free(SWSSConsumerStateTable tbl) { +SWSSResult SWSSConsumerStateTable_free(SWSSConsumerStateTable tbl) { SWSSTry(delete (ConsumerStateTable *)tbl); } -SWSSKeyOpFieldValuesArray SWSSConsumerStateTable_pops(SWSSConsumerStateTable tbl) { +SWSSResult SWSSConsumerStateTable_pops(SWSSConsumerStateTable tbl, + SWSSKeyOpFieldValuesArray *outArr) { SWSSTry({ deque vkco; ((ConsumerStateTable *)tbl)->pops(vkco); - return makeKeyOpFieldValuesArray(vkco); + *outArr = makeKeyOpFieldValuesArray(vkco); }); } -uint32_t SWSSConsumerStateTable_getFd(SWSSConsumerStateTable tbl) { - SWSSTry(return numeric_cast(((ConsumerStateTable *)tbl)->getFd())); +SWSSResult SWSSConsumerStateTable_getFd(SWSSConsumerStateTable tbl, uint32_t *outFd) { + SWSSTry(*outFd = 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)); +SWSSResult SWSSConsumerStateTable_readData(SWSSConsumerStateTable tbl, uint32_t timeout_ms, + uint8_t interrupt_on_signal, + SWSSSelectResult *outResult) { + SWSSTry(*outResult = selectOne((ConsumerStateTable *)tbl, timeout_ms, interrupt_on_signal)); } diff --git a/common/c-api/consumerstatetable.h b/common/c-api/consumerstatetable.h index 468fb644b..43ed4cde4 100644 --- a/common/c-api/consumerstatetable.h +++ b/common/c-api/consumerstatetable.h @@ -2,6 +2,7 @@ #define SWSS_COMMON_C_API_CONSUMERSTATETABLE_H #include "dbconnector.h" +#include "result.h" #include "util.h" #ifdef __cplusplus @@ -12,25 +13,25 @@ extern "C" { 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); +SWSSResult SWSSConsumerStateTable_new(SWSSDBConnector db, const char *tableName, + const int32_t *popBatchSize, const int32_t *pri, + SWSSConsumerStateTable *outTbl); -void SWSSConsumerStateTable_free(SWSSConsumerStateTable tbl); +SWSSResult SWSSConsumerStateTable_free(SWSSConsumerStateTable tbl); // Result array and all of its members must be freed using free() -SWSSKeyOpFieldValuesArray SWSSConsumerStateTable_pops(SWSSConsumerStateTable tbl); +SWSSResult SWSSConsumerStateTable_pops(SWSSConsumerStateTable tbl, SWSSKeyOpFieldValuesArray *outArr); -// Return the underlying fd for polling/selecting on. -// Callers must NOT read/write on fd, it may only be used for epoll or similar. +// Outputs the underlying fd for polling/selecting on. +// Callers must NOT read/write on the 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); +SWSSResult SWSSConsumerStateTable_getFd(SWSSConsumerStateTable tbl, uint32_t *outFd); // 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); +SWSSResult SWSSConsumerStateTable_readData(SWSSConsumerStateTable tbl, uint32_t timeout_ms, + uint8_t interrupt_on_signal, SWSSSelectResult *outResult); #ifdef __cplusplus } diff --git a/common/c-api/dbconnector.cpp b/common/c-api/dbconnector.cpp index 83f237cc0..2a8871481 100644 --- a/common/c-api/dbconnector.cpp +++ b/common/c-api/dbconnector.cpp @@ -9,67 +9,66 @@ using namespace swss; using namespace std; -void SWSSSonicDBConfig_initialize(const char *path) { +SWSSResult SWSSSonicDBConfig_initialize(const char *path) { SWSSTry(SonicDBConfig::initialize(path)); } -void SWSSSonicDBConfig_initializeGlobalConfig(const char *path) { +SWSSResult 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)); +SWSSResult SWSSDBConnector_new_tcp(int32_t dbId, const char *hostname, uint16_t port, + uint32_t timeout, SWSSDBConnector *outDb) { + SWSSTry(*outDb = (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)); +SWSSResult SWSSDBConnector_new_unix(int32_t dbId, const char *sock_path, uint32_t timeout, SWSSDBConnector *outDb) { + SWSSTry(*outDb = (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)); +SWSSResult SWSSDBConnector_new_named(const char *dbName, uint32_t timeout_ms, uint8_t isTcpConn, SWSSDBConnector *outDb) { + SWSSTry(*outDb = (SWSSDBConnector) new DBConnector(string(dbName), timeout_ms, isTcpConn)); } -void SWSSDBConnector_free(SWSSDBConnector db) { - delete (DBConnector *)db; +SWSSResult SWSSDBConnector_free(SWSSDBConnector db) { + SWSSTry(delete (DBConnector *)db); } -int8_t SWSSDBConnector_del(SWSSDBConnector db, const char *key) { - SWSSTry(return ((DBConnector *)db)->del(string(key)) ? 1 : 0); +SWSSResult SWSSDBConnector_del(SWSSDBConnector db, const char *key, int8_t *outStatus) { + SWSSTry(*outStatus = ((DBConnector *)db)->del(string(key)) ? 1 : 0); } -void SWSSDBConnector_set(SWSSDBConnector db, const char *key, SWSSStrRef value) { +SWSSResult SWSSDBConnector_set(SWSSDBConnector db, const char *key, SWSSStrRef value) { SWSSTry(((DBConnector *)db)->set(string(key), takeStrRef(value))); } -SWSSString SWSSDBConnector_get(SWSSDBConnector db, const char *key) { +SWSSResult SWSSDBConnector_get(SWSSDBConnector db, const char *key, SWSSString *outValue) { SWSSTry({ shared_ptr s = ((DBConnector *)db)->get(string(key)); - return s ? makeString(move(*s)) : nullptr; + *outValue = s ? makeString(move(*s)) : nullptr; }); } -int8_t SWSSDBConnector_exists(SWSSDBConnector db, const char *key) { - SWSSTry(return ((DBConnector *)db)->exists(string(key)) ? 1 : 0); +SWSSResult SWSSDBConnector_exists(SWSSDBConnector db, const char *key, int8_t *outExists) { + SWSSTry(*outExists = ((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); +SWSSResult SWSSDBConnector_hdel(SWSSDBConnector db, const char *key, const char *field, int8_t *outResult) { + SWSSTry(*outResult = ((DBConnector *)db)->hdel(string(key), string(field)) ? 1 : 0); } -void SWSSDBConnector_hset(SWSSDBConnector db, const char *key, const char *field, - SWSSStrRef value) { +SWSSResult SWSSDBConnector_hset(SWSSDBConnector db, const char *key, const char *field, SWSSStrRef value) { SWSSTry(((DBConnector *)db)->hset(string(key), string(field), takeStrRef(value))); } -SWSSString SWSSDBConnector_hget(SWSSDBConnector db, const char *key, const char *field) { +SWSSResult SWSSDBConnector_hget(SWSSDBConnector db, const char *key, const char *field, SWSSString *outValue) { SWSSTry({ shared_ptr s = ((DBConnector *)db)->hget(string(key), string(field)); - return s ? makeString(move(*s)) : nullptr; + *outValue = s ? makeString(move(*s)) : nullptr; }); } -SWSSFieldValueArray SWSSDBConnector_hgetall(SWSSDBConnector db, const char *key) { +SWSSResult SWSSDBConnector_hgetall(SWSSDBConnector db, const char *key, SWSSFieldValueArray *outArr) { SWSSTry({ auto map = ((DBConnector *)db)->hgetall(string(key)); @@ -80,14 +79,14 @@ SWSSFieldValueArray SWSSDBConnector_hgetall(SWSSDBConnector db, const char *key) for (auto &pair : map) pairs.push_back(make_pair(pair.first, move(pair.second))); - return makeFieldValueArray(std::move(pairs)); + *outArr = makeFieldValueArray(std::move(pairs)); }); } -int8_t SWSSDBConnector_hexists(SWSSDBConnector db, const char *key, const char *field) { - SWSSTry(return ((DBConnector *)db)->hexists(string(key), string(field)) ? 1 : 0); +SWSSResult SWSSDBConnector_hexists(SWSSDBConnector db, const char *key, const char *field, int8_t *outExists) { + SWSSTry(*outExists = ((DBConnector *)db)->hexists(string(key), string(field)) ? 1 : 0); } -int8_t SWSSDBConnector_flushdb(SWSSDBConnector db) { - SWSSTry(return ((DBConnector *)db)->flushdb() ? 1 : 0); +SWSSResult SWSSDBConnector_flushdb(SWSSDBConnector db, int8_t *outStatus) { + SWSSTry(*outStatus = ((DBConnector *)db)->flushdb() ? 1 : 0); } diff --git a/common/c-api/dbconnector.h b/common/c-api/dbconnector.h index fe4acdf4d..f60026e71 100644 --- a/common/c-api/dbconnector.h +++ b/common/c-api/dbconnector.h @@ -1,61 +1,63 @@ #ifndef SWSS_COMMON_C_API_DBCONNECTOR_H #define SWSS_COMMON_C_API_DBCONNECTOR_H +#include "result.h" #include "util.h" + #ifdef __cplusplus extern "C" { #endif #include -void SWSSSonicDBConfig_initialize(const char *path); +SWSSResult SWSSSonicDBConfig_initialize(const char *path); -void SWSSSonicDBConfig_initializeGlobalConfig(const char *path); +SWSSResult 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); +SWSSResult SWSSDBConnector_new_tcp(int32_t dbId, const char *hostname, uint16_t port, + uint32_t timeout, SWSSDBConnector *outDb); // Pass 0 to timeout for infinity -SWSSDBConnector SWSSDBConnector_new_unix(int32_t dbId, const char *sock_path, uint32_t timeout_ms); +SWSSResult SWSSDBConnector_new_unix(int32_t dbId, const char *sock_path, uint32_t timeout, SWSSDBConnector *outDb); // Pass 0 to timeout for infinity -SWSSDBConnector SWSSDBConnector_new_named(const char *dbName, uint32_t timeout_ms, uint8_t isTcpConn); +SWSSResult SWSSDBConnector_new_named(const char *dbName, uint32_t timeout_ms, uint8_t isTcpConn, SWSSDBConnector *outDb); -void SWSSDBConnector_free(SWSSDBConnector db); +SWSSResult 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); +// Outputs 0 when key doesn't exist, 1 when key was deleted +SWSSResult SWSSDBConnector_del(SWSSDBConnector db, const char *key, int8_t *outStatus); -void SWSSDBConnector_set(SWSSDBConnector db, const char *key, SWSSStrRef value); +SWSSResult SWSSDBConnector_set(SWSSDBConnector db, const char *key, SWSSStrRef value); -// Returns NULL if key doesn't exist -// Result must be freed using SWSSString_free() -SWSSString SWSSDBConnector_get(SWSSDBConnector db, const char *key); +// Outputs NULL if key doesn't exist +// Value must be freed using SWSSString_free() +SWSSResult SWSSDBConnector_get(SWSSDBConnector db, const char *key, SWSSString *outValue); -// Returns 0 for false, 1 for true -int8_t SWSSDBConnector_exists(SWSSDBConnector db, const char *key); +// Outputs 0 for false, 1 for true +SWSSResult SWSSDBConnector_exists(SWSSDBConnector db, const char *key, int8_t *outExists); -// 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); +// Outputs 0 when key or field doesn't exist, 1 when field was deleted +SWSSResult SWSSDBConnector_hdel(SWSSDBConnector db, const char *key, const char *field, int8_t *outResult); -void SWSSDBConnector_hset(SWSSDBConnector db, const char *key, const char *field, SWSSStrRef value); +SWSSResult 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 SWSSString_free() -SWSSString SWSSDBConnector_hget(SWSSDBConnector db, const char *key, const char *field); +// Outputs NULL if key or field doesn't exist +// Value must be freed using SWSSString_free() +SWSSResult SWSSDBConnector_hget(SWSSDBConnector db, const char *key, const char *field, SWSSString *outValue); -// Returns an empty map when the key doesn't exist +// Outputs 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); +SWSSResult SWSSDBConnector_hgetall(SWSSDBConnector db, const char *key, SWSSFieldValueArray *outArr); -// 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); +// Outputs 0 when key or field doesn't exist, 1 when field exists +SWSSResult SWSSDBConnector_hexists(SWSSDBConnector db, const char *key, const char *field, int8_t *outExists); -// Returns 1 on success, 0 on failure -int8_t SWSSDBConnector_flushdb(SWSSDBConnector db); +// Outputs 1 on success, 0 on failure +SWSSResult SWSSDBConnector_flushdb(SWSSDBConnector db, int8_t *outStatus); #ifdef __cplusplus } diff --git a/common/c-api/producerstatetable.cpp b/common/c-api/producerstatetable.cpp index 276d7c680..c9cc34463 100644 --- a/common/c-api/producerstatetable.cpp +++ b/common/c-api/producerstatetable.cpp @@ -10,44 +10,45 @@ using namespace swss; using namespace std; -SWSSProducerStateTable SWSSProducerStateTable_new(SWSSDBConnector db, const char *tableName) { - SWSSTry(return (SWSSProducerStateTable) new ProducerStateTable((DBConnector *)db, - string(tableName))); +SWSSResult SWSSProducerStateTable_new(SWSSDBConnector db, const char *tableName, + SWSSProducerStateTable *outTbl) { + SWSSTry(*outTbl = (SWSSProducerStateTable) new ProducerStateTable((DBConnector *)db, + string(tableName))); } -void SWSSProducerStateTable_free(SWSSProducerStateTable tbl) { +SWSSResult SWSSProducerStateTable_free(SWSSProducerStateTable tbl) { SWSSTry(delete ((ProducerStateTable *)tbl)); } -void SWSSProducerStateTable_setBuffered(SWSSProducerStateTable tbl, uint8_t buffered) { - SWSSTry(((ProducerStateTable *)tbl)->setBuffered((bool)buffered)) +SWSSResult SWSSProducerStateTable_setBuffered(SWSSProducerStateTable tbl, uint8_t buffered) { + SWSSTry(((ProducerStateTable *)tbl)->setBuffered((bool)buffered)); } -void SWSSProducerStateTable_set(SWSSProducerStateTable tbl, const char *key, - SWSSFieldValueArray values) { +SWSSResult SWSSProducerStateTable_set(SWSSProducerStateTable tbl, const char *key, + SWSSFieldValueArray values) { SWSSTry(((ProducerStateTable *)tbl)->set(string(key), takeFieldValueArray(std::move(values)))); } -void SWSSProducerStateTable_del(SWSSProducerStateTable tbl, const char *key) { +SWSSResult SWSSProducerStateTable_del(SWSSProducerStateTable tbl, const char *key) { SWSSTry(((ProducerStateTable *)tbl)->del(string(key))); } -void SWSSProducerStateTable_flush(SWSSProducerStateTable tbl) { +SWSSResult SWSSProducerStateTable_flush(SWSSProducerStateTable tbl) { SWSSTry(((ProducerStateTable *)tbl)->flush()); } -int64_t SWSSProducerStateTable_count(SWSSProducerStateTable tbl) { - SWSSTry(return ((ProducerStateTable *)tbl)->count()); +SWSSResult SWSSProducerStateTable_count(SWSSProducerStateTable tbl, int64_t *outCount) { + SWSSTry(*outCount = ((ProducerStateTable *)tbl)->count()); } -void SWSSProducerStateTable_clear(SWSSProducerStateTable tbl) { +SWSSResult SWSSProducerStateTable_clear(SWSSProducerStateTable tbl) { SWSSTry(((ProducerStateTable *)tbl)->clear()); } -void SWSSProducerStateTable_create_temp_view(SWSSProducerStateTable tbl) { +SWSSResult SWSSProducerStateTable_create_temp_view(SWSSProducerStateTable tbl) { SWSSTry(((ProducerStateTable *)tbl)->create_temp_view()); } -void SWSSProducerStateTable_apply_temp_view(SWSSProducerStateTable tbl) { +SWSSResult 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 index 1acb9af37..463aeaaec 100644 --- a/common/c-api/producerstatetable.h +++ b/common/c-api/producerstatetable.h @@ -2,6 +2,7 @@ #define SWSS_COMMON_C_API_PRODUCERSTATETABLE_H #include "dbconnector.h" +#include "result.h" #include "util.h" #ifdef __cplusplus @@ -11,26 +12,27 @@ extern "C" { #include typedef struct SWSSProducerStateTableOpaque *SWSSProducerStateTable; +SWSSResult SWSSProducerStateTable_new(SWSSDBConnector db, const char *tableName, + SWSSProducerStateTable *outTbl); -SWSSProducerStateTable SWSSProducerStateTable_new(SWSSDBConnector db, const char *tableName); +SWSSResult SWSSProducerStateTable_free(SWSSProducerStateTable tbl); -void SWSSProducerStateTable_free(SWSSProducerStateTable tbl); +SWSSResult SWSSProducerStateTable_setBuffered(SWSSProducerStateTable tbl, uint8_t buffered); -void SWSSProducerStateTable_setBuffered(SWSSProducerStateTable tbl, uint8_t buffered); +SWSSResult SWSSProducerStateTable_set(SWSSProducerStateTable tbl, const char *key, + SWSSFieldValueArray values); -void SWSSProducerStateTable_set(SWSSProducerStateTable tbl, const char *key, SWSSFieldValueArray values); +SWSSResult SWSSProducerStateTable_del(SWSSProducerStateTable tbl, const char *key); -void SWSSProducerStateTable_del(SWSSProducerStateTable tbl, const char *key); +SWSSResult SWSSProducerStateTable_flush(SWSSProducerStateTable tbl); -void SWSSProducerStateTable_flush(SWSSProducerStateTable tbl); +SWSSResult SWSSProducerStateTable_count(SWSSProducerStateTable tbl, int64_t *outCount); -int64_t SWSSProducerStateTable_count(SWSSProducerStateTable tbl); +SWSSResult SWSSProducerStateTable_clear(SWSSProducerStateTable tbl); -void SWSSProducerStateTable_clear(SWSSProducerStateTable tbl); +SWSSResult SWSSProducerStateTable_create_temp_view(SWSSProducerStateTable tbl); -void SWSSProducerStateTable_create_temp_view(SWSSProducerStateTable tbl); - -void SWSSProducerStateTable_apply_temp_view(SWSSProducerStateTable tbl); +SWSSResult SWSSProducerStateTable_apply_temp_view(SWSSProducerStateTable tbl); #ifdef __cplusplus } diff --git a/common/c-api/result.h b/common/c-api/result.h new file mode 100644 index 000000000..9c29d0eea --- /dev/null +++ b/common/c-api/result.h @@ -0,0 +1,45 @@ +#ifndef SWSS_COMMON_C_API_RESULT_H +#define SWSS_COMMON_C_API_RESULT_H + +#include "util.h" + +typedef enum { + // No exception occurred + SWSSException_None, + + // std::exception was thrown + SWSSException_Exception, +} SWSSException; + +// If exception is SWSSException_None, message and location will be null. +// If message and location are non-null, they must be freed using SWSSString_free. +typedef struct { + SWSSException exception; + SWSSString message; + SWSSString location; +} SWSSResult; + +#ifdef __cplusplus +#include +#include +#include + +using namespace swss; + +#define SWSSTry(...) return SWSSTry_([=] { __VA_ARGS__; }, BOOST_CURRENT_FUNCTION) + +template static inline SWSSResult SWSSTry_(Func &&f, const char *funcName) { + SWSSResult result = {SWSSException_None, nullptr, nullptr}; + try { + f(); + } catch (std::exception &e) { + result.exception = SWSSException_Exception; + result.message = makeString(e.what()); + } + if (result.exception != SWSSException_None) + result.location = makeString(funcName); + return result; +} +#endif + +#endif diff --git a/common/c-api/subscriberstatetable.cpp b/common/c-api/subscriberstatetable.cpp index 4d3a04953..3e1deb8ec 100644 --- a/common/c-api/subscriberstatetable.cpp +++ b/common/c-api/subscriberstatetable.cpp @@ -14,34 +14,37 @@ using namespace swss; using namespace std; using boost::numeric_cast; -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)); +SWSSResult SWSSSubscriberStateTable_new(SWSSDBConnector db, const char *tableName, + const int32_t *p_popBatchSize, const int32_t *p_pri, + SWSSSubscriberStateTable *outTbl) { + SWSSTry({ + int popBatchSize = p_popBatchSize ? numeric_cast(*p_popBatchSize) + : TableConsumable::DEFAULT_POP_BATCH_SIZE; + int pri = p_pri ? numeric_cast(*p_pri) : 0; + *outTbl = (SWSSSubscriberStateTable) new SubscriberStateTable( + (DBConnector *)db, string(tableName), popBatchSize, pri); + }); } -void SWSSSubscriberStateTable_free(SWSSSubscriberStateTable tbl) { - delete (SubscriberStateTable *)tbl; +SWSSResult SWSSSubscriberStateTable_free(SWSSSubscriberStateTable tbl) { + SWSSTry(delete (SubscriberStateTable *)tbl); } -SWSSKeyOpFieldValuesArray SWSSSubscriberStateTable_pops(SWSSSubscriberStateTable tbl) { +SWSSResult SWSSSubscriberStateTable_pops(SWSSSubscriberStateTable tbl, + SWSSKeyOpFieldValuesArray *outArr) { SWSSTry({ deque vkco; ((SubscriberStateTable *)tbl)->pops(vkco); - return makeKeyOpFieldValuesArray(vkco); + *outArr = makeKeyOpFieldValuesArray(vkco); }); } -uint32_t SWSSSubscriberStateTable_getFd(SWSSSubscriberStateTable tbl) { - SWSSTry(return numeric_cast(((SubscriberStateTable *)tbl)->getFd())); +SWSSResult SWSSSubscriberStateTable_getFd(SWSSSubscriberStateTable tbl, uint32_t *outFd) { + SWSSTry(*outFd = numeric_cast(((SubscriberStateTable *)tbl)->getFd())); } -SWSSSelectResult SWSSSubscriberStateTable_readData(SWSSSubscriberStateTable tbl, - uint32_t timeout_ms, - uint8_t interrupt_on_signal) { - SWSSTry(return selectOne((SubscriberStateTable *)tbl, timeout_ms, interrupt_on_signal)); +SWSSResult SWSSSubscriberStateTable_readData(SWSSSubscriberStateTable tbl, uint32_t timeout_ms, + uint8_t interrupt_on_signal, + SWSSSelectResult *outResult) { + SWSSTry(*outResult = selectOne((SubscriberStateTable *)tbl, timeout_ms, interrupt_on_signal)); } diff --git a/common/c-api/subscriberstatetable.h b/common/c-api/subscriberstatetable.h index ed0924c81..5f7444063 100644 --- a/common/c-api/subscriberstatetable.h +++ b/common/c-api/subscriberstatetable.h @@ -2,6 +2,7 @@ #define SWSS_COMMON_C_API_SUBSCRIBERSTATETABLE_H #include "dbconnector.h" +#include "result.h" #include "util.h" #ifdef __cplusplus @@ -12,27 +13,28 @@ extern "C" { 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); +SWSSResult SWSSSubscriberStateTable_new(SWSSDBConnector db, const char *tableName, + const int32_t *popBatchSize, const int32_t *pri, + SWSSSubscriberStateTable *outTbl); -void SWSSSubscriberStateTable_free(SWSSSubscriberStateTable tbl); +// Frees the SubscriberStateTable +SWSSResult SWSSSubscriberStateTable_free(SWSSSubscriberStateTable tbl); // Result array and all of its members must be freed using free() -SWSSKeyOpFieldValuesArray SWSSSubscriberStateTable_pops(SWSSSubscriberStateTable tbl); +SWSSResult SWSSSubscriberStateTable_pops(SWSSSubscriberStateTable tbl, + SWSSKeyOpFieldValuesArray *outArr); -// Return the underlying fd for polling/selecting on. -// Callers must NOT read/write on fd, it may only be used for epoll or similar. +// Outputs the underlying fd for polling/selecting on. +// Callers must NOT read/write on the 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); +SWSSResult SWSSSubscriberStateTable_getFd(SWSSSubscriberStateTable tbl, uint32_t *outFd); // 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, - uint8_t interrupt_on_sugnal); +SWSSResult SWSSSubscriberStateTable_readData(SWSSSubscriberStateTable tbl, uint32_t timeout_ms, + uint8_t interrupt_on_signal, + SWSSSelectResult *outResult); #ifdef __cplusplus } diff --git a/common/c-api/table.cpp b/common/c-api/table.cpp index af08cbaa8..d3e88c9cb 100644 --- a/common/c-api/table.cpp +++ b/common/c-api/table.cpp @@ -9,63 +9,65 @@ using namespace swss; using namespace std; -SWSSTable SWSSTable_new(SWSSDBConnector db, const char *tableName) { - SWSSTry(return (SWSSTable) new Table((DBConnector *)db, string(tableName))); +SWSSResult SWSSTable_new(SWSSDBConnector db, const char *tableName, SWSSTable *outTbl) { + SWSSTry(*outTbl = (SWSSTable) new Table((DBConnector *)db, string(tableName))); } -void SWSSTable_free(SWSSTable tbl) { +SWSSResult SWSSTable_free(SWSSTable tbl) { SWSSTry(delete (Table *)tbl); } -int8_t SWSSTable_get(SWSSTable tbl, const char *key, SWSSFieldValueArray *outValues) { +SWSSResult SWSSTable_get(SWSSTable tbl, const char *key, SWSSFieldValueArray *outValues, + int8_t *outExists) { SWSSTry({ vector fvs; bool exists = ((Table *)tbl)->get(string(key), fvs); if (exists) { *outValues = makeFieldValueArray(fvs); - return 1; + *outExists = 1; } else { - return 0; + *outExists = 0; } }); } -int8_t SWSSTable_hget(SWSSTable tbl, const char *key, const char *field, SWSSString *outValue) { +SWSSResult SWSSTable_hget(SWSSTable tbl, const char *key, const char *field, SWSSString *outValue, + int8_t *outExists) { SWSSTry({ string s; bool exists = ((Table *)tbl)->hget(string(key), string(field), s); if (exists) { *outValue = makeString(move(s)); - return 1; + *outExists = 1; } else { - return 0; + *outExists = 0; } }); } -void SWSSTable_set(SWSSTable tbl, const char *key, SWSSFieldValueArray values) { +SWSSResult SWSSTable_set(SWSSTable tbl, const char *key, SWSSFieldValueArray values) { SWSSTry({ vector fvs = takeFieldValueArray(values); ((Table *)tbl)->set(string(key), fvs); }); } -void SWSSTable_hset(SWSSTable tbl, const char *key, const char *field, SWSSStrRef value) { - SWSSTry({ ((Table *)tbl)->hset(string(key), string(field), takeStrRef(value)); }); +SWSSResult SWSSTable_hset(SWSSTable tbl, const char *key, const char *field, SWSSStrRef value) { + SWSSTry(((Table *)tbl)->hset(string(key), string(field), takeStrRef(value))); } -void SWSSTable_del(SWSSTable tbl, const char *key) { - SWSSTry({ ((Table *)tbl)->del(string(key)); }); +SWSSResult SWSSTable_del(SWSSTable tbl, const char *key) { + SWSSTry(((Table *)tbl)->del(string(key))); } -void SWSSTable_hdel(SWSSTable tbl, const char *key, const char *field) { - SWSSTry({ ((Table *)tbl)->hdel(string(key), string(field)); }); +SWSSResult SWSSTable_hdel(SWSSTable tbl, const char *key, const char *field) { + SWSSTry(((Table *)tbl)->hdel(string(key), string(field))); } -SWSSStringArray SWSSTable_getKeys(SWSSTable tbl) { +SWSSResult SWSSTable_getKeys(SWSSTable tbl, SWSSStringArray *outKeys) { SWSSTry({ vector keys; ((Table *)tbl)->getKeys(keys); - return makeStringArray(move(keys)); - }) + *outKeys = makeStringArray(move(keys)); + }); } diff --git a/common/c-api/table.h b/common/c-api/table.h index 0d06e1e7d..087bec815 100644 --- a/common/c-api/table.h +++ b/common/c-api/table.h @@ -2,6 +2,7 @@ #define SWSS_COMMON_C_API_TABLE_H #include "dbconnector.h" +#include "result.h" #include "util.h" #ifdef __cplusplus @@ -12,27 +13,29 @@ extern "C" { typedef struct SWSSTableOpaque *SWSSTable; -SWSSTable SWSSTable_new(SWSSDBConnector db, const char *tableName); +SWSSResult SWSSTable_new(SWSSDBConnector db, const char *tableName, SWSSTable *outTbl); -void SWSSTable_free(SWSSTable tbl); +SWSSResult SWSSTable_free(SWSSTable tbl); -// If the key exists, populates outValues with the table's values and returns 1. -// If the key doesn't exist, returns 0. -int8_t SWSSTable_get(SWSSTable tbl, const char *key, SWSSFieldValueArray *outValues); +// If the key exists, populates outValues with the table's values and outputs 1. +// If the key doesn't exist, outputs 0 and does not touch outValues. +SWSSResult SWSSTable_get(SWSSTable tbl, const char *key, SWSSFieldValueArray *outValues, + int8_t *outExists); -// If the key and field exist, populates outValue with the field's value and returns 1. -// If the key doesn't exist, returns 0. -int8_t SWSSTable_hget(SWSSTable tbl, const char *key, const char *field, SWSSString *outValue); +// If the key and field exist, populates outValue with the field's value and outputs 1. +// If the key doesn't exist, outputs 0 and does not touch outValue. +SWSSResult SWSSTable_hget(SWSSTable tbl, const char *key, const char *field, SWSSString *outValue, + int8_t *outExists); -void SWSSTable_set(SWSSTable tbl, const char *key, SWSSFieldValueArray values); +SWSSResult SWSSTable_set(SWSSTable tbl, const char *key, SWSSFieldValueArray values); -void SWSSTable_hset(SWSSTable tbl, const char *key, const char *field, SWSSStrRef value); +SWSSResult SWSSTable_hset(SWSSTable tbl, const char *key, const char *field, SWSSStrRef value); -void SWSSTable_del(SWSSTable tbl, const char *key); +SWSSResult SWSSTable_del(SWSSTable tbl, const char *key); -void SWSSTable_hdel(SWSSTable tbl, const char *key, const char *field); +SWSSResult SWSSTable_hdel(SWSSTable tbl, const char *key, const char *field); -SWSSStringArray SWSSTable_getKeys(SWSSTable tbl); +SWSSResult SWSSTable_getKeys(SWSSTable tbl, SWSSStringArray *outKeys); #ifdef __cplusplus } diff --git a/common/c-api/util.cpp b/common/c-api/util.cpp index 26cc1b0f4..bc173420c 100644 --- a/common/c-api/util.cpp +++ b/common/c-api/util.cpp @@ -2,36 +2,34 @@ 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)))); + return makeString(std::string(data, numeric_cast(length))); } SWSSString SWSSString_new_c_str(const char *c_str) { - SWSSTry(return makeString(std::string(c_str))); + return makeString(std::string(c_str)); } const char *SWSSStrRef_c_str(SWSSStrRef s) { - SWSSTry(return ((std::string *)s)->c_str()); + return ((std::string *)s)->c_str(); } uint64_t SWSSStrRef_length(SWSSStrRef s) { - SWSSTry(return ((std::string *)s)->length()); + return ((std::string *)s)->length(); } void SWSSString_free(SWSSString s) { - SWSSTry(delete (std::string *)s); + delete (std::string *)s; } void SWSSFieldValueArray_free(SWSSFieldValueArray arr) { - SWSSTry(delete[] arr.data); + delete[] arr.data; } void SWSSKeyOpFieldValuesArray_free(SWSSKeyOpFieldValuesArray kfvs) { - SWSSTry(delete[] kfvs.data); + delete[] kfvs.data; } void SWSSStringArray_free(SWSSStringArray arr) { - SWSSTry(delete[] arr.data); + delete[] arr.data; } diff --git a/common/c-api/util.h b/common/c-api/util.h index aa0d66fbe..42785edcf 100644 --- a/common/c-api/util.h +++ b/common/c-api/util.h @@ -20,12 +20,14 @@ typedef struct SWSSStrRefOpaque *SWSSStrRef; // FFI version of swss::FieldValueTuple // field should be freed with libc's free() +// value should be freed with SWSSString_free() typedef struct { const char *field; SWSSString value; } SWSSFieldValueTuple; // FFI version of std::vector +// data should be freed with SWSSFieldValueArray_free() typedef struct { uint64_t len; SWSSFieldValueTuple *data; @@ -37,6 +39,8 @@ typedef enum { } SWSSKeyOperation; // FFI version of swss::KeyOpFieldValuesTuple +// key should be freed with libc's free() +// fieldValues should be freed with SWSSFieldValueArray_free() typedef struct { const char *key; SWSSKeyOperation operation; @@ -44,6 +48,7 @@ typedef struct { } SWSSKeyOpFieldValues; // FFI version of std::vector +// data should be freed with SWSSKeyOpFieldValuesArray_free() typedef struct { uint64_t len; SWSSKeyOpFieldValues *data; @@ -61,7 +66,8 @@ typedef enum { } SWSSSelectResult; // FFI version of std::vector -// data strings should be freed with libc's free() +// strings in data should be freed with libc's free() +// data should be freed with SWSSStringArray_free() typedef struct { uint64_t len; const char **data; @@ -121,40 +127,20 @@ void SWSSStringArray_free(SWSSStringArray arr); 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 (swss::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, uint8_t interrupt_on_signal) { - Select select; - Selectable *sOut; + swss::Select select; + swss::Selectable *sOut; select.addSelectable(s); int ret = select.select(&sOut, numeric_cast(timeout_ms), interrupt_on_signal); switch (ret) { - case Select::OBJECT: + case swss::Select::OBJECT: return SWSSSelectResult_DATA; - case Select::ERROR: + case swss::Select::ERROR: throw std::system_error(errno, std::generic_category()); - case Select::TIMEOUT: + case swss::Select::TIMEOUT: return SWSSSelectResult_TIMEOUT; - case Select::SIGNALINT: + case swss::Select::SIGNALINT: return SWSSSelectResult_SIGNAL; default: SWSS_LOG_THROW("impossible: unhandled Select::select() return value: %d", ret); @@ -286,7 +272,5 @@ takeKeyOpFieldValuesArray(SWSSKeyOpFieldValuesArray in) { return out; } -} // namespace swss - #endif #endif diff --git a/common/c-api/zmqclient.cpp b/common/c-api/zmqclient.cpp index fa1d59ca2..9b0ef971a 100644 --- a/common/c-api/zmqclient.cpp +++ b/common/c-api/zmqclient.cpp @@ -6,25 +6,24 @@ using namespace swss; using namespace std; -SWSSZmqClient SWSSZmqClient_new(const char *endpoint) { - SWSSTry(return (SWSSZmqClient) new ZmqClient(endpoint)); +SWSSResult SWSSZmqClient_new(const char *endpoint, SWSSZmqClient *outClient) { + SWSSTry(*outClient = (SWSSZmqClient) new ZmqClient(endpoint)); } -void SWSSZmqClient_free(SWSSZmqClient zmqc) { +SWSSResult 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); +SWSSResult SWSSZmqClient_isConnected(SWSSZmqClient zmqc, int8_t *outIsConnected) { + SWSSTry(*outIsConnected = ((ZmqClient *)zmqc)->isConnected() ? 1 : 0); } -void SWSSZmqClient_connect(SWSSZmqClient zmqc) { +SWSSResult SWSSZmqClient_connect(SWSSZmqClient zmqc) { SWSSTry(((ZmqClient *)zmqc)->connect()); } -void SWSSZmqClient_sendMsg(SWSSZmqClient zmqc, const char *dbName, const char *tableName, - SWSSKeyOpFieldValuesArray arr) { +SWSSResult SWSSZmqClient_sendMsg(SWSSZmqClient zmqc, const char *dbName, const char *tableName, + SWSSKeyOpFieldValuesArray arr) { SWSSTry({ vector kcos = takeKeyOpFieldValuesArray(arr); ((ZmqClient *)zmqc) diff --git a/common/c-api/zmqclient.h b/common/c-api/zmqclient.h index da832ab30..d846f598d 100644 --- a/common/c-api/zmqclient.h +++ b/common/c-api/zmqclient.h @@ -1,6 +1,7 @@ #ifndef SWSS_COMMON_C_API_ZMQCLIENT_H #define SWSS_COMMON_C_API_ZMQCLIENT_H +#include "result.h" #include "util.h" #ifdef __cplusplus @@ -11,17 +12,17 @@ extern "C" { typedef struct SWSSZmqClientOpaque *SWSSZmqClient; -SWSSZmqClient SWSSZmqClient_new(const char *endpoint); +SWSSResult SWSSZmqClient_new(const char *endpoint, SWSSZmqClient *outZmqc); -void SWSSZmqClient_free(SWSSZmqClient zmqc); +SWSSResult SWSSZmqClient_free(SWSSZmqClient zmqc); -// Returns 0 for false, 1 for true -int8_t SWSSZmqClient_isConnected(SWSSZmqClient zmqc); +// Outputs 0 for false, 1 for true +SWSSResult SWSSZmqClient_isConnected(SWSSZmqClient zmqc, int8_t *outIsConnected); -void SWSSZmqClient_connect(SWSSZmqClient zmqc); +SWSSResult SWSSZmqClient_connect(SWSSZmqClient zmqc); -void SWSSZmqClient_sendMsg(SWSSZmqClient zmqc, const char *dbName, const char *tableName, - SWSSKeyOpFieldValuesArray kcos); +SWSSResult SWSSZmqClient_sendMsg(SWSSZmqClient zmqc, const char *dbName, const char *tableName, + SWSSKeyOpFieldValuesArray kcos); #ifdef __cplusplus } diff --git a/common/c-api/zmqconsumerstatetable.cpp b/common/c-api/zmqconsumerstatetable.cpp index ed416488e..59ed7dde4 100644 --- a/common/c-api/zmqconsumerstatetable.cpp +++ b/common/c-api/zmqconsumerstatetable.cpp @@ -1,4 +1,3 @@ -#include #include "../zmqconsumerstatetable.h" #include "../table.h" #include "util.h" @@ -11,41 +10,43 @@ 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, - 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)); +SWSSResult SWSSZmqConsumerStateTable_new(SWSSDBConnector db, const char *tableName, + SWSSZmqServer zmqs, const int32_t *p_popBatchSize, + const int32_t *p_pri, SWSSZmqConsumerStateTable *outTbl) { + SWSSTry({ + int popBatchSize = p_popBatchSize ? numeric_cast(*p_popBatchSize) + : TableConsumable::DEFAULT_POP_BATCH_SIZE; + int pri = p_pri ? numeric_cast(*p_pri) : 0; + *outTbl = (SWSSZmqConsumerStateTable) new ZmqConsumerStateTable( + (DBConnector *)db, string(tableName), *(ZmqServer *)zmqs, popBatchSize, pri); + }); } -void SWSSZmqConsumerStateTable_free(SWSSZmqConsumerStateTable tbl) { +SWSSResult SWSSZmqConsumerStateTable_free(SWSSZmqConsumerStateTable tbl) { SWSSTry(delete (ZmqConsumerStateTable *)tbl); } -SWSSKeyOpFieldValuesArray SWSSZmqConsumerStateTable_pops(SWSSZmqConsumerStateTable tbl) { +SWSSResult SWSSZmqConsumerStateTable_pops(SWSSZmqConsumerStateTable tbl, + SWSSKeyOpFieldValuesArray *outArr) { SWSSTry({ deque vkco; ((ZmqConsumerStateTable *)tbl)->pops(vkco); - return makeKeyOpFieldValuesArray(vkco); + *outArr = makeKeyOpFieldValuesArray(vkco); }); } -uint32_t SWSSZmqConsumerStateTable_getFd(SWSSZmqConsumerStateTable tbl) { - SWSSTry(return numeric_cast(((ZmqConsumerStateTable *)tbl)->getFd())); +SWSSResult SWSSZmqConsumerStateTable_getFd(SWSSZmqConsumerStateTable tbl, uint32_t *outFd) { + SWSSTry(*outFd = numeric_cast(((ZmqConsumerStateTable *)tbl)->getFd())); } -SWSSSelectResult SWSSZmqConsumerStateTable_readData(SWSSZmqConsumerStateTable tbl, - uint32_t timeout_ms, - uint8_t interrupt_on_signal) { - SWSSTry(return selectOne((ZmqConsumerStateTable *)tbl, timeout_ms, interrupt_on_signal)); +SWSSResult SWSSZmqConsumerStateTable_readData(SWSSZmqConsumerStateTable tbl, uint32_t timeout_ms, + uint8_t interrupt_on_signal, + SWSSSelectResult *outResult) { + SWSSTry(*outResult = selectOne((ZmqConsumerStateTable *)tbl, timeout_ms, interrupt_on_signal)); } -const struct SWSSDBConnectorOpaque * -SWSSZmqConsumerStateTable_getDbConnector(SWSSZmqConsumerStateTable tbl) { - SWSSTry(return (const SWSSDBConnectorOpaque *)((ZmqConsumerStateTable *)tbl)->getDbConnector()); +SWSSResult SWSSZmqConsumerStateTable_getDbConnector(SWSSZmqConsumerStateTable tbl, + const struct SWSSDBConnectorOpaque **outDb) { + SWSSTry(*outDb = + (const SWSSDBConnectorOpaque *)((ZmqConsumerStateTable *)tbl)->getDbConnector()); } diff --git a/common/c-api/zmqconsumerstatetable.h b/common/c-api/zmqconsumerstatetable.h index f5b934258..f781f6bc8 100644 --- a/common/c-api/zmqconsumerstatetable.h +++ b/common/c-api/zmqconsumerstatetable.h @@ -2,6 +2,7 @@ #define SWSS_COMMON_C_API_ZMQCONSUMERSTATETABLE_H #include "dbconnector.h" +#include "result.h" #include "util.h" #include "zmqserver.h" @@ -13,31 +14,32 @@ extern "C" { 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); +SWSSResult SWSSZmqConsumerStateTable_new(SWSSDBConnector db, const char *tableName, + SWSSZmqServer zmqs, const int32_t *popBatchSize, + const int32_t *pri, SWSSZmqConsumerStateTable *outTbl); -void SWSSZmqConsumerStateTable_free(SWSSZmqConsumerStateTable tbl); +// Outputs NULL for popBatchSize and/or pri to use the default values +SWSSResult SWSSZmqConsumerStateTable_free(SWSSZmqConsumerStateTable tbl); // Result array and all of its members must be freed using free() -SWSSKeyOpFieldValuesArray SWSSZmqConsumerStateTable_pops(SWSSZmqConsumerStateTable tbl); +SWSSResult SWSSZmqConsumerStateTable_pops(SWSSZmqConsumerStateTable tbl, + SWSSKeyOpFieldValuesArray *outArr); -// Return the underlying fd for polling/selecting on. +// Outputs 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); +SWSSResult SWSSZmqConsumerStateTable_getFd(SWSSZmqConsumerStateTable tbl, uint32_t *outFd); // 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, - uint8_t interrupt_on_signal); +SWSSResult SWSSZmqConsumerStateTable_readData(SWSSZmqConsumerStateTable tbl, uint32_t timeout_ms, + uint8_t interrupt_on_signal, + SWSSSelectResult *outResult); -const struct SWSSDBConnectorOpaque * -SWSSZmqConsumerStateTable_getDbConnector(SWSSZmqConsumerStateTable tbl); +SWSSResult +SWSSZmqConsumerStateTable_getDbConnector(SWSSZmqConsumerStateTable tbl, + const struct SWSSDBConnectorOpaque **outDbConnector); #ifdef __cplusplus } diff --git a/common/c-api/zmqproducerstatetable.cpp b/common/c-api/zmqproducerstatetable.cpp index e1c186806..b9192ee39 100644 --- a/common/c-api/zmqproducerstatetable.cpp +++ b/common/c-api/zmqproducerstatetable.cpp @@ -1,32 +1,34 @@ #include -#include "zmqproducerstatetable.h" #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) { - - SWSSTry(return (SWSSZmqProducerStateTable) new ZmqProducerStateTable( - (DBConnector *)db, string(tableName), *(ZmqClient *)zmqc, dbPersistence)); +SWSSResult SWSSZmqProducerStateTable_new(SWSSDBConnector db, const char *tableName, + SWSSZmqClient zmqc, uint8_t dbPersistence, + SWSSZmqProducerStateTable *outTbl) { + SWSSTry(*outTbl = (SWSSZmqProducerStateTable) new ZmqProducerStateTable( + (DBConnector *)db, string(tableName), *(ZmqClient *)zmqc, dbPersistence)); } -void SWSSZmqProducerStateTable_free(SWSSZmqProducerStateTable tbl) { +SWSSResult SWSSZmqProducerStateTable_free(SWSSZmqProducerStateTable tbl) { SWSSTry(delete (ZmqProducerStateTable *)tbl); } -void SWSSZmqProducerStateTable_set(SWSSZmqProducerStateTable tbl, const char *key, - SWSSFieldValueArray values) { +SWSSResult 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) { +SWSSResult 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())); +SWSSResult SWSSZmqProducerStateTable_dbUpdaterQueueSize(SWSSZmqProducerStateTable tbl, + uint64_t *outSize) { + SWSSTry(*outSize = + numeric_cast(((ZmqProducerStateTable *)tbl)->dbUpdaterQueueSize())); } diff --git a/common/c-api/zmqproducerstatetable.h b/common/c-api/zmqproducerstatetable.h index 08d059186..f308fbb02 100644 --- a/common/c-api/zmqproducerstatetable.h +++ b/common/c-api/zmqproducerstatetable.h @@ -2,6 +2,7 @@ #define SWSS_COMMON_C_API_ZMQPRODUCERSTATETABLE_H #include "dbconnector.h" +#include "result.h" #include "util.h" #include "zmqclient.h" @@ -13,17 +14,18 @@ extern "C" { typedef struct SWSSZmqProducerStateTableOpaque *SWSSZmqProducerStateTable; -SWSSZmqProducerStateTable SWSSZmqProducerStateTable_new(SWSSDBConnector db, const char *tableName, - SWSSZmqClient zmqc, uint8_t dbPersistence); +SWSSResult SWSSZmqProducerStateTable_new(SWSSDBConnector db, const char *tableName, + SWSSZmqClient zmqc, uint8_t dbPersistence, + SWSSZmqProducerStateTable *outTbl); -void SWSSZmqProducerStateTable_free(SWSSZmqProducerStateTable tbl); +SWSSResult SWSSZmqProducerStateTable_free(SWSSZmqProducerStateTable tbl); -void SWSSZmqProducerStateTable_set(SWSSZmqProducerStateTable tbl, const char *key, - SWSSFieldValueArray values); +SWSSResult SWSSZmqProducerStateTable_set(SWSSZmqProducerStateTable tbl, const char *key, + SWSSFieldValueArray values); -void SWSSZmqProducerStateTable_del(SWSSZmqProducerStateTable tbl, const char *key); +SWSSResult SWSSZmqProducerStateTable_del(SWSSZmqProducerStateTable tbl, const char *key); -uint64_t SWSSZmqProducerStateTable_dbUpdaterQueueSize(SWSSZmqProducerStateTable tbl); +SWSSResult SWSSZmqProducerStateTable_dbUpdaterQueueSize(SWSSZmqProducerStateTable tbl, uint64_t *outSize); #ifdef __cplusplus } diff --git a/common/c-api/zmqserver.cpp b/common/c-api/zmqserver.cpp index 50452e22d..b2937ae38 100644 --- a/common/c-api/zmqserver.cpp +++ b/common/c-api/zmqserver.cpp @@ -5,10 +5,10 @@ using namespace swss; using namespace std; -SWSSZmqServer SWSSZmqServer_new(const char *endpoint) { - SWSSTry(return (SWSSZmqServer) new ZmqServer(string(endpoint))); +SWSSResult SWSSZmqServer_new(const char *endpoint, SWSSZmqServer *outZmqServer) { + SWSSTry(*outZmqServer = (SWSSZmqServer) new ZmqServer(string(endpoint))); } -void SWSSZmqServer_free(SWSSZmqServer zmqs) { +SWSSResult SWSSZmqServer_free(SWSSZmqServer zmqs) { SWSSTry(delete (ZmqServer *)zmqs); } diff --git a/common/c-api/zmqserver.h b/common/c-api/zmqserver.h index decd0e0dc..ee76af420 100644 --- a/common/c-api/zmqserver.h +++ b/common/c-api/zmqserver.h @@ -1,6 +1,7 @@ #ifndef SWSS_COMMON_C_API_ZMQSERVER_H #define SWSS_COMMON_C_API_ZMQSERVER_H +#include "result.h" #include "util.h" #ifdef __cplusplus @@ -9,9 +10,9 @@ extern "C" { typedef struct SWSSZmqServerOpaque *SWSSZmqServer; -SWSSZmqServer SWSSZmqServer_new(const char *endpoint); +SWSSResult SWSSZmqServer_new(const char *endpoint, SWSSZmqServer *outZmqServer); -void SWSSZmqServer_free(SWSSZmqServer zmqs); +SWSSResult SWSSZmqServer_free(SWSSZmqServer zmqs); #ifdef __cplusplus } diff --git a/tests/c_api_ut.cpp b/tests/c_api_ut.cpp index 90fffdaa2..3bee372c1 100644 --- a/tests/c_api_ut.cpp +++ b/tests/c_api_ut.cpp @@ -1,9 +1,12 @@ +#include +#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/result.h" #include "common/c-api/subscriberstatetable.h" #include "common/c-api/table.h" #include "common/c-api/util.h" @@ -59,9 +62,17 @@ static void freeKeyOpFieldValuesArray(SWSSKeyOpFieldValuesArray arr) { struct SWSSStringManager { vector m_strings; + bool use_c_str = false; SWSSString makeString(const char *c_str) { - SWSSString s = SWSSString_new_c_str(c_str); + use_c_str = !use_c_str; + + SWSSString s; + if (use_c_str) { + s = SWSSString_new_c_str(c_str); + } else { + s = SWSSString_new(c_str, strlen(c_str)); + } m_strings.push_back(s); return s; } @@ -80,33 +91,61 @@ 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_EQ(SWSSDBConnector_get(db, "mykey"), nullptr); - EXPECT_FALSE(SWSSDBConnector_exists(db, "mykey")); + SWSSDBConnector db; + SWSSDBConnector_new_named("TEST_DB", 1000, true, &db); + + SWSSString val; + SWSSDBConnector_get(db, "mykey", &val); + EXPECT_EQ(val, nullptr); + + int8_t exists; + SWSSDBConnector_exists(db, "mykey", &exists); + EXPECT_FALSE(exists); SWSSDBConnector_set(db, "mykey", sm.makeStrRef("myval")); - SWSSString val = SWSSDBConnector_get(db, "mykey"); + SWSSDBConnector_get(db, "mykey", &val); 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_exists(db, "mykey", &exists); + EXPECT_TRUE(exists); + + int8_t status; + SWSSDBConnector_del(db, "mykey", &status); + EXPECT_TRUE(status); + + SWSSDBConnector_del(db, "mykey", &status); + EXPECT_FALSE(status); + + SWSSDBConnector_hget(db, "mykey", "myfield", &val); + EXPECT_EQ(val, nullptr); + + SWSSDBConnector_hexists(db, "mykey", "myfield", &exists); + EXPECT_FALSE(exists); + SWSSDBConnector_hset(db, "mykey", "myfield", sm.makeStrRef("myval")); - val = SWSSDBConnector_hget(db, "mykey", "myfield"); + SWSSDBConnector_hget(db, "mykey", "myfield", &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")); - EXPECT_TRUE(SWSSDBConnector_hdel(db, "mykey", "myfield")); - EXPECT_FALSE(SWSSDBConnector_hdel(db, "mykey", "myfield")); + SWSSDBConnector_hexists(db, "mykey", "myfield", &exists); + EXPECT_TRUE(exists); + + SWSSDBConnector_hget(db, "mykey", "notmyfield", &val); + EXPECT_EQ(val, nullptr); + + SWSSDBConnector_hexists(db, "mykey", "notmyfield", &exists); + EXPECT_FALSE(exists); + + SWSSDBConnector_hdel(db, "mykey", "myfield", &status); + EXPECT_TRUE(status); + + SWSSDBConnector_hdel(db, "mykey", "myfield", &status); + EXPECT_FALSE(status); + + SWSSDBConnector_flushdb(db, &status); + EXPECT_TRUE(status); - EXPECT_TRUE(SWSSDBConnector_flushdb(db)); SWSSDBConnector_free(db); } @@ -114,31 +153,39 @@ TEST(c_api, Table) { clearDB(); SWSSStringManager sm; - SWSSDBConnector db = SWSSDBConnector_new_named("TEST_DB", 1000, true); - SWSSTable tbl = SWSSTable_new(db, "mytable"); + SWSSDBConnector db; + SWSSDBConnector_new_named("TEST_DB", 1000, true, &db); + SWSSTable tbl; + SWSSTable_new(db, "mytable", &tbl); SWSSFieldValueArray fvs; SWSSString ss; - EXPECT_FALSE(SWSSTable_get(tbl, "mykey", &fvs)); - EXPECT_FALSE(SWSSTable_hget(tbl, "mykey", "myfield", &ss)); - - SWSSStringArray keys = SWSSTable_getKeys(tbl); + int8_t exists; + SWSSTable_get(tbl, "mykey", &fvs, &exists); + EXPECT_FALSE(exists); + SWSSTable_hget(tbl, "mykey", "myfield", &ss, &exists); + EXPECT_FALSE(exists); + + SWSSStringArray keys; + SWSSTable_getKeys(tbl, &keys); EXPECT_EQ(keys.len, 0); SWSSStringArray_free(keys); SWSSTable_hset(tbl, "mykey", "myfield", sm.makeStrRef("myvalue")); - keys = SWSSTable_getKeys(tbl); + SWSSTable_getKeys(tbl, &keys); ASSERT_EQ(keys.len, 1); EXPECT_STREQ(keys.data[0], "mykey"); free(keys.data[0]); SWSSStringArray_free(keys); - ASSERT_TRUE(SWSSTable_hget(tbl, "mykey", "myfield", &ss)); + SWSSTable_hget(tbl, "mykey", "myfield", &ss, &exists); + ASSERT_TRUE(exists); EXPECT_STREQ(SWSSStrRef_c_str((SWSSStrRef)ss), "myvalue"); SWSSString_free(ss); SWSSTable_hdel(tbl, "mykey", "myfield"); - EXPECT_FALSE(SWSSTable_hget(tbl, "mykey", "myfield", &ss)); + SWSSTable_hget(tbl, "mykey", "myfield", &ss, &exists); + EXPECT_FALSE(exists); SWSSFieldValueTuple data[2] = {{.field = "myfield1", .value = sm.makeString("myvalue1")}, {.field = "myfield2", .value = sm.makeString("myvalue2")}}; @@ -146,14 +193,15 @@ TEST(c_api, Table) { fvs.data = data; SWSSTable_set(tbl, "mykey", fvs); - ASSERT_TRUE(SWSSTable_get(tbl, "mykey", &fvs)); - EXPECT_EQ(fvs.len, 2); + SWSSTable_get(tbl, "mykey", &fvs, &exists); + ASSERT_TRUE(exists); + ASSERT_EQ(fvs.len, 2); EXPECT_STREQ(data[0].field, fvs.data[0].field); EXPECT_STREQ(data[1].field, fvs.data[1].field); freeFieldValuesArray(fvs); SWSSTable_del(tbl, "mykey"); - keys = SWSSTable_getKeys(tbl); + SWSSTable_getKeys(tbl, &keys); EXPECT_EQ(keys.len, 0); SWSSStringArray_free(keys); @@ -165,13 +213,18 @@ TEST(c_api, ConsumerProducerStateTables) { clearDB(); SWSSStringManager sm; - SWSSDBConnector db = SWSSDBConnector_new_named("TEST_DB", 1000, true); - SWSSProducerStateTable pst = SWSSProducerStateTable_new(db, "mytable"); - SWSSConsumerStateTable cst = SWSSConsumerStateTable_new(db, "mytable", nullptr, nullptr); + SWSSDBConnector db; + SWSSDBConnector_new_named("TEST_DB", 1000, true, &db); + SWSSProducerStateTable pst; + SWSSProducerStateTable_new(db, "mytable", &pst); + SWSSConsumerStateTable cst; + SWSSConsumerStateTable_new(db, "mytable", nullptr, nullptr, &cst); - SWSSConsumerStateTable_getFd(cst); + uint32_t fd; + SWSSConsumerStateTable_getFd(cst, &fd); - SWSSKeyOpFieldValuesArray arr = SWSSConsumerStateTable_pops(cst); + SWSSKeyOpFieldValuesArray arr; + SWSSConsumerStateTable_pops(cst, &arr); ASSERT_EQ(arr.len, 0); freeKeyOpFieldValuesArray(arr); @@ -187,8 +240,9 @@ 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); + SWSSSelectResult result; + SWSSConsumerStateTable_readData(cst, 300, true, &result); + SWSSConsumerStateTable_pops(cst, &arr); vector kfvs = takeKeyOpFieldValuesArray(arr); sortKfvs(kfvs); freeKeyOpFieldValuesArray(arr); @@ -210,7 +264,7 @@ TEST(c_api, ConsumerProducerStateTables) { EXPECT_EQ(fieldValues1[0].first, "myfield3"); EXPECT_EQ(fieldValues1[0].second, "myvalue3"); - arr = SWSSConsumerStateTable_pops(cst); + SWSSConsumerStateTable_pops(cst, &arr); EXPECT_EQ(arr.len, 0); freeKeyOpFieldValuesArray(arr); @@ -218,7 +272,7 @@ TEST(c_api, ConsumerProducerStateTables) { SWSSProducerStateTable_del(pst, "mykey4"); SWSSProducerStateTable_del(pst, "mykey5"); - arr = SWSSConsumerStateTable_pops(cst); + SWSSConsumerStateTable_pops(cst, &arr); kfvs = takeKeyOpFieldValuesArray(arr); sortKfvs(kfvs); freeKeyOpFieldValuesArray(arr); @@ -236,7 +290,8 @@ TEST(c_api, ConsumerProducerStateTables) { SWSSProducerStateTable_free(pst); SWSSConsumerStateTable_free(cst); - SWSSDBConnector_flushdb(db); + int8_t flushStatus; + SWSSDBConnector_flushdb(db, &flushStatus); SWSSDBConnector_free(db); } @@ -244,19 +299,26 @@ TEST(c_api, SubscriberStateTable) { clearDB(); SWSSStringManager sm; - SWSSDBConnector db = SWSSDBConnector_new_named("TEST_DB", 1000, true); - SWSSSubscriberStateTable sst = SWSSSubscriberStateTable_new(db, "mytable", nullptr, nullptr); + SWSSDBConnector db; + SWSSDBConnector_new_named("TEST_DB", 1000, true, &db); + SWSSSubscriberStateTable sst; + SWSSSubscriberStateTable_new(db, "mytable", nullptr, nullptr, &sst); - SWSSSubscriberStateTable_getFd(sst); + uint32_t fd; + SWSSSubscriberStateTable_getFd(sst, &fd); - EXPECT_EQ(SWSSSubscriberStateTable_readData(sst, 300, true), SWSSSelectResult_TIMEOUT); - SWSSKeyOpFieldValuesArray arr = SWSSSubscriberStateTable_pops(sst); + SWSSSelectResult result; + SWSSSubscriberStateTable_readData(sst, 300, true, &result); + EXPECT_EQ(result, SWSSSelectResult_TIMEOUT); + SWSSKeyOpFieldValuesArray arr; + SWSSSubscriberStateTable_pops(sst, &arr); EXPECT_EQ(arr.len, 0); freeKeyOpFieldValuesArray(arr); SWSSDBConnector_hset(db, "mytable:mykey", "myfield", sm.makeStrRef("myvalue")); - EXPECT_EQ(SWSSSubscriberStateTable_readData(sst, 300, true), SWSSSelectResult_DATA); - arr = SWSSSubscriberStateTable_pops(sst); + SWSSSubscriberStateTable_readData(sst, 300, true, &result); + EXPECT_EQ(result, SWSSSelectResult_DATA); + SWSSSubscriberStateTable_pops(sst, &arr); vector kfvs = takeKeyOpFieldValuesArray(arr); sortKfvs(kfvs); freeKeyOpFieldValuesArray(arr); @@ -269,7 +331,8 @@ TEST(c_api, SubscriberStateTable) { EXPECT_EQ(kfvFieldsValues(kfvs[0])[0].second, "myvalue"); SWSSSubscriberStateTable_free(sst); - SWSSDBConnector_flushdb(db); + int8_t flushStatus; + SWSSDBConnector_flushdb(db, &flushStatus); SWSSDBConnector_free(db); } @@ -277,27 +340,35 @@ TEST(c_api, ZmqConsumerProducerStateTable) { clearDB(); SWSSStringManager sm; - SWSSDBConnector db = SWSSDBConnector_new_named("TEST_DB", 1000, true); + SWSSDBConnector db; + SWSSDBConnector_new_named("TEST_DB", 1000, true, &db); - 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 + SWSSZmqServer srv; + SWSSZmqServer_new("tcp://127.0.0.1:42312", &srv); + SWSSZmqClient cli; + SWSSZmqClient_new("tcp://127.0.0.1:42312", &cli); + int8_t isConnected; + SWSSZmqClient_isConnected(cli, &isConnected); + ASSERT_TRUE(isConnected); + SWSSZmqClient_connect(cli); - SWSSZmqProducerStateTable pst = SWSSZmqProducerStateTable_new(db, "mytable", cli, false); - SWSSZmqConsumerStateTable cst = - SWSSZmqConsumerStateTable_new(db, "mytable", srv, nullptr, nullptr); + SWSSZmqProducerStateTable pst; + SWSSZmqProducerStateTable_new(db, "mytable", cli, false, &pst); + SWSSZmqConsumerStateTable cst; + SWSSZmqConsumerStateTable_new(db, "mytable", srv, nullptr, nullptr, &cst); - SWSSZmqConsumerStateTable_getFd(cst); + uint32_t fd; + SWSSZmqConsumerStateTable_getFd(cst, &fd); - ASSERT_EQ(SWSSZmqConsumerStateTable_getDbConnector(cst), db); + const SWSSDBConnectorOpaque *dbConnector; + SWSSZmqConsumerStateTable_getDbConnector(cst, &dbConnector); + ASSERT_EQ(dbConnector, db); - SWSSKeyOpFieldValuesArray arr = SWSSZmqConsumerStateTable_pops(cst); + SWSSKeyOpFieldValuesArray arr; + SWSSZmqConsumerStateTable_pops(cst, &arr); 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++) { SWSSFieldValueTuple values_key1_data[2] = { {.field = "myfield1", .value = sm.makeString("myvalue1")}, @@ -325,8 +396,9 @@ TEST(c_api, ZmqConsumerProducerStateTable) { else SWSSZmqClient_sendMsg(cli, "TEST_DB", "mytable", arr); - ASSERT_EQ(SWSSZmqConsumerStateTable_readData(cst, 1500, true), SWSSSelectResult_DATA); - arr = SWSSZmqConsumerStateTable_pops(cst); + SWSSSelectResult result; + SWSSZmqConsumerStateTable_readData(cst, 1500, true, &result); + SWSSZmqConsumerStateTable_pops(cst, &arr); vector kfvs = takeKeyOpFieldValuesArray(arr); sortKfvs(kfvs); @@ -349,7 +421,7 @@ TEST(c_api, ZmqConsumerProducerStateTable) { EXPECT_EQ(fieldValues1[0].first, "myfield3"); EXPECT_EQ(fieldValues1[0].second, "myvalue3"); - arr = SWSSZmqConsumerStateTable_pops(cst); + SWSSZmqConsumerStateTable_pops(cst, &arr); ASSERT_EQ(arr.len, 0); freeKeyOpFieldValuesArray(arr); @@ -363,8 +435,8 @@ TEST(c_api, ZmqConsumerProducerStateTable) { else SWSSZmqClient_sendMsg(cli, "TEST_DB", "mytable", arr); - ASSERT_EQ(SWSSZmqConsumerStateTable_readData(cst, 500, true), SWSSSelectResult_DATA); - arr = SWSSZmqConsumerStateTable_pops(cst); + SWSSZmqConsumerStateTable_readData(cst, 500, true, &result); + SWSSZmqConsumerStateTable_pops(cst, &arr); kfvs = takeKeyOpFieldValuesArray(arr); sortKfvs(kfvs); @@ -379,7 +451,6 @@ TEST(c_api, ZmqConsumerProducerStateTable) { EXPECT_EQ(kfvFieldsValues(kfvs[1]).size(), 0); } - // Server must be freed first to safely release message handlers (ZmqConsumerStateTable) SWSSZmqServer_free(srv); SWSSZmqProducerStateTable_free(pst); @@ -387,6 +458,26 @@ TEST(c_api, ZmqConsumerProducerStateTable) { SWSSZmqClient_free(cli); - SWSSDBConnector_flushdb(db); + int8_t flushStatus; + SWSSDBConnector_flushdb(db, &flushStatus); SWSSDBConnector_free(db); } + +TEST(c_api, exceptions) { + SWSSDBConnector db = nullptr; + SWSSResult result = SWSSDBConnector_new_tcp(0, "127.0.0.1", 1, 1000, &db); + ASSERT_EQ(db, nullptr); + ASSERT_NE(result.exception, SWSSException_None); + ASSERT_NE(result.location, nullptr); + ASSERT_NE(result.message, nullptr); + + const char *cstr = SWSSStrRef_c_str((SWSSStrRef)result.location); + EXPECT_EQ(SWSSStrRef_length((SWSSStrRef)result.location), strlen(cstr)); + printf("Exception location: %s\n", cstr); + cstr = SWSSStrRef_c_str((SWSSStrRef)result.message); + EXPECT_EQ(SWSSStrRef_length((SWSSStrRef)result.message), strlen(cstr)); + printf("Exception message: %s\n", cstr); + + SWSSString_free(result.location); + SWSSString_free(result.message); +} diff --git a/tests/main.cpp b/tests/main.cpp index 440978a46..bfdd9fcdb 100644 --- a/tests/main.cpp +++ b/tests/main.cpp @@ -85,9 +85,6 @@ class SwsscommonEnvironment : public ::testing::Environment { SonicDBConfig::initializeGlobalConfig(global_existing_file); cout<<"INIT: load global db config file, isInit = "< Date: Mon, 27 Jan 2025 17:30:20 -0500 Subject: [PATCH 27/29] Add new software bfd state db table in schema (#957) According this HDL, add a new definition for software BFD table in state DB in schema header file: https://github.com/kperumalbfn/SONiC/blob/kperumal/bfd/doc/smart-switch/BFD/SmartSwitchDpuLivenessUsingBfd.md --- common/schema.h | 1 + 1 file changed, 1 insertion(+) diff --git a/common/schema.h b/common/schema.h index ade98f9f5..8c9fb6674 100644 --- a/common/schema.h +++ b/common/schema.h @@ -557,6 +557,7 @@ namespace swss { #define STATE_TUNNEL_DECAP_TERM_TABLE_NAME "TUNNEL_DECAP_TERM_TABLE" #define STATE_BFD_SESSION_TABLE_NAME "BFD_SESSION_TABLE" +#define STATE_BFD_SOFTWARE_SESSION_TABLE_NAME "BFD_SOFTWARE_SESSION_TABLE" #define STATE_ROUTE_TABLE_NAME "ROUTE_TABLE" #define STATE_VNET_RT_TUNNEL_TABLE_NAME "VNET_ROUTE_TUNNEL_TABLE" #define STATE_ADVERTISE_NETWORK_TABLE_NAME "ADVERTISE_NETWORK_TABLE" From 505381e314872edca11ffb6a864edf29f9cb3fb4 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak <38952541+stepanblyschak@users.noreply.github.com> Date: Mon, 3 Feb 2025 15:45:47 +0200 Subject: [PATCH 28/29] Handle 'bulkget' in consumer_table_pops.lua (#966) To support SAIRedis bulkget operations in async mode. Signed-off-by: Stepan Blyschak --- common/consumer_table_pops.lua | 1 + 1 file changed, 1 insertion(+) diff --git a/common/consumer_table_pops.lua b/common/consumer_table_pops.lua index 959bcd0fa..91e22a6f7 100644 --- a/common/consumer_table_pops.lua +++ b/common/consumer_table_pops.lua @@ -79,6 +79,7 @@ for i = n, 1, -3 do op == 'flush' or op == 'flushresponse' or op == 'get' or + op == 'bulkget' or op == 'getresponse' or op == 'notify' or op == 'get_stats' or From 1593cc64305c3df68f504c1555b78f5e1a483e62 Mon Sep 17 00:00:00 2001 From: divyagayathri-hcl <159437886+divyagayathri-hcl@users.noreply.github.com> Date: Wed, 5 Feb 2025 01:41:54 +0530 Subject: [PATCH 29/29] ZMQ lib change. (#958) * zmq wait * ut pass case * swig_verify * Revert "swig_verify" * ... * zmqwait * const * ... * constant * Without debugs --- common/zmqclient.cpp | 18 +++++- common/zmqclient.h | 13 +++- common/zmqproducerstatetable.cpp | 29 +++++---- common/zmqproducerstatetable.h | 5 ++ common/zmqserver.cpp | 68 +++++++++++--------- common/zmqserver.h | 10 +++ tests/c_api_ut.cpp | 2 + tests/zmq_state_ut.cpp | 104 +++++++++++++++++++++++++++++++ 8 files changed, 204 insertions(+), 45 deletions(-) diff --git a/common/zmqclient.cpp b/common/zmqclient.cpp index 5a84160e9..45f118990 100644 --- a/common/zmqclient.cpp +++ b/common/zmqclient.cpp @@ -25,6 +25,12 @@ ZmqClient::ZmqClient(const std::string& endpoint, const std::string& vrf) initialize(endpoint, vrf); } +ZmqClient::ZmqClient(const std::string& endpoint, uint32_t waitTimeMs) +: m_waitTimeMs(waitTimeMs) +{ + initialize(endpoint); +} + ZmqClient::~ZmqClient() { std::lock_guard lock(m_socketMutex); @@ -55,7 +61,7 @@ void ZmqClient::initialize(const std::string& endpoint, const std::string& vrf) connect(); } - + bool ZmqClient::isConnected() { return m_connected; @@ -137,7 +143,7 @@ void ZmqClient::sendMsg( int zmq_err = 0; int retry_delay = 10; int rc = 0; - for (int i = 0; i <= MQ_MAX_RETRY; ++i) + for (int i = 0; i <= MQ_MAX_RETRY; ++i) { { // ZMQ socket is not thread safe: http://api.zeromq.org/2-1:zmq @@ -146,7 +152,6 @@ void ZmqClient::sendMsg( // Use none block mode to use all bandwidth: http://api.zeromq.org/2-1%3Azmq-send rc = zmq_send(m_socket, m_sendbuffer.data(), serializedlen, ZMQ_NOBLOCK); } - if (rc >= 0) { SWSS_LOG_DEBUG("zmq sended %d bytes", serializedlen); @@ -197,4 +202,11 @@ void ZmqClient::sendMsg( throw system_error(make_error_code(errc::io_error), message); } +// TODO: To be implemented later, required for ZMQ_CLIENT & ZMQ_SERVER +// socket types in response path. +bool ZmqClient::wait( + const std::string &dbName, const std::string &tableName, + const std::vector> &kcos) { + return false; +} } diff --git a/common/zmqclient.h b/common/zmqclient.h index adc36b053..fdfe9e343 100644 --- a/common/zmqclient.h +++ b/common/zmqclient.h @@ -12,8 +12,10 @@ namespace swss { class ZmqClient { public: + ZmqClient(const std::string& endpoint); ZmqClient(const std::string& endpoint, const std::string& vrf); + ZmqClient(const std::string& endpoint, uint32_t waitTimeMs); ~ZmqClient(); bool isConnected(); @@ -23,8 +25,13 @@ class ZmqClient void sendMsg(const std::string& dbName, const std::string& tableName, const std::vector& kcos); + + bool wait(const std::string& dbName, + const std::string& tableName, + const std::vector>& kcos); + private: - void initialize(const std::string& endpoint, const std::string& vrf); + void initialize(const std::string& endpoint, const std::string& vrf = ""); std::string m_endpoint; @@ -36,8 +43,10 @@ class ZmqClient bool m_connected; + uint32_t m_waitTimeMs; + std::mutex m_socketMutex; - + std::vector m_sendbuffer; }; diff --git a/common/zmqproducerstatetable.cpp b/common/zmqproducerstatetable.cpp index e2a31446b..6260f4767 100644 --- a/common/zmqproducerstatetable.cpp +++ b/common/zmqproducerstatetable.cpp @@ -1,18 +1,18 @@ -#include -#include -#include -#include +#include "zmqproducerstatetable.h" +#include "binaryserializer.h" +#include "redisapi.h" +#include "redispipeline.h" +#include "redisreply.h" +#include "table.h" +#include "zmqconsumerstatetable.h" #include #include #include +#include +#include +#include +#include #include -#include "redisreply.h" -#include "table.h" -#include "redisapi.h" -#include "redispipeline.h" -#include "zmqproducerstatetable.h" -#include "zmqconsumerstatetable.h" -#include "binaryserializer.h" using namespace std; @@ -164,6 +164,13 @@ void ZmqProducerStateTable::send(const std::vector &kcos } } +bool ZmqProducerStateTable::wait(const std::string& dbName, + const std::string& tableName, + const std::vector>& kcos) +{ + return m_zmqClient.wait(dbName, tableName, kcos); +} + size_t ZmqProducerStateTable::dbUpdaterQueueSize() { if (m_asyncDBUpdater == nullptr) diff --git a/common/zmqproducerstatetable.h b/common/zmqproducerstatetable.h index 015419bd2..09778d47a 100644 --- a/common/zmqproducerstatetable.h +++ b/common/zmqproducerstatetable.h @@ -37,6 +37,11 @@ class ZmqProducerStateTable : public ProducerStateTable // Batched send that can include both SET and DEL requests. virtual void send(const std::vector &kcos); + // To wait for the response from the peer. + virtual bool wait(const std::string& dbName, + const std::string& tableName, + const std::vector>& kcos); + size_t dbUpdaterQueueSize(); private: void initialize(DBConnector *db, const std::string &tableName, bool dbPersistence); diff --git a/common/zmqserver.cpp b/common/zmqserver.cpp index dca107405..a6383866a 100644 --- a/common/zmqserver.cpp +++ b/common/zmqserver.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -20,6 +21,7 @@ ZmqServer::ZmqServer(const std::string& endpoint, const std::string& vrf) : m_endpoint(endpoint), m_vrf(vrf) { + connect(); m_buffer.resize(MQ_RESPONSE_MAX_COUNT); m_runThread = true; m_mqPollThread = std::make_shared(&ZmqServer::mqPollThread, this); @@ -31,6 +33,33 @@ ZmqServer::~ZmqServer() { m_runThread = false; m_mqPollThread->join(); + + zmq_close(m_socket); + zmq_ctx_destroy(m_context); +} + +void ZmqServer::connect() +{ + SWSS_LOG_ENTER(); + m_context = zmq_ctx_new(); + m_socket = zmq_socket(m_context, ZMQ_PULL); + + // Increase recv buffer for use all bandwidth: http://api.zeromq.org/4-2:zmq-setsockopt + int high_watermark = MQ_WATERMARK; + zmq_setsockopt(m_socket, ZMQ_RCVHWM, &high_watermark, sizeof(high_watermark)); + + if (!m_vrf.empty()) + { + zmq_setsockopt(m_socket, ZMQ_BINDTODEVICE, m_vrf.c_str(), m_vrf.length()); + } + + int rc = zmq_bind(m_socket, m_endpoint.c_str()); + if (rc != 0) + { + SWSS_LOG_THROW("zmq_bind failed on endpoint: %s, zmqerrno: %d", + m_endpoint.c_str(), + zmq_errno()); + } } void ZmqServer::registerMessageHandler( @@ -90,32 +119,10 @@ void ZmqServer::mqPollThread() SWSS_LOG_ENTER(); SWSS_LOG_NOTICE("mqPollThread begin"); - // Producer/Consumer state table are n:1 mapping, so need use PUSH/PULL pattern http://api.zeromq.org/master:zmq-socket - void* context = zmq_ctx_new();; - void* socket = zmq_socket(context, ZMQ_PULL); - - // Increase recv buffer for use all bandwidth: http://api.zeromq.org/4-2:zmq-setsockopt - int high_watermark = MQ_WATERMARK; - zmq_setsockopt(socket, ZMQ_RCVHWM, &high_watermark, sizeof(high_watermark)); - - if (!m_vrf.empty()) - { - zmq_setsockopt(socket, ZMQ_BINDTODEVICE, m_vrf.c_str(), m_vrf.length()); - } - - int rc = zmq_bind(socket, m_endpoint.c_str()); - if (rc != 0) - { - SWSS_LOG_THROW("zmq_bind failed on endpoint: %s, zmqerrno: %d, message: %s", - m_endpoint.c_str(), - zmq_errno(), - strerror(zmq_errno())); - } - // zmq_poll will use less CPU zmq_pollitem_t poll_item; poll_item.fd = 0; - poll_item.socket = socket; + poll_item.socket = m_socket; poll_item.events = ZMQ_POLLIN; poll_item.revents = 0; @@ -123,7 +130,7 @@ void ZmqServer::mqPollThread() while (m_runThread) { // receive message - rc = zmq_poll(&poll_item, 1, 1000); + auto rc = zmq_poll(&poll_item, 1, 1000); if (rc == 0 || !(poll_item.revents & ZMQ_POLLIN)) { // timeout or other event @@ -132,7 +139,7 @@ void ZmqServer::mqPollThread() } // receive message - rc = zmq_recv(socket, m_buffer.data(), MQ_RESPONSE_MAX_COUNT, ZMQ_DONTWAIT); + rc = zmq_recv(m_socket, m_buffer.data(), MQ_RESPONSE_MAX_COUNT, ZMQ_DONTWAIT); if (rc < 0) { int zmq_err = zmq_errno(); @@ -160,11 +167,14 @@ void ZmqServer::mqPollThread() // deserialize and write to redis: handleReceivedData(m_buffer.data(), rc); } - - zmq_close(socket); - zmq_ctx_destroy(context); - SWSS_LOG_NOTICE("mqPollThread end"); } +// TODO: To be implemented later, required for ZMQ_CLIENT & ZMQ_SERVER +// socket types in response path. +void ZmqServer::sendMsg( + const std::string &dbName, const std::string &tableName, + const std::vector &values) { + return; +} } diff --git a/common/zmqserver.h b/common/zmqserver.h index 8afe18d7c..1b78b7a25 100644 --- a/common/zmqserver.h +++ b/common/zmqserver.h @@ -39,7 +39,13 @@ class ZmqServer const std::string tableName, ZmqMessageHandler* handler); + void sendMsg(const std::string& dbName, const std::string& tableName, + const std::vector& values); + private: + + void connect(); + void handleReceivedData(const char* buffer, const size_t size); void mqPollThread(); @@ -56,6 +62,10 @@ class ZmqServer std::string m_vrf; + void* m_context; + + void* m_socket; + std::map> m_HandlerMap; }; diff --git a/tests/c_api_ut.cpp b/tests/c_api_ut.cpp index 3bee372c1..7d3ed019d 100644 --- a/tests/c_api_ut.cpp +++ b/tests/c_api_ut.cpp @@ -395,6 +395,7 @@ TEST(c_api, ZmqConsumerProducerStateTable) { SWSSZmqProducerStateTable_set(pst, arr.data[i].key, arr.data[i].fieldValues); else SWSSZmqClient_sendMsg(cli, "TEST_DB", "mytable", arr); + sleep(2); SWSSSelectResult result; SWSSZmqConsumerStateTable_readData(cst, 1500, true, &result); @@ -434,6 +435,7 @@ TEST(c_api, ZmqConsumerProducerStateTable) { SWSSZmqProducerStateTable_del(pst, arr.data[i].key); else SWSSZmqClient_sendMsg(cli, "TEST_DB", "mytable", arr); + sleep(2); SWSSZmqConsumerStateTable_readData(cst, 500, true, &result); SWSSZmqConsumerStateTable_pops(cst, &arr); diff --git a/tests/zmq_state_ut.cpp b/tests/zmq_state_ut.cpp index 56a8299f9..2b0b60d73 100644 --- a/tests/zmq_state_ut.cpp +++ b/tests/zmq_state_ut.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include "gtest/gtest.h" #include "common/dbconnector.h" #include "common/notificationconsumer.h" @@ -14,6 +15,7 @@ #include "common/zmqclient.h" #include "common/zmqproducerstatetable.h" #include "common/zmqconsumerstatetable.h" +#include "common/binaryserializer.h" using namespace std; using namespace swss; @@ -257,6 +259,9 @@ static void consumerWorker(string tableName, string endpoint, bool dbPersistence } } + // Wait for some time to write into the DB. + sleep(3); + allDataReceived = true; if (dbPersistence) @@ -288,6 +293,9 @@ static void testMethod(bool producerPersistence) // start consumer first, SHM can only have 1 consumer per table. thread *consumerThread = new thread(consumerWorker, testTableName, pullEndpoint, !producerPersistence); + // Wait for the consumer to start. + sleep(1); + cout << "Starting " << NUMBER_OF_THREADS << " producers" << endl; /* Starting the producer before the producer */ for (int i = 0; i < NUMBER_OF_THREADS; i++) @@ -351,6 +359,9 @@ static void testBatchMethod(bool producerPersistence) // start consumer first, SHM can only have 1 consumer per table. thread *consumerThread = new thread(consumerWorker, testTableName, pullEndpoint, !producerPersistence); + // Wait for the consumer to start. + sleep(1); + cout << "Starting " << NUMBER_OF_THREADS << " producers" << endl; /* Starting the producer before the producer */ for (int i = 0; i < NUMBER_OF_THREADS; i++) @@ -465,3 +476,96 @@ TEST(ZmqProducerStateTableDeleteAfterSend, test) table.getKeys(keys); EXPECT_EQ(keys.front(), testKey); } + +static bool zmq_done = false; + +static void zmqConsumerWorker(string tableName, string endpoint, + bool dbPersistence) { + cout << "Consumer thread started: " << tableName << endl; + DBConnector db(TEST_DB, 0, true); + ZmqServer server(endpoint, ""); + ZmqConsumerStateTable c(&db, tableName, server, 128, 0, dbPersistence); + // validate received data + std::vector values; + values.push_back(KeyOpFieldsValuesTuple{ + "k", SET_COMMAND, + std::vector{FieldValueTuple{"f", "v"}}}); + + while (!zmq_done) { + sleep(2); + std::string recDbName, recTableName; + std::vector> recKcos; + std::vector deserializedKcos; + + BinarySerializer::deserializeBuffer(server.m_buffer.data(), + server.m_buffer.size(), recDbName, + recTableName, recKcos); + + for (auto kcoPtr : recKcos) + { + deserializedKcos.push_back(*kcoPtr); + } + EXPECT_EQ(recDbName, TEST_DB); + EXPECT_EQ(recTableName, tableName); + EXPECT_EQ(deserializedKcos, values); + } + + allDataReceived = true; + if (dbPersistence) + { + // wait all persist data write to redis + while (c.dbUpdaterQueueSize() > 0) + { + sleep(1); + } + } + + zmq_done = true; + cout << "Consumer thread ended: " << tableName << endl; +} + +static void ZmqWithResponse(bool producerPersistence) +{ + std::string testTableName = "ZMQ_PROD_CONS_UT"; + std::string pushEndpoint = "tcp://localhost:1234"; + std::string pullEndpoint = "tcp://*:1234"; + // start consumer first, SHM can only have 1 consumer per table. + thread *consumerThread = new thread(zmqConsumerWorker, testTableName, pullEndpoint, !producerPersistence); + + // Wait for the consumer to be ready. + sleep(1); + DBConnector db(TEST_DB, 0, true); + ZmqClient client(pushEndpoint, 3000); + ZmqProducerStateTable p(&db, testTableName, client, true); + std::vector kcos; + kcos.push_back(KeyOpFieldsValuesTuple{"k", SET_COMMAND, std::vector{FieldValueTuple{"f", "v"}}}); + for (int i = 0; i < 3; ++i) { + p.send(kcos); + } + + zmq_done = true; + consumerThread->join(); + delete consumerThread; +} + +TEST(ZmqWithResponse, test) +{ + // test with persist by consumer + ZmqWithResponse(false); +} + +TEST(ZmqWithResponseClientError, test) +{ + std::string testTableName = "ZMQ_PROD_CONS_UT"; + std::string pushEndpoint = "tcp://localhost:1234"; + DBConnector db(TEST_DB, 0, true); + ZmqClient client(pushEndpoint, 3000); + ZmqProducerStateTable p(&db, testTableName, client, true); + std::vector kcos; + kcos.push_back(KeyOpFieldsValuesTuple{"k", SET_COMMAND, std::vector{}}); + std::vector> kcosPtr; + std::string dbName, tableName; + p.send(kcos); + // Wait will timeout without server reply. + EXPECT_FALSE(p.wait(dbName, tableName, kcosPtr)); +}