Skip to content

Commit f509ddb

Browse files
VasanthKumar-palcSubhasri-Ramasamy
VasanthKumar-palc
authored and
Subhasri-Ramasamy
committed
Cleaned up the code
1 parent d5a2813 commit f509ddb

7 files changed

Lines changed: 283 additions & 288 deletions

File tree

‎cfgmgr/teammgr.cpp‎

Lines changed: 150 additions & 151 deletions
Original file line numberDiff line numberDiff line change
@@ -80,26 +80,26 @@ TeamMgr::TeamMgr(DBConnector *confDb, DBConnector *applDb, DBConnector *statDb,
8080
}
8181

8282
if (m_teamdMultiProcMode == "multi-process") {
83-
m_teamdUnifiedProcMode = false;
84-
SWSS_LOG_INFO("start multi process with teamd...");
83+
m_teamdUnifiedProcMode = false;
84+
SWSS_LOG_INFO("start multi process with teamd...");
8585
} else {
86-
m_teamdUnifiedProcMode = true;
87-
const string dump_path = "/var/warmboot/teamd/";
88-
string res;
89-
stringstream cmd;
90-
cmd << TEAMD_CMD
86+
m_teamdUnifiedProcMode = true;
87+
const string dump_path = "/var/warmboot/teamd/";
88+
string res;
89+
stringstream cmd;
90+
cmd << TEAMD_CMD
9191
<< " -t " << "teamd-unified"
9292
<< " -L " << dump_path
9393
<< " -g -d";
9494

95-
if (exec(cmd.str(), res) != 0)
96-
{
97-
SWSS_LOG_INFO("Failed to start single process with teamd...");
98-
return;
99-
}
100-
ipcInitTeamd();
95+
if (exec(cmd.str(), res) != 0)
96+
{
97+
SWSS_LOG_INFO("Failed to start single process with teamd...");
98+
return;
99+
}
100+
ipcInitTeamd();
101101

102-
SWSS_LOG_INFO("start single process with teamd...");
102+
SWSS_LOG_INFO("start single process with teamd...");
103103
}
104104

105105
}
@@ -218,45 +218,46 @@ void TeamMgr::cleanTeamProcesses()
218218
if (m_teamdUnifiedProcMode == false) {
219219
for (const auto& alias: m_lagList)
220220
{
221-
pid_t pid;
222-
// Sleep for 10 milliseconds so as to not overwhelm the netlink
223-
// socket buffers with events about interfaces going down
224-
std::this_thread::sleep_for(std::chrono::milliseconds(10));
221+
pid_t pid;
222+
// Sleep for 10 milliseconds so as to not overwhelm the netlink
223+
// socket buffers with events about interfaces going down
224+
std::this_thread::sleep_for(std::chrono::milliseconds(10));
225225

226-
try
227-
{
228-
ifstream pidFile("/var/run/teamd/" + alias + ".pid");
229-
if (pidFile.is_open())
226+
try
230227
{
231-
pidFile >> pid;
232-
aliasPidMap[alias] = pid;
233-
SWSS_LOG_INFO("Read port channel %s pid %d", alias.c_str(), pid);
228+
ifstream pidFile("/var/run/teamd/" + alias + ".pid");
229+
if (pidFile.is_open())
230+
{
231+
pidFile >> pid;
232+
aliasPidMap[alias] = pid;
233+
SWSS_LOG_INFO("Read port channel %s pid %d", alias.c_str(), pid);
234+
}
235+
else
236+
{
237+
SWSS_LOG_NOTICE("Unable to read pid file for %s, skipping...", alias.c_str());
238+
continue;
239+
}
234240
}
235-
else
241+
catch (const std::exception &e)
236242
{
237-
SWSS_LOG_NOTICE("Unable to read pid file for %s, skipping...", alias.c_str());
243+
// Handle Warm/Fast reboot scenario
244+
SWSS_LOG_NOTICE("Skipping non-existent port channel %s pid...", alias.c_str());
238245
continue;
239246
}
240-
}
241-
catch (const std::exception &e)
242-
{
243-
// Handle Warm/Fast reboot scenario
244-
SWSS_LOG_NOTICE("Skipping non-existent port channel %s pid...", alias.c_str());
245-
continue;
246-
}
247247

248-
if (kill(pid, SIGTERM))
249-
{
250-
SWSS_LOG_ERROR("Failed to send SIGTERM to port channel %s pid %d: %s", alias.c_str(), pid, strerror(errno));
251-
aliasPidMap.erase(alias);
252-
}
253-
else
254-
{
255-
SWSS_LOG_NOTICE("Sent SIGTERM to port channel %s pid %d", alias.c_str(), pid);
256-
}
248+
if (kill(pid, SIGTERM))
249+
{
250+
SWSS_LOG_ERROR("Failed to send SIGTERM to port channel %s pid %d: %s", alias.c_str(), pid, strerror(errno));
251+
aliasPidMap.erase(alias);
252+
}
253+
else
254+
{
255+
SWSS_LOG_NOTICE("Sent SIGTERM to port channel %s pid %d", alias.c_str(), pid);
256+
}
257257
}
258258
}
259-
else {
259+
else
260+
{
260261
std::string alias = "teamd-unified";
261262
pid_t pid;
262263
// Sleep for 10 milliseconds so as to not overwhelm the netlink
@@ -726,7 +727,7 @@ int TeamMgr::sendIpcToTeamd(const std::string& command, const std::vector<std::s
726727
SWSS_LOG_WARN("No response from teamd or recv failed: %s", strerror(errno));
727728
return task_need_retry;
728729
}
729-
730+
730731
SWSS_LOG_INFO("Response from teamd to teammgrd: %s", buffer);
731732

732733
return task_success;
@@ -799,30 +800,30 @@ task_process_status TeamMgr::addLag(const string &alias, int min_links, bool fal
799800
string warmstart_flag = WarmStart::isWarmStart() ? " -w -o" : " -r";
800801

801802
if (m_teamdUnifiedProcMode) {
802-
std::string jsonConf = conf.str();
803+
std::string jsonConf = conf.str();
803804

804805
// Remove surrounding single quotes if present
805806
if (!jsonConf.empty() && jsonConf.front() == '\'' && jsonConf.back() == '\'')
806807
{
807-
jsonConf = jsonConf.substr(1, jsonConf.size() - 2);
808+
jsonConf = jsonConf.substr(1, jsonConf.size() - 2);
808809
}
809810
sendIpcToTeamd("PortChannelAdd", {alias, jsonConf});
810811
}
812+
else
813+
{
814+
cmd << TEAMD_CMD
815+
<< warmstart_flag
816+
<< " -t " << alias
817+
<< " -c " << conf.str()
818+
<< " -L " << dump_path
819+
<< " -g -d";
811820

812-
else {
813-
cmd << TEAMD_CMD
814-
<< warmstart_flag
815-
<< " -t " << alias
816-
<< " -c " << conf.str()
817-
<< " -L " << dump_path
818-
<< " -g -d";
819-
820-
if (exec(cmd.str(), res) != 0)
821-
{
822-
SWSS_LOG_INFO("Failed to start port channel %s with teamd, retry...",
823-
alias.c_str());
824-
return task_need_retry;
825-
}
821+
if (exec(cmd.str(), res) != 0)
822+
{
823+
SWSS_LOG_INFO("Failed to start port channel %s with teamd, retry...",
824+
alias.c_str());
825+
return task_need_retry;
826+
}
826827
}
827828

828829
SWSS_LOG_NOTICE("Start port channel %s with teamd", alias.c_str());
@@ -835,32 +836,33 @@ bool TeamMgr::removeLag(const string &alias)
835836
SWSS_LOG_ENTER();
836837

837838
if (m_teamdUnifiedProcMode) {
838-
sendIpcToTeamd("PortChannelRemove", { alias });
839-
} else {
840-
841-
pid_t pid;
842-
839+
sendIpcToTeamd("PortChannelRemove", { alias });
840+
}
841+
else
843842
{
844-
ifstream pidfile("/var/run/teamd/" + alias + ".pid");
845-
if (pidfile.is_open())
843+
pid_t pid;
844+
846845
{
847-
pidfile >> pid;
848-
SWSS_LOG_INFO("Read port channel %s pid %d", alias.c_str(), pid);
846+
ifstream pidfile("/var/run/teamd/" + alias + ".pid");
847+
if (pidfile.is_open())
848+
{
849+
pidfile >> pid;
850+
SWSS_LOG_INFO("Read port channel %s pid %d", alias.c_str(), pid);
851+
}
852+
else
853+
{
854+
SWSS_LOG_NOTICE("Failed to remove non-existent port channel %s pid...", alias.c_str());
855+
return false;
856+
}
849857
}
850-
else
858+
859+
if (kill(pid, SIGTERM))
851860
{
852-
SWSS_LOG_NOTICE("Failed to remove non-existent port channel %s pid...", alias.c_str());
861+
SWSS_LOG_ERROR("Failed to send SIGTERM to port channel %s pid %d: %s", alias.c_str(), pid, strerror(errno));
853862
return false;
854863
}
855-
}
856-
857-
if (kill(pid, SIGTERM))
858-
{
859-
SWSS_LOG_ERROR("Failed to send SIGTERM to port channel %s pid %d: %s", alias.c_str(), pid, strerror(errno));
860-
return false;
861-
}
862864

863-
SWSS_LOG_NOTICE("Stop port channel %s", alias.c_str());
865+
SWSS_LOG_NOTICE("Stop port channel %s", alias.c_str());
864866
}
865867

866868
return true;
@@ -942,74 +944,71 @@ task_process_status TeamMgr::addLagMember(const string &lag, const string &membe
942944
cmd.clear();
943945

944946
if (m_teamdUnifiedProcMode) {
945-
cmd << IP_CMD << " link set dev " << shellquote(member) << " down";
946-
if (exec(cmd.str(), res) != 0)
947-
{
948-
SWSS_LOG_WARN("Failed to bring down port %s before IPC LAG add", member.c_str());
949-
return task_failed;
950-
}
951-
std::string portConfig = "{\"lacp_key\":" + std::to_string(keyId) +
952-
",\"link_watch\": {\"name\": \"ethtool\"} }";
947+
cmd << IP_CMD << " link set dev " << shellquote(member) << " down";
948+
if (exec(cmd.str(), res) != 0)
949+
{
950+
SWSS_LOG_WARN("Failed to bring down port %s before IPC LAG add", member.c_str());
951+
return task_failed;
952+
}
953+
std::string portConfig = "{\"lacp_key\":" + std::to_string(keyId) +
954+
",\"link_watch\": {\"name\": \"ethtool\"} }";
953955

954-
if (sendIpcToTeamd("PortConfigUpdate", { lag, member, portConfig }) != 0)
955-
{
956-
SWSS_LOG_ERROR("IPC: Failed to send PortConfigUpdate for %s in %s", member.c_str(), lag.c_str());
957-
return task_need_retry;
958-
}
959-
960-
SWSS_LOG_INFO("IPC: Sent PortConfigUpdate for %s to port channel %s", member.c_str(), lag.c_str());
961-
962-
// Step 2: Send PortAdd via IPC.
963-
if (sendIpcToTeamd("PortAdd", { lag, member }) != 0)
964-
{
965-
if (checkPortIffUp(member))
966-
{
967-
SWSS_LOG_INFO("IPC: Failed to add %s to port channel %s, %s .", member.c_str(), lag.c_str(), checkPortIffUp(member)?"retry..":"");
968-
return task_need_retry;
969-
}
970-
else
971-
{
972-
SWSS_LOG_ERROR("IPC: Failed to add %s to port channel %s", member.c_str(), lag.c_str());
973-
return task_failed;
974-
}
975-
}
976-
977-
SWSS_LOG_NOTICE("IPC: Successfully added %s to port channel %s", member.c_str(), lag.c_str());
978-
}
956+
if (sendIpcToTeamd("PortConfigUpdate", { lag, member, portConfig }) != 0)
957+
{
958+
SWSS_LOG_ERROR("IPC: Failed to send PortConfigUpdate for %s in %s", member.c_str(), lag.c_str());
959+
return task_need_retry;
960+
}
961+
962+
SWSS_LOG_INFO("IPC: Sent PortConfigUpdate for %s to port channel %s", member.c_str(), lag.c_str());
979963

980-
else {
981-
// Set admin down LAG member (required by teamd) and enslave it
982-
// ip link set dev <member> down;
983-
// teamdctl <port_channel_name> port config update <member> { "lacp_key": <lacp_key>, "link_watch": { "name": "ethtool" } };
984-
// teamdctl <port_channel_name> port add <member>;
985-
cmd << IP_CMD << " link set dev " << shellquote(member) << " down; ";
986-
cmd << TEAMDCTL_CMD << " " << shellquote(lag) << " port config update " << shellquote(member)
987-
<< " '{\"lacp_key\":"
988-
<< keyId
989-
<< ",\"link_watch\": {\"name\": \"ethtool\"} }'; ";
990-
SWSS_LOG_NOTICE("Add port update first %s to port channel %s", member.c_str(), lag.c_str());
991-
cmd << TEAMDCTL_CMD << " " << shellquote(lag) << " port add " << shellquote(member);
992-
if (exec(cmd.str(), res) != 0)
993-
{
994-
// teamdctl port add command will fail when the member port is not
995-
// set to admin status down; it is possible that some other processes
996-
// or users (e.g. portmgrd) are executing the command to bring up the
997-
// member port while adding this port into the port channel. This piece
998-
// of code will check if the port is set to admin status up. If yes,
999-
// it will retry to add the port into the port channel.
1000-
if (checkPortIffUp(member))
1001-
{
1002-
SWSS_LOG_INFO("Failed to add %s to port channel %s, retry...",
1003-
member.c_str(), lag.c_str());
1004-
return task_need_retry;
1005-
}
1006-
else
1007-
{
1008-
SWSS_LOG_ERROR("Failed to add %s to port channel %s",
1009-
member.c_str(), lag.c_str());
1010-
return task_failed;
1011-
}
1012-
}
964+
// Step 2: Send PortAdd via IPC.
965+
if (sendIpcToTeamd("PortAdd", { lag, member }) != 0)
966+
{
967+
if (checkPortIffUp(member))
968+
{
969+
SWSS_LOG_INFO("IPC: Failed to add %s to port channel %s, %s .", member.c_str(), lag.c_str(), checkPortIffUp(member)?"retry..":"");
970+
return task_need_retry;
971+
}
972+
else
973+
{
974+
SWSS_LOG_ERROR("IPC: Failed to add %s to port channel %s", member.c_str(), lag.c_str());
975+
return task_failed;
976+
}
977+
}
978+
SWSS_LOG_NOTICE("IPC: Successfully added %s to port channel %s", member.c_str(), lag.c_str());
979+
}
980+
else
981+
{
982+
// Set admin down LAG member (required by teamd) and enslave it
983+
// ip link set dev <member> down;
984+
// teamdctl <port_channel_name> port config update <member> { "lacp_key": <lacp_key>, "link_watch": { "name": "ethtool" } };
985+
// teamdctl <port_channel_name> port add <member>;
986+
cmd << IP_CMD << " link set dev " << shellquote(member) << " down; ";
987+
cmd << TEAMDCTL_CMD << " " << shellquote(lag) << " port config update " << shellquote(member)
988+
<< " '{\"lacp_key\":" << keyId << ",\"link_watch\": {\"name\": \"ethtool\"} }'; ";
989+
SWSS_LOG_NOTICE("Add port update first %s to port channel %s", member.c_str(), lag.c_str());
990+
cmd << TEAMDCTL_CMD << " " << shellquote(lag) << " port add " << shellquote(member);
991+
if (exec(cmd.str(), res) != 0)
992+
{
993+
// teamdctl port add command will fail when the member port is not
994+
// set to admin status down; it is possible that some other processes
995+
// or users (e.g. portmgrd) are executing the command to bring up the
996+
// member port while adding this port into the port channel. This piece
997+
// of code will check if the port is set to admin status up. If yes,
998+
// it will retry to add the port into the port channel.
999+
if (checkPortIffUp(member))
1000+
{
1001+
SWSS_LOG_INFO("Failed to add %s to port channel %s, retry...",
1002+
member.c_str(), lag.c_str());
1003+
return task_need_retry;
1004+
}
1005+
else
1006+
{
1007+
SWSS_LOG_ERROR("Failed to add %s to port channel %s",
1008+
member.c_str(), lag.c_str());
1009+
return task_failed;
1010+
}
1011+
}
10131012
}
10141013

10151014
vector<FieldValueTuple> fvs;
@@ -1063,12 +1062,12 @@ bool TeamMgr::removeLagMember(const string &lag, const string &member)
10631062
stringstream cmd;
10641063
string res;
10651064
if (m_teamdUnifiedProcMode) {
1066-
sendIpcToTeamd("PortRemove", { lag, member });
1065+
sendIpcToTeamd("PortRemove", { lag, member });
10671066
}
1068-
else {
1069-
// teamdctl <port_channel_name> port remove <member>;
1070-
cmd << TEAMDCTL_CMD << " " << lag << " port remove " << member << "; ";
1071-
1067+
else
1068+
{
1069+
// teamdctl <port_channel_name> port remove <member>;
1070+
cmd << TEAMDCTL_CMD << " " << lag << " port remove " << member << "; ";
10721071
}
10731072

10741073
vector<FieldValueTuple> fvs;

0 commit comments

Comments
 (0)