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.

diff --git a/src/decode.c b/src/decode.c
index 4a574ac064a43e9d828ace22d6d72044cd9927f1..e790f4707850197a68075013a3b1b18fb0b98151 100644 (file)
--- a/src/decode.c
+++ b/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:

diff --git a/src/decode.h b/src/decode.h
index e5d74028ec4c11111a8da08f8b247699f37d15a6..e3c2e61a39bdbe673971a32777d5df0626d5de65 100644 (file)
--- a/src/decode.h
+++ b/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 */
-    TmEcode (*ReleaseData)(ThreadVars *, struct Packet_ *);
+    /** The release function for packet structure and data */
+    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);

diff --git a/src/source-af-packet.c b/src/source-af-packet.c
index 3803427b1509ed7cfcb5aa05df635480b4c04012..b7a7725b945f3f52b5d9e7f10d3200d9283da9fe 100644 (file)
--- a/src/source-af-packet.c
+++ b/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);
 

diff --git a/src/tmqh-packetpool.c b/src/tmqh-packetpool.c
index d8f3cc3c3099e1584d116f7bf2713cd736cc2b0c..0b2b862940ec18befdde82ac139ebf7860ee4406 100644 (file)
--- a/src/tmqh-packetpool.c
+++ b/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) {
-            PACKET_CLEANUP(p->root);
-            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");
-        }
+        p->root->ReleasePacket(p->root);
+        p->root = NULL;
     }
 
     /* 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);
-    if (p->flags & PKT_ALLOC) {
-        PACKET_CLEANUP(p);
-        SCFree(p);
-    } else {
-        PACKET_RECYCLE(p);
-        RingBufferMrMwPut(ringbuffer, (void *)p);
-    }
+    p->ReleasePacket(p);
 
     SCReturn;
 }

diff --git a/src/tmqh-packetpool.h b/src/tmqh-packetpool.h
index b4138324f8f7081018f9a41db91eeeba27f332c5..a248c037a8ec79b853db9a3384bba94e88a0478a 100644 (file)
--- a/src/tmqh-packetpool.h
+++ b/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);