Skip to content

Commit

Permalink
Refactor code so that portsyncd does not deal with configurations at …
Browse files Browse the repository at this point in the history
…run time

portmgrd will deal with configurations for both linux side and appDB.
Added vs test cases for port config changes

Signed-off-by: Zhenggen Xu <zxu@linkedin.com>
  • Loading branch information
zhenggen-xu committed Oct 8, 2019
1 parent 953474a commit 8c9eac6
Show file tree
Hide file tree
Showing 5 changed files with 448 additions and 94 deletions.
182 changes: 142 additions & 40 deletions cfgmgr/portmgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,6 @@ bool PortMgr::setPortMtu(const string &alias, const string &mtu)
cmd << IP_CMD << " link set dev " << alias << " mtu " << mtu;
EXEC_WITH_ERROR_THROW(cmd.str(), res);

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

Expand All @@ -47,11 +40,6 @@ bool PortMgr::setPortAdminStatus(const string &alias, const bool up)
cmd << IP_CMD << " link set dev " << 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;
}

Expand All @@ -68,6 +56,107 @@ bool PortMgr::isPortStateOk(const string &alias)
return false;
}

// This function set the default value of the kernelsettings' field
// to fvs if the field does not exist in fvs
void PortMgr::constructKernelSettings(const KernelSettingMap &kernelsettings,
vector<FieldValueTuple> &fvs)
{
bool found = false;

for (auto it : kernelsettings)
{
for (auto i : fvs)
{
if (fvField(i) == it.first)
{
found = true;
break;
}
}

if (!found)
{
FieldValueTuple fv(it.first, it.second);
fvs.push_back(fv);
}
}

return;
}

// Get kernel settings from fvs if exist
void PortMgr::getKernelSettingsFromFVS(KernelSettingMap &kernelsettings,
const vector<FieldValueTuple> &fvs)
{
for (auto& it : kernelsettings)
{
for (auto i : fvs)
{
if (fvField(i) == it.first)
{
it.second = fvValue(i);
break;
}
}
}
}

// This function save the kernelsettings to a global map
// If kernelsettings is empty, return false. Else return true.
bool PortMgr::saveKernelSettingsMap(const string &alias,
KernelSettingMap &kernelsettings)
{
string admin_status = "admin_status";
string mtu = "mtu";

/* no need to update the map if no new settings*/
if (kernelsettings[admin_status].empty()
&& kernelsettings[mtu].empty())
{
return false;
}

auto exist_kofvs = m_kernelSettingMap.find(alias);

// no settings exist, use the new settings
if (exist_kofvs == m_kernelSettingMap.end())
{
m_kernelSettingMap[alias] = kernelsettings;
return true;
}

// update the old settings with new non-empty settings
for (auto& newsettings : kernelsettings)
{
if (!newsettings.second.empty())
{
m_kernelSettingMap[alias][newsettings.first] = newsettings.second;
}
}

return true;
}

void PortMgr::doConfigKernelSettings(const string &alias, KernelSettingMap settings)
{
string mtu, admin_status;

mtu = settings["mtu"];
admin_status = settings["admin_status"];

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

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());
}
}

void PortMgr::doTask(Consumer &consumer)
{
SWSS_LOG_ENTER();
Expand All @@ -81,17 +170,14 @@ void PortMgr::doTask(Consumer &consumer)

string alias = kfvKey(t);
string op = kfvOp(t);
auto fvs = kfvFieldsValues(t);

if (op == SET_COMMAND)
{
if (!isPortStateOk(alias))
{
SWSS_LOG_INFO("Port %s is not ready, pending...", alias.c_str());
it++;
continue;
}
map <string, string> kernelsecttings;

string admin_status, mtu;
kernelsecttings["admin_status"] = "";
kernelsecttings["mtu"] = "";

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

Expand All @@ -100,37 +186,53 @@ void PortMgr::doTask(Consumer &consumer)
*/
if (!configured)
{
admin_status = DEFAULT_ADMIN_STATUS_STR;
mtu = DEFAULT_MTU_STR;
kernelsecttings["admin_status"] = DEFAULT_ADMIN_STATUS_STR;
kernelsecttings["mtu"] = DEFAULT_MTU_STR;

constructKernelSettings(kernelsecttings, fvs);
m_portList.insert(alias);
}

for (auto i : kfvFieldsValues(t))
{
if (fvField(i) == "mtu")
{
mtu = fvValue(i);
}
else if (fvField(i) == "admin_status")
{
admin_status = fvValue(i);
}
}
// pass to appDB
m_appPortTable.set(alias, fvs);

if (!mtu.empty())
{
setPortMtu(alias, mtu);
SWSS_LOG_NOTICE("Configure %s MTU to %s", alias.c_str(), mtu.c_str());
}
// Save the settings to kernelsettingmap
// Get from fvs first if exists
getKernelSettingsFromFVS(kernelsecttings, fvs);

if (!admin_status.empty())
// If new kernel configurations exists and portstate is ok, apply it now
// Otherwise, it will be done in doKernelSettingTask()
if (saveKernelSettingsMap(alias, kernelsecttings))
{
setPortAdminStatus(alias, admin_status == "up");
SWSS_LOG_NOTICE("Configure %s admin status to %s", alias.c_str(), admin_status.c_str());
if (isPortStateOk(alias))
{
doConfigKernelSettings(alias, m_kernelSettingMap[alias]);
m_kernelSettingMap.erase(alias);
}
}
}

it = consumer.m_toSync.erase(it);
}
}

void PortMgr::doKernelSettingTask()
{
SWSS_LOG_ENTER();

auto it = m_kernelSettingMap.begin();
while (it != m_kernelSettingMap.end())
{
string alias = it->first;
auto settings = it->second;

if (!isPortStateOk(alias))
{
SWSS_LOG_INFO("Port %s is not ready, retry later...", alias.c_str());
it++;
continue;
}
doConfigKernelSettings(alias, settings);
it = m_kernelSettingMap.erase(it);
}
}
13 changes: 13 additions & 0 deletions cfgmgr/portmgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,34 @@ namespace swss {
#define DEFAULT_ADMIN_STATUS_STR "down"
#define DEFAULT_MTU_STR "9100"

typedef std::map<std::string, std::string> KernelSettingMap;
typedef std::map<std::string, KernelSettingMap> AllPortsKernelSettingMap;

class PortMgr : public Orch
{
public:
PortMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, const std::vector<std::string> &tableNames);

using Orch::doTask;
void doKernelSettingTask();
private:
Table m_cfgPortTable;
Table m_cfgLagMemberTable;
Table m_statePortTable;
ProducerStateTable m_appPortTable;

std::set<std::string> m_portList;
AllPortsKernelSettingMap m_kernelSettingMap;

void doTask(Consumer &consumer);
void constructKernelSettings(const KernelSettingMap &kernelsettings,
vector<FieldValueTuple> &fvs);
void getKernelSettingsFromFVS(KernelSettingMap &kernelsettings,
const vector<FieldValueTuple> &fvs);
bool saveKernelSettingsMap(const std::string &alias,
KernelSettingMap &kernelsettings);
void doConfigKernelSettings(const std::string &alias,
KernelSettingMap settings);
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);
Expand Down
1 change: 1 addition & 0 deletions cfgmgr/portmgrd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ int main(int argc, char **argv)
if (ret == Select::TIMEOUT)
{
portmgr.doTask();
portmgr.doKernelSettingTask();
continue;
}

Expand Down
54 changes: 0 additions & 54 deletions portsyncd/portsyncd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,13 @@ void usage()
void handlePortConfigFile(ProducerStateTable &p, string file, bool warm);
bool 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;
string port_config_file;
map<string, KeyOpFieldsValuesTuple> port_cfg_map;

while ((opt = getopt(argc, argv, "p:v:h")) != -1 )
{
Expand Down Expand Up @@ -103,7 +101,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,29 +132,6 @@ int main(int argc, char **argv)

g_init = true;
}
if (!port_cfg_map.empty())
{
handlePortConfig(p, port_cfg_map);
}
}

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);
}
}
}
Expand Down Expand Up @@ -313,31 +287,3 @@ void handlePortConfigFile(ProducerStateTable &p, string file, bool warm)
}
}

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++;
}
}
}
Loading

0 comments on commit 8c9eac6

Please sign in to comment.