From 8c005c68c94d76923a24886f2103faf272db701c Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Wed, 23 Oct 2019 08:44:35 +0000 Subject: [PATCH 01/11] [portsorch] fix PortsOrch::allPortsReady() returns true when it should not Warm start flow before the change: 1st iteration: - BufferOrch::doTask(): returns since PortInitDone hasn't arived yet - PortsOrch::doTask(): processes all PORT_TABLE untill PortInitDone flag m_pendingPortSet is empty yet and m_portInitDone is true so allPortsReady() will return true - AnyOrch::doTask(): check g_portsOrch->allPortsRead() 2nd iteration: - BufferOrch::doTask(): now buffers are applied This causes BufferOrch override PfcWdOrch's zero-buffer profile. The change swaps BufferOrch and PortsOrch in m_orchList, because 1st BufferOrch iteration will always skip processing and eliminates possibility of having m_pendingPortSet not filled with ports after m_initDone is set to true. Signed-off-by: Stepan Blyschak --- orchagent/orchdaemon.cpp | 2 +- orchagent/portsorch.cpp | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index df151f9966..75fd8d50fa 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -208,7 +208,7 @@ bool OrchDaemon::init() * when iterating ConsumerMap. * That is ensured implicitly by the order of map key, "LAG_TABLE" is smaller than "VLAN_TABLE" in lexicographic order. */ - m_orchList = { gSwitchOrch, gCrmOrch, gBufferOrch, gPortsOrch, gIntfsOrch, gNeighOrch, gRouteOrch, copp_orch, tunnel_decap_orch, qos_orch, wm_orch, policer_orch, sflow_orch}; + m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, gIntfsOrch, gNeighOrch, gRouteOrch, copp_orch, tunnel_decap_orch, qos_orch, wm_orch, policer_orch }; bool initialize_dtel = false; diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index ae876467fb..443f31383f 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1689,13 +1689,6 @@ void PortsOrch::doPortTask(Consumer &consumer) } } - if (!m_portConfigDone) - { - // Not yet receive PortConfigDone. Save it for future retry - it++; - continue; - } - if (alias == "PortConfigDone") { it = consumer.m_toSync.erase(it); @@ -1714,6 +1707,14 @@ void PortsOrch::doPortTask(Consumer &consumer) m_pendingPortSet.erase(alias); } + if (!m_portConfigDone) + { + // Not yet receive PortConfigDone. Save it for future retry + it++; + continue; + } + + Port p; if (!getPort(alias, p)) { From 2be41b47aef812799f8182aab700888342874aa7 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 24 Oct 2019 10:18:55 +0000 Subject: [PATCH 02/11] remove extra newline Signed-off-by: Stepan Blyschak --- orchagent/portsorch.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 443f31383f..6916e83aa8 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1714,7 +1714,6 @@ void PortsOrch::doPortTask(Consumer &consumer) continue; } - Port p; if (!getPort(alias, p)) { From 99123333851d3938b76f007dbb1a1d0c24633f24 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 24 Oct 2019 15:34:57 +0000 Subject: [PATCH 03/11] [pfcwdorch] fix PfcWdSwOrch::doTask() starts WD action when WD wasn't started in warm boot It appeared that pfc watchdog relied on a buggy behaviour of PortsOrch::allPortsReady(). In fixed PortsOrch::allPortsReady() you'll see that watchdog action is trying to start before watchdog was started, because allPortsReady() in PfcWdOrch::doTask() returned false. Before the fix watchdog was started before, because allPortsReady() lied that ports are ready when they were not. Signed-off-by: Stepan Blyschak --- orchagent/pfcwdorch.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/orchagent/pfcwdorch.cpp b/orchagent/pfcwdorch.cpp index c4002a1c3d..d6635b49b3 100644 --- a/orchagent/pfcwdorch.cpp +++ b/orchagent/pfcwdorch.cpp @@ -759,6 +759,11 @@ void PfcWdSwOrch::doTask(Consumer& consumer) { PfcWdOrch::doTask(consumer); + if (!gPortsOrch->allPortsReady()) + { + return; + } + if ((consumer.getDbId() == APPL_DB) && (consumer.getTableName() == APP_PFC_WD_TABLE_NAME)) { auto it = consumer.m_toSync.begin(); From 59ad10bead62107d42449e3761029c6471b135f6 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Sun, 27 Oct 2019 23:03:15 +0000 Subject: [PATCH 04/11] [portsorch] populate m_pendingPortSet in PortsOrch::bake() Signed-off-by: Stepan Blyschak --- orchagent/portsorch.cpp | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 6916e83aa8..8958ea85dc 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -1471,6 +1471,16 @@ bool PortsOrch::bake() return false; } + for (const auto& alias: keys) + { + if (alias == "PortConfigDone" || alias == "PortInitDone") + { + continue; + } + + m_pendingPortSet.emplace(alias); + } + addExistingData(m_portTable.get()); addExistingData(APP_LAG_TABLE_NAME); addExistingData(APP_LAG_MEMBER_TABLE_NAME); @@ -1689,6 +1699,13 @@ void PortsOrch::doPortTask(Consumer &consumer) } } + if (!m_portConfigDone) + { + // Not yet receive PortConfigDone. Save it for future retry + it++; + continue; + } + if (alias == "PortConfigDone") { it = consumer.m_toSync.erase(it); @@ -1707,13 +1724,6 @@ void PortsOrch::doPortTask(Consumer &consumer) m_pendingPortSet.erase(alias); } - if (!m_portConfigDone) - { - // Not yet receive PortConfigDone. Save it for future retry - it++; - continue; - } - Port p; if (!getPort(alias, p)) { From c3fd43cbb7562923e3d5205e4e548a3c26f233be Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Mon, 28 Oct 2019 11:34:14 +0000 Subject: [PATCH 05/11] [portsorch] optimize to 3 iterations instead of 4 Signed-off-by: Stepan Blyschak --- orchagent/orchdaemon.cpp | 16 +++++++--------- orchagent/portsorch.cpp | 16 ++++++++++++++++ orchagent/portsorch.h | 1 + 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 75fd8d50fa..a4e3155b67 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -490,22 +490,20 @@ bool OrchDaemon::warmRestoreAndSyncUp() } /* - * Four iterations are needed. + * Three iterations are needed. * - * First iteration: switchorch, Port init/hostif create part of portorch. + * First iteration: switchorch, Port init/hostif create part of portorch, buffers configuration * - * Second iteratoin: gBufferOrch which requires port created, - * then port speed/mtu/fec_mode/pfc_asym/admin_status config. + * Second iteratoin: port speed/mtu/fec_mode/pfc_asym/admin_status config, + * other orch(s) which wait for port to become ready. * - * Third iteration: other orch(s) which wait for port init done. - * - * Fourth iteration: Drain remaining data that are out of order like LAG_MEMBER_TABLE and + * Third iteration: Drain remaining data that are out of order like LAG_MEMBER_TABLE and * VLAN_MEMBER_TABLE since they were checked before LAG_TABLE and VLAN_TABLE within gPortsOrch. */ - for (auto it = 0; it < 4; it++) + for (auto it = 0; it < 3; it++) { - SWSS_LOG_DEBUG("The current iteration is %d", it); + SWSS_LOG_NOTICE("The current iteration is %d", it); for (Orch *o : m_orchList) { diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 8958ea85dc..6b375e2ba7 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -2372,6 +2372,22 @@ void PortsOrch::doLagMemberTask(Consumer &consumer) } } +void PortsOrch::doTask() +{ + auto portConsumer = getExecutor(APP_PORT_TABLE_NAME); + portConsumer->drain(); + + for (auto& it: m_consumerMap) + { + auto consumer = it.second.get(); + if (consumer == portConsumer) + { + continue; + } + consumer->drain(); + } +} + void PortsOrch::doTask(Consumer &consumer) { SWSS_LOG_ENTER(); diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 6b12dd24fb..a74e23a21b 100644 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -127,6 +127,7 @@ class PortsOrch : public Orch, public Subject NotificationConsumer* m_portStatusNotificationConsumer; + void doTask() override; void doTask(Consumer &consumer); void doPortTask(Consumer &consumer); void doVlanTask(Consumer &consumer); From 50bc218577fc53fee199b0e5334016fc7931ae2b Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Mon, 28 Oct 2019 15:19:05 +0000 Subject: [PATCH 06/11] Revert "[portsorch] optimize to 3 iterations instead of 4" This reverts commit 84f80e0270c19a678eeb5c6e43f650ebee219058. --- orchagent/orchdaemon.cpp | 16 +++++++++------- orchagent/portsorch.cpp | 16 ---------------- orchagent/portsorch.h | 1 - 3 files changed, 9 insertions(+), 24 deletions(-) diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index a4e3155b67..75fd8d50fa 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -490,20 +490,22 @@ bool OrchDaemon::warmRestoreAndSyncUp() } /* - * Three iterations are needed. + * Four iterations are needed. * - * First iteration: switchorch, Port init/hostif create part of portorch, buffers configuration + * First iteration: switchorch, Port init/hostif create part of portorch. * - * Second iteratoin: port speed/mtu/fec_mode/pfc_asym/admin_status config, - * other orch(s) which wait for port to become ready. + * Second iteratoin: gBufferOrch which requires port created, + * then port speed/mtu/fec_mode/pfc_asym/admin_status config. * - * Third iteration: Drain remaining data that are out of order like LAG_MEMBER_TABLE and + * Third iteration: other orch(s) which wait for port init done. + * + * Fourth iteration: Drain remaining data that are out of order like LAG_MEMBER_TABLE and * VLAN_MEMBER_TABLE since they were checked before LAG_TABLE and VLAN_TABLE within gPortsOrch. */ - for (auto it = 0; it < 3; it++) + for (auto it = 0; it < 4; it++) { - SWSS_LOG_NOTICE("The current iteration is %d", it); + SWSS_LOG_DEBUG("The current iteration is %d", it); for (Orch *o : m_orchList) { diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index 6b375e2ba7..8958ea85dc 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -2372,22 +2372,6 @@ void PortsOrch::doLagMemberTask(Consumer &consumer) } } -void PortsOrch::doTask() -{ - auto portConsumer = getExecutor(APP_PORT_TABLE_NAME); - portConsumer->drain(); - - for (auto& it: m_consumerMap) - { - auto consumer = it.second.get(); - if (consumer == portConsumer) - { - continue; - } - consumer->drain(); - } -} - void PortsOrch::doTask(Consumer &consumer) { SWSS_LOG_ENTER(); diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index a74e23a21b..6b12dd24fb 100644 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -127,7 +127,6 @@ class PortsOrch : public Orch, public Subject NotificationConsumer* m_portStatusNotificationConsumer; - void doTask() override; void doTask(Consumer &consumer); void doPortTask(Consumer &consumer); void doVlanTask(Consumer &consumer); From bb345ba743bf44e7c9ea1d4da345ff09772cd589 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Mon, 28 Oct 2019 15:20:54 +0000 Subject: [PATCH 07/11] revert change of order in m_orchList Signed-off-by: Stepan Blyschak --- orchagent/orchdaemon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 75fd8d50fa..7e7575385c 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -208,7 +208,7 @@ bool OrchDaemon::init() * when iterating ConsumerMap. * That is ensured implicitly by the order of map key, "LAG_TABLE" is smaller than "VLAN_TABLE" in lexicographic order. */ - m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, gIntfsOrch, gNeighOrch, gRouteOrch, copp_orch, tunnel_decap_orch, qos_orch, wm_orch, policer_orch }; + m_orchList = { gSwitchOrch, gCrmOrch, gBufferOrch, gPortsOrch, gIntfsOrch, gNeighOrch, gRouteOrch, copp_orch, tunnel_decap_orch, qos_orch, wm_orch, policer_orch }; bool initialize_dtel = false; From 39c950bd80a5ee5fafbd472a5a088af125cd7229 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 31 Oct 2019 09:32:26 +0000 Subject: [PATCH 08/11] [mock_tests] fix tests build Signed-off-by: Stepan Blyschak --- tests/Makefile.am | 2 ++ tests/mock_tests/Makefile.am | 61 ++++++++++++++++++------------------ 2 files changed, 33 insertions(+), 30 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index b6bff493a6..0196e0a1a2 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -2,6 +2,8 @@ CFLAGS_SAI = -I /usr/include/sai TESTS = tests +SUBDIRS = mock_tests + noinst_PROGRAMS = tests if DEBUG diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am index 8ff7d81597..4815d18b4e 100644 --- a/tests/mock_tests/Makefile.am +++ b/tests/mock_tests/Makefile.am @@ -13,42 +13,43 @@ endif CFLAGS_GTEST = LDADD_GTEST = -L/usr/src/gtest -tests_SOURCES = swssnet_ut.cpp request_parser_ut.cpp aclorch_ut.cpp saispy_ut.cpp \ +tests_SOURCES = aclorch_ut.cpp saispy_ut.cpp \ mock_orchagent_main.cpp \ mock_dbconnector.cpp \ mock_consumerstatetable.cpp \ mock_hiredis.cpp \ mock_redisreply.cpp \ - ../orchagent/orchdaemon.cpp \ - ../orchagent/orch.cpp \ - ../orchagent/notifications.cpp \ - ../orchagent/routeorch.cpp \ - ../orchagent/neighorch.cpp \ - ../orchagent/intfsorch.cpp \ - ../orchagent/portsorch.cpp \ - ../orchagent/copporch.cpp \ - ../orchagent/tunneldecaporch.cpp \ - ../orchagent/qosorch.cpp \ - ../orchagent/bufferorch.cpp \ - ../orchagent/mirrororch.cpp \ - ../orchagent/fdborch.cpp \ - ../orchagent/aclorch.cpp \ - ../orchagent/saihelper.cpp \ - ../orchagent/switchorch.cpp \ - ../orchagent/pfcwdorch.cpp \ - ../orchagent/pfcactionhandler.cpp \ - ../orchagent/policerorch.cpp \ - ../orchagent/crmorch.cpp \ - ../orchagent/request_parser.cpp \ - ../orchagent/vrforch.cpp \ - ../orchagent/countercheckorch.cpp \ - ../orchagent/vxlanorch.cpp \ - ../orchagent/vnetorch.cpp \ - ../orchagent/dtelorch.cpp \ - ../orchagent/flexcounterorch.cpp \ - ../orchagent/watermarkorch.cpp + $(top_srcdir)/orchagent/orchdaemon.cpp \ + $(top_srcdir)/orchagent/orch.cpp \ + $(top_srcdir)/orchagent/notifications.cpp \ + $(top_srcdir)/orchagent/routeorch.cpp \ + $(top_srcdir)/orchagent/neighorch.cpp \ + $(top_srcdir)/orchagent/intfsorch.cpp \ + $(top_srcdir)/orchagent/portsorch.cpp \ + $(top_srcdir)/orchagent/copporch.cpp \ + $(top_srcdir)/orchagent/tunneldecaporch.cpp \ + $(top_srcdir)/orchagent/qosorch.cpp \ + $(top_srcdir)/orchagent/bufferorch.cpp \ + $(top_srcdir)/orchagent/mirrororch.cpp \ + $(top_srcdir)/orchagent/fdborch.cpp \ + $(top_srcdir)/orchagent/aclorch.cpp \ + $(top_srcdir)/orchagent/saihelper.cpp \ + $(top_srcdir)/orchagent/switchorch.cpp \ + $(top_srcdir)/orchagent/pfcwdorch.cpp \ + $(top_srcdir)/orchagent/pfcactionhandler.cpp \ + $(top_srcdir)/orchagent/policerorch.cpp \ + $(top_srcdir)/orchagent/crmorch.cpp \ + $(top_srcdir)/orchagent/request_parser.cpp \ + $(top_srcdir)/orchagent/vrforch.cpp \ + $(top_srcdir)/orchagent/countercheckorch.cpp \ + $(top_srcdir)/orchagent/vxlanorch.cpp \ + $(top_srcdir)/orchagent/vnetorch.cpp \ + $(top_srcdir)/orchagent/dtelorch.cpp \ + $(top_srcdir)/orchagent/flexcounterorch.cpp \ + $(top_srcdir)/orchagent/watermarkorch.cpp \ + $(top_srcdir)/orchagent/chassisorch.cpp tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) -tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) -I../orchagent +tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) -I$(top_srcdir)/orchagent tests_LDADD = $(LDADD_GTEST) $(LDADD_SAI) -lnl-genl-3 -lhiredis -lhiredis -lpthread \ -lswsscommon -lswsscommon -lgtest -lgtest_main From 6a2073b0e0b98728689630d9ea16bb7572dda2d5 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 31 Oct 2019 11:55:46 +0200 Subject: [PATCH 09/11] [mock_test] create unittest for PortsOrch::allPortsReady cold/warm flows Signed-off-by: Stepan Blyschak --- tests/mock_tests/.gitignore | 1 + tests/mock_tests/Makefile.am | 9 +- tests/mock_tests/aclorch_ut.cpp | 18 +- tests/mock_tests/mock_consumerstatetable.cpp | 2 +- tests/mock_tests/mock_dbconnector.cpp | 4 +- tests/mock_tests/mock_orchagent_main.h | 56 ++++ tests/mock_tests/mock_table.cpp | 74 +++++ tests/mock_tests/mock_table.h | 6 + tests/mock_tests/portsorch_ut.cpp | 313 +++++++++++++++++++ tests/mock_tests/ut_helper.h | 8 + tests/mock_tests/ut_saihelper.cpp | 163 ++++++++++ 11 files changed, 640 insertions(+), 14 deletions(-) create mode 100644 tests/mock_tests/.gitignore create mode 100644 tests/mock_tests/mock_orchagent_main.h create mode 100644 tests/mock_tests/mock_table.cpp create mode 100644 tests/mock_tests/mock_table.h create mode 100644 tests/mock_tests/portsorch_ut.cpp create mode 100644 tests/mock_tests/ut_saihelper.cpp diff --git a/tests/mock_tests/.gitignore b/tests/mock_tests/.gitignore new file mode 100644 index 0000000000..2b29f27645 --- /dev/null +++ b/tests/mock_tests/.gitignore @@ -0,0 +1 @@ +tests diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am index 4815d18b4e..69b1744836 100644 --- a/tests/mock_tests/Makefile.am +++ b/tests/mock_tests/Makefile.am @@ -13,10 +13,14 @@ endif CFLAGS_GTEST = LDADD_GTEST = -L/usr/src/gtest -tests_SOURCES = aclorch_ut.cpp saispy_ut.cpp \ +tests_SOURCES = aclorch_ut.cpp \ + portsorch_ut.cpp \ + saispy_ut.cpp \ + ut_saihelper.cpp \ mock_orchagent_main.cpp \ mock_dbconnector.cpp \ mock_consumerstatetable.cpp \ + mock_table.cpp \ mock_hiredis.cpp \ mock_redisreply.cpp \ $(top_srcdir)/orchagent/orchdaemon.cpp \ @@ -47,7 +51,8 @@ tests_SOURCES = aclorch_ut.cpp saispy_ut.cpp \ $(top_srcdir)/orchagent/dtelorch.cpp \ $(top_srcdir)/orchagent/flexcounterorch.cpp \ $(top_srcdir)/orchagent/watermarkorch.cpp \ - $(top_srcdir)/orchagent/chassisorch.cpp + $(top_srcdir)/orchagent/chassisorch.cpp \ + $(top_srcdir)/orchagent/sfloworch.cpp tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) -I$(top_srcdir)/orchagent diff --git a/tests/mock_tests/aclorch_ut.cpp b/tests/mock_tests/aclorch_ut.cpp index ec1e2d4316..ae4267ddb5 100644 --- a/tests/mock_tests/aclorch_ut.cpp +++ b/tests/mock_tests/aclorch_ut.cpp @@ -643,16 +643,16 @@ namespace aclorch_test // leakage check bool validateResourceCountWithLowerLayerDb(const AclOrch *aclOrch) { - // TODO: Using the need to include "sai_vs_state.h". That will need the include path from `configure` - // Do this later ... + // TODO: Using the need to include "sai_vs_state.h". That will need the include path from `configure` + // Do this later ... #if WITH_SAI == LIBVS - // { - // auto& aclTableHash = g_switch_state_map.at(gSwitchId)->objectHash.at(SAI_OBJECT_TYPE_ACL_TABLE); - // - // return aclTableHash.size() == Portal::AclOrchInternal::getAclTables(aclOrch).size(); - // } - // - // TODO: add rule check + // { + // auto& aclTableHash = g_switch_state_map.at(gSwitchId)->objectHash.at(SAI_OBJECT_TYPE_ACL_TABLE); + // + // return aclTableHash.size() == Portal::AclOrchInternal::getAclTables(aclOrch).size(); + // } + // + // TODO: add rule check #endif return true; diff --git a/tests/mock_tests/mock_consumerstatetable.cpp b/tests/mock_tests/mock_consumerstatetable.cpp index 0af574a5f5..822727929a 100644 --- a/tests/mock_tests/mock_consumerstatetable.cpp +++ b/tests/mock_tests/mock_consumerstatetable.cpp @@ -7,4 +7,4 @@ namespace swss TableName_KeySet(tableName) { } -} \ No newline at end of file +} diff --git a/tests/mock_tests/mock_dbconnector.cpp b/tests/mock_tests/mock_dbconnector.cpp index 14f955eb3e..6a021e036a 100644 --- a/tests/mock_tests/mock_dbconnector.cpp +++ b/tests/mock_tests/mock_dbconnector.cpp @@ -40,6 +40,6 @@ namespace swss int DBConnector::getDbId() const { - return 12345; + return m_dbId; } -} \ No newline at end of file +} diff --git a/tests/mock_tests/mock_orchagent_main.h b/tests/mock_tests/mock_orchagent_main.h new file mode 100644 index 0000000000..d2d313279d --- /dev/null +++ b/tests/mock_tests/mock_orchagent_main.h @@ -0,0 +1,56 @@ +#pragma once + +#include "orch.h" +#include "switchorch.h" +#include "crmorch.h" +#include "portsorch.h" +#include "routeorch.h" +#include "intfsorch.h" +#include "neighorch.h" +#include "fdborch.h" +#include "mirrororch.h" +#include "bufferorch.h" +#include "vrforch.h" +#include "vnetorch.h" +#include "vxlanorch.h" +#include "policerorch.h" + +extern int gBatchSize; +extern bool gSwssRecord; +extern bool gSairedisRecord; +extern bool gLogRotate; +extern ofstream gRecordOfs; +extern string gRecordFile; + +extern MacAddress gMacAddress; +extern MacAddress gVxlanMacAddress; + +extern sai_object_id_t gSwitchId; +extern sai_object_id_t gVirtualRouterId; +extern sai_object_id_t gUnderlayIfId; + +extern SwitchOrch *gSwitchOrch; +extern CrmOrch *gCrmOrch; +extern PortsOrch *gPortsOrch; +extern RouteOrch *gRouteOrch; +extern IntfsOrch *gIntfsOrch; +extern NeighOrch *gNeighOrch; +extern FdbOrch *gFdbOrch; +extern MirrorOrch *gMirrorOrch; +extern BufferOrch *gBufferOrch; +extern VRFOrch *gVrfOrch; + +extern sai_acl_api_t *sai_acl_api; +extern sai_switch_api_t *sai_switch_api; +extern sai_virtual_router_api_t *sai_virtual_router_api; +extern sai_port_api_t *sai_port_api; +extern sai_lag_api_t *sai_lag_api; +extern sai_vlan_api_t *sai_vlan_api; +extern sai_bridge_api_t *sai_bridge_api; +extern sai_router_interface_api_t *sai_router_intfs_api; +extern sai_route_api_t *sai_route_api; +extern sai_neighbor_api_t *sai_neighbor_api; +extern sai_tunnel_api_t *sai_tunnel_api; +extern sai_next_hop_api_t *sai_next_hop_api; +extern sai_hostif_api_t *sai_hostif_api; +extern sai_buffer_api_t *sai_buffer_api; diff --git a/tests/mock_tests/mock_table.cpp b/tests/mock_tests/mock_table.cpp new file mode 100644 index 0000000000..7cd532c208 --- /dev/null +++ b/tests/mock_tests/mock_table.cpp @@ -0,0 +1,74 @@ +#include "table.h" + +using TableDataT = std::map>; +using TablesT = std::map; + +namespace testing_db +{ + + TableDataT gTableData; + TablesT gTables; + std::map gDB; + + void reset() + { + gDB.clear(); + } +} + +namespace swss +{ + + using namespace testing_db; + + bool Table::get(const std::string &key, std::vector &ovalues) + { + auto table = gDB[getDbId()][getTableName()]; + if (table.find(key) == table.end()) + { + return false; + } + + ovalues = table[key]; + return true; + } + + bool Table::hget(const std::string &key, const std::string &field, std::string &value) + { + auto table = gDB[getDbId()][getTableName()]; + if (table.find(key) == table.end()) + { + return false; + } + + for (const auto &it : table[key]) + { + if (it.first == field) + { + value = it.second; + return true; + } + } + + return false; + } + + void Table::set(const std::string &key, + const std::vector &values, + const std::string &op, + const std::string &prefix) + { + auto &table = gDB[getDbId()][getTableName()]; + table[key] = values; + } + + void Table::getKeys(std::vector &keys) + { + keys.clear(); + auto table = gDB[getDbId()][getTableName()]; + for (const auto &it : table) + { + keys.push_back(it.first); + } + } +} diff --git a/tests/mock_tests/mock_table.h b/tests/mock_tests/mock_table.h new file mode 100644 index 0000000000..bf8a4bdef6 --- /dev/null +++ b/tests/mock_tests/mock_table.h @@ -0,0 +1,6 @@ +#include "table.h" + +namespace testing_db +{ + void reset(); +} diff --git a/tests/mock_tests/portsorch_ut.cpp b/tests/mock_tests/portsorch_ut.cpp new file mode 100644 index 0000000000..40cfbd3ecc --- /dev/null +++ b/tests/mock_tests/portsorch_ut.cpp @@ -0,0 +1,313 @@ +#include "ut_helper.h" +#include "mock_orchagent_main.h" +#include "mock_table.h" + +#include + +namespace portsorch_test +{ + + using namespace std; + + struct PortsOrchTest : public ::testing::Test + { + shared_ptr m_app_db; + shared_ptr m_config_db; + shared_ptr m_state_db; + + PortsOrchTest() + { + // FIXME: move out from constructor + m_app_db = make_shared( + APPL_DB, swss::DBConnector::DEFAULT_UNIXSOCKET, 0); + m_config_db = make_shared( + CONFIG_DB, swss::DBConnector::DEFAULT_UNIXSOCKET, 0); + m_state_db = make_shared( + STATE_DB, swss::DBConnector::DEFAULT_UNIXSOCKET, 0); + } + + virtual void SetUp() override + { + ::testing_db::reset(); + } + + virtual void TearDown() override + { + ::testing_db::reset(); + delete gPortsOrch; + gPortsOrch = nullptr; + delete gBufferOrch; + gBufferOrch = nullptr; + } + + static void SetUpTestCase() + { + // Init switch and create dependencies + + map profile = { + { "SAI_VS_SWITCH_TYPE", "SAI_VS_SWITCH_TYPE_BCM56850" }, + { "KV_DEVICE_MAC_ADDRESS", "20:03:04:05:06:00" } + }; + + auto status = ut_helper::initSaiApi(profile); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + sai_attribute_t attr; + + attr.id = SAI_SWITCH_ATTR_INIT_SWITCH; + attr.value.booldata = true; + + status = sai_switch_api->create_switch(&gSwitchId, 1, &attr); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + // Get switch source MAC address + attr.id = SAI_SWITCH_ATTR_SRC_MAC_ADDRESS; + status = sai_switch_api->get_switch_attribute(gSwitchId, 1, &attr); + + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + gMacAddress = attr.value.mac; + + // Get the default virtual router ID + attr.id = SAI_SWITCH_ATTR_DEFAULT_VIRTUAL_ROUTER_ID; + status = sai_switch_api->get_switch_attribute(gSwitchId, 1, &attr); + + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + gVirtualRouterId = attr.value.oid; + } + + static void TearDownTestCase() + { + auto status = sai_switch_api->remove_switch(gSwitchId); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + gSwitchId = 0; + + ut_helper::uninitSaiApi(); + } + }; + + TEST_F(PortsOrchTest, PortReadinessColdBoot) + { + Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME); + Table pgTable = Table(m_config_db.get(), CFG_BUFFER_PG_TABLE_NAME); + Table profileTable = Table(m_config_db.get(), CFG_BUFFER_PROFILE_TABLE_NAME); + Table poolTable = Table(m_config_db.get(), CFG_BUFFER_POOL_TABLE_NAME); + + // Get SAI default ports to populate DB + + auto ports = ut_helper::getInitialSaiPorts(); + + // Create test buffer pool + poolTable.set( + "test_pool", + { + { "type", "ingress" }, + { "mode", "dynamic" }, + { "size", "4200000" }, + }); + + // Create test buffer profile + profileTable.set("test_profile", { { "pool", "[BUFFER_POOL|test_pool]" }, + { "xon", "14832" }, + { "xoff", "14832" }, + { "size", "35000" }, + { "dynamic_th", "0" } }); + + // Apply profile on PGs 3-4 all ports + for (const auto &it : ports) + { + std::ostringstream oss; + oss << it.first << "|3-4"; + pgTable.set(oss.str(), { { "profile", "[BUFFER_PROFILE|test_profile]" } }); + } + + // Create dependencies ... + + const int portsorch_base_pri = 40; + + vector ports_tables = { + { APP_PORT_TABLE_NAME, portsorch_base_pri + 5 }, + { APP_VLAN_TABLE_NAME, portsorch_base_pri + 2 }, + { APP_VLAN_MEMBER_TABLE_NAME, portsorch_base_pri }, + { APP_LAG_TABLE_NAME, portsorch_base_pri + 4 }, + { APP_LAG_MEMBER_TABLE_NAME, portsorch_base_pri } + }; + + ASSERT_EQ(gPortsOrch, nullptr); + gPortsOrch = new PortsOrch(m_app_db.get(), ports_tables); + vector buffer_tables = { CFG_BUFFER_POOL_TABLE_NAME, + CFG_BUFFER_PROFILE_TABLE_NAME, + CFG_BUFFER_QUEUE_TABLE_NAME, + CFG_BUFFER_PG_TABLE_NAME, + CFG_BUFFER_PORT_INGRESS_PROFILE_LIST_NAME, + CFG_BUFFER_PORT_EGRESS_PROFILE_LIST_NAME }; + + ASSERT_EQ(gBufferOrch, nullptr); + gBufferOrch = new BufferOrch(m_config_db.get(), buffer_tables); + + // Populate pot table with SAI ports + for (const auto &it : ports) + { + portTable.set(it.first, it.second); + } + + // Set PortConfigDone + portTable.set("PortConfigDone", { { "count", to_string(ports.size()) } }); + + // refill consumer + gPortsOrch->addExistingData(&portTable); + gBufferOrch->addExistingData(&pgTable); + gBufferOrch->addExistingData(&poolTable); + gBufferOrch->addExistingData(&profileTable); + + // Apply configuration : + // create ports + + static_cast(gBufferOrch)->doTask(); + + static_cast(gPortsOrch)->doTask(); + + // Ports are not ready yet + + ASSERT_FALSE(gPortsOrch->allPortsReady()); + + // Ports host interfaces are created + + portTable.set("PortInitDone", { { "lanes", "0" } }); + gPortsOrch->addExistingData(&portTable); + + // Apply configuration + // configure buffers + // ports + static_cast(gPortsOrch)->doTask(); + + // Since init done is set now, apply buffers + static_cast(gBufferOrch)->doTask(); + + // Ports are not ready yet, mtu, speed left + ASSERT_FALSE(gPortsOrch->allPortsReady()); + + static_cast(gPortsOrch)->doTask(); + ASSERT_TRUE(gPortsOrch->allPortsReady()); + + // No more tasks + + vector ts; + + gPortsOrch->dumpPendingTasks(ts); + ASSERT_TRUE(ts.empty()); + + ts.clear(); + + gBufferOrch->dumpPendingTasks(ts); + ASSERT_TRUE(ts.empty()); + } + + TEST_F(PortsOrchTest, PortReadinessWarmBoot) + { + + Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME); + Table pgTable = Table(m_config_db.get(), CFG_BUFFER_PG_TABLE_NAME); + Table profileTable = Table(m_config_db.get(), CFG_BUFFER_PROFILE_TABLE_NAME); + Table poolTable = Table(m_config_db.get(), CFG_BUFFER_POOL_TABLE_NAME); + + // Get SAI default ports to populate DB + + auto ports = ut_helper::getInitialSaiPorts(); + + // Create test buffer pool + poolTable.set( + "test_pool", + { + { "type", "ingress" }, + { "mode", "dynamic" }, + { "size", "4200000" }, + }); + + // Create test buffer profile + profileTable.set("test_profile", { { "pool", "[BUFFER_POOL|test_pool]" }, + { "xon", "14832" }, + { "xoff", "14832" }, + { "size", "35000" }, + { "dynamic_th", "0" } }); + + // Apply profile on PGs 3-4 all ports + for (const auto &it : ports) + { + std::ostringstream oss; + oss << it.first << "|3-4"; + pgTable.set(oss.str(), { { "profile", "[BUFFER_PROFILE|test_profile]" } }); + } + + // Populate pot table with SAI ports + for (const auto &it : ports) + { + portTable.set(it.first, it.second); + } + + // Set PortConfigDone, PortInitDone + + portTable.set("PortConfigDone", { { "count", to_string(ports.size()) } }); + portTable.set("PortInitDone", { { "lanes", "0" } }); + + // Create dependencies ... + + const int portsorch_base_pri = 40; + + vector ports_tables = { + { APP_PORT_TABLE_NAME, portsorch_base_pri + 5 }, + { APP_VLAN_TABLE_NAME, portsorch_base_pri + 2 }, + { APP_VLAN_MEMBER_TABLE_NAME, portsorch_base_pri }, + { APP_LAG_TABLE_NAME, portsorch_base_pri + 4 }, + { APP_LAG_MEMBER_TABLE_NAME, portsorch_base_pri } + }; + + ASSERT_EQ(gPortsOrch, nullptr); + gPortsOrch = new PortsOrch(m_app_db.get(), ports_tables); + vector buffer_tables = { CFG_BUFFER_POOL_TABLE_NAME, + CFG_BUFFER_PROFILE_TABLE_NAME, + CFG_BUFFER_QUEUE_TABLE_NAME, + CFG_BUFFER_PG_TABLE_NAME, + CFG_BUFFER_PORT_INGRESS_PROFILE_LIST_NAME, + CFG_BUFFER_PORT_EGRESS_PROFILE_LIST_NAME }; + + ASSERT_EQ(gBufferOrch, nullptr); + gBufferOrch = new BufferOrch(m_config_db.get(), buffer_tables); + + // warm start, bake fill refill consumer + + gBufferOrch->bake(); + gPortsOrch->bake(); + + // Create ports, BufferOrch skips processing + static_cast(gBufferOrch)->doTask(); + static_cast(gPortsOrch)->doTask(); + + // Ports should not be ready here, buffers not applied, + // BufferOrch depends on ports to be created + + ASSERT_FALSE(gPortsOrch->allPortsReady()); + + // Drain remaining + + static_cast(gBufferOrch)->doTask(); + static_cast(gPortsOrch)->doTask(); + + // Now ports should be ready + + ASSERT_TRUE(gPortsOrch->allPortsReady()); + + // No more tasks + + vector ts; + + gPortsOrch->dumpPendingTasks(ts); + ASSERT_TRUE(ts.empty()); + + ts.clear(); + + gBufferOrch->dumpPendingTasks(ts); + ASSERT_TRUE(ts.empty()); + } +} diff --git a/tests/mock_tests/ut_helper.h b/tests/mock_tests/ut_helper.h index ba1a649d8f..baaf8184d8 100644 --- a/tests/mock_tests/ut_helper.h +++ b/tests/mock_tests/ut_helper.h @@ -9,3 +9,11 @@ #include "saispy.h" #include "check.h" + +namespace ut_helper +{ + sai_status_t initSaiApi(const std::map &profile); + void uninitSaiApi(); + + map> getInitialSaiPorts(); +} diff --git a/tests/mock_tests/ut_saihelper.cpp b/tests/mock_tests/ut_saihelper.cpp new file mode 100644 index 0000000000..5d5d07c190 --- /dev/null +++ b/tests/mock_tests/ut_saihelper.cpp @@ -0,0 +1,163 @@ +#include "ut_helper.h" +#include "mock_orchagent_main.h" + +namespace ut_helper +{ + map gProfileMap; + map::iterator gProfileIter; + + const char *profile_get_value( + sai_switch_profile_id_t profile_id, + const char *variable) + { + map::const_iterator it = gProfileMap.find(variable); + if (it == gProfileMap.end()) + { + return NULL; + } + + return it->second.c_str(); + } + + int profile_get_next_value( + sai_switch_profile_id_t profile_id, + const char **variable, + const char **value) + { + if (value == NULL) + { + gProfileIter = gProfileMap.begin(); + return 0; + } + + if (variable == NULL) + { + return -1; + } + + if (gProfileIter == gProfileMap.end()) + { + return -1; + } + + *variable = gProfileIter->first.c_str(); + *value = gProfileIter->second.c_str(); + + gProfileIter++; + + return 0; + } + + sai_status_t initSaiApi(const std::map &profile) + { + sai_service_method_table_t services = { + profile_get_value, + profile_get_next_value + }; + + gProfileMap = profile; + + auto status = sai_api_initialize(0, (sai_service_method_table_t *)&services); + if (status != SAI_STATUS_SUCCESS) + { + return status; + } + + sai_api_query(SAI_API_SWITCH, (void **)&sai_switch_api); + sai_api_query(SAI_API_BRIDGE, (void **)&sai_bridge_api); + sai_api_query(SAI_API_VIRTUAL_ROUTER, (void **)&sai_virtual_router_api); + sai_api_query(SAI_API_PORT, (void **)&sai_port_api); + sai_api_query(SAI_API_LAG, (void **)&sai_lag_api); + sai_api_query(SAI_API_VLAN, (void **)&sai_vlan_api); + sai_api_query(SAI_API_ROUTER_INTERFACE, (void **)&sai_router_intfs_api); + sai_api_query(SAI_API_ROUTE, (void **)&sai_route_api); + sai_api_query(SAI_API_NEIGHBOR, (void **)&sai_neighbor_api); + sai_api_query(SAI_API_TUNNEL, (void **)&sai_tunnel_api); + sai_api_query(SAI_API_NEXT_HOP, (void **)&sai_next_hop_api); + sai_api_query(SAI_API_ACL, (void **)&sai_acl_api); + sai_api_query(SAI_API_HOSTIF, (void **)&sai_hostif_api); + sai_api_query(SAI_API_BUFFER, (void **)&sai_buffer_api); + + return SAI_STATUS_SUCCESS; + } + + void uninitSaiApi() + { + sai_api_uninitialize(); + + sai_switch_api = nullptr; + sai_bridge_api = nullptr; + sai_virtual_router_api = nullptr; + sai_port_api = nullptr; + sai_lag_api = nullptr; + sai_vlan_api = nullptr; + sai_router_intfs_api = nullptr; + sai_route_api = nullptr; + sai_neighbor_api = nullptr; + sai_tunnel_api = nullptr; + sai_next_hop_api = nullptr; + sai_acl_api = nullptr; + sai_hostif_api = nullptr; + sai_buffer_api = nullptr; + } + + map> getInitialSaiPorts() + { + vector port_list; + sai_attribute_t attr; + sai_status_t status; + uint32_t port_count; + + attr.id = SAI_SWITCH_ATTR_PORT_NUMBER; + status = sai_switch_api->get_switch_attribute(gSwitchId, 1, &attr); + if (status != SAI_STATUS_SUCCESS) + { + throw std::runtime_error("failed to get port count"); + } + port_count = attr.value.u32; + + port_list.resize(port_count); + attr.id = SAI_SWITCH_ATTR_PORT_LIST; + attr.value.objlist.count = port_count; + attr.value.objlist.list = port_list.data(); + status = sai_switch_api->get_switch_attribute(gSwitchId, 1, &attr); + if (status != SAI_STATUS_SUCCESS || attr.value.objlist.count != port_count) + { + throw std::runtime_error("failed to get port list"); + } + + std::map> ports; + for (uint32_t i = 0; i < port_count; ++i) + { + string lanes_str = ""; + sai_uint32_t lanes[8] = { 0, 0, 0, 0, 0, 0, 0, 0 }; + attr.id = SAI_PORT_ATTR_HW_LANE_LIST; + attr.value.u32list.count = 8; + attr.value.u32list.list = lanes; + status = sai_port_api->get_port_attribute(port_list[i], 1, &attr); + if (status != SAI_STATUS_SUCCESS) + { + throw std::runtime_error("failed to get hw lane list"); + } + + for (uint32_t j = 0; j < attr.value.u32list.count; ++j) + { + lanes_str += (j == 0) ? "" : ","; + lanes_str += to_string(attr.value.u32list.list[j]); + } + + vector fvs = { + { "lanes", lanes_str }, + { "speed", "1000" }, + { "mtu", "6000" }, + { "admin_status", "up" } + }; + + auto key = FRONT_PANEL_PORT_PREFIX + to_string(i); + + ports[key] = fvs; + } + + return ports; + } +} From 51c230e4f8c8d3734197f759a68b34e859395853 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Thu, 31 Oct 2019 12:20:00 +0200 Subject: [PATCH 10/11] [orchdaemon] fix removed sfloworch Signed-off-by: Stepan Blyschak --- orchagent/orchdaemon.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 7e7575385c..df151f9966 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -208,7 +208,7 @@ bool OrchDaemon::init() * when iterating ConsumerMap. * That is ensured implicitly by the order of map key, "LAG_TABLE" is smaller than "VLAN_TABLE" in lexicographic order. */ - m_orchList = { gSwitchOrch, gCrmOrch, gBufferOrch, gPortsOrch, gIntfsOrch, gNeighOrch, gRouteOrch, copp_orch, tunnel_decap_orch, qos_orch, wm_orch, policer_orch }; + m_orchList = { gSwitchOrch, gCrmOrch, gBufferOrch, gPortsOrch, gIntfsOrch, gNeighOrch, gRouteOrch, copp_orch, tunnel_decap_orch, qos_orch, wm_orch, policer_orch, sflow_orch}; bool initialize_dtel = false; From 86b45eb603a0caec495cab04b050c053ef29bf15 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Fri, 1 Nov 2019 11:06:14 +0000 Subject: [PATCH 11/11] [mock_tests] make mock_tests run on "make check" Signed-off-by: Stepan Blyschak --- tests/mock_tests/Makefile.am | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am index 69b1744836..247a33bf1a 100644 --- a/tests/mock_tests/Makefile.am +++ b/tests/mock_tests/Makefile.am @@ -1,6 +1,8 @@ CFLAGS_SAI = -I /usr/include/sai -bin_PROGRAMS = tests +TESTS = tests + +noinst_PROGRAMS = tests LDADD_SAI = -lsaimeta -lsaimetadata -lsaivs -lsairedis