Merge branch 'master' into ntt_fpmsyncd_enhanced

Author: dgsudharsan

.gitignore

@@ -83,6 +83,8 @@ tests/mock_tests/tests_portsyncd
 
 # Test Files #
 ##############
+*gcda
+*gcno
 tests/log
 tests/mock_tests/test-suite.log
 tests/mock_tests/tests.log
@@ -92,5 +94,3 @@ tests/tests.log
 tests/tests.trs
 tests/mock_tests/**/*log
 tests/mock_tests/**/*trs
-orchagent/p4orch/tests/**/*gcda
-orchagent/p4orch/tests/**/*gcno

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;
     }
 

cfgmgr/vlanmgr.cpp

@@ -83,7 +83,11 @@ VlanMgr::VlanMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c
     //               /sbin/ip link del dummy 2>/dev/null;
     //               /sbin/ip link add dummy type dummy &&
     //               /sbin/ip link set dummy master Bridge &&
-    //               /sbin/ip link set dummy up"
+    //               /sbin/ip link set dummy up;
+    //               /sbin/ip link set Bridge down &&
+    //               /sbin/ip link set Bridge up"
+    // Note: We shutdown and start-up the Bridge at the end to ensure that its
+    //       link-local IPv6 address matches its MAC address.
 
     const std::string cmds = std::string("")
       + BASH_CMD + " -c \""
@@ -95,7 +99,9 @@ VlanMgr::VlanMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c
       + IP_CMD + " link del dev dummy 2>/dev/null; "
       + IP_CMD + " link add dummy type dummy && "
       + IP_CMD + " link set dummy master " + DOT1Q_BRIDGE_NAME + " && "
-      + IP_CMD + " link set dummy up" + "\"";
+      + IP_CMD + " link set dummy up; "
+      + IP_CMD + " link set " + DOT1Q_BRIDGE_NAME + " down && "
+      + IP_CMD + " link set " + DOT1Q_BRIDGE_NAME + " up\"";
 
     std::string res;
     EXEC_WITH_ERROR_THROW(cmds, res);
@@ -193,15 +199,34 @@ bool VlanMgr::setHostVlanMac(int vlan_id, const string &mac)
 {
     SWSS_LOG_ENTER();
 
+    std::string res;
+
+    /*
+     * Bring down the bridge before changing MAC addresses of the bridge and the VLAN interface.
+     * This is done so that the IPv6 link-local addresses of the bridge and the VLAN interface
+     * are updated after MAC change.
+     * /sbin/ip link set Bridge down
+     */
+    string bridge_down(IP_CMD " link set " DOT1Q_BRIDGE_NAME " down");
+    EXEC_WITH_ERROR_THROW(bridge_down, res);
+
     // The command should be generated as:
-    // /sbin/ip link set Vlan{{vlan_id}} address {{mac}}
+    // /sbin/ip link set Vlan{{vlan_id}} address {{mac}} &&
+    // /sbin/ip link set Bridge address {{mac}}
     ostringstream cmds;
     cmds << IP_CMD " link set " VLAN_PREFIX + std::to_string(vlan_id) + " address " << shellquote(mac) << " && "
             IP_CMD " link set " DOT1Q_BRIDGE_NAME " address " << shellquote(mac);
-
-    std::string res;
+    res.clear();
     EXEC_WITH_ERROR_THROW(cmds.str(), res);
 
+    /*
+     * Start up the bridge again.
+     * /sbin/ip link set Bridge up
+     */
+    string bridge_up(IP_CMD " link set " DOT1Q_BRIDGE_NAME " up");
+    res.clear();
+    EXEC_WITH_ERROR_THROW(bridge_up, res);
+
     return true;
 }
 

debian/rules

@@ -44,6 +44,7 @@ override_dh_auto_install:
 ifeq ($(ENABLE_GCOV), y)
 	mkdir -p debian/swss/tmp/gcov
 	lcov -c --directory . --no-external --exclude "$(shell pwd)/tests/*" --exclude "$(shell pwd)/**/tests/*" --ignore-errors gcov --output-file coverage.info
+	lcov --add-tracefile coverage.info -o coverage.info
 	lcov_cobertura coverage.info -o coverage.xml
 	find ./ -type f -regex '.*\.\(h\|cpp\|gcno\|info\)' | tar -cf debian/swss/tmp/gcov/gcov-source.tar -T -
 endif

orchagent/Makefile.am

@@ -138,7 +138,7 @@ orchagent_SOURCES += p4orch/p4orch.cpp \
 
 orchagent_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN)
 orchagent_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) $(CFLAGS_ASAN)
-orchagent_LDADD = $(LDFLAGS_ASAN) -lnl-3 -lnl-route-3 -lpthread -lsairedis -lsaimeta -lsaimetadata -lswsscommon -lzmq -lprotobuf -ldashapi
+orchagent_LDADD = $(LDFLAGS_ASAN) -lnl-3 -lnl-route-3 -lpthread -lsairedis -lsaimeta -lsaimetadata -lswsscommon -lzmq -lprotobuf -ldashapi -ljemalloc
 
 routeresync_SOURCES = routeresync.cpp
 routeresync_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_ASAN)

orchagent/aclorch.cpp

@@ -11,6 +11,7 @@
 #include "timer.h"
 #include "crmorch.h"
 #include "sai_serialize.h"
+#include "directory.h"
 
 using namespace std;
 using namespace swss;
@@ -30,6 +31,7 @@ extern PortsOrch*        gPortsOrch;
 extern CrmOrch *gCrmOrch;
 extern SwitchOrch *gSwitchOrch;
 extern string gMySwitchType;
+extern Directory<Orch*> gDirectory;
 
 #define MIN_VLAN_ID 1    // 0 is a reserved VLAN ID
 #define MAX_VLAN_ID 4095 // 4096 is a reserved VLAN ID
@@ -617,6 +619,35 @@ bool AclTableRangeMatch::validateAclRuleMatch(const AclRule& rule) const
     return true;
 }
 
+void AclRule::TunnelNH::load(const std::string& target)
+{
+    parse(target);
+
+    VxlanTunnelOrch* vxlan_orch = gDirectory.get<VxlanTunnelOrch*>();
+    /* Only the first call creates the SAI object, further calls just increment the ref count */
+    oid = vxlan_orch->createNextHopTunnel(tunnel_name, endpoint_ip, mac, vni);
+}
+
+void AclRule::TunnelNH::parse(const std::string& target)
+{
+    /* Supported Format: endpoint_ip@tunnel_name */
+    auto at_pos = target.find('@');
+    if (at_pos == std::string::npos)
+    {
+        throw std::logic_error("Invalid format for Tunnel Next Hop");
+    }
+
+    endpoint_ip = swss::IpAddress(target.substr(0, at_pos));
+    tunnel_name = target.substr(at_pos + 1);
+}
+
+void AclRule::TunnelNH::clear()
+{
+    oid = SAI_NULL_OBJECT_ID;
+    VxlanTunnelOrch* vxlan_orch = gDirectory.get<VxlanTunnelOrch*>();
+    vxlan_orch->removeNextHopTunnel(tunnel_name, endpoint_ip, mac, vni);
+}
+
 string AclTableType::getName() const
 {
     return m_name;
@@ -1308,7 +1339,17 @@ void AclRule::decreaseNextHopRefCount()
         }
         m_redirect_target_next_hop_group.clear();
     }
-
+    if (m_redirect_target_tun_nh.oid != SAI_NULL_OBJECT_ID)
+    {
+        try
+        {
+            m_redirect_target_tun_nh.clear();
+        }
+        catch (const std::runtime_error& e)
+        {
+            SWSS_LOG_ERROR("Failed to remove tunnel nh reference %s, ACL Rule: %s", e.what(), m_id.c_str());
+        }
+    }
     return;
 }
 
@@ -2001,21 +2042,38 @@ sai_object_id_t AclRulePacket::getRedirectObjectId(const string& redirect_value)
     try
     {
         NextHopKey nh(target);
-        if (!m_pAclOrch->m_neighOrch->hasNextHop(nh))
+        if (m_pAclOrch->m_neighOrch->hasNextHop(nh))
         {
-            SWSS_LOG_ERROR("ACL Redirect action target next hop ip: '%s' doesn't exist on the switch", nh.to_string().c_str());
-            return SAI_NULL_OBJECT_ID;
+            m_redirect_target_next_hop = target;
+            m_pAclOrch->m_neighOrch->increaseNextHopRefCount(nh);
+            return m_pAclOrch->m_neighOrch->getNextHopId(nh);
         }
-
-        m_redirect_target_next_hop = target;
-        m_pAclOrch->m_neighOrch->increaseNextHopRefCount(nh);
-        return m_pAclOrch->m_neighOrch->getNextHopId(nh);
     }
     catch (...)
     {
         // no error, just try next variant
     }
 
+    // Try to parse if this is a tunnel nexthop.
+    try
+    {
+        m_redirect_target_tun_nh.load(target);
+        if (SAI_NULL_OBJECT_ID != m_redirect_target_tun_nh.oid)
+        {
+            SWSS_LOG_INFO("Tunnel Next Hop Found: oid:0x%" PRIx64 ", target: %s", m_redirect_target_tun_nh.oid, target.c_str());
+            return m_redirect_target_tun_nh.oid;
+        }
+    }
+    catch (std::logic_error& e)
+    {
+        // no error, just try next variant
+    }
+    catch (const std::runtime_error& e)
+    {
+        SWSS_LOG_ERROR("Failed to create/fetch tunnel next hop, %s, err: %s", target.c_str(), e.what());
+        return SAI_NULL_OBJECT_ID;
+    }
+
     // try to parse nh group the set of <ip address, interface name>
     try
     {