Re: [dpdk-dev] [PATCH v3 1/2] ethdev: free all common data when releasing port

[Thomas Monjalon <thomas@monjalon.net>]

16/10/2018 14:47, Andrew Rybchenko:
> On 10/16/18 3:22 PM, Thomas Monjalon wrote:
> > 16/10/2018 13:16, Andrew Rybchenko:
> >> On 10/15/18 2:20 AM, Thomas Monjalon wrote:
> >>> This is a clean-up of common ethdev data freeing.
> >>> All data freeing are moved to rte_eth_dev_release_port()
> >>> and done only in case of primary process.
> >>>
> >>> It is probably fixing some memory leaks for PMDs which were
> >>> not freeing all data.
> > [...]
> >> In general it looks good, really big work, but ideally it should be
> >> acked by cxgbe maintainer as well.
> > Actually, after more thoughts, I think this patch is not correct.
> > It is freeing MAC adresses in ethdev, even if there was no specific
> > allocation done for it in the PMD. Only the PMD can know what are the
> > memory blocks to free.
> > And it is the same for data->dev_private: are we sure it has been malloc'ed?
> > Are we sure it has not been allocated as part of a bigger block?
> > Historically, ethdev was freeing data->dev_private in some cases.
> > So maybe we can keep this assumption.
> > Or we can move all data freeing to the PMD?
> 
> It is allocated in rte_eth_dev_create(), rte_eth_vdev_allocate(),
> rte_eth_dev_pci_allocate() and some PMDs. I can't say that I'm happy
> with it, but it could be really required. May be it should be default
> behaviour and if PMD wants override it, just free before and
> set dev_private to NULL.

Yes, you're right!
So freeing is done by default in rte_eth_dev_release_port(),
and PMD can avoid it by setting fields to NULL before calling it.

In this case, I just need to double check the MAC freeing,
and set NULL in some PMDs which do not allocate MAC separately.