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.
