From 550d0250e773bffb4f5a3c5e6d8d5d2c7f6ebb34 Mon Sep 17 00:00:00 2001
From: Prabhjot Singh Sethi <prabhjot@juniper.net>
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.

Closes-Bug: 1619433
Change-Id: I36aacf35d83c06e3329531d20b8dd6ad6a828f19
(cherry picked from commit d32524d9e07040cf60aa5f81e44a5a56fcf6596c)
---
 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 5ef57e86cc9..8895cdcb581 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;
 }
@@ -386,17 +388,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;
     }
@@ -412,7 +403,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 08336b9d130..acb42136e10 100644
--- a/src/vnsw/agent/pkt/pkt_handler.cc
+++ b/src/vnsw/agent/pkt/pkt_handler.cc
@@ -293,16 +293,6 @@ void PktHandler::PktModuleEnqueue(PktModuleName mod, const AgentHdr &hdr,
     Enqueue(mod, pkt_info);
 }
 
-PktHandler::PktModuleName PktHandler::ParseFlowPacket(
-                          boost::shared_ptr<PktInfo> 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 7413da95ff7..7df3266b291 100644
--- a/src/vnsw/agent/pkt/pkt_handler.h
+++ b/src/vnsw/agent/pkt/pkt_handler.h
@@ -249,8 +249,6 @@ class PktHandler {
 
     PktModuleName ParsePacket(const AgentHdr &hdr, PktInfo *pkt_info,
                               uint8_t *pkt);
-    PktModuleName ParseFlowPacket(boost::shared_ptr<PktInfo> pkt_info,
-                                  uint8_t *pkt);
     int ParseUserPkt(PktInfo *pkt_info, Interface *intf,
                      PktType::Type &pkt_type, uint8_t *pkt);
     bool ProcessPacket(boost::shared_ptr<PacketBufferEnqueueItem> item);