Skip to content

[201911][pfcwd] Avoid ingress drop by not attaching zero profiles when pfc storm is detected #2279

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

Merged
lguohan merged 2 commits into from
May 19, 2022
Merged

[201911][pfcwd] Avoid ingress drop by not attaching zero profiles when pfc storm is detected #2279

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a6e60f4
Allow ingress traffic forwarding when pfc storm is detected
neethajohn May 17, 2022
17d662f
Change test pool type to egress in UT
neethajohn May 18, 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
5 changes: 0 additions & 5 deletions orchagent/bufferorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -728,11 +728,6 @@ task_process_status BufferOrch::processPriorityGroup(Consumer &consumer)
SWSS_LOG_ERROR("Invalid pg index specified:%zd", ind);
return task_process_status::task_invalid_entry;
}
if (port.m_priority_group_lock[ind])
{
SWSS_LOG_WARN("Priority group %zd on port %s is locked, will retry", ind, port_name.c_str());
return task_process_status::task_need_retry;
}
pg_id = port.m_priority_group_ids[ind];
SWSS_LOG_DEBUG("Applying buffer profile:0x%" PRIx64 " to port:%s pg index:%zd, pg sai_id:0x%" PRIx64, sai_buffer_profile, port_name.c_str(), ind, pg_id);
sai_status_t sai_status = sai_buffer_api->set_ingress_priority_group_attribute(pg_id, &attr);
Expand Down
82 changes: 19 additions & 63 deletions orchagent/pfcactionhandler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ PfcWdZeroBufferHandler::PfcWdZeroBufferHandler(sai_object_id_t port,
return;
}

setPriorityGroupAndQueueLockFlag(portInstance, true);
setQueueLockFlag(portInstance, true);

sai_attribute_t attr;
attr.id = SAI_QUEUE_ATTR_BUFFER_PROFILE_ID;
Expand All @@ -457,7 +457,7 @@ PfcWdZeroBufferHandler::PfcWdZeroBufferHandler(sai_object_id_t port,
sai_object_id_t oldQueueProfileId = attr.value.oid;

attr.id = SAI_QUEUE_ATTR_BUFFER_PROFILE_ID;
attr.value.oid = ZeroBufferProfile::getZeroBufferProfile(false);
attr.value.oid = ZeroBufferProfile::getZeroBufferProfile();

// Set our zero buffer profile
status = sai_queue_api->set_queue_attribute(queue, &attr);
Expand All @@ -469,35 +469,6 @@ PfcWdZeroBufferHandler::PfcWdZeroBufferHandler(sai_object_id_t port,

// Save original buffer profile
m_originalQueueBufferProfile = oldQueueProfileId;

// Get PG
sai_object_id_t pg = portInstance.m_priority_group_ids[static_cast <size_t> (queueId)];

attr.id = SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE;

// Get PG's buffer profile
status = sai_buffer_api->get_ingress_priority_group_attribute(pg, 1, &attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to get buffer profile ID on PG 0x%" PRIx64 ": %d", pg, status);
return;
}

// Set zero profile to PG
sai_object_id_t oldPgProfileId = attr.value.oid;

attr.id = SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE;
attr.value.oid = ZeroBufferProfile::getZeroBufferProfile(true);

status = sai_buffer_api->set_ingress_priority_group_attribute(pg, &attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to set buffer profile ID on pg 0x%" PRIx64 ": %d", pg, status);
return;
}

// Save original buffer profile
m_originalPgBufferProfile = oldPgProfileId;
}

PfcWdZeroBufferHandler::~PfcWdZeroBufferHandler(void)
Expand All @@ -523,26 +494,12 @@ PfcWdZeroBufferHandler::~PfcWdZeroBufferHandler(void)
return;
}

sai_object_id_t pg = portInstance.m_priority_group_ids[size_t(getQueueId())];

attr.id = SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE;
attr.value.oid = m_originalPgBufferProfile;

// Set our zero buffer profile
status = sai_buffer_api->set_ingress_priority_group_attribute(pg, &attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to set buffer profile ID on queue 0x%" PRIx64 ": %d", getQueue(), status);
return;
}

setPriorityGroupAndQueueLockFlag(portInstance, false);
setQueueLockFlag(portInstance, false);
}

void PfcWdZeroBufferHandler::setPriorityGroupAndQueueLockFlag(Port& port, bool isLocked) const
void PfcWdZeroBufferHandler::setQueueLockFlag(Port& port, bool isLocked) const
{
// set lock bits on PG and queue
port.m_priority_group_lock[static_cast<size_t>(getQueueId())] = isLocked;
// set lock bits on queue
for (size_t i = 0; i < port.m_queue_ids.size(); ++i)
{
if (port.m_queue_ids[i] == getQueue())
Expand All @@ -562,9 +519,8 @@ PfcWdZeroBufferHandler::ZeroBufferProfile::~ZeroBufferProfile(void)
{
SWSS_LOG_ENTER();

// Destory ingress and egress prifiles and pools
destroyZeroBufferProfile(true);
destroyZeroBufferProfile(false);
// Destory egress profiles and pools
destroyZeroBufferProfile();
}

PfcWdZeroBufferHandler::ZeroBufferProfile &PfcWdZeroBufferHandler::ZeroBufferProfile::getInstance(void)
Expand All @@ -576,19 +532,19 @@ PfcWdZeroBufferHandler::ZeroBufferProfile &PfcWdZeroBufferHandler::ZeroBufferPro
return instance;
}

sai_object_id_t PfcWdZeroBufferHandler::ZeroBufferProfile::getZeroBufferProfile(bool ingress)
sai_object_id_t PfcWdZeroBufferHandler::ZeroBufferProfile::getZeroBufferProfile(void)
{
SWSS_LOG_ENTER();

if (getInstance().getProfile(ingress) == SAI_NULL_OBJECT_ID)
if (getInstance().getProfile() == SAI_NULL_OBJECT_ID)
{
getInstance().createZeroBufferProfile(ingress);
getInstance().createZeroBufferProfile();
}

return getInstance().getProfile(ingress);
return getInstance().getProfile();
}

void PfcWdZeroBufferHandler::ZeroBufferProfile::createZeroBufferProfile(bool ingress)
void PfcWdZeroBufferHandler::ZeroBufferProfile::createZeroBufferProfile(void)
{
SWSS_LOG_ENTER();

Expand All @@ -601,15 +557,15 @@ void PfcWdZeroBufferHandler::ZeroBufferProfile::createZeroBufferProfile(bool ing
attribs.push_back(attr);

attr.id = SAI_BUFFER_POOL_ATTR_TYPE;
attr.value.u32 = ingress ? SAI_BUFFER_POOL_TYPE_INGRESS : SAI_BUFFER_POOL_TYPE_EGRESS;
attr.value.u32 = SAI_BUFFER_POOL_TYPE_EGRESS;
attribs.push_back(attr);

attr.id = SAI_BUFFER_POOL_ATTR_THRESHOLD_MODE;
attr.value.u32 = SAI_BUFFER_POOL_THRESHOLD_MODE_DYNAMIC;
attribs.push_back(attr);

sai_status_t status = sai_buffer_api->create_buffer_pool(
&getPool(ingress),
&getPool(),
gSwitchId,
static_cast<uint32_t>(attribs.size()),
attribs.data());
Expand All @@ -623,7 +579,7 @@ void PfcWdZeroBufferHandler::ZeroBufferProfile::createZeroBufferProfile(bool ing
attribs.clear();

attr.id = SAI_BUFFER_PROFILE_ATTR_POOL_ID;
attr.value.oid = getPool(ingress);
attr.value.oid = getPool();
attribs.push_back(attr);

attr.id = SAI_BUFFER_PROFILE_ATTR_THRESHOLD_MODE;
Expand All @@ -639,7 +595,7 @@ void PfcWdZeroBufferHandler::ZeroBufferProfile::createZeroBufferProfile(bool ing
attribs.push_back(attr);

status = sai_buffer_api->create_buffer_profile(
&getProfile(ingress),
&getProfile(),
gSwitchId,
static_cast<uint32_t>(attribs.size()),
attribs.data());
Expand All @@ -650,18 +606,18 @@ void PfcWdZeroBufferHandler::ZeroBufferProfile::createZeroBufferProfile(bool ing
}
}

void PfcWdZeroBufferHandler::ZeroBufferProfile::destroyZeroBufferProfile(bool ingress)
void PfcWdZeroBufferHandler::ZeroBufferProfile::destroyZeroBufferProfile(void)
{
SWSS_LOG_ENTER();

sai_status_t status = sai_buffer_api->remove_buffer_profile(getProfile(ingress));
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;
}

status = sai_buffer_api->remove_buffer_pool(getPool(ingress));
status = sai_buffer_api->remove_buffer_pool(getPool());
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to remove static zero buffer pool for PFC WD: %d", status);
Expand Down
21 changes: 9 additions & 12 deletions orchagent/pfcactionhandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,42 +124,39 @@ class PfcWdZeroBufferHandler: public PfcWdLossyHandler

private:
/*
* Sets lock bits on port's priority group and queue
* Sets lock bits on port's queue
* to protect them from beeing changed by other Orch's
*/
void setPriorityGroupAndQueueLockFlag(Port& port, bool isLocked) const;
void setQueueLockFlag(Port& port, bool isLocked) const;

// Singletone class for keeping shared data - zero buffer profiles
class ZeroBufferProfile
{
public:
~ZeroBufferProfile(void);
static sai_object_id_t getZeroBufferProfile(bool ingress);
static sai_object_id_t getZeroBufferProfile(void);

private:
ZeroBufferProfile(void);
static ZeroBufferProfile &getInstance(void);
void createZeroBufferProfile(bool ingress);
void destroyZeroBufferProfile(bool ingress);
void createZeroBufferProfile(void);
void destroyZeroBufferProfile(void);

sai_object_id_t& getProfile(bool ingress)
sai_object_id_t& getProfile(void)
{
return ingress ? m_zeroIngressBufferProfile : m_zeroEgressBufferProfile;
return m_zeroEgressBufferProfile;
}

sai_object_id_t& getPool(bool ingress)
sai_object_id_t& getPool(void)
{
return ingress ? m_zeroIngressBufferPool : m_zeroEgressBufferPool;
return m_zeroEgressBufferPool;
}

sai_object_id_t m_zeroIngressBufferPool = SAI_NULL_OBJECT_ID;
sai_object_id_t m_zeroEgressBufferPool = SAI_NULL_OBJECT_ID;
sai_object_id_t m_zeroIngressBufferProfile = SAI_NULL_OBJECT_ID;
sai_object_id_t m_zeroEgressBufferProfile = SAI_NULL_OBJECT_ID;
};

sai_object_id_t m_originalQueueBufferProfile = SAI_NULL_OBJECT_ID;
sai_object_id_t m_originalPgBufferProfile = SAI_NULL_OBJECT_ID;
};

#endif
7 changes: 3 additions & 4 deletions orchagent/port.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,13 @@ class Port
uint32_t m_nat_zone_id = 0;

/*
* Following two bit vectors are used to lock
* the PG/queue from being changed in BufferOrch.
* Following bit vector is used to lock
* the queue from being changed in BufferOrch.
* The use case scenario is when PfcWdZeroBufferHandler
* sets zero buffer profile it should protect PG/queue
* sets zero buffer profile it should protect queue
* from being overwritten in BufferOrch.
*/
std::vector<bool> m_queue_lock;
std::vector<bool> m_priority_group_lock;

bool m_fec_cfg = false;
bool m_an_cfg = false;
Expand Down
1 change: 0 additions & 1 deletion orchagent/portsorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2835,7 +2835,6 @@ void PortsOrch::initializePriorityGroups(Port &port)
SWSS_LOG_INFO("Get %d priority groups for port %s", attr.value.u32, port.m_alias.c_str());

port.m_priority_group_ids.resize(attr.value.u32);
port.m_priority_group_lock.resize(attr.value.u32);

if (attr.value.u32 == 0)
{
Expand Down
24 changes: 11 additions & 13 deletions tests/mock_tests/portsorch_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ namespace portsorch_test
TEST_F(PortsOrchTest, PfcZeroBufferHandlerLocksPortPgAndQueue)
{
Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME);
Table pgTable = Table(m_config_db.get(), CFG_BUFFER_PG_TABLE_NAME);
Table queueTable = Table(m_config_db.get(), CFG_BUFFER_QUEUE_TABLE_NAME);
Table profileTable = Table(m_config_db.get(), CFG_BUFFER_PROFILE_TABLE_NAME);
Table poolTable = Table(m_config_db.get(), CFG_BUFFER_POOL_TABLE_NAME);

Expand Down Expand Up @@ -397,8 +397,6 @@ namespace portsorch_test

// Create test buffer profile
profileTable.set("test_profile", { { "pool", "[BUFFER_POOL|test_pool]" },
{ "xon", "14832" },
{ "xoff", "14832" },
{ "size", "35000" },
{ "dynamic_th", "0" } });

Expand All @@ -407,29 +405,29 @@ namespace portsorch_test
{
std::ostringstream oss;
oss << it.first << "|3-4";
pgTable.set(oss.str(), { { "profile", "[BUFFER_PROFILE|test_profile]" } });
queueTable.set(oss.str(), { { "profile", "[BUFFER_PROFILE|test_profile]" } });
}
gBufferOrch->addExistingData(&pgTable);
gBufferOrch->addExistingData(&queueTable);
gBufferOrch->addExistingData(&poolTable);
gBufferOrch->addExistingData(&profileTable);

// process pool, profile and PGs
// process pool, profile and queues
static_cast<Orch *>(gBufferOrch)->doTask();

auto pgConsumer = static_cast<Consumer*>(gBufferOrch->getExecutor(CFG_BUFFER_PG_TABLE_NAME));
pgConsumer->dumpPendingTasks(ts);
ASSERT_FALSE(ts.empty()); // PG is skipped
auto queueConsumer = static_cast<Consumer*>(gBufferOrch->getExecutor(CFG_BUFFER_QUEUE_TABLE_NAME));
queueConsumer->dumpPendingTasks(ts);
ASSERT_FALSE(ts.empty()); // Queue is skipped
ts.clear();

// release zero buffer drop handler
dropHandler.reset();

// process PGs
// process queue
static_cast<Orch *>(gBufferOrch)->doTask();

pgConsumer = static_cast<Consumer*>(gBufferOrch->getExecutor(CFG_BUFFER_PG_TABLE_NAME));
pgConsumer->dumpPendingTasks(ts);
ASSERT_TRUE(ts.empty()); // PG should be proceesed now
queueConsumer = static_cast<Consumer*>(gBufferOrch->getExecutor(CFG_BUFFER_QUEUE_TABLE_NAME));
queueConsumer->dumpPendingTasks(ts);
ASSERT_TRUE(ts.empty()); // Queue should be proceesed now
ts.clear();
}

Expand Down