Re: DPDK 22.11 - How to fix memory leak for KNI - How to debug

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 5/9/2023 12:13 PM, Yasin CANER wrote:
> Hello,
> 
> I draw a flow via asciiflow to explain myself better. Problem is after
> transmitting packets(mbufs) , it never puts in the kni->free_q to back
> to the original pool. Each cycle, it allocates another 32 units that
> cause leaks. Or I am missing something.
> 
> I already tried the rte_eth_tx_done_cleanup() function but it didn't fix
> anything.
> 
> I am working on a patch to fix this issue but I am not sure if there
> is another way.
> 
> Best regards.
> 
> https://pastebin.ubuntu.com/p/s4h5psqtgZ/
> <https://pastebin.ubuntu.com/p/s4h5psqtgZ/>
> 
> 
> unsigned
> rte_kni_rx_burst(struct rte_kni *kni, struct rte_mbuf **mbufs, unsigned
> int num)
> {
> unsigned int ret = kni_fifo_get(kni->tx_q, (void **)mbufs, num);
> 
> /* If buffers removed, allocate mbufs and then put them into alloc_q */
> /* Question, how to test buffers is removed or not?*/
> if (ret)
>     kni_allocate_mbufs(kni);
> 
> return ret;
> }
> 

Selam Yasin,


You can expect 'kni->alloc_q' fifo to be full, this is not a memory leak.

As you pointed out, number of mbufs consumed by kernel from 'alloc_q'
and number of mbufs added to 'alloc_q' is not equal and this is expected.

Target here is to prevent buffer underflow from kernel perspective, so
it will always have available mbufs for new packets.
That is why new mbufs are added to 'alloc_q' at worst same or sometimes
higher rate than it is consumed.

You should calculate your mbuf requirement with the assumption that
'kni->alloc_q' will be full of mbufs.


'kni->alloc_q' is freed when kni is removed.
Since 'alloc_q' holds physical address of the mbufs, it is a little
challenging to free them in the userspace, that is why first kernel
tries to move mbufs to 'kni->free_q' fifo, please check
'kni_net_release_fifo_phy()' for it.

If all moved to 'free_q' fifo, nothing left to in 'alloc_q', but if not,
userspace frees 'alloc_q' in 'rte_kni_release()', with following call:
`kni_free_fifo_phy(kni->pktmbuf_pool, kni->alloc_q);`


I can see you have submitted fixes for this issue, although as I
explained above I don't think a defect exist, I will review them
today/tomorrow.

Regards,
Ferruh


> Stephen Hemminger <stephen@networkplumber.org
> <mailto:stephen@networkplumber.org>>, 8 May 2023 Pzt, 19:18 tarihinde
> şunu yazdı:
> 
>     On Mon, 8 May 2023 09:01:41 +0300
>     Yasin CANER <yasinncaner@gmail.com <mailto:yasinncaner@gmail.com>>
>     wrote:
> 
>     > Hello Stephen,
>     >
>     > Thank you for response, it helps me a lot. I understand problem
>     better.
>     >
>     > After reading mbuf library (
>     > https://doc.dpdk.org/guides/prog_guide/mempool_lib.html
>     <https://doc.dpdk.org/guides/prog_guide/mempool_lib.html>)  i
>     realized that
>     > 31 units allocation memory slot doesn't return to pool!
> 
>     If receive burst returns 1 mbuf, the other 31 pointers in the array
>     are not valid. They do not point to mbufs.
> 
>     > 1 unit mbuf can be freed via rte_pktmbuf_free so it can back to pool.
>     >
>     > Main problem is that allocation doesn't return to original pool,
>     act as
>     > used. So, after following rte_pktmbuf_free
>     >
>     <http://doc.dpdk.org/api/rte__mbuf_8h.html#a1215458932900b7cd5192326fa4a6902 <http://doc.dpdk.org/api/rte__mbuf_8h.html#a1215458932900b7cd5192326fa4a6902>>
>     > function,
>     > i realized that there is 2 function to helps to mbufs back to pool.
>     >
>     > These are rte_mbuf_raw_free
>     >
>     <http://doc.dpdk.org/api/rte__mbuf_8h.html#a9f188d53834978aca01ea101576d7432 <http://doc.dpdk.org/api/rte__mbuf_8h.html#a9f188d53834978aca01ea101576d7432>>
>     >  and rte_pktmbuf_free_seg
>     >
>     <http://doc.dpdk.org/api/rte__mbuf_8h.html#a006ee80357a78fbb9ada2b0432f82f37 <http://doc.dpdk.org/api/rte__mbuf_8h.html#a006ee80357a78fbb9ada2b0432f82f37>>.
>     > I will focus on them.
>     >
>     > If there is another suggestion, I will be very pleased.
>     >
>     > Best regards.
>     >
>     > Yasin CANER
>     > Ulak
>