Use redis for VID generator and move RID translation to syncd

common/saiserialize.cpp

@@ -27,7 +27,7 @@ sai_serialization_map_t sai_get_serialization_map()
     map[SAI_OBJECT_TYPE_ROUTER_INTERFACE][SAI_ROUTER_INTERFACE_ATTR_VIRTUAL_ROUTER_ID] = SAI_SERIALIZATION_TYPE_OBJECT_ID;
     map[SAI_OBJECT_TYPE_ROUTER_INTERFACE][SAI_ROUTER_INTERFACE_ATTR_TYPE] = SAI_SERIALIZATION_TYPE_INT32;
     map[SAI_OBJECT_TYPE_ROUTER_INTERFACE][SAI_ROUTER_INTERFACE_ATTR_SRC_MAC_ADDRESS] = SAI_SERIALIZATION_TYPE_MAC;
-    map[SAI_OBJECT_TYPE_ROUTER_INTERFACE][SAI_ROUTER_INTERFACE_ATTR_VLAN_ID] = SAI_SERIALIZATION_TYPE_MAC;
+    map[SAI_OBJECT_TYPE_ROUTER_INTERFACE][SAI_ROUTER_INTERFACE_ATTR_VLAN_ID] = SAI_SERIALIZATION_TYPE_UINT16;
 
     map[SAI_OBJECT_TYPE_HOST_INTERFACE][SAI_HOSTIF_ATTR_TYPE] = SAI_SERIALIZATION_TYPE_INT32;
     map[SAI_OBJECT_TYPE_HOST_INTERFACE][SAI_HOSTIF_ATTR_RIF_OR_PORT_ID] = SAI_SERIALIZATION_TYPE_OBJECT_ID;

lib/inc/sai_redis.h

@@ -17,6 +17,7 @@ extern "C" {
 #include "swss/select.h"
 #include "swss/scheme.h"
 #include "swss/logger.h"
+#include "swss/counter.h"
 
 extern service_method_table_t           g_services;
 extern swss::DBConnector               *g_db;
@@ -29,6 +30,8 @@ extern swss::ConsumerTable             *g_redisNotifications;
 extern swss::Table *g_vidToRid;
 extern swss::Table *g_ridToVid;
 
+extern swss::Counter *g_vidCounter;
+
 extern const sai_acl_api_t              redis_acl_api;
 extern const sai_buffer_api_t           redis_buffer_api;
 extern const sai_fdb_api_t              redis_fdb_api;

lib/src/sai_redis_generic_create.cpp

@@ -1,7 +1,5 @@
 #include "sai_redis.h"
 
-uint64_t object_id_counter = 0;
-
 sai_object_id_t redis_create_virtual_object_id(
         _In_ sai_object_type_t object_type)
 {
@@ -16,10 +14,8 @@ sai_object_id_t redis_create_virtual_object_id(
     // in future we could change this to find "holes" after delted
     // objects, but can be tricky since this information would need
     // to be stored somewhere in case of oa restart
-    //
-    // TODO make this atomic
 
-    uint64_t virtual_id = object_id_counter++;
+    uint64_t virtual_id = (uint64_t)g_vidCounter->incr();
 
     return (((sai_object_id_t)object_type) << 48) | virtual_id;
 }

lib/src/sai_redis_generic_get.cpp

@@ -1,108 +1,5 @@
 #include "sai_redis.h"
 
-sai_object_id_t translate_rid_to_vid(
-        _In_ sai_object_type_t object_type,
-        _In_ sai_object_id_t rid)
-{
-    if (rid == SAI_NULL_OBJECT_ID)
-        return SAI_NULL_OBJECT_ID;
-
-    // get from syncd should always return real id
-    sai_object_id_t vid;
-
-    std::string str_rid;
-    std::string str_vid;
-
-    sai_serialize_primitive(rid, str_rid);
-
-    if (g_ridToVid->getField(std::string(), str_rid, str_vid))
-    {
-        // object exists
-
-        int index = 0;
-        sai_deserialize_primitive(str_vid, index, vid);
-
-        return vid;
-    }
-
-    REDIS_LOG_DBG("rid is missing from db");
-
-    vid = redis_create_virtual_object_id(object_type);
-
-    sai_serialize_primitive(vid, str_vid);
-
-    g_ridToVid->setField(std::string(), str_rid, str_vid);
-    g_vidToRid->setField(std::string(), str_vid, str_rid);
-
-    return vid;
-}
-
-template <typename T>
-void translate_list_rid_to_vid(
-        _In_ sai_object_type_t object_type,
-        _In_ T &element)
-{
-    for (uint32_t i = 0; i < element.count; i++)
-    {
-        element.list[i] = translate_rid_to_vid(object_type, element.list[i]); 
-    }
-}
-
-void translate_rid_to_vid(
-        _In_ sai_object_type_t object_type,
-        _In_ uint32_t attr_count,
-        _In_ sai_attribute_t *attr_list)
-{
-    // we receive real id's here, if they are new then create new id 
-    // for them and put in db, if entry exists in db, use it
-
-    for (uint32_t i = 0; i < attr_count; i++)
-    {
-        sai_attribute_t &attr = attr_list[i];
-
-        sai_attr_serialization_type_t serialization_type;
-        sai_status_t status = sai_get_serialization_type(object_type, attr.id, serialization_type);
-
-        if (status != SAI_STATUS_SUCCESS)
-        {
-            throw std::runtime_error("unable to find serialization type");
-        }
-
-        switch (serialization_type)
-        {
-            case SAI_SERIALIZATION_TYPE_OBJECT_ID:
-                attr.value.oid = translate_rid_to_vid(object_type, attr.value.oid);
-                break;
-
-            case SAI_SERIALIZATION_TYPE_OBJECT_LIST:
-                translate_list_rid_to_vid(object_type, attr.value.objlist);
-                break;
-
-            case SAI_SERIALIZATION_TYPE_ACL_FIELD_DATA_OBJECT_ID:
-                attr.value.aclfield.data.oid = translate_rid_to_vid(object_type, attr.value.aclfield.data.oid);
-                break;
-
-            case SAI_SERIALIZATION_TYPE_ACL_FIELD_DATA_OBJECT_LIST:
-                translate_list_rid_to_vid(object_type, attr.value.aclfield.data.objlist);
-                break;
-
-            case SAI_SERIALIZATION_TYPE_ACL_ACTION_DATA_OBJECT_ID:
-                attr.value.aclaction.parameter.oid = translate_rid_to_vid(object_type, attr.value.aclaction.parameter.oid);
-                break;
-
-            case SAI_SERIALIZATION_TYPE_ACL_ACTION_DATA_OBJECT_LIST:
-                translate_list_rid_to_vid(object_type, attr.value.aclaction.parameter.objlist);
-                break;
-
-            case SAI_SERIALIZATION_TYPE_PORT_BREAKOUT:
-                translate_list_rid_to_vid(object_type, attr.value.portbreakout.port_list);
-
-            default:
-                break;
-        }
-    }
-}
-
 sai_status_t internal_redis_get_process(
         _In_ sai_object_type_t object_type,
         _In_ uint32_t attr_count,
@@ -129,8 +26,6 @@ sai_status_t internal_redis_get_process(
         SaiAttributeList list(object_type, values, false);
 
         transfer_attributes(object_type, attr_count, list.get_attr_list(), attr_list, false);
-
-        translate_rid_to_vid(object_type, attr_count, attr_list);
     }
     else if (status == SAI_STATUS_BUFFER_OVERFLOW)
     {

lib/src/sai_redis_interfacequery.cpp

@@ -16,6 +16,8 @@ swss::ConsumerTable   *g_redisNotifications = NULL;
 swss::Table           *g_vidToRid = NULL;
 swss::Table           *g_ridToVid = NULL;
 
+swss::Counter         *g_vidCounter = NULL;
+
 sai_status_t sai_api_initialize(
         _In_ uint64_t flags,
         _In_ const service_method_table_t* services)
@@ -68,6 +70,11 @@ sai_status_t sai_api_initialize(
     g_vidToRid = new swss::Table(g_db, "VIDTORID");
     g_ridToVid = new swss::Table(g_db, "RIDTOVID");
 
+    if (g_vidCounter != NULL)
+        delete g_vidCounter;
+
+    g_vidCounter = new swss::Counter(g_db, "VIDCOUNTER");
+
     g_initialized = true;
 
     return SAI_STATUS_SUCCESS;

lib/src/sai_redis_notifications.cpp

@@ -170,8 +170,6 @@ void handle_packet_event(
 
     SaiAttributeList list(SAI_OBJECT_TYPE_PACKET, values, false);
 
-    translate_rid_to_vid(SAI_OBJECT_TYPE_PACKET, list.get_attr_count(), list.get_attr_list());
-
     auto on_packet_event = redis_switch_notifications.on_packet_event;
 
     if (on_packet_event != NULL)

syncd/syncd.cpp

@@ -4,6 +4,8 @@
 swss::Table *g_vidToRid = NULL;
 swss::Table *g_ridToVid = NULL;
 
+swss::Counter *g_vidCounter = NULL;
+
 void sai_diag_shell()
 {
     sai_status_t status;
@@ -23,6 +25,122 @@ void sai_diag_shell()
     }
 }
 
+sai_object_id_t redis_create_virtual_object_id(
+        _In_ sai_object_type_t object_type)
+{
+    uint64_t virtual_id = (uint64_t)g_vidCounter->incr();
+
+    return (((sai_object_id_t)object_type) << 48) | virtual_id;
+}
+
+sai_object_id_t translate_rid_to_vid(
+        _In_ sai_object_id_t rid)
+{
+    if (rid == SAI_NULL_OBJECT_ID)
+        return SAI_NULL_OBJECT_ID;
+
+    sai_object_id_t vid;
+
+    std::string str_rid;
+    std::string str_vid;
+
+    sai_serialize_primitive(rid, str_rid);
+
+    if (g_ridToVid->getField(std::string(), str_rid, str_vid))
+    {
+        // object exists
+
+        int index = 0;
+        sai_deserialize_primitive(str_vid, index, vid);
+
+        return vid;
+    }
+
+    SYNCD_LOG_DBG("rid is missing from db");
+
+    sai_object_type_t object_type = sai_object_type_query(rid);
+
+    if (object_type == SAI_OBJECT_TYPE_NULL)
+    {
+        SYNCD_LOG_ERR("sai_object_type_query returned NULL type");
+        throw std::runtime_error("sai_object_type_query returned NULL type");
+    }
+
+    vid = redis_create_virtual_object_id(object_type);
+
+    sai_serialize_primitive(vid, str_vid);
+
+    g_ridToVid->setField(std::string(), str_rid, str_vid);
+    g_vidToRid->setField(std::string(), str_vid, str_rid);
+
+    return vid;
+}
+
+template <typename T>
+void translate_list_rid_to_vid(
+        _In_ T &element)
+{
+    for (uint32_t i = 0; i < element.count; i++)
+    {
+        element.list[i] = translate_rid_to_vid(element.list[i]);
+    }
+}
+
+void translate_rid_to_vid(
+        _In_ sai_object_type_t object_type,
+        _In_ uint32_t attr_count,
+        _In_ sai_attribute_t *attr_list)
+{
+    // we receive real id's here, if they are new then create new id
+    // for them and put in db, if entry exists in db, use it
+
+    for (uint32_t i = 0; i < attr_count; i++)
+    {
+        sai_attribute_t &attr = attr_list[i];
+
+        sai_attr_serialization_type_t serialization_type;
+        sai_status_t status = sai_get_serialization_type(object_type, attr.id, serialization_type);
+
+        if (status != SAI_STATUS_SUCCESS)
+        {
+            throw std::runtime_error("unable to find serialization type");
+        }
+
+        switch (serialization_type)
+        {
+            case SAI_SERIALIZATION_TYPE_OBJECT_ID:
+                attr.value.oid = translate_rid_to_vid(attr.value.oid);
+                break;
+
+            case SAI_SERIALIZATION_TYPE_OBJECT_LIST:
+                translate_list_rid_to_vid(attr.value.objlist);
+                break;
+
+            case SAI_SERIALIZATION_TYPE_ACL_FIELD_DATA_OBJECT_ID:
+                attr.value.aclfield.data.oid = translate_rid_to_vid(attr.value.aclfield.data.oid);
+                break;
+
+            case SAI_SERIALIZATION_TYPE_ACL_FIELD_DATA_OBJECT_LIST:
+                translate_list_rid_to_vid(attr.value.aclfield.data.objlist);
+                break;
+
+            case SAI_SERIALIZATION_TYPE_ACL_ACTION_DATA_OBJECT_ID:
+                attr.value.aclaction.parameter.oid = translate_rid_to_vid(attr.value.aclaction.parameter.oid);
+                break;
+
+            case SAI_SERIALIZATION_TYPE_ACL_ACTION_DATA_OBJECT_LIST:
+                translate_list_rid_to_vid(attr.value.aclaction.parameter.objlist);
+                break;
+
+            case SAI_SERIALIZATION_TYPE_PORT_BREAKOUT:
+                translate_list_rid_to_vid(attr.value.portbreakout.port_list);
+
+            default:
+                break;
+        }
+    }
+}
+
 sai_object_id_t translate_vid_to_rid(
         _In_ sai_object_id_t vid)
 {
@@ -113,6 +231,8 @@ void internal_syncd_get_send(
 
     if (status == SAI_STATUS_SUCCESS)
     {
+        translate_rid_to_vid(object_type, attr_count, attr_list);
+
         // XXX: normal serialization + translate reverse
         entry = SaiAttributeList::serialize_attr_list(
                 object_type,
@@ -547,6 +667,8 @@ int main(int argc, char **argv)
     g_vidToRid = new swss::Table(db, "VIDTORID");
     g_ridToVid = new swss::Table(db, "RIDTOVID");
 
+    g_vidCounter = new swss::Counter(db, "VIDCOUNTER");
+
     swss::ConsumerTable *asicState = new swss::ConsumerTable(db, "ASIC_STATE");
 
     // at the end we cant use producer consumer concept since

syncd/syncd.h

@@ -20,6 +20,7 @@ extern "C" {
 #include "swss/scheme.h"
 #include "swss/logger.h"
 #include "swss/table.h"
+#include "swss/counter.h"
 
 #define UNREFERENCED_PARAMETER(X)
 
@@ -40,6 +41,8 @@ extern swss::ProducerTable *notifications;
 extern swss::Table *g_vidToRid;
 extern swss::Table *g_ridToVid;
 
+extern swss::Counter *g_vidCounter;
+
 sai_object_id_t translate_vid_to_rid(
         _In_ sai_object_id_t vid);
 
@@ -58,6 +61,11 @@ void translate_list_vid_to_rid(
     }
 }
 
+void translate_rid_to_vid(
+        _In_ sai_object_type_t object_type,
+        _In_ uint32_t attr_count,
+        _In_ sai_attribute_t *attr_list);
+
 typedef sai_status_t (*create_fn)(
         _Out_ sai_object_id_t *stp_id,
         _In_  uint32_t attr_count,

syncd/syncd_notifications.cpp

@@ -134,6 +134,19 @@ void on_packet_event(
             attr_list,
             false);
 
+    // since attr_list is const, we can't replace rid's
+    // we need to create copy of that list
+
+    SaiAttributeList copy(SAI_OBJECT_TYPE_PACKET, entry, false);
+
+    translate_rid_to_vid(SAI_OBJECT_TYPE_PACKET, copy.get_attr_count(), copy.get_attr_list());
+
+    entry = SaiAttributeList::serialize_attr_list(
+            SAI_OBJECT_TYPE_PACKET,
+            copy.get_attr_count(),
+            copy.get_attr_list(),
+            false);
+
     send_notification("packet_event", s, entry);
 
     SYNCD_LOG_EXIT();