diff --git a/cfgmgr/intfmgr.cpp b/cfgmgr/intfmgr.cpp index f0f4ec97fa..bdfd8ce46c 100644 --- a/cfgmgr/intfmgr.cpp +++ b/cfgmgr/intfmgr.cpp @@ -491,13 +491,24 @@ void IntfMgr::updateSubIntfAdminStatus(const string &alias, const string &admin) std::string IntfMgr::setHostSubIntfAdminStatus(const string &alias, const string &admin_status, const string &parent_admin_status) { stringstream cmd; - string res; + string res, cmd_str; if (parent_admin_status == "up" || admin_status == "down") { SWSS_LOG_INFO("subintf %s admin_status: %s", alias.c_str(), admin_status.c_str()); cmd << IP_CMD " link set " << shellquote(alias) << " " << shellquote(admin_status); - EXEC_WITH_ERROR_THROW(cmd.str(), res); + cmd_str = cmd.str(); + int ret = swss::exec(cmd_str, res); + if (ret && !isIntfStateOk(alias)) + { + // Can happen when a DEL notification is sent by portmgrd immediately followed by a new SET notification + SWSS_LOG_WARN("Setting admin_status to %s netdev failed with cmd:%s, rc:%d, error:%s", + alias.c_str(), cmd_str.c_str(), ret, res.c_str()); + } + else if (ret) + { + throw runtime_error(cmd_str + " : " + res); + } return admin_status; } else diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am index 685a30b53a..e0b429d9c7 100644 --- a/tests/mock_tests/Makefile.am +++ b/tests/mock_tests/Makefile.am @@ -153,7 +153,7 @@ tests_portsyncd_LDADD = $(LDADD_GTEST) -lnl-genl-3 -lhiredis -lhiredis \ ## intfmgrd unit tests -tests_intfmgrd_SOURCES = intfmgrd/add_ipv6_prefix_ut.cpp \ +tests_intfmgrd_SOURCES = intfmgrd/intfmgr_ut.cpp \ $(top_srcdir)/cfgmgr/intfmgr.cpp \ $(top_srcdir)/lib/subintf.cpp \ $(top_srcdir)/orchagent/orch.cpp \ diff --git a/tests/mock_tests/intfmgrd/add_ipv6_prefix_ut.cpp b/tests/mock_tests/intfmgrd/intfmgr_ut.cpp similarity index 76% rename from tests/mock_tests/intfmgrd/add_ipv6_prefix_ut.cpp rename to tests/mock_tests/intfmgrd/intfmgr_ut.cpp index 2ed1a1b6af..ef43cdeb6b 100644 --- a/tests/mock_tests/intfmgrd/add_ipv6_prefix_ut.cpp +++ b/tests/mock_tests/intfmgrd/intfmgr_ut.cpp @@ -1,6 +1,6 @@ #include "gtest/gtest.h" #include -#include +#include #include #include #include "../mock_table.h" @@ -20,6 +20,9 @@ int cb(const std::string &cmd, std::string &stdout){ else if (cmd.find("/sbin/ip -6 address \"add\"") == 0) { return Ethernet0IPv6Set ? 0 : 2; } + else if (cmd == "/sbin/ip link set \"Ethernet64.10\" \"up\""){ + return 1; + } else { return 0; } @@ -27,7 +30,7 @@ int cb(const std::string &cmd, std::string &stdout){ } // Test Fixture -namespace add_ipv6_prefix_ut +namespace intfmgr_ut { struct IntfMgrTest : public ::testing::Test { @@ -35,14 +38,14 @@ namespace add_ipv6_prefix_ut std::shared_ptr m_app_db; std::shared_ptr m_state_db; std::vector cfg_intf_tables; - + virtual void SetUp() override - { + { testing_db::reset(); m_config_db = std::make_shared("CONFIG_DB", 0); m_app_db = std::make_shared("APPL_DB", 0); m_state_db = std::make_shared("STATE_DB", 0); - + swss::WarmStart::initialize("intfmgrd", "swss"); std::vector tables = { @@ -106,4 +109,22 @@ namespace add_ipv6_prefix_ut } ASSERT_EQ(ip_cmd_called, 1); } + + //This test except no runtime error when the set admin status command failed + //and the subinterface has not ok status (for example not existing subinterface) + TEST_F(IntfMgrTest, testSetAdminStatusFailToNotOkSubInt){ + swss::IntfMgr intfmgr(m_config_db.get(), m_app_db.get(), m_state_db.get(), cfg_intf_tables); + intfmgr.setHostSubIntfAdminStatus("Ethernet64.10", "up", "up"); + } + + //This test except runtime error when the set admin status command failed + //and the subinterface has ok status + TEST_F(IntfMgrTest, testSetAdminStatusFailToOkSubInt){ + swss::IntfMgr intfmgr(m_config_db.get(), m_app_db.get(), m_state_db.get(), cfg_intf_tables); + /* Set portStateTable */ + std::vector values; + values.emplace_back("state", "ok"); + intfmgr.m_statePortTable.set("Ethernet64.10", values, "SET", ""); + EXPECT_THROW(intfmgr.setHostSubIntfAdminStatus("Ethernet64.10", "up", "up"), std::runtime_error); + } }