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

[portsorch]: Support LAG MTU configuration #581

Merged
merged 1 commit into from
Aug 21, 2018
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
55 changes: 49 additions & 6 deletions cfgmgr/portmgr.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include <string>

#include "logger.h"
#include "dbconnector.h"
#include "producerstatetable.h"
Expand All @@ -16,17 +17,57 @@ PortMgr::PortMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c
m_cfgPortTable(cfgDb, CFG_PORT_TABLE_NAME),
m_cfgLagTable(cfgDb, CFG_LAG_TABLE_NAME),
m_statePortTable(stateDb, STATE_PORT_TABLE_NAME),
m_stateLagTable(stateDb, STATE_LAG_TABLE_NAME)
m_stateLagTable(stateDb, STATE_LAG_TABLE_NAME),
m_appPortTable(appDb, APP_PORT_TABLE_NAME),
m_appLagTable(appDb, APP_LAG_TABLE_NAME)
{
}

bool PortMgr::setPortMtu(const string &alias, const string &mtu)
bool PortMgr::setPortMtu(const string &table, const string &alias, const string &mtu)
{
stringstream cmd;
string res;

cmd << IP_CMD << " link set dev " << alias << " mtu " << mtu;
return exec(cmd.str(), res) == 0;
EXEC_WITH_ERROR_THROW(cmd.str(), res);

if (table == CFG_PORT_TABLE_NAME)
{
// Set the port MTU in application database to update both
// the port MTU and possibly the port based router interface MTU
vector<FieldValueTuple> fvs;
FieldValueTuple fv("mtu", mtu);
fvs.push_back(fv);
m_appPortTable.set(alias, fvs);
}
else if (table == CFG_LAG_TABLE_NAME)
{
// Set the port channel MTU in application database to update
// the LAG based router interface MTU in orchagent
vector<FieldValueTuple> fvs;
FieldValueTuple fv("mtu", mtu);
fvs.push_back(fv);
m_appLagTable.set(alias, fvs);

m_cfgLagTable.get(alias, fvs);
for (auto fv: fvs)
{
// Set the port channel members MTU in application database
// to update the port MTU in orchagent
if (fvField(fv) == "members")
{
for (auto member : tokenize(fvValue(fv), ','))
{
vector<FieldValueTuple> member_fvs;
FieldValueTuple member_fv("mtu", mtu);
member_fvs.push_back(member_fv);
m_appPortTable.set(member, member_fvs);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the port mtu is overwritten by the lag mtu value, what happens when the port is removed from lag later? Is it handled?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my earlier notes in this pull request says that this won't cover the MTU changes associated with the port channel membership changes. i'll go and cover those cases when i add the support for configuring port channel memberships

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this pull request will focus only on the MTU configurations with out changing the members of the port channels

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh ok, didn't notice the notes.

}
}
}
}

return true;
}

bool PortMgr::setPortAdminStatus(const string &alias, const bool up)
Expand All @@ -35,7 +76,9 @@ bool PortMgr::setPortAdminStatus(const string &alias, const bool up)
string res;

cmd << IP_CMD << " link set dev " << alias << (up ? " up" : " down");
return exec(cmd.str(), res) == 0;
EXEC_WITH_ERROR_THROW(cmd.str(), res);

return true;
}

bool PortMgr::isPortStateOk(const string &table, const string &alias)
Expand Down Expand Up @@ -80,7 +123,7 @@ void PortMgr::doTask(Consumer &consumer)
{
if (!isPortStateOk(table, alias))
{
SWSS_LOG_INFO("Port %s is not ready, pending", alias.c_str());
SWSS_LOG_INFO("Port %s is not ready, pending...", alias.c_str());
it++;
continue;
}
Expand All @@ -90,7 +133,7 @@ void PortMgr::doTask(Consumer &consumer)
if (fvField(i) == "mtu")
{
auto mtu = fvValue(i);
setPortMtu(alias, mtu);
setPortMtu(table, alias, mtu);
SWSS_LOG_NOTICE("Configure %s MTU to %s",
alias.c_str(), mtu.c_str());
}
Expand Down
4 changes: 3 additions & 1 deletion cfgmgr/portmgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ class PortMgr : public Orch
Table m_cfgLagTable;
Table m_statePortTable;
Table m_stateLagTable;
ProducerStateTable m_appPortTable;
ProducerStateTable m_appLagTable;

void doTask(Consumer &consumer);
bool setPortMtu(const string &alias, const string &mtu);
bool setPortMtu(const string &table, const string &alias, const string &mtu);
bool setPortAdminStatus(const string &alias, const bool up);
bool isPortStateOk(const string &table, const string &alias);
};
Expand Down
5 changes: 3 additions & 2 deletions orchagent/intfsorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,13 +310,14 @@ bool IntfsOrch::addRouterIntfs(Port &port)
sai_status_t status = sai_router_intfs_api->create_router_interface(&port.m_rif_id, gSwitchId, (uint32_t)attrs.size(), attrs.data());
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to create router interface for port %s, rv:%d", port.m_alias.c_str(), status);
SWSS_LOG_ERROR("Failed to create router interface %s, rv:%d",
port.m_alias.c_str(), status);
throw runtime_error("Failed to create router interface.");
}

gPortsOrch->setPort(port.m_alias, port);

SWSS_LOG_NOTICE("Create router interface for port %s mtu %u", port.m_alias.c_str(), port.m_mtu);
SWSS_LOG_NOTICE("Create router interface %s MTU %u", port.m_alias.c_str(), port.m_mtu);

return true;
}
Expand Down
58 changes: 47 additions & 11 deletions orchagent/portsorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,8 @@ bool PortsOrch::setPortMtu(sai_object_id_t id, sai_uint32_t mtu)
sai_status_t status = sai_port_api->set_port_attribute(id, &attr);
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to set MTU %u to port pid:%lx", attr.value.u32, id);
SWSS_LOG_ERROR("Failed to set MTU %u to port pid:%lx, rv:%d",
attr.value.u32, id, status);
return false;
}
SWSS_LOG_INFO("Set MTU %u to port pid:%lx", attr.value.u32, id);
Expand Down Expand Up @@ -1276,7 +1277,7 @@ void PortsOrch::doPortTask(Consumer &consumer)
return;
}

if (op == "SET")
if (op == SET_COMMAND)
{
set<int> lane_set;
string admin_status;
Expand All @@ -1303,11 +1304,15 @@ void PortsOrch::doPortTask(Consumer &consumer)

/* Set port admin status */
if (fvField(i) == "admin_status")
{
admin_status = fvValue(i);
}

/* Set port MTU */
if (fvField(i) == "mtu")
{
mtu = (uint32_t)stoul(fvValue(i));
}

/* Set port speed */
if (fvField(i) == "speed")
Expand All @@ -1317,7 +1322,9 @@ void PortsOrch::doPortTask(Consumer &consumer)

/* Set port fec */
if (fvField(i) == "fec")
{
fec_mode = fvValue(i);
}

/* Set autoneg and ignore the port speed setting */
if (fvField(i) == "autoneg")
Expand Down Expand Up @@ -1786,28 +1793,57 @@ void PortsOrch::doLagTask(Consumer &consumer)
{
auto &t = it->second;

string lag_alias = kfvKey(t);
string alias = kfvKey(t);
string op = kfvOp(t);

if (op == SET_COMMAND)
{
/* Duplicate entry */
if (m_portList.find(lag_alias) != m_portList.end())
// Retrieve attributes
uint32_t mtu = 0;
for (auto i : kfvFieldsValues(t))
{
it = consumer.m_toSync.erase(it);
continue;
if (fvField(i) == "mtu")
{
mtu = (uint32_t)stoul(fvValue(i));
}
}

if (addLag(lag_alias))
it = consumer.m_toSync.erase(it);
// Create a new LAG when the new alias comes
if (m_portList.find(alias) == m_portList.end())
{
if (!addLag(alias))
{
it++;
continue;
}
}

// Process attributes
Port l;
if (!getPort(alias, l))
{
SWSS_LOG_ERROR("Failed to get LAG %s", alias.c_str());
}
else
it++;
{
if (mtu != 0 && l.m_rif_id)
{
l.m_mtu = mtu;
m_portList[alias] = l;
if (l.m_rif_id)
{
gIntfsOrch->setRouterIntfsMtu(l);
}
}
}

it = consumer.m_toSync.erase(it);
}
else if (op == DEL_COMMAND)
{
Port lag;
/* Cannot locate LAG */
if (!getPort(lag_alias, lag))
if (!getPort(alias, lag))
{
it = consumer.m_toSync.erase(it);
continue;
Expand Down
11 changes: 4 additions & 7 deletions teamsyncd/teamsync.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,25 +45,22 @@ void TeamSync::onMsg(int nlmsg_type, struct nl_object *obj)

addLag(lagName, rtnl_link_get_ifindex(link),
rtnl_link_get_flags(link) & IFF_UP,
rtnl_link_get_flags(link) & IFF_LOWER_UP,
rtnl_link_get_mtu(link));
rtnl_link_get_flags(link) & IFF_LOWER_UP);
}

void TeamSync::addLag(const string &lagName, int ifindex, bool admin_state,
bool oper_state, unsigned int mtu)
bool oper_state)
{
/* Set the LAG */
std::vector<FieldValueTuple> fvVector;
FieldValueTuple a("admin_status", admin_state ? "up" : "down");
FieldValueTuple o("oper_status", oper_state ? "up" : "down");
FieldValueTuple m("mtu", to_string(mtu));
fvVector.push_back(a);
fvVector.push_back(o);
fvVector.push_back(m);
m_lagTable.set(lagName, fvVector);

SWSS_LOG_INFO("Add %s admin_status:%s oper_status:%s mtu:%d",
lagName.c_str(), admin_state ? "up" : "down", oper_state ? "up" : "down", mtu);
SWSS_LOG_INFO("Add %s admin_status:%s oper_status:%s",
lagName.c_str(), admin_state ? "up" : "down", oper_state ? "up" : "down");

/* Return when the team instance has already been tracked */
if (m_teamPorts.find(lagName) != m_teamPorts.end())
Expand Down
2 changes: 1 addition & 1 deletion teamsyncd/teamsync.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ class TeamSync : public NetMsg

protected:
void addLag(const std::string &lagName, int ifindex, bool admin_state,
bool oper_state, unsigned int mtu);
bool oper_state);
void removeLag(const std::string &lagName);

private:
Expand Down
Loading