On Fri, Sep 26, 2025 at 08:39:28AM +0200, Bastien Curutchet wrote: > Hi Maciej, > > On 9/25/25 3:32 PM, Maciej Fijalkowski wrote: > > On Wed, Sep 24, 2025 at 04:49:39PM +0200, Bastien Curutchet (eBPF > > Foundation) wrote: > > > testapp_stats_rx_dropped() generates pkt_stream twice. The last > > > generated is released by pkt_stream_restore_default() at the end of the > > > test but we lose the pointer of the first pkt_stream. > > > > > > Release the 'middle' pkt_stream when it's getting replaced to prevent > > > memory leaks. > > > > > > Signed-off-by: Bastien Curutchet (eBPF Foundation) > > > <[email protected]> > > > --- > > > tools/testing/selftests/bpf/test_xsk.c | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/tools/testing/selftests/bpf/test_xsk.c > > > b/tools/testing/selftests/bpf/test_xsk.c > > > index > > > 8d7c38eb32ca3537cb019f120c3350ebd9f8c6bc..eb18288ea1e4aa1c9337d16333b7174ecaed0999 > > > 100644 > > > --- a/tools/testing/selftests/bpf/test_xsk.c > > > +++ b/tools/testing/selftests/bpf/test_xsk.c > > > @@ -536,6 +536,13 @@ static void pkt_stream_receive_half(struct test_spec > > > *test) > > > struct pkt_stream *pkt_stream = test->ifobj_tx->xsk->pkt_stream; > > > u32 i; > > > + if (test->ifobj_rx->xsk->pkt_stream != test->rx_pkt_stream_default) > > > + /* Packet stream has already been replaced so we have to > > > release this one. > > > + * The newly created one will be freed by the restore_default() > > > at the > > > + * end of the test > > > + */ > > > + pkt_stream_delete(test->ifobj_rx->xsk->pkt_stream); > > > > I don't see why this one is not addressed within test case > > (testapp_stats_rx_dropped()) and other fix is (testapp_xdp_shared_umem()). > > > > pkt_stream_receive_half() can be used by other tests. I thought it would be
So is pkt_stream_replace_half() and other routines that eventually call pkt_stream_generate() and overwrite the pkt_stream, right? It just feels odd to have a special treatment in one function and other are left as-is just because currently we don't have another abusive test case. Maybe it's enough of bike-shedding here, just wanted to clarify on my POV. In the end don't get me wrong here, this interface is a bit PITA for me and thanks for whole effort! > more convenient for people writing testapp_*() functions if they didn't have > to worry about releasing these kind of pointer themselves. > > The same approach can't be used in testapp_xdp_shared_umem(), because we > need to wait for the test to complete before releasing the pointers. > > > -- > Bastien Curutchet, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com >
