Implement bulk API for NEXT_HOP_GROUP_MEMBER in syncd (sonic-net#243)

* Fix configure parameter name Signed-off-by: Qi Luo <[email protected]> * Move tests to syncd directory, add consumer in tests Signed-off-by: Qi Luo <[email protected]> * Test for bulk create nhgm Signed-off-by: Qi Luo <[email protected]> * Refactor namespace prefix Signed-off-by: Qi Luo <[email protected]> * Fix reference Signed-off-by: Qi Luo <[email protected]> * Fix function paramter reference Signed-off-by: Qi Luo <[email protected]>
wangshengjun · Oct 13, 2017 · 037d2f8 · 037d2f8
1 parent bfe594d
commit 037d2f8
Show file tree

Hide file tree

Showing 8 changed files with 150 additions and 32 deletions.
diff --git a/configure.ac b/configure.ac
@@ -40,7 +40,7 @@ AM_COND_IF([SAITHRIFT], [
         AC_CHECK_LIB([thrift], [main], [AC_MSG_NOTICE(libthrift found)], [AC_MSG_ERROR(libthrift is required for rpcserver)])
 ])
 
-AC_ARG_ENABLE(rtest,
+AC_ARG_ENABLE(redis-test,
 [  --enable-redis-test     test with redis service],
 [case "${enableval}" in
 	yes) rtest=true ;;

diff --git a/lib/src/Makefile.am b/lib/src/Makefile.am
@@ -46,13 +46,3 @@ libsairedis_la_SOURCES = \
 
 libsairedis_la_CPPFLAGS = $(DBGFLAGS) $(AM_CPPFLAGS) $(CFLAGS_COMMON)
 libsairedis_la_LIBADD = -lhiredis -lswsscommon
-
-bin_PROGRAMS = tests
-
-tests_SOURCES = tests.cpp
-tests_CPPFLAGS = $(DBGFLAGS) $(AM_CPPFLAGS) $(CFLAGS_COMMON)
-tests_LDADD = -lhiredis -lswsscommon -lpthread -L$(top_srcdir)/lib/src/.libs -lsairedis -L$(top_srcdir)/meta/.libs -lsaimetadata -lsaimeta
-
-if RTEST
-TESTS = tests
-endif
diff --git a/syncd/Makefile.am b/syncd/Makefile.am
@@ -1,6 +1,6 @@
 AM_CPPFLAGS = -I$(top_srcdir)/vslib/inc -I$(top_srcdir)/lib/inc -I$(top_srcdir)/SAI/inc -I$(top_srcdir)/SAI/meta
 
-bin_PROGRAMS = syncd syncd_request_shutdown
+bin_PROGRAMS = syncd syncd_request_shutdown tests
 
 if DEBUG
 DBGFLAGS = -ggdb -DDEBUG
@@ -15,6 +15,7 @@ SAILIB=-lsai
 endif
 
 syncd_SOURCES = \
+				main.cpp \
 				syncd.cpp \
 				syncd_saiswitch.cpp \
 				syncd_hard_reinit.cpp \
@@ -34,3 +35,20 @@ endif
 syncd_request_shutdown_SOURCES = syncd_request_shutdown.cpp
 syncd_request_shutdown_CPPFLAGS = $(DBGFLAGS) $(AM_CPPFLAGS) $(CFLAGS_COMMON)
 syncd_request_shutdown_LDADD = -lhiredis -lswsscommon -lpthread
+
+tests_SOURCES = \
+				tests.cpp \
+				syncd.cpp \
+				syncd_saiswitch.cpp \
+				syncd_hard_reinit.cpp \
+				syncd_notifications.cpp \
+				syncd_counters.cpp \
+				syncd_applyview.cpp \
+				syncd_pfc_watchdog.cpp
+
+tests_CPPFLAGS = $(DBGFLAGS) $(AM_CPPFLAGS) $(CFLAGS_COMMON)
+tests_LDADD = -lhiredis -lswsscommon -lpthread -L$(top_srcdir)/lib/src/.libs -lsairedis -L$(top_srcdir)/meta/.libs -lsaimetadata -lsaimeta
+
+if RTEST
+TESTS = tests
+endif
diff --git a/syncd/main.cpp b/syncd/main.cpp
@@ -0,0 +1,7 @@
+
+int syncd_main(int argc, char **argv);
+
+int main(int argc, char **argv)
+{
+    return syncd_main(argc, argv);
+}
diff --git a/syncd/syncd.cpp b/syncd/syncd.cpp
@@ -1958,7 +1958,8 @@ sai_object_id_t extractSwitchVid(
     }
 }
 
-sai_status_t handle_bulk_route(
+sai_status_t handle_bulk_generic(
+        _In_ sai_object_type_t object_type,
         _In_ const std::vector<std::string> &object_ids,
         _In_ sai_common_api_t api,
         _In_ const std::vector<std::shared_ptr<SaiAttributeList>> &attributes)
@@ -1979,24 +1980,29 @@ sai_status_t handle_bulk_route(
         sai_attribute_t *attr_list = list->get_attr_list();
         uint32_t attr_count = list->get_attr_count();
 
-        if (api == (sai_common_api_t)SAI_COMMON_API_BULK_SET)
-        {
-            sai_object_meta_key_t meta_key;
+        sai_object_meta_key_t meta_key;
 
+        if (object_type == SAI_OBJECT_TYPE_ROUTE_ENTRY)
+        {
             meta_key.objecttype = SAI_OBJECT_TYPE_ROUTE_ENTRY;
-
             sai_deserialize_route_entry(object_ids[idx], meta_key.objectkey.key.route_entry);
+        }
+        else if (object_type == SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER)
+        {
+            meta_key.objecttype = SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER;
+            sai_deserialize_object_id(object_ids[idx], meta_key.objectkey.key.object_id);
+        }
+        else
+        {
+            throw std::invalid_argument("object_type");
+        }
 
+        if (api == (sai_common_api_t)SAI_COMMON_API_BULK_SET)
+        {
             status = handle_non_object_id(meta_key, SAI_COMMON_API_SET, attr_count, attr_list);
         }
         else if (api == (sai_common_api_t)SAI_COMMON_API_BULK_CREATE)
         {
-            sai_object_meta_key_t meta_key;
-
-            meta_key.objecttype = SAI_OBJECT_TYPE_ROUTE_ENTRY;
-
-            sai_deserialize_route_entry(object_ids[idx], meta_key.objectkey.key.route_entry);
-
             status = handle_non_object_id(meta_key, SAI_COMMON_API_CREATE, attr_count, attr_list);
         }
         else
@@ -2100,7 +2106,8 @@ sai_status_t processBulkEvent(
     switch (object_type)
     {
         case SAI_OBJECT_TYPE_ROUTE_ENTRY:
-            status = handle_bulk_route(object_ids, api, attributes);
+        case SAI_OBJECT_TYPE_NEXT_HOP_GROUP_MEMBER:
+            status = handle_bulk_generic(object_type, object_ids, api, attributes);
             break;
 
         default:
@@ -3004,7 +3011,7 @@ void sai_meta_log_syncd(
     swss::Logger::getInstance().write(p, ":- %s: %s", func, buffer);
 }
 
-int main(int argc, char **argv)
+int syncd_main(int argc, char **argv)
 {
     swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_DEBUG);
 
@@ -3091,7 +3098,7 @@ int main(int argc, char **argv)
     if (status != SAI_STATUS_SUCCESS)
     {
         SWSS_LOG_ERROR("fail to sai_api_initialize: %d", status);
-        exit(EXIT_FAILURE);
+        return EXIT_FAILURE;
     }
 
     int failed = sai_metadata_apis_query(sai_api_query);
@@ -3215,5 +3222,5 @@ int main(int argc, char **argv)
 
     SWSS_LOG_NOTICE("uninitialize finished");
 
-    exit(EXIT_SUCCESS);
+    return EXIT_SUCCESS;
 }
diff --git a/syncd/syncd.h b/syncd/syncd.h
@@ -113,4 +113,8 @@ bool is_set_attribute_workaround(
 void startNotificationsProcessingThread();
 void stopNotificationsProcessingThread();
 
+sai_status_t processBulkEvent(
+        _In_ sai_common_api_t api,
+        _In_ const swss::KeyOpFieldsValuesTuple &kco);
+
 #endif // __SYNCD_H__
diff --git a/lib/src/tests.cpp → syncd/tests.cpp b/lib/src/tests.cpp → syncd/tests.cpp
@@ -5,19 +5,50 @@ extern "C" {
 }
 
 #include "swss/logger.h"
+#include "swss/dbconnector.h"
+#include "swss/schema.h"
+#include "swss/redisreply.h"
 #include "sairedis.h"
 #include "sai_redis.h"
 #include "meta/saiserialize.h"
+#include "syncd.h"
 
 #include <map>
 #include <unordered_map>
 #include <vector>
+#include <thread>
+#include <tuple>
 
 #define ASSERT_SUCCESS(format,...) \
     if ((status)!=SAI_STATUS_SUCCESS) \
         SWSS_LOG_THROW(format ": %s", ##__VA_ARGS__, sai_serialize_status(status).c_str());
 
-const char* profile_get_value(
+static sai_next_hop_group_api_t test_next_hop_group_api;
+static std::vector<std::tuple<sai_object_id_t, sai_object_id_t, std::vector<sai_attribute_t>>> created_next_hop_group_member;
+
+sai_status_t test_create_next_hop_group_member(
+        _Out_ sai_object_id_t *next_hop_group_member_id,
+        _In_ sai_object_id_t switch_id,
+        _In_ uint32_t attr_count,
+        _In_ const sai_attribute_t *attr_list)
+{
+    created_next_hop_group_member.emplace_back();
+    auto& back = created_next_hop_group_member.back();
+    std::get<0>(back) = *next_hop_group_member_id;
+    std::get<1>(back) = switch_id;
+    auto& attrs = std::get<2>(back);
+    attrs.insert(attrs.end(), attr_list, attr_list + attr_count);
+    return 0;
+}
+
+void clearDB()
+{
+    swss::DBConnector db(ASIC_DB, "localhost", 6379, 0);
+    swss::RedisReply r(&db, "FLUSHALL", REDIS_REPLY_STATUS);
+    r.checkStatusOK();
+}
+
+static const char* profile_get_value(
         _In_ sai_switch_profile_id_t profile_id,
         _In_ const char* variable)
 {
@@ -26,7 +57,7 @@ const char* profile_get_value(
     return NULL;
 }
 
-int profile_get_next_value(
+static int profile_get_next_value(
         _In_ sai_switch_profile_id_t profile_id,
         _Out_ const char** variable,
         _Out_ const char** value)
@@ -50,7 +81,7 @@ int profile_get_next_value(
     return -1;
 }
 
-service_method_table_t test_services = {
+static service_method_table_t test_services = {
     profile_get_value,
     profile_get_next_value
 };
@@ -64,6 +95,11 @@ void test_sai_initialize()
     // with enabled unix socket
     sai_status_t status = sai_api_initialize(0, (service_method_table_t*)&test_services);
 
+    // Mock the SAI api
+    test_next_hop_group_api.create_next_hop_group_member = test_create_next_hop_group_member;
+    sai_metadata_sai_next_hop_group_api = &test_next_hop_group_api;
+    created_next_hop_group_member.clear();
+
     ASSERT_SUCCESS("Failed to initialize api");
 }
 
@@ -100,15 +136,55 @@ sai_object_id_t create_dummy_object_id(
     return (((sai_object_id_t)objecttype) << 48) | ++index;
 }
 
+bool starts_with(const std::string& str, const std::string& substr)
+{
+    return strncmp(str.c_str(), substr.c_str(), substr.size()) == 0;
+}
+
+void bulk_nhgm_consumer_worker()
+{
+    std::string tableName = ASIC_STATE_TABLE;
+    swss::DBConnector db(ASIC_DB, "localhost", 6379, 0);
+    swss::ConsumerTable c(&db, tableName);
+    swss::Select cs;
+    swss::Selectable *selectcs;
+    int tmpfd;
+    int ret = 0;
+
+    cs.addSelectable(&c);
+    while ((ret = cs.select(&selectcs, &tmpfd)) == swss::Select::OBJECT)
+    {
+        swss::KeyOpFieldsValuesTuple kco;
+        c.pop(kco);
+
+        auto& key = kfvKey(kco);
+        auto& op = kfvOp(kco);
+        auto& values = kfvFieldsValues(kco);
+
+        if (starts_with(key, "SAI_OBJECT_TYPE_SWITCH")) continue;
+
+        if (op == "bulkcreate")
+        {
+            sai_status_t status = processBulkEvent((sai_common_api_t)SAI_COMMON_API_BULK_CREATE, kco);
+            ASSERT_SUCCESS("Failed to processBulkEvent");
+            break;
+        }
+
+    }
+}
+
 void test_bulk_next_hop_group_member_create()
 {
     SWSS_LOG_ENTER();
 
     swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_NOTICE);
 
+    clearDB();
     meta_init_db();
     redis_clear_switch_ids();
 
+    auto consumerThreads = new std::thread(bulk_nhgm_consumer_worker);
+
     swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_DEBUG);
 
     sai_status_t    status;
@@ -144,6 +220,7 @@ void test_bulk_next_hop_group_member_create()
     sai_object_meta_key_t meta_key_hopgruop = { .objecttype = SAI_OBJECT_TYPE_NEXT_HOP_GROUP, .objectkey = { .key = { .object_id = hopgroup } } };
     std::string hopgroup_key = sai_serialize_object_meta_key(meta_key_hopgruop);
     ObjectAttrHash[hopgroup_key] = { };
+    sai_object_id_t hopgroup_vid = translate_rid_to_vid(hopgroup, switch_id);
 
     for (uint32_t i = 0; i <  count; ++i)
     {
@@ -153,15 +230,16 @@ void test_bulk_next_hop_group_member_create()
         sai_object_meta_key_t meta_key_hop = { .objecttype = SAI_OBJECT_TYPE_NEXT_HOP, .objectkey = { .key = { .object_id = hop } } };
         std::string hop_key = sai_serialize_object_meta_key(meta_key_hop);
         ObjectAttrHash[hop_key] = { };
+        sai_object_id_t hop_vid = translate_rid_to_vid(hop, switch_id);
 
         std::vector<sai_attribute_t> list(2);
         sai_attribute_t &attr1 = list[0];
         sai_attribute_t &attr2 = list[1];
 
         attr1.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_GROUP_ID;
-        attr1.value.oid = hopgroup;
+        attr1.value.oid = hopgroup_vid;
         attr2.id = SAI_NEXT_HOP_GROUP_MEMBER_ATTR_NEXT_HOP_ID;
-        attr2.value.oid = hop;
+        attr2.value.oid = hop_vid;
         nhgm_attrs.push_back(list);
         nhgm_attrs_count.push_back(2);
     }
@@ -181,6 +259,18 @@ void test_bulk_next_hop_group_member_create()
         status = statuses[j];
         ASSERT_SUCCESS("Failed to create nhgm # %zu", j);
     }
+
+    consumerThreads->join();
+    delete consumerThreads;
+
+    // check the SAI api calling
+    for (size_t i = 0; i < created_next_hop_group_member.size(); i++)
+    {
+        auto& created = created_next_hop_group_member[i];
+        auto& created_attrs = std::get<2>(created);
+        assert(created_attrs.size() == 2);
+        assert(created_attrs[1].value.oid == nhgm_attrs[i][1].value.oid);
+    }
 }
 
 void test_bulk_route_set()
@@ -189,6 +279,7 @@ void test_bulk_route_set()
 
     swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_NOTICE);
 
+    clearDB();
     meta_init_db();
     redis_clear_switch_ids();
 

diff --git a/tests/Makefile.am b/tests/Makefile.am
@@ -11,6 +11,7 @@ endif
 SAILIB=-L$(top_srcdir)/vslib/src/.libs -lsaivs
 
 vssyncd_SOURCES = \
+				../syncd/main.cpp \
 				../syncd/syncd.cpp \
 				../syncd/syncd_saiswitch.cpp \
 				../syncd/syncd_hard_reinit.cpp \