diff --git a/src/vnsw/agent/vrouter/flow_stats/flow_export_info.cc b/src/vnsw/agent/vrouter/flow_stats/flow_export_info.cc index e2f687f75b1..2243c4bf0a1 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_export_info.cc +++ b/src/vnsw/agent/vrouter/flow_stats/flow_export_info.cc @@ -3,16 +3,14 @@ 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) { } @@ -20,8 +18,7 @@ FlowExportInfo::FlowExportInfo(const FlowEntryPtr &fe) : 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) { } diff --git a/src/vnsw/agent/vrouter/flow_stats/flow_export_info.h b/src/vnsw/agent/vrouter/flow_stats/flow_export_info.h index 349b7bc86a2..e2e321501dd 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_export_info.h +++ b/src/vnsw/agent/vrouter/flow_stats/flow_export_info.h @@ -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; @@ -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 diff --git a/src/vnsw/agent/vrouter/flow_stats/flow_stats.sandesh b/src/vnsw/agent/vrouter/flow_stats/flow_stats.sandesh index ddca7a26c46..3f99388051c 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_stats.sandesh +++ b/src/vnsw/agent/vrouter/flow_stats/flow_stats.sandesh @@ -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; } diff --git a/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc b/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc index 1863a92138a..2311d3d91dd 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc +++ b/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc @@ -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 @@ -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 @@ -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); @@ -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. @@ -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(); @@ -997,7 +986,7 @@ bool FlowStatsManager::UpdateFlowThreshold() { return true; } -uint32_t FlowStatsCollector::threshold() const { +uint64_t FlowStatsCollector::threshold() const { return flow_stats_manager_->threshold(); } diff --git a/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.h b/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.h index 5eeb9c9ba66..ec68e588af3 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.h +++ b/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.h @@ -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(); diff --git a/src/vnsw/agent/vrouter/flow_stats/flow_stats_manager.cc b/src/vnsw/agent/vrouter/flow_stats/flow_stats_manager.cc index cebf80a6e70..eef55c68af8 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_stats_manager.cc +++ b/src/vnsw/agent/vrouter/flow_stats/flow_stats_manager.cc @@ -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 { diff --git a/src/vnsw/agent/vrouter/flow_stats/flow_stats_manager.h b/src/vnsw/agent/vrouter/flow_stats/flow_stats_manager.h index a5744cb498a..c635c0da00b 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_stats_manager.h +++ b/src/vnsw/agent/vrouter/flow_stats/flow_stats_manager.h @@ -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_; } @@ -173,7 +173,7 @@ 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 > request_queue_; FlowAgingTableMap flow_aging_table_map_; @@ -181,7 +181,7 @@ class FlowStatsManager { tbb::atomic flow_export_count_; uint64_t prev_flow_export_rate_compute_time_; uint32_t flow_export_rate_; - uint32_t threshold_; + uint64_t threshold_; tbb::atomic flow_export_disable_drops_; tbb::atomic flow_export_sampling_drops_; tbb::atomic flow_export_without_sampling_;