[v2,1/3] net/ixgbe: enable hotplug detect in ixgbe

Message ID 1531119413-17298-2-git-send-email-jia.guo@intel.com (mailing list archive)
State Superseded, archived
Headers
Series Enable eal hotplug event detect for i40e/ixgbe |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK

Commit Message

Guo, Jia July 9, 2018, 6:56 a.m. UTC
  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(-)
  

Comments

Wenzhuo Lu July 9, 2018, 7:38 a.m. UTC | #1
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?
  
Matan Azrad July 9, 2018, 7:51 a.m. UTC | #2
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.
  
Andrew Rybchenko July 9, 2018, 8:13 a.m. UTC | #3
On 09.07.2018 09:56, Jeff Guo wrote:
> 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.

I see nothing ixgbe specific in the callback. Yes, support of removal
event should be in drv_flags, but it looks like the callback may be
generic and located in ethdev.

Also search of the device by name could be done using querying
mechanism to be added by Gaetan [1].

[1] https://patches.dpdk.org/project/dpdk/list/?series=419
  
Guo, Jia July 9, 2018, 8:46 a.m. UTC | #4
On 7/9/2018 4:13 PM, Andrew Rybchenko wrote:
> On 09.07.2018 09:56, Jeff Guo wrote:
>> 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.
>
> I see nothing ixgbe specific in the callback. Yes, support of removal
> event should be in drv_flags, but it looks like the callback may be
> generic and located in ethdev.
>

Let it be generic and located in ethdev should be a good idea.

> Also search of the device by name could be done using querying
> mechanism to be added by Gaetan [1].
>
> [1] https://patches.dpdk.org/project/dpdk/list/?series=419

here, i just want to check if the eth port is belong to the removal device.
  
Guo, Jia July 9, 2018, 8:57 a.m. UTC | #5
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.

>
  
Matan Azrad July 9, 2018, 9:04 a.m. UTC | #6
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?
  
Guo, Jia July 9, 2018, 9:54 a.m. UTC | #7
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);
  
Matan Azrad July 9, 2018, 10:01 a.m. UTC | #8
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?
  
Guo, Jia July 9, 2018, 10:14 a.m. UTC | #9
On 7/9/2018 6:01 PM, Matan Azrad wrote:
> 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?

the step should be:
1) add a ethdev driver api "rte_dev_event_callback_register" in the 
rte_ethdev_driver.h, let pmd driver call it.
      rte_eth_dev_event_callback(char *device_name, enum 
rte_dev_event_type event, void *cb_arg);

2) register eth eal device event callback in pmd driver as below, the 
rte eth (eth_dev) could be set to cb_arg of the callback.

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

3)when hotplug event detect, the callback be called, then could be use 
the device name of the eth_dev to compare the hotplug eal device name.

4) go to the common way of RMV eth dev hotplug process.
  

Patch

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;
+	}
+}
+
 /*
  * Virtual Function device init
  */
@@ -1678,6 +1719,9 @@  eth_ixgbevf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_intr_enable(intr_handle);
 	ixgbevf_intr_enable(eth_dev);
 
+	/* register the device event callback */
+	rte_dev_event_callback_register(NULL, eth_dev_event_callback, NULL);
+
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id, "ixgbe_mac_82599_vf");
@@ -1801,7 +1845,7 @@  static int eth_ixgbe_pci_remove(struct rte_pci_device *pci_dev)
 static struct rte_pci_driver rte_ixgbe_pmd = {
 	.id_table = pci_id_ixgbe_map,
 	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
-		     RTE_PCI_DRV_IOVA_AS_VA,
+		     RTE_PCI_DRV_IOVA_AS_VA | RTE_PCI_DRV_INTR_RMV,
 	.probe = eth_ixgbe_pci_probe,
 	.remove = eth_ixgbe_pci_remove,
 };