From 958d5bd4eeac276d16d5b188009c032b1edcdc94 Mon Sep 17 00:00:00 2001 From: Ashok Singh Date: Wed, 20 Jan 2016 20:28:02 -0800 Subject: [PATCH] Send all fields of flow always in FlowLog messages 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 --- src/sandesh/common/flow.sandesh | 11 --- .../vrouter/flow_stats/flow_export_info.cc | 71 ++++++++++++++++++- .../vrouter/flow_stats/flow_export_info.h | 15 ++-- .../flow_stats/flow_stats_collector.cc | 66 +++-------------- .../vrouter/flow_stats/flow_stats_collector.h | 1 - 5 files changed, 84 insertions(+), 80 deletions(-) diff --git a/src/sandesh/common/flow.sandesh b/src/sandesh/common/flow.sandesh index a192f566432..36fc769a63e 100644 --- a/src/sandesh/common/flow.sandesh +++ b/src/sandesh/common/flow.sandesh @@ -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; 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 bd0dc4d2601..d5e35854368 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_export_info.cc +++ b/src/vnsw/agent/vrouter/flow_stats/flow_export_info.cc @@ -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(); @@ -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(); @@ -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)) { 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 8332bd4035a..5ee1870f35c 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_export_info.h +++ b/src/vnsw/agent/vrouter/flow_stats/flow_export_info.h @@ -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_; } @@ -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 @@ -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_; 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 6193431ae24..fd492ca4740 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc +++ b/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc @@ -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); } @@ -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(); @@ -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); } @@ -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. @@ -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; } 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 750f7603a0c..3d6430ab5e7 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.h +++ b/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.h @@ -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,