Skip to content

Commit

Permalink
Enhance mock test for dynamic buffer manager for port removing and qo…
Browse files Browse the repository at this point in the history 
…s reload flows (sonic-net#2262)

What I did
Enhance the mock test of the dynamic buffer manager in port remove and config qos clear flow and fix bugs during mock test implementation
Implement mock method ProduceStateTable::del
Signed-off-by: Stephen Sun stephens@nvidia.com

How I verified it
Run regression test, mock test, vs test, and manual test.

Details if related
1. Support mock test for dynamic buffer manager
config qos clear and reclaiming buffer
Remove port
2. Handle port remove/create flow
Cache cable length for a port
Try reclaiming unused buffer when maximum buffer parameters are received for a port whose state is ADMIN_DOWN and m_bufferCompletelyInitialized is true
3. Handle config qos clear
If all buffer pools are removed when m_bufferPoolReady is true, remove all zero pools and profiles.
Reload zero profiles and pools if they have not been loaded when reclaiming buffer
  • Loading branch information
@stephenxs
stephenxs authored Jun 17, 2022
1 parent 700492f commit 1bb5070
Show file tree
Hide file tree
Showing 4 changed files with 610 additions and 0 deletions.
114 changes: 114 additions & 0 deletions cfgmgr/buffermgrdyn.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1862,6 +1862,14 @@ task_process_status BufferMgrDynamic::handleBufferMaxParam(KeyOpFieldsValuesTupl
SWSS_LOG_INFO("BUFFER_MAX_PARAM: Got port %s's max priority group %s", key.c_str(), value.c_str());

portInfo.maximum_buffer_objects[BUFFER_PG] = (sai_uint32_t)pgCount;

if (m_bufferCompletelyInitialized && portInfo.state == PORT_ADMIN_DOWN)
{
// This is mostly for the case where the port is created only-the-fly
// The maximum buffer parameters can be received after buffer items
reclaimReservedBufferForPort(key, m_portPgLookup, BUFFER_PG);
SWSS_LOG_NOTICE("Admin-down port %s is handled after maximum buffer parameter has been received", key.c_str());
}
}
else if (fvField(i) == "max_queues")
{
Expand All @@ -1875,6 +1883,14 @@ task_process_status BufferMgrDynamic::handleBufferMaxParam(KeyOpFieldsValuesTupl
SWSS_LOG_INFO("BUFFER_MAX_PARAM: Got port %s's max queue %s", key.c_str(), value.c_str());

portInfo.maximum_buffer_objects[BUFFER_QUEUE] = (sai_uint32_t)queueCount;

if (m_bufferCompletelyInitialized && portInfo.state == PORT_ADMIN_DOWN)
{
// This is mostly for the case where the port is created only-the-fly
// The maximum buffer parameters can be received after buffer items
reclaimReservedBufferForPort(key, m_portQueueLookup, BUFFER_QUEUE);
SWSS_LOG_NOTICE("Admin-down port %s is handled after maximum buffer parameter has been received", key.c_str());
}
}
}
}
Expand Down Expand Up @@ -1961,6 +1977,7 @@ task_process_status BufferMgrDynamic::handleCableLenTable(KeyOpFieldsValuesTuple
int failed_item_count = 0;
if (op == SET_COMMAND)
{
m_cableLengths.clear();
for (auto i : kfvFieldsValues(tuple))
{
// receive and cache cable length table
Expand All @@ -1975,6 +1992,8 @@ task_process_status BufferMgrDynamic::handleCableLenTable(KeyOpFieldsValuesTuple
port.c_str(),
portInfo.effective_speed.c_str(), portInfo.cable_length.c_str(), portInfo.gearbox_model.c_str());

m_cableLengths[port] = cable_length;

if (portInfo.cable_length == cable_length)
{
continue;
Expand Down Expand Up @@ -2183,6 +2202,11 @@ task_process_status BufferMgrDynamic::handlePortTable(KeyOpFieldsValuesTuple &tu
string &mtu = portInfo.mtu;
string &effective_speed = portInfo.effective_speed;

if (cable_length.empty() && !m_cableLengths[port].empty())
{
cable_length = m_cableLengths[port];
}

bool need_refresh_all_buffer_objects = false, need_handle_admin_down = false, was_admin_down = false;

if (effective_speed_updated || mtu_updated)
Expand Down Expand Up @@ -2304,6 +2328,28 @@ task_process_status BufferMgrDynamic::handlePortTable(KeyOpFieldsValuesTuple &tu
task_status = refreshPgsForPort(port, portInfo.effective_speed, portInfo.cable_length, portInfo.mtu);
}
}
else if (op == DEL_COMMAND)
{
cleanUpItemsForReclaimingBuffer(port);
if ((m_portPgLookup.find(port) != m_portPgLookup.end()
&& !m_portPgLookup[port].empty())
|| (m_portQueueLookup.find(port) != m_portQueueLookup.end()
&& !m_portQueueLookup[port].empty())
|| (m_portProfileListLookups[BUFFER_INGRESS].find(port) != m_portProfileListLookups[BUFFER_INGRESS].end()
&& !m_portProfileListLookups[BUFFER_INGRESS][port].empty())
|| (m_portProfileListLookups[BUFFER_EGRESS].find(port) != m_portProfileListLookups[BUFFER_EGRESS].end()
&& !m_portProfileListLookups[BUFFER_EGRESS][port].empty()))
{
SWSS_LOG_INFO("Port %s can't be removed before buffer items have been removed", port.c_str());
return task_process_status::task_need_retry;
}
m_portPgLookup.erase(port);
m_portQueueLookup.erase(port);
m_portProfileListLookups[BUFFER_INGRESS].erase(port);
m_portProfileListLookups[BUFFER_EGRESS].erase(port);
m_portInfoLookup.erase(port);
SWSS_LOG_NOTICE("Port %s is removed", port.c_str());
}

return task_status;
}
Expand Down Expand Up @@ -2401,6 +2447,28 @@ task_process_status BufferMgrDynamic::handleBufferPoolTable(KeyOpFieldsValuesTup
m_applBufferPoolTable.del(pool);
m_stateBufferPoolTable.del(pool);
m_bufferPoolLookup.erase(pool);
if (pool == INGRESS_LOSSLESS_PG_POOL_NAME)
{
m_configuredSharedHeadroomPoolSize.clear();
}

if (m_bufferPoolReady && m_bufferPoolLookup.empty())
{
for(auto &port : m_adminDownPorts)
{
cleanUpItemsForReclaimingBuffer(port);
}

// Zero profiles must be unloaded once all pools have been uploaded
// This can be resulted from "config qos reload"
// Any zero profile left can leads to buffer pool not able to be cleared
unloadZeroPoolAndProfiles();

m_bufferPoolReady = false;
m_bufferCompletelyInitialized = false;

m_pendingApplyZeroProfilePorts = m_adminDownPorts;
}
}
else
{
Expand Down Expand Up @@ -2634,6 +2702,12 @@ void BufferMgrDynamic::handleSetSingleBufferObjectOnAdminDownPort(buffer_directi
{
if (idsToZero.empty())
{
// Happens only after "config qos reload"
if (!m_zeroProfilesLoaded)
{
loadZeroPoolAndProfiles();
}

// If initialization finished, no extra handle required.
// Check whether the key overlaps with supported but not configured map
auto const &idsToAdd = parseObjectNameFromKey(key, 1);
Expand Down Expand Up @@ -2749,6 +2823,14 @@ void BufferMgrDynamic::handleDelSingleBufferObjectOnAdminDownPort(buffer_directi

if (idsToZero.empty())
{
if (!m_bufferPoolReady)
{
// Reclaiming buffer has not started yet so just remove it.
// Do not add it to "supported but not configured" set
updateBufferObjectToDb(key, "", false, direction);
return;
}

// For admin down ports, if zero profiles have been applied to all configured items
// do NOT remove it otherwise SDK default value will be set for the items
// Move the key to supported_but_not_configured_items so that the slice of items
Expand Down Expand Up @@ -3125,6 +3207,22 @@ task_process_status BufferMgrDynamic::handleSingleBufferPortProfileListEntry(con
// For admin-down ports, zero profile list has been applied on the port when it entered admin-down state
updateBufferObjectListToDb(key, profileListLookup[port], dir);
}
else
{
const auto &profileList = m_portProfileListLookups[dir][port];
if (!profileList.empty())
{
// Happens only after "config qos reload"
if (!m_zeroProfilesLoaded)
{
loadZeroPoolAndProfiles();
}
vector<FieldValueTuple> fvVector;
const string &zeroProfileNameList = constructZeroProfileListFromNormalProfileList(profileList, port);
fvVector.emplace_back(buffer_profile_list_field_name, zeroProfileNameList);
m_applBufferProfileListTables[dir].set(port, fvVector);
}
}
}
else if (op == DEL_COMMAND)
{
Expand Down Expand Up @@ -3462,9 +3560,25 @@ void BufferMgrDynamic::handlePendingBufferObjects()
}
}

void BufferMgrDynamic::cleanUpItemsForReclaimingBuffer(const string &port)
{
// Clean up zero buffers when the buffer pools or a port has been removed
if (!m_bufferObjectIdsToZero[BUFFER_PG].empty())
{
updateBufferObjectToDb(port + delimiter + m_bufferObjectIdsToZero[BUFFER_PG], "", false, BUFFER_PG);
}
if (!m_bufferObjectIdsToZero[BUFFER_QUEUE].empty())
{
updateBufferObjectToDb(port + delimiter + m_bufferObjectIdsToZero[BUFFER_QUEUE], "", false, BUFFER_QUEUE);
}
removeSupportedButNotConfiguredItemsOnPort(m_portInfoLookup[port], port);
}

void BufferMgrDynamic::doTask(SelectableTimer &timer)
{
checkSharedBufferPoolSize(true);
if (!m_bufferCompletelyInitialized)
{
handlePendingBufferObjects();
}
}
2 changes: 2 additions & 0 deletions cfgmgr/buffermgrdyn.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ class BufferMgrDynamic : public Orch
// key: port name
// updated only when a port's speed and cable length updated
port_info_lookup_t m_portInfoLookup;
std::map<std::string, std::string> m_cableLengths;
std::set<std::string> m_adminDownPorts;
std::set<std::string> m_pendingApplyZeroProfilePorts;
std::set<std::string> m_pendingSupportedButNotConfiguredPorts[BUFFER_DIR_MAX];
Expand Down Expand Up @@ -302,6 +303,7 @@ class BufferMgrDynamic : public Orch
void handleSetSingleBufferObjectOnAdminDownPort(buffer_direction_t direction, const std::string &port, const std::string &key, const std::string &profile);
void handleDelSingleBufferObjectOnAdminDownPort(buffer_direction_t direction, const std::string &port, const std::string &key, port_info_t &portInfo);
bool isReadyToReclaimBufferOnPort(const std::string &port);
void cleanUpItemsForReclaimingBuffer(const std::string &port);

// Main flows
template<class T> task_process_status reclaimReservedBufferForPort(const std::string &port, T &obj, buffer_direction_t dir);
Expand Down
Loading

0 comments on commit 1bb5070

Please sign in to comment.