Memory leak issue in send_pkt_dpdk_queue for odp-dpdk

Message ID CAOdUko01qiULCTTaMuBJQR-uBqXd58EV=gMeZd-r5uG0X3_jmw@mail.gmail.com
State New
Headers show

Commit Message

Wenxian Li March 8, 2016, 9:54 a.m.
Hi teams,

I met an memory leak issue on odp-dpdk.
Seems the odp_packet is not freed after packet is sent out.

I add a quick fix as below. Basically it works in my case.

However, I'm thinking that suppose DPDK has freed the rte_mbuf. What we
need to do is only remove the odp_packet from odp_pool.
The tricky thing here is the rte_mbuf has been freed by DPDK, and after
that we call odp_packet_free for the same odp_packet. Is there any risk for
this scenario?


        if (!pkt_dpdk->lockless_tx)
@@ -413,6 +414,9 @@ static int send_pkt_dpdk_queue(pktio_entry_t
*pktio_entry, int index,

        pkts = rte_eth_tx_burst(pkt_dpdk->portid, index,
                                (struct rte_mbuf **)pkt_table, len);
+      for (i=0; i<pkts; i++) {
+           odp_packet_free(pkt_table[i]);
+       }

        if (!pkt_dpdk->lockless_tx)
                odp_ticketlock_unlock(&pkt_dpdk->tx_lock[index]);

Thanks,
Wenxian

Comments

Zoltan Kiss March 8, 2016, 1:20 p.m. | #1
There was a recent fix for such an issue:

https://git.linaro.org/lng/odp-dpdk.git/commitdiff/b56d8b7405a059b9d4bd4388f0cae774a64f45dc

But it might not catch all cases. Can you send me your console output 
after starting the application? It should reveal which driver you use 
and what RX/TX functions are in use.

Zoli

On 08/03/16 16:54, Wenxian Li wrote:
> Hi teams,

>

> I met an memory leak issue on odp-dpdk.

> Seems the odp_packet is not freed after packet is sent out.

>

> I add a quick fix as below. Basically it works in my case.

>

> However, I'm thinking that suppose DPDK has freed the rte_mbuf. What 

> we need to do is only remove the odp_packet from odp_pool.

> The tricky thing here is the rte_mbuf has been freed by DPDK, and 

> after that we call odp_packet_free for the same odp_packet. Is there 

> any risk for this scenario?

>

>

> diff --git a/platform/linux-dpdk/odp_packet_dpdk.c 

> b/platform/linux-dpdk/odp_packet_dpdk.c

> index fe9b05c..0ccada9 100644

> --- a/platform/linux-dpdk/odp_packet_dpdk.c

> +++ b/platform/linux-dpdk/odp_packet_dpdk.c

> @@ -406,6 +406,7 @@ static int send_pkt_dpdk_queue(pktio_entry_t 

> *pktio_entry, int index,

>                                odp_packet_t pkt_table[], int len)

>  {

>         int pkts;

> +      int i;

>         pkt_dpdk_t * const pkt_dpdk = &pktio_entry->s.pkt_dpdk;

>

>         if (!pkt_dpdk->lockless_tx)

> @@ -413,6 +414,9 @@ static int send_pkt_dpdk_queue(pktio_entry_t 

> *pktio_entry, int index,

>

>         pkts = rte_eth_tx_burst(pkt_dpdk->portid, index,

>                                 (struct rte_mbuf **)pkt_table, len);

> +      for (i=0; i<pkts; i++) {

> +           odp_packet_free(pkt_table[i]);

> +       }

>

>         if (!pkt_dpdk->lockless_tx)

> odp_ticketlock_unlock(&pkt_dpdk->tx_lock[index]);

>

> Thanks,

> Wenxian

>

>

> _______________________________________________

> lng-odp-dpdk mailing list

> lng-odp-dpdk@lists.linaro.org

> https://lists.linaro.org/mailman/listinfo/lng-odp-dpdk
Zoltan Kiss March 8, 2016, 4:09 p.m. | #2
On 08/03/16 16:54, Wenxian Li wrote:
> I add a quick fix as below. Basically it works in my case.
>
> However, I'm thinking that suppose DPDK has freed the rte_mbuf. What 
> we need to do is only remove the odp_packet from odp_pool.
> The tricky thing here is the rte_mbuf has been freed by DPDK, and 
> after that we call odp_packet_free for the same odp_packet. Is there 
> any risk for this scenario?
And to answer that question: yes, it's DPDK which should free the 
packets, your patch does double free which is quite dangerous. I suspect 
this is a similar issue I mentioned in the other message, since that fix 
I've still seen cases where it choose scatter-gather on RX but not on 
TX, despite the buffer size alignment were meant to fix that.

Zoli

Patch hide | download patch | download mbox

diff --git a/platform/linux-dpdk/odp_packet_dpdk.c
b/platform/linux-dpdk/odp_packet_dpdk.c
index fe9b05c..0ccada9 100644
--- a/platform/linux-dpdk/odp_packet_dpdk.c
+++ b/platform/linux-dpdk/odp_packet_dpdk.c
@@ -406,6 +406,7 @@  static int send_pkt_dpdk_queue(pktio_entry_t
*pktio_entry, int index,
                               odp_packet_t pkt_table[], int len)
 {
        int pkts;
+      int i;
        pkt_dpdk_t * const pkt_dpdk = &pktio_entry->s.pkt_dpdk;