Skip to content

Commit

Permalink
Unnecessary sessions getting logged with out SLO rate check
Browse files Browse the repository at this point in the history
Idle sessions for which stats are not changed can be logged with SLO rate match
is not a bug.

This fix has below change:
With this change, all tear down sessions will not be logged.
Tear down sessions which are logged earlier at least once will be logged immidiately
with out checking rate, otherwise it will be considered as normal session and will be
reported only after matching the SLO rate.

Change-Id: I0f1ae5e4239fec9eebe57c8b34b6f20c3431403a
Closes-Bug: 1744914
  • Loading branch information
bmadhu77 committed Apr 10, 2018
1 parent 3f5752d commit 884ab3a
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 24 deletions.
80 changes: 61 additions & 19 deletions src/vnsw/agent/vrouter/flow_stats/session_stats_collector.cc
Expand Up @@ -1041,12 +1041,17 @@ bool SessionStatsCollector::UpdateSloMatchRuleEntry(
bool SessionStatsCollector::CheckPolicyMatch(const SessionSloRuleMap &map,
const std::string &policy_uuid,
const bool &deleted_flag,
bool *match) {
bool *match,
const bool &exported_once) {
SessionSloRuleMap::const_iterator it;
if (!policy_uuid.empty()) {
it = map.find(policy_uuid);
if (it != map.end()) {
if (deleted_flag) {
/* Always logging tear down session, which is exported atleast once
* earlier will be logged other tear down sessions will be reported
* with SLO rate checking
*/
if (deleted_flag && exported_once) {
*match = true;
return true;
}
Expand All @@ -1062,14 +1067,18 @@ bool SessionStatsCollector::FindSloMatchRule(const SessionSloRuleMap &map,
const std::string &nw_policy_uuid,
const std::string &sg_policy_uuid,
const bool &deleted_flag,
bool *match) {
bool *match,
const bool &exported_once) {
SessionSloRuleMap::const_iterator it;
bool fw_logged = false, nw_logged = false, sg_logged = false;
bool fw_match = false, nw_match = false, sg_match = false;

fw_logged = CheckPolicyMatch(map, fw_policy_uuid, deleted_flag, &fw_match);
nw_logged = CheckPolicyMatch(map, nw_policy_uuid, deleted_flag, &nw_match);
sg_logged = CheckPolicyMatch(map, sg_policy_uuid, deleted_flag, &sg_match);
fw_logged = CheckPolicyMatch(map, fw_policy_uuid, deleted_flag,
&fw_match, exported_once);
nw_logged = CheckPolicyMatch(map, nw_policy_uuid, deleted_flag,
&nw_match, exported_once);
sg_logged = CheckPolicyMatch(map, sg_policy_uuid, deleted_flag,
&sg_match, exported_once);

if (fw_match || nw_match || sg_match) {
*match = true;
Expand All @@ -1090,7 +1099,8 @@ bool SessionStatsCollector::MatchSloForFlow(
const std::string &nw_policy_uuid,
const std::string &sg_policy_uuid,
const bool &deleted_flag,
bool *logged) {
bool *logged,
const bool &exported_once) {

bool is_vmi_slo_logged, is_vn_slo_logged, is_global_slo_logged;
bool vmi_slo_match, vn_slo_match, global_slo_match;
Expand All @@ -1114,21 +1124,24 @@ bool SessionStatsCollector::MatchSloForFlow(
nw_policy_uuid,
sg_policy_uuid,
deleted_flag,
&vmi_slo_match);
&vmi_slo_match,
exported_once);

is_vn_slo_logged = FindSloMatchRule(vn_session_slo_rule_map,
fw_policy_uuid,
nw_policy_uuid,
sg_policy_uuid,
deleted_flag,
&vn_slo_match);
&vn_slo_match,
exported_once);

is_global_slo_logged = FindSloMatchRule(global_session_slo_rule_map,
fw_policy_uuid,
nw_policy_uuid,
sg_policy_uuid,
deleted_flag,
&global_slo_match);
&global_slo_match,
exported_once);
if ((is_vmi_slo_logged) ||
(is_vn_slo_logged) ||
(is_global_slo_logged)) {
Expand Down Expand Up @@ -1166,7 +1179,8 @@ void SessionStatsCollector::GetPolicyIdFromFlow(
bool SessionStatsCollector::FlowLogging(
const SessionStatsInfo &stats_info,
const FlowEntry *fe,
bool *logged) {
bool *logged,
const bool &exported_once) {

bool matched = false, deleted_flag=false;
std::string fw_policy_uuid = "", nw_policy_uuid = "", sg_policy_uuid = "";
Expand All @@ -1182,15 +1196,17 @@ bool SessionStatsCollector::FlowLogging(
nw_policy_uuid,
sg_policy_uuid,
deleted_flag,
logged);
logged,
exported_once);

return matched;
}

bool SessionStatsCollector::DeletedFlowLogging(
const SessionStatsInfo &stats_info,
const SessionFlowExportInfo &flow_info,
bool *logged) {
bool *logged,
const bool &exported_once) {

bool matched = false, deleted_flag = true;
std::string fw_policy_uuid = "", nw_policy_uuid = "", sg_policy_uuid = "";
Expand All @@ -1206,7 +1222,8 @@ bool SessionStatsCollector::DeletedFlowLogging(
nw_policy_uuid,
sg_policy_uuid,
deleted_flag,
logged);
logged,
exported_once);

return matched;
}
Expand All @@ -1217,13 +1234,19 @@ bool SessionStatsCollector::HandleDeletedFlowLogging(
bool logged = false;
const SessionExportInfo &info = stats_info.export_info;

/*
* Deleted flow need to to be just checked whether SLO rules matched
* If SLO is macthed, it should be logged irrespective of the rate
*/
if (DeletedFlowLogging(stats_info,
info.fwd_flow,
&logged)) {
&logged,
stats_info.exported_atleast_once)) {
CheckFlowLogging(logged);
} else if (DeletedFlowLogging(stats_info,
info.rev_flow,
&logged)) {
&logged,
stats_info.exported_atleast_once)) {
CheckFlowLogging(logged);
}
return false;
Expand All @@ -1233,13 +1256,23 @@ bool SessionStatsCollector::HandleFlowLogging(
const SessionStatsInfo &stats_info) {
bool logged = false;

/*
* FWD and REV flow of the Session need to be checked for SLO
* separately. If FWD flow matches or logged then rev flow
* is not required to check for SLO match.
* REV flow will be checked for SLO only when FWD flow
* is not matched for the SLO, since SLO is per session
*/

if (FlowLogging(stats_info,
stats_info.fwd_flow.flow.get(),
&logged)) {
&logged,
stats_info.exported_atleast_once)) {
CheckFlowLogging(logged);
} else if (FlowLogging(stats_info,
stats_info.rev_flow.flow.get(),
&logged)) {
&logged,
stats_info.exported_atleast_once)) {
CheckFlowLogging(logged);
}
return false;
Expand All @@ -1254,6 +1287,15 @@ bool SessionStatsCollector::CheckSessionLogging(
return false;
}

/*
* Deleted flow will be logged if SLO is configured.
* Normal case will be logged only when there is a change in the
* stats. If there is no change in the session stats, it will
* not be considered to SLO match and rate. This will avoid logging
* of each session at least once. Also, idle session will not be
* considered for the rate count
*/

if (stats_info.deleted) {
if(HandleDeletedFlowLogging(stats_info)) {
return true;
Expand Down Expand Up @@ -1737,7 +1779,7 @@ bool SessionStatsCollector::ProcessSessionEndpoint
if (!session_map_iter->second.deleted &&
!session_map_iter->second.evicted) {
bool changed = SessionStatsChangedLocked(session_map_iter,
&params);
&params);
if (!changed && session_map_iter->second.exported_atleast_once) {
++session_map_iter;
continue;
Expand Down
15 changes: 10 additions & 5 deletions src/vnsw/agent/vrouter/flow_stats/session_stats_collector.h
Expand Up @@ -305,13 +305,15 @@ class SessionStatsCollector : public StatsCollector {
bool CheckPolicyMatch(const SessionSloRuleMap &map,
const std::string &policy_uuid,
const bool &deleted_flag,
bool *match);
bool *match,
const bool &exported_once);
bool FindSloMatchRule(const SessionSloRuleMap &map,
const std::string &fw_policy_uuid,
const std::string &nw_policy_uuid,
const std::string &sg_policy_uuid,
const bool &deleted_flag,
bool *match);
bool *match,
const bool &exported_once);

void GetPolicyIdFromFlow(const FlowEntry *fe,
std::string &fw_policy_uuid,
Expand All @@ -329,7 +331,8 @@ class SessionStatsCollector : public StatsCollector {
const std::string& nw_policy_uuid,
const std::string& sg_policy_uuid,
const bool &deleted_flag,
bool *logged);
bool *logged,
const bool &exported_once);

void BuildSloList(const SessionStatsInfo &stats_info,
const FlowEntry *fe,
Expand All @@ -342,11 +345,13 @@ class SessionStatsCollector : public StatsCollector {

bool FlowLogging(const SessionStatsInfo &stats_info,
const FlowEntry *fe,
bool *logged);
bool *logged,
const bool &exported_once);

bool DeletedFlowLogging(const SessionStatsInfo &stats_info,
const SessionFlowExportInfo &flow_info,
bool *logged);
bool *logged,
const bool &exported_once);

bool HandleDeletedFlowLogging(const SessionStatsInfo &stats_info);
bool HandleFlowLogging(const SessionStatsInfo &stats_info);
Expand Down

0 comments on commit 884ab3a

Please sign in to comment.