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

[Matan Azrad <matan@mellanox.com>]

Hi

From: Jeff Guo
> 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);
> 

Sorry, but I don't understand, can you explain step by step the notification path?