From 6dbebcacca27d6c8f57fe23eb69826edb9e2a060 Mon Sep 17 00:00:00 2001 From: Venkit Kasiviswanathan Date: Tue, 12 May 2026 00:49:01 +0000 Subject: [PATCH 1/7] [orchagent]: Add ZmqRouteServer for concurrent route updates Introduce a dedicated ZmqRouteServer (and ZmqRouteOrch/ZmqRouteConsumer) used by RouteOrch for receiving APPL_DB route updates from fpmsyncd. Non-fabric/non-dpu switches now create a ZmqRouteServer instead of the generic ZmqServer; fabric and DPU continue to use ZmqServer. ZmqRouteConsumer merges incoming tuples into m_toSync from the mqPollThread ingress callback under m_toSyncMutex, and only notifies the orch main loop once a batch (gMaxBulkSize) has accumulated. To support this concurrent access, ConsumerBase::addToSync and dumpPendingTasks are made virtual so the route consumer can wrap them in a lock, while the default single-threaded base remains lock-free. Also: - create_zmq_route_server() factory added in lib/orch_zmq_config. - getCfgSwitchType() moved earlier in orchagent main so the server type can be chosen based on switch type. - Update fake_zmqserver to return a ZmqMessageHandler* from handleReceivedData to match the new upstream signature. - Add zmq_route_orch_ut.cpp unit tests. Signed-off-by: Venkit Kasiviswanathan --- lib/orch_zmq_config.cpp | 16 + lib/orch_zmq_config.h | 2 + orchagent/Makefile.am | 1 + orchagent/main.cpp | 11 +- orchagent/orch.cpp | 4 + orchagent/orch.h | 14 +- orchagent/orchdaemon.cpp | 2 +- orchagent/p4orch/tests/Makefile.am | 1 + orchagent/p4orch/tests/fake_zmqserver.cpp | 2 +- orchagent/routeorch.cpp | 4 +- orchagent/routeorch.h | 8 +- orchagent/zmqrouteorch.cpp | 114 +++++++ orchagent/zmqrouteorch.h | 49 +++ tests/mock_tests/Makefile.am | 2 + tests/mock_tests/zmq_route_orch_ut.cpp | 359 ++++++++++++++++++++++ 15 files changed, 573 insertions(+), 16 deletions(-) create mode 100644 orchagent/zmqrouteorch.cpp create mode 100644 orchagent/zmqrouteorch.h create mode 100644 tests/mock_tests/zmq_route_orch_ut.cpp diff --git a/lib/orch_zmq_config.cpp b/lib/orch_zmq_config.cpp index 09bc66e0b0c..2775e1981c4 100644 --- a/lib/orch_zmq_config.cpp +++ b/lib/orch_zmq_config.cpp @@ -78,6 +78,22 @@ std::shared_ptr swss::create_zmq_server(std::string zmq_address return std::make_shared(zmq_address, vrf, true); } +std::shared_ptr swss::create_zmq_route_server(std::string zmq_address, std::string vrf) +{ + if (!std::regex_search(zmq_address, ZMQ_NONE_IPV6_ADDRESS_WITH_PORT) + && !std::regex_search(zmq_address, ZMQ_IPV6_ADDRESS_WITH_PORT)) + { + auto zmq_port = get_zmq_port(); + zmq_address = zmq_address + ":" + std::to_string(zmq_port); + } + + SWSS_LOG_NOTICE("Create ZMQ server with address: %s", zmq_address.c_str()); + + // To prevent message loss between ZmqServer's bind operation and the creation of ZmqProducerStateTable, + // use lazy binding and call bind() only after the handler has been registered. + return std::make_shared(zmq_address, vrf, true); +} + bool swss::get_feature_status(std::string feature, bool default_value) { std::shared_ptr enabled = nullptr; diff --git a/lib/orch_zmq_config.h b/lib/orch_zmq_config.h index 68aff440dbd..93da34cdcfb 100644 --- a/lib/orch_zmq_config.h +++ b/lib/orch_zmq_config.h @@ -9,6 +9,7 @@ #include "zmqclient.h" #include "zmqserver.h" #include "zmqproducerstatetable.h" +#include "zmqrouteserver.h" /* * swssconfig will only connect to local orchagent ZMQ endpoint. @@ -34,6 +35,7 @@ int get_zmq_port(); std::shared_ptr create_zmq_client(std::string zmq_address, std::string vrf=""); std::shared_ptr create_zmq_server(std::string zmq_address, std::string vrf=""); +std::shared_ptr create_zmq_route_server(std::string zmq_address, std::string vrf=""); bool get_feature_status(std::string feature, bool default_value); diff --git a/orchagent/Makefile.am b/orchagent/Makefile.am index 9a372674751..ae0f5509705 100644 --- a/orchagent/Makefile.am +++ b/orchagent/Makefile.am @@ -117,6 +117,7 @@ orchagent_SOURCES = \ response_publisher.cpp \ nvgreorch.cpp \ zmqorch.cpp \ + zmqrouteorch.cpp \ dash/dashenifwdorch.cpp \ dash/dashenifwdinfo.cpp \ dash/dashcounter.cpp \ diff --git a/orchagent/main.cpp b/orchagent/main.cpp index 702064a5f37..cc864ba5b56 100644 --- a/orchagent/main.cpp +++ b/orchagent/main.cpp @@ -642,6 +642,9 @@ int main(int argc, char **argv) DBConnector config_db("CONFIG_DB", 0); DBConnector state_db("STATE_DB", 0); + // Get switch_type + getCfgSwitchType(&config_db, gMySwitchType, gMySwitchSubType); + // Instantiate ZMQ server shared_ptr zmq_server = nullptr; if (zmq_server_address.empty()) @@ -651,12 +654,12 @@ int main(int argc, char **argv) else { SWSS_LOG_NOTICE("The ZMQ channel on the northbound side of orchagent has been initialized: %s, %s", zmq_server_address.c_str(), vrf.c_str()); - zmq_server = create_zmq_server(zmq_server_address); + if (gMySwitchType == "fabric" || gMySwitchType == "dpu") + zmq_server = create_zmq_server(zmq_server_address); + else + zmq_server = create_zmq_route_server(zmq_server_address); } - // Get switch_type - getCfgSwitchType(&config_db, gMySwitchType, gMySwitchSubType); - sai_attribute_t attr; vector attrs; diff --git a/orchagent/orch.cpp b/orchagent/orch.cpp index 2a9e72b03a0..1ed99251264 100644 --- a/orchagent/orch.cpp +++ b/orchagent/orch.cpp @@ -411,6 +411,10 @@ size_t ConsumerBase::addToSync(const std::deque &entries recordTuples(entries); } + // Call addToSyncInternal directly so we don't re-enter virtual dispatch + // (and any subclass-installed lock) per entry. Subclasses that need + // locking override addToSync(deque) to take the lock once before calling + // this base implementation. for (auto& entry: entries) { addToSyncInternal(entry, onRetry, onRetry); diff --git a/orchagent/orch.h b/orchagent/orch.h index 35b79ef65fc..8de5ad2452f 100644 --- a/orchagent/orch.h +++ b/orchagent/orch.h @@ -167,7 +167,13 @@ class ConsumerBase : public Executor { } std::string dumpTuple(const swss::KeyOpFieldsValuesTuple &tuple); - void dumpPendingTasks(std::vector &ts); + + /* + * dumpPendingTasks and the addToSync overloads are virtual so concurrent + * subclasses (e.g. ZmqRouteConsumer) can wrap the base implementation in + * a lock. The base class itself is single-threaded and takes no lock. + */ + virtual void dumpPendingTasks(std::vector &ts); /* Store the latest 'golden' status */ // TODO: hide? @@ -177,11 +183,11 @@ class ConsumerBase : public Executor { void recordTuple(const swss::KeyOpFieldsValuesTuple &tuple); void recordTuples(const std::deque &entries); - void addToSync(const swss::KeyOpFieldsValuesTuple &entry, bool onRetry=false); + virtual void addToSync(const swss::KeyOpFieldsValuesTuple &entry, bool onRetry=false); // Returns: the number of entries added to m_toSync - size_t addToSync(const std::deque &entries, bool onRetry=false); - size_t addToSync(std::shared_ptr> entries, bool onRetry=false); + virtual size_t addToSync(const std::deque &entries, bool onRetry=false); + size_t addToSync(std::shared_ptr> entries, bool onRetry=false); /** * @brief Add the failed task and its constraint to the consumer's RetryCache diff --git a/orchagent/orchdaemon.cpp b/orchagent/orchdaemon.cpp index 4c14dad7097..e7623f2f58a 100644 --- a/orchagent/orchdaemon.cpp +++ b/orchagent/orchdaemon.cpp @@ -332,7 +332,7 @@ bool OrchDaemon::init() // Enable the fpmsyncd service to send Route events to orchagent via the ZMQ channel. auto enable_route_zmq = get_feature_status(ORCH_NORTHBOND_ROUTE_ZMQ_ENABLED, false); - auto route_zmq_sever = enable_route_zmq ? m_zmqServer : nullptr; + auto route_zmq_sever = enable_route_zmq ? dynamic_cast(m_zmqServer) : nullptr; gRouteOrch = new RouteOrch(m_applDb, route_tables, gSwitchOrch, gNeighOrch, gIntfsOrch, vrf_orch, gFgNhgOrch, gSrv6Orch, route_zmq_sever); gNhgOrch = new NhgOrch(m_applDb, APP_NEXTHOP_GROUP_TABLE_NAME); diff --git a/orchagent/p4orch/tests/Makefile.am b/orchagent/p4orch/tests/Makefile.am index cfa3881e264..30e0676dfd6 100644 --- a/orchagent/p4orch/tests/Makefile.am +++ b/orchagent/p4orch/tests/Makefile.am @@ -29,6 +29,7 @@ p4orch_tests_SOURCES = $(ORCHAGENT_DIR)/orch.cpp \ $(ORCHAGENT_DIR)/request_parser.cpp \ $(top_srcdir)/lib/recorder.cpp \ $(ORCHAGENT_DIR)/zmqorch.cpp \ + $(ORCHAGENT_DIR)/zmqrouteorch.cpp \ $(ORCHAGENT_DIR)/flex_counter/flex_counter_manager.cpp \ $(ORCHAGENT_DIR)/flex_counter/flow_counter_handler.cpp \ $(ORCHAGENT_DIR)/port/port_capabilities.cpp \ diff --git a/orchagent/p4orch/tests/fake_zmqserver.cpp b/orchagent/p4orch/tests/fake_zmqserver.cpp index 505a5f0acee..64f8d47d8c6 100644 --- a/orchagent/p4orch/tests/fake_zmqserver.cpp +++ b/orchagent/p4orch/tests/fake_zmqserver.cpp @@ -22,7 +22,7 @@ ZmqMessageHandler* ZmqServer::findMessageHandler(const std::string dbName, return nullptr; } -void ZmqServer::handleReceivedData(const char* buffer, const size_t size) {} +ZmqMessageHandler* ZmqServer::handleReceivedData(const char* buffer, const size_t size) { return nullptr; } void ZmqServer::mqPollThread() {} diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index b8fba72469f..bde2a79bcc2 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -37,11 +37,11 @@ extern string gMySwitchType; #define DEFAULT_NUMBER_OF_ECMP_GROUPS 128 #define DEFAULT_MAX_ECMP_GROUP_SIZE 32 -RouteOrch::RouteOrch(DBConnector *db, vector &tableNames, SwitchOrch *switchOrch, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch, FgNhgOrch *fgNhgOrch, Srv6Orch *srv6Orch, swss::ZmqServer *zmqServer) : +RouteOrch::RouteOrch(DBConnector *db, vector &tableNames, SwitchOrch *switchOrch, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch, FgNhgOrch *fgNhgOrch, Srv6Orch *srv6Orch, ZmqRouteServer *zmqRouteServer) : gRouteBulker(sai_route_api, gMaxBulkSize), gLabelRouteBulker(sai_mpls_api, gMaxBulkSize), gNextHopGroupMemberBulker(sai_next_hop_group_api, gSwitchId, gMaxBulkSize), - ZmqOrch(db, tableNames, zmqServer), + ZmqRouteOrch(db, tableNames, zmqRouteServer), m_switchOrch(switchOrch), m_neighOrch(neighOrch), m_intfsOrch(intfsOrch), diff --git a/orchagent/routeorch.h b/orchagent/routeorch.h index 5fdb5b8e462..8f9b458d694 100644 --- a/orchagent/routeorch.h +++ b/orchagent/routeorch.h @@ -16,8 +16,8 @@ #include "bulker.h" #include "fgnhgorch.h" #include -#include "zmqorch.h" -#include "zmqserver.h" +#include "zmqrouteorch.h" +#include "zmqrouteserver.h" #include /* Maximum next hop group number */ @@ -212,10 +212,10 @@ struct LabelRouteBulkContext } }; -class RouteOrch : public ZmqOrch, public Subject +class RouteOrch : public ZmqRouteOrch, public Subject { public: - RouteOrch(DBConnector *db, vector &tableNames, SwitchOrch *switchOrch, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch, FgNhgOrch *fgNhgOrch, Srv6Orch *srv6Orch, swss::ZmqServer *zmqServer = nullptr); + RouteOrch(DBConnector *db, vector &tableNames, SwitchOrch *switchOrch, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch, FgNhgOrch *fgNhgOrch, Srv6Orch *srv6Orch, ZmqRouteServer *zmqServer = nullptr); bool hasNextHopGroup(const NextHopGroupKey&) const; sai_object_id_t getNextHopGroupId(const NextHopGroupKey&); diff --git a/orchagent/zmqrouteorch.cpp b/orchagent/zmqrouteorch.cpp new file mode 100644 index 00000000000..fdb2a4be9c8 --- /dev/null +++ b/orchagent/zmqrouteorch.cpp @@ -0,0 +1,114 @@ +#include "zmqrouteorch.h" + +using namespace swss; +using namespace std; + +extern int gBatchSize; +extern size_t gMaxBulkSize; + +ZmqRouteConsumer::ZmqRouteConsumer(ZmqRouteConsumerStateTable *select, Orch *orch, const std::string &name) + : ConsumerBase(select, orch, name) +{ + // mqPollThread runs the merge inline: kcos go straight into m_toSync + // under m_toSyncMutex. The eventfd is fired only when m_toSync grows past + // gMaxBulkSize (so the orch main loop has a real batch to drain); + // otherwise mqPollThread fires it once per burst after the burst quiesces. + select->setIngressCallback( + [this, select](const std::vector> &kcos) { + std::lock_guard lk(m_toSyncMutex); + for (const auto &kco : kcos) + { + // Qualified call to bypass our own virtual override (which + // would re-acquire m_toSyncMutex per entry). + ConsumerBase::addToSync(*kco, /*onRetry=*/false); + } + if (m_toSync.size() >= gMaxBulkSize) + { + select->notifyPending(); + } + }); +} + +void ZmqRouteConsumer::execute() +{ + SWSS_LOG_ENTER(); + + // Tuples were already merged into m_toSync by the ingress callback running + // on mqPollThread. The main loop's job is just to drain. + drain(); +} + +void ZmqRouteConsumer::drain() +{ + std::lock_guard lk(m_toSyncMutex); + if (!m_toSync.empty()) + (static_cast(m_orch))->doTask(*this); +} + +void ZmqRouteConsumer::addToSync(const KeyOpFieldsValuesTuple &entry, bool onRetry) +{ + std::lock_guard lk(m_toSyncMutex); + ConsumerBase::addToSync(entry, onRetry); +} + +size_t ZmqRouteConsumer::addToSync(const std::deque &entries, bool onRetry) +{ + std::lock_guard lk(m_toSyncMutex); + return ConsumerBase::addToSync(entries, onRetry); +} + +void ZmqRouteConsumer::dumpPendingTasks(std::vector &ts) +{ + std::lock_guard lk(m_toSyncMutex); + ConsumerBase::dumpPendingTasks(ts); +} + + +ZmqRouteOrch::ZmqRouteOrch(DBConnector *db, const vector &tableNames, ZmqRouteServer *zmqServer) +: Orch() +{ + for (auto it : tableNames) + { + addConsumer(db, it, default_orch_pri, zmqServer); + } +} + + +ZmqRouteOrch::ZmqRouteOrch(DBConnector *db, const vector &tableNames_with_pri, ZmqRouteServer *zmqServer) +{ + for (const auto& it : tableNames_with_pri) + { + addConsumer(db, it.first, it.second, zmqServer); + } +} + +void ZmqRouteOrch::addConsumer(DBConnector *db, string tableName, int pri, ZmqRouteServer *zmqServer) +{ + if (db->getDbId() == APPL_DB || db->getDbId() == DPU_APPL_DB) + { + if (zmqServer != nullptr) + { + SWSS_LOG_DEBUG("ZmqRouteConsumer initialize for: %s", tableName.c_str()); + addExecutor( + new ZmqRouteConsumer( + new ZmqRouteConsumerStateTable( + db, tableName, *zmqServer, pri, /* dbPersistence= */false), + this, tableName)); + } + else + { + SWSS_LOG_DEBUG("Consumer initialize for: %s", tableName.c_str()); + addExecutor(new Consumer(new ConsumerStateTable(db, tableName, gBatchSize, pri), this, tableName)); + } + } + else + { + SWSS_LOG_WARN("ZmqRouteOrch does not support create consumer for db: %d, table: %s", db->getDbId(), tableName.c_str()); + } +} + +void ZmqRouteOrch::doTask(Consumer &consumer) +{ + // When ZMQ disabled, forward data from Consumer + doTask((ConsumerBase &)consumer); +} diff --git a/orchagent/zmqrouteorch.h b/orchagent/zmqrouteorch.h new file mode 100644 index 00000000000..aa3169007c4 --- /dev/null +++ b/orchagent/zmqrouteorch.h @@ -0,0 +1,49 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include "zmqrouteserver.h" +#include "zmqrouteconsumerstatetable.h" + +extern int gZmqExecuteTimeQuantaMsecs; + +class ZmqRouteConsumer : public ConsumerBase { +public: + ZmqRouteConsumer(ZmqRouteConsumerStateTable *select, Orch *orch, const std::string &name); + + swss::TableBase *getConsumerTable() const override + { + // ZmqRouteConsumerStateTable is a subclass of TableBase + return static_cast(getSelectable()); + } + + void execute() override; + void drain() override; + + // Locked overrides: take m_toSyncMutex, then forward to ConsumerBase. + // These guard against the ZmqRouteServer mqPollThread (calling addToSync + // via the ingress callback) racing with the orch main thread. + void addToSync(const swss::KeyOpFieldsValuesTuple &entry, bool onRetry=false) override; + size_t addToSync(const std::deque &entries, bool onRetry=false) override; + void dumpPendingTasks(std::vector &ts) override; + +private: + mutable std::mutex m_toSyncMutex; +}; + +class ZmqRouteOrch : public Orch +{ +public: + ZmqRouteOrch(swss::DBConnector *db, const std::vector &tableNames, ZmqRouteServer *zmqServer); + ZmqRouteOrch(swss::DBConnector *db, const std::vector &tableNames_with_pri, ZmqRouteServer *zmqServer); + + virtual void doTask(ConsumerBase &consumer) { }; + void doTask(Consumer &consumer) override; + +private: + void addConsumer(swss::DBConnector *db, std::string tableName, int pri, ZmqRouteServer *zmqServer); +}; diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am index 7bbdda9ffba..5083924aa55 100644 --- a/tests/mock_tests/Makefile.am +++ b/tests/mock_tests/Makefile.am @@ -85,6 +85,7 @@ tests_SOURCES = aclorch_ut.cpp \ mock_orch_test.cpp \ mock_dash_orch_test.cpp \ zmq_orch_ut.cpp \ + zmq_route_orch_ut.cpp \ retrycache_ut.cpp \ mock_saihelper.cpp \ mirrororch_ut.cpp \ @@ -161,6 +162,7 @@ tests_SOURCES = aclorch_ut.cpp \ $(top_srcdir)/cfgmgr/portmgr.cpp \ $(top_srcdir)/cfgmgr/sflowmgr.cpp \ $(top_srcdir)/orchagent/zmqorch.cpp \ + $(top_srcdir)/orchagent/zmqrouteorch.cpp \ $(top_srcdir)/orchagent/dash/dashenifwdorch.cpp \ $(top_srcdir)/orchagent/dash/dashenifwdinfo.cpp \ $(top_srcdir)/orchagent/dash/dashaclorch.cpp \ diff --git a/tests/mock_tests/zmq_route_orch_ut.cpp b/tests/mock_tests/zmq_route_orch_ut.cpp new file mode 100644 index 00000000000..3d94c05cb40 --- /dev/null +++ b/tests/mock_tests/zmq_route_orch_ut.cpp @@ -0,0 +1,359 @@ +#include +#include +#include +#include +#include +#include + +#include "gtest/gtest.h" +#include "schema.h" +#include "ut_helper.h" +#include "orch_zmq_config.h" +#include "dbconnector.h" +#include "mock_table.h" +#include "select.h" +#include "zmqclient.h" +#include "zmqproducerstatetable.h" +#include "zmqrouteserver.h" +#include "zmqrouteconsumerstatetable.h" + +#define protected public +#include "orch.h" +#include "zmqrouteorch.h" +#undef protected + +using namespace std; +using namespace swss; + +extern size_t gMaxBulkSize; + +namespace { + +// Wait until pred() becomes true or deadlineMs elapses; returns the final value. +template +bool waitFor(int deadlineMs, Pred pred) +{ + auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(deadlineMs); + while (std::chrono::steady_clock::now() < deadline) + { + if (pred()) + return true; + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + } + return pred(); +} + +// Minimal subclass of ZmqRouteOrch that records doTask invocations, so tests +// can assert that drain() forwards correctly without needing a full RouteOrch. +class RecordingZmqRouteOrch : public ZmqRouteOrch +{ +public: + RecordingZmqRouteOrch(swss::DBConnector *db, + const std::vector &tables, + ZmqRouteServer *zmqServer) + : ZmqRouteOrch(db, tables, zmqServer) + { + } + + void doTask(ConsumerBase &consumer) override + { + ++doTaskCount; + // Drain the consumer's m_toSync so subsequent drain() calls observe it + // as empty (matches the contract a real orch would honor). + consumer.m_toSync.clear(); + } + + std::atomic doTaskCount{0}; +}; + +} // namespace + +// ZmqRouteOrch with a nullptr server falls back to plain Consumer (legacy +// non-ZMQ path) for APPL_DB tables. +TEST(ZmqRouteOrchTest, NullServerFallsBackToConsumer) +{ + vector tables = { + { "ZMQ_ROUTE_UT_T1", 1 }, + { "ZMQ_ROUTE_UT_T2", 2 }, + }; + auto app_db = make_shared("APPL_DB", 0); + auto orch = make_shared(app_db.get(), tables, nullptr); + + EXPECT_EQ(orch->getSelectables().size(), tables.size()); + // Ensure the executor is a plain Consumer (not a ZmqRouteConsumer): the + // legacy fallback shouldn't pull in the ZmqRouteConsumer machinery. + auto exec = orch->m_consumerMap.begin()->second.get(); + EXPECT_EQ(dynamic_cast(exec), nullptr); +} + +// vector ctor (no per-table priority) — exercises the +// default_orch_pri code path in ZmqRouteOrch::ZmqRouteOrch(vector,...). +TEST(ZmqRouteOrchTest, VectorOfStringsCtor) +{ + vector tables = { "ZMQ_ROUTE_UT_TS1", "ZMQ_ROUTE_UT_TS2" }; + auto app_db = make_shared("APPL_DB", 0); + auto orch = make_shared(app_db.get(), tables, nullptr); + EXPECT_EQ(orch->getSelectables().size(), tables.size()); +} + +// Non-APPL_DB databases are unsupported; addConsumer should warn and create +// no executor. +TEST(ZmqRouteOrchTest, UnsupportedDbProducesNoExecutor) +{ + vector tables = { { "ZMQ_ROUTE_UT_T1", 1 } }; + auto state_db = make_shared("STATE_DB", 0); + auto orch = make_shared(state_db.get(), tables, nullptr); + EXPECT_EQ(orch->getSelectables().size(), 0u); +} + +// With a real ZmqRouteServer, ZmqRouteOrch creates a ZmqRouteConsumer (not a +// plain Consumer). The server must outlive the orch. +TEST(ZmqRouteOrchTest, RealServerCreatesZmqRouteConsumer) +{ + vector tables = { { "ZMQ_ROUTE_UT_T1", 1 } }; + auto app_db = make_shared("APPL_DB", 0); + ZmqRouteServer server("tcp://*:1260", "", /*lazyBind=*/true); + + auto orch = make_shared(app_db.get(), tables, &server); + ASSERT_EQ(orch->getSelectables().size(), tables.size()); + + auto exec = orch->m_consumerMap.begin()->second.get(); + EXPECT_NE(dynamic_cast(exec), nullptr); +} + +// doTask(Consumer&) on the base ZmqRouteOrch is a stub that forwards to the +// virtual doTask(ConsumerBase&) — this is the only piece that ZmqRouteOrch +// itself implements (besides ctors / addConsumer). Cover it. +TEST(ZmqRouteOrchTest, DoTaskConsumerForwardsToConsumerBase) +{ + vector tables = { { "ZMQ_ROUTE_UT_T1", 1 } }; + auto app_db = make_shared("APPL_DB", 0); + auto orch = make_shared(app_db.get(), tables, nullptr); + + auto *exec = orch->m_consumerMap.begin()->second.get(); + auto *consumer = dynamic_cast(exec); + ASSERT_NE(consumer, nullptr); + + // Forge a single entry into m_toSync so that the recording doTask can see + // something and so the subsequent clear() actually does work. SyncMap is a + // multimap, so use insert rather than operator[]. + consumer->m_toSync.insert({ + "k1", + std::make_tuple(std::string("k1"), std::string(SET_COMMAND), + std::vector{{"f", "v"}}) + }); + + // ZmqRouteOrch::doTask(Consumer&) forwards to doTask(ConsumerBase&). + static_cast(orch.get())->doTask(*consumer); + EXPECT_EQ(orch->doTaskCount.load(), 1); + EXPECT_TRUE(consumer->m_toSync.empty()); +} + +// Drain on a ZmqRouteConsumer with empty m_toSync must NOT call doTask. +// Drain on a non-empty m_toSync must call doTask exactly once and the lock +// must allow re-entry afterwards. +TEST(ZmqRouteConsumerTest, DrainGatedByToSyncEmptiness) +{ + vector tables = { { "ZMQ_ROUTE_UT_T1", 1 } }; + auto app_db = make_shared("APPL_DB", 0); + ZmqRouteServer server("tcp://*:1261", "", /*lazyBind=*/true); + auto orch = make_shared(app_db.get(), tables, &server); + + auto *exec = orch->m_consumerMap.begin()->second.get(); + auto *zrc = dynamic_cast(exec); + ASSERT_NE(zrc, nullptr); + + // Empty m_toSync: drain is a no-op. + zrc->drain(); + EXPECT_EQ(orch->doTaskCount.load(), 0); + + // Stage one entry via the locked addToSync override; drain forwards to + // doTask exactly once. RecordingZmqRouteOrch::doTask clears m_toSync. + KeyOpFieldsValuesTuple kfv("route_a", SET_COMMAND, + vector{{"f", "v"}}); + zrc->addToSync(kfv); + EXPECT_EQ(zrc->m_toSync.size(), 1u); + + zrc->drain(); + EXPECT_EQ(orch->doTaskCount.load(), 1); + EXPECT_TRUE(zrc->m_toSync.empty()); + + // A subsequent empty drain still doesn't call doTask, and the lock + // re-acquires cleanly. + zrc->drain(); + EXPECT_EQ(orch->doTaskCount.load(), 1); +} + +// execute() simply calls drain(); cover that override. +TEST(ZmqRouteConsumerTest, ExecuteDelegatesToDrain) +{ + vector tables = { { "ZMQ_ROUTE_UT_T1", 1 } }; + auto app_db = make_shared("APPL_DB", 0); + ZmqRouteServer server("tcp://*:1262", "", /*lazyBind=*/true); + auto orch = make_shared(app_db.get(), tables, &server); + + auto *zrc = dynamic_cast( + orch->m_consumerMap.begin()->second.get()); + ASSERT_NE(zrc, nullptr); + + KeyOpFieldsValuesTuple kfv("route_b", SET_COMMAND, + vector{{"f", "v"}}); + zrc->addToSync(kfv); + + zrc->execute(); + EXPECT_EQ(orch->doTaskCount.load(), 1); +} + +// Locked deque-form addToSync forwards to ConsumerBase::addToSync(deque) and +// returns the count. +TEST(ZmqRouteConsumerTest, AddToSyncDequeReturnsCount) +{ + vector tables = { { "ZMQ_ROUTE_UT_T1", 1 } }; + auto app_db = make_shared("APPL_DB", 0); + ZmqRouteServer server("tcp://*:1263", "", /*lazyBind=*/true); + auto orch = make_shared(app_db.get(), tables, &server); + + auto *zrc = dynamic_cast( + orch->m_consumerMap.begin()->second.get()); + ASSERT_NE(zrc, nullptr); + + std::deque entries; + for (int i = 0; i < 5; ++i) + { + entries.emplace_back("k" + std::to_string(i), SET_COMMAND, + vector{{"f", "v"}}); + } + + EXPECT_EQ(zrc->addToSync(entries), 5u); + EXPECT_EQ(zrc->m_toSync.size(), 5u); +} + +// dumpPendingTasks (locked override) returns the staged entries as strings +// and doesn't deadlock with concurrent addToSync. +TEST(ZmqRouteConsumerTest, DumpPendingTasksLockedAndCorrect) +{ + vector tables = { { "ZMQ_ROUTE_UT_T1", 1 } }; + auto app_db = make_shared("APPL_DB", 0); + ZmqRouteServer server("tcp://*:1264", "", /*lazyBind=*/true); + auto orch = make_shared(app_db.get(), tables, &server); + + auto *zrc = dynamic_cast( + orch->m_consumerMap.begin()->second.get()); + ASSERT_NE(zrc, nullptr); + + zrc->addToSync(KeyOpFieldsValuesTuple("kA", SET_COMMAND, + vector{{"f", "v"}})); + zrc->addToSync(KeyOpFieldsValuesTuple("kB", DEL_COMMAND, + vector{})); + + std::vector ts; + zrc->dumpPendingTasks(ts); + EXPECT_EQ(ts.size(), 2u); +} + +// Concurrent addToSync from multiple threads must not crash, lose entries, or +// deadlock with drain. This guards the locking contract that ZmqRouteServer +// relies on (mqPollThread races with the orch main thread). +TEST(ZmqRouteConsumerTest, ConcurrentAddToSyncIsThreadSafe) +{ + vector tables = { { "ZMQ_ROUTE_UT_T1", 1 } }; + auto app_db = make_shared("APPL_DB", 0); + ZmqRouteServer server("tcp://*:1265", "", /*lazyBind=*/true); + auto orch = make_shared(app_db.get(), tables, &server); + + auto *zrc = dynamic_cast( + orch->m_consumerMap.begin()->second.get()); + ASSERT_NE(zrc, nullptr); + + constexpr int kThreads = 4; + constexpr int kPerThread = 250; + std::vector producers; + for (int t = 0; t < kThreads; ++t) + { + producers.emplace_back([zrc, t]() { + for (int i = 0; i < kPerThread; ++i) + { + std::string k = "t" + std::to_string(t) + "_" + std::to_string(i); + zrc->addToSync(KeyOpFieldsValuesTuple( + k, SET_COMMAND, vector{{"f", "v"}})); + } + }); + } + for (auto &th : producers) + th.join(); + + EXPECT_EQ(zrc->m_toSync.size(), + static_cast(kThreads * kPerThread)); +} + +// End-to-end: ZmqProducerStateTable → ZmqRouteServer → ZmqRouteConsumer +// ingress callback → m_toSync. Verifies the callback wiring set up by +// ZmqRouteConsumer's constructor actually merges entries into m_toSync, and +// (since count < gMaxBulkSize) does not eagerly fire notifyPending — the +// burst quiesce timer fires it instead. +TEST(ZmqRouteConsumerTest, IngressCallbackMergesIntoToSync) +{ + const string tableName = "ZMQ_ROUTE_UT_INGRESS"; + const string pushEndpoint = "tcp://localhost:1266"; + const string pullEndpoint = "tcp://*:1266"; + + vector tables = { { tableName, 1 } }; + auto app_db = make_shared("APPL_DB", 0); + ZmqRouteServer server(pullEndpoint, "", /*lazyBind=*/true); + auto orch = make_shared(app_db.get(), tables, &server); + auto *zrc = dynamic_cast( + orch->m_consumerMap.begin()->second.get()); + ASSERT_NE(zrc, nullptr); + + server.bind(); + + ZmqClient client(pushEndpoint, 0); + ZmqProducerStateTable p(app_db.get(), tableName, client, /*dbPersistence=*/false); + p.set("route_x", vector{{"nh", "1.1.1.1"}}); + + ASSERT_TRUE(waitFor(2000, [&] { return zrc->m_toSync.size() >= 1u; })); + EXPECT_NE(zrc->m_toSync.find("route_x"), zrc->m_toSync.end()); +} + +// When the ingress callback fills m_toSync past gMaxBulkSize, it must fire +// notifyPending mid-burst (rather than waiting for the burst quiesce timer) +// so the orch main loop wakes up and drains immediately. We lower +// gMaxBulkSize to 1 to make this trivially observable. +TEST(ZmqRouteConsumerTest, IngressCallbackFiresNotifyAtMaxBulkSize) +{ + const string tableName = "ZMQ_ROUTE_UT_BULK"; + const string pushEndpoint = "tcp://localhost:1267"; + const string pullEndpoint = "tcp://*:1267"; + + vector tables = { { tableName, 1 } }; + auto app_db = make_shared("APPL_DB", 0); + ZmqRouteServer server(pullEndpoint, "", /*lazyBind=*/true); + auto orch = make_shared(app_db.get(), tables, &server); + auto *zrc = dynamic_cast( + orch->m_consumerMap.begin()->second.get()); + ASSERT_NE(zrc, nullptr); + + server.bind(); + + // Force the mid-burst notify branch to trip on the very first callback. + const size_t savedMaxBulk = gMaxBulkSize; + gMaxBulkSize = 1; + + ZmqClient client(pushEndpoint, 0); + ZmqProducerStateTable p(app_db.get(), tableName, client, /*dbPersistence=*/false); + p.set("route_bulk", vector{{"nh", "2.2.2.2"}}); + + ASSERT_TRUE(waitFor(2000, [&] { return zrc->m_toSync.size() >= 1u; })); + + // Select wake-up should arrive almost immediately because the ingress + // callback fires notifyPending the moment m_toSync reaches gMaxBulkSize=1 + // — without this we'd have to wait for BURST_QUIESCE_MS (~5ms) before the + // post-burst notify fires. + Select sel; + sel.addSelectable(zrc); + Selectable *out = nullptr; + EXPECT_EQ(sel.select(&out, 200), Select::OBJECT); + EXPECT_EQ(out, zrc); + + gMaxBulkSize = savedMaxBulk; +} From 69fdc761a1f14f3fa25c885dca5af3989d1d1cbb Mon Sep 17 00:00:00 2001 From: Venkit Kasiviswanathan Date: Wed, 29 Jul 2026 20:38:45 +0000 Subject: [PATCH 2/7] [orchagent]: Address ZmqRouteConsumer review comments Follow-up to the staging-map redesign of ZmqRouteConsumer, addressing review feedback on the route ingress path. - Rename m_toSyncMutex to m_ingressMutex. The mutex only ever guarded m_ingress; the old name suggested it guarded m_toSync, which is what made the resync path (RouteOrch::doTask calling consumer.addToSync) look like a self-deadlock. Document the threading invariant on the member: m_toSync is owned exclusively by the orch main thread and no lock is held across doTask, so re-entrant addToSync from inside doTask is safe and the base ConsumerBase paths stay lock-free. - Add a TODO in drain() for the bounded, yieldable walk of m_toSync described in sections 7.4 and 7.5 of the route programming HLD (doc/orchagent/orchagent_route_redesign.md, sonic-net/SONiC#2328). Today doTask walks all of m_toSync in one go, which stalls ingress behind a large batch. - Remove the unused extern gZmqExecuteTimeQuantaMsecs declaration. It is a carry-over from zmqorch.h, has no definition anywhere in sonic-swss or sonic-swss-common, and is not referenced by ZmqRouteConsumer. It comes back with the time quanta change that actually uses it. - Add IngressCallbackConcurrentWithDrain: a producer thread stages 1000 routes through the mqPollThread ingress callback while the orch main thread spins execute(), so staging and draining interleave. Asserts every key is delivered exactly once (a new totalEntries counter on the recording orch distinguishes delivered-once from delivered-twice, which the key set alone cannot) and that the run does not deadlock. - Use static_cast instead of a C-style cast in ZmqRouteOrch::doTask(Consumer &). Signed-off-by: Venkit Kasiviswanathan --- orchagent/zmqrouteorch.cpp | 18 +++++--- orchagent/zmqrouteorch.h | 17 +++++--- tests/mock_tests/zmq_route_orch_ut.cpp | 60 ++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 12 deletions(-) diff --git a/orchagent/zmqrouteorch.cpp b/orchagent/zmqrouteorch.cpp index 7f89565b32f..803cbf2e007 100644 --- a/orchagent/zmqrouteorch.cpp +++ b/orchagent/zmqrouteorch.cpp @@ -10,14 +10,14 @@ ZmqRouteConsumer::ZmqRouteConsumer(ZmqRouteConsumerStateTable *select, Orch *orc : ConsumerBase(select, orch, name) { // mqPollThread delivers bursts of tuples through this callback. Stage them - // in the plain m_ingress map under m_toSyncMutex rather than merging into + // in the plain m_ingress map under m_ingressMutex rather than merging into // m_toSync here; the merge into m_toSync happens on the orch main thread in // execute(). The eventfd is fired once the staged batch reaches // gMaxBulkSize (so the main loop has a real batch to drain); otherwise // mqPollThread fires it once per burst after the burst quiesces. select->setIngressCallback( [this, select](const std::vector> &kcos) { - std::lock_guard lk(m_toSyncMutex); + std::lock_guard lk(m_ingressMutex); for (const auto &kco : kcos) { // Plain last-writer-wins staging by key. The SyncMap merge into @@ -39,7 +39,7 @@ void ZmqRouteConsumer::execute() // Drain the staged tuples into m_toSync under the lock, mirroring // ZmqConsumer::execute()'s pops() + addToSync(entries). The lock is // held only while moving tuples out of m_ingress. - std::lock_guard lk(m_toSyncMutex); + std::lock_guard lk(m_ingressMutex); std::deque entries; for (auto &kv : m_ingress) { @@ -50,12 +50,20 @@ void ZmqRouteConsumer::execute() } // m_toSync is mutated only by this (main) thread, so drain() — which reads - // m_toSync and hands it to doTask — does not need to hold m_toSyncMutex. + // m_toSync and hands it to doTask — does not need to hold m_ingressMutex. + // Releasing the lock before drain() also keeps mqPollThread free to stage + // further tuples into m_ingress while doTask is running. drain(); } void ZmqRouteConsumer::drain() { + // TODO: doTask() currently walks the whole of m_toSync in one go. Per the + // route programming HLD -- doc/orchagent/orchagent_route_redesign.md + // sections 7.4 and 7.5 (sonic-net/SONiC#2328) -- this becomes a yieldable + // walk bounded by a time quantum, so the main loop returns to execute() + // and keeps draining m_ingress (and hence the ZMQ socket) instead of + // stalling ingress behind one large batch. if (!m_toSync.empty()) (static_cast(m_orch))->doTask(*this); } @@ -107,5 +115,5 @@ void ZmqRouteOrch::addConsumer(DBConnector *db, string tableName, int pri, ZmqRo void ZmqRouteOrch::doTask(Consumer &consumer) { // When ZMQ disabled, forward data from Consumer - doTask((ConsumerBase &)consumer); + doTask(static_cast(consumer)); } diff --git a/orchagent/zmqrouteorch.h b/orchagent/zmqrouteorch.h index 80315f3449f..6fd1a7a4b5a 100644 --- a/orchagent/zmqrouteorch.h +++ b/orchagent/zmqrouteorch.h @@ -10,8 +10,6 @@ #include "zmqrouteserver.h" #include "zmqrouteconsumerstatetable.h" -extern int gZmqExecuteTimeQuantaMsecs; - class ZmqRouteConsumer : public ConsumerBase { public: ZmqRouteConsumer(ZmqRouteConsumerStateTable *select, Orch *orch, const std::string &name); @@ -27,11 +25,16 @@ class ZmqRouteConsumer : public ConsumerBase { private: // Staging buffer for tuples delivered by the ZmqRouteServer mqPollThread - // ingress callback. The callback writes here under m_toSyncMutex (rather - // than merging into m_toSync directly); execute() drains it into m_toSync - // under the same lock. This keeps m_toSync single-threaded (touched only - // by the orch main thread), so the base ConsumerBase paths need no locking. - std::mutex m_toSyncMutex; + // ingress callback. The callback writes here (rather than merging into + // m_toSync directly); execute() drains it into m_toSync. + // + // Threading invariant: m_ingressMutex guards m_ingress only. m_toSync is + // owned exclusively by the orch main thread (execute() -> drain() -> + // doTask()), and no lock is held across doTask(). That is what keeps + // re-entrant addToSync() calls made from inside doTask() -- e.g. the route + // resync path in RouteOrch::doTask() -- safe rather than a self-deadlock, + // and it lets the base ConsumerBase paths stay lock-free. + std::mutex m_ingressMutex; std::unordered_map m_ingress; }; diff --git a/tests/mock_tests/zmq_route_orch_ut.cpp b/tests/mock_tests/zmq_route_orch_ut.cpp index 3ae59bea123..4df283fafb0 100644 --- a/tests/mock_tests/zmq_route_orch_ut.cpp +++ b/tests/mock_tests/zmq_route_orch_ut.cpp @@ -65,6 +65,7 @@ class RecordingZmqRouteOrch : public ZmqRouteOrch for (const auto &kv : consumer.m_toSync) { seenKeys.insert(kv.first); + ++totalEntries; } // Drain the consumer's m_toSync so subsequent drain() calls observe it // as empty (matches the contract a real orch would honor). @@ -73,6 +74,9 @@ class RecordingZmqRouteOrch : public ZmqRouteOrch std::atomic doTaskCount{0}; std::set seenKeys; + // Total entries handed to doTask across all invocations. Compared against + // seenKeys.size() it distinguishes "delivered once" from "delivered twice". + std::atomic totalEntries{0}; }; } // namespace @@ -335,3 +339,59 @@ TEST(ZmqRouteConsumerTest, IngressCallbackFiresNotifyAtMaxBulkSize) gMaxBulkSize = savedMaxBulk; } + +// The producer (and hence the mqPollThread ingress callback) keeps staging into +// m_ingress while the orch main thread is inside execute()/drain()/doTask(). +// This is the concurrent pair in this design: the callback holds m_ingressMutex +// only to stage, and execute() releases it before drain(), so doTask never runs +// under the lock. Assert that the overlap loses nothing, duplicates nothing and +// does not deadlock. +TEST(ZmqRouteConsumerTest, IngressCallbackConcurrentWithDrain) +{ + const string tableName = "ZMQ_ROUTE_UT_CONCURRENT"; + const string pushEndpoint = "tcp://localhost:1268"; + const string pullEndpoint = "tcp://*:1268"; + constexpr int kRoutes = 1000; + + vector tables = { { tableName, 1 } }; + auto app_db = make_shared("APPL_DB", 0); + ZmqRouteServer server(pullEndpoint, "", /*lazyBind=*/true); + auto orch = make_shared(app_db.get(), tables, &server); + auto *zrc = dynamic_cast( + orch->m_consumerMap.begin()->second.get()); + ASSERT_NE(zrc, nullptr); + + server.bind(); + + ZmqClient client(pushEndpoint, 0); + ZmqProducerStateTable p(app_db.get(), tableName, client, /*dbPersistence=*/false); + + std::atomic producerDone{false}; + std::thread producer([&] { + for (int i = 0; i < kRoutes; ++i) + { + p.set("10.0." + to_string(i / 256) + "." + to_string(i % 256) + "/32", + vector{{"nh", "3.3.3.3"}}); + } + producerDone = true; + }); + + // Spin execute() while the producer is still pushing, so staging and + // draining genuinely interleave rather than running back to back. + waitFor(30000, [&] { + zrc->execute(); + return producerDone.load() && orch->seenKeys.size() == static_cast(kRoutes); + }); + producer.join(); + // Final pass for anything staged after the last predicate evaluation. + ASSERT_TRUE(waitFor(5000, [&] { + zrc->execute(); + return orch->seenKeys.size() == static_cast(kRoutes); + })); + + // Every key delivered, each exactly once (keys are unique per producer + // iteration, so a duplicate delivery would show up as totalEntries drift). + EXPECT_EQ(orch->seenKeys.size(), static_cast(kRoutes)); + EXPECT_EQ(orch->totalEntries.load(), kRoutes); + EXPECT_TRUE(zrc->m_toSync.empty()); +} From 69cadcbfddee0781ad4bf657aed4dbcaea12dabe Mon Sep 17 00:00:00 2001 From: Venkit Kasiviswanathan Date: Thu, 30 Jul 2026 02:30:22 +0000 Subject: [PATCH 3/7] [orchagent]: Sync p4orch RouteOrch test double with ZmqRouteServer ctor p4orch_tests failed to link with: test_main.cpp:288: undefined reference to `RouteOrch::RouteOrch( swss::DBConnector*, std::vector>&, SwitchOrch*, NeighOrch*, IntfsOrch*, VRFOrch*, FgNhgOrch*, Srv6Orch*, swss::ZmqRouteServer*)' orchagent/p4orch/tests keeps a hand-maintained shadow copy of routeorch.h in mock_routeorch.h, reusing the same SWSS_ROUTEORCH_H include guard, so whichever of the two headers a TU sees first wins. fake_routeorch.cpp includes mock_routeorch.h directly and so compiled against the stale declaration, emitting the ctor mangled with swss::ZmqServer*, while test_main.cpp pulls in the real routeorch.h transitively via aclorch.h before its own mock_routeorch.h include and so referenced the ctor mangled with swss::ZmqRouteServer*. Update the shadow declaration and the fake definition to take swss::ZmqRouteServer*, matching routeorch.h. The mock class keeps its Orch base; it already diverges from the real ZmqOrch/ZmqRouteOrch base and only the ctor signature affects linkage. Signed-off-by: Venkit Kasiviswanathan --- orchagent/p4orch/tests/fake_routeorch.cpp | 2 +- orchagent/p4orch/tests/mock_routeorch.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/orchagent/p4orch/tests/fake_routeorch.cpp b/orchagent/p4orch/tests/fake_routeorch.cpp index eb0e8de8e81..e74528df97b 100644 --- a/orchagent/p4orch/tests/fake_routeorch.cpp +++ b/orchagent/p4orch/tests/fake_routeorch.cpp @@ -19,7 +19,7 @@ extern sai_mpls_api_t* sai_mpls_api; extern sai_switch_api_t* sai_switch_api; extern size_t gMaxBulkSize; -RouteOrch::RouteOrch(DBConnector *db, vector &tableNames, SwitchOrch *switchOrch, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch, FgNhgOrch *fgNhgOrch, Srv6Orch *srv6Orch, swss::ZmqServer *zmqServer) : +RouteOrch::RouteOrch(DBConnector *db, vector &tableNames, SwitchOrch *switchOrch, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch, FgNhgOrch *fgNhgOrch, Srv6Orch *srv6Orch, swss::ZmqRouteServer *zmqServer) : gRouteBulker(sai_route_api, gMaxBulkSize), gLabelRouteBulker(sai_mpls_api, gMaxBulkSize), gNextHopGroupMemberBulker(sai_next_hop_group_api, gSwitchId, gMaxBulkSize), diff --git a/orchagent/p4orch/tests/mock_routeorch.h b/orchagent/p4orch/tests/mock_routeorch.h index f3aed67663d..45b63c7a042 100644 --- a/orchagent/p4orch/tests/mock_routeorch.h +++ b/orchagent/p4orch/tests/mock_routeorch.h @@ -15,6 +15,7 @@ #include "bulker.h" #include "fgnhgorch.h" #include +#include "zmqrouteserver.h" /* Maximum next hop group number */ #define NHGRP_MAX_SIZE 128 @@ -182,7 +183,7 @@ struct LabelRouteBulkContext class RouteOrch : public Orch, public Subject { public: - RouteOrch(DBConnector *db, vector &tableNames, SwitchOrch *switchOrch, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch, FgNhgOrch *fgNhgOrch, Srv6Orch *srv6Orch, swss::ZmqServer *zmqServer = nullptr); + RouteOrch(DBConnector *db, vector &tableNames, SwitchOrch *switchOrch, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch, FgNhgOrch *fgNhgOrch, Srv6Orch *srv6Orch, swss::ZmqRouteServer *zmqServer = nullptr); bool hasNextHopGroup(const NextHopGroupKey&) const; sai_object_id_t getNextHopGroupId(const NextHopGroupKey&); From 068a580842fdeac72a8132d6ec84925def3fe37f Mon Sep 17 00:00:00 2001 From: Venkit Kasiviswanathan Date: Thu, 30 Jul 2026 03:32:16 +0000 Subject: [PATCH 4/7] [orchagent]: Update ZmqRouteConsumerExecuteEmpty for the route state table tests/mock_tests failed to compile with: zmq_orch_ut.cpp:76:43: error: invalid conversion from 'swss::ZmqConsumerStateTable*' to 'swss::ZmqRouteConsumerStateTable*' ZmqRouteConsumer now takes a ZmqRouteConsumerStateTable rather than a plain ZmqConsumerStateTable, so the pre-existing ZmqOrchTest. ZmqRouteConsumerExecuteEmpty no longer builds. Construct the route variants instead: create_zmq_route_server() for the server and ZmqRouteConsumerStateTable for the consumer table. The latter takes no popBatchSize argument, it forwards DEFAULT_POP_BATCH_SIZE to ZmqConsumerStateTable, so drop the explicit 128. Behaviour of the test is unchanged: create_zmq_route_server() lazy-binds exactly as create_zmq_server() does, so the server never binds the port, and dbPersistence=false still keeps Redis and AsyncDBUpdater out of the test. Signed-off-by: Venkit Kasiviswanathan --- tests/mock_tests/zmq_orch_ut.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/mock_tests/zmq_orch_ut.cpp b/tests/mock_tests/zmq_orch_ut.cpp index 9ad47e583e9..7ac2f55321a 100644 --- a/tests/mock_tests/zmq_orch_ut.cpp +++ b/tests/mock_tests/zmq_orch_ut.cpp @@ -58,7 +58,7 @@ TEST(ZmqOrchTest, CreateZmqRouteOrchWithTableNames) TEST(ZmqOrchTest, ZmqRouteConsumerExecuteEmpty) { string zmq_server_address = "tcp://127.0.0.1:18100"; - auto zmq_server = swss::create_zmq_server(zmq_server_address); + auto zmq_server = swss::create_zmq_route_server(zmq_server_address); auto app_db = make_shared("APPL_DB", 0); @@ -68,11 +68,11 @@ TEST(ZmqOrchTest, ZmqRouteConsumerExecuteEmpty) vector empty_tables; auto host_orch = make_shared(app_db.get(), empty_tables, nullptr); - // Construct ZmqConsumerStateTable with dbPersistence=false so no + // Construct ZmqRouteConsumerStateTable with dbPersistence=false so no // AsyncDBUpdater / Redis activity happens. - auto* cst = new swss::ZmqConsumerStateTable( + auto* cst = new swss::ZmqRouteConsumerStateTable( app_db.get(), "ROUTE_TABLE_E", *zmq_server, - /*popBatchSize=*/128, /*pri=*/1, /*dbPersistence=*/false); + /*pri=*/1, /*dbPersistence=*/false); auto* consumer = new ZmqRouteConsumer(cst, host_orch.get(), "ROUTE_TABLE_E"); // With no messages received, pops() returns empty, addToSync returns 0, From a4711803c8ca584d9c16731c5a22c2b9f264f266 Mon Sep 17 00:00:00 2001 From: Venkit Kasiviswanathan Date: Thu, 30 Jul 2026 20:13:31 +0000 Subject: [PATCH 5/7] [orchagent]: Narrow the ingress lock and tidy the route consumer Three review comments on the staging-buffer path. - ZmqRouteConsumer::execute() held m_ingressMutex across addToSync(), so mqPollThread could not stage a new burst while the deque was being merged into m_toSync -- a SyncMap merge that is O(n log n) in the batch size, which is exactly the burst this design is meant to keep flowing. The comment above it already claimed the lock was "held only while moving tuples out of m_ingress", which was not true of the code. Hoist entries out of the lock scope, close the scope after m_ingress.clear(), and call addToSync() with the lock released. This is safe rather than merely faster: m_ingress is the only state shared with mqPollThread, m_toSync is owned exclusively by the orch main thread, ZmqRouteConsumer overrides neither addToSync nor dumpPendingTasks, and entries is a local, so nothing is shared once the lock drops. - Take tableNames by const reference in the vector constructor's range-for instead of copying each std::string. addConsumer still takes its name by value, so this removes one copy of two; the sibling tableNames_with_pri constructor already spelled it this way. - Replace the manual save/restore of gMaxBulkSize in IngressCallbackFiresNotifyAtMaxBulkSize with a ScopedMaxBulkSize RAII guard, placed in the file's anonymous namespace next to waitFor(). A throw out of Select, ZmqClient or ZmqProducerStateTable between the set and the restore would otherwise leave the process-global clobbered at 1, silently changing the batching threshold for every later test in the binary. Signed-off-by: Venkit Kasiviswanathan --- orchagent/zmqrouteorch.cpp | 12 +++++++----- tests/mock_tests/zmq_route_orch_ut.cpp | 16 ++++++++++++---- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/orchagent/zmqrouteorch.cpp b/orchagent/zmqrouteorch.cpp index 803cbf2e007..a28e87cc7eb 100644 --- a/orchagent/zmqrouteorch.cpp +++ b/orchagent/zmqrouteorch.cpp @@ -35,19 +35,21 @@ void ZmqRouteConsumer::execute() { SWSS_LOG_ENTER(); + std::deque entries; { - // Drain the staged tuples into m_toSync under the lock, mirroring + // Move the staged tuples out of m_ingress under the lock, mirroring // ZmqConsumer::execute()'s pops() + addToSync(entries). The lock is - // held only while moving tuples out of m_ingress. + // dropped before addToSync() below: m_toSync is owned by this (main) + // thread alone, so the merge needs no lock, and releasing early keeps + // mqPollThread free to stage the next burst while the merge runs. std::lock_guard lk(m_ingressMutex); - std::deque entries; for (auto &kv : m_ingress) { entries.push_back(std::move(kv.second)); } m_ingress.clear(); - addToSync(entries); } + addToSync(entries); // m_toSync is mutated only by this (main) thread, so drain() — which reads // m_toSync and hands it to doTask — does not need to hold m_ingressMutex. @@ -72,7 +74,7 @@ void ZmqRouteConsumer::drain() ZmqRouteOrch::ZmqRouteOrch(DBConnector *db, const vector &tableNames, ZmqRouteServer *zmqServer) : Orch() { - for (auto it : tableNames) + for (const auto& it : tableNames) { addConsumer(db, it, default_orch_pri, zmqServer); } diff --git a/tests/mock_tests/zmq_route_orch_ut.cpp b/tests/mock_tests/zmq_route_orch_ut.cpp index 4df283fafb0..22751d527ef 100644 --- a/tests/mock_tests/zmq_route_orch_ut.cpp +++ b/tests/mock_tests/zmq_route_orch_ut.cpp @@ -30,6 +30,17 @@ extern size_t gMaxBulkSize; namespace { +// Restores gMaxBulkSize on scope exit. The tests below lower it to force the +// mid-burst notify branch; without this, an early return or a throw between +// the set and the restore would leave the global clobbered for every later +// test in the process. +struct ScopedMaxBulkSize +{ + explicit ScopedMaxBulkSize(size_t v) : saved(gMaxBulkSize) { gMaxBulkSize = v; } + ~ScopedMaxBulkSize() { gMaxBulkSize = saved; } + size_t saved; +}; + // Wait until pred() becomes true or deadlineMs elapses; returns the final value. template bool waitFor(int deadlineMs, Pred pred) @@ -320,8 +331,7 @@ TEST(ZmqRouteConsumerTest, IngressCallbackFiresNotifyAtMaxBulkSize) server.bind(); // Force the mid-burst notify branch to trip on the very first callback. - const size_t savedMaxBulk = gMaxBulkSize; - gMaxBulkSize = 1; + ScopedMaxBulkSize maxBulkGuard(1); Select sel; sel.addSelectable(zrc); @@ -336,8 +346,6 @@ TEST(ZmqRouteConsumerTest, IngressCallbackFiresNotifyAtMaxBulkSize) Selectable *out = nullptr; EXPECT_EQ(sel.select(&out, 2000), Select::OBJECT); EXPECT_EQ(out, zrc); - - gMaxBulkSize = savedMaxBulk; } // The producer (and hence the mqPollThread ingress callback) keeps staging into From a1e4a569bb549cb288edd5d4fbd0b1208a5bf7cd Mon Sep 17 00:00:00 2001 From: Venkit Kasiviswanathan Date: Wed, 5 Aug 2026 21:34:31 +0000 Subject: [PATCH 6/7] [orchagent]: Cover ingress staging semantics in the route consumer tests Close the three coverage gaps called out in review: - SameKeyBurstCoalescesInIngress: two updates to the same key inside one batched ZMQ message (ZmqProducerStateTable::set(vector) produces a single message, hence a single ingress-callback invocation), pinning that staging is last-writer-wins wholesale -- exactly one delivery carrying only the second update's fields, never a field union. totalEntries == 1 makes a transport split fail loudly instead of passing as two deliveries. - SelectWakesAfterBurstQuiesce: with gMaxBulkSize pinned high so the mid-burst threshold branch is unreachable, a single staged tuple must still wake the Select loop via mqPollThread's post-burst quiesce notify -- the common wake path at the default bulk size. - m_ingress drain completeness: the staging members move from private to protected so the tests (which compile with protected mapped to public, as they already do for m_toSync) can assert m_ingress.empty() after drains; asserts added to the ingress tests, plus a leak trap in IngressCallbackConcurrentWithDrain -- an extra execute() after convergence must not grow the delivery count. The recording orch now captures the delivered fields per key; the key set alone cannot distinguish last-writer-wins from a field union. Verified against sonic-swss-common master debs: full mock_tests suite 945/945, ZmqRoute suites 14/14. Signed-off-by: Venkit Kasiviswanathan --- orchagent/zmqrouteorch.h | 5 +- tests/mock_tests/zmq_route_orch_ut.cpp | 108 +++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 1 deletion(-) diff --git a/orchagent/zmqrouteorch.h b/orchagent/zmqrouteorch.h index 6fd1a7a4b5a..eabcb95138f 100644 --- a/orchagent/zmqrouteorch.h +++ b/orchagent/zmqrouteorch.h @@ -23,7 +23,10 @@ class ZmqRouteConsumer : public ConsumerBase { void execute() override; void drain() override; -private: +/* protected rather than private so the unit tests (which compile with + * protected mapped to public) can assert staging invariants such as + * m_ingress.empty() after a drain. */ +protected: // Staging buffer for tuples delivered by the ZmqRouteServer mqPollThread // ingress callback. The callback writes here (rather than merging into // m_toSync directly); execute() drains it into m_toSync. diff --git a/tests/mock_tests/zmq_route_orch_ut.cpp b/tests/mock_tests/zmq_route_orch_ut.cpp index 22751d527ef..5582ec20d30 100644 --- a/tests/mock_tests/zmq_route_orch_ut.cpp +++ b/tests/mock_tests/zmq_route_orch_ut.cpp @@ -76,6 +76,7 @@ class RecordingZmqRouteOrch : public ZmqRouteOrch for (const auto &kv : consumer.m_toSync) { seenKeys.insert(kv.first); + lastFields[kv.first] = kfvFieldsValues(kv.second); ++totalEntries; } // Drain the consumer's m_toSync so subsequent drain() calls observe it @@ -85,6 +86,9 @@ class RecordingZmqRouteOrch : public ZmqRouteOrch std::atomic doTaskCount{0}; std::set seenKeys; + // Fields of the most recent delivery per key, captured before the clear + // below. Main-thread only, like seenKeys. + std::map> lastFields; // Total entries handed to doTask across all invocations. Compared against // seenKeys.size() it distinguishes "delivered once" from "delivered twice". std::atomic totalEntries{0}; @@ -308,6 +312,9 @@ TEST(ZmqRouteConsumerTest, IngressCallbackDeliversToDoTask) return orch->doTaskCount.load() >= 1; })); EXPECT_EQ(orch->seenKeys.count("route_x"), 1u); + // The staging map must drain fully into m_toSync; a leak here would be + // invisible to the m_toSync-side asserts. + EXPECT_TRUE(zrc->m_ingress.empty()); } // When the ingress callback stages past gMaxBulkSize entries, it must fire @@ -402,4 +409,105 @@ TEST(ZmqRouteConsumerTest, IngressCallbackConcurrentWithDrain) EXPECT_EQ(orch->seenKeys.size(), static_cast(kRoutes)); EXPECT_EQ(orch->totalEntries.load(), kRoutes); EXPECT_TRUE(zrc->m_toSync.empty()); + EXPECT_TRUE(zrc->m_ingress.empty()); + + // Leak trap: if execute() ever left entries behind in m_ingress, this + // extra pass would resurface them and grow the delivery count. + zrc->execute(); + EXPECT_EQ(orch->totalEntries.load(), kRoutes); +} + +// Two updates to the SAME key inside one ZMQ message exercise the +// m_ingress[key] = *kco overwrite: staging is last-writer-wins, wholesale. +// That is deliberately different from ConsumerBase::addToSync()'s field-union +// merge -- the route producer sends full replaces, so the last update must +// win with exactly its own fields, never a union with the superseded one. +TEST(ZmqRouteConsumerTest, SameKeyBurstCoalescesInIngress) +{ + const string tableName = "ZMQ_ROUTE_UT_COALESCE"; + const string pushEndpoint = "tcp://localhost:1271"; + const string pullEndpoint = "tcp://*:1271"; + + vector tables = { { tableName, 1 } }; + auto app_db = make_shared("APPL_DB", 0); + ZmqRouteServer server(pullEndpoint, "", /*lazyBind=*/true); + auto orch = make_shared(app_db.get(), tables, &server); + auto *zrc = dynamic_cast( + orch->m_consumerMap.begin()->second.get()); + ASSERT_NE(zrc, nullptr); + + server.bind(); + + ZmqClient client(pushEndpoint, 0); + ZmqProducerStateTable p(app_db.get(), tableName, client, /*dbPersistence=*/false); + + // One batched set() -> one ZMQ message -> one handleReceivedData() -> one + // ingress-callback invocation, so both tuples deterministically meet in + // m_ingress with no execute() possible in between. + std::vector burst = { + {"route_c", SET_COMMAND, {{"nh", "1.1.1.1"}}}, + {"route_c", SET_COMMAND, {{"ifname", "Ethernet0"}}}, + }; + p.set(burst); + + ASSERT_TRUE(waitFor(2000, [&] { + zrc->execute(); + return orch->doTaskCount.load() >= 1; + })); + + // Exactly one delivery for the key -- the burst coalesced in staging. + EXPECT_EQ(orch->seenKeys.count("route_c"), 1u); + EXPECT_EQ(orch->totalEntries.load(), 1); + + // And it carries only the second update's fields: no {nh} survivor. + const auto &fields = orch->lastFields["route_c"]; + ASSERT_EQ(fields.size(), 1u); + EXPECT_EQ(fvField(fields[0]), "ifname"); + EXPECT_EQ(fvValue(fields[0]), "Ethernet0"); + + EXPECT_TRUE(zrc->m_ingress.empty()); +} + +// A single staged tuple below gMaxBulkSize must still wake the Select loop: +// mqPollThread fires notifyPending() once the burst quiesces (BURST_QUIESCE_MS +// in swss-common's ZmqRouteServer). At the default bulk size this quiesce +// notify is the common wake path; the mid-burst threshold branch is covered +// by IngressCallbackFiresNotifyAtMaxBulkSize above. +TEST(ZmqRouteConsumerTest, SelectWakesAfterBurstQuiesce) +{ + const string tableName = "ZMQ_ROUTE_UT_QUIESCE"; + const string pushEndpoint = "tcp://localhost:1272"; + const string pullEndpoint = "tcp://*:1272"; + + vector tables = { { tableName, 1 } }; + auto app_db = make_shared("APPL_DB", 0); + ZmqRouteServer server(pullEndpoint, "", /*lazyBind=*/true); + auto orch = make_shared(app_db.get(), tables, &server); + auto *zrc = dynamic_cast( + orch->m_consumerMap.begin()->second.get()); + ASSERT_NE(zrc, nullptr); + + server.bind(); + + // Make the mid-burst threshold branch unreachable: one staged entry is + // far below 1000, so any wake below must come from the quiesce notify. + ScopedMaxBulkSize maxBulkGuard(1000); + + Select sel; + sel.addSelectable(zrc); + + ZmqClient client(pushEndpoint, 0); + ZmqProducerStateTable p(app_db.get(), tableName, client, /*dbPersistence=*/false); + p.set("route_q", vector{{"nh", "3.3.3.3"}}); + + Selectable *out = nullptr; + EXPECT_EQ(sel.select(&out, 2000), Select::OBJECT); + EXPECT_EQ(out, zrc); + + // Drain and confirm the tuple made it through end to end. + ASSERT_TRUE(waitFor(2000, [&] { + zrc->execute(); + return orch->seenKeys.count("route_q") == 1; + })); + EXPECT_TRUE(zrc->m_ingress.empty()); } From 81d467c87c19f9924c4a7159141566fdf9a7121b Mon Sep 17 00:00:00 2001 From: Venkit Kasiviswanathan Date: Fri, 7 Aug 2026 03:55:01 +0000 Subject: [PATCH 7/7] [orchagent]: Qualify ZmqRoute* types with swss:: and log vrf in create_zmq_route_server Use swss:: explicitly for ZmqRouteServer and ZmqRouteConsumerStateTable instead of relying on a header-side using-directive, matching zmqorch.h. Also make the create_zmq_route_server log line distinguishable from create_zmq_server and include the vrf. Signed-off-by: Venkit Kasiviswanathan --- lib/orch_zmq_config.cpp | 2 +- orchagent/routeorch.cpp | 2 +- orchagent/routeorch.h | 2 +- orchagent/zmqrouteorch.cpp | 10 +++++----- orchagent/zmqrouteorch.h | 10 +++++----- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/orch_zmq_config.cpp b/lib/orch_zmq_config.cpp index 5783330c136..d007cc217a2 100644 --- a/lib/orch_zmq_config.cpp +++ b/lib/orch_zmq_config.cpp @@ -87,7 +87,7 @@ std::shared_ptr swss::create_zmq_route_server(std::string zmq_address = zmq_address + ":" + std::to_string(zmq_port); } - SWSS_LOG_NOTICE("Create ZMQ server with address: %s", zmq_address.c_str()); + SWSS_LOG_NOTICE("Create ZMQ route server with address: %s, vrf: %s", zmq_address.c_str(), vrf.c_str()); // To prevent message loss between ZmqServer's bind operation and the creation of ZmqProducerStateTable, // use lazy binding and call bind() only after the handler has been registered. diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index e9990f6f5c6..aa045eec08c 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -40,7 +40,7 @@ extern bool gEnableFibSuppress; #define DEFAULT_NUMBER_OF_ECMP_GROUPS 128 #define DEFAULT_MAX_ECMP_GROUP_SIZE 32 -RouteOrch::RouteOrch(DBConnector *db, vector &tableNames, SwitchOrch *switchOrch, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch, FgNhgOrch *fgNhgOrch, Srv6Orch *srv6Orch, ZmqRouteServer *zmqRouteServer) : +RouteOrch::RouteOrch(DBConnector *db, vector &tableNames, SwitchOrch *switchOrch, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch, FgNhgOrch *fgNhgOrch, Srv6Orch *srv6Orch, swss::ZmqRouteServer *zmqRouteServer) : gRouteBulker(sai_route_api, gMaxBulkSize), gLabelRouteBulker(sai_mpls_api, gMaxBulkSize), gNextHopGroupMemberBulker(sai_next_hop_group_api, gSwitchId, gMaxBulkSize), diff --git a/orchagent/routeorch.h b/orchagent/routeorch.h index 5ae1a8bde6a..b7ed629377f 100644 --- a/orchagent/routeorch.h +++ b/orchagent/routeorch.h @@ -224,7 +224,7 @@ struct LabelRouteBulkContext class RouteOrch : public ZmqRouteOrch, public Subject { public: - RouteOrch(DBConnector *db, vector &tableNames, SwitchOrch *switchOrch, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch, FgNhgOrch *fgNhgOrch, Srv6Orch *srv6Orch, ZmqRouteServer *zmqServer = nullptr); + RouteOrch(DBConnector *db, vector &tableNames, SwitchOrch *switchOrch, NeighOrch *neighOrch, IntfsOrch *intfsOrch, VRFOrch *vrfOrch, FgNhgOrch *fgNhgOrch, Srv6Orch *srv6Orch, swss::ZmqRouteServer *zmqServer = nullptr); bool hasNextHopGroup(const NextHopGroupKey&) const; sai_object_id_t getNextHopGroupId(const NextHopGroupKey&); diff --git a/orchagent/zmqrouteorch.cpp b/orchagent/zmqrouteorch.cpp index a28e87cc7eb..4518a8cb13b 100644 --- a/orchagent/zmqrouteorch.cpp +++ b/orchagent/zmqrouteorch.cpp @@ -6,7 +6,7 @@ using namespace std; extern int gBatchSize; extern size_t gMaxBulkSize; -ZmqRouteConsumer::ZmqRouteConsumer(ZmqRouteConsumerStateTable *select, Orch *orch, const std::string &name) +ZmqRouteConsumer::ZmqRouteConsumer(swss::ZmqRouteConsumerStateTable *select, Orch *orch, const std::string &name) : ConsumerBase(select, orch, name) { // mqPollThread delivers bursts of tuples through this callback. Stage them @@ -71,7 +71,7 @@ void ZmqRouteConsumer::drain() } -ZmqRouteOrch::ZmqRouteOrch(DBConnector *db, const vector &tableNames, ZmqRouteServer *zmqServer) +ZmqRouteOrch::ZmqRouteOrch(DBConnector *db, const vector &tableNames, swss::ZmqRouteServer *zmqServer) : Orch() { for (const auto& it : tableNames) @@ -81,7 +81,7 @@ ZmqRouteOrch::ZmqRouteOrch(DBConnector *db, const vector &tableNames, Zm } -ZmqRouteOrch::ZmqRouteOrch(DBConnector *db, const vector &tableNames_with_pri, ZmqRouteServer *zmqServer) +ZmqRouteOrch::ZmqRouteOrch(DBConnector *db, const vector &tableNames_with_pri, swss::ZmqRouteServer *zmqServer) { for (const auto& it : tableNames_with_pri) { @@ -89,7 +89,7 @@ ZmqRouteOrch::ZmqRouteOrch(DBConnector *db, const vector } } -void ZmqRouteOrch::addConsumer(DBConnector *db, string tableName, int pri, ZmqRouteServer *zmqServer) +void ZmqRouteOrch::addConsumer(DBConnector *db, string tableName, int pri, swss::ZmqRouteServer *zmqServer) { if (db->getDbId() == APPL_DB || db->getDbId() == DPU_APPL_DB) { @@ -98,7 +98,7 @@ void ZmqRouteOrch::addConsumer(DBConnector *db, string tableName, int pri, ZmqRo SWSS_LOG_DEBUG("ZmqRouteConsumer initialize for: %s", tableName.c_str()); addExecutor( new ZmqRouteConsumer( - new ZmqRouteConsumerStateTable( + new swss::ZmqRouteConsumerStateTable( db, tableName, *zmqServer, pri, /* dbPersistence= */false), this, tableName)); } diff --git a/orchagent/zmqrouteorch.h b/orchagent/zmqrouteorch.h index eabcb95138f..11856907eec 100644 --- a/orchagent/zmqrouteorch.h +++ b/orchagent/zmqrouteorch.h @@ -12,12 +12,12 @@ class ZmqRouteConsumer : public ConsumerBase { public: - ZmqRouteConsumer(ZmqRouteConsumerStateTable *select, Orch *orch, const std::string &name); + ZmqRouteConsumer(swss::ZmqRouteConsumerStateTable *select, Orch *orch, const std::string &name); swss::TableBase *getConsumerTable() const override { // ZmqRouteConsumerStateTable is a subclass of TableBase - return static_cast(getSelectable()); + return static_cast(getSelectable()); } void execute() override; @@ -44,12 +44,12 @@ class ZmqRouteConsumer : public ConsumerBase { class ZmqRouteOrch : public Orch { public: - ZmqRouteOrch(swss::DBConnector *db, const std::vector &tableNames, ZmqRouteServer *zmqServer); - ZmqRouteOrch(swss::DBConnector *db, const std::vector &tableNames_with_pri, ZmqRouteServer *zmqServer); + ZmqRouteOrch(swss::DBConnector *db, const std::vector &tableNames, swss::ZmqRouteServer *zmqServer); + ZmqRouteOrch(swss::DBConnector *db, const std::vector &tableNames_with_pri, swss::ZmqRouteServer *zmqServer); virtual void doTask(ConsumerBase &consumer) { }; void doTask(Consumer &consumer) override; private: - void addConsumer(swss::DBConnector *db, std::string tableName, int pri, ZmqRouteServer *zmqServer); + void addConsumer(swss::DBConnector *db, std::string tableName, int pri, swss::ZmqRouteServer *zmqServer); };