From c13055c39641d1f6af50af1183931c850a12903b Mon Sep 17 00:00:00 2001 From: bingwang-ms <66248323+bingwang-ms@users.noreply.github.com> Date: Fri, 29 Apr 2022 09:39:33 +0800 Subject: [PATCH] Update orchagent to support new field `pfcwd_sw_enable` (#2171) Signed-off-by: bingwang bingwang@microsoft.com What I did Currently, the entry pfc_enable in table PORT_QOS_MAP is used to specify pfc and pfc_watchdog are enabled on which queues. To avoid PFC deadlock in Dual-ToR scrnario, we are going to introduce two extra lossless queues to carry bounced back traffic.HLD. The extra lossless queues require another two pfc_watchdogs, and the new watchdogs will be implemented by hardware due to limited resources. The hardware pfc watchdog is not covered in this PR. To specify on which queue to enable pfc watchdog, we need to define new table pfcwd_sw_enable. Table Description pfc_enable Specify on which queues to enable PFC pfcwd_sw_enable Specify on which queues to enable software PFC watchdog This PR is to update orchagent to support new field pfcwd_sw_enable . As two extra lossless PGs (2 and 6) are to be added, buffermgrd is also updated in this PR to generate lossless profile for the new PGs. Why I did it Update orchagent to support new field pfcwd_sw_enable . How I verified it Verified by UT. sudo pytest3 --dvsname=vs tests/test_pfcwd.py::TestPfcwdFunc -v --pdb ========================================================================================= test session starts ========================================================================================= platform linux -- Python 3.6.9, pytest-7.0.1, pluggy-1.0.0 -- /usr/bin/python3 cachedir: .pytest_cache rootdir: /home/bingwang/work/sonic/sonic-buildimage-master/src/sonic-swss plugins: flaky-3.7.0 collected 2 items tests/test_pfcwd.py::TestPfcwdFunc::test_pfcwd_software_single_queue PASSED [ 50%] tests/test_pfcwd.py::TestPfcwdFunc::test_pfcwd_software_multi_queue PASSED sudo pytest test_buffer_traditional.py ===================================================================================================================== test session starts ====================================================================================================================== platform linux -- Python 3.7.5, pytest-7.1.1, pluggy-1.0.0 rootdir: /home/bingwang/work/sonic/sonic-buildimage-master/src/sonic-swss/tests collected 1 item test_buffer_traditional.py . --- cfgmgr/buffermgr.cpp | 187 +++++++++++++++++++++---------- cfgmgr/buffermgr.h | 10 +- cfgmgr/buffermgrd.cpp | 3 +- orchagent/pfcwdorch.cpp | 10 +- orchagent/port.h | 3 +- orchagent/portsorch.cpp | 37 ++++++ orchagent/portsorch.h | 3 + orchagent/qosorch.cpp | 18 ++- orchagent/qosorch.h | 1 + tests/test_buffer_traditional.py | 106 ++++++++++++++++-- tests/test_pfcwd.py | 8 +- 11 files changed, 308 insertions(+), 78 deletions(-) diff --git a/cfgmgr/buffermgr.cpp b/cfgmgr/buffermgr.cpp index 0fb39a862a..5c7d6ae9e6 100644 --- a/cfgmgr/buffermgr.cpp +++ b/cfgmgr/buffermgr.cpp @@ -133,11 +133,11 @@ Create/update two tables: profile (in m_cfgBufferProfileTable) and port buffer ( } } */ -task_process_status BufferMgr::doSpeedUpdateTask(string port, bool admin_up) +task_process_status BufferMgr::doSpeedUpdateTask(string port) { - vector fvVectorPg, fvVectorProfile; string cable; string speed; + string pfc_enable; if (m_cableLenLookup.count(port) == 0) { @@ -152,32 +152,54 @@ task_process_status BufferMgr::doSpeedUpdateTask(string port, bool admin_up) return task_process_status::task_success; } - speed = m_speedLookup[port]; + if (m_portStatusLookup.count(port) == 0) + { + // admin_statue is not available yet. This can happen when notification of `PORT_QOS_MAP` table + // comes first. + SWSS_LOG_INFO("pfc_enable status is not available for port %s", port.c_str()); + return task_process_status::task_need_retry; + } + + if (m_portPfcStatus.count(port) == 0) + { + // PORT_QOS_MAP is not ready yet. The notification is cleared, and buffer pg + // will be handled when `pfc_enable` in `PORT_QOS_MAP` table is available + SWSS_LOG_INFO("pfc_enable status is not available for port %s", port.c_str()); + return task_process_status::task_success; + } + pfc_enable = m_portPfcStatus[port]; - string buffer_pg_key = port + m_cfgBufferPgTable.getTableNameSeparator() + LOSSLESS_PGS; + speed = m_speedLookup[port]; // key format is pg_lossless___profile string buffer_profile_key = "pg_lossless_" + speed + "_" + cable + "_profile"; string profile_ref = buffer_profile_key; + + vector lossless_pgs = tokenize(pfc_enable, ','); - m_cfgBufferPgTable.get(buffer_pg_key, fvVectorPg); - - if (!admin_up && m_platform == "mellanox") + if (m_portStatusLookup[port] == "down" && m_platform == "mellanox") { - // Remove the entry in BUFFER_PG table if any - if (!fvVectorPg.empty()) + for (auto lossless_pg : lossless_pgs) { - for (auto &prop : fvVectorPg) + // Remove the entry in BUFFER_PG table if any + vector fvVectorPg; + string buffer_pg_key = port + m_cfgBufferPgTable.getTableNameSeparator() + lossless_pg; + + m_cfgBufferPgTable.get(buffer_pg_key, fvVectorPg); + if (!fvVectorPg.empty()) { - if (fvField(prop) == "profile") + for (auto &prop : fvVectorPg) { - if (fvValue(prop) == profile_ref) + if (fvField(prop) == "profile") { - SWSS_LOG_NOTICE("Removing PG %s from port %s which is administrative down", buffer_pg_key.c_str(), port.c_str()); - m_cfgBufferPgTable.del(buffer_pg_key); - } - else - { - SWSS_LOG_NOTICE("Not default profile %s is configured on PG %s, won't reclaim buffer", fvValue(prop).c_str(), buffer_pg_key.c_str()); + if (fvValue(prop) == profile_ref) + { + SWSS_LOG_NOTICE("Removing PG %s from port %s which is administrative down", buffer_pg_key.c_str(), port.c_str()); + m_cfgBufferPgTable.del(buffer_pg_key); + } + else + { + SWSS_LOG_NOTICE("Not default profile %s is configured on PG %s, won't reclaim buffer", fvValue(prop).c_str(), buffer_pg_key.c_str()); + } } } } @@ -185,14 +207,15 @@ task_process_status BufferMgr::doSpeedUpdateTask(string port, bool admin_up) return task_process_status::task_success; } - + if (m_pgProfileLookup.count(speed) == 0 || m_pgProfileLookup[speed].count(cable) == 0) { - SWSS_LOG_ERROR("Unable to create/update PG profile for port %s. No PG profile configured for speed %s and cable length %s", - port.c_str(), speed.c_str(), cable.c_str()); - return task_process_status::task_invalid_entry; + SWSS_LOG_ERROR("Unable to create/update PG profile for port %s. No PG profile configured for speed %s and cable length %s", + port.c_str(), speed.c_str(), cable.c_str()); + return task_process_status::task_invalid_entry; } + vector fvVectorProfile; // check if profile already exists - if yes - skip creation m_cfgBufferProfileTable.get(buffer_profile_key, fvVectorProfile); // Create record in BUFFER_PROFILE table @@ -213,9 +236,10 @@ task_process_status BufferMgr::doSpeedUpdateTask(string port, bool admin_up) fvVectorProfile.push_back(make_pair("pool", INGRESS_LOSSLESS_PG_POOL_NAME)); fvVectorProfile.push_back(make_pair("xon", m_pgProfileLookup[speed][cable].xon)); - if (m_pgProfileLookup[speed][cable].xon_offset.length() > 0) { + if (m_pgProfileLookup[speed][cable].xon_offset.length() > 0) + { fvVectorProfile.push_back(make_pair("xon_offset", - m_pgProfileLookup[speed][cable].xon_offset)); + m_pgProfileLookup[speed][cable].xon_offset)); } fvVectorProfile.push_back(make_pair("xoff", m_pgProfileLookup[speed][cable].xoff)); fvVectorProfile.push_back(make_pair("size", m_pgProfileLookup[speed][cable].size)); @@ -227,20 +251,28 @@ task_process_status BufferMgr::doSpeedUpdateTask(string port, bool admin_up) SWSS_LOG_NOTICE("Reusing existing profile '%s'", buffer_profile_key.c_str()); } - /* Check if PG Mapping is already then log message and return. */ - for (auto& prop : fvVectorPg) + for (auto lossless_pg : lossless_pgs) { - if ((fvField(prop) == "profile") && (profile_ref == fvValue(prop))) + vector fvVectorPg; + string buffer_pg_key = port + m_cfgBufferPgTable.getTableNameSeparator() + lossless_pg; + + m_cfgBufferPgTable.get(buffer_pg_key, fvVectorPg); + + /* Check if PG Mapping is already then log message and return. */ + for (auto& prop : fvVectorPg) { - SWSS_LOG_NOTICE("PG to Buffer Profile Mapping %s already present", buffer_pg_key.c_str()); - return task_process_status::task_success; + if ((fvField(prop) == "profile") && (profile_ref == fvValue(prop))) + { + SWSS_LOG_NOTICE("PG to Buffer Profile Mapping %s already present", buffer_pg_key.c_str()); + continue; + } } - } - fvVectorPg.clear(); + fvVectorPg.clear(); - fvVectorPg.push_back(make_pair("profile", profile_ref)); - m_cfgBufferPgTable.set(buffer_pg_key, fvVectorPg); + fvVectorPg.push_back(make_pair("profile", profile_ref)); + m_cfgBufferPgTable.set(buffer_pg_key, fvVectorPg); + } return task_process_status::task_success; } @@ -346,6 +378,47 @@ void BufferMgr::doBufferMetaTask(Consumer &consumer) } } +/* +Parse PORT_QOS_MAP to retrieve on which queue PFC is enable, and +cached in a map +*/ +void BufferMgr::doPortQosTableTask(Consumer &consumer) +{ + SWSS_LOG_ENTER(); + + auto it = consumer.m_toSync.begin(); + while (it != consumer.m_toSync.end()) + { + KeyOpFieldsValuesTuple tuple = it->second; + string port_name = kfvKey(tuple); + string op = kfvOp(tuple); + if (op == SET_COMMAND) + { + bool update_pfc_enable = false; + for (auto itp : kfvFieldsValues(tuple)) + { + if (fvField(itp) == "pfc_enable") + { + if (m_portPfcStatus.count(port_name) == 0 || m_portPfcStatus[port_name] != fvValue(itp)) + { + m_portPfcStatus[port_name] = fvValue(itp); + update_pfc_enable = true; + } + SWSS_LOG_INFO("Got pfc enable status for port %s status %s", port_name.c_str(), fvValue(itp).c_str()); + break; + } + } + if (update_pfc_enable) + { + // The return status is ignored + doSpeedUpdateTask(port_name); + } + } + it = consumer.m_toSync.erase(it); + } + +} + void BufferMgr::doTask(Consumer &consumer) { SWSS_LOG_ENTER(); @@ -399,6 +472,12 @@ void BufferMgr::doTask(Consumer &consumer) return; } + if (table_name == CFG_PORT_QOS_MAP_TABLE_NAME) + { + doPortQosTableTask(consumer); + return; + } + auto it = consumer.m_toSync.begin(); while (it != consumer.m_toSync.end()) { @@ -422,7 +501,6 @@ void BufferMgr::doTask(Consumer &consumer) } else if (m_pgfile_processed && table_name == CFG_PORT_TABLE_NAME) { - bool admin_up = false; for (auto i : kfvFieldsValues(t)) { if (fvField(i) == "speed") @@ -431,39 +509,34 @@ void BufferMgr::doTask(Consumer &consumer) } if (fvField(i) == "admin_status") { - admin_up = ("up" == fvValue(i)); + m_portStatusLookup[port] = fvValue(i); } } if (m_speedLookup.count(port) != 0) { // create/update profile for port - task_status = doSpeedUpdateTask(port, admin_up); + task_status = doSpeedUpdateTask(port); } + } - if (task_status != task_process_status::task_success) - { + switch (task_status) + { + case task_process_status::task_failed: + SWSS_LOG_ERROR("Failed to process table update"); + return; + case task_process_status::task_need_retry: + SWSS_LOG_INFO("Unable to process table update. Will retry..."); + ++it; + break; + case task_process_status::task_invalid_entry: + SWSS_LOG_ERROR("Failed to process invalid entry, drop it"); + it = consumer.m_toSync.erase(it); + break; + default: + it = consumer.m_toSync.erase(it); break; - } } } - - switch (task_status) - { - case task_process_status::task_failed: - SWSS_LOG_ERROR("Failed to process table update"); - return; - case task_process_status::task_need_retry: - SWSS_LOG_INFO("Unable to process table update. Will retry..."); - ++it; - break; - case task_process_status::task_invalid_entry: - SWSS_LOG_ERROR("Failed to process invalid entry, drop it"); - it = consumer.m_toSync.erase(it); - break; - default: - it = consumer.m_toSync.erase(it); - break; - } } } diff --git a/cfgmgr/buffermgr.h b/cfgmgr/buffermgr.h index e7f04465ef..b9b3f2c496 100644 --- a/cfgmgr/buffermgr.h +++ b/cfgmgr/buffermgr.h @@ -11,7 +11,6 @@ namespace swss { #define INGRESS_LOSSLESS_PG_POOL_NAME "ingress_lossless_pool" -#define LOSSLESS_PGS "3-4" #define BUFFERMGR_TIMER_PERIOD 10 @@ -28,6 +27,8 @@ typedef std::map pg_profile_lookup_t; typedef std::map port_cable_length_t; typedef std::map port_speed_t; +typedef std::map port_pfc_status_t; +typedef std::map port_admin_status_t; class BufferMgr : public Orch { @@ -56,17 +57,22 @@ class BufferMgr : public Orch pg_profile_lookup_t m_pgProfileLookup; port_cable_length_t m_cableLenLookup; + port_admin_status_t m_portStatusLookup; port_speed_t m_speedLookup; std::string getPgPoolMode(); void readPgProfileLookupFile(std::string); task_process_status doCableTask(std::string port, std::string cable_length); - task_process_status doSpeedUpdateTask(std::string port, bool admin_up); + task_process_status doSpeedUpdateTask(std::string port); void doBufferTableTask(Consumer &consumer, ProducerStateTable &applTable); void transformSeperator(std::string &name); void doTask(Consumer &consumer); void doBufferMetaTask(Consumer &consumer); + + port_pfc_status_t m_portPfcStatus; + void doPortQosTableTask(Consumer &consumer); + }; } diff --git a/cfgmgr/buffermgrd.cpp b/cfgmgr/buffermgrd.cpp index 05932a9e3c..eb5de60b65 100644 --- a/cfgmgr/buffermgrd.cpp +++ b/cfgmgr/buffermgrd.cpp @@ -215,7 +215,8 @@ int main(int argc, char **argv) CFG_BUFFER_QUEUE_TABLE_NAME, CFG_BUFFER_PORT_INGRESS_PROFILE_LIST_NAME, CFG_BUFFER_PORT_EGRESS_PROFILE_LIST_NAME, - CFG_DEVICE_METADATA_TABLE_NAME + CFG_DEVICE_METADATA_TABLE_NAME, + CFG_PORT_QOS_MAP_TABLE_NAME }; cfgOrchList.emplace_back(new BufferMgr(&cfgDb, &applDb, pg_lookup_file, cfg_buffer_tables)); } diff --git a/orchagent/pfcwdorch.cpp b/orchagent/pfcwdorch.cpp index be4c1e51c4..62765ab0a1 100644 --- a/orchagent/pfcwdorch.cpp +++ b/orchagent/pfcwdorch.cpp @@ -399,9 +399,9 @@ void PfcWdSwOrch::enableBigRedSwitchMode() continue; } - if (!gPortsOrch->getPortPfc(port.m_port_id, &pfcMask)) + if (!gPortsOrch->getPortPfcWatchdogStatus(port.m_port_id, &pfcMask)) { - SWSS_LOG_ERROR("Failed to get PFC mask on port %s", port.m_alias.c_str()); + SWSS_LOG_ERROR("Failed to get PFC watchdog mask on port %s", port.m_alias.c_str()); return; } @@ -443,9 +443,9 @@ void PfcWdSwOrch::enableBigRedSwitchMode() continue; } - if (!gPortsOrch->getPortPfc(port.m_port_id, &pfcMask)) + if (!gPortsOrch->getPortPfcWatchdogStatus(port.m_port_id, &pfcMask)) { - SWSS_LOG_ERROR("Failed to get PFC mask on port %s", port.m_alias.c_str()); + SWSS_LOG_ERROR("Failed to get PFC watchdog mask on port %s", port.m_alias.c_str()); return; } @@ -489,7 +489,7 @@ bool PfcWdSwOrch::registerInWdDb(const Port& port, uint8_t pfcMask = 0; - if (!gPortsOrch->getPortPfc(port.m_port_id, &pfcMask)) + if (!gPortsOrch->getPortPfcWatchdogStatus(port.m_port_id, &pfcMask)) { SWSS_LOG_ERROR("Failed to get PFC mask on port %s", port.m_alias.c_str()); return false; diff --git a/orchagent/port.h b/orchagent/port.h index 83f61e1b1c..db9f2b7bff 100644 --- a/orchagent/port.h +++ b/orchagent/port.h @@ -139,7 +139,8 @@ class Port std::vector m_queue_ids; std::vector m_priority_group_ids; sai_port_priority_flow_control_mode_t m_pfc_asym = SAI_PORT_PRIORITY_FLOW_CONTROL_MODE_COMBINED; - uint8_t m_pfc_bitmask = 0; + uint8_t m_pfc_bitmask = 0; // PFC enable bit mask + uint8_t m_pfcwd_sw_bitmask = 0; // PFC software watchdog enable uint16_t m_tpid = DEFAULT_TPID; uint32_t m_nat_zone_id = 0; uint32_t m_vnid = VNID_NONE; diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index d394b53f46..b08c5f00ab 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1193,6 +1193,43 @@ bool PortsOrch::setPortPfc(sai_object_id_t portId, uint8_t pfc_bitmask) return true; } +bool PortsOrch::setPortPfcWatchdogStatus(sai_object_id_t portId, uint8_t pfcwd_bitmask) +{ + SWSS_LOG_ENTER(); + + Port p; + + if (!getPort(portId, p)) + { + 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); + return true; +} + +bool PortsOrch::getPortPfcWatchdogStatus(sai_object_id_t portId, uint8_t *pfcwd_bitmask) +{ + SWSS_LOG_ENTER(); + + Port p; + + if (!pfcwd_bitmask || !getPort(portId, p)) + { + 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; +} + bool PortsOrch::setPortPfcAsym(Port &port, string pfc_asym) { SWSS_LOG_ENTER(); diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index bdfcf47ad0..554d49a2fc 100755 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -124,6 +124,9 @@ class PortsOrch : public Orch, public Subject bool getPortPfc(sai_object_id_t portId, uint8_t *pfc_bitmask); bool setPortPfc(sai_object_id_t portId, uint8_t pfc_bitmask); + bool setPortPfcWatchdogStatus(sai_object_id_t portId, uint8_t pfc_bitmask); + bool getPortPfcWatchdogStatus(sai_object_id_t portId, uint8_t *pfc_bitmask); + void generateQueueMap(); void generatePriorityGroupMap(); void generatePortCounterMap(); diff --git a/orchagent/qosorch.cpp b/orchagent/qosorch.cpp index 24814c3710..e71fb8f255 100644 --- a/orchagent/qosorch.cpp +++ b/orchagent/qosorch.cpp @@ -1779,6 +1779,7 @@ task_process_status QosOrch::handlePortQosMapTable(Consumer& consumer, KeyOpFiel } sai_uint8_t pfc_enable = 0; + sai_uint8_t pfcwd_sw_enable = 0; map> update_list; for (auto it = kfvFieldsValues(tuple).begin(); it != kfvFieldsValues(tuple).end(); it++) { @@ -1800,14 +1801,24 @@ task_process_status QosOrch::handlePortQosMapTable(Consumer& consumer, KeyOpFiel setObjectReference(m_qos_maps, CFG_PORT_QOS_MAP_TABLE_NAME, key, map_type_name, object_name); } - if (fvField(*it) == pfc_enable_name) + else if (fvField(*it) == pfc_enable_name || fvField(*it) == pfcwd_sw_enable_name) { + sai_uint8_t bitmask = 0; vector queue_indexes; queue_indexes = tokenize(fvValue(*it), list_item_delimiter); for(string q_ind : queue_indexes) { sai_uint8_t q_val = (uint8_t)stoi(q_ind); - pfc_enable |= (uint8_t)(1 << q_val); + bitmask |= (uint8_t)(1 << q_val); + } + + if (fvField(*it) == pfc_enable_name) + { + pfc_enable = bitmask; + } + else + { + pfcwd_sw_enable = bitmask; } } } @@ -1876,6 +1887,9 @@ task_process_status QosOrch::handlePortQosMapTable(Consumer& consumer, KeyOpFiel SWSS_LOG_INFO("Applied PFC bits 0x%x to port %s", pfc_enable, port_name.c_str()); } + + // Save pfd_wd bitmask unconditionally + gPortsOrch->setPortPfcWatchdogStatus(port.m_port_id, pfcwd_sw_enable); } SWSS_LOG_NOTICE("Applied QoS maps to ports"); diff --git a/orchagent/qosorch.h b/orchagent/qosorch.h index d0429f119d..34be88a23f 100644 --- a/orchagent/qosorch.h +++ b/orchagent/qosorch.h @@ -14,6 +14,7 @@ const string dot1p_to_tc_field_name = "dot1p_to_tc_map"; const string pfc_to_pg_map_name = "pfc_to_pg_map"; const string pfc_to_queue_map_name = "pfc_to_queue_map"; const string pfc_enable_name = "pfc_enable"; +const string pfcwd_sw_enable_name = "pfcwd_sw_enable"; const string tc_to_pg_map_field_name = "tc_to_pg_map"; const string tc_to_queue_field_name = "tc_to_queue_map"; const string scheduler_field_name = "scheduler"; diff --git a/tests/test_buffer_traditional.py b/tests/test_buffer_traditional.py index e955390fde..3d2285fd7b 100644 --- a/tests/test_buffer_traditional.py +++ b/tests/test_buffer_traditional.py @@ -3,7 +3,7 @@ class TestBuffer(object): - LOSSLESS_PGS = [3, 4] + lossless_pgs = [] INTF = "Ethernet0" def setup_db(self, dvs): @@ -15,6 +15,10 @@ def setup_db(self, dvs): # enable PG watermark self.set_pg_wm_status('enable') + def get_pfc_enable_queues(self): + qos_map = self.config_db.get_entry("PORT_QOS_MAP", self.INTF) + return qos_map['pfc_enable'].split(',') + def get_pg_oid(self, pg): fvs = dict() fvs = self.counter_db.get_entry("COUNTERS_PG_NAME_MAP", "") @@ -51,19 +55,32 @@ def get_asic_buf_pg_profiles(self): buf_pg_entries = self.asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP", self.pg_name_map[pg]) self.buf_pg_profile[pg] = buf_pg_entries["SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE"] - def change_cable_len(self, cable_len): + def change_cable_len(self, cable_len, extra_port=None): fvs = dict() fvs[self.INTF] = cable_len + if extra_port: + fvs[extra_port] = cable_len self.config_db.update_entry("CABLE_LENGTH", "AZURE", fvs) + def set_port_qos_table(self, port, pfc_enable_flag): + fvs=dict() + fvs['pfc_enable'] = pfc_enable_flag + self.config_db.update_entry("PORT_QOS_MAP", port, fvs) + self.lossless_pgs = pfc_enable_flag.split(',') + + def get_pg_name_map(self): + pg_name_map = dict() + for pg in self.lossless_pgs: + pg_name = "{}:{}".format(self.INTF, pg) + pg_name_map[pg_name] = self.get_pg_oid(pg_name) + return pg_name_map + @pytest.fixture def setup_teardown_test(self, dvs): try: self.setup_db(dvs) - pg_name_map = dict() - for pg in self.LOSSLESS_PGS: - pg_name = "{}:{}".format(self.INTF, pg) - pg_name_map[pg_name] = self.get_pg_oid(pg_name) + self.set_port_qos_table(self.INTF, '2,3,4,6') + pg_name_map = self.get_pg_name_map() yield pg_name_map finally: self.teardown() @@ -119,7 +136,8 @@ def test_zero_cable_len_profile_update(self, dvs, setup_teardown_test): self.app_db.wait_for_deleted_entry("BUFFER_PROFILE_TABLE", test_lossless_profile) # buffer pgs should still point to the original buffer profile - self.app_db.wait_for_field_match("BUFFER_PG_TABLE", self.INTF + ":3-4", {"profile": orig_lossless_profile}) + for pg in self.lossless_pgs: + self.app_db.wait_for_field_match("BUFFER_PG_TABLE", self.INTF + ":" + pg, {"profile": orig_lossless_profile}) fvs = dict() for pg in self.pg_name_map: fvs["SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE"] = self.buf_pg_profile[pg] @@ -152,3 +170,77 @@ def test_zero_cable_len_profile_update(self, dvs, setup_teardown_test): if orig_speed: dvs.port_field_set(self.INTF, "speed", orig_speed) dvs.port_admin_set(self.INTF, "down") + + # To verify the BUFFER_PG is not hardcoded to 3,4 + # buffermgrd will read 'pfc_enable' entry and apply lossless profile to that queue + def test_buffer_pg_update(self, dvs, setup_teardown_test): + self.pg_name_map = setup_teardown_test + orig_cable_len = None + orig_speed = None + test_speed = None + extra_port = "Ethernet4" + try: + # Retrieve cable len + fvs_cable_len = self.config_db.get_entry("CABLE_LENGTH", "AZURE") + orig_cable_len = fvs_cable_len[self.INTF] + if orig_cable_len == "0m": + cable_len_for_test = "300m" + fvs_cable_len[self.INTF] = cable_len_for_test + fvs_cable_len[extra_port] = cable_len_for_test + + self.config_db.update_entry("CABLE_LENGTH", "AZURE", fvs_cable_len) + else: + cable_len_for_test = orig_cable_len + # Ethernet4 is set to up, while no 'pfc_enable' available. `Ethernet0` is not supposed to be impacted + dvs.port_admin_set(extra_port, "up") + + dvs.port_admin_set(self.INTF, "up") + + # Retrieve port speed + fvs_port = self.config_db.get_entry("PORT", self.INTF) + orig_speed = fvs_port["speed"] + + # Make sure the buffer PG has been created + orig_lossless_profile = "pg_lossless_{}_{}_profile".format(orig_speed, cable_len_for_test) + self.app_db.wait_for_entry("BUFFER_PROFILE_TABLE", orig_lossless_profile) + self.orig_profiles = self.get_asic_buf_profile() + + # get the orig buf profiles attached to the pgs + self.get_asic_buf_pg_profiles() + + # Update port speed + if orig_speed == "100000": + test_speed = "40000" + elif orig_speed == "40000": + test_speed = "100000" + # change intf speed to 'test_speed' + dvs.port_field_set(self.INTF, "speed", test_speed) + dvs.port_field_set(extra_port, "speed", test_speed) + # Verify new profile is generated + new_lossless_profile = "pg_lossless_{}_{}_profile".format(test_speed, cable_len_for_test) + self.app_db.wait_for_entry("BUFFER_PROFILE_TABLE", new_lossless_profile) + + # Verify BUFFER_PG is updated + for pg in self.lossless_pgs: + self.app_db.wait_for_field_match("BUFFER_PG_TABLE", self.INTF + ":" + pg, {"profile": new_lossless_profile}) + + fvs_negative = {} + for pg in self.pg_name_map: + # verify that buffer pgs do not point to the old profile since we cannot deduce the new profile oid + fvs_negative["SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE"] = self.buf_pg_profile[pg] + self.asic_db.wait_for_field_negative_match("ASIC_STATE:SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP", self.pg_name_map[pg], fvs_negative) + + # Add pfc_enable field for extra port + self.set_port_qos_table(extra_port, '2,3,4,6') + time.sleep(1) + # Verify BUFFER_PG is updated when pfc_enable is available + for pg in self.lossless_pgs: + self.app_db.wait_for_field_match("BUFFER_PG_TABLE", extra_port + ":" + pg, {"profile": new_lossless_profile}) + finally: + if orig_cable_len: + self.change_cable_len(orig_cable_len, extra_port) + if orig_speed: + dvs.port_field_set(self.INTF, "speed", orig_speed) + dvs.port_field_set(extra_port, "speed", orig_speed) + dvs.port_admin_set(self.INTF, "down") + dvs.port_admin_set(extra_port, "down") diff --git a/tests/test_pfcwd.py b/tests/test_pfcwd.py index 2707588580..c88b6f6e96 100644 --- a/tests/test_pfcwd.py +++ b/tests/test_pfcwd.py @@ -148,9 +148,11 @@ def _get_bitmask(self, queues): return str(mask) def set_ports_pfc(self, status='enable', pfc_queues=[3,4]): + keyname = 'pfcwd_sw_enable' for port in self.test_ports: if 'enable' in status: - fvs = {'pfc_enable': ",".join([str(q) for q in pfc_queues])} + queues = ",".join([str(q) for q in pfc_queues]) + fvs = {keyname: queues, 'pfc_enable': queues} self.config_db.create_entry("PORT_QOS_MAP", port, fvs) else: self.config_db.delete_entry("PORT_QOS_MAP", port) @@ -212,7 +214,7 @@ def set_storm_state(self, queues, state="enabled"): queue_name = port + ":" + str(queue) self.counters_db.update_entry("COUNTERS", self.queue_oids[queue_name], fvs) - def test_pfcwd_single_queue(self, dvs, setup_teardown_test): + def test_pfcwd_software_single_queue(self, dvs, setup_teardown_test): try: # enable PFC on queues test_queues = [3, 4] @@ -253,7 +255,7 @@ def test_pfcwd_single_queue(self, dvs, setup_teardown_test): self.reset_pfcwd_counters(storm_queue) self.stop_pfcwd_on_ports() - def test_pfcwd_multi_queue(self, dvs, setup_teardown_test): + def test_pfcwd_software_multi_queue(self, dvs, setup_teardown_test): try: # enable PFC on queues test_queues = [3, 4]