Skip to content

Commit

Permalink
Send all fields of flow always in FlowLog messages
Browse files Browse the repository at this point in the history
Currently vrouter-agent sends the following fields only once

setup-time
vrouter-ip
peer-vrouter-ip
underlay-port
tunnel-type

With flow sampling in place, if flow sample with above information is dropped,
the rest of the samples for this flow, which make it to collector will never
have this info. To overcome this we send all the fields always.

Also remove teardown-time since it is sent only once for delete and there are
chances of this sample getting dropped

Also remove unused flow fields from flow.sandesh.

Change-Id: Ib17fad30bc9d859375f82de363e57f97251686db
Closes-Bug: #1536118
  • Loading branch information
ashoksr committed Jan 21, 2016
1 parent ae3ea2f commit 958d5bd
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 80 deletions.
11 changes: 0 additions & 11 deletions src/sandesh/common/flow.sandesh
Expand Up @@ -18,24 +18,13 @@ struct FlowLogData {
7: optional byte protocol;
8: optional i16 sport;
9: optional i16 dport;
10: optional byte tos;
11: optional u16 tcp_flags;
12: optional string vm;
13: optional string input_interface;
14: optional string output_interface;
15: optional i32 mpls_label;
16: optional string reverse_uuid;
17: optional i64 setup_time;
18: optional i64 teardown_time;

19: optional i32 min_interarrival;
20: optional i32 max_interarrival;
21: optional i32 mean_interarrival;
22: optional i32 stddev_interarrival;

23: optional i64 bytes;
24: optional i64 packets;
25: optional binary data_sample;
26: optional i64 diff_bytes;
27: optional i64 diff_packets;
28: optional string action;
Expand Down
71 changes: 68 additions & 3 deletions src/vnsw/agent/vrouter/flow_stats/flow_export_info.cc
Expand Up @@ -7,7 +7,7 @@ FlowExportInfo::FlowExportInfo() :
bytes_(0), packets_(0), flags_(0),
flow_handle_(FlowEntry::kInvalidFlowHandle), action_info_(),
vm_cfg_name_(), peer_vrouter_(), tunnel_type_(TunnelType::INVALID),
underlay_source_port_(0), underlay_sport_exported_(false), exported_(false),
underlay_source_port_(0), changed_(false),
fip_(0), fip_vmi_(AgentKey::ADD_DEL_CHANGE, nil_uuid(), ""), tcp_flags_(0) {
drop_reason_ = FlowEntry::DropReasonStr(FlowEntry::DROP_UNKNOWN);
rev_flow_key_.Reset();
Expand All @@ -22,8 +22,7 @@ FlowExportInfo::FlowExportInfo(FlowEntry *fe, uint64_t setup_time) :
flow_handle_(fe->flow_handle()), action_info_(fe->match_p().action_info),
vm_cfg_name_(fe->data().vm_cfg_name), peer_vrouter_(fe->peer_vrouter()),
tunnel_type_(fe->tunnel_type()), underlay_source_port_(0),
underlay_sport_exported_(false), exported_(false), fip_(fe->fip()),
fip_vmi_(fe->fip_vmi()), tcp_flags_(0) {
changed_(true), fip_(fe->fip()), fip_vmi_(fe->fip_vmi()), tcp_flags_(0) {
flow_uuid_ = FlowTable::rand_gen_();
egress_uuid_ = FlowTable::rand_gen_();
FlowEntry *rflow = fe->reverse_flow_entry();
Expand All @@ -40,6 +39,72 @@ FlowExportInfo::FlowExportInfo(FlowEntry *fe, uint64_t setup_time) :
drop_reason_ = FlowEntry::DropReasonStr(fe->data().drop_reason);
}

/* This API compares only fields which are copied from FlowEntry. Fields which
* are locally generated or read from vrouter are not compared. */
bool FlowExportInfo::IsEqual(const FlowExportInfo &rhs) const {
if (source_vn_ != rhs.source_vn()) {
return false;
}
if (dest_vn_ != rhs.dest_vn()) {
return false;
}
if (sg_rule_uuid_ != rhs.sg_rule_uuid()) {
return false;
}
if (nw_ace_uuid_ != rhs.nw_ace_uuid()) {
return false;
}
if (flags_ != rhs.flags()) {
return false;
}
if (action_info_.action != rhs.action_info().action) {
return false;
}
if (vm_cfg_name_ != rhs.vm_cfg_name()) {
return false;
}
if (peer_vrouter_ != rhs.peer_vrouter()) {
return false;
}
if (!tunnel_type_.Compare(rhs.tunnel_type())) {
return false;
}
if (fip_ != rhs.fip()) {
return false;
}
if (!fip_vmi_.IsEqual(rhs.fip_vmi())) {
return false;
}
if (!rev_flow_key_.IsEqual(rhs.rev_flow_key())) {
return false;
}
if (interface_uuid_ != rhs.interface_uuid()) {
return false;
}
if (drop_reason_ != rhs.drop_reason()) {
return false;
}
return true;
}

void FlowExportInfo::Copy(const FlowExportInfo &rhs) {
source_vn_ = rhs.source_vn();
dest_vn_ = rhs.dest_vn();
sg_rule_uuid_ = rhs.sg_rule_uuid();
nw_ace_uuid_ = rhs.nw_ace_uuid();
flags_ = rhs.flags();
action_info_ = rhs.action_info();
vm_cfg_name_ = rhs.vm_cfg_name();
peer_vrouter_ = rhs.peer_vrouter();
tunnel_type_ = rhs.tunnel_type();
fip_ = rhs.fip();
fip_vmi_ = rhs.fip_vmi();
rev_flow_key_ = rhs.rev_flow_key();
interface_uuid_ = rhs.interface_uuid();
drop_reason_ = rhs.drop_reason();
changed_ = true;
}

bool FlowExportInfo::IsActionLog() const {
uint32_t fe_action = action_info_.action;
if (fe_action & (1 << TrafficAction::LOG)) {
Expand Down
15 changes: 6 additions & 9 deletions src/vnsw/agent/vrouter/flow_stats/flow_export_info.h
Expand Up @@ -41,13 +41,9 @@ class FlowExportInfo {
void set_underlay_source_port(uint16_t port) {
underlay_source_port_ = port;
}
bool underlay_sport_exported() const { return underlay_sport_exported_; }
void set_underlay_sport_exported(bool value) {
underlay_sport_exported_ = value;
}
bool exported() const { return exported_; }
void set_exported(bool value) {
exported_ = value;
bool changed() const { return changed_; }
void set_changed(bool value) {
changed_ = value;
}
uint32_t fip() const { return fip_; }
VmInterfaceKey fip_vmi() const { return fip_vmi_; }
Expand All @@ -63,6 +59,8 @@ class FlowExportInfo {
void set_tcp_flags(uint16_t tflags) {
tcp_flags_ = tflags;
}
bool IsEqual(const FlowExportInfo &rhs) const;
void Copy(const FlowExportInfo &rhs);
private:
boost::uuids::uuid flow_uuid_;
boost::uuids::uuid egress_uuid_; // used/applicable only for local flows
Expand All @@ -87,8 +85,7 @@ class FlowExportInfo {
TunnelType tunnel_type_;
//Underlay source port. 0 for local flows. Used during flow-export
uint16_t underlay_source_port_;
bool underlay_sport_exported_;
bool exported_;
bool changed_;
uint32_t fip_;
VmInterfaceKey fip_vmi_;
boost::uuids::uuid interface_uuid_;
Expand Down
66 changes: 10 additions & 56 deletions src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc
Expand Up @@ -423,7 +423,7 @@ bool FlowStatsCollector::Run() {
k_flow->fe_stats.flow_packets,
k_flow->fe_stats.flow_packets_oflow,
curr_time, false);
} else if (!info->exported()) {
} else if (info->changed()) {
/* export flow (reverse) for which traffic is not seen yet. */
ExportFlow(key, info, 0, 0);
}
Expand Down Expand Up @@ -529,32 +529,6 @@ void FlowStatsCollector::UpdateStatsEvent(const FlowKey &key, uint32_t bytes,
request_queue_.Enqueue(req);
}

bool FlowStatsCollector::SetUnderlayPort(FlowExportInfo *info,
FlowLogData &s_flow) {
uint16_t underlay_src_port = 0;
bool exported = false;
if (info->is_flags_set(FlowEntry::LocalFlow)) {
/* Set source_port as 0 for local flows. Source port is calculated by
* vrouter irrespective of whether flow is local or not. So for local
* flows we need to ignore port given by vrouter
*/
s_flow.set_underlay_source_port(0);
exported = true;
} else {
if (info->tunnel_type().GetType() != TunnelType::MPLS_GRE) {
underlay_src_port = info->underlay_source_port();
if (underlay_src_port) {
exported = true;
}
} else {
exported = true;
}
s_flow.set_underlay_source_port(underlay_src_port);
}
info->set_underlay_sport_exported(exported);
return exported;
}

void FlowStatsCollector::SetUnderlayInfo(FlowExportInfo *info,
FlowLogData &s_flow) {
string rid = agent_uve_->agent()->router_id().to_string();
Expand All @@ -567,17 +541,11 @@ void FlowStatsCollector::SetUnderlayInfo(FlowExportInfo *info,
* flows we need to ignore port given by vrouter
*/
s_flow.set_underlay_source_port(0);
info->set_underlay_sport_exported(true);
} else {
s_flow.set_vrouter_ip(rid);
s_flow.set_other_vrouter_ip(info->peer_vrouter());
if (info->tunnel_type().GetType() != TunnelType::MPLS_GRE) {
underlay_src_port = info->underlay_source_port();
if (underlay_src_port) {
info->set_underlay_sport_exported(true);
}
} else {
info->set_underlay_sport_exported(true);
}
s_flow.set_underlay_source_port(underlay_src_port);
}
Expand Down Expand Up @@ -731,28 +699,9 @@ void FlowStatsCollector::ExportFlow(const FlowKey &key,
std::string action_str;
GetFlowSandeshActionParams(info->action_info(), action_str);
s_flow.set_action(action_str);
if (!info->exported()) {
s_flow.set_setup_time(info->setup_time());
info->set_exported(true);
SetUnderlayInfo(info, s_flow);
} else {
/* When the flow is being exported for first time, underlay port
* info is set as part of SetUnderlayInfo. At this point it is possible
* that port is not yet populated to flow-entry because of either
* (i) flow-entry has not got chance to be evaluated by
* flow-stats-collector
* (ii) there is no flow entry in vrouter yet
* (iii) the flow entry in vrouter does not have underlay source port
* populated yet
*/
if (!info->underlay_sport_exported()) {
SetUnderlayPort(info, s_flow);
}
}

if (info->teardown_time()) {
s_flow.set_teardown_time(info->teardown_time());
}
s_flow.set_setup_time(info->setup_time());
SetUnderlayInfo(info, s_flow);
info->set_changed(false);

if (info->is_flags_set(FlowEntry::LocalFlow)) {
/* For local flows we need to send two flow log messages.
Expand Down Expand Up @@ -953,7 +902,12 @@ void FlowStatsCollector::NewFlow(const FlowKey &key,
void FlowStatsCollector::AddFlow(const FlowKey &key, FlowExportInfo info) {
FlowEntryTree::iterator it = flow_tree_.find(key);
if (it != flow_tree_.end()) {
it->second = info;
/* If entry is already present, copy only those fields which are
* inherited from FlowEntry to existing entry. Copy this only if there
* are changes in fields inherited from FlowEntry */
if (!it->second.IsEqual(info)) {
it->second.Copy(info);
}
return;
}

Expand Down
1 change: 0 additions & 1 deletion src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.h
Expand Up @@ -150,7 +150,6 @@ class FlowStatsCollector : public StatsCollector {
void GetFlowSandeshActionParams(const FlowAction &action_info,
std::string &action_str);
void SetUnderlayInfo(FlowExportInfo *info, FlowLogData &s_flow);
bool SetUnderlayPort(FlowExportInfo *info, FlowLogData &s_flow);
void UpdateThreshold(uint32_t new_value);

void UpdateInterVnStats(FlowExportInfo *info,
Expand Down

0 comments on commit 958d5bd

Please sign in to comment.