From 21766889f143b3183cb2f5f61befbc96cab8ac5f Mon Sep 17 00:00:00 2001 From: pavel-shirshov Date: Fri, 8 Jun 2018 16:46:02 -0700 Subject: [PATCH] [portsorch]: Bring the physical ports up are only after buffer configuration was applied (#515) * Don't up ports, until buffer configuration is applied * Set MTU first, then set port state to UP * Introduce the test * Use logical operator && for boolean values --- orchagent/bufferorch.cpp | 78 ++++++++++++++++++++++++++++++++++- orchagent/bufferorch.h | 7 ++++ orchagent/orchdaemon.cpp | 5 ++- orchagent/portsorch.cpp | 29 ++++++++----- tests/test_port_buffer_rel.py | 33 +++++++++++++++ 5 files changed, 138 insertions(+), 14 deletions(-) create mode 100644 tests/test_port_buffer_rel.py diff --git a/orchagent/bufferorch.cpp b/orchagent/bufferorch.cpp index 170fa178b6..bfef6a4989 100644 --- a/orchagent/bufferorch.cpp +++ b/orchagent/bufferorch.cpp @@ -29,6 +29,7 @@ BufferOrch::BufferOrch(DBConnector *db, vector &tableNames) : Orch(db, t { SWSS_LOG_ENTER(); initTableHandlers(); + initBufferReadyLists(db); }; void BufferOrch::initTableHandlers() @@ -42,6 +43,59 @@ void BufferOrch::initTableHandlers() m_bufferHandlerMap.insert(buffer_handler_pair(CFG_BUFFER_PORT_EGRESS_PROFILE_LIST_NAME, &BufferOrch::processEgressBufferProfileList)); } +void BufferOrch::initBufferReadyLists(DBConnector *db) +{ + SWSS_LOG_ENTER(); + + Table pg_table(db, CFG_BUFFER_PG_TABLE_NAME); + initBufferReadyList(pg_table); + + Table queue_table(db, CFG_BUFFER_QUEUE_TABLE_NAME); + initBufferReadyList(queue_table); +} + +void BufferOrch::initBufferReadyList(Table& table) +{ + SWSS_LOG_ENTER(); + + std::vector keys; + table.getKeys(keys); + + for (const auto& key: keys) + { + m_ready_list[key] = false; + + auto tokens = tokenize(key, config_db_key_delimiter); + if (tokens.size() != 2) + { + SWSS_LOG_ERROR("Wrong format of a table '%s' key '%s'. Skip it", table.getTableName().c_str(), key.c_str()); + continue; + } + + auto port_names = tokenize(tokens[0], list_item_delimiter); + + for(const auto& port_name: port_names) + { + m_port_ready_list_ref[port_name].push_back(key); + } + } +} + +bool BufferOrch::isPortReady(const std::string& port_name) const +{ + SWSS_LOG_ENTER(); + + const auto& list_of_keys = m_port_ready_list_ref.at(port_name); + + bool result = true; + for (const auto& key: list_of_keys) + { + result = result && m_ready_list.at(key); + } + + return result; +} + task_process_status BufferOrch::processBufferPool(Consumer &consumer) { SWSS_LOG_ENTER(); @@ -315,7 +369,7 @@ task_process_status BufferOrch::processQueue(Consumer &consumer) auto it = consumer.m_toSync.begin(); KeyOpFieldsValuesTuple tuple = it->second; sai_object_id_t sai_buffer_profile; - string key = kfvKey(tuple); + const string key = kfvKey(tuple); string op = kfvOp(tuple); vector tokens; sai_uint32_t range_low, range_high; @@ -374,6 +428,16 @@ task_process_status BufferOrch::processQueue(Consumer &consumer) } } } + + if (m_ready_list.find(key) != m_ready_list.end()) + { + m_ready_list[key] = true; + } + else + { + SWSS_LOG_ERROR("Queue profile '%s' was inserted after BufferOrch init", key.c_str()); + } + return task_process_status::task_success; } @@ -386,7 +450,7 @@ task_process_status BufferOrch::processPriorityGroup(Consumer &consumer) auto it = consumer.m_toSync.begin(); KeyOpFieldsValuesTuple tuple = it->second; sai_object_id_t sai_buffer_profile; - string key = kfvKey(tuple); + const string key = kfvKey(tuple); string op = kfvOp(tuple); vector tokens; sai_uint32_t range_low, range_high; @@ -451,6 +515,16 @@ task_process_status BufferOrch::processPriorityGroup(Consumer &consumer) } } } + + if (m_ready_list.find(key) != m_ready_list.end()) + { + m_ready_list[key] = true; + } + else + { + SWSS_LOG_ERROR("PG profile '%s' was inserted after BufferOrch init", key.c_str()); + } + return task_process_status::task_success; } diff --git a/orchagent/bufferorch.h b/orchagent/bufferorch.h index f05f8071aa..23f56e6147 100644 --- a/orchagent/bufferorch.h +++ b/orchagent/bufferorch.h @@ -1,7 +1,9 @@ #ifndef SWSS_BUFFORCH_H #define SWSS_BUFFORCH_H +#include #include +#include #include "orch.h" #include "portsorch.h" @@ -26,6 +28,7 @@ class BufferOrch : public Orch { public: BufferOrch(DBConnector *db, vector &tableNames); + bool isPortReady(const std::string& port_name) const; static type_map m_buffer_type_maps; private: typedef task_process_status (BufferOrch::*buffer_table_handler)(Consumer& consumer); @@ -34,6 +37,8 @@ class BufferOrch : public Orch virtual void doTask(Consumer& consumer); void initTableHandlers(); + void initBufferReadyLists(DBConnector *db); + void initBufferReadyList(Table& table); task_process_status processBufferPool(Consumer &consumer); task_process_status processBufferProfile(Consumer &consumer); task_process_status processQueue(Consumer &consumer); @@ -42,6 +47,8 @@ class BufferOrch : public Orch task_process_status processEgressBufferProfileList(Consumer &consumer); buffer_table_handler_map m_bufferHandlerMap; + std::unordered_map m_ready_list; + std::unordered_map> m_port_ready_list_ref; }; #endif /* SWSS_BUFFORCH_H */ diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 402266edef..1cc1e4dfc2 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -26,6 +26,7 @@ NeighOrch *gNeighOrch; RouteOrch *gRouteOrch; AclOrch *gAclOrch; CrmOrch *gCrmOrch; +BufferOrch *gBufferOrch; OrchDaemon::OrchDaemon(DBConnector *applDb, DBConnector *configDb, DBConnector *stateDb) : m_applDb(applDb), @@ -90,7 +91,7 @@ bool OrchDaemon::init() CFG_BUFFER_PORT_INGRESS_PROFILE_LIST_NAME, CFG_BUFFER_PORT_EGRESS_PROFILE_LIST_NAME }; - BufferOrch *buffer_orch = new BufferOrch(m_configDb, buffer_tables); + gBufferOrch = new BufferOrch(m_configDb, buffer_tables); TableConnector appDbMirrorSession(m_applDb, APP_MIRROR_SESSION_TABLE_NAME); TableConnector confDbMirrorSession(m_configDb, CFG_MIRROR_SESSION_TABLE_NAME); @@ -109,7 +110,7 @@ bool OrchDaemon::init() gAclOrch = new AclOrch(acl_table_connectors, gPortsOrch, mirror_orch, gNeighOrch, gRouteOrch); - m_orchList = { switch_orch, gCrmOrch, gPortsOrch, intfs_orch, gNeighOrch, gRouteOrch, copp_orch, tunnel_decap_orch, qos_orch, buffer_orch, mirror_orch, gAclOrch, gFdbOrch, vrf_orch }; + m_orchList = { switch_orch, gCrmOrch, gPortsOrch, intfs_orch, gNeighOrch, gRouteOrch, copp_orch, tunnel_decap_orch, qos_orch, gBufferOrch, mirror_orch, gAclOrch, gFdbOrch, vrf_orch }; m_select = new Select(); diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index ad5c040de6..70991edfe8 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -18,6 +18,7 @@ #include "sai_serialize.h" #include "crmorch.h" #include "countercheckorch.h" +#include "bufferorch.h" #include "notifier.h" extern sai_switch_api_t *sai_switch_api; @@ -31,6 +32,7 @@ extern sai_queue_api_t *sai_queue_api; extern sai_object_id_t gSwitchId; extern NeighOrch *gNeighOrch; extern CrmOrch *gCrmOrch; +extern BufferOrch *gBufferOrch; #define VLAN_PREFIX "Vlan" #define DEFAULT_VLAN_ID 1 @@ -1280,6 +1282,13 @@ void PortsOrch::doPortTask(Consumer &consumer) continue; } + if (alias != "PortConfigDone" && !gBufferOrch->isPortReady(alias)) + { + // buffer configuration hasn't been applied yet. save it for future retry + it++; + continue; + } + Port p; if (!getPort(alias, p) && alias != "PortConfigDone") { @@ -1332,31 +1341,31 @@ void PortsOrch::doPortTask(Consumer &consumer) } } - if (admin_status != "") + if (mtu != 0) { - if (setPortAdminStatus(p.m_port_id, admin_status == "up")) + if (setPortMtu(p.m_port_id, mtu)) { - SWSS_LOG_NOTICE("Set port %s admin status to %s", alias.c_str(), admin_status.c_str()); + p.m_mtu = mtu; + m_portList[alias] = p; + SWSS_LOG_NOTICE("Set port %s MTU to %u", alias.c_str(), mtu); } else { - SWSS_LOG_ERROR("Failed to set port %s admin status to %s", alias.c_str(), admin_status.c_str()); + SWSS_LOG_ERROR("Failed to set port %s MTU to %u", alias.c_str(), mtu); it++; continue; } } - if (mtu != 0) + if (admin_status != "") { - if (setPortMtu(p.m_port_id, mtu)) + if (setPortAdminStatus(p.m_port_id, admin_status == "up")) { - p.m_mtu = mtu; - m_portList[alias] = p; - SWSS_LOG_NOTICE("Set port %s MTU to %u", alias.c_str(), mtu); + SWSS_LOG_NOTICE("Set port %s admin status to %s", alias.c_str(), admin_status.c_str()); } else { - SWSS_LOG_ERROR("Failed to set port %s MTU to %u", alias.c_str(), mtu); + SWSS_LOG_ERROR("Failed to set port %s admin status to %s", alias.c_str(), admin_status.c_str()); it++; continue; } diff --git a/tests/test_port_buffer_rel.py b/tests/test_port_buffer_rel.py new file mode 100644 index 0000000000..d8c3497121 --- /dev/null +++ b/tests/test_port_buffer_rel.py @@ -0,0 +1,33 @@ +from swsscommon import swsscommon +import time + +# The test check that the ports will be up, when the admin state is UP by conf db. + +def test_PortsAreUpAfterBuffers(dvs): + num_ports = 32 + asic_db = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0) + conf_db = swsscommon.DBConnector(swsscommon.CONFIG_DB, dvs.redis_sock, 0) + + conf_port_table = swsscommon.Table(conf_db, "PORT") + asic_port_table = swsscommon.Table(asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_PORT") + + # enable all ports + fvs = swsscommon.FieldValuePairs([("admin_status", "up")]) + for i in range(0, num_ports): + conf_port_table.set("Ethernet%d" % (i*4), fvs) + + time.sleep(5) + + # check that the ports are enabled in ASIC + asic_port_records = asic_port_table.getKeys() + assert len(asic_port_records) == (num_ports + 1), "Number of port records doesn't match number of ports" # +CPU port + num_set = 0 + for k in asic_port_records: + status, fvs = asic_port_table.get(k) + assert status, "Got an error when get a key" + for fv in fvs: + if fv[0] == "SAI_PORT_ATTR_ADMIN_STATE": + assert fv[1] == "true", "The port isn't UP as expected" + num_set += 1 + # make sure that state is set for all "num_ports" ports + assert num_set == num_ports, "Not all ports are up"