Skip to content

Commit e82c77f

Browse files
committed
Remove update of mgmt oper status in swss
<!-- Please make sure you have read and understood the contribution guildlines: https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md 1. Make sure your commit includes a signature generted with `git commit -s` 2. Make sure your commit title follows the correct format: [component]: description 3. Make sure your commit message contains enough details about the change and related tests 4. Make sure your pull request adds related reviewers, asignees, labels Please also provide the following information in this pull request: --> **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 1. verified on single-asic platform and multi-asic Chassis platfrom. Single ASIC Arista device verification along with 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 ``` **Details if related**
1 parent 2c12aab commit e82c77f

File tree

3 files changed

+3
-97
lines changed

3 files changed

+3
-97
lines changed

portsyncd/linksync.cpp

+2-61
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ using namespace swss;
2727
#define VLAN_DRV_NAME "bridge"
2828
#define TEAM_DRV_NAME "team"
2929

30-
const string MGMT_PREFIX = "eth";
3130
const string INTFS_PREFIX = "Ethernet";
3231
const string LAG_PREFIX = "PortChannel";
3332

@@ -38,57 +37,11 @@ extern string g_switchType;
3837
LinkSync::LinkSync(DBConnector *appl_db, DBConnector *state_db) :
3938
m_portTableProducer(appl_db, APP_PORT_TABLE_NAME),
4039
m_portTable(appl_db, APP_PORT_TABLE_NAME),
41-
m_statePortTable(state_db, STATE_PORT_TABLE_NAME),
42-
m_stateMgmtPortTable(state_db, STATE_MGMT_PORT_TABLE_NAME)
40+
m_statePortTable(state_db, STATE_PORT_TABLE_NAME)
4341
{
4442
std::shared_ptr<struct if_nameindex> if_ni(if_nameindex(), if_freenameindex);
4543
struct if_nameindex *idx_p;
4644

47-
for (idx_p = if_ni.get();
48-
idx_p != NULL && idx_p->if_index != 0 && idx_p->if_name != NULL;
49-
idx_p++)
50-
{
51-
string key = idx_p->if_name;
52-
53-
/* Explicitly store management ports oper status into the state database.
54-
* This piece of information is used by SNMP. */
55-
if (!key.compare(0, MGMT_PREFIX.length(), MGMT_PREFIX))
56-
{
57-
ostringstream cmd;
58-
string res;
59-
cmd << "cat /sys/class/net/" << shellquote(key) << "/operstate";
60-
try
61-
{
62-
EXEC_WITH_ERROR_THROW(cmd.str(), res);
63-
}
64-
catch (...)
65-
{
66-
SWSS_LOG_WARN("Failed to get %s oper status", key.c_str());
67-
continue;
68-
}
69-
70-
/* Remove the trailing newline */
71-
if (res.length() >= 1 && res.at(res.length() - 1) == '\n')
72-
{
73-
res.erase(res.length() - 1);
74-
/* The value of operstate will be either up or down */
75-
if (res != "up" && res != "down")
76-
{
77-
SWSS_LOG_WARN("Unknown %s oper status %s",
78-
key.c_str(), res.c_str());
79-
}
80-
FieldValueTuple fv("oper_status", res);
81-
vector<FieldValueTuple> fvs;
82-
fvs.push_back(fv);
83-
84-
m_stateMgmtPortTable.set(key, fvs);
85-
SWSS_LOG_INFO("Store %s oper status %s to state DB",
86-
key.c_str(), res.c_str());
87-
}
88-
continue;
89-
}
90-
}
91-
9245
if (!WarmStart::isWarmStart())
9346
{
9447
/* See the comments for g_portSet in portsyncd.cpp */
@@ -168,8 +121,7 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
168121
string key = rtnl_link_get_name(link);
169122

170123
if (key.compare(0, INTFS_PREFIX.length(), INTFS_PREFIX) &&
171-
key.compare(0, LAG_PREFIX.length(), LAG_PREFIX) &&
172-
key.compare(0, MGMT_PREFIX.length(), MGMT_PREFIX))
124+
key.compare(0, LAG_PREFIX.length(), LAG_PREFIX))
173125
{
174126
return;
175127
}
@@ -197,17 +149,6 @@ void LinkSync::onMsg(int nlmsg_type, struct nl_object *obj)
197149
nlmsg_type, key.c_str(), admin, oper, addrStr, ifindex, master);
198150
}
199151

200-
if (!key.compare(0, MGMT_PREFIX.length(), MGMT_PREFIX))
201-
{
202-
FieldValueTuple fv("oper_status", oper ? "up" : "down");
203-
vector<FieldValueTuple> fvs;
204-
fvs.push_back(fv);
205-
m_stateMgmtPortTable.set(key, fvs);
206-
SWSS_LOG_INFO("Store %s oper status %s to state DB",
207-
key.c_str(), oper ? "up" : "down");
208-
return;
209-
}
210-
211152
/* teamd instances are dealt in teamsyncd */
212153
if (type && !strcmp(type, TEAM_DRV_NAME))
213154
{

portsyncd/linksync.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class LinkSync : public NetMsg
2020

2121
private:
2222
ProducerStateTable m_portTableProducer;
23-
Table m_portTable, m_statePortTable, m_stateMgmtPortTable;
23+
Table m_portTable, m_statePortTable;
2424

2525
std::map<unsigned int, std::string> m_ifindexNameMap;
2626
std::map<unsigned int, std::string> m_ifindexOldNameMap;

tests/mock_tests/portsyncd/portsyncd_ut.cpp

-35
Original file line numberDiff line numberDiff line change
@@ -187,18 +187,6 @@ namespace portsyncd_ut
187187

188188
namespace portsyncd_ut
189189
{
190-
TEST_F(PortSyncdTest, test_linkSyncInit)
191-
{
192-
if_ni_mock = populateNetDev();
193-
mockCmdStdcout = "up\n";
194-
swss::LinkSync sync(m_app_db.get(), m_state_db.get());
195-
std::vector<std::string> keys;
196-
sync.m_stateMgmtPortTable.getKeys(keys);
197-
ASSERT_EQ(keys.size(), 1);
198-
ASSERT_EQ(keys.back(), "eth0");
199-
ASSERT_EQ(mockCallArgs.back(), "cat /sys/class/net/\"eth0\"/operstate");
200-
}
201-
202190
TEST_F(PortSyncdTest, test_cacheOldIfaces)
203191
{
204192
if_ni_mock = populateNetDevAdvanced();
@@ -295,29 +283,6 @@ namespace portsyncd_ut
295283
ASSERT_EQ(sync.m_statePortTable.get("Ethernet0", ovalues), false);
296284
}
297285

298-
TEST_F(PortSyncdTest, test_onMsgMgmtIface){
299-
swss::LinkSync sync(m_app_db.get(), m_state_db.get());
300-
301-
/* Generate a netlink notification about the eth0 netdev iface */
302-
std::vector<unsigned int> flags = {IFF_UP};
303-
struct nl_object* msg = draft_nlmsg("eth0",
304-
flags,
305-
"",
306-
"00:50:56:28:0e:4a",
307-
16222,
308-
9100,
309-
0);
310-
sync.onMsg(RTM_NEWLINK, msg);
311-
312-
/* Verify if the update has been written to State DB */
313-
std::string oper_status;
314-
ASSERT_EQ(sync.m_stateMgmtPortTable.hget("eth0", "oper_status", oper_status), true);
315-
ASSERT_EQ(oper_status, "down");
316-
317-
/* Free Nl_object */
318-
free_nlobj(msg);
319-
}
320-
321286
TEST_F(PortSyncdTest, test_onMsgIgnoreOldNetDev){
322287
if_ni_mock = populateNetDevAdvanced();
323288
swss::LinkSync sync(m_app_db.get(), m_state_db.get());

0 commit comments

Comments
 (0)