From 257b512b3eb078c9874e7ec370c4685f6593fce8 Mon Sep 17 00:00:00 2001 From: Ashok Singh Date: Wed, 2 Mar 2016 02:35:03 -0800 Subject: [PATCH] Update Aging time for TCP flow-stats-collector When aging time is changed via introspect it was updated only for default flow-stats-collector. We need to update the aging time for implicitly added TCP flow-stats-collector as well. However this update should not be done when TCP flow-stats-collector is configured explicitly via configuration (Protocol based aging config) Closes-Bug: #1551201 (cherry picked from commit 54becf8482dfcdcc3a2180f4cf4ff348c815bfbc) Change-Id: I91fa0dbce6918ff12ab3c1c436e6f2dec7d0f0f7 --- src/vnsw/agent/pkt/pkt.sandesh | 2 + src/vnsw/agent/pkt/pkt_sandesh_flow.cc | 29 ++++++--- .../vrouter/flow_stats/flow_stats_collector.h | 3 + .../vrouter/flow_stats/flow_stats_manager.cc | 19 +++--- .../vrouter/flow_stats/flow_stats_manager.h | 8 ++- .../flow_stats/test/test_flow_stats.cc | 61 +++++++++++++++++++ 6 files changed, 102 insertions(+), 20 deletions(-) diff --git a/src/vnsw/agent/pkt/pkt.sandesh b/src/vnsw/agent/pkt/pkt.sandesh index e02bf1264b0..7f21b7978fb 100644 --- a/src/vnsw/agent/pkt/pkt.sandesh +++ b/src/vnsw/agent/pkt/pkt.sandesh @@ -372,6 +372,8 @@ traceobject sandesh FlowRouteUpdate { response sandesh FlowAgeTimeResp { 1: u32 old_age_time; 2: u32 new_age_time; + 3: u32 old_tcp_age_time; + 4: u32 new_tcp_age_time; } /** diff --git a/src/vnsw/agent/pkt/pkt_sandesh_flow.cc b/src/vnsw/agent/pkt/pkt_sandesh_flow.cc index 115ffbdf7fb..85267059742 100644 --- a/src/vnsw/agent/pkt/pkt_sandesh_flow.cc +++ b/src/vnsw/agent/pkt/pkt_sandesh_flow.cc @@ -412,20 +412,31 @@ void FlowAgeTimeReq::HandleRequest() const { FlowStatsCollector *collector = agent->flow_stats_manager()->default_flow_stats_collector(); + FlowStatsCollector *tcp_col = + agent->flow_stats_manager()->tcp_flow_stats_collector(); FlowAgeTimeResp *resp = new FlowAgeTimeResp(); - if (collector == NULL) { - goto done; + if (collector) { + resp->set_old_age_time(collector->flow_age_time_intvl_in_secs()); + + if (age_time && age_time != resp->get_old_age_time()) { + collector->UpdateFlowAgeTimeInSecs(age_time); + resp->set_new_age_time(age_time); + } else { + resp->set_new_age_time(resp->get_old_age_time()); + } } - resp->set_old_age_time(collector->flow_age_time_intvl_in_secs()); + if (tcp_col) { + resp->set_old_tcp_age_time(tcp_col->flow_age_time_intvl_in_secs()); - if (age_time && age_time != resp->get_old_age_time()) { - collector->UpdateFlowAgeTimeInSecs(age_time); - resp->set_new_age_time(age_time); - } else { - resp->set_new_age_time(resp->get_old_age_time()); + if (age_time && age_time != resp->get_old_age_time() && + !tcp_col->user_configured()) { + tcp_col->UpdateFlowAgeTimeInSecs(age_time); + resp->set_new_tcp_age_time(age_time); + } else { + resp->set_new_tcp_age_time(resp->get_old_tcp_age_time()); + } } -done: resp->set_context(context()); resp->set_more(false); resp->Response(); 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 3561aef1be1..0e76ecff7ea 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.h +++ b/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.h @@ -101,6 +101,8 @@ class FlowStatsCollector : public StatsCollector { bool deleted() const { return deleted_; } + bool user_configured() const { return user_configured_; } + void set_user_configured(bool value) { user_configured_ = value; } const FlowAgingTableKey& flow_aging_key() const { return flow_aging_key_; } @@ -194,6 +196,7 @@ class FlowStatsCollector : public StatsCollector { FlowAgingTableKey flow_aging_key_; uint32_t instance_id_; FlowStatsManager *flow_stats_manager_; + bool user_configured_; DISALLOW_COPY_AND_ASSIGN(FlowStatsCollector); }; #endif //vnsw_agent_flow_stats_collector_h 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 67dc5e0180e..0a9a2391309 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_stats_manager.cc +++ b/src/vnsw/agent/vrouter/flow_stats/flow_stats_manager.cc @@ -119,6 +119,7 @@ void FlowStatsManager::AddReqHandler(boost::shared_ptr it->second->set_flow_age_time_intvl( 1000000L * (uint64_t)req->flow_cache_timeout); it->second->set_deleted(false); + it->second->set_user_configured(req->user_configured); return; } @@ -137,6 +138,7 @@ void FlowStatsManager::AddReqHandler(boost::shared_ptr if (req->key.port == 0) { protocol_list_[req->key.proto] = aging_table.get(); } + aging_table->set_user_configured(req->user_configured); } void FlowStatsManager::DeleteReqHandler(boost::shared_ptr @@ -175,12 +177,13 @@ void FlowStatsManager::FreeReqHandler(boost::shared_ptr } void FlowStatsManager::Add(const FlowAgingTableKey &key, - uint64_t flow_stats_interval, - uint64_t flow_cache_timeout) { + uint64_t flow_stats_interval, + uint64_t flow_cache_timeout, bool user_configured) { boost::shared_ptr req(new FlowStatsCollectorReq( FlowStatsCollectorReq::ADD_FLOW_STATS_COLLECTOR, - key, flow_stats_interval, flow_cache_timeout)); + key, flow_stats_interval, flow_cache_timeout, + user_configured)); request_queue_.Enqueue(req); } @@ -197,7 +200,7 @@ void FlowStatsManager::Delete(const FlowAgingTableKey &key) { req.reset(new FlowStatsCollectorReq( FlowStatsCollectorReq::ADD_FLOW_STATS_COLLECTOR, key, agent_->params()->flow_stats_interval(), - agent_->params()->flow_cache_timeout())); + agent_->params()->flow_cache_timeout(), false)); } else { req.reset(new FlowStatsCollectorReq( FlowStatsCollectorReq::DELETE_FLOW_STATS_COLLECTOR, key)); @@ -339,7 +342,7 @@ void FlowStatsManager::FlowStatsReqHandler(Agent *agent, } else { agent->flow_stats_manager()->Add( FlowAgingTableKey(protocol, port), - agent->params()->flow_stats_interval(), timeout); + agent->params()->flow_stats_interval(), timeout, true); } } @@ -348,9 +351,9 @@ void FlowStatsManager::Init(uint64_t flow_stats_interval, agent_->set_flow_stats_req_handler(&(FlowStatsManager::FlowStatsReqHandler)); Add(FlowAgingTableKey(kCatchAllProto, 0), flow_stats_interval, - flow_cache_timeout); + flow_cache_timeout, false); Add(FlowAgingTableKey(IPPROTO_TCP, 0), flow_stats_interval, - flow_cache_timeout); + flow_cache_timeout, false); timer_->Start(FlowThresoldUpdateTime, boost::bind(&FlowStatsManager::UpdateFlowThreshold, this)); } @@ -391,7 +394,7 @@ void ShowAgingConfig::HandleRequest() const { void AddAgingConfig::HandleRequest() const { FlowStatsManager *fam = Agent::GetInstance()->flow_stats_manager(); fam->Add(FlowAgingTableKey(get_protocol(), get_port()), - get_stats_interval(), get_cache_timeout()); + get_stats_interval(), get_cache_timeout(), true); SandeshResponse *resp = new FlowStatsCfgResp(); resp->set_context(context()); resp->Response(); 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 f9f4b3ecd32..1f695735a0f 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_stats_manager.h +++ b/src/vnsw/agent/vrouter/flow_stats/flow_stats_manager.h @@ -44,9 +44,10 @@ struct FlowStatsCollectorReq { }; FlowStatsCollectorReq(Event ev, const FlowAgingTableKey &k, - uint64_t interval, uint64_t timeout) : + uint64_t interval, uint64_t timeout, + bool user_cfgd) : event(ev), key(k), flow_stats_interval(interval), - flow_cache_timeout(timeout) {} + flow_cache_timeout(timeout), user_configured(user_cfgd) {} FlowStatsCollectorReq(Event ev, const FlowAgingTableKey &k): event(ev), key(k) {} @@ -55,6 +56,7 @@ struct FlowStatsCollectorReq { FlowAgingTableKey key; uint64_t flow_stats_interval; uint64_t flow_cache_timeout; + bool user_configured; }; class FlowStatsManager { @@ -84,7 +86,7 @@ class FlowStatsManager { //Add protocol + port based flow aging table void Add(const FlowAgingTableKey &key, uint64_t flow_stats_interval, - uint64_t flow_cache_timeout); + uint64_t flow_cache_timeout, bool user_configured); void Delete(const FlowAgingTableKey &key); void Free(const FlowAgingTableKey &key); diff --git a/src/vnsw/agent/vrouter/flow_stats/test/test_flow_stats.cc b/src/vnsw/agent/vrouter/flow_stats/test/test_flow_stats.cc index 1944edb7083..8bbc662541b 100644 --- a/src/vnsw/agent/vrouter/flow_stats/test/test_flow_stats.cc +++ b/src/vnsw/agent/vrouter/flow_stats/test/test_flow_stats.cc @@ -39,6 +39,25 @@ class FlowStatsTest : public ::testing::Test { type_specific_response_count_++; } } + void FlowAgeTimeSet(uint64_t age_time_secs) { + FlowAgeTimeReq *req = new FlowAgeTimeReq(); + req->set_new_age_time(age_time_secs); + Sandesh::set_response_callback( + boost::bind(&FlowStatsTest::FlowAgeTimeResponse, this, _1)); + req->HandleRequest(); + client->WaitForIdle(); + req->Release(); + } + void FlowAgeTimeResponse(Sandesh *sandesh) { + response_count_++; + FlowAgeTimeResp *resp = + dynamic_cast(sandesh); + if (resp != NULL) { + type_specific_response_count_++; + age_resp_.set_new_age_time(resp->get_new_age_time()); + age_resp_.set_new_tcp_age_time(resp->get_new_tcp_age_time()); + } + } void FlowParamsGet() { FlowStatsCollectionParamsReq *req = new FlowStatsCollectionParamsReq(); Sandesh::set_response_callback( @@ -105,6 +124,7 @@ class FlowStatsTest : public ::testing::Test { uint32_t num_entries_; Agent *agent_; FlowProto *flow_proto_; + FlowAgeTimeResp age_resp_; }; TEST_F(FlowStatsTest, SandeshFlowParams) { @@ -195,6 +215,47 @@ TEST_F(FlowStatsTest, FlowTreeSize) { FlowTeardown(); } +TEST_F(FlowStatsTest, FlowAgeIntrospect_1) { + ClearCount(); + FlowAgeTimeSet(100); + client->WaitForIdle(); + WAIT_FOR(1000, 1000, (response_count_ == 1)); + EXPECT_EQ(1U, type_specific_response_count_); + EXPECT_EQ(100U, age_resp_.get_new_age_time()); + EXPECT_EQ(100U, age_resp_.get_new_tcp_age_time()); + + //cleanup + ClearCount(); + uint64_t default_age_time = FlowStatsCollector::FlowAgeTime/(1000 * 1000); + FlowAgeTimeSet(default_age_time); + client->WaitForIdle(); + WAIT_FOR(1000, 1000, (response_count_ == 1)); +} + +TEST_F(FlowStatsTest, FlowAgeIntrospect_2) { + ClearCount(); + /* Mark TCP flow-stats-collector as user_configured */ + FlowStatsCollector *tcp_col = + agent_->flow_stats_manager()->tcp_flow_stats_collector(); + tcp_col->set_user_configured(true); + uint64_t old_age_time = tcp_col->flow_age_time_intvl_in_secs(); + FlowAgeTimeSet(100); + client->WaitForIdle(); + WAIT_FOR(1000, 1000, (response_count_ == 1)); + EXPECT_EQ(1U, type_specific_response_count_); + + //Verify that age is updated only for default collector. + EXPECT_EQ(100U, age_resp_.get_new_age_time()); + EXPECT_EQ(old_age_time, age_resp_.get_new_tcp_age_time()); + + //cleanup + ClearCount(); + uint64_t default_age_time = FlowStatsCollector::FlowAgeTime/(1000 * 1000); + FlowAgeTimeSet(default_age_time); + client->WaitForIdle(); + WAIT_FOR(1000, 1000, (response_count_ == 1)); +} + int main(int argc, char *argv[]) { int ret; GETUSERARGS();