[dpdk-dev] [PATCH v11 01/19] ethdev: add function to release port in local process

Andrew Rybchenko arybchenko at solarflare.com
Thu Jul 12 11:49:32 CEST 2018


On 12.07.2018 03:23, Zhang, Qi Z wrote:
>
>> -----Original Message-----
>> From: Andrew Rybchenko [mailto:arybchenko at solarflare.com]
>> Sent: Thursday, July 12, 2018 12:05 AM
>> To: Zhang, Qi Z <qi.z.zhang at intel.com>; thomas at monjalon.net; Burakov,
>> Anatoly <anatoly.burakov at intel.com>
>> Cc: Ananyev, Konstantin <konstantin.ananyev at intel.com>; dev at dpdk.org;
>> Richardson, Bruce <bruce.richardson at intel.com>; Yigit, Ferruh
>> <ferruh.yigit at intel.com>; Shelton, Benjamin H
>> <benjamin.h.shelton at intel.com>; Vangati, Narender
>> <narender.vangati at intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v11 01/19] ethdev: add function to release port
>> in local process
>>
>> On 11.07.2018 15:30, Zhang, Qi Z wrote:
>>>> -----Original Message-----
>>>> From: Andrew Rybchenko [mailto:arybchenko at solarflare.com]
>>>> Sent: Wednesday, July 11, 2018 5:27 PM
>>>> To: Zhang, Qi Z <qi.z.zhang at intel.com>; thomas at monjalon.net; Burakov,
>>>> Anatoly <anatoly.burakov at intel.com>
>>>> Cc: Ananyev, Konstantin <konstantin.ananyev at intel.com>; dev at dpdk.org;
>>>> Richardson, Bruce <bruce.richardson at intel.com>; Yigit, Ferruh
>>>> <ferruh.yigit at intel.com>; Shelton, Benjamin H
>>>> <benjamin.h.shelton at intel.com>; Vangati, Narender
>>>> <narender.vangati at intel.com>
>>>> Subject: Re: [dpdk-dev] [PATCH v11 01/19] ethdev: add function to
>>>> release port in local process
>>>>
>>>> On 11.07.2018 06:08, Qi Zhang wrote:
>>>>> Add driver API rte_eth_release_port_private to support the case when
>>>>> an ethdev need to be detached on a secondary process.
>>>>> Local state is set to unused and shared data will not be reset so
>>>>> the primary process can still use it.
>>>>>
>>>>> Signed-off-by: Qi Zhang <qi.z.zhang at intel.com>
>>>>> Reviewed-by: Andrew Rybchenko <arybchenko at solarflare.com>
>>>>> Acked-by: Remy Horton <remy.horton at intel.com>
>>>>> ---
>>> <...>
>>>>> +	/**
>>>>> +	 * PCI device can only be globally detached directly by a
>>>>> +	 * primary process. In secondary process, we only need to
>>>>> +	 * release port.
>>>>> +	 */
>>>>> +	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>>>>> +		return rte_eth_dev_release_port_private(eth_dev);
>>>> I've realized that some uninit functions which will not be called
>>>> anymore in secondary processes have check for process type and
>>>> handling of secondary process case. It makes code inconsistent and should
>> be fixed.
>>> Good point, I did a scan and check all the places that
>> rte_eth_dev_pci_generic_remove be involved.
>>> I found only sfc driver (sfc_eth_dev_unit) will call some cleanup on
>> secondary process as below.
>>
>> The patch makes impossible dev_uninit to be executed for secondary process
>> for all cases if rte_eth_dev_pci_generic_remove() is used. However, many
>> drivers still check for process type. Yes, sfc does cleanup, but some drivers
>> return -EPERM, some return 0. In fact it does not matter. It leaves dead code
>> which is really confusing.
> OK, l can do a cleanup in a separate patchset if this one will be merged.

For now, I'd like to revoke my Reviewed-by. I'll review once again when
complete solution is suggested.


More information about the dev mailing list