dpdk: move to native queue-sized mempools

As a legacy from Suricata 7, mempool size was kept even internally
as a global size. Since in Suricata 8, mempool allocation moved to
per queue model, it makes sense to calculate the correct size right
when loading the configuration.

The previous implementation misscalculated in some scenarios the size
of the mempool which prevented Suricata starting successfully.
This happened, e.g. when there were 32k RX descriptors and 64 TX descs.
The resulting mempool was incorrectly deduced as 32767 (2^n-1), which
didn't even hold all RX descriptors. Instead, it should result to 65535.

Ticket: 8370

Author: Lukas Sismis

qa/live/dpdk/dpdk-testsuite.sh

@@ -36,31 +36,31 @@ run_test "IDS: auto mempool, auto cache (1 thread)" \
     --interface-cfg-set net_null0.threads=1 \
     --interface-cfg-set net_null0.mempool-size=auto \
     --interface-cfg-set net_null0.mempool-cache-size=auto \
-    --suricata-log-check-grep "1 packet mempools of size 31, cache size 1" \
+    --suricata-log-check-grep "1 packet mempools of size 63, cache size 21" \
     --expect-start \
     "$IDS_YAML"
 
 run_test "IDS: auto mempool, auto cache (2 threads)" \
     --interface-cfg-set net_null0.threads=2 \
     --interface-cfg-set net_null0.mempool-size=auto \
     --interface-cfg-set net_null0.mempool-cache-size=auto \
-    --suricata-log-check-grep "2 packet mempools of size 31, cache size 1" \
+    --suricata-log-check-grep "2 packet mempools of size 63, cache size 21" \
     --expect-start \
     "$IDS_YAML"
 
 run_test "IDS: auto mempool, static cache=1 (1 thread)" \
     --interface-cfg-set net_null0.threads=1 \
     --interface-cfg-set net_null0.mempool-size=auto \
     --interface-cfg-set net_null0.mempool-cache-size=1 \
-    --suricata-log-check-grep "1 packet mempools of size 31, cache size 1" \
+    --suricata-log-check-grep "1 packet mempools of size 63, cache size 1" \
     --expect-start \
     "$IDS_YAML"
 
 run_test "IDS: auto mempool, static cache=1 (2 threads)" \
     --interface-cfg-set net_null0.threads=2 \
     --interface-cfg-set net_null0.mempool-size=auto \
     --interface-cfg-set net_null0.mempool-cache-size=1 \
-    --suricata-log-check-grep "2 packet mempools of size 31, cache size 1" \
+    --suricata-log-check-grep "2 packet mempools of size 63, cache size 1" \
     --expect-start \
     "$IDS_YAML"
 
@@ -105,6 +105,29 @@ run_test "IDS: mempool too small (fail)" \
     --expect-fail \
     "$IDS_YAML"
 
+run_test "IDS: mempool too small for queues (fail)" \
+    --interface-cfg-set net_null0.threads=2 \
+    --interface-cfg-set net_null0.mempool-size=1 \
+    --suricata-log-check-grep "mempool size is likely too small" \
+    --expect-fail \
+    "$IDS_YAML"
+
+# power-of-two boundary: 32 is exactly nb_rx_desc + nb_tx_desc, must not undercount
+run_test "IDS: static mempool=32, power-of-two boundary (fail)" \
+    --interface-cfg-set net_null0.threads=1 \
+    --interface-cfg-set net_null0.mempool-size=32 \
+    --suricata-log-check-grep "mempool size is likely too small" \
+    --expect-fail \
+    "$IDS_YAML"
+
+run_test "IDS: static mempool=64, power-of-two boundary (1 thread)" \
+    --interface-cfg-set net_null0.threads=1 \
+    --interface-cfg-set net_null0.mempool-size=64 \
+    --interface-cfg-set net_null0.mempool-cache-size=auto \
+    --suricata-log-check-grep "1 packet mempools of size 63, cache size 21" \
+    --expect-start \
+    "$IDS_YAML"
+
 run_test "IDS: auto descriptors, auto mempool (OOM expected with nohuge)" \
     --interface-cfg-set net_null0.threads=1 \
     --interface-cfg-set net_null0.mempool-size=auto \
@@ -122,31 +145,31 @@ run_test "Bond: auto mempool, auto cache (1 thread)" \
     --interface-cfg-set net_bonding0.threads=1 \
     --interface-cfg-set net_bonding0.mempool-size=auto \
     --interface-cfg-set net_bonding0.mempool-cache-size=auto \
-    --suricata-log-check-grep "1 packet mempools of size 63, cache size 21" \
+    --suricata-log-check-grep "1 packet mempools of size 127, cache size 1" \
     --expect-start \
     "$BOND_YAML"
 
 run_test "Bond: auto mempool, auto cache (2 threads)" \
     --interface-cfg-set net_bonding0.threads=2 \
     --interface-cfg-set net_bonding0.mempool-size=auto \
     --interface-cfg-set net_bonding0.mempool-cache-size=auto \
-    --suricata-log-check-grep "2 packet mempools of size 63, cache size 21" \
+    --suricata-log-check-grep "2 packet mempools of size 127, cache size 1" \
     --expect-start \
     "$BOND_YAML"
 
 run_test "Bond: auto mempool, static cache=7 (1 thread)" \
     --interface-cfg-set net_bonding0.threads=1 \
     --interface-cfg-set net_bonding0.mempool-size=auto \
     --interface-cfg-set net_bonding0.mempool-cache-size=7 \
-    --suricata-log-check-grep "1 packet mempools of size 63, cache size 7" \
+    --suricata-log-check-grep "1 packet mempools of size 127, cache size 7" \
     --expect-start \
     "$BOND_YAML"
 
 run_test "Bond: auto mempool, static cache=7 (2 threads)" \
     --interface-cfg-set net_bonding0.threads=2 \
     --interface-cfg-set net_bonding0.mempool-size=auto \
     --interface-cfg-set net_bonding0.mempool-cache-size=7 \
-    --suricata-log-check-grep "2 packet mempools of size 63, cache size 7" \
+    --suricata-log-check-grep "2 packet mempools of size 127, cache size 7" \
     --expect-start \
     "$BOND_YAML"
 

src/runmode-dpdk.c

@@ -530,20 +530,53 @@ static int ConfigSetTxQueues(
     SCReturnInt(0);
 }
 
-static uint32_t MempoolSizeCalculate(
+static uint32_t MempoolSizeCalculateAutosize(
         const DPDKIfaceConfig *iconf, const struct rte_eth_dev_info *dev_info)
 {
-    uint32_t sz = iconf->nb_rx_queues * iconf->nb_rx_desc + iconf->nb_tx_queues * iconf->nb_tx_desc;
-    if (!iconf->nb_tx_queues || !iconf->nb_tx_desc)
-        sz *= 2; // double to have enough space for RX descriptors
+    uint32_t next_p2 =
+            rte_align32pow2(iconf->nb_rx_desc + iconf->nb_tx_desc +
+                            1); // + 1 in case number of descriptors is already a power of 2
 
+    uint32_t mp_size = next_p2;
     if (dev_info != NULL) {
         if (strcmp(dev_info->driver_name, "net_bonding") == 0) {
-            sz = BondingMempoolSizeCalculate(iconf->port_id, dev_info, sz);
+            mp_size = BondingMempoolSizeCalculate(iconf->port_id, dev_info, mp_size);
         }
     }
 
-    return sz;
+    return mp_size -
+           1; // mempool size should be n = (2^q - 1) to have all descriptors available for use
+}
+
+static uint32_t MempoolSizeDistributeToQueues(uint32_t global_mp_size, uint16_t nic_queues)
+{
+    if (nic_queues == 0) {
+        return 0;
+    }
+
+    uint32_t mp_size_per_queue = global_mp_size / nic_queues;
+    if (mp_size_per_queue == 0) {
+        return 0;
+    }
+
+    uint32_t next_p2 = rte_align32pow2(mp_size_per_queue);
+    return (mp_size_per_queue == next_p2 || mp_size_per_queue == next_p2 - 1)
+                   ? next_p2 - 1
+                   : (next_p2 >> 1) - 1; // we must fit in the globally specified mempool size
+}
+
+static uint32_t MempoolSizeCalculateMinimal(
+        const DPDKIfaceConfig *iconf, const struct rte_eth_dev_info *dev_info)
+{
+    uint32_t mp_size =
+            rte_align32pow2(iconf->nb_rx_desc + iconf->nb_tx_desc +
+                            1); // + 1 in case number of descriptors is already a power of 2
+    if (dev_info != NULL) {
+        if (strcmp(dev_info->driver_name, "net_bonding") == 0) {
+            mp_size = BondingMempoolSizeCalculate(iconf->port_id, dev_info, mp_size);
+        }
+    }
+    return mp_size - 1;
 }
 
 static int ConfigSetMempoolSize(
@@ -571,27 +604,30 @@ static int ConfigSetMempoolSize(
             SCReturnInt(-EINVAL);
         }
 
-        iconf->mempool_size = MempoolSizeCalculate(iconf, dev_info);
+        iconf->queue_mempool_size = MempoolSizeCalculateAutosize(iconf, dev_info);
         SCReturnInt(0);
     }
 
-    if (StringParseUint32(&iconf->mempool_size, 10, 0, entry_str) < 0) {
+    uint32_t global_mempool_size;
+    if (StringParseUint32(&global_mempool_size, 10, 0, entry_str) < 0) {
         SCLogError("%s: mempool size entry contain non-numerical characters - \"%s\"", iconf->iface,
                 entry_str);
         SCReturnInt(-EINVAL);
     }
 
-    uint32_t required_mp_size = MempoolSizeCalculate(iconf, dev_info);
-    if (required_mp_size >
-            iconf->mempool_size + 1) { // +1 to mask mempool size advice given in Suricata 7.0.x -
-                                       // mp_size should be n = (2^q - 1)
+    iconf->queue_mempool_size =
+            MempoolSizeDistributeToQueues(global_mempool_size, iconf->nb_rx_queues);
+    uint32_t required_mp_size = MempoolSizeCalculateMinimal(iconf, dev_info);
+    if (required_mp_size > iconf->queue_mempool_size) {
+        uint32_t required_global_mp_size =
+                required_mp_size * iconf->nb_rx_queues + iconf->nb_rx_queues - 1;
         SCLogError("%s: mempool size is likely too small for the number of descriptors and queues, "
                    "set to \"auto\" or adjust to the value of \"%" PRIu32 "\"",
-                iconf->iface, required_mp_size);
+                iconf->iface, required_global_mp_size);
         SCReturnInt(-ERANGE);
     }
 
-    if (iconf->mempool_size == 0) {
+    if (iconf->queue_mempool_size == 0) {
         SCLogError("%s: mempool size requires a positive integer", iconf->iface);
         SCReturnInt(-ERANGE);
     }
@@ -613,9 +649,9 @@ static int ConfigSetMempoolCacheSize(DPDKIfaceConfig *iconf, const char *entry_s
     SCEnter();
     if (entry_str == NULL || entry_str[0] == '\0' || strcmp(entry_str, "auto") == 0) {
         // calculate the mempool size based on the mempool size (it needs to be already filled in)
-        if (iconf->mempool_size == 0) {
-            SCLogError("%s: cannot calculate mempool cache size of a mempool with size %d",
-                    iconf->iface, iconf->mempool_size);
+        if (iconf->queue_mempool_size == 0) {
+            SCLogError("%s: cannot calculate mempool cache size of a mempool with size %" PRIu32,
+                    iconf->iface, iconf->queue_mempool_size);
             SCReturnInt(-EINVAL);
         }
 
@@ -1459,20 +1495,16 @@ static int DeviceConfigureQueues(DPDKIfaceConfig *iconf, const struct rte_eth_de
     // +4 for VLAN header
     uint16_t mtu_size = iconf->mtu + RTE_ETHER_CRC_LEN + RTE_ETHER_HDR_LEN + 4;
     uint16_t mbuf_size = ROUNDUP(mtu_size, 1024) + RTE_PKTMBUF_HEADROOM;
-    // Follows DPDK recommendation of having a mempool size that is a power of 2 minus one.
-    // So e.g. mp_size of 262144 and 262143 both lead to 65535 on 4 rx queues
-    uint32_t raw = iconf->mempool_size / iconf->nb_rx_queues;
-    uint32_t next_p2 = rte_align32pow2(raw + 1);
-    uint32_t q_mp_sz = (next_p2 == raw + 1) ? raw : (next_p2 >> 1) - 1;
-    uint32_t q_mp_cache_sz = iconf->mempool_cache_size_auto ? MempoolCacheSizeCalculate(q_mp_sz)
-                                                            : iconf->mempool_cache_size;
+    uint32_t q_mp_cache_sz = iconf->mempool_cache_size_auto
+                                     ? MempoolCacheSizeCalculate(iconf->queue_mempool_size)
+                                     : iconf->mempool_cache_size;
     SCLogInfo("%s: creating %u packet mempools of size %u, cache size %u, mbuf size %u",
-            iconf->iface, iconf->nb_rx_queues, q_mp_sz, q_mp_cache_sz, mbuf_size);
+            iconf->iface, iconf->nb_rx_queues, iconf->queue_mempool_size, q_mp_cache_sz, mbuf_size);
     for (int i = 0; i < iconf->nb_rx_queues; i++) {
         char mempool_name[64];
         snprintf(mempool_name, sizeof(mempool_name), "mp_%d_%.20s", i, iconf->iface);
-        iconf->pkt_mempools->pkt_mp[i] = rte_pktmbuf_pool_create(
-                mempool_name, q_mp_sz, q_mp_cache_sz, 0, mbuf_size, (int)iconf->socket_id);
+        iconf->pkt_mempools->pkt_mp[i] = rte_pktmbuf_pool_create(mempool_name,
+                iconf->queue_mempool_size, q_mp_cache_sz, 0, mbuf_size, (int)iconf->socket_id);
         if (iconf->pkt_mempools->pkt_mp[i] == NULL) {
             retval = -rte_errno;
             SCLogError("%s: rte_pktmbuf_pool_create failed with code %d (mempool: %s) - %s",

src/source-dpdk.h

@@ -71,7 +71,7 @@ typedef struct DPDKIfaceConfig_ {
     uint16_t nb_rx_desc;
     uint16_t nb_tx_queues;
     uint16_t nb_tx_desc;
-    uint32_t mempool_size;
+    uint32_t queue_mempool_size;
     uint32_t mempool_cache_size;
     bool mempool_cache_size_auto; // auto cache size based on mempool size
     DPDKDeviceResources *pkt_mempools;