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.

Sign up for GitHub

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

[portmgrd] prevent runtime exception (crash) in setting MTU on portchannel member #3432

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[portmgrd] prevent runtime exception (crash) in setting MTU on portchannel member #3432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
201d5b1
portmgrd: prevent runtime failure in setting MTU on portchannel member
bradh352 Dec 17, 2024
4bc50aa
isLag -> isLagMember as requested by @dgsudharsan
bradh352 Jan 7, 2025
d4b4b98
Add test case as per @dgsudharsan
bradh352 Jan 7, 2025
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
33 changes: 30 additions & 3 deletions cfgmgr/portmgr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,28 @@ PortMgr::PortMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c
{
}

bool PortMgr::isLagMember(const std::string &port)
{
SWSS_LOG_ENTER();

vector<string> keys;
m_cfgLagMemberTable.getKeys(keys);

for (auto key: keys)
{
auto tokens = tokenize(key, config_db_key_delimiter);
auto lag = tokens[0];
auto member = tokens[1];

if (port == member)
{
return true;
}
}

return false;
}

bool PortMgr::setPortMtu(const string &alias, const string &mtu)
{
stringstream cmd;
Expand Down Expand Up @@ -128,6 +150,7 @@ void PortMgr::doSendToIngressPortTask(Consumer &consumer)

}


void PortMgr::doTask(Consumer &consumer)
{
SWSS_LOG_ENTER();
Expand Down Expand Up @@ -221,14 +244,18 @@ void PortMgr::doTask(Consumer &consumer)

if (!mtu.empty())
{
setPortMtu(alias, mtu);
SWSS_LOG_NOTICE("Configure %s MTU to %s", alias.c_str(), mtu.c_str());
if (isLagMember(alias)) {
SWSS_LOG_NOTICE("Skipping Configure %s MTU to %s as interface is part of a PortChannel", alias.c_str(), mtu.c_str());
} else {
SWSS_LOG_NOTICE("Configure %s MTU to %s", alias.c_str(), mtu.c_str());
setPortMtu(alias, mtu);
}
}

if (!admin_status.empty())
{
setPortAdminStatus(alias, admin_status == "up");
SWSS_LOG_NOTICE("Configure %s admin status to %s", alias.c_str(), admin_status.c_str());
setPortAdminStatus(alias, admin_status == "up");
}
}
else if (op == DEL_COMMAND)
Expand Down
1 change: 1 addition & 0 deletions cfgmgr/portmgr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class PortMgr : public Orch
bool setPortMtu(const std::string &alias, const std::string &mtu);
bool setPortAdminStatus(const std::string &alias, const bool up);
bool isPortStateOk(const std::string &alias);
bool isLagMember(const std::string &port);
};

}
45 changes: 45 additions & 0 deletions tests/test_portchannel.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,51 @@ def test_portchannel_member_netdev_oper_status(self, dvs, testlog):
# wait for port-channel deletion
time.sleep(1)

# Make sure if a PortChannel member port tries to set an MTU that it is
# ignored and does not cause a runtime error.
def test_portchannel_member_mtu(self, dvs, testlog):
config_db = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0)
app_db = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0)

# create port-channel
tbl = swsscommon.Table(config_db, "PORTCHANNEL")
fvs = swsscommon.FieldValuePairs([("admin_status", "up"),("mtu", "9100"),("oper_status", "up")])
tbl.set("PortChannel111", fvs)

# set port-channel oper status
tbl = swsscommon.ProducerStateTable(app_db, "LAG_TABLE")
fvs = swsscommon.FieldValuePairs([("admin_status", "up"),("mtu", "9100"),("oper_status", "up")])
tbl.set("PortChannel111", fvs)

# add members to port-channel
tbl = swsscommon.Table(config_db, "PORTCHANNEL_MEMBER")
fvs = swsscommon.FieldValuePairs([("NULL", "NULL")])
tbl.set("PortChannel111|Ethernet0", fvs)
tbl.set("PortChannel111|Ethernet4", fvs)

# wait for port-channel netdev creation
time.sleep(1)

tbl = swsscommon.Table(config_db, "PORT")
fvs = swsscommon.FieldValuePairs([("mtu", "9100")])
tbl.set("Ethernet0", fvs)

# wait for attempted configuration to be applied
time.sleep(1)

# remove port-channel members
tbl = swsscommon.Table(config_db, "PORTCHANNEL_MEMBER")
tbl._del("PortChannel111|Ethernet0")
tbl._del("PortChannel111|Ethernet4")

# remove port-channel
tbl = swsscommon.Table(config_db, "PORTCHANNEL")
tbl._del("PortChannel111")

# wait for port-channel deletion
time.sleep(1)


# Add Dummy always-pass test at end as workaroud
# for issue when Flaky fail on final test it invokes module tear-down before retrying
def test_nonflaky_dummy():
Expand Down
Loading