Hi Jan, Some comments.
On Saturday 09 July 2016 12:39 AM, Jan Viktorin wrote: > Move all PMD_VDEV-specific code into a separate module and header > file to not polute the generic code anymore. There is now a list > of virtual devices available. > > The rte_vdev_driver integrates the original rte_driver inside > (C inheritance). The rte_driver will be however change in the > future to serve as a common base for all other types of drivers. > > The existing PMDs (PMD_VDEV) are to be modified later (there is > no change for them at the moment). > > There is however a inconsistency. The functions rte_eal_vdev_init > and rte_eal_vdev_uninit are still placed in the rte_dev.h (instead > of the rte_vdev.h). > > Signed-off-by: Jan Viktorin <viktorin at rehivetech.com> > --- > lib/librte_eal/bsdapp/eal/Makefile | 1 + > lib/librte_eal/common/Makefile | 2 +- > lib/librte_eal/common/eal_common_dev.c | 54 +--------------- > lib/librte_eal/common/eal_common_vdev.c | 104 > +++++++++++++++++++++++++++++++ > lib/librte_eal/common/include/rte_dev.h | 1 + > lib/librte_eal/common/include/rte_vdev.h | 83 ++++++++++++++++++++++++ > lib/librte_eal/linuxapp/eal/Makefile | 1 + > 7 files changed, 192 insertions(+), 54 deletions(-) > create mode 100644 lib/librte_eal/common/eal_common_vdev.c > create mode 100644 lib/librte_eal/common/include/rte_vdev.h > > diff --git a/lib/librte_eal/bsdapp/eal/Makefile > b/lib/librte_eal/bsdapp/eal/Makefile > index 698fa0a..b7e94a4 100644 > --- a/lib/librte_eal/bsdapp/eal/Makefile > +++ b/lib/librte_eal/bsdapp/eal/Makefile [...] > diff --git a/lib/librte_eal/common/eal_common_vdev.c > b/lib/librte_eal/common/eal_common_vdev.c > new file mode 100644 > index 0000000..ea83c41 > --- /dev/null > +++ b/lib/librte_eal/common/eal_common_vdev.c > @@ -0,0 +1,104 @@ > +/*- > + * BSD LICENSE > + * > + * Copyright(c) 2016 RehiveTech. All rights reserved. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * > + * * Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * * Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in > + * the documentation and/or other materials provided with the > + * distribution. > + * * Neither the name of RehiveTech nor the names of its > + * contributors may be used to endorse or promote products derived > + * from this software without specific prior written permission. > + * > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > + */ > + > +#include <string.h> > +#include <sys/queue.h> > +#include <rte_vdev.h> > + > +struct vdev_driver_list vdev_driver_list = > + TAILQ_HEAD_INITIALIZER(vdev_driver_list); > + > +/* register a driver */ > +void > +rte_eal_vdrv_register(struct rte_vdev_driver *driver) > +{ > + TAILQ_INSERT_TAIL(&vdev_driver_list, driver, next); > +} > + > +/* unregister a driver */ > +void > +rte_eal_vdrv_unregister(struct rte_vdev_driver *driver) > +{ > + TAILQ_REMOVE(&vdev_driver_list, driver, next); > +} > + > +int > +rte_eal_vdev_init(const char *name, const char *args) > +{ > + struct rte_vdev_driver *driver; > + > + if (name == NULL) > + return -EINVAL; > + > + TAILQ_FOREACH(driver, &vdev_driver_list, next) { > + if (driver->driver.type != PMD_VDEV) > + continue; Now that two separate lists for vdev and pdev exist, we don't need this check anymore. In fact, PMD_VDEV might not even exist. > + > + /* > + * 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->driver.name, name, > strlen(driver->driver.name))) > + return driver->driver.init(name, args); > + } > + > + RTE_LOG(ERR, EAL, "no driver found for %s\n", name); > + return -EINVAL; > +} > + > +int > +rte_eal_vdev_uninit(const char *name) > +{ > + struct rte_vdev_driver *driver; > + > + if (name == NULL) > + return -EINVAL; > + > + TAILQ_FOREACH(driver, &vdev_driver_list, next) { > + if (driver->driver.type != PMD_VDEV) > + continue; Same as above, redundant check. > + > + /* > + * 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->driver.name, name, > strlen(driver->driver.name))) > + return driver->driver.uninit(name); > + } > + > + RTE_LOG(ERR, EAL, "no driver found for %s\n", name); > + return -EINVAL; > +} > diff --git a/lib/librte_eal/common/include/rte_dev.h > b/lib/librte_eal/common/include/rte_dev.h > index b1c0520..2aeb752 100644 > --- a/lib/librte_eal/common/include/rte_dev.h > +++ b/lib/librte_eal/common/include/rte_dev.h > @@ -210,6 +210,7 @@ static void devinitfn_ ##d(void)\ > rte_eal_driver_register(&d);\ > } > > + Probably a stray newline. > #ifdef __cplusplus > } > #endif > diff --git a/lib/librte_eal/common/include/rte_vdev.h > b/lib/librte_eal/common/include/rte_vdev.h > new file mode 100644 > index 0000000..523bd92 > --- /dev/null > +++ b/lib/librte_eal/common/include/rte_vdev.h > @@ -0,0 +1,83 @@ > +/*- > + * BSD LICENSE > + * > + * Copyright(c) 2016 RehiveTech. All rights reserved. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * > + * * Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * * Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in > + * the documentation and/or other materials provided with the > + * distribution. > + * * Neither the name of RehiveTech nor the names of its > + * contributors may be used to endorse or promote products derived > + * from this software without specific prior written permission. > + * > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > + */ > + > +#ifndef RTE_VDEV_H > +#define RTE_VDEV_H > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > +#include <sys/queue.h> > +#include <rte_dev.h> > + > +/** Double linked list of virtual device drivers. */ > +TAILQ_HEAD(vdev_driver_list, rte_vdev_driver); > + > +/** > + * A virtual device driver abstraction. > + */ > +struct rte_vdev_driver { > + TAILQ_ENTRY(rte_vdev_driver) next; /**< Next in list. */ > + struct rte_driver driver; /**< Inherited general driver. */ > +}; > + > +/** > + * Register a virtual device driver. > + * > + * @param driver > + * A pointer to a rte_vdev_driver structure describing the driver > + * to be registered. > + */ > +void rte_eal_vdrv_register(struct rte_vdev_driver *driver); > + > +/** > + * Unregister a virtual device driver. > + * > + * @param driver > + * A pointer to a rte_vdev_driver structure describing the driver > + * to be unregistered. > + */ > +void rte_eal_vdrv_unregister(struct rte_vdev_driver *driver); > + > +#define RTE_EAL_VDRV_REGISTER(d)\ In the recent commits, I noticed that macros have taken the (name, driver) format. PMD_REGISTER_DRIVER() (now redundant), DRIVER_REGISTER_PCI_TABLE() ... etc It might be better to stick to the same format. > +RTE_INIT(vdrvinitfn_ ##d);\ > +static void vdrvinitfn_ ##d(void)\ > +{\ > + rte_eal_vdrv_register(&d);\ > +} > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif > diff --git a/lib/librte_eal/linuxapp/eal/Makefile > b/lib/librte_eal/linuxapp/eal/Makefile > index 30b30f3..9553e97 100644 > --- a/lib/librte_eal/linuxapp/eal/Makefile > +++ b/lib/librte_eal/linuxapp/eal/Makefile > @@ -85,6 +85,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_timer.c > SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_memzone.c > SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_log.c > SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_launch.c > +SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_vdev.c > SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_pci.c > SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_pci_uio.c > SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_memory.c >