[dpdk-dev] mbuf cleanup in i40e/ixgbe

[dpdk-dev] mbuf cleanup in i40e/ixgbe

[Evgeniy Marchenko]

Hello

I'm checking mbuf consumption issues in TX path and it looks like i40e and 
ixgbe drivers consume all mbufs in "full featured" path and free them one-by-
one only after TX queue wraps.

Upstream drivers are more conservative with memory consumption and free up to 
256 SKBs on every napi_poll invocation. And this makes sense because there is 
indeed not too much work for cleanup and freeing as much memory buffers as 
possible would lower memory pressure and memory requirements and allow bigger 
TX bursts without cleanup procedures and better CPU cache utilization.

Why cannot we bulk free mbuf in i40e_xmit_cleanup ? Why do we need 
nb_tx_to_clean calculations? Isn't it always equal to txq->tx_rs_thresh?

Here is a proposed patch for i40e PMD to bulk free unused mbufs:

----------------------------------- CUT --------------------------------------
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 8731712..9e3a333 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -883,8 +883,12 @@ i40e_xmit_cleanup(struct i40e_tx_queue *txq)
        uint16_t nb_tx_desc = txq->nb_tx_desc;
        uint16_t desc_to_clean_to;
        uint16_t nb_tx_to_clean;
+       struct i40e_tx_entry *txe;
+       int i;
 
-       desc_to_clean_to = (uint16_t)(last_desc_cleaned + txq->tx_rs_thresh);
+       nb_tx_to_clean = txq->tx_rs_thresh;
+
+       desc_to_clean_to = (uint16_t)(last_desc_cleaned + nb_tx_to_clean);
        if (desc_to_clean_to >= nb_tx_desc)
                desc_to_clean_to = (uint16_t)(desc_to_clean_to - nb_tx_desc);
 
@@ -898,12 +902,18 @@ i40e_xmit_cleanup(struct i40e_tx_queue *txq)
                return -1;
        }
 
-       if (last_desc_cleaned > desc_to_clean_to)
-               nb_tx_to_clean = (uint16_t)((nb_tx_desc - last_desc_cleaned) +
-                                                       desc_to_clean_to);
-       else
-               nb_tx_to_clean = (uint16_t)(desc_to_clean_to -
-                                       last_desc_cleaned);
+       i = last_desc_cleaned;
+       while (i++ != desc_to_clean_to) {
+               if (i >= nb_tx_desc)
+                       i -= nb_tx_desc;
+
+               txe = &sw_ring[i];
+               RTE_MBUF_PREFETCH_TO_FREE(txe->mbuf);
+               if (txe->mbuf) {
+                       rte_pktmbuf_free_seg(txe->mbuf);
+                       txe->mbuf = NULL;
+               }
+       }
 
        txd[desc_to_clean_to].cmd_type_offset_bsz = 0;
 
----------------------------------- CUT --------------------------------------

What do you think about cleaning not just txq->tx_rs_thresh mbufs but as many 
as possible?

Regards, Evgeniy Marchenko
DDoS-Guard.net