[dpdk-dev] [PATCH v2] librte_ether: release memory in uninit function.

[Iremonger, Bernard]

> -----Original Message-----
> From: Tetsuya Mukawa [mailto:mukawa at igel.co.jp]
> Sent: Tuesday, July 7, 2015 4:38 AM
> To: dev at dpdk.org
> Cc: Qiu, Michael; Iremonger, Bernard; thomas.monjalon at 6wind.com;
> Ananyev, Konstantin; Stephen Hemminger; Zhang, Helin; Chen, Jing D; Neil
> Horman
> Subject: Re: [dpdk-dev] [PATCH v2] librte_ether: release memory in uninit
> function.
> 
> On 2015/07/06 20:35, Qiu, Michael wrote:
> > Hi, all
> >
> > As we has gap on the memory release action to be done in which step, I
> > appreciate all your comments on this patch.
> >
> > Currently, the correct quit sequence for testpmd is stop() --->
> > port_stop() --> port_close() --> quit(). This will lead lots of memory
> > not released by default, like queues.

There is also the scenario (outside of testpmd) where an application can call rte_eth_dev_close() or rte_eth_dev_detach() which calls rte_eth_dev_uninit() directly from an application.
In these cases the memory allocated in the EAL layer should be released in both rte_eth_dev_close() and rte_eth_dev_detach().
This patch is intended to address the rte_eth_dev_detach() case only (hotplug case).
Perhaps there should be a separate patch to address the rte_eth_dev_close() case.

Regards,

Bernard.

> >
> > We have 3 potential proposals(normal case means without hotplug):
> >
> > 1. Those memory release in close()  other left in uninit()
> >     This will work fine for both hotplug case and normal case.
> 
> +1
> 
> I assume that once close() is called, we cannot start the port again without
> hotplugging.
> This is my premise.
> 
> It might be good that we move finalization code to close() as much as
> possible, because of followings.
> 1. Anyway, once close() is called, we cannot start the port again. So it should
> be ok to free resources in close().
> 2. Non-hotplugging applications can release resources if we implement
> finalization code to close().
> 
> Also I guess Stephen's suggestion is important to keep code clean.
> It may be good that 'dev->data->rx/tx_queues[queue_id]' are freed in
> rx/tx_queue_release() of each PMD, and rx/tx_queue_release() will be
> called by rte_eth_dev_close(). Also 'dev->data->rx/tx_queues[]' should be
> freed in ethdev.c.
> 
> > 3. Combine close() and uninit(), only one will be left.
> >     This will work fine for both hotplug case and normal case. But it
> > may change a lot, such as logic.
> 
> I guess this will be second option.  But I agree we need to change a lot. Also
> after enabling hotplug by default, when someone only wants to close the
> port, it will be impossible.
> 
> Regards,
> Tetsuya