Skip to content

Commit

Permalink
Merge "Fix issue of high stats value being sent from VN UVE." into R3.2
Browse files Browse the repository at this point in the history
  • Loading branch information
Zuul authored and opencontrail-ci-admin committed Feb 9, 2017
2 parents 52f3517 + e5557d8 commit 362b35e
Show file tree
Hide file tree
Showing 20 changed files with 457 additions and 159 deletions.
5 changes: 2 additions & 3 deletions src/vnsw/agent/pkt/flow_entry.cc
Expand Up @@ -463,9 +463,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
12 changes: 7 additions & 5 deletions src/vnsw/agent/pkt/flow_mgmt.cc
Expand Up @@ -153,7 +153,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 @@ -164,7 +165,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 @@ -472,7 +473,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 @@ -687,10 +688,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 @@ -1107,7 +1107,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 @@ -1144,7 +1145,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 @@ -1665,10 +1665,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 @@ -37,7 +37,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 @@ -373,7 +373,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 @@ -3450,7 +3450,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 @@ -3466,7 +3467,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),
visit_time_(0), exported_atleast_once_(false) {
visit_time_(0), 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),
visit_time_(0), exported_atleast_once_(false) {
visit_time_(0), 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),
visit_time_(0), exported_atleast_once_(false) {
visit_time_(0), 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 @@ -53,6 +53,20 @@ class FlowExportInfo : public boost::intrusive::list_base_hook<> {

uint64_t visit_time() const { return visit_time_; }
void set_visit_time(uint64_t t) { visit_time_ = t; }
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 @@ -71,6 +85,11 @@ class FlowExportInfo : public boost::intrusive::list_base_hook<> {
uint64_t evict_enqueue_time_;
uint64_t visit_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_;
};

typedef boost::intrusive::list<FlowExportInfo> FlowExportInfoList;
Expand Down
7 changes: 5 additions & 2 deletions src/vnsw/agent/vrouter/flow_stats/flow_export_request.h
Expand Up @@ -30,9 +30,10 @@ class FlowExportReq {
}

FlowExportReq(Event event, const FlowExportInfo &info, uint32_t bytes,
uint32_t packets, uint32_t oflow_bytes) :
uint32_t packets, uint32_t oflow_bytes,
const boost::uuids::uuid &u) :
event_(event), info_(info), bytes_(bytes), packets_(packets),
oflow_bytes_(oflow_bytes) {
oflow_bytes_(oflow_bytes), uuid_(u) {
}

~FlowExportReq() { }
Expand All @@ -44,6 +45,7 @@ class FlowExportReq {
uint32_t packets() const { return packets_;}
uint32_t oflow_bytes() const { return oflow_bytes_;}
const RevFlowDepParams& params() const { return params_; }
boost::uuids::uuid uuid() const { return uuid_; }

private:
Event event_;
Expand All @@ -53,6 +55,7 @@ class FlowExportReq {
uint32_t packets_;
uint32_t oflow_bytes_;
RevFlowDepParams params_;
boost::uuids::uuid uuid_;
DISALLOW_COPY_AND_ASSIGN(FlowExportReq);
};
#endif // __AGENT_FLOW_EXPORT_REQUEST_H__

0 comments on commit 362b35e

Please sign in to comment.