Skip to content

Commit

Permalink
Port configuration incremental update support (sonic-net#2305)
Browse files Browse the repository at this point in the history 
*portsyncd no longer handle port configuration change and put it to APP DB
*Implement incremental configuration change in portmgr
*Adjust portsorch to meet incremental configuration change requirement
  • Loading branch information
@Junchao-Mellanox @preetham-singh
Junchao-Mellanox authored and preetham-singh committed Aug 6, 2022
1 parent 90df911 commit 8a052f1
Show file tree
Hide file tree
Showing 8 changed files with 205 additions and 122 deletions.
109 changes: 46 additions & 63 deletions cfgmgr/portmgr.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,29 +31,9 @@ bool PortMgr::setPortMtu(const string &alias, const string &mtu)

// 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);

return true;
return writeConfigToAppDb(alias, "mtu", mtu);
}

bool PortMgr::setPortTpid(const string &alias, const string &tpid)
{
stringstream cmd;
string res;

// Set the port TPID in application database to update port TPID
vector<FieldValueTuple> fvs;
FieldValueTuple fv("tpid", tpid);
fvs.push_back(fv);
m_appPortTable.set(alias, fvs);

return true;
}


bool PortMgr::setPortAdminStatus(const string &alias, const bool up)
{
stringstream cmd;
Expand All @@ -63,23 +43,7 @@ bool PortMgr::setPortAdminStatus(const string &alias, const bool up)
cmd << IP_CMD << " link set dev " << shellquote(alias) << (up ? " up" : " down");
EXEC_WITH_ERROR_THROW(cmd.str(), res);

vector<FieldValueTuple> fvs;
FieldValueTuple fv("admin_status", (up ? "up" : "down"));
fvs.push_back(fv);
m_appPortTable.set(alias, fvs);

return true;
}

bool PortMgr::setPortLearnMode(const string &alias, const string &learn_mode)
{
// Set the port MAC learn mode in application database
vector<FieldValueTuple> fvs;
FieldValueTuple fv("learn_mode", learn_mode);
fvs.push_back(fv);
m_appPortTable.set(alias, fvs);

return true;
return writeConfigToAppDb(alias, "admin_status", (up ? "up" : "down"));
}

bool PortMgr::isPortStateOk(const string &alias)
Expand Down Expand Up @@ -117,14 +81,14 @@ void PortMgr::doTask(Consumer &consumer)

if (op == SET_COMMAND)
{
if (!isPortStateOk(alias))
{
SWSS_LOG_INFO("Port %s is not ready, pending...", alias.c_str());
it++;
continue;
}
/* portOk=true indicates that the port has been created in kernel.
* We should not call any ip command if portOk=false. However, it is
* valid to put port configuration to APP DB which will trigger port creation in kernel.
*/
bool portOk = isPortStateOk(alias);

string admin_status, mtu, learn_mode, tpid;
string admin_status, mtu;
std::vector<FieldValueTuple> field_values;

bool configured = (m_portList.find(alias) != m_portList.end());

Expand All @@ -138,6 +102,11 @@ void PortMgr::doTask(Consumer &consumer)

m_portList.insert(alias);
}
else if (!portOk)
{
it++;
continue;
}

for (auto i : kfvFieldsValues(t))
{
Expand All @@ -149,38 +118,42 @@ void PortMgr::doTask(Consumer &consumer)
{
admin_status = fvValue(i);
}
else if (fvField(i) == "learn_mode")
{
learn_mode = fvValue(i);
}
else if (fvField(i) == "tpid")
else
{
tpid = fvValue(i);
field_values.emplace_back(i);
}
}

if (!mtu.empty())
for (auto &entry : field_values)
{
setPortMtu(alias, mtu);
SWSS_LOG_NOTICE("Configure %s MTU to %s", alias.c_str(), mtu.c_str());
writeConfigToAppDb(alias, fvField(entry), fvValue(entry));
SWSS_LOG_NOTICE("Configure %s %s to %s", alias.c_str(), fvField(entry).c_str(), fvValue(entry).c_str());
}

if (!admin_status.empty())
if (!portOk)
{
setPortAdminStatus(alias, admin_status == "up");
SWSS_LOG_NOTICE("Configure %s admin status to %s", alias.c_str(), admin_status.c_str());
SWSS_LOG_INFO("Port %s is not ready, pending...", alias.c_str());

writeConfigToAppDb(alias, "mtu", mtu);
writeConfigToAppDb(alias, "admin_status", admin_status);
field_values.clear();
field_values.emplace_back("mtu", mtu);
field_values.emplace_back("admin_status", admin_status);
it->second = KeyOpFieldsValuesTuple{alias, SET_COMMAND, field_values};
it++;
continue;
}

if (!learn_mode.empty())
if (!mtu.empty())
{
setPortLearnMode(alias, learn_mode);
SWSS_LOG_NOTICE("Configure %s MAC learn mode to %s", alias.c_str(), learn_mode.c_str());
setPortMtu(alias, mtu);
SWSS_LOG_NOTICE("Configure %s MTU to %s", alias.c_str(), mtu.c_str());
}

if (!tpid.empty())
if (!admin_status.empty())
{
setPortTpid(alias, tpid);
SWSS_LOG_NOTICE("Configure %s TPID to %s", alias.c_str(), tpid.c_str());
setPortAdminStatus(alias, admin_status == "up");
SWSS_LOG_NOTICE("Configure %s admin status to %s", alias.c_str(), admin_status.c_str());
}
}
else if (op == DEL_COMMAND)
Expand All @@ -193,3 +166,13 @@ void PortMgr::doTask(Consumer &consumer)
it = consumer.m_toSync.erase(it);
}
}

bool PortMgr::writeConfigToAppDb(const std::string &alias, const std::string &field, const std::string &value)
{
vector<FieldValueTuple> fvs;
FieldValueTuple fv(field, value);
fvs.push_back(fv);
m_appPortTable.set(alias, fvs);

return true;
}
3 changes: 1 addition & 2 deletions cfgmgr/portmgr.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,9 @@ class PortMgr : public Orch
std::set<std::string> m_portList;

void doTask(Consumer &consumer);
bool writeConfigToAppDb(const std::string &alias, const std::string &field, const std::string &value);
bool setPortMtu(const std::string &alias, const std::string &mtu);
bool setPortTpid(const std::string &alias, const std::string &tpid);
bool setPortAdminStatus(const std::string &alias, const bool up);
bool setPortLearnMode(const std::string &alias, const std::string &learn_mode);
bool isPortStateOk(const std::string &alias);
};

Expand Down
10 changes: 8 additions & 2 deletions orchagent/port.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,12 @@ class Port
UNKNOWN
} ;

enum AutoNegMode {
AUTONEG_NOT_SET = -1,
AUTONEG_OFF = 0,
AUTONEG_ON = 1
};

Port() {};
Port(std::string alias, Type type) :
m_alias(alias), m_type(type) {};
Expand All @@ -112,7 +118,7 @@ class Port
uint32_t m_mtu = DEFAULT_MTU;
uint32_t m_speed = 0; // Mbps
std::string m_learn_mode = "hardware";
int m_autoneg = -1; // -1 means not set, 0 = disabled, 1 = enabled
AutoNegMode m_autoneg = Port::AutoNegMode::AUTONEG_NOT_SET;
bool m_admin_state_up = false;
bool m_init = false;
bool m_l3_vni = false;
Expand Down Expand Up @@ -148,7 +154,7 @@ class Port
uint32_t m_up_member_count = 0;
uint32_t m_maximum_headroom = 0;
std::vector<uint32_t> m_adv_speeds;
sai_port_interface_type_t m_interface_type;
sai_port_interface_type_t m_interface_type = SAI_PORT_INTERFACE_TYPE_NONE;
std::vector<uint32_t> m_adv_interface_types;
bool m_mpls = false;
/*
Expand Down
7 changes: 6 additions & 1 deletion orchagent/portsorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2966,6 +2966,11 @@ void PortsOrch::doPortTask(Consumer &consumer)
}
else
{
if (admin_status.empty())
{
admin_status = p.m_admin_state_up ? "up" : "down";
}

if (!an_str.empty())
{
if (autoneg_mode_map.find(an_str) == autoneg_mode_map.end())
Expand Down Expand Up @@ -3008,7 +3013,7 @@ void PortsOrch::doPortTask(Consumer &consumer)
continue;
}
SWSS_LOG_NOTICE("Set port %s AutoNeg from %d to %d", alias.c_str(), p.m_autoneg, an);
p.m_autoneg = an;
p.m_autoneg = static_cast<swss::Port::AutoNegMode>(an);
m_portList[alias] = p;
}
}
Expand Down
54 changes: 0 additions & 54 deletions portsyncd/portsyncd.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,12 @@ void usage()
void handlePortConfigFile(ProducerStateTable &p, string file, bool warm);
void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb, bool warm);
void handleVlanIntfFile(string file);
void handlePortConfig(ProducerStateTable &p, map<string, KeyOpFieldsValuesTuple> &port_cfg_map);
void checkPortInitDone(DBConnector *appl_db);

int main(int argc, char **argv)
{
Logger::linkToDbNative("portsyncd");
int opt;
map<string, KeyOpFieldsValuesTuple> port_cfg_map;

while ((opt = getopt(argc, argv, "v:h")) != -1 )
{
Expand All @@ -71,7 +69,6 @@ int main(int argc, char **argv)
DBConnector appl_db("APPL_DB", 0);
DBConnector state_db("STATE_DB", 0);
ProducerStateTable p(&appl_db, APP_PORT_TABLE_NAME);
SubscriberStateTable portCfg(&cfgDb, CFG_PORT_TABLE_NAME);

WarmStart::initialize("portsyncd", "swss");
WarmStart::checkWarmStart("portsyncd", "swss");
Expand All @@ -93,7 +90,6 @@ int main(int argc, char **argv)
NetDispatcher::getInstance().registerMessageHandler(RTM_DELLINK, &sync);

s.addSelectable(&netlink);
s.addSelectable(&portCfg);

while (true)
{
Expand Down Expand Up @@ -135,28 +131,6 @@ int main(int argc, char **argv)

g_init = true;
}
if (!port_cfg_map.empty())
{
handlePortConfig(p, port_cfg_map);
}
}
else if (temps == (Selectable *)&portCfg)
{
std::deque<KeyOpFieldsValuesTuple> entries;
portCfg.pops(entries);

for (auto entry: entries)
{
string key = kfvKey(entry);

if (port_cfg_map.find(key) != port_cfg_map.end())
{
/* For now we simply drop previous pending port config */
port_cfg_map.erase(key);
}
port_cfg_map[key] = entry;
}
handlePortConfig(p, port_cfg_map);
}
else
{
Expand Down Expand Up @@ -225,31 +199,3 @@ void handlePortConfigFromConfigDB(ProducerStateTable &p, DBConnector &cfgDb, boo
}

}

void handlePortConfig(ProducerStateTable &p, map<string, KeyOpFieldsValuesTuple> &port_cfg_map)
{
auto it = port_cfg_map.begin();
while (it != port_cfg_map.end())
{
KeyOpFieldsValuesTuple entry = it->second;
string key = kfvKey(entry);
string op = kfvOp(entry);
auto values = kfvFieldsValues(entry);

/* only push down port config when port is not in hostif create pending state */
if (g_portSet.find(key) == g_portSet.end())
{
/* No support for port delete yet */
if (op == SET_COMMAND)
{
p.set(key, values);
}

it = port_cfg_map.erase(it);
}
else
{
it++;
}
}
}
3 changes: 3 additions & 0 deletions tests/mock_tests/Makefile.am
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ tests_SOURCES = aclorch_ut.cpp \
mock_table.cpp \
mock_hiredis.cpp \
mock_redisreply.cpp \
mock_shell_command.cpp \
bulker_ut.cpp \
portmgr_ut.cpp \
fake_response_publisher.cpp \
swssnet_ut.cpp \
flowcounterrouteorch_ut.cpp \
Expand Down Expand Up @@ -98,6 +100,7 @@ tests_SOURCES = aclorch_ut.cpp \
$(top_srcdir)/orchagent/bfdorch.cpp \
$(top_srcdir)/orchagent/srv6orch.cpp \
$(top_srcdir)/orchagent/nvgreorch.cpp \
$(top_srcdir)/cfgmgr/portmgr.cpp \
$(top_srcdir)/cfgmgr/buffermgrdyn.cpp

tests_SOURCES += $(FLEX_CTR_DIR)/flex_counter_manager.cpp $(FLEX_CTR_DIR)/flex_counter_stat_manager.cpp $(FLEX_CTR_DIR)/flow_counter_handler.cpp $(FLEX_CTR_DIR)/flowcounterrouteorch.cpp
Expand Down
15 changes: 15 additions & 0 deletions tests/mock_tests/mock_shell_command.cpp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
#include <string>
#include <vector>

int mockCmdReturn = 0;
std::string mockCmdStdcout = "";
std::vector<std::string> mockCallArgs;

namespace swss {
int exec(const std::string &cmd, std::string &stdout)
{
mockCallArgs.push_back(cmd);
stdout = mockCmdStdcout;
return mockCmdReturn;
}
}
Loading

0 comments on commit 8a052f1

Please sign in to comment.