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

Fix tables handling race condition in buffermgr #484

Merged
merged 2 commits into from
Apr 24, 2018
Merged

Fix tables handling race condition in buffermgr #484

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
4160b00
Fix tables handling race condition in buffermgr
andriymoroz-mlnx Apr 23, 2018
ef9df5c
Fixed status lose in loop
andriymoroz-mlnx Apr 24, 2018
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
36 changes: 25 additions & 11 deletions cfgmgr/buffermgr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,10 @@ void BufferMgr::readPgProfileLookupFile(string file)
infile.close();
}

void BufferMgr::doCableTask(string port, string cable_length)
task_process_status BufferMgr::doCableTask(string port, string cable_length)
{
m_cableLenLookup[port] = cable_length;
return task_process_status::task_success;
}

string BufferMgr::getPgPoolMode()
Expand Down Expand Up @@ -108,15 +109,15 @@ Create/update two tables: profile (in m_cfgBufferProfileTable) and port buffer (
}
}
*/
void BufferMgr::doSpeedUpdateTask(string port, string speed)
task_process_status BufferMgr::doSpeedUpdateTask(string port, string speed)
{
vector<FieldValueTuple> fvVector;
string cable;

if (m_cableLenLookup.count(port) == 0)
{
SWSS_LOG_ERROR("Unable to create/update PG profile for port %s. Cable length is not set", port.c_str());
return;
SWSS_LOG_WARN("Unable to create/update PG profile for port %s. Cable length is not set", port.c_str());
return task_process_status::task_need_retry;
}

cable = m_cableLenLookup[port];
Expand All @@ -125,7 +126,7 @@ void BufferMgr::doSpeedUpdateTask(string port, string speed)
{
SWSS_LOG_ERROR("Unable to create/update PG profile for port %s. No PG profile configured for speed %s and cable length %s",
port.c_str(), speed.c_str(), cable.c_str());
return;
return task_process_status::task_failed;
}

// Crete record in BUFFER_PROFILE table
Expand All @@ -142,8 +143,8 @@ void BufferMgr::doSpeedUpdateTask(string port, string speed)
if (mode.empty())
{
// this should never happen if switch initialized properly
SWSS_LOG_ERROR("PG lossless pool is not yet created");
return;
SWSS_LOG_WARN("PG lossless pool is not yet created");
return task_process_status::task_need_retry;
}

// profile threshold field name
Expand Down Expand Up @@ -180,6 +181,7 @@ void BufferMgr::doSpeedUpdateTask(string port, string speed)

fvVector.push_back(make_pair("profile", profile_ref));
m_cfgBufferPgTable.set(buffer_pg_key, fvVector);
return task_process_status::task_success;
}

void BufferMgr::doTask(Consumer &consumer)
Expand All @@ -198,25 +200,37 @@ void BufferMgr::doTask(Consumer &consumer)
string port(keys[0]);

string op = kfvOp(t);
task_process_status task_status = task_process_status::task_success;
if (op == SET_COMMAND)
{
for (auto i : kfvFieldsValues(t))
{
if (table_name == CFG_PORT_CABLE_LEN_TABLE_NAME)
{
// receive and cache cable length table
doCableTask(fvField(i), fvValue(i));
task_status = doCableTask(fvField(i), fvValue(i));
}
// In case of PORT table update, Buffer Manager is interested in speed update only
if (table_name == CFG_PORT_TABLE_NAME && fvField(i) == "speed")
{
// create/update profile for port
doSpeedUpdateTask(port, fvValue(i));
task_status = doSpeedUpdateTask(port, fvValue(i));
Copy link
Contributor

Choose a reason for hiding this comment

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

task_status is going to get overridden in the for loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right... fixed

}
}
}

it = consumer.m_toSync.erase(it);
continue;
switch (task_status)
{
case task_process_status::task_failed:
SWSS_LOG_ERROR("Failed to process table update");
return;
case task_process_status::task_need_retry:
SWSS_LOG_INFO("Unable to process table update. Will retry...");
++it;
break;
default:
it = consumer.m_toSync.erase(it);
break;
}
}
}
4 changes: 2 additions & 2 deletions cfgmgr/buffermgr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ class BufferMgr : public Orch
port_cable_length_t m_cableLenLookup;
std::string getPgPoolMode();
void readPgProfileLookupFile(std::string);
void doCableTask(string port, string cable_length);
void doSpeedUpdateTask(string port, string speed);
task_process_status doCableTask(string port, string cable_length);
task_process_status doSpeedUpdateTask(string port, string speed);

void doTask(Consumer &consumer);
};
Expand Down