From a5d9df4b853f706d0609b54fa2aaab8644365aeb Mon Sep 17 00:00:00 2001 From: Hrachya Mughnetsyan Date: Thu, 14 Apr 2016 18:26:38 -0700 Subject: [PATCH] [orchagent] adding support for multiple consumers in Orch classes * follow up on notes from https://github.com/Azure/sonic-swss/pull/13 Signed-off-by: Hrachya Mughnetsyan hrachya@mellanox.com --- orchagent/Makefile.am | 2 +- orchagent/intfsorch.cpp | 22 ++++++++++----------- orchagent/intfsorch.h | 2 +- orchagent/neighorch.cpp | 28 +++++++++++++-------------- orchagent/neighorch.h | 4 +--- orchagent/orch.cpp | 27 +++++++++++++------------- orchagent/orch.h | 11 ++++------- orchagent/orchdaemon.cpp | 42 +++++++++------------------------------- orchagent/orchdaemon.h | 3 --- orchagent/portsorch.cpp | 10 +++++----- orchagent/portsorch.h | 2 +- orchagent/routeorch.cpp | 30 ++++++++++++++-------------- orchagent/routeorch.h | 2 +- 13 files changed, 77 insertions(+), 108 deletions(-) diff --git a/orchagent/Makefile.am b/orchagent/Makefile.am index db68e6d06c..c227ee71d0 100644 --- a/orchagent/Makefile.am +++ b/orchagent/Makefile.am @@ -10,7 +10,7 @@ else DBGFLAGS = -g endif -orchagent_SOURCES = main.cpp orchdaemon.cpp orch.cpp routeorch.cpp neighorch.cpp intfsorch.cpp portsorch.cpp qosorch.cpp +orchagent_SOURCES = main.cpp orchdaemon.cpp orch.cpp routeorch.cpp neighorch.cpp intfsorch.cpp portsorch.cpp orchagent_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) orchagent_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_SAI) diff --git a/orchagent/intfsorch.cpp b/orchagent/intfsorch.cpp index d2ed2f05ea..83247a56bd 100644 --- a/orchagent/intfsorch.cpp +++ b/orchagent/intfsorch.cpp @@ -18,13 +18,13 @@ IntfsOrch::IntfsOrch(DBConnector *db, string tableName, PortsOrch *portsOrch) : { } -void IntfsOrch::doTask(_in_ Consumer& consumer_info) +void IntfsOrch::doTask(Consumer &consumer) { - if (consumer_info.m_toSync.empty()) + if (consumer.m_toSync.empty()) return; - auto it = consumer_info.m_toSync.begin(); - while (it != consumer_info.m_toSync.end()) + auto it = consumer.m_toSync.begin(); + while (it != consumer.m_toSync.end()) { KeyOpFieldsValuesTuple t = it->second; @@ -33,7 +33,7 @@ void IntfsOrch::doTask(_in_ Consumer& consumer_info) if (found == string::npos) { SWSS_LOG_ERROR("Failed to parse task key %s\n", key.c_str()); - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); continue; } string alias = key.substr(0, found); @@ -41,7 +41,7 @@ void IntfsOrch::doTask(_in_ Consumer& consumer_info) IpPrefix ip_prefix(key.substr(found+1)); if (!ip_prefix.isV4()) { - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); continue; } @@ -52,7 +52,7 @@ void IntfsOrch::doTask(_in_ Consumer& consumer_info) /* Duplicate entry */ if (m_intfs.find(alias) != m_intfs.end() && m_intfs[alias] == ip_prefix.getIp()) { - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); continue; } @@ -60,7 +60,7 @@ void IntfsOrch::doTask(_in_ Consumer& consumer_info) if (!m_portsOrch->getPort(alias, p)) { SWSS_LOG_ERROR("Failed to locate interface %s\n", alias.c_str()); - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); continue; } @@ -110,7 +110,7 @@ void IntfsOrch::doTask(_in_ Consumer& consumer_info) { SWSS_LOG_NOTICE("Create packet action trap route ip:%s\n", ip_prefix.getIp().to_string().c_str()); m_intfs[alias] = ip_prefix.getIp(); - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); } } else if (op == DEL_COMMAND) @@ -119,7 +119,7 @@ void IntfsOrch::doTask(_in_ Consumer& consumer_info) if (!m_portsOrch->getPort(alias, p)) { SWSS_LOG_ERROR("Failed to locate interface %s\n", alias.c_str()); - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); continue; } @@ -152,7 +152,7 @@ void IntfsOrch::doTask(_in_ Consumer& consumer_info) { SWSS_LOG_NOTICE("Remove packet action trap route ip:%s\n", ip_prefix.getIp().to_string().c_str()); m_intfs.erase(alias); - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); } } } diff --git a/orchagent/intfsorch.h b/orchagent/intfsorch.h index 6d3be0823f..aefdded3ba 100644 --- a/orchagent/intfsorch.h +++ b/orchagent/intfsorch.h @@ -16,7 +16,7 @@ class IntfsOrch : public Orch public: IntfsOrch(DBConnector *db, string tableName, PortsOrch *portsOrch); private: - virtual void doTask(_in_ Consumer& consumer_info); + void doTask(Consumer &consumer); PortsOrch *m_portsOrch; IntfsTable m_intfs; }; diff --git a/orchagent/neighorch.cpp b/orchagent/neighorch.cpp index 998025438b..7955972147 100644 --- a/orchagent/neighorch.cpp +++ b/orchagent/neighorch.cpp @@ -5,22 +5,22 @@ extern sai_neighbor_api_t* sai_neighbor_api; extern sai_next_hop_api_t* sai_next_hop_api; -void NeighOrch::doTask(_in_ Consumer& consumer_info) +void NeighOrch::doTask(Consumer &consumer) { - if (consumer_info.m_toSync.empty()) + if (consumer.m_toSync.empty()) return; - auto it = consumer_info.m_toSync.begin(); - while (it != consumer_info.m_toSync.end()) + auto it = consumer.m_toSync.begin(); + while (it != consumer.m_toSync.end()) { KeyOpFieldsValuesTuple t = it->second; string key = kfvKey(t); - size_t found = key.find(delimiter); + size_t found = key.find(':'); if (found == string::npos) { SWSS_LOG_ERROR("Failed to parse task key %s\n", key.c_str()); - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); continue; } string alias = key.substr(0, found); @@ -28,14 +28,14 @@ void NeighOrch::doTask(_in_ Consumer& consumer_info) if (!m_portsOrch->getPort(alias, p)) { - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); continue; } IpAddress ip_address(key.substr(found+1)); if (!ip_address.isV4()) { - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); continue; } @@ -49,37 +49,37 @@ void NeighOrch::doTask(_in_ Consumer& consumer_info) for (auto i = kfvFieldsValues(t).begin(); i != kfvFieldsValues(t).end(); i++) { - if (fvField(*i) == neigh_field_name) + if (fvField(*i) == "neigh") mac_address = MacAddress(fvValue(*i)); } if (m_syncdNeighbors.find(neighbor_entry) == m_syncdNeighbors.end() || m_syncdNeighbors[neighbor_entry] != mac_address) { if (addNeighbor(neighbor_entry, mac_address)) - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); else it++; } else - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); } else if (op == DEL_COMMAND) { if (m_syncdNeighbors.find(neighbor_entry) != m_syncdNeighbors.end()) { if (removeNeighbor(neighbor_entry)) - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); else it++; } /* Cannot locate the neighbor */ else - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); } else { SWSS_LOG_ERROR("Unknown operation type %s\n", op.c_str()); - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); } } } diff --git a/orchagent/neighorch.h b/orchagent/neighorch.h index 6026530eb5..f7601fdb23 100644 --- a/orchagent/neighorch.h +++ b/orchagent/neighorch.h @@ -25,13 +25,11 @@ class NeighOrch : public Orch public: NeighOrch(DBConnector *db, string tableName, PortsOrch *portsOrch, RouteOrch *routeOrch) : Orch(db, tableName), m_portsOrch(portsOrch), m_routeOrch(routeOrch) {}; - const char* const delimiter = ";"; - const char* const neigh_field_name = "neigh"; private: PortsOrch *m_portsOrch; RouteOrch *m_routeOrch; - virtual void doTask(_in_ Consumer& consumer_info); + void doTask(Consumer &consumer); NeighborTable m_syncdNeighbors; diff --git a/orchagent/orch.cpp b/orchagent/orch.cpp index 3b584d5d36..2c7dd604ee 100644 --- a/orchagent/orch.cpp +++ b/orchagent/orch.cpp @@ -6,38 +6,39 @@ using namespace swss; Orch::Orch(DBConnector *db, string tableName) : m_db(db) { - Consumer c_info(new ConsumerTable(m_db, tableName)); - m_consumer_map.insert(ConsumerMapPair(tableName, c_info)); + Consumer consumer(new ConsumerTable(m_db, tableName)); + m_consumerMap.insert(ConsumerMapPair(tableName, consumer)); } Orch::Orch(DBConnector *db, vector &tableNames) : m_db(db) { for( auto it = tableNames.begin(); it != tableNames.end(); it++) { - Consumer c_info(new ConsumerTable(m_db, *it)); - m_consumer_map.insert(ConsumerMapPair(*it, c_info)); + Consumer consumer(new ConsumerTable(m_db, *it)); + m_consumerMap.insert(ConsumerMapPair(*it, consumer)); } } Orch::~Orch() { delete(m_db); - for(auto it : m_consumer_map) { + for(auto it : m_consumerMap) { delete it.second.m_consumer; } } -void Orch::getSelectables(_out_ std::vector &consumers) +std::vector Orch::getConsumers() { - consumers.clear(); - for(auto it : m_consumer_map) { + std::vector consumers; + for(auto it : m_consumerMap) { consumers.push_back(it.second.m_consumer); } + return consumers; } -bool Orch::is_owned_consumer(ConsumerTable*consumer) const +bool Orch::hasConsumer(ConsumerTable *consumer) const { - for(auto it : m_consumer_map) { + for(auto it : m_consumerMap) { if(it.second.m_consumer == consumer) { return true; } @@ -47,8 +48,8 @@ bool Orch::is_owned_consumer(ConsumerTable*consumer) const bool Orch::execute(string tableName) { - auto consumer_it = m_consumer_map.find(tableName); - if(consumer_it == m_consumer_map.end()) { + auto consumer_it = m_consumerMap.find(tableName); + if(consumer_it == m_consumerMap.end()) { SWSS_LOG_ERROR("Unrecognized tableName:%s\n", tableName.c_str()); return false; } @@ -77,7 +78,7 @@ bool Orch::execute(string tableName) { KeyOpFieldsValuesTuple existing_data = consumer.m_toSync[key]; - auto new_values = kfvFieldsValues(new_data); + auto new_values = kfvFieldsValues(new_data); auto existing_values = kfvFieldsValues(existing_data); diff --git a/orchagent/orch.h b/orchagent/orch.h index cbc5d7ddf1..2edfcec2f6 100644 --- a/orchagent/orch.h +++ b/orchagent/orch.h @@ -32,21 +32,18 @@ class Orch Orch(DBConnector *db, vector &tableNames); ~Orch(); - void getSelectables( _out_ std::vector& selectables); - bool is_owned_consumer(ConsumerTable* s)const; + std::vector getConsumers(); + bool hasConsumer(ConsumerTable* s)const; bool execute(string tableName); - inline string getOrchName() { return m_name; } - protected: - virtual void doTask(_in_ Consumer& consumer_info) = 0; + virtual void doTask(Consumer &consumer) = 0; private: DBConnector *m_db; - const string m_name;// TODO: where is this field initialized?? protected: - ConsumerMap m_consumer_map; + ConsumerMap m_consumerMap; }; diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 9244200732..ab4462e793 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -31,16 +31,6 @@ bool OrchDaemon::init() m_intfsO = new IntfsOrch(m_applDb, APP_INTF_TABLE_NAME, m_portsO); m_routeO = new RouteOrch(m_applDb, APP_ROUTE_TABLE_NAME, m_portsO); m_neighO = new NeighOrch(m_applDb, APP_NEIGH_TABLE_NAME, m_portsO, m_routeO); - std::vector qos_tables = { - APP_TC_TO_QUEUE_MAP_TABLE_NAME, - APP_SCHEDULER_TABLE_NAME, - APP_DSCP_TO_TC_MAP_TABLE_NAME, - APP_QUEUE_TABLE_NAME, - APP_PORT_QOS_MAP_TABLE_NAME, - APP_WRED_PROFILE_TABLE_NAME - }; - - m_qosO = new QosOrch(m_applDb, qos_tables, m_portsO); m_select = new Select(); return true; @@ -49,22 +39,10 @@ bool OrchDaemon::init() void OrchDaemon::start() { int ret; - std::vector selectables; - - m_portsO->getSelectables(selectables); - m_select->addSelectables(selectables); - - m_intfsO->getSelectables(selectables); - m_select->addSelectables(selectables); - - m_neighO->getSelectables(selectables); - m_select->addSelectables(selectables); - - m_routeO->getSelectables(selectables); - m_select->addSelectables(selectables); - - m_qosO->getSelectables(selectables); - m_select->addSelectables(selectables); + m_select->addSelectables(m_portsO->getConsumers()); + m_select->addSelectables(m_intfsO->getConsumers()); + m_select->addSelectables(m_neighO->getConsumers()); + m_select->addSelectables(m_routeO->getConsumers()); while (true) { @@ -78,22 +56,20 @@ void OrchDaemon::start() if (ret == Select::TIMEOUT) continue; - Orch *o = getOrchByConsumer((ConsumerTable *)s);// NOTE: o can be nullptr - - SWSS_LOG_INFO("Get message from Orch: %s\n", o->getOrchName().c_str()); + Orch *o = getOrchByConsumer((ConsumerTable *)s); o->execute(((ConsumerTable *)s)->getTableName()); } } Orch *OrchDaemon::getOrchByConsumer(ConsumerTable *c) { - if (m_portsO->is_owned_consumer(c)) + if (m_portsO->hasConsumer(c)) return m_portsO; - if (m_intfsO->is_owned_consumer(c)) + if (m_intfsO->hasConsumer(c)) return m_intfsO; - if (m_neighO->is_owned_consumer(c)) + if (m_neighO->hasConsumer(c)) return m_neighO; - if (m_routeO->is_owned_consumer(c)) + if (m_routeO->hasConsumer(c)) return m_routeO; return nullptr; } diff --git a/orchagent/orchdaemon.h b/orchagent/orchdaemon.h index 3c5b1ead6d..7656021515 100644 --- a/orchagent/orchdaemon.h +++ b/orchagent/orchdaemon.h @@ -10,8 +10,6 @@ #include "intfsorch.h" #include "neighorch.h" #include "routeorch.h" -#include "qosorch.h" - using namespace swss; @@ -31,7 +29,6 @@ class OrchDaemon IntfsOrch *m_intfsO; NeighOrch *m_neighO; RouteOrch *m_routeO; - QosOrch *m_qosO; Select *m_select; diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 84943988e7..4e7f353c3b 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -149,13 +149,13 @@ bool PortsOrch::setPortAdminStatus(sai_object_id_t id, bool up) return true; } -void PortsOrch::doTask(_in_ Consumer& consumer_info) +void PortsOrch::doTask(Consumer &consumer) { - if (consumer_info.m_toSync.empty()) + if (consumer.m_toSync.empty()) return; - auto it = consumer_info.m_toSync.begin(); - while (it != consumer_info.m_toSync.end()) + auto it = consumer.m_toSync.begin(); + while (it != consumer.m_toSync.end()) { KeyOpFieldsValuesTuple t = it->second; @@ -243,7 +243,7 @@ void PortsOrch::doTask(_in_ Consumer& consumer_info) else SWSS_LOG_ERROR("Unknown operation type %s\n", op.c_str()); - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); } } diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 1dbbb15c26..678df22784 100644 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -17,7 +17,7 @@ class PortsOrch : public Orch bool setPortAdminStatus(sai_object_id_t id, bool up); private: - virtual void doTask(_in_ Consumer& consumer_info); + void doTask(Consumer &consumer); sai_object_id_t m_cpuPort; diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index b67a3d6b92..4bfff7958a 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -7,13 +7,13 @@ extern sai_next_hop_group_api_t* sai_next_hop_group_api; extern sai_object_id_t gVirtualRouterId; -void RouteOrch::doTask(Consumer& consumer_info) +void RouteOrch::doTask(Consumer& consumer) { - if (consumer_info.m_toSync.empty()) + if (consumer.m_toSync.empty()) return; - auto it = consumer_info.m_toSync.begin(); - while (it != consumer_info.m_toSync.end()) + auto it = consumer.m_toSync.begin(); + while (it != consumer.m_toSync.end()) { KeyOpFieldsValuesTuple t = it->second; @@ -32,13 +32,13 @@ void RouteOrch::doTask(Consumer& consumer_info) { if (op == "SET") { - /* Mark all current routes as dirty (DEL) in consumer_info.m_toSync map */ + /* Mark all current routes as dirty (DEL) in consumer.m_toSync map */ SWSS_LOG_NOTICE("Start resync routes\n"); for (auto i = m_syncdRoutes.begin(); i != m_syncdRoutes.end(); i++) { vector v; auto x = KeyOpFieldsValuesTuple(i->first.to_string(), DEL_COMMAND, v); - consumer_info.m_toSync[i->first.to_string()] = x; + consumer.m_toSync[i->first.to_string()] = x; } m_resync = true; } @@ -48,7 +48,7 @@ void RouteOrch::doTask(Consumer& consumer_info) m_resync = false; } - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); continue; } @@ -61,7 +61,7 @@ void RouteOrch::doTask(Consumer& consumer_info) IpPrefix ip_prefix = IpPrefix(key); if (!ip_prefix.isV4()) { - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); continue; } @@ -83,7 +83,7 @@ void RouteOrch::doTask(Consumer& consumer_info) // TODO: set to blackhold if nexthop is empty? if (ip_addresses.getSize() == 0) { - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); continue; } @@ -91,38 +91,38 @@ void RouteOrch::doTask(Consumer& consumer_info) // TODO: need to split aliases with ',' and verify the next hops? if (alias == "eth0" || alias == "lo" || alias == "docker0") { - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); continue; } if (m_syncdRoutes.find(ip_prefix) == m_syncdRoutes.end() || m_syncdRoutes[ip_prefix] != ip_addresses) { if (addRoute(ip_prefix, ip_addresses)) - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); else it++; } else /* Duplicate entry */ - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); } else if (op == DEL_COMMAND) { if (m_syncdRoutes.find(ip_prefix) != m_syncdRoutes.end()) { if (removeRoute(ip_prefix)) - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); else it++; } /* Cannot locate the route */ else - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); } else { SWSS_LOG_ERROR("Unknown operation type %s\n", op.c_str()); - it = consumer_info.m_toSync.erase(it); + it = consumer.m_toSync.erase(it); } } } diff --git a/orchagent/routeorch.h b/orchagent/routeorch.h index 7b7d8f2a3f..c3b3bacade 100644 --- a/orchagent/routeorch.h +++ b/orchagent/routeorch.h @@ -45,7 +45,7 @@ class RouteOrch : public Orch int getNextHopRefCount(IpAddresses); private: - virtual void doTask(Consumer& consumer_info); + void doTask(Consumer& consumer); private: PortsOrch *m_portsOrch;