From ae96260bc4d7bb31e6385743f80e96f50fea9d8d Mon Sep 17 00:00:00 2001 From: Doron Barashi Date: Mon, 24 Oct 2022 17:15:44 +0000 Subject: [PATCH] Align watermark flow with port configuration --- orchagent/flexcounterorch.cpp | 14 ++ orchagent/portsorch.cpp | 234 ++++++++++++++++++++++++++-------- orchagent/portsorch.h | 14 ++ tests/test_flex_counters.py | 113 +++++++++------- 4 files changed, 278 insertions(+), 97 deletions(-) diff --git a/orchagent/flexcounterorch.cpp b/orchagent/flexcounterorch.cpp index ffaac6daaf5..5a25827d022 100644 --- a/orchagent/flexcounterorch.cpp +++ b/orchagent/flexcounterorch.cpp @@ -28,7 +28,9 @@ extern FlowCounterRouteOrch *gFlowCounterRouteOrch; #define PORT_KEY "PORT" #define PORT_BUFFER_DROP_KEY "PORT_BUFFER_DROP" #define QUEUE_KEY "QUEUE" +#define QUEUE_WATERMARK "QUEUE_WATERMARK" #define PG_WATERMARK_KEY "PG_WATERMARK" +#define PG_DROP_KEY "PG_DROP" #define RIF_KEY "RIF" #define ACL_KEY "ACL" #define TUNNEL_KEY "TUNNEL" @@ -158,10 +160,22 @@ void FlexCounterOrch::doTask(Consumer &consumer) else if(key == QUEUE_KEY) { gPortsOrch->generateQueueMap(); + gPortsOrch->addQueueFlexCounters(); + } + else if(key == QUEUE_WATERMARK) + { + gPortsOrch->generateQueueMap(); + gPortsOrch->addQueueWatermarkFlexCounters(); + } + else if(key == PG_DROP_KEY) + { + gPortsOrch->generatePriorityGroupMap(); + gPortsOrch->addPriorityGroupFlexCounters(); } else if(key == PG_WATERMARK_KEY) { gPortsOrch->generatePriorityGroupMap(); + gPortsOrch->addPriorityGroupWatermarkFlexCounters(); } } if(gIntfsOrch && (key == RIF_KEY) && (value == "enable")) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 79c6d0daa7c..891a0cad723 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1069,9 +1069,9 @@ void PortsOrch::getCpuPort(Port &port) port = m_cpuPort; } -/* - * Create host_tx_ready field in PORT_TABLE of STATE-DB - * and set the field to false by default for the +/* + * Create host_tx_ready field in PORT_TABLE of STATE-DB + * and set the field to false by default for the * front port. */ void PortsOrch::initHostTxReadyState(Port &port) @@ -1096,7 +1096,7 @@ void PortsOrch::initHostTxReadyState(Port &port) if (hostTxReady.empty()) { m_portStateTable.hset(port.m_alias, "host_tx_ready", "false"); - SWSS_LOG_INFO("initalize hostTxReady %s with status %s", + SWSS_LOG_INFO("initalize hostTxReady %s with status %s", port.m_alias.c_str(), hostTxReady.c_str()); } } @@ -1110,13 +1110,13 @@ bool PortsOrch::setPortAdminStatus(Port &port, bool state) attr.value.booldata = state; /* Update the host_tx_ready to false before setting admin_state, when admin state is false */ - if (!state) + if (!state) { m_portStateTable.hset(port.m_alias, "host_tx_ready", "false"); SWSS_LOG_INFO("Set admin status DOWN host_tx_ready to false to port pid:%" PRIx64, port.m_port_id); } - + sai_status_t status = sai_port_api->set_port_attribute(port.m_port_id, &attr); if (status != SAI_STATUS_SUCCESS) { @@ -1135,14 +1135,14 @@ bool PortsOrch::setPortAdminStatus(Port &port, bool state) { m_portStateTable.hset(port.m_alias, "host_tx_ready", "false"); } - + /* Update the state table for host_tx_ready*/ if (state && (gbstatus == true) && (status == SAI_STATUS_SUCCESS) ) { m_portStateTable.hset(port.m_alias, "host_tx_ready", "true"); SWSS_LOG_INFO("Set admin status UP host_tx_ready to true to port pid:%" PRIx64, port.m_port_id); - } + } return true; } @@ -1376,9 +1376,9 @@ bool PortsOrch::setPortPfcWatchdogStatus(sai_object_id_t portId, uint8_t pfcwd_b SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64, portId); return false; } - + p.m_pfcwd_sw_bitmask = pfcwd_bitmask; - + m_portList[p.m_alias] = p; SWSS_LOG_INFO("Set PFC watchdog port id=0x%" PRIx64 ", bitmast=0x%x", portId, pfcwd_bitmask); @@ -1396,9 +1396,9 @@ bool PortsOrch::getPortPfcWatchdogStatus(sai_object_id_t portId, uint8_t *pfcwd_ SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64, portId); return false; } - + *pfcwd_bitmask = p.m_pfcwd_sw_bitmask; - + return true; } @@ -2798,12 +2798,28 @@ bool PortsOrch::initPort(const string &alias, const string &role, const int inde if (m_isPriorityGroupMapGenerated) { generatePriorityGroupMapPerPort(p); + if (m_isPriorityGroupFlexCountersAdded) + { + addPriorityGroupFlexCountersPerPort(p); + } + if (m_isPriorityGroupWatermarkFlexCountersAdded) + { + addPriorityGroupWatermarkFlexCountersPerPort(p); + } } /* 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, false); + if (m_isQueueFlexCountersAdded) + { + addQueueFlexCountersPerPort(p); + } + if (m_isQueueWatermarkFlexCountersAdded) + { + addQueueWatermarkFlexCountersPerPort(p); + } } PortUpdate update = { p, true }; @@ -3778,10 +3794,10 @@ void PortsOrch::doPortTask(Consumer &consumer) continue; } } - + /* create host_tx_ready field in state-db */ initHostTxReadyState(p); - + /* Last step set port admin status */ if (!admin_status.empty() && (p.m_admin_state_up != (admin_status == "up"))) { @@ -6135,18 +6151,90 @@ void PortsOrch::generateQueueMapPerPort(const Port& port, bool voq) queueTypeVector.emplace_back(id, queueType); queueIndexVector.emplace_back(id, to_string(queueRealIndex)); } + } - // Install a flex counter for this queue to track stats - std::unordered_set counter_stats; - for (const auto& it: queue_stat_ids) + if (voq) + { + m_voqTable->set("", queueVector); + } + else + { + m_queueTable->set("", queueVector); + } + m_queuePortTable->set("", queuePortVector); + m_queueIndexTable->set("", queueIndexVector); + m_queueTypeTable->set("", queueTypeVector); + + CounterCheckOrch::getInstance().addPort(port); +} + +void PortsOrch::addQueueFlexCounters() +{ + if (m_isQueueFlexCountersAdded) + { + return; + } + + for (const auto& it: m_portList) + { + if (it.second.m_type == Port::PHY) { - counter_stats.emplace(sai_serialize_queue_stat(it)); + addQueueFlexCountersPerPort(it.second); } - queue_stat_manager.setCounterIdList(queue_ids[queueIndex], CounterType::QUEUE, counter_stats); + } - if (voq) { - continue; - } + m_isQueueFlexCountersAdded = true; +} + + +void PortsOrch::addQueueFlexCountersPerPort(const Port& port) +{ + std::vector queue_ids; + if (voq) + { + queue_ids = m_port_voq_ids[port.m_alias]; + } + else + { + queue_ids = port.m_queue_ids; + } + for (size_t queueIndex = 0; queueIndex < queue_ids.size(); ++queueIndex) + { + // 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.setCounterIdList(queue_ids[queueIndex], CounterType::QUEUE, counter_stats); + } +} + +void PortsOrch::addQueueWatermarkFlexCounters() +{ + if (m_isQueueWatermarkFlexCountersAdded) + { + return; + } + + for (const auto& it: m_portList) + { + if (it.second.m_type == Port::PHY) + { + addQueueWatermarkFlexCountersPerPort(it.second); + } + } + + m_isQueueWatermarkFlexCountersAdded = true; +} + +void PortsOrch::addQueueWatermarkFlexCountersPerPort(const Port& port) +{ + /* Add stat counters to flex_counter */ + + for (size_t queueIndex = 0; queueIndex < port.m_queue_ids.size(); ++queueIndex) + { + const auto id = sai_serialize_object_id(port.m_queue_ids[queueIndex]); /* add watermark queue counters */ string key = getQueueWatermarkFlexCounterTableKey(id); @@ -6164,20 +6252,6 @@ void PortsOrch::generateQueueMapPerPort(const Port& port, bool voq) m_flexCounterTable->set(key, fieldValues); } - - if (voq) - { - m_voqTable->set("", queueVector); - } - else - { - m_queueTable->set("", queueVector); - } - m_queuePortTable->set("", queuePortVector); - m_queueIndexTable->set("", queueIndexVector); - m_queueTypeTable->set("", queueTypeVector); - - CounterCheckOrch::getInstance().addPort(port); } void PortsOrch::generatePriorityGroupMap() @@ -6242,25 +6316,42 @@ void PortsOrch::generatePriorityGroupMapPerPort(const Port& port) pgVector.emplace_back(name.str(), id); pgPortVector.emplace_back(id, sai_serialize_object_id(port.m_port_id)); pgIndexVector.emplace_back(id, to_string(pgIndex)); + } - string key = getPriorityGroupWatermarkFlexCounterTableKey(id); + m_pgTable->set("", pgVector); + m_pgPortTable->set("", pgPortVector); + m_pgIndexTable->set("", pgIndexVector); - std::string delimiter = ""; - std::ostringstream counters_stream; - /* Add watermark counters to flex_counter */ - for (const auto& it: ingressPriorityGroupWatermarkStatIds) + CounterCheckOrch::getInstance().addPort(port); +} + +void PortsOrch::addPriorityGroupFlexCounters() +{ + if (m_isPriorityGroupFlexCountersAdded) + { + return; + } + + for (const auto& it: m_portList) + { + if (it.second.m_type == Port::PHY) { - counters_stream << delimiter << sai_serialize_ingress_priority_group_stat(it); - delimiter = comma; + addPriorityGroupFlexCountersPerPort(it.second); } + } - vector fieldValues; - fieldValues.emplace_back(PG_COUNTER_ID_LIST, counters_stream.str()); - m_flexCounterTable->set(key, fieldValues); + m_isPriorityGroupFlexCountersAdded = true; +} + +void PortsOrch::addPriorityGroupFlexCountersPerPort(const Port& port) +{ + 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]); - delimiter = ""; + string delimiter = ""; std::ostringstream ingress_pg_drop_packets_counters_stream; - key = getPriorityGroupDropPacketsFlexCounterTableKey(id); + string key = getPriorityGroupDropPacketsFlexCounterTableKey(id); /* Add dropped packets counters to flex_counter */ for (const auto& it: ingressPriorityGroupDropStatIds) { @@ -6270,16 +6361,53 @@ void PortsOrch::generatePriorityGroupMapPerPort(const Port& port) delimiter = comma; } } - fieldValues.clear(); + vector fieldValues; fieldValues.emplace_back(PG_COUNTER_ID_LIST, ingress_pg_drop_packets_counters_stream.str()); m_flexCounterTable->set(key, fieldValues); } +} - m_pgTable->set("", pgVector); - m_pgPortTable->set("", pgPortVector); - m_pgIndexTable->set("", pgIndexVector); +void PortsOrch::addPriorityGroupWatermarkFlexCounters() +{ + if (m_isPriorityGroupWatermarkFlexCountersAdded) + { + return; + } - CounterCheckOrch::getInstance().addPort(port); + for (const auto& it: m_portList) + { + if (it.second.m_type == Port::PHY) + { + addPriorityGroupWatermarkFlexCountersPerPort(it.second); + } + } + + m_isPriorityGroupWatermarkFlexCountersAdded = true; +} + +void PortsOrch::addPriorityGroupWatermarkFlexCountersPerPort(const Port& port) +{ + /* Add stat counters to flex_counter */ + + 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); + + std::string delimiter = ""; + std::ostringstream counters_stream; + /* Add watermark counters to flex_counter */ + for (const auto& it: ingressPriorityGroupWatermarkStatIds) + { + counters_stream << delimiter << sai_serialize_ingress_priority_group_stat(it); + delimiter = comma; + } + + vector fieldValues; + fieldValues.emplace_back(PG_COUNTER_ID_LIST, counters_stream.str()); + m_flexCounterTable->set(key, fieldValues); + } } void PortsOrch::generatePortCounterMap() diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 29769feaa98..c957128d461 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -133,7 +133,11 @@ class PortsOrch : public Orch, public Subject bool getPortPfcWatchdogStatus(sai_object_id_t portId, uint8_t *pfc_bitmask); void generateQueueMap(); + void addQueueFlexCounters(); + void addQueueWatermarkFlexCounters(); void generatePriorityGroupMap(); + void addPriorityGroupFlexCounters(); + void addPriorityGroupWatermarkFlexCounters(); void generatePortCounterMap(); void generatePortBufferDropCounterMap(); @@ -351,10 +355,20 @@ class PortsOrch : public Orch, public Subject bool m_isQueueMapGenerated = false; void generateQueueMapPerPort(const Port& port, bool voq); void removeQueueMapPerPort(const Port& port); + bool m_isQueueFlexCountersAdded = false; + void addQueueFlexCountersPerPort(const Port& port); + + bool m_isQueueWatermarkFlexCountersAdded = false; + void addQueueWatermarkFlexCountersPerPort(const Port& port); bool m_isPriorityGroupMapGenerated = false; void generatePriorityGroupMapPerPort(const Port& port); void removePriorityGroupMapPerPort(const Port& port); + bool m_isPriorityGroupFlexCountersAdded = false; + void addPriorityGroupFlexCountersPerPort(const Port& port); + + bool m_isPriorityGroupWatermarkFlexCountersAdded = false; + void addPriorityGroupWatermarkFlexCountersPerPort(const Port& port); bool m_isPortCounterMapGenerated = false; bool m_isPortBufferDropCounterMapGenerated = false; diff --git a/tests/test_flex_counters.py b/tests/test_flex_counters.py index 76a1a535f9a..267d7b53265 100644 --- a/tests/test_flex_counters.py +++ b/tests/test_flex_counters.py @@ -22,6 +22,11 @@ 'group_name': 'QUEUE_STAT_COUNTER', 'name_map': 'COUNTERS_QUEUE_NAME_MAP', }, + 'queue_watermark_counter': { + 'key': 'QUEUE_WATERMARK', + 'group_name': 'QUEUE_WATERMARK_STAT_COUNTER', + 'name_map': 'COUNTERS_QUEUE_NAME_MAP', + }, 'rif_counter': { 'key': 'RIF', 'group_name': 'RIF_STAT_COUNTER', @@ -39,6 +44,11 @@ 'group_name': 'PORT_BUFFER_DROP_STAT', 'name_map': 'COUNTERS_PORT_NAME_MAP', }, + 'pg_drop_counter': { + 'key': 'PG_DROP', + 'group_name': 'PG_DROP_STAT_COUNTER', + 'name_map': 'COUNTERS_PG_NAME_MAP', + }, 'pg_watermark_counter': { 'key': 'PG_WATERMARK', 'group_name': 'PG_WATERMARK_STAT_COUNTER', @@ -695,26 +705,29 @@ def set_admin_status(self, interface, status): def test_add_remove_ports(self, dvs): self.setup_dbs(dvs) - - # set flex counter - 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", "") - 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) + + oid_list_dict = {} + # set flex counter + for k,v in counter_group_meta.items(): + if 'queue' in k or 'pg' in k: + counter_key = v['key'] + counter_stat = v['group_name'] + counter_map_name = v['name_map'] + self.set_flex_counter_group_status(counter_key, counter_map_name) + + time.sleep(1) + # save all the oids of the flex counters + oid_list = [] + counters_map = self.counters_db.get_entry(counter_map_name, "") + for key, oid in counters_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_dict[k] = oid_list # get port oid port_oid = self.counters_db.get_entry(PORT_MAP, "")[PORT] @@ -722,34 +735,46 @@ def test_add_remove_ports(self, dvs): # 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) - assert len(fields) == 0 - - # verify that port counter maps were removed from counters db - counters_queue_map = self.counters_db.get_entry("COUNTERS_QUEUE_NAME_MAP", "") - for key in counters_queue_map.keys(): - if PORT in key: - assert False - + + for k,v in counter_group_meta.items(): + if 'queue' in k or 'pg' in k: + counter_key = v['key'] + counter_stat = v['group_name'] + counter_map_name = v['name_map'] + # verify counters were removed from flex counter table + for oid in oid_list_dict[k]: + fields = self.flex_db.get_entry("FLEX_COUNTER_TABLE", counter_stat + ":%s" % oid) + assert len(fields) == 0 + + # verify that port counter maps were removed from counters db + counters_map = self.counters_db.get_entry(counter_map_name, "") + for key in counters_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")) - + 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", "") - - 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) + + time.sleep(1) + for k,v in counter_group_meta.items(): + if 'queue' in k or 'pg' in k: + counter_key = v['key'] + counter_stat = v['group_name'] + counter_map_name = v['name_map'] + dvs.get_counters_db().wait_for_fields(counter_map_name, "", ["%s:0"%(PORT)]) + + # verify counters were added + oid_list = [] + counters_map = self.counters_db.get_entry(counter_map_name, "") + + for key, oid in counters_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 len(oid_list_dict[k]) == len(oid_list)