From d32524d9e07040cf60aa5f81e44a5a56fcf6596c Mon Sep 17 00:00:00 2001 From: Prabhjot Singh Sethi Date: Fri, 2 Sep 2016 09:05:57 -0700 Subject: [PATCH] Fix inconsistent state in flow handling from pkt Issue: ------ pkt is already parsed and enqueue to the right flow table partition, where while parsing it also calculate source and destination ports based on fat flow configuration from interface, using the flow key it identifies flow table partition But now in processing flow module do parsing of pkt again where fat flow config is available on interface (which was not available earlier) where it will end up changing the port to zero and can cause a different table hash than the flow event queue processing resulting in in-consistent state and crash. Fix: ---- remove code causing re-parsing of pkt. Change-Id: I36aacf35d83c06e3329531d20b8dd6ad6a828f19 Closes-Bug: 1619433 --- src/vnsw/agent/pkt/flow_proto.cc | 14 ++------------ src/vnsw/agent/pkt/pkt_handler.cc | 10 ---------- src/vnsw/agent/pkt/pkt_handler.h | 2 -- 3 files changed, 2 insertions(+), 24 deletions(-) diff --git a/src/vnsw/agent/pkt/flow_proto.cc b/src/vnsw/agent/pkt/flow_proto.cc index 51652e299fd..757078eb0e5 100644 --- a/src/vnsw/agent/pkt/flow_proto.cc +++ b/src/vnsw/agent/pkt/flow_proto.cc @@ -217,6 +217,8 @@ bool FlowProto::Enqueue(PktInfoPtr msg) { if (Validate(msg.get()) == false) { return true; } + + FreeBuffer(msg.get()); EnqueueFlowEvent(new FlowEvent(FlowEvent::VROUTER_FLOW_MSG, msg, NULL, 0)); return true; } @@ -392,17 +394,6 @@ bool FlowProto::FlowEventHandler(FlowEvent *req, FlowTable *table) { switch (req->event()) { case FlowEvent::VROUTER_FLOW_MSG: { - // packet parsing is not done, invoke the same here - uint8_t *pkt = req->pkt_info()->packet_buffer()->data(); - PktInfoPtr info = req->pkt_info(); - PktHandler::PktModuleName mod = - agent()->pkt()->pkt_handler()->ParseFlowPacket(info, pkt); - // if packet wasnt for flow module, it would've got enqueued to the - // correct module in the above call. Nothing else to do. - if (mod != PktHandler::FLOW) { - break; - } - FreeBuffer(info.get()); ProcessProto(req->pkt_info()); break; } @@ -418,7 +409,6 @@ bool FlowProto::FlowEventHandler(FlowEvent *req, FlowTable *table) { FlowEntry *flow = req->flow(); FlowTaskMsg *flow_msg = new FlowTaskMsg(flow); PktInfoPtr pkt_info(new PktInfo(PktHandler::FLOW, flow_msg)); - FreeBuffer(pkt_info.get()); FlowHandler *handler = new FlowHandler(agent(), pkt_info, io_, this, table->table_index()); RunProtoHandler(handler); diff --git a/src/vnsw/agent/pkt/pkt_handler.cc b/src/vnsw/agent/pkt/pkt_handler.cc index 6772df0fc37..de0e03fb4c6 100644 --- a/src/vnsw/agent/pkt/pkt_handler.cc +++ b/src/vnsw/agent/pkt/pkt_handler.cc @@ -294,16 +294,6 @@ void PktHandler::PktModuleEnqueue(PktModuleName mod, const AgentHdr &hdr, Enqueue(mod, pkt_info); } -PktHandler::PktModuleName PktHandler::ParseFlowPacket( - boost::shared_ptr pkt_info, uint8_t *pkt) { - PktModuleName mod = ParsePacket(pkt_info->agent_hdr, pkt_info.get(), pkt); - // In case it is not a flow packet, enqueue it back to the right module - if (mod != FLOW) { - PktModuleEnqueue(mod, pkt_info->agent_hdr, pkt_info, pkt); - } - return mod; -} - // Compute L2/L3 forwarding mode for pacekt. // Forwarding mode is L3 if, // - Packet uses L3 label diff --git a/src/vnsw/agent/pkt/pkt_handler.h b/src/vnsw/agent/pkt/pkt_handler.h index 578350cae1a..24be4aae20b 100644 --- a/src/vnsw/agent/pkt/pkt_handler.h +++ b/src/vnsw/agent/pkt/pkt_handler.h @@ -254,8 +254,6 @@ class PktHandler { PktModuleName ParsePacket(const AgentHdr &hdr, PktInfo *pkt_info, uint8_t *pkt); - PktModuleName ParseFlowPacket(boost::shared_ptr pkt_info, - uint8_t *pkt); int ParseUserPkt(PktInfo *pkt_info, Interface *intf, PktType::Type &pkt_type, uint8_t *pkt); bool ProcessPacket(boost::shared_ptr item);