[dpdk-dev] [RFC] ethdev: complete closing to free all resources

[Andrew Rybchenko]

On 09/12/2018 05:57 PM, Thomas Monjalon wrote:
> 10/09/2018 10:54, Andrew Rybchenko:
>> On 09/10/2018 11:42 AM, Thomas Monjalon wrote:
>>> 10/09/2018 10:03, Andrew Rybchenko:
>>>> On 09/08/2018 02:39 AM, Thomas Monjalon wrote:
>>>>> After closing a port, it cannot be restarted.
>>>>> So there is no reason to not free all associated resources.
>>>>>
>>>>> The last step was done with rte_eth_dev_detach() which is deprecated.
>>>>> Instead of removing the associated rte_device, the driver should check
>>>>> if no more port (ethdev, cryptodev, etc) is still open for the device.
>>>>> Then the device resources can be freed by the driver inside the
>>>>> dev_close() driver callback operation.
>>>>>
>>>>> The last ethdev freeing (dev_private and final release), which were done
>>>>> by rte_eth_dev_detach(), are now done at the end of rte_eth_dev_close().
>>>> For me, it sounds more logical to kill dev_close and keep detach.
>>>> IMHO, dev_close is artificial and hardly useful. detach is a local pair
>>>> to attach.
>>> I don't get your point.
>>>
>>> In order to free a port, we need close + detach.
>>> We can keep only one.
>>> I choose close because:
>>> 	1) attach/detach are deprecated
>>> 	2) probe/close is a more obvious pair
>>> 	3) we need the driver to free the lower level resources
>> Yes, I'm sorry I used bad terminology.
>> We have probe/remove pair for both PCI and vdev drivers and I mean
>> that remove is a better candidate to be kept (as a pair for probe which
>> allocates all resources).
> OK, yes probe/remove is the pair at EAL level.
> But if we want to request removal at ethdev level, rte_eth_dev_close
> is the function.
>
> Note that there is no function to request creation of an ethdev port.
> Adding a new port is done only by the PMD during probing (rte_bus level).

The overall picture is too vague. May be it is simply incomplete looking
at the patch only. I understand that restructuring is required looking at
rte_eth_dev_destroy() (which is used by i40e and ixgbe drivers only) and
rte_eth_dev_pci_release() used by other PCI drivers.

Right now it looks symmetric at least on drivers level which provides
init callback to probe to do driver-specific job and uninit callback to
remove to free driver-specific resources.

I can't say if the patch is right or wrong direction since final design
is unclear.