From d0a8d14c55734060cb23b9073c8e209e9fa0fbf5 Mon Sep 17 00:00:00 2001 From: tomeri Date: Sun, 24 Oct 2021 11:55:28 +0000 Subject: [PATCH 01/15] countrs support for dynamic port add/remove --- orchagent/debugcounterorch.cpp | 68 ++++++++++++++++++ orchagent/debugcounterorch.h | 10 ++- orchagent/orchdaemon.cpp | 7 +- orchagent/portsorch.cpp | 123 +++++++++++++++++++++++++++++++-- orchagent/portsorch.h | 2 + tests/test_drop_counters.py | 84 ++++++++++++++++++++++ tests/test_flex_counters.py | 93 ++++++++++++++++++++++--- tests/test_pg_drop_counter.py | 65 ++++++++++++++++- 8 files changed, 429 insertions(+), 23 deletions(-) diff --git a/orchagent/debugcounterorch.cpp b/orchagent/debugcounterorch.cpp index 25d0b94589..538f10e724 100644 --- a/orchagent/debugcounterorch.cpp +++ b/orchagent/debugcounterorch.cpp @@ -5,6 +5,7 @@ #include "schema.h" #include "drop_counter.h" #include +#include "observer.h" using std::string; using std::unordered_map; @@ -34,6 +35,8 @@ DebugCounterOrch::DebugCounterOrch(DBConnector *db, const vector& table_ { SWSS_LOG_ENTER(); publishDropCounterCapabilities(); + + gPortsOrch->attach(this); } DebugCounterOrch::~DebugCounterOrch(void) @@ -41,6 +44,24 @@ DebugCounterOrch::~DebugCounterOrch(void) SWSS_LOG_ENTER(); } +void DebugCounterOrch::update(SubjectType type, void *cntx) +{ + SWSS_LOG_ENTER(); + + assert(cntx); + + if (type == SUBJECT_TYPE_PORT_CHANGE) { + PortUpdate *update = static_cast(cntx); + Port &port = update->port; + + if (update->add) { + addPortDebugCounter(port.m_port_id); + } else { + removePortDebugCounter(port.m_port_id); + } + } +} + // doTask processes updates from the consumer and modifies the state of the // following components: // 1) The ASIC, by creating, modifying, and deleting debug counters @@ -616,3 +637,50 @@ bool DebugCounterOrch::isDropReasonValid(const string& drop_reason) const return true; } + +void DebugCounterOrch::addPortDebugCounter(sai_object_id_t port_id) +{ + SWSS_LOG_ENTER(); + + SWSS_LOG_NOTICE("add debug counter for port 0x%" PRIu64 , port_id); + + for (auto it = debug_counters.begin(); it != debug_counters.end(); it++) { + DebugCounter *counter = dynamic_cast(it->second.get()); + string counter_type = counter->getCounterType(); + string counter_stat = counter->getDebugCounterSAIStat(); + CounterType flex_counter_type = getFlexCounterType(counter_type); + + if (flex_counter_type == CounterType::PORT_DEBUG){ + flex_counter_manager.addFlexCounterStat( + port_id, + flex_counter_type, + counter_stat); + } + } +} + +void DebugCounterOrch::removePortDebugCounter(sai_object_id_t port_id) +{ + SWSS_LOG_ENTER(); + + SWSS_LOG_NOTICE("remove debug counter for port 0x%" PRIu64 , port_id); + + for (auto it = debug_counters.begin(); it != debug_counters.end(); it++) { + DebugCounter *counter = dynamic_cast(it->second.get()); + + string counter_type = counter->getCounterType(); + string counter_stat = counter->getDebugCounterSAIStat(); + CounterType flex_counter_type = getFlexCounterType(counter_type); + + if (flex_counter_type == CounterType::PORT_DEBUG){ + flex_counter_manager.removeFlexCounterStat( + port_id, + flex_counter_type, + counter_stat); + } + } +} + + + + diff --git a/orchagent/debugcounterorch.h b/orchagent/debugcounterorch.h index e5b512c8e4..ffa25ac37e 100644 --- a/orchagent/debugcounterorch.h +++ b/orchagent/debugcounterorch.h @@ -10,6 +10,7 @@ #include "flex_counter_stat_manager.h" #include "debug_counter.h" #include "drop_counter.h" +#include "observer.h" extern "C" { #include "sai.h" @@ -17,9 +18,11 @@ extern "C" { #define DEBUG_COUNTER_FLEX_COUNTER_GROUP "DEBUG_COUNTER" +typedef std::unordered_map> DebugCounterMap; + // DebugCounterOrch is an orchestrator for managing debug counters. It handles // the creation, deletion, and modification of debug counters. -class DebugCounterOrch: public Orch +class DebugCounterOrch: public Orch, public Subject, public Observer { public: DebugCounterOrch(swss::DBConnector *db, const std::vector& table_names, int poll_interval); @@ -27,6 +30,7 @@ class DebugCounterOrch: public Orch void doTask(Consumer& consumer); + void update(SubjectType, void *cntx); private: // Debug Capability Reporting Functions void publishDropCounterCapabilities(); @@ -52,6 +56,8 @@ class DebugCounterOrch: public Orch void uninstallDebugFlexCounters( const std::string& counter_type, const std::string& counter_stat); + void addPortDebugCounter(sai_object_id_t port_id); + void removePortDebugCounter(sai_object_id_t port_id); // Debug Counter Initialization Helper Functions std::string getDebugCounterType( @@ -83,7 +89,7 @@ class DebugCounterOrch: public Orch FlexCounterStatManager flex_counter_manager; - std::unordered_map> debug_counters; + DebugCounterMap debug_counters; // free_drop_counters are drop counters that have been created by a user // that do not have any drop reasons associated with them yet. Because diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index d4fe4828b4..3d683ce522 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -52,6 +52,7 @@ CoppOrch *gCoppOrch; P4Orch *gP4Orch; BfdOrch *gBfdOrch; Srv6Orch *gSrv6Orch; +DebugCounterOrch *gDebugCounterOrch; bool gIsNatSupported = false; @@ -282,7 +283,7 @@ bool OrchDaemon::init() CFG_DEBUG_COUNTER_DROP_REASON_TABLE_NAME }; - DebugCounterOrch *debug_counter_orch = new DebugCounterOrch(m_configDb, debug_counter_tables, 1000); + gDebugCounterOrch = new DebugCounterOrch(m_configDb, debug_counter_tables, 1000); const int natorch_base_pri = 50; @@ -330,7 +331,7 @@ bool OrchDaemon::init() * when iterating ConsumerMap. This is ensured implicitly by the order of keys in ordered map. * For cases when Orch has to process tables in specific order, like PortsOrch during warm start, it has to override Orch::doTask() */ - m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, mux_orch, mux_cb_orch, gIntfsOrch, gNeighOrch, gNhgMapOrch, gNhgOrch, gCbfNhgOrch, gRouteOrch, gCoppOrch, gQosOrch, wm_orch, policer_orch, tunnel_decap_orch, sflow_orch, debug_counter_orch, gMacsecOrch, gBfdOrch, gSrv6Orch}; + m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, mux_orch, mux_cb_orch, gIntfsOrch, gNeighOrch, gNhgMapOrch, gNhgOrch, gCbfNhgOrch, gRouteOrch, gCoppOrch, gQosOrch, wm_orch, policer_orch, tunnel_decap_orch, sflow_orch, gDebugCounterOrch, gMacsecOrch, gBfdOrch, gSrv6Orch}; bool initialize_dtel = false; if (platform == BFN_PLATFORM_SUBSTRING || platform == VS_PLATFORM_SUBSTRING) @@ -428,6 +429,8 @@ bool OrchDaemon::init() m_orchList.push_back(gMlagOrch); m_orchList.push_back(gIsoGrpOrch); m_orchList.push_back(gFgNhgOrch); + m_orchList.push_back(mux_orch); + m_orchList.push_back(mux_cb_orch); m_orchList.push_back(mux_st_orch); m_orchList.push_back(nvgre_tunnel_orch); m_orchList.push_back(nvgre_tunnel_map_orch); diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 9eca1439ed..c95b7fc0d1 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -2378,6 +2378,18 @@ bool PortsOrch::initPort(const string &alias, const string &role, const int inde port_buffer_drop_stat_manager.setCounterIdList(p.m_port_id, CounterType::PORT, port_buffer_drop_stats); } + /* add pg counters */ + if (m_isPriorityGroupMapGenerated) { + generatePriorityGroupMapPerPort(p); + } + + /* add queue port counters */ + if (m_isQueueMapGenerated) { + generateQueueMapPerPort(p); + } + + /* debug counters will be added during the SUBJECT_TYPE_PORT_CHANGE notify */ + PortUpdate update = { p, true }; notify(SUBJECT_TYPE_PORT_CHANGE, static_cast(&update)); @@ -2410,8 +2422,13 @@ void PortsOrch::deInitPort(string alias, sai_object_id_t port_id) { SWSS_LOG_ENTER(); - Port p(alias, Port::PHY); - p.m_port_id = port_id; + Port p; + + if (!getPort(port_id, p)) + { + SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64, port_id); + return; + } /* remove port from flex_counter_table for updating counters */ auto flex_counters_orch = gDirectory.get(); @@ -2425,9 +2442,18 @@ void PortsOrch::deInitPort(string alias, sai_object_id_t port_id) port_buffer_drop_stat_manager.clearCounterIdList(p.m_port_id); } + /* remove pg port counters */ + if (m_isPriorityGroupMapGenerated) { + removePriorityGroupMapPerPort(p); + } + + /* remove queue port counters */ + if (m_isQueueMapGenerated) { + removeQueueMapPerPort(p); + } /* remove port name map from counter table */ - m_counter_db->hdel(COUNTERS_PORT_NAME_MAP, alias); + m_counterTable->hdel("", alias); /* Remove the associated port serdes attribute */ removePortSerdesAttribute(p.m_port_id); @@ -2436,7 +2462,6 @@ void PortsOrch::deInitPort(string alias, sai_object_id_t port_id) SWSS_LOG_NOTICE("De-Initialized port %s", alias.c_str()); } - bool PortsOrch::bake() { SWSS_LOG_ENTER(); @@ -3305,7 +3330,6 @@ void PortsOrch::doPortTask(Consumer &consumer) { throw runtime_error("Remove hostif for the port failed"); } - Port p; if (getPort(port_id, p)) { @@ -5398,6 +5422,55 @@ void PortsOrch::generateQueueMap() m_isQueueMapGenerated = true; } +void PortsOrch::removeQueueMapPerPort(const Port& port) +{ + /* Create the Queue map in the Counter DB */ + /* Add stat counters to flex_counter */ + vector queueVector; + vector queuePortVector; + vector queueIndexVector; + vector queueTypeVector; + + for (size_t queueIndex = 0; queueIndex < port.m_queue_ids.size(); ++queueIndex) + { + std::ostringstream name; + name << port.m_alias << ":" << queueIndex; + + const auto id = sai_serialize_object_id(port.m_queue_ids[queueIndex]); + + queueVector.emplace_back(name.str()); + queuePortVector.emplace_back(id); + + string queueType; + uint8_t queueRealIndex = 0; + if (getQueueTypeAndIndex(port.m_queue_ids[queueIndex], queueType, queueRealIndex)) + { + queueTypeVector.emplace_back(id); + queueIndexVector.emplace_back(id); + } + + // Install a flex counter for this queue to track stats + std::unordered_set counter_stats; + for (const auto& it: queue_stat_ids) + { + counter_stats.emplace(sai_serialize_queue_stat(it)); + } + queue_stat_manager.clearCounterIdList(port.m_queue_ids[queueIndex]); + + /* add watermark queue counters */ + string key = getQueueWatermarkFlexCounterTableKey(id); + + m_flexCounterTable->del(key); + } + + m_counter_db->hdel(COUNTERS_QUEUE_NAME_MAP, queueVector); + m_counter_db->hdel(COUNTERS_QUEUE_PORT_MAP, queuePortVector); + m_counter_db->hdel(COUNTERS_QUEUE_INDEX_MAP, queueIndexVector); + m_counter_db->hdel(COUNTERS_QUEUE_TYPE_MAP, queueTypeVector); + + CounterCheckOrch::getInstance().removePort(port); +} + void PortsOrch::generateQueueMapPerPort(const Port& port) { /* Create the Queue map in the Counter DB */ @@ -5476,6 +5549,41 @@ void PortsOrch::generatePriorityGroupMap() m_isPriorityGroupMapGenerated = true; } +void PortsOrch::removePriorityGroupMapPerPort(const Port& port) +{ + /* Create the PG map in the Counter DB */ + /* Add stat counters to flex_counter */ + vector pgVector; + vector pgPortVector; + vector pgIndexVector; + + for (size_t pgIndex = 0; pgIndex < port.m_priority_group_ids.size(); ++pgIndex) + { + std::ostringstream name; + name << port.m_alias << ":" << pgIndex; + + const auto id = sai_serialize_object_id(port.m_priority_group_ids[pgIndex]); + + pgVector.emplace_back(name.str()); + pgPortVector.emplace_back(id); + pgIndexVector.emplace_back(id); + + string key = getPriorityGroupWatermarkFlexCounterTableKey(id); + + m_flexCounterTable->del(key); + + key = getPriorityGroupDropPacketsFlexCounterTableKey(id); + /* Add dropped packets counters to flex_counter */ + m_flexCounterTable->del(key); + } + + m_counter_db->hdel(COUNTERS_PG_NAME_MAP, pgVector); + m_counter_db->hdel(COUNTERS_PG_PORT_MAP, pgPortVector); + m_counter_db->hdel(COUNTERS_PG_INDEX_MAP, pgIndexVector); + + CounterCheckOrch::getInstance().removePort(port); +} + void PortsOrch::generatePriorityGroupMapPerPort(const Port& port) { /* Create the PG map in the Counter DB */ @@ -5615,7 +5723,8 @@ void PortsOrch::doTask(NotificationConsumer &consumer) if (!getPort(id, port)) { - SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64, id); + /* consider changing it to notice */ + SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64 " - its ok to ignore this message if this port was just removed", id); continue; } @@ -5633,7 +5742,7 @@ void PortsOrch::doTask(NotificationConsumer &consumer) updateDbPortOperSpeed(port, 0); } } - + /* update m_portList */ m_portList[port.m_alias] = port; } diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 843ffbd7f1..bdfcf47ad0 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -312,9 +312,11 @@ class PortsOrch : public Orch, public Subject bool m_isQueueMapGenerated = false; void generateQueueMapPerPort(const Port& port); + void removeQueueMapPerPort(const Port& port); bool m_isPriorityGroupMapGenerated = false; void generatePriorityGroupMapPerPort(const Port& port); + void removePriorityGroupMapPerPort(const Port& port); bool m_isPortCounterMapGenerated = false; bool m_isPortBufferDropCounterMapGenerated = false; diff --git a/tests/test_drop_counters.py b/tests/test_drop_counters.py index b003876f1a..f7ba5d4fb2 100644 --- a/tests/test_drop_counters.py +++ b/tests/test_drop_counters.py @@ -1,5 +1,6 @@ import time import pytest +import redis from swsscommon import swsscommon @@ -654,6 +655,89 @@ def test_removeInvalidDropReason(self, dvs, testlog): # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason1) + + def getPortOid(self, dvs, port_name): + cnt_r = redis.Redis(unix_socket_path=dvs.redis_sock, db=swsscommon.COUNTERS_DB, + encoding="utf-8", decode_responses=True) + return cnt_r.hget("COUNTERS_PORT_NAME_MAP", port_name); + + def test_add_remove_port(self, dvs, testlog): + + """ + This test verifies that debug counters are removed when we remove a port + and debug counters are added each time we add ports (if debug counter is enabled) + """ + self.setup_db(dvs) + + # save ethernet0 info + cdb = swsscommon.DBConnector(4, dvs.redis_sock, 0) + tbl = swsscommon.Table(cdb, "PORT") + (status, fvs) = tbl.get("Ethernet0") + assert status == True + + # get counter oid + oid = self.getPortOid(dvs, "Ethernet0") + + # verifies debug coutner dont exist for ethernet0 + flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) + status, fields = flex_counter_table.get(oid) + assert len(fields) == 0 + + # add debug counters + name1 = 'DEBUG_0' + reason1 = 'L3_ANY' + + name2 = 'DEBUG_1' + reason2 = 'L2_ANY' + + self.create_drop_counter(name1, PORT_INGRESS_DROPS) + self.add_drop_reason(name1, reason1) + + self.create_drop_counter(name2, PORT_EGRESS_DROPS) + self.add_drop_reason(name2, reason2) + + time.sleep(5) + + # verifies debug coutner exist for ethernet0 + flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) + status, fields = flex_counter_table.get(oid) + assert status == True + assert len(fields) == 1 + + # remove Ethernet0 port + cdb.hdel("CABLE_LENGTH|AZURE", "Ethernet0") + ethernet0_bufferpg_keys = cdb.keys("BUFFER_PG|Ethernet0|*") + for key in ethernet0_bufferpg_keys: + cdb._del(key) + ethernet0_bufferqueue_keys = cdb.keys("BUFFER_QUEUE|Ethernet0|*") + for key in ethernet0_bufferqueue_keys: + cdb._del(key) + cdb._del("BREAKOUT_CFG|Ethernet0") + tbl._del("Ethernet0") + time.sleep(5) + + # verify that debug counter were removed + status, fields = flex_counter_table.get(oid) + assert len(fields) == 0 + + # add Ethernet0 + tbl.set("Ethernet0", fvs) + time.sleep(5) + + # verifies that debug counters were added for ethernet0 + oid = self.getPortOid(dvs, "Ethernet0") + + status, fields = flex_counter_table.get(oid) + assert status == True + assert len(fields) == 1 + + # Cleanup for the next test. + self.delete_drop_counter(name1) + self.remove_drop_reason(name1, reason1) + + self.delete_drop_counter(name2) + self.remove_drop_reason(name2, reason2) + def test_createAndDeleteMultipleCounters(self, dvs, testlog): """ diff --git a/tests/test_flex_counters.py b/tests/test_flex_counters.py index ea950af7c1..c098cd91f2 100644 --- a/tests/test_flex_counters.py +++ b/tests/test_flex_counters.py @@ -3,7 +3,6 @@ from swsscommon import swsscommon -TUNNEL_TYPE_MAP = "COUNTERS_TUNNEL_TYPE_MAP" NUMBER_OF_RETRIES = 10 CPU_PORT_OID = "0x0" @@ -63,7 +62,6 @@ } } - class TestFlexCounters(object): def setup_dbs(self, dvs): @@ -127,15 +125,6 @@ def verify_no_flex_counters_tables(self, counter_stat): counters_stat_keys = self.flex_db.get_keys("FLEX_COUNTER_TABLE:" + counter_stat) assert len(counters_stat_keys) == 0, "FLEX_COUNTER_TABLE:" + str(counter_stat) + " tables exist before enabling the flex counter group" - def verify_no_flex_counters_tables_after_delete(self, counter_stat): - for retry in range(NUMBER_OF_RETRIES): - counters_stat_keys = self.flex_db.get_keys("FLEX_COUNTER_TABLE:" + counter_stat + ":") - if len(counters_stat_keys) == 0: - return - else: - time.sleep(1) - assert False, "FLEX_COUNTER_TABLE:" + str(counter_stat) + " tables exist after removing the entries" - def verify_flex_counters_populated(self, map, stat): counters_keys = self.counters_db.db_connection.hgetall(map) for counter_entry in counters_keys.items(): @@ -372,3 +361,85 @@ def test_remove_trap_group(self, dvs): assert trap_id not in counters_keys self.set_flex_counter_group_status(meta_data['key'], meta_data['group_name'], 'disable') + + def remove_port(self, config_db, port): + config_db.delete_entry("CABLE_LENGTH|AZURE", "") + ethernet0_bufferpg_keys = config_db.get_keys("BUFFER_PG|%s"%port) + for key in ethernet0_bufferpg_keys: + config_db.delete_entry("BUFFER_PG|Ethernet0|%s"%key, "") + ethernet0_bufferqueue_keys = config_db.get_keys("BUFFER_QUEUE|%s"%port) + for key in ethernet0_bufferqueue_keys: + config_db.delete_entry("BUFFER_QUEUE|Ethernet0|%s"%key, "") + config_db.delete_entry("BREAKOUT_CFG|%s"%port, "") + config_db.delete_entry("INTERFACE|%s"%port, "") + config_db.delete_entry("PORT", port) + + def test_add_remove_ports(self, dvs): + self.setup_dbs(dvs) + + # set flex counter + counter_key = counter_type_dict['queue_counter'][0] + counter_stat = counter_type_dict['queue_counter'][1] + counter_map = counter_type_dict['queue_counter'][2] + self.enable_flex_counter_group(counter_key, counter_map) + time.sleep(3) + + # receive Ethernet0 info + fvs = self.config_db.get_entry("PORT", "Ethernet0") + assert len(fvs) > 0 + + # save all the oids of the pg drop counters + oid_list = [] + counters_queue_map = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP","") + + for i in range(0,100): + if 'Ethernet0:%d'%i in counters_queue_map: + oid_list.append(counters_queue_map['Ethernet0:%d'%i]) + else: + break + + # verify that counters exists on flex counter + for oid in oid_list: + fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s"%oid) + assert len(fields) == 1 + + # remove port Ethernet0 + self.remove_port(self.config_db, "Ethernet0") + time.sleep(3) + + + # verify counters were removed from flex counter table + for oid in oid_list: + fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s"%oid) + assert len(fields) == 0 + + # verify that Ethernet0 counter maps were removed + oid_list = [] + counters_queue_map = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP","") + + for i in range(0,100): + if 'Ethernet0:%d'%i in counters_queue_map: + oid_list.append(counters_queue_map['Ethernet0:%d'%i]) + else: + break + assert oid_list == [] + + + # add port Ethernet 0 + self.config_db.create_entry("PORT","Ethernet0",fvs) + time.sleep(3) + + # verify counter was added + oid_list = [] + counters_queue_map = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP","") + + for i in range(0,100): + if 'Ethernet0:%d'%i in counters_queue_map: + oid_list.append(counters_queue_map['Ethernet0:%d'%i]) + else: + break + + # verify that counters exists on flex counter + for oid in oid_list: + fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s"%oid) + assert len(fields) == 1 diff --git a/tests/test_pg_drop_counter.py b/tests/test_pg_drop_counter.py index 1cdd834747..da49d52bc6 100644 --- a/tests/test_pg_drop_counter.py +++ b/tests/test_pg_drop_counter.py @@ -74,13 +74,25 @@ def clear_flex_counter(self): self.config_db.delete_entry("FLEX_COUNTER_TABLE", "PG_DROP") self.config_db.delete_entry("FLEX_COUNTER_TABLE", "PG_WATERMARK") - + def remove_port(self, config_db, port): + config_db.hdel("CABLE_LENGTH|AZURE", port) + ethernet0_bufferpg_keys = config_db.keys("BUFFER_PG|%s|*"%port) + for key in ethernet0_bufferpg_keys: + config_db._del(key) + ethernet0_bufferqueue_keys = config_db.keys("BUFFER_QUEUE|%s|*"%port) + for key in ethernet0_bufferqueue_keys: + config_db._del(key) + config_db._del("BREAKOUT_CFG|%s"%port) + port_table = swsscommon.Table(config_db, "PORT") + port_table._del(port) + def test_pg_drop_counters(self, dvs): self.setup_dbs(dvs) self.pgs = self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP") try: self.set_up_flex_counter() + self.populate_asic(dvs, "0") time.sleep(self.DEFAULT_POLL_INTERVAL) self.verify_value(dvs, self.pgs, pg_drop_attr, "0") @@ -94,3 +106,54 @@ def test_pg_drop_counters(self, dvs): self.verify_value(dvs, self.pgs, pg_drop_attr, "123") finally: self.clear_flex_counter() + + def test_pg_drop_counter_port_add_remove(self, dvs): + self.setup_dbs(dvs) + + try: + # configure pg drop flex counter + self.set_up_flex_counter() + time.sleep(5) + + # receive Ethernet0 info + config_db = swsscommon.DBConnector(4, dvs.redis_sock, 0) + port_table = swsscommon.Table(config_db, "PORT") + (status, fvs) = port_table.get("Ethernet0") + assert status == True + + + # save all the oids of the pg drop counters + cnt_r = redis.Redis(unix_socket_path=dvs.redis_sock, db=swsscommon.COUNTERS_DB, + encoding="utf-8", decode_responses=True) + + oid_list = [] + for priority in range(0,7): + oid_list.append(cnt_r.hget("COUNTERS_PG_NAME_MAP", "Ethernet0:%d"%priority)) + + # verify that counters exists on flex counter + fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", "PG_WATERMARK_STAT_COUNTER:%s"%oid_list[-1]) + assert len(fields) == 1 + + # remove port Ethernet0 + self.remove_port(config_db, "Ethernet0") + time.sleep(3) + + # verify counters were removed from flex counter table + for oid in oid_list: + fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", "PG_WATERMARK_STAT_COUNTER:%s"%oid) + assert len(fields) == 0 + + # add port Ethernet 0 + port_table.set("Ethernet0", fvs) + time.sleep(3) + + # verify counter was added + for priority in range(0,7): + oid = cnt_r.hget("COUNTERS_PG_NAME_MAP", "Ethernet0:%d"%priority) + + # verify that counters exists on flex counter + fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", "PG_WATERMARK_STAT_COUNTER:%s"%oid) + assert len(fields) == 1 + + finally: + self.clear_flex_counter() From c0fda4be5389525e55c33405bf52bd17782cb9a8 Mon Sep 17 00:00:00 2001 From: tomeri Date: Sun, 24 Oct 2021 11:55:28 +0000 Subject: [PATCH 02/15] countrs support for dynamic port add/remove --- orchagent/debugcounterorch.cpp | 27 +-- orchagent/debugcounterorch.h | 4 +- orchagent/portsorch.cpp | 45 ++--- tests/conftest.py | 7 +- tests/dvslib/dvs_database.py | 11 ++ tests/dvslib/dvs_port.py | 20 +++ tests/test_drop_counters.py | 316 ++++++++++++++++----------------- tests/test_flex_counters.py | 95 +++++----- tests/test_pg_drop_counter.py | 49 ++--- 9 files changed, 301 insertions(+), 273 deletions(-) create mode 100644 tests/dvslib/dvs_port.py diff --git a/orchagent/debugcounterorch.cpp b/orchagent/debugcounterorch.cpp index 538f10e724..a2d28ee7ab 100644 --- a/orchagent/debugcounterorch.cpp +++ b/orchagent/debugcounterorch.cpp @@ -48,9 +48,14 @@ void DebugCounterOrch::update(SubjectType type, void *cntx) { SWSS_LOG_ENTER(); - assert(cntx); if (type == SUBJECT_TYPE_PORT_CHANGE) { + + if (!cntx) { + SWSS_LOG_ERROR("cntx is NULL"); + return; + } + PortUpdate *update = static_cast(cntx); Port &port = update->port; @@ -644,11 +649,11 @@ void DebugCounterOrch::addPortDebugCounter(sai_object_id_t port_id) SWSS_LOG_NOTICE("add debug counter for port 0x%" PRIu64 , port_id); - for (auto it = debug_counters.begin(); it != debug_counters.end(); it++) { - DebugCounter *counter = dynamic_cast(it->second.get()); - string counter_type = counter->getCounterType(); - string counter_stat = counter->getDebugCounterSAIStat(); - CounterType flex_counter_type = getFlexCounterType(counter_type); + for (const auto& debug_counter: debug_counters) { + DebugCounter *counter = debug_counter.second.get(); + auto counter_type = counter->getCounterType(); + auto counter_stat = counter->getDebugCounterSAIStat(); + auto flex_counter_type = getFlexCounterType(counter_type); if (flex_counter_type == CounterType::PORT_DEBUG){ flex_counter_manager.addFlexCounterStat( @@ -665,12 +670,12 @@ void DebugCounterOrch::removePortDebugCounter(sai_object_id_t port_id) SWSS_LOG_NOTICE("remove debug counter for port 0x%" PRIu64 , port_id); - for (auto it = debug_counters.begin(); it != debug_counters.end(); it++) { - DebugCounter *counter = dynamic_cast(it->second.get()); + for (const auto& debug_counter: debug_counters) { + DebugCounter *counter = debug_counter.second.get(); - string counter_type = counter->getCounterType(); - string counter_stat = counter->getDebugCounterSAIStat(); - CounterType flex_counter_type = getFlexCounterType(counter_type); + auto counter_type = counter->getCounterType(); + auto counter_stat = counter->getDebugCounterSAIStat(); + auto flex_counter_type = getFlexCounterType(counter_type); if (flex_counter_type == CounterType::PORT_DEBUG){ flex_counter_manager.removeFlexCounterStat( diff --git a/orchagent/debugcounterorch.h b/orchagent/debugcounterorch.h index ffa25ac37e..15f4860143 100644 --- a/orchagent/debugcounterorch.h +++ b/orchagent/debugcounterorch.h @@ -18,11 +18,11 @@ extern "C" { #define DEBUG_COUNTER_FLEX_COUNTER_GROUP "DEBUG_COUNTER" -typedef std::unordered_map> DebugCounterMap; +using DebugCounterMap = std::unordered_map>; // DebugCounterOrch is an orchestrator for managing debug counters. It handles // the creation, deletion, and modification of debug counters. -class DebugCounterOrch: public Orch, public Subject, public Observer +class DebugCounterOrch: public Orch, public Observer { public: DebugCounterOrch(swss::DBConnector *db, const std::vector& table_names, int poll_interval); diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index c95b7fc0d1..ba06c542f1 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -2378,18 +2378,16 @@ bool PortsOrch::initPort(const string &alias, const string &role, const int inde port_buffer_drop_stat_manager.setCounterIdList(p.m_port_id, CounterType::PORT, port_buffer_drop_stats); } - /* add pg counters */ + /* when a port is added and priority group map counter is enabled --> we need to add pg counter for it */ if (m_isPriorityGroupMapGenerated) { generatePriorityGroupMapPerPort(p); } - /* add queue port counters */ + /* when a port is added and queue map counter is enabled --> we need to add queue map counter for it */ if (m_isQueueMapGenerated) { generateQueueMapPerPort(p); } - /* debug counters will be added during the SUBJECT_TYPE_PORT_CHANGE notify */ - PortUpdate update = { p, true }; notify(SUBJECT_TYPE_PORT_CHANGE, static_cast(&update)); @@ -3330,6 +3328,7 @@ void PortsOrch::doPortTask(Consumer &consumer) { throw runtime_error("Remove hostif for the port failed"); } + Port p; if (getPort(port_id, p)) { @@ -5424,8 +5423,7 @@ void PortsOrch::generateQueueMap() void PortsOrch::removeQueueMapPerPort(const Port& port) { - /* Create the Queue map in the Counter DB */ - /* Add stat counters to flex_counter */ + /* Remove the Queue map in the Counter DB */ vector queueVector; vector queuePortVector; vector queueIndexVector; @@ -5448,8 +5446,17 @@ void PortsOrch::removeQueueMapPerPort(const Port& port) queueTypeVector.emplace_back(id); queueIndexVector.emplace_back(id); } + } + + m_counter_db->hdel(COUNTERS_QUEUE_NAME_MAP, queueVector); + m_counter_db->hdel(COUNTERS_QUEUE_PORT_MAP, queuePortVector); + m_counter_db->hdel(COUNTERS_QUEUE_INDEX_MAP, queueIndexVector); + m_counter_db->hdel(COUNTERS_QUEUE_TYPE_MAP, queueTypeVector); + + for (size_t queueIndex = 0; queueIndex < port.m_queue_ids.size(); ++queueIndex) + { + const auto id = sai_serialize_object_id(port.m_queue_ids[queueIndex]); - // Install a flex counter for this queue to track stats std::unordered_set counter_stats; for (const auto& it: queue_stat_ids) { @@ -5457,17 +5464,12 @@ void PortsOrch::removeQueueMapPerPort(const Port& port) } queue_stat_manager.clearCounterIdList(port.m_queue_ids[queueIndex]); - /* add watermark queue counters */ + /* remove watermark queue counters */ string key = getQueueWatermarkFlexCounterTableKey(id); m_flexCounterTable->del(key); } - m_counter_db->hdel(COUNTERS_QUEUE_NAME_MAP, queueVector); - m_counter_db->hdel(COUNTERS_QUEUE_PORT_MAP, queuePortVector); - m_counter_db->hdel(COUNTERS_QUEUE_INDEX_MAP, queueIndexVector); - m_counter_db->hdel(COUNTERS_QUEUE_TYPE_MAP, queueTypeVector); - CounterCheckOrch::getInstance().removePort(port); } @@ -5551,8 +5553,7 @@ void PortsOrch::generatePriorityGroupMap() void PortsOrch::removePriorityGroupMapPerPort(const Port& port) { - /* Create the PG map in the Counter DB */ - /* Add stat counters to flex_counter */ + /* Remove the PG map in the Counter DB */ vector pgVector; vector pgPortVector; vector pgIndexVector; @@ -5567,20 +5568,24 @@ void PortsOrch::removePriorityGroupMapPerPort(const Port& port) pgVector.emplace_back(name.str()); pgPortVector.emplace_back(id); pgIndexVector.emplace_back(id); + } + + m_counter_db->hdel(COUNTERS_PG_NAME_MAP, pgVector); + m_counter_db->hdel(COUNTERS_PG_PORT_MAP, pgPortVector); + m_counter_db->hdel(COUNTERS_PG_INDEX_MAP, pgIndexVector); + for (size_t pgIndex = 0; pgIndex < port.m_priority_group_ids.size(); ++pgIndex) + { + const auto id = sai_serialize_object_id(port.m_priority_group_ids[pgIndex]); string key = getPriorityGroupWatermarkFlexCounterTableKey(id); m_flexCounterTable->del(key); key = getPriorityGroupDropPacketsFlexCounterTableKey(id); - /* Add dropped packets counters to flex_counter */ + /* remove dropped packets counters to flex_counter */ m_flexCounterTable->del(key); } - m_counter_db->hdel(COUNTERS_PG_NAME_MAP, pgVector); - m_counter_db->hdel(COUNTERS_PG_PORT_MAP, pgPortVector); - m_counter_db->hdel(COUNTERS_PG_INDEX_MAP, pgIndexVector); - CounterCheckOrch::getInstance().removePort(port); } diff --git a/tests/conftest.py b/tests/conftest.py index f1e8248a14..39706295af 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -22,6 +22,7 @@ from dvslib.dvs_pbh import DVSPbh from dvslib.dvs_route import DVSRoute from dvslib import dvs_vlan +from dvslib import dvs_port from dvslib import dvs_lag from dvslib import dvs_mirror from dvslib import dvs_policer @@ -1765,7 +1766,11 @@ def dvs_vlan_manager(request, dvs): dvs.get_counters_db(), dvs.get_app_db()) - +@pytest.yield_fixture(scope="class") +def dvs_port_manager(request, dvs): + request.cls.dvs_port = dvs_port.DVSPort(dvs.get_asic_db(), + dvs.get_config_db()) + @pytest.yield_fixture(scope="class") def dvs_mirror_manager(request, dvs): request.cls.dvs_mirror = dvs_mirror.DVSMirror(dvs.get_asic_db(), diff --git a/tests/dvslib/dvs_database.py b/tests/dvslib/dvs_database.py index f2657f7516..22f7b0f451 100644 --- a/tests/dvslib/dvs_database.py +++ b/tests/dvslib/dvs_database.py @@ -74,6 +74,17 @@ def delete_entry(self, table_name: str, key: str) -> None: table = swsscommon.Table(self.db_connection, table_name) table._del(key) # pylint: disable=protected-access + def delete_field(self, table_name: str, key: str, field: str) -> None: + """Remove a field from an entry stored at `key` in the specified table. + + Args: + table_name: The name of the table where the entry is being removed. + key: The key that maps to the entry being removed. + field: The field that needs to be removed + """ + table = swsscommon.Table(self.db_connection, table_name) + table.hdel(key, field) + def get_keys(self, table_name: str) -> List[str]: """Get all of the keys stored in the specified table. diff --git a/tests/dvslib/dvs_port.py b/tests/dvslib/dvs_port.py new file mode 100644 index 0000000000..8c53994242 --- /dev/null +++ b/tests/dvslib/dvs_port.py @@ -0,0 +1,20 @@ + +class DVSPort(object): + def __init__(self, adb, cdb): + self.asic_db = adb + self.config_db = cdb + + def remove_port(self, port_name): + self.config_db.delete_field("CABLE_LENGTH", "AZURE", port_name) + + port_bufferpg_keys = self.config_db.get_keys("BUFFER_PG|%s" % port_name) + for key in port_bufferpg_keys: + self.config_db.delete_entry("BUFFER_PG|%s|%s" % (port_name, key), "") + + port_bufferqueue_keys = self.config_db.get_keys("BUFFER_QUEUE|%s" % port_name) + for key in port_bufferqueue_keys: + self.config_db.delete_entry("BUFFER_QUEUE|%s|%s" % (port_name, key), "") + + self.config_db.delete_entry("BREAKOUT_CFG|%s" % port_name, "") + self.config_db.delete_entry("INTERFACE|%s" % port_name, "") + self.config_db.delete_entry("PORT", port_name) diff --git a/tests/test_drop_counters.py b/tests/test_drop_counters.py index f7ba5d4fb2..dc6c8adce3 100644 --- a/tests/test_drop_counters.py +++ b/tests/test_drop_counters.py @@ -12,7 +12,7 @@ # Debug Counter Table DEBUG_COUNTER_TABLE = 'DEBUG_COUNTER' -DROP_REASON_TABLE = 'DEBUG_COUNTER_DROP_REASON' +DROP_REASON_TABLE = 'DEBUG_COUNTER_DROP_REASON' # Debug Counter Capability Table CAPABILITIES_TABLE = 'DEBUG_COUNTER_CAPABILITIES' @@ -55,16 +55,22 @@ EXPECTED_ASIC_FIELDS = [ASIC_COUNTER_TYPE_FIELD, ASIC_COUNTER_INGRESS_REASON_LIST_FIELD, ASIC_COUNTER_EGRESS_REASON_LIST_FIELD] EXPECTED_NUM_ASIC_FIELDS = 2 +# port to be add and removed +PORT = "Ethernet0" +PORT_TABLE_NAME = "PORT" +@pytest.mark.usefixtures('dvs_port_manager') # FIXME: It is really annoying to have to re-run tests due to inconsistent timing, should # implement some sort of polling interface for checking ASIC/flex counter tables after # applying changes to config DB class TestDropCounters(object): + def setup_db(self, dvs): self.asic_db = swsscommon.DBConnector(1, dvs.redis_sock, 0) self.config_db = swsscommon.DBConnector(4, dvs.redis_sock, 0) self.flex_db = swsscommon.DBConnector(5, dvs.redis_sock, 0) self.state_db = swsscommon.DBConnector(6, dvs.redis_sock, 0) + self.counters_db = swsscommon.DBConnector(2, dvs.redis_sock, 0) def genericGetAndAssert(self, table, key): status, fields = table.get(key) @@ -157,30 +163,30 @@ def test_deviceCapabilitiesTablePopulated(self, dvs, testlog): correct values. """ self.setup_db(dvs) - + # Check that the capabilities table 1) exists and 2) has been populated # for each type of counter capabilities_table = swsscommon.Table(self.state_db, CAPABILITIES_TABLE) counter_types = capabilities_table.getKeys() assert len(counter_types) == len(SUPPORTED_COUNTER_TYPES) - + # Check that the data for each counter type is consistent for counter_type in SUPPORTED_COUNTER_TYPES: assert counter_type in counter_types - + # By definiton, each capability entry should contain exactly the same fields counter_capabilities = self.genericGetAndAssert(capabilities_table, counter_type) assert len(counter_capabilities) == len(SUPPORTED_COUNTER_CAPABILITIES) - + # Check that the values of each field actually match the # capabilities currently defined in the virtual switch for capability in counter_capabilities: assert len(capability) == 2 capability_name = capability[0] capability_contents = capability[1] - + assert capability_name in SUPPORTED_COUNTER_CAPABILITIES - + if capability_name == CAP_COUNT: assert int(capability_contents) == 3 elif capability_name == CAP_REASONS and counter_type in INGRESS_COUNTER_TYPES: @@ -189,7 +195,7 @@ def test_deviceCapabilitiesTablePopulated(self, dvs, testlog): assert len(capability_contents.split(',')) == 2 else: assert False - + def test_flexCounterGroupInitialized(self, dvs, testlog): """ This test verifies that DebugCounterOrch has succesfully @@ -197,51 +203,51 @@ def test_flexCounterGroupInitialized(self, dvs, testlog): """ self.setup_db(dvs) self.checkFlexCounterGroup() - + def test_createAndRemoveDropCounterBasic(self, dvs, testlog): """ This test verifies that a drop counter can succesfully be created and deleted. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'TEST' reason = 'L3_ANY' - + self.create_drop_counter(name, PORT_INGRESS_DROPS) - + # Because no reasons have been added to the counter yet, nothing should # be put in ASIC DB and the flex counters should not start polling yet. assert len(asic_state_table.getKeys()) == 0 assert len(flex_counter_table.getKeys()) == 0 - + self.add_drop_reason(name, reason) time.sleep(3) - + # Verify that the flex counters have been created to poll the new # counter. self.checkFlexState([PORT_STAT_BASE], PORT_DEBUG_COUNTER_LIST) - + # Verify that the drop counter has been added to ASIC DB with the # correct reason added. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_PORT_IN_TYPE, [reason]) - + self.delete_drop_counter(name) time.sleep(3) - + # Verify that the counter has been removed from ASIC DB and the flex # counters have been torn down. assert len(asic_state_table.getKeys()) == 0 assert len(flex_counter_table.getKeys()) == 0 - + # Cleanup for the next test. self.remove_drop_reason(name, reason) - + def test_createAndRemoveDropCounterReversed(self, dvs, testlog): """ This test verifies that a drop counter can succesfully be created @@ -249,436 +255,436 @@ def test_createAndRemoveDropCounterReversed(self, dvs, testlog): created. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'TEST' reason = 'L3_ANY' - + self.add_drop_reason(name, reason) - + # Because the actual counter has not been created yet, nothing should # be put in ASIC DB and the flex counters should not start polling yet. assert len(asic_state_table.getKeys()) == 0 assert len(flex_counter_table.getKeys()) == 0 - + self.create_drop_counter(name, PORT_INGRESS_DROPS) time.sleep(3) - + # Verify that the flex counters have been created to poll the new # counter. self.checkFlexState([PORT_STAT_BASE], PORT_DEBUG_COUNTER_LIST) - + # Verify that the drop counter has been added to ASIC DB with the # correct reason added. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_PORT_IN_TYPE, [reason]) - + self.delete_drop_counter(name) time.sleep(3) - + # Verify that the counter has been removed from ASIC DB and the flex # counters have been torn down. assert len(asic_state_table.getKeys()) == 0 assert len(flex_counter_table.getKeys()) == 0 - + # Cleanup for the next test. self.remove_drop_reason(name, reason) - + def test_createCounterWithInvalidCounterType(self, dvs, testlog): """ This test verifies that the state of the system is unaffected when an invalid counter type is passed to CONFIG DB. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'BAD_CTR' reason = 'L3_ANY' - + self.create_drop_counter(name, 'THIS_IS_DEFINITELY_NOT_A_VALID_COUNTER_TYPE') self.add_drop_reason(name, reason) time.sleep(3) - + # Verify that nothing has been added to ASIC DB and no flex counters # were created. assert len(asic_state_table.getKeys()) == 0 assert len(flex_counter_table.getKeys()) == 0 - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason) - + def test_createCounterWithInvalidDropReason(self, dvs, testlog): """ This test verifies that the state of the system is unaffected when an invalid drop reason is passed to CONFIG DB. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'BAD_CTR' reason = 'THIS_IS_DEFINITELY_NOT_A_VALID_DROP_REASON' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason) time.sleep(3) - + # Verify that nothing has been added to ASIC DB and no flex counters # were created. assert len(asic_state_table.getKeys()) == 0 assert len(flex_counter_table.getKeys()) == 0 - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason) - + def test_addReasonToInitializedCounter(self, dvs, testlog): """ This test verifies that a drop reason can be added to a counter that has already been initialized. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) time.sleep(3) - + # Verify that a counter has been created. We will verify the state of # the counter in the next step. assert len(asic_state_table.getKeys()) == 1 self.checkFlexState([SWITCH_STAT_BASE], SWITCH_DEBUG_COUNTER_LIST) - + reason2 = 'ACL_ANY' self.add_drop_reason(name, reason2) time.sleep(3) - + # Verify that the drop counter has been added to ASIC DB, including the # reason that was added. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1, reason2]) - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason1) self.remove_drop_reason(name, reason2) - + def test_removeReasonFromInitializedCounter(self, dvs, testlog): """ This test verifies that a drop reason can be removed from a counter that has already been initialized without deleting the counter. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' reason2 = 'ACL_ANY' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) self.add_drop_reason(name, reason2) time.sleep(3) - + # Verify that a counter has been created. We will verify the state of # the counter in the next step. assert len(asic_state_table.getKeys()) == 1 self.checkFlexState([SWITCH_STAT_BASE], SWITCH_DEBUG_COUNTER_LIST) - + self.remove_drop_reason(name, reason2) time.sleep(3) - + # Verify that the drop counter has been added to ASIC DB, excluding the # reason that was removed. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason1) - + def test_removeAllDropReasons(self, dvs, testlog): """ This test verifies that it is not possible to remove all drop reasons from a drop counter. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) time.sleep(3) - + # Verify that a counter has been created. We will verify the state of # the counter in the next step. assert len(asic_state_table.getKeys()) == 1 self.checkFlexState([SWITCH_STAT_BASE], SWITCH_DEBUG_COUNTER_LIST) - + self.remove_drop_reason(name, reason1) time.sleep(3) - + # Verify that the drop counter has been added to ASIC DB, including the # last reason that we attempted to remove. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + # Cleanup for the next test. self.delete_drop_counter(name) - + def test_addDropReasonMultipleTimes(self, dvs, testlog): """ This test verifies that the same drop reason can be added multiple times without affecting the system. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) time.sleep(3) - + # Verify that a counter has been created. We will verify the state of # the counter in the next step. assert len(asic_state_table.getKeys()) == 1 self.checkFlexState([SWITCH_STAT_BASE], SWITCH_DEBUG_COUNTER_LIST) - + reason2 = 'ACL_ANY' self.add_drop_reason(name, reason2) time.sleep(3) - + # Verify that the drop counter has been added to ASIC DB, including the # reason that was added. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1, reason2]) - + self.add_drop_reason(name, reason2) time.sleep(3) - + # Verify that the ASIC state is the same as before adding the redundant # drop reason. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1, reason2]) - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason1) self.remove_drop_reason(name, reason2) - + def test_addInvalidDropReason(self, dvs, testlog): """ This test verifies that adding a drop reason to a counter that is not recognized will not affect the system. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) time.sleep(3) - + # Verify that a counter has been created. We will verify the state of # the counter in the next step. assert len(asic_state_table.getKeys()) == 1 self.checkFlexState([SWITCH_STAT_BASE], SWITCH_DEBUG_COUNTER_LIST) - + reason2 = 'ACL_ANY' self.add_drop_reason(name, reason2) time.sleep(3) - + # Verify that the drop counter has been added to ASIC DB, including the # reason that was added. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1, reason2]) - + dummy_reason = 'ZOBOOMBAFOO' self.add_drop_reason(name, dummy_reason) time.sleep(3) - + # Verify that the ASIC state is the same as before adding the invalid # drop reason. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1, reason2]) - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason1) self.remove_drop_reason(name, reason2) - + def test_removeDropReasonMultipleTimes(self, dvs, testlog): """ This test verifies that removing a drop reason multiple times will not affect the system. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' reason2 = 'ACL_ANY' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) self.add_drop_reason(name, reason2) time.sleep(3) - + # Verify that a counter has been created. We will verify the state of # the counter in the next step. assert len(asic_state_table.getKeys()) == 1 self.checkFlexState([SWITCH_STAT_BASE], SWITCH_DEBUG_COUNTER_LIST) - + self.remove_drop_reason(name, reason2) time.sleep(3) - + # Verify that the drop counter has been added to ASIC DB, excluding the # reason that was removed. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + self.remove_drop_reason(name, reason2) time.sleep(3) - + # Verify that the ASIC state is the same as before the redundant # remove operation. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason1) - + def test_removeNonexistentDropReason(self, dvs, testlog): """ This test verifies that removing a drop reason that does not exist on the device will not affect the system. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' reason2 = 'ACL_ANY' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) time.sleep(3) - + # Verify the counter has been created and is in the correct state. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + self.remove_drop_reason(name, reason2) time.sleep(3) - + # Verify that the ASIC state is unchanged after the nonexistent remove. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason1) - + def test_removeInvalidDropReason(self, dvs, testlog): """ This test verifies that removing a drop reason that is not recognized will not affect the system. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' bogus_reason = 'LIVE_LAUGH_LOVE' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) time.sleep(3) - + # Verify the counter has been created and is in the correct state. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + self.remove_drop_reason(name, bogus_reason) time.sleep(3) - + # Verify that the ASIC state is unchanged after the bad remove. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason1) - + def getPortOid(self, dvs, port_name): - cnt_r = redis.Redis(unix_socket_path=dvs.redis_sock, db=swsscommon.COUNTERS_DB, - encoding="utf-8", decode_responses=True) - return cnt_r.hget("COUNTERS_PORT_NAME_MAP", port_name); + port_name_map = swsscommon.Table(self.counters_db, "COUNTERS_PORT_NAME_MAP") + status, returned_value = port_name_map.hget("", port_name); + assert status == True + return returned_value def test_add_remove_port(self, dvs, testlog): - """ This test verifies that debug counters are removed when we remove a port and debug counters are added each time we add ports (if debug counter is enabled) """ self.setup_db(dvs) - # save ethernet0 info + # save port info cdb = swsscommon.DBConnector(4, dvs.redis_sock, 0) - tbl = swsscommon.Table(cdb, "PORT") - (status, fvs) = tbl.get("Ethernet0") + tbl = swsscommon.Table(cdb, PORT_TABLE_NAME) + (status, fvs) = tbl.get(PORT) assert status == True # get counter oid - oid = self.getPortOid(dvs, "Ethernet0") + oid = self.getPortOid(dvs, PORT) - # verifies debug coutner dont exist for ethernet0 + # verifies debug coutner dont exist for port flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) status, fields = flex_counter_table.get(oid) assert len(fields) == 0 @@ -686,47 +692,38 @@ def test_add_remove_port(self, dvs, testlog): # add debug counters name1 = 'DEBUG_0' reason1 = 'L3_ANY' - name2 = 'DEBUG_1' reason2 = 'L2_ANY' - + self.create_drop_counter(name1, PORT_INGRESS_DROPS) self.add_drop_reason(name1, reason1) - + self.create_drop_counter(name2, PORT_EGRESS_DROPS) self.add_drop_reason(name2, reason2) - - time.sleep(5) + time.sleep(3) - # verifies debug coutner exist for ethernet0 + # verifies debug counter exist for port flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) status, fields = flex_counter_table.get(oid) assert status == True assert len(fields) == 1 - # remove Ethernet0 port - cdb.hdel("CABLE_LENGTH|AZURE", "Ethernet0") - ethernet0_bufferpg_keys = cdb.keys("BUFFER_PG|Ethernet0|*") - for key in ethernet0_bufferpg_keys: - cdb._del(key) - ethernet0_bufferqueue_keys = cdb.keys("BUFFER_QUEUE|Ethernet0|*") - for key in ethernet0_bufferqueue_keys: - cdb._del(key) - cdb._del("BREAKOUT_CFG|Ethernet0") - tbl._del("Ethernet0") - time.sleep(5) - + # remove port and wait until it was removed from ASIC DB + self.dvs_port.remove_port(PORT) + dvs.get_asic_db().wait_for_deleted_entry("ASIC_STATE:SAI_OBJECT_TYPE_PORT", oid) + # verify that debug counter were removed status, fields = flex_counter_table.get(oid) assert len(fields) == 0 - # add Ethernet0 - tbl.set("Ethernet0", fvs) - time.sleep(5) - - # verifies that debug counters were added for ethernet0 - oid = self.getPortOid(dvs, "Ethernet0") - + # add port and wait until the port is added on asic db + num_of_keys_without_port = len(dvs.get_asic_db().get_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT")) + tbl.set(PORT, fvs) + dvs.get_asic_db().wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", num_of_keys_without_port + 1) + dvs.get_counters_db().wait_for_fields("COUNTERS_PORT_NAME_MAP", "", [PORT]) + + # verifies that debug counters were added for port + oid = self.getPortOid(dvs, PORT) status, fields = flex_counter_table.get(oid) assert status == True assert len(fields) == 1 @@ -737,7 +734,6 @@ def test_add_remove_port(self, dvs, testlog): self.delete_drop_counter(name2) self.remove_drop_reason(name2, reason2) - def test_createAndDeleteMultipleCounters(self, dvs, testlog): """ @@ -745,47 +741,47 @@ def test_createAndDeleteMultipleCounters(self, dvs, testlog): at the same time works correctly. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name1 = 'DEBUG_0' reason1 = 'L3_ANY' - + name2 = 'DEBUG_1' reason2 = 'ACL_ANY' - + self.create_drop_counter(name1, PORT_INGRESS_DROPS) self.add_drop_reason(name1, reason1) - + self.create_drop_counter(name2, PORT_INGRESS_DROPS) self.add_drop_reason(name2, reason2) - + time.sleep(5) - + # Verify that the flex counters are correctly tracking two different # drop counters. self.checkFlexState([PORT_STAT_BASE, PORT_STAT_INDEX_1], PORT_DEBUG_COUNTER_LIST) - + # Verify that there are two entries in the ASIC DB, one for each counter. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 2 for key in asic_keys: assert (self.checkASICState(key, ASIC_COUNTER_PORT_IN_TYPE, [reason1]) or self.checkASICState(key, ASIC_COUNTER_PORT_IN_TYPE, [reason2])) - + self.delete_drop_counter(name2) self.remove_drop_reason(name2, reason2) time.sleep(3) - + # Verify that the flex counters are tracking ONE drop counter after # the update. self.checkFlexState([PORT_STAT_BASE], PORT_DEBUG_COUNTER_LIST) - + # Verify that there is ONE entry in the ASIC DB after the update. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_PORT_IN_TYPE, [reason1]) - + # Cleanup for the next test. self.delete_drop_counter(name1) self.remove_drop_reason(name1, reason1) diff --git a/tests/test_flex_counters.py b/tests/test_flex_counters.py index c098cd91f2..1fbedce2c6 100644 --- a/tests/test_flex_counters.py +++ b/tests/test_flex_counters.py @@ -5,6 +5,7 @@ NUMBER_OF_RETRIES = 10 CPU_PORT_OID = "0x0" +PORT = "Ethernet0" counter_group_meta = { 'port_counter': { @@ -62,6 +63,7 @@ } } +@pytest.mark.usefixtures('dvs_port_manager') class TestFlexCounters(object): def setup_dbs(self, dvs): @@ -362,84 +364,83 @@ def test_remove_trap_group(self, dvs): self.set_flex_counter_group_status(meta_data['key'], meta_data['group_name'], 'disable') - def remove_port(self, config_db, port): - config_db.delete_entry("CABLE_LENGTH|AZURE", "") - ethernet0_bufferpg_keys = config_db.get_keys("BUFFER_PG|%s"%port) - for key in ethernet0_bufferpg_keys: - config_db.delete_entry("BUFFER_PG|Ethernet0|%s"%key, "") - ethernet0_bufferqueue_keys = config_db.get_keys("BUFFER_QUEUE|%s"%port) - for key in ethernet0_bufferqueue_keys: - config_db.delete_entry("BUFFER_QUEUE|Ethernet0|%s"%key, "") - config_db.delete_entry("BREAKOUT_CFG|%s"%port, "") - config_db.delete_entry("INTERFACE|%s"%port, "") - config_db.delete_entry("PORT", port) - def test_add_remove_ports(self, dvs): self.setup_dbs(dvs) # set flex counter - counter_key = counter_type_dict['queue_counter'][0] - counter_stat = counter_type_dict['queue_counter'][1] - counter_map = counter_type_dict['queue_counter'][2] - self.enable_flex_counter_group(counter_key, counter_map) - time.sleep(3) - - # receive Ethernet0 info - fvs = self.config_db.get_entry("PORT", "Ethernet0") + counter_key = counter_group_meta['queue_counter']['key'] + counter_stat = counter_group_meta['queue_counter']['group_name'] + counter_map = counter_group_meta['queue_counter']['name_map'] + self.set_flex_counter_group_status(counter_key, counter_map) + + # receive port info + fvs = self.config_db.get_entry("PORT", PORT) assert len(fvs) > 0 # save all the oids of the pg drop counters oid_list = [] - counters_queue_map = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP","") + counters_queue_map = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP", "") - for i in range(0,100): - if 'Ethernet0:%d'%i in counters_queue_map: - oid_list.append(counters_queue_map['Ethernet0:%d'%i]) + i = 0 + while True: + if '%s:%d' % (PORT, i) in counters_queue_map: + oid_list.append(counters_queue_map['%s:%d' % (PORT, i)]) + i += 1 else: break # verify that counters exists on flex counter - for oid in oid_list: - fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s"%oid) + for oid in oid_list: + fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s" % oid) assert len(fields) == 1 - # remove port Ethernet0 - self.remove_port(self.config_db, "Ethernet0") - time.sleep(3) - + # get port oid + port_oid = self.counters_db.get_entry(PORT_MAP, "")[PORT] + + # remove port and verify that it was removed properly + self.dvs_port.remove_port(PORT) + dvs.get_asic_db().wait_for_deleted_entry("ASIC_STATE:SAI_OBJECT_TYPE_PORT", port_oid) # verify counters were removed from flex counter table for oid in oid_list: - fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s"%oid) + fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s" % oid) assert len(fields) == 0 - # verify that Ethernet0 counter maps were removed + # verify that port counter maps were removed oid_list = [] - counters_queue_map = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP","") + counters_queue_map = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP", "") - for i in range(0,100): - if 'Ethernet0:%d'%i in counters_queue_map: - oid_list.append(counters_queue_map['Ethernet0:%d'%i]) + i = 0 + while True: + if '%s:%d' % (PORT, i) in counters_queue_map: + oid_list.append(counters_queue_map['%s:%d' % (PORT, i)]) + i += 1 else: break assert oid_list == [] - - # add port Ethernet 0 - self.config_db.create_entry("PORT","Ethernet0",fvs) - time.sleep(3) - # verify counter was added + # add port and wait until the port is added on asic db + num_of_keys_without_port = len(dvs.get_asic_db().get_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT")) + + self.config_db.create_entry("PORT", PORT, fvs) + + dvs.get_asic_db().wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", num_of_keys_without_port + 1) + dvs.get_counters_db().wait_for_fields("COUNTERS_QUEUE_NAME_MAP", "", ["%s:0"%(PORT)]) + + # verify queue counters were added oid_list = [] - counters_queue_map = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP","") + counters_queue_map = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP", "") - for i in range(0,100): - if 'Ethernet0:%d'%i in counters_queue_map: - oid_list.append(counters_queue_map['Ethernet0:%d'%i]) + while True: + if '%s:%d' % (PORT, i) in counters_queue_map: + oid_list.append(counters_queue_map['%s:%d' % (PORT, i)]) + i += 1 else: break + assert len(oid_list) > 0 # verify that counters exists on flex counter - for oid in oid_list: - fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s"%oid) + for oid in oid_list: + fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s" % oid) assert len(fields) == 1 diff --git a/tests/test_pg_drop_counter.py b/tests/test_pg_drop_counter.py index da49d52bc6..fe8321e90d 100644 --- a/tests/test_pg_drop_counter.py +++ b/tests/test_pg_drop_counter.py @@ -9,6 +9,8 @@ pg_drop_attr = "SAI_INGRESS_PRIORITY_GROUP_STAT_DROPPED_PACKETS" +PORT = "Ethernet0" +@pytest.mark.usefixtures('dvs_port_manager') class TestPGDropCounter(object): DEFAULT_POLL_INTERVAL = 10 pgs = {} @@ -73,18 +75,6 @@ def clear_flex_counter(self): self.config_db.delete_entry("FLEX_COUNTER_TABLE", "PG_DROP") self.config_db.delete_entry("FLEX_COUNTER_TABLE", "PG_WATERMARK") - - def remove_port(self, config_db, port): - config_db.hdel("CABLE_LENGTH|AZURE", port) - ethernet0_bufferpg_keys = config_db.keys("BUFFER_PG|%s|*"%port) - for key in ethernet0_bufferpg_keys: - config_db._del(key) - ethernet0_bufferqueue_keys = config_db.keys("BUFFER_QUEUE|%s|*"%port) - for key in ethernet0_bufferqueue_keys: - config_db._del(key) - config_db._del("BREAKOUT_CFG|%s"%port) - port_table = swsscommon.Table(config_db, "PORT") - port_table._del(port) def test_pg_drop_counters(self, dvs): self.setup_dbs(dvs) @@ -113,43 +103,38 @@ def test_pg_drop_counter_port_add_remove(self, dvs): try: # configure pg drop flex counter self.set_up_flex_counter() - time.sleep(5) # receive Ethernet0 info - config_db = swsscommon.DBConnector(4, dvs.redis_sock, 0) - port_table = swsscommon.Table(config_db, "PORT") - (status, fvs) = port_table.get("Ethernet0") - assert status == True - - + fvs = self.config_db.get_entry("PORT", PORT) + assert len(fvs) > 0 + # save all the oids of the pg drop counters - cnt_r = redis.Redis(unix_socket_path=dvs.redis_sock, db=swsscommon.COUNTERS_DB, - encoding="utf-8", decode_responses=True) - oid_list = [] for priority in range(0,7): - oid_list.append(cnt_r.hget("COUNTERS_PG_NAME_MAP", "Ethernet0:%d"%priority)) - + oid_list.append(dvs.get_counters_db().get_entry("COUNTERS_PG_NAME_MAP", "")["%s:%d"%(PORT, priority)]) # verify that counters exists on flex counter fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", "PG_WATERMARK_STAT_COUNTER:%s"%oid_list[-1]) assert len(fields) == 1 - # remove port Ethernet0 - self.remove_port(config_db, "Ethernet0") - time.sleep(3) - + # remove port + port_oid = self.counters_db.get_entry("COUNTERS_PORT_NAME_MAP", "")["Ethernet0"] + self.dvs_port.remove_port(PORT) + dvs.get_asic_db().wait_for_deleted_entry("ASIC_STATE:SAI_OBJECT_TYPE_PORT", port_oid) + # verify counters were removed from flex counter table for oid in oid_list: fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", "PG_WATERMARK_STAT_COUNTER:%s"%oid) assert len(fields) == 0 - # add port Ethernet 0 - port_table.set("Ethernet0", fvs) - time.sleep(3) + # add port and wait until the port is added on asic db + num_of_keys_without_port = len(dvs.get_asic_db().get_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT")) + self.config_db.create_entry("PORT", PORT, fvs) + dvs.get_asic_db().wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", num_of_keys_without_port + 1) + dvs.get_counters_db().wait_for_fields("COUNTERS_PG_NAME_MAP", "", ["%s:0"%(PORT)]) # verify counter was added for priority in range(0,7): - oid = cnt_r.hget("COUNTERS_PG_NAME_MAP", "Ethernet0:%d"%priority) + oid = dvs.get_counters_db().get_entry("COUNTERS_PG_NAME_MAP", "")["%s:%d"%(PORT, priority)] # verify that counters exists on flex counter fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", "PG_WATERMARK_STAT_COUNTER:%s"%oid) From 42de3628ab37dc5cba03752a0216e5cca728f224 Mon Sep 17 00:00:00 2001 From: tomeri Date: Sun, 24 Oct 2021 11:55:28 +0000 Subject: [PATCH 03/15] countrs support for dynamic port add/remove --- orchagent/debugcounterorch.cpp | 2 -- orchagent/portsorch.cpp | 3 +-- tests/test_drop_counters.py | 1 - tests/test_flex_counters.py | 48 +++++++++------------------------- tests/test_pg_drop_counter.py | 6 ++--- 5 files changed, 16 insertions(+), 44 deletions(-) diff --git a/orchagent/debugcounterorch.cpp b/orchagent/debugcounterorch.cpp index a2d28ee7ab..7ec467ae31 100644 --- a/orchagent/debugcounterorch.cpp +++ b/orchagent/debugcounterorch.cpp @@ -48,9 +48,7 @@ void DebugCounterOrch::update(SubjectType type, void *cntx) { SWSS_LOG_ENTER(); - if (type == SUBJECT_TYPE_PORT_CHANGE) { - if (!cntx) { SWSS_LOG_ERROR("cntx is NULL"); return; diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index ba06c542f1..e6ebde3c24 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -5728,8 +5728,7 @@ void PortsOrch::doTask(NotificationConsumer &consumer) if (!getPort(id, port)) { - /* consider changing it to notice */ - SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64 " - its ok to ignore this message if this port was just removed", id); + SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64 ", id); continue; } diff --git a/tests/test_drop_counters.py b/tests/test_drop_counters.py index dc6c8adce3..0e547d3335 100644 --- a/tests/test_drop_counters.py +++ b/tests/test_drop_counters.py @@ -1,6 +1,5 @@ import time import pytest -import redis from swsscommon import swsscommon diff --git a/tests/test_flex_counters.py b/tests/test_flex_counters.py index 1fbedce2c6..451d4ab07c 100644 --- a/tests/test_flex_counters.py +++ b/tests/test_flex_counters.py @@ -380,19 +380,10 @@ def test_add_remove_ports(self, dvs): # save all the oids of the pg drop counters oid_list = [] counters_queue_map = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP", "") - - i = 0 - while True: - if '%s:%d' % (PORT, i) in counters_queue_map: - oid_list.append(counters_queue_map['%s:%d' % (PORT, i)]) - i += 1 - else: - break - - # verify that counters exists on flex counter - for oid in oid_list: - fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s" % oid) - assert len(fields) == 1 + for key, oid in counters_queue_map.items(): + if PORT in key: + fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s" % oid) + assert len(fields) == 1 # get port oid port_oid = self.counters_db.get_entry(PORT_MAP, "")[PORT] @@ -406,19 +397,12 @@ def test_add_remove_ports(self, dvs): fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s" % oid) assert len(fields) == 0 - # verify that port counter maps were removed + # verify that port counter maps were removed from counters db oid_list = [] counters_queue_map = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP", "") - - i = 0 - while True: - if '%s:%d' % (PORT, i) in counters_queue_map: - oid_list.append(counters_queue_map['%s:%d' % (PORT, i)]) - i += 1 - else: - break - assert oid_list == [] - + for key in counters_queue_map.keys(): + if PORT in key: + assert False # add port and wait until the port is added on asic db num_of_keys_without_port = len(dvs.get_asic_db().get_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT")) @@ -431,16 +415,8 @@ def test_add_remove_ports(self, dvs): # verify queue counters were added oid_list = [] counters_queue_map = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP", "") - - while True: - if '%s:%d' % (PORT, i) in counters_queue_map: - oid_list.append(counters_queue_map['%s:%d' % (PORT, i)]) - i += 1 - else: - break - assert len(oid_list) > 0 - # verify that counters exists on flex counter - for oid in oid_list: - fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s" % oid) - assert len(fields) == 1 + for key, oid in counters_queue_map.items(): + if PORT in key: + fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s" % oid) + assert len(fields) == 1 diff --git a/tests/test_pg_drop_counter.py b/tests/test_pg_drop_counter.py index fe8321e90d..b3682881de 100644 --- a/tests/test_pg_drop_counter.py +++ b/tests/test_pg_drop_counter.py @@ -10,6 +10,7 @@ pg_drop_attr = "SAI_INGRESS_PRIORITY_GROUP_STAT_DROPPED_PACKETS" PORT = "Ethernet0" + @pytest.mark.usefixtures('dvs_port_manager') class TestPGDropCounter(object): DEFAULT_POLL_INTERVAL = 10 @@ -82,7 +83,6 @@ def test_pg_drop_counters(self, dvs): try: self.set_up_flex_counter() - self.populate_asic(dvs, "0") time.sleep(self.DEFAULT_POLL_INTERVAL) self.verify_value(dvs, self.pgs, pg_drop_attr, "0") @@ -104,7 +104,7 @@ def test_pg_drop_counter_port_add_remove(self, dvs): # configure pg drop flex counter self.set_up_flex_counter() - # receive Ethernet0 info + # receive port info fvs = self.config_db.get_entry("PORT", PORT) assert len(fvs) > 0 @@ -117,7 +117,7 @@ def test_pg_drop_counter_port_add_remove(self, dvs): assert len(fields) == 1 # remove port - port_oid = self.counters_db.get_entry("COUNTERS_PORT_NAME_MAP", "")["Ethernet0"] + port_oid = self.counters_db.get_entry("COUNTERS_PORT_NAME_MAP", "")[PORT] self.dvs_port.remove_port(PORT) dvs.get_asic_db().wait_for_deleted_entry("ASIC_STATE:SAI_OBJECT_TYPE_PORT", port_oid) From eb82018544db48488f73c4ff84bb16d0c085170b Mon Sep 17 00:00:00 2001 From: tomeri Date: Sun, 24 Oct 2021 11:55:28 +0000 Subject: [PATCH 04/15] countrs support for dynamic port add/remove --- orchagent/orchdaemon.cpp | 2 - orchagent/portsorch.cpp | 2 +- tests/test_drop_counters.py | 247 ++++++++++++++++++------------------ tests/test_flex_counters.py | 23 +++- 4 files changed, 146 insertions(+), 128 deletions(-) diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 3d683ce522..a6563d45ca 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -429,8 +429,6 @@ bool OrchDaemon::init() m_orchList.push_back(gMlagOrch); m_orchList.push_back(gIsoGrpOrch); m_orchList.push_back(gFgNhgOrch); - m_orchList.push_back(mux_orch); - m_orchList.push_back(mux_cb_orch); m_orchList.push_back(mux_st_orch); m_orchList.push_back(nvgre_tunnel_orch); m_orchList.push_back(nvgre_tunnel_map_orch); diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index e6ebde3c24..d8e7bbe5e1 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -5728,7 +5728,7 @@ void PortsOrch::doTask(NotificationConsumer &consumer) if (!getPort(id, port)) { - SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64 ", id); + SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64, id); continue; } diff --git a/tests/test_drop_counters.py b/tests/test_drop_counters.py index 0e547d3335..cd089be917 100644 --- a/tests/test_drop_counters.py +++ b/tests/test_drop_counters.py @@ -63,7 +63,6 @@ # implement some sort of polling interface for checking ASIC/flex counter tables after # applying changes to config DB class TestDropCounters(object): - def setup_db(self, dvs): self.asic_db = swsscommon.DBConnector(1, dvs.redis_sock, 0) self.config_db = swsscommon.DBConnector(4, dvs.redis_sock, 0) @@ -162,30 +161,30 @@ def test_deviceCapabilitiesTablePopulated(self, dvs, testlog): correct values. """ self.setup_db(dvs) - + # Check that the capabilities table 1) exists and 2) has been populated # for each type of counter capabilities_table = swsscommon.Table(self.state_db, CAPABILITIES_TABLE) counter_types = capabilities_table.getKeys() assert len(counter_types) == len(SUPPORTED_COUNTER_TYPES) - + # Check that the data for each counter type is consistent for counter_type in SUPPORTED_COUNTER_TYPES: assert counter_type in counter_types - + # By definiton, each capability entry should contain exactly the same fields counter_capabilities = self.genericGetAndAssert(capabilities_table, counter_type) assert len(counter_capabilities) == len(SUPPORTED_COUNTER_CAPABILITIES) - + # Check that the values of each field actually match the # capabilities currently defined in the virtual switch for capability in counter_capabilities: assert len(capability) == 2 capability_name = capability[0] capability_contents = capability[1] - + assert capability_name in SUPPORTED_COUNTER_CAPABILITIES - + if capability_name == CAP_COUNT: assert int(capability_contents) == 3 elif capability_name == CAP_REASONS and counter_type in INGRESS_COUNTER_TYPES: @@ -194,7 +193,7 @@ def test_deviceCapabilitiesTablePopulated(self, dvs, testlog): assert len(capability_contents.split(',')) == 2 else: assert False - + def test_flexCounterGroupInitialized(self, dvs, testlog): """ This test verifies that DebugCounterOrch has succesfully @@ -202,51 +201,51 @@ def test_flexCounterGroupInitialized(self, dvs, testlog): """ self.setup_db(dvs) self.checkFlexCounterGroup() - + def test_createAndRemoveDropCounterBasic(self, dvs, testlog): """ This test verifies that a drop counter can succesfully be created and deleted. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'TEST' reason = 'L3_ANY' - + self.create_drop_counter(name, PORT_INGRESS_DROPS) - + # Because no reasons have been added to the counter yet, nothing should # be put in ASIC DB and the flex counters should not start polling yet. assert len(asic_state_table.getKeys()) == 0 assert len(flex_counter_table.getKeys()) == 0 - + self.add_drop_reason(name, reason) time.sleep(3) - + # Verify that the flex counters have been created to poll the new # counter. self.checkFlexState([PORT_STAT_BASE], PORT_DEBUG_COUNTER_LIST) - + # Verify that the drop counter has been added to ASIC DB with the # correct reason added. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_PORT_IN_TYPE, [reason]) - + self.delete_drop_counter(name) time.sleep(3) - + # Verify that the counter has been removed from ASIC DB and the flex # counters have been torn down. assert len(asic_state_table.getKeys()) == 0 assert len(flex_counter_table.getKeys()) == 0 - + # Cleanup for the next test. self.remove_drop_reason(name, reason) - + def test_createAndRemoveDropCounterReversed(self, dvs, testlog): """ This test verifies that a drop counter can succesfully be created @@ -254,409 +253,409 @@ def test_createAndRemoveDropCounterReversed(self, dvs, testlog): created. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'TEST' reason = 'L3_ANY' - + self.add_drop_reason(name, reason) - + # Because the actual counter has not been created yet, nothing should # be put in ASIC DB and the flex counters should not start polling yet. assert len(asic_state_table.getKeys()) == 0 assert len(flex_counter_table.getKeys()) == 0 - + self.create_drop_counter(name, PORT_INGRESS_DROPS) time.sleep(3) - + # Verify that the flex counters have been created to poll the new # counter. self.checkFlexState([PORT_STAT_BASE], PORT_DEBUG_COUNTER_LIST) - + # Verify that the drop counter has been added to ASIC DB with the # correct reason added. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_PORT_IN_TYPE, [reason]) - + self.delete_drop_counter(name) time.sleep(3) - + # Verify that the counter has been removed from ASIC DB and the flex # counters have been torn down. assert len(asic_state_table.getKeys()) == 0 assert len(flex_counter_table.getKeys()) == 0 - + # Cleanup for the next test. self.remove_drop_reason(name, reason) - + def test_createCounterWithInvalidCounterType(self, dvs, testlog): """ This test verifies that the state of the system is unaffected when an invalid counter type is passed to CONFIG DB. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'BAD_CTR' reason = 'L3_ANY' - + self.create_drop_counter(name, 'THIS_IS_DEFINITELY_NOT_A_VALID_COUNTER_TYPE') self.add_drop_reason(name, reason) time.sleep(3) - + # Verify that nothing has been added to ASIC DB and no flex counters # were created. assert len(asic_state_table.getKeys()) == 0 assert len(flex_counter_table.getKeys()) == 0 - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason) - + def test_createCounterWithInvalidDropReason(self, dvs, testlog): """ This test verifies that the state of the system is unaffected when an invalid drop reason is passed to CONFIG DB. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'BAD_CTR' reason = 'THIS_IS_DEFINITELY_NOT_A_VALID_DROP_REASON' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason) time.sleep(3) - + # Verify that nothing has been added to ASIC DB and no flex counters # were created. assert len(asic_state_table.getKeys()) == 0 assert len(flex_counter_table.getKeys()) == 0 - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason) - + def test_addReasonToInitializedCounter(self, dvs, testlog): """ This test verifies that a drop reason can be added to a counter that has already been initialized. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) time.sleep(3) - + # Verify that a counter has been created. We will verify the state of # the counter in the next step. assert len(asic_state_table.getKeys()) == 1 self.checkFlexState([SWITCH_STAT_BASE], SWITCH_DEBUG_COUNTER_LIST) - + reason2 = 'ACL_ANY' self.add_drop_reason(name, reason2) time.sleep(3) - + # Verify that the drop counter has been added to ASIC DB, including the # reason that was added. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1, reason2]) - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason1) self.remove_drop_reason(name, reason2) - + def test_removeReasonFromInitializedCounter(self, dvs, testlog): """ This test verifies that a drop reason can be removed from a counter that has already been initialized without deleting the counter. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' reason2 = 'ACL_ANY' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) self.add_drop_reason(name, reason2) time.sleep(3) - + # Verify that a counter has been created. We will verify the state of # the counter in the next step. assert len(asic_state_table.getKeys()) == 1 self.checkFlexState([SWITCH_STAT_BASE], SWITCH_DEBUG_COUNTER_LIST) - + self.remove_drop_reason(name, reason2) time.sleep(3) - + # Verify that the drop counter has been added to ASIC DB, excluding the # reason that was removed. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason1) - + def test_removeAllDropReasons(self, dvs, testlog): """ This test verifies that it is not possible to remove all drop reasons from a drop counter. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) time.sleep(3) - + # Verify that a counter has been created. We will verify the state of # the counter in the next step. assert len(asic_state_table.getKeys()) == 1 self.checkFlexState([SWITCH_STAT_BASE], SWITCH_DEBUG_COUNTER_LIST) - + self.remove_drop_reason(name, reason1) time.sleep(3) - + # Verify that the drop counter has been added to ASIC DB, including the # last reason that we attempted to remove. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + # Cleanup for the next test. self.delete_drop_counter(name) - + def test_addDropReasonMultipleTimes(self, dvs, testlog): """ This test verifies that the same drop reason can be added multiple times without affecting the system. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) time.sleep(3) - + # Verify that a counter has been created. We will verify the state of # the counter in the next step. assert len(asic_state_table.getKeys()) == 1 self.checkFlexState([SWITCH_STAT_BASE], SWITCH_DEBUG_COUNTER_LIST) - + reason2 = 'ACL_ANY' self.add_drop_reason(name, reason2) time.sleep(3) - + # Verify that the drop counter has been added to ASIC DB, including the # reason that was added. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1, reason2]) - + self.add_drop_reason(name, reason2) time.sleep(3) - + # Verify that the ASIC state is the same as before adding the redundant # drop reason. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1, reason2]) - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason1) self.remove_drop_reason(name, reason2) - + def test_addInvalidDropReason(self, dvs, testlog): """ This test verifies that adding a drop reason to a counter that is not recognized will not affect the system. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) time.sleep(3) - + # Verify that a counter has been created. We will verify the state of # the counter in the next step. assert len(asic_state_table.getKeys()) == 1 self.checkFlexState([SWITCH_STAT_BASE], SWITCH_DEBUG_COUNTER_LIST) - + reason2 = 'ACL_ANY' self.add_drop_reason(name, reason2) time.sleep(3) - + # Verify that the drop counter has been added to ASIC DB, including the # reason that was added. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1, reason2]) - + dummy_reason = 'ZOBOOMBAFOO' self.add_drop_reason(name, dummy_reason) time.sleep(3) - + # Verify that the ASIC state is the same as before adding the invalid # drop reason. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1, reason2]) - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason1) self.remove_drop_reason(name, reason2) - + def test_removeDropReasonMultipleTimes(self, dvs, testlog): """ This test verifies that removing a drop reason multiple times will not affect the system. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' reason2 = 'ACL_ANY' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) self.add_drop_reason(name, reason2) time.sleep(3) - + # Verify that a counter has been created. We will verify the state of # the counter in the next step. assert len(asic_state_table.getKeys()) == 1 self.checkFlexState([SWITCH_STAT_BASE], SWITCH_DEBUG_COUNTER_LIST) - + self.remove_drop_reason(name, reason2) time.sleep(3) - + # Verify that the drop counter has been added to ASIC DB, excluding the # reason that was removed. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + self.remove_drop_reason(name, reason2) time.sleep(3) - + # Verify that the ASIC state is the same as before the redundant # remove operation. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason1) - + def test_removeNonexistentDropReason(self, dvs, testlog): """ This test verifies that removing a drop reason that does not exist on the device will not affect the system. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' reason2 = 'ACL_ANY' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) time.sleep(3) - + # Verify the counter has been created and is in the correct state. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + self.remove_drop_reason(name, reason2) time.sleep(3) - + # Verify that the ASIC state is unchanged after the nonexistent remove. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason1) - + def test_removeInvalidDropReason(self, dvs, testlog): """ This test verifies that removing a drop reason that is not recognized will not affect the system. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name = 'ADD_TEST' reason1 = 'L3_ANY' bogus_reason = 'LIVE_LAUGH_LOVE' - + self.create_drop_counter(name, SWITCH_INGRESS_DROPS) self.add_drop_reason(name, reason1) time.sleep(3) - + # Verify the counter has been created and is in the correct state. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + self.remove_drop_reason(name, bogus_reason) time.sleep(3) - + # Verify that the ASIC state is unchanged after the bad remove. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_SWITCH_IN_TYPE, [reason1]) - + # Cleanup for the next test. self.delete_drop_counter(name) self.remove_drop_reason(name, reason1) @@ -740,47 +739,47 @@ def test_createAndDeleteMultipleCounters(self, dvs, testlog): at the same time works correctly. """ self.setup_db(dvs) - + asic_state_table = swsscommon.Table(self.asic_db, ASIC_STATE_TABLE) flex_counter_table = swsscommon.Table(self.flex_db, FLEX_COUNTER_TABLE) - + name1 = 'DEBUG_0' reason1 = 'L3_ANY' - + name2 = 'DEBUG_1' reason2 = 'ACL_ANY' - + self.create_drop_counter(name1, PORT_INGRESS_DROPS) self.add_drop_reason(name1, reason1) - + self.create_drop_counter(name2, PORT_INGRESS_DROPS) self.add_drop_reason(name2, reason2) - + time.sleep(5) - + # Verify that the flex counters are correctly tracking two different # drop counters. self.checkFlexState([PORT_STAT_BASE, PORT_STAT_INDEX_1], PORT_DEBUG_COUNTER_LIST) - + # Verify that there are two entries in the ASIC DB, one for each counter. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 2 for key in asic_keys: assert (self.checkASICState(key, ASIC_COUNTER_PORT_IN_TYPE, [reason1]) or self.checkASICState(key, ASIC_COUNTER_PORT_IN_TYPE, [reason2])) - + self.delete_drop_counter(name2) self.remove_drop_reason(name2, reason2) time.sleep(3) - + # Verify that the flex counters are tracking ONE drop counter after # the update. self.checkFlexState([PORT_STAT_BASE], PORT_DEBUG_COUNTER_LIST) - + # Verify that there is ONE entry in the ASIC DB after the update. asic_keys = asic_state_table.getKeys() assert len(asic_keys) == 1 assert self.checkASICState(asic_keys[0], ASIC_COUNTER_PORT_IN_TYPE, [reason1]) - + # Cleanup for the next test. self.delete_drop_counter(name1) self.remove_drop_reason(name1, reason1) diff --git a/tests/test_flex_counters.py b/tests/test_flex_counters.py index 451d4ab07c..c4582c0817 100644 --- a/tests/test_flex_counters.py +++ b/tests/test_flex_counters.py @@ -3,6 +3,7 @@ from swsscommon import swsscommon +TUNNEL_TYPE_MAP = "COUNTERS_TUNNEL_TYPE_MAP" NUMBER_OF_RETRIES = 10 CPU_PORT_OID = "0x0" PORT = "Ethernet0" @@ -127,6 +128,15 @@ def verify_no_flex_counters_tables(self, counter_stat): counters_stat_keys = self.flex_db.get_keys("FLEX_COUNTER_TABLE:" + counter_stat) assert len(counters_stat_keys) == 0, "FLEX_COUNTER_TABLE:" + str(counter_stat) + " tables exist before enabling the flex counter group" + def verify_no_flex_counters_tables_after_delete(self, counter_stat): + for retry in range(NUMBER_OF_RETRIES): + counters_stat_keys = self.flex_db.get_keys("FLEX_COUNTER_TABLE:" + counter_stat + ":") + if len(counters_stat_keys) == 0: + return + else: + time.sleep(1) + assert False, "FLEX_COUNTER_TABLE:" + str(counter_stat) + " tables exist after removing the entries" + def verify_flex_counters_populated(self, map, stat): counters_keys = self.counters_db.db_connection.hgetall(map) for counter_entry in counters_keys.items(): @@ -364,6 +374,13 @@ def test_remove_trap_group(self, dvs): self.set_flex_counter_group_status(meta_data['key'], meta_data['group_name'], 'disable') + if counter_type == "vxlan_tunnel_counter": + self.verify_tunnel_type_vxlan(counter_map, TUNNEL_TYPE_MAP) + self.config_db.delete_entry("VLAN","Vlan10") + self.config_db.delete_entry("VLAN_TUNNEL","vtep1") + self.config_db.delete_entry("VLAN_TUNNEL_MAP","vtep1|map_100_Vlan10") + self.verify_no_flex_counters_tables_after_delete(counter_stat) + def test_add_remove_ports(self, dvs): self.setup_dbs(dvs) @@ -382,8 +399,10 @@ def test_add_remove_ports(self, dvs): counters_queue_map = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP", "") for key, oid in counters_queue_map.items(): if PORT in key: + oid_list.append(oid) fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s" % oid) assert len(fields) == 1 + oid_list_len = len(oid_list) # get port oid port_oid = self.counters_db.get_entry(PORT_MAP, "")[PORT] @@ -398,7 +417,6 @@ def test_add_remove_ports(self, dvs): assert len(fields) == 0 # verify that port counter maps were removed from counters db - oid_list = [] counters_queue_map = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP", "") for key in counters_queue_map.keys(): if PORT in key: @@ -418,5 +436,8 @@ def test_add_remove_ports(self, dvs): for key, oid in counters_queue_map.items(): if PORT in key: + oid_list.append(oid) fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s" % oid) assert len(fields) == 1 + # the number of the oids needs to be the same as the original number of oids (before removing a port and adding) + assert oid_list_len == len(oid_list) From b3c6197a3d2c975aa1b0eaefed6dbb105878e363 Mon Sep 17 00:00:00 2001 From: tomeri Date: Tue, 30 Nov 2021 11:42:13 +0200 Subject: [PATCH 05/15] countrs support for dynamic port add/remove - code review fixes --- orchagent/debugcounterorch.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/orchagent/debugcounterorch.cpp b/orchagent/debugcounterorch.cpp index 7ec467ae31..c15f384349 100644 --- a/orchagent/debugcounterorch.cpp +++ b/orchagent/debugcounterorch.cpp @@ -645,15 +645,17 @@ void DebugCounterOrch::addPortDebugCounter(sai_object_id_t port_id) { SWSS_LOG_ENTER(); - SWSS_LOG_NOTICE("add debug counter for port 0x%" PRIu64 , port_id); + SWSS_LOG_INFO("add debug counter for port 0x%" PRIu64 , port_id); - for (const auto& debug_counter: debug_counters) { + for (const auto& debug_counter: debug_counters) + { DebugCounter *counter = debug_counter.second.get(); auto counter_type = counter->getCounterType(); auto counter_stat = counter->getDebugCounterSAIStat(); auto flex_counter_type = getFlexCounterType(counter_type); - if (flex_counter_type == CounterType::PORT_DEBUG){ + if (flex_counter_type == CounterType::PORT_DEBUG) + { flex_counter_manager.addFlexCounterStat( port_id, flex_counter_type, @@ -666,16 +668,18 @@ void DebugCounterOrch::removePortDebugCounter(sai_object_id_t port_id) { SWSS_LOG_ENTER(); - SWSS_LOG_NOTICE("remove debug counter for port 0x%" PRIu64 , port_id); + SWSS_LOG_INFO("remove debug counter for port 0x%" PRIu64 , port_id); - for (const auto& debug_counter: debug_counters) { + for (const auto& debug_counter: debug_counters) + { DebugCounter *counter = debug_counter.second.get(); auto counter_type = counter->getCounterType(); auto counter_stat = counter->getDebugCounterSAIStat(); auto flex_counter_type = getFlexCounterType(counter_type); - if (flex_counter_type == CounterType::PORT_DEBUG){ + if (flex_counter_type == CounterType::PORT_DEBUG) + { flex_counter_manager.removeFlexCounterStat( port_id, flex_counter_type, From fd79762494acdaca842df835cb8cc4a23ad5d3f4 Mon Sep 17 00:00:00 2001 From: tomer-israel <76040066+tomer-israel@users.noreply.github.com> Date: Sun, 5 Dec 2021 21:33:56 +0200 Subject: [PATCH 06/15] Update test_flex_counters.py --- tests/test_flex_counters.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_flex_counters.py b/tests/test_flex_counters.py index c4582c0817..c6e2884404 100644 --- a/tests/test_flex_counters.py +++ b/tests/test_flex_counters.py @@ -7,6 +7,7 @@ NUMBER_OF_RETRIES = 10 CPU_PORT_OID = "0x0" PORT = "Ethernet0" +PORT_MAP = "COUNTERS_PORT_NAME_MAP" counter_group_meta = { 'port_counter': { From 2c23a2df6b4df6e5ea260a2fe9f976094610f25d Mon Sep 17 00:00:00 2001 From: tomer-israel <76040066+tomer-israel@users.noreply.github.com> Date: Sun, 5 Dec 2021 21:33:56 +0200 Subject: [PATCH 07/15] Update test_flex_counters.py --- orchagent/debugcounterorch.cpp | 94 +++++++++++++++------------------- orchagent/debugcounterorch.h | 6 ++- 2 files changed, 46 insertions(+), 54 deletions(-) diff --git a/orchagent/debugcounterorch.cpp b/orchagent/debugcounterorch.cpp index c15f384349..9fd4632872 100644 --- a/orchagent/debugcounterorch.cpp +++ b/orchagent/debugcounterorch.cpp @@ -58,9 +58,29 @@ void DebugCounterOrch::update(SubjectType type, void *cntx) Port &port = update->port; if (update->add) { - addPortDebugCounter(port.m_port_id); + for (const auto& debug_counter: debug_counters) + { + DebugCounter *counter = debug_counter.second.get(); + auto counter_type = counter->getCounterType(); + auto counter_stat = counter->getDebugCounterSAIStat(); + auto flex_counter_type = getFlexCounterType(counter_type); + if (flex_counter_type == CounterType::PORT_DEBUG) + { + installDebugFlexCounters(counter_type, counter_stat, port.m_port_id); + } + } } else { - removePortDebugCounter(port.m_port_id); + for (const auto& debug_counter: debug_counters) + { + DebugCounter *counter = debug_counter.second.get(); + auto counter_type = counter->getCounterType(); + auto counter_stat = counter->getDebugCounterSAIStat(); + auto flex_counter_type = getFlexCounterType(counter_type); + if (flex_counter_type == CounterType::PORT_DEBUG) + { + uninstallDebugFlexCounters(counter_type, counter_stat, port.m_port_id); + } + } } } } @@ -500,7 +520,8 @@ CounterType DebugCounterOrch::getFlexCounterType(const string& counter_type) } void DebugCounterOrch::installDebugFlexCounters(const string& counter_type, - const string& counter_stat) + const string& counter_stat, + sai_object_id_t port_id) { SWSS_LOG_ENTER(); CounterType flex_counter_type = getFlexCounterType(counter_type); @@ -513,6 +534,14 @@ void DebugCounterOrch::installDebugFlexCounters(const string& counter_type, { for (auto const &curr : gPortsOrch->getAllPorts()) { + if (port_id != SAI_NULL_OBJECT_ID) + { + if (curr.second.m_port_id != port_id) + { + continue; + } + } + if (curr.second.m_type != Port::Type::PHY) { continue; @@ -527,7 +556,8 @@ void DebugCounterOrch::installDebugFlexCounters(const string& counter_type, } void DebugCounterOrch::uninstallDebugFlexCounters(const string& counter_type, - const string& counter_stat) + const string& counter_stat, + sai_object_id_t port_id) { SWSS_LOG_ENTER(); CounterType flex_counter_type = getFlexCounterType(counter_type); @@ -540,6 +570,14 @@ void DebugCounterOrch::uninstallDebugFlexCounters(const string& counter_type, { for (auto const &curr : gPortsOrch->getAllPorts()) { + if (port_id != SAI_NULL_OBJECT_ID) + { + if (curr.second.m_port_id != port_id) + { + continue; + } + } + if (curr.second.m_type != Port::Type::PHY) { continue; @@ -641,53 +679,5 @@ bool DebugCounterOrch::isDropReasonValid(const string& drop_reason) const return true; } -void DebugCounterOrch::addPortDebugCounter(sai_object_id_t port_id) -{ - SWSS_LOG_ENTER(); - - SWSS_LOG_INFO("add debug counter for port 0x%" PRIu64 , port_id); - - for (const auto& debug_counter: debug_counters) - { - DebugCounter *counter = debug_counter.second.get(); - auto counter_type = counter->getCounterType(); - auto counter_stat = counter->getDebugCounterSAIStat(); - auto flex_counter_type = getFlexCounterType(counter_type); - - if (flex_counter_type == CounterType::PORT_DEBUG) - { - flex_counter_manager.addFlexCounterStat( - port_id, - flex_counter_type, - counter_stat); - } - } -} - -void DebugCounterOrch::removePortDebugCounter(sai_object_id_t port_id) -{ - SWSS_LOG_ENTER(); - - SWSS_LOG_INFO("remove debug counter for port 0x%" PRIu64 , port_id); - - for (const auto& debug_counter: debug_counters) - { - DebugCounter *counter = debug_counter.second.get(); - - auto counter_type = counter->getCounterType(); - auto counter_stat = counter->getDebugCounterSAIStat(); - auto flex_counter_type = getFlexCounterType(counter_type); - - if (flex_counter_type == CounterType::PORT_DEBUG) - { - flex_counter_manager.removeFlexCounterStat( - port_id, - flex_counter_type, - counter_stat); - } - } -} - - diff --git a/orchagent/debugcounterorch.h b/orchagent/debugcounterorch.h index 15f4860143..d9c8a992ec 100644 --- a/orchagent/debugcounterorch.h +++ b/orchagent/debugcounterorch.h @@ -52,10 +52,12 @@ class DebugCounterOrch: public Orch, public Observer CounterType getFlexCounterType(const std::string& counter_type) noexcept(false); void installDebugFlexCounters( const std::string& counter_type, - const std::string& counter_stat); + const std::string& counter_stat, + sai_object_id_t port_id = SAI_NULL_OBJECT_ID); void uninstallDebugFlexCounters( const std::string& counter_type, - const std::string& counter_stat); + const std::string& counter_stat, + sai_object_id_t port_id = SAI_NULL_OBJECT_ID); void addPortDebugCounter(sai_object_id_t port_id); void removePortDebugCounter(sai_object_id_t port_id); From 2728ba0a1ac08334849b815ff9526be38dd66062 Mon Sep 17 00:00:00 2001 From: tomer-israel <76040066+tomer-israel@users.noreply.github.com> Date: Tue, 21 Dec 2021 11:56:40 +0200 Subject: [PATCH 08/15] Update debugcounterorch.cpp changes according to coding style --- orchagent/debugcounterorch.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/orchagent/debugcounterorch.cpp b/orchagent/debugcounterorch.cpp index 9fd4632872..ed27f400d4 100644 --- a/orchagent/debugcounterorch.cpp +++ b/orchagent/debugcounterorch.cpp @@ -48,8 +48,10 @@ void DebugCounterOrch::update(SubjectType type, void *cntx) { SWSS_LOG_ENTER(); - if (type == SUBJECT_TYPE_PORT_CHANGE) { - if (!cntx) { + if (type == SUBJECT_TYPE_PORT_CHANGE) + { + if (!cntx) + { SWSS_LOG_ERROR("cntx is NULL"); return; } @@ -57,7 +59,8 @@ void DebugCounterOrch::update(SubjectType type, void *cntx) PortUpdate *update = static_cast(cntx); Port &port = update->port; - if (update->add) { + if (update->add) + { for (const auto& debug_counter: debug_counters) { DebugCounter *counter = debug_counter.second.get(); @@ -69,7 +72,9 @@ void DebugCounterOrch::update(SubjectType type, void *cntx) installDebugFlexCounters(counter_type, counter_stat, port.m_port_id); } } - } else { + } + else + { for (const auto& debug_counter: debug_counters) { DebugCounter *counter = debug_counter.second.get(); From cf77ec14b95baa5e735bee0beeb4df301e21c712 Mon Sep 17 00:00:00 2001 From: tomer-israel <76040066+tomer-israel@users.noreply.github.com> Date: Tue, 21 Dec 2021 12:02:55 +0200 Subject: [PATCH 09/15] Update debugcounterorch.h remove unused function declerations --- orchagent/debugcounterorch.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/orchagent/debugcounterorch.h b/orchagent/debugcounterorch.h index d9c8a992ec..edfb5d98e0 100644 --- a/orchagent/debugcounterorch.h +++ b/orchagent/debugcounterorch.h @@ -58,8 +58,6 @@ class DebugCounterOrch: public Orch, public Observer const std::string& counter_type, const std::string& counter_stat, sai_object_id_t port_id = SAI_NULL_OBJECT_ID); - void addPortDebugCounter(sai_object_id_t port_id); - void removePortDebugCounter(sai_object_id_t port_id); // Debug Counter Initialization Helper Functions std::string getDebugCounterType( From 7087ed45750df0b64d321d05798f9ee727fc8928 Mon Sep 17 00:00:00 2001 From: dprital Date: Mon, 7 Feb 2022 14:00:29 +0000 Subject: [PATCH 10/15] Fix PR review comment --- orchagent/portsorch.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index d8e7bbe5e1..9eb9521f7e 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -5448,10 +5448,10 @@ void PortsOrch::removeQueueMapPerPort(const Port& port) } } - m_counter_db->hdel(COUNTERS_QUEUE_NAME_MAP, queueVector); - m_counter_db->hdel(COUNTERS_QUEUE_PORT_MAP, queuePortVector); - m_counter_db->hdel(COUNTERS_QUEUE_INDEX_MAP, queueIndexVector); - m_counter_db->hdel(COUNTERS_QUEUE_TYPE_MAP, queueTypeVector); + m_queueTable->hdel("", queueVector); + m_queuePortTable->hdel("", queuePortVector); + m_queueIndexTable->hdel("", queueIndexVector); + m_queueTypeTable->hdel("", queueTypeVector); for (size_t queueIndex = 0; queueIndex < port.m_queue_ids.size(); ++queueIndex) { @@ -5570,9 +5570,9 @@ void PortsOrch::removePriorityGroupMapPerPort(const Port& port) pgIndexVector.emplace_back(id); } - m_counter_db->hdel(COUNTERS_PG_NAME_MAP, pgVector); - m_counter_db->hdel(COUNTERS_PG_PORT_MAP, pgPortVector); - m_counter_db->hdel(COUNTERS_PG_INDEX_MAP, pgIndexVector); + m_pgTable->hdel("", pgVector); + m_pgPortTable->hdel("", pgPortVector); + m_pgIndexTable->hdel("", pgIndexVector); for (size_t pgIndex = 0; pgIndex < port.m_priority_group_ids.size(); ++pgIndex) { From b16ee2ae7ef38fdf145fd19db9f07e3bb11989d4 Mon Sep 17 00:00:00 2001 From: dprital Date: Mon, 7 Feb 2022 17:06:46 +0000 Subject: [PATCH 11/15] replace code --- orchagent/portsorch.cpp | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 9eb9521f7e..2ad3f9991d 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -2451,7 +2451,7 @@ void PortsOrch::deInitPort(string alias, sai_object_id_t port_id) } /* remove port name map from counter table */ - m_counterTable->hdel("", alias); + m_counterTable->del(alias); /* Remove the associated port serdes attribute */ removePortSerdesAttribute(p.m_port_id); @@ -5424,10 +5424,6 @@ void PortsOrch::generateQueueMap() void PortsOrch::removeQueueMapPerPort(const Port& port) { /* Remove the Queue map in the Counter DB */ - vector queueVector; - vector queuePortVector; - vector queueIndexVector; - vector queueTypeVector; for (size_t queueIndex = 0; queueIndex < port.m_queue_ids.size(); ++queueIndex) { @@ -5436,26 +5432,16 @@ void PortsOrch::removeQueueMapPerPort(const Port& port) const auto id = sai_serialize_object_id(port.m_queue_ids[queueIndex]); - queueVector.emplace_back(name.str()); - queuePortVector.emplace_back(id); + m_queueTable->del(name.str()); + m_queuePortTable->del(id); string queueType; uint8_t queueRealIndex = 0; if (getQueueTypeAndIndex(port.m_queue_ids[queueIndex], queueType, queueRealIndex)) { - queueTypeVector.emplace_back(id); - queueIndexVector.emplace_back(id); + m_queueTypeTable->del(id); + m_queueIndexTable->del(id); } - } - - m_queueTable->hdel("", queueVector); - m_queuePortTable->hdel("", queuePortVector); - m_queueIndexTable->hdel("", queueIndexVector); - m_queueTypeTable->hdel("", queueTypeVector); - - for (size_t queueIndex = 0; queueIndex < port.m_queue_ids.size(); ++queueIndex) - { - const auto id = sai_serialize_object_id(port.m_queue_ids[queueIndex]); std::unordered_set counter_stats; for (const auto& it: queue_stat_ids) From a7814717aba11ff21580c7b78bce76657bc65f1f Mon Sep 17 00:00:00 2001 From: dprital Date: Mon, 7 Feb 2022 18:04:23 +0000 Subject: [PATCH 12/15] fix pg counter code --- orchagent/portsorch.cpp | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 2ad3f9991d..d0eebffc28 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -5549,22 +5549,13 @@ void PortsOrch::removePriorityGroupMapPerPort(const Port& port) std::ostringstream name; name << port.m_alias << ":" << pgIndex; - const auto id = sai_serialize_object_id(port.m_priority_group_ids[pgIndex]); - - pgVector.emplace_back(name.str()); - pgPortVector.emplace_back(id); - pgIndexVector.emplace_back(id); - } - - m_pgTable->hdel("", pgVector); - m_pgPortTable->hdel("", pgPortVector); - m_pgIndexTable->hdel("", pgIndexVector); - - for (size_t pgIndex = 0; pgIndex < port.m_priority_group_ids.size(); ++pgIndex) - { const auto id = sai_serialize_object_id(port.m_priority_group_ids[pgIndex]); string key = getPriorityGroupWatermarkFlexCounterTableKey(id); + m_pgTable->del(name.str()); + m_pgPortTable->del(id); + m_pgIndexTable->del(id); + m_flexCounterTable->del(key); key = getPriorityGroupDropPacketsFlexCounterTableKey(id); From 8d0c34559856cd7392e1be86c185040f07cbb775 Mon Sep 17 00:00:00 2001 From: dprital Date: Tue, 22 Mar 2022 02:33:01 +0000 Subject: [PATCH 13/15] Fix the way we delete the counters upon port delete --- orchagent/portsorch.cpp | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index d0eebffc28..846984ce1f 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -2451,7 +2451,7 @@ void PortsOrch::deInitPort(string alias, sai_object_id_t port_id) } /* remove port name map from counter table */ - m_counterTable->del(alias); + m_counterTable->hdel("", alias); /* Remove the associated port serdes attribute */ removePortSerdesAttribute(p.m_port_id); @@ -5429,21 +5429,21 @@ void PortsOrch::removeQueueMapPerPort(const Port& port) { std::ostringstream name; name << port.m_alias << ":" << queueIndex; + std::unordered_set counter_stats; const auto id = sai_serialize_object_id(port.m_queue_ids[queueIndex]); - m_queueTable->del(name.str()); - m_queuePortTable->del(id); + m_queueTable->hdel("",name.str()); + m_queuePortTable->hdel("",id); string queueType; uint8_t queueRealIndex = 0; if (getQueueTypeAndIndex(port.m_queue_ids[queueIndex], queueType, queueRealIndex)) { - m_queueTypeTable->del(id); - m_queueIndexTable->del(id); + m_queueTypeTable->hdel("",id); + m_queueIndexTable->hdel("",id); } - std::unordered_set counter_stats; for (const auto& it: queue_stat_ids) { counter_stats.emplace(sai_serialize_queue_stat(it)); @@ -5540,9 +5540,6 @@ void PortsOrch::generatePriorityGroupMap() void PortsOrch::removePriorityGroupMapPerPort(const Port& port) { /* Remove the PG map in the Counter DB */ - vector pgVector; - vector pgPortVector; - vector pgIndexVector; for (size_t pgIndex = 0; pgIndex < port.m_priority_group_ids.size(); ++pgIndex) { @@ -5552,9 +5549,9 @@ void PortsOrch::removePriorityGroupMapPerPort(const Port& port) const auto id = sai_serialize_object_id(port.m_priority_group_ids[pgIndex]); string key = getPriorityGroupWatermarkFlexCounterTableKey(id); - m_pgTable->del(name.str()); - m_pgPortTable->del(id); - m_pgIndexTable->del(id); + m_pgTable->hdel("",name.str()); + m_pgPortTable->hdel("",id); + m_pgIndexTable->hdel("",id); m_flexCounterTable->del(key); From e92133c03df9228fa473b669f6bba07d13132603 Mon Sep 17 00:00:00 2001 From: Dror Prital Date: Sun, 27 Mar 2022 17:04:47 +0000 Subject: [PATCH 14/15] Fix merge issues --- tests/test_flex_counters.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/test_flex_counters.py b/tests/test_flex_counters.py index c6e2884404..bd95aa433d 100644 --- a/tests/test_flex_counters.py +++ b/tests/test_flex_counters.py @@ -374,13 +374,6 @@ def test_remove_trap_group(self, dvs): assert trap_id not in counters_keys self.set_flex_counter_group_status(meta_data['key'], meta_data['group_name'], 'disable') - - if counter_type == "vxlan_tunnel_counter": - self.verify_tunnel_type_vxlan(counter_map, TUNNEL_TYPE_MAP) - self.config_db.delete_entry("VLAN","Vlan10") - self.config_db.delete_entry("VLAN_TUNNEL","vtep1") - self.config_db.delete_entry("VLAN_TUNNEL_MAP","vtep1|map_100_Vlan10") - self.verify_no_flex_counters_tables_after_delete(counter_stat) def test_add_remove_ports(self, dvs): self.setup_dbs(dvs) From ca47331b89c3a039051f5775142711b6ffa85699 Mon Sep 17 00:00:00 2001 From: Dror Prital Date: Mon, 28 Mar 2022 20:14:00 +0000 Subject: [PATCH 15/15] Align coding style --- orchagent/portsorch.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 846984ce1f..62449c622c 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -2379,12 +2379,14 @@ bool PortsOrch::initPort(const string &alias, const string &role, const int inde } /* when a port is added and priority group map counter is enabled --> we need to add pg counter for it */ - if (m_isPriorityGroupMapGenerated) { + if (m_isPriorityGroupMapGenerated) + { generatePriorityGroupMapPerPort(p); } /* when a port is added and queue map counter is enabled --> we need to add queue map counter for it */ - if (m_isQueueMapGenerated) { + if (m_isQueueMapGenerated) + { generateQueueMapPerPort(p); } @@ -2441,12 +2443,14 @@ void PortsOrch::deInitPort(string alias, sai_object_id_t port_id) } /* remove pg port counters */ - if (m_isPriorityGroupMapGenerated) { + if (m_isPriorityGroupMapGenerated) + { removePriorityGroupMapPerPort(p); } /* remove queue port counters */ - if (m_isQueueMapGenerated) { + if (m_isQueueMapGenerated) + { removeQueueMapPerPort(p); }