Re: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect in ixgbe

[Jeff Guo <jia.guo@intel.com>]

On 7/9/2018 5:04 PM, Matan Azrad wrote:
> Hi
>
> From: Jeff Guo
>> hi, wenzhuo and matan.
>>
>>
>> On 7/9/2018 3:51 PM, Matan Azrad wrote:
>>> Hi
>>>
>>> From: Lu, Wenzhuo
>>>> Hi Jeff,
>>>>
>>>>> -----Original Message-----
>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jeff Guo
>>>>> Sent: Monday, July 9, 2018 2:57 PM
>>>>> To: stephen@networkplumber.org; Richardson, Bruce
>>>>> <bruce.richardson@intel.com>; Yigit, Ferruh
>>>>> <ferruh.yigit@intel.com>; Ananyev, Konstantin
>>>>> <konstantin.ananyev@intel.com>; gaetan.rivet@6wind.com; Wu,
>> Jingjing
>>>>> <jingjing.wu@intel.com>; thomas@monjalon.net;
>> motih@mellanox.com;
>>>>> matan@mellanox.com;
>>>> Van
>>>>> Haaren, Harry <harry.van.haaren@intel.com>; Zhang, Qi Z
>>>>> <qi.z.zhang@intel.com>; He, Shaopeng <shaopeng.he@intel.com>;
>>>>> Iremonger, Bernard <bernard.iremonger@intel.com>;
>>>>> arybchenko@solarflare.com
>>>>> Cc: jblunck@infradead.org; shreyansh.jain@nxp.com; dev@dpdk.org;
>>>>> Guo, Jia <jia.guo@intel.com>; Zhang, Helin <helin.zhang@intel.com>
>>>>> Subject: [dpdk-dev] [PATCH v2 1/3] net/ixgbe: enable hotplug detect
>>>>> in ixgbe
>>>>>
>>>>> This patch aim to enable hotplug detect in ixgbe pmd driver. Firstly
>>>>> it set the flags RTE_PCI_DRV_INTR_RMV in drv_flags to announce the
>>>>> hotplug ability, and then use rte_dev_event_callback_register to
>>>>> register the hotplug event callback to eal. When eal detect the
>>>>> hotplug event, it will call the callback to process it, if the event
>>>>> is hotplug remove, it will trigger the RTE_ETH_EVENT_INTR_RMV event
>>>>> into ethdev callback to let app process the hotplug for the ethdev.
>>>>>
>>>>> This is an example for other driver, that if any driver support
>>>>> hotplug feature could be use this way to enable hotplug detect.
>>>>>
>>>>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
>>>>> ---
>>>>> v2->v1:
>>>>> refine some doc.
>>>>> ---
>>>>>    drivers/net/ixgbe/ixgbe_ethdev.c | 46
>>>>> +++++++++++++++++++++++++++++++++++++++-
>>>>>    1 file changed, 45 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> index 87d2ad0..83ce026 100644
>>>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>>>>> @@ -1534,6 +1534,47 @@ generate_random_mac_addr(struct
>> ether_addr
>>>>> *mac_addr)
>>>>>    	memcpy(&mac_addr->addr_bytes[3], &random, 3);  }
>>>>>
>>>>> +static void
>>>>> +eth_dev_event_callback(char *device_name, enum
>> rte_dev_event_type
>>>>> type,
>>>>> +		       __rte_unused void *arg)
>>>>> +{
>>>>> +	uint32_t pid;
>>>>> +
>>>>> +	if (type >= RTE_DEV_EVENT_MAX) {
>>>>> +		fprintf(stderr, "%s called upon invalid event %d\n",
>>>>> +			__func__, type);
>>>>> +		fflush(stderr);
>>>>> +	}
>>>>> +
>>>>> +	switch (type) {
>>>>> +	case RTE_DEV_EVENT_REMOVE:
>>>>> +		PMD_DRV_LOG(INFO, "The device: %s has been
>>>> removed!\n",
>>>>> +			    device_name);
>>>>> +
>>>>> +		if (!device_name)
>>>>> +			return;
>>>>> +
>>>>> +		for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) {
>>>>> +			if (rte_eth_devices[pid].device) {
>>>>> +				if (!strcmp(device_name,
>>>>> +				    rte_eth_devices[pid].device->name)) {
>>>>> +					_rte_eth_dev_callback_process(
>>>>> +						&rte_eth_devices[pid],
>>>>> +						RTE_ETH_EVENT_INTR_RMV,
>>>>> NULL);
>>>>> +					continue;
>>>>> +				}
>>>>> +			}
>>>>> +		}
>>>>> +		break;
>>>>> +	case RTE_DEV_EVENT_ADD:
>>>>> +		RTE_LOG(INFO, EAL, "The device: %s has been added!\n",
>>>>> +			device_name);
>>>>> +		break;
>>>>> +	default:
>>>>> +		break;
>>>>> +	}
>>>>> +}
>>>> I don't get the point. Looks like this's a very common rte code. Why
>>>> is it put in ixgbe pmd?
>>> Jeff needs to detect if the removed device is related to this PMD, than to
>> raise RMV events for all this PMD ethdev associated ports.
>>> He should not raise RMV events for other PMD  ports.
>>>
>> It should be like wenzhuo said that i could no strong reason to let common
>> way in ixgbe pmd.  And sure raise RMV events for none related PMD ports is
>> not my hope.
>> Will plan to let it go into the eth dev layer to process it.
>>
> How can you run ethdev function from EAL context?
> How can the ethdev layer know which ports are related to the EAL device removal?
> How can ethdev layer know if the port supports removal?

i mean that still let driver manage the callback , just let the common 
ethdev functional in ethdev layer.
It just define "rte_eth_dev_event_callback" in ethdev layer, and 
register the common ethdev callback in pmd driver as bellow. the eth_dev 
could be pass by the whole process.

     rte_dev_event_callback_register(eth_dev->device->name,
                     rte_eth_dev_event_callback,
                     (void *)eth_dev);