Skip to content

Commit

Permalink
Merge "Fix flow sampling threshold value from overflowing." into R3.0
Browse files Browse the repository at this point in the history
  • Loading branch information
Zuul authored and opencontrail-ci-admin committed Jan 17, 2017
2 parents 33a418f + 76e4ef1 commit 43d1188
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 42 deletions.
9 changes: 3 additions & 6 deletions src/vnsw/agent/vrouter/flow_stats/flow_export_info.cc
Expand Up @@ -3,25 +3,22 @@

FlowExportInfo::FlowExportInfo() :
flow_(), setup_time_(0), teardown_time_(0), last_modified_time_(0),
bytes_(0), packets_(0), prev_diff_bytes_(0), prev_diff_packets_(0),
underlay_source_port_(0), changed_(false),
bytes_(0), packets_(0), underlay_source_port_(0), changed_(false),
tcp_flags_(0), delete_enqueue_time_(0), evict_enqueue_time_(0),
exported_atleast_once_(false) {
}

FlowExportInfo::FlowExportInfo(const FlowEntryPtr &fe) :
flow_(fe), setup_time_(0), teardown_time_(0), last_modified_time_(0),
bytes_(0), packets_(0), prev_diff_bytes_(0), prev_diff_packets_(0),
underlay_source_port_(0), changed_(true),
bytes_(0), packets_(0), underlay_source_port_(0), changed_(true),
tcp_flags_(0), delete_enqueue_time_(0), evict_enqueue_time_(0),
exported_atleast_once_(false) {
}

FlowExportInfo::FlowExportInfo(const FlowEntryPtr &fe, uint64_t setup_time) :
flow_(fe), setup_time_(setup_time),
teardown_time_(0), last_modified_time_(setup_time),
bytes_(0), packets_(0), prev_diff_bytes_(0), prev_diff_packets_(0),
underlay_source_port_(0), changed_(true),
bytes_(0), packets_(0), underlay_source_port_(0), changed_(true),
tcp_flags_(0), delete_enqueue_time_(0), evict_enqueue_time_(0),
exported_atleast_once_(false) {
}
Expand Down
10 changes: 0 additions & 10 deletions src/vnsw/agent/vrouter/flow_stats/flow_export_info.h
Expand Up @@ -27,10 +27,6 @@ class FlowExportInfo {
void set_bytes(uint64_t value) { bytes_ = value; }
uint64_t packets() const { return packets_; }
void set_packets(uint64_t value) { packets_ = value; }
uint64_t prev_diff_bytes() const { return prev_diff_bytes_; }
void set_prev_diff_bytes(uint64_t value) { prev_diff_bytes_ = value; }
uint64_t prev_diff_packets() const { return prev_diff_packets_; }
void set_prev_diff_packets(uint64_t value) { prev_diff_packets_ = value; }
uint16_t underlay_source_port() const { return underlay_source_port_; }
void set_underlay_source_port(uint16_t port) {
underlay_source_port_ = port;
Expand Down Expand Up @@ -61,12 +57,6 @@ class FlowExportInfo {
uint64_t last_modified_time_; //used for aging
uint64_t bytes_;
uint64_t packets_;
/* When flow samples are dropped the diff stats for that sample is
* accumulated in the following fields. This used to compute aggregate diff
* when the flow is being sent again. On successful send the following
* fields are reset */
uint64_t prev_diff_bytes_;
uint64_t prev_diff_packets_;
//IP address of the src vrouter for egress flows and dst vrouter for
//ingress flows. Used only during flow-export
//Underlay IP protocol type. Used only during flow-export
Expand Down
4 changes: 2 additions & 2 deletions src/vnsw/agent/vrouter/flow_stats/flow_stats.sandesh
Expand Up @@ -177,6 +177,6 @@ request sandesh DeleteAgingConfig {
traceobject sandesh FlowExportStatsTrace {
1: u32 export_rate;
2: u32 export_rate_without_sampling;
3: u32 old_threshold;
4: u32 new_threshold;
3: u64 old_threshold;
4: u64 new_threshold;
}
27 changes: 8 additions & 19 deletions src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc
Expand Up @@ -769,11 +769,6 @@ void FlowStatsCollector::ExportFlow(FlowExportInfo *info,
return;
}

/* Compute diff stats by adding the previous diff stats of sample that
* was dropped */
diff_bytes += info->prev_diff_bytes();
diff_pkts += info->prev_diff_packets();

/* Subject a flow to sampling algorithm only when all of below is met:-
* a. if Log is not configured as action for flow
* b. actual flow-export-rate is >= 80% of configured flow-export-rate. This
Expand Down Expand Up @@ -801,8 +796,6 @@ void FlowStatsCollector::ExportFlow(FlowExportInfo *info,
/* Do not export the flow, if the random number generated is more
* than the diff_bytes */
flow_stats_manager_->flow_export_sampling_drops_++;
info->set_prev_diff_bytes(diff_bytes);
info->set_prev_diff_packets(diff_pkts);
/* The second part of the if condition below is not required but
* added for better readability. It is not required because
* exported_atleast_once() will always be false if teardown time is
Expand All @@ -822,10 +815,6 @@ void FlowStatsCollector::ExportFlow(FlowExportInfo *info,
diff_pkts = diff_pkts/probability;
}
}
/* Reset diff stats since flow will be exported now */
info->set_prev_diff_bytes(0);
info->set_prev_diff_packets(0);

/* Mark the flow as exported */
info->set_exported_atleast_once(true);

Expand Down Expand Up @@ -964,7 +953,7 @@ bool FlowStatsManager::UpdateFlowThreshold() {
(cfg_rate == prev_cfg_flow_export_rate_)) {
return true;
}
uint32_t cur_t = threshold(), new_t = 0;
uint64_t cur_t = threshold(), new_t = 0;
// Update sampling threshold based on flow_export_rate_
if (flow_export_rate_ < ((double)cfg_rate) * 0.8) {
/* There are two reasons why we can be here.
Expand All @@ -975,20 +964,20 @@ bool FlowStatsManager::UpdateFlowThreshold() {
* Threshold should be updated here depending on which of the above two
* situations we are in. */
if (!flows_sampled_atleast_once_) {
UpdateThreshold(kDefaultFlowSamplingThreshold);
UpdateThreshold(kDefaultFlowSamplingThreshold, false);
} else {
if (flow_export_rate_ < ((double)cfg_rate) * 0.5) {
UpdateThreshold((threshold_ / 4));
UpdateThreshold((threshold_ / 4), false);
} else {
UpdateThreshold((threshold_ / 2));
UpdateThreshold((threshold_ / 2), false);
}
}
} else if (flow_export_rate_ > (cfg_rate * 3)) {
UpdateThreshold((threshold_ * 4));
UpdateThreshold((threshold_ * 4), true);
} else if (flow_export_rate_ > (cfg_rate * 2)) {
UpdateThreshold((threshold_ * 3));
UpdateThreshold((threshold_ * 3), true);
} else if (flow_export_rate_ > ((double)cfg_rate) * 1.25) {
UpdateThreshold((threshold_ * 2));
UpdateThreshold((threshold_ * 2), true);
}
prev_cfg_flow_export_rate_ = cfg_rate;
new_t = threshold();
Expand All @@ -997,7 +986,7 @@ bool FlowStatsManager::UpdateFlowThreshold() {
return true;
}

uint32_t FlowStatsCollector::threshold() const {
uint64_t FlowStatsCollector::threshold() const {
return flow_stats_manager_->threshold();
}

Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.h
Expand Up @@ -103,7 +103,7 @@ class FlowStatsCollector : public StatsCollector {
void set_flow_tcp_syn_age_time(uint64_t interval) {
flow_tcp_syn_age_time_ = interval;
}
uint32_t threshold() const;
uint64_t threshold() const;
boost::uuids::uuid rand_gen();
bool Run();
bool RunAgeingTask();
Expand Down
6 changes: 5 additions & 1 deletion src/vnsw/agent/vrouter/flow_stats/flow_stats_manager.cc
Expand Up @@ -34,7 +34,11 @@ SandeshTraceBufferPtr FlowExportStatsTraceBuf(SandeshTraceBufferCreate(
"FlowExportStats", 3000));
const uint8_t FlowStatsManager::kCatchAllProto;

void FlowStatsManager::UpdateThreshold(uint32_t new_value) {
void FlowStatsManager::UpdateThreshold(uint64_t new_value, bool check_oflow) {
if (check_oflow && new_value < threshold_) {
/* Retain the same value for threshold if it results in overflow */
return;
}
if (new_value < kMinFlowSamplingThreshold) {
threshold_ = kMinFlowSamplingThreshold;
} else {
Expand Down
6 changes: 3 additions & 3 deletions src/vnsw/agent/vrouter/flow_stats/flow_stats_manager.h
Expand Up @@ -149,7 +149,7 @@ class FlowStatsManager {
return flow_export_drops_;
}

uint32_t threshold() const { return threshold_;}
uint64_t threshold() const { return threshold_;}
bool delete_short_flow() const {
return delete_short_flow_;
}
Expand All @@ -173,15 +173,15 @@ class FlowStatsManager {
friend class FlowStatsCollectorReq;
friend class FlowStatsCollector;
bool UpdateFlowThreshold(void);
void UpdateThreshold(uint32_t new_value);
void UpdateThreshold(uint64_t new_value, bool check_oflow);
Agent *agent_;
WorkQueue<boost::shared_ptr<FlowStatsCollectorReq> > request_queue_;
FlowAgingTableMap flow_aging_table_map_;
FlowAgingTablePtr default_flow_stats_collector_;
tbb::atomic<uint32_t> flow_export_count_;
uint64_t prev_flow_export_rate_compute_time_;
uint32_t flow_export_rate_;
uint32_t threshold_;
uint64_t threshold_;
tbb::atomic<uint64_t> flow_export_disable_drops_;
tbb::atomic<uint64_t> flow_export_sampling_drops_;
tbb::atomic<uint32_t> flow_export_without_sampling_;
Expand Down

0 comments on commit 43d1188

Please sign in to comment.