Skip to content

Commit b9ade5d

Browse files
[orchagent] Fix issue: ip prefix shall be inited even if VRF/VNET is not ready (sonic-net#2461)
*Flexcounter - Fix issue: ip prefix of a route flow pattern shall be initialized even if VRF/VNET is not ready
1 parent f0f1eb4 commit b9ade5d

File tree

2 files changed

+54
-16
lines changed

2 files changed

+54
-16
lines changed

orchagent/flex_counter/flowcounterrouteorch.cpp

+3-4
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,7 @@ void FlowCounterRouteOrch::createRouteFlowCounterByPattern(const RoutePattern &r
636636
{
637637
return;
638638
}
639-
639+
640640
if (route_pattern.is_match(route_pattern.vrf_id, entry.first))
641641
{
642642
if (isRouteAlreadyBound(route_pattern, entry.first))
@@ -885,7 +885,7 @@ void FlowCounterRouteOrch::handleRouteRemove(sai_object_id_t vrf_id, const IpPre
885885
{
886886
return;
887887
}
888-
888+
889889
for (const auto &route_pattern : mRoutePatternSet)
890890
{
891891
if (route_pattern.is_match(vrf_id, ip_prefix))
@@ -953,6 +953,7 @@ bool FlowCounterRouteOrch::parseRouteKeyForRoutePattern(const std::string &key,
953953
else
954954
{
955955
vrf_name = key.substr(0, found);
956+
ip_prefix = IpPrefix(key.substr(found+1));
956957
auto *vrf_orch = gDirectory.get<VRFOrch*>();
957958
if (!key.compare(0, strlen(VRF_PREFIX), VRF_PREFIX) && vrf_orch->isVRFexists(vrf_name))
958959
{
@@ -966,8 +967,6 @@ bool FlowCounterRouteOrch::parseRouteKeyForRoutePattern(const std::string &key,
966967
return false;
967968
}
968969
}
969-
970-
ip_prefix = IpPrefix(key.substr(found+1));
971970
}
972971

973972
return true;

tests/mock_tests/flowcounterrouteorch_ut.cpp

+51-12
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ namespace flowcounterrouteorch_test
2525
sai_remove_counter_fn old_remove_counter;
2626

2727
sai_status_t _ut_stub_create_counter(
28-
_Out_ sai_object_id_t *counter_id,
29-
_In_ sai_object_id_t switch_id,
30-
_In_ uint32_t attr_count,
28+
_Out_ sai_object_id_t *counter_id,
29+
_In_ sai_object_id_t switch_id,
30+
_In_ uint32_t attr_count,
3131
_In_ const sai_attribute_t *attr_list)
3232
{
3333
num_created_counter ++;
@@ -98,7 +98,7 @@ namespace flowcounterrouteorch_test
9898

9999
gVirtualRouterId = attr.value.oid;
100100

101-
101+
102102
ASSERT_EQ(gCrmOrch, nullptr);
103103
gCrmOrch = new CrmOrch(m_config_db.get(), CFG_CRM_TABLE_NAME);
104104

@@ -200,6 +200,12 @@ namespace flowcounterrouteorch_test
200200
};
201201
gSrv6Orch = new Srv6Orch(m_app_db.get(), srv6_tables, gSwitchOrch, gVrfOrch, gNeighOrch);
202202

203+
// Start FlowCounterRouteOrch
204+
static const vector<string> route_pattern_tables = {
205+
CFG_FLOW_COUNTER_ROUTE_PATTERN_TABLE_NAME,
206+
};
207+
gFlowCounterRouteOrch = new FlowCounterRouteOrch(m_config_db.get(), route_pattern_tables);
208+
203209
ASSERT_EQ(gRouteOrch, nullptr);
204210
const int routeorch_pri = 5;
205211
vector<table_name_with_pri_t> route_tables = {
@@ -276,14 +282,7 @@ namespace flowcounterrouteorch_test
276282
consumer->addToSync(entries);
277283
static_cast<Orch *>(flexCounterOrch)->doTask();
278284

279-
// Start FlowCounterRouteOrch
280-
static const vector<string> route_pattern_tables = {
281-
CFG_FLOW_COUNTER_ROUTE_PATTERN_TABLE_NAME,
282-
};
283-
gFlowCounterRouteOrch = new FlowCounterRouteOrch(m_config_db.get(), route_pattern_tables);
284-
285285
static_cast<Orch *>(gFlowCounterRouteOrch)->doTask();
286-
287286
return;
288287
}
289288

@@ -311,7 +310,7 @@ namespace flowcounterrouteorch_test
311310

312311
delete gIntfsOrch;
313312
gIntfsOrch = nullptr;
314-
313+
315314
delete gFgNhgOrch;
316315
gFgNhgOrch = nullptr;
317316

@@ -358,4 +357,44 @@ namespace flowcounterrouteorch_test
358357
ASSERT_TRUE(current_counter_num - num_created_counter == 1);
359358

360359
}
360+
361+
TEST_F(FlowcounterRouteOrchTest, DelayAddVRF)
362+
{
363+
std::deque<KeyOpFieldsValuesTuple> entries;
364+
// Setting route pattern with VRF does not exist
365+
auto current_counter_num = num_created_counter;
366+
entries.push_back({"Vrf1|1.1.1.0/24", "SET", { {"max_match_count", "10"}}});
367+
auto consumer = dynamic_cast<Consumer *>(gFlowCounterRouteOrch->getExecutor(CFG_FLOW_COUNTER_ROUTE_PATTERN_TABLE_NAME));
368+
consumer->addToSync(entries);
369+
static_cast<Orch *>(gFlowCounterRouteOrch)->doTask();
370+
ASSERT_TRUE(num_created_counter - current_counter_num == 0);
371+
372+
// Create VRF
373+
entries.push_back({"Vrf1", "SET", { {"v4", "true"} }});
374+
auto vrf_consumer = dynamic_cast<Consumer *>(gVrfOrch->getExecutor(APP_VRF_TABLE_NAME));
375+
vrf_consumer->addToSync(entries);
376+
static_cast<Orch *>(gVrfOrch)->doTask();
377+
ASSERT_TRUE(num_created_counter - current_counter_num == 0);
378+
379+
// Add route to VRF
380+
Table routeTable = Table(m_app_db.get(), APP_ROUTE_TABLE_NAME);
381+
routeTable.set("Vrf1:1.1.1.1/32", { {"ifname", "Ethernet0" },
382+
{"nexthop", "10.0.0.2" }});
383+
gRouteOrch->addExistingData(&routeTable);
384+
static_cast<Orch *>(gRouteOrch)->doTask();
385+
ASSERT_TRUE(num_created_counter - current_counter_num == 1);
386+
387+
// Deleting route pattern
388+
current_counter_num = num_created_counter;
389+
entries.clear();
390+
entries.push_back({"Vrf1|1.1.1.0/24", "DEL", { {"max_match_count", "10"}}});
391+
consumer->addToSync(entries);
392+
static_cast<Orch *>(gFlowCounterRouteOrch)->doTask();
393+
ASSERT_TRUE(current_counter_num - num_created_counter == 1);
394+
395+
// Deleting VRF
396+
entries.push_back({"Vrf1", "DEL", { {"v4", "true"} }});
397+
vrf_consumer->addToSync(entries);
398+
static_cast<Orch *>(gVrfOrch)->doTask();
399+
}
361400
}

0 commit comments

Comments
 (0)