Re: [dpdk-dev] [PATCH] net/tap: release port upon close

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 9/16/2020 8:20 AM, wangyunjian wrote:
>> -----Original Message-----
>> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
>> Sent: Tuesday, September 15, 2020 10:53 PM
>> To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org
>> Cc: thomas@monjalon.net; Lilijun (Jerry) <jerry.lilijun@huawei.com>; xudingke
>> <xudingke@huawei.com>
>> Subject: Re: [dpdk-dev] [PATCH] net/tap: release port upon close
>>
>> On 8/28/2020 1:37 PM, wangyunjian wrote:
>>> From: Yunjian Wang <wangyunjian@huawei.com>
>>>
>>> Set RTE_ETH_DEV_CLOSE_REMOVE upon probe so all the private resources
>>> for the port can be freed by rte_eth_dev_close().
>>>
>>> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
>>
>> <...>
>>
>>> @@ -1040,6 +1044,9 @@ tap_dev_close(struct rte_eth_dev *dev)
>>>   	struct pmd_process_private *process_private = dev->process_private;
>>>   	struct rx_queue *rxq;
>>>
>>> +	if (process_private == NULL)
>>> +		return;
>>
>> Why this check is required?
> 
> When user first call 'close()' and later call 'remove()' the tap PMD, in this case,
> the tap_dev_close() will be called twice. The second call of tap_dev_close()
> shouldn't do any process, we can use this check to return immediately.
> 


When first call is 'close()', it memset the 'eth_dev->data', so the in 
the later 'remove()' call, 'rte_eth_dev_allocated()' will always return 
NULL and 'remove()' will exit without calling 'close()'.

Also multiple 'close()' calls look safe, because of 
'RTE_ETH_VALID_PORTID_OR_RET()' checks is 'rte_eth_dev_close()'.

So, as far as I can see this additional check is not needed, but please 
double check.