From e82c77f8d9c56bdf3c0e7c0059461b2014c6e063 Mon Sep 17 00:00:00 2001 From: Sonic Build Admin Date: Fri, 10 Jan 2025 03:17:32 +0000 Subject: [PATCH] Remove update of mgmt oper status in swss **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: https://github.com/sonic-net/sonic-swss/pull/630 https://github.com/sonic-net/sonic-buildimage/pull/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 1. verified on single-asic platform and multi-asic Chassis platfrom. Single ASIC Arista device verification along with https://github.com/sonic-net/sonic-buildimage/pull/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 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 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 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 ``` **Details if related** --- portsyncd/linksync.cpp | 63 +-------------------- portsyncd/linksync.h | 2 +- tests/mock_tests/portsyncd/portsyncd_ut.cpp | 35 ------------ 3 files changed, 3 insertions(+), 97 deletions(-) diff --git a/portsyncd/linksync.cpp b/portsyncd/linksync.cpp index 66cdc4df..1648a40f 100644 --- a/portsyncd/linksync.cpp +++ b/portsyncd/linksync.cpp @@ -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"; @@ -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 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 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 */ @@ -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; } @@ -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 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)) { diff --git a/portsyncd/linksync.h b/portsyncd/linksync.h index d72e1ba1..5b31ed9b 100644 --- a/portsyncd/linksync.h +++ b/portsyncd/linksync.h @@ -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 m_ifindexNameMap; std::map m_ifindexOldNameMap; diff --git a/tests/mock_tests/portsyncd/portsyncd_ut.cpp b/tests/mock_tests/portsyncd/portsyncd_ut.cpp index f97a80e3..93575f68 100644 --- a/tests/mock_tests/portsyncd/portsyncd_ut.cpp +++ b/tests/mock_tests/portsyncd/portsyncd_ut.cpp @@ -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 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(); @@ -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 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());