Skip to content

Commit

Permalink
Support mock_test infra for dynamic buffer manager and fix issues fou…
Browse files Browse the repository at this point in the history 
…nd during mock test (#2234)

* Support mock_test infra for dynamic buffer manager and fix issues found during mock test 
Signed-off-by: Stephen Sun <stephens@nvidia.com>
  • Loading branch information
@stephenxs
stephenxs authored May 28, 2022
1 parent 910bfd4 commit c73cf10
Show file tree
Hide file tree
Showing 5 changed files with 1,031 additions and 33 deletions.
148 changes: 120 additions & 28 deletions cfgmgr/buffermgrdyn.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,11 @@ BufferMgrDynamic::BufferMgrDynamic(DBConnector *cfgDb, DBConnector *stateDb, DBC
}
catch (...)
{
SWSS_LOG_ERROR("Lua scripts for buffer calculation were not loaded successfully, buffermgrd won't start");
return;
if (platform != "mock_test")
{
SWSS_LOG_ERROR("Lua scripts for buffer calculation were not loaded successfully, buffermgrd won't start");
return;
}
}

// Init timer
Expand Down Expand Up @@ -718,7 +721,13 @@ void BufferMgrDynamic::recalculateSharedBufferPool()
// - In case the shared headroom pool size is statically configured, as it is programmed to APPL_DB during buffer pool handling,
// - any change from lua plugin will be ignored.
// - will handle ingress_lossless_pool in the way all other pools are handled in this case
auto &pool = m_bufferPoolLookup[poolName];
const auto &poolRef = m_bufferPoolLookup.find(poolName);
if (poolRef == m_bufferPoolLookup.end())
{
SWSS_LOG_WARN("Unconfigured buffer pool %s got from lua plugin", poolName.c_str());
continue;
}
auto &pool = poolRef->second;
auto &poolSizeStr = pairs[1];
auto old_xoff = pool.xoff;
bool xoff_updated = false;
Expand Down Expand Up @@ -875,10 +884,8 @@ void BufferMgrDynamic::updateBufferProfileToDb(const string &name, const buffer_
}

vector<FieldValueTuple> fvVector;
string mode = getPgPoolMode();

// profile threshold field name
mode += "_th";
const string &&mode = profile.threshold_mode.empty() ? getPgPoolMode() + "_th" : profile.threshold_mode;

if (profile.lossless)
{
Expand Down Expand Up @@ -959,7 +966,7 @@ task_process_status BufferMgrDynamic::allocateProfile(const string &speed, const
string mode = getPgPoolMode();
if (mode.empty())
{
SWSS_LOG_NOTICE("BUFFER_PROFILE %s cannot be created because the buffer pool isn't ready", profile_name.c_str());
SWSS_LOG_INFO("BUFFER_PROFILE %s cannot be created because the buffer pool isn't ready", profile_name.c_str());
return task_process_status::task_need_retry;
}

Expand Down Expand Up @@ -1430,9 +1437,10 @@ task_process_status BufferMgrDynamic::refreshPgsForPort(const string &port, cons
return task_process_status::task_success;
}

if (!m_bufferPoolReady)
if (!m_bufferPoolReady || m_defaultThreshold.empty())
{
SWSS_LOG_INFO("Nothing to be done since the buffer pool is not ready");
SWSS_LOG_INFO("Nothing to be done since either the buffer pool or default threshold is not ready");
m_bufferObjectsPending = true;
return task_process_status::task_success;
}

Expand All @@ -1454,6 +1462,12 @@ task_process_status BufferMgrDynamic::refreshPgsForPort(const string &port, cons

if (portPg.dynamic_calculated)
{
if (portInfo.state != PORT_READY)
{
SWSS_LOG_INFO("Nothing to be done for %s since port is not ready", key.c_str());
continue;
}

string threshold;
// Calculate new headroom size
if (portPg.static_configured)
Expand Down Expand Up @@ -1892,10 +1906,16 @@ task_process_status BufferMgrDynamic::handleBufferMaxParam(KeyOpFieldsValuesTupl
task_process_status BufferMgrDynamic::handleDefaultLossLessBufferParam(KeyOpFieldsValuesTuple &tuple)
{
string op = kfvOp(tuple);
string newRatio = "0";
string newRatio = "";

if (op == SET_COMMAND)
{
if (m_bufferPoolLookup.find(INGRESS_LOSSLESS_PG_POOL_NAME) == m_bufferPoolLookup.end())
{
SWSS_LOG_INFO("%s has not been configured, need to retry", INGRESS_LOSSLESS_PG_POOL_NAME);
return task_process_status::task_need_retry;
}

for (auto i : kfvFieldsValues(tuple))
{
if (fvField(i) == "default_dynamic_th")
Expand All @@ -1910,6 +1930,10 @@ task_process_status BufferMgrDynamic::handleDefaultLossLessBufferParam(KeyOpFiel
}
}
}
else if (op == DEL_COMMAND)
{
newRatio = "";
}
else
{
SWSS_LOG_ERROR("Unsupported command %s received for DEFAULT_LOSSLESS_BUFFER_PARAMETER table", op.c_str());
Expand Down Expand Up @@ -2398,6 +2422,10 @@ task_process_status BufferMgrDynamic::handleBufferProfileTable(KeyOpFieldsValues
// For set command:
// 1. Create the corresponding table entries in APPL_DB
// 2. Record the table in the internal cache m_bufferProfileLookup

// If the profile did not exist, it will be created in the next line by the [] operator with incomplete data.
// In case the flow does not finish successfully, the incomplete profile should be removed
bool needRemoveOnFailure = (m_bufferProfileLookup.find(profileName) == m_bufferProfileLookup.end());
buffer_profile_t &profileApp = m_bufferProfileLookup[profileName];

profileApp.static_configured = true;
Expand All @@ -2418,24 +2446,44 @@ task_process_status BufferMgrDynamic::handleBufferProfileTable(KeyOpFieldsValues
if (!value.empty())
{
auto &poolName = value;
if (poolName.empty())
{
SWSS_LOG_ERROR("BUFFER_PROFILE: Invalid format of reference to pool: %s", value.c_str());
return task_process_status::task_invalid_entry;
}

auto poolRef = m_bufferPoolLookup.find(poolName);
if (poolRef == m_bufferPoolLookup.end())
{
SWSS_LOG_WARN("Pool %s hasn't been configured yet, need retry", poolName.c_str());
SWSS_LOG_INFO("Pool %s hasn't been configured yet, need retry", poolName.c_str());
if (needRemoveOnFailure)
{
m_bufferProfileLookup.erase(profileName);
}
return task_process_status::task_need_retry;
}
profileApp.pool_name = poolName;
profileApp.direction = poolRef->second.direction;
auto threshold_mode = poolRef->second.mode + "_th";
if (profileApp.threshold_mode.empty())
{
profileApp.threshold_mode = threshold_mode;
}
else if (profileApp.threshold_mode != threshold_mode)
{
SWSS_LOG_ERROR("Buffer profile %s's mode %s doesn't match with buffer pool %s whose mode is %s",
profileName.c_str(),
profileApp.threshold_mode.c_str(),
poolName.c_str(),
threshold_mode.c_str());
if (needRemoveOnFailure)
{
m_bufferProfileLookup.erase(profileName);
}
return task_process_status::task_failed;
}
}
else
{
SWSS_LOG_ERROR("Pool for BUFFER_PROFILE %s hasn't been specified", field.c_str());
if (needRemoveOnFailure)
{
m_bufferProfileLookup.erase(profileName);
}
return task_process_status::task_failed;
}
}
Expand All @@ -2456,12 +2504,25 @@ task_process_status BufferMgrDynamic::handleBufferProfileTable(KeyOpFieldsValues
{
profileApp.size = value;
}
else if (field == buffer_dynamic_th_field_name)
{
profileApp.threshold = value;
}
else if (field == buffer_static_th_field_name)
else if (field == buffer_dynamic_th_field_name || field == buffer_static_th_field_name)
{
if (profileApp.threshold_mode.empty())
{
profileApp.threshold_mode = field;
}
else if (profileApp.threshold_mode != field)
{
SWSS_LOG_ERROR("Buffer profile %s's mode %s doesn't align with buffer pool %s whose mode is %s",
profileName.c_str(),
field.c_str(),
profileApp.pool_name.c_str(),
profileApp.threshold_mode.c_str());
if (needRemoveOnFailure)
{
m_bufferProfileLookup.erase(profileName);
}
return task_process_status::task_failed;
}
profileApp.threshold = value;
}
else if (field == buffer_headroom_type_field_name)
Expand All @@ -2484,7 +2545,11 @@ task_process_status BufferMgrDynamic::handleBufferProfileTable(KeyOpFieldsValues
if (profileApp.direction != BUFFER_INGRESS)
{
SWSS_LOG_ERROR("BUFFER_PROFILE %s is ingress but referencing an egress pool %s", profileName.c_str(), profileApp.pool_name.c_str());
return task_process_status::task_success;
if (needRemoveOnFailure)
{
m_bufferProfileLookup.erase(profileName);
}
return task_process_status::task_failed;
}

if (profileApp.dynamic_calculated)
Expand Down Expand Up @@ -2752,6 +2817,9 @@ void BufferMgrDynamic::handleDelSingleBufferObjectOnAdminDownPort(buffer_directi
task_process_status BufferMgrDynamic::handleSingleBufferPgEntry(const string &key, const string &port, const KeyOpFieldsValuesTuple &tuple)
{
string op = kfvOp(tuple);
// If the buffer PG did not exist, it will be created in the next line by the [] operator with incomplete data.
// In case the flow does not finish successfully, the incomplete profile should be removed
bool needRemoveOnFailure = (m_portPgLookup[port].find(key) == m_portPgLookup[port].end());
buffer_pg_t &bufferPg = m_portPgLookup[port][key];
port_info_t &portInfo = m_portInfoLookup[port];

Expand Down Expand Up @@ -2787,6 +2855,10 @@ task_process_status BufferMgrDynamic::handleSingleBufferPgEntry(const string &ke
if (profileName.empty())
{
SWSS_LOG_ERROR("BUFFER_PG: Invalid format of reference to profile: %s", value.c_str());
if (needRemoveOnFailure)
{
m_portPgLookup[port].erase(key);
}
return task_process_status::task_invalid_entry;
}

Expand All @@ -2795,13 +2867,25 @@ task_process_status BufferMgrDynamic::handleSingleBufferPgEntry(const string &ke
{
// In this case, we shouldn't set the dynamic calculated flag to true
// It will be updated when its profile configured.
bufferPg.dynamic_calculated = false;
SWSS_LOG_WARN("Profile %s hasn't been configured yet, skip", profileName.c_str());
if (needRemoveOnFailure)
{
m_portPgLookup[port].erase(key);
}
SWSS_LOG_INFO("Profile %s hasn't been configured yet, skip", profileName.c_str());
return task_process_status::task_need_retry;
}
else
{
buffer_profile_t &profileRef = searchRef->second;
if (profileRef.direction == BUFFER_EGRESS)
{
if (needRemoveOnFailure)
{
m_portPgLookup[port].erase(key);
}
SWSS_LOG_ERROR("Egress buffer profile configured on PG %s", key.c_str());
return task_process_status::task_failed;
}
bufferPg.dynamic_calculated = profileRef.dynamic_calculated;
bufferPg.configured_profile_name = profileName;
bufferPg.lossless = profileRef.lossless;
Expand All @@ -2813,6 +2897,10 @@ task_process_status BufferMgrDynamic::handleSingleBufferPgEntry(const string &ke
if (field != buffer_profile_field_name)
{
SWSS_LOG_ERROR("BUFFER_PG: Invalid field %s", field.c_str());
if (needRemoveOnFailure)
{
m_portPgLookup[port].erase(key);
}
return task_process_status::task_invalid_entry;
}

Expand Down Expand Up @@ -2896,6 +2984,7 @@ task_process_status BufferMgrDynamic::handleSingleBufferPgEntry(const string &ke
else
{
SWSS_LOG_ERROR("Unknown operation type %s", op.c_str());
m_portPgLookup[port].erase(key);
return task_process_status::task_invalid_entry;
}

Expand All @@ -2911,7 +3000,7 @@ task_process_status BufferMgrDynamic::checkBufferProfileDirection(const string &
auto profileSearchRef = m_bufferProfileLookup.find(profileName);
if (profileSearchRef == m_bufferProfileLookup.end())
{
SWSS_LOG_NOTICE("Profile %s doesn't exist, need retry", profileName.c_str());
SWSS_LOG_INFO("Profile %s doesn't exist, need retry", profileName.c_str());
return task_process_status::task_need_retry;
}

Expand Down Expand Up @@ -2983,6 +3072,8 @@ task_process_status BufferMgrDynamic::handleSingleBufferQueueEntry(const string
}
SWSS_LOG_INFO("Removing entry %s from APPL_DB", key.c_str());
m_portQueueLookup[port].erase(queues);
if (m_portQueueLookup[port].empty())
m_portQueueLookup.erase(port);
if (PORT_ADMIN_DOWN == portInfo.state)
{
handleDelSingleBufferObjectOnAdminDownPort(BUFFER_QUEUE, port, key, portInfo);
Expand Down Expand Up @@ -3189,7 +3280,8 @@ void BufferMgrDynamic::doTask(Consumer &consumer)
{
case task_process_status::task_failed:
SWSS_LOG_ERROR("Failed to process table update");
return;
it = consumer.m_toSync.erase(it);
break;
case task_process_status::task_need_retry:
SWSS_LOG_INFO("Unable to process table update. Will retry...");
it++;
Expand Down Expand Up @@ -3238,7 +3330,7 @@ void BufferMgrDynamic::doTask(Consumer &consumer)
*/
void BufferMgrDynamic::handlePendingBufferObjects()
{
if (m_bufferPoolReady)
if (m_bufferPoolReady && !m_defaultThreshold.empty())
{
if (!m_pendingApplyZeroProfilePorts.empty())
{
Expand Down
3 changes: 2 additions & 1 deletion cfgmgr/buffermgrdyn.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ typedef struct {
std::string xon_offset;
std::string xoff;
std::string threshold;
std::string threshold_mode;
std::string pool_name;
// port_pgs - stores pgs referencing this profile
// An element will be added or removed when a PG added or removed
Expand Down Expand Up @@ -177,7 +178,7 @@ class BufferMgrDynamic : public Orch

std::string m_configuredSharedHeadroomPoolSize;

std::shared_ptr<DBConnector> m_applDb = nullptr;
DBConnector *m_applDb = nullptr;
SelectableTimer *m_buffermgrPeriodtimer = nullptr;

// Fields for zero pool and profiles
Expand Down
6 changes: 4 additions & 2 deletions tests/mock_tests/Makefile.am
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ FLEX_CTR_DIR = $(top_srcdir)/orchagent/flex_counter
DEBUG_CTR_DIR = $(top_srcdir)/orchagent/debug_counter
P4_ORCH_DIR = $(top_srcdir)/orchagent/p4orch

INCLUDES = -I $(FLEX_CTR_DIR) -I $(DEBUG_CTR_DIR) -I $(top_srcdir)/lib
INCLUDES = -I $(FLEX_CTR_DIR) -I $(DEBUG_CTR_DIR) -I $(top_srcdir)/lib -I $(top_srcdir)/cfgmgr

CFLAGS_SAI = -I /usr/include/sai

Expand All @@ -26,6 +26,7 @@ tests_SOURCES = aclorch_ut.cpp \
routeorch_ut.cpp \
qosorch_ut.cpp \
bufferorch_ut.cpp \
buffermgrdyn_ut.cpp \
fdborch/flush_syncd_notif_ut.cpp \
copporch_ut.cpp \
saispy_ut.cpp \
Expand Down Expand Up @@ -95,7 +96,8 @@ tests_SOURCES = aclorch_ut.cpp \
$(top_srcdir)/orchagent/lagid.cpp \
$(top_srcdir)/orchagent/bfdorch.cpp \
$(top_srcdir)/orchagent/srv6orch.cpp \
$(top_srcdir)/orchagent/nvgreorch.cpp
$(top_srcdir)/orchagent/nvgreorch.cpp \
$(top_srcdir)/cfgmgr/buffermgrdyn.cpp

tests_SOURCES += $(FLEX_CTR_DIR)/flex_counter_manager.cpp $(FLEX_CTR_DIR)/flex_counter_stat_manager.cpp $(FLEX_CTR_DIR)/flow_counter_handler.cpp $(FLEX_CTR_DIR)/flowcounterrouteorch.cpp
tests_SOURCES += $(DEBUG_CTR_DIR)/debug_counter.cpp $(DEBUG_CTR_DIR)/drop_counter.cpp
Expand Down
Loading

0 comments on commit c73cf10

Please sign in to comment.