]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
packetpool: move return stack to pool earlier
authorVictor Julien <victor@inliniac.net>
Sat, 25 May 2019 18:56:27 +0000 (20:56 +0200)
committerVictor Julien <victor@inliniac.net>
Wed, 29 May 2019 13:34:15 +0000 (15:34 +0200)
If waiting for N packets move the return stack to the main
stack every time we take the return stack lock.

Make sure we consider enough packets when setting the pending pool
flush logic. This should at least make sure to have the 9 packets
the flow manager requires per run.

src/tmqh-packetpool.c

index df23bb4f6f45b366bea522d8d0203c9effdc55cd..cea1212197b7ae782606c2c79994b300a21888e8 100644 (file)
@@ -168,37 +168,42 @@ void PacketPoolWait(void)
  *
  *  This function only returns when at least N packets are in our pool.
  *
+ *  If counting in our pool's main stack didn't give us the number we
+ *  are seeking, we check if the return stack is filled and add those
+ *  to our main stack. Then we retry.
+ *
  *  \param n number of packets needed
  */
 void PacketPoolWaitForN(int n)
 {
     PktPool *my_pool = GetThreadPacketPool();
-    Packet *p = NULL;
+    Packet *p, *pp;
 
     while (1) {
-        int i = 0;
         PacketPoolWait();
 
         /* count packets in our stack */
-        p = my_pool->head;
+        int i = 0;
+        pp = p = my_pool->head;
         while (p != NULL) {
             if (++i == n)
                 return;
 
+            pp = p;
             p = p->next;
         }
 
-        /* continue counting in the return stack */
+        /* check return stack, return to our pool and retry counting */
         if (my_pool->return_stack.head != NULL) {
             SCMutexLock(&my_pool->return_stack.mutex);
-            p = my_pool->return_stack.head;
-            while (p != NULL) {
-                if (++i == n) {
-                    SCMutexUnlock(&my_pool->return_stack.mutex);
-                    return;
-                }
-                p = p->next;
+            /* Move all the packets from the locked return stack to the local stack. */
+            if (pp) {
+                pp->next = my_pool->return_stack.head;
+            } else {
+                my_pool->head = my_pool->return_stack.head;
             }
+            my_pool->return_stack.head = NULL;
+            SC_ATOMIC_RESET(my_pool->return_stack.sync_now);
             SCMutexUnlock(&my_pool->return_stack.mutex);
 
         /* or signal that we need packets and wait */
@@ -559,6 +564,12 @@ void TmqhReleasePacketsToPacketPool(PacketQueue *pq)
     return;
 }
 
+/** number of packets to keep reserved when calculating the the pending
+ *  return packets count. This assumes we need at max 10 packets in one
+ *  PacketPoolWaitForN call. The actual number is 9 now, so this has a
+ *  bit of margin. */
+#define RESERVED_PACKETS 10
+
 /**
  *  \brief Set the max_pending_return_packets value
  *
@@ -575,17 +586,23 @@ void TmqhReleasePacketsToPacketPool(PacketQueue *pq)
 void PacketPoolPostRunmodes(void)
 {
     extern intmax_t max_pending_packets;
-
+    intmax_t pending_packets = max_pending_packets;
+    if (pending_packets < RESERVED_PACKETS) {
+        FatalError(SC_ERR_INVALID_ARGUMENT, "'max-pending-packets' setting "
+                "must be at least %d", RESERVED_PACKETS);
+    }
     uint32_t threads = TmThreadCountThreadsByTmmFlags(TM_FLAG_DETECT_TM);
     if (threads == 0)
         return;
-    if (threads > max_pending_packets)
-        return;
 
-    uint32_t packets = (max_pending_packets / threads) - 1;
+    uint32_t packets = (pending_packets / threads) - 1;
     if (packets < max_pending_return_packets)
         max_pending_return_packets = packets;
 
+    /* make sure to have a margin in the return logic */
+    if (max_pending_return_packets >= RESERVED_PACKETS)
+        max_pending_return_packets -= RESERVED_PACKETS;
+
     SCLogDebug("detect threads %u, max packets %u, max_pending_return_packets %u",
-            threads, threads, max_pending_return_packets);
+            threads, packets, max_pending_return_packets);
 }