From 139d9718ee29062e2c02570bb49dec5db95c80f8 Mon Sep 17 00:00:00 2001 From: Kamil Cudnik Date: Wed, 20 Jul 2022 14:51:32 +0200 Subject: [PATCH] [syncd] Remove error message when tryTranslateVidToRid fails (#1048) --- syncd/VirtualOidTranslator.cpp | 69 +++++++-- unittest/syncd/Makefile.am | 3 +- unittest/syncd/MockHelper.cpp | 8 - unittest/syncd/TestFlexCounter.cpp | 52 +++---- unittest/syncd/TestVirtualOidTranslator.cpp | 162 ++++++++++++++++++++ 5 files changed, 249 insertions(+), 45 deletions(-) create mode 100644 unittest/syncd/TestVirtualOidTranslator.cpp diff --git a/syncd/VirtualOidTranslator.cpp b/syncd/VirtualOidTranslator.cpp index 37125674d..a2188a673 100644 --- a/syncd/VirtualOidTranslator.cpp +++ b/syncd/VirtualOidTranslator.cpp @@ -338,16 +338,45 @@ bool VirtualOidTranslator::tryTranslateVidToRid( { SWSS_LOG_ENTER(); - try + std::lock_guard lock(m_mutex); + + if (vid == SAI_NULL_OBJECT_ID) + { + SWSS_LOG_DEBUG("translated VID null to RID null"); + + rid = SAI_NULL_OBJECT_ID; + return true; + } + + auto it = m_vid2rid.find(vid); + + if (it != m_vid2rid.end()) { - rid = translateVidToRid(vid); + rid = it->second; return true; } - catch (const std::exception& e) + + rid = m_client->getRidForVid(vid); + + if (rid == SAI_NULL_OBJECT_ID) { - // message was logged already when throwing + SWSS_LOG_INFO("unable to get RID for VID %s", + sai_serialize_object_id(vid).c_str()); return false; } + + /* + * We got this RID from redis db, so put it also to local db so it will be + * faster to retrieve it late on. + */ + + m_vid2rid[vid] = rid; + + SWSS_LOG_DEBUG("translated VID %s to RID %s", + sai_serialize_object_id(vid).c_str(), + sai_serialize_object_id(rid).c_str()); + + return true; } bool VirtualOidTranslator::tryTranslateVidToRid( @@ -355,16 +384,36 @@ bool VirtualOidTranslator::tryTranslateVidToRid( { SWSS_LOG_ENTER(); - try + auto info = sai_metadata_get_object_type_info(metaKey.objecttype); + + if (info->isobjectid) { - translateVidToRid(metaKey); - return true; + return tryTranslateVidToRid( + metaKey.objectkey.key.object_id, + metaKey.objectkey.key.object_id); } - catch (const std::exception& e) + + for (size_t j = 0; j < info->structmemberscount; ++j) { - // message was logged already when throwing - return false; + const sai_struct_member_info_t *m = info->structmembers[j]; + + if (m->membervaluetype == SAI_ATTR_VALUE_TYPE_OBJECT_ID) + { + sai_object_id_t vid = m->getoid(&metaKey); + + sai_object_id_t rid; + + if (tryTranslateVidToRid(vid, rid)) + { + m->setoid(&metaKey, rid); + continue; + } + + return false; + } } + + return true; } void VirtualOidTranslator::translateVidToRid( diff --git a/unittest/syncd/Makefile.am b/unittest/syncd/Makefile.am index 5148f70a9..6564f222f 100644 --- a/unittest/syncd/Makefile.am +++ b/unittest/syncd/Makefile.am @@ -10,9 +10,10 @@ tests_SOURCES = main.cpp \ MockHelper.cpp \ TestCommandLineOptions.cpp \ TestFlexCounter.cpp \ + TestVirtualOidTranslator.cpp \ TestNotificationQueue.cpp tests_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) -tests_LDADD = $(LDADD_GTEST) $(top_srcdir)/syncd/libSyncd.a -lhiredis -lswsscommon -lpthread -L$(top_srcdir)/lib/.libs -lsairedis -L$(top_srcdir)/meta/.libs -lsaimetadata -lsaimeta -lzmq $(CODE_COVERAGE_LIBS) +tests_LDADD = $(LDADD_GTEST) $(top_srcdir)/syncd/libSyncd.a $(top_srcdir)/vslib/libSaiVS.a -lhiredis -lswsscommon -lnl-genl-3 -lnl-nf-3 -lnl-route-3 -lnl-3 -lpthread -L$(top_srcdir)/lib/.libs -lsairedis -L$(top_srcdir)/meta/.libs -lsaimetadata -lsaimeta -lzmq $(CODE_COVERAGE_LIBS) TESTS = tests diff --git a/unittest/syncd/MockHelper.cpp b/unittest/syncd/MockHelper.cpp index 929df8620..fa6af8e11 100644 --- a/unittest/syncd/MockHelper.cpp +++ b/unittest/syncd/MockHelper.cpp @@ -10,11 +10,3 @@ namespace test_syncd { mock_objectTypeQuery_result = mock_result; } } - -namespace syncd { - sai_object_type_t VidManager::objectTypeQuery(_In_ sai_object_id_t objectId) - { - SWSS_LOG_ENTER(); - return test_syncd::mock_objectTypeQuery_result; - } -} diff --git a/unittest/syncd/TestFlexCounter.cpp b/unittest/syncd/TestFlexCounter.cpp index 090b7c533..ea2aaf560 100644 --- a/unittest/syncd/TestFlexCounter.cpp +++ b/unittest/syncd/TestFlexCounter.cpp @@ -13,8 +13,8 @@ TEST(FlexCounter, addRemoveCounterForFlowCounter) std::shared_ptr sai(new MockableSaiInterface()); FlexCounter fc("test", sai, "COUNTERS_DB"); - sai_object_id_t counterVid{100}; - sai_object_id_t counterRid{100}; + sai_object_id_t counterVid{0x54000000000000}; + sai_object_id_t counterRid{0x54000000000000}; std::vector values; values.emplace_back(FLOW_COUNTER_ID_LIST, "SAI_COUNTER_STAT_PACKETS,SAI_COUNTER_STAT_BYTES"); @@ -46,12 +46,12 @@ TEST(FlexCounter, addRemoveCounterForFlowCounter) std::vector keys; countersTable.getKeys(keys); EXPECT_EQ(keys.size(), size_t(1)); - EXPECT_EQ(keys[0], "oid:0x64"); + EXPECT_EQ(keys[0], "oid:0x54000000000000"); std::string value; - countersTable.hget("oid:0x64", "SAI_COUNTER_STAT_PACKETS", value); + countersTable.hget("oid:0x54000000000000", "SAI_COUNTER_STAT_PACKETS", value); EXPECT_EQ(value, "100"); - countersTable.hget("oid:0x64", "SAI_COUNTER_STAT_BYTES", value); + countersTable.hget("oid:0x54000000000000", "SAI_COUNTER_STAT_BYTES", value); EXPECT_EQ(value, "200"); fc.removeCounter(counterVid); @@ -81,8 +81,8 @@ TEST(FlexCounter, addRemoveCounterForMACsecFlow) std::shared_ptr sai(new MockableSaiInterface()); FlexCounter fc("test", sai, "COUNTERS_DB"); - sai_object_id_t macsecFlowVid{101}; - sai_object_id_t macsecFlowRid{101}; + sai_object_id_t macsecFlowVid{0x5a000000000000}; + sai_object_id_t macsecFlowRid{0x5a000000000000}; std::vector values; values.emplace_back(MACSEC_FLOW_COUNTER_ID_LIST, "SAI_MACSEC_FLOW_STAT_CONTROL_PKTS,SAI_MACSEC_FLOW_STAT_PKTS_UNTAGGED"); @@ -114,17 +114,17 @@ TEST(FlexCounter, addRemoveCounterForMACsecFlow) std::vector keys; countersTable.getKeys(keys); EXPECT_EQ(keys.size(), size_t(1)); - EXPECT_EQ(keys[0], "oid:0x65"); + EXPECT_EQ(keys[0], "oid:0x5a000000000000"); std::string value; - countersTable.hget("oid:0x65", "SAI_MACSEC_FLOW_STAT_CONTROL_PKTS", value); + countersTable.hget("oid:0x5a000000000000", "SAI_MACSEC_FLOW_STAT_CONTROL_PKTS", value); //EXPECT_EQ(value, "100"); - countersTable.hget("oid:0x65", "SAI_MACSEC_FLOW_STAT_PKTS_UNTAGGED", value); + countersTable.hget("oid:0x5a000000000000", "SAI_MACSEC_FLOW_STAT_PKTS_UNTAGGED", value); //EXPECT_EQ(value, "200"); fc.removeCounter(macsecFlowVid); EXPECT_EQ(fc.isEmpty(), true); - countersTable.del("oid:0x65"); + countersTable.del("oid:0x5a000000000000"); countersTable.getKeys(keys); ASSERT_TRUE(keys.empty()); @@ -136,8 +136,8 @@ TEST(FlexCounter, addRemoveCounterForMACsecSA) std::shared_ptr sai(new MockableSaiInterface()); FlexCounter fc("test", sai, "COUNTERS_DB"); - sai_object_id_t macsecSAVid{102}; - sai_object_id_t macsecSARid{102}; + sai_object_id_t macsecSAVid{0x5c000000000000}; + sai_object_id_t macsecSARid{0x5c000000000000}; std::vector values; values.emplace_back(MACSEC_SA_COUNTER_ID_LIST, "SAI_MACSEC_SA_STAT_OCTETS_ENCRYPTED,SAI_MACSEC_SA_STAT_OCTETS_PROTECTED"); @@ -169,17 +169,17 @@ TEST(FlexCounter, addRemoveCounterForMACsecSA) std::vector keys; countersTable.getKeys(keys); EXPECT_EQ(keys.size(), size_t(1)); - EXPECT_EQ(keys[0], "oid:0x66"); + EXPECT_EQ(keys[0], "oid:0x5c000000000000"); std::string value; - countersTable.hget("oid:0x66", "SAI_MACSEC_SA_STAT_OCTETS_ENCRYPTED", value); + countersTable.hget("oid:0x5c000000000000", "SAI_MACSEC_SA_STAT_OCTETS_ENCRYPTED", value); //EXPECT_EQ(value, "100"); - countersTable.hget("oid:0x66", "SAI_MACSEC_SA_STAT_OCTETS_PROTECTED", value); + countersTable.hget("oid:0x5c000000000000", "SAI_MACSEC_SA_STAT_OCTETS_PROTECTED", value); //EXPECT_EQ(value, "200"); fc.removeCounter(macsecSAVid); EXPECT_EQ(fc.isEmpty(), true); - countersTable.del("oid:0x66"); + countersTable.del("oid:0x5c000000000000"); countersTable.getKeys(keys); ASSERT_TRUE(keys.empty()); @@ -191,8 +191,8 @@ TEST(FlexCounter, addRemoveCounterForPort) std::shared_ptr sai(new MockableSaiInterface()); FlexCounter fc("test", sai, "COUNTERS_DB"); - sai_object_id_t counterVid{100}; - sai_object_id_t counterRid{100}; + sai_object_id_t counterVid{0x1000000000000}; + sai_object_id_t counterRid{0x1000000000000}; std::vector values; values.emplace_back(PORT_COUNTER_ID_LIST, "SAI_PORT_STAT_IF_IN_OCTETS,SAI_PORT_STAT_IF_IN_ERRORS"); @@ -232,17 +232,17 @@ TEST(FlexCounter, addRemoveCounterForPort) std::vector keys; countersTable.getKeys(keys); EXPECT_EQ(keys.size(), size_t(1)); - EXPECT_EQ(keys[0], "oid:0x64"); + EXPECT_EQ(keys[0], "oid:0x1000000000000"); std::string value; - countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_OCTETS", value); + countersTable.hget("oid:0x1000000000000", "SAI_PORT_STAT_IF_IN_OCTETS", value); EXPECT_EQ(value, "100"); - countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_ERRORS", value); + countersTable.hget("oid:0x1000000000000", "SAI_PORT_STAT_IF_IN_ERRORS", value); EXPECT_EQ(value, "200"); fc.removeCounter(counterVid); EXPECT_EQ(fc.isEmpty(), true); - countersTable.del("oid:0x64"); + countersTable.del("oid:0x1000000000000"); countersTable.getKeys(keys); ASSERT_TRUE(keys.empty()); @@ -265,14 +265,14 @@ TEST(FlexCounter, addRemoveCounterForPort) EXPECT_EQ(fc.isEmpty(), false); usleep(1000*1000); - countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_OCTETS", value); + countersTable.hget("oid:0x1000000000000", "SAI_PORT_STAT_IF_IN_OCTETS", value); EXPECT_EQ(value, "100"); - countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_ERRORS", value); + countersTable.hget("oid:0x1000000000000", "SAI_PORT_STAT_IF_IN_ERRORS", value); EXPECT_EQ(value, "200"); fc.removeCounter(counterVid); EXPECT_EQ(fc.isEmpty(), true); - countersTable.del("oid:0x64"); + countersTable.del("oid:0x1000000000000"); countersTable.getKeys(keys); ASSERT_TRUE(keys.empty()); } diff --git a/unittest/syncd/TestVirtualOidTranslator.cpp b/unittest/syncd/TestVirtualOidTranslator.cpp new file mode 100644 index 000000000..90ac58461 --- /dev/null +++ b/unittest/syncd/TestVirtualOidTranslator.cpp @@ -0,0 +1,162 @@ +#include "VirtualOidTranslator.h" +#include "VendorSai.h" +#include "lib/RedisVidIndexGenerator.h" +#include "lib/sairediscommon.h" +#include "ServiceMethodTable.h" +#include "vslib/Sai.h" + +#include + +using namespace syncd; +using namespace std::placeholders; + +static std::map profileMap; + +static std::map::iterator profileIter; + +static const char* profileGetValue( + _In_ sai_switch_profile_id_t profile_id, + _In_ const char* variable) +{ + SWSS_LOG_ENTER(); + + if (variable == NULL) + { + SWSS_LOG_WARN("variable is null"); + return NULL; + } + + auto it = profileMap.find(variable); + + if (it == profileMap.end()) + { + SWSS_LOG_NOTICE("%s: NULL", variable); + return NULL; + } + + SWSS_LOG_NOTICE("%s: %s", variable, it->second.c_str()); + + return it->second.c_str(); +} + +static int profileGetNextValue( + _In_ sai_switch_profile_id_t profile_id, + _Out_ const char** variable, + _Out_ const char** value) +{ + SWSS_LOG_ENTER(); + + if (value == NULL) + { + SWSS_LOG_INFO("resetting profile map iterator"); + + profileIter = profileMap.begin(); + return 0; + } + + if (variable == NULL) + { + SWSS_LOG_WARN("variable is null"); + return -1; + } + + if (profileIter == profileMap.end()) + { + SWSS_LOG_INFO("iterator reached end"); + return -1; + } + + *variable = profileIter->first.c_str(); + *value = profileIter->second.c_str(); + + SWSS_LOG_INFO("key: %s:%s", *variable, *value); + + profileIter++; + + return 0; +} + +TEST(VirtualOidTranslator, tryTranslateVidToRid) +{ + profileMap["SAI_VS_SWITCH_TYPE"] = "SAI_VS_SWITCH_TYPE_BCM56850"; + + auto dbAsic = std::make_shared("ASIC_DB", 0); + auto client = std::make_shared(dbAsic); + auto sai = std::make_shared(); + + ServiceMethodTable smt; + + smt.profileGetValue = std::bind(&profileGetValue, _1, _2); + smt.profileGetNextValue = std::bind(&profileGetNextValue, _1, _2, _3); + + sai_service_method_table_t test_services = smt.getServiceMethodTable(); + + sai_status_t status = sai->initialize(0, &test_services); + + EXPECT_EQ(status, SAI_STATUS_SUCCESS); + + auto switchConfigContainer = std::make_shared(); + auto redisVidIndexGenerator = std::make_shared(dbAsic, REDIS_KEY_VIDCOUNTER); + + auto virtualObjectIdManager = + std::make_shared( + 0, + switchConfigContainer, + redisVidIndexGenerator); + + VirtualOidTranslator vot(client, virtualObjectIdManager, sai); + + sai_object_id_t rid; + + EXPECT_TRUE(vot.tryTranslateVidToRid(0, rid)); + + EXPECT_EQ(rid, 0); + + EXPECT_FALSE(vot.tryTranslateVidToRid(0x21, rid)); + + sai_attribute_t attr; + + attr.id = SAI_SWITCH_ATTR_INIT_SWITCH; + attr.value.booldata = true; + + sai_object_id_t swid; + + status = sai->create(SAI_OBJECT_TYPE_SWITCH, &swid, SAI_NULL_OBJECT_ID, 1, &attr); + + EXPECT_EQ(status, SAI_STATUS_SUCCESS); + + vot.insertRidAndVid(0x2100000000,0x21000000000000); + + swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_DEBUG); + + vot.translateRidToVid(0x2100000000,0x21000000000000); + + EXPECT_TRUE(vot.tryTranslateVidToRid(0x21000000000000, rid)); + + vot.clearLocalCache(); + + EXPECT_TRUE(vot.tryTranslateVidToRid(0x21000000000000, rid)); + + swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_NOTICE); + + // meta key + + sai_object_meta_key_t mk; + + mk.objecttype = SAI_OBJECT_TYPE_PORT; + mk.objectkey.key.object_id = 0; + + EXPECT_TRUE(vot.tryTranslateVidToRid(mk)); + + mk.objecttype = SAI_OBJECT_TYPE_FDB_ENTRY; + mk.objectkey.key.fdb_entry.switch_id = 0x21000000000000; + mk.objectkey.key.fdb_entry.bv_id = 0; + + EXPECT_TRUE(vot.tryTranslateVidToRid(mk)); + + mk.objectkey.key.fdb_entry.bv_id = 0x21; + + EXPECT_FALSE(vot.tryTranslateVidToRid(mk)); + + sai->uninitialize(); +}