Replace ReleaseData function on Packet Structure with ReleasePacket.

This commit allows handling Packets allocated by different methods.
The ReleaseData function pointer in the Packet structure is replaced
with ReleasePacket function pointer, which is then always called to
release the memory associated with a Packet.

Currently, the only usage of ReleaseData is in AF Packet. Previously
ReleaseData was only called when it was not NULL. To implement the
same functionality as before in AF Packet, a new function is defined
in AF Packet to first call the AFP specific ReleaseData function and
then releases the Packet structure.

Three new general functions are defined for releasing packets in the
default case:
    1) PacketFree() - To release a packet alloced with SCMalloc()
    2) PacketPoolReturnPacket() - For packets allocated from the Packet Pool.
                                  Calls RECYCLE_PACKET(p)
    3) PacketFreeOrRelease() - Calls PacketFree() or PacketPoolReturnPacket()
                                 based on the PKT_ALLOC flag.

Having these functions removes the need to check the PKT_ALLOC flag
when releasing a packet in most cases, since the ReleasePacket
function encodes how the Packet was allocated. The PKT_ALLOC flag is
still set and is needed when AF Packet releases a packet, since it
replaces the ReleasePacket function pointer with its own function and
then calls PacketFreeOfRelease(), which uses the PKT_ALLOC flag.

src/decode.c

@@ -1,4 +1,4 @@
-/* Copyright (C) 2007-2012 Open Information Security Foundation
+/* Copyright (C) 2007-2013 Open Information Security Foundation
  *
  * You can copy, redistribute or modify this Program under the terms of
  * the GNU General Public License version 2 as published by the Free
@@ -58,6 +58,7 @@
 #include "util-print.h"
 #include "tmqh-packetpool.h"
 #include "util-profiling.h"
+#include "pkt-var.h"
 
 void DecodeTunnel(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
         uint8_t *pkt, uint16_t len, PacketQueue *pq, uint8_t proto)
@@ -77,6 +78,15 @@ void DecodeTunnel(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
     }
 }
 
+/**
+ * \brief Return a malloced packet.
+ */
+void PacketFree(Packet *p)
+{
+    PACKET_CLEANUP(p);
+    SCFree(p);
+}
+
 /**
  * \brief Get a malloced packet.
  *
@@ -91,6 +101,7 @@ Packet *PacketGetFromAlloc(void)
 
     memset(p, 0, SIZE_OF_PACKET);
     PACKET_INITIALIZE(p);
+    p->ReleasePacket = PacketFree;
     p->flags |= PKT_ALLOC;
 
     SCLogDebug("allocated a new packet only using alloc...");
@@ -99,6 +110,17 @@ Packet *PacketGetFromAlloc(void)
     return p;
 }
 
+/**
+ * \brief Return a packet to where it was allocated.
+ */
+void PacketFreeOrRelease(Packet *p)
+{
+    if (p->flags & PKT_ALLOC)
+        PacketFree(p);
+    else
+        PacketPoolReturnPacket(p);
+}
+
 /**
  *  \brief Get a packet. We try to get a packet from the packetpool first, but
  *         if that is empty we alloc a packet that is free'd again after
@@ -182,8 +204,6 @@ inline int PacketCopyData(Packet *p, uint8_t *pktdata, int pktlen)
     return PacketCopyDataOffset(p, 0, pktdata, pktlen);
 }
 
-
-
 /**
  *  \brief Setup a pseudo packet (tunnel)
  *
@@ -248,7 +268,8 @@ Packet *PacketPseudoPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t
  *
  *  \retval p the pseudo packet or NULL if out of memory
  */
-Packet *PacketDefragPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t proto) {
+Packet *PacketDefragPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t proto)
+{
     SCEnter();
 
     /* get us a packet */
@@ -381,7 +402,8 @@ void DecodeRegisterPerfCounters(DecodeThreadVars *dtv, ThreadVars *tv)
  *
  *  \todo IPv6
  */
-void AddressDebugPrint(Address *a) {
+void AddressDebugPrint(Address *a)
+{
     if (a == NULL)
         return;
 
@@ -397,7 +419,8 @@ void AddressDebugPrint(Address *a) {
 }
 
 /** \brief Alloc and setup DecodeThreadVars */
-DecodeThreadVars *DecodeThreadVarsAlloc() {
+DecodeThreadVars *DecodeThreadVarsAlloc()
+{
 
     DecodeThreadVars *dtv = NULL;
 
@@ -412,7 +435,6 @@ DecodeThreadVars *DecodeThreadVarsAlloc() {
     return dtv;
 }
 
-
 /**
  * \brief Set data for Packet and set length when zeo copy is used
  *
@@ -432,7 +454,8 @@ inline int PacketSetData(Packet *p, uint8_t *pktdata, int pktlen)
     return 0;
 }
 
-const char *PktSrcToString(enum PktSrcEnum pkt_src) {
+const char *PktSrcToString(enum PktSrcEnum pkt_src)
+{
     char *pkt_src_str = "<unknown>";
     switch (pkt_src) {
         case PKT_SRC_WIRE:

src/decode.h

@@ -406,8 +406,8 @@ typedef struct Packet_
     int debuglog_flowbits_names_len;
     const char **debuglog_flowbits_names;
 
-    /** The release function for packet data */
+    /** The release function for packet structure and data */
-    TmEcode (*ReleaseData)(ThreadVars *, struct Packet_ *);
+    void (*ReleasePacket)(struct Packet_ *);
 
     /* pkt vars */
     PktVar *pktvar;
@@ -686,7 +686,6 @@ typedef struct DecodeThreadVars_
         (p)->prev = NULL;                       \
         (p)->root = NULL;                       \
         (p)->livedev = NULL;                    \
-        (p)->ReleaseData = NULL;                \
         PACKET_RESET_CHECKSUMS((p));            \
         PACKET_PROFILING_RESET((p));            \
     } while (0)
@@ -777,6 +776,8 @@ Packet *PacketPseudoPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t
 Packet *PacketDefragPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t proto);
 Packet *PacketGetFromQueueOrAlloc(void);
 Packet *PacketGetFromAlloc(void);
+void PacketFree(Packet *p);
+void PacketFreeOrRelease(Packet *p);
 int PacketCopyData(Packet *p, uint8_t *pktdata, int pktlen);
 int PacketSetData(Packet *p, uint8_t *pktdata, int pktlen);
 int PacketCopyDataOffset(Packet *p, int offset, uint8_t *data, int datalen);

src/source-af-packet.c

@@ -1,4 +1,4 @@
-/* Copyright (C) 2011,2012 Open Information Security Foundation
+/* Copyright (C) 2011-2013 Open Information Security Foundation
  *
  * You can copy, redistribute or modify this Program under the terms of
  * the GNU General Public License version 2 as published by the Free
@@ -665,13 +665,12 @@ TmEcode AFPWritePacket(Packet *p)
     return TM_ECODE_OK;
 }
 
-TmEcode AFPReleaseDataFromRing(ThreadVars *t, Packet *p)
+void AFPReleaseDataFromRing(Packet *p)
 {
-    int ret = TM_ECODE_OK;
     /* Need to be in copy mode and need to detect early release
        where Ethernet header could not be set (and pseudo packet) */
     if ((p->afp_v.copy_mode != AFP_COPY_MODE_NONE) && !PKT_IS_PSEUDOPKT(p)) {
-        ret = AFPWritePacket(p);
+        AFPWritePacket(p);
     }
 
     if (AFPDerefSocket(p->afp_v.mpeer) == 0)
@@ -685,7 +684,12 @@ TmEcode AFPReleaseDataFromRing(ThreadVars *t, Packet *p)
 
 cleanup:
     AFPV_CLEANUP(&p->afp_v);
-    return ret;
+}
+
+void AFPReleasePacket(Packet *p)
+{
+    AFPReleaseDataFromRing(p);
+    PacketFreeOrRelease(p);
 }
 
 /**
@@ -788,7 +792,7 @@ int AFPReadFromRing(AFPThreadVars *ptv)
                 SCReturnInt(AFP_FAILURE);
             } else {
                 p->afp_v.relptr = h.raw;
-                p->ReleaseData = AFPReleaseDataFromRing;
+                p->ReleasePacket = AFPReleasePacket;
                 p->afp_v.mpeer = ptv->mpeer;
                 AFPRefSocket(ptv->mpeer);
 

src/tmqh-packetpool.c

@@ -99,8 +99,9 @@ void PacketPoolStorePacket(Packet *p) {
     /* Clear the PKT_ALLOC flag, since that indicates to push back
      * onto the ring buffer. */
     p->flags &= ~PKT_ALLOC;
+    p->ReleasePacket = PacketPoolReturnPacket;
+    PacketPoolReturnPacket(p);
 
-    RingBufferMrMwPut(ringbuffer, (void *)p);
     SCLogDebug("buffersize %u", RingBufferSize(ringbuffer));
 }
 
@@ -115,6 +116,15 @@ Packet *PacketPoolGetPacket(void) {
     return p;
 }
 
+/** \brief Return packet to Packet pool
+ *
+ */
+void PacketPoolReturnPacket(Packet *p)
+{
+    PACKET_RECYCLE(p);
+    RingBufferMrMwPut(ringbuffer, (void *)p);
+}
+
 void PacketPoolInit(intmax_t max_pending_packets) {
     /* pre allocate packets */
     SCLogDebug("preallocating packets... packet size %" PRIuMAX "", (uintmax_t)SIZE_OF_PACKET);
@@ -253,33 +263,14 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p)
 
         FlowDeReference(&p->root->flow);
         /* if p->root uses extended data, free them */
-        if (p->root->ReleaseData) {
-            if (p->root->ReleaseData(t, p->root) == TM_ECODE_FAILED) {
-                SCLogWarning(SC_ERR_INVALID_ACTION,
-                        "Unable to release packet data");
-            }
-        }
         if (p->root->ext_pkt) {
             if (!(p->root->flags & PKT_ZERO_COPY)) {
                 SCFree(p->root->ext_pkt);
             }
             p->root->ext_pkt = NULL;
         }
-        if (p->root->flags & PKT_ALLOC) {
+        p->root->ReleasePacket(p->root);
-            PACKET_CLEANUP(p->root);
+        p->root = NULL;
-            SCFree(p->root);
-            p->root = NULL;
-        } else {
-            PACKET_RECYCLE(p->root);
-            RingBufferMrMwPut(ringbuffer, (void *)p->root);
-        }
-
-    }
-
-    if (p->ReleaseData) {
-        if (p->ReleaseData(t, p) == TM_ECODE_FAILED) {
-            SCLogWarning(SC_ERR_INVALID_ACTION, "Unable to release packet data");
-        }
     }
 
     /* if p uses extended data, free them */
@@ -292,14 +283,7 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p)
 
     PACKET_PROFILING_END(p);
 
-    SCLogDebug("getting rid of tunnel pkt... alloc'd %s (root %p)", p->flags & PKT_ALLOC ? "true" : "false", p->root);
+    p->ReleasePacket(p);
-    if (p->flags & PKT_ALLOC) {
-        PACKET_CLEANUP(p);
-        SCFree(p);
-    } else {
-        PACKET_RECYCLE(p);
-        RingBufferMrMwPut(ringbuffer, (void *)p);
-    }
 
     SCReturn;
 }

src/tmqh-packetpool.h

@@ -1,4 +1,4 @@
-/* Copyright (C) 2007-2010 Open Information Security Foundation
+/* Copyright (C) 2007-2013 Open Information Security Foundation
  *
  * You can copy, redistribute or modify this Program under the terms of
  * the GNU General Public License version 2 as published by the Free
@@ -33,7 +33,7 @@ Packet *PacketPoolGetPacket(void);
 uint16_t PacketPoolSize(void);
 void PacketPoolStorePacket(Packet *);
 void PacketPoolWait(void);
-
+void PacketPoolReturnPacket(Packet *p);
 void PacketPoolInit(intmax_t max_pending_packets);
 void PacketPoolDestroy(void);