From b9ade5d248a0ae9f0d975ace35eeb18f81a74599 Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox <57339448+Junchao-Mellanox@users.noreply.github.com> Date: Sat, 24 Sep 2022 05:13:09 +0800 Subject: [PATCH] [orchagent] Fix issue: ip prefix shall be inited even if VRF/VNET is not ready (#2461) *Flexcounter - Fix issue: ip prefix of a route flow pattern shall be initialized even if VRF/VNET is not ready --- .../flex_counter/flowcounterrouteorch.cpp | 7 +-- tests/mock_tests/flowcounterrouteorch_ut.cpp | 63 +++++++++++++++---- 2 files changed, 54 insertions(+), 16 deletions(-) diff --git a/orchagent/flex_counter/flowcounterrouteorch.cpp b/orchagent/flex_counter/flowcounterrouteorch.cpp index b82d66e27aa7..49107cfce6ad 100644 --- a/orchagent/flex_counter/flowcounterrouteorch.cpp +++ b/orchagent/flex_counter/flowcounterrouteorch.cpp @@ -636,7 +636,7 @@ void FlowCounterRouteOrch::createRouteFlowCounterByPattern(const RoutePattern &r { return; } - + if (route_pattern.is_match(route_pattern.vrf_id, entry.first)) { if (isRouteAlreadyBound(route_pattern, entry.first)) @@ -885,7 +885,7 @@ void FlowCounterRouteOrch::handleRouteRemove(sai_object_id_t vrf_id, const IpPre { return; } - + for (const auto &route_pattern : mRoutePatternSet) { if (route_pattern.is_match(vrf_id, ip_prefix)) @@ -953,6 +953,7 @@ bool FlowCounterRouteOrch::parseRouteKeyForRoutePattern(const std::string &key, else { vrf_name = key.substr(0, found); + ip_prefix = IpPrefix(key.substr(found+1)); auto *vrf_orch = gDirectory.get(); if (!key.compare(0, strlen(VRF_PREFIX), VRF_PREFIX) && vrf_orch->isVRFexists(vrf_name)) { @@ -966,8 +967,6 @@ bool FlowCounterRouteOrch::parseRouteKeyForRoutePattern(const std::string &key, return false; } } - - ip_prefix = IpPrefix(key.substr(found+1)); } return true; diff --git a/tests/mock_tests/flowcounterrouteorch_ut.cpp b/tests/mock_tests/flowcounterrouteorch_ut.cpp index 25ed95cb1e05..10a52c6b058d 100644 --- a/tests/mock_tests/flowcounterrouteorch_ut.cpp +++ b/tests/mock_tests/flowcounterrouteorch_ut.cpp @@ -25,9 +25,9 @@ namespace flowcounterrouteorch_test sai_remove_counter_fn old_remove_counter; sai_status_t _ut_stub_create_counter( - _Out_ sai_object_id_t *counter_id, - _In_ sai_object_id_t switch_id, - _In_ uint32_t attr_count, + _Out_ sai_object_id_t *counter_id, + _In_ sai_object_id_t switch_id, + _In_ uint32_t attr_count, _In_ const sai_attribute_t *attr_list) { num_created_counter ++; @@ -98,7 +98,7 @@ namespace flowcounterrouteorch_test gVirtualRouterId = attr.value.oid; - + ASSERT_EQ(gCrmOrch, nullptr); gCrmOrch = new CrmOrch(m_config_db.get(), CFG_CRM_TABLE_NAME); @@ -200,6 +200,12 @@ namespace flowcounterrouteorch_test }; gSrv6Orch = new Srv6Orch(m_app_db.get(), srv6_tables, gSwitchOrch, gVrfOrch, gNeighOrch); + // Start FlowCounterRouteOrch + static const vector route_pattern_tables = { + CFG_FLOW_COUNTER_ROUTE_PATTERN_TABLE_NAME, + }; + gFlowCounterRouteOrch = new FlowCounterRouteOrch(m_config_db.get(), route_pattern_tables); + ASSERT_EQ(gRouteOrch, nullptr); const int routeorch_pri = 5; vector route_tables = { @@ -276,14 +282,7 @@ namespace flowcounterrouteorch_test consumer->addToSync(entries); static_cast(flexCounterOrch)->doTask(); - // Start FlowCounterRouteOrch - static const vector route_pattern_tables = { - CFG_FLOW_COUNTER_ROUTE_PATTERN_TABLE_NAME, - }; - gFlowCounterRouteOrch = new FlowCounterRouteOrch(m_config_db.get(), route_pattern_tables); - static_cast(gFlowCounterRouteOrch)->doTask(); - return; } @@ -311,7 +310,7 @@ namespace flowcounterrouteorch_test delete gIntfsOrch; gIntfsOrch = nullptr; - + delete gFgNhgOrch; gFgNhgOrch = nullptr; @@ -358,4 +357,44 @@ namespace flowcounterrouteorch_test ASSERT_TRUE(current_counter_num - num_created_counter == 1); } + + TEST_F(FlowcounterRouteOrchTest, DelayAddVRF) + { + std::deque entries; + // Setting route pattern with VRF does not exist + auto current_counter_num = num_created_counter; + entries.push_back({"Vrf1|1.1.1.0/24", "SET", { {"max_match_count", "10"}}}); + auto consumer = dynamic_cast(gFlowCounterRouteOrch->getExecutor(CFG_FLOW_COUNTER_ROUTE_PATTERN_TABLE_NAME)); + consumer->addToSync(entries); + static_cast(gFlowCounterRouteOrch)->doTask(); + ASSERT_TRUE(num_created_counter - current_counter_num == 0); + + // Create VRF + entries.push_back({"Vrf1", "SET", { {"v4", "true"} }}); + auto vrf_consumer = dynamic_cast(gVrfOrch->getExecutor(APP_VRF_TABLE_NAME)); + vrf_consumer->addToSync(entries); + static_cast(gVrfOrch)->doTask(); + ASSERT_TRUE(num_created_counter - current_counter_num == 0); + + // Add route to VRF + Table routeTable = Table(m_app_db.get(), APP_ROUTE_TABLE_NAME); + routeTable.set("Vrf1:1.1.1.1/32", { {"ifname", "Ethernet0" }, + {"nexthop", "10.0.0.2" }}); + gRouteOrch->addExistingData(&routeTable); + static_cast(gRouteOrch)->doTask(); + ASSERT_TRUE(num_created_counter - current_counter_num == 1); + + // Deleting route pattern + current_counter_num = num_created_counter; + entries.clear(); + entries.push_back({"Vrf1|1.1.1.0/24", "DEL", { {"max_match_count", "10"}}}); + consumer->addToSync(entries); + static_cast(gFlowCounterRouteOrch)->doTask(); + ASSERT_TRUE(current_counter_num - num_created_counter == 1); + + // Deleting VRF + entries.push_back({"Vrf1", "DEL", { {"v4", "true"} }}); + vrf_consumer->addToSync(entries); + static_cast(gVrfOrch)->doTask(); + } } \ No newline at end of file