Implement sai_bulk_create_route_entry in sairedis and syncd

* internal_redis_bulk_generic_create() support multiple attrs for one entry * implement bulkcreate in syncd * add bulk API for rotue create and set * add bulk route set api test Signed-off-by: Qi Luo <qiluo-msft@users.noreply.github.com>
dmytroxshevchuk · Oct 2, 2017 · e4d2427 · e4d2427
1 parent 3ac7cc3
commit e4d2427
Show file tree

Hide file tree

Showing 9 changed files with 285 additions and 26 deletions.
diff --git a/configure.ac b/configure.ac
@@ -12,7 +12,7 @@ AC_PROG_LIBTOOL
 AC_HEADER_STDC
 
 AC_ARG_ENABLE(debug,
-[  --enable-debug    Turn on debugging],
+[  --enable-debug          turn on debugging],
 [case "${enableval}" in
 	yes) debug=true ;;
 	no)  debug=false ;;
@@ -21,15 +21,15 @@ esac],[debug=false])
 AM_CONDITIONAL(DEBUG, test x$debug = xtrue)
 
 AC_ARG_WITH(sai,
-[  --with-sai        Compiling against specific SAI],
+[  --with-sai              compiling against specific SAI],
 [case "${withval}" in
 	vs)   sai=vs ;;
 	*) AC_MSG_ERROR(bad value ${withval} for --with-sai) ;;
 esac])
 AM_CONDITIONAL(SAIVS,   test x$sai = xvs)
 
 AC_ARG_ENABLE(rpcserver,
-[  --enable-rpcserver  Compile with thrift rpcserver included],
+[  --enable-rpcserver      compile with thrift rpcserver included],
 [case "${enableval}" in
    yes) rpcserver=true ;;
    no)  rpcserver=false ;;
@@ -40,6 +40,15 @@ 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,
+[  --enable-redis-test     test with redis service],
+[case "${enableval}" in
+	yes) rtest=true ;;
+	no)  rtest=false ;;
+	*) AC_MSG_ERROR(bad value ${enableval} for --enable-redis-test) ;;
+esac],[rtest=false])
+AM_CONDITIONAL(RTEST, test x$rtest = xtrue)
+
 CFLAGS_COMMON=""
 CFLAGS_COMMON+=" -ansi"
 CFLAGS_COMMON+=" -fPIC"

diff --git a/lib/inc/sai_redis.h b/lib/inc/sai_redis.h
@@ -140,6 +140,13 @@ sai_status_t redis_generic_create_route_entry(
         _In_ uint32_t attr_count,
         _In_ const sai_attribute_t *attr_list);
 
+sai_status_t internal_redis_bulk_generic_create(
+        _In_ sai_object_type_t object_type,
+        _In_ const std::vector<std::string> &serialized_object_ids,
+        _In_ const uint32_t *attr_count,
+        _In_ const sai_attribute_t *const *attr_list,
+        _Inout_ sai_status_t *object_statuses);
+
 // REMOVE
 
 sai_status_t redis_generic_remove(

diff --git a/lib/inc/sairedis.h b/lib/inc/sairedis.h
@@ -146,7 +146,7 @@ sai_status_t sai_bulk_create_route_entry(
         _In_ uint32_t object_count,
         _In_ const sai_route_entry_t *route_entry,
         _In_ const uint32_t *attr_count,
-        _In_ const sai_attribute_t **attr_list,
+        _In_ const sai_attribute_t *const *attr_list,
         _In_ sai_bulk_op_type_t type,
         _Out_ sai_status_t *object_statuses);
 

diff --git a/lib/src/Makefile.am b/lib/src/Makefile.am
@@ -53,4 +53,6 @@ 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/lib/src/sai_redis_generic_create.cpp b/lib/src/sai_redis_generic_create.cpp
@@ -268,6 +268,92 @@ sai_status_t redis_generic_create(
             attr_list);
 }
 
+sai_status_t internal_redis_bulk_generic_create(
+        _In_ sai_object_type_t object_type,
+        _In_ const std::vector<std::string> &serialized_object_ids,
+        _In_ const uint32_t *attr_count,
+        _In_ const sai_attribute_t *const *attr_list,
+        _Inout_ sai_status_t *object_statuses)
+{
+    SWSS_LOG_ENTER();
+
+    std::string str_object_type = sai_serialize_object_type(object_type);
+
+    std::vector<swss::FieldValueTuple> entries;
+    std::vector<swss::FieldValueTuple> entriesWithStatus;
+
+    /*
+     * We are recording all entries and their statuses, but we send to sairedis
+     * only those that succeeded metadata check, since only those will be
+     * executed on syncd, so there is no need with bothering decoding statuses
+     * on syncd side.
+     */
+
+    for (size_t idx = 0; idx < serialized_object_ids.size(); ++idx)
+    {
+        std::vector<swss::FieldValueTuple> entry =
+            SaiAttributeList::serialize_attr_list(object_type, attr_count[idx], &attr_list[idx][0], false);
+
+        std::string str_attr = joinFieldValues(entry);
+
+        std::string str_status = sai_serialize_status(object_statuses[idx]);
+
+        std::string joined = str_attr + "|" + str_status;
+
+        swss::FieldValueTuple fvt(serialized_object_ids[idx] , joined);
+
+        entriesWithStatus.push_back(fvt);
+
+        if (object_statuses[idx] != SAI_STATUS_SUCCESS)
+        {
+            SWSS_LOG_WARN("skipping %s since status is %s",
+                    serialized_object_ids[idx].c_str(),
+                    str_status.c_str());
+
+            continue;
+        }
+
+        swss::FieldValueTuple fvtNoStatus(serialized_object_ids[idx] , str_attr);
+
+        entries.push_back(fvtNoStatus);
+    }
+
+    /*
+     * We are adding number of entries to actualy add ':' to be compatible
+     * with previous
+     */
+
+    if (g_record)
+    {
+        std::string joined;
+
+        for (const auto &e: entriesWithStatus)
+        {
+            // ||obj_id|attr=val|attr=val|status||obj_id|attr=val|attr=val|status
+
+            joined += "||" + fvField(e) + "|" + fvValue(e);
+        }
+
+        /*
+         * Capital 'C' stads for bulk CREATE operation.
+         */
+
+        recordLine("C|" + str_object_type + joined);
+    }
+
+    // key:         object_type:count
+    // field:       object_id
+    // value:       object_attrs
+    std::string key = str_object_type + ":" + std::to_string(entries.size());
+
+    if (entries.size())
+    {
+        g_asicState->set(key, entries, "bulkcreate");
+    }
+
+    return SAI_STATUS_SUCCESS;
+}
+
 sai_status_t redis_generic_create_fdb_entry(
         _In_ const sai_fdb_entry_t *fdb_entry,
         _In_ uint32_t attr_count,

diff --git a/lib/src/sai_redis_generic_get.cpp b/lib/src/sai_redis_generic_get.cpp
@@ -10,7 +10,9 @@ sai_status_t internal_redis_get_process(
 {
     SWSS_LOG_ENTER();
 
-    // key is: object_type:object_id:sai_status
+    // key:         sai_status
+    // field:       attr_id
+    // value:       attr_value
 
     const std::string &key = kfvKey(kco);
     const std::vector<swss::FieldValueTuple> &values = kfvFieldsValues(kco);

diff --git a/lib/src/sai_redis_route.cpp b/lib/src/sai_redis_route.cpp
@@ -10,20 +10,135 @@ const sai_route_api_t redis_route_api = {
     REDIS_GENERIC_QUAD_API(route_entry)
 };
 
+sai_status_t redis_dummy_create_route_entry(
+        _In_ const sai_route_entry_t *route_entry,
+        _In_ uint32_t attr_count,
+        _In_ const sai_attribute_t *attr_list)
+{
+    SWSS_LOG_ENTER();
+
+    /*
+     * Since we are using validation for each route in bulk operations, we
+     * can't execute actual CREATE, we need to do dummy create and then introduce
+     * internal bulk_create operation that will only touch redis db only once.
+     * So we are returning success here.
+     */
+
+    return SAI_STATUS_SUCCESS;
+}
 
 sai_status_t sai_bulk_create_route_entry(
         _In_ uint32_t object_count,
         _In_ const sai_route_entry_t *route_entry,
         _In_ const uint32_t *attr_count,
-        _In_ const sai_attribute_t **attr_list,
+        _In_ const sai_attribute_t *const *attr_list,
         _In_ sai_bulk_op_type_t type,
         _Out_ sai_status_t *object_statuses)
 {
     std::lock_guard<std::mutex> lock(g_apimutex);
 
     SWSS_LOG_ENTER();
 
-    return SAI_STATUS_NOT_IMPLEMENTED;
+    if (object_count < 1)
+    {
+        SWSS_LOG_ERROR("expected at least 1 object to create");
+
+        return SAI_STATUS_INVALID_PARAMETER;
+    }
+
+    if (route_entry == NULL)
+    {
+        SWSS_LOG_ERROR("route_entry is NULL");
+
+        return SAI_STATUS_INVALID_PARAMETER;
+    }
+
+    if (attr_count == NULL)
+    {
+        SWSS_LOG_ERROR("attr_count is NULL");
+
+        return SAI_STATUS_INVALID_PARAMETER;
+    }
+
+    if (attr_list == NULL)
+    {
+        SWSS_LOG_ERROR("attr_list is NULL");
+
+        return SAI_STATUS_INVALID_PARAMETER;
+    }
+
+    switch (type)
+    {
+        case SAI_BULK_OP_TYPE_STOP_ON_ERROR:
+        case SAI_BULK_OP_TYPE_INGORE_ERROR:
+             // ok
+             break;
+
+        default:
+
+             SWSS_LOG_ERROR("invalid bulk operation type %d", type);
+
+             return SAI_STATUS_INVALID_PARAMETER;
+    }
+
+    if (object_statuses == NULL)
+    {
+        SWSS_LOG_ERROR("object_statuses is NULL");
+
+        return SAI_STATUS_INVALID_PARAMETER;
+    }
+
+    std::vector<std::string> serialized_object_ids;
+
+    for (uint32_t idx = 0; idx < object_count; ++idx)
+    {
+        /*
+         * At the beginning set all statuses to not executed.
+         */
+
+        object_statuses[idx] = SAI_STATUS_NOT_EXECUTED;
+
+        serialized_object_ids.push_back(
+                sai_serialize_route_entry(route_entry[idx]));
+    }
+
+    for (uint32_t idx = 0; idx < object_count; ++idx)
+    {
+        sai_status_t status =
+            meta_sai_create_route_entry(
+                    &route_entry[idx],
+                    attr_count[idx],
+                    attr_list[idx],
+                    &redis_dummy_create_route_entry);
+
+        object_statuses[idx] = status;
+
+        if (status != SAI_STATUS_SUCCESS)
+        {
+            // TODO add attr id and value
+
+            SWSS_LOG_ERROR("failed on index %u: %s",
+                    idx,
+                    serialized_object_ids[idx].c_str());
+
+            if (type == SAI_BULK_OP_TYPE_STOP_ON_ERROR)
+            {
+                SWSS_LOG_NOTICE("stop on error since previous operation failed");
+                break;
+            }
+        }
+    }
+
+    /*
+     * TODO: we need to record operation type
+     */
+
+    return internal_redis_bulk_generic_create(
+            SAI_OBJECT_TYPE_ROUTE_ENTRY,
+            serialized_object_ids,
+            attr_count,
+            attr_list,
+            object_statuses);
 }
 
 sai_status_t sai_bulk_remove_route_entry(