Skip to content

Commit

Permalink
[orchagent] Fix issue: ip prefix shall be inited even if VRF/VNET is …
Browse files Browse the repository at this point in the history 
…not ready (#2461)

*Flexcounter - Fix issue: ip prefix of a route flow pattern shall be initialized even if VRF/VNET is not ready
  • Loading branch information
@Junchao-Mellanox @yxieca
Junchao-Mellanox authored and yxieca committed Sep 26, 2022
1 parent 3dcd6ff commit d9cbf44
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 16 deletions.
7 changes: 3 additions & 4 deletions orchagent/flex_counter/flowcounterrouteorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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<VRFOrch*>();
if (!key.compare(0, strlen(VRF_PREFIX), VRF_PREFIX) && vrf_orch->isVRFexists(vrf_name))
{
Expand All @@ -966,8 +967,6 @@ bool FlowCounterRouteOrch::parseRouteKeyForRoutePattern(const std::string &key,
return false;
}
}

ip_prefix = IpPrefix(key.substr(found+1));
}

return true;
Expand Down
63 changes: 51 additions & 12 deletions tests/mock_tests/flowcounterrouteorch_ut.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 ++;
Expand Down Expand Up @@ -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);

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

// Start FlowCounterRouteOrch
static const vector<string> 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<table_name_with_pri_t> route_tables = {
Expand Down Expand Up @@ -276,14 +282,7 @@ namespace flowcounterrouteorch_test
consumer->addToSync(entries);
static_cast<Orch *>(flexCounterOrch)->doTask();

// Start FlowCounterRouteOrch
static const vector<string> route_pattern_tables = {
CFG_FLOW_COUNTER_ROUTE_PATTERN_TABLE_NAME,
};
gFlowCounterRouteOrch = new FlowCounterRouteOrch(m_config_db.get(), route_pattern_tables);

static_cast<Orch *>(gFlowCounterRouteOrch)->doTask();

return;
}

Expand Down Expand Up @@ -311,7 +310,7 @@ namespace flowcounterrouteorch_test

delete gIntfsOrch;
gIntfsOrch = nullptr;

delete gFgNhgOrch;
gFgNhgOrch = nullptr;

Expand Down Expand Up @@ -358,4 +357,44 @@ namespace flowcounterrouteorch_test
ASSERT_TRUE(current_counter_num - num_created_counter == 1);

}

TEST_F(FlowcounterRouteOrchTest, DelayAddVRF)
{
std::deque<KeyOpFieldsValuesTuple> 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<Consumer *>(gFlowCounterRouteOrch->getExecutor(CFG_FLOW_COUNTER_ROUTE_PATTERN_TABLE_NAME));
consumer->addToSync(entries);
static_cast<Orch *>(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<Consumer *>(gVrfOrch->getExecutor(APP_VRF_TABLE_NAME));
vrf_consumer->addToSync(entries);
static_cast<Orch *>(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<Orch *>(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<Orch *>(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<Orch *>(gVrfOrch)->doTask();
}
}

0 comments on commit d9cbf44

Please sign in to comment.