From 9974a14a78080214c737788f5d76ceb08a997c7c Mon Sep 17 00:00:00 2001 From: dgsudharsan Date: Thu, 11 Jul 2019 12:50:53 -0700 Subject: [PATCH 1/8] Sflow orchagent changes Mgr changes Mgr changes 2 Test cases Changing Global to global optimizations Reverting copp changes clean up UT Fixes Addressing code review comments Updating UT Build fix Adding mgr changes --- cfgmgr/Makefile.am | 9 +- cfgmgr/sflowmgr.cpp | 117 +++++++ cfgmgr/sflowmgr.h | 50 +++ cfgmgr/sflowmgrd.cpp | 87 +++++ orchagent/Makefile.am | 3 +- orchagent/orchdaemon.cpp | 8 +- orchagent/orchdaemon.h | 1 + orchagent/saihelper.cpp | 3 + orchagent/sfloworch.cpp | 674 +++++++++++++++++++++++++++++++++++++++ orchagent/sfloworch.h | 55 ++++ tests/test_sflow.py | 310 ++++++++++++++++++ 11 files changed, 1313 insertions(+), 4 deletions(-) create mode 100644 cfgmgr/sflowmgr.cpp create mode 100644 cfgmgr/sflowmgr.h create mode 100644 cfgmgr/sflowmgrd.cpp create mode 100644 orchagent/sfloworch.cpp create mode 100644 orchagent/sfloworch.h create mode 100644 tests/test_sflow.py diff --git a/cfgmgr/Makefile.am b/cfgmgr/Makefile.am index ee5c3b5ce6..c1e3f06996 100644 --- a/cfgmgr/Makefile.am +++ b/cfgmgr/Makefile.am @@ -3,7 +3,7 @@ CFLAGS_SAI = -I /usr/include/sai LIBNL_CFLAGS = -I/usr/include/libnl3 LIBNL_LIBS = -lnl-genl-3 -lnl-route-3 -lnl-3 -bin_PROGRAMS = vlanmgrd teammgrd portmgrd intfmgrd buffermgrd vrfmgrd nbrmgrd vxlanmgrd +bin_PROGRAMS = vlanmgrd teammgrd portmgrd intfmgrd buffermgrd vrfmgrd nbrmgrd vxlanmgrd sflowmgrd if DEBUG DBGFLAGS = -ggdb -DDEBUG @@ -49,4 +49,9 @@ nbrmgrd_LDADD = -lswsscommon $(LIBNL_LIBS) vxlanmgrd_SOURCES = vxlanmgrd.cpp vxlanmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp shellcmd.h vxlanmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) vxlanmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) -vxlanmgrd_LDADD = -lswsscommon \ No newline at end of file +vxlanmgrd_LDADD = -lswsscommon + +sflowmgrd_SOURCES = sflowmgrd.cpp sflowmgr.cpp $(top_srcdir)/orchagent/orch.cpp $(top_srcdir)/orchagent/request_parser.cpp shellcmd.h +sflowmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) +sflowmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) +sflowmgrd_LDADD = -lswsscommon diff --git a/cfgmgr/sflowmgr.cpp b/cfgmgr/sflowmgr.cpp new file mode 100644 index 0000000000..7395adcfaa --- /dev/null +++ b/cfgmgr/sflowmgr.cpp @@ -0,0 +1,117 @@ +#include "logger.h" +#include "dbconnector.h" +#include "producerstatetable.h" +#include "tokenize.h" +#include "ipprefix.h" +#include "sflowmgr.h" +#include "exec.h" +#include "shellcmd.h" + +using namespace std; +using namespace swss; + +SflowMgr::SflowMgr(DBConnector *cfgDb, DBConnector *appDb, const vector &tableNames) : + Orch(cfgDb, tableNames), + m_cfgSflowTable(cfgDb, CFG_SFLOW_TABLE_NAME), + m_cfgSflowSessionTable(cfgDb, CFG_SFLOW_SESSION_TABLE_NAME), + m_appSflowTable(appDb, APP_SFLOW_TABLE_NAME), + m_appSflowSessionTable(appDb, APP_SFLOW_SESSION_TABLE_NAME), + m_appSflowSpeedRateTable(appDb, APP_SFLOW_SAMPLE_RATE_TABLE_NAME) +{ + vector fieldValues; + + fieldValues.emplace_back(SFLOW_SAMPLE_RATE_KEY_400G, SFLOW_SAMPLE_RATE_VALUE_400G); + fieldValues.emplace_back(SFLOW_SAMPLE_RATE_KEY_100G, SFLOW_SAMPLE_RATE_VALUE_100G); + fieldValues.emplace_back(SFLOW_SAMPLE_RATE_KEY_50G, SFLOW_SAMPLE_RATE_VALUE_50G); + fieldValues.emplace_back(SFLOW_SAMPLE_RATE_KEY_40G, SFLOW_SAMPLE_RATE_VALUE_40G); + fieldValues.emplace_back(SFLOW_SAMPLE_RATE_KEY_25G, SFLOW_SAMPLE_RATE_VALUE_25G); + fieldValues.emplace_back(SFLOW_SAMPLE_RATE_KEY_10G, SFLOW_SAMPLE_RATE_VALUE_10G); + fieldValues.emplace_back(SFLOW_SAMPLE_RATE_KEY_1G, SFLOW_SAMPLE_RATE_VALUE_1G); + + m_appSflowSpeedRateTable.set("global", fieldValues); +} + +void SflowMgr::handleSflowTableConfig(Consumer &consumer) +{ + stringstream cmd; + string res; + + auto it = consumer.m_toSync.begin(); + + while (it != consumer.m_toSync.end()) + { + auto t = it->second; + + string key = kfvKey(t); + string op = kfvOp(t); + auto values = kfvFieldsValues(t); + + if (op == SET_COMMAND) + { + for (auto i : kfvFieldsValues(t)) + { + if (fvField(i) == "admin_state") + { + if (fvValue(i) == "enable") + { + cmd << "service hsflowd restart"; + } + else + { + cmd << "service hsflowd stop"; + } + + int ret = swss::exec(cmd.str(), res); + if (ret) + { + SWSS_LOG_ERROR("Command '%s' failed with rc %d", cmd.str().c_str(), ret); + } + else + { + SWSS_LOG_INFO("Command '%s' succeeded", cmd.str().c_str()); + } + } + } + m_appSflowTable.set(key, values); + } + else if(op == DEL_COMMAND) + { + m_appSflowTable.del(key); + } + it = consumer.m_toSync.erase(it); + } +} + +void SflowMgr::doTask(Consumer &consumer) +{ + SWSS_LOG_ENTER(); + + auto table = consumer.getTableName(); + + if(table == CFG_SFLOW_TABLE_NAME) + { + handleSflowTableConfig(consumer); + return; + } + + auto it = consumer.m_toSync.begin(); + while (it != consumer.m_toSync.end()) + { + KeyOpFieldsValuesTuple t = it->second; + + string key = kfvKey(t); + string op = kfvOp(t); + auto values = kfvFieldsValues(t); + + if (op == SET_COMMAND) + { + m_appSflowSessionTable.set(key, values); + } + else if (op == DEL_COMMAND) + { + m_appSflowSessionTable.del(key); + } + + it = consumer.m_toSync.erase(it); + } +} diff --git a/cfgmgr/sflowmgr.h b/cfgmgr/sflowmgr.h new file mode 100644 index 0000000000..2f399534eb --- /dev/null +++ b/cfgmgr/sflowmgr.h @@ -0,0 +1,50 @@ +#pragma once + +#include "dbconnector.h" +#include "orch.h" +#include "producerstatetable.h" + +#include +#include +#include + +namespace swss { + +/* Port default admin status is down */ +#define DEFAULT_ADMIN_STATUS_STR "down" +#define DEFAULT_MTU_STR "9100" + +#define SFLOW_SAMPLE_RATE_KEY_400G "400000" +#define SFLOW_SAMPLE_RATE_KEY_100G "100000" +#define SFLOW_SAMPLE_RATE_KEY_50G "50000" +#define SFLOW_SAMPLE_RATE_KEY_40G "40000" +#define SFLOW_SAMPLE_RATE_KEY_25G "25000" +#define SFLOW_SAMPLE_RATE_KEY_10G "10000" +#define SFLOW_SAMPLE_RATE_KEY_1G "1000" + +#define SFLOW_SAMPLE_RATE_VALUE_400G "40000" +#define SFLOW_SAMPLE_RATE_VALUE_100G "10000" +#define SFLOW_SAMPLE_RATE_VALUE_50G "5000" +#define SFLOW_SAMPLE_RATE_VALUE_40G "4000" +#define SFLOW_SAMPLE_RATE_VALUE_25G "2500" +#define SFLOW_SAMPLE_RATE_VALUE_10G "1000" +#define SFLOW_SAMPLE_RATE_VALUE_1G "100" + +class SflowMgr : public Orch +{ +public: + SflowMgr(DBConnector *cfgDb, DBConnector *appDb, const vector &tableNames); + + using Orch::doTask; +private: + Table m_cfgSflowTable; + Table m_cfgSflowSessionTable; + Table m_appSflowSpeedRateTable; + ProducerStateTable m_appSflowTable; + ProducerStateTable m_appSflowSessionTable; + + void doTask(Consumer &consumer); + void handleSflowTableConfig(Consumer &consumer); +}; + +} diff --git a/cfgmgr/sflowmgrd.cpp b/cfgmgr/sflowmgrd.cpp new file mode 100644 index 0000000000..35630c5c51 --- /dev/null +++ b/cfgmgr/sflowmgrd.cpp @@ -0,0 +1,87 @@ +#include +#include +#include +#include +#include + +#include "exec.h" +#include "sflowmgr.h" +#include "schema.h" +#include "select.h" + +using namespace std; +using namespace swss; + +/* select() function timeout retry time, in millisecond */ +#define SELECT_TIMEOUT 1000 + +/* + * Following global variables are defined here for the purpose of + * using existing Orch class which is to be refactored soon to + * eliminate the direct exposure of the global variables. + * + * Once Orch class refactoring is done, these global variables + * should be removed from here. + */ +int gBatchSize = 0; +bool gSwssRecord = false; +bool gLogRotate = false; +ofstream gRecordOfs; +string gRecordFile; +/* Global database mutex */ +mutex gDbMutex; + +int main(int argc, char **argv) +{ + Logger::linkToDbNative("sflowmgrd"); + SWSS_LOG_ENTER(); + + SWSS_LOG_NOTICE("--- Starting sflowmgrd ---"); + + try + { + vector cfg_sflow_tables = { + CFG_SFLOW_TABLE_NAME, + CFG_SFLOW_SESSION_TABLE_NAME + }; + + DBConnector cfgDb(CONFIG_DB, DBConnector::DEFAULT_UNIXSOCKET, 0); + DBConnector appDb(APPL_DB, DBConnector::DEFAULT_UNIXSOCKET, 0); + + SflowMgr sflowmgr(&cfgDb, &appDb, cfg_sflow_tables); + + vector cfgOrchList = {&sflowmgr}; + + swss::Select s; + for (Orch *o : cfgOrchList) + { + s.addSelectables(o->getSelectables()); + } + + while (true) + { + Selectable *sel; + int ret; + + ret = s.select(&sel, SELECT_TIMEOUT); + if (ret == Select::ERROR) + { + SWSS_LOG_NOTICE("Error: %s!", strerror(errno)); + continue; + } + if (ret == Select::TIMEOUT) + { + sflowmgr.doTask(); + continue; + } + + auto *c = (Executor *)sel; + c->execute(); + } + } + catch (const exception &e) + { + SWSS_LOG_ERROR("Runtime error: %s", e.what()); + } + return -1; +} diff --git a/orchagent/Makefile.am b/orchagent/Makefile.am index 22600af304..e7af6b070a 100644 --- a/orchagent/Makefile.am +++ b/orchagent/Makefile.am @@ -52,7 +52,8 @@ orchagent_SOURCES = \ dtelorch.cpp \ flexcounterorch.cpp \ watermarkorch.cpp \ - policerorch.cpp + policerorch.cpp \ + sfloworch.cpp orchagent_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) orchagent_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index af5edaa028..1974971cfa 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -177,6 +177,12 @@ bool OrchDaemon::init() WatermarkOrch *wm_orch = new WatermarkOrch(m_configDb, wm_tables); + vector sflow_tables = { + APP_SFLOW_TABLE_NAME, + APP_SFLOW_SESSION_TABLE_NAME + }; + SflowOrch *sflow_orch = new SflowOrch(m_applDb, sflow_tables); + /* * The order of the orch list is important for state restore of warm start and * the queued processing in m_toSync map after gPortsOrch->allPortsReady() is set. @@ -185,7 +191,7 @@ bool OrchDaemon::init() * when iterating ConsumerMap. * That is ensured implicitly by the order of map key, "LAG_TABLE" is smaller than "VLAN_TABLE" in lexicographic order. */ - m_orchList = { gSwitchOrch, gCrmOrch, gBufferOrch, gPortsOrch, gIntfsOrch, gNeighOrch, gRouteOrch, copp_orch, tunnel_decap_orch, qos_orch, wm_orch, policer_orch }; + m_orchList = { gSwitchOrch, gCrmOrch, gBufferOrch, gPortsOrch, gIntfsOrch, gNeighOrch, gRouteOrch, copp_orch, tunnel_decap_orch, qos_orch, wm_orch, policer_orch, sflow_orch}; bool initialize_dtel = false; diff --git a/orchagent/orchdaemon.h b/orchagent/orchdaemon.h index 14d43fb2c1..bfa52fdef8 100644 --- a/orchagent/orchdaemon.h +++ b/orchagent/orchdaemon.h @@ -27,6 +27,7 @@ #include "flexcounterorch.h" #include "watermarkorch.h" #include "policerorch.h" +#include "sfloworch.h" #include "directory.h" using namespace swss; diff --git a/orchagent/saihelper.cpp b/orchagent/saihelper.cpp index 4b01ddeb39..dbadc7e288 100644 --- a/orchagent/saihelper.cpp +++ b/orchagent/saihelper.cpp @@ -41,6 +41,7 @@ sai_mirror_api_t* sai_mirror_api; sai_fdb_api_t* sai_fdb_api; sai_dtel_api_t* sai_dtel_api; sai_bmtor_api_t* sai_bmtor_api; +sai_samplepacket_api_t* sai_samplepacket_api; extern sai_object_id_t gSwitchId; extern bool gSairedisRecord; @@ -130,6 +131,7 @@ void initSaiApi() sai_api_query(SAI_API_ACL, (void **)&sai_acl_api); sai_api_query(SAI_API_DTEL, (void **)&sai_dtel_api); sai_api_query((sai_api_t)SAI_API_BMTOR, (void **)&sai_bmtor_api); + sai_api_query(SAI_API_SAMPLEPACKET, (void **)&sai_samplepacket_api); sai_log_set(SAI_API_SWITCH, SAI_LOG_LEVEL_NOTICE); sai_log_set(SAI_API_BRIDGE, SAI_LOG_LEVEL_NOTICE); @@ -156,6 +158,7 @@ void initSaiApi() sai_log_set(SAI_API_ACL, SAI_LOG_LEVEL_NOTICE); sai_log_set(SAI_API_DTEL, SAI_LOG_LEVEL_NOTICE); sai_log_set((sai_api_t)SAI_API_BMTOR, SAI_LOG_LEVEL_NOTICE); + sai_log_set(SAI_API_SAMPLEPACKET, SAI_LOG_LEVEL_NOTICE); } void initSaiRedis(const string &record_location) diff --git a/orchagent/sfloworch.cpp b/orchagent/sfloworch.cpp new file mode 100644 index 0000000000..c369a1382a --- /dev/null +++ b/orchagent/sfloworch.cpp @@ -0,0 +1,674 @@ +#include "sai.h" +#include "sfloworch.h" +#include "tokenize.h" + +using namespace std; +using namespace swss; + +extern sai_samplepacket_api_t* sai_samplepacket_api; +extern sai_port_api_t* sai_port_api; + +extern sai_object_id_t gSwitchId; +extern PortsOrch* gPortsOrch; + + +bool SflowOrch::sflowGetDefaultSampleRate(Port port, uint32_t &rate) +{ + string speed_str = to_string(port.m_speed); + string rate_str; + bool ret = m_sflowSampleRateTable->hget("global", speed_str, rate_str); + + + if(ret == false) + { + SWSS_LOG_ERROR("Unable to find default rate for speed %d", port.m_speed); + return false; + } + + rate = (uint32_t)stoul(rate_str); + return true; +} + +SflowOrch::SflowOrch(DBConnector* db, vector &tableNames) : + Orch(db, tableNames) +{ + SWSS_LOG_ENTER(); + m_sflowSampleRateTable = unique_ptr(new Table(db, APP_SFLOW_SAMPLE_RATE_TABLE_NAME)); + gEnable = true; + sflowStatus = false; +} + +bool SflowOrch::sflowCreateSession(SflowSession &session) +{ + sai_attribute_t attr; + sai_object_id_t session_id = SAI_NULL_OBJECT_ID; + sai_status_t sai_rc; + + attr.id = SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE; + attr.value.u32 = session.rate; + + sai_rc = sai_samplepacket_api->create_samplepacket(&session_id, gSwitchId, + 1, &attr); + if(sai_rc != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to create sample packet session with rate %d", + session.rate); + return false; + } + + session.m_sample_id = session_id; + return true; +} + +bool SflowOrch::sflowDestroySession(SflowSession &session) +{ + sai_status_t sai_rc; + + sai_rc = sai_samplepacket_api->remove_samplepacket(session.m_sample_id); + if(sai_rc != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to destroy sample packet session with id %lx", + session.m_sample_id); + return false; + } + + return true; +} + +bool SflowOrch::sflowUpdateRate(sai_object_id_t port_id, uint32_t rate) +{ + auto old_session = m_sflowPortSessionMap.find(port_id); + auto sample_obj = m_sflowRateSampleMap.find(rate); + SflowSession new_session; + + if (sample_obj == m_sflowRateSampleMap.end()) + { + new_session.rate = rate; + if (!sflowCreateSession(new_session)) + { + SWSS_LOG_ERROR("Creating sflow session with rate %d failed", rate); + return false; + } + m_sflowRateSampleMap[rate] = new_session.m_sample_id; + m_sflowSampleRefMap[new_session.m_sample_id] = 0; + } + else + { + new_session.m_sample_id = sample_obj->second; + } + + if (old_session->second.adminState) + { + if (!sflowAddPort(new_session, port_id)) + { + return false; + } + } + + m_sflowSampleRefMap[new_session.m_sample_id]++; + m_sflowSampleRefMap[old_session->second.m_sample_id]--; + + if (m_sflowSampleRefMap[old_session->second.m_sample_id] == 0) + { + if (!sflowDestroySession(old_session->second)) + { + SWSS_LOG_ERROR("Failed to clean old session %lx", + old_session->second.m_sample_id); + } + else + { + m_sflowRateSampleMap.erase(old_session->second.rate); + m_sflowSampleRefMap.erase(old_session->second.m_sample_id); + } + } + new_session.globalConfigured = old_session->second.globalConfigured; + new_session.adminState = old_session->second.adminState; + + m_sflowPortSessionMap[port_id] = new_session; + return true; +} + +bool SflowOrch::sflowAddPort(SflowSession &session, sai_object_id_t port_id) +{ + sai_attribute_t attr; + sai_status_t sai_rc; + + if (!sflowStatus) + { + return true; + } + + attr.id = SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE; + attr.value.oid = session.m_sample_id; + + sai_rc = sai_port_api->set_port_attribute(port_id, &attr); + + if (sai_rc != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to set session %lx on port %lx", + session.m_sample_id, port_id); + return false; + } + return true; +} + +bool SflowOrch::sflowDelPort(SflowSession &session, sai_object_id_t port_id) +{ + sai_attribute_t attr; + sai_status_t sai_rc; + + + if (!sflowStatus) + { + return true; + } + + attr.id = SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE; + attr.value.oid = SAI_NULL_OBJECT_ID; + + sai_rc = sai_port_api->set_port_attribute(port_id, &attr); + + if(sai_rc != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR("Failed to delete session %lx on port %lx", + session.m_sample_id, port_id); + return false; + } + return true; +} + +bool SflowOrch::sflowGlobalConfigure(bool enable) +{ + bool ret = true; + + for(auto& pair: gPortsOrch->getAllPorts()) + { + auto& port = pair.second; + if (port.m_type != Port::PHY) continue; + + auto sflowInfo = m_sflowPortSessionMap.find(port.m_port_id); + if(sflowInfo == m_sflowPortSessionMap.end()) + { + if(!enable) + { + continue; + } + SflowSession session; + uint32_t rate = 0; + + sflowGetDefaultSampleRate(port, rate); + session.rate = rate; + session.globalConfigured = true; + session.adminState = true; + auto e_sampleObj = m_sflowRateSampleMap.find(rate); + + if (e_sampleObj != m_sflowRateSampleMap.end()) + { + session.m_sample_id = e_sampleObj->second; + } + else + { + if (!sflowCreateSession(session)) + { + SWSS_LOG_NOTICE("Creating sflow session with rate %d failed", rate); + ret = false; + continue; + } + m_sflowRateSampleMap[rate] = session.m_sample_id; + m_sflowSampleRefMap[session.m_sample_id] = 0; + } + + if (sflowAddPort(session, port.m_port_id)) + { + m_sflowPortSessionMap[port.m_port_id] = session; + m_sflowSampleRefMap[session.m_sample_id]++; + } + else + { + SWSS_LOG_ERROR("Failed to add port %s to global session", + port.m_alias.c_str()); + ret = false; + + if (m_sflowSampleRefMap[session.m_sample_id] == 0) + { + if (sflowDestroySession(session)) + { + m_sflowSampleRefMap.erase(session.m_sample_id); + m_sflowRateSampleMap.erase(rate); + } + } + } + } + else + { + if(!sflowInfo->second.globalConfigured) + { + continue; + } + if(sflowInfo->second.adminState != enable) + { + if(!enable) + { + if (sflowDelPort(sflowInfo->second, port.m_port_id)) + { + sflowInfo->second.adminState = enable; + } + else + { + SWSS_LOG_ERROR("Failed to disable global sflow on port %s", + port.m_alias.c_str()); + ret = false; + } + } + else + { + if (sflowAddPort(sflowInfo->second, port.m_port_id)) + { + sflowInfo->second.adminState = enable; + } + else + { + SWSS_LOG_ERROR("Failed to enable global sflow on port %s", + port.m_alias.c_str()); + ret = false; + } + } + } + } + } + return ret; +} + +bool SflowOrch::sflowStatusSet(bool enable, bool remove_session) +{ + bool ret = true; + sai_attribute_t attr; + sai_status_t sai_rc; + + attr.id = SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE; + + if (enable) + { + if (!sflowGlobalConfigure(gEnable)) + { + ret = false; + } + } + + for (auto& pair: gPortsOrch->getAllPorts()) + { + auto& port = pair.second; + if (port.m_type != Port::PHY) continue; + + auto sflowInfo = m_sflowPortSessionMap.find(port.m_port_id); + if (sflowInfo != m_sflowPortSessionMap.end()) + { + if (sflowInfo->second.adminState) + { + if (!enable) + { + attr.value.oid = SAI_NULL_OBJECT_ID; + } + else + { + attr.value.oid = sflowInfo->second.m_sample_id; + } + + sai_rc = sai_port_api->set_port_attribute(port.m_port_id, &attr); + + if (sai_rc != SAI_STATUS_SUCCESS) + { + ret = false; + SWSS_LOG_ERROR("Failed to re-configure sflow on port %s", + port.m_alias.c_str()); + } + } + + if (remove_session) + { + m_sflowSampleRefMap[sflowInfo->second.m_sample_id]--; + if (m_sflowSampleRefMap[sflowInfo->second.m_sample_id] == 0) + { + if (sflowDestroySession(sflowInfo->second)) + { + m_sflowSampleRefMap.erase(sflowInfo->second.m_sample_id); + m_sflowRateSampleMap.erase(sflowInfo->second.rate); + } + } + m_sflowPortSessionMap.erase(port.m_port_id); + } + } + } + return ret; +} + +bool SflowOrch::sflowPortApplyGlobalSetting(Port port, SflowSession &session) +{ + uint32_t rate = 0; + + sflowGetDefaultSampleRate(port, rate); + + if (rate != session.rate) + { + if (!sflowUpdateRate(port.m_port_id, rate)) + { + return false; + } + } + + if(session.adminState != gEnable) + { + if (gEnable) + { + if(!sflowAddPort(session, port.m_port_id)) + { + SWSS_LOG_ERROR("Updating port with session %lx failed", session.m_sample_id); + return false; + } + } + else + { + if(!sflowDelPort(session, port.m_port_id)) + { + SWSS_LOG_ERROR("Updating port with session %lx failed", session.m_sample_id); + return false; + } + + } + } + session.adminState = gEnable; + session.globalConfigured = true; + + return true; +} + +bool SflowOrch::handleSflowStatus(KeyOpFieldsValuesTuple tuple) +{ + bool ret = true; + string op = kfvOp(tuple); + + if (op == SET_COMMAND) + { + for (auto i : kfvFieldsValues(tuple)) + { + if (fvField(i) == "admin_state") + { + if (fvValue(i) == "enable") + { + if(sflowStatus) + { + continue; + } + sflowStatus = true; + } + else if (fvValue(i) == "disable") + { + if(!sflowStatus) + { + continue; + } + sflowStatus = false; + } + if (!sflowStatusSet(sflowStatus, false)) + { + ret = false; + continue; + } + } + } + } + else if (op == DEL_COMMAND) + { + if (!sflowStatusSet(false, true)) + { + ret = false; + } + sflowStatus = false; + } + return ret; +} + +bool SflowOrch::handleGlobalConfig(KeyOpFieldsValuesTuple tuple) +{ + bool ret = true; + string op = kfvOp(tuple); + + if (op == SET_COMMAND) + { + for (auto i : kfvFieldsValues(tuple)) + { + if (fvField(i) == "admin_state") + { + if(fvValue(i) == "enable") + { + gEnable = true; + } + else if(fvValue(i) == "disable") + { + gEnable = false; + } + if(!sflowGlobalConfigure(gEnable)) + { + ret = false; + continue; + } + } + else + { + ret = false; + } + } + } + else if (op == DEL_COMMAND) + { + /* By default global configure is true*/ + if (!sflowGlobalConfigure(true)) + { + ret = false; + } + else + { + gEnable = true; + } + } + return ret; +} + +void SflowOrch::doSflowStatusTask(Consumer &consumer) +{ + auto it = consumer.m_toSync.begin(); + + while (it != consumer.m_toSync.end()) + { + auto tuple = it->second; + string op = kfvOp(tuple); + + if (!handleSflowStatus(tuple)) + { + it++; + continue; + } + it = consumer.m_toSync.erase(it); + } +} + +void SflowOrch::doTask(Consumer &consumer) +{ + SWSS_LOG_ENTER(); + Port port; + + string table_name = consumer.getTableName(); + + if (!gPortsOrch->allPortsReady()) + { + return; + } + + if (table_name == APP_SFLOW_TABLE_NAME) + { + doSflowStatusTask(consumer); + return; + } + + auto it = consumer.m_toSync.begin(); + while (it != consumer.m_toSync.end()) + { + auto tuple = it->second; + string op = kfvOp(tuple); + + string alias = kfvKey(tuple); + + if (alias == "all") + { + if (handleGlobalConfig(tuple)) + { + it = consumer.m_toSync.erase(it); + } + else + { + it++; + } + continue; + } + + gPortsOrch->getPort(alias, port); + + if (op == SET_COMMAND) + { + bool adminState = gEnable; + uint32_t rate = 0; + + bool rateSet = false; + bool adminSet = false; + + sflowGetDefaultSampleRate(port, rate); + + for (auto i : kfvFieldsValues(tuple)) + { + if (fvField(i) == "admin_state") + { + if (fvValue(i) == "enable") + { + adminState = true; + } + else if(fvValue(i) == "disable") + { + adminState = false; + } + adminSet = true; + } + + if(fvField(i) == "sample_rate") + { + rate = (uint32_t)stoul(fvValue(i)); + rateSet = true; + } + } + auto sflowInfo = m_sflowPortSessionMap.find(port.m_port_id); + + if (sflowInfo == m_sflowPortSessionMap.end()) + { + SflowSession session; + + session.rate = rate; + session.adminState = adminState; + session.globalConfigured = false; + auto e_sampleObj = m_sflowRateSampleMap.find(rate); + + if (e_sampleObj != m_sflowRateSampleMap.end()) + { + session.m_sample_id = e_sampleObj->second; + } + else + { + if (!sflowCreateSession(session)) + { + it++; + continue; + } + m_sflowRateSampleMap[rate] = session.m_sample_id; + m_sflowSampleRefMap[session.m_sample_id] = 0; + } + if (adminState == true) + { + if (!sflowAddPort(session, port.m_port_id)) + { + it++; + continue; + } + } + m_sflowPortSessionMap[port.m_port_id] = session; + m_sflowSampleRefMap[session.m_sample_id]++; + } + else + { + if ((rateSet) && (rate != sflowInfo->second.rate)) + { + if (sflowUpdateRate(port.m_port_id, rate)) + { + it++; + continue; + } + } + if ((adminSet) && (adminState != sflowInfo->second.adminState)) + { + if(adminState) + { + if (!sflowAddPort(sflowInfo->second, port.m_port_id)) + { + it++; + continue; + } + } + else + { + if (!sflowDelPort(sflowInfo->second, port.m_port_id)) + { + it++; + continue; + } + } + sflowInfo->second.adminState = adminState; + } + sflowInfo->second.globalConfigured = false; + } + it = consumer.m_toSync.erase(it); + } + else if (op == DEL_COMMAND) + { + auto sflowInfo = m_sflowPortSessionMap.find(port.m_port_id); + if (sflowInfo != m_sflowPortSessionMap.end()) + { + if(gEnable) + { + sflowPortApplyGlobalSetting(port, sflowInfo->second); + it = consumer.m_toSync.erase(it); + continue; + } + if(sflowInfo->second.adminState) + { + if (!sflowDelPort(sflowInfo->second, port.m_port_id)) + { + it++; + continue; + } + sflowInfo->second.adminState = false; + } + + m_sflowPortSessionMap.erase(port.m_port_id); + m_sflowSampleRefMap[sflowInfo->second.m_sample_id]--; + if (m_sflowSampleRefMap[sflowInfo->second.m_sample_id] == 0) + { + if (!sflowDestroySession(sflowInfo->second)) + { + it++; + continue; + } + m_sflowSampleRefMap.erase(sflowInfo->second.m_sample_id); + m_sflowRateSampleMap.erase(sflowInfo->second.rate); + } + } + it = consumer.m_toSync.erase(it); + } + } +} diff --git a/orchagent/sfloworch.h b/orchagent/sfloworch.h new file mode 100644 index 0000000000..e3442b5bc0 --- /dev/null +++ b/orchagent/sfloworch.h @@ -0,0 +1,55 @@ +#pragma once + +#include +#include + +#include "orch.h" +#include "portsorch.h" + +using namespace std; + +struct SflowSession +{ + bool globalConfigured; + bool adminState; + sai_object_id_t m_sample_id; + uint32_t rate; +}; + +/* SAI Port to Sflow session map */ +typedef map SflowPortSessionMap; + +/* Sample-rate(unsigned int) to SAI Samplesession object map */ +typedef map SflowRateSampleMap; + +/* SAI Samplesession object to reference(number of ports referencing this object) map*/ +typedef map SflowSampleRefMap; + +class SflowOrch : public Orch +{ +public: + SflowOrch(DBConnector* db, vector &tableNames); + +private: + SflowPortSessionMap m_sflowPortSessionMap; + SflowRateSampleMap m_sflowRateSampleMap; + SflowSampleRefMap m_sflowSampleRefMap; + unique_ptr
m_sflowDefaults; + bool gEnable; + bool sflowStatus; + unique_ptr
m_sflowSampleRateTable; + + virtual void doTask(Consumer& consumer); + void doSflowStatusTask(Consumer &consumer); + bool sflowCreateSession(SflowSession &session); + bool sflowDestroySession(SflowSession &session); + bool sflowAddPort(SflowSession &session_id, sai_object_id_t port_id); + bool sflowDelPort(SflowSession &session_id, sai_object_id_t port_id); + bool sflowStatusSet(bool enable, bool remove_session); + bool handleSflowStatus(KeyOpFieldsValuesTuple tuple); + bool handleGlobalConfig(KeyOpFieldsValuesTuple tuple); + bool sflowGetDefaultSampleRate(Port port, uint32_t &rate); + bool sflowGlobalConfigure(bool enable); + bool sflowPortApplyGlobalSetting(Port port, SflowSession &session); + bool sflowUpdateRate(sai_object_id_t port_id, uint32_t rate); +}; diff --git a/tests/test_sflow.py b/tests/test_sflow.py new file mode 100644 index 0000000000..fde226813c --- /dev/null +++ b/tests/test_sflow.py @@ -0,0 +1,310 @@ +from swsscommon import swsscommon + +import time +import os + +class TestSflow(object): + def setup_sflow(self, dvs): + self.pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0) + self.adb = swsscommon.DBConnector(1, dvs.redis_sock, 0) + rtbl = swsscommon.Table(self.pdb, "SFLOW_SAMPLE_RATE_TABLE") + fvs = swsscommon.FieldValuePairs([("100000", "10000"), + ("40000", "4000"), + ("10000", "1000"), + ("1000", "100")]) + rtbl.set("global", fvs) + + + ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_TABLE") + fvs = swsscommon.FieldValuePairs([("admin_state", "enable")]) + ptbl.set("global", fvs) + + time.sleep(1) + + def test_defaultGlobal(self, dvs, testlog): + self.setup_sflow(dvs) + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") + (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) + + assert status == True + + sample_session = "" + speed = "" + for fv in fvs: + if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": + sample_session = fv[1] + elif fv[0] == "SAI_PORT_ATTR_SPEED": + speed = fv[1] + + assert sample_session != "" + assert speed != "" + + + rate = "" + dtbl = swsscommon.Table(self.pdb, "SFLOW_SAMPLE_RATE_TABLE") + (status, fvs) = dtbl.get("global") + + for fv in fvs: + if fv[0] == speed: + rate = fv[1] + + assert rate != "" + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_SAMPLEPACKET") + (status, fvs) = atbl.get(sample_session) + + assert status == True + + for fv in fvs: + if fv[0] == "SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE": + assert fv[1] == rate + + ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_TABLE") + fvs = swsscommon.FieldValuePairs([("admin_state", "disable")]) + ptbl.set("global", fvs) + + time.sleep(1) + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") + (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) + + assert status == True + + sample_session = "" + speed = "" + for fv in fvs: + if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": + sample_session = fv[1] + + assert sample_session == "oid:0x0" + + def test_globalAll(self, dvs, testlog): + self.setup_sflow(dvs) + + ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_SESSION_TABLE") + fvs = swsscommon.FieldValuePairs([("admin_state", "disable")]) + ptbl.set("all", fvs) + + time.sleep(1) + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") + (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) + + assert status == True + + sample_session = "" + speed = "" + for fv in fvs: + if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": + sample_session = fv[1] + + assert sample_session == "oid:0x0" + + fvs = swsscommon.FieldValuePairs([("admin_state", "enable")]) + ptbl.set("all", fvs) + + time.sleep(1) + + (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) + + assert status == True + + sample_session = "" + for fv in fvs: + if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": + sample_session = fv[1] + + assert sample_session != "" + assert sample_session != "oid:0x0" + + ptbl._del("all") + + time.sleep(1) + + (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) + + assert status == True + + sample_session = "" + for fv in fvs: + if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": + sample_session = fv[1] + + assert sample_session != "" + assert sample_session != "oid:0x0" + + + def test_InterfaceSet(self, dvs, testlog): + self.setup_sflow(dvs) + ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_SESSION_TABLE") + gtbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_TABLE") + fvs = swsscommon.FieldValuePairs([("admin_state", "enable"),("sample_rate","1000")]) + ptbl.set("Ethernet0", fvs) + + time.sleep(1) + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") + (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) + + assert status == True + + sample_session = "" + for fv in fvs: + if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": + sample_session = fv[1] + + assert sample_session != "" + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_SAMPLEPACKET") + (status, fvs) = atbl.get(sample_session) + + assert status == True + + for fv in fvs: + if fv[0] == "SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE": + assert fv[1] == "1000" + + fvs = swsscommon.FieldValuePairs([("admin_state", "disable")]) + ptbl.set("all", fvs) + + time.sleep(1) + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") + (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) + + assert status == True + + sample_session = "" + for fv in fvs: + if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": + sample_session = fv[1] + assert sample_session != "" + assert sample_session != "oid:0x0" + + fvs = swsscommon.FieldValuePairs([("admin_state", "disable")]) + gtbl.set("global", fvs) + + time.sleep(1) + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") + (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) + + assert status == True + + sample_session = "" + for fv in fvs: + if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": + sample_session = fv[1] + + assert sample_session == "oid:0x0" + ptbl._del("all") + ptbl._del("Ethernet0") + + def test_defaultRate(self, dvs, testlog): + self.setup_sflow(dvs) + ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_SESSION_TABLE") + fvs = swsscommon.FieldValuePairs([("admin_state", "enable")]) + ptbl.set("Ethernet4", fvs) + + time.sleep(1) + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") + (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet4"]) + + assert status == True + + sample_session = "" + speed = "" + for fv in fvs: + if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": + sample_session = fv[1] + elif fv[0] == "SAI_PORT_ATTR_SPEED": + speed = fv[1] + + assert sample_session != "" + assert speed != "" + + rate = "" + dtbl = swsscommon.Table(self.pdb, "SFLOW_SAMPLE_RATE_TABLE") + (status, fvs) = dtbl.get("global") + + for fv in fvs: + if fv[0] == speed: + rate = fv[1] + + assert rate != "" + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_SAMPLEPACKET") + (status, fvs) = atbl.get(sample_session) + + assert status == True + + for fv in fvs: + if fv[0] == "SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE": + assert fv[1] == rate + + ptbl._del("Ethernet4") + + def test_ConfigDel(self, dvs, testlog): + self.setup_sflow(dvs) + ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_SESSION_TABLE") + fvs = swsscommon.FieldValuePairs([("admin_state", "enable"),("sample_rate","1000")]) + ptbl.set("Ethernet0", fvs) + + time.sleep(1) + + ptbl._del("Ethernet0") + + time.sleep(1) + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") + (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) + + assert status == True + + sample_session = "" + speed = "" + + for fv in fvs: + if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": + sample_session = fv[1] + elif fv[0] == "SAI_PORT_ATTR_SPEED": + speed = fv[1] + + assert speed != "" + assert sample_session != "" + assert sample_session != "oid:0x0" + + rate = "" + dtbl = swsscommon.Table(self.pdb, "SFLOW_SAMPLE_RATE_TABLE") + (status, fvs) = dtbl.get("global") + + for fv in fvs: + if fv[0] == speed: + rate = fv[1] + + assert rate != "" + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_SAMPLEPACKET") + (status, fvs) = atbl.get(sample_session) + + assert status == True + + rf = False + for fv in fvs: + if fv[0] == "SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE": + assert fv[1] == rate + rf = True + + assert rf == True + + def test_Teardown(self, dvs, testlog): + self.setup_sflow(dvs) + ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_TABLE") + ptbl._del("global") + + time.sleep(1) + + + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_SAMPLEPACKET") + assert len(atbl.getKeys()) == 0 From 1714af3e309d28cc771d01a2b82507cc2bc65327 Mon Sep 17 00:00:00 2001 From: dgsudharsan Date: Fri, 6 Sep 2019 11:16:33 -0700 Subject: [PATCH 2/8] Addressing review comments --- cfgmgr/sflowmgr.cpp | 130 ++++++++++++++++++++------------------- cfgmgr/sflowmgr.h | 3 +- orchagent/orchdaemon.cpp | 3 +- orchagent/sfloworch.cpp | 88 ++++++++++++++++---------- orchagent/sfloworch.h | 11 ++-- tests/test_sflow.py | 2 +- 6 files changed, 134 insertions(+), 103 deletions(-) diff --git a/cfgmgr/sflowmgr.cpp b/cfgmgr/sflowmgr.cpp index 7395adcfaa..4fcbadc8a0 100644 --- a/cfgmgr/sflowmgr.cpp +++ b/cfgmgr/sflowmgr.cpp @@ -10,6 +10,16 @@ using namespace std; using namespace swss; +const map sflowSpeedRateInitMap = +{ + {SFLOW_SAMPLE_RATE_KEY_400G, SFLOW_SAMPLE_RATE_VALUE_400G}, + {SFLOW_SAMPLE_RATE_KEY_100G, SFLOW_SAMPLE_RATE_VALUE_100G}, + {SFLOW_SAMPLE_RATE_KEY_50G, SFLOW_SAMPLE_RATE_VALUE_50G}, + {SFLOW_SAMPLE_RATE_KEY_40G, SFLOW_SAMPLE_RATE_VALUE_40G}, + {SFLOW_SAMPLE_RATE_KEY_25G, SFLOW_SAMPLE_RATE_VALUE_25G}, + {SFLOW_SAMPLE_RATE_KEY_10G, SFLOW_SAMPLE_RATE_VALUE_10G}, + {SFLOW_SAMPLE_RATE_KEY_1G, SFLOW_SAMPLE_RATE_VALUE_1G} +}; SflowMgr::SflowMgr(DBConnector *cfgDb, DBConnector *appDb, const vector &tableNames) : Orch(cfgDb, tableNames), m_cfgSflowTable(cfgDb, CFG_SFLOW_TABLE_NAME), @@ -20,80 +30,23 @@ SflowMgr::SflowMgr(DBConnector *cfgDb, DBConnector *appDb, const vector { vector fieldValues; - fieldValues.emplace_back(SFLOW_SAMPLE_RATE_KEY_400G, SFLOW_SAMPLE_RATE_VALUE_400G); - fieldValues.emplace_back(SFLOW_SAMPLE_RATE_KEY_100G, SFLOW_SAMPLE_RATE_VALUE_100G); - fieldValues.emplace_back(SFLOW_SAMPLE_RATE_KEY_50G, SFLOW_SAMPLE_RATE_VALUE_50G); - fieldValues.emplace_back(SFLOW_SAMPLE_RATE_KEY_40G, SFLOW_SAMPLE_RATE_VALUE_40G); - fieldValues.emplace_back(SFLOW_SAMPLE_RATE_KEY_25G, SFLOW_SAMPLE_RATE_VALUE_25G); - fieldValues.emplace_back(SFLOW_SAMPLE_RATE_KEY_10G, SFLOW_SAMPLE_RATE_VALUE_10G); - fieldValues.emplace_back(SFLOW_SAMPLE_RATE_KEY_1G, SFLOW_SAMPLE_RATE_VALUE_1G); + for (auto it : sflowSpeedRateInitMap) + { + fieldValues.emplace_back(it.first, it.second); + } m_appSflowSpeedRateTable.set("global", fieldValues); } -void SflowMgr::handleSflowTableConfig(Consumer &consumer) +void SflowMgr::doTask(Consumer &consumer) { stringstream cmd; string res; - auto it = consumer.m_toSync.begin(); - - while (it != consumer.m_toSync.end()) - { - auto t = it->second; - - string key = kfvKey(t); - string op = kfvOp(t); - auto values = kfvFieldsValues(t); - - if (op == SET_COMMAND) - { - for (auto i : kfvFieldsValues(t)) - { - if (fvField(i) == "admin_state") - { - if (fvValue(i) == "enable") - { - cmd << "service hsflowd restart"; - } - else - { - cmd << "service hsflowd stop"; - } - - int ret = swss::exec(cmd.str(), res); - if (ret) - { - SWSS_LOG_ERROR("Command '%s' failed with rc %d", cmd.str().c_str(), ret); - } - else - { - SWSS_LOG_INFO("Command '%s' succeeded", cmd.str().c_str()); - } - } - } - m_appSflowTable.set(key, values); - } - else if(op == DEL_COMMAND) - { - m_appSflowTable.del(key); - } - it = consumer.m_toSync.erase(it); - } -} - -void SflowMgr::doTask(Consumer &consumer) -{ SWSS_LOG_ENTER(); auto table = consumer.getTableName(); - if(table == CFG_SFLOW_TABLE_NAME) - { - handleSflowTableConfig(consumer); - return; - } - auto it = consumer.m_toSync.begin(); while (it != consumer.m_toSync.end()) { @@ -105,11 +58,60 @@ void SflowMgr::doTask(Consumer &consumer) if (op == SET_COMMAND) { - m_appSflowSessionTable.set(key, values); + if (table == CFG_SFLOW_TABLE_NAME) + { + for (auto i : values) + { + if (fvField(i) == "admin_state") + { + if (fvValue(i) == "enable") + { + cmd << "service hsflowd restart"; + } + else + { + cmd << "service hsflowd stop"; + } + + int ret = swss::exec(cmd.str(), res); + if (ret) + { + SWSS_LOG_ERROR("Command '%s' failed with rc %d", cmd.str().c_str(), ret); + } + else + { + SWSS_LOG_INFO("Command '%s' succeeded", cmd.str().c_str()); + } + } + } + m_appSflowTable.set(key, values); + } + else + { + m_appSflowSessionTable.set(key, values); + } } else if (op == DEL_COMMAND) { - m_appSflowSessionTable.del(key); + if (table == CFG_SFLOW_TABLE_NAME) + { + cmd << "service hsflowd stop"; + + int ret = swss::exec(cmd.str(), res); + if (ret) + { + SWSS_LOG_ERROR("Command '%s' failed with rc %d", cmd.str().c_str(), ret); + } + else + { + SWSS_LOG_INFO("Command '%s' succeeded", cmd.str().c_str()); + } + m_appSflowTable.del(key); + } + else + { + m_appSflowSessionTable.del(key); + } } it = consumer.m_toSync.erase(it); diff --git a/cfgmgr/sflowmgr.h b/cfgmgr/sflowmgr.h index 2f399534eb..d2d63e3834 100644 --- a/cfgmgr/sflowmgr.h +++ b/cfgmgr/sflowmgr.h @@ -39,12 +39,11 @@ class SflowMgr : public Orch private: Table m_cfgSflowTable; Table m_cfgSflowSessionTable; - Table m_appSflowSpeedRateTable; + ProducerStateTable m_appSflowSpeedRateTable; ProducerStateTable m_appSflowTable; ProducerStateTable m_appSflowSessionTable; void doTask(Consumer &consumer); - void handleSflowTableConfig(Consumer &consumer); }; } diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 1974971cfa..638c3d0cb1 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -179,7 +179,8 @@ bool OrchDaemon::init() vector sflow_tables = { APP_SFLOW_TABLE_NAME, - APP_SFLOW_SESSION_TABLE_NAME + APP_SFLOW_SESSION_TABLE_NAME, + APP_SFLOW_SAMPLE_RATE_TABLE_NAME }; SflowOrch *sflow_orch = new SflowOrch(m_applDb, sflow_tables); diff --git a/orchagent/sfloworch.cpp b/orchagent/sfloworch.cpp index c369a1382a..ddaeed5b50 100644 --- a/orchagent/sfloworch.cpp +++ b/orchagent/sfloworch.cpp @@ -14,18 +14,15 @@ extern PortsOrch* gPortsOrch; bool SflowOrch::sflowGetDefaultSampleRate(Port port, uint32_t &rate) { - string speed_str = to_string(port.m_speed); - string rate_str; - bool ret = m_sflowSampleRateTable->hget("global", speed_str, rate_str); + auto speedRate = m_speedRateMap.find(port.m_speed); - - if(ret == false) + if (speedRate == m_speedRateMap.end()) { SWSS_LOG_ERROR("Unable to find default rate for speed %d", port.m_speed); return false; } - rate = (uint32_t)stoul(rate_str); + rate = speedRate->second; return true; } @@ -33,7 +30,6 @@ SflowOrch::SflowOrch(DBConnector* db, vector &tableNames) : Orch(db, tableNames) { SWSS_LOG_ENTER(); - m_sflowSampleRateTable = unique_ptr
(new Table(db, APP_SFLOW_SAMPLE_RATE_TABLE_NAME)); gEnable = true; sflowStatus = false; } @@ -49,7 +45,7 @@ bool SflowOrch::sflowCreateSession(SflowSession &session) sai_rc = sai_samplepacket_api->create_samplepacket(&session_id, gSwitchId, 1, &attr); - if(sai_rc != SAI_STATUS_SUCCESS) + if (sai_rc != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to create sample packet session with rate %d", session.rate); @@ -65,7 +61,7 @@ bool SflowOrch::sflowDestroySession(SflowSession &session) sai_status_t sai_rc; sai_rc = sai_samplepacket_api->remove_samplepacket(session.m_sample_id); - if(sai_rc != SAI_STATUS_SUCCESS) + if (sai_rc != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to destroy sample packet session with id %lx", session.m_sample_id); @@ -157,7 +153,6 @@ bool SflowOrch::sflowDelPort(SflowSession &session, sai_object_id_t port_id) sai_attribute_t attr; sai_status_t sai_rc; - if (!sflowStatus) { return true; @@ -168,7 +163,7 @@ bool SflowOrch::sflowDelPort(SflowSession &session, sai_object_id_t port_id) sai_rc = sai_port_api->set_port_attribute(port_id, &attr); - if(sai_rc != SAI_STATUS_SUCCESS) + if (sai_rc != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to delete session %lx on port %lx", session.m_sample_id, port_id); @@ -184,12 +179,15 @@ bool SflowOrch::sflowGlobalConfigure(bool enable) for(auto& pair: gPortsOrch->getAllPorts()) { auto& port = pair.second; - if (port.m_type != Port::PHY) continue; + if (port.m_type != Port::PHY) + { + continue; + } auto sflowInfo = m_sflowPortSessionMap.find(port.m_port_id); - if(sflowInfo == m_sflowPortSessionMap.end()) + if (sflowInfo == m_sflowPortSessionMap.end()) { - if(!enable) + if (!enable) { continue; } @@ -241,13 +239,13 @@ bool SflowOrch::sflowGlobalConfigure(bool enable) } else { - if(!sflowInfo->second.globalConfigured) + if (!sflowInfo->second.globalConfigured) { continue; } - if(sflowInfo->second.adminState != enable) + if (sflowInfo->second.adminState != enable) { - if(!enable) + if (!enable) { if (sflowDelPort(sflowInfo->second, port.m_port_id)) { @@ -356,11 +354,11 @@ bool SflowOrch::sflowPortApplyGlobalSetting(Port port, SflowSession &session) } } - if(session.adminState != gEnable) + if (session.adminState != gEnable) { if (gEnable) { - if(!sflowAddPort(session, port.m_port_id)) + if (!sflowAddPort(session, port.m_port_id)) { SWSS_LOG_ERROR("Updating port with session %lx failed", session.m_sample_id); return false; @@ -368,7 +366,7 @@ bool SflowOrch::sflowPortApplyGlobalSetting(Port port, SflowSession &session) } else { - if(!sflowDelPort(session, port.m_port_id)) + if (!sflowDelPort(session, port.m_port_id)) { SWSS_LOG_ERROR("Updating port with session %lx failed", session.m_sample_id); return false; @@ -395,7 +393,7 @@ bool SflowOrch::handleSflowStatus(KeyOpFieldsValuesTuple tuple) { if (fvValue(i) == "enable") { - if(sflowStatus) + if (sflowStatus) { continue; } @@ -403,7 +401,7 @@ bool SflowOrch::handleSflowStatus(KeyOpFieldsValuesTuple tuple) } else if (fvValue(i) == "disable") { - if(!sflowStatus) + if (!sflowStatus) { continue; } @@ -439,15 +437,15 @@ bool SflowOrch::handleGlobalConfig(KeyOpFieldsValuesTuple tuple) { if (fvField(i) == "admin_state") { - if(fvValue(i) == "enable") + if (fvValue(i) == "enable") { gEnable = true; } - else if(fvValue(i) == "disable") + else if (fvValue(i) == "disable") { gEnable = false; } - if(!sflowGlobalConfigure(gEnable)) + if (!sflowGlobalConfigure(gEnable)) { ret = false; continue; @@ -492,6 +490,30 @@ void SflowOrch::doSflowStatusTask(Consumer &consumer) } } +void SflowOrch::sflowUpdateSpeedRateMap(Consumer &consumer) +{ + auto it = consumer.m_toSync.begin(); + uint32_t speed = 0; + uint32_t rate = 0; + + while (it != consumer.m_toSync.end()) + { + auto tuple = it->second; + string op = kfvOp(tuple); + + for (auto i : kfvFieldsValues(tuple)) + { + speed = (uint32_t)stoul(fvField(i)); + rate = (uint32_t)stoul(fvValue(i)); + if (op == SET_COMMAND) + { + m_speedRateMap[speed] = rate; + } + } + it = consumer.m_toSync.erase(it); + } +} + void SflowOrch::doTask(Consumer &consumer) { SWSS_LOG_ENTER(); @@ -504,7 +526,11 @@ void SflowOrch::doTask(Consumer &consumer) return; } - if (table_name == APP_SFLOW_TABLE_NAME) + if (table_name == APP_SFLOW_SAMPLE_RATE_TABLE_NAME) + { + sflowUpdateSpeedRateMap(consumer); + } + else if (table_name == APP_SFLOW_TABLE_NAME) { doSflowStatusTask(consumer); return; @@ -551,14 +577,14 @@ void SflowOrch::doTask(Consumer &consumer) { adminState = true; } - else if(fvValue(i) == "disable") + else if (fvValue(i) == "disable") { adminState = false; } adminSet = true; } - if(fvField(i) == "sample_rate") + if (fvField(i) == "sample_rate") { rate = (uint32_t)stoul(fvValue(i)); rateSet = true; @@ -612,7 +638,7 @@ void SflowOrch::doTask(Consumer &consumer) } if ((adminSet) && (adminState != sflowInfo->second.adminState)) { - if(adminState) + if (adminState) { if (!sflowAddPort(sflowInfo->second, port.m_port_id)) { @@ -639,13 +665,13 @@ void SflowOrch::doTask(Consumer &consumer) auto sflowInfo = m_sflowPortSessionMap.find(port.m_port_id); if (sflowInfo != m_sflowPortSessionMap.end()) { - if(gEnable) + if (gEnable) { sflowPortApplyGlobalSetting(port, sflowInfo->second); it = consumer.m_toSync.erase(it); continue; } - if(sflowInfo->second.adminState) + if (sflowInfo->second.adminState) { if (!sflowDelPort(sflowInfo->second, port.m_port_id)) { diff --git a/orchagent/sfloworch.h b/orchagent/sfloworch.h index e3442b5bc0..b4ea38f833 100644 --- a/orchagent/sfloworch.h +++ b/orchagent/sfloworch.h @@ -25,6 +25,9 @@ typedef map SflowRateSampleMap; /* SAI Samplesession object to reference(number of ports referencing this object) map*/ typedef map SflowSampleRefMap; +/* Speed to Samplerate map*/ +typedef map SflowSpeedRateMap; + class SflowOrch : public Orch { public: @@ -34,10 +37,9 @@ class SflowOrch : public Orch SflowPortSessionMap m_sflowPortSessionMap; SflowRateSampleMap m_sflowRateSampleMap; SflowSampleRefMap m_sflowSampleRefMap; - unique_ptr
m_sflowDefaults; - bool gEnable; - bool sflowStatus; - unique_ptr
m_sflowSampleRateTable; + SflowSpeedRateMap m_speedRateMap; + bool gEnable; + bool sflowStatus; virtual void doTask(Consumer& consumer); void doSflowStatusTask(Consumer &consumer); @@ -52,4 +54,5 @@ class SflowOrch : public Orch bool sflowGlobalConfigure(bool enable); bool sflowPortApplyGlobalSetting(Port port, SflowSession &session); bool sflowUpdateRate(sai_object_id_t port_id, uint32_t rate); + void sflowUpdateSpeedRateMap(Consumer &consumer); }; diff --git a/tests/test_sflow.py b/tests/test_sflow.py index fde226813c..2db9f47af7 100644 --- a/tests/test_sflow.py +++ b/tests/test_sflow.py @@ -7,7 +7,7 @@ class TestSflow(object): def setup_sflow(self, dvs): self.pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0) self.adb = swsscommon.DBConnector(1, dvs.redis_sock, 0) - rtbl = swsscommon.Table(self.pdb, "SFLOW_SAMPLE_RATE_TABLE") + rtbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_SAMPLE_RATE_TABLE") fvs = swsscommon.FieldValuePairs([("100000", "10000"), ("40000", "4000"), ("10000", "1000"), From 5d20b8ef9202323ee9aba8d05515d3469e419529 Mon Sep 17 00:00:00 2001 From: dgsudharsan Date: Thu, 3 Oct 2019 22:38:25 -0700 Subject: [PATCH 3/8] Addressig review comments --- cfgmgr/sflowmgr.cpp | 321 ++++++++++++++++++--- cfgmgr/sflowmgr.h | 37 ++- cfgmgr/sflowmgrd.cpp | 3 +- orchagent/sfloworch.cpp | 599 +++++++++------------------------------- orchagent/sfloworch.h | 47 ++-- tests/test_sflow.py | 216 ++------------- 6 files changed, 476 insertions(+), 747 deletions(-) diff --git a/cfgmgr/sflowmgr.cpp b/cfgmgr/sflowmgr.cpp index 4fcbadc8a0..03d477f5a9 100644 --- a/cfgmgr/sflowmgr.cpp +++ b/cfgmgr/sflowmgr.cpp @@ -10,7 +10,7 @@ using namespace std; using namespace swss; -const map sflowSpeedRateInitMap = +map sflowSpeedRateInitMap = { {SFLOW_SAMPLE_RATE_KEY_400G, SFLOW_SAMPLE_RATE_VALUE_400G}, {SFLOW_SAMPLE_RATE_KEY_100G, SFLOW_SAMPLE_RATE_VALUE_100G}, @@ -20,33 +20,233 @@ const map sflowSpeedRateInitMap = {SFLOW_SAMPLE_RATE_KEY_10G, SFLOW_SAMPLE_RATE_VALUE_10G}, {SFLOW_SAMPLE_RATE_KEY_1G, SFLOW_SAMPLE_RATE_VALUE_1G} }; + SflowMgr::SflowMgr(DBConnector *cfgDb, DBConnector *appDb, const vector &tableNames) : Orch(cfgDb, tableNames), m_cfgSflowTable(cfgDb, CFG_SFLOW_TABLE_NAME), m_cfgSflowSessionTable(cfgDb, CFG_SFLOW_SESSION_TABLE_NAME), m_appSflowTable(appDb, APP_SFLOW_TABLE_NAME), - m_appSflowSessionTable(appDb, APP_SFLOW_SESSION_TABLE_NAME), - m_appSflowSpeedRateTable(appDb, APP_SFLOW_SAMPLE_RATE_TABLE_NAME) + m_appSflowSessionTable(appDb, APP_SFLOW_SESSION_TABLE_NAME) { - vector fieldValues; + intf_all_conf = true; + gEnable = false; +} + +void SflowMgr::sflowHandleService(bool enable) +{ + stringstream cmd; + string res; + + SWSS_LOG_ENTER(); - for (auto it : sflowSpeedRateInitMap) + if (enable) + { + cmd << "service hsflowd restart"; + } + else { - fieldValues.emplace_back(it.first, it.second); + cmd << "service hsflowd stop"; } - m_appSflowSpeedRateTable.set("global", fieldValues); + int ret = swss::exec(cmd.str(), res); + if (ret) + { + SWSS_LOG_ERROR("Command '%s' failed with rc %d", cmd.str().c_str(), ret); + } + else + { + SWSS_LOG_INFO("Command '%s' succeeded", cmd.str().c_str()); + } + } -void SflowMgr::doTask(Consumer &consumer) +void SflowMgr::sflowUpdatePortInfo(Consumer &consumer) { - stringstream cmd; - string res; + auto it = consumer.m_toSync.begin(); + + while (it != consumer.m_toSync.end()) + { + KeyOpFieldsValuesTuple t = it->second; + + string key = kfvKey(t); + string op = kfvOp(t); + auto values = kfvFieldsValues(t); + + if (op == SET_COMMAND) + { + SflowLocalPortInfo port_info; + bool new_port = false; + + auto sflowPortConf = m_sflowPortLocalConfMap.find(key); + if (sflowPortConf == m_sflowPortLocalConfMap.end()) + { + new_port = true; + port_info.local_conf = false; + port_info.speed = SFLOW_ERROR_SPEED_STR; + port_info.local_rate = ""; + port_info.local_admin = ""; + m_sflowPortLocalConfMap[key] = port_info; + } + for (auto i : values) + { + if (fvField(i) == "speed") + { + m_sflowPortLocalConfMap[key].speed = fvValue(i); + } + } + + if (new_port) + { + if (gEnable && intf_all_conf) + { + vector fvs; + sflowGetGlobalFvs(fvs, m_sflowPortLocalConfMap[key].speed); + m_appSflowSessionTable.set(key, fvs); + } + } + } + else if (op == DEL_COMMAND) + { + auto sflowPortConf = m_sflowPortLocalConfMap.find(key); + if (sflowPortConf != m_sflowPortLocalConfMap.end()) + { + bool local_cfg = m_sflowPortLocalConfMap[key].local_conf; + + m_sflowPortLocalConfMap.erase(key); + if ((intf_all_conf && gEnable) || local_cfg) + { + m_appSflowSessionTable.del(key); + } + } + } + it = consumer.m_toSync.erase(it); + } +} + +void SflowMgr::sflowHandleSessionAll(bool enable) +{ + for (auto it: m_sflowPortLocalConfMap) + { + if (!it.second.local_conf) + { + vector fvs; + sflowGetGlobalFvs(fvs, it.second.speed); + if (enable) + { + m_appSflowSessionTable.set(it.first, fvs); + } + else + { + m_appSflowSessionTable.del(it.first); + } + } + } +} + +void SflowMgr::sflowHandleSessionLocal(bool enable) +{ + for (auto it: m_sflowPortLocalConfMap) + { + if (it.second.local_conf) + { + vector fvs; + sflowGetLocalFvs(fvs, it.second); + if (enable) + { + m_appSflowSessionTable.set(it.first, fvs); + } + else + { + m_appSflowSessionTable.del(it.first); + } + } + } +} +void SflowMgr::sflowGetGlobalFvs(vector &fvs, string speed) +{ + string rate; + FieldValueTuple fv1("admin_state", "enable"); + fvs.push_back(fv1); + + if (speed != SFLOW_ERROR_SPEED_STR) + { + rate = sflowSpeedRateInitMap[speed]; + } + else + { + rate = SFLOW_ERROR_SPEED_STR; + } + FieldValueTuple fv2("sample_rate",rate); + fvs.push_back(fv2); +} + +void SflowMgr::sflowGetLocalFvs(vector &fvs, SflowLocalPortInfo &local_info) +{ + if (local_info.local_admin.length() > 0) + { + FieldValueTuple fv1("admin_state", local_info.local_admin); + fvs.push_back(fv1); + } + + FieldValueTuple fv2("sample_rate", local_info.local_rate); + fvs.push_back(fv2); +} + +void SflowMgr::sflowUpdateLocalPortInfo(string alias, vector &fvs) +{ + for (auto i : fvs) + { + if (fvField(i) == "admin_state") + { + m_sflowPortLocalConfMap[alias].local_admin = fvValue(i); + } + else if (fvField(i) == "sample_rate") + { + m_sflowPortLocalConfMap[alias].local_rate = fvValue(i); + } + } +} + +void SflowMgr::sflowCheckAndFillRate(string alias, vector &fvs) +{ + string rate; + + for (auto i : fvs) + { + if (fvField(i) == "sample_rate") + { + /* Rate exists already. */ + return; + } + } + string speed = m_sflowPortLocalConfMap[alias].speed; + + if (speed != SFLOW_ERROR_SPEED_STR) + { + rate = sflowSpeedRateInitMap[speed]; + } + else + { + rate = SFLOW_ERROR_SPEED_STR; + } + m_sflowPortLocalConfMap[alias].local_rate = rate; + FieldValueTuple fv("sample_rate",rate); + fvs.push_back(fv); +} + +void SflowMgr::doTask(Consumer &consumer) +{ SWSS_LOG_ENTER(); auto table = consumer.getTableName(); + if (table == CFG_PORT_TABLE_NAME) + { + sflowUpdatePortInfo(consumer); + return; + } + auto it = consumer.m_toSync.begin(); while (it != consumer.m_toSync.end()) { @@ -64,56 +264,107 @@ void SflowMgr::doTask(Consumer &consumer) { if (fvField(i) == "admin_state") { + bool enable = false; if (fvValue(i) == "enable") { - cmd << "service hsflowd restart"; - } - else - { - cmd << "service hsflowd stop"; + enable = true; } - - int ret = swss::exec(cmd.str(), res); - if (ret) + if (enable == gEnable) { - SWSS_LOG_ERROR("Command '%s' failed with rc %d", cmd.str().c_str(), ret); + break; } - else + gEnable = enable; + sflowHandleService(enable); + if (intf_all_conf) { - SWSS_LOG_INFO("Command '%s' succeeded", cmd.str().c_str()); + sflowHandleSessionAll(enable); } + sflowHandleSessionLocal(enable); } } m_appSflowTable.set(key, values); } - else + else if (table == CFG_SFLOW_SESSION_TABLE_NAME) { - m_appSflowSessionTable.set(key, values); + if (key == "all") + { + for (auto i : values) + { + if (fvField(i) == "admin_state") + { + bool enable = false; + + if (fvValue(i) == "enable") + { + enable = true; + } + if ((enable != intf_all_conf) && (gEnable)) + { + sflowHandleSessionAll(enable); + } + intf_all_conf = enable; + } + } + } + else + { + auto sflowPortConf = m_sflowPortLocalConfMap.find(key); + + if (sflowPortConf == m_sflowPortLocalConfMap.end()) + { + it++; + continue; + } + if ((m_sflowPortLocalConfMap[key].local_rate == "") || + (m_sflowPortLocalConfMap[key].local_rate == SFLOW_ERROR_SPEED_STR)) + { + sflowCheckAndFillRate(key,values); + } + sflowUpdateLocalPortInfo(key,values); + m_sflowPortLocalConfMap[key].local_conf = true; + m_appSflowSessionTable.set(key, values); + } } } else if (op == DEL_COMMAND) { if (table == CFG_SFLOW_TABLE_NAME) { - cmd << "service hsflowd stop"; - - int ret = swss::exec(cmd.str(), res); - if (ret) - { - SWSS_LOG_ERROR("Command '%s' failed with rc %d", cmd.str().c_str(), ret); - } - else + if (gEnable) { - SWSS_LOG_INFO("Command '%s' succeeded", cmd.str().c_str()); + sflowHandleService(false); + sflowHandleSessionAll(false); } + gEnable = false; m_appSflowTable.del(key); } - else + else if (table == CFG_SFLOW_SESSION_TABLE_NAME) { - m_appSflowSessionTable.del(key); + if (key == "all") + { + if (!intf_all_conf) + { + sflowHandleSessionAll(true); + } + intf_all_conf = true; + } + else + { + m_appSflowSessionTable.del(key); + m_sflowPortLocalConfMap[key].local_conf = false; + m_sflowPortLocalConfMap[key].local_rate = ""; + m_sflowPortLocalConfMap[key].local_admin = ""; + + /* If Global configured, set global session on port after local config is deleted */ + if (intf_all_conf) + { + vector fvs; + sflowGetGlobalFvs(fvs, m_sflowPortLocalConfMap[key].speed); + m_appSflowSessionTable.set(key,fvs); + } + } } } - it = consumer.m_toSync.erase(it); } } diff --git a/cfgmgr/sflowmgr.h b/cfgmgr/sflowmgr.h index d2d63e3834..8e1d82c715 100644 --- a/cfgmgr/sflowmgr.h +++ b/cfgmgr/sflowmgr.h @@ -10,10 +10,6 @@ namespace swss { -/* Port default admin status is down */ -#define DEFAULT_ADMIN_STATUS_STR "down" -#define DEFAULT_MTU_STR "9100" - #define SFLOW_SAMPLE_RATE_KEY_400G "400000" #define SFLOW_SAMPLE_RATE_KEY_100G "100000" #define SFLOW_SAMPLE_RATE_KEY_50G "50000" @@ -30,6 +26,19 @@ namespace swss { #define SFLOW_SAMPLE_RATE_VALUE_10G "1000" #define SFLOW_SAMPLE_RATE_VALUE_1G "100" +#define SFLOW_ERROR_SPEED_STR "error" + +struct SflowLocalPortInfo +{ + bool local_conf; + string speed; + string local_rate; + string local_admin; +}; + +/* Port to Local config map */ +typedef map SflowPortLocalConfMap; + class SflowMgr : public Orch { public: @@ -37,13 +46,23 @@ class SflowMgr : public Orch using Orch::doTask; private: - Table m_cfgSflowTable; - Table m_cfgSflowSessionTable; - ProducerStateTable m_appSflowSpeedRateTable; - ProducerStateTable m_appSflowTable; - ProducerStateTable m_appSflowSessionTable; + Table m_cfgSflowTable; + Table m_cfgSflowSessionTable; + ProducerStateTable m_appSflowTable; + ProducerStateTable m_appSflowSessionTable; + SflowPortLocalConfMap m_sflowPortLocalConfMap; + bool intf_all_conf; + bool gEnable; void doTask(Consumer &consumer); + void sflowHandleService(bool enable); + void sflowUpdatePortInfo(Consumer &consumer); + void sflowHandleSessionAll(bool enable); + void sflowHandleSessionLocal(bool enable); + void sflowCheckAndFillRate(string alias, vector &fvs); + void sflowGetLocalFvs(vector &fvs, SflowLocalPortInfo &local_info); + void sflowGetGlobalFvs(vector &fvs, string speed); + void sflowUpdateLocalPortInfo(string alias, vector &fvs); }; } diff --git a/cfgmgr/sflowmgrd.cpp b/cfgmgr/sflowmgrd.cpp index 35630c5c51..343f0ead0a 100644 --- a/cfgmgr/sflowmgrd.cpp +++ b/cfgmgr/sflowmgrd.cpp @@ -42,7 +42,8 @@ int main(int argc, char **argv) { vector cfg_sflow_tables = { CFG_SFLOW_TABLE_NAME, - CFG_SFLOW_SESSION_TABLE_NAME + CFG_SFLOW_SESSION_TABLE_NAME, + CFG_PORT_TABLE_NAME }; DBConnector cfgDb(CONFIG_DB, DBConnector::DEFAULT_UNIXSOCKET, 0); diff --git a/orchagent/sfloworch.cpp b/orchagent/sfloworch.cpp index ddaeed5b50..97ba7eeb2a 100644 --- a/orchagent/sfloworch.cpp +++ b/orchagent/sfloworch.cpp @@ -5,54 +5,36 @@ using namespace std; using namespace swss; -extern sai_samplepacket_api_t* sai_samplepacket_api; -extern sai_port_api_t* sai_port_api; - -extern sai_object_id_t gSwitchId; -extern PortsOrch* gPortsOrch; - - -bool SflowOrch::sflowGetDefaultSampleRate(Port port, uint32_t &rate) -{ - auto speedRate = m_speedRateMap.find(port.m_speed); - - if (speedRate == m_speedRateMap.end()) - { - SWSS_LOG_ERROR("Unable to find default rate for speed %d", port.m_speed); - return false; - } - - rate = speedRate->second; - return true; -} +extern sai_samplepacket_api_t* sai_samplepacket_api; +extern sai_port_api_t* sai_port_api; +extern sai_object_id_t gSwitchId; +extern PortsOrch* gPortsOrch; SflowOrch::SflowOrch(DBConnector* db, vector &tableNames) : Orch(db, tableNames) { SWSS_LOG_ENTER(); - gEnable = true; sflowStatus = false; } -bool SflowOrch::sflowCreateSession(SflowSession &session) +bool SflowOrch::sflowCreateSession(uint32_t rate, SflowSession &session) { sai_attribute_t attr; sai_object_id_t session_id = SAI_NULL_OBJECT_ID; sai_status_t sai_rc; attr.id = SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE; - attr.value.u32 = session.rate; + attr.value.u32 = rate; sai_rc = sai_samplepacket_api->create_samplepacket(&session_id, gSwitchId, 1, &attr); if (sai_rc != SAI_STATUS_SUCCESS) { - SWSS_LOG_ERROR("Failed to create sample packet session with rate %d", - session.rate); + SWSS_LOG_ERROR("Failed to create sample packet session with rate %d", rate); return false; } - session.m_sample_id = session_id; + session.ref_count = 0; return true; } @@ -67,472 +49,196 @@ bool SflowOrch::sflowDestroySession(SflowSession &session) session.m_sample_id); return false; } - return true; } bool SflowOrch::sflowUpdateRate(sai_object_id_t port_id, uint32_t rate) { - auto old_session = m_sflowPortSessionMap.find(port_id); - auto sample_obj = m_sflowRateSampleMap.find(rate); + auto port_info = m_sflowPortInfoMap.find(port_id); + auto session = m_sflowRateSampleMap.find(rate); SflowSession new_session; + uint32_t old_rate = sflowSessionGetRate(port_info->second.m_sample_id); - if (sample_obj == m_sflowRateSampleMap.end()) + if (session == m_sflowRateSampleMap.end()) { - new_session.rate = rate; - if (!sflowCreateSession(new_session)) + if (!sflowCreateSession(rate, new_session)) { SWSS_LOG_ERROR("Creating sflow session with rate %d failed", rate); return false; } - m_sflowRateSampleMap[rate] = new_session.m_sample_id; - m_sflowSampleRefMap[new_session.m_sample_id] = 0; + m_sflowRateSampleMap[rate] = new_session; } else { - new_session.m_sample_id = sample_obj->second; + new_session = session->second; } - if (old_session->second.adminState) + if (port_info->second.admin_state) { - if (!sflowAddPort(new_session, port_id)) + if (!sflowAddPort(new_session.m_sample_id, port_id)) { return false; } } + port_info->second.m_sample_id = new_session.m_sample_id; - m_sflowSampleRefMap[new_session.m_sample_id]++; - m_sflowSampleRefMap[old_session->second.m_sample_id]--; - - if (m_sflowSampleRefMap[old_session->second.m_sample_id] == 0) + m_sflowRateSampleMap[rate].ref_count++; + m_sflowRateSampleMap[old_rate].ref_count--; + if (m_sflowRateSampleMap[old_rate].ref_count == 0) { - if (!sflowDestroySession(old_session->second)) + if (!sflowDestroySession(m_sflowRateSampleMap[old_rate])) { SWSS_LOG_ERROR("Failed to clean old session %lx", - old_session->second.m_sample_id); + m_sflowRateSampleMap[old_rate].m_sample_id); } else { - m_sflowRateSampleMap.erase(old_session->second.rate); - m_sflowSampleRefMap.erase(old_session->second.m_sample_id); + m_sflowRateSampleMap.erase(old_rate); } } - new_session.globalConfigured = old_session->second.globalConfigured; - new_session.adminState = old_session->second.adminState; - - m_sflowPortSessionMap[port_id] = new_session; return true; } -bool SflowOrch::sflowAddPort(SflowSession &session, sai_object_id_t port_id) +bool SflowOrch::sflowAddPort(sai_object_id_t sample_id, sai_object_id_t port_id) { sai_attribute_t attr; sai_status_t sai_rc; - if (!sflowStatus) - { - return true; - } - attr.id = SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE; - attr.value.oid = session.m_sample_id; - + attr.value.oid = sample_id; sai_rc = sai_port_api->set_port_attribute(port_id, &attr); if (sai_rc != SAI_STATUS_SUCCESS) { - SWSS_LOG_ERROR("Failed to set session %lx on port %lx", - session.m_sample_id, port_id); + SWSS_LOG_ERROR("Failed to set session %lx on port %lx", sample_id, port_id); return false; } return true; } -bool SflowOrch::sflowDelPort(SflowSession &session, sai_object_id_t port_id) +bool SflowOrch::sflowDelPort(sai_object_id_t port_id) { sai_attribute_t attr; sai_status_t sai_rc; - if (!sflowStatus) - { - return true; - } - attr.id = SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE; attr.value.oid = SAI_NULL_OBJECT_ID; - sai_rc = sai_port_api->set_port_attribute(port_id, &attr); if (sai_rc != SAI_STATUS_SUCCESS) { - SWSS_LOG_ERROR("Failed to delete session %lx on port %lx", - session.m_sample_id, port_id); + SWSS_LOG_ERROR("Failed to delete session on port %lx", port_id); return false; } return true; } -bool SflowOrch::sflowGlobalConfigure(bool enable) +void SflowOrch::sflowExtractFvs(vector &fvs, bool &admin, uint32_t &rate) { - bool ret = true; - - for(auto& pair: gPortsOrch->getAllPorts()) + for (auto i : fvs) { - auto& port = pair.second; - if (port.m_type != Port::PHY) - { - continue; - } - - auto sflowInfo = m_sflowPortSessionMap.find(port.m_port_id); - if (sflowInfo == m_sflowPortSessionMap.end()) + if (fvField(i) == "admin_state") { - if (!enable) - { - continue; - } - SflowSession session; - uint32_t rate = 0; - - sflowGetDefaultSampleRate(port, rate); - session.rate = rate; - session.globalConfigured = true; - session.adminState = true; - auto e_sampleObj = m_sflowRateSampleMap.find(rate); - - if (e_sampleObj != m_sflowRateSampleMap.end()) + if (fvValue(i) == "enable") { - session.m_sample_id = e_sampleObj->second; - } - else + admin = true; + } + else if (fvValue(i) == "disable") { - if (!sflowCreateSession(session)) - { - SWSS_LOG_NOTICE("Creating sflow session with rate %d failed", rate); - ret = false; - continue; - } - m_sflowRateSampleMap[rate] = session.m_sample_id; - m_sflowSampleRefMap[session.m_sample_id] = 0; - } - - if (sflowAddPort(session, port.m_port_id)) - { - m_sflowPortSessionMap[port.m_port_id] = session; - m_sflowSampleRefMap[session.m_sample_id]++; - } - else - { - SWSS_LOG_ERROR("Failed to add port %s to global session", - port.m_alias.c_str()); - ret = false; - - if (m_sflowSampleRefMap[session.m_sample_id] == 0) - { - if (sflowDestroySession(session)) - { - m_sflowSampleRefMap.erase(session.m_sample_id); - m_sflowRateSampleMap.erase(rate); - } - } + admin = false; } } - else + else if (fvField(i) == "sample_rate") { - if (!sflowInfo->second.globalConfigured) + if (fvValue(i) != "error") { - continue; + rate = (uint32_t)stoul(fvValue(i)); } - if (sflowInfo->second.adminState != enable) - { - if (!enable) - { - if (sflowDelPort(sflowInfo->second, port.m_port_id)) - { - sflowInfo->second.adminState = enable; - } - else - { - SWSS_LOG_ERROR("Failed to disable global sflow on port %s", - port.m_alias.c_str()); - ret = false; - } - } - else - { - if (sflowAddPort(sflowInfo->second, port.m_port_id)) - { - sflowInfo->second.adminState = enable; - } - else - { - SWSS_LOG_ERROR("Failed to enable global sflow on port %s", - port.m_alias.c_str()); - ret = false; - } - } - } - } - } - return ret; -} - -bool SflowOrch::sflowStatusSet(bool enable, bool remove_session) -{ - bool ret = true; - sai_attribute_t attr; - sai_status_t sai_rc; - - attr.id = SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE; - - if (enable) - { - if (!sflowGlobalConfigure(gEnable)) - { - ret = false; - } - } - - for (auto& pair: gPortsOrch->getAllPorts()) - { - auto& port = pair.second; - if (port.m_type != Port::PHY) continue; - - auto sflowInfo = m_sflowPortSessionMap.find(port.m_port_id); - if (sflowInfo != m_sflowPortSessionMap.end()) - { - if (sflowInfo->second.adminState) - { - if (!enable) - { - attr.value.oid = SAI_NULL_OBJECT_ID; - } - else - { - attr.value.oid = sflowInfo->second.m_sample_id; - } - - sai_rc = sai_port_api->set_port_attribute(port.m_port_id, &attr); - - if (sai_rc != SAI_STATUS_SUCCESS) - { - ret = false; - SWSS_LOG_ERROR("Failed to re-configure sflow on port %s", - port.m_alias.c_str()); - } - } - - if (remove_session) + else { - m_sflowSampleRefMap[sflowInfo->second.m_sample_id]--; - if (m_sflowSampleRefMap[sflowInfo->second.m_sample_id] == 0) - { - if (sflowDestroySession(sflowInfo->second)) - { - m_sflowSampleRefMap.erase(sflowInfo->second.m_sample_id); - m_sflowRateSampleMap.erase(sflowInfo->second.rate); - } - } - m_sflowPortSessionMap.erase(port.m_port_id); + rate = 0; } } } - return ret; } -bool SflowOrch::sflowPortApplyGlobalSetting(Port port, SflowSession &session) +void SflowOrch::sflowStatusSet(Consumer &consumer) { - uint32_t rate = 0; - - sflowGetDefaultSampleRate(port, rate); + auto it = consumer.m_toSync.begin(); - if (rate != session.rate) + while (it != consumer.m_toSync.end()) { - if (!sflowUpdateRate(port.m_port_id, rate)) - { - return false; - } - } + auto tuple = it->second; + string op = kfvOp(tuple); + uint32_t rate = 0; - if (session.adminState != gEnable) - { - if (gEnable) + if (op == SET_COMMAND) { - if (!sflowAddPort(session, port.m_port_id)) - { - SWSS_LOG_ERROR("Updating port with session %lx failed", session.m_sample_id); - return false; - } + sflowExtractFvs (kfvFieldsValues(tuple), sflowStatus, rate); } - else + else if (op == DEL_COMMAND) { - if (!sflowDelPort(session, port.m_port_id)) - { - SWSS_LOG_ERROR("Updating port with session %lx failed", session.m_sample_id); - return false; - } - + sflowStatus = false; } + it = consumer.m_toSync.erase(it); } - session.adminState = gEnable; - session.globalConfigured = true; - - return true; } -bool SflowOrch::handleSflowStatus(KeyOpFieldsValuesTuple tuple) +uint32_t SflowOrch::sflowSessionGetRate(sai_object_id_t m_sample_id) { - bool ret = true; - string op = kfvOp(tuple); - - if (op == SET_COMMAND) - { - for (auto i : kfvFieldsValues(tuple)) - { - if (fvField(i) == "admin_state") - { - if (fvValue(i) == "enable") - { - if (sflowStatus) - { - continue; - } - sflowStatus = true; - } - else if (fvValue(i) == "disable") - { - if (!sflowStatus) - { - continue; - } - sflowStatus = false; - } - if (!sflowStatusSet(sflowStatus, false)) - { - ret = false; - continue; - } - } - } - } - else if (op == DEL_COMMAND) + for (auto it: m_sflowRateSampleMap) { - if (!sflowStatusSet(false, true)) + if (it.second.m_sample_id == m_sample_id) { - ret = false; + return it.first; } - sflowStatus = false; } - return ret; + return 0; } -bool SflowOrch::handleGlobalConfig(KeyOpFieldsValuesTuple tuple) +bool SflowOrch::handleSflowSessionDel(sai_object_id_t port_id) { - bool ret = true; - string op = kfvOp(tuple); + auto sflowInfo = m_sflowPortInfoMap.find(port_id); - if (op == SET_COMMAND) + if (sflowInfo != m_sflowPortInfoMap.end()) { - for (auto i : kfvFieldsValues(tuple)) + uint32_t rate = sflowSessionGetRate(sflowInfo->second.m_sample_id); + if (sflowInfo->second.admin_state) { - if (fvField(i) == "admin_state") + if (!sflowDelPort(port_id)) { - if (fvValue(i) == "enable") - { - gEnable = true; - } - else if (fvValue(i) == "disable") - { - gEnable = false; - } - if (!sflowGlobalConfigure(gEnable)) - { - ret = false; - continue; - } - } - else - { - ret = false; + return false; } + sflowInfo->second.admin_state = false; } - } - else if (op == DEL_COMMAND) - { - /* By default global configure is true*/ - if (!sflowGlobalConfigure(true)) - { - ret = false; - } - else - { - gEnable = true; - } - } - return ret; -} - -void SflowOrch::doSflowStatusTask(Consumer &consumer) -{ - auto it = consumer.m_toSync.begin(); - - while (it != consumer.m_toSync.end()) - { - auto tuple = it->second; - string op = kfvOp(tuple); - - if (!handleSflowStatus(tuple)) - { - it++; - continue; - } - it = consumer.m_toSync.erase(it); - } -} - -void SflowOrch::sflowUpdateSpeedRateMap(Consumer &consumer) -{ - auto it = consumer.m_toSync.begin(); - uint32_t speed = 0; - uint32_t rate = 0; - - while (it != consumer.m_toSync.end()) - { - auto tuple = it->second; - string op = kfvOp(tuple); - for (auto i : kfvFieldsValues(tuple)) + m_sflowPortInfoMap.erase(port_id); + m_sflowRateSampleMap[rate].ref_count--; + if (m_sflowRateSampleMap[rate].ref_count == 0) { - speed = (uint32_t)stoul(fvField(i)); - rate = (uint32_t)stoul(fvValue(i)); - if (op == SET_COMMAND) + if (!sflowDestroySession(m_sflowRateSampleMap[rate])) { - m_speedRateMap[speed] = rate; + return false; } + m_sflowRateSampleMap.erase(rate); } - it = consumer.m_toSync.erase(it); } + return true; } void SflowOrch::doTask(Consumer &consumer) { SWSS_LOG_ENTER(); - Port port; - + Port port; string table_name = consumer.getTableName(); - if (!gPortsOrch->allPortsReady()) + if (table_name == APP_SFLOW_TABLE_NAME) { + sflowStatusSet(consumer); return; } - - if (table_name == APP_SFLOW_SAMPLE_RATE_TABLE_NAME) - { - sflowUpdateSpeedRateMap(consumer); - } - else if (table_name == APP_SFLOW_TABLE_NAME) + if (!gPortsOrch->allPortsReady()) { - doSflowStatusTask(consumer); return; } @@ -541,160 +247,101 @@ void SflowOrch::doTask(Consumer &consumer) { auto tuple = it->second; string op = kfvOp(tuple); - string alias = kfvKey(tuple); - if (alias == "all") - { - if (handleGlobalConfig(tuple)) - { - it = consumer.m_toSync.erase(it); - } - else - { - it++; - } - continue; - } - gPortsOrch->getPort(alias, port); - if (op == SET_COMMAND) { - bool adminState = gEnable; + bool admin_state = false; uint32_t rate = 0; - bool rateSet = false; - bool adminSet = false; - - sflowGetDefaultSampleRate(port, rate); - - for (auto i : kfvFieldsValues(tuple)) + if (!sflowStatus) { - if (fvField(i) == "admin_state") - { - if (fvValue(i) == "enable") - { - adminState = true; - } - else if (fvValue(i) == "disable") - { - adminState = false; - } - adminSet = true; - } + return; + } + auto sflowInfo = m_sflowPortInfoMap.find(port.m_port_id); + if (sflowInfo != m_sflowPortInfoMap.end()) + { + rate = sflowSessionGetRate(sflowInfo->second.m_sample_id); + admin_state = sflowInfo->second.admin_state; + } - if (fvField(i) == "sample_rate") + sflowExtractFvs(kfvFieldsValues(tuple), admin_state, rate); + if (sflowInfo == m_sflowPortInfoMap.end()) + { + if (rate == 0) { - rate = (uint32_t)stoul(fvValue(i)); - rateSet = true; + it++; + continue; } - } - auto sflowInfo = m_sflowPortSessionMap.find(port.m_port_id); - if (sflowInfo == m_sflowPortSessionMap.end()) - { - SflowSession session; - - session.rate = rate; - session.adminState = adminState; - session.globalConfigured = false; - auto e_sampleObj = m_sflowRateSampleMap.find(rate); - - if (e_sampleObj != m_sflowRateSampleMap.end()) + SflowPortInfo port_info; + auto session_info = m_sflowRateSampleMap.find(rate); + if (session_info != m_sflowRateSampleMap.end()) { - session.m_sample_id = e_sampleObj->second; + port_info.m_sample_id = session_info->second.m_sample_id; } else { - if (!sflowCreateSession(session)) + SflowSession session; + if (!sflowCreateSession(rate, session)) { it++; continue; } - m_sflowRateSampleMap[rate] = session.m_sample_id; - m_sflowSampleRefMap[session.m_sample_id] = 0; + m_sflowRateSampleMap[rate] = session; + port_info.m_sample_id = session.m_sample_id; } - if (adminState == true) + if (admin_state) { - if (!sflowAddPort(session, port.m_port_id)) + if (!sflowAddPort(port_info.m_sample_id, port.m_port_id)) { it++; continue; } } - m_sflowPortSessionMap[port.m_port_id] = session; - m_sflowSampleRefMap[session.m_sample_id]++; + port_info.admin_state = admin_state; + m_sflowPortInfoMap[port.m_port_id] = port_info; + m_sflowRateSampleMap[rate].ref_count++; } else { - if ((rateSet) && (rate != sflowInfo->second.rate)) + if (rate != sflowSessionGetRate(sflowInfo->second.m_sample_id)) { - if (sflowUpdateRate(port.m_port_id, rate)) + if (!sflowUpdateRate(port.m_port_id, rate)) { it++; continue; } } - if ((adminSet) && (adminState != sflowInfo->second.adminState)) + if (admin_state != sflowInfo->second.admin_state) { - if (adminState) + bool ret = false; + if (admin_state) { - if (!sflowAddPort(sflowInfo->second, port.m_port_id)) - { - it++; - continue; - } + ret = sflowAddPort(sflowInfo->second.m_sample_id, port.m_port_id); } else { - if (!sflowDelPort(sflowInfo->second, port.m_port_id)) - { - it++; - continue; - } + ret = sflowDelPort(port.m_port_id); } - sflowInfo->second.adminState = adminState; + if (!ret) + { + it++; + continue; + } + sflowInfo->second.admin_state = admin_state; } - sflowInfo->second.globalConfigured = false; } - it = consumer.m_toSync.erase(it); } else if (op == DEL_COMMAND) { - auto sflowInfo = m_sflowPortSessionMap.find(port.m_port_id); - if (sflowInfo != m_sflowPortSessionMap.end()) + if (!handleSflowSessionDel(port.m_port_id)) { - if (gEnable) - { - sflowPortApplyGlobalSetting(port, sflowInfo->second); - it = consumer.m_toSync.erase(it); - continue; - } - if (sflowInfo->second.adminState) - { - if (!sflowDelPort(sflowInfo->second, port.m_port_id)) - { - it++; - continue; - } - sflowInfo->second.adminState = false; - } - - m_sflowPortSessionMap.erase(port.m_port_id); - m_sflowSampleRefMap[sflowInfo->second.m_sample_id]--; - if (m_sflowSampleRefMap[sflowInfo->second.m_sample_id] == 0) - { - if (!sflowDestroySession(sflowInfo->second)) - { - it++; - continue; - } - m_sflowSampleRefMap.erase(sflowInfo->second.m_sample_id); - m_sflowRateSampleMap.erase(sflowInfo->second.rate); - } + it++; + continue; } - it = consumer.m_toSync.erase(it); } + it = consumer.m_toSync.erase(it); } } diff --git a/orchagent/sfloworch.h b/orchagent/sfloworch.h index b4ea38f833..c6104114d3 100644 --- a/orchagent/sfloworch.h +++ b/orchagent/sfloworch.h @@ -8,25 +8,23 @@ using namespace std; -struct SflowSession +struct SflowPortInfo { - bool globalConfigured; - bool adminState; + bool admin_state; sai_object_id_t m_sample_id; - uint32_t rate; }; -/* SAI Port to Sflow session map */ -typedef map SflowPortSessionMap; - -/* Sample-rate(unsigned int) to SAI Samplesession object map */ -typedef map SflowRateSampleMap; +struct SflowSession +{ + sai_object_id_t m_sample_id; + uint32_t ref_count; +}; -/* SAI Samplesession object to reference(number of ports referencing this object) map*/ -typedef map SflowSampleRefMap; +/* SAI Port to Sflow Port Info Map */ +typedef map SflowPortInfoMap; -/* Speed to Samplerate map*/ -typedef map SflowSpeedRateMap; +/* Sample-rate(unsigned int) to Sflow session map */ +typedef map SflowRateSampleMap; class SflowOrch : public Orch { @@ -34,25 +32,18 @@ class SflowOrch : public Orch SflowOrch(DBConnector* db, vector &tableNames); private: - SflowPortSessionMap m_sflowPortSessionMap; + SflowPortInfoMap m_sflowPortInfoMap; SflowRateSampleMap m_sflowRateSampleMap; - SflowSampleRefMap m_sflowSampleRefMap; - SflowSpeedRateMap m_speedRateMap; - bool gEnable; bool sflowStatus; virtual void doTask(Consumer& consumer); - void doSflowStatusTask(Consumer &consumer); - bool sflowCreateSession(SflowSession &session); + bool sflowCreateSession(uint32_t rate, SflowSession &session); bool sflowDestroySession(SflowSession &session); - bool sflowAddPort(SflowSession &session_id, sai_object_id_t port_id); - bool sflowDelPort(SflowSession &session_id, sai_object_id_t port_id); - bool sflowStatusSet(bool enable, bool remove_session); - bool handleSflowStatus(KeyOpFieldsValuesTuple tuple); - bool handleGlobalConfig(KeyOpFieldsValuesTuple tuple); - bool sflowGetDefaultSampleRate(Port port, uint32_t &rate); - bool sflowGlobalConfigure(bool enable); - bool sflowPortApplyGlobalSetting(Port port, SflowSession &session); + bool sflowAddPort(sai_object_id_t sample_id, sai_object_id_t port_id); + bool sflowDelPort(sai_object_id_t port_id); + void sflowStatusSet(Consumer &consumer); bool sflowUpdateRate(sai_object_id_t port_id, uint32_t rate); - void sflowUpdateSpeedRateMap(Consumer &consumer); + uint32_t sflowSessionGetRate(sai_object_id_t sample_id); + bool handleSflowSessionDel(sai_object_id_t port_id); + void sflowExtractFvs(vector &fvs, bool &admin, uint32_t &rate); }; diff --git a/tests/test_sflow.py b/tests/test_sflow.py index 2db9f47af7..008637ea96 100644 --- a/tests/test_sflow.py +++ b/tests/test_sflow.py @@ -7,104 +7,44 @@ class TestSflow(object): def setup_sflow(self, dvs): self.pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0) self.adb = swsscommon.DBConnector(1, dvs.redis_sock, 0) - rtbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_SAMPLE_RATE_TABLE") - fvs = swsscommon.FieldValuePairs([("100000", "10000"), - ("40000", "4000"), - ("10000", "1000"), - ("1000", "100")]) - rtbl.set("global", fvs) - - ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_TABLE") fvs = swsscommon.FieldValuePairs([("admin_state", "enable")]) ptbl.set("global", fvs) time.sleep(1) - def test_defaultGlobal(self, dvs, testlog): + def test_SflowDisble(self, dvs, testlog): self.setup_sflow(dvs) - atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") - (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) - - assert status == True - - sample_session = "" - speed = "" - for fv in fvs: - if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": - sample_session = fv[1] - elif fv[0] == "SAI_PORT_ATTR_SPEED": - speed = fv[1] - - assert sample_session != "" - assert speed != "" - - - rate = "" - dtbl = swsscommon.Table(self.pdb, "SFLOW_SAMPLE_RATE_TABLE") - (status, fvs) = dtbl.get("global") - - for fv in fvs: - if fv[0] == speed: - rate = fv[1] - - assert rate != "" - - atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_SAMPLEPACKET") - (status, fvs) = atbl.get(sample_session) - - assert status == True - - for fv in fvs: - if fv[0] == "SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE": - assert fv[1] == rate - - ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_TABLE") + ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_SESSION_TABLE") + gtbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_TABLE") fvs = swsscommon.FieldValuePairs([("admin_state", "disable")]) - ptbl.set("global", fvs) + gtbl.set("global", fvs) time.sleep(1) - - atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") - (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) - - assert status == True - - sample_session = "" - speed = "" - for fv in fvs: - if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": - sample_session = fv[1] - - assert sample_session == "oid:0x0" - - def test_globalAll(self, dvs, testlog): - self.setup_sflow(dvs) - - ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_SESSION_TABLE") - fvs = swsscommon.FieldValuePairs([("admin_state", "disable")]) - ptbl.set("all", fvs) + fvs = swsscommon.FieldValuePairs([("admin_state", "enable"),("sample_rate","1000")]) + ptbl.set("Ethernet0", fvs) time.sleep(1) + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) assert status == True sample_session = "" - speed = "" for fv in fvs: if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": sample_session = fv[1] - assert sample_session == "oid:0x0" + assert sample_session == "" fvs = swsscommon.FieldValuePairs([("admin_state", "enable")]) - ptbl.set("all", fvs) + gtbl.set("global", fvs) time.sleep(1) + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) assert status == True @@ -114,25 +54,19 @@ def test_globalAll(self, dvs, testlog): if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": sample_session = fv[1] - assert sample_session != "" assert sample_session != "oid:0x0" + assert sample_session != "" - ptbl._del("all") - - time.sleep(1) - - (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) - + atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_SAMPLEPACKET") + (status, fvs) = atbl.get(sample_session) + assert status == True - sample_session = "" for fv in fvs: - if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": - sample_session = fv[1] - - assert sample_session != "" - assert sample_session != "oid:0x0" + if fv[0] == "SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE": + assert fv[1] == "1000" + ptbl._del("Ethernet0") def test_InterfaceSet(self, dvs, testlog): self.setup_sflow(dvs) @@ -164,87 +98,8 @@ def test_InterfaceSet(self, dvs, testlog): if fv[0] == "SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE": assert fv[1] == "1000" - fvs = swsscommon.FieldValuePairs([("admin_state", "disable")]) - ptbl.set("all", fvs) - - time.sleep(1) - - atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") - (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) - - assert status == True - - sample_session = "" - for fv in fvs: - if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": - sample_session = fv[1] - assert sample_session != "" - assert sample_session != "oid:0x0" - - fvs = swsscommon.FieldValuePairs([("admin_state", "disable")]) - gtbl.set("global", fvs) - - time.sleep(1) - - atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") - (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet0"]) - - assert status == True - - sample_session = "" - for fv in fvs: - if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": - sample_session = fv[1] - - assert sample_session == "oid:0x0" - ptbl._del("all") ptbl._del("Ethernet0") - def test_defaultRate(self, dvs, testlog): - self.setup_sflow(dvs) - ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_SESSION_TABLE") - fvs = swsscommon.FieldValuePairs([("admin_state", "enable")]) - ptbl.set("Ethernet4", fvs) - - time.sleep(1) - - atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") - (status, fvs) = atbl.get(dvs.asicdb.portnamemap["Ethernet4"]) - - assert status == True - - sample_session = "" - speed = "" - for fv in fvs: - if fv[0] == "SAI_PORT_ATTR_INGRESS_SAMPLEPACKET_ENABLE": - sample_session = fv[1] - elif fv[0] == "SAI_PORT_ATTR_SPEED": - speed = fv[1] - - assert sample_session != "" - assert speed != "" - - rate = "" - dtbl = swsscommon.Table(self.pdb, "SFLOW_SAMPLE_RATE_TABLE") - (status, fvs) = dtbl.get("global") - - for fv in fvs: - if fv[0] == speed: - rate = fv[1] - - assert rate != "" - - atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_SAMPLEPACKET") - (status, fvs) = atbl.get(sample_session) - - assert status == True - - for fv in fvs: - if fv[0] == "SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE": - assert fv[1] == rate - - ptbl._del("Ethernet4") - def test_ConfigDel(self, dvs, testlog): self.setup_sflow(dvs) ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_SESSION_TABLE") @@ -272,39 +127,4 @@ def test_ConfigDel(self, dvs, testlog): speed = fv[1] assert speed != "" - assert sample_session != "" - assert sample_session != "oid:0x0" - - rate = "" - dtbl = swsscommon.Table(self.pdb, "SFLOW_SAMPLE_RATE_TABLE") - (status, fvs) = dtbl.get("global") - - for fv in fvs: - if fv[0] == speed: - rate = fv[1] - - assert rate != "" - - atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_SAMPLEPACKET") - (status, fvs) = atbl.get(sample_session) - - assert status == True - - rf = False - for fv in fvs: - if fv[0] == "SAI_SAMPLEPACKET_ATTR_SAMPLE_RATE": - assert fv[1] == rate - rf = True - - assert rf == True - - def test_Teardown(self, dvs, testlog): - self.setup_sflow(dvs) - ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_TABLE") - ptbl._del("global") - - time.sleep(1) - - - atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_SAMPLEPACKET") - assert len(atbl.getKeys()) == 0 + assert sample_session == "oid:0x0" From 0a784d809e604775cf3a00c173cf5073bbd5e7df Mon Sep 17 00:00:00 2001 From: dgsudharsan Date: Thu, 3 Oct 2019 22:55:04 -0700 Subject: [PATCH 4/8] Fixing build --- cfgmgr/sflowmgr.h | 20 ++++++++++---------- orchagent/sfloworch.h | 10 ++++------ 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/cfgmgr/sflowmgr.h b/cfgmgr/sflowmgr.h index 8e1d82c715..cd13802b5e 100644 --- a/cfgmgr/sflowmgr.h +++ b/cfgmgr/sflowmgr.h @@ -30,19 +30,19 @@ namespace swss { struct SflowLocalPortInfo { - bool local_conf; - string speed; - string local_rate; - string local_admin; + bool local_conf; + std::string speed; + std::string local_rate; + std::string local_admin; }; /* Port to Local config map */ -typedef map SflowPortLocalConfMap; +typedef std::map SflowPortLocalConfMap; class SflowMgr : public Orch { public: - SflowMgr(DBConnector *cfgDb, DBConnector *appDb, const vector &tableNames); + SflowMgr(DBConnector *cfgDb, DBConnector *appDb, const std::vector &tableNames); using Orch::doTask; private: @@ -59,10 +59,10 @@ class SflowMgr : public Orch void sflowUpdatePortInfo(Consumer &consumer); void sflowHandleSessionAll(bool enable); void sflowHandleSessionLocal(bool enable); - void sflowCheckAndFillRate(string alias, vector &fvs); - void sflowGetLocalFvs(vector &fvs, SflowLocalPortInfo &local_info); - void sflowGetGlobalFvs(vector &fvs, string speed); - void sflowUpdateLocalPortInfo(string alias, vector &fvs); + void sflowCheckAndFillRate(std::string alias, std::vector &fvs); + void sflowGetLocalFvs(std::vector &fvs, SflowLocalPortInfo &local_info); + void sflowGetGlobalFvs(std::vector &fvs, std::string speed); + void sflowUpdateLocalPortInfo(std::string alias, std::ector &fvs); }; } diff --git a/orchagent/sfloworch.h b/orchagent/sfloworch.h index c6104114d3..0340b82798 100644 --- a/orchagent/sfloworch.h +++ b/orchagent/sfloworch.h @@ -6,8 +6,6 @@ #include "orch.h" #include "portsorch.h" -using namespace std; - struct SflowPortInfo { bool admin_state; @@ -21,15 +19,15 @@ struct SflowSession }; /* SAI Port to Sflow Port Info Map */ -typedef map SflowPortInfoMap; +typedef std::map SflowPortInfoMap; /* Sample-rate(unsigned int) to Sflow session map */ -typedef map SflowRateSampleMap; +typedef std::map SflowRateSampleMap; class SflowOrch : public Orch { public: - SflowOrch(DBConnector* db, vector &tableNames); + SflowOrch(DBConnector* db, std::vector &tableNames); private: SflowPortInfoMap m_sflowPortInfoMap; @@ -45,5 +43,5 @@ class SflowOrch : public Orch bool sflowUpdateRate(sai_object_id_t port_id, uint32_t rate); uint32_t sflowSessionGetRate(sai_object_id_t sample_id); bool handleSflowSessionDel(sai_object_id_t port_id); - void sflowExtractFvs(vector &fvs, bool &admin, uint32_t &rate); + void sflowExtractFvs(std::vector &fvs, bool &admin, uint32_t &rate); }; From d4a2b526210e45a48776fcf9f74707328fd3ed0f Mon Sep 17 00:00:00 2001 From: dgsudharsan Date: Thu, 3 Oct 2019 23:04:32 -0700 Subject: [PATCH 5/8] Fixing build --- cfgmgr/sflowmgr.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cfgmgr/sflowmgr.h b/cfgmgr/sflowmgr.h index cd13802b5e..b2684d3d17 100644 --- a/cfgmgr/sflowmgr.h +++ b/cfgmgr/sflowmgr.h @@ -62,7 +62,7 @@ class SflowMgr : public Orch void sflowCheckAndFillRate(std::string alias, std::vector &fvs); void sflowGetLocalFvs(std::vector &fvs, SflowLocalPortInfo &local_info); void sflowGetGlobalFvs(std::vector &fvs, std::string speed); - void sflowUpdateLocalPortInfo(std::string alias, std::ector &fvs); + void sflowUpdateLocalPortInfo(std::string alias, std::vector &fvs); }; } From 7674af79487edd10d9c28f00e74bdade28ff88d6 Mon Sep 17 00:00:00 2001 From: dgsudharsan Date: Fri, 11 Oct 2019 13:31:29 -0700 Subject: [PATCH 6/8] Addressing code review comments --- cfgmgr/sflowmgr.cpp | 120 +++++++++++++++++++++------------------- cfgmgr/sflowmgr.h | 15 +++-- orchagent/sfloworch.cpp | 18 +++--- orchagent/sfloworch.h | 4 +- 4 files changed, 81 insertions(+), 76 deletions(-) diff --git a/cfgmgr/sflowmgr.cpp b/cfgmgr/sflowmgr.cpp index 03d477f5a9..e6b4f3b6cf 100644 --- a/cfgmgr/sflowmgr.cpp +++ b/cfgmgr/sflowmgr.cpp @@ -28,8 +28,8 @@ SflowMgr::SflowMgr(DBConnector *cfgDb, DBConnector *appDb, const vector m_appSflowTable(appDb, APP_SFLOW_TABLE_NAME), m_appSflowSessionTable(appDb, APP_SFLOW_SESSION_TABLE_NAME) { - intf_all_conf = true; - gEnable = false; + m_intfAllConf = true; + m_gEnable = false; } void SflowMgr::sflowHandleService(bool enable) @@ -55,6 +55,7 @@ void SflowMgr::sflowHandleService(bool enable) } else { + SWSS_LOG_NOTICE("Starting hsflowd service"); SWSS_LOG_INFO("Command '%s' succeeded", cmd.str().c_str()); } @@ -83,8 +84,8 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer) new_port = true; port_info.local_conf = false; port_info.speed = SFLOW_ERROR_SPEED_STR; - port_info.local_rate = ""; - port_info.local_admin = ""; + port_info.rate = ""; + port_info.admin = ""; m_sflowPortLocalConfMap[key] = port_info; } for (auto i : values) @@ -97,10 +98,10 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer) if (new_port) { - if (gEnable && intf_all_conf) + if (m_gEnable && m_intfAllConf) { vector fvs; - sflowGetGlobalFvs(fvs, m_sflowPortLocalConfMap[key].speed); + sflowGetGlobalInfo(fvs, m_sflowPortLocalConfMap[key].speed); m_appSflowSessionTable.set(key, fvs); } } @@ -113,7 +114,7 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer) bool local_cfg = m_sflowPortLocalConfMap[key].local_conf; m_sflowPortLocalConfMap.erase(key); - if ((intf_all_conf && gEnable) || local_cfg) + if ((m_intfAllConf && m_gEnable) || local_cfg) { m_appSflowSessionTable.del(key); } @@ -130,7 +131,7 @@ void SflowMgr::sflowHandleSessionAll(bool enable) if (!it.second.local_conf) { vector fvs; - sflowGetGlobalFvs(fvs, it.second.speed); + sflowGetGlobalInfo(fvs, it.second.speed); if (enable) { m_appSflowSessionTable.set(it.first, fvs); @@ -150,7 +151,7 @@ void SflowMgr::sflowHandleSessionLocal(bool enable) if (it.second.local_conf) { vector fvs; - sflowGetLocalFvs(fvs, it.second); + sflowGetLocalPortInfo(fvs, it.second); if (enable) { m_appSflowSessionTable.set(it.first, fvs); @@ -163,7 +164,7 @@ void SflowMgr::sflowHandleSessionLocal(bool enable) } } -void SflowMgr::sflowGetGlobalFvs(vector &fvs, string speed) +void SflowMgr::sflowGetGlobalInfo(vector &fvs, string speed) { string rate; FieldValueTuple fv1("admin_state", "enable"); @@ -181,58 +182,68 @@ void SflowMgr::sflowGetGlobalFvs(vector &fvs, string speed) fvs.push_back(fv2); } -void SflowMgr::sflowGetLocalFvs(vector &fvs, SflowLocalPortInfo &local_info) +void SflowMgr::sflowGetLocalPortInfo(vector &fvs, SflowLocalPortInfo &local_info) { - if (local_info.local_admin.length() > 0) + if (local_info.admin.length() > 0) { - FieldValueTuple fv1("admin_state", local_info.local_admin); + FieldValueTuple fv1("admin_state", local_info.admin); fvs.push_back(fv1); } - FieldValueTuple fv2("sample_rate", local_info.local_rate); + FieldValueTuple fv2("sample_rate", local_info.rate); fvs.push_back(fv2); } -void SflowMgr::sflowUpdateLocalPortInfo(string alias, vector &fvs) +void SflowMgr::sflowCheckAndFillValues(string alias, vector &fvs) { + string rate; + bool admin_present = false; + bool rate_present = false; + for (auto i : fvs) { - if (fvField(i) == "admin_state") + if (fvField(i) == "sample_rate") { - m_sflowPortLocalConfMap[alias].local_admin = fvValue(i); + rate_present = true; + m_sflowPortLocalConfMap[alias].rate = fvValue(i); } - else if (fvField(i) == "sample_rate") + if (fvField(i) == "admin_state") { - m_sflowPortLocalConfMap[alias].local_rate = fvValue(i); + admin_present = true; + m_sflowPortLocalConfMap[alias].admin = fvValue(i); } } -} - -void SflowMgr::sflowCheckAndFillRate(string alias, vector &fvs) -{ - string rate; - for (auto i : fvs) + if (!rate_present) { - if (fvField(i) == "sample_rate") + if (m_sflowPortLocalConfMap[alias].rate == "") { - /* Rate exists already. */ - return; + string speed = m_sflowPortLocalConfMap[alias].speed; + + if (speed != SFLOW_ERROR_SPEED_STR) + { + rate = sflowSpeedRateInitMap[speed]; + } + else + { + rate = SFLOW_ERROR_SPEED_STR; + } + m_sflowPortLocalConfMap[alias].rate = rate; } + FieldValueTuple fv("sample_rate", m_sflowPortLocalConfMap[alias].rate); + fvs.push_back(fv); } - string speed = m_sflowPortLocalConfMap[alias].speed; - if (speed != SFLOW_ERROR_SPEED_STR) + if (!admin_present) { - rate = sflowSpeedRateInitMap[speed]; - } - else - { - rate = SFLOW_ERROR_SPEED_STR; + if (m_sflowPortLocalConfMap[alias].admin == "") + { + /* By default admin state is enable if not set explicitely */ + m_sflowPortLocalConfMap[alias].admin = "enable"; + } + FieldValueTuple fv("admin_state", m_sflowPortLocalConfMap[alias].admin); + fvs.push_back(fv); } - m_sflowPortLocalConfMap[alias].local_rate = rate; - FieldValueTuple fv("sample_rate",rate); - fvs.push_back(fv); } void SflowMgr::doTask(Consumer &consumer) @@ -269,13 +280,13 @@ void SflowMgr::doTask(Consumer &consumer) { enable = true; } - if (enable == gEnable) + if (enable == m_gEnable) { break; } - gEnable = enable; + m_gEnable = enable; sflowHandleService(enable); - if (intf_all_conf) + if (m_intfAllConf) { sflowHandleSessionAll(enable); } @@ -298,11 +309,11 @@ void SflowMgr::doTask(Consumer &consumer) { enable = true; } - if ((enable != intf_all_conf) && (gEnable)) + if ((enable != m_intfAllConf) && (m_gEnable)) { sflowHandleSessionAll(enable); } - intf_all_conf = enable; + m_intfAllConf = enable; } } } @@ -315,12 +326,7 @@ void SflowMgr::doTask(Consumer &consumer) it++; continue; } - if ((m_sflowPortLocalConfMap[key].local_rate == "") || - (m_sflowPortLocalConfMap[key].local_rate == SFLOW_ERROR_SPEED_STR)) - { - sflowCheckAndFillRate(key,values); - } - sflowUpdateLocalPortInfo(key,values); + sflowCheckAndFillValues(key,values); m_sflowPortLocalConfMap[key].local_conf = true; m_appSflowSessionTable.set(key, values); } @@ -330,36 +336,36 @@ void SflowMgr::doTask(Consumer &consumer) { if (table == CFG_SFLOW_TABLE_NAME) { - if (gEnable) + if (m_gEnable) { sflowHandleService(false); sflowHandleSessionAll(false); } - gEnable = false; + m_gEnable = false; m_appSflowTable.del(key); } else if (table == CFG_SFLOW_SESSION_TABLE_NAME) { if (key == "all") { - if (!intf_all_conf) + if (!m_intfAllConf) { sflowHandleSessionAll(true); } - intf_all_conf = true; + m_intfAllConf = true; } else { m_appSflowSessionTable.del(key); m_sflowPortLocalConfMap[key].local_conf = false; - m_sflowPortLocalConfMap[key].local_rate = ""; - m_sflowPortLocalConfMap[key].local_admin = ""; + m_sflowPortLocalConfMap[key].rate = ""; + m_sflowPortLocalConfMap[key].admin = ""; /* If Global configured, set global session on port after local config is deleted */ - if (intf_all_conf) + if (m_intfAllConf) { vector fvs; - sflowGetGlobalFvs(fvs, m_sflowPortLocalConfMap[key].speed); + sflowGetGlobalInfo(fvs, m_sflowPortLocalConfMap[key].speed); m_appSflowSessionTable.set(key,fvs); } } diff --git a/cfgmgr/sflowmgr.h b/cfgmgr/sflowmgr.h index b2684d3d17..e2e6106cf3 100644 --- a/cfgmgr/sflowmgr.h +++ b/cfgmgr/sflowmgr.h @@ -32,8 +32,8 @@ struct SflowLocalPortInfo { bool local_conf; std::string speed; - std::string local_rate; - std::string local_admin; + std::string rate; + std::string admin; }; /* Port to Local config map */ @@ -51,18 +51,17 @@ class SflowMgr : public Orch ProducerStateTable m_appSflowTable; ProducerStateTable m_appSflowSessionTable; SflowPortLocalConfMap m_sflowPortLocalConfMap; - bool intf_all_conf; - bool gEnable; + bool m_intfAllConf; + bool m_gEnable; void doTask(Consumer &consumer); void sflowHandleService(bool enable); void sflowUpdatePortInfo(Consumer &consumer); void sflowHandleSessionAll(bool enable); void sflowHandleSessionLocal(bool enable); - void sflowCheckAndFillRate(std::string alias, std::vector &fvs); - void sflowGetLocalFvs(std::vector &fvs, SflowLocalPortInfo &local_info); - void sflowGetGlobalFvs(std::vector &fvs, std::string speed); - void sflowUpdateLocalPortInfo(std::string alias, std::vector &fvs); + void sflowCheckAndFillValues(std::string alias, std::vector &fvs); + void sflowGetLocalPortInfo(std::vector &fvs, SflowLocalPortInfo &local_info); + void sflowGetGlobalInfo(std::vector &fvs, std::string speed); }; } diff --git a/orchagent/sfloworch.cpp b/orchagent/sfloworch.cpp index 97ba7eeb2a..251362afe2 100644 --- a/orchagent/sfloworch.cpp +++ b/orchagent/sfloworch.cpp @@ -14,7 +14,7 @@ SflowOrch::SflowOrch(DBConnector* db, vector &tableNames) : Orch(db, tableNames) { SWSS_LOG_ENTER(); - sflowStatus = false; + m_sflowStatus = false; } bool SflowOrch::sflowCreateSession(uint32_t rate, SflowSession &session) @@ -88,8 +88,8 @@ bool SflowOrch::sflowUpdateRate(sai_object_id_t port_id, uint32_t rate) { if (!sflowDestroySession(m_sflowRateSampleMap[old_rate])) { - SWSS_LOG_ERROR("Failed to clean old session %lx", - m_sflowRateSampleMap[old_rate].m_sample_id); + SWSS_LOG_ERROR("Failed to clean old session %lx with rate %d", + m_sflowRateSampleMap[old_rate].m_sample_id, old_rate); } else { @@ -133,7 +133,7 @@ bool SflowOrch::sflowDelPort(sai_object_id_t port_id) return true; } -void SflowOrch::sflowExtractFvs(vector &fvs, bool &admin, uint32_t &rate) +void SflowOrch::sflowExtractInfo(vector &fvs, bool &admin, uint32_t &rate) { for (auto i : fvs) { @@ -174,11 +174,11 @@ void SflowOrch::sflowStatusSet(Consumer &consumer) if (op == SET_COMMAND) { - sflowExtractFvs (kfvFieldsValues(tuple), sflowStatus, rate); + sflowExtractInfo(kfvFieldsValues(tuple), m_sflowStatus, rate); } else if (op == DEL_COMMAND) { - sflowStatus = false; + m_sflowStatus = false; } it = consumer.m_toSync.erase(it); } @@ -252,10 +252,10 @@ void SflowOrch::doTask(Consumer &consumer) gPortsOrch->getPort(alias, port); if (op == SET_COMMAND) { - bool admin_state = false; + bool admin_state = m_sflowStatus; uint32_t rate = 0; - if (!sflowStatus) + if (!m_sflowStatus) { return; } @@ -266,7 +266,7 @@ void SflowOrch::doTask(Consumer &consumer) admin_state = sflowInfo->second.admin_state; } - sflowExtractFvs(kfvFieldsValues(tuple), admin_state, rate); + sflowExtractInfo(kfvFieldsValues(tuple), admin_state, rate); if (sflowInfo == m_sflowPortInfoMap.end()) { if (rate == 0) diff --git a/orchagent/sfloworch.h b/orchagent/sfloworch.h index 0340b82798..ea63c092a4 100644 --- a/orchagent/sfloworch.h +++ b/orchagent/sfloworch.h @@ -32,7 +32,7 @@ class SflowOrch : public Orch private: SflowPortInfoMap m_sflowPortInfoMap; SflowRateSampleMap m_sflowRateSampleMap; - bool sflowStatus; + bool m_sflowStatus; virtual void doTask(Consumer& consumer); bool sflowCreateSession(uint32_t rate, SflowSession &session); @@ -43,5 +43,5 @@ class SflowOrch : public Orch bool sflowUpdateRate(sai_object_id_t port_id, uint32_t rate); uint32_t sflowSessionGetRate(sai_object_id_t sample_id); bool handleSflowSessionDel(sai_object_id_t port_id); - void sflowExtractFvs(std::vector &fvs, bool &admin, uint32_t &rate); + void sflowExtractInfo(std::vector &fvs, bool &admin, uint32_t &rate); }; From d31ccd13193cd1bd6fc9a0241c0c15176da56733 Mon Sep 17 00:00:00 2001 From: dgsudharsan Date: Fri, 11 Oct 2019 22:27:24 -0700 Subject: [PATCH 7/8] Renaming variables --- cfgmgr/sflowmgr.cpp | 60 ++++++++++++++++++++++----------------------- cfgmgr/sflowmgr.h | 8 +++--- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/cfgmgr/sflowmgr.cpp b/cfgmgr/sflowmgr.cpp index e6b4f3b6cf..2c040fb4bc 100644 --- a/cfgmgr/sflowmgr.cpp +++ b/cfgmgr/sflowmgr.cpp @@ -75,24 +75,24 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer) if (op == SET_COMMAND) { - SflowLocalPortInfo port_info; + SflowPortInfo port_info; bool new_port = false; - auto sflowPortConf = m_sflowPortLocalConfMap.find(key); - if (sflowPortConf == m_sflowPortLocalConfMap.end()) + auto sflowPortConf = m_sflowPortConfMap.find(key); + if (sflowPortConf == m_sflowPortConfMap.end()) { new_port = true; port_info.local_conf = false; port_info.speed = SFLOW_ERROR_SPEED_STR; port_info.rate = ""; port_info.admin = ""; - m_sflowPortLocalConfMap[key] = port_info; + m_sflowPortConfMap[key] = port_info; } for (auto i : values) { if (fvField(i) == "speed") { - m_sflowPortLocalConfMap[key].speed = fvValue(i); + m_sflowPortConfMap[key].speed = fvValue(i); } } @@ -101,19 +101,19 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer) if (m_gEnable && m_intfAllConf) { vector fvs; - sflowGetGlobalInfo(fvs, m_sflowPortLocalConfMap[key].speed); + sflowGetGlobalInfo(fvs, m_sflowPortConfMap[key].speed); m_appSflowSessionTable.set(key, fvs); } } } else if (op == DEL_COMMAND) { - auto sflowPortConf = m_sflowPortLocalConfMap.find(key); - if (sflowPortConf != m_sflowPortLocalConfMap.end()) + auto sflowPortConf = m_sflowPortConfMap.find(key); + if (sflowPortConf != m_sflowPortConfMap.end()) { - bool local_cfg = m_sflowPortLocalConfMap[key].local_conf; + bool local_cfg = m_sflowPortConfMap[key].local_conf; - m_sflowPortLocalConfMap.erase(key); + m_sflowPortConfMap.erase(key); if ((m_intfAllConf && m_gEnable) || local_cfg) { m_appSflowSessionTable.del(key); @@ -126,7 +126,7 @@ void SflowMgr::sflowUpdatePortInfo(Consumer &consumer) void SflowMgr::sflowHandleSessionAll(bool enable) { - for (auto it: m_sflowPortLocalConfMap) + for (auto it: m_sflowPortConfMap) { if (!it.second.local_conf) { @@ -146,12 +146,12 @@ void SflowMgr::sflowHandleSessionAll(bool enable) void SflowMgr::sflowHandleSessionLocal(bool enable) { - for (auto it: m_sflowPortLocalConfMap) + for (auto it: m_sflowPortConfMap) { if (it.second.local_conf) { vector fvs; - sflowGetLocalPortInfo(fvs, it.second); + sflowGetPortInfo(fvs, it.second); if (enable) { m_appSflowSessionTable.set(it.first, fvs); @@ -182,7 +182,7 @@ void SflowMgr::sflowGetGlobalInfo(vector &fvs, string speed) fvs.push_back(fv2); } -void SflowMgr::sflowGetLocalPortInfo(vector &fvs, SflowLocalPortInfo &local_info) +void SflowMgr::sflowGetPortInfo(vector &fvs, SflowPortInfo &local_info) { if (local_info.admin.length() > 0) { @@ -205,20 +205,20 @@ void SflowMgr::sflowCheckAndFillValues(string alias, vector &fv if (fvField(i) == "sample_rate") { rate_present = true; - m_sflowPortLocalConfMap[alias].rate = fvValue(i); + m_sflowPortConfMap[alias].rate = fvValue(i); } if (fvField(i) == "admin_state") { admin_present = true; - m_sflowPortLocalConfMap[alias].admin = fvValue(i); + m_sflowPortConfMap[alias].admin = fvValue(i); } } if (!rate_present) { - if (m_sflowPortLocalConfMap[alias].rate == "") + if (m_sflowPortConfMap[alias].rate == "") { - string speed = m_sflowPortLocalConfMap[alias].speed; + string speed = m_sflowPortConfMap[alias].speed; if (speed != SFLOW_ERROR_SPEED_STR) { @@ -228,20 +228,20 @@ void SflowMgr::sflowCheckAndFillValues(string alias, vector &fv { rate = SFLOW_ERROR_SPEED_STR; } - m_sflowPortLocalConfMap[alias].rate = rate; + m_sflowPortConfMap[alias].rate = rate; } - FieldValueTuple fv("sample_rate", m_sflowPortLocalConfMap[alias].rate); + FieldValueTuple fv("sample_rate", m_sflowPortConfMap[alias].rate); fvs.push_back(fv); } if (!admin_present) { - if (m_sflowPortLocalConfMap[alias].admin == "") + if (m_sflowPortConfMap[alias].admin == "") { /* By default admin state is enable if not set explicitely */ - m_sflowPortLocalConfMap[alias].admin = "enable"; + m_sflowPortConfMap[alias].admin = "enable"; } - FieldValueTuple fv("admin_state", m_sflowPortLocalConfMap[alias].admin); + FieldValueTuple fv("admin_state", m_sflowPortConfMap[alias].admin); fvs.push_back(fv); } } @@ -319,15 +319,15 @@ void SflowMgr::doTask(Consumer &consumer) } else { - auto sflowPortConf = m_sflowPortLocalConfMap.find(key); + auto sflowPortConf = m_sflowPortConfMap.find(key); - if (sflowPortConf == m_sflowPortLocalConfMap.end()) + if (sflowPortConf == m_sflowPortConfMap.end()) { it++; continue; } sflowCheckAndFillValues(key,values); - m_sflowPortLocalConfMap[key].local_conf = true; + m_sflowPortConfMap[key].local_conf = true; m_appSflowSessionTable.set(key, values); } } @@ -357,15 +357,15 @@ void SflowMgr::doTask(Consumer &consumer) else { m_appSflowSessionTable.del(key); - m_sflowPortLocalConfMap[key].local_conf = false; - m_sflowPortLocalConfMap[key].rate = ""; - m_sflowPortLocalConfMap[key].admin = ""; + m_sflowPortConfMap[key].local_conf = false; + m_sflowPortConfMap[key].rate = ""; + m_sflowPortConfMap[key].admin = ""; /* If Global configured, set global session on port after local config is deleted */ if (m_intfAllConf) { vector fvs; - sflowGetGlobalInfo(fvs, m_sflowPortLocalConfMap[key].speed); + sflowGetGlobalInfo(fvs, m_sflowPortConfMap[key].speed); m_appSflowSessionTable.set(key,fvs); } } diff --git a/cfgmgr/sflowmgr.h b/cfgmgr/sflowmgr.h index e2e6106cf3..16cd223798 100644 --- a/cfgmgr/sflowmgr.h +++ b/cfgmgr/sflowmgr.h @@ -28,7 +28,7 @@ namespace swss { #define SFLOW_ERROR_SPEED_STR "error" -struct SflowLocalPortInfo +struct SflowPortInfo { bool local_conf; std::string speed; @@ -37,7 +37,7 @@ struct SflowLocalPortInfo }; /* Port to Local config map */ -typedef std::map SflowPortLocalConfMap; +typedef std::map SflowPortConfMap; class SflowMgr : public Orch { @@ -50,7 +50,7 @@ class SflowMgr : public Orch Table m_cfgSflowSessionTable; ProducerStateTable m_appSflowTable; ProducerStateTable m_appSflowSessionTable; - SflowPortLocalConfMap m_sflowPortLocalConfMap; + SflowPortConfMap m_sflowPortConfMap; bool m_intfAllConf; bool m_gEnable; @@ -60,7 +60,7 @@ class SflowMgr : public Orch void sflowHandleSessionAll(bool enable); void sflowHandleSessionLocal(bool enable); void sflowCheckAndFillValues(std::string alias, std::vector &fvs); - void sflowGetLocalPortInfo(std::vector &fvs, SflowLocalPortInfo &local_info); + void sflowGetPortInfo(std::vector &fvs, SflowPortInfo &local_info); void sflowGetGlobalInfo(std::vector &fvs, std::string speed); }; From b34b48ee3055003cca37de492fe1cd2fb30e4873 Mon Sep 17 00:00:00 2001 From: Garrick He Date: Thu, 17 Oct 2019 14:15:57 -0700 Subject: [PATCH 8/8] Change admin_state * Change admin_state from enable/disable to up/down to match SONiC YANG. Signed-off-by: Garrick He --- cfgmgr/sflowmgr.cpp | 10 +++++----- orchagent/sfloworch.cpp | 10 +++++----- tests/test_sflow.py | 16 ++++++++-------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/cfgmgr/sflowmgr.cpp b/cfgmgr/sflowmgr.cpp index 2c040fb4bc..98d6a77756 100644 --- a/cfgmgr/sflowmgr.cpp +++ b/cfgmgr/sflowmgr.cpp @@ -58,7 +58,7 @@ void SflowMgr::sflowHandleService(bool enable) SWSS_LOG_NOTICE("Starting hsflowd service"); SWSS_LOG_INFO("Command '%s' succeeded", cmd.str().c_str()); } - + } void SflowMgr::sflowUpdatePortInfo(Consumer &consumer) @@ -167,7 +167,7 @@ void SflowMgr::sflowHandleSessionLocal(bool enable) void SflowMgr::sflowGetGlobalInfo(vector &fvs, string speed) { string rate; - FieldValueTuple fv1("admin_state", "enable"); + FieldValueTuple fv1("admin_state", "up"); fvs.push_back(fv1); if (speed != SFLOW_ERROR_SPEED_STR) @@ -239,7 +239,7 @@ void SflowMgr::sflowCheckAndFillValues(string alias, vector &fv if (m_sflowPortConfMap[alias].admin == "") { /* By default admin state is enable if not set explicitely */ - m_sflowPortConfMap[alias].admin = "enable"; + m_sflowPortConfMap[alias].admin = "up"; } FieldValueTuple fv("admin_state", m_sflowPortConfMap[alias].admin); fvs.push_back(fv); @@ -276,7 +276,7 @@ void SflowMgr::doTask(Consumer &consumer) if (fvField(i) == "admin_state") { bool enable = false; - if (fvValue(i) == "enable") + if (fvValue(i) == "up") { enable = true; } @@ -305,7 +305,7 @@ void SflowMgr::doTask(Consumer &consumer) { bool enable = false; - if (fvValue(i) == "enable") + if (fvValue(i) == "up") { enable = true; } diff --git a/orchagent/sfloworch.cpp b/orchagent/sfloworch.cpp index 251362afe2..3c00c23f9f 100644 --- a/orchagent/sfloworch.cpp +++ b/orchagent/sfloworch.cpp @@ -139,11 +139,11 @@ void SflowOrch::sflowExtractInfo(vector &fvs, bool &admin, uint { if (fvField(i) == "admin_state") { - if (fvValue(i) == "enable") + if (fvValue(i) == "up") { admin = true; - } - else if (fvValue(i) == "disable") + } + else if (fvValue(i) == "down") { admin = false; } @@ -281,7 +281,7 @@ void SflowOrch::doTask(Consumer &consumer) { port_info.m_sample_id = session_info->second.m_sample_id; } - else + else { SflowSession session; if (!sflowCreateSession(rate, session)) @@ -304,7 +304,7 @@ void SflowOrch::doTask(Consumer &consumer) m_sflowPortInfoMap[port.m_port_id] = port_info; m_sflowRateSampleMap[rate].ref_count++; } - else + else { if (rate != sflowSessionGetRate(sflowInfo->second.m_sample_id)) { diff --git a/tests/test_sflow.py b/tests/test_sflow.py index 008637ea96..1e064d699e 100644 --- a/tests/test_sflow.py +++ b/tests/test_sflow.py @@ -8,7 +8,7 @@ def setup_sflow(self, dvs): self.pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0) self.adb = swsscommon.DBConnector(1, dvs.redis_sock, 0) ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_TABLE") - fvs = swsscommon.FieldValuePairs([("admin_state", "enable")]) + fvs = swsscommon.FieldValuePairs([("admin_state", "up")]) ptbl.set("global", fvs) time.sleep(1) @@ -17,11 +17,11 @@ def test_SflowDisble(self, dvs, testlog): self.setup_sflow(dvs) ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_SESSION_TABLE") gtbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_TABLE") - fvs = swsscommon.FieldValuePairs([("admin_state", "disable")]) + fvs = swsscommon.FieldValuePairs([("admin_state", "down")]) gtbl.set("global", fvs) time.sleep(1) - fvs = swsscommon.FieldValuePairs([("admin_state", "enable"),("sample_rate","1000")]) + fvs = swsscommon.FieldValuePairs([("admin_state", "up"),("sample_rate","1000")]) ptbl.set("Ethernet0", fvs) time.sleep(1) @@ -39,7 +39,7 @@ def test_SflowDisble(self, dvs, testlog): assert sample_session == "" - fvs = swsscommon.FieldValuePairs([("admin_state", "enable")]) + fvs = swsscommon.FieldValuePairs([("admin_state", "up")]) gtbl.set("global", fvs) time.sleep(1) @@ -59,7 +59,7 @@ def test_SflowDisble(self, dvs, testlog): atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_SAMPLEPACKET") (status, fvs) = atbl.get(sample_session) - + assert status == True for fv in fvs: @@ -72,7 +72,7 @@ def test_InterfaceSet(self, dvs, testlog): self.setup_sflow(dvs) ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_SESSION_TABLE") gtbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_TABLE") - fvs = swsscommon.FieldValuePairs([("admin_state", "enable"),("sample_rate","1000")]) + fvs = swsscommon.FieldValuePairs([("admin_state", "up"),("sample_rate","1000")]) ptbl.set("Ethernet0", fvs) time.sleep(1) @@ -91,7 +91,7 @@ def test_InterfaceSet(self, dvs, testlog): atbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_SAMPLEPACKET") (status, fvs) = atbl.get(sample_session) - + assert status == True for fv in fvs: @@ -103,7 +103,7 @@ def test_InterfaceSet(self, dvs, testlog): def test_ConfigDel(self, dvs, testlog): self.setup_sflow(dvs) ptbl = swsscommon.ProducerStateTable(self.pdb, "SFLOW_SESSION_TABLE") - fvs = swsscommon.FieldValuePairs([("admin_state", "enable"),("sample_rate","1000")]) + fvs = swsscommon.FieldValuePairs([("admin_state", "up"),("sample_rate","1000")]) ptbl.set("Ethernet0", fvs) time.sleep(1)