[dpdk-dev] [PATCH 7/9] pci: add a helper for device name
On Wed, Feb 10, 2016 at 12:10 PM, Jan Viktorin wrote: > On Fri, 29 Jan 2016 15:08:34 +0100 > David Marchand wrote: > >> eal is a better place than ethdev for naming resources. >> Add a helper here, and make use of it in ethdev hotplug code. >> >> Signed-off-by: David Marchand >> --- >> lib/librte_eal/common/include/rte_pci.h | 28 >> lib/librte_ether/rte_ethdev.c | 22 ++ >> 2 files changed, 30 insertions(+), 20 deletions(-) >> >> diff --git a/lib/librte_eal/common/include/rte_pci.h >> b/lib/librte_eal/common/include/rte_pci.h >> index 334c12e..9edd5f5 100644 >> --- a/lib/librte_eal/common/include/rte_pci.h >> +++ b/lib/librte_eal/common/include/rte_pci.h >> @@ -309,6 +309,34 @@ eal_parse_pci_DomBDF(const char *input, struct >> rte_pci_addr *dev_addr) >> } >> #undef GET_PCIADDR_FIELD >> >> +/** >> + * Utility function to write a pci device name, this device name can later >> be >> + * used to retrieve the corresponding rte_pci_addr using above functions. >> + * >> + * @param addr >> + * The PCI Bus-Device-Function address >> + * @param output >> + * The output buffer string >> + * @param size >> + * The output buffer size >> + * @return >> + * 0 on success, negative on error. >> + */ >> +static inline int >> +eal_pci_device_name(const struct rte_pci_addr *addr, >> + char *output, int size) > > "size_t size" (or unsigned int) seems to be better to me. Yes. > >> +{ >> + int ret; >> + >> + ret = snprintf(output, size, PCI_PRI_FMT, >> +addr->domain, addr->bus, >> +addr->devid, addr->function); >> + if (ret < 0 || ret >= size) >> + return -1; > > The return value is not checked (below). I think, such functions > are usually missing error checking as nobody expects them to fail. > Wouldn't it be better to panic here? assert or panic, yes. -- David Marchand
[dpdk-dev] [PATCH 7/9] pci: add a helper for device name
On Fri, 29 Jan 2016 15:08:34 +0100 David Marchand wrote: > eal is a better place than ethdev for naming resources. > Add a helper here, and make use of it in ethdev hotplug code. > > Signed-off-by: David Marchand > --- > lib/librte_eal/common/include/rte_pci.h | 28 > lib/librte_ether/rte_ethdev.c | 22 ++ > 2 files changed, 30 insertions(+), 20 deletions(-) > > diff --git a/lib/librte_eal/common/include/rte_pci.h > b/lib/librte_eal/common/include/rte_pci.h > index 334c12e..9edd5f5 100644 > --- a/lib/librte_eal/common/include/rte_pci.h > +++ b/lib/librte_eal/common/include/rte_pci.h > @@ -309,6 +309,34 @@ eal_parse_pci_DomBDF(const char *input, struct > rte_pci_addr *dev_addr) > } > #undef GET_PCIADDR_FIELD > > +/** > + * Utility function to write a pci device name, this device name can later be > + * used to retrieve the corresponding rte_pci_addr using above functions. > + * > + * @param addr > + * The PCI Bus-Device-Function address > + * @param output > + * The output buffer string > + * @param size > + * The output buffer size > + * @return > + * 0 on success, negative on error. > + */ > +static inline int > +eal_pci_device_name(const struct rte_pci_addr *addr, > + char *output, int size) "size_t size" (or unsigned int) seems to be better to me. > +{ > + int ret; > + > + ret = snprintf(output, size, PCI_PRI_FMT, > +addr->domain, addr->bus, > +addr->devid, addr->function); > + if (ret < 0 || ret >= size) > + return -1; The return value is not checked (below). I think, such functions are usually missing error checking as nobody expects them to fail. Wouldn't it be better to panic here? > + > + return 0; > +} > + > /* Compare two PCI device addresses. */ > /** > * Utility function to compare two PCI device addresses. > diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c > index 17e4f4d..5ba7479 100644 > --- a/lib/librte_ether/rte_ethdev.c > +++ b/lib/librte_ether/rte_ethdev.c > @@ -214,20 +214,6 @@ rte_eth_dev_allocate(const char *name, enum > rte_eth_dev_type type) > return eth_dev; > } > > -static int > -rte_eth_dev_create_unique_device_name(char *name, size_t size, > - struct rte_pci_device *pci_dev) > -{ > - int ret; > - > - ret = snprintf(name, size, "%d:%d.%d", > - pci_dev->addr.bus, pci_dev->addr.devid, > - pci_dev->addr.function); > - if (ret < 0) > - return ret; > - return 0; > -} > - > int > rte_eth_dev_release_port(struct rte_eth_dev *eth_dev) > { > @@ -251,9 +237,7 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv, > > eth_drv = (struct eth_driver *)pci_drv; > > - /* Create unique Ethernet device name using PCI address */ > - rte_eth_dev_create_unique_device_name(ethdev_name, > - sizeof(ethdev_name), pci_dev); > + eal_pci_device_name(_dev->addr, ethdev_name, sizeof(ethdev_name)); > > eth_dev = rte_eth_dev_allocate(ethdev_name, RTE_ETH_DEV_PCI); > if (eth_dev == NULL) > @@ -304,9 +288,7 @@ rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev) > if (pci_dev == NULL) > return -EINVAL; > > - /* Create unique Ethernet device name using PCI address */ > - rte_eth_dev_create_unique_device_name(ethdev_name, > - sizeof(ethdev_name), pci_dev); > + eal_pci_device_name(_dev->addr, ethdev_name, sizeof(ethdev_name)); > > eth_dev = rte_eth_dev_allocated(ethdev_name); > if (eth_dev == NULL) -- Jan Viktorin E-mail: Viktorin at RehiveTech.com System Architect Web:www.RehiveTech.com RehiveTech Brno, Czech Republic
[dpdk-dev] [PATCH 7/9] pci: add a helper for device name
eal is a better place than ethdev for naming resources. Add a helper here, and make use of it in ethdev hotplug code. Signed-off-by: David Marchand --- lib/librte_eal/common/include/rte_pci.h | 28 lib/librte_ether/rte_ethdev.c | 22 ++ 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h index 334c12e..9edd5f5 100644 --- a/lib/librte_eal/common/include/rte_pci.h +++ b/lib/librte_eal/common/include/rte_pci.h @@ -309,6 +309,34 @@ eal_parse_pci_DomBDF(const char *input, struct rte_pci_addr *dev_addr) } #undef GET_PCIADDR_FIELD +/** + * Utility function to write a pci device name, this device name can later be + * used to retrieve the corresponding rte_pci_addr using above functions. + * + * @param addr + * The PCI Bus-Device-Function address + * @param output + * The output buffer string + * @param size + * The output buffer size + * @return + * 0 on success, negative on error. + */ +static inline int +eal_pci_device_name(const struct rte_pci_addr *addr, + char *output, int size) +{ + int ret; + + ret = snprintf(output, size, PCI_PRI_FMT, + addr->domain, addr->bus, + addr->devid, addr->function); + if (ret < 0 || ret >= size) + return -1; + + return 0; +} + /* Compare two PCI device addresses. */ /** * Utility function to compare two PCI device addresses. diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c index 17e4f4d..5ba7479 100644 --- a/lib/librte_ether/rte_ethdev.c +++ b/lib/librte_ether/rte_ethdev.c @@ -214,20 +214,6 @@ rte_eth_dev_allocate(const char *name, enum rte_eth_dev_type type) return eth_dev; } -static int -rte_eth_dev_create_unique_device_name(char *name, size_t size, - struct rte_pci_device *pci_dev) -{ - int ret; - - ret = snprintf(name, size, "%d:%d.%d", - pci_dev->addr.bus, pci_dev->addr.devid, - pci_dev->addr.function); - if (ret < 0) - return ret; - return 0; -} - int rte_eth_dev_release_port(struct rte_eth_dev *eth_dev) { @@ -251,9 +237,7 @@ rte_eth_dev_pci_probe(struct rte_pci_driver *pci_drv, eth_drv = (struct eth_driver *)pci_drv; - /* Create unique Ethernet device name using PCI address */ - rte_eth_dev_create_unique_device_name(ethdev_name, - sizeof(ethdev_name), pci_dev); + eal_pci_device_name(_dev->addr, ethdev_name, sizeof(ethdev_name)); eth_dev = rte_eth_dev_allocate(ethdev_name, RTE_ETH_DEV_PCI); if (eth_dev == NULL) @@ -304,9 +288,7 @@ rte_eth_dev_pci_remove(struct rte_pci_device *pci_dev) if (pci_dev == NULL) return -EINVAL; - /* Create unique Ethernet device name using PCI address */ - rte_eth_dev_create_unique_device_name(ethdev_name, - sizeof(ethdev_name), pci_dev); + eal_pci_device_name(_dev->addr, ethdev_name, sizeof(ethdev_name)); eth_dev = rte_eth_dev_allocated(ethdev_name); if (eth_dev == NULL) -- 1.9.1