From 3ed4cc80476d3fff7ad2ed03e74c6cb1844b79af Mon Sep 17 00:00:00 2001 From: Jipan Yang Date: Fri, 20 Oct 2017 22:40:19 -0700 Subject: [PATCH 1/6] configDB enforcer for interface Support VLAN interface only for now Signed-off-by: Jipan Yang --- Makefile.am | 2 +- configure.ac | 1 + intfconfd/Makefile.am | 15 +++++ intfconfd/intfconf.cpp | 120 ++++++++++++++++++++++++++++++++++++++++ intfconfd/intfconf.h | 31 +++++++++++ intfconfd/intfconfd.cpp | 78 ++++++++++++++++++++++++++ 6 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 intfconfd/Makefile.am create mode 100644 intfconfd/intfconf.cpp create mode 100644 intfconfd/intfconf.h create mode 100644 intfconfd/intfconfd.cpp diff --git a/Makefile.am b/Makefile.am index 20553983e15..784d731f3ac 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,4 +1,4 @@ -SUBDIRS = fpmsyncd neighsyncd intfsyncd portsyncd orchagent swssconfig +SUBDIRS = fpmsyncd neighsyncd intfsyncd portsyncd orchagent swssconfig intfconfd if HAVE_LIBTEAM SUBDIRS += teamsyncd diff --git a/configure.ac b/configure.ac index 145a0a1d6ad..dcba39955cd 100644 --- a/configure.ac +++ b/configure.ac @@ -87,6 +87,7 @@ AC_CONFIG_FILES([ teamsyncd/Makefile swssconfig/Makefile tests/Makefile + intfconfd/Makefile ]) AC_OUTPUT diff --git a/intfconfd/Makefile.am b/intfconfd/Makefile.am new file mode 100644 index 00000000000..613aa1428ad --- /dev/null +++ b/intfconfd/Makefile.am @@ -0,0 +1,15 @@ +INCLUDES = -I $(top_srcdir) -I $(top_srcdir)/cfgorch + +bin_PROGRAMS = intfconfd + +if DEBUG +DBGFLAGS = -ggdb -DDEBUG +else +DBGFLAGS = -g +endif + +intfconfd_SOURCES = intfconfd.cpp intfconf.cpp $(top_srcdir)/cfgorch/cfgorch.cpp + +intfconfd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) +intfconfd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) +intfconfd_LDADD = -lswsscommon diff --git a/intfconfd/intfconf.cpp b/intfconfd/intfconf.cpp new file mode 100644 index 00000000000..552954d16d0 --- /dev/null +++ b/intfconfd/intfconf.cpp @@ -0,0 +1,120 @@ +#include +#include "logger.h" +#include "dbconnector.h" +#include "producerstatetable.h" +#include "tokenize.h" +#include "ipprefix.h" +#include "intfconf.h" +#include "exec.h" + +using namespace std; +using namespace swss; + +#define VLAN_PREFIX "Vlan" +#define LAG_PREFIX "PortChannel" + +IntfConf::IntfConf(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, vector tableNames) : + CfgOrch(cfgDb, tableNames), + m_cfgIntfTable(cfgDb, CFG_INTF_TABLE_NAME, CONFIGDB_TABLE_NAME_SEPARATOR), + m_cfgVlanIntfTable(cfgDb, CFG_VLAN_INTF_TABLE_NAME, CONFIGDB_TABLE_NAME_SEPARATOR), + m_statePortTable(stateDb, STATE_PORT_TABLE_NAME, CONFIGDB_TABLE_NAME_SEPARATOR), + m_stateLagTable(stateDb, STATE_LAG_TABLE_NAME, CONFIGDB_TABLE_NAME_SEPARATOR), + m_stateVlanTable(stateDb, STATE_VLAN_TABLE_NAME, CONFIGDB_TABLE_NAME_SEPARATOR), + m_appIntfTableProducer(appDb, APP_INTF_TABLE_NAME) +{ + +} + +void IntfConf::syncCfgDB() +{ + CfgOrch::syncCfgDB(CFG_INTF_TABLE_NAME, m_cfgIntfTable); + CfgOrch::syncCfgDB(CFG_VLAN_INTF_TABLE_NAME, m_cfgVlanIntfTable); +} + +bool IntfConf::setIntfIp(string &alias, string &opCmd, string &ipPrefixStr) +{ + string cmd, res; + + cmd = "ip address " + opCmd + " "; + cmd += ipPrefixStr + " dev " + alias; + swss::exec(cmd, res); + return true; +} + +bool IntfConf::isIntfStateOk(string &alias) +{ + vector temp; + + if (!alias.compare(0, strlen(VLAN_PREFIX), VLAN_PREFIX)) + { + if (m_stateVlanTable.get(alias, temp)) + { + SWSS_LOG_DEBUG("Port %s is ready\n", alias.c_str()); + return true; + } + } + else if (!alias.compare(0, strlen(LAG_PREFIX), LAG_PREFIX)) + { + if (m_stateLagTable.get(alias, temp)) + { + SWSS_LOG_DEBUG("Lag %s is ready\n", alias.c_str()); + return true; + } + } + else if (m_statePortTable.get(alias, temp)) + { + SWSS_LOG_DEBUG("Port %s is ready\n", alias.c_str()); + return true; + } + + return false; +} +void IntfConf::doTask(Consumer &consumer) +{ + SWSS_LOG_ENTER(); + + auto it = consumer.m_toSync.begin(); + while (it != consumer.m_toSync.end()) + { + KeyOpFieldsValuesTuple t = it->second; + + string keySeparator = CONFIGDB_KEY_SEPARATOR; + vector keys = tokenize(kfvKey(t), keySeparator[0]); + string alias(keys[0]); + + if (alias.compare(0, strlen(VLAN_PREFIX), VLAN_PREFIX)) + { + /* handle IP over vlan Only for now, skip the rest */ + it = consumer.m_toSync.erase(it); + continue; + } + + IpPrefix ip_prefix(kfvKey(t).substr(kfvKey(t).find(CONFIGDB_KEY_SEPARATOR)+1)); + + SWSS_LOG_DEBUG("intfs doTask: %s", (dumpTuple(consumer, t)).c_str()); + + string op = kfvOp(t); + if (op == SET_COMMAND) + { + /* Don't proceed if port/lag/VLAN is not ready yet */ + if (!isIntfStateOk(alias)) + { + SWSS_LOG_DEBUG("Interface is not ready, skipping %s", kfvKey(t).c_str()); + it++; + continue; + } + string opCmd("add"); + string ipPrefixStr = ip_prefix.to_string(); + setIntfIp(alias, opCmd, ipPrefixStr); + } + else if (op == DEL_COMMAND) + { + string opCmd("del"); + string ipPrefixStr = ip_prefix.to_string(); + setIntfIp(alias, opCmd, ipPrefixStr); + } + + it = consumer.m_toSync.erase(it); + continue; + } +} \ No newline at end of file diff --git a/intfconfd/intfconf.h b/intfconfd/intfconf.h new file mode 100644 index 00000000000..9aa06cb82d7 --- /dev/null +++ b/intfconfd/intfconf.h @@ -0,0 +1,31 @@ +#ifndef __INTFCONF__ +#define __INTFCONF__ + +#include "dbconnector.h" +#include "producerstatetable.h" +#include "cfgorch.h" + +#include +#include + +namespace swss { + +class IntfConf : public CfgOrch +{ +public: + IntfConf(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, vector tableNames); + void syncCfgDB(); + +private: + ProducerStateTable m_appIntfTableProducer; + Table m_cfgIntfTable, m_cfgVlanIntfTable; + Table m_statePortTable, m_stateLagTable, m_stateVlanTable; + + bool setIntfIp(string &alias, string &opCmd, string &ipPrefixStr); + void doTask(Consumer &consumer); + bool isIntfStateOk(string &alias); +}; + +} + +#endif diff --git a/intfconfd/intfconfd.cpp b/intfconfd/intfconfd.cpp new file mode 100644 index 00000000000..5a56fa7d8e3 --- /dev/null +++ b/intfconfd/intfconfd.cpp @@ -0,0 +1,78 @@ +#include +#include +#include "dbconnector.h" +#include "select.h" +#include "exec.h" +#include "schema.h" +#include "intfconf.h" + +using namespace std; +using namespace swss; + +/* select() function timeout retry time, in millisecond */ +#define SELECT_TIMEOUT 1000 + +int main(int argc, char **argv) +{ + Logger::linkToDbNative("intfconfd"); + SWSS_LOG_ENTER(); + + SWSS_LOG_NOTICE("--- Starting intfconfd ---"); + + try + { + vector cfg_intf_tables = { + CFG_INTF_TABLE_NAME, + CFG_LAG_INTF_TABLE_NAME, + CFG_VLAN_INTF_TABLE_NAME, + }; + + DBConnector cfgDb(CONFIG_DB, DBConnector::DEFAULT_UNIXSOCKET, 0); + DBConnector appDb(APPL_DB, DBConnector::DEFAULT_UNIXSOCKET, 0); + DBConnector stateDb(STATE_DB, DBConnector::DEFAULT_UNIXSOCKET, 0); + + IntfConf intfconf(&cfgDb, &appDb, &stateDb, cfg_intf_tables); + + // TODO: add tables in stateDB which interface depends on to monitor list + std::vector cfgOrchList = {&intfconf}; + + swss::Select s; + for (CfgOrch *o : cfgOrchList) + { + s.addSelectables(o->getSelectables()); + } + + SWSS_LOG_NOTICE("starting main loop"); + while (true) + { + Selectable *sel; + int fd, ret; + + ret = s.select(&sel, &fd, SELECT_TIMEOUT); + if (ret == Select::ERROR) + { + SWSS_LOG_NOTICE("Error: %s!\n", strerror(errno)); + continue; + } + if (ret == Select::TIMEOUT) + { + ((CfgOrch *)&intfconf)->doTask(); + continue; + } + + for (CfgOrch *o : cfgOrchList) + { + TableConsumable *c = (TableConsumable *)sel; + if (o->hasSelectable(c)) + { + o->execute(c->getTableName()); + } + } + } + } + catch(const std::exception &e) + { + SWSS_LOG_ERROR("Runtime error: %s", e.what()); + } + return 0; +} From 7c7764cb143aa2f4cc7f8c74df8b0d44fa0264a5 Mon Sep 17 00:00:00 2001 From: Jipan Yang Date: Tue, 24 Oct 2017 22:43:50 -0700 Subject: [PATCH 2/6] Use OrchBase for configDB intf configuration enforcer Signed-off-by: Jipan Yang --- .gitignore | 1 + Makefile.am | 2 +- cfgagent/Makefile.am | 17 +++++++++++++++++ {intfconfd => cfgagent}/intfconf.cpp | 16 +++++++++++----- {intfconfd => cfgagent}/intfconf.h | 4 ++-- {intfconfd => cfgagent}/intfconfd.cpp | 10 +++++----- configure.ac | 2 +- intfconfd/Makefile.am | 15 --------------- 8 files changed, 38 insertions(+), 29 deletions(-) create mode 100644 cfgagent/Makefile.am rename {intfconfd => cfgagent}/intfconf.cpp (87%) rename {intfconfd => cfgagent}/intfconf.h (91%) rename {intfconfd => cfgagent}/intfconfd.cpp (89%) delete mode 100644 intfconfd/Makefile.am diff --git a/.gitignore b/.gitignore index ba3fa90eb41..8b5e426d7c8 100644 --- a/.gitignore +++ b/.gitignore @@ -43,6 +43,7 @@ deps/ teamsyncd/teamsyncd fpmsyncd/fpmsyncd intfsyncd/intfsyncd +cfgagent/intfconfd neighsyncd/neighsyncd portsyncd/portsyncd orchagent/orchagent diff --git a/Makefile.am b/Makefile.am index 784d731f3ac..3811bdbc2be 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,4 +1,4 @@ -SUBDIRS = fpmsyncd neighsyncd intfsyncd portsyncd orchagent swssconfig intfconfd +SUBDIRS = fpmsyncd neighsyncd intfsyncd portsyncd orchagent swssconfig cfgagent if HAVE_LIBTEAM SUBDIRS += teamsyncd diff --git a/cfgagent/Makefile.am b/cfgagent/Makefile.am new file mode 100644 index 00000000000..460247a9ed4 --- /dev/null +++ b/cfgagent/Makefile.am @@ -0,0 +1,17 @@ +INCLUDES = -I $(top_srcdir) -I $(top_srcdir)/orchagent +CFLAGS_SAI = -I /usr/include/sai + +bin_PROGRAMS = intfconfd + +if DEBUG +DBGFLAGS = -ggdb -DDEBUG +else +DBGFLAGS = -g +endif + + +intfconfd_SOURCES = intfconfd.cpp intfconf.cpp $(top_srcdir)/orchagent/orchbase.cpp +intfconfd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) +intfconfd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) +intfconfd_LDADD = -lswsscommon + diff --git a/intfconfd/intfconf.cpp b/cfgagent/intfconf.cpp similarity index 87% rename from intfconfd/intfconf.cpp rename to cfgagent/intfconf.cpp index 552954d16d0..508abc0687f 100644 --- a/intfconfd/intfconf.cpp +++ b/cfgagent/intfconf.cpp @@ -14,7 +14,7 @@ using namespace swss; #define LAG_PREFIX "PortChannel" IntfConf::IntfConf(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, vector tableNames) : - CfgOrch(cfgDb, tableNames), + OrchBase(cfgDb, tableNames), m_cfgIntfTable(cfgDb, CFG_INTF_TABLE_NAME, CONFIGDB_TABLE_NAME_SEPARATOR), m_cfgVlanIntfTable(cfgDb, CFG_VLAN_INTF_TABLE_NAME, CONFIGDB_TABLE_NAME_SEPARATOR), m_statePortTable(stateDb, STATE_PORT_TABLE_NAME, CONFIGDB_TABLE_NAME_SEPARATOR), @@ -22,13 +22,12 @@ IntfConf::IntfConf(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, m_stateVlanTable(stateDb, STATE_VLAN_TABLE_NAME, CONFIGDB_TABLE_NAME_SEPARATOR), m_appIntfTableProducer(appDb, APP_INTF_TABLE_NAME) { - } void IntfConf::syncCfgDB() { - CfgOrch::syncCfgDB(CFG_INTF_TABLE_NAME, m_cfgIntfTable); - CfgOrch::syncCfgDB(CFG_VLAN_INTF_TABLE_NAME, m_cfgVlanIntfTable); + OrchBase::syncDB(CFG_INTF_TABLE_NAME, m_cfgIntfTable); + OrchBase::syncDB(CFG_VLAN_INTF_TABLE_NAME, m_cfgVlanIntfTable); } bool IntfConf::setIntfIp(string &alias, string &opCmd, string &ipPrefixStr) @@ -89,7 +88,14 @@ void IntfConf::doTask(Consumer &consumer) continue; } - IpPrefix ip_prefix(kfvKey(t).substr(kfvKey(t).find(CONFIGDB_KEY_SEPARATOR)+1)); + size_t pos = kfvKey(t).find(CONFIGDB_KEY_SEPARATOR); + if (pos == string::npos) + { + SWSS_LOG_DEBUG("Invalid key %s", kfvKey(t).c_str()); + it = consumer.m_toSync.erase(it); + continue; + } + IpPrefix ip_prefix(kfvKey(t).substr(pos+1)); SWSS_LOG_DEBUG("intfs doTask: %s", (dumpTuple(consumer, t)).c_str()); diff --git a/intfconfd/intfconf.h b/cfgagent/intfconf.h similarity index 91% rename from intfconfd/intfconf.h rename to cfgagent/intfconf.h index 9aa06cb82d7..5a2d417b553 100644 --- a/intfconfd/intfconf.h +++ b/cfgagent/intfconf.h @@ -3,14 +3,14 @@ #include "dbconnector.h" #include "producerstatetable.h" -#include "cfgorch.h" +#include "orchbase.h" #include #include namespace swss { -class IntfConf : public CfgOrch +class IntfConf : public OrchBase { public: IntfConf(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, vector tableNames); diff --git a/intfconfd/intfconfd.cpp b/cfgagent/intfconfd.cpp similarity index 89% rename from intfconfd/intfconfd.cpp rename to cfgagent/intfconfd.cpp index 5a56fa7d8e3..ad82bf15bb5 100644 --- a/intfconfd/intfconfd.cpp +++ b/cfgagent/intfconfd.cpp @@ -34,10 +34,10 @@ int main(int argc, char **argv) IntfConf intfconf(&cfgDb, &appDb, &stateDb, cfg_intf_tables); // TODO: add tables in stateDB which interface depends on to monitor list - std::vector cfgOrchList = {&intfconf}; + std::vector cfgOrchList = {&intfconf}; swss::Select s; - for (CfgOrch *o : cfgOrchList) + for (OrchBase *o : cfgOrchList) { s.addSelectables(o->getSelectables()); } @@ -56,11 +56,11 @@ int main(int argc, char **argv) } if (ret == Select::TIMEOUT) { - ((CfgOrch *)&intfconf)->doTask(); + ((OrchBase *)&intfconf)->doTask(); continue; } - for (CfgOrch *o : cfgOrchList) + for (OrchBase *o : cfgOrchList) { TableConsumable *c = (TableConsumable *)sel; if (o->hasSelectable(c)) @@ -74,5 +74,5 @@ int main(int argc, char **argv) { SWSS_LOG_ERROR("Runtime error: %s", e.what()); } - return 0; + return -1; } diff --git a/configure.ac b/configure.ac index dcba39955cd..ae5b49a8b20 100644 --- a/configure.ac +++ b/configure.ac @@ -87,7 +87,7 @@ AC_CONFIG_FILES([ teamsyncd/Makefile swssconfig/Makefile tests/Makefile - intfconfd/Makefile + cfgagent/Makefile ]) AC_OUTPUT diff --git a/intfconfd/Makefile.am b/intfconfd/Makefile.am deleted file mode 100644 index 613aa1428ad..00000000000 --- a/intfconfd/Makefile.am +++ /dev/null @@ -1,15 +0,0 @@ -INCLUDES = -I $(top_srcdir) -I $(top_srcdir)/cfgorch - -bin_PROGRAMS = intfconfd - -if DEBUG -DBGFLAGS = -ggdb -DDEBUG -else -DBGFLAGS = -g -endif - -intfconfd_SOURCES = intfconfd.cpp intfconf.cpp $(top_srcdir)/cfgorch/cfgorch.cpp - -intfconfd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) -intfconfd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) -intfconfd_LDADD = -lswsscommon From ac1b6baff18f15c20e79019e6e0d6c28cb700bfc Mon Sep 17 00:00:00 2001 From: Jipan Yang Date: Wed, 25 Oct 2017 15:31:41 -0700 Subject: [PATCH 3/6] Rename intfconf to intfmgr Signed-off-by: Jipan Yang --- .gitignore | 2 +- cfgagent/Makefile.am | 11 +++++---- cfgagent/intfconf.h | 31 ------------------------ cfgagent/{intfconf.cpp => intfmgr.cpp} | 24 ++++++++++-------- cfgagent/intfmgr.h | 31 ++++++++++++++++++++++++ cfgagent/{intfconfd.cpp => intfmgrd.cpp} | 12 ++++----- 6 files changed, 58 insertions(+), 53 deletions(-) delete mode 100644 cfgagent/intfconf.h rename cfgagent/{intfconf.cpp => intfmgr.cpp} (80%) create mode 100644 cfgagent/intfmgr.h rename cfgagent/{intfconfd.cpp => intfmgrd.cpp} (85%) diff --git a/.gitignore b/.gitignore index 8b5e426d7c8..b935894df9f 100644 --- a/.gitignore +++ b/.gitignore @@ -43,7 +43,7 @@ deps/ teamsyncd/teamsyncd fpmsyncd/fpmsyncd intfsyncd/intfsyncd -cfgagent/intfconfd +cfgagent/intfmgrd neighsyncd/neighsyncd portsyncd/portsyncd orchagent/orchagent diff --git a/cfgagent/Makefile.am b/cfgagent/Makefile.am index 460247a9ed4..a58f90a3b0d 100644 --- a/cfgagent/Makefile.am +++ b/cfgagent/Makefile.am @@ -1,7 +1,7 @@ INCLUDES = -I $(top_srcdir) -I $(top_srcdir)/orchagent CFLAGS_SAI = -I /usr/include/sai -bin_PROGRAMS = intfconfd +bin_PROGRAMS = intfmgrd if DEBUG DBGFLAGS = -ggdb -DDEBUG @@ -10,8 +10,9 @@ DBGFLAGS = -g endif -intfconfd_SOURCES = intfconfd.cpp intfconf.cpp $(top_srcdir)/orchagent/orchbase.cpp -intfconfd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) -intfconfd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) -intfconfd_LDADD = -lswsscommon +intfmgrd_SOURCES = intfmgrd.cpp intfmgr.cpp $(top_srcdir)/orchagent/orchbase.cpp +intfmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) +intfmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) +intfmgrd_LDADD = -lswsscommon +SUBDIRS = cfgmgrtest diff --git a/cfgagent/intfconf.h b/cfgagent/intfconf.h deleted file mode 100644 index 5a2d417b553..00000000000 --- a/cfgagent/intfconf.h +++ /dev/null @@ -1,31 +0,0 @@ -#ifndef __INTFCONF__ -#define __INTFCONF__ - -#include "dbconnector.h" -#include "producerstatetable.h" -#include "orchbase.h" - -#include -#include - -namespace swss { - -class IntfConf : public OrchBase -{ -public: - IntfConf(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, vector tableNames); - void syncCfgDB(); - -private: - ProducerStateTable m_appIntfTableProducer; - Table m_cfgIntfTable, m_cfgVlanIntfTable; - Table m_statePortTable, m_stateLagTable, m_stateVlanTable; - - bool setIntfIp(string &alias, string &opCmd, string &ipPrefixStr); - void doTask(Consumer &consumer); - bool isIntfStateOk(string &alias); -}; - -} - -#endif diff --git a/cfgagent/intfconf.cpp b/cfgagent/intfmgr.cpp similarity index 80% rename from cfgagent/intfconf.cpp rename to cfgagent/intfmgr.cpp index 508abc0687f..0b2788a021d 100644 --- a/cfgagent/intfconf.cpp +++ b/cfgagent/intfmgr.cpp @@ -4,7 +4,7 @@ #include "producerstatetable.h" #include "tokenize.h" #include "ipprefix.h" -#include "intfconf.h" +#include "intfmgr.h" #include "exec.h" using namespace std; @@ -13,7 +13,7 @@ using namespace swss; #define VLAN_PREFIX "Vlan" #define LAG_PREFIX "PortChannel" -IntfConf::IntfConf(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, vector tableNames) : +IntfMgr::IntfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, const vector &tableNames) : OrchBase(cfgDb, tableNames), m_cfgIntfTable(cfgDb, CFG_INTF_TABLE_NAME, CONFIGDB_TABLE_NAME_SEPARATOR), m_cfgVlanIntfTable(cfgDb, CFG_VLAN_INTF_TABLE_NAME, CONFIGDB_TABLE_NAME_SEPARATOR), @@ -24,23 +24,22 @@ IntfConf::IntfConf(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, { } -void IntfConf::syncCfgDB() +void IntfMgr::syncCfgDB() { OrchBase::syncDB(CFG_INTF_TABLE_NAME, m_cfgIntfTable); OrchBase::syncDB(CFG_VLAN_INTF_TABLE_NAME, m_cfgVlanIntfTable); } -bool IntfConf::setIntfIp(string &alias, string &opCmd, string &ipPrefixStr) +bool IntfMgr::setIntfIp(const string &alias, const string &opCmd, const string &ipPrefixStr) { string cmd, res; - cmd = "ip address " + opCmd + " "; - cmd += ipPrefixStr + " dev " + alias; + cmd = "ip address " + opCmd + " " + ipPrefixStr + " dev " + alias;; swss::exec(cmd, res); return true; } -bool IntfConf::isIntfStateOk(string &alias) +bool IntfMgr::isIntfStateOk(const string &alias) { vector temp; @@ -48,7 +47,7 @@ bool IntfConf::isIntfStateOk(string &alias) { if (m_stateVlanTable.get(alias, temp)) { - SWSS_LOG_DEBUG("Port %s is ready\n", alias.c_str()); + SWSS_LOG_DEBUG("Vlan %s is ready\n", alias.c_str()); return true; } } @@ -68,7 +67,7 @@ bool IntfConf::isIntfStateOk(string &alias) return false; } -void IntfConf::doTask(Consumer &consumer) +void IntfMgr::doTask(Consumer &consumer) { SWSS_LOG_ENTER(); @@ -102,7 +101,12 @@ void IntfConf::doTask(Consumer &consumer) string op = kfvOp(t); if (op == SET_COMMAND) { - /* Don't proceed if port/lag/VLAN is not ready yet */ + /* + * Don't proceed if port/lag/VLAN is not ready yet. + * The pending task will be checked periodially and retried. + * TODO: Subscribe to stateDB for port/lag/VLAN state and retry + * pending tasks immediately upon state change. + */ if (!isIntfStateOk(alias)) { SWSS_LOG_DEBUG("Interface is not ready, skipping %s", kfvKey(t).c_str()); diff --git a/cfgagent/intfmgr.h b/cfgagent/intfmgr.h new file mode 100644 index 00000000000..797817df714 --- /dev/null +++ b/cfgagent/intfmgr.h @@ -0,0 +1,31 @@ +#ifndef __INTFMGR__ +#define __INTFMGR__ + +#include "dbconnector.h" +#include "producerstatetable.h" +#include "orchbase.h" + +#include +#include + +namespace swss { + +class IntfMgr : public OrchBase +{ +public: + IntfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, const vector &tableNames); + void syncCfgDB(); + +private: + ProducerStateTable m_appIntfTableProducer; + Table m_cfgIntfTable, m_cfgVlanIntfTable; + Table m_statePortTable, m_stateLagTable, m_stateVlanTable; + + bool setIntfIp(const string &alias, const string &opCmd, const string &ipPrefixStr); + void doTask(Consumer &consumer); + bool isIntfStateOk(const string &alias); +}; + +} + +#endif diff --git a/cfgagent/intfconfd.cpp b/cfgagent/intfmgrd.cpp similarity index 85% rename from cfgagent/intfconfd.cpp rename to cfgagent/intfmgrd.cpp index ad82bf15bb5..0f52a851427 100644 --- a/cfgagent/intfconfd.cpp +++ b/cfgagent/intfmgrd.cpp @@ -4,7 +4,7 @@ #include "select.h" #include "exec.h" #include "schema.h" -#include "intfconf.h" +#include "intfmgr.h" using namespace std; using namespace swss; @@ -14,10 +14,10 @@ using namespace swss; int main(int argc, char **argv) { - Logger::linkToDbNative("intfconfd"); + Logger::linkToDbNative("intfmgrd"); SWSS_LOG_ENTER(); - SWSS_LOG_NOTICE("--- Starting intfconfd ---"); + SWSS_LOG_NOTICE("--- Starting intfmgrd ---"); try { @@ -31,10 +31,10 @@ int main(int argc, char **argv) DBConnector appDb(APPL_DB, DBConnector::DEFAULT_UNIXSOCKET, 0); DBConnector stateDb(STATE_DB, DBConnector::DEFAULT_UNIXSOCKET, 0); - IntfConf intfconf(&cfgDb, &appDb, &stateDb, cfg_intf_tables); + IntfMgr intfmgr(&cfgDb, &appDb, &stateDb, cfg_intf_tables); // TODO: add tables in stateDB which interface depends on to monitor list - std::vector cfgOrchList = {&intfconf}; + std::vector cfgOrchList = {&intfmgr}; swss::Select s; for (OrchBase *o : cfgOrchList) @@ -56,7 +56,7 @@ int main(int argc, char **argv) } if (ret == Select::TIMEOUT) { - ((OrchBase *)&intfconf)->doTask(); + ((OrchBase *)&intfmgr)->doTask(); continue; } From 416f4f453bda7a4c4e18ea965112cff79e924ac8 Mon Sep 17 00:00:00 2001 From: Jipan Yang Date: Sun, 29 Oct 2017 18:18:38 +0800 Subject: [PATCH 4/6] Use shell full path command macros Signed-off-by: Jipan Yang --- cfgagent/intfmgr.cpp | 14 ++++++++------ cfgagent/intfmgrd.cpp | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/cfgagent/intfmgr.cpp b/cfgagent/intfmgr.cpp index 0b2788a021d..bb4d4146245 100644 --- a/cfgagent/intfmgr.cpp +++ b/cfgagent/intfmgr.cpp @@ -6,6 +6,7 @@ #include "ipprefix.h" #include "intfmgr.h" #include "exec.h" +#include "shellcmd.h" using namespace std; using namespace swss; @@ -32,10 +33,11 @@ void IntfMgr::syncCfgDB() bool IntfMgr::setIntfIp(const string &alias, const string &opCmd, const string &ipPrefixStr) { - string cmd, res; + stringstream cmd; + string res; - cmd = "ip address " + opCmd + " " + ipPrefixStr + " dev " + alias;; - swss::exec(cmd, res); + cmd << IP_CMD << " address " << opCmd << " " << ipPrefixStr << " dev " << alias;; + swss::exec(cmd.str(), res); return true; } @@ -47,7 +49,7 @@ bool IntfMgr::isIntfStateOk(const string &alias) { if (m_stateVlanTable.get(alias, temp)) { - SWSS_LOG_DEBUG("Vlan %s is ready\n", alias.c_str()); + SWSS_LOG_DEBUG("Vlan %s is ready", alias.c_str()); return true; } } @@ -55,13 +57,13 @@ bool IntfMgr::isIntfStateOk(const string &alias) { if (m_stateLagTable.get(alias, temp)) { - SWSS_LOG_DEBUG("Lag %s is ready\n", alias.c_str()); + SWSS_LOG_DEBUG("Lag %s is ready", alias.c_str()); return true; } } else if (m_statePortTable.get(alias, temp)) { - SWSS_LOG_DEBUG("Port %s is ready\n", alias.c_str()); + SWSS_LOG_DEBUG("Port %s is ready", alias.c_str()); return true; } diff --git a/cfgagent/intfmgrd.cpp b/cfgagent/intfmgrd.cpp index 0f52a851427..5e03d3e1f39 100644 --- a/cfgagent/intfmgrd.cpp +++ b/cfgagent/intfmgrd.cpp @@ -51,7 +51,7 @@ int main(int argc, char **argv) ret = s.select(&sel, &fd, SELECT_TIMEOUT); if (ret == Select::ERROR) { - SWSS_LOG_NOTICE("Error: %s!\n", strerror(errno)); + SWSS_LOG_NOTICE("Error: %s!", strerror(errno)); continue; } if (ret == Select::TIMEOUT) From 16bfac184c036a49ccadb04d90b9be046464ac4b Mon Sep 17 00:00:00 2001 From: Jipan Yang Date: Tue, 31 Oct 2017 10:46:20 +0800 Subject: [PATCH 5/6] Optimize shell command execution error handling Signed-off-by: Jipan Yang --- cfgagent/intfmgr.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cfgagent/intfmgr.cpp b/cfgagent/intfmgr.cpp index bb4d4146245..6e2b3f097f0 100644 --- a/cfgagent/intfmgr.cpp +++ b/cfgagent/intfmgr.cpp @@ -37,8 +37,12 @@ bool IntfMgr::setIntfIp(const string &alias, const string &opCmd, const string & string res; cmd << IP_CMD << " address " << opCmd << " " << ipPrefixStr << " dev " << alias;; - swss::exec(cmd.str(), res); - return true; + int ret = swss::exec(cmd.str(), res); + if (ret == 0) + { + return true; + } + return false; } bool IntfMgr::isIntfStateOk(const string &alias) @@ -129,4 +133,4 @@ void IntfMgr::doTask(Consumer &consumer) it = consumer.m_toSync.erase(it); continue; } -} \ No newline at end of file +} From c24f0c0d920189fe73b5b0c0e4a03003227bc3ff Mon Sep 17 00:00:00 2001 From: Jipan Yang Date: Thu, 2 Nov 2017 18:44:42 +0800 Subject: [PATCH 6/6] Use Orch class for intfmgrd orchestration Signed-off-by: Jipan Yang --- .gitignore | 2 +- Makefile.am | 2 +- {cfgagent => cfgmgr}/Makefile.am | 3 ++- {cfgagent => cfgmgr}/intfmgr.cpp | 8 +------- {cfgagent => cfgmgr}/intfmgr.h | 5 ++--- {cfgagent => cfgmgr}/intfmgrd.cpp | 27 +++++++++++++++++++++++---- configure.ac | 2 +- 7 files changed, 31 insertions(+), 18 deletions(-) rename {cfgagent => cfgmgr}/Makefile.am (94%) rename {cfgagent => cfgmgr}/intfmgr.cpp (95%) rename {cfgagent => cfgmgr}/intfmgr.h (89%) rename {cfgagent => cfgmgr}/intfmgrd.cpp (73%) diff --git a/.gitignore b/.gitignore index b935894df9f..d1f93831555 100644 --- a/.gitignore +++ b/.gitignore @@ -43,7 +43,7 @@ deps/ teamsyncd/teamsyncd fpmsyncd/fpmsyncd intfsyncd/intfsyncd -cfgagent/intfmgrd +cfgmgr/intfmgrd neighsyncd/neighsyncd portsyncd/portsyncd orchagent/orchagent diff --git a/Makefile.am b/Makefile.am index 3811bdbc2be..a86163ae2b8 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,4 +1,4 @@ -SUBDIRS = fpmsyncd neighsyncd intfsyncd portsyncd orchagent swssconfig cfgagent +SUBDIRS = fpmsyncd neighsyncd intfsyncd portsyncd orchagent swssconfig cfgmgr if HAVE_LIBTEAM SUBDIRS += teamsyncd diff --git a/cfgagent/Makefile.am b/cfgmgr/Makefile.am similarity index 94% rename from cfgagent/Makefile.am rename to cfgmgr/Makefile.am index a58f90a3b0d..4718f2ebf81 100644 --- a/cfgagent/Makefile.am +++ b/cfgmgr/Makefile.am @@ -10,7 +10,8 @@ DBGFLAGS = -g endif -intfmgrd_SOURCES = intfmgrd.cpp intfmgr.cpp $(top_srcdir)/orchagent/orchbase.cpp + +intfmgrd_SOURCES = intfmgrd.cpp intfmgr.cpp $(top_srcdir)/orchagent/orch.cpp shellcmd.h intfmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) intfmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) intfmgrd_LDADD = -lswsscommon diff --git a/cfgagent/intfmgr.cpp b/cfgmgr/intfmgr.cpp similarity index 95% rename from cfgagent/intfmgr.cpp rename to cfgmgr/intfmgr.cpp index 6e2b3f097f0..5cc28065370 100644 --- a/cfgagent/intfmgr.cpp +++ b/cfgmgr/intfmgr.cpp @@ -15,7 +15,7 @@ using namespace swss; #define LAG_PREFIX "PortChannel" IntfMgr::IntfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, const vector &tableNames) : - OrchBase(cfgDb, tableNames), + Orch(cfgDb, tableNames), m_cfgIntfTable(cfgDb, CFG_INTF_TABLE_NAME, CONFIGDB_TABLE_NAME_SEPARATOR), m_cfgVlanIntfTable(cfgDb, CFG_VLAN_INTF_TABLE_NAME, CONFIGDB_TABLE_NAME_SEPARATOR), m_statePortTable(stateDb, STATE_PORT_TABLE_NAME, CONFIGDB_TABLE_NAME_SEPARATOR), @@ -25,12 +25,6 @@ IntfMgr::IntfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, c { } -void IntfMgr::syncCfgDB() -{ - OrchBase::syncDB(CFG_INTF_TABLE_NAME, m_cfgIntfTable); - OrchBase::syncDB(CFG_VLAN_INTF_TABLE_NAME, m_cfgVlanIntfTable); -} - bool IntfMgr::setIntfIp(const string &alias, const string &opCmd, const string &ipPrefixStr) { stringstream cmd; diff --git a/cfgagent/intfmgr.h b/cfgmgr/intfmgr.h similarity index 89% rename from cfgagent/intfmgr.h rename to cfgmgr/intfmgr.h index 797817df714..a2c89d0cc24 100644 --- a/cfgagent/intfmgr.h +++ b/cfgmgr/intfmgr.h @@ -3,18 +3,17 @@ #include "dbconnector.h" #include "producerstatetable.h" -#include "orchbase.h" +#include "orch.h" #include #include namespace swss { -class IntfMgr : public OrchBase +class IntfMgr : public Orch { public: IntfMgr(DBConnector *cfgDb, DBConnector *appDb, DBConnector *stateDb, const vector &tableNames); - void syncCfgDB(); private: ProducerStateTable m_appIntfTableProducer; diff --git a/cfgagent/intfmgrd.cpp b/cfgmgr/intfmgrd.cpp similarity index 73% rename from cfgagent/intfmgrd.cpp rename to cfgmgr/intfmgrd.cpp index 5e03d3e1f39..fe72d3c5b79 100644 --- a/cfgagent/intfmgrd.cpp +++ b/cfgmgr/intfmgrd.cpp @@ -1,10 +1,13 @@ #include #include +#include #include "dbconnector.h" #include "select.h" #include "exec.h" #include "schema.h" #include "intfmgr.h" +#include +#include using namespace std; using namespace swss; @@ -12,6 +15,22 @@ using namespace swss; /* select() function timeout retry time, in millisecond */ #define SELECT_TIMEOUT 1000 +/* + * Following global variables are defined here for the purpose of + * using existing Orch class which is to be refactored soon to + * eliminate the direct exposure of the global variables. + * + * Once Orch class refactoring is done, these global variables + * should be removed from here. + */ +int gBatchSize = 0; +bool gSwssRecord = false; +bool gLogRotate = false; +ofstream gRecordOfs; +string gRecordFile; +/* Global database mutex */ +mutex gDbMutex; + int main(int argc, char **argv) { Logger::linkToDbNative("intfmgrd"); @@ -34,10 +53,10 @@ int main(int argc, char **argv) IntfMgr intfmgr(&cfgDb, &appDb, &stateDb, cfg_intf_tables); // TODO: add tables in stateDB which interface depends on to monitor list - std::vector cfgOrchList = {&intfmgr}; + std::vector cfgOrchList = {&intfmgr}; swss::Select s; - for (OrchBase *o : cfgOrchList) + for (Orch *o : cfgOrchList) { s.addSelectables(o->getSelectables()); } @@ -56,11 +75,11 @@ int main(int argc, char **argv) } if (ret == Select::TIMEOUT) { - ((OrchBase *)&intfmgr)->doTask(); + ((Orch *)&intfmgr)->doTask(); continue; } - for (OrchBase *o : cfgOrchList) + for (Orch *o : cfgOrchList) { TableConsumable *c = (TableConsumable *)sel; if (o->hasSelectable(c)) diff --git a/configure.ac b/configure.ac index ae5b49a8b20..8f14f83c789 100644 --- a/configure.ac +++ b/configure.ac @@ -87,7 +87,7 @@ AC_CONFIG_FILES([ teamsyncd/Makefile swssconfig/Makefile tests/Makefile - cfgagent/Makefile + cfgmgr/Makefile ]) AC_OUTPUT