I have some comments inline.
2014-04-15 14:06, Neil Horman:
> Currently, physical device pmds use a separate initalization path
> (rte_pmd_init_all) while virtual devices use a constructor registration and
> rte_eal_dev_init. Theres no reason to have them be separate. This patch
> removes the vdev specific nomenclature from the vdev init path and makes it
> more generic for use with all pmds. This is the first step in converting
> the physical device pmds to using the same constructor based registration
> path that the virtual devices use
>
> Signed-off-by: Neil Horman <nhorman at tuxdriver.com>
> - if (rte_eal_vdev_init() < 0)
> + if (rte_eal_dev_init() < 0)
> rte_panic("Cannot init virtual devices\n");
You should update the panic log here.
> +/** Global list of virtual device drivers. */
> +static struct rte_driver_list dev_driver_list =
> + TAILQ_HEAD_INITIALIZER(dev_driver_list);
Same comment about "virtual device".
> + /* No need to register drivers that are embeded in DPDK
> + * (pmd_pcap, pmd_ring, ...). The initialization function have
> + * the ((constructor)) attribute so they will register at
> + * startup. */
Should we keep this comment?
> +#ifndef _RTE_VDEV_H_
> +#define _RTE_VDEV_H_
Should be _RTE_DEV_H_
> +/**
> + * @file
> + *
> + * RTE Virtual Devices Interface
> + *
> + * This file manages the list of the virtual device drivers.
> + */
Not only virtual.
> +/** Double linked list of virtual device drivers. */
[...]
> + * Initialization function called for each virtual device probing.
[...]
> +/**
> + * A structure describing a virtual device driver.
> + */
[...]
> + * Register a virtual device driver.
[...]
> + * Unregister a virtual device driver.
You probably understood the idea ;)
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> - if (rte_eal_vdev_init() < 0)
> + if (rte_eal_dev_init() < 0)
> rte_panic("Cannot init virtual devices\n");
Still "virtual" typo
Except typos, it seems a good step.
I think we could abstract more things in order to have even simpler API and
simpler command line. But we'll see it in another step.
Thanks
--
Thomas
