Skip to content

Commit 32bb66f

Browse files
andriymoroz-mlnxlguohan
authored andcommitted
Fix tables handling race condition in buffermgr (sonic-net#484)
* Fix tables handling race condition in buffermgr Signed-off-by: Andriy Moroz <[email protected]> * Fixed status lose in loop Signed-off-by: Andriy Moroz <[email protected]>
1 parent ccaa769 commit 32bb66f

File tree

2 files changed

+31
-13
lines changed

2 files changed

+31
-13
lines changed

cfgmgr/buffermgr.cpp

+29-11
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,10 @@ void BufferMgr::readPgProfileLookupFile(string file)
7272
infile.close();
7373
}
7474

75-
void BufferMgr::doCableTask(string port, string cable_length)
75+
task_process_status BufferMgr::doCableTask(string port, string cable_length)
7676
{
7777
m_cableLenLookup[port] = cable_length;
78+
return task_process_status::task_success;
7879
}
7980

8081
string BufferMgr::getPgPoolMode()
@@ -108,15 +109,15 @@ Create/update two tables: profile (in m_cfgBufferProfileTable) and port buffer (
108109
}
109110
}
110111
*/
111-
void BufferMgr::doSpeedUpdateTask(string port, string speed)
112+
task_process_status BufferMgr::doSpeedUpdateTask(string port, string speed)
112113
{
113114
vector<FieldValueTuple> fvVector;
114115
string cable;
115116

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

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

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

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

181182
fvVector.push_back(make_pair("profile", profile_ref));
182183
m_cfgBufferPgTable.set(buffer_pg_key, fvVector);
184+
return task_process_status::task_success;
183185
}
184186

185187
void BufferMgr::doTask(Consumer &consumer)
@@ -198,25 +200,41 @@ void BufferMgr::doTask(Consumer &consumer)
198200
string port(keys[0]);
199201

200202
string op = kfvOp(t);
203+
task_process_status task_status = task_process_status::task_success;
201204
if (op == SET_COMMAND)
202205
{
203206
for (auto i : kfvFieldsValues(t))
204207
{
205208
if (table_name == CFG_PORT_CABLE_LEN_TABLE_NAME)
206209
{
207210
// receive and cache cable length table
208-
doCableTask(fvField(i), fvValue(i));
211+
task_status = doCableTask(fvField(i), fvValue(i));
209212
}
210213
// In case of PORT table update, Buffer Manager is interested in speed update only
211214
if (table_name == CFG_PORT_TABLE_NAME && fvField(i) == "speed")
212215
{
213216
// create/update profile for port
214-
doSpeedUpdateTask(port, fvValue(i));
217+
task_status = doSpeedUpdateTask(port, fvValue(i));
218+
}
219+
if (task_status != task_process_status::task_success)
220+
{
221+
break;
215222
}
216223
}
217224
}
218225

219-
it = consumer.m_toSync.erase(it);
220-
continue;
226+
switch (task_status)
227+
{
228+
case task_process_status::task_failed:
229+
SWSS_LOG_ERROR("Failed to process table update");
230+
return;
231+
case task_process_status::task_need_retry:
232+
SWSS_LOG_INFO("Unable to process table update. Will retry...");
233+
++it;
234+
break;
235+
default:
236+
it = consumer.m_toSync.erase(it);
237+
break;
238+
}
221239
}
222240
}

cfgmgr/buffermgr.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ class BufferMgr : public Orch
4444
port_cable_length_t m_cableLenLookup;
4545
std::string getPgPoolMode();
4646
void readPgProfileLookupFile(std::string);
47-
void doCableTask(string port, string cable_length);
48-
void doSpeedUpdateTask(string port, string speed);
47+
task_process_status doCableTask(string port, string cable_length);
48+
task_process_status doSpeedUpdateTask(string port, string speed);
4949

5050
void doTask(Consumer &consumer);
5151
};

0 commit comments

Comments
 (0)