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

[201911] [PfcWdZeroBufferHandler] Apple zero buffer profile to only egress queues and not ingress PG's #10

Closed
wants to merge 2 commits into from
+19 −56
Closed

[201911] [PfcWdZeroBufferHandler] Apple zero buffer profile to only egress queues and not ingress PG's #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
91d686a
ZeroBuffer Handler Updated
vivekrnv May 17, 2022
9f2d54b
Updated source and ut
May 17, 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
51 changes: 4 additions & 47 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 Down Expand Up @@ -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 Down
7 changes: 3 additions & 4 deletions orchagent/pfcactionhandler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,10 @@ class PfcWdZeroBufferHandler: public PfcWdLossyHandler

private:
/*
* Sets lock bits on port's priority group and queue
* to protect them from beeing changed by other Orch's
* Sets lock bits on port's queue
* to protect them from being 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
Expand Down Expand Up @@ -159,7 +159,6 @@ class PfcWdZeroBufferHandler: public PfcWdLossyHandler
};

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

#endif
17 changes: 12 additions & 5 deletions tests/mock_tests/portsorch_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,10 +315,11 @@ namespace portsorch_test
ASSERT_TRUE(ts.empty());
}

TEST_F(PortsOrchTest, PfcZeroBufferHandlerLocksPortPgAndQueue)
TEST_F(PortsOrchTest, PfcZeroBufferHandlerLocksPortQueue)
{
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 @@ -408,8 +409,10 @@ 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]" } });
Copy link

Choose a reason for hiding this comment

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

better to use an egress buffer pool and profile for BUFFER_QUEUE table

        poolTable.set(
            "test_egress_pool",
            {
                { "type", "egress" },
                { "mode", "dynamic" },
                { "size", "4200000" },
            });
...
        profileTable.set("test_egress_profile", { { "pool", "[BUFFER_POOL|test_egress_pool]" },
                                           { "dynamic_th", "0" } });

}
gBufferOrch->addExistingData(&pgTable);
gBufferOrch->addExistingData(&queueTable);
gBufferOrch->addExistingData(&poolTable);
gBufferOrch->addExistingData(&profileTable);

Expand All @@ -418,7 +421,12 @@ namespace portsorch_test

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

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

// release zero buffer drop handler
Expand All @@ -427,9 +435,8 @@ namespace portsorch_test
// process PGs
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->dumpPendingTasks(ts);
ASSERT_TRUE(ts.empty()); // Queue should be proceesed now
ts.clear();
}

Expand Down