Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PFC_WD] Avoid applying ZeroBuffer Profiles to ingress PG when a PFC storm is detected #2304

Merged
merged 11 commits into from
Jun 24, 2022
Merged

[PFC_WD] Avoid applying ZeroBuffer Profiles to ingress PG when a PFC storm is detected #2304

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
748bdaf
PfcWdZeroBufferHandler only applies zero buffer profile on egress queue
vivekrnv May 19, 2022
1ae35ab
Merge branch 'Azure:master' into pfc_pg
vivekrnv May 19, 2022
7702108
Test Updated
vivekrnv May 31, 2022
6f449cd
Merge branch 'pfc_pg' of https://github.com/vivekreddynv/sonic-swss i…
vivekrnv May 31, 2022
7dd0e2d
Merge branch 'Azure:master' into pfc_pg
vivekrnv Jun 14, 2022
ed7b28c
Removed the bufferorch's ability to cache previousy created pool's an…
vivekrnv Jun 17, 2022
1e485cc
Updated the ZeroBufferHandler UT to the new design
vivekrnv Jun 17, 2022
49cd8b1
Merge branch 'Azure:master' into pfc_pg
vivekrnv Jun 21, 2022
9c25179
Comment handled
vivekrnv Jun 22, 2022
c97c25c
Merge branch 'pfc_pg' of https://github.com/vivekreddynv/sonic-swss i…
vivekrnv Jun 22, 2022
4e821bf
Update dynamic_th
vivekrnv Jun 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 15 additions & 151 deletions orchagent/bufferorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,7 @@ BufferOrch::BufferOrch(DBConnector *applDb, DBConnector *confDb, DBConnector *st
m_flexCounterTable(new ProducerTable(m_flexCounterDb.get(), FLEX_COUNTER_TABLE)),
m_flexCounterGroupTable(new ProducerTable(m_flexCounterDb.get(), FLEX_COUNTER_GROUP_TABLE)),
m_countersDb(new DBConnector("COUNTERS_DB", 0)),
m_stateBufferMaximumValueTable(stateDb, STATE_BUFFER_MAXIMUM_VALUE_TABLE),
m_ingressZeroBufferPool(SAI_NULL_OBJECT_ID),
m_egressZeroBufferPool(SAI_NULL_OBJECT_ID),
m_ingressZeroPoolRefCount(0),
m_egressZeroPoolRefCount(0)
neethajohn marked this conversation as resolved.
Show resolved Hide resolved
m_stateBufferMaximumValueTable(stateDb, STATE_BUFFER_MAXIMUM_VALUE_TABLE)
{
SWSS_LOG_ENTER();
initTableHandlers();
Expand Down Expand Up @@ -314,65 +310,6 @@ const object_reference_map &BufferOrch::getBufferPoolNameOidMap(void)
return *m_buffer_type_maps[APP_BUFFER_POOL_TABLE_NAME];
}

void BufferOrch::lockZeroBufferPool(bool ingress)
{
if (ingress)
m_ingressZeroPoolRefCount++;
else
m_egressZeroPoolRefCount++;
}

void BufferOrch::unlockZeroBufferPool(bool ingress)
{
sai_object_id_t pool = SAI_NULL_OBJECT_ID;
if (ingress)
{
if (--m_ingressZeroPoolRefCount <= 0)
{
pool = m_ingressZeroBufferPool;
m_ingressZeroBufferPool = SAI_NULL_OBJECT_ID;
}
}
else
{
if (--m_egressZeroPoolRefCount <= 0)
{
pool = m_egressZeroBufferPool;
m_egressZeroBufferPool = SAI_NULL_OBJECT_ID;
}
}

if (pool != SAI_NULL_OBJECT_ID)
{
auto sai_status = sai_buffer_api->remove_buffer_pool(pool);
if (SAI_STATUS_SUCCESS != sai_status)
{
SWSS_LOG_ERROR("Failed to remove buffer pool, rv:%d", sai_status);
task_process_status handle_status = handleSaiRemoveStatus(SAI_API_BUFFER, sai_status);
if (handle_status != task_process_status::task_success)
{
return;
}
}
else
{
SWSS_LOG_NOTICE("Zero buffer pool has been successfully removed");
}
}
}

void BufferOrch::setZeroBufferPool(bool ingress, sai_object_id_t pool)
{
if (ingress)
{
m_ingressZeroBufferPool = pool;
}
else
{
m_egressZeroBufferPool = pool;
}
}

task_process_status BufferOrch::processBufferPool(KeyOpFieldsValuesTuple &tuple)
{
SWSS_LOG_ENTER();
Expand All @@ -381,8 +318,6 @@ task_process_status BufferOrch::processBufferPool(KeyOpFieldsValuesTuple &tuple)
string map_type_name = APP_BUFFER_POOL_TABLE_NAME;
string object_name = kfvKey(tuple);
string op = kfvOp(tuple);
sai_buffer_pool_type_t pool_direction = SAI_BUFFER_POOL_TYPE_INGRESS;
bool creating_zero_pool = false;

SWSS_LOG_DEBUG("object name:%s", object_name.c_str());
if (m_buffer_type_maps[map_type_name]->find(object_name) != m_buffer_type_maps[map_type_name]->end())
Expand All @@ -396,16 +331,6 @@ task_process_status BufferOrch::processBufferPool(KeyOpFieldsValuesTuple &tuple)
}
}
SWSS_LOG_DEBUG("processing command:%s", op.c_str());
if (object_name == "ingress_zero_pool")
{
creating_zero_pool = true;
pool_direction = SAI_BUFFER_POOL_TYPE_INGRESS;
}
else if (object_name == "egress_zero_pool")
{
creating_zero_pool = true;
pool_direction = SAI_BUFFER_POOL_TYPE_EGRESS;
}

if (op == SET_COMMAND)
{
Expand Down Expand Up @@ -453,11 +378,6 @@ task_process_status BufferOrch::processBufferPool(KeyOpFieldsValuesTuple &tuple)
return task_process_status::task_invalid_entry;
}
attr.id = SAI_BUFFER_POOL_ATTR_TYPE;
if (creating_zero_pool && pool_direction != static_cast<sai_buffer_pool_type_t>(attr.value.u32))
{
SWSS_LOG_ERROR("Wrong pool direction for pool %s", object_name.c_str());
return task_process_status::task_invalid_entry;
}
attribs.push_back(attr);
}
else if (field == buffer_pool_mode_field_name)
Expand Down Expand Up @@ -523,54 +443,20 @@ task_process_status BufferOrch::processBufferPool(KeyOpFieldsValuesTuple &tuple)
}
else
{
if (creating_zero_pool)
{
if (pool_direction == SAI_BUFFER_POOL_TYPE_INGRESS)
{
sai_object = m_ingressZeroBufferPool;
}
else if (pool_direction == SAI_BUFFER_POOL_TYPE_EGRESS)
{
sai_object = m_egressZeroBufferPool;
}
}

if (SAI_NULL_OBJECT_ID == sai_object)
{
sai_status = sai_buffer_api->create_buffer_pool(&sai_object, gSwitchId, (uint32_t)attribs.size(), attribs.data());
if (SAI_STATUS_SUCCESS != sai_status)
{
SWSS_LOG_ERROR("Failed to create buffer pool %s with type %s, rv:%d", object_name.c_str(), map_type_name.c_str(), sai_status);
task_process_status handle_status = handleSaiCreateStatus(SAI_API_BUFFER, sai_status);
if (handle_status != task_process_status::task_success)
{
return handle_status;
}
}

SWSS_LOG_NOTICE("Created buffer pool %s with type %s", object_name.c_str(), map_type_name.c_str());
}
else
{
SWSS_LOG_NOTICE("No need to create buffer pool %s since it has been created", object_name.c_str());
}

if (creating_zero_pool)
sai_status = sai_buffer_api->create_buffer_pool(&sai_object, gSwitchId, (uint32_t)attribs.size(), attribs.data());
if (SAI_STATUS_SUCCESS != sai_status)
{
if (pool_direction == SAI_BUFFER_POOL_TYPE_INGRESS)
{
m_ingressZeroPoolRefCount++;
m_ingressZeroBufferPool = sai_object;
}
else if (pool_direction == SAI_BUFFER_POOL_TYPE_EGRESS)
SWSS_LOG_ERROR("Failed to create buffer pool %s with type %s, rv:%d", object_name.c_str(), map_type_name.c_str(), sai_status);
task_process_status handle_status = handleSaiCreateStatus(SAI_API_BUFFER, sai_status);
if (handle_status != task_process_status::task_success)
{
m_egressZeroPoolRefCount++;
m_egressZeroBufferPool = sai_object;
return handle_status;
}
}

(*(m_buffer_type_maps[map_type_name]))[object_name].m_saiObjectId = sai_object;
(*(m_buffer_type_maps[map_type_name]))[object_name].m_pendingRemove = false;
SWSS_LOG_NOTICE("Created buffer pool %s with type %s", object_name.c_str(), map_type_name.c_str());
// Here we take the PFC watchdog approach to update the COUNTERS_DB metadata (e.g., PFC_WD_DETECTION_TIME per queue)
// at initialization (creation and registration phase)
// Specifically, we push the buffer pool name to oid mapping upon the creation of the oid
Expand All @@ -593,39 +479,17 @@ task_process_status BufferOrch::processBufferPool(KeyOpFieldsValuesTuple &tuple)
if (SAI_NULL_OBJECT_ID != sai_object)
{
clearBufferPoolWatermarkCounterIdList(sai_object);
bool remove = true;
if (sai_object == m_ingressZeroBufferPool)
{
if (--m_ingressZeroPoolRefCount > 0)
remove = false;
else
m_ingressZeroBufferPool = SAI_NULL_OBJECT_ID;
}
else if (sai_object == m_egressZeroBufferPool)
{
if (--m_egressZeroPoolRefCount > 0)
remove = false;
else
m_egressZeroBufferPool = SAI_NULL_OBJECT_ID;
}
if (remove)
sai_status = sai_buffer_api->remove_buffer_pool(sai_object);
if (SAI_STATUS_SUCCESS != sai_status)
{
sai_status = sai_buffer_api->remove_buffer_pool(sai_object);
if (SAI_STATUS_SUCCESS != sai_status)
SWSS_LOG_ERROR("Failed to remove buffer pool %s with type %s, rv:%d", object_name.c_str(), map_type_name.c_str(), sai_status);
task_process_status handle_status = handleSaiRemoveStatus(SAI_API_BUFFER, sai_status);
if (handle_status != task_process_status::task_success)
{
SWSS_LOG_ERROR("Failed to remove buffer pool %s with type %s, rv:%d", object_name.c_str(), map_type_name.c_str(), sai_status);
task_process_status handle_status = handleSaiRemoveStatus(SAI_API_BUFFER, sai_status);
if (handle_status != task_process_status::task_success)
{
return handle_status;
}
return handle_status;
}
SWSS_LOG_NOTICE("Removed buffer pool %s with type %s", object_name.c_str(), map_type_name.c_str());
}
else
{
SWSS_LOG_NOTICE("Will not remove buffer pool %s since it is still referenced", object_name.c_str());
}
SWSS_LOG_NOTICE("Removed buffer pool %s with type %s", object_name.c_str(), map_type_name.c_str());
}
auto it_to_delete = (m_buffer_type_maps[map_type_name])->find(object_name);
(m_buffer_type_maps[map_type_name])->erase(it_to_delete);
Expand Down
12 changes: 0 additions & 12 deletions orchagent/bufferorch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,6 @@ class BufferOrch : public Orch
static type_map m_buffer_type_maps;
void generateBufferPoolWatermarkCounterIdList(void);
const object_reference_map &getBufferPoolNameOidMap(void);
sai_object_id_t getZeroBufferPool(bool ingress)
{
return ingress ? m_ingressZeroBufferPool : m_egressZeroBufferPool;
}

void lockZeroBufferPool(bool ingress);
void unlockZeroBufferPool(bool ingress);
void setZeroBufferPool(bool direction, sai_object_id_t pool);

private:
typedef task_process_status (BufferOrch::*buffer_table_handler)(KeyOpFieldsValuesTuple &tuple);
Expand Down Expand Up @@ -80,10 +72,6 @@ class BufferOrch : public Orch

bool m_isBufferPoolWatermarkCounterIdListGenerated = false;

sai_object_id_t m_ingressZeroBufferPool;
sai_object_id_t m_egressZeroBufferPool;
int m_ingressZeroPoolRefCount;
int m_egressZeroPoolRefCount;
};
#endif /* SWSS_BUFFORCH_H */

83 changes: 26 additions & 57 deletions orchagent/pfcactionhandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#include "logger.h"
#include "sai_serialize.h"
#include "portsorch.h"
#include "bufferorch.h"
#include <vector>
#include <inttypes.h>

Expand All @@ -27,7 +26,6 @@
extern sai_object_id_t gSwitchId;
extern PortsOrch *gPortsOrch;
extern AclOrch * gAclOrch;
extern BufferOrch *gBufferOrch;
extern sai_port_api_t *sai_port_api;
extern sai_queue_api_t *sai_queue_api;
extern sai_buffer_api_t *sai_buffer_api;
Expand Down Expand Up @@ -658,24 +656,6 @@ PfcWdZeroBufferHandler::ZeroBufferProfile &PfcWdZeroBufferHandler::ZeroBufferPro
return instance;
}

sai_object_id_t& PfcWdZeroBufferHandler::ZeroBufferProfile::getPool()
{
// If there is a cached zero buffer pool, just use it
// else fetch zero buffer pool from buffer orch
// If there is one, use it and increase the reference number.
// otherwise, just return NULL OID
// PfcWdZeroBufferHandler will create it later and notify buffer orch later
if (m_zeroEgressBufferPool == SAI_NULL_OBJECT_ID)
{
m_zeroEgressBufferPool = gBufferOrch->getZeroBufferPool(false);
if (m_zeroEgressBufferPool != SAI_NULL_OBJECT_ID)
{
gBufferOrch->lockZeroBufferPool(false);
}
}
return m_zeroEgressBufferPool;
}

sai_object_id_t PfcWdZeroBufferHandler::ZeroBufferProfile::getZeroBufferProfile()
{
SWSS_LOG_ENTER();
Expand All @@ -696,37 +676,28 @@ void PfcWdZeroBufferHandler::ZeroBufferProfile::createZeroBufferProfile()
vector<sai_attribute_t> attribs;
sai_status_t status;

auto &poolId = getPool();

if (SAI_NULL_OBJECT_ID == poolId)
{
// Create zero pool
attr.id = SAI_BUFFER_POOL_ATTR_SIZE;
attr.value.u64 = 0;
attribs.push_back(attr);

attr.id = SAI_BUFFER_POOL_ATTR_TYPE;
attr.value.u32 = SAI_BUFFER_POOL_TYPE_EGRESS;
attribs.push_back(attr);
// Create zero pool
attr.id = SAI_BUFFER_POOL_ATTR_SIZE;
attr.value.u64 = 0;
attribs.push_back(attr);

attr.id = SAI_BUFFER_POOL_ATTR_THRESHOLD_MODE;
attr.value.u32 = SAI_BUFFER_POOL_THRESHOLD_MODE_STATIC;
attribs.push_back(attr);
attr.id = SAI_BUFFER_POOL_ATTR_TYPE;
attr.value.u32 = SAI_BUFFER_POOL_TYPE_EGRESS;
attribs.push_back(attr);

status = sai_buffer_api->create_buffer_pool(
&poolId,
gSwitchId,
static_cast<uint32_t>(attribs.size()),
attribs.data());
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to create dynamic zero buffer pool for PFC WD: %d", status);
return;
}
attr.id = SAI_BUFFER_POOL_ATTR_THRESHOLD_MODE;
attr.value.u32 = SAI_BUFFER_POOL_THRESHOLD_MODE_STATIC;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use dynamic mode and dynamic_th -8 for creating profile. this is what was used before
L 689

    attr.value.u32 = SAI_BUFFER_POOL_THRESHOLD_MODE_DYNAMIC;

L 711

    attr.value.u32 = SAI_BUFFER_PROFILE_THRESHOLD_MODE_DYNAMIC;

L 718~719

    attr.id = SAI_BUFFER_PROFILE_ATTR_SHARED_DYNAMIC_TH;
    attr.value.s8 = 0;

attribs.push_back(attr);

// Pass the ownership to BufferOrch
gBufferOrch->setZeroBufferPool(false, poolId);
gBufferOrch->lockZeroBufferPool(false);
status = sai_buffer_api->create_buffer_pool(
&getPool(),
gSwitchId,
static_cast<uint32_t>(attribs.size()),
attribs.data());
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to create dynamic zero buffer pool for PFC WD: %d", status);
return;
}

// Create zero profile
Expand Down Expand Up @@ -764,18 +735,16 @@ void PfcWdZeroBufferHandler::ZeroBufferProfile::destroyZeroBufferProfile()
{
SWSS_LOG_ENTER();

if (getProfile() != SAI_NULL_OBJECT_ID)
sai_status_t status = sai_buffer_api->remove_buffer_profile(getProfile());
if (status != SAI_STATUS_SUCCESS)
{
sai_status_t status = sai_buffer_api->remove_buffer_profile(getProfile());
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to remove static zero buffer profile for PFC WD: %d", status);
return;
}
SWSS_LOG_ERROR("Failed to remove static zero buffer profile for PFC WD: %d", status);
return;
}

if (m_zeroEgressBufferPool != SAI_NULL_OBJECT_ID)
status = sai_buffer_api->remove_buffer_pool(getPool());
if (status != SAI_STATUS_SUCCESS)
{
gBufferOrch->unlockZeroBufferPool(false);
SWSS_LOG_ERROR("Failed to remove static zero buffer pool for PFC WD: %d", status);
}
}
5 changes: 4 additions & 1 deletion orchagent/pfcactionhandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,10 @@ class PfcWdZeroBufferHandler: public PfcWdLossyHandler
return m_zeroEgressBufferProfile;
}

sai_object_id_t& getPool();
sai_object_id_t& getPool()
{
return m_zeroEgressBufferPool;
}

sai_object_id_t m_zeroEgressBufferPool = SAI_NULL_OBJECT_ID;
sai_object_id_t m_zeroEgressBufferProfile = SAI_NULL_OBJECT_ID;
Expand Down
Loading