Skip to content

Commit

Permalink
Fix flow sampling threshold value from overflowing.
Browse files Browse the repository at this point in the history
1.  When a flow-sample is getting dropped, reset its diff statistics. The lost
    statistics will be accounted by normalizing of stats of flows that get
    exported. This reset will reduce the number flows with high statistics and
    prevent threshold from overflowing.
2.  Change the flow threshold value from u32 to u64 field
3.  Also, don’t let u64 threshold value to overflow. If any increment of
    threshold causes its value to overflow, don’t increment/change the value of
    threshold

Closes-Bug: #1655276
(cherry picked from commit a69c957)

Change-Id: If9a50828bdbddcbeb67bd89073bd24b80a00db99
  • Loading branch information
ashoksr committed Jan 16, 2017
1 parent f54d197 commit 76e4ef1
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 76e4ef1

Please sign in to comment.