Skip to content

Commit

Permalink
Merge "Fix issue of high stats value being sent from VN UVE." into R3.1
Browse files Browse the repository at this point in the history
  • Loading branch information
Zuul authored and opencontrail-ci-admin committed Feb 14, 2017
2 parents a1dd20a + 80250de commit 5390a8c
Show file tree
Hide file tree
Showing 20 changed files with 464 additions and 168 deletions.
5 changes: 2 additions & 3 deletions src/vnsw/agent/pkt/flow_entry.cc
Expand Up @@ -470,9 +470,8 @@ void FlowEntry::Copy(FlowEntry *rhs, bool update) {
gen_id_ = rhs->gen_id_;
flow_handle_ = rhs->flow_handle_;
/* Flow Entry is being re-used. Generate a new UUID for it. */
// results is delete miss for previous uuid to stats collector
// with eviction disabled following is not required
// uuid_ = flow_table_->rand_gen();
uuid_ = flow_table_->rand_gen();
egress_uuid_ = flow_table_->rand_gen();
}
}

Expand Down
40 changes: 21 additions & 19 deletions src/vnsw/agent/pkt/flow_mgmt.cc
Expand Up @@ -140,7 +140,8 @@ void FlowMgmtManager::DeleteEvent(FlowEntry *flow,

void FlowMgmtManager::FlowStatsUpdateEvent(FlowEntry *flow, uint32_t bytes,
uint32_t packets,
uint32_t oflow_bytes) {
uint32_t oflow_bytes,
const boost::uuids::uuid &u) {
if (bytes == 0 && packets == 0 && oflow_bytes == 0) {
return;
}
Expand All @@ -151,7 +152,7 @@ void FlowMgmtManager::FlowStatsUpdateEvent(FlowEntry *flow, uint32_t bytes,
}
FlowMgmtRequestPtr req(new FlowMgmtRequest
(FlowMgmtRequest::UPDATE_FLOW_STATS, flow,
bytes, packets, oflow_bytes));
bytes, packets, oflow_bytes, u));
request_queue_.Enqueue(req);
}

Expand Down Expand Up @@ -430,13 +431,27 @@ bool FlowMgmtManager::RequestHandler(FlowMgmtRequestPtr req) {
(FlowMgmtRequest::ADD_FLOW,
req->flow().get()));
log_queue_.Enqueue(log_req);

//Enqueue Add request to flow-stats-collector
agent_->flow_stats_manager()->AddEvent(req->flow());

//Enqueue Add request to UVE module for ACE stats
EnqueueUveAddEvent(flow);

AddFlow(req->flow());

} else {
FlowMgmtRequestPtr log_req(new FlowMgmtRequest
(FlowMgmtRequest::DELETE_FLOW,
req->flow().get(), req->params()));
log_queue_.Enqueue(log_req);

//Enqueue Delete request to flow-stats-collector
agent_->flow_stats_manager()->DeleteEvent(flow, req->params());

//Enqueue Delete request to UVE module for ACE stats
EnqueueUveDeleteEvent(flow);

DeleteFlow(req->flow(), req->params());
}
break;
Expand All @@ -445,7 +460,7 @@ bool FlowMgmtManager::RequestHandler(FlowMgmtRequestPtr req) {
case FlowMgmtRequest::UPDATE_FLOW_STATS: {
//Handle Flow stats update for flow-mgmt
UpdateFlowStats(req->flow(), req->bytes(), req->packets(),
req->oflow_bytes());
req->oflow_bytes(), req->flow_uuid());
break;
}

Expand Down Expand Up @@ -493,25 +508,11 @@ bool FlowMgmtManager::LogHandler(FlowMgmtRequestPtr req) {
switch (req->event()) {
case FlowMgmtRequest::ADD_FLOW: {
LogFlowUnlocked(flow, "ADD");

//Enqueue Add request to flow-stats-collector
agent_->flow_stats_manager()->AddEvent(req->flow());

//Enqueue Add request to UVE module for ACE stats
EnqueueUveAddEvent(flow);

break;
}

case FlowMgmtRequest::DELETE_FLOW: {
LogFlowUnlocked(flow, "DEL");

//Enqueue Delete request to flow-stats-collector
agent_->flow_stats_manager()->DeleteEvent(flow, req->params());

//Enqueue Delete request to UVE module for ACE stats
EnqueueUveDeleteEvent(flow);

break;
}

Expand Down Expand Up @@ -674,10 +675,11 @@ void FlowMgmtManager::DeleteFlow(FlowEntryPtr &flow,
}

void FlowMgmtManager::UpdateFlowStats(FlowEntryPtr &flow, uint32_t bytes,
uint32_t packets, uint32_t oflow_bytes) {
uint32_t packets, uint32_t oflow_bytes,
const boost::uuids::uuid &u) {
//Enqueue Flow Index Update Event request to flow-stats-collector
agent_->flow_stats_manager()->UpdateStatsEvent(flow, bytes, packets,
oflow_bytes);
oflow_bytes, u);
}

bool FlowMgmtManager::HasVrfFlows(uint32_t vrf_id) {
Expand Down
5 changes: 3 additions & 2 deletions src/vnsw/agent/pkt/flow_mgmt.h
Expand Up @@ -1108,7 +1108,8 @@ class FlowMgmtManager {
void AddEvent(FlowEntry *low);
void DeleteEvent(FlowEntry *flow, const RevFlowDepParams &params);
void FlowStatsUpdateEvent(FlowEntry *flow, uint32_t bytes, uint32_t packets,
uint32_t oflow_bytes);
uint32_t oflow_bytes,
const boost::uuids::uuid &u);
void AddDBEntryEvent(const DBEntry *entry, uint32_t gen_id);
void ChangeDBEntryEvent(const DBEntry *entry, uint32_t gen_id);
void DeleteDBEntryEvent(const DBEntry *entry, uint32_t gen_id);
Expand Down Expand Up @@ -1145,7 +1146,7 @@ class FlowMgmtManager {
// Handle Delete of a flow. Updates FlowMgmtKeyTree for all objects
void DeleteFlow(FlowEntryPtr &flow, const RevFlowDepParams &p);
void UpdateFlowStats(FlowEntryPtr &flow, uint32_t bytes, uint32_t packets,
uint32_t oflow_bytes);
uint32_t oflow_bytes, const boost::uuids::uuid &u);

// Add a FlowMgmtKey into the FlowMgmtKeyTree for an object
// The FlowMgmtKeyTree for object is passed as argument
Expand Down
8 changes: 6 additions & 2 deletions src/vnsw/agent/pkt/flow_mgmt_request.h
Expand Up @@ -41,9 +41,11 @@ class FlowMgmtRequest {
}

FlowMgmtRequest(Event event, FlowEntry *flow, uint32_t bytes,
uint32_t packets, uint32_t oflow_bytes) :
uint32_t packets, uint32_t oflow_bytes,
const boost::uuids::uuid &u) :
event_(event), flow_(flow), db_entry_(NULL), vrf_id_(0), gen_id_(),
bytes_(bytes), packets_(packets), oflow_bytes_(oflow_bytes), params_() {
bytes_(bytes), packets_(packets), oflow_bytes_(oflow_bytes), params_(),
flow_uuid_(u) {
if (event == RETRY_DELETE_VRF)
assert(vrf_id_);
}
Expand Down Expand Up @@ -109,6 +111,7 @@ class FlowMgmtRequest {
void set_params(const RevFlowDepParams &params) {
params_ = params;
}
boost::uuids::uuid flow_uuid() const { return flow_uuid_; }

private:
Event event_;
Expand All @@ -123,6 +126,7 @@ class FlowMgmtRequest {
uint32_t packets_;
uint32_t oflow_bytes_;
RevFlowDepParams params_;
boost::uuids::uuid flow_uuid_;

DISALLOW_COPY_AND_ASSIGN(FlowMgmtRequest);
};
Expand Down
3 changes: 2 additions & 1 deletion src/vnsw/agent/pkt/flow_table.cc
Expand Up @@ -754,7 +754,8 @@ void FlowTable::ProcessKSyncFlowEvent(const FlowEventKSync *req,
mgr->FlowStatsUpdateEvent(evicted_flow.get(),
req->evict_flow_bytes(),
req->evict_flow_packets(),
req->evict_flow_oflow());
req->evict_flow_oflow(),
evicted_flow->uuid());
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/vnsw/agent/pkt/pkt_flow_info.cc
Expand Up @@ -1674,10 +1674,11 @@ void PktFlowInfo::UpdateEvictedFlowStats(const PktInfo *pkt) {
FlowMgmtManager *mgr = agent->pkt()->flow_mgmt_manager(
flow_table->table_index());

/* Enqueue stats update request with UUID of the flow */
if (flow.get() && flow->deleted() == false) {
mgr->FlowStatsUpdateEvent(flow.get(), pkt->agent_hdr.cmd_param_2,
pkt->agent_hdr.cmd_param_3,
pkt->agent_hdr.cmd_param_4);
pkt->agent_hdr.cmd_param_4, flow->uuid());
}
}

Expand Down
59 changes: 59 additions & 0 deletions src/vnsw/agent/pkt/test/test_flow_table.cc
Expand Up @@ -288,6 +288,65 @@ TEST_F(TestFlowTable, EvictPktTrapBeforeReverseFlowResp) {
client->WaitForIdle();
}

TEST_F(TestFlowTable, ResetFlowStats) {
TxTcpMplsPacket(eth->id(), remote_compute, router_id_, vif0->label(),
remote_vm1_ip, vm1_ip, 1000, 200, 1, 1);
client->WaitForIdle();

FlowEntry *flow = FlowGet(remote_vm1_ip, vm1_ip, IPPROTO_TCP, 1000,
200, vif0->flow_key_nh()->id(), 1);
EXPECT_TRUE(flow != NULL);
FlowEntry *rflow = flow->reverse_flow_entry();
EXPECT_TRUE(rflow != NULL);

//Invoke FlowStatsCollector to check whether flow gets evicted
util_.EnqueueFlowStatsCollectorTask();
client->WaitForIdle();

//Verify the stats of flow
EXPECT_TRUE(FlowStatsMatch("vrf1", remote_vm1_ip, vm1_ip, IPPROTO_TCP, 1000,
200, 1, 30, vif0->flow_key_nh()->id(), 1));

//Change the stats
KSyncSockTypeMap::IncrFlowStats(1, 1, 30);

//Invoke FlowStatsCollector to enqueue delete for evicted flow.
util_.EnqueueFlowStatsCollectorTask();
client->WaitForIdle();

//Verify the stats of flow
EXPECT_TRUE(FlowStatsMatch("vrf1", remote_vm1_ip, vm1_ip, IPPROTO_TCP, 1000,
200, 2, 60, vif0->flow_key_nh()->id(), 1));

TxTcpMplsPacket(eth->id(), remote_compute, router_id_, vif0->label(),
remote_vm1_ip, vm1_ip, 1000, 200, 1, 10);
client->WaitForIdle();

flow = FlowGet(remote_vm1_ip, vm1_ip, IPPROTO_TCP, 1000,
200, vif0->flow_key_nh()->id(), 10);
EXPECT_TRUE(flow != NULL);

//Verify that flow-handle is updated in flow-stats module
FlowStatsCollector *fsc = flow->fsc();
EXPECT_TRUE(fsc != NULL);
FlowExportInfo *info = fsc->FindFlowExportInfo(flow);
EXPECT_TRUE(info != NULL);
EXPECT_TRUE(info->flow_handle() == 10);

//Verify the stats of flow are reset after change of flow-handle
EXPECT_TRUE(FlowStatsMatch("vrf1", remote_vm1_ip, vm1_ip, IPPROTO_TCP, 1000,
200, 0, 0, vif0->flow_key_nh()->id(), 10));

//Invoke FlowStatsCollector to enqueue delete for reverse flow which is
//marked as short flow
util_.EnqueueFlowStatsCollectorTask();
client->WaitForIdle();

//Verify the stats of flow
EXPECT_TRUE(FlowStatsMatch("vrf1", remote_vm1_ip, vm1_ip, IPPROTO_TCP, 1000,
200, 1, 30, vif0->flow_key_nh()->id(), 10));
}

int main(int argc, char *argv[]) {
int ret = 0;
GETUSERARGS();
Expand Down
2 changes: 1 addition & 1 deletion src/vnsw/agent/test-xml/test_xml.cc
Expand Up @@ -35,7 +35,7 @@ class FlowExportTask : public Task {
return true;
}

fec->ExportFlow(info, bytes_, pkts_, NULL);
fec->ExportFlow(info, bytes_, pkts_, NULL, true);
return true;
}
std::string Description() const { return "FlowExportTask"; }
Expand Down
3 changes: 2 additions & 1 deletion src/vnsw/agent/test/test_cmn_util.h
Expand Up @@ -369,7 +369,8 @@ FlowEntry* FlowGet(int vrf_id, std::string sip, std::string dip, uint8_t proto,
uint16_t sport, uint16_t dport, int nh_id);
bool FlowStatsMatch(const string &vrf_name, const char *sip, const char *dip,
uint8_t proto, uint16_t sport, uint16_t dport,
uint64_t pkts, uint64_t bytes, int nh_id);
uint64_t pkts, uint64_t bytes, int nh_id,
uint32_t flow_handle = FlowEntry::kInvalidFlowHandle);
bool FindFlow(const string &vrf_name, const char *sip, const char *dip,
uint8_t proto, uint16_t sport, uint16_t dport, bool nat,
const string &nat_vrf_name, const char *nat_sip,
Expand Down
15 changes: 13 additions & 2 deletions src/vnsw/agent/test/test_util.cc
Expand Up @@ -3452,7 +3452,8 @@ bool FlowGet(const string &vrf_name, const char *sip, const char *dip,

bool FlowStatsMatch(const string &vrf_name, const char *sip,
const char *dip, uint8_t proto, uint16_t sport,
uint16_t dport, uint64_t pkts, uint64_t bytes, int nh_id) {
uint16_t dport, uint64_t pkts, uint64_t bytes, int nh_id,
uint32_t flow_handle) {
Agent *agent = Agent::GetInstance();
VrfEntry *vrf = agent->vrf_table()->FindVrfFromName(vrf_name);
EXPECT_TRUE(vrf != NULL);
Expand All @@ -3468,7 +3469,17 @@ bool FlowStatsMatch(const string &vrf_name, const char *sip,
key.protocol = proto;
key.family = key.src_addr.is_v4() ? Address::INET : Address::INET6;

FlowEntry *fe = agent->pkt()->get_flow_proto()->Find(key, 0);
FlowEntry *fe = NULL;
if (flow_handle != FlowEntry::kInvalidFlowHandle) {
FlowTable *table = agent->pkt()->get_flow_proto()->GetFlowTable(key,
flow_handle);
if (table == NULL) {
return NULL;
}
fe = table->Find(key);
} else {
fe = agent->pkt()->get_flow_proto()->Find(key, 0);
}
EXPECT_TRUE(fe != NULL);
if (fe == NULL) {
return false;
Expand Down
28 changes: 25 additions & 3 deletions src/vnsw/agent/vrouter/flow_stats/flow_export_info.cc
Expand Up @@ -5,22 +5,28 @@ FlowExportInfo::FlowExportInfo() :
flow_(), setup_time_(0), teardown_time_(0), last_modified_time_(0),
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) {
exported_atleast_once_(false), gen_id_(0),
flow_handle_(FlowEntry::kInvalidFlowHandle),
rev_flow_egress_uuid_(nil_uuid()) {
}

FlowExportInfo::FlowExportInfo(const FlowEntryPtr &fe) :
flow_(fe), setup_time_(0), teardown_time_(0), last_modified_time_(0),
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) {
exported_atleast_once_(false), gen_id_(0),
flow_handle_(FlowEntry::kInvalidFlowHandle),
rev_flow_egress_uuid_(nil_uuid()) {
}

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), underlay_source_port_(0), changed_(true),
tcp_flags_(0), delete_enqueue_time_(0), evict_enqueue_time_(0),
exported_atleast_once_(false) {
exported_atleast_once_(false), gen_id_(0),
flow_handle_(FlowEntry::kInvalidFlowHandle),
rev_flow_egress_uuid_(nil_uuid()) {
}

FlowEntry* FlowExportInfo::reverse_flow() const {
Expand All @@ -39,6 +45,22 @@ bool FlowExportInfo::IsActionLog() const {
return false;
}

void FlowExportInfo::CopyFlowInfo(FlowEntry *fe) {
gen_id_ = fe->gen_id();
flow_handle_ = fe->flow_handle();
uuid_ = fe->uuid();
flags_ = fe->flags();
FlowEntry *rflow = reverse_flow();
if (rflow) {
rev_flow_egress_uuid_ = rflow->egress_uuid();
}
}

void FlowExportInfo::ResetStats() {
bytes_ = packets_ = 0;
tcp_flags_ = 0;
underlay_source_port_ = 0;
}
///////////////////////////////////////////////////////////////////
// APIs used only by UT
//////////////////////////////////////////////////////////////////
Expand Down
19 changes: 19 additions & 0 deletions src/vnsw/agent/vrouter/flow_stats/flow_export_info.h
Expand Up @@ -50,6 +50,20 @@ class FlowExportInfo {
uint64_t delete_enqueue_time() const { return delete_enqueue_time_; }
void set_evict_enqueue_time(uint64_t value) { evict_enqueue_time_ = value; }
uint64_t evict_enqueue_time() const { return evict_enqueue_time_; }
uint8_t gen_id() const { return gen_id_; }
void set_gen_id(uint8_t value) { gen_id_ = value; }
uint32_t flow_handle() const { return flow_handle_; }
void set_flow_handle(uint32_t value) { flow_handle_ = value; }
const boost::uuids::uuid &uuid() const { return uuid_; }
const boost::uuids::uuid &rev_flow_egress_uuid() const {
return rev_flow_egress_uuid_;
}
uint32_t flags() const { return flags_; }
bool is_flags_set(const FlowEntry::FlowEntryFlags &flags) const {
return (flags_ & flags);
}
void CopyFlowInfo(FlowEntry *fe);
void ResetStats();
private:
FlowEntryPtr flow_;
uint64_t setup_time_;
Expand All @@ -67,6 +81,11 @@ class FlowExportInfo {
uint64_t delete_enqueue_time_;
uint64_t evict_enqueue_time_;
bool exported_atleast_once_;
uint8_t gen_id_;
uint32_t flow_handle_;
boost::uuids::uuid uuid_;
boost::uuids::uuid rev_flow_egress_uuid_;
uint32_t flags_;
};

#endif // __AGENT_FLOW_EXPORT_INFO_H__

0 comments on commit 5390a8c

Please sign in to comment.