[dpdk-dev] [PATCH v11 12/13] eal/pci: Add rte_eal_dev_attach/detach() functions

Maxime Leroy maxime.leroy at 6wind.com
Mon Feb 23 14:29:08 CET 2015


Hi Tetsuya,

On Mon, Feb 23, 2015 at 6:09 AM, Tetsuya Mukawa <mukawa at igel.co.jp> wrote:
> These functions are used for attaching or detaching a port.
[...]
>
> +static int
> +rte_eal_vdev_init(const char *name, const char *args)
> +{
> +       struct rte_driver *driver;
> +
> +       if (name == NULL)
> +               return -EINVAL;
> +
> +       TAILQ_FOREACH(driver, &dev_driver_list, next) {
> +               if (driver->type != PMD_VDEV)
> +                       continue;
> +
> +               /*
> +                * search a driver prefix in virtual device name.
> +                * For example, if the driver is pcap PMD, driver->name
> +                * will be "eth_pcap", but "name" will be "eth_pcapN".
> +                * So use strncmp to compare.
> +                */
> +               if (!strncmp(driver->name, name, strlen(driver->name))) {
> +                       driver->init(name, args);
> +                       break;

Please return the value given by init: return driver->init(name, args); .

> +               }
> +       }
> +
> +       if (driver == NULL) {
> +               RTE_LOG(WARNING, EAL, "no driver found for %s\n", name);
--> should be : RTE_LOG(ERR .


> +               return -EINVAL;
> +       }
> +       return 0;
> +}
> +
>  int
>  rte_eal_dev_init(void)
>  {
> @@ -79,23 +113,10 @@ rte_eal_dev_init(void)
>                 if (devargs->type != RTE_DEVTYPE_VIRTUAL)
>                         continue;
>
> -               TAILQ_FOREACH(driver, &dev_driver_list, next) {
> -                       if (driver->type != PMD_VDEV)
> -                               continue;
> -
> -                       /* search a driver prefix in virtual device name */
> -                       if (!strncmp(driver->name, devargs->virtual.drv_name,
> -                                       strlen(driver->name))) {
> -                               driver->init(devargs->virtual.drv_name,
> -                                       devargs->args);
> -                               break;
> -                       }
> -               }
> -
> -               if (driver == NULL) {
> +               if (rte_eal_vdev_init(devargs->virtual.drv_name,
> +                                       devargs->args))
>                         rte_panic("no driver found for %s\n",
>                                   devargs->virtual.drv_name);
instead of that:

if (rte_eal_vdev_init(devargs->virtual.drv_name, devargs->args)) {
          RTE_LOG(ERR, "failed to initialize %s device\n",
devargs->virtual.drv_name);
          return -1;
}

> -               }
>         }
>
>         /* Once the vdevs are initalized, start calling all the pdev drivers */
> @@ -107,3 +128,237 @@ rte_eal_dev_init(void)
>         }
>         return 0;
>  }
> +
> +/* So far, DPDK hotplug function only supports linux */
> +#ifdef RTE_LIBRTE_EAL_HOTPLUG
> +static int
> +rte_eal_vdev_uninit(const char *name)
> +{
> +       struct rte_driver *driver;
> +
> +       if (name == NULL)
> +               return -EINVAL;
> +
> +       TAILQ_FOREACH(driver, &dev_driver_list, next) {
> +               if (driver->type != PMD_VDEV)
> +                       continue;
> +
> +               /*
> +                * search a driver prefix in virtual device name.
> +                * For example, if the driver is pcap PMD, driver->name
> +                * will be "eth_pcap", but "name" will be "eth_pcapN".
> +                * So use strncmp to compare.
> +                */
> +               if (!strncmp(driver->name, name, strlen(driver->name))) {
> +                       driver->uninit(name);

Please return the value given by uninit: return driver->uninit(name, args);

> +                       break;
> +               }
> +       }
> +
> +       if (driver == NULL) {
> +               RTE_LOG(WARNING, EAL, "no driver found for %s\n", name);
> +               return 1;

As it's an error, the function should return a negative value ( i.e. -EINVAL).
Please set the log level to ERR.

> +       }
> +       return 0;
> +}
> +
[...]
> +}
> +
> +/* attach the new virtual device, then store port_id of the device */
> +static int
> +rte_eal_dev_attach_vdev(const char *vdevargs, uint8_t *port_id)
> +{
> +       char *name = NULL, *args = NULL;
> +       uint8_t new_port_id;
> +       struct rte_eth_dev devs[RTE_MAX_ETHPORTS];
> +       int ret = -1;
> +
> +       if ((vdevargs == NULL) || (port_id == NULL))
> +               goto end;
> +
> +       /* parse vdevargs, then retrieve device name and args */
> +       if (rte_eal_parse_devargs_str(vdevargs, &name, &args))
> +               goto end;
> +
> +       /* save current port status */
> +       if (rte_eth_dev_save(devs, sizeof(devs)))
> +               goto end;
> +       /* walk around dev_driver_list to find the driver of the device,
> +        * then invoke probe function o the driver.
> +        * TODO:
> +        * rte_eal_vdev_init() should return port_id,
> +        * And rte_eth_dev_save() and rte_eth_dev_get_changed_port()
> +        * should be removed. */
> +       if (rte_eal_vdev_init(name, args))
> +               goto end;
> +       /* get port_id enabled by above procedures */
> +       if (rte_eth_dev_get_changed_port(devs, &new_port_id))
> +               goto end;
> +       ret = 0;
> +

Please set port_id here, i.e. :  *port_id = new_port_id;

> +end:
> +       if (name)
> +               free(name);
> +       if (args)
> +               free(args);
> +
> +       *port_id = new_port_id;

and not here.


> +       if (ret < 0)
> +               RTE_LOG(ERR, EAL, "Driver, cannot attach the device\n");
> +       return ret;
> +}
> +
e\n");
> +       return -1;
> +}
> +

Regards,

Maxime


More information about the dev mailing list