> -----Original Message-----
> From: Guo, Jia
> Sent: Sunday, May 28, 2017 11:45 PM
> To: Zhang, Helin <helin.zh...@intel.com>; Wu, Jingjing
> <jingjing...@intel.com>; Richardson,
> Bruce <bruce.richard...@intel.com>; Ananyev, Konstantin
> <konstantin.anan...@intel.com>;
> Liu, Yuanhan <yuanhan....@intel.com>; gaetan.ri...@6wind.com
> Cc: dev@dpdk.org; Guo, Jia <jia....@intel.com>
> Subject: [dpdk-dev] [RFC] Add hot plug event in rte eal interrupt and
> inplement it in i40e
> driver.
>
> For HW hotplug feature, we had already got upstream that removal event adding
> from 6wind
> as bellow.
>
> dependency of “add device removal event” patch set:
> http://dpdk.org/dev/patchwork/patch/23693/
> [dpdk-dev,v2,1/5] ethdev: introduce device removal event
> http://dpdk.org/dev/patchwork/patch/23694/
> [dpdk-dev,v2,2/5] net/mlx4: device removal event support
> http://dpdk.org/dev/patchwork/patch/23695/
> [dpdk-dev,v2,3/5] app/testpmd: generic event handler
> http://dpdk.org/dev/patchwork/patch/23696/
> [dpdk-dev,v2,4/5] app/testpmd: request link status interrupt
> http://dpdk.org/dev/patchwork/patch/23697/
> [dpdk-dev,v2,5/5] app/testpmd: request device removal interrupt
>
> From the patches, we can see a new event type “RTE_ETH_DEV_INTR_RMV” has been
> added
> into the ethdev, and the event has been implemented in mlx4 driver, and
> Testpmd be use for
> testing purposes and as a practical usage example for how to use these event.
> The progress is
> use the mlx4 driver to register interrupt callback function to rte eal
> interrupt source, and
> when rte epolling detect the IBV_EVENT_DEVICE_FATAL , which is identify the
> device remove
> behavior, it will callback into the driver’s interrupt handle to handle it,
> and then callback to
> the user app, such as testpmd, to detach the pci device.
>
> So far, except the mlx4 driver, other driver like i40, that not have the
> remove interrupt from
> hw, will not be able to monitoring the remove behavior, so in order to expand
> the hot plug
> feature for all driver cases, something must be done ot detect the remove
> event at the kernel
> level and offer a new line of interrupt to the userland. The idea is coming
> as below.
>
> Use I40e as example, we know that the rmv interrupt is not added in hw, but
> we could
> monitor the uio file descriptor to catch the device remove event as bellow.
>
> The info of uevent form FD monitoring:
> remove@/devices/pci0000:80/0000:80:02.2/0000:82:00.0/0000:83:03.0/0000:84:00.2/uio
> /uio2
> ACTION=remove
> DEVPATH=/devices/pci0000:80/0000:80:02.2/0000:82:00.0/0000:83:03.0/0000:84:00.2/ui
> o/uio2
> SUBSYSTEM=uio
> MAJOR=243
> MINOR=2
> DEVNAME=uio2
> SEQNUM=11366
>
> Firstly, in order to monitor the uio file descriptor, we need to create
> socket to monitor the uio
> fd, that is defined as “hotplug_fd”, and then add the uio fd into the epoll
> fd list, rte eal could
> epoll all of the interrupt event from hw interrupt and also include the
> uevent from
> hotplug_fd.
>
> Secondly, in order to read out the uevent that monitoring, we need to add
> uevent API in rte
> layer. We plan add 2 , rte_uevent_connect and rte_get_uevent. All driver
> interrupt handler
> could use these API to enable the uevent monitoring, and read out the uevent
> type , then
> corresponding to handle these uevent, such as detach the device when get the
> remove type.
>
> Signed-off-by: Jeff Guo <jia....@intel.com>
> ---
> drivers/net/i40e/i40e_ethdev.c | 15 +++
> lib/librte_eal/linuxapp/eal/eal_interrupts.c | 146
> ++++++++++++++++++++-
> .../linuxapp/eal/include/exec-env/rte_interrupts.h | 32 +++++
> 3 files changed, 191 insertions(+), 2 deletions(-)
>
It will be better to split the patch to two sub patches, one is for eal change,
the other is for driver enabling.
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 4c49673..0336f82 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -66,6 +66,8 @@
> #include "i40e_pf.h"
> #include "i40e_regs.h"
>
> +extern int hotplug_fd;
> +
> #define ETH_I40E_FLOATING_VEB_ARG "enable_floating_veb"
> #define ETH_I40E_FLOATING_VEB_LIST_ARG "floating_veb_list"
>
> @@ -5808,8 +5810,21 @@ i40e_dev_interrupt_handler(void *param)
> {
> struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
> struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> + struct rte_uevent event;
> uint32_t icr0;
>
> + /* check device uevent */
> + while (rte_get_uevent(hotplug_fd, &event) > 0) {
The hotplug_fd is used as uevent fd for all devices, right? but in i40e driver,
how distinguish the event is to this dev?
I saw you check the src in eal_intr_process_interrupts, but you didn't assign
the fd in i40e device's intr_handle.
Is that to say all driver's callback will be triggered?
> + if (event.subsystem == 1) {
What is the 1 meaning?
> + if (event.action == RTE_UEVENT_ADD) {
> + //do nothing here
Will you plan do anything later? Such as RTE_ETH_EVENT_INTR_NEW? If no, please
remove it.
And {} can be omit.
> + } else if (event.action == RTE_UEVENT_REMOVE) {
> + _rte_eth_dev_callback_process(dev,
> + RTE_ETH_EVENT_INTR_RMV, NULL);
> + }
> + }
> +
> + /* connection closed */
> + if (ret == 0) {
> + return -1;
> + }
{} can be omit. Serval in this patch. Please check.
> +/**
> + * It read out the uevent from the specific file descriptor.
> + *
> + * @param fd
> + * The fd which the uevent associated to
> + * @param uevent
> + * Pointer to the uevent which read from the monitoring fd.
> + * @return
> + * - On success, one.
> + * - On failure, zeor or a negative value.
Zeor -> zero
Generally speaking, we are using negative value to indicate failure, and 0
indicate success. Expect the result has more than two options (success,
failure).
> +int
> +rte_get_uevent(int fd, struct rte_uevent *uevent);
> +
> +/**
> + * Connect to the device uevent file descriptor.
> + * @return
> + * return the connected uevent fd.
> + */
Any return code for failure?
Thanks
Jingjing
