On Wed, 13 Dec 2017 17:58:53 +0100
Ricardo Roldan <[email protected]> wrote:
> Hi,
>
> We have a multi-process application and we need to support
> attaching/detaching of ports. We are using the 17.11 version with the
> Intel x520 (ixgbe driver) and virtio.
>
> At the time we initialize our processes there are not any devices binded
> with the DPDK drivers, so we initialize all processes (primary and
> secondaries) with 0 ports.
>
> This seems to work fine only on the primary processes, but on the
> secondary processes we see some problems. In the following paragraphs I
> describe the procedure used to attach/detach interfaces with DPDK.
>
> For the attach procedure (all processes initially have no devices
> attached):
>
> - Bind the devices we want to attach to the DPDK driver (with the script
> dpdk-devbind, from external process)
>
> - Primary process: Call rte_eth_dev_attach
>
> - Primary process: Configure ports using ...
>
> - Secondary processes: Call to rte_eth_dev_attach
>
>
> Start to send/receive packets from all processes.
>
>
> For the detach procedure:
>
> - Secondary processes: For each port, call rte_eth_dev_stop(port),
> rte_eth_dev_close(port) and rte_eth_dev_detach(port, dev).
>
> - Primary process: After the secondary processes have detach all their
> ports, for each port call rte_eth_dev_stop(port),
> rte_eth_dev_close(port) and rte_eth_dev_detach(port, dev).
>
> - Bind the device to the original Linux driver (with the script
> dpdk-devbind, from external process)
>
>
> With this approach we have noticed that when the secondary processes
> call rte_dev_detach there is an error, because it calls the remove
> operation, which ends up calling eth_ixgbe_dev_uninit that returns
> -EPERM (because it does not allow a non primary process to uninitialize
> the driver).
>
>
> Therefore, the port attach never works again on the secondary processes
> as the function rte_eal_hotplug_add fails because it cannot find the
> device.
>
>
> dev = bus->find_device(NULL, cmp_detached_dev_name, devname);
> if (dev == NULL) {
> RTE_LOG(ERR, EAL, "Cannot find unplugged device (%s)\n",
> devname);
> ret = -ENODEV;
> goto err_devarg;
> }
>
>
> This happens because in order to check unplugged devices, the function
> cmp_detached_dev_name checks if there is a pointer to the driver and
> fails if it is already set, because the detach procedure never set the
> driver variable to NULL.
>
> static int cmp_detached_dev_name(const struct rte_device *dev,
> const void *_name)
> {
> const char *name = _name;
>
> /* skip attached devices */
> RTE_LOG(ERR, EAL, "cmp_detached_dev_name dev %p name %s driver %p"
> " search %s\n",
> dev, dev->name, dev->driver, name);
> if (dev->driver != NULL)
> return 1;
>
> return strcmp(dev->name, name);
> }
>
> To fix this behavior we have done the following changes on the DPDK code.
>
> First, in order to prevent cmp_detached_dev_name from failing,
> rte_eal_dev_detach sets driver to NULL.
>
> diff --git a/lib/librte_eal/common/eal_common_dev.c
> b/lib/librte_eal/common/eal_common_dev.c
>
> index dda8f5835..9a363dcf7 100644
> --- a/lib/librte_eal/common/eal_common_dev.c
> +++ b/lib/librte_eal/common/eal_common_dev.c
> @@ -114,6 +114,7 @@ int rte_eal_dev_detach(struct rte_device *dev)
> if (ret)
> RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
> dev->name);
> + dev->driver = NULL;
> return ret;
> }/*
> */
>
>
> Then, in the rte_eth_dev_pci_generic_remove function, the call to
> dev_uninit does not consider -EPERM an error, because when detaching a
> port, some drivers return 0 and other drivers return -EPERM to indicate
> that it is called from a secondary process.
>
> diff --git a/lib/librte_ether/rte_ethdev_pci.h
> b/lib/librte_ether/rte_ethdev_pci.h
> @@ -184,7 +184,7 @@ rte_eth_dev_pci_generic_remove(struct rte_pci_device
> *pci_dev,
>
> if (dev_uninit) {
> ret = dev_uninit(eth_dev);
> - if (ret)
> + if (ret && ret != -EPERM)
> return ret;
> }
>
> Finally, in the rte_eth_dev_pci_release function, only the fields in the
> shared memory region are reset if called from a primary process.
>
> /**/diff --git a/lib/librte_ether/rte_ethdev_pci.h
> b/lib/librte_ether/rte_ethdev_pci.h
> index 722075e09..a79188fbf 100644
> --- a/lib/librte_ether/rte_ethdev_pci.h
> +++ b/lib/librte_ether/rte_ethdev_pci.h
> @@ -125,16 +125,16 @@ rte_eth_dev_pci_release(struct rte_eth_dev *eth_dev)
> /* free ether device */
> rte_eth_dev_release_port(eth_dev);
>
> - if (rte_eal_process_type() == RTE_PROC_PRIMARY)
> + if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> rte_free(eth_dev->data->dev_private);
> + eth_dev->data->dev_private = NULL;
>
> - eth_dev->data->dev_private = NULL;
> -
> - /*
> - * Secondary process will check the name to attach.
> - * Clear this field to avoid attaching a released ports.
> - */
> - eth_dev->data->name[0] = '\0';
> + /*
> + * Secondary process will check the name to attach.
> + * Clear this field to avoid attaching a released ports.
> + */
> + eth_dev->data->name[0] = '\0';
> + }
>
> eth_dev->device = NULL;
> eth_dev->intr_handle = NULL;
>
>
> After applying these changes, it seems like attaching and detaching
> ports multiple times works without problems, at least with the ixgbe and
> virtio drivers.
>
> In order to generate a patch, It would be very helpful if anyone can
> confirm that this approach is correct and that we do not break any other
> parts.
>
> Best regards,
> Ricardo
>
>
> On 12/13/2017 03:31 PM, Ricardo Roldan wrote:
> >
> >
> > Hi,
> >
> > We have a multi-process application and we need to support
> > attaching/detaching of ports.
> >
> > At the time we initialize our process they aren't any devices binded
> > with the dpdk drivers, so we initialize in all processes with 0 ports.
> >
> > We manage to work fine only on the primary processes, on the secondary
> > process we had some problems. Here is the way call the dpdk
> > interface to attach/detach from our process
> >
> > Attach - At this moment all process (primary and secondaries)
> > has not devices attached
> >
> > - Bind the devices we want to attach to the DPDK driver (with
> > the script dpdk-devbind, from external process)
> >
> > - Call rte_eth_dev_attach from primary process
> >
> > - Configure ports from primary process
> >
> > - From secondary process call to rte_eth_dev_attach
> >
> >
> > Began to send/receive packets from all process
> >
> >
> >
> > Detach
> >
> > - From secondary process call to:
> >
> > rte_eth_dev_stop(port);
> >
> > rte_eth_dev_close(port);
> > rte_eth_dev_detach(port, dev);
> >
> >
> > - From primary process call to:
> >
> > rte_eth_dev_stop(port);
> >
> > rte_eth_dev_close(port);
> > rte_eth_dev_detach(port, dev);
> >
> >
> > - Bind the device to the original Linux driver (with the script
> > dpdk-devbind, from external process)
> >
> >
> > With this second approach we notice that the detach from the
> > secondary process returns with an error. This is because the function
> > eth_ixgbe_dev_uninit has a check to not uninitialize the driver from a
> > not primary process.
> >
> > So that the detach can't finish all its task.
> >
> > After this, the attach never works again on the secondary process
> > as the function rte_eal_hotplug_add began to fail due that it cant not
> > find the device.
> >
> > */ dev = bus->find_device(NULL, cmp_detached_dev_name,
> > devname); /**/
> > /**/ if (dev == NULL) { /**/
> > /**/ RTE_LOG(ERR, EAL, "Cannot find unplugged device
> > (%s)\n", /**/
> > /**/ devname); /**/
> > /**/ ret = -ENODEV; /**/
> > /**/ goto err_devarg; /**/
> > /**/ } /**/
> > /**//*
> > This is due that to check unplugged devices, what the comparation
> > function cmp_detached_dev_name do, is to check if there is a pointer
> > to the driver.
> >
> > */static int cmp_detached_dev_name(const struct rte_device *dev, /**/
> > /**/ const void *_name) /**/
> > /**/{ /**/
> > /**/ const char *name = _name; /**/
> > /**//**/
> > /**/ /* skip attached devices */ /**/
> > /**/ if (dev->driver != NULL) /**/
> > /**/ return 1; /**/
> > /**//**/
> > /**/ return strcmp(dev->name, name); /**/
> > /**/} /*/
> > /
> >
> > And as detach has not finish correctly on the secondary process, the
> > device continues with the pointer to the driver setted. So the attach
> > fails as the device is not found.
> >
> >
> > To overcome this behavior we had done this changes on the DPDK code:
> >
> > We have modify the dpdk to clean the pointer to the driver on the
> > detach.We had modify also the function rte_eth_dev_pci_generic_remove
> > so even if the uninit
> > of the driver return with -EPERM the function continue executing the
> > rest of the code. We had done this as we had seen that the check on
> > the uninit testing to
> > see if the process is not a primary is donein all drivers, but some
> > drivers return with no error ( 0) and others with (-EPERM). So on
> > rte_eth_dev_pci_generic if the call
> > to uninit returns with -EPERM we continue executing calling
> > rte_eth_dev_pci_release. To that last function we had also done some
> > changes, as only primary process
> > should be able to uninitialized some common values, that a detach on a
> > secondary process should never do.
> >
> > These are the changes:
> >
> > /*diff --git a/lib/librte_eal/common/eal_common_dev.c
> > b/lib/librte_eal/common/eal_common_dev.c*//*
> > *//*index dda8f5835..9a363dcf7 100644*//*
> > *//*--- a/lib/librte_eal/common/eal_common_dev.c*//*
> > *//*+++ b/lib/librte_eal/common/eal_common_dev.c*//*
> > *//*@@ -114,6 +114,7 @@ int rte_eal_dev_detach(struct rte_device *dev)*//*
> > *//* if (ret)*//*
> > *//* RTE_LOG(ERR, EAL, "Driver cannot detach the device
> > (%s)\n",*//*
> > *//* dev->name);*//*
> > *//*+ dev->driver = NULL;*//*
> > *//* return ret;*//*
> > *//* }*//*
> > *//**//*
> > *//*diff --git a/lib/librte_ether/rte_ethdev_pci.h
> > b/lib/librte_ether/rte_ethdev_pci.h*//*
> > *//*index 722075e09..a79188fbf 100644*//*
> > *//*--- a/lib/librte_ether/rte_ethdev_pci.h*//*
> > *//*+++ b/lib/librte_ether/rte_ethdev_pci.h*//*
> > *//*@@ -125,16 +125,16 @@ rte_eth_dev_pci_release(struct rte_eth_dev
> > *eth_dev)*//*
> > *//* /* free ether device */*//*
> > *//* rte_eth_dev_release_port(eth_dev);*//*
> > *//**//*
> > *//*- if (rte_eal_process_type() == RTE_PROC_PRIMARY)*//*
> > *//*+ if (rte_eal_process_type() == RTE_PROC_PRIMARY) {*//*
> > *//*rte_free(eth_dev->data->dev_private);*//*
> > *//*+ eth_dev->data->dev_private = NULL;*//*
> > *//**//*
> > *//*- eth_dev->data->dev_private = NULL;*//*
> > *//*-*//*
> > *//*- /**//*
> > *//*- * Secondary process will check the name to attach.*//*
> > *//*- * Clear this field to avoid attaching a released ports.*//*
> > *//*- */*//*
> > *//*- eth_dev->data->name[0] = '\0';*//*
> > *//*+ /**//*
> > *//*+ * Secondary process will check the name to
> > attach.*//*
> > *//*+ * Clear this field to avoid attaching a released
> > ports.*//*
> > *//*+ */*//*
> > *//*+ eth_dev->data->name[0] = '\0';*//*
> > *//*+ }*//*
> > *//**//*
> > *//* eth_dev->device = NULL;*//*
> > *//* eth_dev->intr_handle = NULL;*//*
> > *//*@@ -184,7 +184,7 @@ rte_eth_dev_pci_generic_remove(struct
> > rte_pci_device *pci_dev,*//*
> > *//**//*
> > *//* if (dev_uninit) {*//*
> > *//* ret = dev_uninit(eth_dev);*//*
> > *//*- if (ret)*//*
> > *//*+ if (ret && ret != -EPERM)*//*
> > *//* return ret;*//*
> > *//* }*//*
> > *//**//*
> > */
> >
> >
> > And now seems to work. Is this a correct way to proceed?
> >
> > Could some one that have work with this functionalities can advise us?
> >
> > Best regards,
> >
> > Ricardo
> >
> >
>
Many DPDK drivers require that setup and initialization be done by
the primary process. This is mostly to avoid dealing with concurrency since
there can be multiple secondary processes.
