Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed set admin_status for deleted subintf due to late notification #2659

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions cfgmgr/intfmgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/mock_tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include "gtest/gtest.h"
#include <iostream>
#include <fstream>
#include <fstream>
#include <unistd.h>
#include <sys/stat.h>
#include "../mock_table.h"
Expand All @@ -20,29 +20,32 @@ 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;
}
return 0;
}

// Test Fixture
namespace add_ipv6_prefix_ut
namespace intfmgr_ut
{
struct IntfMgrTest : public ::testing::Test
{
std::shared_ptr<swss::DBConnector> m_config_db;
std::shared_ptr<swss::DBConnector> m_app_db;
std::shared_ptr<swss::DBConnector> m_state_db;
std::vector<std::string> cfg_intf_tables;

virtual void SetUp() override
{
{
testing_db::reset();
m_config_db = std::make_shared<swss::DBConnector>("CONFIG_DB", 0);
m_app_db = std::make_shared<swss::DBConnector>("APPL_DB", 0);
m_state_db = std::make_shared<swss::DBConnector>("STATE_DB", 0);

swss::WarmStart::initialize("intfmgrd", "swss");

std::vector<std::string> tables = {
Expand Down Expand Up @@ -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<swss::FieldValueTuple> 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);
}
}