On 9/17/25 8:33 PM, Maciej Fijalkowski wrote:
On Wed, Sep 17, 2025 at 05:32:55PM +0200, Bastien Curutchet wrote:
Hi Maciej
On 9/16/25 7:58 PM, Maciej Fijalkowski wrote:
On Thu, Sep 04, 2025 at 12:10:18PM +0200, Bastien Curutchet (eBPF Foundation)
wrote:
Some tests introduce memory leaks by not freeing all the pkt_stream
objects they're creating.
Fix these memory leaks.
I would appreciate being more explicit here as I've been scratching my
head here.
Indeed it lacks details sorry. IIRC I spotted these with valgrind, maybe I
can add valgrind's output to the commit log in next iteration.
From what I see the problem is with testapp_stats_rx_dropped() as it's the
one case that uses replace and receive half of pkt streams, both of which
overwrite the default pkt stream. So we lose a pointer to one of pkt
streams and leak it eventually.
Exactly, we lose pointers in some cases when xsk->pkt_stream gets replaced
by a new stream. testapp_stats_rx_dropped() is the most convoluted of these
cases.
pkt_stream_restore_default() is supposed to delete overwritten pkt_stream
and set ::pkt_stream to default one, explicit pkt_stream_delete() in bunch
of tests is redundant IMHO.
Per my understanding testapp_stats_rx_dropped() and
testapp_xdp_shared_umem() need fixing. First generate pkt_stream twice and
second generates pkt_stream on each xsk from xsk_arr, where normally
xsk_arr[0] gets pkt_streams and xsk_arr[1] have them NULLed.
I took another look at it, and I agree with you: the pkt_stream_delete()
calls I added in testapp_stats_rx_full() and testapp_stats_fill_empty()
don't seem necessary.
It still feels a bit strange to overwrite a pointer without freeing it
right away, but I don't have a strong opinion on this. I'm fine with
only fixing testapp_stats_rx_dropped() and testapp_xdp_shared_umem() in
the next iteration.
Best regards,
--
Bastien Curutchet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
