From d41ed7183b7879559f0d8b0d1428aec569d93bd4 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Thu, 15 Jul 2021 02:45:31 +0000 Subject: [PATCH 1/2] Bug fix: Don't create lossless buffer profile for ports without speed configured Root cause: - In handlePortTableUpdate, refreshPgsFromPort is called if admin status is updated even if the speed is not configured. This is reasonable because the port can be configured as headroom override and the profile should be applied in that case. - However, the port's state is set to PORT_READY in refreshPgsForPort regardless whether speed is configured, which is not correct. This is should be avoided and PORT_READY should be set by caller if necessary Fix: - Don't set port's state to READY in refreshPgsForPort - Explicitly set port's state to READY in handlePortState. This has been done by all other callers. Signed-off-by: Stephen Sun --- cfgmgr/buffermgrdyn.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cfgmgr/buffermgrdyn.cpp b/cfgmgr/buffermgrdyn.cpp index 7b4b19388db..6e2be5c1952 100644 --- a/cfgmgr/buffermgrdyn.cpp +++ b/cfgmgr/buffermgrdyn.cpp @@ -987,8 +987,6 @@ task_process_status BufferMgrDynamic::refreshPgsForPort(const string &port, cons SWSS_LOG_DEBUG("Nothing to do for port %s since no PG configured on it", port.c_str()); } - portInfo.state = PORT_READY; - // Remove the old profile which is probably not referenced anymore. if (!profilesToBeReleased.empty()) { @@ -1452,6 +1450,7 @@ task_process_status BufferMgrDynamic::handlePortStateTable(KeyOpFieldsValuesTupl { if (isNonZero(portInfo.cable_length) && portInfo.state != PORT_ADMIN_DOWN) { + portInfo.state = PORT_READY; refreshPgsForPort(port, portInfo.effective_speed, portInfo.cable_length, portInfo.mtu); } } From eb75044e4b54debb038bd44d97ad27f1d11644ed Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Thu, 15 Jul 2021 15:24:38 +0800 Subject: [PATCH 2/2] Check the port's state before calling refreshPgsForPort Signed-off-by: Stephen Sun --- cfgmgr/buffermgrdyn.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cfgmgr/buffermgrdyn.cpp b/cfgmgr/buffermgrdyn.cpp index 6e2be5c1952..071fec6f788 100644 --- a/cfgmgr/buffermgrdyn.cpp +++ b/cfgmgr/buffermgrdyn.cpp @@ -1215,6 +1215,9 @@ task_process_status BufferMgrDynamic::doUpdateBufferProfileForDynamicTh(buffer_p SWSS_LOG_DEBUG("Checking PG %s for dynamic profile %s", key.c_str(), profileName.c_str()); portsChecked.insert(portName); + if (port.state != PORT_READY) + continue; + rc = refreshPgsForPort(portName, port.effective_speed, port.cable_length, port.mtu); if (task_process_status::task_success != rc) {