> -----Original Message-----
> From: Ilya Maximets <[email protected]>
> Sent: Thursday, August 12, 2021 4:27 PM
> To: Van Haaren, Harry <[email protected]>; [email protected]
> Cc: [email protected]; [email protected]; Stokes, Ian
> <[email protected]>; Flavio Leitner <[email protected]>
> Subject: Re: [ovs-dev] [PATCH] dpif: fix memory leak of pmd_list after usage
>
> On 8/12/21 5:11 PM, Harry van Haaren wrote:
> > This commit fixes a memory leak when a pmd_list is retrieved
> > from the sorted_poll_thread_list() function. Inside the function,
> > the pmd list is allocated, but it was not freed once no longer
> > required for the command functionality. These leaks were found
> > by a static analysis tool.
>
> Hmm. Thanks for spotting this!
Automated tooling is great to have.
> Could you, please, add unit tests for these functions? Something very
> basic will work. Even if these functions will not do anything useful,
> e.g. changing priority of the only implementation, asan should spot the
> leak and fail the CI job that we have.
Yes will look to add some tests in OVS 2.17 timeframe if that helps.
> > Fixes: 3d8f47bc04 ("dpif-netdev: Add command line and function pointer for
> miniflow extract")
> > Fixes: abb807e27d ("dpif-netdev: Add command to switch dpif
> > implementation.")
> > Fixes: 3d018c3ea7 ("dpif-netdev: Add subtable lookup prio set command.")
>
> Please, split the change for the last one to the separate patch, so
> it can be backported to earlier branches.
Yes, patch split done in v2;
https://patchwork.ozlabs.org/project/openvswitch/list/?series=257839
> > Signed-off-by: Harry van Haaren <[email protected]>
> >
> > ---
> >
> > lib/dpif-netdev.c | 18 +++++++++++++-----
> > 1 file changed, 13 insertions(+), 5 deletions(-)
>
> The patch name should have a 'dpif-netdev' area prefix.
Sure, fixed.
> > /* Get PMD threads list, required to get DPCLS instances. */
> > size_t n;
> > - struct dp_netdev_pmd_thread **pmd_list;
> > sorted_poll_thread_list(dp, &pmd_list, &n);
>
> This is called in a loop, so we're leaking the array on every
> iteration. Same for other parts of the patch.
Doh. This is the result of rushing a fix I suppose. Thanks for pointing
out, and indeed for the other occurrences too. Fixed in V2.
<snip patch>
Kevin Traynor; you mentioned in the other email a better way
to handle the below variables. They're no longer part of the V2 patchset,
but noted your review of V1, thanks.
> > + struct dp_netdev_pmd_thread **pmd_list = NULL;
Regards, -Harry
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
