Add a delay between killing teamd processes (sonic-net#3325)

* Add a delay between killing teamd processes

When killing 10 or more teamd processes, add a delay of 0.1 seconds
after every 10 kill signals/proceses. This is because in the LAG scale
tests (in `ecmp/inner_hashing/test_inner_hashing_lag.py` in sonic-mgmt),
it may create 100 LAGs, and when destroying them all, some of those LAGs
may fail to be properly destroyed, leaving some stale port channels
around. This seems to be because the netlink socket buffers on which the
teamd processes get notifications become full with events of the other
port channels/interfaces going down.

As a workaround, add some delays in killing the teamd processes, so that
the netlink buffers don't become full, causing messages to get dropped.

This delay was randomly chosen, and it seems to work well with 100 LAGs
on a KVM. It can probably made to be a bit more aggressive if needed
(i.e. maybe 0.05 seconds every 20 processes).

Author: saiarcot895

cfgmgr/teammgr.cpp

@@ -16,6 +16,8 @@
 #include <net/if.h>
 #include <sys/ioctl.h>
 #include <sys/stat.h>
+#include <sys/wait.h>
+#include <sys/types.h>
 #include <signal.h>
 
 
@@ -171,18 +173,29 @@ void TeamMgr::cleanTeamProcesses()
     SWSS_LOG_ENTER();
     SWSS_LOG_NOTICE("Cleaning up LAGs during shutdown...");
 
-    std::unordered_map<std::string, pid_t> aliasPidMap;
+    std::unordered_map<std::string, int> aliasPidMap;
 
     for (const auto& alias: m_lagList)
     {
-        std::string res;
         pid_t pid;
+        // Sleep for 10 milliseconds so as to not overwhelm the netlink
+        // socket buffers with events about interfaces going down
+        std::this_thread::sleep_for(std::chrono::milliseconds(10));
 
         try
         {
-            std::stringstream cmd;
-            cmd << "cat " << shellquote("/var/run/teamd/" + alias + ".pid");
-            EXEC_WITH_ERROR_THROW(cmd.str(), res);
+            ifstream pidFile("/var/run/teamd/" + alias + ".pid");
+            if (pidFile.is_open())
+            {
+                pidFile >> pid;
+                aliasPidMap[alias] = pid;
+                SWSS_LOG_INFO("Read port channel %s pid %d", alias.c_str(), pid);
+            }
+            else
+            {
+                SWSS_LOG_NOTICE("Unable to read pid file for %s, skipping...", alias.c_str());
+                continue;
+            }
         }
         catch (const std::exception &e)
         {
@@ -191,46 +204,28 @@ void TeamMgr::cleanTeamProcesses()
             continue;
         }
 
-        try
-        {
-            pid = static_cast<pid_t>(std::stoul(res, nullptr, 10));
-            aliasPidMap[alias] = pid;
-
-            SWSS_LOG_INFO("Read port channel %s pid %d", alias.c_str(), pid);
-        }
-        catch (const std::exception &e)
+        if (kill(pid, SIGTERM))
         {
-            SWSS_LOG_ERROR("Failed to read port channel %s pid: %s", alias.c_str(), e.what());
-            continue;
+            SWSS_LOG_ERROR("Failed to send SIGTERM to port channel %s pid %d: %s", alias.c_str(), pid, strerror(errno));
+            aliasPidMap.erase(alias);
         }
-
-        try
+        else
         {
-            std::stringstream cmd;
-            cmd << "kill -TERM " << pid;
-            EXEC_WITH_ERROR_THROW(cmd.str(), res);
-
             SWSS_LOG_NOTICE("Sent SIGTERM to port channel %s pid %d", alias.c_str(), pid);
         }
-        catch (const std::exception &e)
-        {
-            SWSS_LOG_ERROR("Failed to send SIGTERM to port channel %s pid %d: %s", alias.c_str(), pid, e.what());
-            aliasPidMap.erase(alias);
-        }
     }
 
     for (const auto& cit: aliasPidMap)
     {
         const auto &alias = cit.first;
         const auto &pid = cit.second;
 
-        std::stringstream cmd;
-        std::string res;
-
         SWSS_LOG_NOTICE("Waiting for port channel %s pid %d to stop...", alias.c_str(), pid);
 
-        cmd << "tail -f --pid=" << pid << " /dev/null";
-        EXEC_WITH_ERROR_THROW(cmd.str(), res);
+        while (!kill(pid, 0))
+        {
+            std::this_thread::sleep_for(std::chrono::milliseconds(10));
+        }
     }
 
     SWSS_LOG_NOTICE("LAGs cleanup is done");
@@ -658,42 +653,25 @@ bool TeamMgr::removeLag(const string &alias)
 {
     SWSS_LOG_ENTER();
 
-    stringstream cmd;
-    string res;
     pid_t pid;
 
-    try
-    {
-        std::stringstream cmd;
-        cmd << "cat " << shellquote("/var/run/teamd/" + alias + ".pid");
-        EXEC_WITH_ERROR_THROW(cmd.str(), res);
-    }
-    catch (const std::exception &e)
     {
-        SWSS_LOG_NOTICE("Failed to remove non-existent port channel %s pid...", alias.c_str());
-        return false;
-    }
-
-    try
-    {
-        pid = static_cast<pid_t>(std::stoul(res, nullptr, 10));
-        SWSS_LOG_INFO("Read port channel %s pid %d", alias.c_str(), pid);
-    }
-    catch (const std::exception &e)
-    {
-        SWSS_LOG_ERROR("Failed to read port channel %s pid: %s", alias.c_str(), e.what());
-        return false;
+        ifstream pidfile("/var/run/teamd/" + alias + ".pid");
+        if (pidfile.is_open())
+        {
+            pidfile >> pid;
+            SWSS_LOG_INFO("Read port channel %s pid %d", alias.c_str(), pid);
+        }
+        else
+        {
+            SWSS_LOG_NOTICE("Failed to remove non-existent port channel %s pid...", alias.c_str());
+            return false;
+        }
     }
 
-    try
-    {
-        std::stringstream cmd;
-        cmd << "kill -TERM " << pid;
-        EXEC_WITH_ERROR_THROW(cmd.str(), res);
-    }
-    catch (const std::exception &e)
+    if (kill(pid, SIGTERM))
     {
-        SWSS_LOG_ERROR("Failed to send SIGTERM to port channel %s pid %d: %s", alias.c_str(), pid, e.what());
+        SWSS_LOG_ERROR("Failed to send SIGTERM to port channel %s pid %d: %s", alias.c_str(), pid, strerror(errno));
         return false;
     }
 

tests/mock_tests/Makefile.am

@@ -227,8 +227,8 @@ tests_teammgrd_SOURCES = teammgrd/teammgr_ut.cpp \
 tests_teammgrd_INCLUDES = $(tests_INCLUDES) -I$(top_srcdir)/cfgmgr -I$(top_srcdir)/lib
 tests_teammgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI)
 tests_teammgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) $(tests_teammgrd_INCLUDES)
-tests_teammgrd_LDADD = $(LDADD_GTEST) $(LDADD_SAI) -lnl-genl-3 -lhiredis -lhiredis \
-        -lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3 -lpthread -lgmock -lgmock_main
+tests_teammgrd_LDADD = $(LDADD_GTEST) $(LDADD_SAI) -ldl -lhiredis \
+        -lswsscommon -lgtest -lgtest_main -lzmq -lpthread -lgmock -lgmock_main
 
 ## fpmsyncd unit tests