diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp index a9eaeb3664..344307a8da 100644 --- a/orchagent/routeorch.cpp +++ b/orchagent/routeorch.cpp @@ -48,8 +48,6 @@ RouteOrch::RouteOrch(DBConnector *db, vector &tableNames, { SWSS_LOG_ENTER(); - m_publisher.setBuffered(true); - sai_attribute_t attr; attr.id = SAI_SWITCH_ATTR_NUMBER_OF_ECMP_GROUPS; @@ -502,7 +500,7 @@ void RouteOrch::doTask(Consumer& consumer) auto rc = toBulk.emplace(std::piecewise_construct, std::forward_as_tuple(key, op), - std::forward_as_tuple(key, (op == SET_COMMAND))); + std::forward_as_tuple()); bool inserted = rc.second; auto& ctx = rc.first->second; @@ -633,11 +631,6 @@ void RouteOrch::doTask(Consumer& consumer) if (fvField(i) == "seg_src") srv6_source = fvValue(i); - - if (fvField(i) == "protocol") - { - ctx.protocol = fvValue(i); - } } /* @@ -870,10 +863,6 @@ void RouteOrch::doTask(Consumer& consumer) /* fullmask subnet route is same as ip2me route */ else if (ip_prefix.isFullMask() && m_intfsOrch->isPrefixSubnet(ip_prefix, alsv[0])) { - /* The prefix is full mask (/32 or /128) and it is an interface subnet route, so IntfOrch has already - * created an IP2ME route for it and we skip programming such route here as it already exists. - * However, to keep APPL_DB and APPL_STATE_DB consistent we have to publish it. */ - publishRouteState(ctx); it = consumer.m_toSync.erase(it); } /* subnet route, vrf leaked route, etc */ @@ -903,9 +892,7 @@ void RouteOrch::doTask(Consumer& consumer) } else { - /* Duplicate entry. Publish route state anyway since there could be multiple DEL, SET operations - * consolidated by ConsumerStateTable leading to orchagent receiving only the last SET update. */ - publishRouteState(ctx); + /* Duplicate entry */ it = consumer.m_toSync.erase(it); } @@ -2342,9 +2329,6 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey notifyNextHopChangeObservers(vrf_id, ipPrefix, nextHops, true); - /* Publish and update APPL STATE DB route entry programming status */ - publishRouteState(ctx); - /* * If the route uses a temporary synced NHG owned by NhgOrch, return false * in order to keep trying to update the route in case the NHG is updated, @@ -2560,9 +2544,6 @@ bool RouteOrch::removeRoutePost(const RouteBulkContext& ctx) SWSS_LOG_INFO("Remove route %s with next hop(s) %s", ipPrefix.to_string().c_str(), it_route->second.nhg_key.to_string().c_str()); - - /* Publish removal status, removes route entry from APPL STATE DB */ - publishRouteState(ctx); if (ipPrefix.isDefaultRoute() && vrf_id == gVirtualRouterId) { @@ -2719,22 +2700,3 @@ void RouteOrch::decNhgRefCount(const std::string &nhg_index) gCbfNhgOrch->decNhgRefCount(nhg_index); } } - -void RouteOrch::publishRouteState(const RouteBulkContext& ctx, const ReturnCode& status) -{ - SWSS_LOG_ENTER(); - - std::vector fvs; - - /* Leave the fvs empty if the operation type is "DEL". - * An empty fvs makes ResponsePublisher::publish() remove the state entry from APPL_STATE_DB - */ - if (ctx.is_set) - { - fvs.emplace_back("protocol", ctx.protocol); - } - - const bool replace = false; - - m_publisher.publish(APP_ROUTE_TABLE_NAME, ctx.key, fvs, status, replace); -} diff --git a/orchagent/routeorch.h b/orchagent/routeorch.h index b232137766..f87b56b4d3 100644 --- a/orchagent/routeorch.h +++ b/orchagent/routeorch.h @@ -122,12 +122,8 @@ struct RouteBulkContext // using_temp_nhg will track if the NhgOrch's owned NHG is temporary or not bool using_temp_nhg; - std::string key; // Key in database table - std::string protocol; // Protocol string - bool is_set; // True if set operation - - RouteBulkContext(const std::string& key, bool is_set) - : key(key), excp_intfs_flag(false), using_temp_nhg(false), is_set(is_set) + RouteBulkContext() + : excp_intfs_flag(false), using_temp_nhg(false) { } @@ -143,8 +139,6 @@ struct RouteBulkContext excp_intfs_flag = false; vrf_id = SAI_NULL_OBJECT_ID; using_temp_nhg = false; - key.clear(); - protocol.clear(); } }; @@ -276,8 +270,6 @@ class RouteOrch : public Orch, public Subject const NhgBase &getNhg(const std::string& nhg_index); void incNhgRefCount(const std::string& nhg_index); void decNhgRefCount(const std::string& nhg_index); - - void publishRouteState(const RouteBulkContext& ctx, const ReturnCode& status = ReturnCode(SAI_STATUS_SUCCESS)); }; #endif /* SWSS_ROUTEORCH_H */ diff --git a/tests/mock_tests/Makefile.am b/tests/mock_tests/Makefile.am index 24b0d513ec..7bafd29e3e 100644 --- a/tests/mock_tests/Makefile.am +++ b/tests/mock_tests/Makefile.am @@ -201,7 +201,7 @@ tests_intfmgrd_INCLUDES = $(tests_INCLUDES) -I$(top_srcdir)/cfgmgr -I$(top_srcdi tests_intfmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) tests_intfmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) $(tests_intfmgrd_INCLUDES) tests_intfmgrd_LDADD = $(LDADD_GTEST) $(LDADD_SAI) -lnl-genl-3 -lhiredis -lhiredis \ - -lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3 -lpthread -lgmock -lgmock_main + -lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3 -lpthread ## teammgrd unit tests diff --git a/tests/mock_tests/fake_response_publisher.cpp b/tests/mock_tests/fake_response_publisher.cpp index 5e6aa0f2a0..c2cbc9fcd6 100644 --- a/tests/mock_tests/fake_response_publisher.cpp +++ b/tests/mock_tests/fake_response_publisher.cpp @@ -2,11 +2,6 @@ #include #include "response_publisher.h" -#include "mock_response_publisher.h" - -/* This mock plugs into this fake response publisher implementation - * when needed to test code that uses response publisher. */ -std::unique_ptr gMockResponsePublisher; ResponsePublisher::ResponsePublisher(const std::string& dbName, bool buffered, bool db_write_thread) : m_db(std::make_unique(dbName, 0)), m_buffered(buffered) {} @@ -17,24 +12,12 @@ void ResponsePublisher::publish( const std::string& table, const std::string& key, const std::vector& intent_attrs, const ReturnCode& status, - const std::vector& state_attrs, bool replace) -{ - if (gMockResponsePublisher) - { - gMockResponsePublisher->publish(table, key, intent_attrs, status, state_attrs, replace); - } -} + const std::vector& state_attrs, bool replace) {} void ResponsePublisher::publish( const std::string& table, const std::string& key, const std::vector& intent_attrs, - const ReturnCode& status, bool replace) -{ - if (gMockResponsePublisher) - { - gMockResponsePublisher->publish(table, key, intent_attrs, status, replace); - } -} + const ReturnCode& status, bool replace) {} void ResponsePublisher::writeToDB( const std::string& table, const std::string& key, diff --git a/tests/mock_tests/routeorch_ut.cpp b/tests/mock_tests/routeorch_ut.cpp index aa749a72a2..f9f92ce397 100644 --- a/tests/mock_tests/routeorch_ut.cpp +++ b/tests/mock_tests/routeorch_ut.cpp @@ -7,14 +7,10 @@ #include "ut_helper.h" #include "mock_orchagent_main.h" #include "mock_table.h" -#include "mock_response_publisher.h" #include "bulker.h" extern string gMySwitchType; -extern std::unique_ptr gMockResponsePublisher; - -using ::testing::_; namespace routeorch_test { @@ -291,10 +287,6 @@ namespace routeorch_test {"mac_addr", "00:00:00:00:00:00" }}); intfTable.set("Ethernet0:10.0.0.1/24", { { "scope", "global" }, { "family", "IPv4" }}); - intfTable.set("Ethernet4", { {"NULL", "NULL" }, - {"mac_addr", "00:00:00:00:00:00" }}); - intfTable.set("Ethernet4:11.0.0.1/32", { { "scope", "global" }, - { "family", "IPv4" }}); gIntfsOrch->addExistingData(&intfTable); static_cast(gIntfsOrch)->doTask(); @@ -439,60 +431,6 @@ namespace routeorch_test ASSERT_EQ(sai_fail_count, 0); } - TEST_F(RouteOrchTest, RouteOrchTestSetDelResponse) - { - gMockResponsePublisher = std::make_unique(); - - std::deque entries; - std::string key = "2.2.2.0/24"; - std::vector fvs{{"ifname", "Ethernet0,Ethernet0"}, {"nexthop", "10.0.0.2,10.0.0.3"}, {"protocol", "bgp"}}; - entries.push_back({key, "SET", fvs}); - - auto consumer = dynamic_cast(gRouteOrch->getExecutor(APP_ROUTE_TABLE_NAME)); - consumer->addToSync(entries); - - EXPECT_CALL(*gMockResponsePublisher, publish(APP_ROUTE_TABLE_NAME, key, std::vector{{"protocol", "bgp"}}, ReturnCode(SAI_STATUS_SUCCESS), false)).Times(1); - static_cast(gRouteOrch)->doTask(); - - // add entries again to the consumer queue (in case of rapid DEL/SET operations from fpmsyncd, routeorch just gets the last SET update) - consumer->addToSync(entries); - - EXPECT_CALL(*gMockResponsePublisher, publish(APP_ROUTE_TABLE_NAME, key, std::vector{{"protocol", "bgp"}}, ReturnCode(SAI_STATUS_SUCCESS), false)).Times(1); - static_cast(gRouteOrch)->doTask(); - - entries.clear(); - - // Route deletion - - entries.clear(); - entries.push_back({key, "DEL", {}}); - - consumer->addToSync(entries); - - EXPECT_CALL(*gMockResponsePublisher, publish(APP_ROUTE_TABLE_NAME, key, std::vector{}, ReturnCode(SAI_STATUS_SUCCESS), false)).Times(1); - static_cast(gRouteOrch)->doTask(); - - gMockResponsePublisher.reset(); - } - - TEST_F(RouteOrchTest, RouteOrchSetFullMaskSubnetPrefix) - { - gMockResponsePublisher = std::make_unique(); - - std::deque entries; - std::string key = "11.0.0.1/32"; - std::vector fvs{{"ifname", "Ethernet4"}, {"nexthop", "0.0.0.0"}, {"protocol", "bgp"}}; - entries.push_back({key, "SET", fvs}); - - auto consumer = dynamic_cast(gRouteOrch->getExecutor(APP_ROUTE_TABLE_NAME)); - consumer->addToSync(entries); - - EXPECT_CALL(*gMockResponsePublisher, publish(APP_ROUTE_TABLE_NAME, key, std::vector{{"protocol", "bgp"}}, ReturnCode(SAI_STATUS_SUCCESS), false)).Times(1); - static_cast(gRouteOrch)->doTask(); - - gMockResponsePublisher.reset(); - } - TEST_F(RouteOrchTest, RouteOrchTestInvalidEvpnRoute) { std::deque entries;