From f2456fc8baeab24e2ef3857d6a971d17daa455ef Mon Sep 17 00:00:00 2001 From: Praveen K V Date: Mon, 7 Sep 2015 10:20:35 +0530 Subject: [PATCH] Run packet-parse routins in ASIO context In an earlier commit f148293fb1f803a7b60faae2159bbde0b7968404, the packet-parse routines were moved out of ASIO context assuming it will improve performance. Further measurements show that, performing packet-parse in ASIO context itself does not affect performance. Hence reverting the change. Change-Id: Ifd6d8a5536b1e20314e2a314a4b57c7e1634ec09 Partial-Bug: #1479295 --- src/vnsw/agent/pkt/flow_proto.h | 12 +++++++ src/vnsw/agent/pkt/pkt_handler.cc | 57 +++++-------------------------- src/vnsw/agent/pkt/pkt_handler.h | 1 + src/vnsw/agent/pkt/proto.cc | 39 +++++++++++---------- src/vnsw/agent/pkt/proto.h | 13 ++++--- 5 files changed, 51 insertions(+), 71 deletions(-) diff --git a/src/vnsw/agent/pkt/flow_proto.h b/src/vnsw/agent/pkt/flow_proto.h index d62f4d97eb8..fa9a27295fc 100644 --- a/src/vnsw/agent/pkt/flow_proto.h +++ b/src/vnsw/agent/pkt/flow_proto.h @@ -27,6 +27,18 @@ class FlowProto : public Proto { void Init() {} void Shutdown() {} + bool Validate(PktInfo *msg) { + if (msg->l3_forwarding && msg->ip == NULL && msg->ip6 == NULL && + msg->type != PktType::MESSAGE) { + FLOW_TRACE(DetailErr, msg->agent_hdr.cmd_param, + msg->agent_hdr.ifindex, msg->agent_hdr.vrf, + msg->ether_type, 0, "Flow : Non-IP packet. Dropping", + msg->l3_forwarding); + return false; + } + return true; + } + FlowHandler *AllocProtoHandler(boost::shared_ptr info, boost::asio::io_service &io) { return new FlowHandler(agent(), info, io); diff --git a/src/vnsw/agent/pkt/pkt_handler.cc b/src/vnsw/agent/pkt/pkt_handler.cc index 0c7e0b84a9b..a60153f2f70 100644 --- a/src/vnsw/agent/pkt/pkt_handler.cc +++ b/src/vnsw/agent/pkt/pkt_handler.cc @@ -188,58 +188,15 @@ PktHandler::PktModuleName PktHandler::ParsePacket(const AgentHdr &hdr, return INVALID; } -static PktHandler::PktModuleName AgentHdrToModule(const AgentHdr &hdr) { - PktHandler::PktModuleName module = PktHandler::INVALID; - - switch (hdr.cmd) { - case AgentHdr::TRAP_RESOLVE: - case AgentHdr::TRAP_ARP: - module = PktHandler::ARP; - break; - - case AgentHdr::TRAP_TOR_CONTROL_PKT: - case AgentHdr::TRAP_L3_PROTOCOLS: - module = PktHandler::DHCP; - break; - - case AgentHdr::TRAP_NEXTHOP: - module = PktHandler::DNS; - break; - case AgentHdr::TRAP_ZERO_TTL: - case AgentHdr::TRAP_DIAG: - module = PktHandler::DIAG; - break; - - case AgentHdr::TRAP_ICMP_ERROR: - case AgentHdr::TRAP_HANDLE_DF: - module = PktHandler::ICMP_ERROR; - break; - - case AgentHdr::TRAP_FLOW_MISS: - case AgentHdr::TRAP_ECMP_RESOLVE: - module = PktHandler::FLOW; - break; - - case AgentHdr::TRAP_L2_PROTOCOL: - case AgentHdr::TRAP_SOURCE_MISMATCH: - assert(0); - break; - - case AgentHdr::TX_SWITCH: - case AgentHdr::TX_ROUTE: - default: - break; - } - - return module; -} - void PktHandler::HandleRcvPkt(const AgentHdr &hdr, const PacketBufferPtr &buff){ boost::shared_ptr pkt_info(new PktInfo(buff)); - PktModuleName mod = AgentHdrToModule(hdr); - pkt_info->agent_hdr = hdr; + PktModuleName mod = INVALID; + mod = ParsePacket(hdr, pkt_info.get(), pkt_info->packet_buffer()->data()); + pkt_info->packet_buffer()->set_module(mod); + pkt_info->module = mod; stats_.PktRcvd(mod); if (mod == INVALID) { + AddPktTrace(mod, PktTrace::In, pkt_info.get()); agent_->stats()->incr_pkt_dropped(); return; } @@ -926,6 +883,10 @@ const AgentHdr &PktInfo::GetAgentHdr() const { return agent_hdr; } +void PktInfo::reset_packet_buffer() { + packet_buffer_.reset(); +} + void PktInfo::AllocPacketBuffer(Agent *agent, uint32_t module, uint16_t len, uint32_t mdata) { packet_buffer_ = agent->pkt()->packet_buffer_manager()->Allocate diff --git a/src/vnsw/agent/pkt/pkt_handler.h b/src/vnsw/agent/pkt/pkt_handler.h index 9fa81ebbf0f..91d854881e4 100644 --- a/src/vnsw/agent/pkt/pkt_handler.h +++ b/src/vnsw/agent/pkt/pkt_handler.h @@ -333,6 +333,7 @@ struct PktInfo { void AllocPacketBuffer(Agent *agent, uint32_t module, uint16_t len, uint32_t mdata); void set_len(uint32_t len); + void reset_packet_buffer(); private: PacketBufferPtr packet_buffer_; diff --git a/src/vnsw/agent/pkt/proto.cc b/src/vnsw/agent/pkt/proto.cc index e03365b1261..d0d76dbbe00 100644 --- a/src/vnsw/agent/pkt/proto.cc +++ b/src/vnsw/agent/pkt/proto.cc @@ -12,7 +12,7 @@ Proto::Proto(Agent *agent, const char *task_name, PktHandler::PktModuleName mod, boost::asio::io_service &io) - : agent_(agent), module_(mod), trace_(true), io_(io), + : agent_(agent), module_(mod), trace_(true), free_buffer_(false), io_(io), work_queue_(TaskScheduler::GetInstance()->GetTaskId(task_name), mod, boost::bind(&Proto::ProcessProto, this, _1)) { agent->pkt()->pkt_handler()->Register(mod, this); @@ -20,7 +20,7 @@ Proto::Proto(Agent *agent, const char *task_name, PktHandler::PktModuleName mod, Proto::Proto(Agent *agent, const char *task_name, PktHandler::PktModuleName mod, boost::asio::io_service &io, uint32_t workq_iterations) : - agent_(agent), module_(mod), trace_(true), io_(io), + agent_(agent), module_(mod), trace_(true), free_buffer_(false), io_(io), work_queue_(TaskScheduler::GetInstance()->GetTaskId(task_name), mod, boost::bind(&Proto::ProcessProto, this, _1), workq_iterations, workq_iterations) { @@ -31,7 +31,25 @@ Proto::~Proto() { work_queue_.Shutdown(); } +void Proto::FreeBuffer(PktInfo *msg) { + msg->pkt = NULL; + msg->eth = NULL; + msg->arp = NULL; + msg->ip = NULL; + msg->transp.tcp = NULL; + msg->data = NULL; + msg->reset_packet_buffer(); +} + bool Proto::Enqueue(boost::shared_ptr msg) { + if (Validate(msg.get()) == false) { + return true; + } + + if (free_buffer_) { + FreeBuffer(msg.get()); + } + return work_queue_.Enqueue(msg); } @@ -40,22 +58,7 @@ bool Proto::Enqueue(boost::shared_ptr msg) { // change based on packet decode bool Proto::ProcessProto(boost::shared_ptr msg_info) { PktHandler *pkt_handler = agent_->pkt()->pkt_handler(); - if (msg_info->module == PktHandler::INVALID) { - msg_info->module = - pkt_handler->ParsePacket(msg_info->agent_hdr, msg_info.get(), - msg_info->packet_buffer()->data()); - if (msg_info->module == PktHandler::INVALID) { - agent_->stats()->incr_pkt_dropped(); - return true; - } - - msg_info->packet_buffer()->set_module(msg_info->module); - if (msg_info->module != module_) { - pkt_handler->Enqueue(msg_info->module, msg_info); - return true; - } - } - + assert(msg_info->module != PktHandler::INVALID); if (trace_) { pkt_handler->AddPktTrace(module_, PktTrace::In, msg_info.get()); } diff --git a/src/vnsw/agent/pkt/proto.h b/src/vnsw/agent/pkt/proto.h index 64dd0de4fe5..44d34dd6f51 100644 --- a/src/vnsw/agent/pkt/proto.h +++ b/src/vnsw/agent/pkt/proto.h @@ -19,18 +19,21 @@ class Proto { boost::asio::io_service &io, uint32_t workq_iterations); virtual ~Proto(); - void set_trace(bool val) { trace_ = val; } - Agent *agent() const { return agent_; } - + virtual bool Validate(PktInfo *msg) { return true; } + virtual bool Enqueue(boost::shared_ptr msg); virtual ProtoHandler *AllocProtoHandler(boost::shared_ptr info, boost::asio::io_service &io) = 0; - virtual bool Enqueue(boost::shared_ptr msg); - bool ProcessProto(boost::shared_ptr msg_info); + void FreeBuffer(PktInfo *msg); + bool ProcessProto(boost::shared_ptr msg_info); + void set_trace(bool val) { trace_ = val; } + void set_free_buffer(bool val) { free_buffer_ = val; } + Agent *agent() const { return agent_; } protected: Agent *agent_; PktHandler::PktModuleName module_; bool trace_; + bool free_buffer_; boost::asio::io_service &io_; private: