Skip to content

Commit

Permalink
[action] [PR:3439] Remove update of mgmt oper status in swss
Browse files Browse the repository at this point in the history 
What I did
Issue to be fix: Currently operational status of mgmt interface is not present or correct for multi-asic devices.

Why I did it
Initial PR that added mgmt oper status feature in swss: sonic-net/sonic-swss#630
sonic-net/sonic-buildimage#21245 adds a script to update oper status of management interface periodically. In doing so, we no longer need to update oper status of mgmt interface in swss.

How I verified it
Verified on single-asic platform

verified on single-asic platform and multi-asic Chassis platfrom.
Single ASIC Arista device verification along with Add script to periodically update oper status of management interface sonic-net/sonic-buildimage#21245 changes
Ran the below bash script to verify the state of STATE_DB: MGMT_OPER_STATUS table and also execute config_reload, verify if STATE_DB is flushed out and repopulated after monit starts periodic script.
#!/bin/bash
CUR_STATUS=`sonic-db-cli STATE_DB hgetall "MGMT_PORT_TABLE|eth0"`
echo "current status in STATE_DB is $CUR_STATUS :: expected to have up state"
snmp_result=`docker exec snmp snmpwalk -v2c -c <comm> <IP> 1.3.6.1.2.1.2.2.1.8.10000`
echo "snmp result  before config reload 5min $snmp_result :: expected to have 1 in snmp result"
sudo config reload -y
CUR_STATUS=`sonic-db-cli STATE_DB hgetall "MGMT_PORT_TABLE|eth0"`
echo "current status in STATE_DB after config_reload is $CUR_STATUS :: expected to have empty"
# sleep for snmp to start
sleep 60
snmp_result=`docker exec snmp snmpwalk -v2c -c <comm> <IP> 1.3.6.1.2.1.2.2.1.8.10000`
echo "snmp result  after config reload $snmp_result :: expected to return error since STATE_DB is not yet populated"
# monit will populate mgmt oper status after each 5min cycle
sleep 240
CUR_STATUS=`sonic-db-cli STATE_DB hgetall "MGMT_PORT_TABLE|eth0"`
echo "current status in STATE_DB after config_reload after 5min $CUR_STATUS :: expected to have up state"
snmp_result=`docker exec snmp snmpwalk -v2c -c <comm> <IP> 1.3.6.1.2.1.2.2.1.8.10000`
echo "snmp result  after config reload after 5min $snmp_result :: expected to have 1 in snmp result"
Result of above script:

current status in STATE_DB is {'oper_status': 'up'} :: expected to have up state
snmp result  before config reload 5min iso.3.6.1.2.1.2.2.1.8.10000 = INTEGER: 1 :: expected to have 1 in snmp result
Acquired lock on /etc/sonic/reload.lock
Disabling container and routeCheck monitoring ...
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -j /etc/sonic/init_cfg.json -j /etc/sonic/config_db.json --write-to-db
Running command: /usr/local/bin/db_migrator.py -o migrate
Running command: /usr/local/bin/sonic-cfggen -d -y /etc/sonic/sonic_version.yml -t /usr/share/sonic/templates/sonic-environment.j2,/etc/sonic/sonic-environment
Restarting SONiC target ...
Enabling container and routeCheck monitoring ...
Reloading Monit configuration ...
Reinitializing monit daemon
Released lock on /etc/sonic/reload.lock
current status in STATE_DB after config_reload is {} :: expected to have empty
snmp result  after config reload iso.3.6.1.2.1.2.2.1.8.10000 = No Such Object available on this agent at this OID :: expected to return error since STATE_DB is not yet populated
current status in STATE_DB after config_reload after 5min {'oper_status': 'up'} :: expected to have up state
snmp result  after config reload after 5min iso.3.6.1.2.1.2.2.1.8.10000 = INTEGER: 1 :: expected to have 1 in snmp result
  • Loading branch information
@arlakshm
arlakshm authored Jan 10, 2025
2 parents 2c12aab + e82c77f commit 9f336b1
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 97 deletions.
63 changes: 2 additions & 61 deletions portsyncd/linksync.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ using namespace swss;
#define VLAN_DRV_NAME "bridge"
#define TEAM_DRV_NAME "team"

const string MGMT_PREFIX = "eth";
const string INTFS_PREFIX = "Ethernet";
const string LAG_PREFIX = "PortChannel";

Expand All @@ -38,57 +37,11 @@ extern string g_switchType;
LinkSync::LinkSync(DBConnector *appl_db, DBConnector *state_db) :
m_portTableProducer(appl_db, APP_PORT_TABLE_NAME),
m_portTable(appl_db, APP_PORT_TABLE_NAME),
m_statePortTable(state_db, STATE_PORT_TABLE_NAME),
m_stateMgmtPortTable(state_db, STATE_MGMT_PORT_TABLE_NAME)
m_statePortTable(state_db, STATE_PORT_TABLE_NAME)
{
std::shared_ptr<struct if_nameindex> if_ni(if_nameindex(), if_freenameindex);
struct if_nameindex *idx_p;

for (idx_p = if_ni.get();
idx_p != NULL && idx_p->if_index != 0 && idx_p->if_name != NULL;
idx_p++)
{
string key = idx_p->if_name;

/* Explicitly store management ports oper status into the state database.
* This piece of information is used by SNMP. */
if (!key.compare(0, MGMT_PREFIX.length(), MGMT_PREFIX))
{
ostringstream cmd;
string res;
cmd << "cat /sys/class/net/" << shellquote(key) << "/operstate";
try
{
EXEC_WITH_ERROR_THROW(cmd.str(), res);
}
catch (...)
{
SWSS_LOG_WARN("Failed to get %s oper status", key.c_str());
continue;
}

/* Remove the trailing newline */
if (res.length() >= 1 && res.at(res.length() - 1) == '\n')
{
res.erase(res.length() - 1);
/* The value of operstate will be either up or down */
if (res != "up" && res != "down")
{
SWSS_LOG_WARN("Unknown %s oper status %s",
key.c_str(), res.c_str());
}
FieldValueTuple fv("oper_status", res);
vector<FieldValueTuple> fvs;
fvs.push_back(fv);

m_stateMgmtPortTable.set(key, fvs);
SWSS_LOG_INFO("Store %s oper status %s to state DB",
key.c_str(), res.c_str());
}
continue;
}
}

if (!WarmStart::isWarmStart())
{
/* See the comments for g_portSet in portsyncd.cpp */
Expand Down Expand Up @@ -168,8 +121,7 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
string key = rtnl_link_get_name(link);

if (key.compare(0, INTFS_PREFIX.length(), INTFS_PREFIX) &&
key.compare(0, LAG_PREFIX.length(), LAG_PREFIX) &&
key.compare(0, MGMT_PREFIX.length(), MGMT_PREFIX))
key.compare(0, LAG_PREFIX.length(), LAG_PREFIX))
{
return;
}
Expand Down Expand Up @@ -197,17 +149,6 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
nlmsg_type, key.c_str(), admin, oper, addrStr, ifindex, master);
}

if (!key.compare(0, MGMT_PREFIX.length(), MGMT_PREFIX))
{
FieldValueTuple fv("oper_status", oper ? "up" : "down");
vector<FieldValueTuple> fvs;
fvs.push_back(fv);
m_stateMgmtPortTable.set(key, fvs);
SWSS_LOG_INFO("Store %s oper status %s to state DB",
key.c_str(), oper ? "up" : "down");
return;
}

/* teamd instances are dealt in teamsyncd */
if (type && !strcmp(type, TEAM_DRV_NAME))
{
Expand Down
2 changes: 1 addition & 1 deletion portsyncd/linksync.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class LinkSync : public NetMsg

private:
ProducerStateTable m_portTableProducer;
Table m_portTable, m_statePortTable, m_stateMgmtPortTable;
Table m_portTable, m_statePortTable;

std::map<unsigned int, std::string> m_ifindexNameMap;
std::map<unsigned int, std::string> m_ifindexOldNameMap;
Expand Down
35 changes: 0 additions & 35 deletions tests/mock_tests/portsyncd/portsyncd_ut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,18 +187,6 @@ namespace portsyncd_ut

namespace portsyncd_ut
{
TEST_F(PortSyncdTest, test_linkSyncInit)
{
if_ni_mock = populateNetDev();
mockCmdStdcout = "up\n";
swss::LinkSync sync(m_app_db.get(), m_state_db.get());
std::vector<std::string> keys;
sync.m_stateMgmtPortTable.getKeys(keys);
ASSERT_EQ(keys.size(), 1);
ASSERT_EQ(keys.back(), "eth0");
ASSERT_EQ(mockCallArgs.back(), "cat /sys/class/net/\"eth0\"/operstate");
}

TEST_F(PortSyncdTest, test_cacheOldIfaces)
{
if_ni_mock = populateNetDevAdvanced();
Expand Down Expand Up @@ -295,29 +283,6 @@ namespace portsyncd_ut
ASSERT_EQ(sync.m_statePortTable.get("Ethernet0", ovalues), false);
}

TEST_F(PortSyncdTest, test_onMsgMgmtIface){
swss::LinkSync sync(m_app_db.get(), m_state_db.get());

/* Generate a netlink notification about the eth0 netdev iface */
std::vector<unsigned int> flags = {IFF_UP};
struct nl_object* msg = draft_nlmsg("eth0",
flags,
"",
"00:50:56:28:0e:4a",
16222,
9100,
0);
sync.onMsg(RTM_NEWLINK, msg);

/* Verify if the update has been written to State DB */
std::string oper_status;
ASSERT_EQ(sync.m_stateMgmtPortTable.hget("eth0", "oper_status", oper_status), true);
ASSERT_EQ(oper_status, "down");

/* Free Nl_object */
free_nlobj(msg);
}

TEST_F(PortSyncdTest, test_onMsgIgnoreOldNetDev){
if_ni_mock = populateNetDevAdvanced();
swss::LinkSync sync(m_app_db.get(), m_state_db.get());
Expand Down

0 comments on commit 9f336b1

Please sign in to comment.