| Message ID | 20180702054450.29269-2-qi.z.zhang@intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Thomas Monjalon |
| Headers | show |
| Series | enable hotplug on multi-process | expand |
| Context | Check | Description |
|---|---|---|
| ci/checkpatch | success | coding style OK |
| ci/Intel-compilation | fail | Compilation issues |
Hi, 02/07/2018 07:44, Qi Zhang: > --- a/lib/librte_ethdev/rte_ethdev_driver.h > +++ b/lib/librte_ethdev/rte_ethdev_driver.h > @@ -70,6 +70,19 @@ int rte_eth_dev_release_port(struct rte_eth_dev *eth_dev); > > /** > * @internal > + * Release the specified ethdev port in local process, only set to ethdev > + * state to unused, but not reset share data since it assume other process > + * is still using it, typically it is called by secondary process. Please check grammar in doxygen comments, and do not make long sentences. "only set to ethdev state to unused" -> "only set ethdev state to unused" "share" -> "shared" "it assume" -> "it assumes" Please split sentences with a dot and an uppercase. I expect all patches of the series must be reviewed for english wording. > + * > + * @param eth_dev > + * The *eth_dev* pointer is the address of the *rte_eth_dev* structure. This comment is useless. You can just say "Device to be detached". > + * @return > + * - 0 on success, negative on error > + */ > +int rte_eth_dev_release_port_private(struct rte_eth_dev *eth_dev); [...] > --- a/lib/librte_ethdev/rte_ethdev_pci.h > +++ b/lib/librte_ethdev/rte_ethdev_pci.h > @@ -197,6 +197,9 @@ rte_eth_dev_pci_generic_remove(struct rte_pci_device *pci_dev, > + if (rte_eal_process_type() != RTE_PROC_PRIMARY) > + return rte_eth_dev_release_port_private(eth_dev); This is a behaviour change. It means a PCI device cannot be globally detached directly by a secondary. It must be justified by a comment in the code. Please explain.
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index a9977df97..52a97694c 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -359,6 +359,18 @@ rte_eth_dev_attach_secondary(const char *name) } int +rte_eth_dev_release_port_private(struct rte_eth_dev *eth_dev) +{ + if (eth_dev == NULL) + return -EINVAL; + + _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_DESTROY, NULL); + eth_dev->state = RTE_ETH_DEV_UNUSED; + + return 0; +} + +int rte_eth_dev_release_port(struct rte_eth_dev *eth_dev) { if (eth_dev == NULL) diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h index c9c825e3f..49c27223d 100644 --- a/lib/librte_ethdev/rte_ethdev_driver.h +++ b/lib/librte_ethdev/rte_ethdev_driver.h @@ -70,6 +70,19 @@ int rte_eth_dev_release_port(struct rte_eth_dev *eth_dev); /** * @internal + * Release the specified ethdev port in local process, only set to ethdev + * state to unused, but not reset share data since it assume other process + * is still using it, typically it is called by secondary process. + * + * @param eth_dev + * The *eth_dev* pointer is the address of the *rte_eth_dev* structure. + * @return + * - 0 on success, negative on error + */ +int rte_eth_dev_release_port_private(struct rte_eth_dev *eth_dev); + +/** + * @internal * Release device queues and clear its configuration to force the user * application to reconfigure it. It is for internal use only. * diff --git a/lib/librte_ethdev/rte_ethdev_pci.h b/lib/librte_ethdev/rte_ethdev_pci.h index 2cfd37274..eeb944146 100644 --- a/lib/librte_ethdev/rte_ethdev_pci.h +++ b/lib/librte_ethdev/rte_ethdev_pci.h @@ -197,6 +197,9 @@ rte_eth_dev_pci_generic_remove(struct rte_pci_device *pci_dev, if (!eth_dev) return -ENODEV; + if (rte_eal_process_type() != RTE_PROC_PRIMARY) + return rte_eth_dev_release_port_private(eth_dev); + if (dev_uninit) { ret = dev_uninit(eth_dev); if (ret)