From f568ba9a1c4cb30e7c652840db3e8685a271aaed Mon Sep 17 00:00:00 2001 From: Naveen N Date: Fri, 19 Feb 2016 12:21:01 +0530 Subject: [PATCH] * Calculate fat flow protocol in packet processing context Calculate port in packet processing context such that packet gets enqueued to right partition Store flow partition in stats collector context and use that for enqueueing delete request. Closes-bug:#1542268 Change-Id: I4cd4a1f250581dcfa09e9b23108e4a9736850488 --- src/vnsw/agent/pkt/flow_event.h | 32 +++++----- src/vnsw/agent/pkt/flow_proto.cc | 9 +-- src/vnsw/agent/pkt/flow_proto.h | 3 +- src/vnsw/agent/pkt/pkt_flow_info.cc | 47 -------------- src/vnsw/agent/pkt/pkt_flow_info.h | 1 - src/vnsw/agent/pkt/pkt_handler.cc | 62 +++++++++++++++++++ src/vnsw/agent/pkt/pkt_handler.h | 1 + src/vnsw/agent/pkt/test/test_flow_eviction.cc | 7 ++- .../vrouter/flow_stats/flow_export_info.cc | 7 ++- .../vrouter/flow_stats/flow_export_info.h | 4 ++ .../flow_stats/flow_stats_collector.cc | 3 +- 11 files changed, 104 insertions(+), 72 deletions(-) diff --git a/src/vnsw/agent/pkt/flow_event.h b/src/vnsw/agent/pkt/flow_event.h index ecd8e7894a1..b41fc23fdf9 100644 --- a/src/vnsw/agent/pkt/flow_event.h +++ b/src/vnsw/agent/pkt/flow_event.h @@ -62,68 +62,71 @@ class FlowEvent { event_(INVALID), flow_(NULL), pkt_info_(), db_entry_(NULL), gen_id_(0), del_rev_flow_(false), flow_handle_(FlowEntry::kInvalidFlowHandle), ksync_entry_(NULL), - ksync_event_(), ksync_error_(0) { + ksync_event_(), ksync_error_(0), table_index_(0) { } FlowEvent(Event event) : event_(event), flow_(NULL), pkt_info_(), db_entry_(NULL), gen_id_(0), del_rev_flow_(false), ksync_entry_(NULL), ksync_event_(), - ksync_error_(0) { + ksync_error_(0), table_index_(0) { } FlowEvent(Event event, FlowEntry *flow) : event_(event), flow_(flow), pkt_info_(), db_entry_(NULL), gen_id_(0), del_rev_flow_(false), flow_handle_(FlowEntry::kInvalidFlowHandle), ksync_entry_(NULL), - ksync_event_(), ksync_error_(0) { + ksync_event_(), ksync_error_(0), table_index_(0){ } FlowEvent(Event event, FlowEntry *flow, uint32_t flow_handle) : event_(event), flow_(flow), pkt_info_(), db_entry_(NULL), gen_id_(0), del_rev_flow_(false), flow_handle_(flow_handle), - ksync_entry_(NULL), ksync_event_(), ksync_error_(0) { + ksync_entry_(NULL), ksync_event_(), ksync_error_(0), + table_index_(0) { } FlowEvent(Event event, FlowEntry *flow, const DBEntry *db_entry) : event_(event), flow_(flow), pkt_info_(), db_entry_(db_entry), gen_id_(0), del_rev_flow_(false), flow_handle_(FlowEntry::kInvalidFlowHandle), ksync_entry_(NULL), - ksync_event_(), ksync_error_(0) { + ksync_event_(), ksync_error_(0), table_index_(0) { } FlowEvent(Event event, const DBEntry *db_entry, uint32_t gen_id) : event_(event), flow_(NULL), pkt_info_(), db_entry_(db_entry), gen_id_(gen_id), del_rev_flow_(false), flow_handle_(FlowEntry::kInvalidFlowHandle), ksync_entry_(NULL), - ksync_event_(), ksync_error_(0) { + ksync_event_(), ksync_error_(0), table_index_(0) { } - FlowEvent(Event event, const FlowKey &key, bool del_rev_flow) : + FlowEvent(Event event, const FlowKey &key, bool del_rev_flow, + uint32_t table_index) : event_(event), flow_(NULL), pkt_info_(), db_entry_(NULL), gen_id_(0), flow_key_(key), del_rev_flow_(del_rev_flow), flow_handle_(FlowEntry::kInvalidFlowHandle), ksync_entry_(NULL), - ksync_event_(), ksync_error_(0) { + ksync_event_(), ksync_error_(0), table_index_(table_index) { } FlowEvent(Event event, const FlowKey &key, uint32_t flow_handle) : event_(event), flow_(NULL), pkt_info_(), db_entry_(NULL), gen_id_(0), flow_key_(key), del_rev_flow_(false), flow_handle_(flow_handle), ksync_entry_(NULL), ksync_event_(), - ksync_error_(0) { + ksync_error_(0), table_index_(0) { } FlowEvent(Event event, PktInfoPtr pkt_info) : event_(event), flow_(NULL), pkt_info_(pkt_info), db_entry_(NULL), gen_id_(0), flow_key_(), del_rev_flow_(), flow_handle_(FlowEntry::kInvalidFlowHandle), - ksync_entry_(NULL), ksync_event_(), ksync_error_(0) { + ksync_entry_(NULL), ksync_event_(), ksync_error_(0), table_index_(0) { } FlowEvent(KSyncEntry *entry, KSyncEntry::KSyncEvent event) : event_(KSYNC_EVENT), flow_(NULL), pkt_info_(), db_entry_(NULL), gen_id_(0), flow_key_(), del_rev_flow_(), flow_handle_(FlowEntry::kInvalidFlowHandle), - ksync_entry_(entry), ksync_event_(event), ksync_error_(0) { + ksync_entry_(entry), ksync_event_(event), ksync_error_(0), + table_index_(0) { } FlowEvent(KSyncEntry *entry, uint32_t flow_handle) : @@ -137,7 +140,7 @@ class FlowEvent { event_(KSYNC_VROUTER_ERROR), flow_(NULL), pkt_info_(), db_entry_(NULL), gen_id_(0), flow_key_(), del_rev_flow_(), flow_handle_(FlowEntry::kInvalidFlowHandle), ksync_entry_(entry), - ksync_event_(), ksync_error_(0) { + ksync_event_(), ksync_error_(0), table_index_(0) { } FlowEvent(const FlowEvent &rhs) : @@ -146,7 +149,7 @@ class FlowEvent { flow_key_(rhs.flow_key_), del_rev_flow_(rhs.del_rev_flow_), flow_handle_(rhs.flow_handle_), ksync_entry_(rhs.ksync_entry_), ksync_event_(rhs.ksync_event_), - ksync_error_(0) { + ksync_error_(0), table_index_(rhs.table_index_) { } virtual ~FlowEvent() { } @@ -167,7 +170,7 @@ class FlowEvent { void set_ksync_error(int error) { ksync_error_ = error; } int ksync_error() const { return ksync_error_; } - + uint32_t table_index() const { return table_index_;} private: Event event_; FlowEntryPtr flow_; @@ -180,6 +183,7 @@ class FlowEvent { KSyncEntry::KSyncEntryPtr ksync_entry_; KSyncEntry::KSyncEvent ksync_event_; int ksync_error_; + uint32_t table_index_; }; #endif // __AGENT_FLOW_EVENT_H__ diff --git a/src/vnsw/agent/pkt/flow_proto.cc b/src/vnsw/agent/pkt/flow_proto.cc index abd181eb8b8..d2676b38a75 100644 --- a/src/vnsw/agent/pkt/flow_proto.cc +++ b/src/vnsw/agent/pkt/flow_proto.cc @@ -275,7 +275,7 @@ bool FlowProto::FlowEventHandler(FlowEvent *req, FlowTable *table) { } case FlowEvent::DELETE_FLOW: { - FlowTable *table = GetFlowTable(req->get_flow_key()); + FlowTable *table = GetTable(req->table_index()); table->Delete(req->get_flow_key(), req->get_del_rev_flow()); break; } @@ -378,9 +378,10 @@ bool FlowProto::FlowEventHandler(FlowEvent *req, FlowTable *table) { return true; } -void FlowProto::DeleteFlowRequest(const FlowKey &flow_key, bool del_rev_flow) { - EnqueueFlowEvent(new FlowEvent(FlowEvent::DELETE_FLOW, flow_key, - del_rev_flow)); +void FlowProto::DeleteFlowRequest(const FlowKey &flow_key, bool del_rev_flow, + uint32_t table_index) { + EnqueueFlowEvent(new FlowEvent(FlowEvent::DELETE_FLOW, flow_key, del_rev_flow, + table_index)); return; } diff --git a/src/vnsw/agent/pkt/flow_proto.h b/src/vnsw/agent/pkt/flow_proto.h index b6c205e0ba5..fee58a83f86 100644 --- a/src/vnsw/agent/pkt/flow_proto.h +++ b/src/vnsw/agent/pkt/flow_proto.h @@ -64,7 +64,8 @@ class FlowProto : public Proto { void EnqueueEvent(FlowEvent *event, FlowTable *table); void EnqueueFlowEvent(FlowEvent *event); - void DeleteFlowRequest(const FlowKey &flow_key, bool del_rev_flow); + void DeleteFlowRequest(const FlowKey &flow_key, bool del_rev_flow, + uint32_t table_index); void EvictFlowRequest(FlowEntry *flow, uint32_t flow_handle); void RetryIndexAcquireRequest(FlowEntry *flow, uint32_t flow_handle); void CreateAuditEntry(const FlowKey &key, uint32_t flow_handle); diff --git a/src/vnsw/agent/pkt/pkt_flow_info.cc b/src/vnsw/agent/pkt/pkt_flow_info.cc index a0cd9115ff6..fed6cfe49d8 100644 --- a/src/vnsw/agent/pkt/pkt_flow_info.cc +++ b/src/vnsw/agent/pkt/pkt_flow_info.cc @@ -1121,7 +1121,6 @@ void PktFlowInfo::IngressProcess(const PktInfo *pkt, PktControlInfo *in, return; } - CalculatePort(pkt, in->intf_); // We always expect route for source-ip for ingress flows. // If route not present, return from here so that a short flow is added UpdateRoute(&in->rt_, in->vrf_, pkt->ip_saddr, pkt->smac, @@ -1271,7 +1270,6 @@ void PktFlowInfo::EgressProcess(const PktInfo *pkt, PktControlInfo *in, } if (out->intf_ && out->intf_->type() == Interface::VM_INTERFACE) { - CalculatePort(pkt, out->intf_); const VmInterface *vm_intf = static_cast(out->intf_); if (vm_intf->IsFloatingIp(pkt->ip_daddr)) { pkt->l3_forwarding = true; @@ -1475,51 +1473,6 @@ bool PktFlowInfo::Process(const PktInfo *pkt, PktControlInfo *in, return true; } -//Mask source port or destination port if a port has fat flow -//configuration. If both source port and destination port are -//present in configuration then the lowest of the port takes -//priority -void PktFlowInfo::CalculatePort(const PktInfo *cpkt, const Interface *in) { - const VmInterface *intf = NULL; - PktInfo *pkt = const_cast(cpkt); - - if (in == NULL || in->type() != Interface::VM_INTERFACE) { - return; - } - - intf = static_cast(in); - if (intf->fat_flow_list().list_.size() == 0) { - return; - } - - uint16_t sport = pkt->sport; - if (pkt->ip_proto == IPPROTO_ICMP) { - sport = 0; - } - if (pkt->sport < pkt->dport) { - if (intf->IsFatFlow(pkt->ip_proto, sport)) { - pkt->dport = 0; - return; - } - - if (intf->IsFatFlow(pkt->ip_proto, pkt->dport)) { - pkt->sport = 0; - return; - } - return; - } - - if (intf->IsFatFlow(pkt->ip_proto, pkt->dport)) { - pkt->sport = 0; - return; - } - - if (intf->IsFatFlow(pkt->ip_proto, sport)) { - pkt->dport = 0; - return; - } -} - // A flow can mean that traffic is seen on an interface. The path preference // module can potentially be interested in this event. Check and generate // traffic seen event diff --git a/src/vnsw/agent/pkt/pkt_flow_info.h b/src/vnsw/agent/pkt/pkt_flow_info.h index a8cfe68db4d..b89a0eb3253 100644 --- a/src/vnsw/agent/pkt/pkt_flow_info.h +++ b/src/vnsw/agent/pkt/pkt_flow_info.h @@ -115,7 +115,6 @@ class PktFlowInfo { const IpAddress &addr, const MacAddress &mac, FlowRouteRefMap &ref_map); uint8_t RouteToPrefixLen(const AgentRoute *route); - void CalculatePort(const PktInfo *p, const Interface *intf); bool RouteAllowNatLookupCommon(const AgentRoute *rt, uint32_t sport, uint32_t dport, diff --git a/src/vnsw/agent/pkt/pkt_handler.cc b/src/vnsw/agent/pkt/pkt_handler.cc index 68f167ca757..1d2f801a42e 100644 --- a/src/vnsw/agent/pkt/pkt_handler.cc +++ b/src/vnsw/agent/pkt/pkt_handler.cc @@ -74,6 +74,64 @@ void PktHandler::Send(const AgentHdr &hdr, const PacketBufferPtr &buff) { return; } +void PktHandler::CalculatePort(PktInfo *pkt) { + const Interface *in = NULL; + const VmInterface *intf = NULL; + + const NextHop *nh = + agent()->nexthop_table()->FindNextHop(pkt->agent_hdr.nh); + if (!nh) { + return; + } + + if (nh->GetType() == NextHop::INTERFACE) { + const InterfaceNH *intf_nh = static_cast(nh); + in = intf_nh->GetInterface(); + } else if (nh->GetType() == NextHop::VLAN) { + const VlanNH *vlan_nh = static_cast(nh); + in = vlan_nh->GetInterface(); + } + + if (in) { + intf = dynamic_cast(in); + } + + if (!intf) { + return; + } + + if (intf->fat_flow_list().list_.size() == 0) { + return; + } + + uint16_t sport = pkt->sport; + if (pkt->ip_proto == IPPROTO_ICMP) { + sport = 0; + } + if (pkt->sport < pkt->dport) { + if (intf->IsFatFlow(pkt->ip_proto, sport)) { + pkt->dport = 0; + return; + } + + if (intf->IsFatFlow(pkt->ip_proto, pkt->dport)) { + pkt->sport = 0; + return; + } + return; + } + + if (intf->IsFatFlow(pkt->ip_proto, pkt->dport)) { + pkt->sport = 0; + return; + } + + if (intf->IsFatFlow(pkt->ip_proto, sport)) { + pkt->dport = 0; + return; + } +} + // Process the packet received from tap interface PktHandler::PktModuleName PktHandler::ParsePacket(const AgentHdr &hdr, PktInfo *pkt_info, @@ -127,6 +185,10 @@ PktHandler::PktModuleName PktHandler::ParsePacket(const AgentHdr &hdr, return ARP; } + if (IsFlowPacket(pkt_info)) { + CalculatePort(pkt_info); + } + // Packets needing flow if (IsFlowPacket(pkt_info)) { if ((pkt_info->ip && pkt_info->family == Address::INET) || diff --git a/src/vnsw/agent/pkt/pkt_handler.h b/src/vnsw/agent/pkt/pkt_handler.h index fa10963284a..abbaedf3d44 100644 --- a/src/vnsw/agent/pkt/pkt_handler.h +++ b/src/vnsw/agent/pkt/pkt_handler.h @@ -266,6 +266,7 @@ class PktHandler { PktModule *pkt_module() const { return pkt_module_; } void Enqueue(PktModuleName module, boost::shared_ptr pkt_info); bool IsFlowPacket(PktInfo *pkt_info); + void CalculatePort(PktInfo *pkt_info); private: int ParseEthernetHeader(PktInfo *pkt_info, uint8_t *pkt); diff --git a/src/vnsw/agent/pkt/test/test_flow_eviction.cc b/src/vnsw/agent/pkt/test/test_flow_eviction.cc index ec27775bcea..0009f7f1978 100644 --- a/src/vnsw/agent/pkt/test/test_flow_eviction.cc +++ b/src/vnsw/agent/pkt/test/test_flow_eviction.cc @@ -426,7 +426,7 @@ TEST_F(FlowEvictionTest, Delete_Evicted_Flow_1) { EXPECT_TRUE(FlowGet(vrf_id, remote_vm1_ip, vm1_ip, 2, 0, 0, vif0->flow_key_nh()->id()) == false); - flow_proto_->DeleteFlowRequest(key, true); + flow_proto_->DeleteFlowRequest(key, true, flow->flow_table()->table_index()); client->WaitForIdle(); // New flow should be present @@ -450,7 +450,8 @@ TEST_F(FlowEvictionTest, Delete_Evicted_Flow_2) { EXPECT_TRUE(flow != NULL); // Generate delete request followed by flow-evict - flow_proto_->DeleteFlowRequest(flow->key(), true); + flow_proto_->DeleteFlowRequest(flow->key(), true, + flow->flow_table()->table_index()); // Generate a flow that evicts flow created above TxIpMplsPacket(eth->id(), remote_compute, router_id_, vif0->label(), remote_vm1_ip, vm1_ip, 1, 1); @@ -483,7 +484,7 @@ TEST_F(FlowEvictionTest, Delete_Index_Unassigned_Flow_1) { EXPECT_TRUE(flow != NULL); FlowKey key = flow->key(); - flow_proto_->DeleteFlowRequest(key, true); + flow_proto_->DeleteFlowRequest(key, true, flow->flow_table()->table_index()); client->WaitForIdle(); ksync_sock_->DisableReceiveQueue(false); 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 d8f21a4279e..aa128057aca 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_export_info.cc +++ b/src/vnsw/agent/vrouter/flow_stats/flow_export_info.cc @@ -11,7 +11,7 @@ FlowExportInfo::FlowExportInfo() : vm_cfg_name_(), peer_vrouter_(), tunnel_type_(TunnelType::INVALID), underlay_source_port_(0), changed_(false), fip_(0), fip_vmi_(AgentKey::ADD_DEL_CHANGE, nil_uuid(), ""), tcp_flags_(0), - delete_enqueued_(false) { + delete_enqueued_(false), flow_partition_(0) { key_.Reset(); drop_reason_ = FlowEntry::DropReasonStr(FlowEntry::DROP_UNKNOWN); interface_uuid_ = boost::uuids::nil_uuid(); @@ -41,6 +41,11 @@ FlowExportInfo::FlowExportInfo(FlowEntry *fe, uint64_t setup_time) : interface_uuid_ = boost::uuids::nil_uuid(); } drop_reason_ = FlowEntry::DropReasonStr(fe->data().drop_reason); + if (fe->flow_table()) { + flow_partition_ = fe->flow_table()->table_index(); + } else { + flow_partition_ = 0; + } } /* This API compares only fields which are copied from FlowEntry. Fields which 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 6b575ad0606..d3212a125bc 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_export_info.h +++ b/src/vnsw/agent/vrouter/flow_stats/flow_export_info.h @@ -64,6 +64,9 @@ class FlowExportInfo { void Copy(const FlowExportInfo &rhs); void set_delete_enqueued(bool value) { delete_enqueued_ = value; } bool delete_enqueued() const { return delete_enqueued_; } + uint32_t flow_partition() const { + return flow_partition_; + } private: boost::uuids::uuid flow_uuid_; boost::uuids::uuid egress_uuid_; // used/applicable only for local flows @@ -96,6 +99,7 @@ class FlowExportInfo { std::string drop_reason_; uint16_t tcp_flags_; bool delete_enqueued_; + uint32_t flow_partition_; }; #endif // __AGENT_FLOW_EXPORT_INFO_H__ 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 9d2efa027dd..b53f206b8c1 100644 --- a/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc +++ b/src/vnsw/agent/vrouter/flow_stats/flow_stats_collector.cc @@ -288,7 +288,8 @@ void FlowStatsCollector::UpdateStatsAndExportFlow(FlowExportInfo *info, void FlowStatsCollector::FlowDeleteEnqueue(FlowExportInfo *info) { agent_uve_->agent()->pkt()->get_flow_proto()->DeleteFlowRequest(info->key(), - true); + true, + info->flow_partition()); info->set_delete_enqueued(true); FlowExportInfo *rev_info = FindFlowExportInfo(info->rev_flow_uuid()); if (rev_info) {