[dpdk-dev] [PATCH 12/20] net/pcap: release port upon close

[Ferruh Yigit]

On 9/23/2020 9:44 PM, Thomas Monjalon wrote:
> 23/09/2020 18:44, Ferruh Yigit:
>> On 9/13/2020 11:07 PM, Thomas Monjalon wrote:
>>> The flag RTE_ETH_DEV_CLOSE_REMOVE is set so all port resources
>>> can be freed by rte_eth_dev_close().
>>>
>>> Freeing of private port resources is moved
>>> from the ".remove(device)" to the ".dev_close(port)" operation.
>>>
>>> Signed-off-by: Thomas Monjalon <thomas at monjalon.net>
>>> ---
>>>    drivers/net/pcap/rte_eth_pcap.c | 29 ++++++++++++++---------------
>>>    1 file changed, 14 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/net/pcap/rte_eth_pcap.c b/drivers/net/pcap/rte_eth_pcap.c
>>> index 76e704a65a..a946fa9a1a 100644
>>> --- a/drivers/net/pcap/rte_eth_pcap.c
>>> +++ b/drivers/net/pcap/rte_eth_pcap.c
>>> @@ -734,6 +734,12 @@ eth_dev_close(struct rte_eth_dev *dev)
>>>    	unsigned int i;
>>>    	struct pmd_internals *internals = dev->data->dev_private;
>>>    
>>> +	if (internals == NULL)
>>> +		return 0;
>>
>> Not sure if this check needed, can 'internals' be null at this stage?
> 
> I think yes we need to protect against a double close.
 >

'eth_dev_close()' can be called by 'pmd_pcap_remove()' or 
'rte_eth_dev_close()' both should be blocked to call 'eth_dev_close()' 
after first close.

And same thing for all PMDs, if they don't need, this one also shouldn't 
need.

> 
>> But perhaps need to add 'RTE_PROC_PRIMARY' check.
> 
> Yes but that's not the goal of this patch.
> 

Yes, the check should be there already, but now more stuff added to 
close dev_ops, and calling it from secondary will cause problem. Since 
you are already here, I think would be nice to add it.


>>> +	rte_free(dev->process_private);
>>
>> Can we move freeing 'process_private' to 'rte_eth_dev_release_port()'
> 
> Yes we could.
> 
>