2015-02-16 13:14, Tetsuya Mukawa: > The patch adds function pointer to rte_pci_driver and eth_driver > structure. These function pointers are used when ports are detached. > Also, the patch adds rte_eth_dev_uninit(). So far, it's not called > by anywhere, but it will be called when port hotplug function is > implemented. > > v6: > - Fix rte_eth_dev_uninit() to handle a return value of uninit > function of PMD. > v4: > - Add parameter checking. > - Change function names. > > Signed-off-by: Tetsuya Mukawa <mukawa at igel.co.jp> > --- > lib/librte_eal/common/include/rte_pci.h | 7 +++++ > lib/librte_ether/rte_ethdev.c | 47 > +++++++++++++++++++++++++++++++++ > lib/librte_ether/rte_ethdev.h | 24 +++++++++++++++++ > 3 files changed, 78 insertions(+) > > diff --git a/lib/librte_eal/common/include/rte_pci.h > b/lib/librte_eal/common/include/rte_pci.h > index 4814cd7..87ca4cf 100644 > --- a/lib/librte_eal/common/include/rte_pci.h > +++ b/lib/librte_eal/common/include/rte_pci.h > @@ -189,12 +189,19 @@ struct rte_pci_driver; > typedef int (pci_devinit_t)(struct rte_pci_driver *, struct rte_pci_device > *); > > /** > + * Uninitialisation function for the driver called during hotplugging. > + */ > +typedef int (pci_devuninit_t)( > + struct rte_pci_driver *, struct rte_pci_device *); > + > +/** > * A structure describing a PCI driver. > */ > struct rte_pci_driver { > TAILQ_ENTRY(rte_pci_driver) next; /**< Next in list. */ > const char *name; /**< Driver name. */ > pci_devinit_t *devinit; /**< Device init. function. */ > + pci_devuninit_t *devuninit; /**< Device uninit function. */ > struct rte_pci_id *id_table; /**< ID table, NULL terminated. > */ > uint32_t drv_flags; /**< Flags contolling handling > of device. */ > }; > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index 2463d18..58d8072 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -326,6 +326,52 @@ rte_eth_dev_init(struct rte_pci_driver *pci_drv, > return diag; > } > > +static int > +rte_eth_dev_uninit(struct rte_pci_driver *pci_drv, > + struct rte_pci_device *pci_dev)
There's something strange here: this is an uninit of an ethdev but the parameter is a pci_dev. The driver parameter shouldn't be needed. > +{ > + struct eth_driver *eth_drv; > + struct rte_eth_dev *eth_dev; > + char ethdev_name[RTE_ETH_NAME_MAX_LEN]; > + int ret; > + > + if ((pci_drv == NULL) || (pci_dev == NULL)) > + return -EINVAL; > + > + /* Create unique Ethernet device name using PCI address */ > + snprintf(ethdev_name, RTE_ETH_NAME_MAX_LEN, "%d:%d.%d", > + pci_dev->addr.bus, pci_dev->addr.devid, > + pci_dev->addr.function); You should call a function common to init and uninit to generate the same unique name. > + > + eth_dev = rte_eth_dev_allocated(ethdev_name); > + if (eth_dev == NULL) > + return -ENODEV; > + > + eth_drv = (struct eth_driver *)pci_drv; > + > + /* Invoke PMD device uninit function */ > + if (*eth_drv->eth_dev_uninit) { > + ret = (*eth_drv->eth_dev_uninit)(eth_drv, eth_dev); > + if (ret) > + return ret; > + } > + > + /* free ether device */ > + rte_eth_dev_free(eth_dev); > + > + /* init user callbacks */ > + TAILQ_INIT(&(eth_dev->callbacks)); Please comment more why you are resetting callbacks. An init in an uninit function seems weird ;) > + > + if (rte_eal_process_type() == RTE_PROC_PRIMARY) > + rte_free(eth_dev->data->dev_private); > + > + eth_dev->pci_dev = NULL; > + eth_dev->driver = NULL; > + eth_dev->data = NULL; > + > + return 0; > +} > + > /** > * Register an Ethernet [Poll Mode] driver. > * > @@ -344,6 +390,7 @@ void > rte_eth_driver_register(struct eth_driver *eth_drv) > { > eth_drv->pci_drv.devinit = rte_eth_dev_init; > + eth_drv->pci_drv.devuninit = rte_eth_dev_uninit; > rte_eal_pci_register(ð_drv->pci_drv); > } > > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h > index fbe7ac1..91d9e86 100644 > --- a/lib/librte_ether/rte_ethdev.h > +++ b/lib/librte_ether/rte_ethdev.h > @@ -1678,6 +1678,27 @@ typedef int (*eth_dev_init_t)(struct eth_driver > *eth_drv, > > /** > * @internal > + * Finalization function of an Ethernet driver invoked for each matching > + * Ethernet PCI device detected during the PCI closing phase. > + * > + * @param eth_drv > + * The pointer to the [matching] Ethernet driver structure supplied by > + * the PMD when it registered itself. > + * @param eth_dev > + * The *eth_dev* pointer is the address of the *rte_eth_dev* structure > + * associated with the matching device and which have been [automatically] > + * allocated in the *rte_eth_devices* array. > + * @return > + * - 0: Success, the device is properly finalized by the driver. > + * In particular, the driver MUST free the *dev_ops* pointer > + * of the *eth_dev* structure. > + * - <0: Error code of the device initialization failure. > + */ > +typedef int (*eth_dev_uninit_t)(struct eth_driver *eth_drv, > + struct rte_eth_dev *eth_dev); > + > +/** > + * @internal > * The structure associated with a PMD Ethernet driver. > * > * Each Ethernet driver acts as a PCI driver and is represented by a generic > @@ -1687,11 +1708,14 @@ typedef int (*eth_dev_init_t)(struct eth_driver > *eth_drv, > * > * - The *eth_dev_init* function invoked for each matching PCI device. > * > + * - The *eth_dev_uninit* function invoked for each matching PCI device. > + * > * - The size of the private data to allocate for each matching device. > */ > struct eth_driver { > struct rte_pci_driver pci_drv; /**< The PMD is also a PCI driver. */ > eth_dev_init_t eth_dev_init; /**< Device init function. */ > + eth_dev_uninit_t eth_dev_uninit; /**< Device uninit function. */ > unsigned int dev_private_size; /**< Size of device private data. */ > }; > >