From f11ccfccc28dad64b262548206643832b6940a10 Mon Sep 17 00:00:00 2001 From: kktheballer Date: Tue, 22 Dec 2020 18:28:35 -0800 Subject: [PATCH] [portsorch] Refactor portsorch to use FlexCounterManager to setup port and queue stats (#1170) - Updates portsorch to use FlexCounterManager for port and queue stats instead of direct redis operations - Updates FlexCounterManager to allow clients to enable the group in the constructor, for convenience - Updates the makefile for cfgmgr to include new flex_counter dependency from portsorch Signed-off-by: Danny Allen --- cfgmgr/Makefile.am | 2 +- orchagent/debugcounterorch.cpp | 3 +- .../flex_counter/flex_counter_manager.cpp | 10 ++- orchagent/flex_counter/flex_counter_manager.h | 5 +- .../flex_counter_stat_manager.cpp | 5 +- .../flex_counter/flex_counter_stat_manager.h | 3 +- orchagent/portsorch.cpp | 90 +++++++++---------- orchagent/portsorch.h | 7 +- 8 files changed, 66 insertions(+), 59 deletions(-) diff --git a/cfgmgr/Makefile.am b/cfgmgr/Makefile.am index 5a783ce5ff..7178cad1c6 100644 --- a/cfgmgr/Makefile.am +++ b/cfgmgr/Makefile.am @@ -1,4 +1,4 @@ -INCLUDES = -I $(top_srcdir) -I $(top_srcdir)/orchagent -I $(top_srcdir)/warmrestart +INCLUDES = -I $(top_srcdir) -I $(top_srcdir)/orchagent -I $(top_srcdir)/warmrestart -I $(top_srcdir)/orchagent/flex_counter CFLAGS_SAI = -I /usr/include/sai LIBNL_CFLAGS = -I/usr/include/libnl3 LIBNL_LIBS = -lnl-genl-3 -lnl-route-3 -lnl-3 diff --git a/orchagent/debugcounterorch.cpp b/orchagent/debugcounterorch.cpp index ab92daf8bd..31ec150276 100644 --- a/orchagent/debugcounterorch.cpp +++ b/orchagent/debugcounterorch.cpp @@ -25,7 +25,7 @@ static const unordered_map flex_counter_type_lookup = { // object should only be initialized once. DebugCounterOrch::DebugCounterOrch(DBConnector *db, const vector& table_names, int poll_interval) : Orch(db, table_names), - flex_counter_manager(DEBUG_COUNTER_FLEX_COUNTER_GROUP, StatsMode::READ, poll_interval), + flex_counter_manager(DEBUG_COUNTER_FLEX_COUNTER_GROUP, StatsMode::READ, poll_interval, true), m_stateDb(new DBConnector("STATE_DB", 0)), m_debugCapabilitiesTable(new Table(m_stateDb.get(), STATE_DEBUG_COUNTER_CAPABILITIES_NAME)), m_countersDb(new DBConnector("COUNTERS_DB", 0)), @@ -34,7 +34,6 @@ DebugCounterOrch::DebugCounterOrch(DBConnector *db, const vector& table_ { SWSS_LOG_ENTER(); publishDropCounterCapabilities(); - flex_counter_manager.enableFlexCounterGroup(); } DebugCounterOrch::~DebugCounterOrch(void) diff --git a/orchagent/flex_counter/flex_counter_manager.cpp b/orchagent/flex_counter/flex_counter_manager.cpp index 323bd309c4..130ea2833d 100644 --- a/orchagent/flex_counter/flex_counter_manager.cpp +++ b/orchagent/flex_counter/flex_counter_manager.cpp @@ -34,17 +34,19 @@ const unordered_map FlexCounterManager::counter_id_field_lo { { CounterType::PORT_DEBUG, PORT_DEBUG_COUNTER_ID_LIST }, { CounterType::SWITCH_DEBUG, SWITCH_DEBUG_COUNTER_ID_LIST }, + { CounterType::PORT, PORT_COUNTER_ID_LIST }, + { CounterType::QUEUE, QUEUE_COUNTER_ID_LIST } }; -// This constructor will create a group that is disabled by default. FlexCounterManager::FlexCounterManager( const string& group_name, const StatsMode stats_mode, - const uint polling_interval) : + const uint polling_interval, + const bool enabled) : group_name(group_name), stats_mode(stats_mode), polling_interval(polling_interval), - enabled(false), + enabled(enabled), flex_counter_db(new DBConnector("FLEX_COUNTER_DB", 0)), flex_counter_group_table(new ProducerTable(flex_counter_db.get(), FLEX_COUNTER_GROUP_TABLE)), flex_counter_table(new ProducerTable(flex_counter_db.get(), FLEX_COUNTER_TABLE)) @@ -72,6 +74,8 @@ FlexCounterManager::~FlexCounterManager() void FlexCounterManager::applyGroupConfiguration() { + SWSS_LOG_ENTER(); + vector field_values = { FieldValueTuple(STATS_MODE_FIELD, stats_mode_lookup.at(stats_mode)), diff --git a/orchagent/flex_counter/flex_counter_manager.h b/orchagent/flex_counter/flex_counter_manager.h index 0d444d57ed..19ff26ccf0 100644 --- a/orchagent/flex_counter/flex_counter_manager.h +++ b/orchagent/flex_counter/flex_counter_manager.h @@ -18,6 +18,8 @@ enum class StatsMode enum class CounterType { + PORT, + QUEUE, PORT_DEBUG, SWITCH_DEBUG }; @@ -33,7 +35,8 @@ class FlexCounterManager FlexCounterManager( const std::string& group_name, const StatsMode stats_mode, - const uint polling_interval); + const uint polling_interval, + const bool enabled); FlexCounterManager(const FlexCounterManager&) = delete; FlexCounterManager& operator=(const FlexCounterManager&) = delete; diff --git a/orchagent/flex_counter/flex_counter_stat_manager.cpp b/orchagent/flex_counter/flex_counter_stat_manager.cpp index 4f6cab843e..f636e5af1e 100644 --- a/orchagent/flex_counter/flex_counter_stat_manager.cpp +++ b/orchagent/flex_counter/flex_counter_stat_manager.cpp @@ -13,8 +13,9 @@ using swss::FieldValueTuple; FlexCounterStatManager::FlexCounterStatManager( const string& group_name, const StatsMode stats_mode, - const int polling_interval) : - FlexCounterManager(group_name, stats_mode, polling_interval) + const int polling_interval, + const bool enabled) : + FlexCounterManager(group_name, stats_mode, polling_interval, enabled) { SWSS_LOG_ENTER(); } diff --git a/orchagent/flex_counter/flex_counter_stat_manager.h b/orchagent/flex_counter/flex_counter_stat_manager.h index 250adb724c..7a388ae100 100644 --- a/orchagent/flex_counter/flex_counter_stat_manager.h +++ b/orchagent/flex_counter/flex_counter_stat_manager.h @@ -11,7 +11,8 @@ class FlexCounterStatManager : public FlexCounterManager FlexCounterStatManager( const std::string& group_name, const StatsMode stats_mode, - const int polling_interval); + const int polling_interval, + const bool enabled); FlexCounterStatManager(const FlexCounterStatManager&) = delete; FlexCounterStatManager& operator=(const FlexCounterStatManager&) = delete; diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index d925f80d76..130e0ccfea 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include "net/if.h" @@ -47,8 +48,11 @@ extern FdbOrch *gFdbOrch; #define PORT_FLEX_STAT_COUNTER_POLL_MSECS "1000" #define PORT_BUFFER_DROP_STAT_POLLING_INTERVAL_MS "60000" #define QUEUE_FLEX_STAT_COUNTER_POLL_MSECS "10000" + +#define PORT_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS 1000 +#define QUEUE_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS 10000 #define QUEUE_WATERMARK_FLEX_STAT_COUNTER_POLL_MSECS "10000" -#define PG_WATERMARK_FLEX_STAT_COUNTER_POLL_MSECS "10000" +#define PG_WATERMARK_FLEX_STAT_COUNTER_POLL_MSECS "10000" static map fec_mode_map = @@ -74,7 +78,7 @@ static map learn_mode_map = { "notification", SAI_BRIDGE_PORT_FDB_LEARNING_MODE_FDB_NOTIFICATION} }; -const vector portStatIds = +const vector port_stat_ids = { SAI_PORT_STAT_IF_IN_OCTETS, SAI_PORT_STAT_IF_IN_UCAST_PKTS, @@ -117,6 +121,7 @@ const vector portStatIds = SAI_PORT_STAT_ETHER_IN_PKTS_128_TO_255_OCTETS, }; + static const vector port_buffer_drop_stat_ids = { SAI_PORT_STAT_IN_DROPPED_PKTS, @@ -131,6 +136,14 @@ static const vector queueStatIds = SAI_QUEUE_STAT_DROPPED_BYTES, }; +static const vector queue_stat_ids = +{ + SAI_QUEUE_STAT_PACKETS, + SAI_QUEUE_STAT_BYTES, + SAI_QUEUE_STAT_DROPPED_PACKETS, + SAI_QUEUE_STAT_DROPPED_BYTES, +}; + static const vector queueWatermarkStatIds = { SAI_QUEUE_STAT_SHARED_WATERMARK_BYTES, @@ -161,7 +174,9 @@ static char* hostif_vlan_tag[] = { * default VLAN and all ports removed from .1Q bridge. */ PortsOrch::PortsOrch(DBConnector *db, vector &tableNames) : - Orch(db, tableNames) + Orch(db, tableNames), + port_stat_manager(PORT_STAT_COUNTER_FLEX_COUNTER_GROUP, StatsMode::READ, PORT_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS, true), + queue_stat_manager(QUEUE_STAT_COUNTER_FLEX_COUNTER_GROUP, StatsMode::READ, QUEUE_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS, true) { SWSS_LOG_ENTER(); @@ -1499,6 +1514,7 @@ bool PortsOrch::removePort(sai_object_id_t port_id) return true; } +//candidate for omission string PortsOrch::getPortFlexCounterTableKey(string key) { return string(PORT_STAT_COUNTER_FLEX_COUNTER_GROUP) + ":" + key; @@ -1508,11 +1524,15 @@ string PortsOrch::getPortBuffDropFlexCounterTableKey(string key) { return string(PORT_DROP_STAT_COUNTER_FLEX_COUNTER_GROUP) + ":" + key; } +//candidate for omission + +//string PortsOrch::getQueueFlexCounterTableKey(string key) +//{ +// return string(QUEUE_STAT_COUNTER_FLEX_COUNTER_GROUP) + ":" + key; +//} + +//candidate for omission -string PortsOrch::getQueueFlexCounterTableKey(string key) -{ - return string(QUEUE_STAT_COUNTER_FLEX_COUNTER_GROUP) + ":" + key; -} string PortsOrch::getQueueWatermarkFlexCounterTableKey(string key) { @@ -1557,35 +1577,17 @@ bool PortsOrch::initPort(const string &alias, const set &lane_set) fields.push_back(tuple); m_counterTable->set("", fields); - /* Add port to flex_counter for updating stat counters */ - string key = getPortFlexCounterTableKey(sai_serialize_object_id(p.m_port_id)); - std::string delimiter = ""; - std::ostringstream counters_stream; - for (const auto &id: portStatIds) + // Install a flex counter for this port to track stats + std::unordered_set counter_stats; + for (const auto& it: port_stat_ids) { - counters_stream << delimiter << sai_serialize_port_stat(id); - delimiter = comma; + counter_stats.emplace(sai_serialize_port_stat(it)); } + port_stat_manager.setCounterIdList(p.m_port_id, CounterType::PORT, counter_stats); - fields.clear(); - fields.emplace_back(PORT_COUNTER_ID_LIST, counters_stream.str()); - - m_flexCounterTable->set(key, fields); - - delimiter = ""; - string port_drop_key = getPortBuffDropFlexCounterTableKey(sai_serialize_object_id(p.m_port_id)); - std::ostringstream port_buffer_drop_stream; - for (const auto& it: port_buffer_drop_stat_ids) - { - port_buffer_drop_stream << delimiter << sai_serialize_port_stat(it); - delimiter = comma; - } - fields.clear(); - fields.emplace_back(PORT_COUNTER_ID_LIST, port_buffer_drop_stream.str()); - m_flexCounterTable->set(port_drop_key, fields); + PortUpdate update = { p, true }; - PortUpdate update = {p, true }; notify(SUBJECT_TYPE_PORT_CHANGE, static_cast(&update)); SWSS_LOG_NOTICE("Initialized port %s", alias.c_str()); @@ -3632,34 +3634,26 @@ void PortsOrch::generateQueueMapPerPort(const Port& port) queueIndexVector.emplace_back(id, to_string(queueRealIndex)); } - /* add ordinary Queue stat counters */ - string key = getQueueFlexCounterTableKey(id); - - std::string delimiter = ""; - std::ostringstream counters_stream; - for (const auto& it: queueStatIds) + // Install a flex counter for this queue to track stats + std::unordered_set counter_stats; + for (const auto& it: queue_stat_ids) { - counters_stream << delimiter << sai_serialize_queue_stat(it); - delimiter = comma; + counter_stats.emplace(sai_serialize_queue_stat(it)); } - - vector fieldValues; - fieldValues.emplace_back(QUEUE_COUNTER_ID_LIST, counters_stream.str()); - - m_flexCounterTable->set(key, fieldValues); + queue_stat_manager.setCounterIdList(port.m_queue_ids[queueIndex], CounterType::QUEUE, counter_stats); /* add watermark queue counters */ - key = getQueueWatermarkFlexCounterTableKey(id); + string key = getQueueWatermarkFlexCounterTableKey(id); - delimiter = ""; - counters_stream.str(""); + string delimiter(""); + std::ostringstream counters_stream; for (const auto& it: queueWatermarkStatIds) { counters_stream << delimiter << sai_serialize_queue_stat(it); delimiter = comma; } - fieldValues.clear(); + vector fieldValues; fieldValues.emplace_back(QUEUE_COUNTER_ID_LIST, counters_stream.str()); m_flexCounterTable->set(key, fieldValues); diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index d0a6677072..c6d310ec9e 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -9,6 +9,7 @@ #include "observer.h" #include "macaddress.h" #include "producertable.h" +#include "flex_counter_manager.h" #define FCS_LEN 4 #define VLAN_TAG_LEN 4 @@ -106,15 +107,19 @@ class PortsOrch : public Orch, public Subject unique_ptr m_flexCounterTable; unique_ptr m_flexCounterGroupTable; - std::string getQueueFlexCounterTableKey(std::string s); std::string getQueueWatermarkFlexCounterTableKey(std::string s); + //candidate for omission std::string getPortFlexCounterTableKey(std::string s); std::string getPortBuffDropFlexCounterTableKey(std::string s); + //candidate for omission std::string getPriorityGroupWatermarkFlexCounterTableKey(std::string s); shared_ptr m_counter_db; shared_ptr m_flex_db; + FlexCounterManager port_stat_manager; + FlexCounterManager queue_stat_manager; + std::map m_portSupportedSpeeds; bool m_initDone = false;