Cleanup packet recycling code. Fix issues in the packet tunnel/pseudo code.

src/decode-gre.c

@@ -193,12 +193,10 @@ void DecodeGRE(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt, u
         case ETHERNET_TYPE_IP:
             {
                 if (pq != NULL) {
-
+                    Packet *tp = PacketPseudoPktSetup(p, pkt + header_len,
-                    Packet *tp = TunnelPktSetup(tv, dtv, p, pkt + header_len, len - header_len, IPPROTO_IP);
+                            len - header_len, IPPROTO_IP);
                     DecodeTunnel(tv, dtv, tp, tp->pkt, tp->pktlen, pq);
                     PacketEnqueue(pq,tp);
-
-                    SET_TUNNEL_PKT(p);
                 }
                 break;
             }
@@ -206,12 +204,10 @@ void DecodeGRE(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt, u
         case GRE_PROTO_PPP:
             {
                 if (pq != NULL) {
-
+                    Packet *tp = PacketPseudoPktSetup(p, pkt + header_len,
-                    Packet *tp = TunnelPktSetup(tv, dtv, p, pkt + header_len, len - header_len, PPP_OVER_GRE);
+                            len - header_len, PPP_OVER_GRE);
                     DecodeTunnel(tv, dtv, tp, tp->pkt, tp->pktlen, pq);
                     PacketEnqueue(pq,tp);
-
-                    SET_TUNNEL_PKT(p);
                 }
                 break;
             }
@@ -219,12 +215,10 @@ void DecodeGRE(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt, u
         case ETHERNET_TYPE_IPV6:
             {
                 if (pq != NULL) {
-
+                    Packet *tp = PacketPseudoPktSetup(p, pkt + header_len,
-                    Packet *tp = TunnelPktSetup(tv, dtv, p, pkt + header_len, len - header_len, IPPROTO_IPV6);
+                            len - header_len, IPPROTO_IPV6);
                     DecodeTunnel(tv, dtv, tp, tp->pkt, tp->pktlen, pq);
                     PacketEnqueue(pq,tp);
-
-                    SET_TUNNEL_PKT(p);
                 }
                 break;
             }
@@ -232,12 +226,10 @@ void DecodeGRE(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt, u
         case ETHERNET_TYPE_VLAN:
             {
                 if (pq != NULL) {
-
+                    Packet *tp = PacketPseudoPktSetup(p, pkt + header_len,
-                    Packet *tp = TunnelPktSetup(tv, dtv, p, pkt + header_len, len - header_len, VLAN_OVER_GRE);
+                            len - header_len, VLAN_OVER_GRE);
                     DecodeTunnel(tv, dtv, tp, tp->pkt, tp->pktlen, pq);
                     PacketEnqueue(pq,tp);
-
-                    SET_TUNNEL_PKT(p);
                 }
                 break;
             }

src/decode-ipv4.c

@@ -557,20 +557,16 @@ void DecodeIPV4(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt,
         case IPPROTO_IPV6:
             {
                 if (pq != NULL) {
-                    //printf("DecodeIPV4: next layer is IPV6\n");
-                    //printf("DecodeIPV4: we are p %p\n", p);
-
                     /* spawn off tunnel packet */
-                    Packet *tp = TunnelPktSetup(tv, dtv, p, pkt + IPV4_GET_HLEN(p), IPV4_GET_IPLEN(p) - IPV4_GET_HLEN(p), IPV4_GET_IPPROTO(p));
+                    Packet *tp = PacketPseudoPktSetup(p, pkt + IPV4_GET_HLEN(p),
-                    //printf("DecodeIPV4: tunnel is tp %p\n", tp);
+                            IPV4_GET_IPLEN(p) - IPV4_GET_HLEN(p),
+                            IPV4_GET_IPPROTO(p));
 
                     /* send that to the Tunnel decoder */
                     DecodeTunnel(tv, dtv, tp, tp->pkt, tp->pktlen, pq);
+
                     /* add the tp to the packet queue. */
                     PacketEnqueue(pq,tp);
-
-                    /* the current packet is now a tunnel packet */
-                    SET_TUNNEL_PKT(p);
                 }
                 break;
             }
@@ -587,10 +583,6 @@ void DecodeIPV4(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt,
             /* Got re-assembled packet, re-run through decoder. */
             DecodeIPV4(tv, dtv, rp, rp->pkt, rp->pktlen, pq);
             PacketEnqueue(pq, rp);
-
-            /* Not really a tunnel packet, but we're piggybacking that
-             * functionality for now. */
-            SET_TUNNEL_PKT(p);
         }
     }
 

src/decode.c

@@ -24,6 +24,7 @@
  */
 
 #include "suricata-common.h"
+#include "suricata.h"
 #include "decode.h"
 #include "util-debug.h"
 
@@ -44,6 +45,87 @@ void DecodeTunnel(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, uint8_t *pkt
     }
 }
 
+/**
+ *  \brief Get a pseudo packet for tunnels and defrag reassembly. We try to
+ *         get a packet from the packet_q first, but if that is empty we alloc
+ *         a packet that is free'd again after processing.
+ *
+ *  \retval p packet, NULL on error
+ */
+static Packet *PacketGetPseudoPkt (void)
+{
+    Packet *p = NULL;
+
+    SCMutexLock(&packet_q.mutex_q);
+    p = PacketDequeue(&packet_q);
+    SCMutexUnlock(&packet_q.mutex_q);
+
+    if (p == NULL) {
+        /* non fatal, we're just not processing a packet then */
+        p = SCMalloc(sizeof(Packet));
+        if (p == NULL) {
+            SCLogError(SC_ERR_MEM_ALLOC, "SCMalloc failed: %s", strerror(errno));
+            return NULL;
+        }
+
+        PACKET_INITIALIZE(p);
+        p->flags |= PKT_ALLOC;
+
+        SCLogDebug("allocated a new packet...");
+    }
+
+    return p;
+}
+
+/**
+ *  \brief Setup a pseudo packet (tunnel or reassembled frags)
+ *
+ *  \param parent parent packet for this pseudo pkt
+ *  \param pkt raw packet data
+ *  \param len packet data length
+ *  \param proto protocol of the tunneled packet
+ *
+ *  \retval p the pseudo packet or NULL if out of memory
+ */
+Packet *PacketPseudoPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t proto)
+{
+    /* get us a packet */
+    Packet *p = PacketGetPseudoPkt();
+    if (p == NULL) {
+        return NULL;
+    }
+
+    /* set the root ptr to the lowest layer */
+    if (parent->root != NULL)
+        p->root = parent->root;
+    else
+        p->root = parent;
+
+    /* copy packet and set lenght, proto */
+    p->tunnel_proto = proto;
+    p->pktlen = len;
+    memcpy(&p->pkt, pkt, len);
+    p->recursion_level = parent->recursion_level + 1;
+    p->ts.tv_sec = parent->ts.tv_sec;
+    p->ts.tv_usec = parent->ts.tv_usec;
+
+    /* set tunnel flags */
+
+    /* tell new packet it's part of a tunnel */
+    SET_TUNNEL_PKT(p);
+    /* tell parent packet it's part of a tunnel */
+    SET_TUNNEL_PKT(parent);
+
+    /* increment tunnel packet refcnt in the root packet */
+    TUNNEL_INCR_PKT_TPR(p);
+
+    /* disable payload (not packet) inspection on the parent, as the payload
+     * is the packet we will now run through the system separately. We do
+     * check it against the ip/port/other header checks though */
+    DecodeSetNoPayloadInspectionFlag(parent);
+    return p;
+}
+
 void DecodeRegisterPerfCounters(DecodeThreadVars *dtv, ThreadVars *tv)
 {
     /* register counters */

src/decode.h

@@ -400,23 +400,48 @@ typedef struct DecodeThreadVars_
     uint16_t counter_defrag_ipv6_timeouts;
 } DecodeThreadVars;
 
-/* clear key vars so we don't need to call the expensive
+/**
- * memset or bzero
+ *  \brief reset these to -1(indicates that the packet is fresh from the queue)
+ */
+#define PACKET_RESET_CHECKSUMS(p) { \
+    (p)->ip4c.comp_csum = -1; \
+    (p)->tcpc.comp_csum = -1; \
+    (p)->udpc.comp_csum = -1;  \
+    (p)->icmpv4c.comp_csum = -1; \
+    (p)->icmpv6c.comp_csum = -1; \
+}
+
+/**
+ *  \brief Initialize a packet structure for use.
+ */
+#define PACKET_INITIALIZE(p) { \
+    memset((p), 0x00, sizeof(Packet)); \
+    SCMutexInit(&(p)->mutex_rtv_cnt, NULL); \
+    PACKET_RESET_CHECKSUMS((p)); \
+}
+
+/**
+ *  \brief Recycle a packet structure for reuse.
+ *  \todo the mutex destroy & init is necessary because of the memset, reconsider
  */
-#define CLEAR_PACKET(p) { \
+#define PACKET_RECYCLE(p) { \
     if ((p)->pktvar != NULL) { \
         PktVarFree((p)->pktvar); \
     } \
+    SCMutexDestroy(&(p)->mutex_rtv_cnt); \
     memset((p), 0x00, sizeof(Packet)); \
+    SCMutexInit(&(p)->mutex_rtv_cnt, NULL); \
+    PACKET_RESET_CHECKSUMS((p)); \
 }
 
-/* reset these to -1(indicates that the packet is fresh from the queue) */
+/**
-#define RESET_PACKET_CSUMS(p) { \
+ *  \brief Cleanup a packet so that we can free it. No memset needed..
-    (p)->ip4c.comp_csum = -1; \
+ */
-    (p)->tcpc.comp_csum = -1; \
+#define PACKET_CLEANUP(p) { \
-    (p)->udpc.comp_csum = -1;  \
+    if ((p)->pktvar != NULL) { \
-    (p)->icmpv4c.comp_csum = -1; \
+        PktVarFree((p)->pktvar); \
-    (p)->icmpv6c.comp_csum = -1; \
+    } \
+    SCMutexDestroy(&(p)->mutex_rtv_cnt); \
 }
 
 
@@ -462,6 +487,7 @@ typedef struct DecodeThreadVars_
 
 
 void DecodeRegisterPerfCounters(DecodeThreadVars *, ThreadVars *);
+Packet *PacketPseudoPktSetup(Packet *parent, uint8_t *pkt, uint16_t len, uint8_t proto);
 
 /* decoder functions */
 void DecodeEthernet(ThreadVars *, DecodeThreadVars *, Packet *, uint8_t *, uint16_t, PacketQueue *);
@@ -480,9 +506,6 @@ void DecodeUDP(ThreadVars *, DecodeThreadVars *, Packet *, uint8_t *, uint16_t,
 void DecodeGRE(ThreadVars *, DecodeThreadVars *, Packet *, uint8_t *, uint16_t, PacketQueue *);
 void DecodeVLAN(ThreadVars *, DecodeThreadVars *, Packet *, uint8_t *, uint16_t, PacketQueue *);
 
-Packet *SetupPkt (void);
-Packet *TunnelPktSetup(ThreadVars *, DecodeThreadVars *, Packet *, uint8_t *, uint16_t, uint8_t);
-
 void AddressDebugPrint(Address *);
 
 /** \brief Set the No payload inspection Flag for the packet.

src/defrag.c

@@ -534,20 +534,14 @@ Defrag4Reassemble(ThreadVars *tv, DefragContext *dc, DefragTracker *tracker,
 
     /* Allocate a Packet for the reassembled packet.  On failure we
      * SCFree all the resources held by this tracker. */
-    if (tv == NULL) {
+    rp = PacketPseudoPktSetup(p, (uint8_t *)p->ip4h, IPV4_GET_IPLEN(p),
-        /* Unit test. */
-        rp = SetupPkt();
-    }
-    else {
-        /* Not really a tunnel packet, but more of a pseudo packet.
-         * But for the most part we should get the same result. */
-        rp = TunnelPktSetup(tv, NULL, p, (uint8_t *)p->ip4h, IPV4_GET_IPLEN(p),
             IPV4_GET_IPPROTO(p));
-    }
     if (rp == NULL) {
-        SCLogError(SC_ERR_MEM_ALLOC, "Failed to allocate packet for fragmentation re-assembly, dumping fragments.");
+        SCLogError(SC_ERR_MEM_ALLOC, "Failed to allocate packet for "
+                "fragmentation re-assembly, dumping fragments.");
         goto remove_tracker;
     }
+    SCLogDebug("Packet rp %p, p %p, rp->root %p", rp, p, rp->root);
 
     int fragmentable_offset = 0;
     int fragmentable_len = 0;
@@ -652,18 +646,11 @@ Defrag6Reassemble(ThreadVars *tv, DefragContext *dc, DefragTracker *tracker,
 
     /* Allocate a Packet for the reassembled packet.  On failure we
      * SCFree all the resources held by this tracker. */
-    if (tv == NULL) {
+    rp = PacketPseudoPktSetup(p, (uint8_t *)p->ip6h,
-        /* Unit test. */
-        rp = SetupPkt();
-    }
-    else {
-        /* Not really a tunnel packet, but more of a pseudo packet.
-         * But for the most part we should get the same result. */
-        rp = TunnelPktSetup(tv, NULL, p, (uint8_t *)p->ip6h,
             IPV6_GET_PLEN(p) + sizeof(IPV6Hdr), 0);
-    }
     if (rp == NULL) {
-        SCLogError(SC_ERR_MEM_ALLOC, "Failed to allocate packet for fragmentation re-assembly, dumping fragments.");
+        SCLogError(SC_ERR_MEM_ALLOC, "Failed to allocate packet for "
+                "fragmentation re-assembly, dumping fragments.");
         goto remove_tracker;
     }
 

src/packet-queue.c

@@ -27,15 +27,122 @@
 #include "decode.h"
 #include "packet-queue.h"
 #include "threads.h"
+#include "suricata.h"
+#include "util-var.h"
+#include "pkt-var.h"
+
+#ifdef DEBUG
+void PacketQueueValidateDebug(PacketQueue *q) {
+    SCLogDebug("q->len %u, q->top %p, q->bot %p", q->len, q->top, q->bot);
+
+    if (q->len == 0) {
+        BUG_ON(q->top != NULL);
+        BUG_ON(q->bot != NULL);
+    } else if(q->len == 1) {
+        SCLogDebug("q->top->next %p, q->top->prev %p", q->top->next, q->top->prev);
+        SCLogDebug("q->bot->next %p, q->bot->prev %p", q->bot->next, q->bot->prev);
+
+        BUG_ON(q->top != q->bot);
+        BUG_ON(q->top->next != NULL);
+        BUG_ON(q->bot->next != NULL);
+        BUG_ON(q->top->prev != NULL);
+        BUG_ON(q->bot->prev != NULL);
+    } else if (q->len == 2) {
+        SCLogDebug("q->top->next %p, q->top->prev %p", q->top->next, q->top->prev);
+        SCLogDebug("q->bot->next %p, q->bot->prev %p", q->bot->next, q->bot->prev);
+
+        BUG_ON(q->top == NULL);
+        BUG_ON(q->bot == NULL);
+
+        BUG_ON(q->top == q->bot);
+
+        BUG_ON(q->top->prev != NULL);
+        BUG_ON(q->top->next != q->bot);
+
+        BUG_ON(q->bot->prev != q->top);
+        BUG_ON(q->bot->next != NULL);
+    } else {
+        BUG_ON(q->top == NULL);
+        BUG_ON(q->bot == NULL);
+
+        SCLogDebug("q->top->next %p, q->top->prev %p", q->top->next, q->top->prev);
+        SCLogDebug("q->bot->next %p, q->bot->prev %p", q->bot->next, q->bot->prev);
+
+        BUG_ON(q->top == q->bot);
+        BUG_ON(q->top->prev != NULL);
+        BUG_ON(q->bot->next != NULL);
+
+        BUG_ON(q->top->next == q->bot);
+        BUG_ON(q->bot->prev == q->top);
+
+        Packet *p, *pp;
+        for (p = q->top, pp = p->prev; p != NULL; pp = p, p = p->next) {
+            SCLogDebug("p %p, pp %p, p->next %p, p->prev %p", p, pp, p->next, p->prev);
+            BUG_ON(pp != p->prev);
+        }
+
+    }
+}
+
+#define BUGGER_ON(cond) { \
+    if ((cond)) { \
+        PacketQueueValidateDebug(q); \
+    } \
+}
+
+void PacketQueueValidate(PacketQueue *q) {
+    if (q->len == 0) {
+        BUGGER_ON(q->top != NULL);
+        BUGGER_ON(q->bot != NULL);
+    } else if(q->len == 1) {
+        BUGGER_ON(q->top != q->bot);
+        BUGGER_ON(q->top->next != NULL);
+        BUGGER_ON(q->bot->next != NULL);
+        BUGGER_ON(q->top->prev != NULL);
+        BUGGER_ON(q->bot->prev != NULL);
+    } else if (q->len == 2) {
+        BUGGER_ON(q->top == NULL);
+        BUGGER_ON(q->bot == NULL);
+
+        BUGGER_ON(q->top == q->bot);
+
+        BUGGER_ON(q->top->prev != NULL);
+        BUGGER_ON(q->top->next != q->bot);
+
+        BUGGER_ON(q->bot->prev != q->top);
+        BUGGER_ON(q->bot->next != NULL);
+    } else {
+        BUGGER_ON(q->top == NULL);
+        BUGGER_ON(q->bot == NULL);
+
+        BUGGER_ON(q->top == q->bot);
+        BUGGER_ON(q->top->prev != NULL);
+        BUGGER_ON(q->bot->next != NULL);
+
+        BUGGER_ON(q->top->next == q->bot);
+        BUGGER_ON(q->bot->prev == q->top);
+
+        Packet *p, *pp;
+        for (p = q->top, pp = p->prev; p != NULL; pp = p, p = p->next) {
+            BUGGER_ON(pp != p->prev);
+        }
+
+    }
+}
 
 void PacketEnqueue (PacketQueue *q, Packet *p) {
+    //PacketQueueValidateDebug(q);
+
     /* more packets in queue */
     if (q->top != NULL) {
+        p->prev = NULL;
         p->next = q->top;
         q->top->prev = p;
         q->top = p;
     /* only packet */
     } else {
+        p->prev = NULL;
+        p->next = NULL;
         q->top = p;
         q->bot = p;
     }
@@ -44,23 +151,23 @@ void PacketEnqueue (PacketQueue *q, Packet *p) {
     if (q->len > q->dbg_maxlen)
         q->dbg_maxlen = q->len;
 #endif /* DBG_PERF */
+    //PacketQueueValidateDebug(q);
 }
+#endif /* DEBUG */
 
 Packet *PacketDequeue (PacketQueue *q) {
+    Packet *p = NULL;
+
+    //PacketQueueValidateDebug(q);
     /* if the queue is empty there are no packets left. */
     if (q->len == 0) {
         return NULL;
     }
 
-    /* If we are going to get the last packet, set len to 0
-     * before doing anything else (to make the threads to follow
-     * the SCondWait as soon as possible) */
-    q->len--;
-
     /* pull the bottom packet from the queue */
-    Packet *p = q->bot;
+    p = q->bot;
     /* Weird issue: sometimes it looks that two thread arrive
-     * here at the same time so the bot ptr is NULL
+     * here at the same time so the bot ptr is NULL (only on OS X?)
      */
     if (p == NULL) {
         return NULL;
@@ -76,8 +183,9 @@ Packet *PacketDequeue (PacketQueue *q) {
         q->bot = NULL;
     }
 
-    p->next = NULL;
+    q->len--;
-    p->prev = NULL;
+
+    //PacketQueueValidateDebug(q);
     return p;
 }
 

src/suricata.c

@@ -193,91 +193,6 @@ SignalHandlerSetup(int sig, void (*handler)())
 }
 #endif /* OS_WIN32 */
 
-Packet *SetupPktWait (void)
-{
-    Packet *p = NULL;
-    int r = 0;
-    do {
-        r = SCMutexLock(&packet_q.mutex_q);
-        p = PacketDequeue(&packet_q);
-        SCMutexUnlock(&packet_q.mutex_q);
-
-        if (p == NULL) {
-            //TmqDebugList();
-            usleep(1000); /* sleep 1ms */
-
-            /* XXX check for recv'd signals, so
-             * we can exit on signals received */
-        }
-    } while (p == NULL);
-
-    memset(p, 0, sizeof(Packet));
-
-    return p;
-}
-
-Packet *SetupTunnelPkt (void)
-{
-    Packet *p = NULL;
-
-    SCMutexLock(&packet_q.mutex_q);
-    p = PacketDequeue(&packet_q);
-    SCMutexUnlock(&packet_q.mutex_q);
-
-    if (p == NULL) {
-        p = SCMalloc(sizeof(Packet));
-        if (p == NULL) {
-            printf("ERROR: SCMalloc failed: %s\n", strerror(errno));
-            exit(EXIT_FAILURE);
-        }
-
-        memset(p, 0, sizeof(Packet));
-
-        SCMutexInit(&p->mutex_rtv_cnt, NULL);
-
-        p->flags |= PKT_ALLOC;
-
-        SCLogDebug("allocated a new packet...");
-    }
-
-    /* reset the packet csum fields */
-    RESET_PACKET_CSUMS(p);
-
-    return p;
-}
-
-Packet *SetupPkt (void)
-{
-    Packet *p = NULL;
-
-    SCMutexLock(&packet_q.mutex_q);
-    while ((p = PacketDequeue(&packet_q)) == NULL) {
-        SCondWait(&packet_q.cond_q, &packet_q.mutex_q);
-    }
-    SCMutexUnlock(&packet_q.mutex_q);
-/*
-    if (p == NULL) {
-        TmqDebugList();
-
-        p = SCMalloc(sizeof(Packet));
-        if (p == NULL) {
-            printf("ERROR: SCMalloc failed: %s\n", strerror(errno));
-            exit(EXIT_FAILURE);
-        }
-
-        memset(p, 0, sizeof(Packet));
-
-        r = SCMutexInit(&p->mutex_rtv_cnt, NULL);
-
-        SCLogDebug("allocated a new packet...");
-    }
-*/
-    /* reset the packet csum fields */
-    RESET_PACKET_CSUMS(p);
-
-    return p;
-}
-
 void GlobalInits()
 {
     memset(trans_q, 0, sizeof(trans_q));
@@ -295,49 +210,12 @@ void GlobalInits()
         exit(EXIT_FAILURE);
     }
 
-    //SCMutexInit(&mutex_pending, NULL);
-    //SCCondInit(&cond_pending, NULL);
-
     /* initialize packet queues Here! */
     memset(&packet_q,0,sizeof(packet_q));
     SCMutexInit(&packet_q.mutex_q, NULL);
     SCCondInit(&packet_q.cond_q, NULL);
 }
 
-/* \todo dtv not used. */
-Packet *TunnelPktSetup(ThreadVars *t, DecodeThreadVars *dtv, Packet *parent, uint8_t *pkt, uint16_t len, uint8_t proto)
-{
-    //printf("TunnelPktSetup: pkt %p, len %" PRIu32 ", proto %" PRIu32 "\n", pkt, len, proto);
-
-    /* get us a packet */
-    Packet *p = SetupTunnelPkt();
-
-    /* set the root ptr to the lowest layer */
-    if (parent->root != NULL)
-        p->root = parent->root;
-    else
-        p->root = parent;
-
-    /* copy packet and set lenght, proto */
-    p->tunnel_proto = proto;
-    p->pktlen = len;
-    memcpy(&p->pkt, pkt, len);
-    p->recursion_level = parent->recursion_level + 1;
-
-    p->ts.tv_sec = parent->ts.tv_sec;
-    p->ts.tv_usec = parent->ts.tv_usec;
-
-    /* set tunnel flags */
-    SET_TUNNEL_PKT(p);
-    TUNNEL_INCR_PKT_TPR(p);
-
-    /* disable payload (not packet) inspection on the parent, as the payload
-     * is the packet we will now run through the system separately. We do
-     * check it against the ip/port/other header checks though */
-    DecodeSetNoPayloadInspectionFlag(parent);
-    return p;
-}
-
 /* XXX hack: make sure threads can stop the engine by calling this
    function. Purpose: pcap file mode needs to be able to tell the
    engine the file eof is reached. */
@@ -928,8 +806,7 @@ int main(int argc, char **argv)
             printf("ERROR: SCMalloc failed: %s\n", strerror(errno));
             exit(EXIT_FAILURE);
         }
-        memset(p, 0, sizeof(Packet));
+        PACKET_INITIALIZE(p);
-        SCMutexInit(&p->mutex_rtv_cnt, NULL);
 
         PacketEnqueue(&packet_q,p);
     }

src/tmqh-packetpool.c

@@ -20,7 +20,9 @@
  *
  * \author Victor Julien <victor@inliniac.net>
  *
- * Packetpool queue handlers
+ * Packetpool queue handlers. Packet pool is a simple locking FIFO queue.
+ *
+ * \todo see if we can replace this by a lockless queue
  */
 
 #include "suricata.h"
@@ -38,8 +40,6 @@
 
 #include "tmqh-packetpool.h"
 
-extern int max_pending_packets;
-
 void TmqhPacketpoolRegister (void) {
     tmqh_table[TMQH_PACKETPOOL].name = "packetpool";
     tmqh_table[TMQH_PACKETPOOL].InHandler = TmqhInputPacketpool;
@@ -48,68 +48,59 @@ void TmqhPacketpoolRegister (void) {
 
 Packet *TmqhInputPacketpool(ThreadVars *t)
 {
-    /* XXX */
+    Packet *p = NULL;
-    Packet *p = SetupPkt();
+
-#if 0
+    SCMutexLock(&packet_q.mutex_q);
-    SCMutexLock(&mutex_pending);
+    while (p == NULL) {
-    pending++;
+        p = PacketDequeue(&packet_q);
-    //printf("PcapFileCallback: pending %" PRIu32 "\n", pending);
+        if (p == NULL) {
-#ifdef DBG_PERF
+            SCondWait(&packet_q.cond_q, &packet_q.mutex_q);
-    if (pending > dbg_maxpending)
-        dbg_maxpending = pending;
-#endif /* DBG_PERF */
-    SCMutexUnlock(&mutex_pending);
-#endif
-/*
- * Disabled because it can enter a 'wait' state, while
- * keeping the nfq queue locked thus making it impossble
- * to free packets, the exact condition we are waiting
- * for. VJ 09-01-16
- *
-    SCMutexLock(&mutex_pending);
-    if (pending > MAX_PENDING) {
-        SCondWait(&cond_pending, &mutex_pending);
         }
-    SCMutexUnlock(&mutex_pending);
+    }
-*/
+    SCMutexUnlock(&packet_q.mutex_q);
+
+    /* packet is clean */
+
     return p;
 }
 
 void TmqhOutputPacketpool(ThreadVars *t, Packet *p)
 {
+    SCEnter();
+
+    SCLogDebug("Packet %p, p->root %p, alloced %s", p, p->root, p->flags & PKT_ALLOC ? "true" : "false");
+
     PacketQueue *q = &packet_q;
     char proot = 0;
 
-    if (p == NULL)
-        return;
-
     if (IS_TUNNEL_PKT(p)) {
-        //printf("TmqhOutputPacketpool: tunnel packet: %p %s\n", p,p->root ? "upper layer":"root");
+        SCLogDebug("Packet %p is a tunnel packet: %s",
+            p,p->root ? "upper layer" : "tunnel root");
 
         /* get a lock */
         SCMutex *m = p->root ? &p->root->mutex_rtv_cnt : &p->mutex_rtv_cnt;
         SCMutexLock(m);
 
         if (IS_TUNNEL_ROOT_PKT(p)) {
-            //printf("TmqhOutputPacketpool: IS_TUNNEL_ROOT_PKT\n");
+            SCLogDebug("IS_TUNNEL_ROOT_PKT == TRUE");
             if (TUNNEL_PKT_TPR(p) == 0) {
-                //printf("TmqhOutputPacketpool: TUNNEL_PKT_TPR(p) == 0\n");
+                SCLogDebug("TUNNEL_PKT_TPR(p) == 0, no more tunnel packet depending on this root");
                 /* if this packet is the root and there are no
                  * more tunnel packets, enqueue it */
 
                 /* fall through */
             } else {
-                //printf("TmqhOutputPacketpool: TUNNEL_PKT_TPR(p) > 0\n");
+                SCLogDebug("tunnel root Packet %p: TUNNEL_PKT_TPR(p) > 0, packets are still depending on this root, setting p->tunnel_verdicted == 1", p);
                 /* if this is the root and there are more tunnel
                  * packets, don't add this. It's still referenced
                  * by the tunnel packets, and we will enqueue it
                  * when we handle them */
                 p->tunnel_verdicted = 1;
                 SCMutexUnlock(m);
-                return;
+                SCReturn;
             }
         } else {
-            //printf("TmqhOutputPacketpool: NOT IS_TUNNEL_ROOT_PKT\n");
+            SCLogDebug("NOT IS_TUNNEL_ROOT_PKT, so tunnel pkt");
 
             /* the p->root != NULL here seems unnecessary: IS_TUNNEL_PKT checks
              * that p->tunnel_pkt == 1, IS_TUNNEL_ROOT_PKT checks that +
@@ -119,35 +110,41 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *p)
             if (p->root != NULL && p->root->tunnel_verdicted == 1 &&
                     TUNNEL_PKT_TPR(p) == 1)
             {
-                //printf("TmqhOutputPacketpool: p->root->tunnel_verdicted == 1 && TUNNEL_PKT_TPR(p) == 1\n");
+                SCLogDebug("p->root->tunnel_verdicted == 1 && TUNNEL_PKT_TPR(p) == 1");
                 /* the root is ready and we are the last tunnel packet,
                  * lets enqueue them both. */
                 TUNNEL_DECR_PKT_TPR_NOLOCK(p);
 
                 /* handle the root */
-                //printf("TmqhOutputPacketpool: calling PacketEnqueue for root pkt, p->root %p (%p)\n", p->root, p);
+                SCLogDebug("calling PacketEnqueue for root pkt, p->root %p (tunnel packet %p)", p->root, p);
                 proot = 1;
 
                 /* fall through */
             } else {
-                //printf("TmqhOutputPacketpool: NOT p->root->tunnel_verdicted == 1 && TUNNEL_PKT_TPR(p) == 1 (%" PRIu32 ")\n", TUNNEL_PKT_TPR(p));
+                /* root not ready yet, so get rid of the tunnel pkt only */
+
+                SCLogDebug("NOT p->root->tunnel_verdicted == 1 (%d) && TUNNEL_PKT_TPR(p) == 1 (%" PRIu32 ")", p->root->tunnel_verdicted, TUNNEL_PKT_TPR(p));
                 TUNNEL_DECR_PKT_TPR_NOLOCK(p);
 
                  /* fall through */
             }
         }
         SCMutexUnlock(m);
-        //printf("TmqhOutputPacketpool: tunnel stuff done, move on\n");
+        SCLogDebug("tunnel stuff done, move on (proot %d)", proot);
     }
 
     FlowDecrUsecnt(t,p);
 
-    if (proot && p->root != NULL) {
+    /* we're done with the tunnel root now as well */
+    if (proot == 1) {
+        SCLogDebug("getting rid of root pkt... alloc'd %s", p->root->flags & PKT_ALLOC ? "true" : "false");
         if (p->root->flags & PKT_ALLOC) {
-            SCMutexDestroy(&p->root->mutex_rtv_cnt);
+            PACKET_CLEANUP(p->root);
-            CLEAR_PACKET(p->root);
             SCFree(p->root);
+            p->root = NULL;
         } else {
+            PACKET_RECYCLE(p->root);
+
             SCMutexLock(&q->mutex_q);
             PacketEnqueue(q, p->root);
             SCCondSignal(&q->cond_q);
@@ -155,17 +152,20 @@ void TmqhOutputPacketpool(ThreadVars *t, Packet *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) {
-        SCMutexDestroy(&p->mutex_rtv_cnt);
+        PACKET_CLEANUP(p);
-        CLEAR_PACKET(p);
         SCFree(p);
     } else {
-        CLEAR_PACKET(p);
+        PACKET_RECYCLE(p);
+
         SCMutexLock(&q->mutex_q);
         PacketEnqueue(q, p);
         SCCondSignal(&q->cond_q);
         SCMutexUnlock(&q->mutex_q);
     }
+
+    SCReturn;
 }
 
 /**

src/tmqh-simple.c

@@ -45,6 +45,7 @@ Packet *TmqhInputSimple(ThreadVars *t)
     PacketQueue *q = &trans_q[t->inq->id];
 
     SCMutexLock(&q->mutex_q);
+
     if (q->len == 0) {
         /* if we have no packets in queue, wait... */
         SCondWait(&q->cond_q, &q->mutex_q);
@@ -66,6 +67,8 @@ Packet *TmqhInputSimple(ThreadVars *t)
 
 void TmqhOutputSimple(ThreadVars *t, Packet *p)
 {
+    SCLogDebug("Packet %p, p->root %p, alloced %s", p, p->root, p->flags & PKT_ALLOC ? "true":"false");
+
     PacketQueue *q = &trans_q[t->outq->id];
 
     SCMutexLock(&q->mutex_q);